From 09a5effc434e031684d025b69b19b963f3f126f9 Mon Sep 17 00:00:00 2001 From: Karim Masarweh Date: Fri, 18 Sep 2026 10:47:05 +0300 Subject: [PATCH] fix(auth): exclude client_id from token body when using client_secret_basic RFC 6749 Section 2.3 states that when using HTTP Basic authentication, the client credentials (client_id and client_secret) must be sent in the Authorization header only and must not appear in the request body. Previously, prepare_token_auth() stripped client_secret from the body but left client_id in when client_secret_basic was in use, causing servers like Notion's MCP implementation to reject token requests due to duplicate credentials. Fix: extend the body filter from k != 'client_secret' to k not in ('client_secret', 'client_id') Tests: update existing basic-auth assertions to reflect correct behaviour and add a dedicated regression test (test_basic_auth_does_not_send_client_id_in_body). Fixes #3138 --- src/mcp/client/auth/oauth2.py | 4 ++-- tests/client/test_auth.py | 44 ++++++++++++++++++++++++++++++++--- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/src/mcp/client/auth/oauth2.py b/src/mcp/client/auth/oauth2.py index 8588208924..9cbd2423cb 100644 --- a/src/mcp/client/auth/oauth2.py +++ b/src/mcp/client/auth/oauth2.py @@ -264,8 +264,8 @@ def prepare_token_auth( credentials = f"{encoded_id}:{encoded_secret}" encoded_credentials = base64.b64encode(credentials.encode()).decode() headers["Authorization"] = f"Basic {encoded_credentials}" - # Don't include client_secret in body for basic auth - data = {k: v for k, v in data.items() if k != "client_secret"} + # Don't include client_id or client_secret in body for basic auth (RFC 6749 §2.3) + data = {k: v for k, v in data.items() if k not in ("client_secret", "client_id")} elif auth_method == "client_secret_post" and self.client_info.client_secret: # Include client_id and client_secret in request body (RFC 6749 §2.3.1) data["client_id"] = self.client_info.client_id diff --git a/tests/client/test_auth.py b/tests/client/test_auth.py index 18a1566705..42c012eddc 100644 --- a/tests/client/test_auth.py +++ b/tests/client/test_auth.py @@ -678,10 +678,10 @@ async def test_basic_auth_token_exchange(self, oauth_provider: OAuthClientProvid assert unquote(client_id) == client_id_raw assert unquote(client_secret) == client_secret_raw - # client_secret should NOT be in body for basic auth + # Neither client_secret nor client_id should be in body for basic auth (RFC 6749 §2.3) content = request.content.decode() assert "client_secret=" not in content - assert "client_id=test%40client" in content # client_id still in body + assert "client_id=" not in content @pytest.mark.anyio async def test_basic_auth_refresh_token(self, oauth_provider: OAuthClientProvider, valid_tokens: OAuthToken): @@ -714,9 +714,47 @@ async def test_basic_auth_refresh_token(self, oauth_provider: OAuthClientProvide decoded = base64.b64decode(encoded_creds).decode() assert decoded == f"{client_id}:{client_secret}" - # client_secret should NOT be in body + # Neither client_secret nor client_id should be in body for basic auth (RFC 6749 §2.3) content = request.content.decode() assert "client_secret=" not in content + assert "client_id=" not in content + + @pytest.mark.anyio + async def test_basic_auth_does_not_send_client_id_in_body(self, oauth_provider: OAuthClientProvider): + """Regression test for RFC 6749 §2.3: client_id must not appear in the + token-request body when client_secret_basic is used, because the + Authorization header already carries the client identity. + + Previously, prepare_token_auth() only stripped client_secret from the + body but left client_id in — violating the spec and breaking servers + such as Notion's MCP implementation that reject duplicate credentials. + """ + oauth_provider.context.oauth_metadata = OAuthMetadata( + issuer=AnyHttpUrl("https://auth.example.com"), + authorization_endpoint=AnyHttpUrl("https://auth.example.com/authorize"), + token_endpoint=AnyHttpUrl("https://auth.example.com/token"), + token_endpoint_auth_methods_supported=["client_secret_basic"], + ) + + oauth_provider.context.client_info = OAuthClientInformationFull( + client_id="my_client", + client_secret="my_secret", + redirect_uris=[AnyUrl("http://localhost:3030/callback")], + token_endpoint_auth_method="client_secret_basic", + ) + + request = await oauth_provider._exchange_token_authorization_code("auth_code", "verifier") + + # Credentials must appear in the Authorization header only + assert request.headers["Authorization"].startswith("Basic ") + + body = request.content.decode() + assert "client_id=" not in body, ( + "client_id must not be sent in the request body when using " + "client_secret_basic — it is already present in the Authorization header " + "(RFC 6749 §2.3)" + ) + assert "client_secret=" not in body @pytest.mark.anyio async def test_none_auth_method(self, oauth_provider: OAuthClientProvider):