fix: omit RFC 8707 resource param on refresh_token grants and strip root trailing slash from PRM resource#2853
Open
fede-kamel wants to merge 1 commit into
Conversation
…railing slash from PRM resource Two compounding bugs broke silent token refresh against Microsoft Entra ID v2.0 (AADSTS9010010), causing MCP servers using Entra OAuth to lose authentication after ~1 hour: - _refresh_token() sent the RFC 8707 resource parameter on refresh_token grants, which Entra v2.0 strictly rejects since March 2026. The parameter is now omitted from refresh requests. - Pydantic's AnyHttpUrl normalizes bare-domain PRM resource URLs to include a trailing slash, so the resource audience never matched the IdP app registration. get_resource_url() now strips the slash, but only when the path is exactly "/" with no query or fragment - RFC 9728 requires exact-string identity, so intentional trailing slashes on deeper paths are preserved. Implements the approach maintainers endorsed when consolidating earlier attempts (modelcontextprotocol#2590, since closed unmerged; see discussion on modelcontextprotocol#2645/modelcontextprotocol#2646). Fixes modelcontextprotocol#2578
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.
Motivation and Context
Fixes #2578.
Two compounding bugs break silent token refresh against Microsoft Entra ID v2.0 (
AADSTS9010010), causing MCP servers using Entra OAuth to lose authentication after ~1 hour:_refresh_token()sends the RFC 8707resourceparameter onrefresh_tokengrants. Entra v2.0 strictly validates and rejects it on refresh (since March 2026).AnyHttpUrlnormalizes bare-domain PRM resource URLs to include a trailing slash (https://mcp-server.example.com→https://mcp-server.example.com/), so the resource audience never matches the IdP app registration.Changes
_refresh_token()no longer includes theresourceparameter. Initial authorization and token-exchange requests are unchanged.get_resource_url()normalizes the PRM resource via a new_normalize_resource_url()helper that strips the trailing slash only when the path is exactly/with no query or fragment. RFC 9728 requires exact-string identity on the resource identifier, so intentional trailing slashes on deeper paths (e.g.…/mcp/) are preserved.This implements the approach maintainers endorsed when consolidating the earlier attempts: #2590 (closed unmerged during the author's backlog cleanup, never reviewed) and the duplicate-resolution discussion on #2645/#2646, which specifically preferred root-only normalization over a blanket
rstrip("/"). Credit to @he-yufeng for the original #2590.One delta vs. #2590: the newer
tests/interaction/auth/test_lifecycle.py::test_an_expired_access_token_is_transparently_refreshed_before_the_next_request(added after #2590 was written) still snapshot-asserted theresourcekey in the refresh body; this PR updates that snapshot and its docstring.How Has This Been Tested?
test_get_resource_url_removes_root_prm_trailing_slash, andtest_get_resource_url_preserves_non_root_trailing_slash(guards the RFC 9728 exact-identity concern)../scripts/testgate passes locally: 1783 passed, 100% coverage,strict-no-cover,ruff, andpyrightclean.Breaking Changes
No API changes. Wire-level behavior change only: refresh requests no longer carry
resource. Clients talking to providers that tolerated the parameter are unaffected; providers that rejected it (Entra v2.0) start working.