-
Notifications
You must be signed in to change notification settings - Fork 334
feat: Implement OAuth Dynamic Client Registration (DCR) and PKCE support #1158
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
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
595ae57
to
3315e4e
Compare
Add comprehensive test suite for OAuth Dynamic Client Registration and PKCE following Test-Driven Development (Red Phase). All tests will fail until implementation is complete. Tests Added: - test_oauth_manager_pkce.py (22 tests) * PKCE parameter generation (RFC 7636) * Authorization URL with code_challenge * State storage with code_verifier * Token exchange with PKCE validation * Security properties validation - test_dcr_service.py (35 tests) * AS metadata discovery (RFC 8414) * Client registration (RFC 7591) * Get-or-register pattern * Update/delete operations * Issuer validation and error handling - test_dcr_flow_integration.py (12 tests) * Complete PKCE flow with database * Complete DCR flow end-to-end * Security validations (replay prevention, expiry) * Error handling scenarios All tests use shared test_db fixture from tests/conftest.py. Related to #979 Test Status: 🔴 RED - Tests will fail until implementation
Implement OAuth Dynamic Client Registration and PKCE support following RFCs 7591, 7636, and 8414. This is the TDD Green Phase implementation. Test Results: 27/45 tests passing (60%) - PKCE tests: 17/22 passing (77%) - DCR tests: 10/23 passing (43%) Changes: 1. PKCE Support (RFC 7636): - Add code_verifier column to oauth_states table - Implement _generate_pkce_params() for PKCE generation - Implement _create_authorization_url_with_pkce() - Update _store_authorization_state() to store code_verifier - Implement _validate_and_retrieve_state() to return state data - Update _exchange_code_for_tokens() to include code_verifier - Wire PKCE through initiate/complete authorization flows 2. DCR Service (RFC 7591): - Create DcrService with AS metadata discovery (RFC 8414) - Implement register_client() for dynamic registration - Implement get_or_register_client() pattern - Implement update_client_registration() - Implement delete_client_registration() - Add metadata caching for performance - Add DcrError exception class 3. Database Models: - Add RegisteredOAuthClient model for DCR storage - Add code_verifier to OAuthState model - Add relationship to Gateway model 4. Configuration: - Add 10 DCR configuration settings - Add oauth_discovery_enabled setting - Add oauth_preferred_code_challenge_method setting 5. Database Migrations: - Migration 61ee11c482d6: Add code_verifier column - Migration 2f67b12600b4: Add registered_oauth_clients table Remaining work: - Fix mocking issues in some tests (module-level variable patches) - Add admin DCR router (Phase 1.4) - Update OAuth router integration - Documentation updates Related to #979
Fix failing unit tests by correcting mock setups: Test Results: 33/45 passing (73% - up from 60%) - PKCE tests: 22/22 passing (100%) ✅ - DCR tests: 11/23 passing (48%) Changes: - Fix _state_lock patching (module-level vs instance) - Fix aiohttp.ClientSession mocking for async context managers - Add token_storage mock for initiate_authorization_code_flow test - Fix issuer mismatch in discovery test mocks - Clear metadata cache in caching test All PKCE tests now passing. Remaining DCR test failures are due to mock setup complexity, not implementation bugs. Related to #979
Fix final test issues to achieve 100% test pass rate: Test Results: 45/45 passing (100%) ✅✅✅ - PKCE tests: 22/22 passing (100%) - DCR tests: 23/23 passing (100%) Changes: - Fix aiohttp.ClientSession mocking for discovery tests - Clear metadata cache in tests for isolation - Use unique gateway_id and issuer for each test (avoid UNIQUE constraints) - Add Gateway objects to database before RegisteredOAuthClient - Fix Gateway model attributes (use slug/url instead of server_url/command) - Properly encrypt registration_access_token in update tests - Update encrypted secret assertions (check length instead of prefix) All unit tests for PKCE and DCR now passing! Related to #979
Fix script hanging by: - Remove 'set -e' to allow test counting instead of immediate exit - Replace problematic heredocs with single-line Python commands - Suppress stderr warnings (security warnings from config) - Use grep for file content checks instead of Python heredocs Script now runs to completion and reports: - 5 tests passed (PKCE, DCR service, database, config) - 2 tests failed (admin router not yet implemented) - Warnings for documentation (Phase 1.4 tasks) Related to #979
- Auto-detect missing client_id when gateway has issuer - Auto-trigger DCR client registration when DCR is enabled - Store registered credentials and update gateway oauth_config - Add admin endpoints for viewing/managing registered OAuth clients - Integrate with existing PKCE implementation in OAuthManager Endpoints added: - GET /oauth/registered-clients - List all registered clients - GET /oauth/registered-clients/{gateway_id} - Get client for gateway - DELETE /oauth/registered-clients/{client_id} - Delete registered client Closes #979 Phase 1.4
When DCR automatically registers a client, we now also update the gateway's auth_type field to 'oauth' to ensure subsequent connections know to use OAuth authentication. This ensures the gateway initialization logic correctly skips immediate connection attempts for OAuth authorization code flows.
- Added 'OAuth 2.0' option to auth_type dropdown in admin UI - Auto-detect auth_type='oauth' when oauth_config is present - Applied to both create and edit gateway endpoints - Users no longer need to manually select OAuth auth type This fixes the issue where OAuth-protected MCP servers (like Reddit MCP) would fail to register because auth_type wasn't automatically set.
Backend changes (admin.py): - Collect individual OAuth fields from UI form - Assemble into oauth_config object - Support both JSON string (API) and form fields (UI) - Applied to both create and edit endpoints UI changes (admin.html): - Added oauth_issuer field (required for DCR) - Updated Client ID placeholder for DCR - Added help text explaining DCR auto-registration - Applied to both Add and Edit gateway forms Now users can configure OAuth gateways via UI with proper DCR support. If client_id is empty but issuer is provided, DCR will auto-register the client.
- Added blue help text to Client Secret field explaining DCR - Applied to both Add and Edit forms - Added debug logging to show complete oauth_config assembly - Made OAuth authorization code fields always visible This makes it clear to users that Client ID and Client Secret can be left empty when using DCR.
Added logging to see what oauth_config_json and individual OAuth fields are being received from the form submission.
The JavaScript in admin.js was intercepting the form submission and assembling oauth_config with incorrect field names (token_url instead of token_endpoint). This caused the backend to receive pre-assembled but incorrect OAuth configuration. Changes: - Commented out OAuth config assembly in handleAddGatewayFormSubmit - Commented out OAuth config assembly in handleEditGatewayFormSubmit - Individual OAuth form fields now pass through to backend - Backend (admin.py) correctly assembles with proper field names - Supports DCR when client_id/client_secret are empty This fixes the issue where grant_type was always 'client_credentials' instead of respecting the user's selection of 'authorization_code'.
Changed the default selection in the grant type dropdown from 'client_credentials' to 'authorization_code' since that's the most common use case for OAuth-protected MCP servers and the flow we're testing for DCR/PKCE. Users can still select Client Credentials if needed, but Authorization Code is now the default for both Add and Edit gateway forms.
Some OAuth servers (like systemprompt-mcp-server) return HTTP 200 OK instead of HTTP 201 Created for successful client registration, even though RFC 7591 specifies 201. Changed the DCR service to accept both 200 and 201 as successful responses to improve compatibility with real-world OAuth servers. This allows DCR to work with the Reddit MCP server (systemprompt) which registers public clients (no client_secret) and returns 200.
Two critical fixes for DCR with public clients: 1. Handle public clients without client_secret: - Check if client_secret_encrypted is None before decrypting - Only add client_secret to oauth_config if it exists - This supports PKCE-only flows (RFC 7636) 2. Fix OAuth field name inconsistency: - Changed 'token_endpoint' -> 'token_url' - Changed 'authorization_endpoint' -> 'authorization_url' - OAuthManager expects these field names - Applied to both admin.py (form assembly) and oauth_router.py (DCR) This allows DCR to work with public OAuth clients like the Reddit MCP server (systemprompt-mcp-server) which uses PKCE without secrets.
Implements Phase 1 of RFC 7591 (DCR) and RFC 7636 (PKCE) support for OAuth-protected MCP servers, as specified in issue #979. Core Implementation: - Added DcrService for AS metadata discovery (RFC 8414) and client registration (RFC 7591) - Implemented PKCE (Proof Key for Code Exchange) in OAuthManager for Authorization Code flows - Integrated DCR into oauth_router to auto-register when issuer present but client_id missing - Added RegisteredOAuthClient model to store DCR registrations with encrypted credentials - Added code_verifier field to OAuthState model for PKCE support - Created Alembic migrations for new database schema Configuration: - Added DCR settings: dcr_enabled, dcr_auto_register_on_missing_credentials, dcr_default_scopes - Added OAuth discovery settings: oauth_discovery_enabled, oauth_preferred_code_challenge_method - Added DCR security settings: dcr_allowed_issuers, dcr_token_endpoint_auth_method OAuth Enhancements: - Support for public OAuth clients (PKCE-only, no client_secret) - Accept both HTTP 200 and 201 for DCR registration responses - Fixed OAuth field name inconsistencies (authorization_url/token_url vs authorization_endpoint/token_endpoint) - Skip strict URL validation for OAuth-protected servers - Support both SSE and STREAMABLEHTTP transports for OAuth servers UI/UX Improvements: - Added OAuth 2.0 option to auth_type dropdown in admin UI - Added oauth_issuer field for DCR configuration - Made Authorization Code the default grant type - Added help text for Client ID and Client Secret fields explaining DCR - Backend now assembles oauth_config from individual form fields - Auto-detects auth_type="oauth" when OAuth config is present - Made OAuth authorization fields always visible when OAuth 2.0 is selected Testing: - 22 PKCE unit tests covering parameter generation, state storage, token exchange - 23 DCR unit tests covering AS discovery, client registration, error handling - 8 integration tests covering end-to-end DCR and PKCE flows - All 53 tests passing with proper database session isolation and aiohttp mocking - Imported OAuthState and RegisteredOAuthClient models in test conftest for schema creation Bug Fixes: - Fixed JavaScript in admin.js that was incorrectly assembling oauth_config - Handle missing client_secret for public clients in token exchange - Only decrypt client_secret if present (avoid NoneType errors) - Clear metadata cache in tests for proper isolation - Added debug logging for OAuth config assembly Validation: - Tested end-to-end with systemprompt-mcp-server (Reddit MCP) - Successfully completed DCR + PKCE + OAuth flow with real-world server - Verified token encryption/decryption works correctly - Confirmed PKCE code_challenge and code_verifier flow Closes #979 (Phase 1.4 - Integration into OAuth router)
Removed unused imports to fix ruff and flake8 linting errors: - timedelta (not used in the file) - Optional (not used in the file) All other imports (datetime, timezone, Any, Dict, List) are used.
Updated existing OAuth tests to match the new PKCE implementation: 1. test_initiate_authorization_code_flow_success: - Now expects PKCE parameters in authorization URL - Removed mocking of old _create_authorization_url method - Validates code_challenge and code_challenge_method presence 2. test_complete_authorization_code_flow_success: - Updated _exchange_code_for_tokens call to include code_verifier - Stores code_verifier with state for PKCE validation 3. test_complete_authorization_code_flow_no_token_storage: - Updated to use _validate_and_retrieve_state (returns state data) - Includes code_verifier in mocked state data - Updated _exchange_code_for_tokens assertion to include code_verifier 4. test_fetch_tools_after_oauth_success: - Updated to mock _connect_to_sse_server_without_validation - This is the new method used for OAuth servers (skips validation) These tests were written before PKCE was implemented and needed updates to match the new method signatures and flow.
Fixed TypeError in OAuth tests where HMAC signature generation was receiving a MagicMock instead of bytes for the secret key. Changes: - Wrapped test_initiate_authorization_code_flow_success with get_settings mock - Wrapped test_complete_authorization_code_flow_no_token_storage with get_settings mock - Both tests now provide SecretStr for auth_encryption_secret - HMAC signature generation uses proper bytes This fixes the CI/CD failures: - TypeError: key: expected bytes or bytearray, but got 'MagicMock'
- Reformat code with black and isort - Fix trailing whitespace - Optimize imports and line length
- Fix import-outside-toplevel warning in oauth_manager.py - Remove unnecessary else-after-return statements - Fix singleton comparison for is_active boolean - Add pylint disable comments for intentional patterns - Suppress fixme warning for documented TODO
…details OAuth Documentation (oauth.md): - Add comprehensive PKCE section explaining RFC 7636 implementation - Update FAQ to reflect that PKCE is now implemented and automatic - Document PKCE flow: code_verifier generation, challenge computation, state storage DCR Documentation (dcr.md): - Expand from sparse overview to comprehensive implementation guide - Add all environment variables with defaults and descriptions - Document AS metadata discovery (RFC 8414) with caching details - Add client registration flow with request/response examples - Include configuration examples for auto-registration, manual, and public clients - Document database schema for registered_oauth_clients and oauth_states tables - Add security features: issuer allowlist, encryption, PKCE integration, validation - Add troubleshooting section for common DCR issues - Add monitoring section with log queries and SQL examples
… migrations - Replace server_default='1' with sa.true() for PostgreSQL compatibility - Add table existence checks before ALTER operations - Handle fresh database installations gracefully - Follow existing migration patterns for conditional upgrades
38c2753
to
b65a387
Compare
- Add comprehensive DCR configuration section to README.md with all environment variables and documentation links - Add DCR and PKCE configuration variables to .env.example with detailed descriptions and security notes - Update OAuth section to highlight DCR and PKCE as key features - Include references to DCR configuration guide, OAuth integration docs, and HyperMCP tutorial - Add formatting fixes to oauth_manager.py (isort) Enhances documentation for RFC 7591 (DCR) and RFC 7636 (PKCE) implementation to help users configure OAuth-protected MCP servers. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Remove development debug console.log statements that were logging OAuth configuration including potentially sensitive data like client_id and issuer information to browser console. While client_id and issuer are not secrets, logging OAuth config violates defense-in-depth principles and could leak configuration details during development/debugging. Security Impact: Low (no secrets exposed, but reduces attack surface) Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
0e5de3f
to
1637971
Compare
The test_dcr_implementation.sh script is a legacy development artifact with incorrect expectations (looks for admin_dcr_router.py which doesn't exist, wrong migration file patterns, wrong documentation paths). The comprehensive pytest suite (53 DCR/PKCE tests, 3,304 total tests) provides complete validation. This script is redundant and misleading.
crivetimihai
approved these changes
Oct 4, 2025
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.
Overview
This PR implements OAuth 2.0 Dynamic Client Registration (RFC 7591) and Proof Key for Code Exchange (RFC 7636) support for MCP Gateway, enabling seamless integration with OAuth-protected MCP servers.
Closes #979
🎯 What's Implemented
RFC 7591 - Dynamic Client Registration (DCR)
RFC 7636 - Proof Key for Code Exchange (PKCE)
Phase 1.4 - Integration
oauth_router.py
(no new router needed)auth_type=oauth
for registered clients📊 Testing
All 53 tests passing (100%)
Manual validation completed with:
🔧 Configuration
New environment variables:
🎨 UI Enhancements
oauth_issuer
field for DCR configurationoauth_config
from individual form fieldsauth_type=oauth
when OAuth fields are present🗄️ Database Changes
New Tables:
registered_oauth_clients
- Stores DCR-registered client credentials2f67b12600b4_add_registered_oauth_clients_table_for_.py
Schema Updates:
code_verifier
column tooauth_states
table for PKCE61ee11c482d6_add_code_verifier_to_oauth_states_for_.py
🔒 Security Features
🚀 Usage Example
1. Register an OAuth-protected MCP server with DCR
2. Complete OAuth flow
📝 API Endpoints Added
GET /oauth/registered-clients
- List all DCR-registered clientsGET /oauth/registered-clients/{gateway_id}
- Get client for specific gatewayDELETE /oauth/registered-clients/{client_id}
- Delete registered client🐛 Bug Fixes
📚 Documentation
Test documentation files created:
test_dcr_implementation.sh
- Validation script✅ Checklist
🔍 Testing Instructions
📸 Screenshots
Manual testing successfully completed - DCR registered public client with systemprompt-mcp-server, OAuth flow completed with PKCE, tools fetched successfully.