Skip to content

Commit de77e9c

Browse files
Preserve query components in PRM resource URLs
1 parent 53117cb commit de77e9c

6 files changed

Lines changed: 82 additions & 11 deletions

File tree

src/mcp/client/auth/utils.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import re
2-
from urllib.parse import urljoin, urlparse
2+
from urllib.parse import urljoin, urlparse, urlunparse
33

44
from httpx import Request, Response
55
from mcp_types import LATEST_PROTOCOL_VERSION
@@ -63,7 +63,7 @@ def build_protected_resource_metadata_discovery_urls(www_auth_url: str | None, s
6363
6464
Per SEP-985, the client MUST:
6565
1. Try resource_metadata from WWW-Authenticate header (if present)
66-
2. Fall back to path-based well-known URI: /.well-known/oauth-protected-resource/{path}
66+
2. Fall back to path/query-based well-known URI: /.well-known/oauth-protected-resource/{path}?{query}
6767
3. Fall back to root-based well-known URI: /.well-known/oauth-protected-resource
6868
6969
Args:
@@ -83,9 +83,11 @@ def build_protected_resource_metadata_discovery_urls(www_auth_url: str | None, s
8383
parsed = urlparse(server_url)
8484
base_url = f"{parsed.scheme}://{parsed.netloc}"
8585

86-
# Priority 2: Path-based well-known URI (if server has a path component)
87-
if parsed.path and parsed.path != "/":
88-
path_based_url = urljoin(base_url, f"/.well-known/oauth-protected-resource{parsed.path}")
86+
# Priority 2: Path/query-based well-known URI (if server has a path or query component)
87+
if (parsed.path and parsed.path != "/") or parsed.query:
88+
resource_path = parsed.path if parsed.path != "/" else ""
89+
metadata_path = f"/.well-known/oauth-protected-resource{resource_path}"
90+
path_based_url = urlunparse((parsed.scheme, parsed.netloc, metadata_path, parsed.params, parsed.query, ""))
8991
urls.append(path_based_url)
9092

9193
# Priority 3: Root-based well-known URI

src/mcp/server/auth/routes.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
from collections.abc import Awaitable, Callable
22
from typing import Any
3-
from urllib.parse import urlparse
3+
from urllib.parse import urlparse, urlunparse
44

55
from pydantic import AnyHttpUrl
66
from starlette.middleware.cors import CORSMiddleware
@@ -202,7 +202,7 @@ def build_metadata(
202202
def build_resource_metadata_url(resource_server_url: AnyHttpUrl) -> AnyHttpUrl:
203203
"""Build RFC 9728 compliant protected resource metadata URL.
204204
205-
Inserts /.well-known/oauth-protected-resource between host and resource path
205+
Inserts /.well-known/oauth-protected-resource between host and resource path/query
206206
as specified in RFC 9728 §3.1.
207207
208208
Args:
@@ -214,7 +214,8 @@ def build_resource_metadata_url(resource_server_url: AnyHttpUrl) -> AnyHttpUrl:
214214
parsed = urlparse(str(resource_server_url))
215215
# Handle trailing slash: if path is just "/", treat as empty
216216
resource_path = parsed.path if parsed.path != "/" else ""
217-
return AnyHttpUrl(f"{parsed.scheme}://{parsed.netloc}/.well-known/oauth-protected-resource{resource_path}")
217+
metadata_path = f"/.well-known/oauth-protected-resource{resource_path}"
218+
return AnyHttpUrl(urlunparse((parsed.scheme, parsed.netloc, metadata_path, parsed.params, parsed.query, "")))
218219

219220

220221
def create_protected_resource_routes(

src/mcp/shared/auth_utils.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,10 @@ def check_resource_allowed(requested_resource: str, configured_resource: str) ->
3232
"""Check if a requested resource URL matches a configured resource URL.
3333
3434
A requested resource matches if it has the same scheme, domain, port,
35-
and its path starts with the configured resource's path. This allows
36-
hierarchical matching where a token for a parent resource can be used
37-
for child resources.
35+
the same query component, and its path starts with the configured
36+
resource's path. This allows hierarchical matching where a token for a
37+
parent resource can be used for child resources without collapsing
38+
query-routed resource identifiers.
3839
3940
Args:
4041
requested_resource: The resource URL being requested
@@ -51,6 +52,9 @@ def check_resource_allowed(requested_resource: str, configured_resource: str) ->
5152
if requested.scheme.lower() != configured.scheme.lower() or requested.netloc.lower() != configured.netloc.lower():
5253
return False
5354

55+
if requested.query != configured.query:
56+
return False
57+
5458
# Normalize trailing slashes before comparison so that
5559
# "/foo" and "/foo/" are treated as equivalent.
5660
requested_path = requested.path

tests/client/test_auth.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -863,6 +863,26 @@ async def test_validate_resource_rejects_mismatched_resource(
863863
await provider._validate_resource_match(prm)
864864

865865

866+
@pytest.mark.anyio
867+
async def test_validate_resource_rejects_mismatched_query_component(
868+
client_metadata: OAuthClientMetadata, mock_storage: MockTokenStorage
869+
) -> None:
870+
"""Client rejects PRM resources with a different query-routed resource identifier."""
871+
provider = OAuthClientProvider(
872+
server_url="https://api.example.com/v1/mcp?tenant=a",
873+
client_metadata=client_metadata,
874+
storage=mock_storage,
875+
)
876+
provider._initialized = True
877+
878+
prm = ProtectedResourceMetadata(
879+
resource=AnyHttpUrl("https://api.example.com/v1/mcp?tenant=b"),
880+
authorization_servers=[AnyHttpUrl("https://auth.example.com")],
881+
)
882+
with pytest.raises(OAuthFlowError, match="does not match expected"):
883+
await provider._validate_resource_match(prm)
884+
885+
866886
@pytest.mark.anyio
867887
async def test_validate_resource_accepts_matching_resource(
868888
client_metadata: OAuthClientMetadata, mock_storage: MockTokenStorage
@@ -1827,6 +1847,23 @@ async def callback_handler() -> AuthorizationCodeResult:
18271847
assert discovery_urls[0] == "https://api.example.com/.well-known/oauth-protected-resource/v1/mcp"
18281848
assert discovery_urls[1] == "https://api.example.com/.well-known/oauth-protected-resource"
18291849

1850+
def test_prm_discovery_preserves_server_url_query_component(self):
1851+
"""Fallback PRM discovery preserves RFC 9728 query components."""
1852+
init_response = httpx.Response(
1853+
status_code=401, headers={}, request=httpx.Request("GET", "https://api.example.com/v1/mcp?tenant=a")
1854+
)
1855+
1856+
discovery_urls = build_protected_resource_metadata_discovery_urls(
1857+
extract_resource_metadata_from_www_auth(init_response), "https://api.example.com/v1/mcp?tenant=a"
1858+
)
1859+
1860+
assert discovery_urls == snapshot(
1861+
[
1862+
"https://api.example.com/.well-known/oauth-protected-resource/v1/mcp?tenant=a",
1863+
"https://api.example.com/.well-known/oauth-protected-resource",
1864+
]
1865+
)
1866+
18301867
@pytest.mark.anyio
18311868
async def test_root_based_fallback_after_path_based_404(
18321869
self, client_metadata: OAuthClientMetadata, mock_storage: MockTokenStorage

tests/server/auth/test_protected_resource.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,13 @@ def test_metadata_url_construction_url_with_path_component():
122122
assert str(result) == "https://example.com/.well-known/oauth-protected-resource/mcp"
123123

124124

125+
def test_metadata_url_construction_preserves_query_component():
126+
"""Resource metadata URL construction preserves RFC 9728 query components."""
127+
resource_url = AnyHttpUrl("https://example.com/mcp?tenant=a")
128+
result = build_resource_metadata_url(resource_url)
129+
assert str(result) == "https://example.com/.well-known/oauth-protected-resource/mcp?tenant=a"
130+
131+
125132
def test_metadata_url_construction_url_with_trailing_slash_only():
126133
"""Test URL construction for resource with trailing slash only."""
127134
resource_url = AnyHttpUrl("https://example.com/")
@@ -136,6 +143,8 @@ def test_metadata_url_construction_url_with_trailing_slash_only():
136143
("https://example.com", "https://example.com/.well-known/oauth-protected-resource"),
137144
("https://example.com/", "https://example.com/.well-known/oauth-protected-resource"),
138145
("https://example.com/mcp", "https://example.com/.well-known/oauth-protected-resource/mcp"),
146+
("https://example.com/mcp?tenant=a", "https://example.com/.well-known/oauth-protected-resource/mcp?tenant=a"),
147+
("https://example.com?tenant=a", "https://example.com/.well-known/oauth-protected-resource?tenant=a"),
139148
("http://localhost:8001/mcp", "http://localhost:8001/.well-known/oauth-protected-resource/mcp"),
140149
],
141150
)

tests/shared/test_auth_utils.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,24 @@ def test_check_resource_allowed_path_boundary_matching():
100100
assert check_resource_allowed("https://example.com/api/v1", "https://example.com/api/") is True
101101

102102

103+
def test_check_resource_allowed_rejects_different_queries():
104+
"""Query-routed resources with different query components are distinct."""
105+
assert (
106+
check_resource_allowed(
107+
"https://example.com/api?tenant=a",
108+
"https://example.com/api?tenant=b",
109+
)
110+
is False
111+
)
112+
assert (
113+
check_resource_allowed(
114+
"https://example.com/api?tenant=a",
115+
"https://example.com/api",
116+
)
117+
is False
118+
)
119+
120+
103121
def test_check_resource_allowed_trailing_slash_handling():
104122
"""Trailing slashes should be handled correctly."""
105123
# With and without trailing slashes

0 commit comments

Comments
 (0)