diff --git a/.github/actions/conformance/expected-failures.yml b/.github/actions/conformance/expected-failures.yml index b542e788b4..dc18a647d5 100644 --- a/.github/actions/conformance/expected-failures.yml +++ b/.github/actions/conformance/expected-failures.yml @@ -1,4 +1,11 @@ # Known conformance test failures for v1.x # These are tracked and should be removed as they're fixed. server: [] -client: [] +client: + # The pinned harness (0.1.13) serves authorization server metadata whose `issuer` + # omits the tenant path its resource metadata advertises (`/tenant1`), so a client + # that checks RFC 8414 section 3.3 refuses it. The mock includes the path from + # conformance 0.1.15 (modelcontextprotocol/conformance#152); drop these two entries + # when the pin moves past it. + - auth/metadata-var2 + - auth/metadata-var3 diff --git a/docs/authorization.md b/docs/authorization.md index 171871ee58..a4c86d9797 100644 --- a/docs/authorization.md +++ b/docs/authorization.md @@ -40,8 +40,9 @@ mcp = FastMCP( # Auth settings for RFC 9728 Protected Resource Metadata auth=AuthSettings( issuer_url=AnyHttpUrl("https://auth.example.com"), # Authorization Server URL - resource_server_url=AnyHttpUrl("http://localhost:3001"), # This server's URL + resource_server_url=AnyHttpUrl("http://127.0.0.1:8000/mcp"), # This server's URL (mcp.run() default) required_scopes=["user"], + validate_token_resource=True, ), ) @@ -74,6 +75,12 @@ For a complete example with separate Authorization Server and Resource Server im See [TokenVerifier](https://github.com/modelcontextprotocol/python-sdk/blob/v1.x/src/mcp/server/auth/provider.py) for more details on implementing token validation. +A verifier should report who the token was issued for (its `aud`) in `AccessToken.resource`. `AuthSettings(validate_token_resource=True)` then refuses any token whose `resource` is not `resource_server_url`. Leaving it unset while `resource_server_url` is set warns (`DeprecationWarning`) and behaves as `False`; 3.0 makes `True` the default for resource servers. + +- Turn it on when your authorization server binds tokens to the `resource` the client requested, which MCP clients always send. Keep `resource_server_url` the exact URL clients connect to. +- Leave it off when your authorization server uses its own audience identifiers (an Auth0 API identifier, an Entra application ID) and check `aud` in your verifier instead, returning `None` for a token that isn't for this server. +- If `aud` is a list, put the entry that equals `resource_server_url` in `resource`. + ## Client-Side Authentication The SDK includes [authorization support](https://modelcontextprotocol.io/specification/2025-11-25/basic/authorization) for connecting to protected MCP servers: @@ -151,7 +158,7 @@ async def main(): callback_handler=handle_callback, ) - async with httpx.AsyncClient(auth=oauth_auth, follow_redirects=True) as custom_client: + async with httpx.AsyncClient(auth=oauth_auth) as custom_client: async with streamable_http_client("http://localhost:8001/mcp", http_client=custom_client) as (read, write, _): async with ClientSession(read, write) as session: await session.initialize() diff --git a/docs/client.md b/docs/client.md index 77c2729f27..2241cfd260 100644 --- a/docs/client.md +++ b/docs/client.md @@ -130,6 +130,29 @@ if __name__ == "__main__": _Full example: [examples/snippets/clients/streamable_basic.py](https://github.com/modelcontextprotocol/python-sdk/blob/v1.x/examples/snippets/clients/streamable_basic.py)_ +To configure headers, authentication or timeouts, create an `httpx.AsyncClient` and pass it as `http_client=`. + +## HTTP redirects + +The transport connects to the URL you gave it, and only that origin. + +* A `307`/`308` redirect that stays on the same scheme, host and port is followed, and so is `http://` → `https://` on the same host. That covers the usual `/mcp` → `/mcp/` trailing-slash redirect. +* A redirect anywhere else is **not** followed. Connecting fails with: + + ```text + httpx.HTTPStatusError: Redirect to https://other.example.com/mcp not followed; use that URL as the endpoint if it is the intended server + ``` + + If that URL is the server you meant, put it in your config. If it isn't, the server or a proxy in front of it is misconfigured. + +This holds for any `httpx.AsyncClient` you pass in: its `follow_redirects` setting is not consulted for MCP requests, in either direction. The SDK's OAuth providers apply the same rule to their own requests, and so does `sse_client()`. + +!!! tip + `Redirect to http://… not followed: it would downgrade this HTTPS endpoint to plain HTTP` means the + server sits behind a TLS-terminating proxy it doesn't know about and is issuing `http://` redirects. + That is fixed on the server (for uvicorn: `--proxy-headers` and `--forwarded-allow-ips`), or by + using the exact `https://…/` URL the message suggests. + ## Client Display Utilities When building MCP clients, the SDK provides utilities to help display human-readable names for tools, resources, and prompts: diff --git a/docs/server.md b/docs/server.md index 75516f699a..780e82d3d4 100644 --- a/docs/server.md +++ b/docs/server.md @@ -1254,6 +1254,7 @@ The FastMCP server instance accessible via `ctx.fastmcp` provides access to serv - `mount_path`, `sse_path`, `streamable_http_path` - Transport paths - `stateless_http` - Whether the server operates in stateless mode - `max_request_body_size` - Maximum HTTP request body size in bytes (Streamable HTTP and SSE) + - `session_idle_timeout` and `max_sessions` - Streamable HTTP session expiry and session cap - And other configuration options ```python @@ -1426,6 +1427,9 @@ messages, configure the smallest suitable byte limit: mcp = FastMCP("Large messages", max_request_body_size=8 * 1024 * 1024) ``` +Stateful sessions expire and are capped per process. See +[Session lifetime and limits](#session-lifetime-and-limits) below. + ```python """ @@ -1536,6 +1540,39 @@ The streamable HTTP transport supports: - JSON or SSE response formats - Better scalability for multi-node deployments +#### Session lifetime and limits + +A stateful session does not live forever, and one process does not hold an unlimited number of +them. Two settings control this. Both are keyword arguments on `FastMCP(...)`. `stateless_http=True` +keeps no sessions, so neither applies there. + +| Setting | Default | What it does | What the client sees | Turn it off | +|---|---|---|---|---| +| `session_idle_timeout` | `1800` (30 min) | Closes a session that has had nothing in flight for that long. | `404 Session not found`. It has to `initialize` again. | `None` | +| `max_sessions` | `10_000` | Refuses to open a session beyond that many. Existing sessions are untouched and nothing is evicted. | `503 Too many open sessions` with JSON-RPC code `-32603`. | `None` | + +What counts as "in flight": + +- An open `GET` stream. The SDK clients keep one open, so a connected client's session never + expires. +- A request that is still being answered. A tool call that runs longer than the timeout is not + interrupted, and the countdown only starts once it finishes. +- Nothing else. Between requests the clock runs. Any request on the session restarts it, + `ping` included. Once a session has expired, nothing revives it. + +A client that ends its session with `DELETE` frees it immediately. So does a client whose +opening request was refused. + +```python +mcp = FastMCP("My server", session_idle_timeout=None, max_sessions=50_000) +``` + +Both events show up in the server log. An expiry is `Session idle timeout` at `INFO`. A +refused open is `Refusing to open a new session: sessions are already open` at `WARNING`. + +The limits are per process. With four workers the ceiling is four times `max_sessions`, and each +worker expires its own sessions. + #### CORS Configuration for Browser-Based Clients If you'd like your server to be accessible by browser-based MCP clients, you'll need to configure CORS headers. The `Mcp-Session-Id` header must be exposed for browser clients to access it: diff --git a/examples/clients/simple-auth-client/mcp_simple_auth_client/main.py b/examples/clients/simple-auth-client/mcp_simple_auth_client/main.py index a88c4ea6b6..01d1ac709a 100644 --- a/examples/clients/simple-auth-client/mcp_simple_auth_client/main.py +++ b/examples/clients/simple-auth-client/mcp_simple_auth_client/main.py @@ -212,7 +212,7 @@ async def _default_redirect_handler(authorization_url: str) -> None: await self._run_session(read_stream, write_stream, None) else: print("📡 Opening StreamableHTTP transport connection with auth...") - async with httpx.AsyncClient(auth=oauth_auth, follow_redirects=True) as custom_client: + async with httpx.AsyncClient(auth=oauth_auth) as custom_client: async with streamable_http_client( url=self.server_url, http_client=custom_client, diff --git a/examples/servers/simple-auth/mcp_simple_auth/server.py b/examples/servers/simple-auth/mcp_simple_auth/server.py index 5d88505708..46bfbcc2a9 100644 --- a/examples/servers/simple-auth/mcp_simple_auth/server.py +++ b/examples/servers/simple-auth/mcp_simple_auth/server.py @@ -75,6 +75,7 @@ def create_resource_server(settings: ResourceServerSettings) -> FastMCP: issuer_url=settings.auth_server_url, required_scopes=[settings.mcp_scope], resource_server_url=settings.server_url, + validate_token_resource=True, # tokens must be reported as issued for server_url ), ) diff --git a/examples/servers/simple-auth/mcp_simple_auth/token_verifier.py b/examples/servers/simple-auth/mcp_simple_auth/token_verifier.py index 641095a125..c86f7c5553 100644 --- a/examples/servers/simple-auth/mcp_simple_auth/token_verifier.py +++ b/examples/servers/simple-auth/mcp_simple_auth/token_verifier.py @@ -69,12 +69,19 @@ async def verify_token(self, token: str) -> AccessToken | None: logger.warning(f"Token resource validation failed. Expected: {self.resource_url}") return None + # `aud` may be a string or a list; report the entry naming this server when there is + # one, otherwise what the token was issued for, so the server can compare it. + aud: str | list[str] | None = data.get("aud") + audiences = aud if isinstance(aud, list) else [aud] if aud else [] + own = self.resource_url.rstrip("/") + resource = next((a for a in audiences if a.rstrip("/") == own), audiences[0] if audiences else None) + return AccessToken( token=token, client_id=data.get("client_id", "unknown"), scopes=data.get("scope", "").split() if data.get("scope") else [], expires_at=data.get("exp"), - resource=data.get("aud"), # Include resource in token + resource=resource, subject=data.get("sub"), # RFC 7662 subject (resource owner) claims=data, ) diff --git a/examples/servers/simple-tool/mcp_simple_tool/server.py b/examples/servers/simple-tool/mcp_simple_tool/server.py index 5b2b7d068d..b75e328e01 100644 --- a/examples/servers/simple-tool/mcp_simple_tool/server.py +++ b/examples/servers/simple-tool/mcp_simple_tool/server.py @@ -2,9 +2,9 @@ import anyio import click +import httpx import mcp.types as types from mcp.server.lowlevel import Server -from mcp.shared._httpx_utils import create_mcp_http_client from starlette.requests import Request @@ -12,7 +12,8 @@ async def fetch_website( url: str, ) -> list[types.ContentBlock]: headers = {"User-Agent": "MCP Test Server (github.com/modelcontextprotocol/python-sdk)"} - async with create_mcp_http_client(headers=headers) as client: + timeout = httpx.Timeout(30, read=300) + async with httpx.AsyncClient(headers=headers, timeout=timeout, follow_redirects=True) as client: response = await client.get(url) response.raise_for_status() return [types.TextContent(type="text", text=response.text)] diff --git a/examples/snippets/clients/oauth_client.py b/examples/snippets/clients/oauth_client.py index 140b38aedb..523dfdf099 100644 --- a/examples/snippets/clients/oauth_client.py +++ b/examples/snippets/clients/oauth_client.py @@ -69,7 +69,7 @@ async def main(): callback_handler=handle_callback, ) - async with httpx.AsyncClient(auth=oauth_auth, follow_redirects=True) as custom_client: + async with httpx.AsyncClient(auth=oauth_auth) as custom_client: async with streamable_http_client("http://localhost:8001/mcp", http_client=custom_client) as (read, write, _): async with ClientSession(read, write) as session: await session.initialize() diff --git a/examples/snippets/servers/oauth_server.py b/examples/snippets/servers/oauth_server.py index 3717c66de8..8e63ea5565 100644 --- a/examples/snippets/servers/oauth_server.py +++ b/examples/snippets/servers/oauth_server.py @@ -26,8 +26,9 @@ async def verify_token(self, token: str) -> AccessToken | None: # Auth settings for RFC 9728 Protected Resource Metadata auth=AuthSettings( issuer_url=AnyHttpUrl("https://auth.example.com"), # Authorization Server URL - resource_server_url=AnyHttpUrl("http://localhost:3001"), # This server's URL + resource_server_url=AnyHttpUrl("http://127.0.0.1:8000/mcp"), # This server's URL (mcp.run() default) required_scopes=["user"], + validate_token_resource=True, ), ) diff --git a/src/mcp/client/auth/extensions/client_credentials.py b/src/mcp/client/auth/extensions/client_credentials.py index e2f3f08a4d..4aa18d6425 100644 --- a/src/mcp/client/auth/extensions/client_credentials.py +++ b/src/mcp/client/auth/extensions/client_credentials.py @@ -9,8 +9,10 @@ """ import time +import warnings from collections.abc import Awaitable, Callable from typing import Any, Literal +from urllib.parse import urlparse from uuid import uuid4 import httpx @@ -18,14 +20,59 @@ from pydantic import BaseModel, Field from mcp.client.auth import OAuthClientProvider, OAuthFlowError, OAuthTokenError, TokenStorage +from mcp.client.auth.oauth2 import OAuthContext +from mcp.client.auth.utils import issuers_match from mcp.shared.auth import OAuthClientInformationFull, OAuthClientMetadata +def _checked_issuer(issuer: str | None) -> str | None: + if issuer is None: + warnings.warn( + "Omitting `issuer` is deprecated and it will be required in 3.0. Without it, the MCP server " + "decides which authorization server receives this client's credentials; pass " + "issuer= so they are only ever sent there.", + DeprecationWarning, + stacklevel=3, + ) + return None + if urlparse(issuer).scheme not in ("http", "https"): + raise ValueError(f"issuer must be the authorization server's http(s) issuer URL, got {issuer!r}") + return issuer + + +def _preferred_authorization_server(advertised: list[str], issuer: str | None) -> str: + """The advertised server matching the configured issuer if there is one, else the first.""" + return next( + (server for server in advertised if issuer is not None and issuers_match(server, issuer)), advertised[0] + ) + + +def _require_metadata_for_configured_issuer(context: OAuthContext, issuer: str | None) -> None: + """With an issuer configured, a token request is only built from metadata discovered for that issuer. + + Anything else held is dropped along with the tokens, so the next request starts discovery afresh + rather than refreshing against it. + """ + if issuer is None: + return + metadata = context.oauth_metadata + if metadata is not None and issuers_match(str(metadata.issuer), issuer): + return + context.oauth_metadata = None + context.clear_tokens() + if metadata is None: + raise OAuthFlowError(f"No authorization server metadata discovered for configured issuer {issuer}") + raise OAuthFlowError(f"Authorization server metadata issuer mismatch: {metadata.issuer} != {issuer}") + + class ClientCredentialsOAuthProvider(OAuthClientProvider): """OAuth provider for client_credentials grant with client_id + client_secret. This provider sets client_info directly, bypassing dynamic client registration. Use this when you already have client credentials (client_id and client_secret). + Pass `issuer` to name the authorization server those credentials belong to: token + requests are then only built from authorization server metadata for that issuer, and + the flow stops if the MCP server leads anywhere else. Example: ```python @@ -34,6 +81,7 @@ class ClientCredentialsOAuthProvider(OAuthClientProvider): storage=my_token_storage, client_id="my-client-id", client_secret="my-client-secret", + issuer="https://auth.example.com", ) ``` """ @@ -46,6 +94,7 @@ def __init__( client_secret: str, token_endpoint_auth_method: Literal["client_secret_basic", "client_secret_post"] = "client_secret_basic", scopes: str | None = None, + issuer: str | None = None, ) -> None: """Initialize client_credentials OAuth provider. @@ -57,6 +106,12 @@ def __init__( token_endpoint_auth_method: Authentication method for token endpoint. Either "client_secret_basic" (default) or "client_secret_post". scopes: Optional space-separated list of scopes to request. + issuer: The issuer identifier of the authorization server that issued + `client_id` and `client_secret`. When set, token requests are only built from + discovered authorization server metadata whose `issuer` is exactly this string; + otherwise the flow stops with `OAuthFlowError`. Omitting it is deprecated + (`DeprecationWarning`) and it will be required in 3.0; until then, whichever + authorization server discovery yields is used. """ # Build minimal client_metadata for the base class client_metadata = OAuthClientMetadata( @@ -66,6 +121,7 @@ def __init__( scope=scopes, ) super().__init__(server_url, client_metadata, storage, None, None, 300.0) + self._issuer = _checked_issuer(issuer) # Store client_info to be set during _initialize - no dynamic registration needed self._fixed_client_info = OAuthClientInformationFull( redirect_uris=None, @@ -82,12 +138,17 @@ async def _initialize(self) -> None: self.context.client_info = self._fixed_client_info self._initialized = True + def _select_authorization_server(self, advertised: list[str]) -> str: + return _preferred_authorization_server(advertised, self._issuer) + async def _perform_authorization(self) -> httpx.Request: """Perform client_credentials authorization.""" return await self._exchange_token_client_credentials() async def _exchange_token_client_credentials(self) -> httpx.Request: """Build token exchange request for client_credentials grant.""" + _require_metadata_for_configured_issuer(self.context, self._issuer) + token_data: dict[str, Any] = { "grant_type": "client_credentials", } @@ -120,6 +181,7 @@ def static_assertion_provider(token: str) -> Callable[[str], Awaitable[str]]: storage=my_token_storage, client_id="my-client-id", assertion_provider=static_assertion_provider(my_prebuilt_jwt), + issuer="https://auth.example.com", ) ``` @@ -154,6 +216,7 @@ class SignedJWTParameters(BaseModel): storage=my_token_storage, client_id="my-client-id", assertion_provider=jwt_params.create_assertion_provider(), + issuer="https://auth.example.com", ) ``` """ @@ -198,7 +261,10 @@ class PrivateKeyJWTOAuthProvider(OAuthClientProvider): The JWT assertion's audience MUST be the authorization server's issuer identifier (per RFC 7523bis security updates). The `assertion_provider` callback receives - this audience value and must return a JWT with that audience. + this audience value and must return a JWT with that audience. Pass `issuer` to name + the authorization server this client is registered with: an assertion is then only + minted once metadata for that issuer has been discovered, and token requests are only + built from that metadata. **Option 1: Pre-built JWT via Workload Identity Federation** @@ -216,6 +282,7 @@ async def get_workload_identity_token(audience: str) -> str: storage=my_token_storage, client_id="my-client-id", assertion_provider=get_workload_identity_token, + issuer="https://auth.example.com", ) ``` @@ -229,6 +296,7 @@ async def get_workload_identity_token(audience: str) -> str: storage=my_token_storage, client_id="my-client-id", assertion_provider=static_assertion_provider(my_prebuilt_jwt), + issuer="https://auth.example.com", ) ``` @@ -247,6 +315,7 @@ async def get_workload_identity_token(audience: str) -> str: storage=my_token_storage, client_id="my-client-id", assertion_provider=jwt_params.create_assertion_provider(), + issuer="https://auth.example.com", ) ``` """ @@ -258,6 +327,7 @@ def __init__( client_id: str, assertion_provider: Callable[[str], Awaitable[str]], scopes: str | None = None, + issuer: str | None = None, ) -> None: """Initialize private_key_jwt OAuth provider. @@ -271,6 +341,12 @@ def __init__( `static_assertion_provider()` for pre-built JWTs, or provide your own callback for workload identity federation. scopes: Optional space-separated list of scopes to request. + issuer: The issuer identifier of the authorization server `client_id` is + registered with. When set, an assertion is only minted, and token requests + are only built, once authorization server metadata whose `issuer` is exactly this + string has been discovered; otherwise the flow stops with `OAuthFlowError`. + Omitting it is deprecated (`DeprecationWarning`) and it will be required in + 3.0; until then, whichever authorization server discovery yields is used. """ # Build minimal client_metadata for the base class client_metadata = OAuthClientMetadata( @@ -281,6 +357,7 @@ def __init__( ) super().__init__(server_url, client_metadata, storage, None, None, 300.0) self._assertion_provider = assertion_provider + self._issuer = _checked_issuer(issuer) # Store client_info to be set during _initialize - no dynamic registration needed self._fixed_client_info = OAuthClientInformationFull( redirect_uris=None, @@ -296,6 +373,9 @@ async def _initialize(self) -> None: self.context.client_info = self._fixed_client_info self._initialized = True + def _select_authorization_server(self, advertised: list[str]) -> str: + return _preferred_authorization_server(advertised, self._issuer) + async def _perform_authorization(self) -> httpx.Request: """Perform client_credentials authorization with private_key_jwt.""" return await self._exchange_token_client_credentials() @@ -316,6 +396,8 @@ async def _add_client_authentication_jwt(self, *, token_data: dict[str, Any]) -> async def _exchange_token_client_credentials(self) -> httpx.Request: """Build token exchange request for client_credentials grant with private_key_jwt.""" + _require_metadata_for_configured_issuer(self.context, self._issuer) + token_data: dict[str, Any] = { "grant_type": "client_credentials", } @@ -409,8 +491,6 @@ def __init__( timeout: float = 300.0, jwt_parameters: JWTParameters | None = None, ) -> None: - import warnings - warnings.warn( "RFC7523OAuthClientProvider is deprecated. Use ClientCredentialsOAuthProvider " "or PrivateKeyJWTOAuthProvider instead.", diff --git a/src/mcp/client/auth/oauth2.py b/src/mcp/client/auth/oauth2.py index 0ec0879688..680cbfd022 100644 --- a/src/mcp/client/auth/oauth2.py +++ b/src/mcp/client/auth/oauth2.py @@ -17,7 +17,7 @@ import anyio import httpx -from pydantic import BaseModel, Field, ValidationError +from pydantic import AnyHttpUrl, BaseModel, Field, ValidationError from mcp.client.auth.exceptions import OAuthFlowError, OAuthTokenError from mcp.client.auth.utils import ( @@ -26,6 +26,7 @@ create_client_info_from_metadata_url, create_client_registration_request, create_oauth_metadata_request, + credentials_match_issuer, extract_field_from_www_auth, extract_resource_metadata_from_www_auth, extract_scope_from_www_auth, @@ -36,8 +37,10 @@ handle_token_response_scopes, is_valid_client_metadata_url, should_use_client_metadata_url, + validate_metadata_issuer, ) from mcp.client.streamable_http import MCP_PROTOCOL_VERSION +from mcp.shared._httpx_utils import RedirectAwareAuth, redirect_note from mcp.shared.auth import ( OAuthClientInformationFull, OAuthClientMetadata, @@ -214,7 +217,15 @@ def prepare_token_auth( return data, headers -class OAuthClientProvider(httpx.Auth): +def _origin_issuer(server_url: str) -> str: + """The resource server's origin as an issuer identifier: `scheme://authority`, rendered the way + `OAuthMetadata.issuer` renders URLs (host case, default ports, trailing slash) so the two compare + as strings.""" + parsed = urlparse(server_url) + return str(AnyHttpUrl(f"{parsed.scheme}://{parsed.netloc}")) + + +class OAuthClientProvider(RedirectAwareAuth): """ OAuth2 authentication for httpx. Handles OAuth flow with automatic client registration and token storage. @@ -411,7 +422,9 @@ async def _handle_token_response(self, response: httpx.Response) -> None: if response.status_code != 200: body = await response.aread() # pragma: no cover body_text = body.decode("utf-8") # pragma: no cover - raise OAuthTokenError(f"Token exchange failed ({response.status_code}): {body_text}") # pragma: no cover + raise OAuthTokenError( # pragma: no cover + f"Token exchange failed ({response.status_code}){redirect_note(response)}: {body_text}" + ) # Parse and validate response with scope validation token_response = await handle_token_response_scopes(response) @@ -454,7 +467,7 @@ async def _refresh_token(self) -> httpx.Request: async def _handle_refresh_response(self, response: httpx.Response) -> bool: # pragma: no cover """Handle token refresh response. Returns True if successful.""" if response.status_code != 200: - logger.warning(f"Token refresh failed: {response.status_code}") + logger.warning(f"Token refresh failed: {response.status_code}{redirect_note(response)}") self.context.clear_tokens() return False @@ -488,8 +501,18 @@ async def _handle_oauth_metadata_response(self, response: httpx.Response) -> Non metadata = OAuthMetadata.model_validate_json(content) self.context.oauth_metadata = metadata - async def async_auth_flow(self, request: httpx.Request) -> AsyncGenerator[httpx.Request, httpx.Response]: - """HTTPX auth flow integration.""" + def _select_authorization_server(self, advertised: list[str]) -> str: + """Which of the servers listed in protected resource metadata to use: the first (the list is never empty).""" + return advertised[0] + + def _expected_issuer(self) -> str: + """The issuer that authorization server metadata and client credentials must belong to: the + PRM-advertised server, or on the legacy no-PRM path the resource server's origin, which is what + the 2025-03-26 well-known URL is built from (RFC 8414 §3.3).""" + return self.context.auth_server_url or _origin_issuer(self.context.server_url) + + async def _auth_flow(self, request: httpx.Request) -> AsyncGenerator[httpx.Request, httpx.Response]: + """The OAuth flow proper; `async_auth_flow` drives it (see `RedirectAwareAuth`).""" async with self.context.lock: if not self._initialized: await self._initialize() # pragma: no cover @@ -511,55 +534,89 @@ async def async_auth_flow(self, request: httpx.Request) -> AsyncGenerator[httpx. response = yield request - if response.status_code == 401: + step_up = ( + response.status_code == 403 and extract_field_from_www_auth(response, "error") == "insufficient_scope" + ) + + if response.status_code == 401 or step_up: # Perform full OAuth flow try: - # OAuth flow must be inline due to generator constraints - www_auth_resource_metadata_url = extract_resource_metadata_from_www_auth(response) - - # Step 1: Discover protected resource metadata (SEP-985 with fallback support) - prm_discovery_urls = build_protected_resource_metadata_discovery_urls( - www_auth_resource_metadata_url, self.context.server_url - ) - - for url in prm_discovery_urls: # pragma: no branch - discovery_request = create_oauth_metadata_request(url) - - discovery_response = yield discovery_request # sending request - - prm = await handle_protected_resource_response(discovery_response) - if prm: - # Validate PRM resource matches server URL (RFC 8707) - await self._validate_resource_match(prm) - self.context.protected_resource_metadata = prm - - # todo: try all authorization_servers to find the OASM - assert ( - len(prm.authorization_servers) > 0 - ) # this is always true as authorization_servers has a min length of 1 + # OAuth flow must be inline due to generator constraints. + # Steps 1-2 run on every 401. A scope step-up reuses the metadata discovered earlier + # in this process, and discovers it first when none is held yet (for example when + # tokens were loaded from storage), so re-authorization targets the right server. + if response.status_code == 401 or self.context.oauth_metadata is None: + www_auth_resource_metadata_url = extract_resource_metadata_from_www_auth(response) + + # Step 1: Discover protected resource metadata (SEP-985 with fallback support) + prm_discovery_urls = build_protected_resource_metadata_discovery_urls( + www_auth_resource_metadata_url, self.context.server_url + ) - self.context.auth_server_url = str(prm.authorization_servers[0]) - break + prm_request_failed: int | None = None + for url in prm_discovery_urls: + discovery_request = create_oauth_metadata_request(url) + + discovery_response = yield discovery_request # sending request + + if discovery_response.status_code >= 500 or discovery_response.status_code == 429: + prm_request_failed = discovery_response.status_code + prm = await handle_protected_resource_response(discovery_response) + if prm: + # Validate PRM resource matches server URL (RFC 8707) + await self._validate_resource_match(prm) + self.context.protected_resource_metadata = prm + self.context.auth_server_url = self._select_authorization_server( + [str(url) for url in prm.authorization_servers] + ) + break + else: + logger.debug(f"Protected resource metadata discovery failed: {url}") else: - logger.debug(f"Protected resource metadata discovery failed: {url}") - - asm_discovery_urls = build_oauth_authorization_server_metadata_discovery_urls( - self.context.auth_server_url, self.context.server_url - ) + if prm_request_failed is not None: + # A server error says nothing about whether the resource publishes + # metadata, so it must not send the flow down the legacy path. + raise OAuthFlowError( + f"Protected resource metadata request failed: HTTP {prm_request_failed}" + ) + + expected_issuer = self._expected_issuer() + + # SEP-2352: stored credentials are bound to the issuer that registered them. + # Decided before any metadata is fetched: if the expected issuer is a different + # server, drop them (and the old tokens) so the flow re-registers instead of + # presenting another server's credentials. + if self.context.client_info is not None and not credentials_match_issuer( + self.context.client_info, expected_issuer, self.context.client_metadata_url + ): + logger.debug( + "Authorization server changed; discarding bound credentials and re-registering" + ) + self.context.client_info = None + self.context.clear_tokens() + # Any cached AS metadata is for the old server; drop it so a failed + # rediscovery cannot leak the old registration/token endpoints into Step 4. + self.context.oauth_metadata = None + + asm_discovery_urls = build_oauth_authorization_server_metadata_discovery_urls( + self.context.auth_server_url, self.context.server_url + ) - # Step 2: Discover OAuth Authorization Server Metadata (OASM) (with fallback for legacy servers) - for url in asm_discovery_urls: # pragma: no cover - oauth_metadata_request = create_oauth_metadata_request(url) - oauth_metadata_response = yield oauth_metadata_request - - ok, asm = await handle_auth_metadata_response(oauth_metadata_response) - if not ok: - break - if ok and asm: - self.context.oauth_metadata = asm - break - else: - logger.debug(f"OAuth metadata discovery failed: {url}") + # Step 2: Discover OAuth Authorization Server Metadata (OASM) (with fallback for legacy servers) + for url in asm_discovery_urls: # pragma: no branch + oauth_metadata_request = create_oauth_metadata_request(url) + oauth_metadata_response = yield oauth_metadata_request + + ok, asm = await handle_auth_metadata_response(oauth_metadata_response) + if not ok: + break + if ok and asm: + # SEP-2468 / RFC 8414 section 3.3: the metadata must name the expected issuer + validate_metadata_issuer(asm, expected_issuer) + self.context.oauth_metadata = asm + break + else: + logger.debug(f"OAuth metadata discovery failed: {url}") # Step 3: Apply scope selection strategy self.context.client_metadata.scope = get_client_metadata_scopes( @@ -570,58 +627,56 @@ async def async_auth_flow(self, request: httpx.Request) -> AsyncGenerator[httpx. # Step 4: Register client or use URL-based client ID (CIMD) if not self.context.client_info: + # SEP-2352: the issuer to bind these credentials to, once metadata for it + # was actually found. + discovered_issuer = self._expected_issuer() if self.context.oauth_metadata is not None else None + if should_use_client_metadata_url( self.context.oauth_metadata, self.context.client_metadata_url ): - # Use URL-based client ID (CIMD) + # Use URL-based client ID (CIMD). CIMD records are portable across + # authorization servers, so the issuer stamp is informational. logger.debug(f"Using URL-based client ID (CIMD): {self.context.client_metadata_url}") client_information = create_client_info_from_metadata_url( self.context.client_metadata_url, # type: ignore[arg-type] redirect_uris=self.context.client_metadata.redirect_uris, ) + client_information.issuer = discovered_issuer self.context.client_info = client_information await self.context.storage.set_client_info(client_information) else: # Fallback to Dynamic Client Registration + fallback_base = self.context.get_authorization_base_url(self.context.server_url) registration_request = create_client_registration_request( - self.context.oauth_metadata, - self.context.client_metadata, - self.context.get_authorization_base_url(self.context.server_url), + self.context.oauth_metadata, self.context.client_metadata, fallback_base ) registration_response = yield registration_request client_information = await handle_registration_response(registration_response) + # Only record the issuer when the registration above actually targeted + # the discovered AS - either via its published registration_endpoint, + # or because the resource-origin /register fallback is on the issuer's + # own host (legacy same-origin embedded AS). Otherwise the fallback hit + # a different server and recording a binding to the PRM-advertised AS + # would persist a binding that was never established. + if ( + self.context.oauth_metadata is not None + and discovered_issuer is not None + and ( + self.context.oauth_metadata.registration_endpoint is not None + or self.context.get_authorization_base_url(discovered_issuer) == fallback_base + ) + ): + client_information.issuer = discovered_issuer self.context.client_info = client_information await self.context.storage.set_client_info(client_information) # Step 5: Perform authorization and complete token exchange token_response = yield await self._perform_authorization() await self._handle_token_response(token_response) - except Exception: # pragma: no cover + except Exception: logger.exception("OAuth flow error") raise # Retry with new tokens self._add_auth_header(request) yield request - elif response.status_code == 403: - # Step 1: Extract error field from WWW-Authenticate header - error = extract_field_from_www_auth(response, "error") - - # Step 2: Check if we need to step-up authorization - if error == "insufficient_scope": # pragma: no branch - try: - # Step 2a: Update the required scopes - self.context.client_metadata.scope = get_client_metadata_scopes( - extract_scope_from_www_auth(response), self.context.protected_resource_metadata - ) - - # Step 2b: Perform (re-)authorization and token exchange - token_response = yield await self._perform_authorization() - await self._handle_token_response(token_response) - except Exception: # pragma: no cover - logger.exception("OAuth flow error") - raise - - # Retry with new tokens - self._add_auth_header(request) - yield request diff --git a/src/mcp/client/auth/utils.py b/src/mcp/client/auth/utils.py index b4426be7f8..413ac8405e 100644 --- a/src/mcp/client/auth/utils.py +++ b/src/mcp/client/auth/utils.py @@ -1,12 +1,15 @@ import logging import re +from typing import Any, cast from urllib.parse import urljoin, urlparse from httpx import Request, Response from pydantic import AnyUrl, ValidationError +from pydantic_core import from_json -from mcp.client.auth import OAuthRegistrationError, OAuthTokenError +from mcp.client.auth import OAuthFlowError, OAuthRegistrationError, OAuthTokenError from mcp.client.streamable_http import MCP_PROTOCOL_VERSION +from mcp.shared._httpx_utils import redirect_note from mcp.shared.auth import ( OAuthClientInformationFull, OAuthClientMetadata, @@ -58,7 +61,7 @@ def extract_resource_metadata_from_www_auth(response: Response) -> str | None: Returns: Resource metadata URL if found in WWW-Authenticate header, None otherwise """ - if not response or response.status_code != 401: + if not response or response.status_code not in (401, 403): return None # pragma: no cover return extract_field_from_www_auth(response, "resource_metadata") @@ -203,9 +206,38 @@ async def handle_auth_metadata_response(response: Response) -> tuple[bool, OAuth return True, asm except ValidationError: # pragma: no cover return True, None - elif response.status_code < 400 or response.status_code >= 500: - return False, None # Non-4XX error, stop trying - return True, None + elif 300 <= response.status_code < 500: + return True, None # Not served at this URL (redirects are not followed) - try the next candidate + return False, None # Server error or unexpected status, stop trying + + +def validate_metadata_issuer(oauth_metadata: OAuthMetadata, expected_issuer: str) -> None: + """Validate that authorization server metadata `issuer` matches the discovery issuer. + + Per RFC 8414 section 3.3 / SEP-2468, the `issuer` in the metadata must match the issuer + used to construct the well-known URL, compared as a simple string (RFC 3986 section 6.2.1). + The one tolerance is an origin with an empty path versus the same origin with a lone `/` + (RFC 3986 section 6.2.3): the SDK's URL type always renders a root issuer with the `/`, and + servers commonly render it either way. + + Raises: + OAuthFlowError: If the metadata issuer does not match `expected_issuer`. + """ + if not issuers_match(str(oauth_metadata.issuer), expected_issuer): + raise OAuthFlowError( + f"Authorization server metadata issuer mismatch: {oauth_metadata.issuer} != {expected_issuer}" + ) + + +def issuers_match(a: str, b: str) -> bool: + """Simple string comparison of two issuer identifiers (RFC 8414 section 3.3), except that a root + issuer with and without its trailing slash (`scheme://authority` and `scheme://authority/`) name + the same server.""" + if a == b: + return True + shorter, longer = sorted((a, b), key=len) + parsed = urlparse(shorter) + return longer == f"{shorter}/" and shorter == f"{parsed.scheme}://{parsed.netloc}" def create_oauth_metadata_request(url: str) -> Request: @@ -231,16 +263,23 @@ async def handle_registration_response(response: Response) -> OAuthClientInforma """Handle registration response.""" if response.status_code not in (200, 201): await response.aread() - raise OAuthRegistrationError(f"Registration failed: {response.status_code} {response.text}") + raise OAuthRegistrationError( + f"Registration failed: {response.status_code}{redirect_note(response)} {response.text}" + ) try: content = await response.aread() - client_info = OAuthClientInformationFull.model_validate_json(content) - return client_info - # self.context.client_info = client_info - # await self.context.storage.set_client_info(client_info) - except ValidationError as e: # pragma: no cover - raise OAuthRegistrationError(f"Invalid registration response: {e}") + body = from_json(content) + # `issuer` is the SDK's own binding of these credentials to the server they were + # registered with (SEP-2352), stamped by the auth flow - never sourced from the + # wire, so it is dropped before the body is parsed rather than trusted or cleared. + if isinstance(body, dict): + cast(dict[str, Any], body).pop("issuer", None) + return OAuthClientInformationFull.model_validate(body) + except ValueError as e: + # `from_json` reports malformed bytes/JSON as ValueError, and pydantic's + # ValidationError is itself a ValueError, so both parse layers surface here. + raise OAuthRegistrationError(f"Invalid registration response: {e}") from e def is_valid_client_metadata_url(url: str | None) -> bool: @@ -263,6 +302,26 @@ def is_valid_client_metadata_url(url: str | None) -> bool: return False +def credentials_match_issuer( + client_info: OAuthClientInformationFull, issuer: str, client_metadata_url: str | None +) -> bool: + """Whether stored client credentials may be reused against `issuer` (SEP-2352). + + A URL-based client ID (CIMD) is portable across authorization servers - the same self-hosted + document is resolved by whichever server is in use - so it always matches; CIMD is identified + by the client ID being the configured `client_metadata_url`, not by URL shape (a registration + server may also issue URL-shaped IDs that are bound to it). Credentials with a recorded issuer + match only when it names the same server as `issuer` (`issuers_match`). Credentials with no + recorded issuer (pre-registered, or stored before issuer binding existed) carry no binding to + enforce and are left as-is. + """ + if client_metadata_url is not None and client_info.client_id == client_metadata_url: + return True + if client_info.issuer is None: + return True + return issuers_match(client_info.issuer, issuer) + + def should_use_client_metadata_url( oauth_metadata: OAuthMetadata | None, client_metadata_url: str | None, diff --git a/src/mcp/client/session.py b/src/mcp/client/session.py index 86f2676dcb..d40d767dde 100644 --- a/src/mcp/client/session.py +++ b/src/mcp/client/session.py @@ -428,17 +428,24 @@ async def _validate_tool_result(self, name: str, result: types.CallToolResult) - if output_schema is not None: from jsonschema import SchemaError, ValidationError, validate + from referencing import Registry + from referencing.exceptions import Unresolvable if result.structuredContent is None: raise RuntimeError( f"Tool {name} has an output schema but did not return structured content" ) # pragma: no cover + # An explicit empty registry: `$ref`s resolve within the schema document and the bundled metaschemas. + registry: Registry[Any] = Registry() try: - validate(result.structuredContent, output_schema) + validate(result.structuredContent, output_schema, registry=registry) except ValidationError as e: raise RuntimeError(f"Invalid structured content returned by tool {name}: {e}") # pragma: no cover except SchemaError as e: # pragma: no cover raise RuntimeError(f"Invalid schema for tool {name}: {e}") # pragma: no cover + except Unresolvable as e: + # A `$ref` did not resolve within the schema document. + raise RuntimeError(f"Invalid schema for tool {name}: {e}") from e @overload @deprecated("Use list_prompts(params=PaginatedRequestParams(...)) instead") diff --git a/src/mcp/client/sse.py b/src/mcp/client/sse.py index 0d7fa0fb46..08e8927887 100644 --- a/src/mcp/client/sse.py +++ b/src/mcp/client/sse.py @@ -8,10 +8,15 @@ import httpx from anyio.abc import TaskStatus from anyio.streams.memory import MemoryObjectReceiveStream, MemoryObjectSendStream -from httpx_sse import SSEError, aconnect_sse +from httpx_sse import SSEError import mcp.types as types -from mcp.shared._httpx_utils import McpHttpClientFactory, create_mcp_http_client +from mcp.shared._httpx_utils import ( + McpHttpClientFactory, + create_mcp_http_client, + request_within_origin, + sse_within_origin, +) from mcp.shared.message import SessionMessage logger = logging.getLogger(__name__) @@ -47,6 +52,13 @@ async def sse_client( headers: Optional headers to include in requests. timeout: HTTP timeout for regular operations. sse_read_timeout: Timeout for SSE read operations. + httpx_client_factory: Factory function for creating the httpx client. Whichever client it + returns, MCP requests follow a redirect only when it stays on the endpoint's origin + (same scheme, host and port, or http to https on the same host with default ports) and + keeps the request method (any status for the SSE GET, 307/308 for a message POST); any + other redirect is not followed, so connecting fails with `httpx.HTTPStatusError` for + the redirect response. The client's `follow_redirects` setting is not consulted; the + SDK's OAuth providers apply the same rule to the requests they make. auth: Optional HTTPX authentication handler. on_session_created: Optional callback invoked with the session ID when received. """ @@ -65,11 +77,7 @@ async def sse_client( async with httpx_client_factory( headers=headers, auth=auth, timeout=httpx.Timeout(timeout, read=sse_read_timeout) ) as client: - async with aconnect_sse( - client, - "GET", - url, - ) as event_source: + async with sse_within_origin(client, url) as event_source: event_source.response.raise_for_status() logger.debug("SSE connection established") @@ -135,7 +143,9 @@ async def post_writer(endpoint_url: str): async with write_stream_reader: async for session_message in write_stream_reader: logger.debug(f"Sending client message: {session_message}") - response = await client.post( + response = await request_within_origin( + client, + "POST", endpoint_url, json=session_message.message.model_dump( by_alias=True, @@ -161,3 +171,7 @@ async def post_writer(endpoint_url: str): finally: await read_stream_writer.aclose() await write_stream.aclose() + # The receive sides too, so that failing to connect (which raises before the + # streams are handed to the caller) does not leave them to the garbage collector. + await read_stream.aclose() + await write_stream_reader.aclose() diff --git a/src/mcp/client/streamable_http.py b/src/mcp/client/streamable_http.py index ed28fcc275..4fd743b9c4 100644 --- a/src/mcp/client/streamable_http.py +++ b/src/mcp/client/streamable_http.py @@ -19,12 +19,16 @@ import httpx from anyio.abc import TaskGroup from anyio.streams.memory import MemoryObjectReceiveStream, MemoryObjectSendStream -from httpx_sse import EventSource, ServerSentEvent, aconnect_sse +from httpx_sse import EventSource, ServerSentEvent from typing_extensions import deprecated from mcp.shared._httpx_utils import ( McpHttpClientFactory, create_mcp_http_client, + redirect_location, + request_within_origin, + sse_within_origin, + stream_within_origin, ) from mcp.shared.message import ClientMessageMetadata, SessionMessage from mcp.types import ( @@ -72,6 +76,28 @@ class ResumptionError(StreamableHTTPError): """Raised when resumption request is invalid.""" +def _unfollowed_redirect(response: httpx.Response) -> str | None: + """Describe a redirect `stream_within_origin` left unfollowed, or None if `response` is not one.""" + location = redirect_location(response) + if location is None: + return None + if response.request.url.scheme == "https" and location.scheme == "http": + return ( + f"Redirect to {location} not followed: it would downgrade this HTTPS endpoint to plain HTTP.\n" + "The server is likely behind a TLS-terminating proxy whose forwarded headers it does not trust,\n" + f"often combined with a trailing-slash difference. Try {location.copy_with(scheme='https')} instead, " + "or fix the proxy settings." + ) + return f"Redirect to {location} not followed; use that URL as the endpoint if it is the intended server" + + +def _raise_for_unfollowed_redirect(response: httpx.Response) -> None: + """Raise `httpx.HTTPStatusError`, as `raise_for_status()` does for a redirect response, saying why + this one was not followed.""" + if (redirect := _unfollowed_redirect(response)) is not None: + raise httpx.HTTPStatusError(redirect, request=response.request, response=response) + + @dataclass class RequestContext: """Context for a request operation.""" @@ -263,12 +289,11 @@ async def handle_get_stream( if last_event_id: headers[LAST_EVENT_ID] = last_event_id # pragma: no cover - async with aconnect_sse( - client, - "GET", - self.url, - headers=headers, - ) as event_source: + async with sse_within_origin(client, self.url, headers=headers) as event_source: + if (redirect := _unfollowed_redirect(event_source.response)) is not None: + # The same GET would be redirected again, so retrying cannot help. + logger.warning(f"GET stream not opened: {redirect}") + return event_source.response.raise_for_status() logger.debug("GET SSE connection established") @@ -311,12 +336,8 @@ async def _handle_resumption_request(self, ctx: RequestContext) -> None: if isinstance(ctx.session_message.message.root, JSONRPCRequest): # pragma: no branch original_request_id = ctx.session_message.message.root.id - async with aconnect_sse( - ctx.client, - "GET", - self.url, - headers=headers, - ) as event_source: + async with sse_within_origin(ctx.client, self.url, headers=headers) as event_source: + _raise_for_unfollowed_redirect(event_source.response) event_source.response.raise_for_status() logger.debug("Resumption GET SSE connection established") @@ -337,7 +358,8 @@ async def _handle_post_request(self, ctx: RequestContext) -> None: message = ctx.session_message.message is_initialization = self._is_initialization_request(message) - async with ctx.client.stream( + async with stream_within_origin( + ctx.client, "POST", self.url, json=message.model_dump(by_alias=True, mode="json", exclude_none=True), @@ -355,6 +377,7 @@ async def _handle_post_request(self, ctx: RequestContext) -> None: ) # pragma: no cover return # pragma: no cover + _raise_for_unfollowed_redirect(response) response.raise_for_status() if is_initialization: self._maybe_extract_session_id_from_response(response) @@ -460,12 +483,7 @@ async def _handle_reconnection( original_request_id = ctx.session_message.message.root.id try: - async with aconnect_sse( - ctx.client, - "GET", - self.url, - headers=headers, - ) as event_source: + async with sse_within_origin(ctx.client, self.url, headers=headers) as event_source: event_source.response.raise_for_status() logger.info("Reconnected to SSE stream") @@ -583,7 +601,7 @@ async def terminate_session(self, client: httpx.AsyncClient) -> None: # pragma: try: headers = self._prepare_headers() - response = await client.delete(self.url, headers=headers) + response = await request_within_origin(client, "DELETE", self.url, headers=headers) if response.status_code == 405: logger.debug("Server does not allow session termination") @@ -619,6 +637,13 @@ async def streamable_http_client( http_client: Optional pre-configured httpx.AsyncClient. If None, a default client with recommended MCP timeouts will be created. To configure headers, authentication, or other HTTP settings, create an httpx.AsyncClient and pass it here. + Whichever client is used, MCP requests follow a redirect only when it stays on the + endpoint's origin (same scheme, host and port, or http to https on the same host with + default ports) and keeps the request method (307/308 for a POST; any status for the GET + stream); any other redirect is not followed and, like any non-2xx response, raises + `httpx.HTTPStatusError`, here naming the location. The client's `follow_redirects` + setting is not consulted; the SDK's OAuth providers apply the same rule to the + requests they make. terminate_on_close: If True, send a DELETE request to terminate the session when the context exits. diff --git a/src/mcp/server/auth/middleware/bearer_auth.py b/src/mcp/server/auth/middleware/bearer_auth.py index 300b298924..cfa0a345c6 100644 --- a/src/mcp/server/auth/middleware/bearer_auth.py +++ b/src/mcp/server/auth/middleware/bearer_auth.py @@ -1,14 +1,17 @@ import json +import logging import time from typing import Any, TypedDict -from pydantic import AnyHttpUrl +from pydantic import AnyHttpUrl, ValidationError from starlette.authentication import AuthCredentials, AuthenticationBackend, SimpleUser from starlette.requests import HTTPConnection from starlette.types import Receive, Scope, Send from mcp.server.auth.provider import AccessToken, TokenVerifier +logger = logging.getLogger(__name__) + class AuthenticatedUser(SimpleUser): """User with authentication info.""" @@ -46,10 +49,14 @@ def authorization_context(user: AuthenticatedUser) -> AuthorizationContext: class BearerAuthBackend(AuthenticationBackend): """ Authentication backend that validates Bearer tokens using a TokenVerifier. + + When `resource_server_url` is given, only a token whose `AccessToken.resource` + (its RFC 8707 resource indicator / audience) is that URL is accepted. """ - def __init__(self, token_verifier: TokenVerifier): + def __init__(self, token_verifier: TokenVerifier, *, resource_server_url: AnyHttpUrl | None = None): self.token_verifier = token_verifier + self.resource_server_url = resource_server_url async def authenticate(self, conn: HTTPConnection): auth_header = next( @@ -70,8 +77,22 @@ async def authenticate(self, conn: HTTPConnection): if auth_info.expires_at and auth_info.expires_at < int(time.time()): return None + if self.resource_server_url and not self._issued_for_this_resource(auth_info.resource): + logger.warning( + "Bearer token resource %r is not resource_server_url %s", auth_info.resource, self.resource_server_url + ) + return None + return AuthCredentials(auth_info.scopes), AuthenticatedUser(auth_info) + def _issued_for_this_resource(self, resource: str | None) -> bool: + """Compare as URLs (so case and default-port spelling do not matter), a trailing slash aside.""" + try: + token_resource = str(AnyHttpUrl(resource or "")) + except ValidationError: + return False + return token_resource.removesuffix("/") == str(self.resource_server_url).removesuffix("/") + class RequireAuthMiddleware: """ diff --git a/src/mcp/server/auth/provider.py b/src/mcp/server/auth/provider.py index 310baff5fd..ff462c7a34 100644 --- a/src/mcp/server/auth/provider.py +++ b/src/mcp/server/auth/provider.py @@ -33,6 +33,7 @@ class RefreshToken(BaseModel): client_id: str scopes: list[str] expires_at: int | None = None + resource: str | None = None # RFC 8707 resource indicator; propagate to refreshed AccessTokens subject: str | None = None # resource owner; propagate to refreshed AccessTokens @@ -97,7 +98,15 @@ class TokenVerifier(Protocol): """Protocol for verifying bearer tokens.""" async def verify_token(self, token: str) -> AccessToken | None: - """Verify a bearer token and return access info if valid.""" + """Verify a bearer token and return access info if valid. + + Set `AccessToken.resource` to the resource the token was issued for (its RFC 8707 + resource indicator / `aud`; for a list, the entry equal to the server's + `AuthSettings.resource_server_url`). With `AuthSettings.validate_token_resource` the + bearer middleware then refuses any token whose resource is not `resource_server_url`; + without it, confirming the token was issued for this server (for example by passing the + expected audience to your JWT library) is up to the verifier. + """ # NOTE: FastMCP doesn't render any of these types in the user response, so it's diff --git a/src/mcp/server/auth/settings.py b/src/mcp/server/auth/settings.py index 1649826db2..6d2042e1af 100644 --- a/src/mcp/server/auth/settings.py +++ b/src/mcp/server/auth/settings.py @@ -1,4 +1,7 @@ -from pydantic import AnyHttpUrl, BaseModel, Field +import warnings + +from pydantic import AnyHttpUrl, BaseModel, Field, model_validator +from typing_extensions import Self class ClientRegistrationOptions(BaseModel): @@ -28,3 +31,26 @@ class AuthSettings(BaseModel): description="The URL of the MCP server to be used as the resource identifier " "and base route to look up OAuth Protected Resource Metadata.", ) + validate_token_resource: bool | None = Field( + default=None, + description="Only accept tokens the token verifier reports as issued for `resource_server_url` " + "(`AccessToken.resource`, the RFC 8707 resource indicator). Enable it when your authorization " + "server binds tokens to the `resource` the client requested; set it to False when your token " + "verifier checks the token's audience itself. With `resource_server_url` set, leaving it unset warns " + "and behaves as False; 3.0 makes True the default there.", + ) + + @model_validator(mode="after") + def _check_validate_token_resource(self) -> Self: + if self.validate_token_resource and self.resource_server_url is None: + raise ValueError("validate_token_resource requires resource_server_url") + if self.validate_token_resource is None and self.resource_server_url is not None: + warnings.warn( + "`AuthSettings.validate_token_resource` is not set, so bearer tokens are not checked " + "against `resource_server_url`; it will default to True in 3.0 when `resource_server_url` is " + "set. Set it to True to have the server refuse tokens issued for another resource, or to " + "False if your TokenVerifier validates the token's audience itself.", + DeprecationWarning, + stacklevel=3, + ) + return self diff --git a/src/mcp/server/fastmcp/server.py b/src/mcp/server/fastmcp/server.py index e412b81833..931379ca0b 100644 --- a/src/mcp/server/fastmcp/server.py +++ b/src/mcp/server/fastmcp/server.py @@ -62,7 +62,11 @@ from mcp.server.sse import SseServerTransport from mcp.server.stdio import stdio_server from mcp.server.streamable_http import EventStore -from mcp.server.streamable_http_manager import StreamableHTTPSessionManager +from mcp.server.streamable_http_manager import ( + DEFAULT_MAX_SESSIONS, + DEFAULT_SESSION_IDLE_TIMEOUT, + StreamableHTTPSessionManager, +) from mcp.server.transport_security import DEFAULT_MAX_REQUEST_BODY_SIZE, TransportSecuritySettings from mcp.shared.context import LifespanContextT, RequestContext, RequestT from mcp.types import Annotations, AnyFunction, ContentBlock, GetPromptResult, Icon, ToolAnnotations @@ -108,6 +112,10 @@ class Settings(BaseSettings, Generic[LifespanResultT]): """Define if the server should create a new transport per request.""" max_request_body_size: int """Maximum request body size in bytes for the Streamable HTTP endpoint and the SSE message endpoint.""" + session_idle_timeout: float | None + """Seconds a stateful session may have no request in flight before it is closed. None disables expiry.""" + max_sessions: int | None + """Maximum number of concurrent stateful sessions. None removes the limit.""" # resource settings warn_on_duplicate_resources: bool @@ -169,6 +177,8 @@ def __init__( # noqa: PLR0913 json_response: bool = False, stateless_http: bool = False, max_request_body_size: int = DEFAULT_MAX_REQUEST_BODY_SIZE, + session_idle_timeout: float | None = DEFAULT_SESSION_IDLE_TIMEOUT, + max_sessions: int | None = DEFAULT_MAX_SESSIONS, warn_on_duplicate_resources: bool = True, warn_on_duplicate_tools: bool = True, warn_on_duplicate_prompts: bool = True, @@ -197,6 +207,8 @@ def __init__( # noqa: PLR0913 json_response=json_response, stateless_http=stateless_http, max_request_body_size=max_request_body_size, + session_idle_timeout=session_idle_timeout, + max_sessions=max_sessions, warn_on_duplicate_resources=warn_on_duplicate_resources, warn_on_duplicate_tools=warn_on_duplicate_tools, warn_on_duplicate_prompts=warn_on_duplicate_prompts, @@ -869,7 +881,12 @@ async def handle_sse(scope: Scope, receive: Receive, send: Send): # pragma: no # extract auth info from request (but do not require it) Middleware( AuthenticationMiddleware, - backend=BearerAuthBackend(self._token_verifier), + backend=BearerAuthBackend( + self._token_verifier, + resource_server_url=self.settings.auth.resource_server_url + if self.settings.auth.validate_token_resource + else None, + ), ), # Add the auth context middleware to store # authenticated user in a contextvar @@ -966,6 +983,8 @@ def streamable_http_app(self) -> Starlette: stateless=self.settings.stateless_http, # Use the stateless setting security_settings=self.settings.transport_security, max_request_body_size=self.settings.max_request_body_size, + session_idle_timeout=self.settings.session_idle_timeout, + max_sessions=self.settings.max_sessions, ) # Create the ASGI handler @@ -985,7 +1004,12 @@ def streamable_http_app(self) -> Starlette: middleware = [ Middleware( AuthenticationMiddleware, - backend=BearerAuthBackend(self._token_verifier), + backend=BearerAuthBackend( + self._token_verifier, + resource_server_url=self.settings.auth.resource_server_url + if self.settings.auth.validate_token_resource + else None, + ), ), Middleware(AuthContextMiddleware), ] diff --git a/src/mcp/server/streamable_http.py b/src/mcp/server/streamable_http.py index 8e8d902ccc..2e8236d973 100644 --- a/src/mcp/server/streamable_http.py +++ b/src/mcp/server/streamable_http.py @@ -9,6 +9,7 @@ import json import logging +import math import re from abc import ABC, abstractmethod from collections.abc import AsyncGenerator, Awaitable, Callable @@ -150,6 +151,7 @@ def __init__( event_store: EventStore | None = None, security_settings: TransportSecuritySettings | None = None, retry_interval: int | None = None, + idle_timeout: float | None = None, ) -> None: """ Initialize a new StreamableHTTP server transport. @@ -167,12 +169,22 @@ def __init__( retry field. When set, the server will send a retry field in SSE priming events to control client reconnection timing for polling behavior. Only used when event_store is provided. + idle_timeout: Seconds the session may go without any request in flight before + `idle_scope` is cancelled. A request being served or an open GET + stream holds the session open; the countdown starts each time the + last in-flight request completes. The host waits on `idle_scope` + (available once `connect()` has been entered) and ends the session + when it fires. Default is None: no `idle_scope`, the session never + expires. Raises: - ValueError: If the session ID contains invalid characters. + ValueError: If the session ID contains invalid characters, or if `idle_timeout` + is not a positive, finite number. """ if mcp_session_id is not None and not SESSION_ID_PATTERN.fullmatch(mcp_session_id): raise ValueError("Session ID must only contain visible ASCII characters (0x21-0x7E)") + if idle_timeout is not None and not (math.isfinite(idle_timeout) and idle_timeout > 0): + raise ValueError("idle_timeout must be a positive, finite number of seconds") self.mcp_session_id = mcp_session_id self.is_json_response_enabled = is_json_response_enabled @@ -188,8 +200,11 @@ def __init__( ] = {} self._sse_stream_writers: dict[RequestId, MemoryObjectSendStream[SSEEvent]] = {} self._terminated = False - # Idle timeout cancel scope; managed by the session manager. + self._idle_timeout = idle_timeout + self._requests_in_flight = 0 self.idle_scope: anyio.CancelScope | None = None + """Created when `connect()` is entered if `idle_timeout` is set; cancelled once no request has been in + flight for `idle_timeout` seconds.""" @property def is_terminated(self) -> bool: @@ -402,6 +417,32 @@ async def _clean_up_memory_streams(self, request_id: RequestId) -> None: # prag async def handle_request(self, scope: Scope, receive: Receive, send: Send) -> None: """Application entry point that handles all HTTP requests""" + if self.idle_scope is None or self._idle_timeout is None: + await self._handle_request(scope, receive, send) + return + + if self.idle_scope.cancel_called: + # The idle period already ran out and the host is ending this + # session: answer as terminated rather than dispatch into a + # message loop that is going away. + if not self._terminated: + await self.terminate() + await self._handle_request(scope, receive, send) + return + + # A request in flight (an open GET stream included) holds the session: + # the idle countdown is suspended while any is being served and + # restarts when the last one completes. + self._requests_in_flight += 1 + self.idle_scope.deadline = math.inf + try: + await self._handle_request(scope, receive, send) + finally: + self._requests_in_flight -= 1 + if not self._requests_in_flight: + self.idle_scope.deadline = anyio.current_time() + self._idle_timeout + + async def _handle_request(self, scope: Scope, receive: Receive, send: Send) -> None: request = Request(scope, receive) # Validate request headers for DNS rebinding protection @@ -643,8 +684,9 @@ async def _handle_post_request(self, scope: Scope, request: Request, receive: Re except Exception: logger.exception("SSE response error") await sse_stream_writer.aclose() - await sse_stream_reader.aclose() await self._clean_up_memory_streams(request_id) + finally: + await sse_stream_reader.aclose() except Exception as err: logger.exception("Error handling POST request") @@ -747,9 +789,10 @@ async def standalone_sse_writer(): await response(request.scope, request.receive, send) except Exception: logger.exception("Error in standalone SSE response") + await self._clean_up_memory_streams(GET_STREAM_KEY) + finally: await sse_stream_writer.aclose() await sse_stream_reader.aclose() - await self._clean_up_memory_streams(GET_STREAM_KEY) async def _handle_delete_request(self, request: Request, send: Send) -> None: # pragma: no cover """Handle DELETE requests for explicit session termination.""" @@ -992,6 +1035,8 @@ async def connect( Yields: Tuple of (read_stream, write_stream) for bidirectional communication """ + if self._idle_timeout is not None: + self.idle_scope = anyio.CancelScope() # Create the memory streams for this connection diff --git a/src/mcp/server/streamable_http_manager.py b/src/mcp/server/streamable_http_manager.py index dd8c2ae245..eb36296e8f 100644 --- a/src/mcp/server/streamable_http_manager.py +++ b/src/mcp/server/streamable_http_manager.py @@ -4,15 +4,16 @@ import contextlib import logging +import math from collections.abc import AsyncIterator -from typing import Any +from typing import Any, Final from uuid import uuid4 import anyio from anyio.abc import TaskStatus from starlette.requests import Request from starlette.responses import Response -from starlette.types import Receive, Scope, Send +from starlette.types import ASGIApp, Message, Receive, Scope, Send from mcp.server.auth.middleware.bearer_auth import AuthenticatedUser, AuthorizationContext, authorization_context from mcp.server.lowlevel.server import Server as MCPServer @@ -26,12 +27,24 @@ RequestBodyLimitMiddleware, TransportSecuritySettings, ) -from mcp.types import INVALID_REQUEST, ErrorData, JSONRPCError +from mcp.types import INTERNAL_ERROR, INVALID_REQUEST, ErrorData, JSONRPCError -__all__ = ["DEFAULT_MAX_REQUEST_BODY_SIZE", "RequestBodyLimitMiddleware", "StreamableHTTPSessionManager"] +__all__ = [ + "DEFAULT_MAX_REQUEST_BODY_SIZE", + "DEFAULT_MAX_SESSIONS", + "DEFAULT_SESSION_IDLE_TIMEOUT", + "RequestBodyLimitMiddleware", + "StreamableHTTPSessionManager", +] logger = logging.getLogger(__name__) +DEFAULT_SESSION_IDLE_TIMEOUT: Final = 30 * 60 +"""Default idle period in seconds after which a stateful Streamable HTTP session is closed (30 minutes).""" + +DEFAULT_MAX_SESSIONS: Final = 10_000 +"""Default maximum number of concurrent stateful Streamable HTTP sessions per session manager.""" + class StreamableHTTPSessionManager: """ @@ -44,7 +57,7 @@ class StreamableHTTPSessionManager: 2. Resumability via an optional event store 3. Connection management and lifecycle 4. Request handling and transport setup - 5. Idle session cleanup via optional timeout + 5. Idle session cleanup Important: Only one StreamableHTTPSessionManager instance should be created per application. The instance cannot be reused after its run() context has @@ -61,13 +74,18 @@ class StreamableHTTPSessionManager: security_settings: Optional transport security settings. retry_interval: Retry interval in milliseconds to suggest to clients in SSE retry field. Used for SSE polling behavior. - session_idle_timeout: Optional idle timeout in seconds for stateful sessions. If set, sessions that - receive no HTTP requests for this duration will be automatically terminated and removed. When - retry_interval is also configured, ensure the idle timeout comfortably exceeds the retry interval to - avoid reaping sessions during normal SSE polling gaps. Default is None (no timeout). A value of 1800 - (30 minutes) is recommended for most deployments. + session_idle_timeout: Idle timeout in seconds for stateful sessions. A session that has had no HTTP + request in flight for this long (no request being served, no open GET stream) is terminated and + removed; its ID then answers 404 and the client has to initialize a new session. When retry_interval + is also configured, ensure the idle timeout comfortably exceeds the retry interval to avoid reaping + sessions during normal SSE polling gaps. Defaults to 1800 (30 minutes); None disables the timeout so + sessions live until the client deletes them or the manager shuts down. Unused in stateless mode. max_request_body_size: Maximum size in bytes for Streamable HTTP request bodies. Requests that exceed this limit receive a 413 response before parsing or session creation. Defaults to 4 MiB. + max_sessions: Maximum number of concurrent stateful sessions. While that many sessions are open, a + request that would open another one receives a 503 response; existing sessions are unaffected and + room frees up as they end or expire. Defaults to 10 000; None removes the limit. Unused in stateless + mode. """ def __init__( @@ -78,15 +96,16 @@ def __init__( stateless: bool = False, security_settings: TransportSecuritySettings | None = None, retry_interval: int | None = None, - session_idle_timeout: float | None = None, + session_idle_timeout: float | None = DEFAULT_SESSION_IDLE_TIMEOUT, max_request_body_size: int = DEFAULT_MAX_REQUEST_BODY_SIZE, + max_sessions: int | None = DEFAULT_MAX_SESSIONS, ): - if session_idle_timeout is not None and session_idle_timeout <= 0: - raise ValueError("session_idle_timeout must be a positive number of seconds") - if stateless and session_idle_timeout is not None: - raise RuntimeError("session_idle_timeout is not supported in stateless mode") + if session_idle_timeout is not None and not (math.isfinite(session_idle_timeout) and session_idle_timeout > 0): + raise ValueError("session_idle_timeout must be a positive, finite number of seconds") if max_request_body_size <= 0: raise ValueError("max_request_body_size must be a positive number of bytes") + if max_sessions is not None and max_sessions <= 0: + raise ValueError("max_sessions must be a positive number of sessions or None") self.app = app self.event_store = event_store @@ -96,6 +115,7 @@ def __init__( self.retry_interval = retry_interval self.session_idle_timeout = session_idle_timeout self.max_request_body_size = max_request_body_size + self.max_sessions = max_sessions self.asgi_app = RequestBodyLimitMiddleware(self._handle_request, max_request_body_size) # Session tracking (only used if not stateless) @@ -224,16 +244,15 @@ async def run_stateless_server(*, task_status: TaskStatus[None] = anyio.TASK_STA except Exception: # pragma: no cover logger.exception("Stateless session crashed") - # Assert task group is not None for type checking + # The per-request server task only ends once the transport is + # terminated, so terminate it even if the request was cancelled. assert self._task_group is not None - # Start the server task - await self._task_group.start(run_stateless_server) - - # Handle the HTTP request and return the response - await http_transport.handle_request(scope, receive, send) - - # Terminate the transport after the request is handled - await http_transport.terminate() + try: + await self._task_group.start(run_stateless_server) + await http_transport.handle_request(scope, receive, send) + finally: + with anyio.CancelScope(shield=True): + await http_transport.terminate() async def _handle_stateful_request( self, @@ -265,100 +284,142 @@ async def _handle_stateful_request( "Rejecting request for session %s: credential does not match the one that created the session", request_mcp_session_id[:64], ) - body = JSONRPCError( - jsonrpc="2.0", id="server-error", error=ErrorData(code=INVALID_REQUEST, message="Session not found") - ) - response = Response( - body.model_dump_json(by_alias=True, exclude_none=True), - status_code=404, - media_type="application/json", - ) - await response(scope, receive, send) + await _error_response("Session not found", 404)(scope, receive, send) return logger.debug("Session already exists, handling request directly") - # Push back idle deadline on activity - if transport.idle_scope is not None and self.session_idle_timeout is not None: # pragma: no cover - transport.idle_scope.deadline = anyio.current_time() + self.session_idle_timeout await transport.handle_request(scope, receive, send) + if transport.is_terminated: + # The client ended the session (DELETE): forget it now rather + # than when its server task winds down. + await self._discard_session(request_mcp_session_id, transport) return if request_mcp_session_id is None: - # New session case - logger.debug("Creating new transport") + # New session case. Admission (the session limit and registration) + # is decided under the lock; the request itself is served outside + # it, so one client that is slow to send its opening request does + # not hold up the others. async with self._session_creation_lock: - new_session_id = uuid4().hex - http_transport = StreamableHTTPServerTransport( - mcp_session_id=new_session_id, - is_json_response_enabled=self.json_response, - event_store=self.event_store, # May be None (no resumability) - security_settings=self.security_settings, - retry_interval=self.retry_interval, - ) - - assert http_transport.mcp_session_id is not None - if requestor is not None: - self._session_owners[http_transport.mcp_session_id] = requestor - self._server_instances[http_transport.mcp_session_id] = http_transport - logger.info(f"Created new transport with session ID: {new_session_id}") - - # Define the server runner - async def run_server(*, task_status: TaskStatus[None] = anyio.TASK_STATUS_IGNORED) -> None: - async with http_transport.connect() as streams: - read_stream, write_stream = streams - task_status.started() - try: - # Use a cancel scope for idle timeout — when the - # deadline passes the scope cancels app.run() and - # execution continues after the ``with`` block. - # Incoming requests push the deadline forward. - idle_scope = anyio.CancelScope() - if self.session_idle_timeout is not None: - idle_scope.deadline = anyio.current_time() + self.session_idle_timeout - http_transport.idle_scope = idle_scope - - with idle_scope: - await self.app.run( - read_stream, - write_stream, - self.app.create_initialization_options(), - stateless=False, - ) - - if idle_scope.cancelled_caught: - assert http_transport.mcp_session_id is not None - logger.info(f"Session {http_transport.mcp_session_id} idle timeout") - self._server_instances.pop(http_transport.mcp_session_id, None) - self._session_owners.pop(http_transport.mcp_session_id, None) - await http_transport.terminate() - except Exception: - logger.exception(f"Session {http_transport.mcp_session_id} crashed") - finally: - if ( # pragma: no branch - http_transport.mcp_session_id - and http_transport.mcp_session_id in self._server_instances - and not http_transport.is_terminated - ): - logger.info( - "Cleaning up crashed session " - f"{http_transport.mcp_session_id} from " - "active instances." - ) - del self._server_instances[http_transport.mcp_session_id] - self._session_owners.pop(http_transport.mcp_session_id, None) - - # Assert task group is not None for type checking - assert self._task_group is not None - # Start the server task - await self._task_group.start(run_server) - - # Handle the HTTP request and return the response - await http_transport.handle_request(scope, receive, send) + http_transport = self._admit_session(requestor) + if http_transport is None: + logger.warning("Refusing to open a new session: %d sessions are already open", self.max_sessions) + await _error_response("Too many open sessions", 503, INTERNAL_ERROR)(scope, receive, send) + return + await self._serve_opening_request(http_transport, scope, receive, send) else: # Unknown or expired session ID - return 404 per MCP spec - body = JSONRPCError( - jsonrpc="2.0", id="server-error", error=ErrorData(code=INVALID_REQUEST, message="Session not found") - ) - response = Response( - body.model_dump_json(by_alias=True, exclude_none=True), status_code=404, media_type="application/json" - ) - await response(scope, receive, send) + await _error_response("Session not found", 404)(scope, receive, send) + + def _admit_session(self, requestor: AuthorizationContext | None) -> StreamableHTTPServerTransport | None: + """Register a new session for `requestor` and return its transport, or None at the session limit.""" + if self.max_sessions is not None and len(self._server_instances) >= self.max_sessions: + return None + http_transport = StreamableHTTPServerTransport( + mcp_session_id=uuid4().hex, + is_json_response_enabled=self.json_response, + event_store=self.event_store, # May be None (no resumability) + security_settings=self.security_settings, + retry_interval=self.retry_interval, + idle_timeout=self.session_idle_timeout, + ) + session_id = http_transport.mcp_session_id + assert session_id is not None + if requestor is not None: + self._session_owners[session_id] = requestor + self._server_instances[session_id] = http_transport + logger.info(f"Created new transport with session ID: {session_id}") + return http_transport + + async def _serve_opening_request( + self, http_transport: StreamableHTTPServerTransport, scope: Scope, receive: Receive, send: Send + ) -> None: + """Start the session's server task and let its transport answer the request that opens it. + + Without a session ID only an initialize request can succeed, so if this + one is refused, fails or is cancelled (or the session's server task + cannot even be started) nothing was established: the session is + discarded again rather than kept (with its server task) around. + """ + session_id = http_transport.mcp_session_id + assert session_id is not None + + async def run_server(*, task_status: TaskStatus[None] = anyio.TASK_STATUS_IGNORED) -> None: + async with http_transport.connect() as streams: + read_stream, write_stream = streams + task_status.started() + try: + async with anyio.create_task_group() as session_tg: + if http_transport.idle_scope is not None: + + async def end_when_idle(idle_scope: anyio.CancelScope) -> None: + # The transport cancels this scope once no request + # has been in flight for `session_idle_timeout`. + with idle_scope: + await anyio.sleep_forever() + logger.info(f"Session {session_id} idle timeout") + # Discarding the session closes the transport's + # streams, so app.run() returns the way it does + # after a client DELETE and the server's lifespan + # for this session is torn down normally. + await self._discard_session(session_id, http_transport) + + session_tg.start_soon(end_when_idle, http_transport.idle_scope) + await self.app.run( + read_stream, + write_stream, + self.app.create_initialization_options(), + stateless=False, + ) + # The session ended some other way; stop waiting for it to go idle. + session_tg.cancel_scope.cancel() + except Exception: + logger.exception(f"Session {session_id} crashed") + finally: + # However the session ended (client DELETE, idle + # timeout, crash), discard it. + await self._discard_session(session_id, http_transport) + + established = False + try: + assert self._task_group is not None + await self._task_group.start(run_server) + status = await _send_and_report_status(http_transport.handle_request, scope, receive, send) + established = status is not None and status < 400 + finally: + if not established: # pragma: no branch + await self._discard_session(session_id, http_transport) + + async def _discard_session(self, session_id: str, transport: StreamableHTTPServerTransport) -> None: + """Stop tracking the session and make sure its transport refuses anything that still reaches it. + + The session is forgotten first, before any await, so its ID answers 404 + from the moment this is called; terminating the transport is shielded so + it completes even while the caller is being cancelled. + """ + self._server_instances.pop(session_id, None) + self._session_owners.pop(session_id, None) + if not transport.is_terminated: + with anyio.CancelScope(shield=True): + await transport.terminate() + + +def _error_response(message: str, status_code: int, code: int = INVALID_REQUEST) -> Response: + """A JSON-RPC error body (no usable request id) with the given HTTP status.""" + body = JSONRPCError(jsonrpc="2.0", id="server-error", error=ErrorData(code=code, message=message)) + return Response( + body.model_dump_json(by_alias=True, exclude_none=True), status_code=status_code, media_type="application/json" + ) + + +async def _send_and_report_status(app: ASGIApp, scope: Scope, receive: Receive, send: Send) -> int | None: + """Run `app` for one request and return the HTTP status it answered with (None if it sent no response).""" + status: int | None = None + + async def watch_status(message: Message) -> None: + nonlocal status + if message["type"] == "http.response.start": + status = message["status"] + await send(message) + + await app(scope, receive, watch_status) + return status diff --git a/src/mcp/shared/_httpx_utils.py b/src/mcp/shared/_httpx_utils.py index 945ef80955..248e797efd 100644 --- a/src/mcp/shared/_httpx_utils.py +++ b/src/mcp/shared/_httpx_utils.py @@ -1,8 +1,12 @@ -"""Utilities for creating standardized httpx AsyncClient instances.""" +"""Utilities for creating and using httpx AsyncClient instances in the MCP transports.""" +from abc import ABC, abstractmethod +from collections.abc import AsyncGenerator +from contextlib import asynccontextmanager from typing import Any, Protocol import httpx +from httpx_sse import EventSource __all__ = ["create_mcp_http_client", "MCP_DEFAULT_TIMEOUT", "MCP_DEFAULT_SSE_READ_TIMEOUT"] @@ -10,6 +14,12 @@ MCP_DEFAULT_TIMEOUT = 30.0 # General operations (seconds) MCP_DEFAULT_SSE_READ_TIMEOUT = 300.0 # SSE streams - 5 minutes (seconds) +# The headers httpx_sse.aconnect_sse() adds to an event-stream request. +_SSE_HEADERS = {"Accept": "text/event-stream", "Cache-Control": "no-store"} + +# How many redirects one auth-flow request may follow within its origin (see RedirectAwareAuth). +_AUTH_REDIRECT_LIMIT = 5 + class McpHttpClientFactory(Protocol): # pragma: no branch def __call__( # pragma: no branch @@ -25,63 +35,185 @@ def create_mcp_http_client( timeout: httpx.Timeout | None = None, auth: httpx.Auth | None = None, ) -> httpx.AsyncClient: - """Create a standardized httpx AsyncClient with MCP defaults. + """Create an httpx AsyncClient with the MCP transports' default timeouts. - This function provides common defaults used throughout the MCP codebase: - - follow_redirects=True (always enabled) - - Default timeout of 30 seconds if not specified + The client uses a 30-second timeout for connect/write/pool and a 300-second + read timeout, because a server may hold a response stream open. Redirect + following is left at the httpx default (off): the MCP transports follow + redirects within the endpoint's origin themselves, see `stream_within_origin`. Args: headers: Optional headers to include with all requests. - timeout: Request timeout as httpx.Timeout object. - Defaults to 30 seconds if not specified. + timeout: Request timeout as httpx.Timeout object. Defaults to 30s for + connect/write/pool and 300s for read (for long-lived SSE streams). auth: Optional authentication handler. Returns: - Configured httpx.AsyncClient instance with MCP defaults. + Configured httpx.AsyncClient instance. Note: The returned AsyncClient must be used as a context manager to ensure proper cleanup of connections. - - Examples: - # Basic usage with MCP defaults - async with create_mcp_http_client() as client: - response = await client.get("https://api.example.com") - - # With custom headers - headers = {"Authorization": "Bearer token"} - async with create_mcp_http_client(headers) as client: - response = await client.get("/endpoint") - - # With both custom headers and timeout - timeout = httpx.Timeout(60.0, read=300.0) - async with create_mcp_http_client(headers, timeout) as client: - response = await client.get("/long-request") - - # With authentication - from httpx import BasicAuth - auth = BasicAuth(username="user", password="pass") - async with create_mcp_http_client(headers, timeout, auth) as client: - response = await client.get("/protected-endpoint") """ - # Set MCP defaults - kwargs: dict[str, Any] = { - "follow_redirects": True, - } - - # Handle timeout if timeout is None: - kwargs["timeout"] = httpx.Timeout(MCP_DEFAULT_TIMEOUT, read=MCP_DEFAULT_SSE_READ_TIMEOUT) - else: - kwargs["timeout"] = timeout - - # Handle headers + timeout = httpx.Timeout(MCP_DEFAULT_TIMEOUT, read=MCP_DEFAULT_SSE_READ_TIMEOUT) + kwargs: dict[str, Any] = {"timeout": timeout} if headers is not None: kwargs["headers"] = headers - - # Handle authentication if auth is not None: # pragma: no cover kwargs["auth"] = auth - return httpx.AsyncClient(**kwargs) + + +def _within_origin(url: httpx.URL, location: httpx.URL) -> bool: + """Whether `location` is on `url`'s origin, or is its https upgrade on the default ports. + + httpx normalises a scheme's default port to None and lower-cases hosts, so + plain tuple comparison is exact. The upgrade rule is the one httpx itself + uses to decide a redirect has not left the origin (`_is_https_redirect`). + """ + if (url.scheme, url.host, url.port) == (location.scheme, location.host, location.port): + return True + return ( + url.host == location.host + and url.scheme == "http" + and url.port is None + and location.scheme == "https" + and location.port is None + ) + + +def next_request_within_origin(response: httpx.Response) -> httpx.Request | None: + """The request that follows `response`'s redirect, if it is one the MCP transports follow. + + That is when httpx built a next request for it (a redirect status with a + Location), the next request keeps the method (307/308, or any redirect of a + GET: httpx turns a POST into a body-less GET for 301/302/303, which would + drop the message), its URL stays within the origin of the request just sent + (same scheme, host and port, or http to https on the same host with default + ports), and the Location does not bring userinfo of its own (which httpx + would otherwise send as Basic auth; userinfo the configured URL already had + is kept by a relative Location and is fine). None for anything else, + including a non-redirect. + """ + next_request = response.next_request + if next_request is None: + return None + sent = response.request + if ( + next_request.method != sent.method + or (next_request.url.userinfo and next_request.url.userinfo != sent.url.userinfo) + or not _within_origin(sent.url, next_request.url) + ): + return None + return next_request + + +@asynccontextmanager +async def stream_within_origin( + client: httpx.AsyncClient, method: str, url: httpx.URL | str, **kwargs: Any +) -> AsyncGenerator[httpx.Response, None]: + """`client.stream(...)`, following redirects only while they stay within the request's origin. + + An MCP transport talks to one configured endpoint, and everything on a request + (headers, auth, body) was configured for that endpoint. A redirect that + `next_request_within_origin` accepts, such as a 307/308 trailing-slash + normalisation, is followed, at most `client.max_redirects` times. Any other + redirect (or one past that budget) is not followed: the redirect response + itself is yielded, the way httpx hands one back when `follow_redirects` is + off, and the caller treats it as the non-success it is. The client's own + `follow_redirects` setting is not consulted. Requests an `httpx.Auth` flow + makes during the call are sent without following either; the SDK's OAuth + providers apply the same rule to their own requests. + """ + request = client.build_request(method, url, **kwargs) + followed = 0 + while True: + response = await client.send(request, stream=True, follow_redirects=False) + next_request = next_request_within_origin(response) + if next_request is None or followed == client.max_redirects: + break + try: + # Drain the redirect body so the connection returns to the pool, as httpx does when it follows. + await response.aread() + finally: + await response.aclose() + request = next_request + followed += 1 + try: + yield response + finally: + await response.aclose() + + +async def request_within_origin( + client: httpx.AsyncClient, method: str, url: httpx.URL | str, **kwargs: Any +) -> httpx.Response: + """`client.request(...)` with the redirect handling of `stream_within_origin`.""" + async with stream_within_origin(client, method, url, **kwargs) as response: + await response.aread() + return response + + +@asynccontextmanager +async def sse_within_origin( + client: httpx.AsyncClient, url: httpx.URL | str, *, headers: dict[str, str] | None = None +) -> AsyncGenerator[EventSource, None]: + """`httpx_sse.aconnect_sse(client, "GET", url)` with the redirect handling of `stream_within_origin`.""" + merged = httpx.Headers(_SSE_HEADERS) + merged.update(headers or {}) + async with stream_within_origin(client, "GET", url, headers=merged) as response: + yield EventSource(response) + + +def redirect_location(response: httpx.Response) -> httpx.URL | None: + """Where `response` redirects to, for use in a message: without userinfo, query or fragment, + which can carry state that does not belong in an error or a log line. None if not a redirect.""" + if response.next_request is None: + return None + return response.next_request.url.copy_with(userinfo=b"", query=None, fragment=None) + + +def redirect_note(response: httpx.Response) -> str: + """A suffix naming the location of a redirect response that was not followed, else empty.""" + location = redirect_location(response) + if location is None: + return "" + return f" (redirected to {location}; not followed)" + + +class RedirectAwareAuth(ABC, httpx.Auth): + """An `httpx.Auth` whose own requests follow redirects the way MCP transport requests do. + + The transports send every request with redirect following off and follow a + redirect themselves only within the endpoint's origin (`stream_within_origin`). + httpx applies that per-request setting to the requests an auth flow makes + too (metadata discovery, registration, token), so on their own those would + follow nothing. Subclasses write their flow as `_auth_flow`; this class + drives it and, for each request the flow makes other than the one being + authenticated, follows a redirect that `next_request_within_origin` accepts, + up to `_AUTH_REDIRECT_LIMIT` times. Any other redirect response is handed + to the flow as it is. + """ + + @abstractmethod + def _auth_flow(self, request: httpx.Request) -> AsyncGenerator[httpx.Request, httpx.Response]: + """The subclass's flow, written as `httpx.Auth.async_auth_flow` otherwise would be.""" + + async def async_auth_flow(self, request: httpx.Request) -> AsyncGenerator[httpx.Request, httpx.Response]: + flow = self._auth_flow(request) + try: + outgoing = await flow.__anext__() + while True: + response = yield outgoing + if outgoing is not request: + for _ in range(_AUTH_REDIRECT_LIMIT): + follow = next_request_within_origin(response) + if follow is None: + break + response = yield follow + outgoing = await flow.asend(response) + except StopAsyncIteration: + return + finally: + await flow.aclose() diff --git a/src/mcp/shared/auth.py b/src/mcp/shared/auth.py index a985bef3f1..59cf2f5723 100644 --- a/src/mcp/shared/auth.py +++ b/src/mcp/shared/auth.py @@ -121,6 +121,9 @@ class OAuthClientInformationFull(OAuthClientMetadata): client_secret: str | None = None client_id_issued_at: int | None = None client_secret_expires_at: int | None = None + # SEP-2352: the issuer these credentials were registered with, recorded by the SDK (not an + # RFC 7591 field) to detect authorization-server migration and avoid cross-AS credential reuse. + issuer: str | None = None class OAuthMetadata(BaseModel): diff --git a/tests/client/auth/extensions/test_client_credentials.py b/tests/client/auth/extensions/test_client_credentials.py index 6d134af742..bb64590673 100644 --- a/tests/client/auth/extensions/test_client_credentials.py +++ b/tests/client/auth/extensions/test_client_credentials.py @@ -1,9 +1,13 @@ import urllib.parse +from collections.abc import AsyncGenerator +import httpx import jwt import pytest +from inline_snapshot import snapshot from pydantic import AnyHttpUrl, AnyUrl +from mcp.client.auth import OAuthClientProvider, OAuthFlowError from mcp.client.auth.extensions.client_credentials import ( ClientCredentialsOAuthProvider, JWTParameters, @@ -185,6 +189,7 @@ async def test_init_sets_client_info(self, mock_storage: MockTokenStorage): storage=mock_storage, client_id="test-client-id", client_secret="test-client-secret", + issuer="https://api.example.com", ) # client_info is set during _initialize @@ -205,6 +210,7 @@ async def test_init_with_scopes(self, mock_storage: MockTokenStorage): client_id="test-client-id", client_secret="test-client-secret", scopes="read write", + issuer="https://api.example.com", ) await provider._initialize() @@ -220,6 +226,7 @@ async def test_init_with_client_secret_post(self, mock_storage: MockTokenStorage client_id="test-client-id", client_secret="test-client-secret", token_endpoint_auth_method="client_secret_post", + issuer="https://api.example.com", ) await provider._initialize() @@ -235,6 +242,7 @@ async def test_exchange_token_client_credentials(self, mock_storage: MockTokenSt client_id="test-client-id", client_secret="test-client-secret", scopes="read write", + issuer="https://api.example.com", ) provider.context.oauth_metadata = OAuthMetadata( issuer=AnyHttpUrl("https://api.example.com"), @@ -261,6 +269,7 @@ async def test_exchange_token_without_scopes(self, mock_storage: MockTokenStorag storage=mock_storage, client_id="test-client-id", client_secret="test-client-secret", + issuer="https://api.example.com", ) provider.context.oauth_metadata = OAuthMetadata( issuer=AnyHttpUrl("https://api.example.com"), @@ -292,6 +301,7 @@ async def mock_assertion_provider(audience: str) -> str: # pragma: no cover storage=mock_storage, client_id="test-client-id", assertion_provider=mock_assertion_provider, + issuer="https://api.example.com", ) # client_info is set during _initialize @@ -315,6 +325,7 @@ async def mock_assertion_provider(audience: str) -> str: client_id="test-client-id", assertion_provider=mock_assertion_provider, scopes="read write", + issuer="https://auth.example.com", ) provider.context.oauth_metadata = OAuthMetadata( issuer=AnyHttpUrl("https://auth.example.com"), @@ -346,6 +357,7 @@ async def mock_assertion_provider(audience: str) -> str: storage=mock_storage, client_id="test-client-id", assertion_provider=mock_assertion_provider, + issuer="https://auth.example.com", ) provider.context.oauth_metadata = OAuthMetadata( issuer=AnyHttpUrl("https://auth.example.com"), @@ -429,3 +441,266 @@ async def test_returns_static_token(self): assert result1 == token assert result2 == token + + +_SERVER_URL = "https://api.example.com/v1/mcp" +_CONFIGURED_ISSUER = "https://auth.example.com" + + +def _metadata_for(issuer: str) -> dict[str, str]: + return {"issuer": issuer, "authorization_endpoint": f"{issuer}/authorize", "token_endpoint": f"{issuer}/token"} + + +def _provider_with_issuer(kind: str, storage: MockTokenStorage, audiences: list[str]) -> OAuthClientProvider: + """A ClientCredentials ("secret") or PrivateKeyJWT ("jwt") provider configured for _CONFIGURED_ISSUER; + `audiences` records every audience an assertion is minted for.""" + if kind == "secret": + return ClientCredentialsOAuthProvider( + server_url=_SERVER_URL, storage=storage, client_id="cid", client_secret="csecret", issuer=_CONFIGURED_ISSUER + ) + + async def assertion_provider(audience: str) -> str: + audiences.append(audience) + return "signed-assertion" + + return PrivateKeyJWTOAuthProvider( + server_url=_SERVER_URL, + storage=storage, + client_id="cid", + assertion_provider=assertion_provider, + issuer=_CONFIGURED_ISSUER, + ) + + +async def _answer_discovery( + flow: AsyncGenerator[httpx.Request, httpx.Response], + *, + authorization_server: str | list[str] | None, + metadata: dict[str, str] | None, +) -> httpx.Request: + """Answer the provider's first request with a 401 and its discovery requests as described; + return the request it builds once discovery is over. + + `authorization_server` is what protected-resource metadata advertises (None: no PRM is + served); `metadata` is the authorization server metadata document (None: every well-known + 404s). + """ + request = await flow.__anext__() + request = await flow.asend(httpx.Response(401, request=request)) + while "/.well-known/oauth-protected-resource" in str(request.url): + if authorization_server is None: + response = httpx.Response(404, request=request) + else: + advertised = authorization_server if isinstance(authorization_server, list) else [authorization_server] + prm = {"resource": _SERVER_URL, "authorization_servers": advertised} + response = httpx.Response(200, json=prm, request=request) + request = await flow.asend(response) + while "/.well-known/" in str(request.url): + if metadata is None: + response = httpx.Response(404, request=request) + else: + response = httpx.Response(200, json=metadata, request=request) + request = await flow.asend(response) + return request + + +@pytest.mark.anyio +@pytest.mark.parametrize( + "served_issuer", [_CONFIGURED_ISSUER, f"{_CONFIGURED_ISSUER}/"], ids=["as-configured", "root-slash"] +) +@pytest.mark.parametrize("kind", ["secret", "jwt"]) +async def test_provider_with_configured_issuer_exchanges_at_that_issuer( + mock_storage: MockTokenStorage, kind: str, served_issuer: str +): + """SDK-defined: with `issuer=` set and metadata discovered for that issuer (a root issuer served with + its trailing slash is the same server), the token request goes to its token endpoint (positive + control for the refusals below).""" + audiences: list[str] = [] + provider = _provider_with_issuer(kind, mock_storage, audiences) + flow = provider.async_auth_flow(httpx.Request("POST", _SERVER_URL)) + metadata = {**_metadata_for(_CONFIGURED_ISSUER), "issuer": served_issuer} + + token_request = await _answer_discovery(flow, authorization_server=served_issuer, metadata=metadata) + + assert (token_request.method, str(token_request.url)) == ("POST", "https://auth.example.com/token") + # The SDK's URL type renders a root issuer with its trailing slash, which is the audience used. + assert audiences == ([] if kind == "secret" else ["https://auth.example.com/"]) + await flow.aclose() + + +@pytest.mark.anyio +@pytest.mark.parametrize("kind", ["secret", "jwt"]) +async def test_provider_picks_its_configured_issuer_among_several_advertised_servers( + mock_storage: MockTokenStorage, kind: str +): + """SDK-defined: when the resource lists several authorization servers, the one matching `issuer=` is + discovered and used even if it is not listed first.""" + provider = _provider_with_issuer(kind, mock_storage, []) + flow = provider.async_auth_flow(httpx.Request("POST", _SERVER_URL)) + + token_request = await _answer_discovery( + flow, + authorization_server=["https://other-as.example.com", _CONFIGURED_ISSUER], + metadata=_metadata_for(_CONFIGURED_ISSUER), + ) + + assert provider.context.auth_server_url == f"{_CONFIGURED_ISSUER}/" + assert str(token_request.url) == "https://auth.example.com/token" + await flow.aclose() + + +@pytest.mark.parametrize("kind", ["secret", "jwt"]) +def test_constructing_without_issuer_is_deprecated(mock_storage: MockTokenStorage, kind: str) -> None: + """SDK-defined: leaving `issuer` out is allowed but deprecated, and the provider says so at + construction.""" + + async def assertion_provider(audience: str) -> str: + raise NotImplementedError + + with pytest.warns(DeprecationWarning) as recorded: + if kind == "secret": + ClientCredentialsOAuthProvider( + server_url=_SERVER_URL, storage=mock_storage, client_id="c", client_secret="s" + ) + else: + PrivateKeyJWTOAuthProvider( + server_url=_SERVER_URL, storage=mock_storage, client_id="c", assertion_provider=assertion_provider + ) + + [warning] = recorded + assert warning.filename == __file__ + assert str(warning.message) == ( + "Omitting `issuer` is deprecated and it will be required in 3.0. Without it, the MCP server " + "decides which authorization server receives this client's credentials; pass " + "issuer= so they are only ever sent there." + ) + + +@pytest.mark.anyio +@pytest.mark.parametrize("kind", ["secret", "jwt"]) +async def test_without_issuer_the_exchange_follows_whichever_server_was_discovered( + mock_storage: MockTokenStorage, kind: str +) -> None: + """SDK-defined: with no `issuer` configured the token request is built from whatever metadata + discovery produced, as before.""" + + async def assertion_provider(audience: str) -> str: + return "jwt" + + with pytest.warns(DeprecationWarning, match="Omitting `issuer` is deprecated"): + if kind == "secret": + provider: OAuthClientProvider = ClientCredentialsOAuthProvider( + server_url=_SERVER_URL, storage=mock_storage, client_id="c", client_secret="s" + ) + else: + provider = PrivateKeyJWTOAuthProvider( + server_url=_SERVER_URL, storage=mock_storage, client_id="c", assertion_provider=assertion_provider + ) + flow = provider.async_auth_flow(httpx.Request("POST", _SERVER_URL)) + + token_request = await _answer_discovery( + flow, + authorization_server="https://elsewhere.example.com", + metadata=_metadata_for("https://elsewhere.example.com"), + ) + + assert (token_request.method, str(token_request.url)) == ("POST", "https://elsewhere.example.com/token") + await flow.aclose() + + +def test_an_issuer_that_is_not_an_http_url_is_rejected_at_construction(mock_storage: MockTokenStorage) -> None: + """SDK-defined: `issuer=` is the authorization server's issuer URL; anything else is a configuration + error on both machine-to-machine providers.""" + with pytest.raises(ValueError) as cc_error: + ClientCredentialsOAuthProvider( + server_url=_SERVER_URL, storage=mock_storage, client_id="cid", client_secret="s", issuer="auth.example.com" + ) + with pytest.raises(ValueError) as jwt_error: + PrivateKeyJWTOAuthProvider( + server_url=_SERVER_URL, + storage=mock_storage, + client_id="cid", + assertion_provider=static_assertion_provider("jwt"), + issuer="auth.example.com", + ) + assert ( + str(cc_error.value) + == str(jwt_error.value) + == snapshot("issuer must be the authorization server's http(s) issuer URL, got 'auth.example.com'") + ) + + +@pytest.mark.anyio +@pytest.mark.parametrize("kind", ["secret", "jwt"]) +async def test_provider_refuses_metadata_for_a_different_issuer(mock_storage: MockTokenStorage, kind: str): + """SDK-defined: when discovery ends at an authorization server other than the configured `issuer`, + no token request is built and no assertion is minted.""" + audiences: list[str] = [] + provider = _provider_with_issuer(kind, mock_storage, audiences) + flow = provider.async_auth_flow(httpx.Request("POST", _SERVER_URL)) + + with pytest.raises(OAuthFlowError) as exc_info: + await _answer_discovery( + flow, + authorization_server="https://other-as.example.com", + metadata=_metadata_for("https://other-as.example.com"), + ) + + assert str(exc_info.value) == snapshot( + "Authorization server metadata issuer mismatch: https://other-as.example.com/ != https://auth.example.com" + ) + assert audiences == [] + + +@pytest.mark.anyio +@pytest.mark.parametrize("kind", ["secret", "jwt"]) +async def test_provider_refuses_to_exchange_without_metadata_when_issuer_configured( + mock_storage: MockTokenStorage, kind: str +): + """SDK-defined: with `issuer=` set, the 2025-03-26 default `/token` on the resource origin is not + used when no authorization server metadata could be discovered.""" + audiences: list[str] = [] + provider = _provider_with_issuer(kind, mock_storage, audiences) + flow = provider.async_auth_flow(httpx.Request("POST", _SERVER_URL)) + + with pytest.raises(OAuthFlowError) as exc_info: + await _answer_discovery(flow, authorization_server=None, metadata=None) + + assert str(exc_info.value) == snapshot( + "No authorization server metadata discovered for configured issuer https://auth.example.com" + ) + assert audiences == [] + + +@pytest.mark.anyio +@pytest.mark.parametrize("kind", ["secret", "jwt"]) +async def test_a_refused_authorization_server_is_forgotten_so_the_next_request_rediscovers( + mock_storage: MockTokenStorage, kind: str +): + """SDK-defined: when the exchange is refused because discovery ended somewhere other than the + configured issuer, the refused metadata and any token held are dropped; the next request goes out + unauthenticated and discovery starts again, rather than a refresh being built from what was refused.""" + provider = _provider_with_issuer(kind, mock_storage, []) + flow = provider.async_auth_flow(httpx.Request("POST", _SERVER_URL)) + token_request = await _answer_discovery( + flow, authorization_server=_CONFIGURED_ISSUER, metadata=_metadata_for(_CONFIGURED_ISSUER) + ) + token = {"access_token": "first", "token_type": "Bearer", "expires_in": 3600, "refresh_token": "rt"} + retried = await flow.asend(httpx.Response(200, json=token, request=token_request)) + with pytest.raises(StopAsyncIteration): + await flow.asend(httpx.Response(200, request=retried)) + + flow = provider.async_auth_flow(httpx.Request("POST", _SERVER_URL)) + with pytest.raises(OAuthFlowError): + await _answer_discovery( + flow, + authorization_server="https://other-as.example.com", + metadata=_metadata_for("https://other-as.example.com"), + ) + assert provider.context.oauth_metadata is None + assert provider.context.current_tokens is None + + flow = provider.async_auth_flow(httpx.Request("POST", _SERVER_URL)) + request = await flow.__anext__() + assert (str(request.url), request.headers.get("Authorization")) == (_SERVER_URL, None) + await flow.aclose() diff --git a/tests/client/test_auth.py b/tests/client/test_auth.py index 5f8bc14107..7c3e5bd60c 100644 --- a/tests/client/test_auth.py +++ b/tests/client/test_auth.py @@ -3,9 +3,11 @@ """ import base64 +import json import time +from collections.abc import AsyncGenerator from unittest import mock -from urllib.parse import unquote +from urllib.parse import parse_qs, unquote, urlparse import httpx import pytest @@ -13,20 +15,23 @@ from pydantic import AnyHttpUrl, AnyUrl from mcp.client.auth import OAuthClientProvider, PKCEParameters -from mcp.client.auth.exceptions import OAuthFlowError +from mcp.client.auth.exceptions import OAuthFlowError, OAuthRegistrationError from mcp.client.auth.utils import ( build_oauth_authorization_server_metadata_discovery_urls, build_protected_resource_metadata_discovery_urls, create_client_info_from_metadata_url, create_client_registration_request, create_oauth_metadata_request, + credentials_match_issuer, extract_field_from_www_auth, extract_resource_metadata_from_www_auth, extract_scope_from_www_auth, get_client_metadata_scopes, + handle_auth_metadata_response, handle_registration_response, is_valid_client_metadata_url, should_use_client_metadata_url, + validate_metadata_issuer, ) from mcp.shared.auth import ( OAuthClientInformationFull, @@ -821,40 +826,129 @@ async def test_resource_param_included_with_protected_resource_metadata(self, oa assert "resource=" in content +async def _start_discovery( + provider: OAuthClientProvider, +) -> tuple[AsyncGenerator[httpx.Request, httpx.Response], httpx.Request]: + """Drive `provider`'s auth flow to the point where it has sent the MCP request, seen a 401 and + issued its first protected-resource-metadata request; returns (flow, that request).""" + provider.context.current_tokens = None + provider.context.token_expiry_time = None + provider._initialized = True + mcp_request = httpx.Request("POST", "https://api.example.com/v1/mcp") + flow = provider.async_auth_flow(mcp_request) + sent = await flow.__anext__() + assert sent is mcp_request + # No resource_metadata hint, so discovery tries the path-based well-known URL, then the root one. + unauthorized = httpx.Response(401, request=mcp_request) + prm_request = await flow.asend(unauthorized) + assert (prm_request.method, str(prm_request.url)) == ( + "GET", + "https://api.example.com/.well-known/oauth-protected-resource/v1/mcp", + ) + return flow, prm_request + + +async def _redirect(request: httpx.Request, status: int, location: str) -> httpx.Response: + """A redirect answer to `request`, as httpx hands it back when it does not follow it.""" + transport = httpx.MockTransport(lambda r: httpx.Response(status, headers={"location": location})) + async with httpx.AsyncClient(transport=transport) as client: + return await client.send(request) + + +@pytest.mark.anyio +async def test_auth_flow_follows_a_same_origin_redirect_of_its_own_request(oauth_provider: OAuthClientProvider): + """SDK-defined: a request the OAuth flow makes (here protected-resource metadata discovery) + follows a redirect that stays within its origin and keeps its method, like an MCP request.""" + flow, prm_request = await _start_discovery(oauth_provider) + + follow_up = await flow.asend(await _redirect(prm_request, 307, "/.well-known/oauth-protected-resource/v1/mcp/")) + + assert (follow_up.method, str(follow_up.url)) == ( + "GET", + "https://api.example.com/.well-known/oauth-protected-resource/v1/mcp/", + ) + await flow.aclose() + + +@pytest.mark.anyio +async def test_auth_flow_does_not_follow_a_redirect_of_its_own_request_to_another_origin( + oauth_provider: OAuthClientProvider, +): + """SDK-defined: a redirect of a flow request to another origin is handed to the flow unfollowed, + which treats it as "not served here" and moves to its next discovery URL.""" + flow, prm_request = await _start_discovery(oauth_provider) + + next_request = await flow.asend(await _redirect(prm_request, 307, "https://elsewhere.example/prm")) + + assert (next_request.method, str(next_request.url)) == ( + "GET", + "https://api.example.com/.well-known/oauth-protected-resource", + ) + await flow.aclose() + + +@pytest.mark.anyio +async def test_auth_flow_stops_following_a_redirecting_request_after_a_few_hops( + oauth_provider: OAuthClientProvider, +): + """SDK-defined: a flow request that keeps redirecting within its origin is followed a bounded + number of times; the redirect after that is handed to the flow unfollowed.""" + flow, request = await _start_discovery(oauth_provider) + + hops = 0 + while str(request.url) != "https://api.example.com/.well-known/oauth-protected-resource": + request = await flow.asend( + await _redirect(request, 307, f"/.well-known/oauth-protected-resource/v1/mcp/{hops}") + ) + hops += 1 + + assert hops == 6 # five followed, the sixth handed back and taken as "try the next URL" + await flow.aclose() + + +@pytest.mark.anyio +@pytest.mark.parametrize(("status", "keep_trying"), [(404, True), (307, True), (500, False)]) +async def test_auth_metadata_response_says_whether_to_try_the_next_discovery_url( + status: int, keep_trying: bool +) -> None: + """SDK-defined: a 4xx or a 3xx (redirects are not followed on these requests) from a discovery + candidate means the metadata is not served there and the next well-known URL is tried; a 5xx + stops discovery.""" + assert await handle_auth_metadata_response(httpx.Response(status)) == (keep_trying, None) + + class TestRegistrationResponse: """Test client registration response handling.""" @pytest.mark.anyio async def test_handle_registration_response_reads_before_accessing_text(self): - """Test that response.aread() is called before accessing response.text.""" - - # Track if aread() was called - class MockResponse(httpx.Response): - def __init__(self): - self.status_code = 400 - self._aread_called = False - self._text = "Registration failed with error" + """The registration error carries the response text, which for a streamed response means + reading it first (a streamed httpx response raises ResponseNotRead otherwise).""" + response = httpx.Response(400, stream=httpx.ByteStream(b"Registration failed with error")) - async def aread(self): - self._aread_called = True - return b"test content" + with pytest.raises(OAuthRegistrationError) as exc_info: + await handle_registration_response(response) - @property - def text(self): - if not self._aread_called: - raise RuntimeError("Response.text accessed before response.aread()") # pragma: no cover - return self._text + assert str(exc_info.value) == snapshot("Registration failed: 400 Registration failed with error") - mock_response = MockResponse() + @pytest.mark.anyio + async def test_registration_error_names_an_unfollowed_redirect(self): + """SDK-defined: when the registration endpoint answered with a redirect that was not followed, + the error says where it pointed (without userinfo or query) instead of only the bare status.""" + request = httpx.Request("POST", "https://as.example/register") + async with httpx.AsyncClient( + transport=httpx.MockTransport( + lambda r: httpx.Response(307, headers={"location": "https://u:p@elsewhere.example/register?state=x"}) + ) + ) as client: + response = await client.send(request) - # This should call aread() before accessing text - with pytest.raises(Exception) as exc_info: - await handle_registration_response(mock_response) + with pytest.raises(OAuthRegistrationError) as exc_info: + await handle_registration_response(response) - # Verify aread() was called - assert mock_response._aread_called - # Verify the error message includes the response text - assert "Registration failed: 400" in str(exc_info.value) + assert str(exc_info.value) == snapshot( + "Registration failed: 307 (redirected to https://elsewhere.example/register; not followed) " + ) class TestCreateClientRegistrationRequest: @@ -1144,8 +1238,11 @@ async def mock_callback() -> tuple[str, str | None]: request=request, ) - # Trigger step-up - should get token exchange request - token_exchange_request = await auth_flow.asend(response_403) + # Trigger step-up - discovery runs first (nothing published here), then the token exchange + prm_request = await auth_flow.asend(response_403) + prm_request = await auth_flow.asend(httpx.Response(404, request=prm_request)) + asm_request = await auth_flow.asend(httpx.Response(404, request=prm_request)) + token_exchange_request = await auth_flow.asend(httpx.Response(404, request=asm_request)) # Verify scope was updated assert oauth_provider.context.client_metadata.scope == "admin:write admin:delete" @@ -1403,8 +1500,8 @@ async def callback_handler() -> tuple[str, str | None]: prm_request_1 = await auth_flow.asend(response) assert str(prm_request_1.url) == "https://custom.prm.com/.well-known/oauth-protected-resource" - # Returns 500 - prm_response_1 = httpx.Response(500, request=prm_request_1) + # Not served there + prm_response_1 = httpx.Response(404, request=prm_request_1) # Try path-based fallback prm_request_2 = await auth_flow.asend(prm_response_1) @@ -2113,3 +2210,757 @@ async def test_get_resource_url_falls_back_when_prm_mismatches( # get_resource_url should return the canonical server URL, not the PRM resource assert provider.context.get_resource_url() == "https://api.example.com/v1/mcp" + + +def _prepare_full_flow(provider: OAuthClientProvider, client_info: OAuthClientInformationFull | None) -> list[str]: + """Reset `provider` for a full flow with `client_info` as the stored registration, and wire a + redirect/callback pair that echoes the `state` of the last authorization URL it was sent to. + Returns the list the redirect handler appends authorization URLs to.""" + provider.context.current_tokens = None + provider.context.token_expiry_time = None + provider._initialized = True + provider.context.client_info = client_info + redirects: list[str] = [] + + async def record_redirect(url: str) -> None: + redirects.append(url) + + async def echo_callback() -> tuple[str, str | None]: + return "auth_code", parse_qs(urlparse(redirects[-1]).query)["state"][0] + + provider.context.redirect_handler = record_redirect + provider.context.callback_handler = echo_callback + return redirects + + +def _asm(issuer: str, *, token_origin: str | None = None, registration: bool = False) -> bytes: + metadata = { + "issuer": issuer, + "authorization_endpoint": f"{issuer}/authorize", + "token_endpoint": f"{token_origin or issuer}/token", + } + if registration: + metadata["registration_endpoint"] = f"{issuer}/register" + return json.dumps(metadata).encode() + + +@pytest.mark.anyio +async def test_metadata_issuer_must_match_the_advertised_authorization_server(oauth_provider: OAuthClientProvider): + """RFC 8414 section 3.3: metadata fetched for the PRM-advertised authorization server must + name that server as its issuer; metadata naming another issuer is refused before + registration, authorization or token requests are built from it.""" + _prepare_full_flow(oauth_provider, None) + auth_flow = oauth_provider.async_auth_flow(httpx.Request("GET", "https://api.example.com/v1/mcp")) + request = await auth_flow.__anext__() + + prm_req = await auth_flow.asend(httpx.Response(401, request=request)) + prm_response = httpx.Response( + 200, + content=b'{"resource": "https://api.example.com/v1/mcp", "authorization_servers": ["https://auth.example.com"]}', + request=prm_req, + ) + asm_req = await auth_flow.asend(prm_response) + assert str(asm_req.url) == "https://auth.example.com/.well-known/oauth-authorization-server" + + asm_response = httpx.Response(200, content=_asm("https://other-as.example.com", registration=True), request=asm_req) + with pytest.raises(OAuthFlowError) as exc_info: + await auth_flow.asend(asm_response) + + assert str(exc_info.value) == snapshot( + "Authorization server metadata issuer mismatch: https://other-as.example.com/ != https://auth.example.com/" + ) + + +@pytest.mark.anyio +async def test_legacy_fallback_metadata_naming_a_different_issuer_is_refused(oauth_provider: OAuthClientProvider): + """RFC 8414 section 3.3 on the legacy no-PRM path: metadata served from the resource server's + own well-known must name that origin as its issuer. + + Metadata naming a different authorization server is refused before any authorization or + token request is built, so a stored confidential client is never presented to the endpoints + that metadata lists. + """ + _prepare_full_flow( + oauth_provider, + OAuthClientInformationFull( + client_id="confidential-client", + client_secret="stored-secret", + token_endpoint_auth_method="client_secret_post", + redirect_uris=[AnyUrl("http://localhost:3030/callback")], + ), + ) + auth_flow = oauth_provider.async_auth_flow(httpx.Request("GET", "https://api.example.com/v1/mcp")) + request = await auth_flow.__anext__() + + # 401 without WWW-Authenticate; both PRM well-knowns 404; legacy root ASM discovery. + prm_req = await auth_flow.asend(httpx.Response(401, request=request)) + prm_req = await auth_flow.asend(httpx.Response(404, request=prm_req)) + asm_req = await auth_flow.asend(httpx.Response(404, request=prm_req)) + assert str(asm_req.url) == "https://api.example.com/.well-known/oauth-authorization-server" + + # The resource origin's well-known names another server as issuer while listing its own + # token endpoint. + asm_response = httpx.Response( + 200, content=_asm("https://other-as.example.com", token_origin="https://api.example.com"), request=asm_req + ) + with pytest.raises(OAuthFlowError) as exc_info: + await auth_flow.asend(asm_response) + + assert str(exc_info.value) == snapshot( + "Authorization server metadata issuer mismatch: https://other-as.example.com/ != https://api.example.com/" + ) + + +_ISSUER = "https://as.example.com/tenant" + + +def _issuer_metadata(issuer: str = _ISSUER) -> OAuthMetadata: + return OAuthMetadata.model_validate( + {"issuer": issuer, "authorization_endpoint": f"{issuer}/authorize", "token_endpoint": f"{issuer}/token"} + ) + + +def test_validate_metadata_issuer_accepts_match(): + validate_metadata_issuer(_issuer_metadata(_ISSUER), _ISSUER) + + +def test_validate_metadata_issuer_rejects_mismatch(): + with pytest.raises(OAuthFlowError, match="issuer mismatch"): + validate_metadata_issuer(_issuer_metadata("https://other-as.example.com/tenant"), _ISSUER) + + +@pytest.mark.parametrize( + ("issuer", "expected"), + [ + pytest.param("https://as.example.com/", "https://as.example.com", id="metadata-has-root-slash"), + pytest.param("https://as.example.com/", "https://as.example.com/", id="both-have-root-slash"), + ], +) +def test_validate_metadata_issuer_treats_empty_path_and_root_slash_as_the_same_issuer(issuer: str, expected: str): + """SDK-defined tolerance: an origin with an empty path and the same origin with a lone `/` + identify the same server (RFC 3986 section 6.2.3). A root issuer always parses to the `/` + form here, while the legacy discovery URL is built from the bare origin, so the two must + compare equal.""" + validate_metadata_issuer(_issuer_metadata(issuer), expected) + + +@pytest.mark.parametrize( + ("issuer", "expected"), + [ + pytest.param("https://as.example.com/tenant/", "https://as.example.com/tenant", id="non-root-trailing-slash"), + pytest.param("https://as.example.com/tenant", "https://as.example.com", id="different-path"), + pytest.param("http://as.example.com/", "https://as.example.com", id="different-scheme"), + pytest.param("https://as.example.com:8443/", "https://as.example.com", id="different-port"), + pytest.param("https://as.example.com//", "https://as.example.com", id="double-slash"), + ], +) +def test_validate_metadata_issuer_root_slash_tolerance_does_not_extend_further(issuer: str, expected: str): + """The empty-path tolerance is exactly that: any other difference is still a mismatch.""" + with pytest.raises(OAuthFlowError, match="metadata issuer mismatch"): + validate_metadata_issuer(_issuer_metadata(issuer), expected) + + +def test_credentials_match_issuer_same_issuer(): + info = OAuthClientInformationFull(client_id="c", redirect_uris=[AnyUrl("http://localhost/cb")], issuer="https://as") + assert credentials_match_issuer(info, "https://as", None) is True + + +def test_credentials_match_issuer_root_slash_is_the_same_issuer(): + """A binding written as the bare origin matches the `/` form a root URL parses to, and back.""" + info = OAuthClientInformationFull(client_id="c", redirect_uris=[AnyUrl("http://localhost/cb")], issuer="https://as") + assert credentials_match_issuer(info, "https://as/", None) is True + info.issuer = "https://as/" + assert credentials_match_issuer(info, "https://as", None) is True + + +def test_credentials_match_issuer_root_slash_tolerance_does_not_extend_to_other_paths(): + """SDK-defined: a trailing slash on a non-root path is a different issuer.""" + info = OAuthClientInformationFull( + client_id="c", redirect_uris=[AnyUrl("http://localhost/cb")], issuer="https://as.example.com/tenant" + ) + assert credentials_match_issuer(info, "https://as.example.com/tenant/", None) is False + + +def test_credentials_match_issuer_different_issuer(): + info = OAuthClientInformationFull(client_id="c", redirect_uris=[AnyUrl("http://localhost/cb")], issuer="https://as") + assert credentials_match_issuer(info, "https://other", None) is False + + +def test_credentials_match_issuer_no_recorded_issuer_is_left_alone(): + """Credentials with no bound issuer (pre-registered / legacy) carry no binding to enforce.""" + info = OAuthClientInformationFull(client_id="c", redirect_uris=[AnyUrl("http://localhost/cb")]) + assert credentials_match_issuer(info, "https://as", None) is True + + +def test_credentials_match_issuer_cimd_is_portable(): + """A client_id equal to the configured client_metadata_url (CIMD) is portable across servers.""" + cimd_url = "https://client.example/metadata.json" + info = OAuthClientInformationFull( + client_id=cimd_url, + redirect_uris=[AnyUrl("http://localhost/cb")], + token_endpoint_auth_method="none", + issuer="https://as", + ) + assert credentials_match_issuer(info, "https://other", cimd_url) is True + + +def test_credentials_match_issuer_url_shaped_dcr_id_is_not_portable(): + """A URL-shaped client_id from DCR (not the configured CIMD URL) stays bound to its issuer.""" + info = OAuthClientInformationFull( + client_id="https://as.example.com/clients/123", + redirect_uris=[AnyUrl("http://localhost/cb")], + issuer="https://as.example.com", + ) + assert credentials_match_issuer(info, "https://other", "https://client.example/metadata.json") is False + + +@pytest.mark.anyio +@pytest.mark.parametrize("echoed_issuer", ["https://not-the-flow.example", 12345], ids=["string", "not-a-string"]) +async def test_registration_response_does_not_seed_the_issuer_binding_from_the_body(echoed_issuer: object): + """The issuer binding (SEP-2352) is the SDK's record of which server it registered with, + stamped by the auth flow; an "issuer" member in the untrusted response body is dropped + before parsing - never populating the binding, and never failing the parse either, so a + mismatched or malformed value cannot discard the credentials on every 401.""" + body = json.dumps( + {"client_id": "issued-id", "redirect_uris": ["http://localhost:3030/callback"], "issuer": echoed_issuer} + ).encode() + + client_info = await handle_registration_response(httpx.Response(201, content=body)) + + assert client_info.client_id == "issued-id" + assert client_info.issuer is None + + +@pytest.mark.anyio +@pytest.mark.parametrize( + "content", + [b"not json", b'["json", "but", "not", "an", "object"]', '{"client_id": "caf\xe9"}'.encode("latin-1")], + ids=["not-json", "not-an-object", "not-utf8"], +) +async def test_a_2xx_body_that_is_not_client_information_is_an_oauth_registration_error(content: bytes): + """A success status whose body is not client information - unparseable, not an object, or + not valid UTF-8 - surfaces as OAuthRegistrationError rather than a raw parse failure, so a + single OAuthFlowError handler still covers registration.""" + with pytest.raises(OAuthRegistrationError): + await handle_registration_response(httpx.Response(201, content=content)) + + +@pytest.mark.anyio +async def test_stored_credentials_are_not_presented_to_a_different_authorization_server( + oauth_provider: OAuthClientProvider, mock_storage: MockTokenStorage +): + """SEP-2352: stored credentials are bound to the authorization server that registered them. + + Steps: + 1. Storage holds a confidential client bound to `https://auth.example.com/`. + 2. PRM now advertises `https://other-as.example.com` -> the stored client and its tokens are + discarded before that server's metadata is fetched. + 3. Metadata for the new server is discovered -> the flow registers there and the token + request carries the new client, not the discarded `client_id`/`client_secret`. + 4. The new registration is recorded as bound to the new server. + """ + redirects = _prepare_full_flow( + oauth_provider, + OAuthClientInformationFull( + client_id="confidential-client", + client_secret="stored-secret", + token_endpoint_auth_method="client_secret_post", + redirect_uris=[AnyUrl("http://localhost:3030/callback")], + issuer="https://auth.example.com/", + ), + ) + auth_flow = oauth_provider.async_auth_flow(httpx.Request("GET", "https://api.example.com/v1/mcp")) + request = await auth_flow.__anext__() + + prm_req = await auth_flow.asend(httpx.Response(401, request=request)) + prm_response = httpx.Response( + 200, + content=( + b'{"resource": "https://api.example.com/v1/mcp", "authorization_servers": ["https://other-as.example.com"]}' + ), + request=prm_req, + ) + asm_req = await auth_flow.asend(prm_response) + assert str(asm_req.url) == "https://other-as.example.com/.well-known/oauth-authorization-server" + assert oauth_provider.context.client_info is None + + register_req = await auth_flow.asend( + httpx.Response(200, content=_asm("https://other-as.example.com", registration=True), request=asm_req) + ) + assert register_req.method == "POST" + assert str(register_req.url) == "https://other-as.example.com/register" + register_response = httpx.Response( + 201, + json={"client_id": "new-client", "redirect_uris": ["http://localhost:3030/callback"]}, + request=register_req, + ) + + token_req = await auth_flow.asend(register_response) + assert str(token_req.url) == "https://other-as.example.com/token" + assert redirects[-1].startswith("https://other-as.example.com/authorize?") + token_form = parse_qs(token_req.content.decode()) + assert token_form["client_id"] == ["new-client"] + assert "client_secret" not in token_form + + stored = await mock_storage.get_client_info() + assert stored is not None + assert stored.client_id == "new-client" + assert stored.issuer == "https://other-as.example.com/" + await auth_flow.aclose() + + +@pytest.mark.anyio +async def test_stored_credentials_bound_to_the_advertised_authorization_server_are_kept( + oauth_provider: OAuthClientProvider, +): + """SEP-2352 positive control: a stored client bound to the server PRM advertises (written + with or without the root slash) is reused - no registration request is made.""" + _prepare_full_flow( + oauth_provider, + OAuthClientInformationFull( + client_id="bound-client", + redirect_uris=[AnyUrl("http://localhost:3030/callback")], + issuer="https://auth.example.com", + ), + ) + auth_flow = oauth_provider.async_auth_flow(httpx.Request("GET", "https://api.example.com/v1/mcp")) + request = await auth_flow.__anext__() + + prm_req = await auth_flow.asend(httpx.Response(401, request=request)) + prm_response = httpx.Response( + 200, + content=b'{"resource": "https://api.example.com/v1/mcp", "authorization_servers": ["https://auth.example.com"]}', + request=prm_req, + ) + asm_req = await auth_flow.asend(prm_response) + assert oauth_provider.context.client_info is not None + + token_req = await auth_flow.asend( + httpx.Response(200, content=_asm("https://auth.example.com", registration=True), request=asm_req) + ) + assert str(token_req.url) == "https://auth.example.com/token" + assert parse_qs(token_req.content.decode())["client_id"] == ["bound-client"] + await auth_flow.aclose() + + +@pytest.mark.anyio +async def test_issuer_binding_evaluated_against_the_server_origin_when_prm_discovery_failed( + oauth_provider: OAuthClientProvider, +): + """SEP-2352: on the legacy no-PRM path the binding check uses the resource server's origin. + + PRM discovery fails (404) so `auth_server_url` stays `None`; the legacy well-known URL is + built from the resource server's origin, which is therefore the issuer any metadata found + there must carry (RFC 8414 section 3.3). Stored credentials bound to a different issuer are + discarded before that metadata is fetched, and the flow re-registers. + """ + _prepare_full_flow( + oauth_provider, + OAuthClientInformationFull( + client_id="stale-client", + redirect_uris=[AnyUrl("http://localhost:3030/callback")], + issuer="https://old-as.example.com", + ), + ) + auth_flow = oauth_provider.async_auth_flow(httpx.Request("GET", "https://api.example.com/v1/mcp")) + request = await auth_flow.__anext__() + + # PRM discovery: path-based then root, both 404. + prm_req = await auth_flow.asend(httpx.Response(401, request=request)) + assert str(prm_req.url) == "https://api.example.com/.well-known/oauth-protected-resource/v1/mcp" + prm_req = await auth_flow.asend(httpx.Response(404, request=prm_req)) + assert str(prm_req.url) == "https://api.example.com/.well-known/oauth-protected-resource" + + # ASM discovery via root fallback (no auth_server_url): the stale credentials are already + # gone when the request is issued. + asm_req = await auth_flow.asend(httpx.Response(404, request=prm_req)) + assert str(asm_req.url) == "https://api.example.com/.well-known/oauth-authorization-server" + assert oauth_provider.context.client_info is None + + # The stale bound credentials are discarded, so the next yield is a DCR request rather than + # the authorize redirect. + next_req = await auth_flow.asend( + httpx.Response(200, content=_asm("https://api.example.com", registration=True), request=asm_req) + ) + assert oauth_provider.context.auth_server_url is None + assert next_req.method == "POST" + assert str(next_req.url) == "https://api.example.com/register" + await auth_flow.aclose() + + +@pytest.mark.anyio +@pytest.mark.parametrize( + "first_response", + [(401, {}), (403, {"WWW-Authenticate": 'Bearer error="insufficient_scope", scope="admin"'})], + ids=["401", "403-insufficient-scope"], +) +async def test_legacy_fallback_without_metadata_re_registers_instead_of_presenting_credentials_bound_elsewhere( + oauth_provider: OAuthClientProvider, valid_tokens: OAuthToken, first_response: tuple[int, dict[str, str]] +): + """SEP-2352 on the legacy no-PRM path when no metadata is served at all, whether the flow starts + from a 401 or from a 403 scope challenge with no metadata held. + + Steps: + 1. Storage holds a token and a confidential client bound to a different authorization server. + 2. Both PRM well-knowns 404 -> the expected issuer is the resource server's origin, so the + stored client is discarded before ASM discovery. + 3. The origin's ASM well-known 404s too -> the flow registers a fresh client at the + origin's default `/register` and authorizes with it; the token request to the origin's + default `/token` carries the new client and none of the discarded credentials. + """ + redirects = _prepare_full_flow( + oauth_provider, + OAuthClientInformationFull( + client_id="confidential-client", + client_secret="stored-secret", + token_endpoint_auth_method="client_secret_post", + redirect_uris=[AnyUrl("http://localhost:3030/callback")], + issuer="https://other-as.example.com", + ), + ) + oauth_provider.context.current_tokens = valid_tokens + oauth_provider.context.token_expiry_time = time.time() + 1800 + auth_flow = oauth_provider.async_auth_flow(httpx.Request("GET", "https://api.example.com/v1/mcp")) + request = await auth_flow.__anext__() + + status, headers = first_response + prm_req = await auth_flow.asend(httpx.Response(status, headers=headers, request=request)) + prm_req = await auth_flow.asend(httpx.Response(404, request=prm_req)) + asm_req = await auth_flow.asend(httpx.Response(404, request=prm_req)) + assert str(asm_req.url) == "https://api.example.com/.well-known/oauth-authorization-server" + assert oauth_provider.context.client_info is None + + # No metadata at the origin either: register at the origin's default endpoint. + register_req = await auth_flow.asend(httpx.Response(404, request=asm_req)) + assert register_req.method == "POST" + assert str(register_req.url) == "https://api.example.com/register" + register_response = httpx.Response( + 201, + json={ + "client_id": "origin-client", + "redirect_uris": ["http://localhost:3030/callback"], + "token_endpoint_auth_method": "none", + }, + request=register_req, + ) + + token_req = await auth_flow.asend(register_response) + assert str(token_req.url) == "https://api.example.com/token" + assert redirects[-1].startswith("https://api.example.com/authorize?") + token_form = parse_qs(token_req.content.decode()) + assert token_form["client_id"] == ["origin-client"] + assert "client_secret" not in token_form + await auth_flow.aclose() + + +@pytest.mark.anyio +async def test_scope_step_up_discovers_the_authorization_server_before_reauthorizing( + oauth_provider: OAuthClientProvider, valid_tokens: OAuthToken +): + """SDK-defined: a 403 scope challenge runs discovery first when no metadata is held, so + re-authorization targets the advertised server. + + Steps: + 1. A restarted client holds a token and a registration but no authorization server metadata. + 2. The first response is 403 insufficient_scope -> the next requests are PRM (at the challenge's + `resource_metadata` URL) then ASM discovery. + 3. The authorization redirect and the token request use the discovered server's endpoints and + ask for the challenged scope. + """ + redirects = _prepare_full_flow( + oauth_provider, + OAuthClientInformationFull( + client_id="registered-client", redirect_uris=[AnyUrl("http://localhost:3030/callback")] + ), + ) + oauth_provider.context.current_tokens = valid_tokens + oauth_provider.context.token_expiry_time = time.time() + 1800 + auth_flow = oauth_provider.async_auth_flow(httpx.Request("GET", "https://api.example.com/v1/mcp")) + request = await auth_flow.__anext__() + response_403 = httpx.Response( + 403, + headers={ + "WWW-Authenticate": 'Bearer error="insufficient_scope", scope="admin",' + ' resource_metadata="https://api.example.com/v1/mcp/resource-metadata"' + }, + request=request, + ) + + prm_request = await auth_flow.asend(response_403) + assert (prm_request.method, str(prm_request.url)) == ("GET", "https://api.example.com/v1/mcp/resource-metadata") + prm = b'{"resource": "https://api.example.com/v1/mcp", "authorization_servers": ["https://auth.example.com"]}' + asm_request = await auth_flow.asend(httpx.Response(200, content=prm, request=prm_request)) + assert str(asm_request.url) == "https://auth.example.com/.well-known/oauth-authorization-server" + + token_request = await auth_flow.asend( + httpx.Response(200, content=_asm("https://auth.example.com"), request=asm_request) + ) + + assert redirects[-1].startswith("https://auth.example.com/authorize?") + assert parse_qs(urlparse(redirects[-1]).query)["scope"] == ["admin"] + assert (token_request.method, str(token_request.url)) == ("POST", "https://auth.example.com/token") + await auth_flow.aclose() + + +@pytest.mark.anyio +async def test_scope_step_up_reuses_metadata_discovered_earlier_in_the_process( + oauth_provider: OAuthClientProvider, valid_tokens: OAuthToken +): + """SDK-defined: once metadata has been discovered in this process, a step-up re-authorizes with it + directly (no discovery requests) and asks for the challenged scope.""" + redirects = _prepare_full_flow( + oauth_provider, + OAuthClientInformationFull( + client_id="registered-client", redirect_uris=[AnyUrl("http://localhost:3030/callback")] + ), + ) + oauth_provider.context.current_tokens = valid_tokens + oauth_provider.context.token_expiry_time = time.time() + 1800 + oauth_provider.context.auth_server_url = "https://auth.example.com/" + oauth_provider.context.oauth_metadata = OAuthMetadata.model_validate_json(_asm("https://auth.example.com")) + auth_flow = oauth_provider.async_auth_flow(httpx.Request("GET", "https://api.example.com/v1/mcp")) + request = await auth_flow.__anext__() + response_403 = httpx.Response( + 403, headers={"WWW-Authenticate": 'Bearer error="insufficient_scope", scope="admin"'}, request=request + ) + + token_request = await auth_flow.asend(response_403) + + assert (token_request.method, str(token_request.url)) == ("POST", "https://auth.example.com/token") + assert redirects[-1].startswith("https://auth.example.com/authorize?") + assert parse_qs(urlparse(redirects[-1]).query)["scope"] == ["admin"] + await auth_flow.aclose() + + +@pytest.mark.anyio +async def test_403_without_a_scope_challenge_is_returned_to_the_caller( + oauth_provider: OAuthClientProvider, valid_tokens: OAuthToken +): + """SDK-defined: a 403 that is not an insufficient_scope challenge ends the flow; the request is + not retried.""" + _prepare_full_flow(oauth_provider, None) + oauth_provider.context.current_tokens = valid_tokens + oauth_provider.context.token_expiry_time = time.time() + 1800 + auth_flow = oauth_provider.async_auth_flow(httpx.Request("GET", "https://api.example.com/v1/mcp")) + request = await auth_flow.__anext__() + + with pytest.raises(StopAsyncIteration): + await auth_flow.asend( + httpx.Response(403, headers={"WWW-Authenticate": 'Bearer error="access_denied"'}, request=request) + ) + + +@pytest.mark.anyio +@pytest.mark.parametrize("status", [500, 503, 429]) +async def test_a_failing_resource_metadata_request_stops_the_flow_and_keeps_stored_credentials( + oauth_provider: OAuthClientProvider, valid_tokens: OAuthToken, status: int +): + """SDK-defined: a server error (or 429) on a protected resource metadata request says nothing about + whether the server publishes that metadata. The remaining well-known locations are still tried, but + when none answers the flow stops instead of taking the legacy path, and a registration bound to the + advertised authorization server and its tokens stay as they were.""" + bound = OAuthClientInformationFull( + client_id="registered-client", + redirect_uris=[AnyUrl("http://localhost:3030/callback")], + issuer="https://auth.example.com/", + ) + _prepare_full_flow(oauth_provider, bound) + oauth_provider.context.current_tokens = valid_tokens + oauth_provider.context.token_expiry_time = time.time() + 1800 + flow = oauth_provider.async_auth_flow(httpx.Request("GET", "https://api.example.com/v1/mcp")) + request = await flow.__anext__() + prm_request = await flow.asend(httpx.Response(401, request=request)) + root_prm_request = await flow.asend(httpx.Response(status, request=prm_request)) + assert str(root_prm_request.url) == "https://api.example.com/.well-known/oauth-protected-resource" + + with pytest.raises(OAuthFlowError) as exc_info: + await flow.asend(httpx.Response(404, request=root_prm_request)) + + assert str(exc_info.value) == f"Protected resource metadata request failed: HTTP {status}" + assert oauth_provider.context.client_info == bound + assert oauth_provider.context.current_tokens == valid_tokens + + +@pytest.mark.anyio +async def test_a_failing_resource_metadata_location_does_not_matter_when_another_one_answers( + oauth_provider: OAuthClientProvider, +): + """SDK-defined: the well-known locations are tried in order; an error at one of them is forgotten + once a later one returns the metadata.""" + _prepare_full_flow(oauth_provider, None) + flow = oauth_provider.async_auth_flow(httpx.Request("GET", "https://api.example.com/v1/mcp")) + request = await flow.__anext__() + prm_request = await flow.asend(httpx.Response(401, request=request)) + root_prm_request = await flow.asend(httpx.Response(503, request=prm_request)) + prm = b'{"resource": "https://api.example.com/v1/mcp", "authorization_servers": ["https://auth.example.com"]}' + + asm_request = await flow.asend(httpx.Response(200, content=prm, request=root_prm_request)) + + assert str(asm_request.url) == "https://auth.example.com/.well-known/oauth-authorization-server" + await flow.aclose() + + +@pytest.mark.anyio +@pytest.mark.parametrize( + "served_issuer", ["https://api.example.com", "https://api.example.com/"], ids=["bare", "root-slash"] +) +async def test_legacy_fallback_accepts_the_origin_issuer_for_a_server_url_in_any_spelling( + client_metadata: OAuthClientMetadata, mock_storage: MockTokenStorage, served_issuer: str +): + """SDK-defined: on the legacy no-PRM path the expected issuer is the resource server's origin; a + `server_url` written with an upper-case host and an explicit default port still matches metadata + naming that origin, with or without its trailing slash, and the flow proceeds to registration.""" + + async def redirect_handler(url: str) -> None: + raise NotImplementedError + + async def callback_handler() -> tuple[str, str | None]: + raise NotImplementedError + + provider = OAuthClientProvider( + server_url="https://API.Example.com:443/v1/mcp", + client_metadata=client_metadata, + storage=mock_storage, + redirect_handler=redirect_handler, + callback_handler=callback_handler, + ) + auth_flow = provider.async_auth_flow(httpx.Request("GET", "https://API.Example.com:443/v1/mcp")) + request = await auth_flow.__anext__() + prm_req = await auth_flow.asend(httpx.Response(401, request=request)) + prm_req = await auth_flow.asend(httpx.Response(404, request=prm_req)) + asm_req = await auth_flow.asend(httpx.Response(404, request=prm_req)) + asm = { + "issuer": served_issuer, + "authorization_endpoint": "https://api.example.com/authorize", + "token_endpoint": "https://api.example.com/token", + "registration_endpoint": "https://api.example.com/register", + } + + register_req = await auth_flow.asend(httpx.Response(200, json=asm, request=asm_req)) + + assert (register_req.method, str(register_req.url)) == ("POST", "https://api.example.com/register") + await auth_flow.aclose() + + +@pytest.mark.anyio +@pytest.mark.parametrize( + "asm_responses", + [ + pytest.param([httpx.Response(404), httpx.Response(404)], id="asm-discovery-failed"), + pytest.param( + [httpx.Response(200, content=_asm("https://new-as.example.com"))], + id="asm-metadata-without-registration-endpoint", + ), + ], +) +async def test_issuer_is_not_stamped_when_registration_falls_back_to_the_resource_origin( + oauth_provider: OAuthClientProvider, mock_storage: MockTokenStorage, asm_responses: list[httpx.Response] +): + """SEP-2352: a fallback registration is not recorded as bound to the PRM-advertised AS. + + PRM advertises a new authorization server, so the stored credentials (bound to the old + issuer) are discarded. DCR then falls back to the resource-server origin's `/register` + because the new AS's metadata either could not be discovered or omits + `registration_endpoint`. That registration was not derived from the new AS's metadata, + so persisting it as bound to the new AS would wedge the binding check on later flows; + instead the issuer is left unset. + """ + _prepare_full_flow( + oauth_provider, + OAuthClientInformationFull( + client_id="stale-client", + redirect_uris=[AnyUrl("http://localhost:3030/callback")], + issuer="https://api.example.com/", + ), + ) + auth_flow = oauth_provider.async_auth_flow(httpx.Request("GET", "https://api.example.com/v1/mcp")) + request = await auth_flow.__anext__() + response_401 = httpx.Response( + 401, + headers={ + "WWW-Authenticate": 'Bearer resource_metadata="https://api.example.com/.well-known/oauth-protected-resource"' + }, + request=request, + ) + + # PRM succeeds and advertises a new AS - the discard block fires. + prm_req = await auth_flow.asend(response_401) + assert str(prm_req.url) == "https://api.example.com/.well-known/oauth-protected-resource" + prm_response = httpx.Response( + 200, + content=( + b'{"resource": "https://api.example.com/v1/mcp", "authorization_servers": ["https://new-as.example.com"]}' + ), + request=prm_req, + ) + + # ASM discovery for the new AS yields no usable registration_endpoint - either every + # well-known URL 404s, or metadata is returned without one. + next_req = await auth_flow.asend(prm_response) + assert oauth_provider.context.client_info is None + assert oauth_provider.context.oauth_metadata is None + assert str(next_req.url) == "https://new-as.example.com/.well-known/oauth-authorization-server" + for asm_response in asm_responses: + asm_response.request = next_req + next_req = await auth_flow.asend(asm_response) + + # Step 4 falls back to the resource-server origin's /register. + dcr_req = next_req + assert dcr_req.method == "POST" + assert str(dcr_req.url) == "https://api.example.com/register" + dcr_response = httpx.Response( + 201, + json={"client_id": "fallback-client", "redirect_uris": ["http://localhost:3030/callback"]}, + request=dcr_req, + ) + await auth_flow.asend(dcr_response) + + # The persisted record carries no issuer binding - not the PRM-advertised AS we never reached. + stored = await mock_storage.get_client_info() + assert stored is not None + assert stored.client_id == "fallback-client" + assert stored.issuer is None + await auth_flow.aclose() + + +@pytest.mark.anyio +async def test_issuer_is_stamped_when_same_origin_fallback_register_is_on_the_discovered_issuer( + oauth_provider: OAuthClientProvider, mock_storage: MockTokenStorage +): + """SEP-2352: a fallback registration on the discovered issuer's own host is still bound. + + Legacy same-origin embedded AS: PRM is absent, root ASM discovery succeeds with `issuer` + equal to the resource origin and no `registration_endpoint`. DCR falls back to + `/register` - the issuer's own host - so the binding was established and + is recorded, preserving auto-recovery on a later AS migration. + """ + _prepare_full_flow(oauth_provider, None) + auth_flow = oauth_provider.async_auth_flow(httpx.Request("GET", "https://api.example.com/v1/mcp")) + request = await auth_flow.__anext__() + + # PRM discovery 404s on both well-known URLs. + prm_req = await auth_flow.asend(httpx.Response(401, request=request)) + assert str(prm_req.url) == "https://api.example.com/.well-known/oauth-protected-resource/v1/mcp" + prm_req = await auth_flow.asend(httpx.Response(404, request=prm_req)) + assert str(prm_req.url) == "https://api.example.com/.well-known/oauth-protected-resource" + + # Root ASM discovery succeeds with the resource origin as issuer and no registration_endpoint. + asm_req = await auth_flow.asend(httpx.Response(404, request=prm_req)) + assert str(asm_req.url) == "https://api.example.com/.well-known/oauth-authorization-server" + + # DCR falls back to the resource origin's /register - the issuer's own host. + dcr_req = await auth_flow.asend(httpx.Response(200, content=_asm("https://api.example.com"), request=asm_req)) + assert dcr_req.method == "POST" + assert str(dcr_req.url) == "https://api.example.com/register" + dcr_response = httpx.Response( + 201, + json={"client_id": "embedded-client", "redirect_uris": ["http://localhost:3030/callback"]}, + request=dcr_req, + ) + await auth_flow.asend(dcr_response) + + stored = await mock_storage.get_client_info() + assert stored is not None + assert stored.client_id == "embedded-client" + assert stored.issuer == "https://api.example.com/" + await auth_flow.aclose() diff --git a/tests/client/test_output_schema_validation.py b/tests/client/test_output_schema_validation.py index e4a06b7f82..fb158cef98 100644 --- a/tests/client/test_output_schema_validation.py +++ b/tests/client/test_output_schema_validation.py @@ -1,9 +1,11 @@ import logging from contextlib import contextmanager +from pathlib import Path from typing import Any from unittest.mock import patch import pytest +from referencing.exceptions import Unresolvable from mcp.server.lowlevel import Server from mcp.shared.memory import ( @@ -215,3 +217,34 @@ async def call_tool(name: str, arguments: dict[str, Any]) -> dict[str, Any]: # Check that warning was logged assert "Tool mystery_tool not listed" in caplog.text + + +# jsonschema's fallback retriever emits this DeprecationWarning; keep it a plain warning so the +# assertions below decide the outcome rather than the suite's warnings-as-errors filter. +@pytest.mark.filterwarnings("default:Automatically retrieving remote references:DeprecationWarning") +@pytest.mark.anyio +async def test_output_schema_ref_outside_the_document_is_rejected(tmp_path: Path): + """A `$ref` to a URI outside the output schema is not resolved, and a result whose validation + reaches one fails as an invalid schema (spec `$ref` resolution; applying it to `file:` URIs too + is SDK-defined).""" + target = tmp_path / "schema.json" + target.write_text("{}", encoding="utf-8") + server = Server("test-server") + + @server.list_tools() + async def list_tools(): + return [ + Tool(name="probe", description="", inputSchema={"type": "object"}, outputSchema={"$ref": target.as_uri()}) + ] + + @server.call_tool() + async def call_tool(name: str, arguments: dict[str, Any]): + return {"v": 1} + + with bypass_server_output_validation(): + async with client_session(server) as client: + with pytest.raises(RuntimeError) as exc_info: + await client.call_tool("probe", {}) + # SDK-authored prefix only; the tail is `referencing`'s text. + assert str(exc_info.value).startswith("Invalid schema for tool probe: ") + assert isinstance(exc_info.value.__cause__, Unresolvable) diff --git a/tests/conftest.py b/tests/conftest.py index af7e479932..aba9b44330 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,6 +1,27 @@ +from collections.abc import Iterator + import pytest +from sse_starlette.sse import AppStatus @pytest.fixture def anyio_backend(): return "asyncio" + + +@pytest.fixture(autouse=True) +def reset_sse_starlette_exit_event() -> Iterator[None]: + """sse-starlette<2 caches a module-level anyio.Event on AppStatus. Clear it + around each test so it is never bound to a closed event loop: any test that + serves an SSE response in process would otherwise inherit the event a + previous test created on another loop. Clearing it afterwards matters too, + because later test modules fork uvicorn subprocesses on Linux and would + otherwise inherit a stale event.""" + + def clear() -> None: + if hasattr(AppStatus, "should_exit_event"): # pragma: no cover + setattr(AppStatus, "should_exit_event", None) + + clear() + yield + clear() diff --git a/tests/server/auth/middleware/test_bearer_auth.py b/tests/server/auth/middleware/test_bearer_auth.py index e13ab96390..6c86b693cc 100644 --- a/tests/server/auth/middleware/test_bearer_auth.py +++ b/tests/server/auth/middleware/test_bearer_auth.py @@ -6,6 +6,7 @@ from typing import Any, cast import pytest +from pydantic import AnyHttpUrl from starlette.authentication import AuthCredentials from starlette.datastructures import Headers from starlette.requests import Request @@ -265,6 +266,56 @@ async def test_mixed_case_authorization_header( assert user.access_token == valid_access_token +class SingleTokenVerifier: + """A `TokenVerifier` that knows exactly one token.""" + + def __init__(self, access_token: AccessToken) -> None: + self.access_token = access_token + + async def verify_token(self, token: str) -> AccessToken | None: + return self.access_token if token == self.access_token.token else None + + +RS = "https://api.example.com/mcp" + + +@pytest.mark.anyio +@pytest.mark.parametrize( + ("resource_server_url", "token_resource", "accepted"), + [ + (None, "https://other.example.com/mcp", True), # nothing configured to compare against + (None, None, True), + (RS, None, False), # the verifier did not report what the token was issued for + (RS, RS, True), + (RS, RS + "/", True), + (RS, "https://API.EXAMPLE.COM:443/mcp", True), # same URL, different spelling + (RS, "https://api.example.com", False), + (RS, RS + "/child", False), + (RS, "https://api.example.com/other", False), + (RS, "https://other.example.com/mcp", False), + (RS, "api.example.com", False), # not a URL + ], +) +async def test_backend_accepts_only_tokens_issued_for_its_resource( + resource_server_url: str | None, token_resource: str | None, accepted: bool +): + """With `resource_server_url` set, only a token whose `resource` (RFC 8707) is that URL is + accepted and anything else is treated like an unrecognized token (spec-mandated audience + check); without it the verifier's answer stands (SDK-defined, the default wiring).""" + token = AccessToken(token="t", client_id="c", scopes=["read"], resource=token_resource) + backend = BearerAuthBackend( + SingleTokenVerifier(token), + resource_server_url=AnyHttpUrl(resource_server_url) if resource_server_url else None, + ) + + result = await backend.authenticate(Request({"type": "http", "headers": [(b"authorization", b"Bearer t")]})) + + if accepted: + assert result is not None and result[1].access_token == token + else: + assert result is None + + @pytest.mark.anyio class TestRequireAuthMiddleware: """Tests for the RequireAuthMiddleware class.""" diff --git a/tests/server/auth/test_settings.py b/tests/server/auth/test_settings.py new file mode 100644 index 0000000000..a5399dba3c --- /dev/null +++ b/tests/server/auth/test_settings.py @@ -0,0 +1,33 @@ +import warnings + +import pytest +from pydantic import AnyHttpUrl, ValidationError + +from mcp.server.auth.settings import AuthSettings + +ISSUER = AnyHttpUrl("https://auth.example.com") +RESOURCE = AnyHttpUrl("https://mcp.example.com/mcp") + + +def test_validate_token_resource_requires_a_resource_server_url(): + """SDK-defined: asking the bearer gate to compare tokens against `resource_server_url` without + configuring one is refused at construction time rather than silently comparing nothing.""" + AuthSettings(issuer_url=ISSUER, resource_server_url=RESOURCE, validate_token_resource=True) + with pytest.raises(ValidationError, match="validate_token_resource requires resource_server_url"): + AuthSettings(issuer_url=ISSUER, resource_server_url=None, validate_token_resource=True) + + +def test_leaving_validate_token_resource_unset_warns_when_a_resource_server_url_is_configured(): + """Unset behaves as False but says so: a resource server that has not chosen gets a + `DeprecationWarning` pointing at its own `AuthSettings(...)` call (3.0 flips the default).""" + with pytest.warns(DeprecationWarning, match="validate_token_resource") as record: + settings = AuthSettings(issuer_url=ISSUER, resource_server_url=RESOURCE) + assert settings.validate_token_resource is None + assert record[0].filename == __file__ + + +@pytest.mark.parametrize("kwargs", [{"validate_token_resource": False}, {"resource_server_url": None}]) +def test_an_explicit_choice_or_no_resource_server_url_does_not_warn(kwargs: dict[str, object]): + with warnings.catch_warnings(): + warnings.simplefilter("error") + AuthSettings.model_validate({"issuer_url": ISSUER, "resource_server_url": RESOURCE, **kwargs}) diff --git a/tests/server/fastmcp/test_server.py b/tests/server/fastmcp/test_server.py index 1a8b881dc1..9fcc5d0b0f 100644 --- a/tests/server/fastmcp/test_server.py +++ b/tests/server/fastmcp/test_server.py @@ -1544,3 +1544,20 @@ async def test_sse_app_applies_the_configured_request_body_limit() -> None: headers={"Content-Type": "application/json"}, ) assert response.status_code == 413 + + +def test_streamable_http_app_passes_the_configured_session_limits_to_its_manager() -> None: + """SDK-defined: FastMCP forwards `session_idle_timeout` and `max_sessions` to the Streamable HTTP manager; + by default sessions expire after 30 idle minutes and one process holds at most 10 000 of them.""" + default = FastMCP() + default.streamable_http_app() + assert (default.session_manager.session_idle_timeout, default.session_manager.max_sessions) == (30 * 60, 10_000) + + tuned = FastMCP(session_idle_timeout=5, max_sessions=7) + assert (tuned.settings.session_idle_timeout, tuned.settings.max_sessions) == (5, 7) + tuned.streamable_http_app() + assert (tuned.session_manager.session_idle_timeout, tuned.session_manager.max_sessions) == (5, 7) + + unbounded = FastMCP(session_idle_timeout=None, max_sessions=None) + unbounded.streamable_http_app() + assert (unbounded.session_manager.session_idle_timeout, unbounded.session_manager.max_sessions) == (None, None) diff --git a/tests/server/test_sse_security.py b/tests/server/test_sse_security.py index 68a96f1bd1..f262d6b473 100644 --- a/tests/server/test_sse_security.py +++ b/tests/server/test_sse_security.py @@ -4,7 +4,6 @@ import multiprocessing import re import socket -from collections.abc import Iterator from typing import Any import anyio @@ -29,23 +28,6 @@ SERVER_NAME = "test_sse_security_server" -@pytest.fixture(autouse=True) -def reset_sse_starlette_exit_event() -> Iterator[None]: - """sse-starlette<2 caches a module-level anyio.Event on AppStatus; clear it - around each test so it is never bound to a closed event loop. Clearing it - afterwards matters too: later test modules fork uvicorn subprocesses on - Linux and would otherwise inherit a stale event.""" - from sse_starlette.sse import AppStatus - - def clear() -> None: - if hasattr(AppStatus, "should_exit_event"): # pragma: no cover - setattr(AppStatus, "should_exit_event", None) - - clear() - yield - clear() - - @pytest.fixture def server_port() -> int: with socket.socket() as s: diff --git a/tests/server/test_streamable_http_manager.py b/tests/server/test_streamable_http_manager.py index e7f76419a7..9ecfd2764b 100644 --- a/tests/server/test_streamable_http_manager.py +++ b/tests/server/test_streamable_http_manager.py @@ -1,13 +1,17 @@ """Tests for StreamableHTTPSessionManager.""" import json -from collections.abc import Iterator -from typing import Any +import logging +import math +from collections.abc import AsyncIterator, Iterator +from contextlib import asynccontextmanager, contextmanager +from typing import Any, cast from unittest.mock import AsyncMock, patch import anyio +import anyio.lowlevel import pytest -from starlette.types import Message, Scope +from starlette.types import Message, Receive, Scope, Send from mcp.server import streamable_http_manager from mcp.server.auth.middleware.bearer_auth import AuthenticatedUser @@ -16,9 +20,27 @@ from mcp.server.streamable_http import MCP_SESSION_ID_HEADER, StreamableHTTPServerTransport from mcp.server.streamable_http_manager import ( DEFAULT_MAX_REQUEST_BODY_SIZE, + DEFAULT_MAX_SESSIONS, + DEFAULT_SESSION_IDLE_TIMEOUT, StreamableHTTPSessionManager, ) -from mcp.types import INVALID_REQUEST +from mcp.types import INTERNAL_ERROR, INVALID_REQUEST, LATEST_PROTOCOL_VERSION, TextContent + +_JSON_HEADERS = {"accept": "application/json, text/event-stream", "content-type": "application/json"} + +_INITIALIZE_BODY = json.dumps( + { + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { + "protocolVersion": LATEST_PROTOCOL_VERSION, + "capabilities": {}, + "clientInfo": {"name": "test", "version": "0"}, + }, + } +).encode() +"""A wire-level initialize request: the only request that may open a session.""" @pytest.mark.anyio @@ -285,19 +307,7 @@ async def test_stateless_requests_memory_cleanup(): app = Server("test-stateless-real-cleanup") manager = StreamableHTTPSessionManager(app=app, stateless=True) - # Track created transport instances - created_transports: list[StreamableHTTPServerTransport] = [] - - # Patch StreamableHTTPServerTransport constructor to track instances - - original_constructor = streamable_http_manager.StreamableHTTPServerTransport - - def track_transport(*args: Any, **kwargs: Any) -> StreamableHTTPServerTransport: - transport = original_constructor(*args, **kwargs) - created_transports.append(transport) - return transport - - with patch.object(streamable_http_manager, "StreamableHTTPServerTransport", side_effect=track_transport): + with _created_transports() as created_transports: async with manager.run(): # Mock app.run to complete immediately app.run = AsyncMock(return_value=None) @@ -390,81 +400,497 @@ async def mock_receive(): assert error_data["error"]["message"] == "Session not found" +class _IdleTimeoutObserver(logging.Handler): + """Resolves `reaped` when the manager logs that a session's idle timeout fired.""" + + def __init__(self) -> None: + super().__init__() + self.reaped = anyio.Event() + + def emit(self, record: logging.LogRecord) -> None: + if "idle timeout" in record.getMessage(): + self.reaped.set() + + +def _observe_idle_timeout(caplog: pytest.LogCaptureFixture, request: pytest.FixtureRequest) -> _IdleTimeoutObserver: + """Install an observer for the manager's "idle timeout" log record for the rest of the test. + + The manager pops the session synchronously after emitting that record, before its next await, + so a waiter woken by it always finds the session gone. caplog.set_level enables INFO so the + record is created. + """ + observer = _IdleTimeoutObserver() + manager_logger = logging.getLogger(streamable_http_manager.__name__) + manager_logger.addHandler(observer) + request.addfinalizer(lambda: manager_logger.removeHandler(observer)) + caplog.set_level(logging.INFO, logger=streamable_http_manager.__name__) + return observer + + +@contextmanager +def _created_transports() -> Iterator[list[StreamableHTTPServerTransport]]: + """Collect every transport a session manager creates while the context is open.""" + created: list[StreamableHTTPServerTransport] = [] + + def create(*args: Any, **kwargs: Any) -> StreamableHTTPServerTransport: + transport = StreamableHTTPServerTransport(*args, **kwargs) + created.append(transport) + return transport + + with patch.object(streamable_http_manager, "StreamableHTTPServerTransport", side_effect=create): + yield created + + +@asynccontextmanager +async def _open_event_stream( + manager: StreamableHTTPSessionManager, session_id: str +) -> AsyncIterator[StreamableHTTPServerTransport]: + """Hold the session's standalone GET stream open while the context is open, the way a listening client + does, and close it (the client goes away) on exit. Yields the session's transport once the stream has + been answered.""" + stream_opened = anyio.Event() + client_gone = anyio.Event() + sent_messages: list[Message] = [] + request_delivered = False + + async def send(message: Message) -> None: + sent_messages.append(message) + stream_opened.set() + + async def receive() -> Message: + # A GET carries an empty body; after that the client just holds the + # stream open until it goes away. + nonlocal request_delivered + if not request_delivered: + request_delivered = True + return {"type": "http.request", "body": b"", "more_body": False} + await client_gone.wait() + return {"type": "http.disconnect"} + + async with anyio.create_task_group() as tg: + tg.start_soon(manager.handle_request, _request_scope(session_id=session_id, method="GET"), receive, send) + with anyio.fail_after(5): + await stream_opened.wait() + assert (sent_messages[0]["type"], sent_messages[0]["status"]) == ("http.response.start", 200) + yield manager._server_instances[session_id] + client_gone.set() + + @pytest.mark.anyio -async def test_idle_session_is_reaped(): +async def test_idle_session_is_reaped(caplog: pytest.LogCaptureFixture, request: pytest.FixtureRequest): """After idle timeout fires, the session returns 404.""" app = Server("test-idle-reap") manager = StreamableHTTPSessionManager(app=app, session_idle_timeout=0.05) + observer = _observe_idle_timeout(caplog, request) async with manager.run(): - sent_messages: list[Message] = [] + session_id = await _open_session(manager, None) - async def mock_send(message: Message): - sent_messages.append(message) + # Wait for the 50ms idle timeout to fire and the session to be unregistered. Re-requesting + # the session to poll for the 404 would push its idle deadline forward and keep it alive. + with anyio.fail_after(5): + await observer.reaped.wait() - scope = { - "type": "http", - "method": "POST", - "path": "/mcp", - "headers": [(b"content-type", b"application/json")], + # Verify via public API: old session ID now returns 404 + assert await _request_session(manager, session_id, None) == 404 + + +@pytest.mark.anyio +async def test_expired_session_runs_the_server_lifespan_to_completion( + caplog: pytest.LogCaptureFixture, request: pytest.FixtureRequest +) -> None: + """When a session expires, the server's lifespan for it is torn down the way it is after a DELETE: + cleanup that awaits runs to completion instead of being cancelled part way.""" + torn_down = anyio.Event() + teardown: list[str] = [] + + @asynccontextmanager + async def lifespan(server: Server[Any, Any]) -> AsyncIterator[dict[str, Any]]: + try: + yield {} + finally: + try: + await anyio.lowlevel.checkpoint() # stands in for cleanup that has to await + teardown.append("completed") + finally: + torn_down.set() + + manager = StreamableHTTPSessionManager(app=Server("test-lifespan", lifespan=lifespan), session_idle_timeout=0.05) + observer = _observe_idle_timeout(caplog, request) + + async with manager.run(): + await _open_session(manager, None) + with anyio.fail_after(5): + await observer.reaped.wait() + await torn_down.wait() + assert teardown == ["completed"] + + +@pytest.mark.anyio +async def test_request_in_flight_holds_the_session_open( + caplog: pytest.LogCaptureFixture, request: pytest.FixtureRequest +) -> None: + """A session does not expire while one of its requests is still being served, however long that takes; + the idle period is counted from the moment its last request completes.""" + tool_started = anyio.Event() + release_tool = anyio.Event() + app = Server("test-in-flight") + + @app.call_tool() + async def handle_call_tool(name: str, arguments: dict[str, Any]) -> list[TextContent]: + tool_started.set() + await release_tool.wait() + return [TextContent(type="text", text="done")] + + manager = StreamableHTTPSessionManager(app=app, session_idle_timeout=30) + observer = _observe_idle_timeout(caplog, request) + + async with manager.run(): + session_id = await _open_session(manager, None) + transport = manager._server_instances[session_id] + call_tool_body = json.dumps( + {"jsonrpc": "2.0", "id": 2, "method": "tools/call", "params": {"name": "slow", "arguments": {}}} + ).encode() + responses: list[tuple[Message, bytes]] = [] + + async def call_tool() -> None: + responses.append(await _call(manager, _request_scope(session_id=session_id), call_tool_body)) + + async with anyio.create_task_group() as tg: + tg.start_soon(call_tool) + with anyio.fail_after(5): + await tool_started.wait() + # While the call is being served the idle countdown is suspended. + assert transport.idle_scope is not None and transport.idle_scope.deadline == math.inf + # From here on a short idle period, counted from the moment the call completes. + transport._idle_timeout = 0.05 + release_tool.set() + + response_start, response_body = responses[0] + assert response_start["status"] == 200 + assert b'"done"' in response_body + + # Nothing is in flight any more, so the idle period now runs out. + with anyio.fail_after(5): + await observer.reaped.wait() + assert session_id not in manager._server_instances + assert transport.is_terminated + assert await _request_session(manager, session_id, None) == 404 + + +@pytest.mark.anyio +async def test_open_event_stream_holds_the_session_open( + caplog: pytest.LogCaptureFixture, request: pytest.FixtureRequest +) -> None: + """A client listening on the session's GET stream keeps the session, even if it sends nothing; + once the stream closes the idle period runs out and the session is gone.""" + manager = StreamableHTTPSessionManager(app=Server("test-get-stream"), session_idle_timeout=30) + observer = _observe_idle_timeout(caplog, request) + + async with manager.run(): + session_id = await _open_session(manager, None) + + async with _open_event_stream(manager, session_id) as transport: + # The stream has been answered, so it is in flight: the idle countdown is suspended. + assert transport.idle_scope is not None and transport.idle_scope.deadline == math.inf + # From here on a short idle period, counted from the moment the stream closes. + transport._idle_timeout = 0.05 + + with anyio.fail_after(5): + await observer.reaped.wait() + assert session_id not in manager._server_instances + assert transport.is_terminated + assert await _request_session(manager, session_id, None) == 404 + + +@pytest.mark.anyio +async def test_request_completing_under_an_open_event_stream_does_not_start_the_countdown( + caplog: pytest.LogCaptureFixture, request: pytest.FixtureRequest +) -> None: + """A request that completes while the session's GET stream is still open does not start the idle + period: the stream is still in flight, so the countdown only begins once it closes too.""" + manager = StreamableHTTPSessionManager(app=Server("test-get-stream-and-post"), session_idle_timeout=30) + observer = _observe_idle_timeout(caplog, request) + + async with manager.run(): + session_id = await _open_session(manager, None) + + async with _open_event_stream(manager, session_id) as transport: + assert transport.idle_scope is not None and transport.idle_scope.deadline == math.inf + ping = b'{"jsonrpc": "2.0", "id": 2, "method": "ping"}' + response_start, _ = await _call(manager, _request_scope(session_id=session_id), ping) + assert response_start["status"] == 200 + # The ping has completed (in-flight bookkeeping included, since `_call` only returns once the + # manager has), but the open stream still suspends the idle countdown. + assert transport.idle_scope.deadline == math.inf + # From here on a short idle period, counted from the moment the stream closes. + transport._idle_timeout = 0.05 + + with anyio.fail_after(5): + await observer.reaped.wait() + assert session_id not in manager._server_instances + assert transport.is_terminated + assert await _request_session(manager, session_id, None) == 404 + + +def test_session_idle_timeout_defaults_to_thirty_minutes() -> None: + """Stateful sessions expire after 30 minutes without a request in flight unless configured otherwise.""" + manager = StreamableHTTPSessionManager(app=Server("test")) + assert manager.session_idle_timeout == DEFAULT_SESSION_IDLE_TIMEOUT == 30 * 60 + + +@pytest.mark.parametrize("session_idle_timeout", [0, -1, float("inf"), float("nan")]) +def test_session_idle_timeout_rejects_invalid_values(session_idle_timeout: float) -> None: + """The idle timeout is a positive, finite number of seconds, or None for sessions that never expire.""" + with pytest.raises(ValueError) as exc_info: + StreamableHTTPSessionManager(app=Server("test"), session_idle_timeout=session_idle_timeout) + assert str(exc_info.value) == "session_idle_timeout must be a positive, finite number of seconds" + + +@pytest.mark.anyio +async def test_session_idle_timeout_is_unused_in_stateless_mode() -> None: + """Stateless mode keeps no sessions, so the idle timeout is accepted and simply has nothing to expire.""" + manager = StreamableHTTPSessionManager(app=Server("test"), session_idle_timeout=30, stateless=True) + async with manager.run(): + response_start, _ = await _call(manager, _request_scope(), _INITIALIZE_BODY) + assert response_start["status"] == 200 + assert manager._server_instances == {} + + +@pytest.mark.anyio +@pytest.mark.parametrize("session_idle_timeout", [DEFAULT_SESSION_IDLE_TIMEOUT, None]) +async def test_deleted_session_is_forgotten(session_idle_timeout: float | None) -> None: + """A client DELETE ends the session and the manager stops tracking it; the ID is unknown afterwards.""" + manager = StreamableHTTPSessionManager(app=Server("test-delete"), session_idle_timeout=session_idle_timeout) + async with manager.run(): + session_id = await _open_session(manager, None) + assert session_id in manager._server_instances + + assert await _request_session(manager, session_id, None, method="DELETE") == 200 + assert session_id not in manager._server_instances + response_start, response_body = await _call(manager, _request_scope(session_id=session_id)) + assert response_start["status"] == 404 + assert json.loads(response_body) == { + "jsonrpc": "2.0", + "id": "server-error", + "error": {"code": INVALID_REQUEST, "message": "Session not found"}, } - async def mock_receive(): - return {"type": "http.request", "body": b"", "more_body": False} - await manager.handle_request(scope, mock_receive, mock_send) +@pytest.mark.anyio +async def test_opening_request_that_fails_leaves_no_session() -> None: + """If serving the request that would open a session raises, the provisional session is discarded + there and then rather than left registered with its server task running.""" + manager = StreamableHTTPSessionManager(app=Server("test-failed-open")) + with _created_transports() as transports: + async with manager.run(): + with ( + patch.object( + StreamableHTTPServerTransport, "handle_request", AsyncMock(side_effect=RuntimeError("boom")) + ), + pytest.raises(RuntimeError, match="boom"), + anyio.fail_after(5), + ): + await _call(manager, _request_scope(), _INITIALIZE_BODY) + assert manager._server_instances == {} + assert manager._session_owners == {} + (transport,) = transports + assert transport.is_terminated - session_id = None - for msg in sent_messages: # pragma: no branch - if msg["type"] == "http.response.start": # pragma: no branch - for header_name, header_value in msg.get("headers", []): # pragma: no branch - if header_name.decode().lower() == MCP_SESSION_ID_HEADER.lower(): - session_id = header_value.decode() - break - if session_id: # pragma: no branch - break - assert session_id is not None, "Session ID not found in response headers" +@pytest.mark.anyio +async def test_opening_request_that_is_cancelled_leaves_no_session() -> None: + """If the request that would open a session is cancelled while it is being served (the client went + away), the provisional session is discarded rather than left registered.""" + manager = StreamableHTTPSessionManager(app=Server("test-cancelled-open")) + entered = anyio.Event() - # Wait for the 50ms idle timeout to fire and cleanup to complete - await anyio.sleep(0.1) + async def hang(self: StreamableHTTPServerTransport, scope: Scope, receive: Receive, send: Send) -> None: + entered.set() + await anyio.sleep_forever() - # Verify via public API: old session ID now returns 404 - response_messages: list[Message] = [] + opening_request = anyio.CancelScope() - async def capture_send(message: Message): - response_messages.append(message) + async def open_session() -> None: + with opening_request: + await _call(manager, _request_scope(), _INITIALIZE_BODY) - scope_with_session = { - "type": "http", - "method": "POST", - "path": "/mcp", - "headers": [ - (b"content-type", b"application/json"), - (b"mcp-session-id", session_id.encode()), - ], + with _created_transports() as transports, patch.object(StreamableHTTPServerTransport, "handle_request", hang): + async with manager.run(): + async with anyio.create_task_group() as tg: + tg.start_soon(open_session) + with anyio.fail_after(5): + await entered.wait() + assert len(manager._server_instances) == 1 + opening_request.cancel() + assert manager._server_instances == {} + assert manager._session_owners == {} + (transport,) = transports + assert transport.is_terminated + + +@pytest.mark.anyio +async def test_opening_request_whose_session_task_cannot_start_leaves_no_session() -> None: + """If the server task for a would-be session cannot be started, the provisional session is discarded + (forgotten, its transport terminated) rather than left registered without anything serving it.""" + manager = StreamableHTTPSessionManager(app=Server("test-unstartable-open")) + + @asynccontextmanager + async def connect_that_fails(self: StreamableHTTPServerTransport) -> AsyncIterator[None]: + raise RuntimeError("boom") + yield + + with _created_transports() as transports: + async with manager.run(): + with ( + patch.object(StreamableHTTPServerTransport, "connect", connect_that_fails), + pytest.raises(RuntimeError, match="boom"), + anyio.fail_after(5), + ): + await _call(manager, _request_scope(), _INITIALIZE_BODY) + assert manager._server_instances == {} + assert manager._session_owners == {} + (transport,) = transports + assert transport.is_terminated + + +@pytest.mark.anyio +async def test_stateless_request_that_is_cancelled_still_terminates_its_transport() -> None: + """If a stateless request is cancelled while it is being served (the client went away), its transport + is terminated all the same, which is what ends the per-request server task.""" + manager = StreamableHTTPSessionManager(app=Server("test-stateless-cancelled"), stateless=True) + entered = anyio.Event() + + async def hang(self: StreamableHTTPServerTransport, scope: Scope, receive: Receive, send: Send) -> None: + entered.set() + await anyio.sleep_forever() + + stateless_request = anyio.CancelScope() + + async def make_request() -> None: + with stateless_request: + await _call(manager, _request_scope(), _INITIALIZE_BODY) + + with _created_transports() as transports, patch.object(StreamableHTTPServerTransport, "handle_request", hang): + async with manager.run(): + async with anyio.create_task_group() as tg: + tg.start_soon(make_request) + with anyio.fail_after(5): + await entered.wait() + stateless_request.cancel() + (transport,) = transports + assert transport.is_terminated + + +@pytest.mark.anyio +@pytest.mark.parametrize( + ("method", "headers", "body", "expected_status"), + [ + ("POST", _JSON_HEADERS, b'{"jsonrpc": "2.0", "id": 1, "method": "tools/list"}', 400), + ("POST", _JSON_HEADERS, b'{"jsonrpc": "2.0", "method": "notifications/initialized"}', 400), + ("POST", _JSON_HEADERS, b"{not json", 400), + ("POST", _JSON_HEADERS | {"accept": "text/plain"}, _INITIALIZE_BODY, 406), + ("GET", {"accept": "text/event-stream"}, b"", 400), + ("DELETE", _JSON_HEADERS, b"", 400), + ("PATCH", _JSON_HEADERS, b"", 405), + ], + ids=[ + "non-initialize-request", + "notification", + "malformed-json", + "unacceptable-accept-header", + "get-without-session", + "delete-without-session", + "unsupported-method", + ], +) +async def test_refused_opening_request_leaves_no_session( + method: str, headers: dict[str, str], body: bytes, expected_status: int +) -> None: + """Only an accepted initialize opens a session: a request without a session ID that is answered with an + error leaves nothing registered once the manager has answered it.""" + manager = StreamableHTTPSessionManager(app=Server("test-refused")) + scope: Scope = { + "type": "http", + "method": method, + "path": "/mcp", + "headers": [(name.encode(), value.encode()) for name, value in headers.items()], + } + with _created_transports() as transports: + async with manager.run(): + response_start, _ = await _call(manager, scope, body) + assert response_start["status"] == expected_status + assert manager._server_instances == {} + assert manager._session_owners == {} + (transport,) = transports + assert transport.is_terminated + + +@pytest.mark.anyio +async def test_new_session_is_refused_at_max_sessions() -> None: + """At the session limit a further initialize is answered 503 and opens nothing; room frees up as + sessions end.""" + manager = StreamableHTTPSessionManager(app=Server("test-cap"), max_sessions=1) + async with manager.run(): + first = await _open_session(manager, None) + + response_start, response_body = await _call(manager, _request_scope(), _INITIALIZE_BODY) + assert response_start["status"] == 503 + assert json.loads(response_body) == { + "jsonrpc": "2.0", + "id": "server-error", + "error": {"code": INTERNAL_ERROR, "message": "Too many open sessions"}, } + assert list(manager._server_instances) == [first] - await manager.handle_request(scope_with_session, mock_receive, capture_send) + assert await _request_session(manager, first, None, method="DELETE") == 200 + second = await _open_session(manager, None) + assert list(manager._server_instances) == [second] - response_start = next( - (msg for msg in response_messages if msg["type"] == "http.response.start"), - None, - ) - assert response_start is not None - assert response_start["status"] == 404 + +@pytest.mark.anyio +async def test_client_that_is_slow_to_send_its_opening_request_does_not_hold_up_others() -> None: + """While one client has yet to finish sending the request that would open its session, another + client can still open one.""" + manager = StreamableHTTPSessionManager(app=Server("test-slow-open")) + body_awaited = anyio.Event() + + async def stall() -> None: + # This client has sent its headers but never finishes sending the body. + body_awaited.set() + await anyio.sleep_forever() + + slow_client = anyio.CancelScope() + + async def open_slowly() -> None: + with slow_client: + # Nothing is ever sent back to this client, so any `send` will do. + await manager.handle_request(_request_scope(), cast(Receive, stall), AsyncMock()) + + session_id: str | None = None + async with manager.run(): + async with anyio.create_task_group() as tg: + tg.start_soon(open_slowly) + with anyio.fail_after(5): + await body_awaited.wait() + session_id = await _open_session(manager, None) + slow_client.cancel() + assert session_id is not None + assert list(manager._server_instances) == [session_id] -def test_session_idle_timeout_rejects_non_positive(): - with pytest.raises(ValueError, match="positive number"): - StreamableHTTPSessionManager(app=Server("test"), session_idle_timeout=-1) - with pytest.raises(ValueError, match="positive number"): - StreamableHTTPSessionManager(app=Server("test"), session_idle_timeout=0) +def test_max_sessions_defaults_to_ten_thousand() -> None: + """A manager holds at most 10 000 concurrent stateful sessions unless configured otherwise.""" + manager = StreamableHTTPSessionManager(app=Server("test")) + assert manager.max_sessions == DEFAULT_MAX_SESSIONS == 10_000 + assert StreamableHTTPSessionManager(app=Server("test"), max_sessions=None).max_sessions is None -def test_session_idle_timeout_rejects_stateless(): - with pytest.raises(RuntimeError, match="not supported in stateless"): - StreamableHTTPSessionManager(app=Server("test"), session_idle_timeout=30, stateless=True) +@pytest.mark.parametrize("max_sessions", [0, -1]) +def test_max_sessions_rejects_non_positive_values(max_sessions: int) -> None: + with pytest.raises(ValueError) as exc_info: + StreamableHTTPSessionManager(app=Server("test"), max_sessions=max_sessions) + assert str(exc_info.value) == "max_sessions must be a positive number of sessions or None" def _user(client_id: str, subject: str | None = None, issuer: str | None = None) -> AuthenticatedUser: @@ -494,19 +920,33 @@ def _request_scope( return scope -async def _open_session(manager: StreamableHTTPSessionManager, user: AuthenticatedUser | None) -> str: - """Create a new session as `user` and return its session ID.""" +async def _call(manager: StreamableHTTPSessionManager, scope: Scope, body: bytes = b"") -> tuple[Message, bytes]: + """Drive one request through the manager in process; return its `http.response.start` message and body.""" sent_messages: list[Message] = [] + body_delivered = False - async def mock_send(message: Message) -> None: + async def send(message: Message) -> None: sent_messages.append(message) - async def mock_receive() -> Message: - return {"type": "http.request", "body": b"", "more_body": False} + async def receive() -> Message: + # Deliver the body once, then block like a client holding the connection + # open; a streaming response ends when the server closes it. + nonlocal body_delivered + if body_delivered: + await anyio.sleep_forever() + body_delivered = True + return {"type": "http.request", "body": body, "more_body": False} + + await manager.handle_request(scope, receive, send) + response_start = next(msg for msg in sent_messages if msg["type"] == "http.response.start") + response_body = b"".join(msg.get("body", b"") for msg in sent_messages if msg["type"] == "http.response.body") + return response_start, response_body - await manager.handle_request(_request_scope(user=user), mock_receive, mock_send) - response_start = next(msg for msg in sent_messages if msg["type"] == "http.response.start") +async def _open_session(manager: StreamableHTTPSessionManager, user: AuthenticatedUser | None) -> str: + """Create a new session as `user` with an initialize request and return its session ID.""" + response_start, _ = await _call(manager, _request_scope(user=user), _INITIALIZE_BODY) + assert response_start["status"] == 200 headers = dict(response_start.get("headers", [])) return headers[MCP_SESSION_ID_HEADER.encode()].decode() @@ -515,26 +955,14 @@ async def _request_session( manager: StreamableHTTPSessionManager, session_id: str, user: AuthenticatedUser | None, method: str = "POST" ) -> int: """Send a request for an existing session as `user` and return the response status.""" - sent_messages: list[Message] = [] - - async def mock_send(message: Message) -> None: - sent_messages.append(message) - - async def mock_receive() -> Message: - return {"type": "http.request", "body": b"", "more_body": False} - - await manager.handle_request( - _request_scope(session_id=session_id, user=user, method=method), mock_receive, mock_send - ) - - response_start = next(msg for msg in sent_messages if msg["type"] == "http.response.start") + response_start, _ = await _call(manager, _request_scope(session_id=session_id, user=user, method=method)) return response_start["status"] @pytest.fixture async def manager_with_live_session(): - """A running manager around a real `Server`. Sessions remain registered until - `manager.run()` exits because `Server.run` blocks waiting for an initialize message.""" + """A running manager around a real `Server`. Sessions are opened with a real initialize and stay + registered until `manager.run()` exits because nothing in these tests ends them.""" manager = StreamableHTTPSessionManager(app=Server("test-session-credentials")) async with manager.run(): yield manager diff --git a/tests/shared/test_httpx_utils.py b/tests/shared/test_httpx_utils.py index dcc6fd003c..7709fc968c 100644 --- a/tests/shared/test_httpx_utils.py +++ b/tests/shared/test_httpx_utils.py @@ -1,16 +1,27 @@ -"""Tests for httpx utility functions.""" +"""Tests for the httpx helpers the client transports are built on.""" + +from collections.abc import AsyncGenerator, AsyncIterator +from typing import Any import httpx +import pytest + +from mcp.shared._httpx_utils import ( + create_mcp_http_client, + request_within_origin, + sse_within_origin, + stream_within_origin, +) -from mcp.shared._httpx_utils import create_mcp_http_client +pytestmark = pytest.mark.anyio -def test_default_settings(): - """Test that default settings are applied correctly.""" +def test_default_client_uses_mcp_timeouts_and_httpx_redirect_default(): + """The factory applies the transports' timeouts and leaves redirect following to the transports.""" client = create_mcp_http_client() - assert client.follow_redirects is True - assert client.timeout.connect == 30.0 + assert client.follow_redirects is False + assert client.timeout == httpx.Timeout(30.0, read=300.0) def test_custom_parameters(): @@ -22,3 +33,253 @@ def test_custom_parameters(): assert client.headers["Authorization"] == "Bearer token" assert client.timeout.connect == 60.0 + + +class _Body(httpx.AsyncByteStream): + """A response body served as a real stream, recording whether the client closed it.""" + + def __init__(self, data: bytes, closed: list[bool]) -> None: + self._data = data + self._closed = closed + + async def __aiter__(self) -> AsyncIterator[bytes]: + yield self._data + + async def aclose(self) -> None: + self._closed.append(True) + + +def _recording_client( + redirects: dict[str, tuple[int, str]], **client_kwargs: Any +) -> tuple[httpx.AsyncClient, list[str], list[bool]]: + """A client whose server redirects each URL in `redirects` (status, Location) and answers 200 + to anything else; plus the `METHOD url` lines the server received and one entry per redirect + response body the client closed.""" + received: list[str] = [] + closed: list[bool] = [] + + def serve(request: httpx.Request) -> httpx.Response: + received.append(f"{request.method} {request.url}") + if str(request.url) in redirects: + status, location = redirects[str(request.url)] + return httpx.Response(status, headers={"location": location}, stream=_Body(b"moved", closed)) + return httpx.Response(200, text=request.content.decode() or "ok") + + return httpx.AsyncClient(transport=httpx.MockTransport(serve), **client_kwargs), received, closed + + +@pytest.mark.parametrize( + ("url", "location"), + [ + ("http://mcp.example/mcp", "http://mcp.example/mcp/"), + ("http://mcp.example/mcp", "/other/path"), + ("http://mcp.example:8080/mcp", "http://mcp.example:8080/v2/mcp"), + ("http://mcp.example/mcp", "http://MCP.EXAMPLE:80/mcp/"), + ("http://mcp.example/mcp", "https://mcp.example:443/mcp"), + ], +) +async def test_redirect_within_origin_is_followed_with_method_and_body(url: str, location: str): + """A redirect that stays on the request's origin (or upgrades it to https) is followed, and a + 307 keeps the method and body (SDK-defined policy; the re-send itself is httpx's).""" + client, received, closed = _recording_client({url: (307, location)}) + + async with client, stream_within_origin(client, "POST", url, content=b"payload") as response: + await response.aread() + + assert response.status_code == 200 + assert response.text == "payload" + assert received == [f"POST {url}", f"POST {httpx.URL(url).join(location)}"] + assert closed == [True] + + +@pytest.mark.parametrize( + "location", + [ + "http://other.example/mcp", + "http://mcp.example:8080/mcp", + "http://sub.mcp.example/mcp", + "https://mcp.example:8443/mcp", + "ftp://mcp.example/mcp", + ], +) +async def test_redirect_outside_origin_is_not_followed(location: str): + """A redirect to another origin is handed back unfollowed, the way httpx hands back a redirect + with following off, and the location is never requested (SDK-defined policy).""" + url = "http://mcp.example/mcp" + client, received, closed = _recording_client({url: (307, location)}) + + async with client, stream_within_origin(client, "POST", url, content=b"payload") as response: + pass + + assert response.status_code == 307 + assert response.next_request is not None + assert response.next_request.url == location + assert received == [f"POST {url}"] + assert closed == [True] + + +@pytest.mark.parametrize("status", [301, 302, 303]) +async def test_method_changing_redirect_of_a_post_is_not_followed(status: int): + """httpx turns a POST into a body-less GET for 301/302/303, which would drop the message, so a + same-origin redirect with one of those codes is handed back unfollowed (SDK-defined).""" + url = "http://mcp.example/mcp" + client, received, _ = _recording_client({url: (status, "/mcp/")}) + + async with client, stream_within_origin(client, "POST", url, content=b"payload") as response: + pass + + assert response.status_code == status + assert received == [f"POST {url}"] + + +@pytest.mark.parametrize("status", [301, 302, 303, 307, 308]) +async def test_same_origin_redirect_of_a_get_is_followed_for_every_redirect_status(status: int): + """A GET keeps its method under every redirect status, so the SSE GET follows all of them + within the origin (SDK-defined policy over httpx's method rules).""" + url = "http://mcp.example/sse" + client, received, _ = _recording_client({url: (status, "/sse/")}) + + async with client, stream_within_origin(client, "GET", url) as response: + await response.aread() + + assert response.status_code == 200 + assert received == [f"GET {url}", "GET http://mcp.example/sse/"] + + +async def test_https_to_http_on_same_host_is_outside_origin(): + """Only the upgrade direction counts as staying on the origin; a downgrade is not followed.""" + url = "https://mcp.example/mcp" + client, received, _ = _recording_client({url: (302, "http://mcp.example/mcp")}) + + async with client, stream_within_origin(client, "GET", url) as response: + pass + + assert response.status_code == 302 + assert received == [f"GET {url}"] + + +async def test_client_configured_to_follow_redirects_is_still_scoped_to_origin(): + """The client's own follow_redirects=True does not widen the policy: the transport helper + decides per request (SDK-defined).""" + url = "http://mcp.example/mcp" + client, received, _ = _recording_client({url: (307, "http://other.example/mcp")}, follow_redirects=True) + + async with client, stream_within_origin(client, "POST", url) as response: + pass + + assert response.status_code == 307 + assert received == [f"POST {url}"] + + +async def test_redirect_past_the_client_max_redirects_budget_is_handed_back_unfollowed(): + """Same-origin hops are bounded by the client's max_redirects; the redirect after that is not + followed but handed back like any other, so a loop fails the one call rather than raising + (SDK-defined; max_redirects=0 therefore means "follow none").""" + url = "http://mcp.example/a" + client, received, closed = _recording_client( + { + "http://mcp.example/a": (307, "/b"), + "http://mcp.example/b": (307, "/c"), + "http://mcp.example/c": (307, "/d"), + }, + max_redirects=2, + ) + + async with client: + response = await request_within_origin(client, "GET", url) + + assert response.status_code == 307 + assert response.next_request is not None + assert response.next_request.url == "http://mcp.example/d" + assert received == ["GET http://mcp.example/a", "GET http://mcp.example/b", "GET http://mcp.example/c"] + assert closed == [True, True, True] + + +async def test_redirect_location_with_userinfo_is_not_followed(): + """A Location carrying user:password is handed back unfollowed even within the origin, since + httpx would otherwise send that userinfo as Basic auth (SDK-defined).""" + url = "http://mcp.example/mcp" + client, received, _ = _recording_client({url: (307, "http://user:secret@mcp.example/mcp/")}) + + async with client, stream_within_origin(client, "POST", url) as response: + pass + + assert response.status_code == 307 + assert received == [f"POST {url}"] + + +async def test_userinfo_of_the_configured_url_kept_by_a_relative_location_is_followed(): + """Userinfo the caller put in the endpoint URL is carried over by a relative Location (URL join + keeps the authority); that is the caller's own credential for the same origin, so the redirect + is followed as httpx itself would (SDK-defined).""" + url = "http://user:secret@mcp.example/mcp" + client, received, _ = _recording_client({url: (307, "/mcp/")}) + + async with client, stream_within_origin(client, "POST", url, content=b"payload") as response: + await response.aread() + + assert response.status_code == 200 + assert received == [f"POST {url}", "POST http://user:secret@mcp.example/mcp/"] + + +async def test_request_within_origin_returns_a_read_response(): + """The non-streaming form hands back a response whose body is already read.""" + url = "http://mcp.example/mcp" + client, received, _ = _recording_client({url: (307, "/mcp/")}) + + async with client: + response = await request_within_origin(client, "DELETE", url) + + assert response.status_code == 200 + assert response.text == "ok" + assert received == [f"DELETE {url}", "DELETE http://mcp.example/mcp/"] + + +async def test_sse_within_origin_sends_event_stream_headers_and_caller_headers(): + """The SSE form asks for an event stream exactly as httpx_sse.aconnect_sse() does, merged case-insensitively + with the caller's headers, and yields an EventSource over the final response.""" + seen: list[httpx.Headers] = [] + + def serve(request: httpx.Request) -> httpx.Response: + seen.append(request.headers) + return httpx.Response(200, headers={"content-type": "text/event-stream"}, text="data: hello\n\n") + + client = httpx.AsyncClient(transport=httpx.MockTransport(serve)) + async with client: + async with sse_within_origin(client, "http://mcp.example/sse") as source: + events = [event.data async for event in source.aiter_sse()] + async with sse_within_origin(client, "http://mcp.example/sse", headers={"accept": "x/y", "k": "v"}): + pass + + assert events == ["hello"] + assert seen[0]["accept"] == "text/event-stream" + assert seen[0]["cache-control"] == "no-store" + assert seen[1].get_list("accept") == ["x/y"] + assert seen[1]["cache-control"] == "no-store" + assert seen[1]["k"] == "v" + + +async def test_auth_flow_requests_are_not_redirected(): + """Requests an httpx Auth flow issues while a transport request is in flight (a token refresh, + say) inherit the per-request no-follow setting, so a redirect on them is handed back to the + auth flow rather than followed (httpx behaviour the transports rely on).""" + received: list[str] = [] + + class TokenThenRequest(httpx.Auth): + async def async_auth_flow(self, request: httpx.Request) -> AsyncGenerator[httpx.Request, httpx.Response]: + token_response = yield httpx.Request("POST", "http://mcp.example/token", content=b"grant") + request.headers["x-token-status"] = str(token_response.status_code) + yield request + + def serve(request: httpx.Request) -> httpx.Response: + received.append(f"{request.method} {request.url}") + if request.url.path == "/token": + return httpx.Response(307, headers={"location": "http://other.example/token"}) + return httpx.Response(200, text=request.headers["x-token-status"]) + + client = httpx.AsyncClient(transport=httpx.MockTransport(serve), auth=TokenThenRequest(), follow_redirects=True) + async with client: + response = await request_within_origin(client, "POST", "http://mcp.example/mcp") + + assert response.text == "307" + assert received == ["POST http://mcp.example/token", "POST http://mcp.example/mcp"] diff --git a/tests/shared/test_sse.py b/tests/shared/test_sse.py index 7604450f81..77d6cac65f 100644 --- a/tests/shared/test_sse.py +++ b/tests/shared/test_sse.py @@ -4,13 +4,12 @@ import time from collections.abc import AsyncGenerator, Generator from typing import Any -from unittest.mock import AsyncMock, MagicMock, Mock, patch +from unittest.mock import Mock import anyio import httpx import pytest import uvicorn -from httpx_sse import ServerSentEvent from inline_snapshot import snapshot from pydantic import AnyUrl from starlette.applications import Starlette @@ -26,6 +25,7 @@ from mcp.server.sse import SseServerTransport from mcp.server.transport_security import TransportSecuritySettings from mcp.shared.exceptions import McpError +from mcp.shared.message import SessionMessage from mcp.types import ( EmptyResult, ErrorData, @@ -538,12 +538,6 @@ def test_sse_server_transport_endpoint_validation(endpoint: str, expected_result assert sse._endpoint.startswith("/") -# ResourceWarning filter: When mocking aconnect_sse, the sse_client's internal task -# group doesn't receive proper cancellation signals, so the sse_reader task's finally -# block (which closes read_stream_writer) doesn't execute. This is a test artifact - -# the actual code path (`if not sse.data: continue`) IS exercised and works correctly. -# Production code with real SSE connections cleans up properly. -@pytest.mark.filterwarnings("ignore::ResourceWarning") @pytest.mark.anyio async def test_sse_client_handles_empty_keepalive_pings() -> None: """Test that SSE client properly handles empty data lines (keep-alive pings). @@ -552,10 +546,10 @@ async def test_sse_client_handles_empty_keepalive_pings() -> None: send an SSE event consisting of an event ID and an empty data field in order to prime the client to reconnect." - This test mocks the SSE event stream to include empty "message" events and - verifies the client skips them without crashing. + The event stream served here carries an endpoint event, an empty "message" + event (the case under test), then a real response; the client must skip the + empty one and deliver the response. """ - # Build a proper JSON-RPC response using types (not hardcoded strings) init_result = InitializeResult( protocolVersion="2024-11-05", capabilities=ServerCapabilities(), @@ -567,38 +561,82 @@ async def test_sse_client_handles_empty_keepalive_pings() -> None: result=init_result.model_dump(by_alias=True, exclude_none=True), ) response_json = response.model_dump_json(by_alias=True, exclude_none=True) + event_stream = ( + "event: endpoint\ndata: /messages/?session_id=abc123\n\n" + "event: message\ndata: \n\n" + f"event: message\ndata: {response_json}\n\n" + ) - # Create mock SSE events using httpx_sse's ServerSentEvent - async def mock_aiter_sse() -> AsyncGenerator[ServerSentEvent, None]: - # First: endpoint event - yield ServerSentEvent(event="endpoint", data="/messages/?session_id=abc123") - # Empty data keep-alive ping - this is what we're testing - yield ServerSentEvent(event="message", data="") - # Real JSON-RPC response - yield ServerSentEvent(event="message", data=response_json) - - mock_event_source = MagicMock() - mock_event_source.aiter_sse.return_value = mock_aiter_sse() - mock_event_source.response = MagicMock() - mock_event_source.response.raise_for_status = MagicMock() - - mock_aconnect_sse = MagicMock() - mock_aconnect_sse.__aenter__ = AsyncMock(return_value=mock_event_source) - mock_aconnect_sse.__aexit__ = AsyncMock(return_value=None) - - mock_client = MagicMock() - mock_client.__aenter__ = AsyncMock(return_value=mock_client) - mock_client.__aexit__ = AsyncMock(return_value=None) - mock_client.post = AsyncMock(return_value=MagicMock(status_code=200, raise_for_status=MagicMock())) - - with ( - patch("mcp.client.sse.create_mcp_http_client", return_value=mock_client), - patch("mcp.client.sse.aconnect_sse", return_value=mock_aconnect_sse), - ): - async with sse_client("http://test/sse") as (read_stream, _): - # Read the message - should skip the empty one and get the real response + def serve(request: httpx.Request) -> httpx.Response: + assert request.url.path == "/sse" + return httpx.Response(200, headers={"content-type": "text/event-stream"}, text=event_stream) + + def factory( + headers: dict[str, str] | None = None, + timeout: httpx.Timeout | None = None, + auth: httpx.Auth | None = None, + ) -> httpx.AsyncClient: + return httpx.AsyncClient(transport=httpx.MockTransport(serve)) + + with anyio.fail_after(5): + async with sse_client("http://test/sse", httpx_client_factory=factory) as (read_stream, _): msg = await read_stream.receive() - # If we get here without error, the empty message was skipped successfully - assert not isinstance(msg, Exception) + assert isinstance(msg, SessionMessage) assert isinstance(msg.message.root, types.JSONRPCResponse) assert msg.message.root.id == 1 + + +@pytest.mark.anyio +async def test_sse_client_follows_redirect_within_origin_on_connect() -> None: + """SDK-defined: a redirect of the SSE GET that stays on the endpoint's origin is followed by + the transport itself, with a client left at httpx's no-follow default.""" + received: list[str] = [] + + def serve(request: httpx.Request) -> httpx.Response: + received.append(str(request.url)) + if request.url.path == "/sse": + return httpx.Response(307, headers={"location": "/sse/"}) + assert request.url.path == "/sse/" + return httpx.Response( + 200, headers={"content-type": "text/event-stream"}, text="event: endpoint\ndata: /messages/\n\n" + ) + + def factory( + headers: dict[str, str] | None = None, + timeout: httpx.Timeout | None = None, + auth: httpx.Auth | None = None, + ) -> httpx.AsyncClient: + return httpx.AsyncClient(transport=httpx.MockTransport(serve)) + + with anyio.fail_after(5): + async with sse_client("http://test/sse", httpx_client_factory=factory): + pass + + assert received == ["http://test/sse", "http://test/sse/"] + + +@pytest.mark.anyio +async def test_sse_client_does_not_follow_redirect_to_another_origin_on_connect() -> None: + """SDK-defined: a redirect of the SSE GET to another origin is not followed, even with a client + configured to follow redirects: connecting fails with HTTPStatusError for the redirect response + (raised inside sse_client's task group) and that origin is never contacted.""" + received: list[str] = [] + + def serve(request: httpx.Request) -> httpx.Response: + received.append(str(request.url)) + return httpx.Response(307, headers={"location": "http://other.example/sse"}) + + def factory( + headers: dict[str, str] | None = None, + timeout: httpx.Timeout | None = None, + auth: httpx.Auth | None = None, + ) -> httpx.AsyncClient: + return httpx.AsyncClient(transport=httpx.MockTransport(serve), follow_redirects=True) + + with anyio.fail_after(5): + with pytest.raises(Exception) as exc_info: + async with sse_client("http://test/sse", httpx_client_factory=factory): + pytest.fail("should not connect") # pragma: no cover + + assert exc_info.group_contains(httpx.HTTPStatusError, match="307 Temporary Redirect") + assert received == ["http://test/sse"] diff --git a/tests/shared/test_streamable_http.py b/tests/shared/test_streamable_http.py index 631c96e86c..35376bc1c9 100644 --- a/tests/shared/test_streamable_http.py +++ b/tests/shared/test_streamable_http.py @@ -9,6 +9,7 @@ import socket import time from collections.abc import Generator +from concurrent.futures import ThreadPoolExecutor from datetime import timedelta from typing import Any from unittest.mock import MagicMock @@ -19,10 +20,12 @@ import requests import uvicorn from httpx_sse import ServerSentEvent +from inline_snapshot import snapshot from pydantic import AnyUrl from starlette.applications import Starlette from starlette.requests import Request from starlette.routing import Mount +from starlette.types import Message, Scope import mcp.types as types from mcp.client.session import ClientSession @@ -719,6 +722,77 @@ def test_streamable_http_transport_init_validation(): StreamableHTTPServerTransport(mcp_session_id="test\n") +@pytest.mark.parametrize("idle_timeout", [0, -1, float("inf"), float("nan")]) +def test_streamable_http_transport_rejects_invalid_idle_timeout(idle_timeout: float) -> None: + """A transport's idle timeout must be a positive, finite number of seconds; without one it never expires.""" + with pytest.raises(ValueError) as exc_info: + StreamableHTTPServerTransport(mcp_session_id="valid-id", idle_timeout=idle_timeout) + assert str(exc_info.value) == "idle_timeout must be a positive, finite number of seconds" + assert StreamableHTTPServerTransport(mcp_session_id="valid-id").idle_scope is None + + +def test_streamable_http_transport_with_idle_timeout_can_be_created_outside_an_event_loop() -> None: + """The idle scope is only created once connect() is entered, so a transport with a timeout can be + constructed without a running event loop.""" + # A bare thread has no async context; this one does, courtesy of the suite's shared runner. + with ThreadPoolExecutor(max_workers=1) as pool: + transport = pool.submit(StreamableHTTPServerTransport, mcp_session_id="valid-id", idle_timeout=5).result() + assert transport.idle_scope is None + + +@pytest.mark.anyio +async def test_streamable_http_transport_creates_its_idle_scope_on_connect() -> None: + """Entering connect() creates the idle scope the host enters around the session's message loop.""" + transport = StreamableHTTPServerTransport(mcp_session_id="valid-id", idle_timeout=5) + async with transport.connect(): + assert isinstance(transport.idle_scope, anyio.CancelScope) + await transport.terminate() + + +async def _post_to_transport(transport: StreamableHTTPServerTransport, body: dict[str, Any]) -> int: + """POST `body` straight to `transport` in process, as a client that then holds the connection open, + and return the status it answered with.""" + assert transport.mcp_session_id is not None + scope: Scope = { + "type": "http", + "method": "POST", + "path": "/mcp", + "query_string": b"", + "headers": [ + (b"content-type", b"application/json"), + (b"accept", b"application/json, text/event-stream"), + (MCP_SESSION_ID_HEADER.encode(), transport.mcp_session_id.encode()), + ], + } + sent: list[Message] = [] + + async def send(message: Message) -> None: + sent.append(message) + + request_body, incoming = anyio.create_memory_object_stream[Message](1) + async with request_body, incoming: + await request_body.send({"type": "http.request", "body": json.dumps(body).encode(), "more_body": False}) + with anyio.fail_after(5): + await transport.handle_request(scope, incoming.receive, send) + return next(message["status"] for message in sent if message["type"] == "http.response.start") + + +@pytest.mark.anyio +async def test_transport_whose_idle_period_ran_out_answers_as_terminated() -> None: + """Once the idle scope has fired, a request that still reaches the transport is answered 404 and the + transport is terminated, instead of being dispatched into the message loop the host is leaving.""" + transport = StreamableHTTPServerTransport(mcp_session_id="valid-id", idle_timeout=5) + ping = {"jsonrpc": "2.0", "id": 1, "method": "ping"} + async with transport.connect(): + assert transport.idle_scope is not None + # Exactly what the scope's deadline passing does. + transport.idle_scope.cancel() + + assert await _post_to_transport(transport, ping) == 404 + assert transport.is_terminated + assert await _post_to_transport(transport, ping) == 404 + + def test_session_termination(basic_server: None, basic_server_url: str): """Test session termination via DELETE and subsequent request handling.""" response = requests.post( @@ -756,7 +830,7 @@ def test_session_termination(basic_server: None, basic_server_url: str): json={"jsonrpc": "2.0", "method": "ping", "id": 2}, ) assert response.status_code == 404 - assert "Session has been terminated" in response.text + assert response.json()["error"]["message"] == "Session not found" def test_response(basic_server: None, basic_server_url: str): @@ -1182,41 +1256,36 @@ async def test_streamable_http_client_session_termination(basic_server: None, ba @pytest.mark.anyio -async def test_streamable_http_client_session_termination_204( - basic_server: None, basic_server_url: str, monkeypatch: pytest.MonkeyPatch -): +async def test_streamable_http_client_session_termination_204(basic_server: None, basic_server_url: str): """Test client session termination functionality with a 204 response. - This test patches the httpx client to return a 204 response for DELETEs. + The server answers the DELETE with 200; a wrapping HTTP transport rewrites that to 204 on the + way back, which is what some servers send. """ - # Save the original delete method to restore later - original_delete = httpx.AsyncClient.delete - - # Mock the client's delete method to return a 204 - async def mock_delete(self: httpx.AsyncClient, *args: Any, **kwargs: Any) -> httpx.Response: - # Call the original method to get the real response - response = await original_delete(self, *args, **kwargs) + class AnswerDeleteWith204(httpx.AsyncBaseTransport): + def __init__(self) -> None: + self.inner = httpx.AsyncHTTPTransport() - # Create a new response with 204 status code but same headers - mocked_response = httpx.Response( - 204, - headers=response.headers, - content=response.content, - request=response.request, - ) - return mocked_response + async def handle_async_request(self, request: httpx.Request) -> httpx.Response: + response = await self.inner.handle_async_request(request) + if request.method != "DELETE" or response.status_code != 200: + return response + await response.aread() + return httpx.Response(204, headers=response.headers, request=request) - # Apply the patch to the httpx client - monkeypatch.setattr(httpx.AsyncClient, "delete", mock_delete) + async def aclose(self) -> None: + await self.inner.aclose() captured_session_id = None - # Create the streamable_http_client with a custom httpx client to capture headers - async with streamable_http_client(f"{basic_server_url}/mcp") as ( - read_stream, - write_stream, - get_session_id, + async with ( + httpx.AsyncClient(transport=AnswerDeleteWith204()) as terminating_client, + streamable_http_client(f"{basic_server_url}/mcp", http_client=terminating_client) as ( + read_stream, + write_stream, + get_session_id, + ), ): async with ClientSession(read_stream, write_stream) as session: # Initialize the session @@ -2224,7 +2293,7 @@ async def test_streamable_http_client_does_not_mutate_provided_client( "Authorization": "Bearer test-token", } - async with httpx.AsyncClient(headers=original_headers, follow_redirects=True) as custom_client: + async with httpx.AsyncClient(headers=original_headers) as custom_client: # Use the client with streamable_http_client async with streamable_http_client(f"{basic_server_url}/mcp", http_client=custom_client) as ( read_stream, @@ -2255,7 +2324,7 @@ async def test_streamable_http_client_mcp_headers_override_defaults( # httpx.AsyncClient has default "accept: */*" header # We need to verify that our MCP accept header overrides it in actual requests - async with httpx.AsyncClient(follow_redirects=True) as client: + async with httpx.AsyncClient() as client: # Verify client has default accept header assert client.headers.get("accept") == "*/*" @@ -2293,7 +2362,7 @@ async def test_streamable_http_client_preserves_custom_with_mcp_headers( "Authorization": "Bearer test-token", } - async with httpx.AsyncClient(headers=custom_headers, follow_redirects=True) as client: + async with httpx.AsyncClient(headers=custom_headers) as client: async with streamable_http_client(f"{basic_server_url}/mcp", http_client=client) as ( read_stream, write_stream, @@ -2353,3 +2422,172 @@ async def test_streamablehttp_client_deprecation_warning(basic_server: None, bas await session.initialize() tools = await session.list_tools() assert len(tools.tools) > 0 + + +@pytest.mark.anyio +async def test_trailing_slash_redirect_within_origin_is_followed_by_the_transport( + basic_server: None, basic_server_url: str +) -> None: + """SDK-defined: a redirect that stays on the endpoint's origin (here Starlette's Mount sending + /mcp to /mcp/) is followed by the transport itself, so a caller-supplied client left at + httpx's no-follow default still connects.""" + urls: list[str] = [] + + async def record(request: httpx.Request) -> None: + urls.append(str(request.url)) + + with anyio.fail_after(10): + async with ( + httpx.AsyncClient(event_hooks={"request": [record]}) as http, + streamable_http_client(f"{basic_server_url}/mcp", http_client=http) as (read_stream, write_stream, _), + ClientSession(read_stream, write_stream) as session, + ): + result = await session.initialize() + + assert result.serverInfo.name == SERVER_NAME + assert urls[:2] == [f"{basic_server_url}/mcp", f"{basic_server_url}/mcp/"] + + +def _leaf_exception(exc: BaseException) -> BaseException: + """The one exception inside the (possibly nested) exception group an anyio task group raises.""" + while (inner := getattr(exc, "exceptions", None)) is not None: + (exc,) = inner + return exc + + +async def _assert_redirected_post_fails(url: str, location: str, expected_message: str) -> None: + """Send one request through streamable_http_client, with a client configured to follow redirects, + to a server answering `url` with a 307 to `location`, and check that the connection ends with + HTTPStatusError carrying `expected_message` and that nothing but `url` was requested.""" + urls: list[str] = [] + + def handler(request: httpx.Request) -> httpx.Response: + urls.append(str(request.url)) + return httpx.Response(307, headers={"location": location}) + + with anyio.fail_after(5): + # The request's POST fails inside the transport's task group, which ends the connection. + with pytest.raises(Exception) as exc_info: + async with ( # pragma: no branch + httpx.AsyncClient(transport=httpx.MockTransport(handler), follow_redirects=True) as http, + streamable_http_client(url, http_client=http) as (read_stream, write_stream, _), + read_stream, + write_stream, + ): + request = JSONRPCRequest(jsonrpc="2.0", id=1, method="tools/list", params={}) + await write_stream.send(SessionMessage(JSONRPCMessage(request))) + await read_stream.receive() + error = _leaf_exception(exc_info.value) + assert isinstance(error, httpx.HTTPStatusError) + assert error.response.status_code == 307 + assert str(error) == expected_message + assert urls == [url] + + +@pytest.mark.anyio +async def test_redirect_to_another_origin_is_not_followed_and_fails_the_request() -> None: + """SDK-defined: a redirect pointing outside the endpoint's origin is not followed, whatever the + caller's client is configured to do: nothing is sent to the other origin, and the request fails + the way any non-2xx response does, with HTTPStatusError naming the location.""" + await _assert_redirected_post_fails( + "http://mcp.example/mcp", + "http://other.example/x", + snapshot( + "Redirect to http://other.example/x not followed; use that URL as the endpoint if it is the intended server" + ), + ) + + +@pytest.mark.anyio +async def test_https_endpoint_redirected_to_plain_http_is_explained_and_the_https_form_suggested() -> None: + """SDK-authored text: a redirect of an HTTPS endpoint to plain HTTP on the same host (the usual + sign of a TLS-terminating proxy the server does not trust) never suggests the http:// URL.""" + await _assert_redirected_post_fails( + "https://mcp.example/mcp", + "http://mcp.example/mcp/", + snapshot("""\ +Redirect to http://mcp.example/mcp/ not followed: it would downgrade this HTTPS endpoint to plain HTTP. +The server is likely behind a TLS-terminating proxy whose forwarded headers it does not trust, +often combined with a trailing-slash difference. Try https://mcp.example/mcp/ instead, or fix the proxy settings.\ +"""), + ) + + +@pytest.mark.anyio +async def test_unfollowed_redirect_location_is_named_without_its_query_string() -> None: + """SDK-authored text: the location is reported without query or userinfo, which may carry state + that does not belong in an error message or a log line.""" + await _assert_redirected_post_fails( + "http://mcp.example/mcp", + "https://idp.example/l?state=s3cr3t&nonce=n", + snapshot( + "Redirect to https://idp.example/l not followed; use that URL as the endpoint if it is the intended server" + ), + ) + + +@pytest.mark.anyio +async def test_https_endpoint_redirected_to_plain_http_elsewhere_never_suggests_the_http_url() -> None: + """SDK-authored text: the downgrade explanation applies whatever host the http:// location names, + so the message never offers a plain-HTTP URL as the endpoint to configure.""" + await _assert_redirected_post_fails( + "https://mcp.example/mcp", + "http://backend.lan:8000/mcp/", + snapshot("""\ +Redirect to http://backend.lan:8000/mcp/ not followed: it would downgrade this HTTPS endpoint to plain HTTP. +The server is likely behind a TLS-terminating proxy whose forwarded headers it does not trust, +often combined with a trailing-slash difference. Try https://backend.lan:8000/mcp/ instead, or fix the proxy settings.\ +"""), + ) + + +@pytest.mark.anyio +async def test_get_stream_gives_up_without_retrying_when_the_endpoint_redirects_elsewhere() -> None: + """SDK-defined: the standalone GET stream is not opened through a redirect to another origin, + and since the same GET would be redirected again the transport logs it and stops instead of + spending its reconnection attempts.""" + gets: list[str] = [] + + def handler(request: httpx.Request) -> httpx.Response: + gets.append(str(request.url)) + return httpx.Response(307, headers={"location": "http://other.example/mcp"}) + + transport = StreamableHTTPTransport("http://test/mcp") + transport.session_id = "session-1" + writer, reader = anyio.create_memory_object_stream[SessionMessage | Exception](1) + with anyio.fail_after(5): + async with httpx.AsyncClient(transport=httpx.MockTransport(handler), follow_redirects=True) as http: + await transport.handle_get_stream(http, writer) + writer.close() + reader.close() + assert gets == ["http://test/mcp"] + + +@pytest.mark.anyio +async def test_resumption_redirected_elsewhere_fails_the_resumed_request() -> None: + """SDK-defined: a resumption GET answered with a redirect to another origin is not followed; + the resumed request fails with HTTPStatusError naming the location, like a redirected POST.""" + seen: list[tuple[str, str | None]] = [] + + def handler(request: httpx.Request) -> httpx.Response: + seen.append((f"{request.method} {request.url}", request.headers.get("last-event-id"))) + return httpx.Response(307, headers={"location": "http://other.example/mcp"}) + + with anyio.fail_after(5): + with pytest.raises(Exception) as exc_info: + async with ( # pragma: no branch + httpx.AsyncClient(transport=httpx.MockTransport(handler), follow_redirects=True) as http, + streamable_http_client("http://test/mcp", http_client=http) as (read_stream, write_stream, _), + read_stream, + write_stream, + ): + request = JSONRPCRequest(jsonrpc="2.0", id="resume-1", method="tools/call", params={}) + metadata = ClientMessageMetadata(resumption_token="evt-41") + await write_stream.send(SessionMessage(JSONRPCMessage(request), metadata=metadata)) + await read_stream.receive() + error = _leaf_exception(exc_info.value) + assert isinstance(error, httpx.HTTPStatusError) + assert str(error) == snapshot( + "Redirect to http://other.example/mcp not followed; use that URL as the endpoint if it is the intended server" + ) + assert seen == [("GET http://test/mcp", "evt-41")]