diff --git a/.gitignore b/.gitignore index 887877553..ec7cee57b 100644 --- a/.gitignore +++ b/.gitignore @@ -53,6 +53,7 @@ dmypy.json # Local development .python-version .envrc +.envrc.private .direnv/ # Logs and databases diff --git a/docs/servers/elicitation.mdx b/docs/servers/elicitation.mdx index 596e8b892..e7333bc7e 100644 --- a/docs/servers/elicitation.mdx +++ b/docs/servers/elicitation.mdx @@ -3,6 +3,7 @@ title: User Elicitation sidebarTitle: Elicitation description: Request structured input from users during tool execution through the MCP context. icon: message-question +tag: NEW --- import { VersionBadge } from '/snippets/version-badge.mdx' @@ -381,6 +382,41 @@ async def create_task(ctx: Context) -> str: return "Task creation cancelled" ``` +### Default Values + +You can provide default values for elicitation fields using Pydantic's `Field(default=...)`. Clients will pre-populate form fields with these defaults, making it easier for users to provide input. + +Default values are supported for all primitive types: +- Strings: `Field(default="[email protected]")` +- Integers: `Field(default=50)` +- Numbers: `Field(default=3.14)` +- Booleans: `Field(default=False)` +- Enums: `Field(default=EnumValue.A)` + +Fields with default values are automatically marked as optional (not included in the `required` list), so users can accept the default or provide their own value. + +```python +from pydantic import BaseModel, Field +from enum import Enum + +class Priority(Enum): + LOW = "low" + MEDIUM = "medium" + HIGH = "high" + +class TaskDetails(BaseModel): + title: str = Field(description="Task title") + description: str = Field(default="", description="Task description") + priority: Priority = Field(default=Priority.MEDIUM, description="Task priority") + +@mcp.tool +async def create_task(ctx: Context) -> str: + result = await ctx.elicit("Please provide task details", response_type=TaskDetails) + if result.action == "accept": + return f"Created: {result.data.title}" + return "Task creation cancelled" +``` + ## Multi-Turn Elicitation Tools can make multiple elicitation calls to gather information progressively: diff --git a/src/fastmcp/server/auth/auth.py b/src/fastmcp/server/auth/auth.py index e64ab0be8..7f69dfd20 100644 --- a/src/fastmcp/server/auth/auth.py +++ b/src/fastmcp/server/auth/auth.py @@ -1,10 +1,15 @@ from __future__ import annotations +import json from typing import Any, cast from urllib.parse import urlparse +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.auth_context import AuthContextMiddleware from mcp.server.auth.middleware.bearer_auth import BearerAuthBackend +from mcp.server.auth.middleware.client_auth import ClientAuthenticator from mcp.server.auth.provider import ( AccessToken as _SDKAccessToken, ) @@ -17,6 +22,7 @@ from mcp.server.auth.provider import ( TokenVerifier as TokenVerifierProtocol, ) from mcp.server.auth.routes import ( + cors_middleware, create_auth_routes, create_protected_resource_routes, ) @@ -40,6 +46,48 @@ class AccessToken(_SDKAccessToken): claims: dict[str, Any] = Field(default_factory=dict) +class TokenHandler(_SDKTokenHandler): + """TokenHandler that returns OAuth 2.1 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 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. + + This handler transforms 401 responses with `unauthorized_client` to use + `invalid_client` instead, making the error semantics correct per OAuth spec. + """ + + async def handle(self, request: Any): + """Wrap SDK handle() and transform auth error responses.""" + response = await super().handle(request) + + # 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 AuthProvider(TokenVerifierProtocol): """Base class for all FastMCP authentication providers. @@ -368,7 +416,7 @@ class OAuthProvider( self.issuer_url is not None ) # typing check (issuer_url defaults to base_url) - oauth_routes = create_auth_routes( + sdk_routes = create_auth_routes( provider=self, issuer_url=self.base_url, service_documentation_url=self.service_documentation_url, @@ -376,6 +424,32 @@ class OAuthProvider( revocation_options=self.revocation_options, ) + # Replace the token endpoint with our custom handler that returns + # proper OAuth 2.1 error codes (invalid_client instead of unauthorized_client) + oauth_routes: list[Route] = [] + for route in sdk_routes: + if ( + isinstance(route, Route) + and route.path == "/token" + and route.methods is not None + and "POST" in route.methods + ): + # Replace with our OAuth 2.1 compliant token handler + token_handler = TokenHandler( + provider=self, client_authenticator=ClientAuthenticator(self) + ) + oauth_routes.append( + Route( + path="/token", + endpoint=cors_middleware( + token_handler.handle, ["POST", "OPTIONS"] + ), + methods=["POST", "OPTIONS"], + ) + ) + else: + oauth_routes.append(route) + # Get the resource URL based on the MCP path resource_url = self._get_resource_url(mcp_path) diff --git a/src/fastmcp/server/auth/oauth_proxy.py b/src/fastmcp/server/auth/oauth_proxy.py index 8130315ab..f447380bd 100644 --- a/src/fastmcp/server/auth/oauth_proxy.py +++ b/src/fastmcp/server/auth/oauth_proxy.py @@ -36,10 +36,6 @@ 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 -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 from mcp.server.auth.provider import ( AccessToken, AuthorizationCode, @@ -48,7 +44,6 @@ from mcp.server.auth.provider import ( RefreshToken, TokenError, ) -from mcp.server.auth.routes import cors_middleware from mcp.server.auth.settings import ( ClientRegistrationOptions, RevocationOptions, @@ -514,53 +509,6 @@ def create_error_html( ) -# ------------------------------------------------------------------------- -# Handler Classes -# ------------------------------------------------------------------------- - - -class TokenHandler(_SDKTokenHandler): - """TokenHandler that returns OAuth 2.1 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 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. - - This handler transforms 401 responses with `unauthorized_client` to use - `invalid_client` instead, making the error semantics correct per OAuth spec. - """ - - async def handle(self, request: Any): - """Wrap SDK handle() and transform auth error responses.""" - response = await super().handle(request) - - # 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): """OAuth provider that presents a DCR-compliant interface while proxying to non-DCR IDPs. @@ -1668,10 +1616,9 @@ class OAuthProxy(OAuthProvider): This is used to advertise the resource URL in metadata. """ # Get standard OAuth routes from parent class + # Note: parent already replaces /token with TokenHandler for proper error codes routes = super().get_routes(mcp_path) custom_routes = [] - token_route_found = False - authorize_route_found = False logger.debug( f"get_routes called - configuring OAuth routes in {len(routes)} routes" @@ -1689,7 +1636,6 @@ class OAuthProxy(OAuthProvider): and route.methods is not None and ("GET" in route.methods or "POST" in route.methods) ): - authorize_route_found = True # Replace with our enhanced authorization handler # Note: self.base_url is guaranteed to be set in parent __init__ authorize_handler = AuthorizationHandler( @@ -1705,27 +1651,6 @@ class OAuthProxy(OAuthProvider): methods=["GET", "POST"], ) ) - # Replace the token endpoint with our custom handler that returns proper OAuth 2.1 error codes - elif ( - isinstance(route, Route) - and route.path == "/token" - and route.methods is not None - and "POST" in route.methods - ): - token_route_found = True - # Replace with our OAuth 2.1 compliant token handler - token_handler = TokenHandler( - provider=self, client_authenticator=ClientAuthenticator(self) - ) - custom_routes.append( - Route( - path="/token", - endpoint=cors_middleware( - token_handler.handle, ["POST", "OPTIONS"] - ), - methods=["POST", "OPTIONS"], - ) - ) else: # Keep all other standard OAuth routes unchanged custom_routes.append(route) @@ -1747,9 +1672,6 @@ class OAuthProxy(OAuthProvider): ) ) - logger.debug( - f"✅ OAuth routes configured: authorize_endpoint={authorize_route_found}, token_endpoint={token_route_found}, total routes={len(custom_routes)} (includes OAuth callback + consent)" - ) return custom_routes # ------------------------------------------------------------------------- diff --git a/tests/client/test_elicitation.py b/tests/client/test_elicitation.py index 7c0a6092b..f5ec5d7cb 100644 --- a/tests/client/test_elicitation.py +++ b/tests/client/test_elicitation.py @@ -4,7 +4,7 @@ from typing import Literal import pytest from mcp.types import ElicitRequestParams -from pydantic import BaseModel +from pydantic import BaseModel, Field from typing_extensions import TypedDict from fastmcp import Context, FastMCP @@ -939,3 +939,134 @@ async def test_validation_rejects_primitive_arrays(): } with pytest.raises(TypeError, match="arrays are only allowed"): validate_elicitation_json_schema(schema) + + +class TestElicitationDefaults: + """Test suite for default values in elicitation schemas.""" + + def test_string_default_preserved(self): + """Test that string defaults are preserved in the schema.""" + + class Model(BaseModel): + email: str = Field(default="[email protected]") + + schema = get_elicitation_schema(Model) + props = schema.get("properties", {}) + + assert "email" in props + assert "default" in props["email"] + assert props["email"]["default"] == "[email protected]" + assert props["email"]["type"] == "string" + + def test_integer_default_preserved(self): + """Test that integer defaults are preserved in the schema.""" + + class Model(BaseModel): + count: int = Field(default=50) + + schema = get_elicitation_schema(Model) + props = schema.get("properties", {}) + + assert "count" in props + assert "default" in props["count"] + assert props["count"]["default"] == 50 + assert props["count"]["type"] == "integer" + + def test_number_default_preserved(self): + """Test that number defaults are preserved in the schema.""" + + class Model(BaseModel): + price: float = Field(default=3.14) + + schema = get_elicitation_schema(Model) + props = schema.get("properties", {}) + + assert "price" in props + assert "default" in props["price"] + assert props["price"]["default"] == 3.14 + assert props["price"]["type"] == "number" + + def test_boolean_default_preserved(self): + """Test that boolean defaults are preserved in the schema.""" + + class Model(BaseModel): + enabled: bool = Field(default=False) + + schema = get_elicitation_schema(Model) + props = schema.get("properties", {}) + + assert "enabled" in props + assert "default" in props["enabled"] + assert props["enabled"]["default"] is False + assert props["enabled"]["type"] == "boolean" + + def test_enum_default_preserved(self): + """Test that enum defaults are preserved in the schema.""" + + class Priority(Enum): + LOW = "low" + MEDIUM = "medium" + HIGH = "high" + + class Model(BaseModel): + choice: Priority = Field(default=Priority.MEDIUM) + + schema = get_elicitation_schema(Model) + props = schema.get("properties", {}) + + assert "choice" in props + assert "default" in props["choice"] + assert props["choice"]["default"] == "medium" + assert "enum" in props["choice"] + assert props["choice"]["type"] == "string" + + def test_all_defaults_preserved_together(self): + """Test that all default types are preserved when used together.""" + + class Priority(Enum): + A = "A" + B = "B" + + class Model(BaseModel): + string_field: str = Field(default="[email protected]") + integer_field: int = Field(default=50) + number_field: float = Field(default=3.14) + boolean_field: bool = Field(default=False) + enum_field: Priority = Field(default=Priority.A) + + schema = get_elicitation_schema(Model) + props = schema.get("properties", {}) + + assert props["string_field"]["default"] == "[email protected]" + assert props["integer_field"]["default"] == 50 + assert props["number_field"]["default"] == 3.14 + assert props["boolean_field"]["default"] is False + assert props["enum_field"]["default"] == "A" + + def test_mixed_defaults_and_required(self): + """Test that fields with defaults are not in required list.""" + + class Model(BaseModel): + required_field: str = Field(description="Required field") + optional_with_default: int = Field(default=42) + + schema = get_elicitation_schema(Model) + props = schema.get("properties", {}) + required = schema.get("required", []) + + assert "required_field" in required + assert "optional_with_default" not in required + assert props["optional_with_default"]["default"] == 42 + + def test_compress_schema_preserves_defaults(self): + """Test that compress_schema() doesn't strip default values.""" + + class Model(BaseModel): + string_field: str = Field(default="test") + integer_field: int = Field(default=42) + + schema = get_elicitation_schema(Model) + props = schema.get("properties", {}) + + assert "default" in props["string_field"] + assert "default" in props["integer_field"] diff --git a/tests/integration_tests/conftest.py b/tests/integration_tests/conftest.py index d45e7b292..4be7f6a9e 100644 --- a/tests/integration_tests/conftest.py +++ b/tests/integration_tests/conftest.py @@ -3,9 +3,37 @@ import os import pytest +def _is_rate_limit_error(excinfo) -> bool: + """Check if an exception indicates a rate limit error from GitHub API.""" + if excinfo is None: + return False + + exc = excinfo.value + exc_type = excinfo.typename + exc_str = str(exc).lower() + + # BrokenResourceError typically indicates connection closed due to rate limit + if exc_type == "BrokenResourceError": + return True + + # httpx.HTTPStatusError with 429 status + if exc_type == "HTTPStatusError": + try: + if hasattr(exc, "response") and exc.response.status_code == 429: + return True + except Exception: + pass + + # Check for rate limit indicators in exception message + if "429" in exc_str or "rate limit" in exc_str or "too many requests" in exc_str: + return True + + return False + + @pytest.hookimpl(hookwrapper=True) def pytest_runtest_makereport(item, call): - """Convert BrokenResourceError failures to skips only for GitHub rate limits""" + """Convert rate limit failures to skips for GitHub integration tests.""" outcome = yield report = outcome.get_result() @@ -14,12 +42,9 @@ def pytest_runtest_makereport(item, call): report.when == "call" and report.failed and not hasattr(report, "wasxfail") - and call.excinfo - and call.excinfo.typename == "BrokenResourceError" and item.module.__name__ == "tests.integration_tests.test_github_mcp_remote" + and _is_rate_limit_error(call.excinfo) ): - # Only skip if the test is in the GitHub remote test module - # This prevents catching unrelated BrokenResourceErrors report.outcome = "skipped" report.longrepr = ( os.path.abspath(__file__), diff --git a/tests/integration_tests/test_github_mcp_remote.py b/tests/integration_tests/test_github_mcp_remote.py index 6d8b3e2d5..6f86b3611 100644 --- a/tests/integration_tests/test_github_mcp_remote.py +++ b/tests/integration_tests/test_github_mcp_remote.py @@ -34,7 +34,6 @@ def fixture_streamable_http_client() -> Client[StreamableHttpTransport]: ) -@pytest.mark.flaky(retries=2, delay=1) class TestGithubMCPRemote: async def test_connect_disconnect( self, @@ -94,7 +93,7 @@ class TestGithubMCPRemote: """Test calling a non-existing tool""" async with streamable_http_client: assert streamable_http_client.is_connected() - with pytest.raises(McpError, match="tool not found"): + with pytest.raises(McpError, match=r"unknown tool|tool not found"): await streamable_http_client.call_tool("foo") async def test_call_tool_list_commits( diff --git a/tests/server/auth/test_oauth_proxy.py b/tests/server/auth/test_oauth_proxy.py index 56fd1ec30..40edf3114 100644 --- a/tests/server/auth/test_oauth_proxy.py +++ b/tests/server/auth/test_oauth_proxy.py @@ -1254,7 +1254,7 @@ class TestTokenHandlerErrorTransformation: from mcp.server.auth.handlers.token import TokenHandler as SDKTokenHandler - from fastmcp.server.auth.oauth_proxy import TokenHandler + from fastmcp.server.auth.auth import TokenHandler handler = TokenHandler(provider=Mock(), client_authenticator=Mock()) @@ -1283,7 +1283,7 @@ class TestTokenHandlerErrorTransformation: """Test that grant type authorization errors stay as unauthorized_client with 400.""" from mcp.server.auth.handlers.token import TokenErrorResponse - from fastmcp.server.auth.oauth_proxy import TokenHandler + from fastmcp.server.auth.auth import TokenHandler handler = TokenHandler(provider=Mock(), client_authenticator=Mock()) @@ -1303,7 +1303,7 @@ class TestTokenHandlerErrorTransformation: """Test that other error types pass through unchanged.""" from mcp.server.auth.handlers.token import TokenErrorResponse - from fastmcp.server.auth.oauth_proxy import TokenHandler + from fastmcp.server.auth.auth import TokenHandler handler = TokenHandler(provider=Mock(), client_authenticator=Mock())