diff --git a/src/mcp/server/auth/middleware/client_auth.py b/src/mcp/server/auth/middleware/client_auth.py index 2832f8352..1bae22879 100644 --- a/src/mcp/server/auth/middleware/client_auth.py +++ b/src/mcp/server/auth/middleware/client_auth.py @@ -90,6 +90,8 @@ async def authenticate_request(self, request: Request) -> OAuthClientInformation request_client_secret = str(raw_form_data) elif client.token_endpoint_auth_method == "none": + if client.client_secret: + raise AuthenticationError("Require valid auth method, with client secret") request_client_secret = None else: raise AuthenticationError( # pragma: no cover diff --git a/tests/server/mcpserver/auth/test_auth_integration.py b/tests/server/mcpserver/auth/test_auth_integration.py index 35fec1c57..954886693 100644 --- a/tests/server/mcpserver/auth/test_auth_integration.py +++ b/tests/server/mcpserver/auth/test_auth_integration.py @@ -1376,6 +1376,63 @@ async def test_none_auth_method_public_client( token_response = response.json() assert "access_token" in token_response + @pytest.mark.anyio + async def test_none_auth_method_fails_for_confidential_client( + self, + test_client: httpx.AsyncClient, + mock_oauth_provider: MockOAuthProvider, + pkce_challenge: dict[str, str], + ): + """Test that 'none' authentication method fails for confidential clients.""" + # Create a confidential client (has a secret) but try to register with 'none' + # Actually, if we register with 'none', it won't have a secret. + # So we register with 'client_secret_post' first. + client_metadata = { + "redirect_uris": ["https://client.example.com/callback"], + "client_name": "Confidential Client", + "token_endpoint_auth_method": "client_secret_post", + "grant_types": ["authorization_code", "refresh_token"], + } + + response = await test_client.post("/register", json=client_metadata) + assert response.status_code == 201 + client_info = response.json() + assert client_info["client_secret"] is not None + + # Manually change the client's auth method to 'none' in the provider + # to simulate a configuration conflict or a client trying to downgrade. + client_obj = await mock_oauth_provider.get_client(client_info["client_id"]) + assert client_obj is not None + client_obj.token_endpoint_auth_method = "none" + + 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, + ) + + # Token request using 'none' method (no secret provided) + response = await test_client.post( + "/token", + data={ + "grant_type": "authorization_code", + "client_id": client_info["client_id"], + "code": auth_code, + "code_verifier": pkce_challenge["code_verifier"], + "redirect_uri": "https://client.example.com/callback", + }, + ) + + assert response.status_code == 401 + error_response = response.json() + assert error_response["error"] == "invalid_client" + assert "Require valid auth method, with client secret" in error_response["error_description"] + class TestAuthorizeEndpointErrors: """Test error handling in the OAuth authorization endpoint."""