Oauth SA#1218
Open
rboixaderg wants to merge 27 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new guillotina.contrib.oauth contrib package implementing a container-scoped OAuth 2.0 Authorization Code + PKCE (S256) authorization server profile, with PostgreSQL persistence and optional MCP integration for protected-resource metadata and audience binding.
Changes:
- Introduces OAuth endpoints (authorize/token/register/revoke/consents) plus RFC 8414 authorization-server metadata and RFC 9207
issresponse parameter. - Adds PostgreSQL-backed storage (clients, authorization codes, refresh tokens, consents) including periodic cleanup and refresh-token rotation/reuse defense.
- Integrates MCP with OAuth resource indicators and
WWW-Authenticatehints, and adds comprehensive test coverage + docs.
Reviewed changes
Copilot reviewed 52 out of 59 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| guillotina/tests/test_auth.py | Adds targeted unit tests for OAuth token signing/validation, issuer safety, resources, and rate limiting. |
| guillotina/tests/oauth/conftest.py | Provides shared OAuth/MCP test settings and helper functions for registration/authorization/token exchange. |
| guillotina/tests/oauth/test_mcp_oauth.py | Exercises MCP protected-resource metadata, challenges, and OAuth audience enforcement for MCP endpoints. |
| guillotina/tests/oauth/test_oauth_authorize.py | Covers /oauth/authorize behavior including PKCE enforcement, redirects, consent/login flows, and resource handling. |
| guillotina/tests/oauth/test_oauth_consents.py | Tests consent listing/revocation endpoints and consent TTL behavior. |
| guillotina/tests/oauth/test_oauth_metadata.py | Validates RFC 8414 metadata behavior and proxy-header handling options. |
| guillotina/tests/oauth/test_oauth_register.py | Tests dynamic registration validation, redirect URI rules, and rate limiting. |
| guillotina/tests/oauth/test_oauth_revoke.py | Tests refresh-token revocation behavior and revoke endpoint rate limiting. |
| guillotina/tests/oauth/test_oauth_storage_backend.py | Verifies storage interface compliance, schema properties, and store-access behavior without PostgreSQL. |
| guillotina/tests/oauth/test_oauth_store_contract.py | Contract tests for repository behavior (clients/codes/refresh/consents) and end-to-end flow with PostgreSQL storage. |
| guillotina/tests/oauth/test_oauth_token.py | Tests token endpoint behavior, rotation/reuse defense, PKCE validation, and token endpoint rate limiting. |
| guillotina/tests/oauth/test_oauth_validator.py | Confirms OAuth access tokens authenticate requests and audience mismatches fail as expected. |
| guillotina/tests/mcp/test_mcp.py | Ensures non-OAuth MCP deployments do not advertise OAuth challenge metadata. |
| guillotina/contrib/oauth/init.py | Declares default OAuth settings, registers services/utilities, and conditionally enables MCP integration. |
| guillotina/contrib/oauth/install.py | Adds addon install/uninstall hooks (including container data cleanup on uninstall). |
| guillotina/contrib/oauth/interfaces.py | Defines IOAuthStorageUtility interface. |
| guillotina/contrib/oauth/auth/init.py | Package marker for OAuth auth components. |
| guillotina/contrib/oauth/auth/validators.py | Implements OAuth JWT bearer validator (audience/issuer/scope checks + request.oauth attachment). |
| guillotina/contrib/oauth/api/init.py | Package marker for OAuth API components. |
| guillotina/contrib/oauth/api/request.py | Adds request parsing/helpers (duplicate param rejection, form parsing, client identifier, writable GET handling). |
| guillotina/contrib/oauth/api/services.py | Implements OAuth endpoints, well-known handlers, consent flows, rate limiting, and token issuance/rotation. |
| guillotina/contrib/oauth/api/urls.py | Implements issuer/container URL derivation, issuer validation, well-known URL builders, and resource validation. |
| guillotina/contrib/oauth/api/views.py | Renders login/consent/error HTML pages with CSP anti-framing headers. |
| guillotina/contrib/oauth/api/well_known.py | Implements RFC-style well-known routes that map root well-known paths to container-scoped issuers/resources. |
| guillotina/contrib/oauth/api/templates/base.html | Base HTML template for OAuth UI pages. |
| guillotina/contrib/oauth/api/templates/consent.html | Consent page template. |
| guillotina/contrib/oauth/api/templates/error.html | Error page template. |
| guillotina/contrib/oauth/api/templates/hidden_input.html | Template for rendering hidden form inputs. |
| guillotina/contrib/oauth/api/templates/list_item.html | Template for list item rendering. |
| guillotina/contrib/oauth/api/templates/login.html | Login form template. |
| guillotina/contrib/oauth/api/templates/oauth.css | Styling for OAuth HTML UI pages. |
| guillotina/contrib/oauth/api/templates/plain_item.html | Template for plain list items. |
| guillotina/contrib/oauth/api/templates/scope_item.html | Template for scope list items with descriptions. |
| guillotina/contrib/oauth/flow/init.py | Package marker for OAuth flow logic. |
| guillotina/contrib/oauth/flow/clients.py | Implements dynamic client validation/creation, redirect URI rules, and consent key derivation. |
| guillotina/contrib/oauth/flow/csrf.py | Implements signed CSRF tokens for consent decisions with TTL checking. |
| guillotina/contrib/oauth/flow/keys.py | Provides purpose-specific key derivation from jwt.secret for domain separation. |
| guillotina/contrib/oauth/flow/pkce.py | Implements PKCE verifier/challenge validation and S256 verification. |
| guillotina/contrib/oauth/flow/ratelimit.py | Adds best-effort sliding-window rate limiting with optional Redis backend. |
| guillotina/contrib/oauth/flow/resources.py | Implements extensible OAuth resource/audience resolvers (container default + optional MCP). |
| guillotina/contrib/oauth/flow/scopes.py | Defines default scope and configured supported scopes. |
| guillotina/contrib/oauth/flow/tokens.py | Issues JWT access tokens and provides opaque token generation + hashed-at-rest token hashing. |
| guillotina/contrib/oauth/integrations/init.py | Package marker for OAuth integrations. |
| guillotina/contrib/oauth/integrations/mcp.py | Registers MCP resource/audience resolvers and protected-resource well-known metadata + MCP auth policy. |
| guillotina/contrib/oauth/storage/init.py | Package marker for OAuth storage. |
| guillotina/contrib/oauth/storage/access.py | Resolves container DB key, addon installation status, and returns the container-scoped OAuth store. |
| guillotina/contrib/oauth/storage/interfaces.py | Defines the IOAuthStore interface contract. |
| guillotina/contrib/oauth/storage/utility.py | Initializes OAuth tables and runs periodic cleanup for PostgreSQL storage. |
| guillotina/contrib/oauth/storage/pg/init.py | Package marker for PostgreSQL OAuth storage. |
| guillotina/contrib/oauth/storage/pg/repository.py | Implements PostgreSQL OAuth repository (clients/codes/refresh/consents + cleanup function call). |
| guillotina/contrib/oauth/storage/pg/schema.py | Defines PostgreSQL DDL for OAuth tables/indexes and cleanup function. |
| guillotina/contrib/mcp/interfaces.py | Adds IMCPAuthPolicy interface to support optional auth challenge behavior. |
| guillotina/contrib/mcp/services.py | Adjusts MCP protocol endpoint to be Public and enforce auth/permission with optional OAuth challenge headers. |
| guillotina/auth/validators.py | Ensures generic JWT validators ignore OAuth access tokens and uses broader PyJWT exception handling. |
| docs/source/contrib/oauth.md | Adds end-user/operator documentation for configuration and OAuth flows (including MCP behavior). |
| docs/source/contrib/index.rst | Adds OAuth docs to contrib index. |
| docs/source/_static/oauth-flow.svg | Adds an OAuth/MCP flow diagram. |
| CHANGELOG.rst | Adds changelog entries describing OAuth functionality and security posture. |
| .gitignore | Adds /.venv to ignored paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Phases 0-9 implemented: OAuth contrib skeleton, container-scoped annotation storage (.oauth), metadata, dynamic registration, authorization/consent UI, token/refresh/revoke endpoints, OAuth JWT validator, MCP protected resource metadata/challenges, MCP scope-aware cache keys, docs and tests.\n\nValidation attempted:\n- .venv/bin/pytest guillotina/tests/test_login.py guillotina/tests/mcp/test_mcp.py -> failed: .venv/bin/pytest not found in worktree.\n- /Users/rogerboixaderguell/.pyenv/versions/Guillotina/bin/python -m pytest guillotina/tests/oauth/test_oauth_metadata.py -q -> failed during pytest startup: pytest_asyncio has no attribute fixture.\n- python3 -m compileall -q guillotina/contrib/oauth guillotina/contrib/mcp guillotina/tests/oauth -> passed.\n- git diff --check -> passed.
Continue OAuth/MCP phases after creating the worktree virtual environment.\n\nChanges:\n- Consolidate OAuth routes under oauth/{action} and .well-known/{action} to avoid Guillotina route-prefix conflicts.\n- Serve MCP protected resource metadata from the OAuth well-known dispatcher, preserving OAuth -> MCP independence by avoiding MCP imports.\n- Allow GET /oauth/authorize to write authorization codes after remembered consent.\n- Return redirects instead of raising them so authorization-code persistence commits.\n- Use UTC-safe JWT timestamps.\n- Make core JWT validators ignore PyJWT audience errors so OAuth audience rejection does not become a 500.\n- URL-encode OAuth test form bodies.\n- Ignore local .venv.\n\nValidation:\n- .venv/bin/pytest guillotina/tests/oauth -q -> 24 passed, 11 warnings.\n- .venv/bin/pytest guillotina/tests/mcp/test_mcp.py guillotina/tests/test_login.py -q -> 18 passed, 8 warnings.\n- .venv/bin/pytest guillotina/tests/oauth guillotina/tests/mcp/test_mcp.py guillotina/tests/test_login.py -q -> 42 passed, 19 warnings.
Finish remaining OAuth/MCP phase work.\n\nChanges:\n- Keep OAuth independent of MCP by making OAuth expose a generic well-known dispatcher and letting MCP register its protected-resource metadata provider.\n- Add MCP preflight OAuth scope checks for search and serialized content paths so insufficient scopes return HTTP 403.\n- Replay request bodies after preflight inspection so MCP streamable HTTP handling still receives the JSON-RPC payload.\n- Add validation coverage for expired authorization codes, expired refresh tokens, MCP insufficient search scope, and MCP insufficient content scope.\n- Black-format OAuth/MCP touched files.\n\nValidation:\n- .venv/bin/pytest guillotina/tests/oauth guillotina/tests/mcp/test_mcp.py guillotina/tests/test_login.py -q -> 46 passed, 24 warnings.\n- git diff --check -> passed.
- Add configurable consent_ttl (default 30 days, 0 disables expiry) with an expires_at column on oauth_consents, purged by oauth_cleanup_expired. - Expose GET/POST /oauth/consents for authenticated users to list and revoke their own grants (401 for anonymous, 404 for unknown consent_key). - Drop the stored consent on revocation (consent endpoint or refresh-token /oauth/revoke) and clear all refresh tokens for the (user, client) pair so a grant can no longer be re-issued silently. - Add IOAuthStore methods list_consents, delete_consent and revoke_user_client_refresh_tokens; renew expires_at on consent re-grant. - Add oauth consents tests and visual documentation (oauth-overview.html).
- Added validation for required parameters in OAuth token, authorization code, refresh token, and revoke endpoints, returning appropriate HTTPBadRequest responses for missing or invalid requests. - Improved handling of protected resource paths in MCP integration, ensuring correct URL generation and validation. - Updated tests to cover new validation scenarios and ensure compliance with the updated OAuth flow.
- Removed the outdated oauth-overview.html file to streamline documentation. - Updated oauth.md to reflect changes in OAuth 2.0 Authorization Code + PKCE configuration. - Improved request parameter handling in the OAuth API to preserve multiple resource parameters. - Enhanced validation for redirect URIs, including support for loopback and private-use native redirects. - Added tests for new functionality, ensuring compliance with updated OAuth flows and parameter handling.
dd225a4 to
598a2f2
Compare
…nd mcp integration
…and rename symbols to express intent
…user and improve anonymous user handling across OAuth endpoints
…odules, updating timestamp handling, and clarifying write permissions for GET requests
…cluding access token validation, audience resolution, and rate limiting
…andling, streamline validation processes, and enhance integration with MCP
bloodbare
approved these changes
Jun 20, 2026
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.
Summary
This PR adds
guillotina.contrib.oauth, a container-scoped OAuth authorization server profile for Guillotina and MCP clients.The implemented profile is intentionally narrow: OAuth 2.0 Authorization Code with mandatory PKCE (
S256) for public clients. Dynamic registration creates public clients only (token_endpoint_auth_method=none) and does not issue client secrets or support confidential-client authentication.Module organization
The OAuth contrib is organized into focused packages under
guillotina/contrib/oauth/:api/services.py), HTML page rendering (pages.py), and endpoint handlers (endpoints/{authorize,token,register,revoke,consents}).auth/discovery/flow/indicators/integrations/mcp/IMCPAuthPolicyforWWW-Authenticatechallenge headers.storage/IOAuthStoreinterface, container-scoped store access, PostgreSQL DDL and repository, and periodic cleanup.utils/OAuth / RFC coverage
Core OAuth profile
GET/POST /db/container/oauth/authorize), token (POST /db/container/oauth/token), registration, revocation, consent, and metadata endpoints.S256.Relevant RFC alignment:
S256enforcement.Dynamic client registration
POST /db/container/oauth/register.client_id; client-suppliedclient_idis rejected.Relevant RFC alignment:
invalid_redirect_uri,invalid_client_metadata, andunsupported_token_endpoint_auth_method.Authorization server metadata and issuer handling
GET /db/container/.well-known/oauth-authorization-serverGET /.well-known/oauth-authorization-server/db/containerfor issuers with path components./.well-known/openid-configuration, because this PR does not implement OpenID Connect.Relevant RFC alignment:
issparameter to mitigate authorization server mix-up.Resource indicators and MCP integration
resourceparameter and preserves repeatedresourceparameters on authorization requests.guillotina.contrib.mcpis enabled, registers MCP protocol resources and exposes protected-resource metadata.WWW-Authenticatemetadata hints for MCP protected resources.Relevant RFC alignment:
Revocation and consent management
POST /db/container/oauth/revokefor refresh-token revocation.GET /db/container/oauth/consentsPOST /db/container/oauth/consentsRelevant RFC alignment:
Explicitly not included
This PR does not implement:
id_token, UserInfo, OIDC discovery, OIDC JWKS, subject identifier types, claims negotiation).private_key_jwtclient authentication.Testing
Local validation run on this branch:
flake8 guillotina --config=setup.cfgisort --check-only guillotina/black --check --verbose guillotinamypy --config-file setup.cfg guillotina/DATABASE=postgresql pytest -rfE --reruns 2 guillotina/tests/oauthThe focused PostgreSQL OAuth test run passed with 92 tests.
Notes for reviewers
The main review focus should be the OAuth profile boundaries and security-sensitive behavior: