diff --git a/litellm/proxy/_types.py b/litellm/proxy/_types.py index 07d4859220c..9a7457b829b 100644 --- a/litellm/proxy/_types.py +++ b/litellm/proxy/_types.py @@ -1831,8 +1831,8 @@ class LiteLLM_ObjectPermissionTable(LiteLLMPydanticObjectBase): """Represents a LiteLLM_ObjectPermissionTable record""" object_permission_id: str - mcp_servers: Optional[List[str]] = None - mcp_access_groups: Optional[List[str]] = None + mcp_servers: Optional[List[str]] = [] + mcp_access_groups: Optional[List[str]] = [] mcp_tool_permissions: Optional[Dict[str, List[str]]] = None """ Mapping - server_id -> list of tools @@ -1843,10 +1843,10 @@ class LiteLLM_ObjectPermissionTable(LiteLLMPydanticObjectBase): } """ - vector_stores: Optional[List[str]] = None - search_tools: Optional[List[str]] = None - agents: Optional[List[str]] = None - agent_access_groups: Optional[List[str]] = None + vector_stores: Optional[List[str]] = [] + search_tools: Optional[List[str]] = None # NULL = all access, [] = no access + agents: Optional[List[str]] = [] + agent_access_groups: Optional[List[str]] = [] class LiteLLM_TeamTable(TeamBase): @@ -3599,6 +3599,9 @@ class ProxyErrorTypes(str, enum.Enum): return cls.team_search_tool_access_denied elif object_type == "org": return cls.org_search_tool_access_denied + raise ValueError( + f"Unknown object_type '{object_type}' for search tool access error" + ) DB_CONNECTION_ERROR_TYPES = ( diff --git a/litellm/proxy/auth/auth_checks.py b/litellm/proxy/auth/auth_checks.py index 06c6ddd714b..1b7bd645a14 100644 --- a/litellm/proxy/auth/auth_checks.py +++ b/litellm/proxy/auth/auth_checks.py @@ -3543,16 +3543,9 @@ def _can_object_call_vector_stores( if object_permissions.vector_stores is None: return True - # Empty list = no access to any vector stores + # If length is 0, then the object has access to all vector stores. if len(object_permissions.vector_stores) == 0: - raise ProxyException( - message="User not allowed to access any vector stores. No vector stores are configured in object permissions.", - type=ProxyErrorTypes.get_vector_store_access_error_type_for_object( - object_type - ), - param="vector_store", - code=status.HTTP_401_UNAUTHORIZED, - ) + return True for vector_store_id in vector_store_ids_to_run: if vector_store_id not in object_permissions.vector_stores: diff --git a/litellm/proxy/management_endpoints/common_utils.py b/litellm/proxy/management_endpoints/common_utils.py index 06115993245..efc42d3355c 100644 --- a/litellm/proxy/management_endpoints/common_utils.py +++ b/litellm/proxy/management_endpoints/common_utils.py @@ -15,7 +15,6 @@ from litellm.proxy._types import ( UpdateProjectRequest, UserAPIKeyAuth, ) -from litellm.proxy.auth.auth_checks import get_user_object from litellm.proxy.utils import _premium_user_check if TYPE_CHECKING: @@ -55,6 +54,7 @@ async def _is_user_org_admin_for_team( if not team_obj.organization_id or not user_api_key_dict.user_id: return False + from litellm.proxy.auth.auth_checks import get_user_object from litellm.proxy.proxy_server import ( prisma_client, proxy_logging_obj, @@ -124,11 +124,14 @@ async def _user_has_admin_privileges( return False # Get user object to check team and org admin status + from litellm.caching import DualCache as DualCacheImport + from litellm.proxy.auth.auth_checks import get_user_object + try: user_obj = await get_user_object( user_id=user_api_key_dict.user_id, prisma_client=prisma_client, - user_api_key_cache=user_api_key_cache or DualCache(), + user_api_key_cache=user_api_key_cache or DualCacheImport(), user_id_upsert=False, proxy_logging_obj=proxy_logging_obj, ) @@ -270,8 +273,11 @@ async def admin_can_invite_user( if prisma_client is None or user_api_key_dict.user_id is None: return False + from litellm.caching import DualCache as DualCacheImport + from litellm.proxy.auth.auth_checks import get_user_object + try: - cache = user_api_key_cache or DualCache() + cache = user_api_key_cache or DualCacheImport() admin_user_obj = await get_user_object( user_id=user_api_key_dict.user_id, prisma_client=prisma_client, diff --git a/litellm/proxy/management_endpoints/mcp_management_endpoints.py b/litellm/proxy/management_endpoints/mcp_management_endpoints.py index 0080e5b7887..3e5b729cea6 100644 --- a/litellm/proxy/management_endpoints/mcp_management_endpoints.py +++ b/litellm/proxy/management_endpoints/mcp_management_endpoints.py @@ -130,15 +130,7 @@ if MCP_AVAILABLE: ) from litellm.proxy.auth.user_api_key_auth import user_api_key_auth from litellm.proxy.common_utils.http_parsing_utils import _read_request_body - from litellm.proxy.management_endpoints.common_utils import ( - _user_has_admin_view, - check_member_permission, - ) - from litellm.proxy.management_helpers.object_permission_utils import ( - _get_team_allowed_mcp_servers, - add_mcp_server_to_team, - remove_mcp_server_from_team, - ) + from litellm.proxy.management_endpoints.common_utils import _user_has_admin_view from litellm.proxy.management_helpers.utils import management_endpoint_wrapper from litellm.types.mcp import MCPCredentials from litellm.types.mcp_server.mcp_server_manager import MCPServer @@ -1214,50 +1206,15 @@ if MCP_AVAILABLE: # Validate and normalize payload fields validate_and_normalize_mcp_server_payload(payload) - # AuthZ - proxy admins, team admins, or members with mcp:create permission - from litellm.proxy.auth.auth_checks import get_team_object - from litellm.proxy.proxy_server import user_api_key_cache - - team_obj = None - team_id = payload.team_id or user_api_key_dict.team_id - + # AuthZ - restrict only proxy admins to create mcp servers if LitellmUserRoles.PROXY_ADMIN != user_api_key_dict.user_role: - if not team_id: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail={ - "error": "team_id is required for non-proxy-admin users to create MCP servers." - }, - ) - team_obj = await get_team_object( - team_id=team_id, - prisma_client=prisma_client, - user_api_key_cache=user_api_key_cache, - ) - if not check_member_permission( - user_api_key_dict, team_obj, "mcp:create" - ): - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail={ - "error": "User does not have permission to create MCP servers for this team. " - "Requires team admin role or 'mcp:create' permission." - }, - ) - - # Reject special reserved server names - if ( - SpecialMCPServerName.all_team_servers == payload.server_id - or SpecialMCPServerName.all_proxy_servers == payload.server_id - ): raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, + status_code=status.HTTP_403_FORBIDDEN, detail={ - "error": f"MCP Server with id {payload.server_id} is special and cannot be used." + "error": "User does not have permission to create mcp servers. You can only create mcp servers if you are a PROXY_ADMIN." }, ) - - if payload.server_id is not None: + elif payload.server_id is not None: # fail if the mcp server with id already exists mcp_server = await get_mcp_server(prisma_client, payload.server_id) if mcp_server is not None: @@ -1267,6 +1224,16 @@ if MCP_AVAILABLE: "error": f"MCP Server with id {payload.server_id} already exists. Cannot create another." }, ) + elif ( + SpecialMCPServerName.all_team_servers == payload.server_id + or SpecialMCPServerName.all_proxy_servers == payload.server_id + ): + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail={ + "error": f"MCP Server with id {payload.server_id} is special and cannot be used." + }, + ) # TODO: audit log for create @@ -1294,20 +1261,6 @@ if MCP_AVAILABLE: status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail={"error": f"Error creating mcp server: {str(e)}"}, ) - - # Auto-assign server to team's ObjectPermissionTable if team-scoped - if team_id and new_mcp_server.server_id: - try: - await add_mcp_server_to_team( - prisma_client, team_id, new_mcp_server.server_id - ) - except ValueError as e: - # Team not found — surface as 400 so caller knows - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail={"error": str(e)}, - ) - return _redact_mcp_credentials(new_mcp_server) @router.post( @@ -1492,7 +1445,7 @@ if MCP_AVAILABLE: @router.delete( "/server/{server_id}", - description="Allows deleting mcp servers in the db", + description="Allows deleting mcp serves in the db", dependencies=[Depends(user_api_key_auth)], response_class=JSONResponse, status_code=status.HTTP_202_ACCEPTED, @@ -1520,44 +1473,16 @@ if MCP_AVAILABLE: "Database not connected. Connect a database to your proxy - https://docs.litellm.ai/docs/simple_proxy#managing-auth---virtual-keys" ) - # AuthZ - proxy admins, team admins, or members with mcp:delete permission - from litellm.proxy.auth.auth_checks import get_team_object - from litellm.proxy.proxy_server import user_api_key_cache - - team_id = user_api_key_dict.team_id - + # Authz - restrict only admins to delete mcp servers if LitellmUserRoles.PROXY_ADMIN != user_api_key_dict.user_role: - if not team_id: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail={ - "error": "team_id is required for non-proxy-admin users to delete MCP servers." - }, - ) - team_obj = await get_team_object( - team_id=team_id, - prisma_client=prisma_client, - user_api_key_cache=user_api_key_cache, + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail={ + "error": "Call not allowed to delete MCP server. User is not a proxy admin. route={}".format( + "DELETE /v1/mcp/server" + ) + }, ) - if not check_member_permission( - user_api_key_dict, team_obj, "mcp:delete" - ): - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail={ - "error": "User does not have permission to delete MCP servers for this team. " - "Requires team admin role or 'mcp:delete' permission." - }, - ) - # Verify server belongs to this team - team_servers = await _get_team_allowed_mcp_servers(team_obj) - if server_id not in team_servers: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail={ - "error": f"MCP Server {server_id} does not belong to your team." - }, - ) # try to delete the mcp server mcp_server_record_deleted = await delete_mcp_server(prisma_client, server_id) @@ -1572,20 +1497,16 @@ if MCP_AVAILABLE: # Ensure registry is up to date by reloading from database await global_mcp_server_manager.reload_servers_from_database() - # Remove server from team's ObjectPermissionTable - if team_id: - try: - await remove_mcp_server_from_team(prisma_client, team_id, server_id) - except Exception as e: - verbose_proxy_logger.warning( - f"Failed to remove server {server_id} from team {team_id} permissions: {e}. " - "Server was deleted but team's ObjectPermissionTable may contain a stale entry." - ) - # TODO: Enterprise: Finish audit log trail if litellm.store_audit_logs: pass + # TODO: Delete from virtual keys + + # TODO: Delete from teams + + # Update from global mcp store + return Response(status_code=status.HTTP_202_ACCEPTED) @router.post( @@ -1846,7 +1767,7 @@ if MCP_AVAILABLE: @router.put( "/server", - description="Allows updating mcp servers in the db", + description="Allows deleting mcp serves in the db", dependencies=[Depends(user_api_key_auth)], response_model=LiteLLM_MCPServerTable, status_code=status.HTTP_202_ACCEPTED, @@ -1877,50 +1798,16 @@ if MCP_AVAILABLE: # Validate and normalize payload fields validate_and_normalize_mcp_server_payload(payload) - # AuthZ - proxy admins, team admins, or members with mcp:update permission - from litellm.proxy.auth.auth_checks import get_team_object - from litellm.proxy.proxy_server import user_api_key_cache - + # Authz - restrict only admins to delete mcp servers if LitellmUserRoles.PROXY_ADMIN != user_api_key_dict.user_role: - team_id = user_api_key_dict.team_id - if not team_id: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail={ - "error": "team_id is required for non-proxy-admin users to update MCP servers." - }, - ) - team_obj = await get_team_object( - team_id=team_id, - prisma_client=prisma_client, - user_api_key_cache=user_api_key_cache, + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail={ + "error": "Call not allowed to update MCP server. User is not a proxy admin. route={}".format( + "PUT /v1/mcp/server" + ) + }, ) - if not check_member_permission( - user_api_key_dict, team_obj, "mcp:update" - ): - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail={ - "error": "User does not have permission to update MCP servers for this team. " - "Requires team admin role or 'mcp:update' permission." - }, - ) - # Verify server_id is provided and belongs to this team - if payload.server_id is None: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail={ - "error": "server_id is required to update an MCP server." - }, - ) - team_servers = await _get_team_allowed_mcp_servers(team_obj) - if payload.server_id not in team_servers: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail={ - "error": f"MCP Server {payload.server_id} does not belong to your team." - }, - ) # try to update the mcp server mcp_server_record_updated = await update_mcp_server( diff --git a/tests/test_litellm/proxy/auth/test_search_tool_access.py b/tests/test_litellm/proxy/auth/test_search_tool_access.py index 06797760c8d..e915a9b3ba0 100644 --- a/tests/test_litellm/proxy/auth/test_search_tool_access.py +++ b/tests/test_litellm/proxy/auth/test_search_tool_access.py @@ -13,7 +13,6 @@ from typing import List, Optional from unittest.mock import AsyncMock, MagicMock, patch import pytest -from starlette import status from litellm.proxy._types import ( LiteLLM_ObjectPermissionTable, @@ -376,18 +375,18 @@ class TestSearchToolErrorTypes: class TestVectorStoreAccessNotBroken: - """Vector store access follows same semantics: None = all access, [] = no access.""" + """Preserves existing vector store semantics: empty list = allow ALL.""" - def test_should_deny_all_when_vector_stores_is_empty(self): - """Vector stores: empty list = no access (matches search_tools semantics).""" + def test_should_allow_all_when_vector_stores_is_empty(self): + """Vector stores: empty list = access to ALL (existing behavior).""" perm = MagicMock() perm.vector_stores = [] - with pytest.raises(ProxyException): - _can_object_call_vector_stores( - object_type="key", - vector_store_ids_to_run=["store-1"], - object_permissions=perm, - ) + result = _can_object_call_vector_stores( + object_type="key", + vector_store_ids_to_run=["store-1"], + object_permissions=perm, + ) + assert result is True def test_should_allow_when_vector_stores_is_none(self): perm = MagicMock()