Skip to content

Commit e59fbdf

Browse files
committed
apply suggested changes
1 parent d7990fc commit e59fbdf

File tree

3 files changed

+58
-22
lines changed

3 files changed

+58
-22
lines changed

examples/servers/simple-auth/mcp_simple_auth/token_verifier.py

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ async def verify_token(self, token: str) -> AccessToken | None:
7373
client_id=data.get("client_id", "unknown"),
7474
scopes=data.get("scope", "").split() if data.get("scope") else [],
7575
expires_at=data.get("exp"),
76-
resource=data.get("aud") or data.get("resource"), # Include resource in token
76+
resource=data.get("aud"), # Include resource in token
7777
)
7878
except Exception as e:
7979
logger.warning(f"Token introspection failed: {e}")
@@ -82,7 +82,7 @@ async def verify_token(self, token: str) -> AccessToken | None:
8282
def _validate_resource(self, token_data: dict) -> bool:
8383
"""Validate token was issued for this resource server."""
8484
if not self.server_url or not self.resource_url:
85-
return True # No validation if server URL not configured
85+
return False # Fail if strict validation requested but URLs missing
8686

8787
# Check 'aud' claim first (standard JWT audience)
8888
aud = token_data.get("aud")
@@ -94,11 +94,6 @@ def _validate_resource(self, token_data: dict) -> bool:
9494
elif aud:
9595
return self._is_valid_resource(aud)
9696

97-
# Check custom 'resource' claim if no 'aud'
98-
resource = token_data.get("resource")
99-
if resource:
100-
return self._is_valid_resource(resource)
101-
10297
# No resource binding - invalid per RFC 8707
10398
return False
10499

src/mcp/shared/auth_utils.py

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
"""Utilities for OAuth 2.0 Resource Indicators (RFC 8707)."""
22

3-
from urllib.parse import urlparse, urlunparse
3+
from urllib.parse import urlparse, urlsplit, urlunsplit
44

55
from pydantic import AnyUrl, HttpUrl
66

@@ -20,20 +20,9 @@ def resource_url_from_server_url(url: str | HttpUrl | AnyUrl) -> str:
2020
# Convert to string if needed
2121
url_str = str(url)
2222

23-
# Parse the URL
24-
parsed = urlparse(url_str)
25-
26-
# Create canonical form: lowercase scheme and host, no fragment
27-
canonical = urlunparse(
28-
(
29-
parsed.scheme.lower(), # Lowercase scheme
30-
parsed.netloc.lower(), # Lowercase host (includes port)
31-
parsed.path,
32-
parsed.params,
33-
parsed.query,
34-
"", # Remove fragment
35-
)
36-
)
23+
# Parse the URL and remove fragment, create canonical form
24+
parsed = urlsplit(url_str)
25+
canonical = urlunsplit(parsed._replace(scheme=parsed.scheme.lower(), netloc=parsed.netloc.lower(), fragment=""))
3726

3827
return canonical
3928

tests/client/test_auth.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,56 @@ async def test_token_exchange_request(self, oauth_provider):
259259
assert "code_verifier=test_verifier" in content
260260
assert "client_id=test_client" in content
261261
assert "client_secret=test_secret" in content
262+
# Resource parameter should be included per RFC 8707
263+
assert "resource=https%3A%2F%2Fapi.example.com%2Fv1%2Fmcp" in content
264+
265+
@pytest.mark.anyio
266+
async def test_authorization_url_request(self, oauth_provider):
267+
"""Test authorization URL construction with resource parameter."""
268+
from unittest.mock import patch
269+
270+
# Set up required context
271+
oauth_provider.context.client_info = OAuthClientInformationFull(
272+
client_id="test_client",
273+
client_secret="test_secret",
274+
redirect_uris=[AnyUrl("http://localhost:3030/callback")],
275+
)
276+
277+
# Mock the redirect handler to capture the URL
278+
captured_url = None
279+
280+
async def mock_redirect_handler(url: str):
281+
nonlocal captured_url
282+
captured_url = url
283+
284+
oauth_provider.context.redirect_handler = mock_redirect_handler
285+
286+
# Mock callback handler
287+
async def mock_callback_handler():
288+
return "test_auth_code", "test_state"
289+
290+
oauth_provider.context.callback_handler = mock_callback_handler
291+
292+
# Mock pkce and state generation for predictable testing
293+
with (
294+
patch("mcp.client.auth.PKCEParameters.generate") as mock_pkce,
295+
patch("mcp.client.auth.secrets.token_urlsafe") as mock_state,
296+
):
297+
mock_pkce.return_value.code_verifier = "test_verifier"
298+
mock_pkce.return_value.code_challenge = "test_challenge"
299+
mock_state.return_value = "test_state"
300+
301+
# Mock compare_digest to return True
302+
with patch("mcp.client.auth.secrets.compare_digest", return_value=True):
303+
await oauth_provider._perform_authorization()
304+
305+
# Verify the captured URL contains resource parameter
306+
assert captured_url is not None
307+
assert "resource=https%3A%2F%2Fapi.example.com%2Fv1%2Fmcp" in captured_url
308+
assert "client_id=test_client" in captured_url
309+
assert "response_type=code" in captured_url
310+
assert "code_challenge=test_challenge" in captured_url
311+
assert "code_challenge_method=S256" in captured_url
262312

263313
@pytest.mark.anyio
264314
async def test_refresh_token_request(self, oauth_provider, valid_tokens):
@@ -283,6 +333,8 @@ async def test_refresh_token_request(self, oauth_provider, valid_tokens):
283333
assert "refresh_token=test_refresh_token" in content
284334
assert "client_id=test_client" in content
285335
assert "client_secret=test_secret" in content
336+
# Resource parameter should be included per RFC 8707
337+
assert "resource=https%3A%2F%2Fapi.example.com%2Fv1%2Fmcp" in content
286338

287339

288340
class TestAuthFlow:

0 commit comments

Comments
 (0)