Skip to content

Commit e8afa1a

Browse files
fede-kamelfede-kamel
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 0d92192 commit e8afa1a

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
@@ -109,6 +109,18 @@ def check_registration_usable(client_info: OAuthClientInformationFull) -> None:
109109
)
110110

111111

112+
def _normalize_resource_url(resource: str) -> str:
113+
"""Undo the trailing slash URL parsers add to bare-domain URLs (e.g. pydantic's AnyHttpUrl).
114+
115+
RFC 9728 requires exact-string identity on the resource identifier, so only a root path
116+
with no query or fragment is stripped; trailing slashes on deeper paths are preserved.
117+
"""
118+
parsed = urlparse(resource)
119+
if parsed.path == "/" and not parsed.params and not parsed.query and not parsed.fragment:
120+
return f"{parsed.scheme}://{parsed.netloc}"
121+
return resource
122+
123+
112124
class PKCEParameters(BaseModel):
113125
"""PKCE (Proof Key for Code Exchange) parameters."""
114126

@@ -206,7 +218,7 @@ def get_resource_url(self) -> str:
206218

207219
# If PRM provides a resource that's a valid parent, use it
208220
if self.protected_resource_metadata and self.protected_resource_metadata.resource:
209-
prm_resource = str(self.protected_resource_metadata.resource)
221+
prm_resource = _normalize_resource_url(str(self.protected_resource_metadata.resource))
210222
if check_resource_allowed(requested_resource=resource, configured_resource=prm_resource):
211223
resource = prm_resource
212224

@@ -506,9 +518,8 @@ async def _refresh_token(self) -> httpx2.Request:
506518
"client_id": self.context.client_info.client_id,
507519
}
508520

509-
# Only include resource param if conditions are met
510-
if self.context.should_include_resource_param(self.context.protocol_version):
511-
refresh_data["resource"] = self.context.get_resource_url() # RFC 8707
521+
# The RFC 8707 resource param is deliberately omitted: some providers
522+
# (e.g. Entra ID v2.0) reject it on refresh_token grants.
512523

513524
# Prepare authentication based on preferred method
514525
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
@@ -750,7 +750,7 @@ class TestProtectedResourceMetadata:
750750

751751
@pytest.mark.anyio
752752
async def test_resource_param_included_with_recent_protocol_version(self, oauth_provider: OAuthClientProvider):
753-
"""Test resource parameter is included for protocol version >= 2025-06-18."""
753+
"""Test resource parameter is included in initial token requests for protocol version >= 2025-06-18."""
754754
# Set protocol version to 2025-06-18
755755
oauth_provider.context.protocol_version = "2025-06-18"
756756
oauth_provider.context.client_info = OAuthClientInformationFull(
@@ -767,15 +767,16 @@ async def test_resource_param_included_with_recent_protocol_version(self, oauth_
767767
expected_resource = quote(oauth_provider.context.get_resource_url(), safe="")
768768
assert f"resource={expected_resource}" in content
769769

770-
# Test in refresh token
770+
# Refresh requests never include the resource parameter: some providers
771+
# (e.g. Entra ID v2.0) reject RFC 8707 resource values on refresh_token grants.
771772
oauth_provider.context.current_tokens = OAuthToken(
772773
access_token="test_access",
773774
token_type="Bearer",
774775
refresh_token="test_refresh",
775776
)
776777
refresh_request = await oauth_provider._refresh_token()
777778
refresh_content = refresh_request.content.decode()
778-
assert "resource=" in refresh_content
779+
assert "resource=" not in refresh_content
779780

780781
@pytest.mark.anyio
781782
async def test_resource_param_excluded_with_old_protocol_version(self, oauth_provider: OAuthClientProvider):
@@ -805,7 +806,7 @@ async def test_resource_param_excluded_with_old_protocol_version(self, oauth_pro
805806

806807
@pytest.mark.anyio
807808
async def test_resource_param_included_with_protected_resource_metadata(self, oauth_provider: OAuthClientProvider):
808-
"""Test resource parameter is always included when protected resource metadata exists."""
809+
"""Test resource parameter is included in initial token requests when protected resource metadata exists."""
809810
# Set old protocol version but with protected resource metadata
810811
oauth_provider.context.protocol_version = "2025-03-26"
811812
oauth_provider.context.protected_resource_metadata = ProtectedResourceMetadata(
@@ -823,6 +824,16 @@ async def test_resource_param_included_with_protected_resource_metadata(self, oa
823824
content = request.content.decode()
824825
assert "resource=" in content
825826

827+
# Even with PRM present, refresh requests omit the resource parameter
828+
oauth_provider.context.current_tokens = OAuthToken(
829+
access_token="test_access",
830+
token_type="Bearer",
831+
refresh_token="test_refresh",
832+
)
833+
refresh_request = await oauth_provider._refresh_token()
834+
refresh_content = refresh_request.content.decode()
835+
assert "resource=" not in refresh_content
836+
826837

827838
@pytest.mark.parametrize(
828839
("protocol_version", "expected"),
@@ -972,6 +983,47 @@ async def test_get_resource_url_uses_canonical_when_prm_mismatches(
972983
assert provider.context.get_resource_url() == snapshot("https://api.example.com/v1/mcp")
973984

974985

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

‎tests/interaction/auth/test_lifecycle.py‎

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

126128
refresh_body = bodies[1]
127-
assert sorted(refresh_body) == snapshot(["client_id", "client_secret", "grant_type", "refresh_token", "resource"])
129+
assert sorted(refresh_body) == snapshot(["client_id", "client_secret", "grant_type", "refresh_token"])
128130
assert refresh_body["refresh_token"].startswith("refresh_")
129-
assert refresh_body["resource"].startswith(BASE_URL)
130131

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

0 commit comments

Comments
 (0)