diff --git a/litellm/proxy/management_endpoints/common_utils.py b/litellm/proxy/management_endpoints/common_utils.py index 2241884faf1..d05c03112bc 100644 --- a/litellm/proxy/management_endpoints/common_utils.py +++ b/litellm/proxy/management_endpoints/common_utils.py @@ -1,5 +1,7 @@ import math -from typing import TYPE_CHECKING, Any, Final, Optional, Union +from collections.abc import Mapping +from types import MappingProxyType +from typing import TYPE_CHECKING, Any, Final, Literal, Optional, Union from fastapi import HTTPException, status from pydantic import BaseModel @@ -175,6 +177,59 @@ async def _is_user_org_admin_for_team(user_api_key_dict: UserAPIKeyAuth, team_ob return False +TeamMembershipAction = Literal["add", "delete"] + +_TEAM_MEMBERSHIP_LOCKDOWN_SETTINGS: Final[Mapping[TeamMembershipAction, str]] = MappingProxyType( + { + "add": "disable_team_admin_add_team_user", + "delete": "disable_team_admin_delete_team_user", + } +) + +_TEAM_MEMBERSHIP_ACTION_DESCRIPTIONS: Final[Mapping[TeamMembershipAction, str]] = MappingProxyType( + { + "add": "adding members to", + "delete": "removing members from", + } +) + + +async def check_team_admin_can_manage_team_membership( + user_api_key_dict: UserAPIKeyAuth, + team_obj: LiteLLM_TeamTable, + action: TeamMembershipAction, +) -> None: + """ + Raise HTTP 403 when the caller's only authority over the team is team admin and the + matching lockdown setting is on, so deployments that provision membership externally + (e.g. SCIM) keep the roster authoritative. Proxy and org admins stay unrestricted. + """ + if user_api_key_dict.user_role in ( + LitellmUserRoles.PROXY_ADMIN, + LitellmUserRoles.PROXY_ADMIN.value, + ): + return + + from litellm.proxy.proxy_server import general_settings + + if not general_settings.get(_TEAM_MEMBERSHIP_LOCKDOWN_SETTINGS[action], False): + return + + if not _is_user_team_admin(user_api_key_dict=user_api_key_dict, team_obj=team_obj): + return + + if await _is_user_org_admin_for_team(user_api_key_dict=user_api_key_dict, team_obj=team_obj): + return + + raise HTTPException( + status_code=403, + detail=( + f"Team admins are blocked from {_TEAM_MEMBERSHIP_ACTION_DESCRIPTIONS[action]} " + f"team_id={team_obj.team_id}. Contact your proxy admin." + ), + ) + + def _team_member_has_permission( user_api_key_dict: UserAPIKeyAuth, team_obj: LiteLLM_TeamTable, diff --git a/litellm/proxy/management_endpoints/team_endpoints.py b/litellm/proxy/management_endpoints/team_endpoints.py index f2327b18914..4d1ff49d8e9 100644 --- a/litellm/proxy/management_endpoints/team_endpoints.py +++ b/litellm/proxy/management_endpoints/team_endpoints.py @@ -104,6 +104,7 @@ from litellm.proxy.management_endpoints.common_utils import ( _update_metadata_fields, _upsert_budget_and_membership, _user_has_admin_view, + check_team_admin_can_manage_team_membership, validate_budget_duration, ) from litellm.proxy.management_endpoints.organization_endpoints import ( @@ -2515,6 +2516,11 @@ async def _validate_team_member_add_permissions( if getattr(user_api_key_dict, "user_role", None) == LitellmUserRoles.PROXY_ADMIN.value: return if _is_user_team_admin(user_api_key_dict=user_api_key_dict, team_obj=complete_team_data): + await check_team_admin_can_manage_team_membership( + user_api_key_dict=user_api_key_dict, + team_obj=complete_team_data, + action="add", + ) return if await _is_user_org_admin_for_team(user_api_key_dict=user_api_key_dict, team_obj=complete_team_data): return @@ -3259,6 +3265,12 @@ async def team_member_delete( }, ) + await check_team_admin_can_manage_team_membership( + user_api_key_dict=user_api_key_dict, + team_obj=existing_team_row, + action="delete", + ) + ## DELETE MEMBER FROM TEAM # Everything from here on runs under the team's advisory lock, the same one # /team/member_add and /team/delete take: without it, this endpoint's own row-level diff --git a/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py b/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py index a1eb7ed06eb..20767db6c28 100644 --- a/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py +++ b/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py @@ -198,6 +198,11 @@ class UISettings(BaseModel): description="Prevents Team Admins from deleting users from the teams they manage. Useful for SCIM provisioning where team membership is defined externally.", ) + disable_team_admin_add_team_user: bool = Field( + default=False, + description="Prevents Team Admins from adding users to the teams they manage. Useful for SCIM provisioning where team membership is defined externally.", + ) + enabled_ui_pages_internal_users: list[str] | None = Field( default=None, description="List of page keys that internal users (non-admins) can see in the UI sidebar. If not set, all pages are visible based on role permissions.", @@ -284,6 +289,7 @@ class UISettingsResponse(SettingsResponse): ALLOWED_UI_SETTINGS_FIELDS: Final = { "disable_model_add_for_internal_users", "disable_team_admin_delete_team_user", + "disable_team_admin_add_team_user", "enabled_ui_pages_internal_users", "require_auth_for_public_ai_hub", "allow_public_health_readiness_details", @@ -323,6 +329,8 @@ def _derived_ui_setting_value(key: str) -> object: # Flags that must be synced from the persisted UISettings into # general_settings at runtime (on both read and write). _RUNTIME_GENERAL_SETTINGS_FLAGS: Final = [ + "disable_team_admin_delete_team_user", + "disable_team_admin_add_team_user", "allow_public_health_readiness_details", "forward_client_headers_to_llm_api", "forward_llm_provider_auth_headers", diff --git a/tests/test_litellm/proxy/management_endpoints/test_common_utils.py b/tests/test_litellm/proxy/management_endpoints/test_common_utils.py index da8fc760787..7e39520b632 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_common_utils.py +++ b/tests/test_litellm/proxy/management_endpoints/test_common_utils.py @@ -11,6 +11,7 @@ from datetime import datetime, timezone from unittest.mock import AsyncMock, MagicMock, patch import pytest +from fastapi import HTTPException from litellm.proxy._types import ( Member, @@ -29,6 +30,7 @@ from litellm.proxy.management_endpoints.common_utils import ( _user_has_admin_privileges, _user_has_admin_view, admin_can_invite_user, + check_team_admin_can_manage_team_membership, ) from litellm.proxy.management_endpoints.common_utils import _has_non_empty_value @@ -980,6 +982,125 @@ class TestUpdateMetadataFieldMove: assert updated_kv["metadata"]["guardrails"] == ["g1"] +class TestCheckTeamAdminCanManageTeamMembership: + """ + Backend enforcement of the SCIM membership lockdown flags. These must hold for a + direct API call, not just for the dashboard buttons. + """ + + _GS_PATH = "litellm.proxy.proxy_server.general_settings" + _CASES = [ + ("add", "disable_team_admin_add_team_user"), + ("delete", "disable_team_admin_delete_team_user"), + ] + + @staticmethod + def _team(organization_id: str | None = None) -> LiteLLM_TeamTable: + return LiteLLM_TeamTable( + team_id="team-1", + organization_id=organization_id, + members_with_roles=[ + Member(user_id="team-admin-user", role="admin"), + Member(user_id="member-user", role="user"), + ], + ) + + @staticmethod + async def _status_code( + user: UserAPIKeyAuth, + team_obj: LiteLLM_TeamTable, + action: str, + ) -> int | None: + """The HTTP status the lockdown check raises, or None when the caller is allowed.""" + try: + await check_team_admin_can_manage_team_membership( + user_api_key_dict=user, team_obj=team_obj, action=action + ) + except HTTPException as exc: + return exc.status_code + return None + + @pytest.mark.parametrize("action, flag", _CASES) + @pytest.mark.asyncio + async def test_team_admin_blocked_when_flag_on(self, action, flag): + user = UserAPIKeyAuth( + user_id="team-admin-user", user_role=LitellmUserRoles.INTERNAL_USER.value + ) + with patch.dict(self._GS_PATH, {flag: True}, clear=True): + status_code = await self._status_code( + user=user, team_obj=self._team(), action=action + ) + assert status_code == 403 + + @pytest.mark.parametrize("action, flag", _CASES) + @pytest.mark.asyncio + async def test_team_admin_allowed_when_flag_off(self, action, flag): + user = UserAPIKeyAuth( + user_id="team-admin-user", user_role=LitellmUserRoles.INTERNAL_USER.value + ) + with patch.dict(self._GS_PATH, {flag: False}, clear=True): + status_code = await self._status_code( + user=user, team_obj=self._team(), action=action + ) + assert status_code is None + + @pytest.mark.parametrize("action, flag", _CASES) + @pytest.mark.asyncio + async def test_flag_only_gates_its_own_action(self, action, flag): + other_action = "delete" if action == "add" else "add" + user = UserAPIKeyAuth( + user_id="team-admin-user", user_role=LitellmUserRoles.INTERNAL_USER.value + ) + with patch.dict(self._GS_PATH, {flag: True}, clear=True): + status_code = await self._status_code( + user=user, team_obj=self._team(), action=other_action + ) + assert status_code is None + + @pytest.mark.parametrize("action, flag", _CASES) + @pytest.mark.asyncio + async def test_proxy_admin_not_blocked(self, action, flag): + user = UserAPIKeyAuth( + user_id="team-admin-user", user_role=LitellmUserRoles.PROXY_ADMIN.value + ) + with patch.dict(self._GS_PATH, {flag: True}, clear=True): + status_code = await self._status_code( + user=user, team_obj=self._team(), action=action + ) + assert status_code is None + + @pytest.mark.parametrize("action, flag", _CASES) + @pytest.mark.asyncio + async def test_caller_without_team_admin_role_passes_through(self, action, flag): + """Other callers are gated by the endpoint's own authorization checks.""" + user = UserAPIKeyAuth( + user_id="someone-else", user_role=LitellmUserRoles.INTERNAL_USER.value + ) + with patch.dict(self._GS_PATH, {flag: True}, clear=True): + status_code = await self._status_code( + user=user, team_obj=self._team(), action=action + ) + assert status_code is None + + @pytest.mark.parametrize("action, flag", _CASES) + @pytest.mark.asyncio + async def test_org_admin_for_team_not_blocked(self, action, flag): + user = UserAPIKeyAuth( + user_id="team-admin-user", user_role=LitellmUserRoles.INTERNAL_USER.value + ) + with patch.dict(self._GS_PATH, {flag: True}, clear=True): + with patch( + "litellm.proxy.management_endpoints.common_utils._is_user_org_admin_for_team", + new=AsyncMock(return_value=True), + ): + status_code = await self._status_code( + user=user, + team_obj=self._team(organization_id="org-1"), + action=action, + ) + assert status_code is None + + class TestHasNonEmptyValue: """Tests for the _has_non_empty_value helper.""" diff --git a/tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py index 6461245bb2c..f68f40999e0 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py @@ -1294,6 +1294,89 @@ async def test_validate_team_member_add_permissions_non_admin(): assert "not proxy admin OR team admin" in str(exc_info.value.detail) +@pytest.mark.asyncio +async def test_team_member_add_blocked_for_team_admin_when_lockdown_enabled(): + """/team/member_add must reject a team admin directly, not just hide the UI button.""" + from litellm.proxy.management_endpoints.team_endpoints import ( + _validate_team_member_add_permissions, + ) + + team_admin = UserAPIKeyAuth( + user_id="team-admin-user", + user_role=LitellmUserRoles.INTERNAL_USER, + ) + + team = MagicMock(spec=LiteLLM_TeamTable) + team.team_id = "scim-team" + team.members_with_roles = [Member(user_id="team-admin-user", role="admin")] + team.organization_id = None + + with patch.dict( + "litellm.proxy.proxy_server.general_settings", + {"disable_team_admin_add_team_user": True}, + clear=True, + ): + with pytest.raises(HTTPException) as exc_info: + await _validate_team_member_add_permissions( + user_api_key_dict=team_admin, + complete_team_data=team, + data=_make_team_member_add_request( + member_user_id="new-user", role="user" + ), + ) + + assert exc_info.value.status_code == 403 + assert "adding members to" in str(exc_info.value.detail) + + +@pytest.mark.asyncio +async def test_team_member_delete_blocked_for_team_admin_when_lockdown_enabled( + mock_db_client, +): + """/team/member_delete must reject a team admin directly, not just hide the UI button.""" + from litellm.proxy._types import TeamMemberDeleteRequest + from litellm.proxy.management_endpoints.team_endpoints import team_member_delete + + test_team_id = "scim-team" + mock_team_row = MagicMock() + mock_team_row.model_dump.return_value = { + "team_id": test_team_id, + "members_with_roles": [ + {"user_id": "team-admin-user", "user_email": None, "role": "admin"}, + {"user_id": "user@example.com", "user_email": None, "role": "user"}, + ], + "team_member_permissions": [], + "metadata": {}, + "models": [], + "spend": 0.0, + } + mock_db_client.db.litellm_teamtable.find_unique = AsyncMock( + return_value=mock_team_row + ) + mock_db_client.db.litellm_teamtable.update = AsyncMock(return_value=mock_team_row) + + team_admin = UserAPIKeyAuth( + user_id="team-admin-user", + user_role=LitellmUserRoles.INTERNAL_USER, + ) + + with patch.dict( + "litellm.proxy.proxy_server.general_settings", + {"disable_team_admin_delete_team_user": True}, + clear=True, + ): + with pytest.raises(HTTPException) as exc_info: + await team_member_delete( + data=TeamMemberDeleteRequest( + team_id=test_team_id, user_id="user@example.com" + ), + user_api_key_dict=team_admin, + ) + + assert exc_info.value.status_code == 403 + mock_db_client.db.litellm_teamtable.update.assert_not_awaited() + + # ── VERIA-56 regression tests for _is_available_team self-join enforcement ─── diff --git a/ui/litellm-dashboard/src/components/Settings/AdminSettings/UISettings/UISettings.test.tsx b/ui/litellm-dashboard/src/components/Settings/AdminSettings/UISettings/UISettings.test.tsx index c2834e65498..a6a927666b2 100644 --- a/ui/litellm-dashboard/src/components/Settings/AdminSettings/UISettings/UISettings.test.tsx +++ b/ui/litellm-dashboard/src/components/Settings/AdminSettings/UISettings/UISettings.test.tsx @@ -31,6 +31,9 @@ const buildSettingsResponse = (overrides?: Partial>) => disable_team_admin_delete_team_user: { description: "Disable team admin delete team user", }, + disable_team_admin_add_team_user: { + description: "Disable team admin add team user", + }, require_auth_for_public_ai_hub: { description: "Require authentication for public AI Hub", }, @@ -39,6 +42,7 @@ const buildSettingsResponse = (overrides?: Partial>) => values: { disable_model_add_for_internal_users: false, disable_team_admin_delete_team_user: false, + disable_team_admin_add_team_user: false, require_auth_for_public_ai_hub: false, }, }, @@ -66,6 +70,7 @@ describe("UISettings", () => { expect(screen.getByText("UI Settings")).toBeInTheDocument(); expect(screen.getByRole("switch", { name: "Disable model add for internal users" })).toBeInTheDocument(); expect(screen.getByRole("switch", { name: "Disable team admin delete team user" })).toBeInTheDocument(); + expect(screen.getByRole("switch", { name: "Disable team admin add team user" })).toBeInTheDocument(); expect(screen.getByRole("switch", { name: "Require authentication for public AI Hub" })).toBeInTheDocument(); }); @@ -127,6 +132,35 @@ describe("UISettings", () => { expect(toast.success).toHaveBeenCalledWith("UI settings updated successfully"); }); + it("should toggle disable team admin add team user setting and call update", () => { + const mutateMock = vi.fn((_settings, options) => { + options?.onSuccess?.(); + }); + + mockUseUpdateUISettings.mockReturnValue({ + mutate: mutateMock, + isPending: false, + error: null, + }); + + render(); + + const toggle = screen.getByRole("switch", { name: "Disable team admin add team user" }); + + act(() => { + fireEvent.click(toggle); + }); + + expect(mutateMock).toHaveBeenCalledWith( + { disable_team_admin_add_team_user: true }, + expect.objectContaining({ + onSuccess: expect.any(Function), + onError: expect.any(Function), + }), + ); + expect(toast.success).toHaveBeenCalledWith("UI settings updated successfully"); + }); + it("should toggle require auth for public AI Hub setting and call update", () => { const mutateMock = vi.fn((_settings, options) => { options?.onSuccess?.(); diff --git a/ui/litellm-dashboard/src/components/Settings/AdminSettings/UISettings/UISettings.tsx b/ui/litellm-dashboard/src/components/Settings/AdminSettings/UISettings/UISettings.tsx index 612ca05d083..0f1258158ad 100644 --- a/ui/litellm-dashboard/src/components/Settings/AdminSettings/UISettings/UISettings.tsx +++ b/ui/litellm-dashboard/src/components/Settings/AdminSettings/UISettings/UISettings.tsx @@ -53,6 +53,7 @@ export default function UISettings() { const schema = data?.field_schema; const property = schema?.properties?.disable_model_add_for_internal_users; const disableTeamAdminDeleteProperty = schema?.properties?.disable_team_admin_delete_team_user; + const disableTeamAdminAddProperty = schema?.properties?.disable_team_admin_add_team_user; const requireAuthForPublicAIHubProperty = schema?.properties?.require_auth_for_public_ai_hub; const forwardClientHeadersProperty = schema?.properties?.forward_client_headers_to_llm_api; const forwardLLMProviderAuthHeadersProperty = schema?.properties?.forward_llm_provider_auth_headers; @@ -99,6 +100,20 @@ export default function UISettings() { ); }; + const handleToggleTeamAdminAdd = (checked: boolean) => { + updateSettings( + { disable_team_admin_add_team_user: checked }, + { + onSuccess: () => { + toast.success("UI settings updated successfully"); + }, + onError: (error) => { + toast.fromError(error); + }, + }, + ); + }; + const handleUpdatePageVisibility = (settings: { enabled_ui_pages_internal_users: string[] | null }) => { updateSettings(settings, { onSuccess: () => { @@ -311,6 +326,14 @@ export default function UISettings() { label="Disable team admin delete team user" description={disableTeamAdminDeleteProperty?.description} /> + { expect(screen.getAllByTestId("edit-member")).toHaveLength(2); }); + it("should hide Add Member button when disable_team_admin_add_team_user is true and user is team admin", () => { + vi.mocked(isProxyAdminRole).mockReturnValue(false); + vi.mocked(isUserTeamAdminForSingleTeam).mockReturnValue(true); + vi.mocked(useUISettings).mockReturnValue({ + data: { values: { disable_team_admin_add_team_user: true } }, + isLoading: false, + isError: false, + error: null, + isSuccess: true, + isFetching: false, + refetch: vi.fn(), + } as any); + + renderWithProviders( + , + ); + + expect(screen.queryByRole("button", { name: /add member/i })).not.toBeInTheDocument(); + }); + + it("should show Add Member button for proxy admin when disable_team_admin_add_team_user is true", () => { + vi.mocked(isProxyAdminRole).mockReturnValue(true); + vi.mocked(isUserTeamAdminForSingleTeam).mockReturnValue(true); + vi.mocked(useUISettings).mockReturnValue({ + data: { values: { disable_team_admin_add_team_user: true } }, + isLoading: false, + isError: false, + error: null, + isSuccess: true, + isFetching: false, + refetch: vi.fn(), + } as any); + + renderWithProviders( + , + ); + + expect(screen.getByRole("button", { name: /add member/i })).toBeInTheDocument(); + }); + it("should show delete button for proxy admin when canEditTeam is true", () => { vi.mocked(isProxyAdminRole).mockReturnValue(true); vi.mocked(isUserTeamAdminForSingleTeam).mockReturnValue(false); diff --git a/ui/litellm-dashboard/src/components/team/TeamMemberTab.tsx b/ui/litellm-dashboard/src/components/team/TeamMemberTab.tsx index 4d7246eb077..d5b333212e4 100644 --- a/ui/litellm-dashboard/src/components/team/TeamMemberTab.tsx +++ b/ui/litellm-dashboard/src/components/team/TeamMemberTab.tsx @@ -81,8 +81,10 @@ export default function TeamMemberTab({ const { data: uiSettingsData } = useUISettings(); const { userId, userRole } = useAuthorized(); const disableTeamAdminDeleteTeamUser = Boolean(uiSettingsData?.values?.disable_team_admin_delete_team_user); + const disableTeamAdminAddTeamUser = Boolean(uiSettingsData?.values?.disable_team_admin_add_team_user); const isUserTeamAdmin = isUserTeamAdminForSingleTeam(teamData.team_info.members_with_roles, userId || ""); const isProxyAdmin = isProxyAdminRole(userRole || ""); + const canAddMembers = isProxyAdmin || !isUserTeamAdmin || !disableTeamAdminAddTeamUser; const getUserAllowedModels = (userId: string | null): string[] | null => { if (!userId) return null; @@ -201,7 +203,7 @@ export default function TeamMemberTab({ setIsEditMemberModalVisible(true); }} onDelete={handleMemberDelete} - onAddMember={() => setIsAddMemberModalVisible(true)} + onAddMember={canAddMembers ? () => setIsAddMemberModalVisible(true) : undefined} roleColumnTitle="Team Role" roleTooltip="This role applies only to this team and is independent from the user's proxy-level role." extraColumns={extraColumns}