8000 internal/oauthex: fix GetProtectedResourceMetadataFromHeader validation by jhrozek · Pull Request #561 · modelcontextprotocol/go-sdk · GitHub
[go: up one dir, main page]

Skip to content

Conversation

jhrozek
Copy link
Contributor
@jhrozek jhrozek commented Oct 7, 2025

GetProtectedResourceMetadataFromHeader was validating the resource field against the metadata endpoint URL instead of the original server URL. Per RFC 9728 section 3.3, the resource field must match the URL that the client used to make the request to the resource server.

Fixes: #560

Copy link
Contributor
@samthanawalla samthanawalla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! It LGTM once the conflicts are resolved

@@ -167,7 +168,7 @@ func getPRM(ctx context.Context, purl string, c *http.Client, wantResource strin
if err != nil {
return nil, err
}
// Validate the Resource field to thwart impersonation attacks (section 3.3).
// Validate the Resource field (see RFC 9728, section 3.3) as well as issue #560
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove reference to issue #560 and end with a period

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor
@jba jba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after Sam's changes.

GetProtectedResourceMetadataFromHeader was validating the resource field
against the metadata endpoint URL instead of the original server URL.
Per RFC 9728 section 3.3, the resource field must match the URL that the
client used to make the request to the resource server.

Fixes: modelcontextprotocol#560

This fix adds a serverURL parameter and validates against the correct value.
@findleyr
Copy link
Contributor
findleyr commented Oct 8, 2025

@samthanawalla are these good to merge?

@samthanawalla samthanawalla merged commit 1059f59 into modelcontextprotocol:main Oct 8, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GetProtectedResourceMetadataFromHeader validation always fails due to mismatched resource identifiers
4 participants
0