Merge pull request #2507 from jlowin/sdk-auth-updates

[2.14] Update for MCP SDK auth changes
This commit is contained in:
Chris Guidry 2025-12-03 17:14:57 -05:00 committed by GitHub
commit ee63405262
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 96 additions and 83 deletions

View file

@ -7,7 +7,7 @@ dependencies = [
"python-dotenv>=1.1.0",
"exceptiongroup>=1.2.2",
"httpx>=0.28.1",
"mcp>=1.19.0,<2.0.0,!=1.21.1",
"mcp>=1.23.1",
"openapi-pydantic>=0.5.1",
"platformdirs>=4.0.0",
"rich>=13.9.4",

View file

@ -36,7 +36,7 @@ from key_value.aio.adapters.pydantic import PydanticAdapter
from key_value.aio.protocols import AsyncKeyValue
from key_value.aio.stores.disk import DiskStore
from key_value.aio.wrappers.encryption import FernetEncryptionWrapper
from mcp.server.auth.handlers.token import TokenErrorResponse, TokenSuccessResponse
from mcp.server.auth.handlers.token import TokenErrorResponse
from mcp.server.auth.handlers.token import TokenHandler as _SDKTokenHandler
from mcp.server.auth.json_response import PydanticJSONResponse
from mcp.server.auth.middleware.client_auth import ClientAuthenticator
@ -522,50 +522,43 @@ def create_error_html(
class TokenHandler(_SDKTokenHandler):
"""TokenHandler that returns OAuth 2.1 compliant error responses.
The MCP SDK always returns HTTP 400 for all client authentication issues.
However, OAuth 2.1 Section 5.3 and the MCP specification require that
invalid or expired tokens MUST receive a HTTP 401 response.
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 extends the base MCP SDK TokenHandler to transform client
authentication failures into OAuth 2.1 compliant responses:
- Changes 'unauthorized_client' to 'invalid_client' error code
- Returns HTTP 401 status code instead of 400 for client auth failures
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.
Per OAuth 2.1 Section 5.3: "The authorization server MAY return an HTTP 401
(Unauthorized) status code to indicate which HTTP authentication schemes
are supported."
Per MCP spec: "Invalid or expired tokens MUST receive a HTTP 401 response."
This handler transforms 401 responses with `unauthorized_client` to use
`invalid_client` instead, making the error semantics correct per OAuth spec.
"""
def response(self, obj: TokenSuccessResponse | TokenErrorResponse):
"""Override response method to provide OAuth 2.1 compliant error handling."""
# Check if this is a client authentication failure (not just unauthorized for grant type)
# unauthorized_client can mean two things:
# 1. Client authentication failed (client_id not found or wrong credentials) -> invalid_client 401
# 2. Client not authorized for this grant type -> unauthorized_client 400 (correct per spec)
if (
isinstance(obj, TokenErrorResponse)
and obj.error == "unauthorized_client"
and obj.error_description
and "Invalid client_id" in obj.error_description
):
# Transform client auth failure to OAuth 2.1 compliant response
return PydanticJSONResponse(
content=TokenErrorResponse(
error="invalid_client",
error_description=obj.error_description,
error_uri=obj.error_uri,
),
status_code=401,
headers={
"Cache-Control": "no-store",
"Pragma": "no-cache",
},
)
async def handle(self, request: Any):
"""Wrap SDK handle() and transform auth error responses."""
response = await super().handle(request)
# Otherwise use default behavior from parent class
return super().response(obj)
# Transform 401 unauthorized_client -> invalid_client
if response.status_code == 401:
try:
body = json.loads(response.body)
if body.get("error") == "unauthorized_client":
return PydanticJSONResponse(
content=TokenErrorResponse(
error="invalid_client",
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
class OAuthProxy(OAuthProvider):
@ -993,9 +986,13 @@ class OAuthProxy(OAuthProvider):
# Create a ProxyDCRClient with configured redirect URI validation
if client_info.client_id is None:
raise ValueError("client_id is required for client registration")
# We use token_endpoint_auth_method="none" because the proxy handles
# all upstream authentication. The client_secret must also be None
# because the SDK requires secrets to be provided if they're set,
# regardless of auth method.
proxy_client: ProxyDCRClient = ProxyDCRClient(
client_id=client_info.client_id,
client_secret=client_info.client_secret,
client_secret=None,
redirect_uris=client_info.redirect_uris or [AnyUrl("http://localhost")],
grant_types=client_info.grant_types
or ["authorization_code", "refresh_token"],

View file

@ -18,17 +18,16 @@ from mcp.server.lowlevel.helper_types import ReadResourceContents
from mcp.server.lowlevel.server import request_ctx
from mcp.shared.context import RequestContext
from mcp.types import (
AudioContent,
ClientCapabilities,
CreateMessageResult,
GetPromptResult,
ImageContent,
IncludeContext,
ModelHint,
ModelPreferences,
Root,
SamplingCapability,
SamplingMessage,
SamplingMessageContentBlock,
TextContent,
)
from mcp.types import CreateMessageRequestParams as SamplingParams
@ -59,6 +58,7 @@ _clamp_logger(logger=to_client_logger, max_level="DEBUG")
T = TypeVar("T", default=Any)
_current_context: ContextVar[Context | None] = ContextVar("context", default=None) # type: ignore[assignment]
_flush_lock = anyio.Lock()
@ -479,7 +479,7 @@ class Context:
temperature: float | None = None,
max_tokens: int | None = None,
model_preferences: ModelPreferences | str | list[str] | None = None,
) -> TextContent | ImageContent | AudioContent:
) -> SamplingMessageContentBlock | list[SamplingMessageContentBlock]:
"""
Send a sampling request to the client and await the response.

View file

@ -149,6 +149,7 @@ async def test_sampling_with_image(fastmcp_server: FastMCP):
"annotations": None,
"_meta": None,
},
"_meta": None,
},
{
"role": "user",
@ -159,5 +160,6 @@ async def test_sampling_with_image(fastmcp_server: FastMCP):
"annotations": None,
"_meta": None,
},
"_meta": None,
},
]

View file

@ -420,7 +420,8 @@ class TestOAuthProxyClientRegistration:
stored = await oauth_proxy.get_client("original-client")
assert stored is not None
assert stored.client_id == "original-client"
assert stored.client_secret == "original-secret"
# Proxy uses token_endpoint_auth_method="none", so client_secret is not stored
assert stored.client_secret is None
async def test_get_registered_client(self, oauth_proxy):
"""Test retrieving a registered client."""
@ -1247,29 +1248,36 @@ class TestParameterForwarding:
class TestTokenHandlerErrorTransformation:
"""Tests for TokenHandler's OAuth 2.1 compliant error transformation."""
def test_transforms_client_auth_failure_to_invalid_client_401(self):
async def test_transforms_client_auth_failure_to_invalid_client_401(self):
"""Test that client authentication failures return invalid_client with 401."""
from mcp.server.auth.handlers.token import TokenErrorResponse
from unittest.mock import AsyncMock, patch
from mcp.server.auth.handlers.token import TokenHandler as SDKTokenHandler
from fastmcp.server.auth.oauth_proxy import TokenHandler
handler = TokenHandler(provider=Mock(), client_authenticator=Mock())
# Simulate error from ClientAuthenticator.authenticate() failure
error_response = TokenErrorResponse(
error="unauthorized_client",
error_description="Invalid client_id 'test-client-id'",
# Create a mock 401 response like the SDK returns for auth failures
mock_response = Mock()
mock_response.status_code = 401
mock_response.body = (
b'{"error":"unauthorized_client","error_description":"Invalid client_id"}'
)
response = handler.response(error_response)
# 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 OAuth 2.1 compliant response
assert response.status_code == 401
assert b'"error":"invalid_client"' in response.body
assert (
b'"error_description":"Invalid client_id \'test-client-id\'"'
in response.body
)
assert b'"error_description":"Invalid client_id"' in response.body
def test_does_not_transform_grant_type_unauthorized_to_invalid_client(self):
"""Test that grant type authorization errors stay as unauthorized_client with 400."""

View file

@ -75,7 +75,8 @@ class TestOAuthProxyStorage:
client = await proxy.get_client("test-client-123")
assert client is not None
assert client.client_id == "test-client-123"
assert client.client_secret == "secret-456"
# Proxy uses token_endpoint_auth_method="none", so client_secret is not stored
assert client.client_secret is None
assert client.scope == "read write"
async def test_client_persists_across_proxy_instances(
@ -96,7 +97,8 @@ class TestOAuthProxyStorage:
proxy2 = self.create_proxy(jwt_verifier, storage=temp_storage)
client = await proxy2.get_client("persistent-client")
assert client is not None
assert client.client_secret == "persistent-secret"
# Proxy uses token_endpoint_auth_method="none", so client_secret is not stored
assert client.client_secret is None
assert client.scope == "openid profile"
async def test_nonexistent_client_returns_none(
@ -199,7 +201,7 @@ class TestOAuthProxyStorage:
"software_id": None,
"software_version": None,
"client_id": "structured-client",
"client_secret": "secret",
"client_secret": None,
"client_id_issued_at": None,
"client_secret_expires_at": None,
"allowed_redirect_uri_patterns": None,

View file

@ -144,7 +144,7 @@ class TestStructuredLoggingMiddleware:
"event": "request_start",
"source": "client",
"method": "test_method",
"payload": '{"method":"tools/call","params":{"_meta":null,"name":"test_method","arguments":{"param":"value"}}}',
"payload": '{"method":"tools/call","params":{"task":null,"_meta":null,"name":"test_method","arguments":{"param":"value"}}}',
"payload_type": "CallToolRequest",
}
)
@ -159,7 +159,7 @@ class TestStructuredLoggingMiddleware:
"event": "request_start",
"source": "client",
"method": "test_method",
"payload_length": 98,
"payload_length": 110,
}
)
@ -177,8 +177,8 @@ class TestStructuredLoggingMiddleware:
"event": "request_start",
"source": "client",
"method": "test_method",
"payload_tokens": 24,
"payload_length": 98,
"payload_tokens": 27,
"payload_length": 110,
}
)
@ -303,7 +303,7 @@ class TestLoggingMiddleware:
assert get_log_lines(caplog) == snapshot(
[
'{"event": "request_start", "method": "test_method", "source": "client", "payload": "{\\"method\\":\\"resources/read\\",\\"params\\":{\\"_meta\\":null,\\"uri\\":\\"test://example/1\\"}}", "payload_type": "ReadResourceRequest"}',
'{"event": "request_start", "method": "test_method", "source": "client", "payload": "{\\"method\\":\\"resources/read\\",\\"params\\":{\\"task\\":null,\\"_meta\\":null,\\"uri\\":\\"test://example/1\\"}}", "payload_type": "ReadResourceRequest"}',
'{"event": "request_success", "method": "test_method", "source": "client", "duration_ms": 0.02}',
]
)
@ -365,7 +365,7 @@ class TestLoggingMiddleware:
assert get_log_lines(caplog) == snapshot(
[
'{"event": "request_start", "method": "test_method", "source": "client", "payload": "{\\"method\\":\\"tools/call\\",\\"params\\":{\\"_meta\\":null,\\"name\\":\\"test_method\\",\\"arguments\\":{\\"obj\\":\\"NON_SERIALIZABLE\\"}}}", "payload_type": "CallToolRequest"}',
'{"event": "request_start", "method": "test_method", "source": "client", "payload": "{\\"method\\":\\"tools/call\\",\\"params\\":{\\"task\\":null,\\"_meta\\":null,\\"name\\":\\"test_method\\",\\"arguments\\":{\\"obj\\":\\"NON_SERIALIZABLE\\"}}}", "payload_type": "CallToolRequest"}',
'{"event": "request_success", "method": "test_method", "source": "client", "duration_ms": 0.02}',
]
)
@ -546,7 +546,7 @@ class TestLoggingMiddlewareIntegration:
assert get_log_lines(caplog) == snapshot(
[
'event=request_start method=tools/call source=client payload={"_meta":null,"name":"simple_operation","arguments":{"data":"payload_test"}} payload_type=CallToolRequestParams',
'event=request_start method=tools/call source=client payload={"task":null,"_meta":null,"name":"simple_operation","arguments":{"data":"payload_test"}} payload_type=CallToolRequestParams',
"event=request_success method=tools/call source=client duration_ms=0.02",
]
)
@ -570,7 +570,7 @@ class TestLoggingMiddlewareIntegration:
assert get_log_lines(caplog) == snapshot(
[
'{"event": "request_start", "method": "tools/call", "source": "client", "payload": "{\\"_meta\\":null,\\"name\\":\\"simple_operation\\",\\"arguments\\":{\\"data\\":\\"json_test\\"}}", "payload_type": "CallToolRequestParams"}',
'{"event": "request_start", "method": "tools/call", "source": "client", "payload": "{\\"task\\":null,\\"_meta\\":null,\\"name\\":\\"simple_operation\\",\\"arguments\\":{\\"data\\":\\"json_test\\"}}", "payload_type": "CallToolRequestParams"}',
'{"event": "request_success", "method": "tools/call", "source": "client", "duration_ms": 0.02}',
]
)
@ -665,6 +665,6 @@ class TestLoggingMiddlewareIntegration:
# Check that our custom logger captured the logs
log_output = log_buffer.getvalue()
assert log_output == snapshot("""\
event=request_start method=tools/call source=client payload={"_meta":null,"name":"simple_operation","arguments":{"data":"custom_test"}} payload_type=CallToolRequestParams
event=request_start method=tools/call source=client payload={"task":null,"_meta":null,"name":"simple_operation","arguments":{"data":"custom_test"}} payload_type=CallToolRequestParams
event=request_success method=tools/call source=client duration_ms=0.02
""")

View file

@ -366,9 +366,10 @@ class TestAuthEndpoints:
assert metadata["revocation_endpoint"] == "https://auth.example.com/revoke"
assert metadata["response_types_supported"] == ["code"]
assert metadata["code_challenge_methods_supported"] == ["S256"]
assert metadata["token_endpoint_auth_methods_supported"] == [
"client_secret_post"
]
assert set(metadata["token_endpoint_auth_methods_supported"]) == {
"client_secret_post",
"client_secret_basic",
}
assert metadata["grant_types_supported"] == [
"authorization_code",
"refresh_token",
@ -376,8 +377,8 @@ class TestAuthEndpoints:
assert metadata["service_documentation"] == "https://docs.example.com/"
async def test_token_validation_error(self, test_client: httpx.AsyncClient):
"""Test token endpoint error - validation error."""
# Missing required fields
"""Test token endpoint error - missing client_id returns auth error."""
# Missing required fields - SDK validates client_id first
response = await test_client.post(
"/token",
data={
@ -386,10 +387,11 @@ class TestAuthEndpoints:
},
)
error_response = response.json()
assert error_response["error"] == "invalid_request"
assert (
"error_description" in error_response
) # Contains validation error messages
# SDK validates client_id before other fields, returning unauthorized_client
# (FastMCP's OAuthProxy transforms this to invalid_client, but this test
# uses the SDK's create_auth_routes directly)
assert error_response["error"] == "unauthorized_client"
assert "error_description" in error_response
async def test_token_invalid_auth_code(
self, test_client, registered_client, pkce_challenge

12
uv.lock generated
View file

@ -1,5 +1,5 @@
version = 1
revision = 2
revision = 3
requires-python = ">=3.10"
resolution-markers = [
"python_full_version >= '3.11'",
@ -610,7 +610,7 @@ requires-dist = [
{ name = "exceptiongroup", specifier = ">=1.2.2" },
{ name = "httpx", specifier = ">=0.28.1" },
{ name = "jsonschema-path", specifier = ">=0.3.4" },
{ name = "mcp", specifier = ">=1.19.0,!=1.21.1,<2.0.0" },
{ name = "mcp", specifier = ">=1.23.1" },
{ name = "openai", marker = "extra == 'openai'", specifier = ">=1.102.0" },
{ name = "openapi-pydantic", specifier = ">=0.5.1" },
{ name = "platformdirs", specifier = ">=4.0.0" },
@ -949,7 +949,7 @@ wheels = [
[[package]]
name = "mcp"
version = "1.21.0"
version = "1.23.1"
source = { registry = "https://pypi.org/simple" }
dependencies = [
{ name = "anyio" },
@ -963,11 +963,13 @@ dependencies = [
{ name = "pywin32", marker = "sys_platform == 'win32'" },
{ name = "sse-starlette" },
{ name = "starlette" },
{ name = "typing-extensions" },
{ name = "typing-inspection" },
{ name = "uvicorn", marker = "sys_platform != 'emscripten'" },
]
sdist = { url = "https://files.pythonhosted.org/packages/33/54/dd2330ef4611c27ae59124820863c34e1d3edb1133c58e6375e2d938c9c5/mcp-1.21.0.tar.gz", hash = "sha256:bab0a38e8f8c48080d787233343f8d301b0e1e95846ae7dead251b2421d99855", size = 452697, upload-time = "2025-11-06T23:19:58.432Z" }
sdist = { url = "https://files.pythonhosted.org/packages/12/42/10c0c09ca27aceacd8c428956cfabdd67e3d328fe55c4abc16589285d294/mcp-1.23.1.tar.gz", hash = "sha256:7403e053e8e2283b1e6ae631423cb54736933fea70b32422152e6064556cd298", size = 596519, upload-time = "2025-12-02T18:41:12.807Z" }
wheels = [
{ url = "https://files.pythonhosted.org/packages/39/47/850b6edc96c03bd44b00de9a0ca3c1cc71e0ba1cd5822955bc9e4eb3fad3/mcp-1.21.0-py3-none-any.whl", hash = "sha256:598619e53eb0b7a6513db38c426b28a4bdf57496fed04332100d2c56acade98b", size = 173672, upload-time = "2025-11-06T23:19:56.508Z" },
{ url = "https://files.pythonhosted.org/packages/9f/9e/26e1d2d2c6afe15dfba5ca6799eeeea7656dce625c22766e4c57305e9cc2/mcp-1.23.1-py3-none-any.whl", hash = "sha256:3ce897fcc20a41bd50b4c58d3aa88085f11f505dcc0eaed48930012d34c731d8", size = 231433, upload-time = "2025-12-02T18:41:11.195Z" },
]
[[package]]