From d95f3a69223512ffc18cfeb692a567ab3ad57ca8 Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Tue, 17 Mar 2026 11:51:31 +0000 Subject: [PATCH] fix: remove scope registration check from authorize handler The check in validate_scope rejected any requested scope not in the client's registered metadata. This broke the MCP spec's step-up authorization flow: when a server returns 403 insufficient_scope with a WWW-Authenticate challenge containing expanded scopes, the client (see client/auth/oauth2.py) re-authorizes with those scopes and the server would reject them. RFC 7591 Section 2 defines the scope field as scopes the client "can use", with no language restricting requests to that set. Scope policy enforcement belongs in OAuthAuthorizationServerProvider.authorize(), which can already raise AuthorizeError(error="invalid_scope", ...). The TypeScript SDK removed this check in #983 for the same reason. InvalidScopeError is removed as it was only raised from this path. Reported-by: nik1097 Github-Issue: #2216 --- docs/migration.md | 17 ++++++++++ src/mcp/server/auth/handlers/authorize.py | 12 ++----- src/mcp/shared/auth.py | 18 ++++------ .../mcpserver/auth/test_auth_integration.py | 34 ------------------- tests/shared/test_auth.py | 12 ++++++- 5 files changed, 37 insertions(+), 56 deletions(-) diff --git a/docs/migration.md b/docs/migration.md index 7cf032553..338995b74 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -797,6 +797,23 @@ server = Server("my-server") server.experimental.enable_tasks(on_get_task=custom_get_task) ``` +### Server auth: `InvalidScopeError` removed, `validate_scope` no longer enforces + +`OAuthClientMetadata.validate_scope()` no longer rejects scopes outside the client's registered set — it now only parses the scope string. The previous check blocked the MCP spec's step-up authorization flow, where a client must be able to request scopes beyond its initial registration in response to a `WWW-Authenticate: insufficient_scope` challenge. See [TypeScript SDK #983](https://github.com/modelcontextprotocol/typescript-sdk/pull/983) for the equivalent change. + +`InvalidScopeError` (from `mcp.shared.auth`) has been removed — the SDK no longer raises it. + +If your server needs to reject scopes, enforce policy inside `OAuthAuthorizationServerProvider.authorize()`: + +```python +from mcp.server.auth.provider import AuthorizeError + +async def authorize(self, client, params): + if params.scopes and "admin" in params.scopes and not client_is_trusted(client): + raise AuthorizeError(error="invalid_scope", error_description="admin scope requires approval") + ... +``` + ## Deprecations diff --git a/src/mcp/server/auth/handlers/authorize.py b/src/mcp/server/auth/handlers/authorize.py index dec6713b1..9be1f4c86 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__) @@ -185,15 +185,7 @@ 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, - ) + scopes = client.validate_scope(auth_request.scope) # Setup authorization parameters auth_params = AuthorizationParams( diff --git a/src/mcp/shared/auth.py b/src/mcp/shared/auth.py index ca5b7b45a..6c4a4b32b 100644 --- a/src/mcp/shared/auth.py +++ b/src/mcp/shared/auth.py @@ -22,11 +22,6 @@ def normalize_token_type(cls, v: str | None) -> str | None: return v # pragma: no cover -class InvalidScopeError(Exception): - def __init__(self, message: str): - self.message = message - - class InvalidRedirectUriError(Exception): def __init__(self, message: str): self.message = message @@ -68,14 +63,15 @@ class OAuthClientMetadata(BaseModel): software_version: str | None = None def validate_scope(self, requested_scope: str | None) -> list[str] | None: + """Parse the requested scope string into a list. + + Scope policy enforcement is the provider's responsibility: raise + ``AuthorizeError(error="invalid_scope", ...)`` from + ``OAuthAuthorizationServerProvider.authorize()`` to reject. + """ 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: # pragma: no branch - raise InvalidScopeError(f"Client was not registered with scope {scope}") - return requested_scopes # pragma: no cover + return requested_scope.split(" ") def validate_redirect_uri(self, redirect_uri: AnyUrl | None) -> AnyUrl: if redirect_uri is not None: diff --git a/tests/server/mcpserver/auth/test_auth_integration.py b/tests/server/mcpserver/auth/test_auth_integration.py index 602f5cc75..b0b780ec6 100644 --- a/tests/server/mcpserver/auth/test_auth_integration.py +++ b/tests/server/mcpserver/auth/test_auth_integration.py @@ -1607,37 +1607,3 @@ async def test_authorize_missing_pkce_challenge( # State should be preserved assert "state" in query_params assert query_params["state"][0] == "test_state" - - @pytest.mark.anyio - async def test_authorize_invalid_scope( - self, test_client: httpx.AsyncClient, registered_client: dict[str, Any], pkce_challenge: dict[str, str] - ): - """Test authorization endpoint with invalid scope. - - Invalid scope should redirect with invalid_scope error. - """ - - 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": "invalid_scope_that_does_not_exist", - "state": "test_state", - }, - ) - - # Should redirect with error parameters - assert response.status_code == 302 - redirect_url = response.headers["location"] - parsed_url = urlparse(redirect_url) - query_params = parse_qs(parsed_url.query) - - assert "error" in query_params - assert query_params["error"][0] == "invalid_scope" - # State should be preserved - assert "state" in query_params - assert query_params["state"][0] == "test_state" diff --git a/tests/shared/test_auth.py b/tests/shared/test_auth.py index cd3c35332..756a186b5 100644 --- a/tests/shared/test_auth.py +++ b/tests/shared/test_auth.py @@ -1,6 +1,6 @@ """Tests for OAuth 2.0 shared code.""" -from mcp.shared.auth import OAuthMetadata +from mcp.shared.auth import OAuthClientMetadata, OAuthMetadata def test_oauth(): @@ -58,3 +58,13 @@ def test_oauth_with_jarm(): "token_endpoint_auth_methods_supported": ["client_secret_basic", "client_secret_post"], } ) + + +def test_validate_scope_none_returns_none(): + client = OAuthClientMetadata.model_validate({"redirect_uris": ["https://example.com/cb"]}) + assert client.validate_scope(None) is None + + +def test_validate_scope_splits_requested(): + client = OAuthClientMetadata.model_validate({"redirect_uris": ["https://example.com/cb"]}) + assert client.validate_scope("read write admin") == ["read", "write", "admin"]