From b626b110ef19d5caa0825f80f26c81fc31ea9ba6 Mon Sep 17 00:00:00 2001 From: Bruce Schultz Date: Fri, 13 Mar 2026 07:45:20 +0100 Subject: [PATCH 1/4] fix(auth): manually build internal token ep from k8s svc --- hub_adapter/auth.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/hub_adapter/auth.py b/hub_adapter/auth.py index c555f21..533a67c 100644 --- a/hub_adapter/auth.py +++ b/hub_adapter/auth.py @@ -160,7 +160,7 @@ async def verify_idp_token( async def get_internal_token( - oidc_config, hub_adapter_settings: Annotated[Settings, Depends(get_settings)] + hub_adapter_settings: Annotated[Settings, Depends(get_settings)] ) -> dict | None: """If the Hub Adapter is set up tp use an external IDP, it needs to retrieve a JWT from the internal keycloak to make requests to the PO.""" @@ -171,10 +171,11 @@ async def get_internal_token( "client_secret": hub_adapter_settings.API_CLIENT_SECRET, } - with httpx.Client(verify=get_ssl_context(hub_adapter_settings)) as client: - resp = client.post(oidc_config.token_endpoint, data=payload) - resp.raise_for_status() - token_data = resp.json() + int_token_ep = hub_adapter_settings.NODE_SVC_OIDC_URL.rstrip("/") + "/protocol/openid-connect/token" + + resp = httpx.post(int_token_ep, data=payload) + resp.raise_for_status() + token_data = resp.json() token = Token(**token_data) return {"Authorization": f"Bearer {token.access_token}"} @@ -182,11 +183,11 @@ async def get_internal_token( async def add_internal_token_if_missing(request: Request) -> Request: """Adds a JWT from the internal IDP is not present in the request.""" - configs_match, oidc_config = check_oidc_configs_match() + configs_match, _ = check_oidc_configs_match() if not configs_match: logger.debug("External IDP different from internal, retrieving JWT from internal keycloak") - internal_token = await get_internal_token(oidc_config) + internal_token = await get_internal_token(get_settings()) if internal_token: updated_headers = MutableHeaders(request.headers) updated_headers.update(internal_token) From 7f999e5fc7c07292af5f16ab2fb148e1fb99ad07 Mon Sep 17 00:00:00 2001 From: Bruce Schultz Date: Tue, 7 Apr 2026 08:29:24 +0200 Subject: [PATCH 2/4] feat(auth): keep internal token fetch within cluster --- hub_adapter/auth.py | 8 ++++---- tests/conftest.py | 3 ++- tests/constants.py | 11 ++++------- tests/test_auth.py | 17 +++++++++-------- tests/test_oidc.py | 23 ++++++++++++----------- 5 files changed, 31 insertions(+), 31 deletions(-) diff --git a/hub_adapter/auth.py b/hub_adapter/auth.py index 95d266c..756a427 100644 --- a/hub_adapter/auth.py +++ b/hub_adapter/auth.py @@ -161,8 +161,8 @@ async def verify_idp_token( async def _get_internal_token(settings: Annotated[Settings, Depends(get_settings)]) -> dict | None: - """If the Hub Adapter is set up to use an external IDP, it needs to retrieve a JWT from the internal keycloak - to make requests to the PO.""" + """If the Hub Adapter is set up to use an external IDP for user auth, it needs to retrieve a JWT from the + internal keycloak to make requests to the PO.""" payload = { "grant_type": "client_credentials", @@ -183,11 +183,11 @@ async def _get_internal_token(settings: Annotated[Settings, Depends(get_settings async def _add_internal_token_if_missing(request: Request) -> Request: """Adds a JWT from the internal IDP is not present in the request.""" settings = get_settings() - configs_match, oidc_config = check_oidc_configs_match() + configs_match, _ = check_oidc_configs_match() if not configs_match: logger.debug("External IDP different from internal, retrieving JWT from internal keycloak") - internal_token = await _get_internal_token(oidc_config, settings) + internal_token = await _get_internal_token(settings) if internal_token: updated_headers = MutableHeaders(request.headers) updated_headers.update(internal_token) diff --git a/tests/conftest.py b/tests/conftest.py index 4e1e434..b537004 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -16,6 +16,7 @@ from tests.constants import ( FAKE_USER, TEST_MOCK_NODE_CLIENT_ID, + TEST_SVC_URL, TEST_URL, ) @@ -75,7 +76,7 @@ def test_settings() -> Settings: api_client_secret="notASecret", http_proxy="http://squid.proxy:3128", https_proxy="http://squid.proxy:3128", - node_svc_oidc_url=TEST_URL, + node_svc_oidc_url=TEST_SVC_URL, postgres_event_db="test_db", postgres_event_user="test_user", postgres_event_password="test_password", diff --git a/tests/constants.py b/tests/constants.py index 74d3e9f..8233c63 100644 --- a/tests/constants.py +++ b/tests/constants.py @@ -16,7 +16,7 @@ TEST_OIDC = OIDCConfiguration( issuer=TEST_URL, authorization_endpoint=TEST_URL, - token_endpoint=TEST_URL, + token_endpoint=f"{TEST_URL}/protocol/openid-connect/token", jwks_uri=TEST_URL, userinfo_endpoint=TEST_URL, ) @@ -24,12 +24,11 @@ TEST_SVC_OIDC = OIDCConfiguration( issuer=TEST_SVC_URL, authorization_endpoint=TEST_SVC_URL, - token_endpoint=TEST_SVC_URL, + token_endpoint=f"{TEST_SVC_URL}/protocol/openid-connect/token", jwks_uri=TEST_SVC_URL, userinfo_endpoint=TEST_SVC_URL, ) - TEST_MOCK_ANALYSIS_ID = "1c9cb547-4afc-4398-bcb6-954bc61a1bb1" TEST_MOCK_PROJECT_ID = "9cbefefe-2420-4b8e-8ac1-f48148a9fd40" TEST_MOCK_NODE_ID = "9c521144-364d-4cdc-8ec4-cb62a537f10c" @@ -102,7 +101,6 @@ "analysis": MOCK_ANALYSIS, } - ANALYSIS_NODES_RESP = [ { # Shouldn't start because executed @@ -208,7 +206,7 @@ "authorization_endpoint": TEST_URL, "issuer": TEST_URL, "jwks_uri": TEST_URL, - "token_endpoint": TEST_URL, + "token_endpoint": f"{TEST_URL}/protocol/openid-connect/token", "userinfo_endpoint": TEST_URL, } @@ -216,7 +214,7 @@ "authorization_endpoint": TEST_SVC_URL, "issuer": TEST_SVC_URL, "jwks_uri": TEST_SVC_URL, - "token_endpoint": TEST_SVC_URL, + "token_endpoint": f"{TEST_SVC_URL}/protocol/openid-connect/token", "userinfo_endpoint": TEST_SVC_URL, } @@ -320,7 +318,6 @@ "acl": ACL().__dict__, } - FAKE_USER = { "acr": "1", "allowed-origins": ["/*"], diff --git a/tests/test_auth.py b/tests/test_auth.py index f987af5..54217f5 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -29,6 +29,7 @@ TEST_OIDC, TEST_RESEARCHER_DECRYPTED_JWT, TEST_STEWARD_DECRYPTED_JWT, + TEST_SVC_OIDC, ) @@ -99,8 +100,8 @@ async def test_get_internal_token(self, httpx_mock, test_settings): "refresh_token": TEST_JWT, "refresh_expires_in": 1800, } - httpx_mock.add_response(url=TEST_OIDC.token_endpoint, json=fake_token_resp, status_code=200) - assert await _get_internal_token(TEST_OIDC, test_settings) == {"Authorization": f"Bearer {TEST_JWT}"} + httpx_mock.add_response(url=TEST_SVC_OIDC.token_endpoint, json=fake_token_resp, status_code=200) + assert await _get_internal_token(test_settings) == {"Authorization": f"Bearer {TEST_JWT}"} @patch("hub_adapter.auth._get_internal_token") @patch("hub_adapter.auth.check_oidc_configs_match") @@ -170,16 +171,16 @@ async def test_check_rbac_rules(self, mock_logger): await require_steward_role(TEST_STEWARD_DECRYPTED_JWT, mock_settings_with_mismatched_roles) assert steward_error.value.status_code == status.HTTP_403_FORBIDDEN assert ( - steward_error.value.detail["message"] - == f"Insufficient permissions, admin or {STEWARD_ROLE} role not found in token." + steward_error.value.detail["message"] + == f"Insufficient permissions, admin or {STEWARD_ROLE} role not found in token." ) with pytest.raises(HTTPException) as researcher_error: await require_researcher_role(TEST_RESEARCHER_DECRYPTED_JWT, mock_settings_with_mismatched_roles) assert researcher_error.value.status_code == status.HTTP_403_FORBIDDEN assert ( - researcher_error.value.detail["message"] - == f"Insufficient permissions, admin or {RESEARCHER_ROLE} role not found in token." + researcher_error.value.detail["message"] + == f"Insufficient permissions, admin or {RESEARCHER_ROLE} role not found in token." ) # Wrong claim name @@ -196,6 +197,6 @@ async def test_check_rbac_rules(self, mock_logger): mock_logger.warning.assert_any_call(f"No roles found in token using {wrong_claim_name}") assert steward_error.value.status_code == status.HTTP_403_FORBIDDEN assert ( - steward_error.value.detail["message"] - == f"Insufficient permissions, admin or {STEWARD_ROLE} role not found in token." + steward_error.value.detail["message"] + == f"Insufficient permissions, admin or {STEWARD_ROLE} role not found in token." ) diff --git a/tests/test_oidc.py b/tests/test_oidc.py index 06c6a34..3e5a899 100644 --- a/tests/test_oidc.py +++ b/tests/test_oidc.py @@ -21,24 +21,25 @@ def test_basic_oidc_fetching(self, mock_settings, httpx_mock, test_settings): fake_oidc_url = f"{TEST_URL}/.well-known/openid-configuration" fake_oidc_svc_url = f"{TEST_SVC_URL}/.well-known/openid-configuration" - mock_settings.return_value = test_settings + mock_settings.return_value = test_settings # Initializes with different URLs httpx_mock.add_response(url=fake_oidc_url, json=TEST_OIDC_RESPONSE, status_code=200) - - # Same OIDC - match_check, match_config = check_oidc_configs_match() - assert match_check - assert match_config == TEST_OIDC - - # Different OIDC URLs - different_oidc_settings = test_settings.model_copy(update={"node_svc_oidc_url": TEST_SVC_URL}) - mock_settings.return_value = different_oidc_settings - httpx_mock.add_response(url=fake_oidc_svc_url, json=TEST_OIDC_SVC_RESPONSE, status_code=200) + # Different OIDC URLs diff_check, diff_config = check_oidc_configs_match() assert not diff_check assert diff_config == TEST_SVC_OIDC + # Same OIDC + matching_oidc_settings = test_settings.model_copy(update={"node_svc_oidc_url": fake_oidc_url}) + mock_settings.return_value = matching_oidc_settings + + httpx_mock.add_response(url=fake_oidc_url, json=TEST_OIDC_RESPONSE, status_code=200) + + match_check, match_config = check_oidc_configs_match() + assert match_check + assert match_config == TEST_OIDC + @patch("hub_adapter.oidc.logger") def test_fetch_openid_config_errors(self, mock_logger, httpx_mock): """Test the fetch_openid_config method for error handling.""" From 45a4862302603b663f782d26673084195ff71b87 Mon Sep 17 00:00:00 2001 From: Bruce Schultz Date: Tue, 7 Apr 2026 08:44:56 +0200 Subject: [PATCH 3/4] refactor(auth): use provided endpoints for internal token instead of hardcoded --- hub_adapter/auth.py | 3 ++- tests/test_auth.py | 16 +++++++++------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/hub_adapter/auth.py b/hub_adapter/auth.py index 756a427..3766888 100644 --- a/hub_adapter/auth.py +++ b/hub_adapter/auth.py @@ -170,7 +170,8 @@ async def _get_internal_token(settings: Annotated[Settings, Depends(get_settings "client_secret": settings.api_client_secret, } - int_token_ep = settings.node_svc_oidc_url.rstrip("/") + "/protocol/openid-connect/token" + svc_oidc_config = get_svc_oidc_config() + int_token_ep = svc_oidc_config.token_endpoint resp = httpx.post(int_token_ep, data=payload) resp.raise_for_status() diff --git a/tests/test_auth.py b/tests/test_auth.py index 54217f5..0fdf083 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -90,9 +90,11 @@ async def test_verify_idp_token_errors(self, mock_decode, mock_user_oidc, mock_s assert random_error.value.status_code == status.HTTP_401_UNAUTHORIZED assert missing_claim_error.value.detail["message"] == "Unable to parse authentication token" + @patch("hub_adapter.auth.get_svc_oidc_config") @pytest.mark.asyncio - async def test_get_internal_token(self, httpx_mock, test_settings): + async def test_get_internal_token(self, mock_svc_oidc, httpx_mock, test_settings): """Test the get_internal_token method.""" + mock_svc_oidc.return_value = TEST_SVC_OIDC fake_token_resp = { "access_token": TEST_JWT, "token_type": "Bearer", @@ -171,16 +173,16 @@ async def test_check_rbac_rules(self, mock_logger): await require_steward_role(TEST_STEWARD_DECRYPTED_JWT, mock_settings_with_mismatched_roles) assert steward_error.value.status_code == status.HTTP_403_FORBIDDEN assert ( - steward_error.value.detail["message"] - == f"Insufficient permissions, admin or {STEWARD_ROLE} role not found in token." + steward_error.value.detail["message"] + == f"Insufficient permissions, admin or {STEWARD_ROLE} role not found in token." ) with pytest.raises(HTTPException) as researcher_error: await require_researcher_role(TEST_RESEARCHER_DECRYPTED_JWT, mock_settings_with_mismatched_roles) assert researcher_error.value.status_code == status.HTTP_403_FORBIDDEN assert ( - researcher_error.value.detail["message"] - == f"Insufficient permissions, admin or {RESEARCHER_ROLE} role not found in token." + researcher_error.value.detail["message"] + == f"Insufficient permissions, admin or {RESEARCHER_ROLE} role not found in token." ) # Wrong claim name @@ -197,6 +199,6 @@ async def test_check_rbac_rules(self, mock_logger): mock_logger.warning.assert_any_call(f"No roles found in token using {wrong_claim_name}") assert steward_error.value.status_code == status.HTTP_403_FORBIDDEN assert ( - steward_error.value.detail["message"] - == f"Insufficient permissions, admin or {STEWARD_ROLE} role not found in token." + steward_error.value.detail["message"] + == f"Insufficient permissions, admin or {STEWARD_ROLE} role not found in token." ) From 2ff63a2fad458a137f5c9a82615d097c42c2afbc Mon Sep 17 00:00:00 2001 From: Bruce Schultz Date: Tue, 7 Apr 2026 09:37:02 +0200 Subject: [PATCH 4/4] refactor(auth): remove oidc_config from method calls --- hub_adapter/autostart.py | 3 +-- hub_adapter/integration_test.py | 3 +-- hub_adapter/routers/kong.py | 3 ++- hub_adapter/routers/meta.py | 3 +-- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/hub_adapter/autostart.py b/hub_adapter/autostart.py index ab05b94..e600b1a 100644 --- a/hub_adapter/autostart.py +++ b/hub_adapter/autostart.py @@ -243,8 +243,7 @@ async def pod_running(self, analysis_id: str) -> bool | None: async def fetch_token_header(self) -> dict | None: """Append OIDC token to headers.""" try: - _, oidc_config = check_oidc_configs_match() - token = await _get_internal_token(oidc_config, self.settings) + token = await _get_internal_token(self.settings) return token except (HTTPException, HTTPStatusError) as e: diff --git a/hub_adapter/integration_test.py b/hub_adapter/integration_test.py index 18c7c5f..937374d 100644 --- a/hub_adapter/integration_test.py +++ b/hub_adapter/integration_test.py @@ -89,8 +89,7 @@ async def get_auth_token(self) -> dict: if self.token: return self.token - oidc_config = get_svc_oidc_config() - self.token = await _get_internal_token(oidc_config, self.settings) + self.token = await _get_internal_token(self.settings) logger.info("Successfully obtained authentication token") return self.token diff --git a/hub_adapter/routers/kong.py b/hub_adapter/routers/kong.py index 6dd8eef..fcb788a 100644 --- a/hub_adapter/routers/kong.py +++ b/hub_adapter/routers/kong.py @@ -4,6 +4,7 @@ import time import uuid from typing import Annotated +from uuid import UUID import httpx import kong_admin_client @@ -687,7 +688,7 @@ async def create_and_connect_analysis_to_project( @catch_kong_errors async def delete_analysis( settings: Annotated[Settings, Depends(get_settings)], - analysis_id: Annotated[str, Path(description="UUID or unique name of the analysis.")], + analysis_id: Annotated[str | UUID, Path(description="UUID or unique name of the analysis.")], ): """Delete the listed analysis.""" configuration = kong_admin_client.Configuration(host=settings.kong_admin_service_url) diff --git a/hub_adapter/routers/meta.py b/hub_adapter/routers/meta.py index 05853ed..07831dc 100644 --- a/hub_adapter/routers/meta.py +++ b/hub_adapter/routers/meta.py @@ -134,8 +134,7 @@ async def terminate_analysis( """ await delete_analysis(analysis_id=analysis_id, settings=settings) - configs_match, oidc_config = check_oidc_configs_match() - headers = await _get_internal_token(oidc_config, settings) + headers = await _get_internal_token(settings) microsvc_path = f"{settings.podorc_service_url}/po/delete/{analysis_id}"