From 7d398a03c73635247d9f46c88ee25e7cfa5dd3d9 Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Wed, 29 Jul 2026 15:39:04 +0000 Subject: [PATCH 1/2] Delegate authorize-time scope policy to the provider The SDK-hosted authorization server rejected any /authorize request whose scope was not in the client's registered `scope` metadata, and treated a client registered without a scope as allowed to request nothing at all. That broke the spec's step-up flow, in which a client answers a 403 insufficient_scope challenge by re-authorizing for the union of its previous and challenged scopes without re-registering, and it rejected every scoped request from scope-less registrations (#2216) as well as an empty `scope=` parameter (#977). The authorize handler now enforces only the server-wide scope set, `ClientRegistrationOptions.valid_scopes` (the list already advertised as `scopes_supported` and enforced at registration). A requested scope outside it redirects back with `invalid_scope`; when it is unset, every requested scope reaches `provider.authorize()`, which can reject a client's request by raising `AuthorizeError(error="invalid_scope")`. An empty scope parameter is treated as omitted. `OAuthClientInformationFull.validate_scope()` and `InvalidScopeError` are removed; both existed only to implement the deleted check. Github-Issue: #2216 --- docs/migration.md | 34 +++++- src/mcp/server/auth/handlers/authorize.py | 44 +++++-- src/mcp/server/auth/routes.py | 2 +- src/mcp/shared/auth.py | 15 --- tests/interaction/_requirements.py | 11 ++ tests/interaction/auth/test_as_handlers.py | 41 +++++++ tests/interaction/auth/test_lifecycle.py | 14 ++- .../mcpserver/auth/test_auth_integration.py | 115 +++++++++++++++++- 8 files changed, 241 insertions(+), 35 deletions(-) diff --git a/docs/migration.md b/docs/migration.md index 931d470d1a..3dfa3322d1 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -2594,7 +2594,7 @@ class OAuthClientMetadata(OAuthClientMetadataBase): ... # request: stri 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`. +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_redirect_uri()` moved with the record: it is a method of `OAuthClientInformationFull` (the type authorization-server code holds) and is no longer available on `OAuthClientMetadata`. `validate_scope()` is removed altogether; see [The SDK-hosted authorization server no longer enforces a client's registered `scope`](#the-sdk-hosted-authorization-server-no-longer-enforces-a-clients-registered-scope). 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. @@ -2640,6 +2640,38 @@ LEGACY_CLIENT = OAuthClientInformationFull( ) ``` +### The SDK-hosted authorization server no longer enforces a client's registered `scope` + +In v1 (and early v2), the authorize endpoint of the SDK-hosted authorization server (`create_auth_routes`) rejected any requested scope that was not in the client's registered `scope` metadata, and treated a client registered without a `scope` as allowed to request nothing at all: + +```text +# v1: client registered without a scope, then GET /authorize?...&scope=mcp +302 → ?error=invalid_scope&error_description=Client+was+not+registered+with+scope+mcp +``` + +That check is gone. A client's registered `scope` is [RFC 7591](https://datatracker.ietf.org/doc/html/rfc7591#section-2) client metadata - self-asserted, "scope values that the client can use when requesting access tokens" - not an allowlist the authorization server must enforce, and enforcing it broke the spec's [step-up authorization flow](https://modelcontextprotocol.io/specification/2026-07-28/basic/authorization#step-up-authorization-flow), in which a client answers a `403 insufficient_scope` challenge by re-authorizing for the union of its previous and challenged scopes without re-registering. The endpoint now enforces only the server-wide scope set: `ClientRegistrationOptions.valid_scopes` (the same list advertised as `scopes_supported` and already enforced at dynamic client registration). A requested scope outside it is redirected back with `error=invalid_scope`; when `valid_scopes` is unset, every requested scope is passed to the provider. An empty `scope=` parameter is now treated as an omitted one instead of being rejected. + +Per-client scope policy belongs in your provider, whose `authorize()` already receives the parsed request and can reject it with the same wire result - here an operator-maintained ceiling per client, rather than the client's self-asserted metadata: + +```python +from mcp.server.auth.provider import AuthorizationParams, AuthorizeError +from mcp.shared.auth import OAuthClientInformationFull + +# Scope ceilings you configure per client; anything not listed gets the base set. +CLIENT_SCOPE_CEILINGS = {"backend-service": {"mcp", "write", "admin"}} + + +class MyProvider: + async def authorize(self, client: OAuthClientInformationFull, params: AuthorizationParams) -> str: + requested = set(params.scopes or []) + ceiling = CLIENT_SCOPE_CEILINGS.get(client.client_id, {"mcp"}) + if not requested <= ceiling: + raise AuthorizeError(error="invalid_scope", error_description="scope not permitted for this client") + ... +``` + +`OAuthClientInformationFull.validate_scope()` and `mcp.shared.auth.InvalidScopeError` are removed; both existed only to implement the deleted check. Parse a scope string with `str.split()`, and move any `except InvalidScopeError` handling into the provider. + ## Stricter protocol validation and wire behavior ### Server handler results are validated against the protocol schema diff --git a/src/mcp/server/auth/handlers/authorize.py b/src/mcp/server/auth/handlers/authorize.py index 5cf93cf8c2..55ede2ea6b 100644 --- a/src/mcp/server/auth/handlers/authorize.py +++ b/src/mcp/server/auth/handlers/authorize.py @@ -17,7 +17,7 @@ OAuthAuthorizationServerProvider, construct_redirect_uri, ) -from mcp.shared.auth import InvalidRedirectUriError, InvalidScopeError +from mcp.shared.auth import InvalidRedirectUriError logger = logging.getLogger(__name__) @@ -63,9 +63,32 @@ class AnyUrlModel(RootModel[AnyUrl]): root: AnyUrl +def scope_tokens(scope: str | None) -> list[str] | None: + """Split an RFC 6749 §3.3 scope parameter into its space-delimited tokens. + + An absent or empty parameter is no scope request at all, so both return None and the + provider applies its default (RFC 6749 §3.3). + """ + tokens = scope.split() if scope else [] + return tokens or None + + @dataclass class AuthorizationHandler: + """The authorization endpoint (RFC 6749 §4.1.1) of the SDK-hosted authorization server. + + `valid_scopes` is the scope set this authorization server supports - the same list advertised + as `scopes_supported` and enforced at dynamic client registration. A request naming any other + scope is rejected with `invalid_scope`, whichever client sends it. `None` declares no + server-wide scope set, so every requested scope reaches the provider. A registered client's + own `scope` is not consulted: RFC 7591 §2 makes it self-asserted metadata, and enforcing it + would reject the step-up flow, in which a client re-authorizes for scopes beyond its + registration. Per-client scope policy belongs in `OAuthAuthorizationServerProvider.authorize()`, + which rejects a request by raising `AuthorizeError` with `error="invalid_scope"`. + """ + provider: OAuthAuthorizationServerProvider[Any, Any, Any] + valid_scopes: list[str] | None = None async def handle(self, request: Request) -> Response: # implements authorization requests for grant_type=code; @@ -185,15 +208,16 @@ async def error_response( error_description=validation_error.message, ) - # Validate scope - for scope errors, we can redirect - try: - scopes = client.validate_scope(auth_request.scope) - except InvalidScopeError as validation_error: - # For scope errors, redirect with error parameters - return await error_response( - error="invalid_scope", - error_description=validation_error.message, - ) + # Enforce the server-wide scope set; scope policy for the client is the provider's, + # in authorize(). An unsupported scope redirects back with invalid_scope. + scopes = scope_tokens(auth_request.scope) + if scopes is not None and self.valid_scopes is not None: + unsupported = sorted(set(scopes) - set(self.valid_scopes)) + if unsupported: + return await error_response( + error="invalid_scope", + error_description=f"Requested scopes are not valid: {', '.join(unsupported)}", + ) # Setup authorization parameters auth_params = AuthorizationParams( diff --git a/src/mcp/server/auth/routes.py b/src/mcp/server/auth/routes.py index fa88dddcf4..a887916249 100644 --- a/src/mcp/server/auth/routes.py +++ b/src/mcp/server/auth/routes.py @@ -102,7 +102,7 @@ def create_auth_routes( AUTHORIZATION_PATH, # do not allow CORS for authorization endpoint; # clients should just redirect to this - endpoint=AuthorizationHandler(provider).handle, + endpoint=AuthorizationHandler(provider, valid_scopes=client_registration_options.valid_scopes).handle, methods=["GET", "POST"], ), Route( diff --git a/src/mcp/shared/auth.py b/src/mcp/shared/auth.py index 881379d381..c32ad4ce9d 100644 --- a/src/mcp/shared/auth.py +++ b/src/mcp/shared/auth.py @@ -54,11 +54,6 @@ class AuthorizationCodeResult(BaseModel): iss: str | None = None -class InvalidScopeError(Exception): - def __init__(self, message: str): - self.message = message - - class InvalidRedirectUriError(Exception): def __init__(self, message: str): self.message = message @@ -174,16 +169,6 @@ def _placeholder_members_read_as_omitted(cls, data: object) -> object: return {key: value for key, value in members.items() if value is not None and value != ""} return data - def validate_scope(self, requested_scope: str | None) -> list[str] | None: - if requested_scope is None: - return None - requested_scopes = requested_scope.split(" ") - allowed_scopes = [] if self.scope is None else self.scope.split(" ") - for scope in requested_scopes: - if scope not in allowed_scopes: - raise InvalidScopeError(f"Client was not registered with scope {scope}") - return requested_scopes - def validate_redirect_uri(self, redirect_uri: AnyUrl | None) -> AnyUrl: if redirect_uri is not None: # Validate redirect_uri against client's registered redirect URIs diff --git a/tests/interaction/_requirements.py b/tests/interaction/_requirements.py index 964a1829d2..2c0a3e0e80 100644 --- a/tests/interaction/_requirements.py +++ b/tests/interaction/_requirements.py @@ -2942,6 +2942,17 @@ def __post_init__(self) -> None: transports=("streamable-http",), note="Auth is enforced at the HTTP layer; the bundled AS is an ASGI app.", ), + "hosting:auth:as:authorize-scope": Requirement( + source=f"{SPEC_BASE_URL}/basic/authorization#step-up-authorization-flow", + behavior=( + "The bundled authorize endpoint accepts a requested scope beyond the client's registered " + "`scope` when it is within the server's `valid_scopes`, so a client can step up without " + "re-registering, and rejects a scope outside `valid_scopes` with an `invalid_scope` " + "error redirect. Per-client scope policy is delegated to the provider's `authorize()`." + ), + transports=("streamable-http",), + note="Auth is enforced at the HTTP layer; the bundled AS is an ASGI app.", + ), "hosting:auth:as:verifier-mismatch": Requirement( source=f"{SPEC_BASE_URL}/basic/authorization#authorization-code-protection", behavior=( diff --git a/tests/interaction/auth/test_as_handlers.py b/tests/interaction/auth/test_as_handlers.py index 1876cd7181..c0954e973c 100644 --- a/tests/interaction/auth/test_as_handlers.py +++ b/tests/interaction/auth/test_as_handlers.py @@ -129,6 +129,47 @@ async def test_authorize_without_a_code_challenge_is_rejected_with_invalid_reque assert "code_challenge" in params["error_description"][0] +@requirement("hosting:auth:as:authorize-scope") +async def test_authorize_accepts_scope_beyond_registration_within_valid_scopes_and_rejects_the_rest() -> None: + """A client registered with `mcp` may authorize for `mcp write` (`write` is within the server's + `valid_scopes`, so step-up needs no re-registration), while a scope the server does not support + is redirected back with `error=invalid_scope`. The client's registered `scope` is not an allowlist.""" + provider = InMemoryAuthorizationServerProvider() + settings = auth_settings(required_scopes=["mcp"], valid_scopes=["mcp", "write"]) + async with mounted_app( + Server("guarded"), + auth=settings, + token_verifier=ProviderTokenVerifier(provider), + auth_server_provider=provider, + ) as (http, _): + client_info = await _register_client(http) + assert client_info.client_id is not None + assert client_info.scope == "mcp" + + _, challenge = _pkce_pair() + base_params = { + "response_type": "code", + "client_id": client_info.client_id, + "redirect_uri": REDIRECT_URI, + "code_challenge": challenge, + "code_challenge_method": "S256", + "state": "s", + } + widened = await http.get("/authorize", params=base_params | {"scope": "mcp write"}, follow_redirects=False) + unknown = await http.get("/authorize", params=base_params | {"scope": "mcp admin"}, follow_redirects=False) + + assert widened.status_code == 302 + granted = parse_qs(urlsplit(widened.headers["location"]).query) + assert "code" in granted + assert provider.codes[granted["code"][0]].scopes == ["mcp", "write"] + + assert unknown.status_code == 302 + rejected = parse_qs(urlsplit(unknown.headers["location"]).query) + assert rejected["error"] == ["invalid_scope"] + assert rejected["error_description"] == snapshot(["Requested scopes are not valid: admin"]) + assert rejected["state"] == ["s"] + + @requirement("hosting:auth:as:verifier-mismatch") async def test_a_mismatched_code_verifier_is_rejected_with_invalid_grant( as_app: tuple[httpx2.AsyncClient, InMemoryAuthorizationServerProvider], diff --git a/tests/interaction/auth/test_lifecycle.py b/tests/interaction/auth/test_lifecycle.py index 8f45a01510..065b251811 100644 --- a/tests/interaction/auth/test_lifecycle.py +++ b/tests/interaction/auth/test_lifecycle.py @@ -143,13 +143,14 @@ async def test_a_403_insufficient_scope_triggers_one_reauthorize_with_the_challe which reaches the auth flow's step-up handler; the first authenticated POST is the post-401 retry, after which the generator ends without inspecting the response). The challenge names a wider scope; step-up reuses cached metadata and the existing client registration, - re-authorizes with the new scope, and the connect completes. The client is pre-registered - with both scopes so the server's authorize handler accepts the wider second request. One - re-authorize, one retry; the spec's SHOULD-retry-limit ("a few") is not enforced. + re-authorizes with the new scope, and the connect completes. The client is registered with + only `mcp`, so the server's authorize endpoint accepting the wider `mcp write` (both within + its `valid_scopes`) is what lets step-up proceed without re-registering. One re-authorize, + one retry; the spec's SHOULD-retry-limit ("a few") is not enforced. """ recorded, on_request = record_requests() provider = InMemoryAuthorizationServerProvider() - storage = InMemoryTokenStorage(client_info=seeded_client(provider, scope="mcp write")) + storage = InMemoryTokenStorage(client_info=seeded_client(provider, scope="mcp")) server = Server("guarded", on_list_tools=list_tools) settings = auth_settings(required_scopes=["mcp"], valid_scopes=["mcp", "write"]) challenge = 'Bearer error="insufficient_scope", scope="mcp write"' @@ -185,10 +186,11 @@ async def test_a_403_step_up_re_authorizes_with_the_union_of_prior_and_challenge The first authorization requests `mcp`; the 403 challenges a disjoint `write` (not naming `mcp`). Per SEP-2350 the client must re-authorize with `mcp write`, not drop `mcp`. The client - is pre-registered with both scopes so the server's authorize handler accepts the wider request. + is registered with only `mcp`; the server accepts the wider request because `write` is within + its `valid_scopes`, not because the client registered it. """ provider = InMemoryAuthorizationServerProvider() - storage = InMemoryTokenStorage(client_info=seeded_client(provider, scope="mcp write")) + storage = InMemoryTokenStorage(client_info=seeded_client(provider, scope="mcp")) server = Server("guarded", on_list_tools=list_tools) settings = auth_settings(required_scopes=["mcp"], valid_scopes=["mcp", "write"]) challenge = 'Bearer error="insufficient_scope", scope="write"' diff --git a/tests/server/mcpserver/auth/test_auth_integration.py b/tests/server/mcpserver/auth/test_auth_integration.py index e9c1df8465..a14012ea14 100644 --- a/tests/server/mcpserver/auth/test_auth_integration.py +++ b/tests/server/mcpserver/auth/test_auth_integration.py @@ -1628,9 +1628,10 @@ async def test_authorize_missing_pkce_challenge( async def test_authorize_invalid_scope( self, test_client: httpx2.AsyncClient, registered_client: dict[str, Any], pkce_challenge: dict[str, str] ): - """Test authorization endpoint with invalid scope. + """Test authorization endpoint with a scope the server does not support. - Invalid scope should redirect with invalid_scope error. + A requested scope outside the server's `valid_scopes` should redirect with + invalid_scope error. """ response = await test_client.get( @@ -1654,6 +1655,116 @@ async def test_authorize_invalid_scope( assert "error" in query_params assert query_params["error"][0] == "invalid_scope" + assert ( + query_params["error_description"][0] == "Requested scopes are not valid: invalid_scope_that_does_not_exist" + ) # State should be preserved assert "state" in query_params assert query_params["state"][0] == "test_state" + + +@pytest.mark.anyio +@pytest.mark.parametrize("registered_client", [{"scope": "read"}], indirect=True) +async def test_authorize_accepts_a_scope_beyond_the_clients_registered_scope( + test_client: httpx2.AsyncClient, + mock_oauth_provider: MockOAuthProvider, + registered_client: dict[str, Any], + pkce_challenge: dict[str, str], +): + """A client registered with `read` may authorize for `read write`: `write` is within the + server's `valid_scopes`, and the client's registered `scope` is not an allowlist. This is the + request shape of the spec's step-up flow, which re-authorizes without re-registering.""" + assert registered_client["scope"] == "read" + + response = await test_client.get( + "/authorize", + params={ + "response_type": "code", + "client_id": registered_client["client_id"], + "redirect_uri": "https://client.example.com/callback", + "code_challenge": pkce_challenge["code_challenge"], + "code_challenge_method": "S256", + "scope": "read write", + "state": "test_state", + }, + ) + + assert response.status_code == 302 + query_params = parse_qs(urlparse(response.headers["location"]).query) + assert "error" not in query_params + code = query_params["code"][0] + assert mock_oauth_provider.auth_codes[code].scopes == ["read", "write"] + + +@pytest.mark.anyio +async def test_authorize_treats_an_empty_scope_parameter_as_omitted( + test_client: httpx2.AsyncClient, + mock_oauth_provider: MockOAuthProvider, + registered_client: dict[str, Any], + pkce_challenge: dict[str, str], +): + """An empty `scope=` parameter (some clients send one) is no scope request: the provider + receives `None` and applies its default, rather than being handed an empty scope token.""" + response = await test_client.get( + "/authorize", + params={ + "response_type": "code", + "client_id": registered_client["client_id"], + "redirect_uri": "https://client.example.com/callback", + "code_challenge": pkce_challenge["code_challenge"], + "code_challenge_method": "S256", + "scope": "", + "state": "test_state", + }, + ) + + assert response.status_code == 302 + query_params = parse_qs(urlparse(response.headers["location"]).query) + code = query_params["code"][0] + # MockOAuthProvider substitutes its default scopes when `params.scopes` is None. + assert mock_oauth_provider.auth_codes[code].scopes == ["read", "write"] + + +@pytest.mark.anyio +async def test_authorize_passes_every_requested_scope_to_the_provider_when_no_valid_scopes_are_configured( + mock_oauth_provider: MockOAuthProvider, + pkce_challenge: dict[str, str], +): + """With no server-wide `valid_scopes`, no scope is rejected at `/authorize` - not even for a + client whose registered record carries no `scope` - and the provider decides what to grant.""" + auth_routes = create_auth_routes( + mock_oauth_provider, + AnyHttpUrl("https://auth.example.com"), + client_registration_options=ClientRegistrationOptions(enabled=True), + ) + async with httpx2.AsyncClient( + transport=httpx2.ASGITransport(app=Starlette(routes=auth_routes)), base_url="https://mcptest.com" + ) as client: + registration = await client.post( + "/register", + json={ + "redirect_uris": ["https://client.example.com/callback"], + "grant_types": ["authorization_code", "refresh_token"], + }, + ) + assert registration.status_code == 201 + client_info = registration.json() + assert "scope" not in client_info + + response = await client.get( + "/authorize", + params={ + "response_type": "code", + "client_id": client_info["client_id"], + "redirect_uri": "https://client.example.com/callback", + "code_challenge": pkce_challenge["code_challenge"], + "code_challenge_method": "S256", + "scope": "any scope at all", + "state": "test_state", + }, + ) + + assert response.status_code == 302 + query_params = parse_qs(urlparse(response.headers["location"]).query) + code = query_params["code"][0] + assert mock_oauth_provider.auth_codes[code].scopes == ["any", "scope", "at", "all"] From b863e4caa61e246e60b29d9047b516c2b78f2c97 Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Wed, 29 Jul 2026 15:41:56 +0000 Subject: [PATCH 2/2] Document scope policy as the provider's responsibility in authorize() Spell out on OAuthAuthorizationServerProvider.authorize() that the requested scopes have only been checked against the server-wide valid_scopes and that per-client scope policy is enforced here, so the contract lives on the interface operators implement rather than only in the migration guide. No-Verification-Needed: docstring-only change --- src/mcp/server/auth/provider.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/mcp/server/auth/provider.py b/src/mcp/server/auth/provider.py index 644868f3e5..f6c69e32bd 100644 --- a/src/mcp/server/auth/provider.py +++ b/src/mcp/server/auth/provider.py @@ -192,6 +192,12 @@ async def authorize(self, client: OAuthClientInformationFull, params: Authorizat and MUST generate an authorization code with at least 128 bits of entropy. See https://datatracker.ietf.org/doc/html/rfc6749#section-10.10. + Scope policy is this method's responsibility. `params.scopes` has been checked only + against the server-wide `ClientRegistrationOptions.valid_scopes` (when configured); the + client's registered `scope` metadata is self-asserted (RFC 7591 §2) and is not enforced. + Reject a request the client may not make by raising `AuthorizeError` with + `error="invalid_scope"`, or grant a narrower scope set - and issue the token accordingly. + Args: client: The client requesting authorization. params: The parameters of the authorization request.