-
Notifications
You must be signed in to change notification settings - Fork 210
internal/oauthex: fix GetProtectedResourceMetadataFromHeader validation #561
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this 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
oauthex/resource_meta.go
Outdated
@@ -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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this 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.
@samthanawalla are these good to merge? |
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