Skip to content

Commit 005bcbc

Browse files
fix: add RFC 8707 resource validation to OAuth client
Backport from main (PR #2010). The client now validates that the Protected Resource Metadata resource field matches the server URL before proceeding with authorization, rejecting mismatched resources per RFC 8707. This fixes the auth/resource-mismatch conformance test, bringing client conformance to 251/251 (100%) on v1.x.
1 parent d234633 commit 005bcbc

File tree

5 files changed

+102
-18
lines changed

5 files changed

+102
-18
lines changed
Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,4 @@
11
# Known conformance test failures for v1.x
22
# These are tracked and should be removed as they're fixed.
3-
#
4-
# auth/resource-mismatch: Client must validate that the Protected Resource
5-
# Metadata (PRM) resource field matches the server URL before proceeding
6-
# with authorization (RFC 8707). Implemented on main (PR #2010), needs
7-
# backport to v1.x.
8-
client:
9-
- auth/resource-mismatch
3+
server: []
4+
client: []

src/mcp/client/auth/oauth2.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,15 @@ def __init__(
267267
)
268268
self._initialized = False
269269

270+
async def _validate_resource_match(self, prm: ProtectedResourceMetadata) -> None:
271+
"""Validate that PRM resource matches the server URL per RFC 8707."""
272+
prm_resource = str(prm.resource) if prm.resource else None
273+
if not prm_resource:
274+
return # pragma: no cover
275+
default_resource = resource_url_from_server_url(self.context.server_url)
276+
if not check_resource_allowed(requested_resource=default_resource, configured_resource=prm_resource):
277+
raise OAuthFlowError(f"Protected resource {prm_resource} does not match expected {default_resource}")
278+
270279
async def _handle_protected_resource_response(self, response: httpx.Response) -> bool:
271280
"""
272281
Handle protected resource metadata discovery response.
@@ -520,6 +529,8 @@ async def async_auth_flow(self, request: httpx.Request) -> AsyncGenerator[httpx.
520529

521530
prm = await handle_protected_resource_response(discovery_response)
522531
if prm:
532+
# Validate PRM resource matches server URL (RFC 8707)
533+
await self._validate_resource_match(prm)
523534
self.context.protected_resource_metadata = prm
524535

525536
# todo: try all authorization_servers to find the OASM

src/mcp/shared/auth_utils.py

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -51,22 +51,17 @@ def check_resource_allowed(requested_resource: str, configured_resource: str) ->
5151
if requested.scheme.lower() != configured.scheme.lower() or requested.netloc.lower() != configured.netloc.lower():
5252
return False
5353

54-
# Handle cases like requested=/foo and configured=/foo/
54+
# Normalize trailing slashes before comparison so that
55+
# "/foo" and "/foo/" are treated as equivalent.
5556
requested_path = requested.path
5657
configured_path = configured.path
57-
58-
# If requested path is shorter, it cannot be a child
59-
if len(requested_path) < len(configured_path):
60-
return False
61-
62-
# Check if the requested path starts with the configured path
63-
# Ensure both paths end with / for proper comparison
64-
# This ensures that paths like "/api123" don't incorrectly match "/api"
6558
if not requested_path.endswith("/"):
6659
requested_path += "/"
6760
if not configured_path.endswith("/"):
6861
configured_path += "/"
6962

63+
# Check hierarchical match: requested must start with configured path.
64+
# The trailing-slash normalization ensures "/api123/" won't match "/api/".
7065
return requested_path.startswith(configured_path)
7166

7267

tests/client/test_auth.py

Lines changed: 84 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
from pydantic import AnyHttpUrl, AnyUrl
1414

1515
from mcp.client.auth import OAuthClientProvider, PKCEParameters
16+
from mcp.client.auth.exceptions import OAuthFlowError
1617
from mcp.client.auth.utils import (
1718
build_oauth_authorization_server_metadata_discovery_urls,
1819
build_protected_resource_metadata_discovery_urls,
@@ -965,7 +966,7 @@ async def test_auth_flow_with_no_tokens(self, oauth_provider: OAuthClientProvide
965966
# Send a successful discovery response with minimal protected resource metadata
966967
discovery_response = httpx.Response(
967968
200,
968-
content=b'{"resource": "https://api.example.com/mcp", "authorization_servers": ["https://auth.example.com"]}',
969+
content=b'{"resource": "https://api.example.com/v1/mcp", "authorization_servers": ["https://auth.example.com"]}',
969970
request=discovery_request,
970971
)
971972

@@ -2030,3 +2031,85 @@ async def callback_handler() -> tuple[str, str | None]:
20302031
await auth_flow.asend(final_response)
20312032
except StopAsyncIteration:
20322033
pass
2034+
2035+
2036+
@pytest.mark.anyio
2037+
async def test_validate_resource_rejects_mismatched_resource(
2038+
client_metadata: OAuthClientMetadata, mock_storage: MockTokenStorage
2039+
) -> None:
2040+
"""Client must reject PRM resource that doesn't match server URL."""
2041+
provider = OAuthClientProvider(
2042+
server_url="https://api.example.com/v1/mcp",
2043+
client_metadata=client_metadata,
2044+
storage=mock_storage,
2045+
)
2046+
provider._initialized = True
2047+
2048+
prm = ProtectedResourceMetadata(
2049+
resource=AnyHttpUrl("https://evil.example.com/mcp"),
2050+
authorization_servers=[AnyHttpUrl("https://auth.example.com")],
2051+
)
2052+
with pytest.raises(OAuthFlowError, match="does not match expected"):
2053+
await provider._validate_resource_match(prm)
2054+
2055+
2056+
@pytest.mark.anyio
2057+
async def test_validate_resource_accepts_matching_resource(
2058+
client_metadata: OAuthClientMetadata, mock_storage: MockTokenStorage
2059+
) -> None:
2060+
"""Client must accept PRM resource that matches server URL."""
2061+
provider = OAuthClientProvider(
2062+
server_url="https://api.example.com/v1/mcp",
2063+
client_metadata=client_metadata,
2064+
storage=mock_storage,
2065+
)
2066+
provider._initialized = True
2067+
2068+
prm = ProtectedResourceMetadata(
2069+
resource=AnyHttpUrl("https://api.example.com/v1/mcp"),
2070+
authorization_servers=[AnyHttpUrl("https://auth.example.com")],
2071+
)
2072+
# Should not raise
2073+
await provider._validate_resource_match(prm)
2074+
2075+
2076+
@pytest.mark.anyio
2077+
async def test_validate_resource_accepts_root_url_with_trailing_slash(
2078+
client_metadata: OAuthClientMetadata, mock_storage: MockTokenStorage
2079+
) -> None:
2080+
"""Root URLs with trailing slash normalization should match."""
2081+
provider = OAuthClientProvider(
2082+
server_url="https://api.example.com/",
2083+
client_metadata=client_metadata,
2084+
storage=mock_storage,
2085+
)
2086+
provider._initialized = True
2087+
2088+
prm = ProtectedResourceMetadata(
2089+
resource=AnyHttpUrl("https://api.example.com/"),
2090+
authorization_servers=[AnyHttpUrl("https://auth.example.com")],
2091+
)
2092+
# Should not raise - both already have trailing slashes
2093+
await provider._validate_resource_match(prm)
2094+
2095+
2096+
@pytest.mark.anyio
2097+
async def test_get_resource_url_falls_back_when_prm_mismatches(
2098+
client_metadata: OAuthClientMetadata, mock_storage: MockTokenStorage
2099+
) -> None:
2100+
"""get_resource_url returns canonical URL when PRM resource doesn't match."""
2101+
provider = OAuthClientProvider(
2102+
server_url="https://api.example.com/v1/mcp",
2103+
client_metadata=client_metadata,
2104+
storage=mock_storage,
2105+
)
2106+
provider._initialized = True
2107+
2108+
# Set PRM with a resource that is NOT a parent of the server URL
2109+
provider.context.protected_resource_metadata = ProtectedResourceMetadata(
2110+
resource=AnyHttpUrl("https://other.example.com/mcp"),
2111+
authorization_servers=[AnyHttpUrl("https://auth.example.com")],
2112+
)
2113+
2114+
# get_resource_url should return the canonical server URL, not the PRM resource
2115+
assert provider.context.get_resource_url() == "https://api.example.com/v1/mcp"

tests/shared/test_auth_utils.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ def test_trailing_slash_handling(self):
9595
"""Trailing slashes should be handled correctly."""
9696
# With and without trailing slashes
9797
assert check_resource_allowed("https://example.com/api/", "https://example.com/api") is True
98-
assert check_resource_allowed("https://example.com/api", "https://example.com/api/") is False
98+
assert check_resource_allowed("https://example.com/api", "https://example.com/api/") is True
9999
assert check_resource_allowed("https://example.com/api/v1", "https://example.com/api") is True
100100
assert check_resource_allowed("https://example.com/api/v1", "https://example.com/api/") is True
101101

0 commit comments

Comments
 (0)