fix: omit RFC 8707 resource param on refresh_token grants and strip root trailing slash from PRM resource - #2853
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
cc4fd5f to
e8afa1a
Compare
|
@maxisbey @Kludex — final maintainer-selection request for #2578. I have just rebased this branch onto current main (e8afa1a), then rerun the focused OAuth surfaces: 153 passed, 1 expected xfail. I also reran the repository test gate after the rebase.\n\nThe issue is still open and there have been multiple overlapping fixes. This PR keeps the change deliberately small: omit resource from refresh_token grants, and normalize only the artificial root trailing slash while preserving meaningful path slashes.\n\nCould you please make a one-line decision on whether this is the implementation you want to take, whether one of the other proposals should be selected instead, or whether the behavior needs a different design? If it is not going to be taken, I will close this PR in 10 days rather than leave duplicate stale work open. Thank you. |
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.