Fix: add auth headers and body to multitenancy token request

[MonolithicMonk]

Description:

• Reference: Closes ACA-Py Multitenancy Token Request Fails with 401 Unauthorized #895
• Summary:
  • Modified MultiTenantAcapy.get_wallet_token to include Admin API key headers when configured.
  • Added wallet_key to the request body payload as required by the endpoint.
  • Refactored SingleTenantAcapy.get_headers to return an empty dict instead of None values when keys are missing.
  • Updated MultiTenantAcapy to raise Exception instead of AssertionError for better error handling.
  • Updated tests to reflect new implementation logic and error handling.
• Impact: Fixes 401 Unauthorized errors when the controller attempts to get a wallet token from a secured ACA-Py agent instance.

[coveralls]

Pull Request Test Coverage Report for Build 19694902885

Details

• 24 of 24 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.08%) to 92.476%

---

Totals
Change from base Build 19682393107:	0.08%
Covered Lines:	1610
Relevant Lines:	1741

---

💛 - Coveralls

[loneil]

rebased

[Copilot]

Pull request overview

This PR fixes a bug where the OIDC controller failed to authenticate with secured ACA-Py instances when requesting wallet tokens. The fix adds proper authentication headers and the required wallet_key parameter to the multitenancy token request endpoint.

Key Changes:

• Modified MultiTenantAcapy.get_wallet_token to include Admin API key headers when configured and add wallet_key to the request body
• Refactored error handling from assert statements to explicit Exception raising with improved logging
• Updated SingleTenantAcapy.get_headers to return empty dict instead of dict with None values when API key is not configured

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.

File	Description
oidc-controller/api/core/acapy/config.py	Added authentication header logic and request body payload to get_wallet_token, improved error handling with explicit exceptions, and modified SingleTenantAcapy.get_headers to handle missing credentials gracefully
oidc-controller/api/core/acapy/tests/test_config.py	Added comprehensive tests for authenticated and unauthenticated scenarios, updated existing error test to expect Exception instead of AssertionError, and added test for SingleTenantAcapy with missing API key

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This test should also mock MT_ACAPY_WALLET_KEY for consistency and test isolation, similar to the other tests in this file (lines 80-88 and 121-129). Without mocking this setting, the test relies on the actual environment configuration or default value, which could lead to inconsistent test behavior across different environments.

[Copilot]

This test doesn't set wallet_key or mock MT_ACAPY_WALLET_KEY, but the implementation now includes wallet_key in the request body (config.py line 42). For better test isolation and consistency with other tests in this file (e.g., lines 70-110, 113-150), consider mocking MT_ACAPY_WALLET_KEY and explicitly setting acapy.wallet_key.

[Copilot]

The try-except block is redundant. The exception raised at line 58 is immediately caught at line 63, logged, and re-raised. This adds no value since:

1. The exception message is already logged at line 54-56 before raising
2. The except block at line 63-65 logs the same exception again with str(e) which produces the same message
3. The exception is simply re-raised without any transformation

Consider removing the try-except wrapper entirely, or only catch specific exceptions (like requests.RequestException, json.JSONDecodeError) if you need to handle network/parsing errors differently.

Suggested change

	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
	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"]

[loneil]

Tested out locally with our usual single tenant/no-key setup and that's still working for me. Some minor Copilot suggestions that could be looked at. Didn't try out any mult-tenant setup but implementation makes sense to me

[esune]

Tested locally, everything works as expected on a multi-tenant instance of ACA-Py. I think addressing the couple comments from Copilot is beneficial, then we can merge.