From d425b7c50a656215c5cca4547cb76bf6850c8de2 Mon Sep 17 00:00:00 2001 From: afischh Date: Sat, 23 May 2026 10:02:36 +0200 Subject: [PATCH 1/2] 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 35fd692604afff4a1adb5f18017c78da9c20b726 Mon Sep 17 00:00:00 2001 From: afischh Date: Sun, 24 May 2026 20:08:09 +0200 Subject: [PATCH 2/2] fix(auth): satisfy pyright + add coverage for Basic auth fallback edges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two changes to make CI green: 1. token.py: guard the Basic auth fallback assignment so pyright can narrow client_info.client_id from `str | None` to `str` before it lands in starlette's FormData (whose values are `UploadFile | str`). Semantically a no-op — ClientAuthenticator already guarantees a non-empty client_id reached this point. 2. test_auth_integration.py: cover the two previously-uncovered edges in client_auth.py's Basic auth fallback (lines 62-66): - decoded credentials without ':' separator - malformed base64 in the Authorization header Both should be silently swallowed so the request surfaces the normal "Missing client_id" error path rather than a Basic-auth-specific one. Coverage of src/mcp/server/auth/middleware/client_auth.py is now 100%. Also restores ruff-format compliance (single blank line between methods). --- src/mcp/server/auth/handlers/token.py | 4 ++- .../mcpserver/auth/test_auth_integration.py | 29 ++++++++++++++++++- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/mcp/server/auth/handlers/token.py b/src/mcp/server/auth/handlers/token.py index 956962e03d..577249b7cc 100644 --- a/src/mcp/server/auth/handlers/token.py +++ b/src/mcp/server/auth/handlers/token.py @@ -99,7 +99,9 @@ async def handle(self, request: Request): # 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: + # The truthiness check narrows `str | None` to `str` for the type checker; + # ClientAuthenticator guarantees a non-empty client_id reached this point. + if "client_id" not in form_data and client_info.client_id: 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 diff --git a/tests/server/mcpserver/auth/test_auth_integration.py b/tests/server/mcpserver/auth/test_auth_integration.py index e6f8674c74..2516e3e28f 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] @@ -1454,6 +1453,34 @@ async def test_basic_auth_refresh_token_without_client_id_in_body( token_response = response.json() assert "access_token" in token_response + @pytest.mark.anyio + async def test_basic_auth_fallback_invalid_base64_falls_through(self, test_client: httpx.AsyncClient): + """Basic auth fallback (no client_id in body): invalid base64 is silently ignored, + request continues without client_id and surfaces a normal 'invalid_request' error.""" + # client_id missing from body AND the Basic header is malformed base64 + response = await test_client.post( + "/token", + headers={"Authorization": "Basic !!!not-valid-base64!!!"}, + data={"grant_type": "authorization_code", "code": "irrelevant"}, + ) + # The malformed base64 is swallowed; client_id remains missing → standard 401 + assert response.status_code == 401 + assert response.json()["error"] == "invalid_client" + + @pytest.mark.anyio + async def test_basic_auth_fallback_no_colon_falls_through(self, test_client: httpx.AsyncClient): + """Basic auth fallback (no client_id in body): decoded credentials without a colon + are not treated as a valid client_id, and the request fails with 'invalid_client'.""" + # b64("no_colon_here") decodes cleanly but contains no ':' separator + encoded = base64.b64encode(b"no_colon_here").decode() + response = await test_client.post( + "/token", + headers={"Authorization": f"Basic {encoded}"}, + data={"grant_type": "authorization_code", "code": "irrelevant"}, + ) + assert response.status_code == 401 + assert response.json()["error"] == "invalid_client" + class TestAuthorizeEndpointErrors: """Test error handling in the OAuth authorization endpoint."""