fix(auth): preserve resource URI without trailing slash (#1968)#1972
Open
MukundaKatta wants to merge 3 commits intomodelcontextprotocol:v1.xfrom
Open
fix(auth): preserve resource URI without trailing slash (#1968)#1972MukundaKatta wants to merge 3 commits intomodelcontextprotocol:v1.xfrom
MukundaKatta wants to merge 3 commits intomodelcontextprotocol:v1.xfrom
Conversation
…protocol#1968) When handling RFC 9728 protected resource metadata, `selectResourceURL` routed the metadata's `resource` value through `new URL(...).href`. For bare-origin URIs that round trip appends a trailing slash: new URL("https://example.com").href === "https://example.com/" The resulting `resource` parameter no longer matches what the server published in PRM, which breaks providers that require an exact match. Microsoft Entra ID rejects the request with AADSTS9010010 when the `resource` parameter does not match the audience of the requested scope. Return the original metadata string verbatim from `selectResourceURL` and serialize it with `String(resource)` instead of `URL.href` in the authorization and token request paths. The validation step still parses the value as a URL via `checkResourceAllowed`. Also adjusted the cached discovery-state test to expect the un-normalized resource value, and added a regression test for the bare-domain case. Fixes modelcontextprotocol#1968
🦋 Changeset detectedLatest commit: d61bfde The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
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
1 participant
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.
What
Fixes #1968.
When parsing OAuth protected resource metadata,
selectResourceURLreturnednew URL(resourceMetadata.resource)and the auth + token request paths serialized that value viaURL.href. For bare-origin URIs that round trip appends a trailing slash:So a PRM document that publishes
"resource": "https://example.com"ends up sendingresource=https%3A%2F%2Fexample.com%2Fon the wire. That changes the resource indicator from the value the server told us to use, and it breaks any AS that compares it against the configured audience exactly.In particular Microsoft Entra ID rejects the call with:
(see also: modelcontextprotocol/inspector#927 for the symptom downstream.)
How
selectResourceURLnow returnsURL | string | undefinedand returnsresourceMetadata.resourceverbatim from the PRM path. The custom-validation path still returns aURLbecausevalidateResourceURLis typed that way.startAuthorization,executeTokenRequest,exchangeAuthorization,refreshAuthorization, andfetchTokenacceptresource?: URL | stringand serialize it viaString(resource)instead ofresource.href.validateResourceURLprovider hook signature is unchanged.checkResourceAllowedstill validates the metadata value as a URL.Diff is contained to
src/client/auth.ts. Existing PRM-with-path coverage still passes (paths likehttps://api.example.com/mcp-serverare unaffected — theURL.hrefround-trip only added a slash for bare origins).Tests
resource: "https://example.com"in PRM and asserts the auth URL receivesresource=https://example.com(no trailing slash).npm testpasses locally (1580/1580).