Split the registration request model from the registered-client record

Merged
MA
3mo ago
+573-129

OAuthClientInformationFull — the client's parse of the authorization server's Dynamic Client Registration response — inherited from OAuthClientMetadata, the request the client sends. That typed the response as though it had to be a request this SDK would send, so any registered value the server substituted became a ValidationError on a 2xx. This splits the two into siblings so the record parses what a server may legitimately echo.

Motivation and Context

We received a report of an authorization server whose registration response returned an application_type value outside {"web", "native"}. The server answered 201 and provisioned the client; the SDK then raised Invalid registration response: … Input should be 'web' or 'native' [type=literal_error] out of the auth flow, discarding a registration whose client_id had already been minted (and orphaning it server-side on every retry). application_type is never read again anywhere in the SDK — it's a purely cosmetic field killing an otherwise-valid registration.

This isn't specific to that field. The response model is-a request model, so every tight type on the request re-arms the same failure on the response. The same closed-typing had already been widened piecemeal for *_supported metadata, grant_types, response_types, and empty-string URLs — this is the fifth instance of one class, and the class is the inheritance.

Spec. RFC 7591 §3.2.1: the server "MAY reject or replace any of the client's requested metadata values submitted during the registration and substitute them with suitable values", and "The client or developer can check the values in the response to determine if the registration is sufficient for use" — a substituted value is a judgment call for the client, not a parse failure. The MCP spec (SEP-837) constrains only what the client sends (MUST specify an appropriate application_type); it says nothing about the response. The server here is also off-spec — OIDC Registration §2 defines only native/web — but that doesn't oblige the client to abort a successful registration over a field it doesn't use. Both are true.

Cross-SDK. Every other MCP SDK constrains what it sends and parses application_type permissively — the TypeScript SDK types it z.string().optional() with a comment giving exactly this reasoning, go/rust/csharp/ruby likewise. This SDK was the only one that fails on it.

What changed

OAuthClientMetadata (request) and OAuthClientInformationFull (registered-client record) are now siblings over a shared OAuthClientMetadataBase holding only the fields that are genuinely identical between them:

class OAuthClientMetadataBase(BaseModel): ...          # verbatim-shared metadata
class OAuthClientMetadata(OAuthClientMetadataBase): ...          # request: strict (what we send)
class OAuthClientInformationFull(OAuthClientMetadataBase): ...   # record: tolerant (what a server echoes)
  • The request keeps every strict type — you still cannot construct or send application_type="browser"; the MCP MUST on the wire is unchanged and enforced by the type.
  • The record accepts what a server may echo: application_type/token_endpoint_auth_method are str | None (an echoed "" reads as absent, matching the existing URL-field coercion), grant_types is list[str], redirect_uris may be absent or empty. client_id is now required — RFC 7591 §3.2.1 makes it mandatory in the response, and a "registered client" record without one was never meaningful (a body without it should not parse as success).
  • Usability is judged where it matters, not at parse. The one substitution that makes minted credentials unusable — a token-endpoint auth method this client cannot apply — is reported as an OAuthRegistrationError naming the method when the registration completes, before the record is persisted or any interactive authorization begins (per §3.2.1's "sufficient for use" model). prepare_token_auth reports the same for a stored/pre-registered record. The recognized set is derived from one TokenEndpointAuthMethod type via get_args, so send-side and recognize-side cannot drift (private_key_jwt remains recognized — PrivateKeyJWTOAuthProvider's refresh path is unaffected).
  • Server side (bonus fix of the same mechanism): the bundled registration endpoint previously hand-copied ~20 fields into its 201 echo and dropped application_type, so the SDK's own AS reported the default in place of a client's "web". The echo is now built from the validated request's model_dump(), and a test pins that every request field exists on the record — a field can no longer be silently omitted.
  • The except ValidationError in handle_registration_response carried # pragma: no cover — the invalid-response branch was asserted unreachable, which is why this shipped untested. It's now covered and chains the cause (raise … from e).

How Has This Been Tested?

  • Model-level: the record parses off-set/null/empty application_type, an unimplemented token_endpoint_auth_method, extra grant_types, empty redirect_uris; the request still rejects an off-set application_type and requires redirect_uris.
  • Real handle_registration_response path (the layer the failure occurred in) with a substituted body, and with a body that isn't client information at all.
  • End-to-end interaction tests: a shimmed /register returning a substituted 201 completes the full flow (tool call succeeds); a 201 assigning an unusable auth method surfaces OAuthRegistrationError with no /authorize or /token request and nothing persisted.
  • Server: the 201 echoes the client's application_type for both web and native.
  • Driven manually against a live hostile authorization server over a socket: the production-shape body ("confidential", null redirect URIs, extra grants) now completes with HTTP 200 where main dies with the exact literal_error above; a "" auth method reads as absent; a client_secret_jwt assignment fails at registration before any browser round-trip.
  • Full suite: 100% coverage, strict-no-cover clean.

Breaking Changes

Yes — documented in docs/migration.md alongside the existing SEP-837 entry.

  • OAuthClientInformationFull is no longer a subclass of OAuthClientMetadata. Code relying on isinstance(info, OAuthClientMetadata), or passing a record where a request is annotated, must reference the record type directly. validate_scope()/validate_redirect_uri() moved with the record (they're what server code calls) and are no longer on OAuthClientMetadata.
  • On the record: application_type/token_endpoint_auth_method → str | None, grant_types → list[str], redirect_uris optional, client_id required. Reads are unaffected.
  • An unimplementable auth method now fails legibly (at registration / token exchange) instead of an opaque ValidationError during parsing.

Conformance: no scenario puts these values in a 201 body, and SEP-837 defines no response-side requirement, so a send-strict / parse-tolerant client passes every existing check unchanged.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

Two things I considered and want to name:

  • A stored record with an unusable method now errors on refresh instead of falling through. The old fall-through wasn't a working fallback — it looped into a full re-auth against the same record and failed at the token exchange anyway. Naming the permanent condition once is the intent, and such a record could never have been produced by registration before this change, so it only affects hand-supplied pre-registered credentials, where a clear error beats an endless silent invalid_client.
  • The record still serves two seats: the client's parse of an untrusted response and the SDK's own AS's stored-client type (OAuthAuthorizationServerProvider.get_client, authorize). The split stops one class short of separating those, which is why redirect_uris elements stay AnyUrl (the AS compares them) and why hand-built server records lose a couple of constructor invariants they previously borrowed from the request. Requiring client_id restores the one that matters; a fuller server-record / client-response separation is the next seam, deliberately left out of this change.

AI Disclaimer