Repository navigation
fix(oauth): discovery overrides, offline_access and discovery cache (Spec 113-b) - #1499
Merged
Merged
Conversation
…g and discovery cache (Spec 113-b) Add per-server oauth.authorization_endpoint, token_endpoint, registration_endpoint and auth_server_metadata_url overrides (https or loopback http, write-time validation only). Endpoint overrides rewrite the authorization server metadata mcp-go reads, with a synthesized document when it is unreachable. extra_params injection and 201 normalization now match the effective token endpoint URL. offline_access is requested only for PRM-derived scopes when the AS advertises it; a missing refresh_token grant logs one warning per server. Preflight discovery (PRM, AS metadata, scopes) is cached per server URL and override set (1h success, 30s failure, LRU 256). The OAuth HTTP client refuses https->http and cross-origin POST redirects.
Invalidate in-flight discovery fetches, replay cached metadata failures, follow redirects when loading metadata through the wrapper, compare origins with effective ports, record the PRM URL in the POST preflight, keep a PRM-less fallback result only for the failure TTL, warn on an explicitly empty grant_types_supported, reject whitespace in overrides, and revert echoed masks of the override fields on write.
…ht result (Spec 113-b)
Deploying mcpproxy-docs with
|
| Latest commit: |
9b2c9e1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://73c12abc.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://113-b-oauth-discovery-overri.mcpproxy-docs.pages.dev |
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Contributor
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 37500057558 --repo smart-mcp-proxy/mcpproxy-go
|
Dumbris
enabled auto-merge (squash)
October 6, 2026 17:24
…-overrides # Conflicts: # oas/docs.go
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
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.
Problem
Slice 113-b of Spec 113 (gaps G3 and G5 from the Anthropic MCP-proxy talk audit):
offline_access, never warned when the authorization server cannot issue refresh tokens, and had no way to point a server at an authorization server whose metadata is non-standard or advertises an unusable endpoint.createOAuthConfigInternalcall re-fetched the same PRM and authorization server metadata, andextra_paramsinjection used a/token//authorizepath heuristic.What changed
oauth.authorization_endpoint,oauth.token_endpoint,oauth.registration_endpoint,oauth.auth_server_metadata_url(https, or http on a loopback host; no query, fragment, userinfo or whitespace). Validated at write time only (OAuthConfig.Validate,Config.ValidateDetailed); an invalid value already on disk is dropped at load with a field-name-only warning, so boot never fails.OAuthTransportWrapper(mcp-go has no endpoint config). With any endpoint override mcpproxy always passes mcp-go a metadata URL (override, else discovered, else RFC 8414 URL from the override origin), and when that metadata is unreachable and authorization + token overrides are set the wrapper serves a synthesized document, so mcp-go never falls back to/authorize,/token,/register.extra_paramsinjection and 201 to 200 normalization match on the effective (overridden or discovered) token endpoint URL; the path heuristic remains only when no endpoint is known.offline_accessis appended only when the scopes were taken from PRMscopes_supportedand the AS metadatascopes_supportedcontains it. Explicitoauth.scopesand AS-derived scopes are untouched.grant_types_supportedis present (including an empty list) and lacksrefresh_token.internal/oauth/discovery_cache.go): key = hash(server URL, four overrides, kind); 1 h success TTL, 30 s failure TTL, single-flight, LRU 256. Covers the HEAD/POST preflights, PRM fetches, the scope fallbacks and the AS metadata document (shared with the transport wrapper so mcp-go's own metadata GET is served from the cache). A 401 advertising a differentresource_metadataURL invalidates the server's entries, including in-flight fetches.OAuthConfigChanged,MergeOAuthConfig/copyOAuthConfig,ServerFieldMaskDecisionsrows, contracts OAuth projection + runtime/management plumbing,make swagger(oas/ committed;cmd/generate-typesproduced no TS diff),docs/configuration.md. Per-server fields, so noMCPPROXY_env override. Echoed masks of the four fields are reverted on write (UnmaskLiveOAuth), because the generic read view's name rule masks leaves containing "auth"/"token".refresh_manager.goandpersistent_token_store.goare untouched (owned by slice a).Tests run
OAuthHandler(authorization URL host equals override, exchange + refresh hit the override, advertised/std/*endpoints never hit),createOAuthConfigInternalintegration (5 calls = 1 request per discovery URL, separate keys per override set, failure cached briefly, offline_access table, refresh-grant warning, PRM-URL invalidation, unmask of echoed masks).go test -raceoninternal/config,internal/storage,internal/contracts,internal/management,cmd/generate-types,internal/oauth: pass.go test -race -tags server -skip <CI regex>on runtime, httpapi, upstream, config, oauth, server/tokens: pass.internal/server(same flags, 45m timeout) also passes; a first run with the 20m CI timeout hit the timeout on the loaded shared machine (load average about 9), unrelated to this change.go build ./cmd/mcpproxyandgo build -tags server -o /dev/null ./cmd/mcpproxy: pass..github/.golangci.yml, bare and--build-tags server: no findings in touched files (the remaining repo-wide findings are pre-existing Go 1.26 deprecation notices, e.g.ecdsa.PublicKey.X)../scripts/test-api-e2e.shon isolated ports (18177/18179/18180): the only consistent failure isAudit log: server-edition binary present, which fails identically onorigin/main(reproduced on a clean checkout at 18178). Two of the three runs also hit a launcher-test reconnect flake under machine load; the third run was green apart from the audit item.github-copilot/gpt-6.1-sol, 4 rounds (round 1 in three chunks, then three fix-and-re-review rounds). Round 1 found 9 issues, all fixed with tests (in-flight fetch repopulating after invalidation, negative cache bypass, redirected metadata, default-port origin match, POST preflight not recording the PRM URL, transient-PRM-failure fallback pinned for 1 h, emptygrant_types_supported, whitespace validation bypass, masked echo on write); rounds 2 and 3 found two more cache-seeding issues, fixed by storing seeds atomically with the primary result; round 4 verdict CLEAN.Assumptions
NotSecretinServerFieldMaskDecisions(typed projection shows them to the UI); the write door still reverts echoed masks from the generic view.internal/upstream/core/connection.gois not edited: the PRM-URL invalidation hook lives inoauth.handleUnauthorizedResponse(resource auto-detection already sees every live 401resource_metadata), which keeps the slice insideinternal/oauth.internal/upstream/core; it lives ininternal/oauth(oauth_overrides_e2e_test.go) because it only needs mcp-go's handler and the config this package builds.Spec: #1495
Review: opencode github-copilot/gpt-6.1-sol, 4 rounds, clean; follow-ups: after a same-origin 307/308 redirect of a token POST the learned-endpoint match no longer normalizes the 201 response