Skip to content

Commit cc4fd5f

Browse files
committed
Omit RFC 8707 resource param on refresh_token grants and strip root trailing 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 (#2590, since closed unmerged; see discussion on #2645/#2646). Fixes #2578
1 parent cf110e3 commit cc4fd5f

3 files changed

Lines changed: 76 additions & 12 deletions

File tree

src/mcp/client/auth/oauth2.py

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,18 @@
5454
logger = logging.getLogger(__name__)
5555

5656

57+
def _normalize_resource_url(resource: str) -> str:
58+
"""Undo the trailing slash URL parsers add to bare-domain URLs (e.g. pydantic's AnyHttpUrl).
59+
60+
RFC 9728 requires exact-string identity on the resource identifier, so only a root path
61+
with no query or fragment is stripped; trailing slashes on deeper paths are preserved.
62+
"""
63+
parsed = urlparse(resource)
64+
if parsed.path == "/" and not parsed.params and not parsed.query and not parsed.fragment:
65+
return f"{parsed.scheme}://{parsed.netloc}"
66+
return resource
67+
68+
5769
class PKCEParameters(BaseModel):
5870
"""PKCE (Proof Key for Code Exchange) parameters."""
5971

@@ -152,7 +164,7 @@ def get_resource_url(self) -> str:
152164

153165
# If PRM provides a resource that's a valid parent, use it
154166
if self.protected_resource_metadata and self.protected_resource_metadata.resource:
155-
prm_resource = str(self.protected_resource_metadata.resource)
167+
prm_resource = _normalize_resource_url(str(self.protected_resource_metadata.resource))
156168
if check_resource_allowed(requested_resource=resource, configured_resource=prm_resource):
157169
resource = prm_resource
158170

@@ -441,9 +453,8 @@ async def _refresh_token(self) -> httpx.Request:
441453
"client_id": self.context.client_info.client_id,
442454
}
443455

444-
# Only include resource param if conditions are met
445-
if self.context.should_include_resource_param(self.context.protocol_version):
446-
refresh_data["resource"] = self.context.get_resource_url() # RFC 8707
456+
# The RFC 8707 resource param is deliberately omitted: some providers
457+
# (e.g. Entra ID v2.0) reject it on refresh_token grants.
447458

448459
# Prepare authentication based on preferred method
449460
headers = {"Content-Type": "application/x-www-form-urlencoded"}

tests/client/test_auth.py

Lines changed: 56 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -745,7 +745,7 @@ class TestProtectedResourceMetadata:
745745

746746
@pytest.mark.anyio
747747
async def test_resource_param_included_with_recent_protocol_version(self, oauth_provider: OAuthClientProvider):
748-
"""Test resource parameter is included for protocol version >= 2025-06-18."""
748+
"""Test resource parameter is included in initial token requests for protocol version >= 2025-06-18."""
749749
# Set protocol version to 2025-06-18
750750
oauth_provider.context.protocol_version = "2025-06-18"
751751
oauth_provider.context.client_info = OAuthClientInformationFull(
@@ -762,15 +762,16 @@ async def test_resource_param_included_with_recent_protocol_version(self, oauth_
762762
expected_resource = quote(oauth_provider.context.get_resource_url(), safe="")
763763
assert f"resource={expected_resource}" in content
764764

765-
# Test in refresh token
765+
# Refresh requests never include the resource parameter: some providers
766+
# (e.g. Entra ID v2.0) reject RFC 8707 resource values on refresh_token grants.
766767
oauth_provider.context.current_tokens = OAuthToken(
767768
access_token="test_access",
768769
token_type="Bearer",
769770
refresh_token="test_refresh",
770771
)
771772
refresh_request = await oauth_provider._refresh_token()
772773
refresh_content = refresh_request.content.decode()
773-
assert "resource=" in refresh_content
774+
assert "resource=" not in refresh_content
774775

775776
@pytest.mark.anyio
776777
async def test_resource_param_excluded_with_old_protocol_version(self, oauth_provider: OAuthClientProvider):
@@ -800,7 +801,7 @@ async def test_resource_param_excluded_with_old_protocol_version(self, oauth_pro
800801

801802
@pytest.mark.anyio
802803
async def test_resource_param_included_with_protected_resource_metadata(self, oauth_provider: OAuthClientProvider):
803-
"""Test resource parameter is always included when protected resource metadata exists."""
804+
"""Test resource parameter is included in initial token requests when protected resource metadata exists."""
804805
# Set old protocol version but with protected resource metadata
805806
oauth_provider.context.protocol_version = "2025-03-26"
806807
oauth_provider.context.protected_resource_metadata = ProtectedResourceMetadata(
@@ -818,6 +819,16 @@ async def test_resource_param_included_with_protected_resource_metadata(self, oa
818819
content = request.content.decode()
819820
assert "resource=" in content
820821

822+
# Even with PRM present, refresh requests omit the resource parameter
823+
oauth_provider.context.current_tokens = OAuthToken(
824+
access_token="test_access",
825+
token_type="Bearer",
826+
refresh_token="test_refresh",
827+
)
828+
refresh_request = await oauth_provider._refresh_token()
829+
refresh_content = refresh_request.content.decode()
830+
assert "resource=" not in refresh_content
831+
821832

822833
@pytest.mark.parametrize(
823834
("protocol_version", "expected"),
@@ -967,6 +978,47 @@ async def test_get_resource_url_uses_canonical_when_prm_mismatches(
967978
assert provider.context.get_resource_url() == snapshot("https://api.example.com/v1/mcp")
968979

969980

981+
@pytest.mark.anyio
982+
async def test_get_resource_url_removes_root_prm_trailing_slash(
983+
client_metadata: OAuthClientMetadata, mock_storage: MockTokenStorage
984+
) -> None:
985+
"""Bare-domain PRM resources should not pick up the trailing slash AnyHttpUrl adds."""
986+
provider = OAuthClientProvider(
987+
server_url="https://api.example.com",
988+
client_metadata=client_metadata,
989+
storage=mock_storage,
990+
)
991+
provider._initialized = True
992+
993+
# AnyHttpUrl normalizes "https://api.example.com" to "https://api.example.com/"
994+
provider.context.protected_resource_metadata = ProtectedResourceMetadata(
995+
resource=AnyHttpUrl("https://api.example.com"),
996+
authorization_servers=[AnyHttpUrl("https://auth.example.com")],
997+
)
998+
999+
assert provider.context.get_resource_url() == snapshot("https://api.example.com")
1000+
1001+
1002+
@pytest.mark.anyio
1003+
async def test_get_resource_url_preserves_non_root_trailing_slash(
1004+
client_metadata: OAuthClientMetadata, mock_storage: MockTokenStorage
1005+
) -> None:
1006+
"""RFC 9728 requires exact-string identity, so intentional trailing slashes on deeper paths stay."""
1007+
provider = OAuthClientProvider(
1008+
server_url="https://api.example.com/v1/mcp/",
1009+
client_metadata=client_metadata,
1010+
storage=mock_storage,
1011+
)
1012+
provider._initialized = True
1013+
1014+
provider.context.protected_resource_metadata = ProtectedResourceMetadata(
1015+
resource=AnyHttpUrl("https://api.example.com/v1/mcp/"),
1016+
authorization_servers=[AnyHttpUrl("https://auth.example.com")],
1017+
)
1018+
1019+
assert provider.context.get_resource_url() == snapshot("https://api.example.com/v1/mcp/")
1020+
1021+
9701022
class TestRegistrationResponse:
9711023
"""Test client registration response handling."""
9721024

tests/interaction/auth/test_lifecycle.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,10 @@ async def test_an_expired_access_token_is_transparently_refreshed_before_the_nex
104104
The provider tells the client `expires_in=-3600` for the first token while keeping the
105105
server-side `expires_at` in the future, so the connect's retry succeeds and the next
106106
request finds the token expired and refreshes. The recorded requests prove exactly one
107-
`grant_type=refresh_token` exchange carrying the resource indicator, and the bearer used
108-
after the refresh is the second access token, which is the one persisted to storage.
107+
`grant_type=refresh_token` exchange without the resource indicator (some providers,
108+
e.g. Entra ID v2.0, reject RFC 8707 resource values on refresh_token grants), and the
109+
bearer used after the refresh is the second access token, which is the one persisted
110+
to storage.
109111
"""
110112
recorded, on_request = record_requests()
111113
provider = InMemoryAuthorizationServerProvider(issue_expired_first=True)
@@ -123,9 +125,8 @@ async def test_an_expired_access_token_is_transparently_refreshed_before_the_nex
123125
assert [b["grant_type"] for b in bodies] == snapshot(["authorization_code", "refresh_token"])
124126

125127
refresh_body = bodies[1]
126-
assert sorted(refresh_body) == snapshot(["client_id", "client_secret", "grant_type", "refresh_token", "resource"])
128+
assert sorted(refresh_body) == snapshot(["client_id", "client_secret", "grant_type", "refresh_token"])
127129
assert refresh_body["refresh_token"].startswith("refresh_")
128-
assert refresh_body["resource"].startswith(BASE_URL)
129130

130131
bearers = {r.headers["authorization"] for r in recorded if r.path == "/mcp" and "authorization" in r.headers}
131132
assert len(bearers) == 2

0 commit comments

Comments
 (0)