-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Split the registration request model from the registered-client record #3181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2179,6 +2179,27 @@ | |
|
|
||
| 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 (an echoed `""` for `token_endpoint_auth_method` or `application_type` is treated as absent, as it already was for the optional URL fields). Whether a substituted value is usable is decided where the value is used: a registered `token_endpoint_auth_method` this SDK does not recognize (anything other than `none`, `client_secret_post`, `client_secret_basic`, or `private_key_jwt`) now raises `OAuthTokenError` naming the method at the token exchange, rather than the token request being sent unauthenticated for the server to reject. | ||
|
Check warning on line 2199 in docs/migration.md
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 The migration entry says an unrecognized Extended reasoning...What the doc says vs. what the code does. The last paragraph of the new " The DCR path fails earlier, with a different exception. In the dynamic-registration flow — the primary subject of the section, per its own framing ("A registration response the server sends…") — the unusable method is caught by This is pinned by the PR's own tests. Concrete walk-through. (1) Client sends DCR request; server answers 201 with Impact and why this is a nit. A migrating user reading this entry would model the failure as an Fix. One-clause amendment naming both surfaces, e.g.: "…is reported as |
||
|
|
||
| The SDK's own registration endpoint also 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"`). | ||
|
|
||
| ### 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. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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, | ||
|
|
@@ -48,6 +48,7 @@ | |
| OAuthMetadata, | ||
| OAuthToken, | ||
| ProtectedResourceMetadata, | ||
| TokenEndpointAuthMethod, | ||
| ) | ||
| from mcp.shared.auth_utils import ( | ||
| calculate_token_expiry, | ||
|
|
@@ -58,6 +59,33 @@ | |
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| # Methods `OAuthContext.prepare_token_auth` recognizes on a registered client, derived from the | ||
| # same set the SDK is willing to request so the two cannot drift. `None`/"none" send no client | ||
| # secret; `private_key_jwt` adds no secret here (its assertion is supplied by | ||
| # `PrivateKeyJWTOAuthProvider`). Anything else is a method this client cannot apply. | ||
| _RECOGNIZED_TOKEN_ENDPOINT_AUTH_METHODS: tuple[str | None, ...] = (None, *get_args(TokenEndpointAuthMethod)) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: Registration responses that omit Prompt for AI agentsThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: A dynamically registered client echoed as Prompt for AI agents |
||
|
|
||
|
|
||
| def check_registration_usable(client_info: OAuthClientInformationFull) -> None: | ||
| """Confirm a completed registration is one this client 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". The one substitution that makes the minted credentials unusable is a | ||
| token-endpoint auth method this client cannot apply, so it is judged here - before the | ||
| record is persisted or any interactive authorization begins - rather than surfacing later | ||
| as an opaque failure at the token endpoint. | ||
|
|
||
| Raises: | ||
| OAuthRegistrationError: The server registered the client with a | ||
| `token_endpoint_auth_method` this client does not implement. | ||
| """ | ||
| if client_info.token_endpoint_auth_method not in _RECOGNIZED_TOKEN_ENDPOINT_AUTH_METHODS: | ||
| raise OAuthRegistrationError( | ||
| "Authorization server registered the client with unsupported token_endpoint_auth_method " | ||
| f"{client_info.token_endpoint_auth_method!r}" | ||
| ) | ||
|
|
||
|
|
||
| class PKCEParameters(BaseModel): | ||
| """PKCE (Proof Key for Code Exchange) parameters.""" | ||
|
|
@@ -190,6 +218,12 @@ def prepare_token_auth( | |
|
|
||
| Returns: | ||
| Tuple of (updated_data, updated_headers) | ||
|
|
||
| Raises: | ||
| OAuthTokenError: The registered client's `token_endpoint_auth_method` is one this | ||
| client does not implement. The authorization server may assign a method other | ||
| than the one requested (RFC 7591 §3.2.1); the registration record accepts it, | ||
| and the mismatch is reported here, where the method is applied. | ||
| """ | ||
| if headers is None: | ||
| headers = {} # pragma: no cover | ||
|
|
@@ -212,7 +246,10 @@ def prepare_token_auth( | |
| # 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 _RECOGNIZED_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 +701,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 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -112,28 +112,18 @@ | |
| else None | ||
| ) | ||
|
|
||
| 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, | ||
| } | ||
| ) | ||
|
Check warning on line 126 in src/mcp/server/auth/handlers/register.py
|
||
|
Comment on lines
+115
to
126
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 In the default configuration ( Extended reasoning...The bug. 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
)
Why that's off-spec. RFC 7591 §3.2.1 defines: "client_secret_expires_at: REQUIRED if client_secret is issued. Time at which the client secret will expire or 0 if it will not expire." The handler mints a Step-by-step proof. (1) An MCP server hosts the bundled AS with Why the PR's own safeguards miss it. The rewrite states the invariant that "no metadata field can be silently omitted from the echo" and adds Impact. The SDK's own client is unaffected ( Fix. One token at register.py:112: Note on provenance. This is pre-existing — v1's hand-copied constructor passed the same |
||
| try: | ||
| # Register client | ||
| await self.provider.register_client(client_info) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: DCR responses with an unsupported auth method fail immediately with
OAuthRegistrationError, before token exchange, so this migration guidance describes the wrong exception and recovery point. Document the registration-time failure;OAuthTokenErrorremains relevant only for stored records reachingprepare_token_auth.Prompt for AI agents