diff --git a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py index f5c54451006..d995d13d2a9 100644 --- a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py +++ b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py @@ -925,6 +925,25 @@ def _require_s256_pkce( ) +def _cap_requested_scopes(requested: str | None, server: MCPServer) -> str | None: + configured: Final = server.configured_scopes + if not requested or not configured: + return requested + scopes: Final = requested.split() + dropped: Final = [scope for scope in scopes if scope not in configured] + if dropped: + verbose_logger.debug( + "MCP server %s: dropped %d requested OAuth scopes outside the configured scopes", + server.server_id, + len(dropped), + ) + return " ".join(scope for scope in scopes if scope in configured) or None + + +def _default_scope(server: MCPServer) -> str | None: + return " ".join(server.scopes) if server.scopes else None + + def _redirect_to_upstream_authorize( *, mcp_server: MCPServer, @@ -939,7 +958,7 @@ def _redirect_to_upstream_authorize( """The bridge relay arm's authorize redirect: every client-supplied parameter passes through to the upstream authorize endpoint verbatim, no relay state cookie is set, and the upstream enforces its own registered redirect binding for the client.""" - scope_value: Final = scope or (" ".join(mcp_server.scopes) if mcp_server.scopes else None) + scope_value: Final = _cap_requested_scopes(scope, mcp_server) or _default_scope(mcp_server) upstream_resource: Final = resolve_upstream_resource(mcp_server) passthrough_params: Final = { "client_id": client_id, @@ -1038,7 +1057,8 @@ async def _managed_authorization_scope( if scope_user_id and prisma_client is not None else None ) - merged_scope: Final = union_scopes(" ".join(prior_grant.get("scopes") or ()) if prior_grant else None, scope) + prior_scope: Final = " ".join(prior_grant.get("scopes") or ()) if prior_grant else None + merged_scope: Final = union_scopes(_cap_requested_scopes(prior_scope, server), scope) selected_scope: Final = ( get_client_metadata_scopes( merged_scope, @@ -1145,7 +1165,7 @@ async def authorize_with_server( managed_per_user: Final = resolved_server.is_gateway_managed_oauth2 and resolved_server.needs_user_oauth_token initial_scope: Final = union_scopes( "openid" if enforce_binding else None, - scope or (" ".join(resolved_server.scopes) if resolved_server.scopes else None), + _cap_requested_scopes(scope, resolved_server) or _default_scope(resolved_server), ) selected_scope, scope_user_id = ( await _managed_authorization_scope(request, resolved_server, initial_scope, litellm_user_id) @@ -1376,7 +1396,12 @@ async def exchange_token_with_server( "refresh_token": upstream_refresh_token, **token_request.body, } - refresh_request_scope = scope or bridge_upstream_scope + client_refresh_scope: Final = scope or bridge_upstream_scope + refresh_request_scope = ( + _cap_requested_scopes(client_refresh_scope, resolved_server) or _default_scope(resolved_server) + if client_refresh_scope + else None + ) if refresh_request_scope: token_data["scope"] = refresh_request_scope refresh_ownership = ( # rebind-ok: grant-specific branches assign one ownership value diff --git a/tests/unit/proxy/_experimental/mcp_server/test_discoverable_endpoints.py b/tests/unit/proxy/_experimental/mcp_server/test_discoverable_endpoints.py index a43dacaf100..d29ecc6ead2 100644 --- a/tests/unit/proxy/_experimental/mcp_server/test_discoverable_endpoints.py +++ b/tests/unit/proxy/_experimental/mcp_server/test_discoverable_endpoints.py @@ -5076,60 +5076,126 @@ async def test_oauth_authorize_includes_scopes_from_server_config(): ) -@pytest.mark.asyncio -async def test_oauth_authorize_prefers_request_scope_over_server_config(): - """Test that explicit scope parameter takes precedence over server configuration.""" - try: - from fastapi import Request +_READ_SCOPES: Final = ("ir.incidents:read", "ir.services:read") - from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( - authorize_with_server, - ) - from litellm.proxy._types import MCPTransport - from litellm.types.mcp import MCPAuth - from litellm.types.mcp_server.mcp_server_manager import MCPServer - except ImportError: - pytest.skip("MCP discoverable endpoints not available") - oauth_server = MCPServer( - server_id="test_server", - name="test", - server_name="test", - transport=MCPTransport.http, - auth_type=MCPAuth.oauth2, - authorization_url="https://provider.com/oauth/authorize", - token_url="https://provider.com/oauth/token", - client_id="test_client", - scopes=["default_scope1", "default_scope2"], - ) +async def _upstream_authorize_scope(server: "MCPServer", client_scope: str | None) -> list[str] | None: + from urllib.parse import parse_qs, urlparse + + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import authorize_with_server mock_request = MagicMock(spec=Request) mock_request.base_url = "https://litellm.example.com/" mock_request.headers = {} - - with patch("litellm.proxy._experimental.mcp_server.discoverable_endpoints.encrypt_value_helper") as mock_encrypt: - mock_encrypt.return_value = "encrypted_state" - - # Call authorize WITH explicit scope parameter + with patch( # test-quality-ok: state sealing is unrelated to which scope reaches the upstream + "litellm.proxy._experimental.mcp_server.discoverable_endpoints.encrypt_value_helper", + return_value="encrypted_state", + ): response = await authorize_with_server( request=mock_request, - mcp_server=oauth_server, - client_id="test_client", + mcp_server=server, + client_id="caller-client", redirect_uri="http://localhost:3000/callback", - state="test_state", - code_challenge="test_challenge", + state="client-state", + code_challenge="challenge", code_challenge_method="S256", - response_type="code", - scope="custom_scope1 custom_scope2", # Explicit scope should take precedence + scope=client_scope, ) + scope_values = parse_qs(urlparse(response.headers["location"]).query).get("scope") + return None if scope_values is None else scope_values[0].split() - # Should use the explicit scope, not server config - assert response.status_code in (307, 302) - redirect_url = response.headers["location"] - assert ( - "scope=custom_scope1+custom_scope2" in redirect_url or "scope=custom_scope1%20custom_scope2" in redirect_url - ) - assert "default_scope" not in redirect_url + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "client_scope, expected", + [ + ("all", list(_READ_SCOPES)), + ("ir.incidents:read", ["ir.incidents:read"]), + ("ir.incidents:read ir.incidents:write", ["ir.incidents:read"]), + ("ir.incidents:write admin", list(_READ_SCOPES)), + (None, list(_READ_SCOPES)), + ], +) +async def test_authorize_never_requests_a_scope_outside_the_configured_scopes(client_scope, expected): + server = _resource_server(scopes=list(_READ_SCOPES), configured_scopes=_READ_SCOPES) + + assert await _upstream_authorize_scope(server, client_scope) == expected + + +@pytest.mark.asyncio +async def test_authorize_passes_client_scope_through_when_no_scopes_are_configured(): + discovered_only = _resource_server(scopes=["discovered:read"], configured_scopes=None) + + assert await _upstream_authorize_scope(discovered_only, "custom_scope1 custom_scope2") == [ + "custom_scope1", + "custom_scope2", + ] + + +@pytest.mark.asyncio +@pytest.mark.parametrize("client_scope", ["all", None]) +async def test_bridge_relay_authorize_never_requests_a_scope_outside_the_configured_scopes(client_scope): + from urllib.parse import parse_qs, urlparse + + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import authorize_with_server + + response = await authorize_with_server( + request=_bridge_mock_request(), + mcp_server=_bridge_server( + auth_type=MCPAuth.oauth_delegate, scopes=list(_READ_SCOPES), configured_scopes=_READ_SCOPES + ), + client_id="dcr-client-123", + redirect_uri=_BRIDGE_CLIENT_REDIRECT, + state="client-state", + code_challenge="chal", + code_challenge_method="S256", + scope=client_scope, + ) + + assert parse_qs(urlparse(response.headers["location"]).query)["scope"] == [" ".join(_READ_SCOPES)] + + +@pytest.mark.asyncio +async def test_authorize_with_enforced_identity_binding_adds_only_openid_beyond_the_configured_scopes(): + from litellm.types.mcp_server.mcp_server_manager import MCPOAuthIdentityBinding + + server = _resource_server( + scopes=list(_READ_SCOPES), + configured_scopes=_READ_SCOPES, + oauth_identity_binding=MCPOAuthIdentityBinding( + mode="enforce", issuer="https://idp.example.com", audiences=["gateway-client"] + ), + ) + with ( + patch( # test-quality-ok: isolate caller resolution from the scope the upstream is asked for + "litellm.proxy._experimental.mcp_server.discoverable_endpoints._resolve_oauth_authorization_user", + new=AsyncMock(return_value="alice"), + ), + ): + requested = await _upstream_authorize_scope(server, "all ir.incidents:read") + + assert requested == ["openid", "ir.incidents:read"] + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "client_scope, dropped_count", [("all ir.incidents:read", 1), ("all admin ir.incidents:read", 2)] +) +async def test_authorize_logs_dropped_scope_count_and_server_but_not_client_credentials( + caplog, client_scope, dropped_count +): + import logging + + server = _resource_server(scopes=list(_READ_SCOPES), configured_scopes=_READ_SCOPES) + with caplog.at_level(logging.DEBUG, logger="LiteLLM"): + await _upstream_authorize_scope(server, client_scope) + + dropped_logs = [record for record in caplog.records if "outside the configured" in record.getMessage()] + assert [record.levelno for record in dropped_logs] == [logging.DEBUG], caplog.text + message = dropped_logs[0].getMessage() + assert f"dropped {dropped_count} requested" in message and "res-srv" in message + assert "ir.incidents:read" not in message + assert "gateway-client" not in message and "gateway-secret" not in message @pytest.mark.asyncio @@ -7190,6 +7256,73 @@ async def test_bridge_refresh_re_requests_the_sealed_scope_when_client_omits_it( assert captured["client"].post.call_args.kwargs["data"]["scope"] == "read:tools write:tools" +@pytest.mark.asyncio +@pytest.mark.parametrize( + "sealed_scope, client_scope, expected_upstream_scope", + [ + ("ir.incidents:read ir.incidents:write", None, "ir.incidents:read"), + ("ir.incidents:read", "all ir.incidents:read", "ir.incidents:read"), + (None, "all", " ".join(_READ_SCOPES)), + ("ir.incidents:write", None, " ".join(_READ_SCOPES)), + (None, None, None), + ], +) +async def test_bridge_refresh_never_requests_a_scope_outside_the_configured_scopes( + sealed_scope, client_scope, expected_upstream_scope +): + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import exchange_token_with_server + + server = _bridge_server(auth_type=MCPAuth.oauth_delegate, scopes=list(_READ_SCOPES), configured_scopes=_READ_SCOPES) + refresh_env = _mint_test_refresh_envelope(server_id=server.server_id, upstream_refresh="UP", scope=sealed_scope) + upstream: Final = MagicMock() + upstream.json.return_value = {"access_token": "NEW", "token_type": "Bearer", "expires_in": 3600} + http_client: Final = MagicMock(post=AsyncMock(return_value=upstream)) + with ( + patch( # test-quality-ok: fake the upstream token endpoint at the HTTP client boundary + "litellm.proxy._experimental.mcp_server.discoverable_endpoints.get_async_httpx_client", + return_value=http_client, + ), + patch( # test-quality-ok: the sealed key is active; revocation is covered by its own tests + "litellm.proxy._experimental.mcp_server.bridge_token_flow._revalidate_active_subject", + new=AsyncMock(return_value=None), + ), + patch("litellm.proxy.proxy_server.master_key", _BRIDGE_MASTER_KEY), + ): + response = await exchange_token_with_server( + request=_bridge_mock_request(), + mcp_server=server, + grant_type="refresh_token", + code=None, + redirect_uri=None, + client_id="dcr-client-123", + client_secret=None, + code_verifier=None, + refresh_token=refresh_env, + scope=client_scope, + ) + + assert response.status_code == 200, response.body + assert http_client.post.call_args.kwargs["data"].get("scope") == expected_upstream_scope + + +@pytest.mark.asyncio +async def test_refresh_passes_client_scope_through_when_no_scopes_are_configured(): + server = _bridge_server(auth_type=MCPAuth.oauth_delegate, scopes=["discovered:read"], configured_scopes=None) + refresh_env = _mint_test_refresh_envelope(server_id=server.server_id, upstream_refresh="UP", scope="a b") + captured: dict = {} + + response = await _refresh_for_bridge_server( + server, + refresh_env, + {"access_token": "NEW", "token_type": "Bearer", "expires_in": 3600}, + None, + fake_client_out=captured, + ) + + assert response.status_code == 200, response.body + assert captured["client"].post.call_args.kwargs["data"]["scope"] == "a b" + + @pytest.mark.asyncio async def test_bridge_refresh_re_seals_scope_when_upstream_omits_it_so_the_chain_keeps_it(): """RFC 6749 5.1 lets an upstream omit scope in a refresh response when it is unchanged. The re-minted @@ -14833,10 +14966,16 @@ async def test_saved_refresh_metadata_is_bound_to_the_callers_stored_grant( @pytest.mark.asyncio @pytest.mark.parametrize( - "prior_scopes, expected", [(["tools.read"], "tools.read tools.write"), (None, "tools.write"), ([], "tools.write")] + "prior_scopes, configured, expected", + [ + (["tools.read"], None, "tools.read tools.write"), + (None, None, "tools.write"), + ([], None, "tools.write"), + (["tools.read", "admin"], ("tools.read", "tools.write"), "tools.read tools.write"), + ], ) async def test_managed_authorize_retains_the_callers_previous_grant( - jwt_oauth_identity, monkeypatch, prior_scopes, expected + jwt_oauth_identity, monkeypatch, prior_scopes, configured, expected ): from types import SimpleNamespace from urllib.parse import parse_qs, urlsplit @@ -14848,7 +14987,9 @@ async def test_managed_authorize_retains_the_callers_previous_grant( _, signing_key = jwt_oauth_identity monkeypatch.setenv("LITELLM_SALT_KEY", "scope-grant-test-salt") - server = _cimd_oauth_server().model_copy(update={"client_id": "scope-client", "scopes": ["tools.read"]}) + server = _cimd_oauth_server().model_copy( + update={"client_id": "scope-client", "scopes": ["tools.read"], "configured_scopes": configured} + ) global_mcp_server_manager.registry[server.server_id] = server credential = db.encrypt_value_helper( json.dumps({"type": "oauth2", "access_token": "old-access", "scopes": prior_scopes})