fix(auth): add token issuer validation for MCP spec compliance#1447
fix(auth): add token issuer validation for MCP spec compliance#1447Ujjwal-Bajpayee wants to merge 2 commits intomodelcontextprotocol:mainfrom
Conversation
pcarleton
left a comment
There was a problem hiding this comment.
This helps with catching incorrect tokens sent to the MCP Server, preventing a class of MITIM attack. This will only help with JWT-based tokens, leaving the issue unsolved for opaque tokens. A more complete solution would involve e.g. requiring AS's send an ?issuer= param on their callback, and checking that. However, that would require a specification change, so this is still an improvement in the interim.
So, overall 👍 to this approach.
I left a few comments, and then please add a test, both for JWT tokens, opaque tokens, and an opaque token that has .'s but is not a JWT.
| logger.exception("Failed to validate token issuer") | ||
| return False | ||
|
|
||
| def _token_issuer_matches(self, token: str) -> bool: |
There was a problem hiding this comment.
I'd rather we use a library for JWT parsing rather than do it partially here, e.g. PyJWT
|
@pcarleton Thanks for the feedback. I’ll update to use direct access for expected_issuer, switch to PyJWT for parsing, and add the requested tests for JWT and opaque tokens. I’ll push the changes and update the PR soon. |
|
Thanks @Ujjwal-Bajpayee - converting to draft for now while we wait for the updates to remove from our review queue for now. Please re-request a review once you've had a chance to take another look. |
Co-authored-by: Paul Carleton <paulcarletonjr@gmail.com>
|
Closing due to inactivity. |
Summary
This PR implements token issuer validation in the MCP Python SDK client to ensure compliance with the MCP specification requirement:
Previously, the SDK only checked for token existence and expiration.
This change adds verification that tokens were issued by the expected authorization server before they are used.
What I Changed
auth.pyexpected_issuer: str | None = NonetoOAuthContext.is_token_valid()to:expected_issueris not set, preserve the original behavior.expected_issueris set, decode the access token (assuming JWT format), extract theissclaim, and verify that it matchesself.expected_issuer.Falsefor missing/mismatched issuers or parsing errors.Why This Is Safe and Backward-Compatible
expected_issueris not provided, behavior is identical to previous versions.is_token_validand one private helper were modified.Falsereturn value (fail closed).Validation Performed
issclaims.is_token_valid()correctly respects token expiry and issuer matching.🔗 Related Issue
Closes #1442
Type of Change