Skip to content
Closed
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/migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -2618,7 +2618,7 @@ v2 hardens client authentication on SDK-hosted authorization servers (`create_au

`unauthorized_client` is now reserved for a client that authenticated successfully but is not permitted the requested grant. Update any client code, integration tests, or alerting that string-matches the old error code, or accept both while clients and servers migrate at different times.

**Secret-based clients without a stored secret are rejected.** In v1, `ClientAuthenticator` only validated a secret when one was stored, so a hand-provisioned client record with a secret-based auth method but no secret authenticated with no credentials at all. v2 rejects such clients before any grant processing: `/token` returns 401 `invalid_client` and `/revoke` returns 401 `unauthorized_client`, both with the description "Client is registered for secret-based authentication but has no stored secret". Only records that explicitly set `client_secret_post` or `client_secret_basic` with no secret are affected: records left at the default `token_endpoint_auth_method=None` fail in both versions, and DCR-registered clients always receive a generated secret.
**Secret-based clients without a stored secret are rejected.** In v1, `ClientAuthenticator` only validated a secret when one was stored, so a hand-provisioned client record with a secret-based auth method but no secret authenticated with no credentials at all. v2 rejects such clients before any grant processing: `/token` and `/revoke` both return 401 `invalid_client`, with the description "Client is registered for secret-based authentication but has no stored secret". Only records that explicitly set `client_secret_post` or `client_secret_basic` with no secret are affected: records left at the default `token_endpoint_auth_method=None` fail in both versions, and DCR-registered clients always receive a generated secret.

**Before (v1):**

Expand Down
15 changes: 11 additions & 4 deletions src/mcp/server/auth/handlers/revoke.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@
stringify_pydantic_error,
)
from mcp.server.auth.json_response import PydanticJSONResponse
from mcp.server.auth.middleware.client_auth import AuthenticationError, ClientAuthenticator
from mcp.server.auth.middleware.client_auth import (
AuthenticationError,
ClientAuthenticator,
www_authenticate_headers,
)
from mcp.server.auth.provider import AccessToken, OAuthAuthorizationServerProvider, RefreshToken


Expand All @@ -24,7 +28,8 @@ class RevocationRequest(BaseModel):


class RevocationErrorResponse(BaseModel):
error: Literal["invalid_request", "unauthorized_client"]
# RFC 7009 §2.2.1: the error presentation conforms to RFC 6749 §5.2.
error: Literal["invalid_request", "invalid_client", "unauthorized_client"]
error_description: str | None = None


Expand All @@ -37,13 +42,15 @@ async def handle(self, request: Request) -> Response:
"""Handler for the OAuth 2.0 Token Revocation endpoint."""
try:
client = await self.client_authenticator.authenticate_request(request)
except AuthenticationError as e: # pragma: no cover
except AuthenticationError as e:
# RFC 6749 §5.2 / RFC 7009 §2.2.1: failed client authentication is invalid_client.
return PydanticJSONResponse(
status_code=401,
content=RevocationErrorResponse(
error="unauthorized_client",
error="invalid_client",
error_description=e.message,
),
headers=www_authenticate_headers(request),
)

try:
Expand Down
48 changes: 32 additions & 16 deletions src/mcp/server/auth/handlers/token.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,22 @@

from mcp.server.auth.errors import stringify_pydantic_error
from mcp.server.auth.json_response import PydanticJSONResponse
from mcp.server.auth.middleware.client_auth import AuthenticationError, ClientAuthenticator
from mcp.server.auth.middleware.client_auth import AuthenticationError, ClientAuthenticator, www_authenticate_headers
from mcp.server.auth.provider import (
IdentityAssertionParams,
OAuthAuthorizationServerProvider,
TokenError,
TokenErrorCode,
)
from mcp.shared.auth import OAuthToken
from mcp.shared.auth import JWT_BEARER_GRANT_TYPE, OAuthToken


def supported_grant_types(*, identity_assertion_enabled: bool) -> list[str]:
"""Grant types this authorization server implements and advertises."""
grant_types = ["authorization_code", "refresh_token"]
if identity_assertion_enabled:
grant_types.append(JWT_BEARER_GRANT_TYPE)
return grant_types


class AuthorizationCodeRequest(BaseModel):
Expand Down Expand Up @@ -104,7 +112,7 @@ async def handle(self, request: Request):
try:
client_info = await self.client_authenticator.authenticate_request(request)
except AuthenticationError as e:
# Authentication failures should return 401
# RFC 6749 §5.2: failed client authentication is invalid_client.
return PydanticJSONResponse(
content=TokenErrorResponse(
error="invalid_client",
Expand All @@ -114,13 +122,28 @@ async def handle(self, request: Request):
headers={
"Cache-Control": "no-store",
"Pragma": "no-cache",
**www_authenticate_headers(request),
},
)

form_data = await request.form()
# TODO(Marcelo): Can someone check if this `dict()` wrapper is necessary?
form = dict(form_data)
grant_type = form.get("grant_type")
# A grant this server does not implement is unsupported_grant_type, before the
# request union is validated, so the body is not a pydantic error (RFC 6749 §5.2).
if isinstance(grant_type, str) and grant_type not in supported_grant_types(
identity_assertion_enabled=self.identity_assertion_enabled
):
return self.response(
TokenErrorResponse(
error="unsupported_grant_type",
error_description="The authorization grant type is not supported by the authorization server",
)
)

try:
form_data = await request.form()
# TODO(Marcelo): Can someone check if this `dict()` wrapper is necessary?
token_request = token_request_adapter.validate_python(dict(form_data))
token_request = token_request_adapter.validate_python(form)
except ValidationError as validation_error:
return self.response(
TokenErrorResponse(
Expand All @@ -129,11 +152,12 @@ async def handle(self, request: Request):
)
)

# The server implements this grant, but this client is not registered for it.
if token_request.grant_type not in client_info.grant_types:
return self.response(
TokenErrorResponse(
error="unsupported_grant_type",
error_description=(f"Unsupported grant type (supported grant types are {client_info.grant_types})"),
error="unauthorized_client",
error_description="The authenticated client is not authorized to use this authorization grant type",
)
)

Expand Down Expand Up @@ -240,14 +264,6 @@ async def handle(self, request: Request):
return self.response(TokenErrorResponse(error=e.error, error_description=e.error_description))

case JwtBearerRequest(): # pragma: no branch
if not self.identity_assertion_enabled:
return self.response(
TokenErrorResponse(
error="unsupported_grant_type",
error_description="The JWT bearer grant is not supported by this authorization server",
)
)

# SDK policy, adopting the SHOULD in draft-ietf-oauth-identity-assertion-authz-grant-04
# §9.1: only confidential clients may present an ID-JAG. ClientAuthenticator already
# rejects a secret-based method with no stored secret; this additionally rejects the
Expand Down
14 changes: 14 additions & 0 deletions src/mcp/server/auth/middleware/client_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,20 @@ def __init__(self, message: str):
self.message = message


def www_authenticate_headers(request: Request) -> dict[str, str]:
"""RFC 6749 §5.2 challenge when the client attempted HTTP Basic authentication.

A failed attempt via the Authorization header must be a 401 whose
WWW-Authenticate value matches that scheme. Basic challenges include a
realm (RFC 7617 §2); "OAuth" names this server's client-authentication
protection space. Other schemes are not rewritten into a Basic challenge.
"""
authorization = request.headers.get("Authorization", "")
if authorization.split(" ", 1)[0].lower() != "basic":
return {}
return {"WWW-Authenticate": 'Basic realm="OAuth"'}


class ClientAuthenticator:
"""ClientAuthenticator is a callable which validates requests from a client
application, used to verify /token calls.
Expand Down
7 changes: 3 additions & 4 deletions src/mcp/server/auth/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@
from mcp.server.auth.handlers.metadata import MetadataHandler, ProtectedResourceMetadataHandler
from mcp.server.auth.handlers.register import RegistrationHandler
from mcp.server.auth.handlers.revoke import RevocationHandler
from mcp.server.auth.handlers.token import TokenHandler
from mcp.server.auth.handlers.token import TokenHandler, supported_grant_types
from mcp.server.auth.middleware.client_auth import ClientAuthenticator
from mcp.server.auth.provider import OAuthAuthorizationServerProvider
from mcp.server.auth.settings import ClientRegistrationOptions, RevocationOptions
from mcp.server.transport_security import DEFAULT_MAX_REQUEST_BODY_SIZE, RequestBodyLimitMiddleware
from mcp.shared.auth import JWT_BEARER_GRANT_TYPE, OAuthMetadata, ProtectedResourceMetadata
from mcp.shared.auth import OAuthMetadata, ProtectedResourceMetadata
from mcp.shared.inbound import MCP_PROTOCOL_VERSION_HEADER


Expand Down Expand Up @@ -158,12 +158,11 @@ def build_metadata(
authorization_url = AnyHttpUrl(str(issuer_url).rstrip("/") + AUTHORIZATION_PATH)
token_url = AnyHttpUrl(str(issuer_url).rstrip("/") + TOKEN_PATH)

grant_types_supported = ["authorization_code", "refresh_token"]
grant_types_supported = supported_grant_types(identity_assertion_enabled=supports_identity_assertion)
# SEP-990 / ext-auth §6: support for the ID-JAG flow is advertised as a grant PROFILE, not as
# the jwt-bearer grant type (which an AS might support for other purposes).
authorization_grant_profiles_supported: list[str] | None = None
if supports_identity_assertion:
grant_types_supported.append(JWT_BEARER_GRANT_TYPE)
authorization_grant_profiles_supported = [ID_JAG_GRANT_PROFILE]

# Create metadata
Expand Down
170 changes: 170 additions & 0 deletions tests/server/auth/test_error_handling.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import httpx2
import pytest
from httpx2 import ASGITransport
from inline_snapshot import snapshot
from pydantic import AnyHttpUrl
from starlette.applications import Starlette

Expand Down Expand Up @@ -347,3 +348,172 @@ async def test_oversized_cross_origin_request_gets_413_with_cors_headers(client:
)
assert response.status_code == 413
assert response.headers["access-control-allow-origin"] == "*"


async def _register(
client: httpx2.AsyncClient,
*,
grant_types: list[str],
token_endpoint_auth_method: str,
) -> dict[str, Any]:
response = await client.post(
"/register",
json={
"redirect_uris": ["https://client.example.com/callback"],
"token_endpoint_auth_method": token_endpoint_auth_method,
"grant_types": grant_types,
"response_types": ["code"],
"client_name": "Test Client",
},
)
assert response.status_code == 201
body = response.json()
return body


@pytest.mark.anyio
async def test_refresh_for_a_client_not_registered_for_it_is_unauthorized_client(client: httpx2.AsyncClient):
"""A grant this server implements, but this client is not registered for, is unauthorized_client.

RFC 6749 §5.2 reserves unsupported_grant_type for a grant the authorization server
itself does not implement.
"""
registered = await _register(
client, grant_types=["authorization_code"], token_endpoint_auth_method="client_secret_post"
)
client_id = registered["client_id"]
client_secret = registered["client_secret"]
assert isinstance(client_id, str)
assert isinstance(client_secret, str)

response = await client.post(
"/token",
data={
"grant_type": "refresh_token",
"refresh_token": "not-a-real-token",
"client_id": client_id,
"client_secret": client_secret,
},
)

assert response.status_code == 400
assert response.json() == snapshot(
{
"error": "unauthorized_client",
"error_description": "The authenticated client is not authorized to use this authorization grant type",
}
)


@pytest.mark.anyio
@pytest.mark.parametrize("grant_type", ["client_credentials", "password", "not_a_grant"])
async def test_grant_the_server_does_not_implement_is_unsupported_grant_type(
client: httpx2.AsyncClient, grant_type: str
):
"""A grant outside the set this server advertises is unsupported_grant_type, with no validation text.

RFC 6749 §5.2. The code is chosen from grant_type before the request union is validated,
so client_credentials and password do not surface as invalid_request.
"""
registered = await _register(
client, grant_types=["authorization_code"], token_endpoint_auth_method="client_secret_post"
)
client_id = registered["client_id"]
client_secret = registered["client_secret"]
assert isinstance(client_id, str)
assert isinstance(client_secret, str)

response = await client.post(
"/token",
data={"grant_type": grant_type, "client_id": client_id, "client_secret": client_secret},
)

assert response.status_code == 400
assert response.json() == snapshot(
{
"error": "unsupported_grant_type",
"error_description": "The authorization grant type is not supported by the authorization server",
}
)


@pytest.mark.anyio
async def test_authenticated_token_request_without_grant_type_is_invalid_request(client: httpx2.AsyncClient):
"""Omitting grant_type is a malformed request, so the code stays invalid_request.

RFC 6749 §5.2: a missing required parameter is invalid_request. The description is
pydantic's validation text, so only its presence is pinned.
"""
registered = await _register(
client,
grant_types=["authorization_code", "refresh_token"],
token_endpoint_auth_method="client_secret_post",
)
client_id = registered["client_id"]
client_secret = registered["client_secret"]
assert isinstance(client_id, str)
assert isinstance(client_secret, str)

response = await client.post("/token", data={"client_id": client_id, "client_secret": client_secret})

assert response.status_code == 400
body = response.json()
assert body["error"] == "invalid_request"
assert body["error_description"]


@pytest.mark.anyio
@pytest.mark.parametrize("path", ["/token", "/revoke"])
async def test_failed_basic_authentication_includes_www_authenticate(client: httpx2.AsyncClient, path: str):
"""Failed HTTP Basic client authentication is 401 invalid_client with a Basic challenge.

RFC 6749 §5.2 requires WWW-Authenticate matching the scheme the client used, and
RFC 7617 §2 requires a realm on a Basic challenge. RFC 7009 §2.2.1 uses that same
presentation for /revoke, so a bad secret there is invalid_client.
"""
registered = await _register(
client, grant_types=["authorization_code"], token_endpoint_auth_method="client_secret_basic"
)
client_id = registered["client_id"]
assert isinstance(client_id, str)
credentials = base64.b64encode(f"{client_id}:wrong".encode()).decode()

response = await client.post(
path,
headers={"Authorization": f"Basic {credentials}"},
data={"client_id": client_id, "grant_type": "refresh_token", "refresh_token": "rt", "token": "rt"},
)

assert response.status_code == 401
assert response.json() == snapshot({"error": "invalid_client", "error_description": "Invalid client_secret"})
assert response.headers["www-authenticate"] == 'Basic realm="OAuth"'


@pytest.mark.anyio
@pytest.mark.parametrize("path", ["/token", "/revoke"])
async def test_failed_client_secret_post_authentication_omits_www_authenticate(client: httpx2.AsyncClient, path: str):
"""A client_secret_post failure is invalid_client and carries no WWW-Authenticate header.

RFC 6749 §5.2 requires that header only when the client used the Authorization field.
/revoke uses the same code (RFC 7009 §2.2.1).
"""
registered = await _register(
client, grant_types=["authorization_code"], token_endpoint_auth_method="client_secret_post"
)
client_id = registered["client_id"]
assert isinstance(client_id, str)

response = await client.post(
path,
data={
"client_id": client_id,
"client_secret": "wrong",
"grant_type": "refresh_token",
"refresh_token": "rt",
"token": "rt",
},
)

assert response.status_code == 401
assert response.json() == snapshot({"error": "invalid_client", "error_description": "Invalid client_secret"})
assert "www-authenticate" not in response.headers
7 changes: 5 additions & 2 deletions tests/server/auth/test_identity_assertion.py
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,10 @@ async def test_malformed_request_missing_assertion_is_invalid_request(client: ht
async def test_client_without_the_grant_registered_is_rejected(
client: httpx2.AsyncClient, provider: IdentityAssertionProvider
):
"""A confidential client whose registration omits the jwt-bearer grant is refused the grant."""
"""A confidential client whose registration omits the jwt-bearer grant is refused the grant.

RFC 6749 §5.2: the server implements this grant, so the code is unauthorized_client.
"""
provider.clients["no-grant-client"] = OAuthClientInformationFull(
client_id="no-grant-client",
client_secret="s",
Expand All @@ -309,7 +312,7 @@ async def test_client_without_the_grant_registered_is_rejected(
)

assert response.status_code == 400
assert response.json()["error"] == "unsupported_grant_type"
assert response.json()["error"] == "unauthorized_client"
assert provider.last_params is None


Expand Down
Loading