Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/client/oauth-clients.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Stored private_key_jwt records and secret-based records missing client_secret are not rejected while building the token request; they fall through and yield an unauthenticated request. Narrow this to unrecognized methods, or document the other cases separately.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/client/oauth-clients.md, line 134:

<comment>Stored `private_key_jwt` records and secret-based records missing `client_secret` are not rejected while building the token request; they fall through and yield an unauthenticated request. Narrow this to unrecognized methods, or document the other cases separately.</comment>

<file context>
@@ -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 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 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.
</file context>
Suggested change
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.
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 unrecognized authentication method, 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.

Expand Down
21 changes: 21 additions & 0 deletions docs/migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
70 changes: 65 additions & 5 deletions src/mcp/client/auth/oauth2.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@
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
import httpx2
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,
Expand Down Expand Up @@ -48,6 +48,7 @@
OAuthMetadata,
OAuthToken,
ProtectedResourceMetadata,
TokenEndpointAuthMethod,
)
from mcp.shared.auth_utils import (
calculate_token_expiry,
Expand All @@ -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"
)
Comment thread
maxisbey marked this conversation as resolved.


Comment thread
claude[bot] marked this conversation as resolved.
class PKCEParameters(BaseModel):
"""PKCE (Proof Key for Code Exchange) parameters."""
Expand Down Expand Up @@ -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
Expand All @@ -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="")
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down
21 changes: 15 additions & 6 deletions src/mcp/client/auth/utils.py
Original file line number Diff line number Diff line change
@@ -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 (
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
58 changes: 32 additions & 26 deletions src/mcp/server/auth/handlers/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
}
)
Comment thread
maxisbey marked this conversation as resolved.
try:
# Register client
Expand Down
Loading
Loading