From 3fdd5d431643b19508f1227f3d3b7ac2489f2719 Mon Sep 17 00:00:00 2001 From: giulio-leone Date: Sun, 1 Mar 2026 05:39:20 +0100 Subject: [PATCH 1/2] fix(auth): validate both client_id and client_secret for client_secret_post Refs: #2185 --- src/mcp/client/auth/oauth2.py | 5 +- .../extensions/test_client_credentials.py | 66 +++++++++++++++++++ 2 files changed, 69 insertions(+), 2 deletions(-) diff --git a/src/mcp/client/auth/oauth2.py b/src/mcp/client/auth/oauth2.py index 7f5af5186..25075dec3 100644 --- a/src/mcp/client/auth/oauth2.py +++ b/src/mcp/client/auth/oauth2.py @@ -205,8 +205,9 @@ def prepare_token_auth( 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"} - elif auth_method == "client_secret_post" and self.client_info.client_secret: - # Include client_secret in request body + elif auth_method == "client_secret_post" and self.client_info.client_id 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 data["client_secret"] = self.client_info.client_secret # For auth_method == "none", don't add any client_secret diff --git a/tests/client/auth/extensions/test_client_credentials.py b/tests/client/auth/extensions/test_client_credentials.py index 0003b1679..bf35b0ae4 100644 --- a/tests/client/auth/extensions/test_client_credentials.py +++ b/tests/client/auth/extensions/test_client_credentials.py @@ -252,6 +252,72 @@ async def test_exchange_token_client_credentials(self, mock_storage: MockTokenSt assert "scope=read write" in content assert "resource=https://api.example.com/v1/mcp" in content + @pytest.mark.anyio + async def test_exchange_token_client_secret_post_includes_client_id(self, mock_storage: MockTokenStorage): + """Test that client_secret_post includes both client_id and client_secret in body (RFC 6749 §2.3.1).""" + provider = ClientCredentialsOAuthProvider( + server_url="https://api.example.com/v1/mcp", + storage=mock_storage, + client_id="test-client-id", + client_secret="test-client-secret", + token_endpoint_auth_method="client_secret_post", + scopes="read write", + ) + await provider._initialize() + provider.context.oauth_metadata = OAuthMetadata( + issuer=AnyHttpUrl("https://api.example.com"), + authorization_endpoint=AnyHttpUrl("https://api.example.com/authorize"), + token_endpoint=AnyHttpUrl("https://api.example.com/token"), + ) + provider.context.protocol_version = "2025-06-18" + + request = await provider._perform_authorization() + + content = urllib.parse.unquote_plus(request.content.decode()) + assert "grant_type=client_credentials" in content + assert "client_id=test-client-id" in content + assert "client_secret=test-client-secret" in content + # Should NOT have Basic auth header + assert "Authorization" not in request.headers + + @pytest.mark.anyio + async def test_exchange_token_client_secret_post_without_client_id(self, mock_storage: MockTokenStorage): + """Test that client_secret_post skips body credentials when client_id is None (RFC 6749 §2.3.1 requires both).""" + provider = ClientCredentialsOAuthProvider( + server_url="https://api.example.com/v1/mcp", + storage=mock_storage, + client_id="placeholder", + client_secret="test-client-secret", + token_endpoint_auth_method="client_secret_post", + scopes="read write", + ) + await provider._initialize() + provider.context.oauth_metadata = OAuthMetadata( + issuer=AnyHttpUrl("https://api.example.com"), + authorization_endpoint=AnyHttpUrl("https://api.example.com/authorize"), + token_endpoint=AnyHttpUrl("https://api.example.com/token"), + ) + provider.context.protocol_version = "2025-06-18" + # Override client_info to have client_id=None (edge case) + provider.context.client_info = OAuthClientInformationFull( + redirect_uris=None, + client_id=None, + client_secret="test-client-secret", + grant_types=["client_credentials"], + token_endpoint_auth_method="client_secret_post", + scope="read write", + ) + + request = await provider._perform_authorization() + + content = urllib.parse.unquote_plus(request.content.decode()) + assert "grant_type=client_credentials" in content + # Neither client_id nor client_secret should be in body since client_id is None + # (RFC 6749 §2.3.1 requires both for client_secret_post) + assert "client_id=" not in content + assert "client_secret=" not in content + assert "Authorization" not in request.headers + @pytest.mark.anyio async def test_exchange_token_without_scopes(self, mock_storage: MockTokenStorage): """Test token exchange without scopes.""" From fe52e275429f2e64ac3a0136837c7aa176aeff20 Mon Sep 17 00:00:00 2001 From: g97iulio1609 Date: Sun, 1 Mar 2026 05:49:43 +0100 Subject: [PATCH 2/2] fix: shorten docstring to comply with E501 line length limit Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/client/auth/extensions/test_client_credentials.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/client/auth/extensions/test_client_credentials.py b/tests/client/auth/extensions/test_client_credentials.py index bf35b0ae4..09760f453 100644 --- a/tests/client/auth/extensions/test_client_credentials.py +++ b/tests/client/auth/extensions/test_client_credentials.py @@ -282,7 +282,7 @@ async def test_exchange_token_client_secret_post_includes_client_id(self, mock_s @pytest.mark.anyio async def test_exchange_token_client_secret_post_without_client_id(self, mock_storage: MockTokenStorage): - """Test that client_secret_post skips body credentials when client_id is None (RFC 6749 §2.3.1 requires both).""" + """Test client_secret_post skips body credentials when client_id is None.""" provider = ClientCredentialsOAuthProvider( server_url="https://api.example.com/v1/mcp", storage=mock_storage,