feat(oauth): extract Protected Resource Metadata URL from WWW-Authenticate - #808
Closed
Dennisadira wants to merge 5 commits into
Closed
feat(oauth): extract Protected Resource Metadata URL from WWW-Authenticate#808Dennisadira wants to merge 5 commits into
Dennisadira wants to merge 5 commits into
Conversation
…icate Per RFC 9728 §5.1, a resource server indicates its Protected Resource Metadata URL to clients by returning a 401 with a Bearer challenge whose resource_metadata parameter carries the URL. The transport layer now parses that header on every 401 and stores the advertised URL on the OAuthHandler; metadata discovery then prefers it over the origin-based /.well-known/oauth-protected-resource construction. This is the remaining piece of the OAuth RFC compliance work tracked in mark3labs#697 after mark3labs#761 and mark3labs#775 landed the path-preservation fixes, and closes the specific ask in mark3labs#688. Backward compatible: when the server does not send a resource_metadata parameter, the handler falls back to the existing well-known URL construction. - OAuthHandler: add ProtectedResourceMetadataURL field and SetProtectedResourceMetadataURL / ProtectedResourceMetadataURL / HandleUnauthorizedResponse methods - oauth.go: extractResourceMetadataURL helper (RFC 9110 §5.6.2 token + quoted-string parser, case-insensitive parameter names) - sse.go, streamable_http.go: call HandleUnauthorizedResponse at all five 401 sites so the PRM URL is captured wherever it's advertised Closes mark3labs#688
|
Connected to Huly®: MCP_G-374 |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughOAuthHandler now captures RFC 9728 Protected Resource Metadata (PRM) URLs from WWW-Authenticate on 401 responses, exposes setter/getter and HandleUnauthorizedResponse, and prefers/validates the stored PRM during server metadata discovery. SSE and StreamableHTTP call the handler on 401. Tests added. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
client/transport/oauth_test.go (1)
1759-1832: Parser test coverage is thorough.Good mix of happy-path (quoted, unquoted, case-insensitive, multi-param), malformed (missing
=, truncated quote), and adversarial (substring inside another quoted value, first-occurrence preference) cases.One optional addition to consider: an assertion that a pathological non-quoted URL value is truncated at the first non-token char (e.g.
resource_metadata=https://example.com/prmreturns"https"), which is arguably surprising but matches RFC 9110 §5.6.2 tokens. Documenting this via a test would lock in the expected behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/transport/oauth_test.go` around lines 1759 - 1832, Add a test case to TestExtractResourceMetadataURL that documents the current RFC-9110 token behavior for unquoted non-token characters: when extractResourceMetadataURL sees a non-quoted URL like `resource_metadata=https://example.com/prm` it should return the token up to the first non-token char (e.g. "https"), so add a case (name something like "non-quoted URL truncated at first non-token char") with header `Bearer resource_metadata=https://example.com/prm` and want `"https"` to lock in and document this behavior in the test for the extractResourceMetadataURL function.client/transport/oauth.go (2)
422-433: Ordering caveat:metadataOncemay have already fired before any 401 arrives.
getServerMetadatais guarded bysync.Once, so the preference for the advertised PRM URL only takes effect if the first call togetServerMetadatahappens after a 401 has populated it. In the common "401 → authorize flow" path this is fine, but if discovery is triggered earlier (e.g. during a proactiverefreshTokenon startup that fails for reasons other than 401, or via a directGetAuthorizationURLcall), the origin-based PRM URL is cached for the handler's lifetime and a subsequently advertised PRM URL is silently ignored.No action needed if the intended contract is "PRM URL applies only to the first discovery attempt" — worth a sentence in the GoDoc for
HandleUnauthorizedResponse/SetProtectedResourceMetadataURLso callers aren't surprised.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/transport/oauth.go` around lines 422 - 433, The current use of sync.Once (metadataOnce) in getServerMetadata causes an advertised Protected Resource Metadata (PRM) URL from HandleUnauthorizedResponse to be ignored if discovery ran earlier; update the logic so that an advertised PRM URL can override a previously cached origin-built URL: either (a) stop caching the PRM URL permanently in getServerMetadata and store only non-advertised defaults unless an advertised value is later set, or (b) make metadataOnce resettable/conditional so that SetProtectedResourceMetadataURL / HandleUnauthorizedResponse can replace the previously cached value and trigger a fresh metadata fetch; specifically modify getServerMetadata, metadataOnce usage, and ProtectedResourceMetadataURL()/SetProtectedResourceMetadataURL() to prefer an advertised URL when present even after an initial discovery, and add a GoDoc sentence to HandleUnauthorizedResponse / SetProtectedResourceMetadataURL documenting this override behavior.
565-647: Parser LGTM — one small edge case on truncated\tail.The RFC 9110 §5.6.2 token classification and quoted-string handling (with backslash unescape and case-insensitive parameter matching) look correct, and the table-driven tests cover the important forms well.
Minor: in
parseAuthParamValue, if the header ends with a lone trailing backslash inside an unterminated quoted string (e.g.resource_metadata="abc\), the\is written verbatim into the returned value because the guardi+1 < len(s)fails and the byte is then emitted via the fallthroughb.WriteByte(c). Low impact (only affects malformed headers), but dropping the trailing stray\is probably closer to intent.♻️ Optional tweak
if c == '\\' && i+1 < len(s) { b.WriteByte(s[i+1]) i += 2 continue } + if c == '\\' { + // Stray trailing backslash in a truncated quoted string — drop it. + return b.String(), i + 1 + } if c == '"' {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/transport/oauth.go` around lines 565 - 647, In parseAuthParamValue, when scanning a quoted-string in function parseAuthParamValue, the current logic emits a lone trailing backslash if the input ends with `\` (e.g. resource_metadata="abc\"); change the quoted-string loop to treat a backslash with no following character as an unterminated escape and drop the trailing `\` instead of writing it: detect when c == '\\' and i+1 >= len(s) and simply break/return the accumulated b.String() (or advance to end without appending the backslash), ensuring the returned value omits the stray backslash while preserving existing behavior for valid escapes and closing quote handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/transport/oauth.go`:
- Around line 346-358: HandleUnauthorizedResponse only checks the first
WWW-Authenticate header via resp.Header.Get which can miss a later Bearer
challenge containing resource_metadata; change the implementation to iterate all
header values using resp.Header.Values("WWW-Authenticate"), call
extractResourceMetadataURL on each value, and call
h.SetProtectedResourceMetadataURL(u) with the first non-empty u returned (no-op
if none found), preserving the nil resp early return and existing helper names
(HandleUnauthorizedResponse, extractResourceMetadataURL,
SetProtectedResourceMetadataURL).
- Around line 346-358: Handle the attacker-controlled WWW-Authenticate PRM URL
and enforce RFC 9728 resource equality: in SetProtectedResourceMetadataURL
validate the parsed URL from extractResourceMetadataURL to ensure scheme ==
"https" and that host (and optionally port) matches h.baseURL's host before
storing it; in getServerMetadata, after decoding protectedResource.Resource,
verify it exactly equals h.baseURL (or its normalized form) and return an error
or ignore the fetched metadata if it does not match, rather than using the
unvalidated value.
---
Nitpick comments:
In `@client/transport/oauth_test.go`:
- Around line 1759-1832: Add a test case to TestExtractResourceMetadataURL that
documents the current RFC-9110 token behavior for unquoted non-token characters:
when extractResourceMetadataURL sees a non-quoted URL like
`resource_metadata=https://example.com/prm` it should return the token up to the
first non-token char (e.g. "https"), so add a case (name something like
"non-quoted URL truncated at first non-token char") with header `Bearer
resource_metadata=https://example.com/prm` and want `"https"` to lock in and
document this behavior in the test for the extractResourceMetadataURL function.
In `@client/transport/oauth.go`:
- Around line 422-433: The current use of sync.Once (metadataOnce) in
getServerMetadata causes an advertised Protected Resource Metadata (PRM) URL
from HandleUnauthorizedResponse to be ignored if discovery ran earlier; update
the logic so that an advertised PRM URL can override a previously cached
origin-built URL: either (a) stop caching the PRM URL permanently in
getServerMetadata and store only non-advertised defaults unless an advertised
value is later set, or (b) make metadataOnce resettable/conditional so that
SetProtectedResourceMetadataURL / HandleUnauthorizedResponse can replace the
previously cached value and trigger a fresh metadata fetch; specifically modify
getServerMetadata, metadataOnce usage, and
ProtectedResourceMetadataURL()/SetProtectedResourceMetadataURL() to prefer an
advertised URL when present even after an initial discovery, and add a GoDoc
sentence to HandleUnauthorizedResponse / SetProtectedResourceMetadataURL
documenting this override behavior.
- Around line 565-647: In parseAuthParamValue, when scanning a quoted-string in
function parseAuthParamValue, the current logic emits a lone trailing backslash
if the input ends with `\` (e.g. resource_metadata="abc\"); change the
quoted-string loop to treat a backslash with no following character as an
unterminated escape and drop the trailing `\` instead of writing it: detect when
c == '\\' and i+1 >= len(s) and simply break/return the accumulated b.String()
(or advance to end without appending the backslash), ensuring the returned value
omits the stray backslash while preserving existing behavior for valid escapes
and closing quote handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3b49171b-6417-4ac0-8fe7-65c657f491bf
📒 Files selected for processing (6)
client/transport/oauth.goclient/transport/oauth_test.goclient/transport/sse.goclient/transport/sse_oauth_test.goclient/transport/streamable_http.goclient/transport/streamable_http_oauth_test.go
- Iterate every WWW-Authenticate header value (Values, not Get) so a Bearer challenge carrying resource_metadata is found even when a Basic challenge is emitted first (RFC 9110 §11.6.1 allows multiple WWW-Authenticate lines). - Reject advertised PRM URLs whose scheme or host does not match the configured base URL. Prevents a compromised resource from redirecting clients to an attacker's metadata endpoint. - Enforce RFC 9728 §3.3/§7.3 resource-identifier equality on metadata fetched via advertised PRM URLs: if the response declares a `resource` that does not match the base URL, the metadata MUST NOT be used. Scoped to the advertised path so existing origin-based discovery behaviour is unchanged. - parseAuthParamValue now drops a lone trailing backslash in a truncated quoted-string instead of emitting it verbatim. - Document the metadataOnce caveat on SetProtectedResourceMetadataURL: values set after the first metadata discovery are not applied retroactively. Tests cover multi-header iteration, cross-host/scheme-downgrade/ unparseable rejection, resource-identifier mismatch (rejected) and match (accepted), and the resource-identifier normalizer.
Author
|
Addressed the review feedback in d12db0c:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
client/transport/oauth.go (1)
535-551:⚠️ Potential issue | 🟠 MajorReject advertised metadata unless
resourceis present and exactly matches.Line 544 skips validation when
resourceis empty, so an advertised PRM response can omit the requiredresourcefield and still driveauthorization_servers. Also,resourceIdentifiersEqualrelaxes equality, but RFC 9728 requires the returned value to be identical before using the metadata. See RFC 9728 §2, §3.3, and §6: https://www.rfc-editor.org/rfc/rfc9728.html#section-3.3🛡️ Proposed fix
- if prmFromAdvertisement && protectedResource.Resource != "" && - !resourceIdentifiersEqual(protectedResource.Resource, baseURL) { + if prmFromAdvertisement && !resourceIdentifiersEqual(protectedResource.Resource, baseURL) { h.metadataFetchErr = fmt.Errorf( "advertised protected resource metadata declares resource %q which does not match base URL %q", protectedResource.Resource, baseURL, ) return }func resourceIdentifiersEqual(a, b string) bool { - ua, errA := url.Parse(a) - ub, errB := url.Parse(b) - if errA != nil || errB != nil { - return a == b - } - if !strings.EqualFold(ua.Scheme, ub.Scheme) { - return false - } - if !strings.EqualFold(ua.Host, ub.Host) { - return false - } - if strings.TrimSuffix(ua.Path, "/") != strings.TrimSuffix(ub.Path, "/") { - return false - } - if ua.RawQuery != ub.RawQuery { - return false - } - if ua.Fragment != ub.Fragment { - return false - } - return ua.User.String() == ub.User.String() + return a == b }RFC 9728 Section 2 Section 3.3 Section 6 protected resource metadata resource value identical comparisonAlso applies to: 716-744
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/transport/oauth.go` around lines 535 - 551, The advertised PRM metadata is currently accepted when protectedResource.Resource is empty and uses resourceIdentifiersEqual (a relaxed comparison); change the logic so that when prmFromAdvertisement is true you reject the metadata unless protectedResource.Resource is non-empty and exactly equals the expected baseURL (use strict string equality instead of resourceIdentifiersEqual), setting h.metadataFetchErr with a clear message (as done currently) and returning; apply the same strict check wherever resourceIdentifiersEqual is used for advertised metadata validation (e.g., the other block around lines 716-744) so authorization_servers cannot be driven by missing or non-identical resource fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/transport/oauth.go`:
- Around line 370-379: The loop in HandleUnauthorizedResponse currently uses
extractResourceMetadataURL which returns only the first resource_metadata from a
single WWW-Authenticate header value and can miss other candidates in the same
header; change extractResourceMetadataURL to collect and return all
resource_metadata values (e.g., extractResourceMetadataURLs -> []string) by
reusing the current scanner but appending every non-empty match, then update the
caller loop to iterate over each candidate returned and run
h.validateAdvertisedPRMURL(candidate) for each, calling
h.SetProtectedResourceMetadataURL(candidate) and returning on the first valid
one.
---
Duplicate comments:
In `@client/transport/oauth.go`:
- Around line 535-551: The advertised PRM metadata is currently accepted when
protectedResource.Resource is empty and uses resourceIdentifiersEqual (a relaxed
comparison); change the logic so that when prmFromAdvertisement is true you
reject the metadata unless protectedResource.Resource is non-empty and exactly
equals the expected baseURL (use strict string equality instead of
resourceIdentifiersEqual), setting h.metadataFetchErr with a clear message (as
done currently) and returning; apply the same strict check wherever
resourceIdentifiersEqual is used for advertised metadata validation (e.g., the
other block around lines 716-744) so authorization_servers cannot be driven by
missing or non-identical resource fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ca910242-2cb6-4ffd-8eca-2b858b010ed4
📒 Files selected for processing (4)
client/transport/oauth.goclient/transport/oauth_test.goclient/transport/sse_oauth_test.goclient/transport/streamable_http_oauth_test.go
✅ Files skipped from review due to trivial changes (2)
- client/transport/oauth_test.go
- client/transport/streamable_http_oauth_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- client/transport/sse_oauth_test.go
…urce field Two follow-up refinements from review: - Parser now returns every resource_metadata value it sees (plural extractResourceMetadataURLs) instead of only the first one. A single WWW-Authenticate header value can legitimately carry several Bearer challenges each with its own resource_metadata; HandleUnauthorizedResponse iterates all candidates and stores the first one that passes origin validation. Previously, if the first candidate was an attacker-controlled cross-origin URL, later valid candidates in the same header value were missed. - Advertised PRM responses must now declare a non-empty resource field. Because the advertised PRM URL need not share an origin with the protected resource, omitting the resource field leaves no way to bind the metadata to the resource the client addressed. Origin-constructed well-known discovery remains unaffected (the origin binding is implicit there). - resourceIdentifiersEqual retains RFC 3986 case-insensitive scheme/host matching and trailing-slash tolerance. Strict string equality would produce false positives on legitimate OAuth deployments that emit the same resource with and without a trailing slash. Rationale documented on the helper's godoc. Tests updated for the new signature and cover: multiple candidates per header value (bad first, good second), advertised PRM with missing resource (rejected), and empty-header / no-match edge cases.
Author
|
Addressed the second round of feedback in 6d104d5:
On the strict-equality suggestion: keeping the normalized |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/transport/oauth.go`:
- Around line 649-728: The quoted-string parser parseAuthParamValue currently
returns a partial value on unterminated quotes or a trailing backslash; change
it to treat these cases as invalid by returning an empty string and advancing
the index to the end of the input (i.e., return "", len(s)) so
extractResourceMetadataURLs will ignore malformed parameters; update
parseAuthParamValue's two error branches (lone trailing backslash and
end-of-string without closing '"') to discard the accumulated builder and return
"" along with the appropriate next index instead of b.String().
- Around line 730-761: The path comparison in resourceIdentifiersEqual currently
uses ua.Path which is decoded by url.Parse and thus conflates percent-encoded
segments (e.g., %2F) with their decoded form; update the function to compare
ua.EscapedPath() and ub.EscapedPath() (after the same TrimSuffix("/")) so the
percent-encoded form is preserved when checking equality, and add unit tests
exercising reserved-character encodings (e.g., "https://example.com/a%2Fb" vs
"https://example.com/a/b") to prevent regressions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b83618c7-5dd8-4094-b9ae-3c891f956e39
📒 Files selected for processing (2)
client/transport/oauth.goclient/transport/oauth_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- client/transport/oauth_test.go
Two follow-ups from review: - parseAuthParamValue now returns an `ok` flag and treats unterminated quoted strings or lone trailing backslashes as malformed. The caller skips such entries so a truncated header can no longer set PRM state, consistent with HandleUnauthorizedResponse's documented behaviour. Token values yield ok=true only when non-empty. - resourceIdentifiersEqual now compares EscapedPath rather than Path so percent-encoded reserved characters remain distinct from their decoded forms. url.Parse decodes %2F to `/`, which would otherwise allow `https://example.com/a%2Fb` and `https://example.com/a/b` to compare equal and bypass RFC 9728 §3.3 resource binding. Tests cover the rejection paths and the %2F-vs-/ distinction.
Author
|
Addressed both items in 25ec92d:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/transport/oauth.go`:
- Around line 392-410: In validateAdvertisedPRMURL, reject non-absolute URLs by
verifying both the parsed base (from h.baseURL) and the parsed candidate have
non-empty Scheme and Host before doing Equality checks; if base.Scheme=="" ||
base.Host=="" return a descriptive error about an invalid/non-absolute base URL,
and if parsed.Scheme=="" || parsed.Host=="" return an error that the advertised
PRM URL must be absolute, then proceed with the existing EqualFold scheme/host
comparisons (function: validateAdvertisedPRMURL, variables: base, parsed,
h.baseURL, candidate).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 34089bc8-731c-453e-ac45-69a5e4d26261
📒 Files selected for processing (2)
client/transport/oauth.goclient/transport/oauth_test.go
url.Parse accepts relative references (e.g. "prm", "example.com/mcp") without error, producing an empty Scheme and Host. Two empty strings EqualFold each other, so a misconfigured base URL combined with a relative advertised PRM URL would silently pass origin validation. validateAdvertisedPRMURL now explicitly rejects either side when its scheme or host is empty, with distinct error messages for the base and candidate cases. Tests cover relative candidate URLs, schemeless host-only candidates, and a relative base URL paired with a relative candidate (which previously matched by double-empty EqualFold).
Author
|
Addressed in 29b1589:
|
Contributor
|
@Dennisadira thanks for the PR, could you please fix the merge conflicts and I will have a look again. |
Contributor
|
Hey @Dennisadira — heads up, #804 by @MariaChrysafis (opened 3 days earlier) implements the same RFC 9728 §5.1 feature. Both PRs solve the same core problem but take meaningfully different design paths, so I did a side-by-side comparison to help us decide how to move forward. Where you both agree
Where you diverge
My takeBoth PRs have pieces the other is missing. Ideally I'd like to see a combined version that has:
Would you two be open to collaborating on a combined PR? Or if one of you wants to adopt the missing pieces from the other, that works too. Happy to help review either way. |
This was referenced Apr 20, 2026
16 tasks
Contributor
Author
16 tasks
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.
Description
Adds RFC 9728 §5.1 support for extracting the Protected Resource Metadata URL from the
WWW-Authenticateheader on 401 responses.This is the remaining piece of the OAuth RFC-compliance work tracked in #697 after #761 and #775 landed the path-preservation fixes, and closes the specific ask in #688.
Why
Per RFC 9728 §5.1, a resource server that requires OAuth advertises its PRM URL to clients via:
Clients that honour this header can discover the PRM endpoint in deployments where origin-based
/.well-known/oauth-protected-resourceconstruction can't reach it — for example, MCP servers sitting behind a shared gateway with path-based routing (Smithery, Cloudflare, Kubernetes Ingress by path, etc.). Without this support, DCR fails with 404, the authorization URL ends up with an emptyclient_id, and users have to setAuthServerMetadataURLmanually.What changes
client/transport/oauth.goProtectedResourceMetadataURLfield onOAuthHandler, guarded by the existing mutex.SetProtectedResourceMetadataURL/ProtectedResourceMetadataURL.HandleUnauthorizedResponse(resp *http.Response)— extractsresource_metadatafromWWW-Authenticateand stores it. Safe on nil responses / missing headers.extractResourceMetadataURLhelper — a small RFC 9110 §5.6.2 token + quoted-string parser that matches parameter names case-insensitively (RFC 9110 §11.2) and handles both quoted-string and token value forms.getServerMetadatanow prefers the stored PRM URL over the origin-based/.well-known/oauth-protected-resourceconstruction when one has been advertised.client/transport/sse.go,client/transport/streamable_http.go— callHandleUnauthorizedResponse(resp)at all five 401 sites (SSE Start / SendRequest / SendNotification, StreamableHTTP SendRequest / SendNotification) so the PRM URL is captured wherever it's advertised.Backward compatibility
Fully backward compatible. When the server doesn't send a
resource_metadataparameter,HandleUnauthorizedResponseis a no-op and metadata discovery falls through to the existingbuildWellKnownURLpath. No public-API breakage. TestTestSSE_WithOAuth_NoWWWAuthenticate_LeavesPRMUnsetand its streamable-HTTP twin pin this behaviour.Tests
TestExtractResourceMetadataURL— 12 subtests covering: empty header, quoted value, unquoted token, case-insensitive parameter name, escaped quotes in quoted value, tabs/whitespace handling, multiple parameters in a Bearer challenge, word-substring rejection, missing=, truncated quoted value, first-occurrence preference.TestOAuthHandler_ProtectedResourceMetadataURL_SetGet— setter/getter round-trip.TestOAuthHandler_HandleUnauthorizedResponse— 4 subtests: nil response, no header, header without the parameter, header with the parameter.TestOAuthHandler_GetServerMetadata_UsesAdvertisedPRMURL— end-to-end discovery: asserts the advertised URL is fetched instead of the well-known one.TestSSE_WithOAuth_ExtractsPRMFromWWWAuthenticate+TestStreamableHTTP_WithOAuth_ExtractsPRMFromWWWAuthenticate— transport-level: 401 + header → PRM URL stored on handler.TestSSE_WithOAuth_NoWWWAuthenticate_LeavesPRMUnset+ streamable twin — backward-compat regression guard.Validated locally with:
Type of Change
Checklist
MCP Spec Compliance
This PR implements a feature defined in the MCP specification
Link to relevant spec section: MCP Authorization — resource_metadata in WWW-Authenticate
Also aligns with RFC 9728 §5.1 and RFC 9110 §11.2
Implementation follows the specification exactly
Additional Information
The existing PR #718 attempted to cover this plus the path-preservation work in a single patch; this PR is scoped strictly to the WWW-Authenticate parsing that remained unmerged after #761 and #775. Happy to rebase or adjust scope on request.
Summary by CodeRabbit