From f40113b38d16c73d134f9d90389c14c1a30671bd Mon Sep 17 00:00:00 2001 From: Mohammad Torkashvand <mohammad.torkashvand@geant.org> Date: Fri, 12 Jan 2024 18:06:41 +0100 Subject: [PATCH] resolve review notes --- docs/source/glossary.rst | 3 +++ docs/source/module/auth/index.rst | 16 ++++++++++++ .../source/module/auth/oidc_policy_helper.rst | 6 +++++ docs/source/module/auth/security.rst | 6 +++++ docs/source/module/auth/settings.rst | 6 +++++ .../vale/styles/Vocab/geant-jargon/accept.txt | 1 + gso/auth/oidc_policy_helper.py | 25 ++++++------------- gso/auth/settings.py | 5 +--- pyproject.toml | 1 - test/auth/test_oidc_policy_helper.py | 11 +++++--- 10 files changed, 53 insertions(+), 27 deletions(-) create mode 100644 docs/source/module/auth/index.rst create mode 100644 docs/source/module/auth/oidc_policy_helper.rst create mode 100644 docs/source/module/auth/security.rst create mode 100644 docs/source/module/auth/settings.rst diff --git a/docs/source/glossary.rst b/docs/source/glossary.rst index 69f1a655..959d8976 100644 --- a/docs/source/glossary.rst +++ b/docs/source/glossary.rst @@ -63,3 +63,6 @@ Glossary of terms WFO `Workflow Orchestrator <https://workfloworchestrator.org/>`_ + + AAI + Authentication and Authorisation Infrastructure diff --git a/docs/source/module/auth/index.rst b/docs/source/module/auth/index.rst new file mode 100644 index 00000000..0ec5cd1f --- /dev/null +++ b/docs/source/module/auth/index.rst @@ -0,0 +1,16 @@ +``gso.products`` +================ + +.. automodule:: gso.auth + :members: + :show-inheritance: + +Subpackages +----------- + +.. toctree:: + :maxdepth: 1 + + oidc_policy_helper + security + settings diff --git a/docs/source/module/auth/oidc_policy_helper.rst b/docs/source/module/auth/oidc_policy_helper.rst new file mode 100644 index 00000000..b01d9cdf --- /dev/null +++ b/docs/source/module/auth/oidc_policy_helper.rst @@ -0,0 +1,6 @@ +``gso.auth.oidc_policy_helper`` +==================================== + +.. automodule:: gso.auth.oidc_policy_helper + :members: + :show-inheritance: diff --git a/docs/source/module/auth/security.rst b/docs/source/module/auth/security.rst new file mode 100644 index 00000000..c9330542 --- /dev/null +++ b/docs/source/module/auth/security.rst @@ -0,0 +1,6 @@ +``gso.auth.security`` +==================================== + +.. automodule:: gso.auth.security + :members: + :show-inheritance: diff --git a/docs/source/module/auth/settings.rst b/docs/source/module/auth/settings.rst new file mode 100644 index 00000000..2bc37fa8 --- /dev/null +++ b/docs/source/module/auth/settings.rst @@ -0,0 +1,6 @@ +``gso.auth.settings`` +==================================== + +.. automodule:: gso.auth.settings + :members: + :show-inheritance: diff --git a/docs/vale/styles/Vocab/geant-jargon/accept.txt b/docs/vale/styles/Vocab/geant-jargon/accept.txt index 1d257c7c..aba8e760 100644 --- a/docs/vale/styles/Vocab/geant-jargon/accept.txt +++ b/docs/vale/styles/Vocab/geant-jargon/accept.txt @@ -13,3 +13,4 @@ Dark_fiber [A|a]llocate PHASE 1 [Mm]odify +AAI diff --git a/gso/auth/oidc_policy_helper.py b/gso/auth/oidc_policy_helper.py index d9219cdd..945b7496 100644 --- a/gso/auth/oidc_policy_helper.py +++ b/gso/auth/oidc_policy_helper.py @@ -167,7 +167,7 @@ class OIDCUser(HTTPBearer): """OIDCUser class extends the :term:`HTTPBearer` class to do extra verification. The class will act as follows: - 1. Validate the Credentials at AAI proxy by calling the UserInfo endpoint + 1. Validate the Credentials at :term: `AAI` proxy by calling the UserInfo endpoint """ openid_config: OIDCConfig | None = None @@ -245,14 +245,9 @@ class OIDCUser(HTTPBearer): async def userinfo(self, async_request: AsyncClient, token: str) -> OIDCUserModel: """Get the userinfo from the openid server. - Args: - ---- - async_request: The async request - token: the access_token - - Returns: - ------- - OIDCUserModel from openid server + :param AsyncClient async_request: The async request + :param str token: the access_token + :return: OIDCUserModel: OIDC user model from openid server """ await self.check_openid_config(async_request) @@ -289,15 +284,9 @@ class OIDCUser(HTTPBearer): async def introspect_token(self, async_request: AsyncClient, token: str) -> dict: """Introspect the access token to see if it is a valid token. - Args: - ---- - async_request: The async request - token: the access_token - - Returns: - ------- - dict from openid server - + :param async_request: The async request + :param token: the access_token + :return: dict from openid server """ await self.check_openid_config(async_request) assert self.openid_config, "OpenID config should be loaded" # noqa: S101 diff --git a/gso/auth/settings.py b/gso/auth/settings.py index d8b28125..29c1fc80 100644 --- a/gso/auth/settings.py +++ b/gso/auth/settings.py @@ -3,10 +3,7 @@ authentication and authorization, including token validation and user authentication. Integrates with external authentication providers for enhanced security management. -Todo: ----- -Remove token and sensitive data from OPA console and API. - +Todo: Remove token and sensitive data from OPA console and API. """ from pydantic import BaseSettings, Field diff --git a/pyproject.toml b/pyproject.toml index e345710b..5a2ca81c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -39,7 +39,6 @@ ignore = [ "PLR0913", "PLR0904", "PLW1514", - "S106", ] line-length = 120 select = [ diff --git a/test/auth/test_oidc_policy_helper.py b/test/auth/test_oidc_policy_helper.py index a47e2d0f..500b18cb 100644 --- a/test/auth/test_oidc_policy_helper.py +++ b/test/auth/test_oidc_policy_helper.py @@ -54,7 +54,7 @@ def oidc_user(mock_openid_config): user = OIDCUser( openid_url="https://example.proxy.aai.geant.org", resource_server_id="resource_server", - resource_server_secret="secret", + resource_server_secret="secret", # noqa: S106 ) user.openid_config = OIDCConfig.parse_obj(mock_openid_config) return user @@ -75,7 +75,10 @@ def mock_request(): @pytest.fixture() def mock_oidc_user(): oidc_user = AsyncMock( - OIDCUser, openid_url="https://example.com", resource_server_id="test", resource_server_secret="secret" + OIDCUser, + openid_url="https://example.com", + resource_server_id="test", + resource_server_secret="secret", # noqa: S106 ) oidc_user.__call__ = AsyncMock(return_value=OIDCUserModel({"sub": "123", "name": "John Doe"})) return oidc_user @@ -236,7 +239,7 @@ async def test_oidc_user_call_with_token(oidc_user, mock_request, mock_async_cli 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") + result = await oidc_user.__call__(mock_request, token="test_token") # noqa: S106 assert isinstance(result, OIDCUserModel) assert result["sub"] == "123" @@ -248,7 +251,7 @@ async def test_oidc_user_call_inactive_token(oidc_user, mock_request, mock_async oidc_user.introspect_token = AsyncMock(return_value={"active": False}) with pytest.raises(HTTPException) as exc_info: - await oidc_user.__call__(mock_request, token="test_token") + await oidc_user.__call__(mock_request, token="test_token") # noqa: S106 assert exc_info.value.status_code == HTTPStatus.UNAUTHORIZED assert "User is not active" in str(exc_info.value.detail) -- GitLab