feat: Zero-Config OAuth with RFC 8707 support (complete) #165
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
Investigation findings: - OAuth provider requires RFC 8707 resource parameter - MCPProxy successfully detects OAuth and performs discovery - Authorization fails due to missing 'resource' query parameter - Root cause: mcp-go v0.42.0 lacks ExtraParams support Implementation plan structure: - Phase 0 (Priority): OAuth diagnostics and error reporting (2 days) * Fix auth status to show OAuth-configured servers * Parse OAuth error responses for actionable messages * Add OAuth diagnostics to doctor command - Phases 1-5: Config layer, wrapper pattern, integration, tests, docs - Phase 6: Upstream contribution to mcp-go Documentation includes: - runlayer-oauth-investigation.md: Comprehensive investigation findings - 2025-11-27-phase0-oauth-diagnostics.md: Detailed Phase 0 implementation - 2025-11-27-oauth-extra-params.md: Full 6-phase implementation plan - upstream-issue-draft.md: GitHub issue for mcp-go contribution All sensitive data has been sanitized and replaced with generic examples. Next step: Implement Phase 0 to provide OAuth diagnostics
… 1-3)
This commit implements the first batch of Phase 0 OAuth diagnostics
improvements to increase visibility into OAuth authentication failures.
Changes:
- Serialize OAuth configuration in Runtime.GetAllServers() to include
client_id, scopes, auth_url, and token_url in server status
- Extract OAuth config in ManagementService.ListServers() and populate
contracts.OAuthConfig struct for API responses
- Add last_error and authenticated fields to server status
- Implement parseOAuthError() to extract structured error information
from FastAPI validation errors and RFC 6749 OAuth responses
- Add OAuthParameterError type for missing/invalid OAuth parameters
Tests:
- Add unit test for OAuth config serialization in ListServers
- Add comprehensive unit tests for OAuth error parsing covering:
- FastAPI validation errors (Runlayer format)
- RFC 6749 OAuth error responses
- Multiple missing parameters
- Unknown formats and edge cases
- All tests pass
API Changes:
GET /api/v1/servers now returns:
{
"oauth": {
"client_id": "...",
"scopes": [],
"auth_url": "",
"token_url": ""
},
"last_error": "OAuth provider requires 'resource' parameter",
"authenticated": false
}
Related: docs/plans/2025-11-27-phase0-oauth-diagnostics.md
…hase 0 Tasks 4-5)
This commit completes Phase 0 OAuth diagnostics by adding user-facing
error visibility and actionable guidance for OAuth failures.
Changes:
Auth Status Command (auth_cmd.go):
- Add last_error display with clear status indicators
- Show ❌ Authentication Failed, ✅ Authenticated, or ⏳ Pending
- Detect parameter-related OAuth errors
- Provide contextual suggestions for missing parameters
- Link to RFC 8707 documentation and issue tracker
Doctor Command (doctor_cmd.go):
- Add new "OAuth Configuration Issues" section
- Display OAuth parameter mismatches with detailed context
- Show server name, issue type, error message, and impact
- Provide resolution steps and documentation links
Contracts (types.go):
- Add OAuthIssue struct for structured OAuth diagnostics
- Include fields: ServerName, Issue, Error, MissingParams, Resolution, DocumentationURL
- Update Diagnostics struct to include OAuthIssues array
- Update TotalIssues calculation to include OAuth issues
Management Service (diagnostics.go):
- Implement detectOAuthIssues() to identify parameter mismatches
- Add extractParameterName() helper to parse error messages
- Detect "requires 'X' parameter" pattern in OAuth errors
- Generate actionable diagnostics with RFC 8707 context
User Experience:
- Users now see clear error messages instead of generic failures
- Guidance points to upcoming extra_params support
- Links to RFC documentation for technical context
- Consistent messaging across auth status and doctor commands
Example Output:
Server: slack
Status: ❌ Authentication Failed
Error: OAuth provider requires 'resource' parameter
💡 Suggestion:
This OAuth provider requires additional parameters that
MCPProxy doesn't currently support...
Related: docs/plans/2025-11-27-phase0-oauth-diagnostics.md
…egration pending)
- Add TestE2E_ZeroConfigOAuth_ResourceParameterExtraction: validates metadata discovery and resource extraction - Add TestE2E_ManualExtraParamsOverride: validates manual extra_params preservation - Add TestE2E_IsOAuthCapable_ZeroConfig: validates OAuth capability detection - Add TestE2E_ProtectedResourceMetadataDiscovery: validates full metadata discovery flow All tests pass. Tasks 1-3 and 6 validated through E2E testing. Tasks 4-5 remain blocked pending mcp-go upstream support.
Complete speckit-style specification including: - User scenarios with acceptance criteria - Functional and non-functional requirements - Architecture and component interactions - Implementation status (7/9 tasks complete) - Testing coverage and success criteria - Migration path and dependencies - RFC compliance documentation References implementation in PR smart-mcp-proxy#165.
Moved implementation plan and PR drafts to .scratch/zero-config-oauth-pr165/ for better organization. These files are preserved locally but removed from version control as they are working documents for PR smart-mcp-proxy#165.
technicalpickles
changed the title
Contributor
Author
Implementation Update: Feature Complete via URL Injection WorkaroundThe zero-config OAuth implementation is now feature complete following the addition of a URL parameter injection workaround in commit c9df1cb. What Changed Since PR CreationOriginal approach:
Current implementation:
Code location: Technical ApproachWhen
This approach:
Testing StatusManual verification: Confirmed working with Runlayer-backed servers Action item: Review E2E tests to confirm they exercise the workaround, add specific test if needed. Known Issues (UX Polish)Two issues documented in PR body affect status display but not functionality:
Question: Should we address these UX issues in this PR or defer to followup? They predate this PR but are most noticeable with zero-config OAuth where users encounter OAuth more frequently. Upstream ContributionThe mcp-go library would benefit from native ExtraParams support, but it's not blocking for MCPProxy. We can contribute this enhancement as a followup if there's interest from mcp-go maintainers. Ready for review - the core feature is complete and working. Main review focus should be on the URL injection workaround approach and whether it introduces any edge cases. |
Adds comprehensive test coverage for the URL parameter injection workaround (connection.go:1846-1866) that enables RFC 8707 resource parameters without upstream mcp-go support. Test Coverage: - Auto-detected resource parameter injection from zero-config OAuth - Manual extra_params with multiple custom parameters - Empty extraParams does not modify URLs - Special characters are properly URL encoded All 4 test cases pass and verify the workaround correctly injects parameters into OAuth authorization URLs before browser launch. Resolves the E2E test coverage gap identified in PR review.
Introduces a new StatePendingAuth to distinguish OAuth-required servers from actual errors. This improves UX by clearly indicating when user action is needed for authentication. Changes: - Add StatePendingAuth to ConnectionState enum (types/types.go) - Create ErrOAuthPending error type for deferred OAuth (core/connection.go) - Update OAuth deferral to return ErrOAuthPending instead of generic error - Managed client recognizes ErrOAuthPending and transitions to StatePendingAuth Benefits: - OAuth servers no longer show confusing "Error" state during startup - Clear distinction between "needs user auth" vs "actual failure" - Foundation for improved CLI/tray UI indicators Related to PR review feedback on UX issues.
Adds comprehensive unit test coverage for the new OAuth pending state handling: oauth_error_test.go: - TestErrOAuthPending_Error - Tests error message formatting with/without custom message - TestIsOAuthPending - Tests helper function with ErrOAuthPending, regular errors, and nil - TestErrOAuthPending_AsError - Tests error interface and errors.As() compatibility types_test.go: - TestConnectionState_String - Tests all 8 connection states including StatePendingAuth - TestStateManager_TransitionTo_PendingAuth - Tests state transitions to/from PendingAuth - TestStateManager_PendingAuth_WithCallback - Tests callback invocation on transition - TestStateManager_GetConnectionInfo_PendingAuth - Tests connection info retrieval All tests passing, achieving full coverage for the PendingAuth state changes.
The supervisor was using hardcoded "connected", "connecting", "idle" states instead of the detailed ConnectionState strings from the managed client (e.g., "Pending Auth", "Authenticating", "Ready"). Changes: - internal/runtime/supervisor/supervisor.go:552-569: updateStateView() now uses ConnectionInfo.State.String() when available, falling back to simple boolean logic only when ConnectionInfo is nil or StateDisconnected - internal/runtime/supervisor/supervisor.go:754-764: updateSnapshotFromEvent() also uses ConnectionInfo.State.String() for consistent state display This ensures that OAuth-required servers show "Pending Auth" instead of "connecting" in the CLI output from 'mcpproxy upstream list' and in the tray UI. Fixes the "upstream list" CLI command display of PendingAuth state.
Modified internal/tray/managers.go to display ⏳ hourglass icon for servers in "Pending Auth" state, making it clear that OAuth authentication is required. Changes: - internal/tray/managers.go:669-672: Added case for "pending auth" status in getServerStatusDisplay() to show ⏳ icon instead of 🔴 error icon The OAuth login menu item (🔐 OAuth Login) is already shown for PendingAuth servers by the existing createServerActionSubmenus() logic (line 793), which creates OAuth menu items for all non-quarantined servers that support OAuth. This improves the user experience by: - Visually distinguishing auth-required servers from error states - Directing users to use the existing OAuth login action - Matching the "Pending Auth" status shown in CLI 'upstream list' output
Fixed the authenticated field bug in internal/runtime/runtime.go:1534 where it was hardcoded to false. Now properly checks if OAuth servers have valid, non-expired tokens. Changes: - internal/oauth/config.go:181-229: Added HasValidToken() method to TokenStoreManager - Checks if server has valid OAuth token that hasn't expired - Uses PersistentTokenStore.GetToken() to retrieve token from BBolt storage - Validates token expiration with grace period consideration - Returns true for non-OAuth servers (no auth required) - internal/runtime/runtime.go:1542-1576: Added isServerAuthenticated() helper method - Accepts serverName and hasOAuthConfig parameters - Returns true for non-OAuth servers (always "authenticated") - For OAuth servers, queries TokenStoreManager for valid token status - Passes storage manager for persistent token store access - internal/runtime/runtime.go:1517-1539: Updated GetAllServers() to use helper - Line 1517-1519: Check if server has OAuth config and call isServerAuthenticated() - Line 1538: Set authenticated field to actual token state instead of false - internal/runtime/runtime.go:21: Added oauth package import This ensures the "authenticated" field in API responses (used by CLI upstream list and tray UI) accurately reflects whether OAuth servers have valid tokens, not just whether they're connected.
Added TestE2E_OAuthServer_ShowsPendingAuthNotError to verify that OAuth-capable servers show 'Pending Auth' state instead of 'Error' when they defer OAuth authentication. This is part of the OAuth UX improvement initiative. The test structure is in place but needs refinement to properly integrate with the supervisor/StateView architecture. The core authenticated field functionality has been implemented and is working correctly (see previous commits). Test assertions verify: - Status is 'pending auth' (not 'error' or 'disconnected') - authenticated field is false (no token yet) - last_error is empty (OAuth deferral is not an error) - connected is false (waiting for OAuth) Related to PR smart-mcp-proxy#165 - Zero-config OAuth with RFC 8707 support
Added 7 unit tests covering HasValidToken() behavior in internal/oauth/config.go: - TestTokenStoreManager_HasValidToken_NoStore: Validates false when no token store exists - TestTokenStoreManager_HasValidToken_InMemoryStore: Validates true for in-memory stores (no expiration checking) - TestTokenStoreManager_HasValidToken_MockStore_NoToken: Tests mock behavior when GetToken() returns error - TestTokenStoreManager_HasValidToken_MockStore_ValidToken: Tests mock with valid token (expires in 1 hour) - TestTokenStoreManager_HasValidToken_MockStore_ExpiredToken: Tests mock with expired token (expired 1 hour ago) - TestTokenStoreManager_HasValidToken_PersistentStore_NoExpiration: Validates true for tokens with no expiration (zero time) - TestTokenStoreManager_HasValidToken_NilStorage: Validates graceful handling of nil storage parameter MockTokenStore implementation properly simulates client.TokenStore interface for testing. Tests document the type assertion behavior where non-PersistentTokenStore instances fall through to in-memory path (always return true). Part of PR smart-mcp-proxy#165 (zero-config OAuth with RFC 8707 support) - Task smart-mcp-proxy#14 completed.
Changed three debug logging statements in internal/upstream/core/connection.go from ERROR to DEBUG level to reduce log noise: 1. Line 985: "OAUTH AUTH FUNCTION CALLED - START" - Debug trace statement, not an error condition 2. Line 1019: "ABOUT TO CALL oauth.CreateOAuthConfig" - Debug trace statement for config creation 3. Line 1028: "oauth.CreateOAuthConfig RETURNED" - Debug trace statement for return values These are development/debug traces used to track OAuth flow execution. They should not use ERROR level as they don't represent error conditions. The actual OAuth deferral logging (lines 1172-1184) correctly uses INFO level: - "OAuth authorization required during MCP init - deferring OAuth" - "Deferring OAuth to prevent tray UI blocking" - "OAuth login available via system tray menu" This makes OAuth deferral less alarming in logs while keeping actual errors at ERROR level (failed client creation, failed authorization, etc.). Related to PR smart-mcp-proxy#165: Zero-config OAuth with RFC 8707 support Task: Update logs to use INFO level instead of ERROR for OAuth deferral
Enhanced docs/oauth-zero-config.md with detailed documentation for: 1. Server States Section: - Connected states: ready (authenticated), connected (no token) - Waiting states: pending_auth (normal waiting state, not an error) - Transitional states: connecting, authenticating - Error states: disconnected, error 2. Checking Authentication Status: - How to use `mcpproxy auth status` command - Example output with emoji indicators (✅⏳❌) - Status meaning explanations 3. Troubleshooting Section with 4 Common Issues: Issue smart-mcp-proxy#1: Server Shows "Pending Auth" State - Symptoms: ⏳ icon in tray, pending_auth status - Cause: OAuth detected but user hasn't authenticated - Solution: Use `auth login` or tray "Authenticate" action - Clarification: NOT an error, intentional deferral Issue smart-mcp-proxy#2: Authentication Token Expired - Symptoms: Was working, now shows "Auth Failed" - Cause: OAuth token expired (1-24 hour lifetime) - Solution: Re-authenticate with `auth login` Issue smart-mcp-proxy#3: OAuth Detection Not Working - Symptoms: No pending_auth, just connection failures - Diagnosis: Check doctor output, logs, manual OAuth test - Common causes: Non-standard endpoints, firewall issues - Solution: Add explicit OAuth configuration Issue smart-mcp-proxy#4: OAuth Login Opens Browser But Fails - Symptoms: Browser opens, approval given, but still fails - Diagnosis: Check callback logs for authorization code - Common causes: Port conflict, timeout, firewall - Solution: Retry with debug logging 4. Diagnostic Commands Reference: - doctor: Quick OAuth detection check - auth status: View token status - upstream list: Check connection status - upstream logs: View OAuth flow logs - auth login --log-level=debug: Test with debug output - upstream list --format=json | jq: Verify OAuth config This addresses user confusion about "Pending Auth" being displayed as an error state. Documentation now clearly explains it's a normal waiting state and provides step-by-step troubleshooting for all OAuth-related issues. Related to PR smart-mcp-proxy#165: Zero-config OAuth with RFC 8707 support Task: Update documentation for OAuth server states and troubleshooting
Added "Optional Followup Tasks" section to docs/plans/2025-11-27-oauth-extra-params.md documenting two potential future enhancements that are not required for core functionality. Task 1: System Notifications for OAuth Pending State - Status: Optional Enhancement, Priority: Low, Effort: 2-3 hours - Add desktop notifications when servers enter StatePendingAuth - Leverage existing NotificationManager infrastructure - Update StateChangeNotifier() to handle StatePendingAuth transitions - Why optional: UI feedback already good, notifications can be intrusive - Example notification UI mockup included Task 2: Upstream Contribution to mcp-go Library - Status: Optional Enhancement, Priority: Low, Effort: 8-12 hours - Contribute ExtraParams support natively to mcp-go library - Current URL injection workaround works well but upstream support cleaner - Benefits: cleaner code, helps other projects, better maintainability - Why optional: current workaround robust, requires upstream coordination - Steps for contribution outlined with timeline estimates Both tasks documented with: - Current implementation status - Infrastructure already in place - What would need to be added - Why they're optional (not required) - Implementation steps and effort estimates - Testing considerations This provides a clear roadmap for future enhancements while acknowledging that the current implementation meets all core requirements. Related to PR smart-mcp-proxy#165: Zero-config OAuth with RFC 8707 support
Contributor
|
Thanks for the contribution, @technicalpickles! I've also been working on OAuth improvements in #167, but I'll prioritize reviewing your PR first since it's smaller and more focused - which is generally a better |
This commit addresses UX issues identified during zero-config OAuth testing where OAuth-required servers were displaying confusing or alarming states instead of clear "authentication needed" feedback. ## Changes ### Dashboard.vue - Fixed "ErrorsUnknown" text concatenation by wrapping diagnostics in separate divs - Fixed backend field name mapping (server_name → server, error_message → message) - Added comprehensive diagnostics detail modal with OAuth login buttons - Added colored badges for different diagnostic types (red errors, yellow warnings) ### ServerCard.vue - Changed OAuth servers from red "Disconnected" to blue "Needs Auth" badge - Split error display: red alerts for real errors, blue info for OAuth required - Enhanced OAuth detection to include "deferred for tray UI" pattern - Added clear call-to-action messaging for authentication ### Tray managers.go - Added OAuth detection in getServerStatusDisplay() matching web UI logic - Changed tray icon from 🔴 red to 🔐 blue for OAuth servers - Ensures consistent status display between tray and web UI ### Documentation - Created docs/oauth-ui-feedback-fixes.md with comprehensive analysis - Documented all fixes, remaining issues, and future improvements - Included testing instructions and implementation details ## Remaining Issues (Documented) - OAuth login flow panic in GetAuthorizationURL (backend bug) - Transport error messages need better categorization and truncation Fixes smart-mcp-proxy#156 (if applicable - OAuth UI feedback)
…panic Addresses two pending issues from docs/oauth-ui-feedback-fixes.md: 1. Transport/Connection Error Display: - Added error categorization logic to ServerCard.vue - Shows user-friendly messages with icons (⏱️, 🔌, ⚙️,⚠️ ) - Extracts domains from URLs for cleaner display - Adds expandable details for long error messages - Special handling for transient states like 'client already connected' 2. OAuth Login Flow Backend Panic: - Enhanced OAuth handler nil checks in connection.go - Added pre-validation before GetAuthorizationURL calls - Improved panic recovery with helpful error messages - Better error context with hints for troubleshooting - Prevents silent failures during OAuth flows Files modified: - frontend/src/components/ServerCard.vue (error categorization) - internal/upstream/core/connection.go (defensive OAuth handling) - docs/oauth-ui-feedback-fixes.md (status update) All pending issues now resolved ✅
… Login in tray Addresses two UX improvement requests: 1. Surface OAuth-required servers in Dashboard diagnostics: - Enhanced diagnostics to detect zero-config OAuth servers - Detects OAuth requirements from error messages (not just explicit config) - Separates OAuth-required servers from general connection errors - Shows clear 'pending_auth' state with actionable messages - Supports both configured OAuth and auto-detected OAuth 2. Prioritize Login action in tray menu for unauthenticated servers: - Reordered tray menu to show 'Login (OAuth)' FIRST when auth is needed - Menu order when needs auth: Login → Enable/Disable → Quarantine - Menu order when authenticated: Enable/Disable → Login → Quarantine - Clear visual distinction with lock icon 🔐 - Login is now the default/primary action for unauthenticated servers Files modified: - internal/management/diagnostics.go (OAuth detection logic) - internal/tray/managers.go (menu ordering based on auth state) Benefits: - Users immediately see which servers need authentication on dashboard - Tray menu guides users to the correct action (Login first) - Zero-config OAuth servers properly detected and surfaced - Consistent UX across web UI, tray, and CLI
Added comprehensive unit tests to validate OAuth menu behavior: 1. TestServerSupportsOAuth - validates OAuth detection logic: - Explicit oauth config detection - Error message patterns (OAuth, 401, authorization, etc.) - 'deferred for tray UI' state detection - Known OAuth domains (sentry.dev, github.com, etc.) - HTTP/HTTPS server detection - Stdio server exclusion 2. TestOAuthMenuOrdering - validates menu item ordering: - OAuth Login appears first when server needs authentication - OAuth Login is secondary when server is authenticated - OAuth Login hidden when server is disabled or quarantined - Validates needsOAuth condition logic 3. Enhanced serverSupportsOAuth() to check: - Explicit OAuth config (oauth field) - Error messages indicating OAuth requirement - Zero-config OAuth detection from error patterns - Falls back to URL-based heuristics 4. Added debug logging to createServerActionSubmenus: - Logs all server state fields (enabled, authenticated, connected, etc.) - Logs OAuth detection results (supports_oauth, needs_oauth) - Helps troubleshoot menu creation issues Files modified: - internal/tray/managers.go (enhanced OAuth detection + debug logging) - internal/tray/oauth_menu_test.go (NEW - comprehensive test coverage) All tests pass ✅
When users click Login button in tray menu or web UI, the OAuth flow was being incorrectly deferred even though it was a manual trigger. Root cause: The isDeferOAuthForTray() function didn't check the context for the manualOAuthKey value that ForceOAuthFlow() sets to indicate manual vs automatic OAuth flows. Changes: - Modified isDeferOAuthForTray() to accept context.Context parameter - Added check for manualOAuthKey in context before deferring - Manual OAuth flows (Login button) now bypass deferral logic - Automatic OAuth flows during connection still defer correctly Testing: 1. Click Login in tray menu -> Browser should open 2. Click Login in web UI -> Browser should open 3. Automatic connection attempts -> Still defers correctly Fixes: OAuth login flow not triggering browser (issue smart-mcp-proxy#6)
Added comprehensive documentation for the OAuth Login button fix: - Explained root cause: isDeferOAuthForTray didn't check manual OAuth context - Documented solution: Check manualOAuthKey in context to bypass deferral - Included log evidence showing the deferral behavior - Added code snippets showing the implementation - Updated change history with Part 3 details Related to: OAuth login flow not triggering browser (issue smart-mcp-proxy#6)
…tatus Related smart-mcp-proxy#155 Updates zero-config OAuth specification to document expected user experience across all interfaces (web UI, CLI, system tray) and reflect current implementation status (91% complete). ## Changes - Add User Story 5: OAuth UX Across Interfaces with acceptance scenarios - Add "User Experience Specifications" section documenting: - Web control panel behavior (dashboard diagnostics, server cards) - CLI output formats (auth status, auth login, doctor) - System tray menu structure and OAuth flow triggers - Browser OAuth flow with success/failure callbacks - Consistent behavior guidelines across all interfaces - Update implementation status from 7/9 (78%) to 10/11 (91%) - Add completed tasks: Web UI UX improvements, Tray UX improvements, E2E tests - Add mandatory "Commit Message Conventions" section per spec template - Add "Input" field to header per spec template requirements ## Spec Conformance - Conforms to .specify/templates/spec-template.md structure - All mandatory sections present (User Scenarios, Requirements, Success Criteria) - UX specifications focus on observable behavior (WHAT), not implementation (HOW) - Template-compliant commit conventions documented
Contributor
Author
|
Thanks! I had some success today with adding several oauth servers with this. I went to add a new one, and started hitting some edge cases, and in trying to smooth them out broked them 🫠 In particular, actually triggering the oauth broke. I will need to go back and find a good check in point. Part of that edge cases was finding inconsistencies between web/tray/auth, and how needing to login is treated. Spent some time stepping back, and creating a spec with speckit to describe the behaviors I wanted to capture. It is starting to feel like feature creep, but I also don't have an oauth MCP to test with that didn't need the resource parameter |
Contributor
|
OAuth is one of the most complex pieces of logic, with many edge cases. I feel that unit tests don’t cover all, and manual testing is the bottleneck. My current idea is to implement solid end-to-end tests for OAuth first (in separate PR), without mocks and using the real code execution path. |
Contributor
|
I have implemented test mcp server with OAuth |
Contributor
Author
|
I'm not going to be able to get to this right away. If you land anything around this, please at-mention me and I can help test! |
Dumbris
pushed a commit
that referenced
this pull request
Dec 5, 2025
Added 7 unit tests covering HasValidToken() behavior in internal/oauth/config.go: - TestTokenStoreManager_HasValidToken_NoStore: Validates false when no token store exists - TestTokenStoreManager_HasValidToken_InMemoryStore: Validates true for in-memory stores (no expiration checking) - TestTokenStoreManager_HasValidToken_MockStore_NoToken: Tests mock behavior when GetToken() returns error - TestTokenStoreManager_HasValidToken_MockStore_ValidToken: Tests mock with valid token (expires in 1 hour) - TestTokenStoreManager_HasValidToken_MockStore_ExpiredToken: Tests mock with expired token (expired 1 hour ago) - TestTokenStoreManager_HasValidToken_PersistentStore_NoExpiration: Validates true for tokens with no expiration (zero time) - TestTokenStoreManager_HasValidToken_NilStorage: Validates graceful handling of nil storage parameter MockTokenStore implementation properly simulates client.TokenStore interface for testing. Tests document the type assertion behavior where non-PersistentTokenStore instances fall through to in-memory path (always return true). Part of PR #165 (zero-config OAuth with RFC 8707 support) - Task #14 completed.
Dumbris
added a commit
that referenced
this pull request
Dec 5, 2025
- Update OAuth test to match current CreateOAuthConfig signature - Add pending_auth and Error states to valid status list in E2E tests - Remove unused isServerAuthenticated helper from runtime The cherry-picked features from PR #165 include: - IsOAuthCapable() for protocol-based OAuth detection - StatePendingAuth connection state for deferred OAuth - HasValidToken() for token validation - Tray UI improvements with ⏳ icon for PendingAuth servers 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
6 tasks
Dumbris
added a commit
that referenced
this pull request
Dec 5, 2025
* feat: OAuth extra params support and token status display fixes
This commit implements OAuth extra parameters (RFC 8707) support and fixes
OAuth token status display in CLI commands.
Key changes:
- Add extra OAuth params (audience, resource, tenant) to authorization URLs
- Implement automatic OAuth state clearing when config changes
- Fix OAuth token lookup key mismatch (was using server name, now uses
server_name + URL hash to match PersistentTokenStore format)
- Add GetOAuthToken() and ClearOAuthState() methods to storage manager
- Add OAuthConfigChanged() comparison function for hot-reload detection
- Display OAuth token expiration time in `upstream list` and `auth status`
The token lookup fix resolves an issue where OAuth tokens were being stored
with composite keys (e.g., "sentry_abc123") but looked up with just the
server name ("sentry"), causing authenticated servers to show as pending.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
* fix: OAuth token status display for autodiscovery servers
Previously, OAuth token lookup only ran for servers with explicit
oauth config. Servers using OAuth autodiscovery (dynamic client
registration) showed "-" in OAUTH TOKEN column and didn't appear
in `auth status` output, even when they had valid tokens stored.
Changes:
- Move OAuth token lookup outside the `if serverStatus.Config.OAuth != nil`
block so it runs for ALL servers with a URL
- For autodiscovery servers (no explicit OAuth config), create minimal
oauthConfig with `autodiscovery: true` flag when token is found
- Update auth_cmd.go to detect autodiscovery flag and display
"OAuth: Discovered via Dynamic Client Registration" message
Now autorag-cf and other autodiscovery servers correctly show:
- Token expiration time in upstream list (e.g., "49m")
- Full auth status with "Authenticated & Connected"
- Token expiration details
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
* Fix e2e tests
* feat: add IsOAuthCapable for zero-config OAuth capability detection
* feat: add PendingAuth state for OAuth-required servers
Introduces a new StatePendingAuth to distinguish OAuth-required servers
from actual errors. This improves UX by clearly indicating when user action
is needed for authentication.
Changes:
- Add StatePendingAuth to ConnectionState enum (types/types.go)
- Create ErrOAuthPending error type for deferred OAuth (core/connection.go)
- Update OAuth deferral to return ErrOAuthPending instead of generic error
- Managed client recognizes ErrOAuthPending and transitions to StatePendingAuth
Benefits:
- OAuth servers no longer show confusing "Error" state during startup
- Clear distinction between "needs user auth" vs "actual failure"
- Foundation for improved CLI/tray UI indicators
Related to PR review feedback on UX issues.
* test: add unit tests for PendingAuth state and ErrOAuthPending
Adds comprehensive unit test coverage for the new OAuth pending state handling:
oauth_error_test.go:
- TestErrOAuthPending_Error - Tests error message formatting with/without custom message
- TestIsOAuthPending - Tests helper function with ErrOAuthPending, regular errors, and nil
- TestErrOAuthPending_AsError - Tests error interface and errors.As() compatibility
types_test.go:
- TestConnectionState_String - Tests all 8 connection states including StatePendingAuth
- TestStateManager_TransitionTo_PendingAuth - Tests state transitions to/from PendingAuth
- TestStateManager_PendingAuth_WithCallback - Tests callback invocation on transition
- TestStateManager_GetConnectionInfo_PendingAuth - Tests connection info retrieval
All tests passing, achieving full coverage for the PendingAuth state changes.
* fix: populate authenticated field with actual OAuth token state
Fixed the authenticated field bug in internal/runtime/runtime.go:1534 where it was hardcoded to false. Now properly checks if OAuth servers have valid, non-expired tokens.
Changes:
- internal/oauth/config.go:181-229: Added HasValidToken() method to TokenStoreManager
- Checks if server has valid OAuth token that hasn't expired
- Uses PersistentTokenStore.GetToken() to retrieve token from BBolt storage
- Validates token expiration with grace period consideration
- Returns true for non-OAuth servers (no auth required)
- internal/runtime/runtime.go:1542-1576: Added isServerAuthenticated() helper method
- Accepts serverName and hasOAuthConfig parameters
- Returns true for non-OAuth servers (always "authenticated")
- For OAuth servers, queries TokenStoreManager for valid token status
- Passes storage manager for persistent token store access
- internal/runtime/runtime.go:1517-1539: Updated GetAllServers() to use helper
- Line 1517-1519: Check if server has OAuth config and call isServerAuthenticated()
- Line 1538: Set authenticated field to actual token state instead of false
- internal/runtime/runtime.go:21: Added oauth package import
This ensures the "authenticated" field in API responses (used by CLI upstream list and tray UI) accurately reflects whether OAuth servers have valid tokens, not just whether they're connected.
* test: add comprehensive unit tests for OAuth HasValidToken() method
Added 7 unit tests covering HasValidToken() behavior in internal/oauth/config.go:
- TestTokenStoreManager_HasValidToken_NoStore: Validates false when no token store exists
- TestTokenStoreManager_HasValidToken_InMemoryStore: Validates true for in-memory stores (no expiration checking)
- TestTokenStoreManager_HasValidToken_MockStore_NoToken: Tests mock behavior when GetToken() returns error
- TestTokenStoreManager_HasValidToken_MockStore_ValidToken: Tests mock with valid token (expires in 1 hour)
- TestTokenStoreManager_HasValidToken_MockStore_ExpiredToken: Tests mock with expired token (expired 1 hour ago)
- TestTokenStoreManager_HasValidToken_PersistentStore_NoExpiration: Validates true for tokens with no expiration (zero time)
- TestTokenStoreManager_HasValidToken_NilStorage: Validates graceful handling of nil storage parameter
MockTokenStore implementation properly simulates client.TokenStore interface for testing.
Tests document the type assertion behavior where non-PersistentTokenStore instances fall through to in-memory path (always return true).
Part of PR #165 (zero-config OAuth with RFC 8707 support) - Task #14 completed.
* fix: use detailed connection state for CLI/UI status display
The supervisor was using hardcoded "connected", "connecting", "idle" states
instead of the detailed ConnectionState strings from the managed client
(e.g., "Pending Auth", "Authenticating", "Ready").
Changes:
- internal/runtime/supervisor/supervisor.go:552-569: updateStateView() now
uses ConnectionInfo.State.String() when available, falling back to simple
boolean logic only when ConnectionInfo is nil or StateDisconnected
- internal/runtime/supervisor/supervisor.go:754-764: updateSnapshotFromEvent()
also uses ConnectionInfo.State.String() for consistent state display
This ensures that OAuth-required servers show "Pending Auth" instead of
"connecting" in the CLI output from 'mcpproxy upstream list' and in the
tray UI.
Fixes the "upstream list" CLI command display of PendingAuth state.
* feat: add ⏳ icon for PendingAuth servers in tray UI
Modified internal/tray/managers.go to display ⏳ hourglass icon for servers
in "Pending Auth" state, making it clear that OAuth authentication is required.
Changes:
- internal/tray/managers.go:669-672: Added case for "pending auth" status in
getServerStatusDisplay() to show ⏳ icon instead of 🔴 error icon
The OAuth login menu item (🔐 OAuth Login) is already shown for PendingAuth
servers by the existing createServerActionSubmenus() logic (line 793), which
creates OAuth menu items for all non-quarantined servers that support OAuth.
This improves the user experience by:
- Visually distinguishing auth-required servers from error states
- Directing users to use the existing OAuth login action
- Matching the "Pending Auth" status shown in CLI 'upstream list' output
* fix: integrate cherry-picked zero-config OAuth features
- Update OAuth test to match current CreateOAuthConfig signature
- Add pending_auth and Error states to valid status list in E2E tests
- Remove unused isServerAuthenticated helper from runtime
The cherry-picked features from PR #165 include:
- IsOAuthCapable() for protocol-based OAuth detection
- StatePendingAuth connection state for deferred OAuth
- HasValidToken() for token validation
- Tray UI improvements with ⏳ icon for PendingAuth servers
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
---------
Co-authored-by: Claude <[email protected]>
Co-authored-by: Josh Nichols <[email protected]>
Contributor
|
@technicalpickles Could you try the new release v0.10.10 ? Unfortunately, I don't have access to Runlayer server, so I have tested it with Sentry (https://mcp.sentry.dev/mcp) and Cloudflare (https://logs.mcp.cloudflare.com/mcp) MCP servers. |
technicalpickles
added a commit
to technicalpickles/mcpproxy-go
that referenced
this pull request
Dec 10, 2025
Related smart-mcp-proxy#165 Add comprehensive specification and implementation plan for automatic detection of the RFC 8707 resource parameter in OAuth flows. This enables zero-config OAuth for providers like Runlayer that require the resource parameter. Artifacts: - spec.md: Feature specification with user stories and requirements - plan.md: Implementation plan with technical context - research.md: Technical research and implementation approach - data-model.md: Entity definitions and state transitions - quickstart.md: Usage guide and troubleshooting - checklists/requirements.md: Specification quality checklist Key Decisions: - Extract resource from RFC 9728 Protected Resource Metadata - Fallback to server URL if metadata unavailable - Inject params into auth URL after mcp-go constructs it - Manual extra_params override auto-detected values
13 tasks
Contributor
Author
|
Starting fresh in #188 using what I learned here, and pulling in changes from main. |
technicalpickles
added a commit
to technicalpickles/mcpproxy-go
that referenced
this pull request
Dec 10, 2025
Related smart-mcp-proxy#165 Add structured task list with 39 tasks organized by user story: - Phase 1: Setup (1 task) - Phase 2: Foundational - discovery layer (5 tasks) - Phase 3: US1 - Zero-config auto-detection (15 tasks) - MVP - Phase 4: US2 - Manual override (6 tasks) - Phase 5: US3 - Token injection (4 tasks) - Phase 6: US4 - Diagnostic visibility (3 tasks) - Phase 7: Polish (5 tasks) Tasks follow TDD approach per constitution requirements.
technicalpickles
added a commit
to technicalpickles/mcpproxy-go
that referenced
this pull request
Dec 10, 2025
…ection Related smart-mcp-proxy#165 Add new function that returns the full RFC 9728 Protected Resource Metadata struct including the 'resource' field needed for RFC 8707 compliance. Refactor DiscoverScopesFromProtectedResource to delegate to new function. Changes: - Add DiscoverProtectedResourceMetadata() returning *ProtectedResourceMetadata - Refactor DiscoverScopesFromProtectedResource() as wrapper for backward compat - Add comprehensive unit tests for new function Testing: - All TestDiscoverProtectedResourceMetadata_* tests pass - Existing DiscoverScopesFromProtectedResource tests still pass
technicalpickles
added a commit
to technicalpickles/mcpproxy-go
that referenced
this pull request
Dec 10, 2025
Add automatic detection of RFC 8707 resource parameter from Protected Resource Metadata (RFC 9728). This enables zero-config OAuth with providers like Runlayer that require the resource parameter. Key changes: - Add CreateOAuthConfigWithExtraParams() that returns both OAuth config and auto-detected extra params including resource - Add autoDetectResource() helper that: - Makes preflight HEAD request to get WWW-Authenticate header - Extracts resource_metadata URL - Fetches Protected Resource Metadata - Uses metadata.resource or falls back to server URL - Update handleOAuthAuthorization() to accept extraParams and inject them into authorization URL - Update all 6 call sites to use new function and pass extraParams Tests: - TestCreateOAuthConfig_AutoDetectsResource: verifies resource extraction - TestCreateOAuthConfig_FallsBackToServerURL: verifies fallback behavior - E2E tests in e2e_oauth_zero_config_test.go Part of smart-mcp-proxy#165 (RFC 8707 resource auto-detection for zero-config OAuth)
Dumbris
pushed a commit
that referenced
this pull request
Dec 11, 2025
* Create spec * docs: add RFC 8707 resource auto-detection specification and plan Related #165 Add comprehensive specification and implementation plan for automatic detection of the RFC 8707 resource parameter in OAuth flows. This enables zero-config OAuth for providers like Runlayer that require the resource parameter. Artifacts: - spec.md: Feature specification with user stories and requirements - plan.md: Implementation plan with technical context - research.md: Technical research and implementation approach - data-model.md: Entity definitions and state transitions - quickstart.md: Usage guide and troubleshooting - checklists/requirements.md: Specification quality checklist Key Decisions: - Extract resource from RFC 9728 Protected Resource Metadata - Fallback to server URL if metadata unavailable - Inject params into auth URL after mcp-go constructs it - Manual extra_params override auto-detected values * docs: add implementation tasks for RFC 8707 resource auto-detection Related #165 Add structured task list with 39 tasks organized by user story: - Phase 1: Setup (1 task) - Phase 2: Foundational - discovery layer (5 tasks) - Phase 3: US1 - Zero-config auto-detection (15 tasks) - MVP - Phase 4: US2 - Manual override (6 tasks) - Phase 5: US3 - Token injection (4 tasks) - Phase 6: US4 - Diagnostic visibility (3 tasks) - Phase 7: Polish (5 tasks) Tasks follow TDD approach per constitution requirements. * feat: add DiscoverProtectedResourceMetadata for RFC 8707 resource detection Related #165 Add new function that returns the full RFC 9728 Protected Resource Metadata struct including the 'resource' field needed for RFC 8707 compliance. Refactor DiscoverScopesFromProtectedResource to delegate to new function. Changes: - Add DiscoverProtectedResourceMetadata() returning *ProtectedResourceMetadata - Refactor DiscoverScopesFromProtectedResource() as wrapper for backward compat - Add comprehensive unit tests for new function Testing: - All TestDiscoverProtectedResourceMetadata_* tests pass - Existing DiscoverScopesFromProtectedResource tests still pass * feat(oauth): implement RFC 8707 resource auto-detection (User Story 1) Add automatic detection of RFC 8707 resource parameter from Protected Resource Metadata (RFC 9728). This enables zero-config OAuth with providers like Runlayer that require the resource parameter. Key changes: - Add CreateOAuthConfigWithExtraParams() that returns both OAuth config and auto-detected extra params including resource - Add autoDetectResource() helper that: - Makes preflight HEAD request to get WWW-Authenticate header - Extracts resource_metadata URL - Fetches Protected Resource Metadata - Uses metadata.resource or falls back to server URL - Update handleOAuthAuthorization() to accept extraParams and inject them into authorization URL - Update all 6 call sites to use new function and pass extraParams Tests: - TestCreateOAuthConfig_AutoDetectsResource: verifies resource extraction - TestCreateOAuthConfig_FallsBackToServerURL: verifies fallback behavior - E2E tests in e2e_oauth_zero_config_test.go Part of #165 (RFC 8707 resource auto-detection for zero-config OAuth) * feat(oauth): add tests for manual extra_params override (US2) Tests verify that: - T022: Manual extra_params.resource overrides auto-detected value - T023: Manual extra_params are preserved while resource is auto-detected Implementation was already complete from US1: - T024-T026: Merge logic and logging in CreateOAuthConfigWithExtraParams All tests pass: go test ./internal/oauth/... -v -run TestCreateOAuthConfig * feat(oauth): pass auto-detected extraParams to transport wrapper (US3) Enable auto-detected resource parameter injection into token requests: - T029: OAuthTransportWrapper.injectFormParams() handles token exchange/refresh - T030: createOAuthConfigInternal() accepts extraParams for wrapper injection - T031: Existing TestInjectFormParams_TokenRequest covers token request injection Key changes: - CreateOAuthConfig() now delegates to createOAuthConfigInternal() - CreateOAuthConfigWithExtraParams() passes auto-detected params to internal fn - Transport wrapper uses passed extraParams instead of re-reading from config This ensures zero-config OAuth flows inject resource into all OAuth requests: - Authorization URL (via handleOAuthAuthorization) - Token exchange (via transport wrapper) - Token refresh (via transport wrapper) * feat(oauth): add RFC 8707 resource visibility to diagnostics (US4) * fix(oauth): use POST for resource auto-detection per MCP spec MCP spec only requires POST support for the main endpoint. Use POST directly for the preflight request to get WWW-Authenticate header with resource_metadata URL. Updated all tests to use POST method in mock handlers. * fix(oauth): address code review feedback - add timeout, clarify comments, clean tests * docs: add zero-config vs explicit OAuth examples, improve auth status output
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
Implements zero-configuration OAuth for MCPProxy, enabling automatic detection and configuration of OAuth-protected MCP servers without manual setup. Fully supports RFC 8707 resource parameters via URL injection workaround.
Status: Feature Complete ✅ (pending additional E2E test verification and UX polish)
What Works Now
Users can configure OAuth-protected MCP servers with just a URL:
{ "name": "slack", "url": "https://oauth.example.com/mcp" }MCPProxy automatically:
Confirmed working with Runlayer-backed MCP servers (Slack, etc).
Key Implementation: URL Parameter Injection Workaround
The core achievement is a workaround in
internal/upstream/core/connection.go:1846-1866that injects extra OAuth parameters (like RFC 8707resource) directly into the authorization URL:This unblocks zero-config OAuth without waiting for upstream mcp-go enhancements.
Implementation Details
Core Features Implemented
Enhanced Metadata Discovery (
internal/oauth/discovery.go)resource,scopes_supported,authorization_serversfieldsExtraParams Config Field (
internal/config/config.go)extra_paramsmap to OAuthConfig for custom parametersResource Parameter Extraction (
internal/oauth/config.go)CreateOAuthConfig()now returns(*client.OAuthConfig, map[string]string)resourceparameter from Protected Resource Metadataextra_paramsoverride auto-detected valuesOAuth Capability Detection (
internal/oauth/config.go)IsOAuthCapable()identifies servers that support zero-config OAuthauth statusanddoctorcommandsEnhanced Diagnostics (Phase 0)
/api/v1/serversresponsesauth statusshows last error and OAuth detailsdoctorcommand detects OAuth parameter mismatchesTask Completion Status
✅ Phase 0: OAuth Diagnostics (Complete)
auth statuscommand with error displaydoctorcommand✅ Phases 1-5: Zero-Config Implementation (Complete)
9/9 core tasks completed (100%)
Known Issues
These UX issues exist but don't block functionality. Servers work correctly; these affect status display only:
1. OAuth Pending State Shows as Error
Impact: OAuth-required servers show "Error" state during startup instead of "Pending Auth"
Current behavior:
$ mcpproxy upstream list slack yes streamable-http no 0 error # Shows as errorExpected behavior:
$ mcpproxy upstream list slack yes streamable-http no 0 pending-auth # Should show pendingFix: Add
PendingAuthstate separate fromErrorin state machineWorkaround: Servers still work correctly once OAuth completes; status just initially confusing
2.
authenticatedField Bug in API ResponsesImpact:
auth statusshows "⏳ Pending" even after successful authenticationCurrent behavior:
$ mcpproxy auth status Server: slack Status: ⏳ Pending Authentication # Wrong - should be ✅ AuthenticatedBut:
$ mcpproxy upstream list slack yes streamable-http yes 14 connected # Correctly shows connectedRoot cause:
internal/runtime/runtime.go:1534hardcodesauthenticated: falseinstead of checking token stateFix: Populate
authenticatedfield from OAuth token managerWorkaround: Use
upstream listto verify OAuth success; servers work correctlyTesting
E2E Tests:
internal/server/e2e_oauth_zero_config_test.goNote: Additional E2E verification needed to confirm URL injection workaround is covered by tests.
Manual Testing:
Files Changed
Core Implementation:
internal/oauth/discovery.go- Enhanced metadata discoveryinternal/oauth/config.go- Resource extraction and capability detectioninternal/config/config.go- ExtraParams fieldinternal/config/validation.go- Reserved parameter protectioninternal/upstream/core/connection.go- URL parameter injection workaround ⭐Testing:
internal/server/e2e_oauth_zero_config_test.go- E2E test suiteinternal/oauth/discovery_test.go- Metadata discovery testsinternal/config/validation_test.go- Parameter validation testsDocumentation:
docs/oauth-zero-config.md- User guideREADME.md- Zero-Config OAuth sectionspecs/006-zero-config-oauth/spec.md- Complete specificationMigration Notes
CreateOAuthConfig()signature changed but all internal callers updated:No external API changes.
Next Steps
References