diff --git a/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py b/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py index c088602d9f7..ab62a8fce35 100644 --- a/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py +++ b/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py @@ -4092,8 +4092,8 @@ class MCPRequestHandler: """Whether this principal's admin-view role grants the unscoped MCP resolution, whatever credential carries it (admin key, dashboard session, or OAuth-admitted session subject). - A virtual key on a real team never qualifies: its team bounds its MCP access whoever owns it, as it - bounds the key's model access. Two more bounds disqualify, one per ownership of the row. A CREDENTIAL's + A credential on a real team never qualifies, a virtual key or a ``lite login`` session alike: its team + bounds its MCP access whoever owns it, as it bounds the credential's model access. Two more bounds disqualify, one per ownership of the row. A CREDENTIAL's explicit ``object_permission.mcp_servers`` scope wins even for admins, including the empty list. An admitted subject's object_permission is the user's own row, whose ``mcp_servers`` column is [] by DB default, so for that shape the row binds through the entitlement ceiling instead (any non-empty diff --git a/litellm/responses/mcp/litellm_proxy_mcp_handler.py b/litellm/responses/mcp/litellm_proxy_mcp_handler.py index 4f2bed71148..9f17f5f64df 100644 --- a/litellm/responses/mcp/litellm_proxy_mcp_handler.py +++ b/litellm/responses/mcp/litellm_proxy_mcp_handler.py @@ -281,21 +281,22 @@ class LiteLLM_Proxy_MCP_Handler: if prisma_client is not None: toolset = await global_mcp_server_manager.get_toolset_by_name_cached(prisma_client, name) if toolset is not None: - if user_api_key_auth is not None: - from litellm.proxy._experimental.mcp_server.auth.user_api_key_auth_mcp import ( - has_team_unbound_mcp_admin_view, - ) - from litellm.proxy._experimental.mcp_server.ui_session_utils import ( - granted_toolset_ids, - ) + from litellm.proxy._experimental.mcp_server.auth.user_api_key_auth_mcp import ( + has_team_unbound_mcp_admin_view, + ) + from litellm.proxy._experimental.mcp_server.ui_session_utils import ( + granted_toolset_ids, + ) - if not has_team_unbound_mcp_admin_view( - user_api_key_auth - ) and toolset.toolset_id not in ( - await (granted_toolsets or granted_toolset_ids)(user_api_key_auth) - ): - verbose_logger.debug("Key does not have access to toolset '%s', skipping.", name) - continue + if ( + user_api_key_auth is not None + and not has_team_unbound_mcp_admin_view(user_api_key_auth) + and toolset.toolset_id + not in await (granted_toolsets or granted_toolset_ids)(user_api_key_auth) + ): + verbose_logger.debug("Key does not have access to toolset '%s', denying it.", name) + resolved_mcp_servers.append(name) + continue resolved_toolset_ids.append(toolset.toolset_id) # Don't add to resolved_mcp_servers — toolset scope # restricts via object_permission, not server name filter. diff --git a/tests/unit/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py b/tests/unit/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py index 6a23fc6bf17..9a8217c552c 100644 --- a/tests/unit/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py +++ b/tests/unit/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py @@ -12699,12 +12699,14 @@ async def test_personal_key_mcp_access_is_denied_when_owner_teams_cannot_be_read [("team-a", {"srv-a", "srv-open"}), ("team-without-mcp", {"srv-open"})], ids=["team_grants_one_server", "team_grants_no_server"], ) +@pytest.mark.parametrize("is_session_token", [False, True], ids=["virtual_key", "cli_session"]) async def test_admin_owned_team_key_reaches_only_its_team_mcp_servers( monkeypatch: pytest.MonkeyPatch, personal_key_owner_teams: UserAPIKeyAuth, owner_role: LitellmUserRoles, key_team_id: str, expected_servers: set[str], + is_session_token: bool, ) -> None: from litellm.proxy._experimental.mcp_server.mcp_server_manager import global_mcp_server_manager from litellm.proxy.auth import auth_checks @@ -12713,7 +12715,9 @@ async def test_admin_owned_team_key_reaches_only_its_team_mcp_servers( return _personal_key_team(team_id) if team_id in _PERSONAL_KEY_TEAMS else LiteLLM_TeamTable(team_id=team_id) monkeypatch.setattr(auth_checks, "get_team_object", _get_team_object) - team_key: Final = personal_key_owner_teams.model_copy(update={"team_id": key_team_id, "user_role": owner_role}) + team_key: Final = personal_key_owner_teams.model_copy( + update={"team_id": key_team_id, "user_role": owner_role, "is_session_token": is_session_token} + ) assert set(await global_mcp_server_manager.get_allowed_mcp_servers(team_key)) == expected_servers diff --git a/tests/unit/proxy/_experimental/mcp_server/test_mcp_toolset_scope.py b/tests/unit/proxy/_experimental/mcp_server/test_mcp_toolset_scope.py index cc8d2c14e9a..e20870ecffa 100644 --- a/tests/unit/proxy/_experimental/mcp_server/test_mcp_toolset_scope.py +++ b/tests/unit/proxy/_experimental/mcp_server/test_mcp_toolset_scope.py @@ -118,19 +118,14 @@ class TestApplyToolsetScope: [LitellmUserRoles.PROXY_ADMIN, LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY], ids=["proxy_admin", "proxy_admin_viewer"], ) - async def test_admin_owned_team_key_without_the_toolset_grant_is_denied(self, owner_role): + async def test_admin_owned_team_key_without_the_toolset_grant_is_denied(self, owner_role: LitellmUserRoles): from litellm.proxy._experimental.mcp_server.server import apply_toolset_scope team_key: Final = UserAPIKeyAuth( api_key="sk-test", team_id="team-a", user_role=owner_role, object_permission=None ) - with patch( - "litellm.proxy._experimental.mcp_server.server." - "global_mcp_server_manager.resolve_toolset_tool_permissions", - new=AsyncMock(return_value={"server-a": ["tool1"]}), - ): - with pytest.raises(HTTPException) as exc_info: - await apply_toolset_scope(team_key, "toolset-123", granted=_granted_through_team("toolset-other")) + with pytest.raises(HTTPException) as exc_info: + await apply_toolset_scope(team_key, "toolset-123", granted=_granted_through_team("toolset-other")) assert exc_info.value.status_code == 403 diff --git a/tests/unit/responses/mcp/test_litellm_proxy_mcp_handler.py b/tests/unit/responses/mcp/test_litellm_proxy_mcp_handler.py index 1d47fed7a80..3916def3e4a 100644 --- a/tests/unit/responses/mcp/test_litellm_proxy_mcp_handler.py +++ b/tests/unit/responses/mcp/test_litellm_proxy_mcp_handler.py @@ -1553,12 +1553,14 @@ async def test_toolset_gateway_url_scopes_a_team_granted_toolset_for_a_key_witho [LitellmUserRoles.INTERNAL_USER, LitellmUserRoles.PROXY_ADMIN, LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY], ids=["internal_user", "proxy_admin", "proxy_admin_viewer"], ) -async def test_toolset_gateway_url_skips_a_toolset_the_team_does_not_grant(monkeypatch, owner_role): +async def test_toolset_gateway_url_lists_nothing_beyond_a_toolset_the_team_does_not_grant( + monkeypatch: pytest.MonkeyPatch, owner_role: LitellmUserRoles +): kwargs: Final = await _tools_listing_kwargs_for_toolset_url( monkeypatch, team_toolset_id="ts-other", owner_role=owner_role ) assert kwargs["user_api_key_auth"].object_permission is None - assert kwargs["mcp_servers"] is None + assert kwargs["mcp_servers"] == ["team-toolset"] @pytest.mark.asyncio