diff --git a/docs/client/oauth-clients.md b/docs/client/oauth-clients.md index 08ccdca1df..cd7de35626 100644 --- a/docs/client/oauth-clients.md +++ b/docs/client/oauth-clients.md @@ -131,7 +131,7 @@ There is one more no-human situation: the client belongs to an enterprise whose ## When it fails -When the OAuth flow goes wrong, the provider raises an `OAuthFlowError` from `mcp.client.auth`. It has two subclasses. `OAuthRegistrationError` means the authorization server refused to register you. `OAuthTokenError` means the token endpoint said no. One `except OAuthFlowError:` covers discovery, registration, authorization, and exchange. +When the OAuth flow goes wrong, the provider raises an `OAuthFlowError` from `mcp.client.auth`. It has two subclasses. `OAuthRegistrationError` means registration did not yield a client you can use: the authorization server refused to register you, or it did register you but with credentials this flow cannot use (for instance an authentication method it does not implement). `OAuthTokenError` means a token could not be obtained: the token endpoint said no, or a stored client record carries an authentication method this client cannot apply, which is reported while building the token request rather than sent. One `except OAuthFlowError:` covers discovery, registration, authorization, and exchange. Not everything is a flow error. The network can still fail; those are ordinary `httpx2` exceptions and pass through untouched. diff --git a/docs/migration.md b/docs/migration.md index 5eb7659c23..0f62018584 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -2179,6 +2179,27 @@ client_metadata = OAuthClientMetadata( Under OIDC, omitting `application_type` defaults to `"web"`, which an authorization server may reject for the `localhost` redirect URIs native clients use; sending `"native"` avoids that. Non-OIDC servers ignore the parameter. +### `OAuthClientInformationFull` no longer subclasses `OAuthClientMetadata`, and parses server-substituted metadata + +`OAuthClientMetadata` is the registration request a client sends; `OAuthClientInformationFull` is the authorization server's record of a registered client, parsed from its Dynamic Client Registration response. In v1 the second inherited from the first, which typed the response as though it had to be a request this SDK would send. It does not: [RFC 7591 §3.2.1](https://datatracker.ietf.org/doc/html/rfc7591#section-3.2.1) lets the server "reject or replace any of the client's requested metadata values submitted during the registration and substitute them with suitable values", and real servers return an `application_type` outside OIDC Registration's `web`/`native`, an explicit `null`, a `token_endpoint_auth_method` the SDK does not implement, or an empty `redirect_uris`. The inherited strict types turned each of those into a `ValidationError` on a 2xx response - after the server had already provisioned the client, so the registration was discarded and orphaned. + +The two are now siblings over a shared `OAuthClientMetadataBase`. `OAuthClientMetadata` keeps its strict types (the SDK still refuses to *send* an unregistered `application_type`), while `OAuthClientInformationFull` accepts what a server may echo: + +```python +# v1 +class OAuthClientInformationFull(OAuthClientMetadata): ... + +# v2 +class OAuthClientMetadata(OAuthClientMetadataBase): ... # request: strict +class OAuthClientInformationFull(OAuthClientMetadataBase): ... # server record: tolerant +``` + +On `OAuthClientInformationFull`, `application_type` and `token_endpoint_auth_method` are now `str | None`, `grant_types` is `list[str]`, and `redirect_uris` is optional (`list[AnyUrl] | None`, no minimum length). `client_id` is now required (`str`): [RFC 7591 §3.2.1](https://datatracker.ietf.org/doc/html/rfc7591#section-3.2.1) makes it mandatory in the response, and a record of a registered client without one was never meaningful. Code that only reads these fields is unaffected. Code that relied on `isinstance(client_info, OAuthClientMetadata)`, or passed an `OAuthClientInformationFull` where an `OAuthClientMetadata` is expected, must reference the record type directly. `validate_scope()` and `validate_redirect_uri()` moved with the record: they are methods of `OAuthClientInformationFull` (the type authorization-server code holds) and are no longer available on `OAuthClientMetadata`. + +A registration response the server sends is no longer rejected on these fields: a member serialized as a placeholder - an explicit `null`, or `""` - reads as an omitted key, so its default applies. Whether a substituted value is usable is judged where it matters, not at parse. When Dynamic Client Registration completes with credentials the authorization-code flow cannot use - a `token_endpoint_auth_method` other than `none`, `client_secret_post`, or `client_secret_basic` (including `private_key_jwt`, whose assertion that flow has no key to sign), or a secret-based method for which the server issued no `client_secret` - the client raises `OAuthRegistrationError` naming the problem, before the record is stored or authorization begins. Separately, a stored or pre-registered record carrying a method the SDK does not know at all raises `OAuthTokenError` when it reaches the token exchange; `private_key_jwt` on such a record does not raise there, so `PrivateKeyJWTOAuthProvider`, which signs its assertion only in the client-credentials exchange, still recovers from a rejected refresh by exchanging afresh. + +The SDK's own registration endpoint now returns all registered metadata in its 201 response (RFC 7591 §3.2.1) - including the client's `application_type`, which v1 dropped from the echo (silently reporting the default in place of a client's `"web"`), and `client_secret_expires_at` (`0` when the secret never expires) whenever a `client_secret` is issued. It also now answers a `private_key_jwt` registration with `400 invalid_client_metadata` rather than confirming a method it authenticates no requests with. + ### Stricter client authentication at `/token` and `/revoke` v2 hardens client authentication on SDK-hosted authorization servers (`create_auth_routes`) in two ways. Both apply automatically; server code only needs changing if you hand-provision client records. diff --git a/src/mcp/client/auth/oauth2.py b/src/mcp/client/auth/oauth2.py index 07e224148a..7dc62b52b9 100644 --- a/src/mcp/client/auth/oauth2.py +++ b/src/mcp/client/auth/oauth2.py @@ -11,7 +11,7 @@ import time from collections.abc import AsyncGenerator, Awaitable, Callable from dataclasses import dataclass, field -from typing import Any, Protocol +from typing import Any, Protocol, get_args from urllib.parse import quote, urlencode, urljoin, urlparse import anyio @@ -19,7 +19,7 @@ from mcp_types.version import is_version_at_least from pydantic import BaseModel, Field, ValidationError -from mcp.client.auth.exceptions import OAuthFlowError, OAuthTokenError +from mcp.client.auth.exceptions import OAuthFlowError, OAuthRegistrationError, OAuthTokenError from mcp.client.auth.utils import ( build_oauth_authorization_server_metadata_discovery_urls, build_protected_resource_metadata_discovery_urls, @@ -48,6 +48,7 @@ OAuthMetadata, OAuthToken, ProtectedResourceMetadata, + TokenEndpointAuthMethod, ) from mcp.shared.auth_utils import ( calculate_token_expiry, @@ -58,6 +59,55 @@ logger = logging.getLogger(__name__) +# Methods a registered client's record may carry without a token request being an error, +# derived from the set the SDK is willing to request so the two cannot drift. `None`/"none" +# send no client secret. `private_key_jwt` sends none from here either: only +# `PrivateKeyJWTOAuthProvider` signs the assertion, and only in its client-credentials +# exchange, so its inherited refresh path must pass through here without raising - a refresh +# the server then rejects falls back to a fresh client-credentials exchange, which signs. +# Anything else is a method no client here can apply. +_KNOWN_TOKEN_ENDPOINT_AUTH_METHODS: tuple[str | None, ...] = (None, *get_args(TokenEndpointAuthMethod)) + +# Methods that authenticate the token request with the minted `client_secret`; a +# registration assigning one is only usable if the server issued that secret. +_SECRET_TOKEN_ENDPOINT_AUTH_METHODS = ("client_secret_post", "client_secret_basic") + +# Methods a registration completed by the authorization-code flow can act on. That flow +# authenticates the token request with the minted client secret (or nothing); it holds no key +# to sign a `private_key_jwt` assertion, so a server assigning that method has registered a +# client this flow cannot use. `PrivateKeyJWTOAuthProvider` never registers dynamically. +_REGISTRATION_USABLE_TOKEN_ENDPOINT_AUTH_METHODS: tuple[str | None, ...] = tuple( + method for method in _KNOWN_TOKEN_ENDPOINT_AUTH_METHODS if method != "private_key_jwt" +) + + +def check_registration_usable(client_info: OAuthClientInformationFull) -> None: + """Confirm a registration this flow completed is one it can act on. + + RFC 7591 §3.2.1 lets the authorization server replace requested metadata and leaves it to + the client to "check the values in the response to determine if the registration is + sufficient for use". Two substitutions make the minted credentials unusable, and both are + judged here - before the record is persisted or any interactive authorization begins - + rather than surfacing later as an opaque failure at the token endpoint: a token-endpoint + auth method the authorization-code flow cannot apply (one it does not implement, or + `private_key_jwt`, whose assertion this flow has no key to sign), and a secret-based + method the flow could apply but for which the server issued no `client_secret`. + + Raises: + OAuthRegistrationError: The server registered the client with a + `token_endpoint_auth_method` this flow cannot apply, or with a secret-based + method but no `client_secret`. + """ + method = client_info.token_endpoint_auth_method + if method not in _REGISTRATION_USABLE_TOKEN_ENDPOINT_AUTH_METHODS: + raise OAuthRegistrationError( + f"Authorization server registered the client with unsupported token_endpoint_auth_method {method!r}" + ) + if method in _SECRET_TOKEN_ENDPOINT_AUTH_METHODS and client_info.client_secret is None: + raise OAuthRegistrationError( + f"Authorization server registered the client for {method!r} but issued no client_secret" + ) + class PKCEParameters(BaseModel): """PKCE (Proof Key for Code Exchange) parameters.""" @@ -190,6 +240,12 @@ def prepare_token_auth( Returns: Tuple of (updated_data, updated_headers) + + Raises: + OAuthTokenError: The client record carries a `token_endpoint_auth_method` this + client does not know. A dynamic registration assigning an unusable method is + rejected earlier, by `check_registration_usable`; this fires for a stored or + pre-registered record that reaches a token request with such a method. """ if headers is None: headers = {} # pragma: no cover @@ -199,7 +255,7 @@ def prepare_token_auth( auth_method = self.client_info.token_endpoint_auth_method - if auth_method == "client_secret_basic" and self.client_info.client_id and self.client_info.client_secret: + if auth_method == "client_secret_basic" and self.client_info.client_secret: # URL-encode client ID and secret per RFC 6749 Section 2.3.1 encoded_id = quote(self.client_info.client_id, safe="") encoded_secret = quote(self.client_info.client_secret, safe="") @@ -208,11 +264,14 @@ def prepare_token_auth( headers["Authorization"] = f"Basic {encoded_credentials}" # Don't include client_secret in body for basic auth data = {k: v for k, v in data.items() if k != "client_secret"} - elif auth_method == "client_secret_post" and self.client_info.client_id and self.client_info.client_secret: + elif auth_method == "client_secret_post" and self.client_info.client_secret: # Include client_id and client_secret in request body (RFC 6749 §2.3.1) data["client_id"] = self.client_info.client_id data["client_secret"] = self.client_info.client_secret - # For auth_method == "none", don't add any client_secret + elif auth_method not in _KNOWN_TOKEN_ENDPOINT_AUTH_METHODS: + raise OAuthTokenError(f"Registered client uses unsupported token_endpoint_auth_method {auth_method!r}") + # For "none" (or absent), don't add any client_secret; "private_key_jwt" adds its + # assertion in the provider that implements it, not here. return data, headers @@ -664,6 +723,7 @@ async def async_auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx ) registration_response = yield registration_request client_information = await handle_registration_response(registration_response) + check_registration_usable(client_information) # 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 diff --git a/src/mcp/client/auth/utils.py b/src/mcp/client/auth/utils.py index fc87c9e469..31e2e5cade 100644 --- a/src/mcp/client/auth/utils.py +++ b/src/mcp/client/auth/utils.py @@ -1,9 +1,11 @@ import re +from typing import Any, cast from urllib.parse import urljoin, urlparse from httpx2 import Request, Response from mcp_types import LATEST_PROTOCOL_VERSION from pydantic import AnyUrl, ValidationError +from pydantic_core import from_json from mcp.client.auth import OAuthFlowError, OAuthRegistrationError, OAuthTokenError from mcp.shared.auth import ( @@ -299,10 +301,17 @@ async def handle_registration_response(response: Response) -> OAuthClientInforma try: content = await response.aread() - client_info = OAuthClientInformationFull.model_validate_json(content) - return 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: @@ -381,8 +390,8 @@ def create_client_info_from_metadata_url( Args: client_metadata_url: The URL to use as the client_id - redirect_uris: The redirect URIs from the client metadata (passed through for - compatibility with OAuthClientInformationFull which inherits from OAuthClientMetadata) + redirect_uris: The redirect URIs from the client metadata, recorded on the client + information alongside the client_id Returns: OAuthClientInformationFull with the URL as client_id diff --git a/src/mcp/server/auth/handlers/register.py b/src/mcp/server/auth/handlers/register.py index e565b27383..7fb14b2c43 100644 --- a/src/mcp/server/auth/handlers/register.py +++ b/src/mcp/server/auth/handlers/register.py @@ -50,6 +50,18 @@ async def handle(self, request: Request) -> Response: # If auth method is None, default to client_secret_post if client_metadata.token_endpoint_auth_method is None: client_metadata.token_endpoint_auth_method = "client_secret_post" + # This server authenticates token requests with the client secret it mints; it holds + # no client key to verify a private_key_jwt assertion, so confirming that method would + # register a client whose every token request is then rejected. Refuse it instead + # (RFC 7591 §3.2.2), before minting credentials the client could never use. + if client_metadata.token_endpoint_auth_method == "private_key_jwt": + return PydanticJSONResponse( + content=RegistrationErrorResponse( + error="invalid_client_metadata", + error_description="token_endpoint_auth_method 'private_key_jwt' is not supported", + ), + status_code=400, + ) client_secret = None if client_metadata.token_endpoint_auth_method != "none": # pragma: no branch @@ -106,33 +118,27 @@ async def handle(self, request: Request) -> Response: ) client_id_issued_at = int(time.time()) - client_secret_expires_at = ( - client_id_issued_at + self.options.client_secret_expiry_seconds - if self.options.client_secret_expiry_seconds is not None - else None - ) + # RFC 7591 §3.2.1: client_secret_expires_at is REQUIRED whenever a client_secret is + # issued, with 0 (not omission) meaning it never expires; a public client gets none. + client_secret_expires_at = None + if client_secret is not None: + client_secret_expires_at = ( + client_id_issued_at + self.options.client_secret_expiry_seconds + if self.options.client_secret_expiry_seconds is not None + else 0 + ) - client_info = OAuthClientInformationFull( - client_id=client_id, - client_id_issued_at=client_id_issued_at, - client_secret=client_secret, - client_secret_expires_at=client_secret_expires_at, - # passthrough information from the client request - redirect_uris=client_metadata.redirect_uris, - token_endpoint_auth_method=client_metadata.token_endpoint_auth_method, - grant_types=client_metadata.grant_types, - response_types=client_metadata.response_types, - client_name=client_metadata.client_name, - client_uri=client_metadata.client_uri, - logo_uri=client_metadata.logo_uri, - scope=client_metadata.scope, - contacts=client_metadata.contacts, - tos_uri=client_metadata.tos_uri, - policy_uri=client_metadata.policy_uri, - jwks_uri=client_metadata.jwks_uri, - jwks=client_metadata.jwks, - software_id=client_metadata.software_id, - software_version=client_metadata.software_version, + # RFC 7591 §3.2.1: the response returns all registered metadata about the client, so + # the record is the whole validated request plus the credentials minted here - built + # from the request's dump so no metadata field can be silently omitted from the echo. + client_info = OAuthClientInformationFull.model_validate( + { + **client_metadata.model_dump(), + "client_id": client_id, + "client_id_issued_at": client_id_issued_at, + "client_secret": client_secret, + "client_secret_expires_at": client_secret_expires_at, + } ) try: # Register client diff --git a/src/mcp/shared/auth.py b/src/mcp/shared/auth.py index 2bbf7a715a..881379d381 100644 --- a/src/mcp/shared/auth.py +++ b/src/mcp/shared/auth.py @@ -1,10 +1,27 @@ -from typing import Any, Literal +from typing import Any, Literal, cast -from pydantic import AnyHttpUrl, AnyUrl, BaseModel, ConfigDict, Field, field_validator +from pydantic import AnyHttpUrl, AnyUrl, BaseModel, ConfigDict, Field, field_validator, model_validator # RFC 7523 JWT bearer grant; SEP-990 leg 2 uses this to present the ID-JAG. JWT_BEARER_GRANT_TYPE = "urn:ietf:params:oauth:grant-type:jwt-bearer" +# Token-endpoint client authentication methods this SDK's clients request, and the set +# `OAuthContext.prepare_token_auth` recognizes on a registered client (`private_key_jwt` is +# applied by `PrivateKeyJWTOAuthProvider`; the rest send a client secret or nothing). +TokenEndpointAuthMethod = Literal["none", "client_secret_post", "client_secret_basic", "private_key_jwt"] + +# grant_types a client requests when it does not specify its own (RFC 7591 §2). +DEFAULT_GRANT_TYPES = ["authorization_code", "refresh_token"] + + +def _empty_str_to_none(v: object) -> object: + # RFC 7591 §2 marks these URL fields OPTIONAL; a "" placeholder means absent, so it + # must not fail AnyHttpUrl validation. (The registered-client record applies the same + # rule to every member; this coercion serves the request model.) + if v == "": + return None + return v + class OAuthToken(BaseModel): """See https://datatracker.ietf.org/doc/html/rfc6749#section-5.1""" @@ -47,32 +64,21 @@ def __init__(self, message: str): self.message = message -class OAuthClientMetadata(BaseModel): - """RFC 7591 OAuth 2.0 Dynamic Client Registration Metadata. +class OAuthClientMetadataBase(BaseModel): + """RFC 7591 OAuth 2.0 Dynamic Client Registration metadata shared verbatim by the + registration request (`OAuthClientMetadata`) and the authorization server's record of a + registered client (`OAuthClientInformationFull`). Fields whose acceptable values differ + between the two - what this SDK sends versus what a third-party server may echo - are + declared on each model rather than here. See https://datatracker.ietf.org/doc/html/rfc7591#section-2 """ model_config = ConfigDict(url_preserve_empty_path=True) - redirect_uris: list[AnyUrl] | None = Field(..., min_length=1) - # supported auth methods for the token endpoint - token_endpoint_auth_method: ( - Literal["none", "client_secret_post", "client_secret_basic", "private_key_jwt"] | None - ) = None - # supported grant_types of this implementation - grant_types: list[ - Literal["authorization_code", "refresh_token", "urn:ietf:params:oauth:grant-type:jwt-bearer"] | str - ] = [ - "authorization_code", - "refresh_token", - ] # The MCP spec requires the "code" response type, but OAuth # servers may also return additional types they support response_types: list[str] = ["code"] scope: str | None = None - # SEP-837: OIDC application_type. Defaults to "native" since MCP clients typically use - # loopback redirect URIs; set "web" for remote browser-based clients on a non-local host. - application_type: Literal["web", "native"] = "native" # these fields are currently unused, but we support & store them for potential # future use @@ -97,13 +103,76 @@ class OAuthClientMetadata(BaseModel): ) @classmethod def _empty_string_optional_url_to_none(cls, v: object) -> object: - # RFC 7591 §2 marks these URL fields OPTIONAL. Some authorization servers - # echo omitted metadata back as "" instead of dropping the keys, which - # AnyHttpUrl would otherwise reject — throwing away an otherwise valid - # registration response. Treat "" as absent. - if v == "": - return None - return v + # These URL fields are OPTIONAL; an echoed "" would otherwise fail AnyHttpUrl + # and throw away an otherwise valid registration response. + return _empty_str_to_none(v) + + +class OAuthClientMetadata(OAuthClientMetadataBase): + """RFC 7591 OAuth 2.0 Dynamic Client Registration request metadata: what an MCP + client sends when it registers. Field values are narrowed to what this SDK will put + on the wire; parsing the authorization server's response is `OAuthClientInformationFull`'s + job. See https://datatracker.ietf.org/doc/html/rfc7591#section-2 + """ + + redirect_uris: list[AnyUrl] | None = Field(..., min_length=1) + # supported auth methods for the token endpoint + token_endpoint_auth_method: TokenEndpointAuthMethod | None = None + # supported grant_types of this implementation + grant_types: list[ + Literal["authorization_code", "refresh_token", "urn:ietf:params:oauth:grant-type:jwt-bearer"] | str + ] = list(DEFAULT_GRANT_TYPES) + # SEP-837: OIDC application_type. Defaults to "native" since MCP clients typically use + # loopback redirect URIs; set "web" for remote browser-based clients on a non-local host. + application_type: Literal["web", "native"] = "native" + + +class OAuthClientInformationFull(OAuthClientMetadataBase): + """RFC 7591 OAuth 2.0 Dynamic Client Registration client information response + (client information plus metadata) - the authorization server's record of a + registered client. See https://datatracker.ietf.org/doc/html/rfc7591#section-3.2.1 + + A third-party authorization server "MAY reject or replace any of the client's + requested metadata values submitted during the registration and substitute them with + suitable values", so `application_type`, `token_endpoint_auth_method`, and `grant_types` + are typed to accept any string the server echoes, and `redirect_uris` may be absent or + empty. A member the server serializes as a placeholder - an explicit `null`, or `""` - + is read as an omitted key, so the field's default applies rather than the parse failing. + Whether a substituted value is usable is decided where the value is used, not at parse. + `redirect_uris` elements are still parsed as URLs, as the authorization server compares + them against a client's requested `redirect_uri`. + """ + + redirect_uris: list[AnyUrl] | None = None + # RFC 7591 §3.2.1: the server may assign an auth method other than the one requested, + # including methods this SDK does not implement, or omit it. + token_endpoint_auth_method: str | None = None + grant_types: list[str] = list(DEFAULT_GRANT_TYPES) + # SEP-837: OIDC application_type. OIDC Registration §2 defines "web" and "native", but + # servers echo other strings or an explicit null; the value is informational here. + application_type: str | None = None + + # RFC 7591 §3.2.1: client_id is REQUIRED in a client information response - a body + # without one is not a registration, whatever else it echoes. + client_id: str + 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 + + @model_validator(mode="before") + @classmethod + def _placeholder_members_read_as_omitted(cls, data: object) -> object: + # Servers dump unset members of their client record as null, or echo them as "", + # instead of omitting the keys. Either placeholder would otherwise fail the parse of a + # list field (or read "" as an unrecognized method) and discard an already-provisioned + # registration; a placeholder and an absent key mean the same thing. + if isinstance(data, dict): + members = cast(dict[str, Any], data) + return {key: value for key, value in members.items() if value is not None and value != ""} + return data def validate_scope(self, requested_scope: str | None) -> list[str] | None: if requested_scope is None: @@ -118,27 +187,15 @@ def validate_scope(self, requested_scope: str | None) -> list[str] | None: def validate_redirect_uri(self, redirect_uri: AnyUrl | None) -> AnyUrl: if redirect_uri is not None: # Validate redirect_uri against client's registered redirect URIs - if self.redirect_uris is None or redirect_uri not in self.redirect_uris: + if not self.redirect_uris or redirect_uri not in self.redirect_uris: raise InvalidRedirectUriError(f"Redirect URI '{redirect_uri}' not registered for client") return redirect_uri - elif self.redirect_uris is not None and len(self.redirect_uris) == 1: + elif self.redirect_uris and len(self.redirect_uris) == 1: return self.redirect_uris[0] else: - raise InvalidRedirectUriError("redirect_uri must be specified when client has multiple registered URIs") - - -class OAuthClientInformationFull(OAuthClientMetadata): - """RFC 7591 OAuth 2.0 Dynamic Client Registration full response - (client information plus metadata). - """ - - client_id: str | None = None - 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 + raise InvalidRedirectUriError( + "redirect_uri must be specified unless the client has exactly one registered URI" + ) class OAuthMetadata(BaseModel): diff --git a/tests/client/auth/extensions/test_client_credentials.py b/tests/client/auth/extensions/test_client_credentials.py index cb8fd5e2b6..16336f8002 100644 --- a/tests/client/auth/extensions/test_client_credentials.py +++ b/tests/client/auth/extensions/test_client_credentials.py @@ -149,44 +149,6 @@ async def test_exchange_token_client_secret_post_includes_client_id(self, mock_s # Should NOT have Basic auth header assert "Authorization" not in request.headers - @pytest.mark.anyio - async def test_exchange_token_client_secret_post_without_client_id(self, mock_storage: MockTokenStorage): - """Test client_secret_post skips body credentials when client_id is None.""" - provider = ClientCredentialsOAuthProvider( - server_url="https://api.example.com/v1/mcp", - storage=mock_storage, - client_id="placeholder", - client_secret="test-client-secret", - token_endpoint_auth_method="client_secret_post", - scope="read write", - ) - await provider._initialize() - provider.context.oauth_metadata = OAuthMetadata( - issuer=AnyHttpUrl("https://api.example.com"), - authorization_endpoint=AnyHttpUrl("https://api.example.com/authorize"), - token_endpoint=AnyHttpUrl("https://api.example.com/token"), - ) - provider.context.protocol_version = "2025-06-18" - # Override client_info to have client_id=None (edge case) - provider.context.client_info = OAuthClientInformationFull( - redirect_uris=None, - client_id=None, - client_secret="test-client-secret", - grant_types=["client_credentials"], - token_endpoint_auth_method="client_secret_post", - scope="read write", - ) - - request = await provider._perform_authorization() - - content = urllib.parse.unquote_plus(request.content.decode()) - assert "grant_type=client_credentials" in content - # Neither client_id nor client_secret should be in body since client_id is None - # (RFC 6749 §2.3.1 requires both for client_secret_post) - assert "client_id=" not in content - assert "client_secret=" not in content - assert "Authorization" not in request.headers - @pytest.mark.anyio async def test_exchange_token_without_scopes(self, mock_storage: MockTokenStorage): """Test token exchange without scopes.""" diff --git a/tests/client/test_auth.py b/tests/client/test_auth.py index 6bea7bcf70..be96cc8eec 100644 --- a/tests/client/test_auth.py +++ b/tests/client/test_auth.py @@ -12,7 +12,7 @@ from pydantic import AnyHttpUrl, AnyUrl from mcp.client.auth import OAuthClientProvider, PKCEParameters -from mcp.client.auth.exceptions import OAuthFlowError, OAuthTokenError +from mcp.client.auth.exceptions import OAuthFlowError, OAuthRegistrationError, OAuthTokenError from mcp.client.auth.utils import ( build_oauth_authorization_server_metadata_discovery_urls, build_protected_resource_metadata_discovery_urls, @@ -1008,6 +1008,91 @@ def text(self): assert "Registration failed: 400" in str(exc_info.value) +@pytest.mark.anyio +async def test_registration_response_with_substituted_metadata_yields_the_credentials(): + """A 201 whose echoed metadata differs from the request still registers the client. + + The authorization server returned an application_type outside OIDC Registration's set, + a null redirect_uris, and an auth method the SDK does not implement. RFC 7591 §3.2.1 + permits the server to substitute values; the client keeps the credentials it minted. + """ + body = ( + b'{"client_id": "issued-id", "client_secret": "issued-secret", ' + b'"application_type": "confidential", "redirect_uris": null, ' + b'"token_endpoint_auth_method": "client_secret_jwt"}' + ) + response = httpx2.Response(201, content=body) + + client_info = await handle_registration_response(response) + + assert client_info.client_id == "issued-id" + assert client_info.client_secret == "issued-secret" + assert client_info.application_type == "confidential" + + +@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", "issuer": echoed_issuer}).encode() + + client_info = await handle_registration_response(httpx2.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é"}'.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.""" + response = httpx2.Response(201, content=content) + + with pytest.raises(OAuthRegistrationError): + await handle_registration_response(response) + + +@pytest.mark.anyio +async def test_token_exchange_reports_an_unimplemented_registered_auth_method(oauth_provider: OAuthClientProvider): + """A server-assigned auth method the SDK cannot apply (RFC 7591 §3.2.1 lets the server + substitute one) is reported at the token exchange rather than sending the request + unauthenticated for the server to reject as invalid_client.""" + oauth_provider.context.client_info = OAuthClientInformationFull( + client_id="registered-id", + client_secret="registered-secret", + token_endpoint_auth_method="client_secret_jwt", + ) + + with pytest.raises(OAuthTokenError): + await oauth_provider._exchange_token_authorization_code("test_auth_code", "test_verifier") + + +def test_prepare_token_auth_leaves_a_private_key_jwt_client_to_its_provider(oauth_provider: OAuthClientProvider): + """private_key_jwt is recognized, so the base leaves the request untouched rather than + raising - PrivateKeyJWTOAuthProvider's inherited refresh path passes through here, and a + refresh the server then rejects (no assertion is signed on it) falls back to a fresh, + signed client-credentials exchange instead of aborting the flow.""" + oauth_provider.context.client_info = OAuthClientInformationFull( + client_id="registered-id", + client_secret="registered-secret", + token_endpoint_auth_method="private_key_jwt", + ) + + data, headers = oauth_provider.context.prepare_token_auth({"grant_type": "refresh_token"}, {}) + + assert data == {"grant_type": "refresh_token"} + assert headers == {} + + class TestCreateClientRegistrationRequest: """Test client registration request creation.""" diff --git a/tests/interaction/_requirements.py b/tests/interaction/_requirements.py index 17bc3a4b5a..fbc7c13846 100644 --- a/tests/interaction/_requirements.py +++ b/tests/interaction/_requirements.py @@ -2940,6 +2940,17 @@ def __post_init__(self) -> None: transports=("streamable-http",), note="Auth is enforced at the HTTP layer; Cache-Control is an HTTP header.", ), + "hosting:auth:as:register-echo": Requirement( + source="sdk", + behavior=( + "The bundled registration endpoint returns all registered metadata about the client " + "in its 201 response (RFC 7591 §3.2.1) - the client's `application_type` rather than a " + "substituted default, and `client_secret_expires_at` (0 when the secret never expires) " + "whenever a `client_secret` is issued." + ), + transports=("streamable-http",), + note="Auth is enforced at the HTTP layer; the bundled AS is an ASGI app.", + ), "hosting:auth:as:register-error-response": Requirement( source="sdk", behavior=( @@ -3660,6 +3671,19 @@ def __post_init__(self) -> None: transports=("streamable-http",), note="OAuth is HTTP-only.", ), + "client-auth:dcr:substituted-metadata": Requirement( + source="sdk", + behavior=( + "A 201 registration response whose echoed metadata the server substituted (RFC 7591 §3.2.1) - " + "an unregistered application_type, null redirect_uris, extra grant types - completes the flow; " + "substituted credentials the authorization-code flow cannot apply (an unimplemented " + "token_endpoint_auth_method; private_key_jwt, whose assertion it has no key to sign; or a " + "secret-based method with no client_secret issued) are instead reported as an " + "OAuthRegistrationError before the record is persisted or authorization begins." + ), + transports=("streamable-http",), + note="OAuth is HTTP-only.", + ), "client-auth:dcr": Requirement( source=f"{SPEC_BASE_URL}/basic/authorization#dynamic-client-registration", behavior=( diff --git a/tests/interaction/auth/test_as_handlers.py b/tests/interaction/auth/test_as_handlers.py index f59478b49a..1876cd7181 100644 --- a/tests/interaction/auth/test_as_handlers.py +++ b/tests/interaction/auth/test_as_handlers.py @@ -239,10 +239,42 @@ async def test_registration_with_invalid_metadata_is_rejected_with_400( bad_scope = await http.post("/register", json=body | {"scope": "forbidden"}) assert bad_scope.status_code == 400 - body = bad_scope.json() - assert body["error"] == "invalid_client_metadata" + bad_scope_body = bad_scope.json() + assert bad_scope_body["error"] == "invalid_client_metadata" # The description embeds a set difference whose ordering is not stable, so assert the prefix. - assert body["error_description"].startswith("Requested scopes are not valid: ") + assert bad_scope_body["error_description"].startswith("Requested scopes are not valid: ") + + # The server holds no client key to verify a private_key_jwt assertion, so it refuses to + # confirm a registration whose every token request it would then reject (RFC 7591 §3.2.2). + unsignable = await http.post("/register", json=body | {"token_endpoint_auth_method": "private_key_jwt"}) + assert unsignable.status_code == 400 + assert unsignable.json() == snapshot( + { + "error": "invalid_client_metadata", + "error_description": "token_endpoint_auth_method 'private_key_jwt' is not supported", + } + ) + + +@requirement("hosting:auth:as:register-echo") +@pytest.mark.parametrize("application_type", ["web", "native"]) +async def test_registration_response_echoes_the_registered_application_type( + as_app: tuple[httpx2.AsyncClient, InMemoryAuthorizationServerProvider], + application_type: str, +) -> None: + """The 201 body reflects the application_type the client registered (RFC 7591 §3.2.1).""" + http, _ = as_app + body = oauth_client_metadata().model_dump(mode="json", exclude_none=True) + + response = await http.post("/register", json=body | {"application_type": application_type}) + + assert response.status_code == 201 + echoed = response.json() + assert echoed["application_type"] == application_type + # A secret was issued and no expiry is configured, so RFC 7591 §3.2.1 requires the + # response to carry client_secret_expires_at, with 0 (present, not omitted) for "never". + assert echoed["client_secret"] + assert echoed["client_secret_expires_at"] == 0 @requirement("hosting:auth:as:redirect-uri-binding") diff --git a/tests/interaction/auth/test_discovery.py b/tests/interaction/auth/test_discovery.py index dc9f3794af..28685bf8cd 100644 --- a/tests/interaction/auth/test_discovery.py +++ b/tests/interaction/auth/test_discovery.py @@ -17,14 +17,16 @@ import pytest from inline_snapshot import snapshot from mcp_types import ListToolsResult, Tool -from pydantic import AnyHttpUrl +from pydantic import AnyHttpUrl, AnyUrl from mcp.client.auth import OAuthFlowError, OAuthRegistrationError from mcp.server import Server, ServerRequestContext -from mcp.shared.auth import OAuthMetadata, ProtectedResourceMetadata +from mcp.shared.auth import OAuthClientInformationFull, OAuthMetadata, ProtectedResourceMetadata from tests.interaction._connect import BASE_URL, mounted_app from tests.interaction._requirements import requirement from tests.interaction.auth._harness import ( + REDIRECT_URI, + InMemoryTokenStorage, RecordedRequest, auth_settings, connect_with_oauth, @@ -174,6 +176,91 @@ async def test_a_400_from_the_registration_endpoint_surfaces_as_a_registration_e assert [r.path for r in recorded if r.path in ("/authorize", "/token")] == [] +@requirement("client-auth:dcr:substituted-metadata") +async def test_a_registration_response_with_substituted_metadata_completes_the_flow() -> None: + """A 201 whose echoed metadata differs from the request still yields a working client. + + The shim replaces the real `/register` with a body that echoes an `application_type` + outside OIDC Registration's set, a null `redirect_uris`, and an extra grant type - a + substitution RFC 7591 §3.2.1 permits. The registration proceeds and, after `/register` + stopped answering, the client authorizes and exchanges a token as normal. + """ + recorded, on_request = record_requests() + provider = InMemoryAuthorizationServerProvider() + server = Server("guarded", on_list_tools=list_tools) + client_id = "substituted-client" + provider.clients[client_id] = OAuthClientInformationFull( + client_id=client_id, + client_secret="s3cr3t", + redirect_uris=[AnyUrl(REDIRECT_URI)], + token_endpoint_auth_method="client_secret_post", + scope="mcp", + ) + body = json.dumps( + { + "client_id": client_id, + "client_secret": "s3cr3t", + "token_endpoint_auth_method": "client_secret_post", + "application_type": "confidential", + "redirect_uris": None, + "grant_types": ["authorization_code", "refresh_token", "client_credentials"], + } + ).encode() + app_shim = shim(serve={"/register": (201, body)}) + storage = InMemoryTokenStorage() + + with anyio.fail_after(5): + async with connect_with_oauth( + server, provider=provider, storage=storage, app_shim=app_shim, on_request=on_request + ) as (client, _): + result = await client.list_tools() + + assert result.tools[0].name == snapshot("probe") + assert storage.client_info is not None + assert storage.client_info.client_id == client_id + assert storage.client_info.application_type == "confidential" + assert [r.path for r in recorded].index("/register") < [r.path for r in recorded].index("/token") + + +@requirement("client-auth:dcr:substituted-metadata") +@pytest.mark.parametrize( + "credentials", + [ + pytest.param( + {"client_secret": "s3cr3t", "token_endpoint_auth_method": "client_secret_jwt"}, id="unimplemented" + ), + pytest.param({"client_secret": "s3cr3t", "token_endpoint_auth_method": "private_key_jwt"}, id="unsignable"), + pytest.param({"token_endpoint_auth_method": "client_secret_post"}, id="secret-not-issued"), + ], +) +async def test_a_registration_assigning_unusable_credentials_surfaces_as_a_registration_error( + credentials: dict[str, str], +) -> None: + """A 201 assigning credentials this flow cannot apply is a registration error. + + RFC 7591 §3.2.1 leaves it to the client to judge whether a substituted value makes the + registration usable. The authorization-code flow authenticates with the minted secret, so + an unimplemented method, `private_key_jwt` (an assertion it has no key to sign), and a + secret-based method with no secret issued are all unusable; each is reported before + the record is stored or any authorize/token request is made. + """ + recorded, on_request = record_requests() + provider = InMemoryAuthorizationServerProvider() + server = Server("guarded", on_list_tools=list_tools) + body = json.dumps({"client_id": "unusable", **credentials}).encode() + app_shim = shim(serve={"/register": (201, body)}) + storage = InMemoryTokenStorage() + + with anyio.fail_after(5): + with pytest.RaisesGroup(pytest.RaisesExc(OAuthRegistrationError), flatten_subgroups=True): + await connect_with_oauth( + server, provider=provider, storage=storage, app_shim=app_shim, on_request=on_request + ).__aenter__() + + assert storage.client_info is None + assert [r.path for r in recorded if r.path in ("/authorize", "/token")] == [] + + @requirement("client-auth:prm-resource-mismatch") async def test_prm_with_a_mismatched_resource_aborts_the_flow_before_authorize() -> None: """A PRM document whose `resource` does not cover the server URL aborts the flow. diff --git a/tests/shared/test_auth.py b/tests/shared/test_auth.py index 7463bc5a8a..5286b93834 100644 --- a/tests/shared/test_auth.py +++ b/tests/shared/test_auth.py @@ -1,9 +1,9 @@ """Tests for OAuth 2.0 shared code.""" import pytest -from pydantic import ValidationError +from pydantic import AnyUrl, ValidationError -from mcp.shared.auth import OAuthClientInformationFull, OAuthClientMetadata, OAuthMetadata +from mcp.shared.auth import InvalidRedirectUriError, OAuthClientInformationFull, OAuthClientMetadata, OAuthMetadata def test_oauth(): @@ -110,8 +110,8 @@ def test_valid_url_passes_through_unchanged(): def test_information_full_inherits_coercion(): - """OAuthClientInformationFull subclasses OAuthClientMetadata, so the - same coercion applies to DCR responses parsed via the full model.""" + """OAuthClientInformationFull shares the metadata base, so the same + coercion applies to DCR responses parsed via the full model.""" data = { "client_id": "abc123", "redirect_uris": ["https://example.com/callback"], @@ -130,6 +130,107 @@ def test_information_full_inherits_coercion(): assert info.jwks_uri is None +# RFC 7591 §3.2.1 lets the authorization server reject or replace any requested metadata +# value in its registration response. Real servers echo values outside the sets the client +# would send (an unregistered application_type, an explicit null, an auth method the SDK +# does not implement, an empty redirect_uris array); a parse failure there discards a +# registration whose client_id the server has already provisioned. + + +@pytest.mark.parametrize( + "substituted", + [ + pytest.param({"application_type": "confidential"}, id="unregistered-application-type"), + pytest.param({"application_type": ""}, id="empty-application-type"), + pytest.param({"application_type": None}, id="null-application-type"), + pytest.param({"token_endpoint_auth_method": "client_secret_jwt"}, id="unimplemented-auth-method"), + pytest.param({"grant_types": ["authorization_code", "client_credentials"]}, id="extra-grant-type"), + pytest.param({"redirect_uris": []}, id="empty-redirect-uris"), + ], +) +def test_client_information_accepts_server_substituted_metadata(substituted: dict[str, object]): + data = {"client_id": "abc123", "client_secret": "s3cr3t", **substituted} + info = OAuthClientInformationFull.model_validate(data) + assert info.client_id == "abc123" + assert info.client_secret == "s3cr3t" + + +def test_client_information_without_echoed_metadata_still_parses(): + """A response holding only the credentials the server minted is a usable registration.""" + info = OAuthClientInformationFull.model_validate({"client_id": "abc123"}) + assert info.client_id == "abc123" + assert info.redirect_uris is None + assert info.application_type is None + + +def test_every_request_metadata_field_exists_on_the_client_record(): + """The registration handler builds its 201 echo from the request's dump; every request + field must exist on the record so none can be silently dropped from the response.""" + assert set(OAuthClientMetadata.model_fields) <= set(OAuthClientInformationFull.model_fields) + + +def test_a_registration_response_without_a_client_id_is_rejected(): + """RFC 7591 §3.2.1 makes client_id REQUIRED; a body without one is not a registration, + however permissive the parse is about the metadata around it.""" + with pytest.raises(ValidationError): + OAuthClientInformationFull.model_validate({"application_type": "web"}) + + +@pytest.mark.parametrize("placeholder", [None, ""], ids=["null", "empty-string"]) +@pytest.mark.parametrize( + "member", + ["grant_types", "response_types", "redirect_uris", "application_type", "token_endpoint_auth_method", "scope"], +) +def test_client_information_reads_a_placeholder_member_as_an_omitted_key(member: str, placeholder: object): + """A server that dumps unset members as null, or echoes them as "", still yields a + usable registration: a placeholder and an absent key mean the same, so the field's + default applies - including for list fields, where the placeholder is not a valid list.""" + info = OAuthClientInformationFull.model_validate({"client_id": "abc123", member: placeholder}) + defaults = OAuthClientInformationFull.model_validate({"client_id": "abc123"}) + assert getattr(info, member) == getattr(defaults, member) + + +def test_a_placeholder_client_id_is_a_missing_client_id(): + """The placeholder rule applies to the credential too: an empty client_id is no client_id, + so the body is rejected rather than parsing as a registration with an empty identifier.""" + with pytest.raises(ValidationError): + OAuthClientInformationFull.model_validate({"client_id": ""}) + + +def test_client_information_that_is_not_an_object_still_fails_the_parse(): + """The null-as-omitted coercion only touches JSON objects; a body that is not one is + passed through and rejected as a normal validation failure rather than swallowed.""" + with pytest.raises(ValidationError): + OAuthClientInformationFull.model_validate("not-an-object") + + +@pytest.mark.parametrize("redirect_uris", [None, []], ids=["absent", "empty"]) +@pytest.mark.parametrize( + "redirect_uri", [None, AnyUrl("https://example.com/callback")], ids=["unspecified", "specified"] +) +def test_client_with_no_registered_redirect_uris_cannot_resolve_a_redirect( + redirect_uris: list[str] | None, redirect_uri: AnyUrl | None +): + """With no registered redirect URIs (absent or empty), no redirect resolves - neither a + supplied one (nothing to match against) nor an unspecified one (no single default).""" + info = OAuthClientInformationFull.model_validate({"client_id": "abc123", "redirect_uris": redirect_uris}) + with pytest.raises(InvalidRedirectUriError): + info.validate_redirect_uri(redirect_uri) + + +def test_request_metadata_restricts_application_type_to_the_values_the_sdk_sends(): + """What the SDK sends stays narrow even though what it accepts back is wide.""" + with pytest.raises(ValidationError): + OAuthClientMetadata.model_validate( + {"redirect_uris": ["https://example.com/callback"], "application_type": "confidential"} + ) + + +def test_request_metadata_requires_at_least_one_redirect_uri(): + with pytest.raises(ValidationError): + OAuthClientMetadata.model_validate({"redirect_uris": []}) + + def test_invalid_non_empty_url_still_rejected(): """Coercion must only touch empty strings — garbage URLs still raise.""" data = {