POC: experimental Oauth support#4140
Open
timbl-ont wants to merge 13 commits into
Open
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Tim Bloomfield <tim.bloomfield@ontario.ca>
Signed-off-by: timbl-ont <tim.bloomfield@ontario.ca>
Signed-off-by: timbl-ont <tim.bloomfield@ontario.ca>
Signed-off-by: timbl-ont <tim.bloomfield@ontario.ca>
Signed-off-by: Tim Bloomfield <tim.bloomfield@ontario.ca>
Signed-off-by: timbl-ont <tim.bloomfield@ontario.ca>
Author
|
@swcurran can you approve the test workflows. Tests are passing on my local environment. |
Signed-off-by: timbl-ont <tim.bloomfield@ontario.ca>
Author
|
@jamshale @PatStLouis @weiiv if you have the time can you review this PR and provide some feedback on potential readiness to merge and if not what types of remediations are required. The tests are passing locally on my laptop but have not yet run on the latest code push. I see two possible paths:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a proof-of-concept OAuth2 “Resource Server” mode for ACA-Py’s Admin API, enabling external authorization servers (e.g., Keycloak) to issue bearer tokens that ACA-Py validates and authorizes via scopes, alongside a runnable demo environment and accompanying documentation.
Changes:
- Add OAuth token validation (JWKS JWT validation + RFC7662 introspection fallback) and wire it into admin request context setup.
- Extend admin/tenant auth decorators for scope-based authorization and add
require_scope+ auth-context helper utilities. - Add a Keycloak + Docker Compose demo (scripts, realm export) and documentation describing configuration, scopes, and request flow.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/features/OAuthResourceServer.md | Documents OAuth2 RS mode, scopes, configuration, and migration guidance. |
| demo/demo-authserver/scripts/test-oauth-scopes.sh | Adds an automated scope enforcement test script for the demo deployment. |
| demo/demo-authserver/scripts/start-acapy.sh | Starts ACA-Py in the demo container with OAuth RS settings enabled. |
| demo/demo-authserver/scripts/setup-tenant.sh | Provisions a demo sub-wallet and configures Keycloak wallet_id claim + scopes. |
| demo/demo-authserver/scripts/get-user-token.sh | Demonstrates auth-code + PKCE login flow and prints resulting token/claims. |
| demo/demo-authserver/scripts/get-tenant-token.sh | Retrieves tenant access tokens via client credentials for demo testing. |
| demo/demo-authserver/scripts/get-admin-token.sh | Retrieves admin access tokens via client credentials for demo testing. |
| demo/demo-authserver/README.md | Provides end-to-end instructions for running and using the OAuth demo. |
| demo/demo-authserver/oauth-scope-test-report.md | Example output report showing expected scope enforcement results. |
| demo/demo-authserver/keycloak/realm-export.json | Keycloak realm export defining demo clients, scopes, and mappers. |
| demo/demo-authserver/docker-compose.yml | Composes Keycloak + Postgres + ACA-Py into a runnable demo stack. |
| demo/demo-authserver/.gitignore | Ignores the demo .env file. |
| acapy_agent/wallet/routes.py | Adds require_scope to DID creation and uses auth-context helpers for subwallet detection. |
| acapy_agent/settings/routes.py | Hardens metadata access to tolerate missing context.metadata safely. |
| acapy_agent/config/argparse.py | Adds OAuth CLI flags and updates admin auth requirement rules for OAuth mode. |
| acapy_agent/admin/tests/test_auth.py | Adds unit tests for the new require_scope decorator behavior. |
| acapy_agent/admin/tests/test_auth_context.py | Adds tests for auth-context helper functions and settings fallback. |
| acapy_agent/admin/server.py | Integrates OAuth validation into request context setup and Swagger auth definitions. |
| acapy_agent/admin/oauth_validator.py | Implements OAuth token validation via JWKS and introspection fallback. |
| acapy_agent/admin/decorators/auth.py | Extends auth decorators to enforce OAuth scopes and adds require_scope. |
| acapy_agent/admin/auth_context.py | Introduces helpers to read scopes/subject/wallet_id from metadata/settings consistently. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+377
to
+381
| oauth_mode = bool( | ||
| getattr(args, "oauth_enabled", False) | ||
| or getattr(args, "oauth_jwks_uri", None) | ||
| or getattr(args, "oauth_introspection_endpoint", None) | ||
| ) |
Comment on lines
+355
to
+375
| if self.multitenant_manager: | ||
| wallet_id = claims.get("wallet_id") | ||
| if wallet_id: | ||
| try: | ||
| async with self.root_profile.session() as session: | ||
| wallet = await WalletRecord.retrieve_by_id( | ||
| session, wallet_id | ||
| ) | ||
| profile = await self.multitenant_manager.get_wallet_profile( | ||
| self.context, wallet | ||
| ) | ||
| context_wallet_id.set(wallet_id) | ||
| meta_data["wallet_id"] = wallet_id | ||
| request_settings[AUTH_WALLET_ID_SETTING] = wallet_id | ||
| except StorageNotFoundError: | ||
| raise web.HTTPUnauthorized( | ||
| reason=( | ||
| f"Wallet not found for wallet_id claim: {wallet_id}" | ||
| ) | ||
| ) | ||
|
|
Comment on lines
+114
to
+125
| async with aiohttp.ClientSession() as session: | ||
| resp = await session.post( | ||
| self.introspection_endpoint, | ||
| data={"token": token, "token_type_hint": "access_token"}, | ||
| auth=auth, | ||
| ) | ||
| if resp.status != 200: | ||
| raise web.HTTPUnauthorized( | ||
| reason=f"Token introspection returned HTTP {resp.status}" | ||
| ) | ||
| body = await resp.json() | ||
|
|
Comment on lines
+87
to
+99
| http_post_form() { | ||
| local url="$1" token="${2:-}" body="${3:-}" | ||
| if [[ -n "$token" ]]; then | ||
| curl -s -o /dev/null -w "%{http_code}" -X POST \ | ||
| -H "Authorization: Bearer $token" \ | ||
| -H "Content-Type: application/x-www-form-urlencoded" \ | ||
| "$url" | ||
| else | ||
| curl -s -o /dev/null -w "%{http_code}" -X POST \ | ||
| -H "Content-Type: application/x-www-form-urlencoded" \ | ||
| "$url" | ||
| fi | ||
| } |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: timbl-ont <163455524+timbl-ont@users.noreply.github.com>
Review feedback: - Fail fast in argparse when OAuth mode is enabled without a token validation method, and when introspection is configured without a client id. - Reject tenant tokens with no wallet_id claim in multitenant OAuth mode unless the token carries acapy:admin (prevents fall-through to the base wallet). - Harden RFC 7662 introspection: shared session with a configurable timeout, scoped response, and network errors surfaced as 503. - Only verify aud when an audience is configured (fixes rejection of tokens carrying an aud claim when none is expected). - Remove the unused body parameter from the demo test-oauth-scopes.sh http_post_form helper. Configurable AS HTTP timeout: - Add --oauth-http-timeout (default 10s) applied to both the introspection session and the PyJWKClient. - Run the blocking JWKS key fetch in an executor so a slow AS cannot stall the event loop. Security audit fixes: - Scope- and wallet-aware admin event websocket: an acapy:admin token receives all wallets' events, a tenant token only its own wallet's, and send_webhook filters delivery per connection (fixes cross-tenant leak). - Redact oauth.introspection_client_secret from GET /status/config. - Require --oauth-audience whenever --oauth-jwks-uri is set so JWT access tokens are bound to this resource server (fail closed). Adds unit tests for the validator, admin server OAuth middleware and websocket authorization, argparse validation, and auth decorators; updates the OAuthResourceServer docs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Tim Bloomfield <tim.bloomfield@ontario.ca>
- oauth_validator: use a dict literal instead of the dict() constructor (python:S7498). - Extract the OAuth scope check from tenant_authentication into _enforce_tenant_scope to reduce cognitive complexity (python:S3776). - Extract OAuth validation and settings mapping from AdminGroup.get_settings into _validate_admin_auth_args and _oauth_settings helpers, reducing its cognitive complexity and collapsing the nested if (S3776, S1066). - Redirect demo-script ERROR messages to stderr (shell S7677). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Tim Bloomfield <tim.bloomfield@ontario.ca>
Author
|
@jamshale Pushed fixes for the ChatGPT code review, sonar cloud issues (for those that made sense) as well as some fixes as a result of a security audit using Claude Fable. |
Author
Is there a way to dismiss this finding as it is a false positive? |
Contributor
|
The easy approach is that the technical reviewers approve the PR with a note saying that the SonorCloud fail is a false negative and we merge it regardless. For a little more work, someone looks into why SonarCloud is identifying the issue and update its config to ignore that (or that class of) error. I don't know the effort in that. AI could probably help in providing guidance on how to do that. I'm good with the reviewers noting that the its a false negative. |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This is a POC to demonstrate adding OAuth support to ACA-Py. The code was developed via AI and is not expected to be merged as is without appropriate developer input and testing. Contributed here as per discussions at the ACA-Py OWF project meeting today.
The code does the following:
One area of exploration may be to add an orthogonal set of scopes that represent granular roles with ACA-Py.
Type of Change
Checklist