mirror of
https://github.com/PrefectHQ/fastmcp.git
synced 2026-08-09 15:19:10 +02:00
Return 401 for invalid_grant token errors per MCP spec (#2800)
This commit is contained in:
parent
151b0ae46f
commit
10fb217f73
2 changed files with 72 additions and 16 deletions
|
|
@ -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
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue