Split the registration request model from the registered-client record (#3181)

This commit is contained in:
Max 2026-07-27 23:11:01 +01:00 • committed by GitHub
commit d3ffe87960
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 573 additions and 129 deletions

View file

@ -149,44 +149,6 @@ class TestClientCredentialsOAuthProvider:
# 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."""

View file

@ -12,7 +12,7 @@ from inline_snapshot import Is, snapshot
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 @@ class TestRegistrationResponse:
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"<html>not json</html>", 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."""

View file

@ -2940,6 +2940,17 @@ REQUIREMENTS: dict[str, Requirement] = {
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 @@ REQUIREMENTS: dict[str, Requirement] = {
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=(

View file

@ -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")

View file

@ -17,14 +17,16 @@ import mcp_types as types
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.

View file

@ -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 = {