From d425b7c50a656215c5cca4547cb76bf6850c8de2 Mon Sep 17 00:00:00 2001 From: afischh Date: Sat, 23 May 2026 10:02:36 +0200 Subject: [PATCH 1/3] fix(auth): accept client credentials from Basic auth header in token endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a client uses HTTP Basic authentication (RFC 6749 §2.3.1), its client_id and client_secret arrive in the Authorization header rather than the request body. ClientAuthenticator already validates Basic auth correctly for `client_secret_basic` clients, but it failed early with "Missing client_id" when client_id was absent from form data. TokenHandler also required client_id in form data for TokenRequest validation, causing a second failure path. Changes: - ClientAuthenticator.authenticate_request: extract client_id from Basic auth header when not present in form body, before the missing-id check - TokenHandler.handle: populate client_id from the already-authenticated client_info when absent from form data, so TokenRequest validates cleanly Two new tests cover the authorization_code and refresh_token grant flows with client_id supplied only via Authorization header. Fixes #1315 Signed-off-by: afischh --- src/mcp/server/auth/handlers/token.py | 10 ++- src/mcp/server/auth/middleware/client_auth.py | 10 +++ .../mcpserver/auth/test_auth_integration.py | 88 +++++++++++++++++++ 3 files changed, 105 insertions(+), 3 deletions(-) diff --git a/src/mcp/server/auth/handlers/token.py b/src/mcp/server/auth/handlers/token.py index 534a478a91..956962e03d 100644 --- a/src/mcp/server/auth/handlers/token.py +++ b/src/mcp/server/auth/handlers/token.py @@ -95,9 +95,13 @@ async def handle(self, request: Request): ) try: - form_data = await request.form() - # TODO(Marcelo): Can someone check if this `dict()` wrapper is necessary? - token_request = token_request_adapter.validate_python(dict(form_data)) + form_data = dict(await request.form()) + # client_id may have been supplied via HTTP Basic auth header instead of the + # request body (RFC 6749 §2.3.1). ClientAuthenticator already verified it, + # so we can safely populate it from client_info when absent from form data. + if "client_id" not in form_data: + form_data["client_id"] = client_info.client_id + token_request = token_request_adapter.validate_python(form_data) except ValidationError as validation_error: # pragma: no cover return self.response( TokenErrorResponse( diff --git a/src/mcp/server/auth/middleware/client_auth.py b/src/mcp/server/auth/middleware/client_auth.py index 2832f83523..8fbb3c07b3 100644 --- a/src/mcp/server/auth/middleware/client_auth.py +++ b/src/mcp/server/auth/middleware/client_auth.py @@ -53,6 +53,16 @@ async def authenticate_request(self, request: Request) -> OAuthClientInformation """ form_data = await request.form() client_id = form_data.get("client_id") + if not client_id: + # RFC 6749 §2.3.1: client credentials MAY be sent via HTTP Basic auth + auth_header = request.headers.get("Authorization", "") + if auth_header.startswith("Basic "): + try: + decoded = base64.b64decode(auth_header[6:]).decode("utf-8") + if ":" in decoded: + client_id = unquote(decoded.split(":", 1)[0]) + except (ValueError, UnicodeDecodeError, binascii.Error): + pass if not client_id: raise AuthenticationError("Missing client_id") diff --git a/tests/server/mcpserver/auth/test_auth_integration.py b/tests/server/mcpserver/auth/test_auth_integration.py index 602f5cc752..e6f8674c74 100644 --- a/tests/server/mcpserver/auth/test_auth_integration.py +++ b/tests/server/mcpserver/auth/test_auth_integration.py @@ -1367,6 +1367,94 @@ async def test_none_auth_method_public_client( assert "access_token" in token_response + @pytest.mark.anyio + async def test_basic_auth_without_client_id_in_body( + self, test_client: httpx.AsyncClient, mock_oauth_provider: MockOAuthProvider, pkce_challenge: dict[str, str] + ): + """Test RFC 6749 §2.3.1: client_id supplied only via Basic auth header, not in body.""" + client_metadata = { + "redirect_uris": ["https://client.example.com/callback"], + "client_name": "Basic Auth Only Header Client", + "token_endpoint_auth_method": "client_secret_basic", + "grant_types": ["authorization_code", "refresh_token"], + } + + response = await test_client.post("/register", json=client_metadata) + assert response.status_code == 201 + client_info = response.json() + + auth_code = f"code_{int(time.time())}" + mock_oauth_provider.auth_codes[auth_code] = AuthorizationCode( + code=auth_code, + client_id=client_info["client_id"], + code_challenge=pkce_challenge["code_challenge"], + redirect_uri=AnyUrl("https://client.example.com/callback"), + redirect_uri_provided_explicitly=True, + scopes=["read", "write"], + expires_at=time.time() + 600, + ) + + credentials = f"{client_info['client_id']}:{client_info['client_secret']}" + encoded_credentials = base64.b64encode(credentials.encode()).decode() + + # client_id intentionally omitted from body — only in Authorization header + response = await test_client.post( + "/token", + headers={"Authorization": f"Basic {encoded_credentials}"}, + data={ + "grant_type": "authorization_code", + "code": auth_code, + "code_verifier": pkce_challenge["code_verifier"], + "redirect_uri": "https://client.example.com/callback", + }, + ) + assert response.status_code == 200, f"Expected 200, got {response.status_code}: {response.text}" + token_response = response.json() + assert "access_token" in token_response + + @pytest.mark.anyio + async def test_basic_auth_refresh_token_without_client_id_in_body( + self, test_client: httpx.AsyncClient, mock_oauth_provider: MockOAuthProvider, pkce_challenge: dict[str, str] + ): + """Test RFC 6749 §2.3.1: refresh_token grant with client_id only in Basic auth header.""" + client_metadata = { + "redirect_uris": ["https://client.example.com/callback"], + "client_name": "Basic Auth Refresh Client", + "token_endpoint_auth_method": "client_secret_basic", + "grant_types": ["authorization_code", "refresh_token"], + } + + response = await test_client.post("/register", json=client_metadata) + assert response.status_code == 201 + client_info = response.json() + + access_token_str = f"access_{secrets.token_hex(16)}" + refresh_token_str = f"refresh_{int(time.time())}" + mock_oauth_provider.tokens[access_token_str] = AccessToken( + token=access_token_str, + client_id=client_info["client_id"], + scopes=["read"], + expires_at=int(time.time()) + 3600, + ) + mock_oauth_provider.refresh_tokens[refresh_token_str] = access_token_str + + credentials = f"{client_info['client_id']}:{client_info['client_secret']}" + encoded_credentials = base64.b64encode(credentials.encode()).decode() + + # client_id intentionally omitted from body — only in Authorization header + response = await test_client.post( + "/token", + headers={"Authorization": f"Basic {encoded_credentials}"}, + data={ + "grant_type": "refresh_token", + "refresh_token": refresh_token_str, + }, + ) + assert response.status_code == 200, f"Expected 200, got {response.status_code}: {response.text}" + token_response = response.json() + assert "access_token" in token_response + + class TestAuthorizeEndpointErrors: """Test error handling in the OAuth authorization endpoint.""" From 333e8bbec7fff7bb4004c2a2e7fea2e57f1716cc Mon Sep 17 00:00:00 2001 From: afischh Date: Sat, 23 May 2026 10:16:44 +0200 Subject: [PATCH 2/3] fix: include transport path in protected resource metadata URL (RFC 9728) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per RFC 9728 §3, the `resource` field in `/.well-known/oauth-protected-resource` must identify the actual protected endpoint URL — e.g. `http://localhost:8000/mcp`, not the bare server base `http://localhost:8000/`. Without the path, VS Code Copilot and other spec-compliant clients reject the server with: Protected Resource Metadata resource "http://localhost:8000/" does not match MCP server resolved resource "http://localhost:8000/mcp" Fix: append `streamable_http_path` / `sse_path` to `resource_server_url` before passing it to `create_protected_resource_routes` and `build_resource_metadata_url` in both the lowlevel and mcpserver transports. Fixes #1264 Signed-off-by: Alex Fisch Co-Authored-By: Claude Sonnet 4.6 --- src/mcp/server/lowlevel/server.py | 9 ++- src/mcp/server/mcpserver/server.py | 9 ++- tests/server/auth/test_protected_resource.py | 65 ++++++++++++++++++++ 3 files changed, 79 insertions(+), 4 deletions(-) diff --git a/src/mcp/server/lowlevel/server.py b/src/mcp/server/lowlevel/server.py index 59de0ace45..093b5f6400 100644 --- a/src/mcp/server/lowlevel/server.py +++ b/src/mcp/server/lowlevel/server.py @@ -46,6 +46,7 @@ async def main(): import anyio from opentelemetry.trace import SpanKind, StatusCode +from pydantic import AnyHttpUrl from starlette.applications import Starlette from starlette.middleware import Middleware from starlette.middleware.authentication import AuthenticationMiddleware @@ -633,8 +634,11 @@ def streamable_http_app( # Determine resource metadata URL resource_metadata_url = None if auth and auth.resource_server_url: + # RFC 9728: resource identifier must match the URL clients use to access + # the protected resource, including the transport path (e.g. /mcp) + actual_resource_url = AnyHttpUrl(str(auth.resource_server_url).rstrip("/") + streamable_http_path) # Build compliant metadata URL for WWW-Authenticate header - resource_metadata_url = build_resource_metadata_url(auth.resource_server_url) + resource_metadata_url = build_resource_metadata_url(actual_resource_url) routes.append( Route( @@ -653,9 +657,10 @@ def streamable_http_app( # Add protected resource metadata endpoint if configured as RS if auth and auth.resource_server_url: # pragma: no cover + actual_resource_url = AnyHttpUrl(str(auth.resource_server_url).rstrip("/") + streamable_http_path) routes.extend( create_protected_resource_routes( - resource_url=auth.resource_server_url, + resource_url=actual_resource_url, authorization_servers=[auth.issuer_url], scopes_supported=auth.required_scopes, ) diff --git a/src/mcp/server/mcpserver/server.py b/src/mcp/server/mcpserver/server.py index b3471163b7..c0d74db4fe 100644 --- a/src/mcp/server/mcpserver/server.py +++ b/src/mcp/server/mcpserver/server.py @@ -12,6 +12,7 @@ import anyio import pydantic_core +from pydantic import AnyHttpUrl from pydantic.networks import AnyUrl from pydantic_settings import BaseSettings, SettingsConfigDict from starlette.applications import Starlette @@ -987,8 +988,11 @@ async def handle_sse(scope: Scope, receive: Receive, send: Send): # pragma: no if self.settings.auth and self.settings.auth.resource_server_url: from mcp.server.auth.routes import build_resource_metadata_url + # RFC 9728: resource identifier must match the URL clients use to access + # the protected resource, including the transport path (e.g. /sse) + actual_resource_url = AnyHttpUrl(str(self.settings.auth.resource_server_url).rstrip("/") + sse_path) # Build compliant metadata URL for WWW-Authenticate header - resource_metadata_url = build_resource_metadata_url(self.settings.auth.resource_server_url) + resource_metadata_url = build_resource_metadata_url(actual_resource_url) # Auth is enabled, wrap the endpoints with RequireAuthMiddleware routes.append( @@ -1028,9 +1032,10 @@ async def sse_endpoint(request: Request) -> Response: # pragma: no cover if self.settings.auth and self.settings.auth.resource_server_url: # pragma: no cover from mcp.server.auth.routes import create_protected_resource_routes + actual_resource_url = AnyHttpUrl(str(self.settings.auth.resource_server_url).rstrip("/") + sse_path) routes.extend( create_protected_resource_routes( - resource_url=self.settings.auth.resource_server_url, + resource_url=actual_resource_url, authorization_servers=[self.settings.auth.issuer_url], scopes_supported=self.settings.auth.required_scopes, ) diff --git a/tests/server/auth/test_protected_resource.py b/tests/server/auth/test_protected_resource.py index 413a80276e..c3f0d3c54c 100644 --- a/tests/server/auth/test_protected_resource.py +++ b/tests/server/auth/test_protected_resource.py @@ -196,3 +196,68 @@ def test_route_consistency_consistent_paths_for_various_resources(resource_url: assert url_path == expected_path assert route_path == expected_path assert url_path == route_path + + +# Tests for issue #1264: resource URL must include transport path + + +@pytest.mark.parametrize( + "resource_server_url,transport_path,expected_resource,expected_metadata_url", + [ + ( + "http://localhost:8000", + "/mcp", + "http://localhost:8000/mcp", + "http://localhost:8000/.well-known/oauth-protected-resource/mcp", + ), + ( + "http://localhost:8000/", + "/mcp", + "http://localhost:8000/mcp", + "http://localhost:8000/.well-known/oauth-protected-resource/mcp", + ), + ( + "https://mcp.example.com", + "/sse", + "https://mcp.example.com/sse", + "https://mcp.example.com/.well-known/oauth-protected-resource/sse", + ), + ], +) +def test_resource_url_includes_transport_path( + resource_server_url: str, + transport_path: str, + expected_resource: str, + expected_metadata_url: str, +): + """Transport path must be appended to resource_server_url (issue #1264). + + Per RFC 9728, the resource identifier must match the URL clients use to access + the protected resource — e.g. http://localhost:8000/mcp, not http://localhost:8000/. + """ + actual_resource_url = AnyHttpUrl(resource_server_url.rstrip("/") + transport_path) + + assert str(actual_resource_url) == expected_resource + + metadata_url = build_resource_metadata_url(actual_resource_url) + assert str(metadata_url) == expected_metadata_url + + +@pytest.mark.anyio +async def test_protected_resource_metadata_contains_transport_path(): + """Metadata endpoint returns resource URL with transport path, not bare server URL.""" + resource_url = AnyHttpUrl("http://localhost:8000/mcp") + routes = create_protected_resource_routes( + resource_url=resource_url, + authorization_servers=[AnyHttpUrl("https://auth.example.com")], + scopes_supported=["read", "write"], + ) + app = Starlette(routes=routes) + + async with httpx.AsyncClient(transport=httpx.ASGITransport(app=app), base_url="http://localhost:8000") as client: + response = await client.get("/.well-known/oauth-protected-resource/mcp") + assert response.status_code == 200 + data = response.json() + # resource must be the full endpoint URL, not the bare server base + assert data["resource"] == "http://localhost:8000/mcp" + assert data["resource"] != "http://localhost:8000/" From 38a262855623b0c2e832b7d81fca7b52e3a2547e Mon Sep 17 00:00:00 2001 From: afischh Date: Sat, 23 May 2026 18:11:48 +0200 Subject: [PATCH 3/3] style: fix ruff formatting in test_auth_integration.py --- tests/server/mcpserver/auth/test_auth_integration.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/server/mcpserver/auth/test_auth_integration.py b/tests/server/mcpserver/auth/test_auth_integration.py index e6f8674c74..7930b50974 100644 --- a/tests/server/mcpserver/auth/test_auth_integration.py +++ b/tests/server/mcpserver/auth/test_auth_integration.py @@ -1366,7 +1366,6 @@ async def test_none_auth_method_public_client( token_response = response.json() assert "access_token" in token_response - @pytest.mark.anyio async def test_basic_auth_without_client_id_in_body( self, test_client: httpx.AsyncClient, mock_oauth_provider: MockOAuthProvider, pkce_challenge: dict[str, str]