From 8cd2be13744e837ffeff6dd3baabf908be17a128 Mon Sep 17 00:00:00 2001 From: Mohammad Torkashvand <mohammad.torkashvand@geant.org> Date: Thu, 18 Jul 2024 13:04:31 +0200 Subject: [PATCH] remove unncessary migration --- gso/__init__.py | 2 +- gso/api/v1/network.py | 4 +- gso/auth/oidc.py | 7 +- ...289c0_add_orchestrator_2_1_2_migrations.py | 23 --- gso/worker.py | 2 +- log.txt | 0 requirements.txt | 2 +- setup.py | 2 +- ...est_oidc_policy_helper.py => test_oidc.py} | 181 ++++++++---------- test/auth/test_opa.py | 86 +++++++++ test/cli/test_imports.py | 2 +- 11 files changed, 173 insertions(+), 138 deletions(-) delete mode 100644 gso/migrations/versions/2024-04-02_1ec810b289c0_add_orchestrator_2_1_2_migrations.py delete mode 100644 log.txt rename test/auth/{test_oidc_policy_helper.py => test_oidc.py} (63%) create mode 100644 test/auth/test_opa.py diff --git a/gso/__init__.py b/gso/__init__.py index c5dbc6b0..38607bf1 100644 --- a/gso/__init__.py +++ b/gso/__init__.py @@ -34,7 +34,7 @@ def init_worker_app() -> OrchestratorCore: def init_cli_app() -> typer.Typer: """Initialise :term:`GSO` as a CLI application.""" - from gso.cli import imports, netbox # noqa: PLC0415 + from gso.cli import imports, netbox cli_app.add_typer(imports.app, name="import-cli") cli_app.add_typer(netbox.app, name="netbox-cli") diff --git a/gso/api/v1/network.py b/gso/api/v1/network.py index 6dd5ac75..9109fd6f 100644 --- a/gso/api/v1/network.py +++ b/gso/api/v1/network.py @@ -6,17 +6,17 @@ from uuid import UUID from fastapi import APIRouter, Depends from orchestrator.domain import SubscriptionModel from orchestrator.schemas.base import OrchestratorBaseModel +from orchestrator.security import authorize from orchestrator.services.subscriptions import build_extended_domain_model from starlette import status -from gso.auth.security import opa_security_default from gso.products.product_blocks.iptrunk import IptrunkType, PhysicalPortCapacity from gso.products.product_blocks.router import RouterRole from gso.products.product_blocks.site import LatitudeCoordinate, LongitudeCoordinate from gso.services.subscriptions import get_active_iptrunk_subscriptions from gso.utils.shared_enums import Vendor -router = APIRouter(prefix="/networks", tags=["Network"], dependencies=[Depends(opa_security_default)]) +router = APIRouter(prefix="/networks", tags=["Network"], dependencies=[Depends(authorize)]) class SiteBlock(OrchestratorBaseModel): diff --git a/gso/auth/oidc.py b/gso/auth/oidc.py index d9acb245..ce7da2d9 100644 --- a/gso/auth/oidc.py +++ b/gso/auth/oidc.py @@ -25,6 +25,7 @@ _CALLBACK_STEP_API_URL_PATTERN = re.compile( def _is_client_credentials_token(intercepted_token: dict) -> bool: return "sub" not in intercepted_token + def _is_callback_step_endpoint(request: Request) -> bool: """Check if the request is a callback step API call.""" return re.match(_CALLBACK_STEP_API_URL_PATTERN, request.url.path) is not None @@ -65,9 +66,7 @@ class OIDCAuthentication(OIDCAuth): intercepted_token = await self.introspect_token(async_request, token) client_id = intercepted_token.get("client_id") if _is_client_credentials_token(intercepted_token): - return OIDCUserModel( - client_id=client_id - ) + return OIDCUserModel(client_id=client_id) response = await async_request.post( self.openid_config.userinfo_endpoint, @@ -148,7 +147,7 @@ class OIDCAuthentication(OIDCAuth): oidc_instance = OIDCAuthentication( openid_url=oauth2lib_settings.OIDC_BASE_URL, - openid_config_url=oauth2lib_settings.OIDC_CONF_URL, # Corrected parameter name + openid_config_url=oauth2lib_settings.OIDC_CONF_URL, resource_server_id=oauth2lib_settings.OAUTH2_RESOURCE_SERVER_ID, resource_server_secret=oauth2lib_settings.OAUTH2_RESOURCE_SERVER_SECRET, oidc_user_model_cls=OIDCUserModel, diff --git a/gso/migrations/versions/2024-04-02_1ec810b289c0_add_orchestrator_2_1_2_migrations.py b/gso/migrations/versions/2024-04-02_1ec810b289c0_add_orchestrator_2_1_2_migrations.py deleted file mode 100644 index aa9593a8..00000000 --- a/gso/migrations/versions/2024-04-02_1ec810b289c0_add_orchestrator_2_1_2_migrations.py +++ /dev/null @@ -1,23 +0,0 @@ -"""remove subscription cancellation workflow. - -Revision ID: 1ec810b289c0 -Revises: -Create Date: 2024-04-02 10:21:08.539591 - -""" - -# revision identifiers, used by Alembic. -revision = '1ec810b289c0' -down_revision = '4ec89ab289c0' -branch_labels = None -# TODO: check it carefuly -depends_on = '048219045729' # in this revision, SURF has added a new columns to the workflow table like delted_at, so we need to add a dependency on the revision that added the columns to the workflow table. - - -def upgrade() -> None: - pass - - -def downgrade() -> None: - pass - diff --git a/gso/worker.py b/gso/worker.py index b2abfe6f..b1a3db2c 100644 --- a/gso/worker.py +++ b/gso/worker.py @@ -9,7 +9,7 @@ from gso.settings import load_oss_params class OrchestratorCelery(Celery): """A :term:`GSO` instance that functions as a Celery worker.""" - def on_init(self) -> None: # noqa: PLR6301 + def on_init(self) -> None: """Initialise a new Celery worker.""" init_worker_app() diff --git a/log.txt b/log.txt deleted file mode 100644 index e69de29b..00000000 diff --git a/requirements.txt b/requirements.txt index 9c8e28c3..8878a45c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,4 @@ -orchestrator-core==2.3.0rc3 +orchestrator-core==2.6.1 requests==2.31.0 infoblox-client~=0.6.0 pycountry==23.12.11 diff --git a/setup.py b/setup.py index 169f8078..a70761c8 100644 --- a/setup.py +++ b/setup.py @@ -11,7 +11,7 @@ setup( url="https://gitlab.software.geant.org/goat/gap/geant-service-orchestrator", packages=find_packages(), install_requires=[ - "orchestrator-core==2.3.0rc3", + "orchestrator-core==2.6.1", "requests==2.31.0", "infoblox-client~=0.6.0", "pycountry==23.12.11", diff --git a/test/auth/test_oidc_policy_helper.py b/test/auth/test_oidc.py similarity index 63% rename from test/auth/test_oidc_policy_helper.py rename to test/auth/test_oidc.py index 9aa313b4..85abaa9a 100644 --- a/test/auth/test_oidc_policy_helper.py +++ b/test/auth/test_oidc.py @@ -1,20 +1,17 @@ from http import HTTPStatus -from unittest.mock import AsyncMock, MagicMock, Mock, patch +from unittest.mock import AsyncMock, Mock import pytest from fastapi import HTTPException, Request from httpx import AsyncClient, NetworkError, Response +from oauth2_lib.fastapi import OIDCConfig from gso.auth.oidc import ( OIDCAuthentication, - OIDCConfig, OIDCUserModel, - OPAResult, - _evaluate_decision, - _get_decision, _is_callback_step_endpoint, - opa_decision, ) +from gso.auth.opa import _get_decision from gso.auth.settings import oauth2lib_settings @@ -56,6 +53,8 @@ def oidc_user(mock_openid_config): openid_url="https://example.proxy.aai.geant.org", resource_server_id="resource_server", resource_server_secret="secret", # noqa: S106 + openid_config_url="https://example.proxy.aai.geant.org/.well-known/openid-configuration", + oidc_user_model_cls=OIDCUserModel, ) user.openid_config = OIDCConfig.model_validate(mock_openid_config) return user @@ -133,8 +132,25 @@ async def test_introspect_token_unauthorized(oidc_user, mock_async_client): @pytest.mark.asyncio() async def test_userinfo_success(oidc_user, mock_async_client): - mock_response = {"sub": "1234", "name": "John Doe", "email": "johndoe@example.com"} - mock_async_client.post = AsyncMock(return_value=Response(200, json=mock_response)) + mock_response_introspect_token = { + "active": True, + "scope": "openid profile email aarc", + "client_id": "APP-775F0BD8-B1D7-4936-BE2C-A300A6509F0B", + "exp": 1721395275, + "iat": 1721391675, + "sub": "ed145263-b652-3d4x-8f96-4abae9c98124@aai.geant.org", + "iss": "https://proxy.aai.geant.org", + "token_type": "Bearer", + "aud": ["APP-775F0BD8-B1D7-4936-BE2C-A300A6509F0B"], + } + mock_response_userinfo = {"sub": "1234", "name": "John Doe", "email": "johndoe@example.com"} + + mock_async_client.post = AsyncMock( + side_effect=[ + Response(200, json=mock_response_introspect_token), + Response(200, json=mock_response_userinfo), + ] + ) response = await oidc_user.userinfo(mock_async_client, "test_token") @@ -145,21 +161,25 @@ async def test_userinfo_success(oidc_user, mock_async_client): @pytest.mark.asyncio() -async def test_opa_decision_success(mock_request, mock_async_client): - mock_user_info = OIDCUserModel({"sub": "123", "name": "John Doe", "email": "johndoe@example.com"}) - - mock_oidc_user = AsyncMock(spec=OIDCAuthentication) - mock_oidc_user.return_value = AsyncMock(return_value=mock_user_info) +async def test_userinfo_success_client_credential(oidc_user, mock_async_client): + mock_response_introspect_token = { + "active": True, + "scope": "openid profile email eduperson_assurance eduperson_entitlement entitlements", + "client_id": "APP-48A7986C-8776-49D9-AB40-52EFBD432AB1", + "exp": 1721395701, + "iat": 1721392101, + "iss": "https://proxy.aai.geant.org", + "token_type": "Bearer", + } - with patch( - "gso.auth.oidc_policy_helper._get_decision", - return_value=AsyncMock(return_value=OPAResult(result=True, decision_id="1234")), - ): - decision_function = opa_decision("http://mock-opa-url", oidc_security=mock_oidc_user) + mock_async_client.post = AsyncMock( + return_value=Response(200, json=mock_response_introspect_token), + ) - result = await decision_function(mock_request, mock_user_info, mock_async_client) + response = await oidc_user.userinfo(mock_async_client, "test_token") - assert result is True + assert isinstance(response, OIDCUserModel) + assert response == {"client_id": "APP-48A7986C-8776-49D9-AB40-52EFBD432AB1"} @pytest.mark.asyncio() @@ -191,7 +211,7 @@ async def test_get_decision_success(mock_async_client): opa_url = "http://mock-opa-url" opa_input = {"some_input": "value"} - decision = await _get_decision(mock_async_client, opa_url, opa_input) + decision = await _get_decision(opa_url, mock_async_client, opa_input) assert decision.result is True assert decision.decision_id == "123" @@ -205,102 +225,23 @@ async def test_get_decision_network_error(mock_async_client): opa_input = {"some_input": "value"} with pytest.raises(HTTPException) as exc_info: - await _get_decision(mock_async_client, opa_url, opa_input) + await _get_decision(opa_url, mock_async_client, opa_input) assert exc_info.value.status_code == HTTPStatus.SERVICE_UNAVAILABLE assert exc_info.value.detail == "Policy agent is unavailable" -def test_evaluate_decision_allow(): - decision = OPAResult(result=True, decision_id="123") - result = _evaluate_decision(decision, auto_error=True) - - assert result is True - - -def test_evaluate_decision_deny_without_auto_error(): - decision = OPAResult(result=False, decision_id="123") - result = _evaluate_decision(decision, auto_error=False) - - assert result is False - - -def test_evaluate_decision_deny_with_auto_error(): - decision = OPAResult(result=False, decision_id="123") - - with pytest.raises(HTTPException) as exc_info: - _evaluate_decision(decision, auto_error=True) - - assert exc_info.value.status_code == HTTPStatus.FORBIDDEN - assert "Decision was taken with id: 123" in str(exc_info.value.detail) - - -@pytest.mark.asyncio() -async def test_oidc_user_call_with_token(oidc_user, mock_request, mock_async_client): - oidc_user.introspect_token = AsyncMock(return_value={"active": True, "sub": "123", "client_id": "test_client"}) - oidc_user.userinfo = AsyncMock(return_value=OIDCUserModel({"sub": "123", "name": "John Doe"})) - - result = await oidc_user.__call__(mock_request, token="test_token") # noqa: S106 - - assert isinstance(result, OIDCUserModel) - assert result["sub"] == "123" - assert result["name"] == "John Doe" - assert result["client_id"] == "test_client" - - -@pytest.mark.asyncio() -async def test_oidc_user_call_with_client_credential_token(oidc_user, mock_request, mock_async_client): - oidc_user.introspect_token = AsyncMock(return_value={"active": True}) - oidc_user.userinfo = AsyncMock(return_value=OIDCUserModel({"sub": "123", "name": "John Doe"})) - - result = await oidc_user.__call__(mock_request, token="test_token") # noqa: S106 - - assert isinstance(result, OIDCUserModel) - assert result["client_id"] is None - oidc_user.userinfo.assert_not_called() - - @pytest.mark.asyncio() -async def test_oidc_user_call_inactive_token(oidc_user, mock_request, mock_async_client): - oidc_user.introspect_token = AsyncMock(return_value={"active": False, "sub": "123"}) +async def test_oidc_user_call_inactive_token(oidc_user, mock_async_client): + mock_async_client.post = AsyncMock(return_value=Response(200, json={"active": False, "sub": "123"})) with pytest.raises(HTTPException) as exc_info: - await oidc_user.__call__(mock_request, token="test_token") # noqa: S106 + await oidc_user.userinfo(mock_async_client, token="test_token") # noqa: S106 assert exc_info.value.status_code == HTTPStatus.UNAUTHORIZED assert "User is not active" in str(exc_info.value.detail) -@pytest.mark.asyncio() -async def test_oidc_user_call_no_token(oidc_user, mock_request): - with ( - patch("fastapi.security.http.HTTPBearer.__call__", return_value=None), - patch("httpx.AsyncClient.post", new_callable=MagicMock) as mock_post, - patch("httpx.AsyncClient.get", new_callable=MagicMock) as mock_get, - ): - mock_post.return_value = MagicMock(status_code=200, json=lambda: {"active": False}) - mock_get.return_value = MagicMock(status_code=200, json=dict) - - result = await oidc_user.__call__(mock_request) - - assert result is None - - -@pytest.mark.asyncio() -async def test_oidc_user_call_token_from_request(oidc_user, mock_request, mock_async_client): - mock_request.state.credentials = Mock() - mock_request.state.credentials.credentials = "request_token" - - oidc_user.introspect_token = AsyncMock(return_value={"active": True, "sub": "123"}) - oidc_user.userinfo = AsyncMock(return_value=OIDCUserModel({"sub": "123", "name": "John Doe"})) - - result = await oidc_user.__call__(mock_request) - - assert isinstance(result, OIDCUserModel) - assert result["sub"] == "123" - assert result["name"] == "John Doe" - - @pytest.mark.parametrize( ("path", "expected"), [ @@ -320,3 +261,35 @@ def test_is_callback_step_endpoint(path, expected): } ) assert _is_callback_step_endpoint(request) is expected + + +@pytest.mark.asyncio() +async def test_userinfo_invalid_token(oidc_user, mock_async_client): + mock_async_client.post = AsyncMock(return_value=Response(401, json={"error": "invalid_token"})) + + with pytest.raises(HTTPException) as exc_info: + await oidc_user.userinfo(mock_async_client, "invalid_token") + + assert exc_info.value.status_code == 401 + + +@pytest.mark.asyncio() +async def test_introspect_token_missing_active_key(oidc_user, mock_async_client): + mock_async_client.post = AsyncMock(return_value=Response(200, json={})) + + with pytest.raises(HTTPException) as exc_info: + await oidc_user.introspect_token(mock_async_client, "token") + + assert exc_info.value.status_code == 401 + assert "Missing active key" in str(exc_info.value.detail) + + +@pytest.mark.asyncio() +async def test_introspect_token_inactive(oidc_user, mock_async_client): + mock_async_client.post = AsyncMock(return_value=Response(200, json={"active": False})) + + with pytest.raises(HTTPException) as exc_info: + await oidc_user.introspect_token(mock_async_client, "token") + + assert exc_info.value.status_code == 401 + assert "User is not active" in str(exc_info.value.detail) diff --git a/test/auth/test_opa.py b/test/auth/test_opa.py new file mode 100644 index 00000000..d1239f1f --- /dev/null +++ b/test/auth/test_opa.py @@ -0,0 +1,86 @@ +from unittest.mock import AsyncMock + +import pytest +from fastapi.exceptions import HTTPException +from httpx import AsyncClient, NetworkError, Response +from oauth2_lib.fastapi import OPAResult + +from gso.auth.opa import GraphQLOPAAuthZ, OPAAuthZ, _get_decision + + +@pytest.fixture() +def mock_async_client(): + return AsyncClient(verify=False) # noqa: S501 + + +@pytest.mark.asyncio() +async def test_get_decision_success(mock_async_client): + opa_url = "http://opa-server/v1/data/test" + opa_input = {"input": {"user": "test_user", "action": "test_action"}} + mock_response = {"decision_id": "1234", "result": {"allow": True}} + + mock_async_client.post = AsyncMock(return_value=Response(200, json=mock_response)) + + result = await _get_decision(opa_url, mock_async_client, opa_input) + + assert isinstance(result, OPAResult) + assert result.decision_id == "1234" + assert result.result is True + + +@pytest.mark.asyncio() +async def test_get_decision_network_error(mock_async_client): + opa_url = "http://opa-server/v1/data/test" + opa_input = {"input": {"user": "test_user", "action": "test_action"}} + + mock_async_client.post = AsyncMock(side_effect=NetworkError("Network error")) + + with pytest.raises(HTTPException) as exc_info: + await _get_decision(opa_url, mock_async_client, opa_input) + + assert exc_info.value.status_code == 503 + assert exc_info.value.detail == "Policy agent is unavailable" + + +@pytest.mark.asyncio() +async def test_get_decision_invalid_response(mock_async_client): + opa_url = "http://opa-server/v1/data/test" + opa_input = {"input": {"user": "test_user", "action": "test_action"}} + mock_response = {"invalid_key": "value"} + + mock_async_client.post = AsyncMock(return_value=Response(200, json=mock_response)) + + with pytest.raises(KeyError): + await _get_decision(opa_url, mock_async_client, opa_input) + + +@pytest.mark.asyncio() +async def test_opaauthz_get_decision(mock_async_client): + opa_url = "http://opa-server/v1/data/test" + opa_input = {"input": {"user": "test_user", "action": "test_action"}} + mock_response = {"decision_id": "1234", "result": {"allow": True}} + + opa_instance = OPAAuthZ(opa_url=opa_url) + mock_async_client.post = AsyncMock(return_value=Response(200, json=mock_response)) + + result = await opa_instance.get_decision(mock_async_client, opa_input) + + assert isinstance(result, OPAResult) + assert result.decision_id == "1234" + assert result.result is True + + +@pytest.mark.asyncio() +async def test_graphql_opaauthz_get_decision(mock_async_client): + opa_url = "http://opa-server/v1/data/test" + opa_input = {"input": {"user": "test_user", "action": "test_action"}} + mock_response = {"decision_id": "1234", "result": {"allow": True}} + + graphql_opa_instance = GraphQLOPAAuthZ(opa_url=opa_url) + mock_async_client.post = AsyncMock(return_value=Response(200, json=mock_response)) + + result = await graphql_opa_instance.get_decision(mock_async_client, opa_input) + + assert isinstance(result, OPAResult) + assert result.decision_id == "1234" + assert result.result is True diff --git a/test/cli/test_imports.py b/test/cli/test_imports.py index c7331665..8933627b 100644 --- a/test/cli/test_imports.py +++ b/test/cli/test_imports.py @@ -305,7 +305,7 @@ def test_import_iptrunk_invalid_router_id_side_a_and_b(mock_start_process, mock_ """Validation error: 2 validation errors for IptrunkImportModel side_a_node_id Value error, Router not found [type=value_error, input_value='', input_type=str] - For further information visit https://errors.pydantic.dev/2.5/v/value_error + For further information visit https://errors.pydantic.dev/2.7/v/value_error side_b_node_id Value error, Router not found [type=value_error, input_value='', input_type=str]""" in captured_output -- GitLab