feat: OAuth extra params with zero-config OAuth features #173
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
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]>
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]>
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.
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 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.
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
- 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]>
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 19964292268 --repo smart-mcp-proxy/mcpproxy-go
|
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
extra_paramsconfigurationStatePendingAuthstate for OAuth-required servers before authenticationFeatures
RFC 8707 Extra Params Support
extra_paramsfield in OAuth config for resource indicatorsOAuthTransportWrapperinjects params into authorization and token requestsZero-Config OAuth (cherry-picked from PR #165)
IsOAuthCapable()- Detects OAuth capability based on protocolStatePendingAuth- New connection state for deferred OAuthErrOAuthPending- Structured error for OAuth-pending flowsHasValidToken()- Token validation method for TokenStoreManagerUI Improvements
pending_authstatusTest plan
🤖 Generated with Claude Code