From 7d4aba484baf1a6b822cc45951b605b045f45b41 Mon Sep 17 00:00:00 2001 From: Yuki I Date: Fri, 21 Nov 2025 10:16:02 +0000 Subject: [PATCH 1/2] fix: add auth headers and body to multitenancy token request Signed-off-by: Yuki I --- oidc-controller/api/core/acapy/config.py | 63 +++++++-- .../api/core/acapy/tests/test_config.py | 129 +++++++++++++++++- 2 files changed, 180 insertions(+), 12 deletions(-) diff --git a/oidc-controller/api/core/acapy/config.py b/oidc-controller/api/core/acapy/config.py index ac577066..d9803d28 100644 --- a/oidc-controller/api/core/acapy/config.py +++ b/oidc-controller/api/core/acapy/config.py @@ -21,16 +21,50 @@ class MultiTenantAcapy: @cache def get_wallet_token(self): logger.debug(">>> get_wallet_token") - resp_raw = requests.post( - settings.ACAPY_ADMIN_URL + f"/multitenancy/wallet/{self.wallet_id}/token", + + # Check if admin API key is configured + admin_api_key_configured = ( + settings.ST_ACAPY_ADMIN_API_KEY_NAME and settings.ST_ACAPY_ADMIN_API_KEY ) - assert ( - resp_raw.status_code == 200 - ), f"{resp_raw.status_code}::{resp_raw.content}" - resp = json.loads(resp_raw.content) - wallet_token = resp["token"] - logger.debug("<<< get_wallet_token") + headers = {} + + if admin_api_key_configured: + logger.debug("Admin API key is configured, adding to request headers") + headers[settings.ST_ACAPY_ADMIN_API_KEY_NAME] = ( + settings.ST_ACAPY_ADMIN_API_KEY + ) + else: + logger.debug( + "No admin API key configured, proceeding without authentication headers" + ) + + payload = {"wallet_key": self.wallet_key} + + try: + resp_raw = requests.post( + settings.ACAPY_ADMIN_URL + + f"/multitenancy/wallet/{self.wallet_id}/token", + headers=headers, + json=payload, + ) + + if resp_raw.status_code != 200: + error_detail = resp_raw.content.decode() + logger.error( + f"Failed to get wallet token. Status: {resp_raw.status_code}, Detail: {error_detail}" + ) + # Raising Exception to be caught by the except block below or propagated + raise Exception(f"{resp_raw.status_code}::{error_detail}") + + resp = json.loads(resp_raw.content) + wallet_token = resp["token"] + + except Exception as e: + logger.error(f"Exception occurred while getting wallet token: {str(e)}") + raise + + logger.debug("<<< get_wallet_token") return wallet_token def get_headers(self) -> dict[str, str]: @@ -39,4 +73,15 @@ def get_headers(self) -> dict[str, str]: class SingleTenantAcapy: def get_headers(self) -> dict[str, str]: - return {settings.ST_ACAPY_ADMIN_API_KEY_NAME: settings.ST_ACAPY_ADMIN_API_KEY} + # Check if admin API key is configured + admin_api_key_configured = ( + settings.ST_ACAPY_ADMIN_API_KEY_NAME and settings.ST_ACAPY_ADMIN_API_KEY + ) + + if admin_api_key_configured: + return { + settings.ST_ACAPY_ADMIN_API_KEY_NAME: settings.ST_ACAPY_ADMIN_API_KEY + } + else: + logger.debug("No admin API key configured for single tenant agent") + return {} diff --git a/oidc-controller/api/core/acapy/tests/test_config.py b/oidc-controller/api/core/acapy/tests/test_config.py index ed442957..73c60adf 100644 --- a/oidc-controller/api/core/acapy/tests/test_config.py +++ b/oidc-controller/api/core/acapy/tests/test_config.py @@ -13,6 +13,16 @@ async def test_single_tenant_has_expected_headers(): assert headers == {"name": "key"} +@pytest.mark.asyncio +@mock.patch.object(settings, "ST_ACAPY_ADMIN_API_KEY_NAME", "name") +@mock.patch.object(settings, "ST_ACAPY_ADMIN_API_KEY", None) +async def test_single_tenant_empty_headers_not_configured(): + # Test behavior when API key is missing + acapy = SingleTenantAcapy() + headers = acapy.get_headers() + assert headers == {} + + @pytest.mark.asyncio async def test_multi_tenant_get_headers_returns_bearer_token_auth(requests_mock): acapy = MultiTenantAcapy() @@ -45,7 +55,120 @@ async def test_multi_tenant_throws_assertion_error_for_non_200_response(requests ) acapy = MultiTenantAcapy() acapy.wallet_id = "wallet_id" - try: + + # Clear cache to ensure test isolation + acapy.get_wallet_token.cache_clear() + + # The implementation raises Exception, not AssertionError + with pytest.raises(Exception) as exc_info: acapy.get_wallet_token() - except AssertionError as e: - assert e is not None + + assert "400" in str(exc_info.value) + + +@pytest.mark.asyncio +async def test_multi_tenant_get_wallet_token_includes_auth_headers_and_body( + requests_mock, +): + # Verify headers and body payload + wallet_id = "wallet_id" + wallet_key = "wallet_key" + admin_key = "admin_key" + admin_header = "x-api-key" + + # Mock settings for the duration of this test + with mock.patch.object( + settings, "MT_ACAPY_WALLET_ID", wallet_id + ), mock.patch.object( + settings, "MT_ACAPY_WALLET_KEY", wallet_key + ), mock.patch.object( + settings, "ST_ACAPY_ADMIN_API_KEY", admin_key + ), mock.patch.object( + settings, "ST_ACAPY_ADMIN_API_KEY_NAME", admin_header + ): + + acapy = MultiTenantAcapy() + # Ensure we use the values we expect (class init reads settings once) + acapy.wallet_id = wallet_id + acapy.wallet_key = wallet_key + # Ensure we bypass cache from any previous tests + acapy.get_wallet_token.cache_clear() + + requests_mock.post( + settings.ACAPY_ADMIN_URL + f"/multitenancy/wallet/{wallet_id}/token", + json={"token": "token"}, + status_code=200, + ) + + token = acapy.get_wallet_token() + assert token == "token" + + # Verify request details + last_request = requests_mock.last_request + assert last_request.headers[admin_header] == admin_key + assert last_request.json() == {"wallet_key": wallet_key} + + +@pytest.mark.asyncio +async def test_multi_tenant_get_wallet_token_no_auth_headers_when_not_configured( + requests_mock, +): + # Test insecure mode behavior + wallet_id = "wallet_id" + wallet_key = "wallet_key" + + # Mock settings with None for admin key + with mock.patch.object( + settings, "MT_ACAPY_WALLET_ID", wallet_id + ), mock.patch.object( + settings, "MT_ACAPY_WALLET_KEY", wallet_key + ), mock.patch.object( + settings, "ST_ACAPY_ADMIN_API_KEY", None + ), mock.patch.object( + settings, "ST_ACAPY_ADMIN_API_KEY_NAME", "x-api-key" + ): + + acapy = MultiTenantAcapy() + acapy.wallet_id = wallet_id + acapy.wallet_key = wallet_key + acapy.get_wallet_token.cache_clear() + + requests_mock.post( + settings.ACAPY_ADMIN_URL + f"/multitenancy/wallet/{wallet_id}/token", + json={"token": "token"}, + status_code=200, + ) + + token = acapy.get_wallet_token() + assert token == "token" + + # Verify request details + last_request = requests_mock.last_request + # Headers might contain Content-Type, but should not contain the api key + assert "x-api-key" not in last_request.headers + assert last_request.json() == {"wallet_key": wallet_key} + + +@pytest.mark.asyncio +async def test_multi_tenant_throws_exception_for_non_200_response(requests_mock): + wallet_id = "wallet_id" + wallet_key = "wallet_key" + + with mock.patch.object(settings, "MT_ACAPY_WALLET_ID", wallet_id): + acapy = MultiTenantAcapy() + acapy.wallet_id = wallet_id + acapy.wallet_key = wallet_key + acapy.get_wallet_token.cache_clear() + + requests_mock.post( + settings.ACAPY_ADMIN_URL + f"/multitenancy/wallet/{wallet_id}/token", + json={"error": "unauthorized"}, + status_code=401, # Use 401 for a more realistic test + ) + + # Check for generic Exception, as the code now raises Exception(f"{code}::{detail}") + with pytest.raises(Exception) as excinfo: + acapy.get_wallet_token() + + assert "401" in str(excinfo.value) + assert "unauthorized" in str(excinfo.value) From 68c4e6b4b57810f07817943834d9c2153b8d9332 Mon Sep 17 00:00:00 2001 From: Yuki I Date: Fri, 28 Nov 2025 19:25:32 +0000 Subject: [PATCH 2/2] fix: address PR review (redundant try/except, mocks, cleanup tests) Signed-off-by: Yuki I --- oidc-controller/api/core/acapy/config.py | 34 +++++------- .../api/core/acapy/tests/test_config.py | 55 ++++++++----------- 2 files changed, 38 insertions(+), 51 deletions(-) diff --git a/oidc-controller/api/core/acapy/config.py b/oidc-controller/api/core/acapy/config.py index d9803d28..ebaeb795 100644 --- a/oidc-controller/api/core/acapy/config.py +++ b/oidc-controller/api/core/acapy/config.py @@ -41,28 +41,22 @@ def get_wallet_token(self): payload = {"wallet_key": self.wallet_key} - try: - resp_raw = requests.post( - settings.ACAPY_ADMIN_URL - + f"/multitenancy/wallet/{self.wallet_id}/token", - headers=headers, - json=payload, - ) - - if resp_raw.status_code != 200: - error_detail = resp_raw.content.decode() - logger.error( - f"Failed to get wallet token. Status: {resp_raw.status_code}, Detail: {error_detail}" - ) - # Raising Exception to be caught by the except block below or propagated - raise Exception(f"{resp_raw.status_code}::{error_detail}") + resp_raw = requests.post( + settings.ACAPY_ADMIN_URL + f"/multitenancy/wallet/{self.wallet_id}/token", + headers=headers, + json=payload, + ) - resp = json.loads(resp_raw.content) - wallet_token = resp["token"] + if resp_raw.status_code != 200: + error_detail = resp_raw.content.decode() + logger.error( + f"Failed to get wallet token. Status: {resp_raw.status_code}, Detail: {error_detail}" + ) + # Raising Exception to be caught by the except block below or propagated + raise Exception(f"{resp_raw.status_code}::{error_detail}") - except Exception as e: - logger.error(f"Exception occurred while getting wallet token: {str(e)}") - raise + resp = json.loads(resp_raw.content) + wallet_token = resp["token"] logger.debug("<<< get_wallet_token") return wallet_token diff --git a/oidc-controller/api/core/acapy/tests/test_config.py b/oidc-controller/api/core/acapy/tests/test_config.py index 73c60adf..ce115588 100644 --- a/oidc-controller/api/core/acapy/tests/test_config.py +++ b/oidc-controller/api/core/acapy/tests/test_config.py @@ -7,7 +7,7 @@ @pytest.mark.asyncio @mock.patch.object(settings, "ST_ACAPY_ADMIN_API_KEY_NAME", "name") @mock.patch.object(settings, "ST_ACAPY_ADMIN_API_KEY", "key") -async def test_single_tenant_has_expected_headers(): +async def test_single_tenant_has_expected_headers_configured(): acapy = SingleTenantAcapy() headers = acapy.get_headers() assert headers == {"name": "key"} @@ -33,37 +33,27 @@ async def test_multi_tenant_get_headers_returns_bearer_token_auth(requests_mock) @pytest.mark.asyncio async def test_multi_tenant_get_wallet_token_returns_token_at_token_key(requests_mock): - requests_mock.post( - settings.ACAPY_ADMIN_URL + "/multitenancy/wallet/wallet_id/token", - headers={}, - json={"token": "token"}, - status_code=200, - ) - acapy = MultiTenantAcapy() - acapy.wallet_id = "wallet_id" - token = acapy.get_wallet_token() - assert token == "token" - + wallet_id = "wallet_id" + wallet_key = "wallet_key" -@pytest.mark.asyncio -async def test_multi_tenant_throws_assertion_error_for_non_200_response(requests_mock): - requests_mock.post( - settings.ACAPY_ADMIN_URL + "/multitenancy/wallet/wallet_id/token", - headers={}, - json={"token": "token"}, - status_code=400, - ) - acapy = MultiTenantAcapy() - acapy.wallet_id = "wallet_id" + with mock.patch.object( + settings, "MT_ACAPY_WALLET_ID", wallet_id + ), mock.patch.object(settings, "MT_ACAPY_WALLET_KEY", wallet_key): - # Clear cache to ensure test isolation - acapy.get_wallet_token.cache_clear() + requests_mock.post( + settings.ACAPY_ADMIN_URL + f"/multitenancy/wallet/{wallet_id}/token", + headers={}, + json={"token": "token"}, + status_code=200, + ) - # The implementation raises Exception, not AssertionError - with pytest.raises(Exception) as exc_info: - acapy.get_wallet_token() + acapy = MultiTenantAcapy() + acapy.wallet_id = wallet_id + acapy.wallet_key = wallet_key + acapy.get_wallet_token.cache_clear() - assert "400" in str(exc_info.value) + token = acapy.get_wallet_token() + assert token == "token" @pytest.mark.asyncio @@ -150,11 +140,14 @@ async def test_multi_tenant_get_wallet_token_no_auth_headers_when_not_configured @pytest.mark.asyncio -async def test_multi_tenant_throws_exception_for_non_200_response(requests_mock): +async def test_multi_tenant_throws_exception_for_401_unauthorized(requests_mock): wallet_id = "wallet_id" wallet_key = "wallet_key" - with mock.patch.object(settings, "MT_ACAPY_WALLET_ID", wallet_id): + with mock.patch.object( + settings, "MT_ACAPY_WALLET_ID", wallet_id + ), mock.patch.object(settings, "MT_ACAPY_WALLET_KEY", wallet_key): + acapy = MultiTenantAcapy() acapy.wallet_id = wallet_id acapy.wallet_key = wallet_key @@ -163,7 +156,7 @@ async def test_multi_tenant_throws_exception_for_non_200_response(requests_mock) requests_mock.post( settings.ACAPY_ADMIN_URL + f"/multitenancy/wallet/{wallet_id}/token", json={"error": "unauthorized"}, - status_code=401, # Use 401 for a more realistic test + status_code=401, ) # Check for generic Exception, as the code now raises Exception(f"{code}::{detail}")