From 10fb217f732fefe7834f23b80cd276a0e5dd4403 Mon Sep 17 00:00:00 2001 From: Jeremiah Lowin <153965+jlowin@users.noreply.github.com> Date: Tue, 6 Jan 2026 17:51:33 -0500 Subject: [PATCH] Return 401 for invalid_grant token errors per MCP spec (#2800) --- src/fastmcp/server/auth/auth.py | 40 ++++++++++++++++------ tests/server/auth/test_oauth_proxy.py | 48 +++++++++++++++++++++++---- 2 files changed, 72 insertions(+), 16 deletions(-) diff --git a/src/fastmcp/server/auth/auth.py b/src/fastmcp/server/auth/auth.py index c3bbd1b17..95f1ad3a0 100644 --- a/src/fastmcp/server/auth/auth.py +++ b/src/fastmcp/server/auth/auth.py @@ -47,19 +47,19 @@ class AccessToken(_SDKAccessToken): class TokenHandler(_SDKTokenHandler): - """TokenHandler that returns OAuth 2.1 compliant error responses. + """TokenHandler that returns MCP-compliant error responses. - The MCP SDK returns `unauthorized_client` for client authentication failures. - However, per RFC 6749 Section 5.2, authentication failures should return - `invalid_client` with HTTP 401, not `unauthorized_client`. + This handler addresses two SDK issues: - This distinction matters: `unauthorized_client` means "client exists but - can't do this", while `invalid_client` means "client doesn't exist or - credentials are wrong". Claude's OAuth client uses this to decide whether - to re-register. + 1. Error code: The SDK returns `unauthorized_client` for client authentication + failures, but RFC 6749 Section 5.2 requires `invalid_client` with HTTP 401. + This distinction matters for client re-registration behavior. - This handler transforms 401 responses with `unauthorized_client` to use - `invalid_client` instead, making the error semantics correct per OAuth spec. + 2. Status code: The SDK returns HTTP 400 for all token errors including + `invalid_grant` (expired/invalid tokens). However, the MCP spec requires: + "Invalid or expired tokens MUST receive a HTTP 401 response." + + This handler transforms responses to be compliant with both OAuth 2.1 and MCP specs. """ async def handle(self, request: Any): @@ -85,6 +85,26 @@ class TokenHandler(_SDKTokenHandler): except (json.JSONDecodeError, AttributeError): pass # Not JSON or unexpected format, return as-is + # Transform 400 invalid_grant -> 401 for expired/invalid tokens + # Per MCP spec: "Invalid or expired tokens MUST receive a HTTP 401 response." + if response.status_code == 400: + try: + body = json.loads(response.body) + if body.get("error") == "invalid_grant": + return PydanticJSONResponse( + content=TokenErrorResponse( + error="invalid_grant", + error_description=body.get("error_description"), + ), + status_code=401, + headers={ + "Cache-Control": "no-store", + "Pragma": "no-cache", + }, + ) + except (json.JSONDecodeError, AttributeError): + pass # Not JSON or unexpected format, return as-is + return response diff --git a/tests/server/auth/test_oauth_proxy.py b/tests/server/auth/test_oauth_proxy.py index 2ea80a164..631bd61cc 100644 --- a/tests/server/auth/test_oauth_proxy.py +++ b/tests/server/auth/test_oauth_proxy.py @@ -1303,24 +1303,60 @@ class TestTokenHandlerErrorTransformation: assert response.status_code == 400 assert b'"error":"unauthorized_client"' in response.body - def test_does_not_transform_other_errors(self): - """Test that other error types pass through unchanged.""" + async def test_transforms_invalid_grant_to_401(self): + """Test that invalid_grant errors return 401 per MCP spec. + + Per MCP spec: "Invalid or expired tokens MUST receive a HTTP 401 response." + The SDK incorrectly returns 400 for all TokenErrorResponse including invalid_grant. + """ + from unittest.mock import AsyncMock, patch + + from mcp.server.auth.handlers.token import TokenHandler as SDKTokenHandler + + from fastmcp.server.auth.auth import TokenHandler + + handler = TokenHandler(provider=Mock(), client_authenticator=Mock()) + + # Create a mock 400 response like the SDK returns for invalid_grant + mock_response = Mock() + mock_response.status_code = 400 + mock_response.body = ( + b'{"error":"invalid_grant","error_description":"refresh token has expired"}' + ) + + # Patch the parent class's handle() to return our mock response + with patch.object( + SDKTokenHandler, + "handle", + new_callable=AsyncMock, + return_value=mock_response, + ): + response = await handler.handle(Mock()) + + # Should transform to MCP-compliant 401 response + assert response.status_code == 401 + assert b'"error":"invalid_grant"' in response.body + assert b'"error_description":"refresh token has expired"' in response.body + + def test_does_not_transform_other_400_errors(self): + """Test that non-invalid_grant 400 errors pass through unchanged.""" from mcp.server.auth.handlers.token import TokenErrorResponse from fastmcp.server.auth.auth import TokenHandler handler = TokenHandler(provider=Mock(), client_authenticator=Mock()) + # Test with invalid_request error (should stay 400) error_response = TokenErrorResponse( - error="invalid_grant", - error_description="Authorization code has expired", + error="invalid_request", + error_description="Missing required parameter", ) response = handler.response(error_response) - # Should pass through unchanged + # Should pass through unchanged as 400 assert response.status_code == 400 - assert b'"error":"invalid_grant"' in response.body + assert b'"error":"invalid_request"' in response.body class TestErrorPageRendering: