From 7e04f77dea5a96e89556bc53cec97eabc167db49 Mon Sep 17 00:00:00 2001 From: Omer Zuarets Date: Tue, 15 Oct 2024 17:43:39 +0300 Subject: [PATCH 1/2] add get user tenants --- horizon/enforcer/api.py | 45 +++++++++++----- horizon/tests/test_enforcer_api.py | 86 +++++++++++++++++++++--------- 2 files changed, 95 insertions(+), 36 deletions(-) diff --git a/horizon/enforcer/api.py b/horizon/enforcer/api.py index 55087d34..48e06c92 100644 --- a/horizon/enforcer/api.py +++ b/horizon/enforcer/api.py @@ -1,7 +1,7 @@ import asyncio import json import re -from typing import cast, Optional, Union, Dict, List +from typing import cast, Optional, Union, Dict, List, Callable import aiohttp from fastapi import APIRouter, Depends, Header @@ -276,32 +276,46 @@ async def conditional_is_allowed( *, policy_package: str = MAIN_POLICY_PACKAGE, external_data_manager_path: str = "/check", + external_data_manager_method: str = "POST", + external_data_manager_params: dict | None = None, + legacy_parse_func: Callable[[dict | list], dict] | None = None ) -> dict: if sidecar_config.ENABLE_EXTERNAL_DATA_MANAGER: - response = await _is_allowed_data_manager(query, request, path=external_data_manager_path) + response = await _is_allowed_data_manager( + query if external_data_manager_method != "GET" else None, + request, + path=external_data_manager_path, + method=external_data_manager_method, + params=external_data_manager_params, + ) raw_result = json.loads(response.body) log_query_result(query, response, is_inner=True) else: response = await _is_allowed(query, request, policy_package) raw_result = json.loads(response.body).get("result", {}) log_query_result(query, response) + if legacy_parse_func: + raw_result = legacy_parse_func(raw_result) return raw_result async def _is_allowed_data_manager( - query: BaseSchema, request: Request, *, path: str = "/check" + query: BaseSchema | None, request: Request, *, path: str = "/check", method: str = "POST", + params: dict | None = None ): headers = transform_headers(request) url = f"{sidecar_config.DATA_MANAGER_SERVICE_URL}/v1/authz{path}" - payload = {"input": query.dict()} + payload = None if query is None else {"input": query.dict()} exc = None _set_use_debugger(payload) try: logger.info(f"calling Data Manager at '{url}' with input: {payload}") async with aiohttp.ClientSession() as session: - async with session.post( + async with session.request( + method, url, data=json.dumps(payload["input"]) if payload is not None else None, + params=params, headers=headers, timeout=sidecar_config.OPA_CLIENT_QUERY_TIMEOUT, raise_for_status=True, @@ -490,19 +504,26 @@ async def user_tenants( query: UserTenantsQuery, x_permit_sdk_language: Optional[str] = Depends(notify_seen_sdk), ): - response = await _is_allowed(query, request, USER_TENANTS_POLICY_PACKAGE) - log_query_result(query, response) - try: - raw_result = json.loads(response.body).get("result", {}) + def parse_func(result: dict | list) -> dict | list: if isinstance(raw_result, dict): - tenants = raw_result.get("tenants", {}) + tenants = result.get("tenants", []) elif isinstance(raw_result, list): - tenants = raw_result + tenants = result else: raise TypeError( f"Expected raw result to be dict or list, got {type(raw_result)}" ) - result = parse_obj_as(UserTenantsResult, tenants) + return tenants + raw_result = await conditional_is_allowed( + query, + request, + policy_package=USER_TENANTS_POLICY_PACKAGE, + external_data_manager_path=f"/users/{query.user.key}/tenants", + external_data_manager_method="GET", + legacy_parse_func=parse_func, + ) + try: + result = parse_obj_as(UserTenantsResult, raw_result) except: result = parse_obj_as(UserTenantsResult, []) logger.warning( diff --git a/horizon/tests/test_enforcer_api.py b/horizon/tests/test_enforcer_api.py index 2eb6e05e..90388545 100644 --- a/horizon/tests/test_enforcer_api.py +++ b/horizon/tests/test_enforcer_api.py @@ -246,6 +246,35 @@ async def pdp_api_client() -> TestClient: {"allow": [{"allow": True, "result": True}]}, {"allow": [{"allow": True, "result": True}]}, ), + ( + "/user-tenants", + "/users/user1/tenants", + UserTenantsQuery( + user=User(key="user1"), + ), + None, + [ + { + "key": "default-2", + "attributes": {} + }, + { + "key": "default", + "attributes": {} + } + ], + [ + { + "key": "default-2", + "attributes": {} + }, + { + "key": "default", + "attributes": {} + } + ], + ), + ] @@ -455,33 +484,39 @@ def post_endpoint(): f"{sidecar_config.DATA_MANAGER_SERVICE_URL}/v1/authz{datasync_endpoint}" ) + method = "POST" + + match endpoint: + case "/allowed_url": + # allowed_url gonna first call the mapping rules endpoint then the normal OPA allow endpoint + m.post( + url=f"{opal_client_config.POLICY_STORE_URL}/v1/data/mapping_rules", + status=200, + payload={ + "result": { + "all": [ + { + "url": "https://some.url/important_resource", + "http_method": "delete", + "action": "delete", + "resource": "resource1", + } + ] + } + }, + repeat=True, + ) + case "/user-tenants": + method = "GET" + # Test valid response from OPA - m.post( + m.add( datasync_url, + method=method, status=200, payload=datasync_response, ) - if endpoint == "/allowed_url": - # allowed_url gonna first call the mapping rules endpoint then the normal OPA allow endpoint - m.post( - url=f"{opal_client_config.POLICY_STORE_URL}/v1/data/mapping_rules", - status=200, - payload={ - "result": { - "all": [ - { - "url": "https://some.url/important_resource", - "http_method": "delete", - "action": "delete", - "resource": "resource1", - } - ] - } - }, - repeat=True, - ) - response = post_endpoint() assert response.status_code == 200 print(response.json()) @@ -499,8 +534,9 @@ def post_endpoint(): # Test bad status from OPA bad_status = random.choice([401, 404, 400, 500, 503]) - m.post( + m.add( datasync_url, + method=method, status=bad_status, payload=datasync_response, ) @@ -510,8 +546,9 @@ def post_endpoint(): assert f"status: {bad_status}" in response.text # Test connection error - m.post( + m.add( datasync_url, + method=method, exception=aiohttp.ClientConnectionError("don't want to connect"), ) response = post_endpoint() @@ -520,8 +557,9 @@ def post_endpoint(): assert "don't want to connect" in response.text # Test timeout - not working yet - m.post( + m.add( datasync_url, + method=method, exception=asyncio.exceptions.TimeoutError(), ) response = post_endpoint() From 45e8bd5d647f5aa5bf4b142eee28f19d3310e295 Mon Sep 17 00:00:00 2001 From: Omer Zuarets Date: Sun, 27 Oct 2024 15:07:46 +0200 Subject: [PATCH 2/2] fix pre-commit --- horizon/enforcer/api.py | 17 ++++++++++----- horizon/tests/test_enforcer_api.py | 35 ++++++++++-------------------- 2 files changed, 23 insertions(+), 29 deletions(-) diff --git a/horizon/enforcer/api.py b/horizon/enforcer/api.py index 48e06c92..1f84ec7f 100644 --- a/horizon/enforcer/api.py +++ b/horizon/enforcer/api.py @@ -278,7 +278,7 @@ async def conditional_is_allowed( external_data_manager_path: str = "/check", external_data_manager_method: str = "POST", external_data_manager_params: dict | None = None, - legacy_parse_func: Callable[[dict | list], dict] | None = None + legacy_parse_func: Callable[[dict | list], dict] | None = None, ) -> dict: if sidecar_config.ENABLE_EXTERNAL_DATA_MANAGER: response = await _is_allowed_data_manager( @@ -300,8 +300,12 @@ async def conditional_is_allowed( async def _is_allowed_data_manager( - query: BaseSchema | None, request: Request, *, path: str = "/check", method: str = "POST", - params: dict | None = None + query: BaseSchema | None, + request: Request, + *, + path: str = "/check", + method: str = "POST", + params: dict | None = None, ): headers = transform_headers(request) url = f"{sidecar_config.DATA_MANAGER_SERVICE_URL}/v1/authz{path}" @@ -514,6 +518,7 @@ def parse_func(result: dict | list) -> dict | list: f"Expected raw result to be dict or list, got {type(raw_result)}" ) return tenants + raw_result = await conditional_is_allowed( query, request, @@ -628,8 +633,8 @@ async def is_allowed( raise HTTPException( status_code=status.HTTP_421_MISDIRECTED_REQUEST, detail="Mismatch between client version and PDP version," - " required v2 request body, got v1. " - "hint: try to update your client version to v2", + " required v2 request body, got v1. " + "hint: try to update your client version to v2", ) query = cast(AuthorizationQuery, query) @@ -710,7 +715,7 @@ async def is_allowed_kong(request: Request, query: KongAuthorizationQuery): raise HTTPException( status.HTTP_503_SERVICE_UNAVAILABLE, detail="Kong integration is disabled. " - "Please set the PDP_KONG_INTEGRATION variable to true to enable it.", + "Please set the PDP_KONG_INTEGRATION variable to true to enable it.", ) await PersistentStateHandler.get_instance().seen_sdk("kong") diff --git a/horizon/tests/test_enforcer_api.py b/horizon/tests/test_enforcer_api.py index 90388545..cc7e6655 100644 --- a/horizon/tests/test_enforcer_api.py +++ b/horizon/tests/test_enforcer_api.py @@ -253,28 +253,9 @@ async def pdp_api_client() -> TestClient: user=User(key="user1"), ), None, - [ - { - "key": "default-2", - "attributes": {} - }, - { - "key": "default", - "attributes": {} - } - ], - [ - { - "key": "default-2", - "attributes": {} - }, - { - "key": "default", - "attributes": {} - } - ], + [{"key": "default-2", "attributes": {}}, {"key": "default", "attributes": {}}], + [{"key": "default-2", "attributes": {}}, {"key": "default", "attributes": {}}], ), - ] @@ -458,7 +439,14 @@ def post_endpoint(): @pytest.mark.parametrize( - ("endpoint", "datasync_endpoint", "query", "headers", "datasync_response", "expected_response"), + ( + "endpoint", + "datasync_endpoint", + "query", + "headers", + "datasync_response", + "expected_response", + ), ALLOWED_ENDPOINTS_DATASYNC, ) def test_enforce_endpoint_datasync( @@ -475,7 +463,8 @@ def test_enforce_endpoint_datasync( def post_endpoint(): return _client.post( endpoint, - headers={"authorization": f"Bearer {sidecar_config.API_KEY}"} | (headers or {}), + headers={"authorization": f"Bearer {sidecar_config.API_KEY}"} + | (headers or {}), json=jsonable_encoder(query) if query else None, )