Skip to content

Commit 5489e8b

Browse files
abliznyukmaxisbey
andauthored
fix get_client_metadata_scopes on 401 (#1631)
Co-authored-by: Max Isbey <224885523+maxisbey@users.noreply.github.com>
1 parent a357380 commit 5489e8b

File tree

2 files changed

+167
-1
lines changed

2 files changed

+167
-1
lines changed

src/mcp/client/auth/oauth2.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -506,7 +506,7 @@ async def async_auth_flow(self, request: httpx.Request) -> AsyncGenerator[httpx.
506506

507507
# Step 3: Apply scope selection strategy
508508
self.context.client_metadata.scope = get_client_metadata_scopes(
509-
www_auth_resource_metadata_url,
509+
extract_scope_from_www_auth(response),
510510
self.context.protected_resource_metadata,
511511
self.context.oauth_metadata,
512512
)
Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
"""
2+
Regression test for issue #1630: OAuth2 scope incorrectly set to resource_metadata URL.
3+
4+
This test verifies that when a 401 response contains both resource_metadata and scope
5+
in the WWW-Authenticate header, the actual scope is used (not the resource_metadata URL).
6+
"""
7+
8+
from unittest import mock
9+
10+
import httpx
11+
import pytest
12+
from pydantic import AnyUrl
13+
14+
from mcp.client.auth import OAuthClientProvider
15+
from mcp.shared.auth import OAuthClientInformationFull, OAuthClientMetadata, OAuthToken
16+
17+
18+
class MockTokenStorage:
19+
"""Mock token storage for testing."""
20+
21+
def __init__(self) -> None:
22+
self._tokens: OAuthToken | None = None
23+
self._client_info: OAuthClientInformationFull | None = None
24+
25+
async def get_tokens(self) -> OAuthToken | None:
26+
return self._tokens # pragma: no cover
27+
28+
async def set_tokens(self, tokens: OAuthToken) -> None:
29+
self._tokens = tokens
30+
31+
async def get_client_info(self) -> OAuthClientInformationFull | None:
32+
return self._client_info # pragma: no cover
33+
34+
async def set_client_info(self, client_info: OAuthClientInformationFull) -> None:
35+
self._client_info = client_info # pragma: no cover
36+
37+
38+
@pytest.mark.anyio
39+
async def test_401_uses_www_auth_scope_not_resource_metadata_url():
40+
"""
41+
Regression test for #1630: Ensure scope is extracted from WWW-Authenticate header,
42+
not the resource_metadata URL.
43+
44+
When a 401 response contains:
45+
WWW-Authenticate: Bearer resource_metadata="https://...", scope="read write"
46+
47+
The client should use "read write" as the scope, NOT the resource_metadata URL.
48+
"""
49+
50+
async def redirect_handler(url: str) -> None:
51+
pass # pragma: no cover
52+
53+
async def callback_handler() -> tuple[str, str | None]:
54+
return "test_auth_code", "test_state" # pragma: no cover
55+
56+
client_metadata = OAuthClientMetadata(
57+
redirect_uris=[AnyUrl("http://localhost:3030/callback")],
58+
client_name="Test Client",
59+
)
60+
61+
provider = OAuthClientProvider(
62+
server_url="https://api.example.com/mcp",
63+
client_metadata=client_metadata,
64+
storage=MockTokenStorage(),
65+
redirect_handler=redirect_handler,
66+
callback_handler=callback_handler,
67+
)
68+
69+
provider.context.current_tokens = None
70+
provider.context.token_expiry_time = None
71+
provider._initialized = True
72+
73+
# Pre-set client info to skip DCR
74+
provider.context.client_info = OAuthClientInformationFull(
75+
client_id="test_client",
76+
redirect_uris=[AnyUrl("http://localhost:3030/callback")],
77+
)
78+
79+
test_request = httpx.Request("GET", "https://api.example.com/mcp")
80+
auth_flow = provider.async_auth_flow(test_request)
81+
82+
# First request (no auth header yet)
83+
await auth_flow.__anext__()
84+
85+
# 401 response with BOTH resource_metadata URL and scope in WWW-Authenticate
86+
# This is the key: the bug would use the URL as scope instead of "read write"
87+
resource_metadata_url = "https://api.example.com/.well-known/oauth-protected-resource"
88+
expected_scope = "read write"
89+
90+
response_401 = httpx.Response(
91+
401,
92+
headers={"WWW-Authenticate": (f'Bearer resource_metadata="{resource_metadata_url}", scope="{expected_scope}"')},
93+
request=test_request,
94+
)
95+
96+
# Send 401, expect PRM discovery request
97+
prm_request = await auth_flow.asend(response_401)
98+
assert ".well-known/oauth-protected-resource" in str(prm_request.url)
99+
100+
# PRM response with scopes_supported (these should be overridden by WWW-Auth scope)
101+
prm_response = httpx.Response(
102+
200,
103+
content=(
104+
b'{"resource": "https://api.example.com/mcp", '
105+
b'"authorization_servers": ["https://auth.example.com"], '
106+
b'"scopes_supported": ["fallback:scope1", "fallback:scope2"]}'
107+
),
108+
request=prm_request,
109+
)
110+
111+
# Send PRM response, expect OAuth metadata discovery
112+
oauth_metadata_request = await auth_flow.asend(prm_response)
113+
assert ".well-known/oauth-authorization-server" in str(oauth_metadata_request.url)
114+
115+
# OAuth metadata response
116+
oauth_metadata_response = httpx.Response(
117+
200,
118+
content=(
119+
b'{"issuer": "https://auth.example.com", '
120+
b'"authorization_endpoint": "https://auth.example.com/authorize", '
121+
b'"token_endpoint": "https://auth.example.com/token"}'
122+
),
123+
request=oauth_metadata_request,
124+
)
125+
126+
# Mock authorization to skip interactive flow
127+
provider._perform_authorization_code_grant = mock.AsyncMock(return_value=("test_auth_code", "test_code_verifier"))
128+
129+
# Send OAuth metadata response, expect token request
130+
token_request = await auth_flow.asend(oauth_metadata_response)
131+
assert "token" in str(token_request.url)
132+
133+
# NOW CHECK: The scope should be the WWW-Authenticate scope, NOT the URL
134+
# This is where the bug manifested - scope was set to resource_metadata_url
135+
actual_scope = provider.context.client_metadata.scope
136+
137+
# This assertion would FAIL on main (scope would be the URL)
138+
# but PASS on the fix branch (scope is "read write")
139+
assert actual_scope == expected_scope, (
140+
f"Expected scope to be '{expected_scope}' from WWW-Authenticate header, "
141+
f"but got '{actual_scope}'. "
142+
f"If scope is '{resource_metadata_url}', the bug from #1630 is present."
143+
)
144+
145+
# Verify it's definitely not the URL (explicit check for the bug)
146+
assert actual_scope != resource_metadata_url, (
147+
f"BUG #1630: Scope was incorrectly set to resource_metadata URL '{resource_metadata_url}' "
148+
f"instead of the actual scope '{expected_scope}'"
149+
)
150+
151+
# Complete the flow to properly release the lock
152+
token_response = httpx.Response(
153+
200,
154+
content=b'{"access_token": "test_token", "token_type": "Bearer", "expires_in": 3600}',
155+
request=token_request,
156+
)
157+
158+
final_request = await auth_flow.asend(token_response)
159+
assert final_request.headers["Authorization"] == "Bearer test_token"
160+
161+
# Finish the flow
162+
final_response = httpx.Response(200, request=final_request)
163+
try:
164+
await auth_flow.asend(final_response)
165+
except StopAsyncIteration:
166+
pass

0 commit comments

Comments
 (0)