From 4c3afe384529971fcf16366f064590f81c101b96 Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Sat, 1 Aug 2026 12:41:46 -0700 Subject: [PATCH] fix(team): align /team/update authority with the documented team-admin rules --- .../management_endpoints/common_utils.py | 36 +- .../management_endpoints/team_endpoints.py | 183 +++++++-- .../management_endpoints/test_common_utils.py | 121 ++++++ .../test_team_endpoints.py | 385 +++++++++++++++++- .../ModelSelect/ModelSelect.test.tsx | 90 ++++ .../components/ModelSelect/ModelSelect.tsx | 31 +- .../src/components/team/TeamInfo.test.tsx | 132 +++++- .../src/components/team/TeamInfo.tsx | 35 +- 8 files changed, 975 insertions(+), 38 deletions(-) diff --git a/litellm/proxy/management_endpoints/common_utils.py b/litellm/proxy/management_endpoints/common_utils.py index 877130c2066..cf4623839b5 100644 --- a/litellm/proxy/management_endpoints/common_utils.py +++ b/litellm/proxy/management_endpoints/common_utils.py @@ -1,4 +1,5 @@ import math +from collections.abc import Sequence from typing import TYPE_CHECKING, Any, Dict, Optional, Union from fastapi import HTTPException, status @@ -76,27 +77,52 @@ def require_caller_user_id_for_non_admin( return user_api_key_dict.user_id +def _passthrough_routes_differ(requested: object, existing: Sequence[str] | None) -> bool: + """True when `requested` asks for a different route set than what is stored. + + Anything that isn't a list/tuple of route strings counts as a change, so + malformed payloads fall through to the 403 instead of silently passing the + gate (or blowing up on an unhashable element). + """ + if requested is None: + return False + if not isinstance(requested, (list, tuple)): + return True + if any(not isinstance(route, str) for route in requested): + return True + return frozenset(requested) != frozenset(existing or ()) + + def _check_passthrough_routes_caller_permission( data: BaseModel, user_api_key_dict: UserAPIKeyAuth, *, entity: str = "key", + existing_routes: Sequence[str] | None = None, ) -> None: """ - Only proxy admins may set `allowed_passthrough_routes` (top-level or under - `metadata`) — it short-circuits the role-based route gate, so keys and teams - must be gated identically. + Only proxy admins may CHANGE `allowed_passthrough_routes` (top-level or + under `metadata`) — it short-circuits the role-based route gate, so keys and + teams must be gated identically. + + Re-sending the stored routes unchanged is a no-op rather than a 403: an + update writes `metadata` wholesale, so clients have to echo the stored value + back or an unrelated edit would wipe it. Without `existing_routes` the gate + stays strict (any non-empty value is a change), which is what create paths + want. """ # view-only admins excluded by design; blocked upstream from writes anyway if user_api_key_dict.user_role == LitellmUserRoles.PROXY_ADMIN.value: return - if getattr(data, "allowed_passthrough_routes", None): + if _passthrough_routes_differ(getattr(data, "allowed_passthrough_routes", None), existing_routes): raise HTTPException( status_code=403, detail={"error": f"Only proxy admins can set `allowed_passthrough_routes` on a {entity}."}, ) metadata = getattr(data, "metadata", None) - if isinstance(metadata, dict) and metadata.get("allowed_passthrough_routes"): + if isinstance(metadata, dict) and _passthrough_routes_differ( + metadata.get("allowed_passthrough_routes"), existing_routes + ): raise HTTPException( status_code=403, detail={"error": f"Only proxy admins can set `metadata.allowed_passthrough_routes` on a {entity}."}, diff --git a/litellm/proxy/management_endpoints/team_endpoints.py b/litellm/proxy/management_endpoints/team_endpoints.py index 1efd0a747df..a342f5863d4 100644 --- a/litellm/proxy/management_endpoints/team_endpoints.py +++ b/litellm/proxy/management_endpoints/team_endpoints.py @@ -15,6 +15,7 @@ import math import traceback from collections.abc import Sequence from datetime import datetime, timezone +from enum import Enum from typing import ( Annotated, Dict, @@ -83,6 +84,7 @@ from litellm.proxy._types import ( ) from litellm.proxy.auth.auth_checks import ( _cache_team_object, + _model_matches_any_wildcard_pattern_in_list, allowed_route_check_inside_route, can_org_access_model, get_org_object, @@ -315,10 +317,68 @@ async def _refresh_cached_team( ) +class TeamAccessGrant(str, Enum): + """Which authority let a caller through `_verify_team_access`. + + Callers that gate finer-grained actions (budget ceiling, model list) need to + know this: a team admin is bounded by what a proxy admin already granted the + team, while proxy/org admins hold the grant itself. + """ + + PROXY_ADMIN = "proxy_admin" + ORG_ADMIN = "org_admin" + TEAM_ADMIN = "team_admin" + + +async def _is_org_admin_for_team_or_false( + team_obj: LiteLLM_TeamTable, + user_api_key_dict: UserAPIKeyAuth, +) -> bool: + """`_is_user_org_admin_for_team`, but a failed lookup means "no". + + The underlying call raises when the caller's user row can't be read. Every + caller of this uses the answer to hand out authority, so degrading to "not + an org admin" is fail-closed: it can only withhold authority, never grant + it, and it keeps a lookup failure from 500-ing a request the caller is + otherwise allowed to make. + """ + if not team_obj.organization_id: + return False + try: + return await _is_user_org_admin_for_team(user_api_key_dict=user_api_key_dict, team_obj=team_obj) + except Exception as e: + verbose_proxy_logger.warning( + "Could not resolve org-admin status for user=%s on team=%s, treating as non-org-admin. Error: %s", + user_api_key_dict.user_id, + team_obj.team_id, + e, + ) + return False + + +async def _escalate_team_admin_grant_for_org_admin( + team_access: TeamAccessGrant, + team_obj: LiteLLM_TeamTable, + user_api_key_dict: UserAPIKeyAuth, +) -> TeamAccessGrant: + """Upgrade a TEAM_ADMIN grant to ORG_ADMIN when the caller is both. + + `_verify_team_access` answers the cheap team-membership question first and + stops there, so a team's own org admin (usually the team's creator) comes + back as TEAM_ADMIN. Callers that gate on the grant have to resolve the + stronger authority before refusing them. + """ + if team_access is not TeamAccessGrant.TEAM_ADMIN: + return team_access + if await _is_org_admin_for_team_or_false(team_obj=team_obj, user_api_key_dict=user_api_key_dict): + return TeamAccessGrant.ORG_ADMIN + return team_access + + async def _verify_team_access( team_obj: LiteLLM_TeamTable, user_api_key_dict: UserAPIKeyAuth, -) -> None: +) -> TeamAccessGrant: """ Verify the caller is authorized to manage the given team. @@ -327,16 +387,17 @@ async def _verify_team_access( - Caller is an org admin for the team's organization, OR - Caller is a team admin of this team - Raises HTTPException(403) otherwise. + Returns the authority that granted access. Raises HTTPException(403) + otherwise. """ if user_api_key_dict.user_role == LitellmUserRoles.PROXY_ADMIN: - return + return TeamAccessGrant.PROXY_ADMIN if _is_user_team_admin(user_api_key_dict=user_api_key_dict, team_obj=team_obj): - return + return TeamAccessGrant.TEAM_ADMIN - if await _is_user_org_admin_for_team(user_api_key_dict=user_api_key_dict, team_obj=team_obj): - return + if await _is_org_admin_for_team_or_false(team_obj=team_obj, user_api_key_dict=user_api_key_dict): + return TeamAccessGrant.ORG_ADMIN raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, @@ -998,19 +1059,19 @@ async def _check_user_team_limits( def _check_team_budget_update_authority( data: UpdateTeamRequest, - user_api_key_dict: UserAPIKeyAuth, + team_access: TeamAccessGrant, existing_team_max_budget: Optional[float], ) -> None: """ - Restrict who can grow a standalone team's spend ceiling on /team/update. + Restrict who can grow a team's spend ceiling on /team/update. - A team admin (already authorized via _verify_team_access) may keep or lower - the team budget, but only a proxy admin may grow it - by raising max_budget + A team admin may keep or lower the team budget, but only a proxy admin (or + the org admin who owns the team's org) may grow it - by raising max_budget above the team's current value or by removing the cap (setting it to None). Setting a finite budget on a team that has no cap is a restriction and is - allowed. Org-scoped teams are governed by _check_org_team_limits(). + allowed. Org-scoped teams are additionally capped by _check_org_team_limits(). """ - if user_api_key_dict.user_role == LitellmUserRoles.PROXY_ADMIN: + if team_access is not TeamAccessGrant.TEAM_ADMIN: return if existing_team_max_budget is None: return @@ -1033,6 +1094,65 @@ def _check_team_budget_update_authority( ) +def _model_list_grants_every_model(models: Sequence[str]) -> bool: + """Whether a team.models value reaches every deployment on the proxy. + + Mirrors the auth path (`_can_object_call_model`): an empty list, a bare + "*", and the all-proxy-models sentinel are all unrestricted. + """ + return len(models) == 0 or "*" in models or SpecialModelNames.all_proxy_models.value in models + + +def _check_team_models_update_authority( + data: UpdateTeamRequest, + team_access: TeamAccessGrant, + existing_team_models: Sequence[str], +) -> None: + """ + Restrict who can widen a team's allowed-model list on /team/update. + + A team admin may drop models the team already has, but only a proxy admin + (or the org admin who owns the team's org) can grant a team access to a + model it cannot already reach. Reachability follows the auth path, so a team + holding "openai/*" may be narrowed to "openai/gpt-4o", and a team that + already reaches every model can be narrowed to anything. + """ + if team_access is not TeamAccessGrant.TEAM_ADMIN: + return + if data.models is None: + return + if _model_list_grants_every_model(existing_team_models): + return + + if _model_list_grants_every_model(data.models): + raise HTTPException( + status_code=403, + detail={ + "error": ( + "Only a proxy admin can grant a team access to every proxy model. Team's current " + f"models={sorted(set(existing_team_models))}." + ) + }, + ) + + added_models = tuple( + model + for model in data.models + if model not in existing_team_models + and not _model_matches_any_wildcard_pattern_in_list(model=model, allowed_model_list=list(existing_team_models)) + ) + if added_models: + raise HTTPException( + status_code=403, + detail={ + "error": ( + f"Only a proxy admin can add models to a team. Models the team cannot already reach: " + f"{sorted(set(added_models))}. Team's current models={sorted(set(existing_team_models))}." + ) + }, + ) + + def _should_auto_add_team_creator( user_api_key_dict: UserAPIKeyAuth, general_settings: Mapping[str, object], @@ -1851,12 +1971,32 @@ async def update_team( ) # Verify caller has access to manage this team - await _verify_team_access( - team_obj=LiteLLM_TeamTable.model_validate(existing_team_row.model_dump()), + existing_team_obj = LiteLLM_TeamTable.model_validate(existing_team_row.model_dump()) + team_access = await _escalate_team_admin_grant_for_org_admin( + team_access=await _verify_team_access( + team_obj=existing_team_obj, + user_api_key_dict=user_api_key_dict, + ), + team_obj=existing_team_obj, user_api_key_dict=user_api_key_dict, ) - _check_passthrough_routes_caller_permission(data, user_api_key_dict, entity="team") + existing_metadata = existing_team_row.metadata if isinstance(existing_team_row.metadata, dict) else {} + stored_passthrough_routes = existing_metadata.get("allowed_passthrough_routes") + _check_passthrough_routes_caller_permission( + data, + user_api_key_dict, + entity="team", + existing_routes=( + stored_passthrough_routes if isinstance(stored_passthrough_routes, (list, tuple)) else None + ), + ) + + _check_team_models_update_authority( + data=data, + team_access=team_access, + existing_team_models=existing_team_row.models or [], + ) if data.soft_budget is not None: max_budget_to_check = data.max_budget if data.max_budget is not None else existing_team_row.max_budget @@ -1945,14 +2085,11 @@ async def update_team( prisma_client=prisma_client, ) - # Only a proxy admin may grow a standalone team's spend ceiling. - # Org-scoped teams are validated by _check_org_team_limits() above. - if org_id_to_check is None: - _check_team_budget_update_authority( - data=data, - user_api_key_dict=user_api_key_dict, - existing_team_max_budget=existing_team_row.max_budget, - ) + _check_team_budget_update_authority( + data=data, + team_access=team_access, + existing_team_max_budget=existing_team_row.max_budget, + ) updated_kv = data.json(exclude_unset=True) 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 81840745d0e..5ec6c1319d6 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_common_utils.py +++ b/tests/test_litellm/proxy/management_endpoints/test_common_utils.py @@ -715,6 +715,127 @@ class TestCheckPassthroughRoutesCallerPermission: is None ) + def _route_data_model(self): + from pydantic import BaseModel + + class _RouteData(BaseModel): + allowed_passthrough_routes: list | None = None + metadata: dict | None = None + + return _RouteData + + def test_unchanged_echo_of_stored_routes_is_allowed(self): + """An update rewrites metadata wholesale, so a non-admin has to echo the + stored routes back or the save would wipe them. Echoing changes nothing + and must not 403.""" + from litellm.proxy.management_endpoints.common_utils import ( + _check_passthrough_routes_caller_permission, + ) + + data = self._route_data_model()( + metadata={"allowed_passthrough_routes": ["/v1/bar", "/v1/foo"]} + ) + + assert ( + _check_passthrough_routes_caller_permission( + data, + self._non_admin(), + entity="team", + existing_routes=["/v1/foo", "/v1/bar"], + ) + is None + ) + + def test_adding_a_route_to_the_stored_set_is_rejected(self): + from fastapi import HTTPException + + from litellm.proxy.management_endpoints.common_utils import ( + _check_passthrough_routes_caller_permission, + ) + + data = self._route_data_model()( + metadata={"allowed_passthrough_routes": ["/v1/foo", "/v1/baz"]} + ) + + with pytest.raises(HTTPException) as exc_info: + _check_passthrough_routes_caller_permission( + data, + self._non_admin(), + entity="team", + existing_routes=["/v1/foo"], + ) + + assert exc_info.value.status_code == 403 + assert exc_info.value.detail == { + "error": "Only proxy admins can set `metadata.allowed_passthrough_routes` on a team." + } + + def test_clearing_stored_routes_is_rejected(self): + """Wiping an admin-only field is still changing it.""" + from fastapi import HTTPException + + from litellm.proxy.management_endpoints.common_utils import ( + _check_passthrough_routes_caller_permission, + ) + + data = self._route_data_model()(metadata={"allowed_passthrough_routes": []}) + + with pytest.raises(HTTPException) as exc_info: + _check_passthrough_routes_caller_permission( + data, + self._non_admin(), + entity="team", + existing_routes=["/v1/foo"], + ) + + assert exc_info.value.status_code == 403 + + def test_non_list_payload_is_rejected(self): + from fastapi import HTTPException + + from litellm.proxy.management_endpoints.common_utils import ( + _check_passthrough_routes_caller_permission, + ) + + data = self._route_data_model()( + metadata={"allowed_passthrough_routes": "/v1/foo"} + ) + + with pytest.raises(HTTPException) as exc_info: + _check_passthrough_routes_caller_permission( + data, + self._non_admin(), + entity="team", + existing_routes=["/v1/foo"], + ) + + assert exc_info.value.status_code == 403 + + def test_top_level_echo_is_allowed_while_a_change_is_rejected(self): + from fastapi import HTTPException + + from litellm.proxy.management_endpoints.common_utils import ( + _check_passthrough_routes_caller_permission, + ) + + model = self._route_data_model() + assert ( + _check_passthrough_routes_caller_permission( + model(allowed_passthrough_routes=["/v1/foo"]), + self._non_admin(), + existing_routes=["/v1/foo"], + ) + is None + ) + + with pytest.raises(HTTPException) as exc_info: + _check_passthrough_routes_caller_permission( + model(allowed_passthrough_routes=["/v1/foo", "/v1/qux"]), + self._non_admin(), + existing_routes=["/v1/foo"], + ) + assert exc_info.value.status_code == 403 + class TestIsUserOrgAdminForTeam: """The caller must be looked up with its exact identity; a nulled or omitted 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 0de2c1ac71d..97e4d01705d 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py @@ -5410,6 +5410,10 @@ async def test_update_team_org_scoped_budget_exceeds_org_limit(): "litellm.proxy.management_endpoints.team_endpoints.get_org_object", new=AsyncMock(return_value=mock_org), ) as mock_get_org, + patch( + "litellm.proxy.management_endpoints.team_endpoints._is_user_org_admin_for_team", + new=AsyncMock(return_value=True), + ), ): # Mock existing org-scoped team mock_existing_team = MagicMock() @@ -5450,13 +5454,13 @@ async def test_update_team_standalone_models_not_gated_by_user_limit(): Test that /team/update for a standalone team does NOT gate the team's models by the caller's personal allowed models. - A team admin authorized via _verify_team_access() may set the team's models - independently of their own personal model list on update. + A team admin authorized via _verify_team_access() may narrow the team's + models independently of their own personal model list on update. Scenario: - Team admin has personal models=['gpt-3.5-turbo'] - - Standalone team exists (no organization_id) - - Admin updates team models to ['gpt-4'] (not in their personal list) + - Standalone team exists (no organization_id) holding both models + - Admin narrows the team to ['gpt-4'] (not in their personal list) - Expected: Should succeed (personal models are irrelevant on /team/update) """ from fastapi import Request @@ -5489,12 +5493,12 @@ async def test_update_team_standalone_models_not_gated_by_user_limit(): mock_existing_team = MagicMock() mock_existing_team.team_id = "standalone-team-models-123" mock_existing_team.organization_id = None # Standalone team - mock_existing_team.models = ["gpt-3.5-turbo"] + mock_existing_team.models = ["gpt-3.5-turbo", "gpt-4"] mock_existing_team.model_id = None mock_existing_team.model_dump.return_value = { "team_id": "standalone-team-models-123", "organization_id": None, - "models": ["gpt-3.5-turbo"], + "models": ["gpt-3.5-turbo", "gpt-4"], "members_with_roles": [ {"user_id": "non-admin-update-models-test", "role": "admin"} ], @@ -5586,6 +5590,10 @@ async def test_update_team_org_scoped_budget_bypasses_user_limit(): "litellm.proxy.management_endpoints.team_endpoints.get_org_object", new=AsyncMock(return_value=mock_org), ) as mock_get_org, + patch( + "litellm.proxy.management_endpoints.team_endpoints._is_user_org_admin_for_team", + new=AsyncMock(return_value=True), + ), ): # Mock existing org-scoped team mock_existing_team = MagicMock() @@ -5696,6 +5704,10 @@ async def test_update_team_org_scoped_models_bypasses_user_limit(): "litellm.proxy.management_endpoints.team_endpoints.get_org_object", new=AsyncMock(return_value=mock_org), ) as mock_get_org, + patch( + "litellm.proxy.management_endpoints.team_endpoints._is_user_org_admin_for_team", + new=AsyncMock(return_value=True), + ), ): # Mock existing org-scoped team mock_existing_team = MagicMock() @@ -5799,6 +5811,10 @@ async def test_update_team_org_scoped_models_not_in_org_models(): "litellm.proxy.management_endpoints.team_endpoints.get_org_object", new=AsyncMock(return_value=mock_org), ) as mock_get_org, + patch( + "litellm.proxy.management_endpoints.team_endpoints._is_user_org_admin_for_team", + new=AsyncMock(return_value=True), + ), ): # Mock existing org-scoped team mock_existing_team = MagicMock() @@ -5887,6 +5903,10 @@ async def test_update_team_org_scoped_models_with_all_proxy_models(): "litellm.proxy.management_endpoints.team_endpoints.get_org_object", new=AsyncMock(return_value=mock_org), ) as mock_get_org, + patch( + "litellm.proxy.management_endpoints.team_endpoints._is_user_org_admin_for_team", + new=AsyncMock(return_value=True), + ), ): # Mock existing org-scoped team mock_existing_team = MagicMock() @@ -10617,3 +10637,356 @@ def test_validate_member_user_id_provisioning_caps_the_ids_it_echoes_back(): assert f"u{_MAX_REPORTED_UNKNOWN_USER_IDS}" not in detail assert f"and {500 - _MAX_REPORTED_UNKNOWN_USER_IDS} more" in detail assert len(detail) < 1000 + + +async def _run_update_team( + *, + update_request, + caller, + existing_team, + org=None, + caller_is_org_admin=False, +): + """Drive /team/update against a mocked team row and return the handler result. + + `existing_team` is the stored row as a dict; every field the handler reads + off the row has to be present, since a MagicMock attribute would otherwise + stand in as a truthy sentinel. + """ + from fastapi import Request + + from litellm.proxy.management_endpoints.team_endpoints import update_team + + mock_existing_team = MagicMock() + for field, value in existing_team.items(): + setattr(mock_existing_team, field, value) + mock_existing_team.model_dump.return_value = dict(existing_team) + + mock_updated_team = MagicMock() + mock_updated_team.team_id = existing_team["team_id"] + mock_updated_team.litellm_model_table = None + mock_updated_team.model_dump.return_value = dict(existing_team) + + with ( + patch("litellm.proxy.proxy_server.prisma_client") as mock_prisma, + patch("litellm.proxy.proxy_server.user_api_key_cache") as mock_cache, + patch("litellm.proxy.proxy_server.litellm_proxy_admin_name", "admin"), + patch("litellm.proxy.proxy_server.create_audit_log_for_update", new=AsyncMock()), + patch( + "litellm.proxy.management_endpoints.team_endpoints.get_org_object", + new=AsyncMock(return_value=org), + ), + patch( + "litellm.proxy.management_endpoints.team_endpoints._is_user_org_admin_for_team", + new=AsyncMock(return_value=caller_is_org_admin), + ), + ): + mock_prisma.db.litellm_teamtable.find_unique = AsyncMock(return_value=mock_existing_team) + mock_prisma.db.litellm_teamtable.update = AsyncMock(return_value=mock_updated_team) + mock_prisma.jsonify_team_object = lambda db_data: db_data + mock_cache.async_get_cache = AsyncMock(return_value=None) + mock_cache.async_set_cache = AsyncMock() + + return await update_team( + data=update_request, + http_request=MagicMock(spec=Request), + user_api_key_dict=caller, + ) + + +def _team_admin_caller(user_id="team-admin-doc-alignment"): + return UserAPIKeyAuth(user_role=LitellmUserRoles.INTERNAL_USER, user_id=user_id, models=[]) + + +def _stored_team(**overrides): + stored = { + "team_id": "doc-alignment-team", + "organization_id": None, + "max_budget": 30.0, + "models": ["gpt-4"], + "model_id": None, + "metadata": {}, + "object_permission_id": None, + "members_with_roles": [{"user_id": "team-admin-doc-alignment", "role": "admin"}], + } + stored.update(overrides) + return stored + + +@pytest.mark.asyncio +async def test_update_team_model_add_blocked_for_team_admin(): + """The docs say a team admin cannot add global proxy models to their team; + /team/update has to say the same.""" + from litellm.proxy._types import ProxyException, UpdateTeamRequest + + with pytest.raises(ProxyException) as exc_info: + await _run_update_team( + update_request=UpdateTeamRequest(team_id="doc-alignment-team", models=["gpt-4", "claude-opus-4-5"]), + caller=_team_admin_caller(), + existing_team=_stored_team(), + ) + + assert exc_info.value.code == "403" + assert "claude-opus-4-5" in str(exc_info.value.message) + assert "proxy admin" in str(exc_info.value.message).lower() + + +@pytest.mark.asyncio +async def test_update_team_model_add_allowed_for_org_admin(): + """An org admin holds the grant, so widening their own team's models stays + allowed (the org's model list is the ceiling, checked separately).""" + from litellm.proxy._types import ( + LiteLLM_BudgetTable, + LiteLLM_OrganizationTable, + UpdateTeamRequest, + ) + + mock_org = MagicMock(spec=LiteLLM_OrganizationTable) + mock_org.organization_id = "org-doc-alignment" + mock_org.models = ["gpt-4", "claude-opus-4-5"] + mock_org.litellm_budget_table = MagicMock(spec=LiteLLM_BudgetTable) + mock_org.litellm_budget_table.max_budget = None + mock_org.litellm_budget_table.tpm_limit = None + mock_org.litellm_budget_table.rpm_limit = None + + result = await _run_update_team( + update_request=UpdateTeamRequest(team_id="doc-alignment-team", models=["gpt-4", "claude-opus-4-5"]), + caller=_team_admin_caller(), + existing_team=_stored_team(organization_id="org-doc-alignment"), + org=mock_org, + caller_is_org_admin=True, + ) + + assert result is not None + + +@pytest.mark.asyncio +async def test_update_team_model_add_allowed_when_team_already_holds_every_model(): + """A team holding the all-proxy-models sentinel already reaches everything, + so naming a specific model narrows rather than widens.""" + from litellm.proxy._types import UpdateTeamRequest + + result = await _run_update_team( + update_request=UpdateTeamRequest(team_id="doc-alignment-team", models=["claude-opus-4-5"]), + caller=_team_admin_caller(), + existing_team=_stored_team(models=["all-proxy-models"]), + ) + + assert result is not None + + +@pytest.mark.asyncio +async def test_update_team_org_scoped_budget_raise_blocked_for_team_admin(): + """Keep-or-lower applies to a team admin whether or not the team sits in an + org; staying under the org ceiling is a separate question.""" + from litellm.proxy._types import ( + LiteLLM_BudgetTable, + LiteLLM_OrganizationTable, + ProxyException, + UpdateTeamRequest, + ) + + mock_org = MagicMock(spec=LiteLLM_OrganizationTable) + mock_org.organization_id = "org-doc-alignment" + mock_org.models = ["gpt-4"] + mock_org.litellm_budget_table = MagicMock(spec=LiteLLM_BudgetTable) + mock_org.litellm_budget_table.max_budget = 1000.0 + mock_org.litellm_budget_table.tpm_limit = None + mock_org.litellm_budget_table.rpm_limit = None + + with pytest.raises(ProxyException) as exc_info: + await _run_update_team( + update_request=UpdateTeamRequest(team_id="doc-alignment-team", max_budget=900.0), + caller=_team_admin_caller(), + existing_team=_stored_team(organization_id="org-doc-alignment"), + org=mock_org, + ) + + assert exc_info.value.code == "403" + assert "raise" in str(exc_info.value.message).lower() + + +@pytest.mark.asyncio +async def test_update_team_org_scoped_budget_removal_blocked_for_team_admin(): + """Dropping the cap is the strongest raise there is.""" + from litellm.proxy._types import ( + LiteLLM_BudgetTable, + LiteLLM_OrganizationTable, + ProxyException, + UpdateTeamRequest, + ) + + mock_org = MagicMock(spec=LiteLLM_OrganizationTable) + mock_org.organization_id = "org-doc-alignment" + mock_org.models = ["gpt-4"] + mock_org.litellm_budget_table = MagicMock(spec=LiteLLM_BudgetTable) + mock_org.litellm_budget_table.max_budget = 1000.0 + mock_org.litellm_budget_table.tpm_limit = None + mock_org.litellm_budget_table.rpm_limit = None + + request = UpdateTeamRequest(team_id="doc-alignment-team", max_budget=None) + assert "max_budget" in request.model_fields_set + + with pytest.raises(ProxyException) as exc_info: + await _run_update_team( + update_request=request, + caller=_team_admin_caller(), + existing_team=_stored_team(organization_id="org-doc-alignment"), + org=mock_org, + ) + + assert exc_info.value.code == "403" + assert "remove" in str(exc_info.value.message).lower() + + +@pytest.mark.asyncio +async def test_update_team_org_scoped_budget_lower_allowed_for_team_admin(): + from litellm.proxy._types import ( + LiteLLM_BudgetTable, + LiteLLM_OrganizationTable, + UpdateTeamRequest, + ) + + mock_org = MagicMock(spec=LiteLLM_OrganizationTable) + mock_org.organization_id = "org-doc-alignment" + mock_org.models = ["gpt-4"] + mock_org.litellm_budget_table = MagicMock(spec=LiteLLM_BudgetTable) + mock_org.litellm_budget_table.max_budget = 1000.0 + mock_org.litellm_budget_table.tpm_limit = None + mock_org.litellm_budget_table.rpm_limit = None + + result = await _run_update_team( + update_request=UpdateTeamRequest(team_id="doc-alignment-team", max_budget=10.0), + caller=_team_admin_caller(), + existing_team=_stored_team(organization_id="org-doc-alignment"), + org=mock_org, + ) + + assert result is not None + + +@pytest.mark.asyncio +async def test_update_team_unchanged_passthrough_routes_do_not_block_a_team_admin_edit(): + """The dashboard echoes the stored routes back so an unrelated edit doesn't + wipe them; that echo must not read as an attempt to set them.""" + from litellm.proxy._types import UpdateTeamRequest + + result = await _run_update_team( + update_request=UpdateTeamRequest( + team_id="doc-alignment-team", + team_alias="renamed-by-team-admin", + metadata={"allowed_passthrough_routes": ["/v1/foo"]}, + ), + caller=_team_admin_caller(), + existing_team=_stored_team(metadata={"allowed_passthrough_routes": ["/v1/foo"]}), + ) + + assert result is not None + + +@pytest.mark.asyncio +async def test_update_team_changed_passthrough_routes_still_blocked_for_team_admin(): + from litellm.proxy._types import ProxyException, UpdateTeamRequest + + with pytest.raises(ProxyException) as exc_info: + await _run_update_team( + update_request=UpdateTeamRequest( + team_id="doc-alignment-team", + metadata={"allowed_passthrough_routes": ["/v1/foo", "/v1/escalate"]}, + ), + caller=_team_admin_caller(), + existing_team=_stored_team(metadata={"allowed_passthrough_routes": ["/v1/foo"]}), + ) + + assert exc_info.value.code == "403" + assert "allowed_passthrough_routes" in str(exc_info.value.message) + + +@pytest.mark.asyncio +async def test_update_team_model_clear_blocked_for_team_admin(): + """An empty models list is the unrestricted sentinel at auth time, so + clearing the list is the widest possible grant, not a narrowing.""" + from litellm.proxy._types import ProxyException, UpdateTeamRequest + + with pytest.raises(ProxyException) as exc_info: + await _run_update_team( + update_request=UpdateTeamRequest(team_id="doc-alignment-team", models=[]), + caller=_team_admin_caller(), + existing_team=_stored_team(), + ) + + assert exc_info.value.code == "403" + assert "every proxy model" in str(exc_info.value.message) + + +@pytest.mark.asyncio +async def test_update_team_model_wildcard_grant_blocked_for_team_admin(): + """"*" reaches everything the same way the empty list does.""" + from litellm.proxy._types import ProxyException, UpdateTeamRequest + + with pytest.raises(ProxyException) as exc_info: + await _run_update_team( + update_request=UpdateTeamRequest(team_id="doc-alignment-team", models=["*"]), + caller=_team_admin_caller(), + existing_team=_stored_team(), + ) + + assert exc_info.value.code == "403" + + +@pytest.mark.asyncio +async def test_update_team_narrowing_a_wildcard_allowed_for_team_admin(): + """A team holding openai/* already reaches openai/gpt-4o, so pinning the + list to that model narrows the team's access.""" + from litellm.proxy._types import UpdateTeamRequest + + result = await _run_update_team( + update_request=UpdateTeamRequest(team_id="doc-alignment-team", models=["openai/gpt-4o"]), + caller=_team_admin_caller(), + existing_team=_stored_team(models=["openai/*"]), + ) + + assert result is not None + + +@pytest.mark.asyncio +async def test_update_team_model_outside_a_wildcard_blocked_for_team_admin(): + from litellm.proxy._types import ProxyException, UpdateTeamRequest + + with pytest.raises(ProxyException) as exc_info: + await _run_update_team( + update_request=UpdateTeamRequest(team_id="doc-alignment-team", models=["anthropic/claude-opus-4-5"]), + caller=_team_admin_caller(), + existing_team=_stored_team(models=["openai/*"]), + ) + + assert exc_info.value.code == "403" + + +@pytest.mark.asyncio +async def test_update_team_budget_raise_allowed_for_team_admin_who_is_also_org_admin(): + """The org admin who created the team is a team admin too; the stronger + grant has to win or they lose authority over their own org's team.""" + from litellm.proxy._types import ( + LiteLLM_BudgetTable, + LiteLLM_OrganizationTable, + UpdateTeamRequest, + ) + + mock_org = MagicMock(spec=LiteLLM_OrganizationTable) + mock_org.organization_id = "org-doc-alignment" + mock_org.models = ["gpt-4"] + mock_org.litellm_budget_table = MagicMock(spec=LiteLLM_BudgetTable) + mock_org.litellm_budget_table.max_budget = 1000.0 + mock_org.litellm_budget_table.tpm_limit = None + mock_org.litellm_budget_table.rpm_limit = None + + result = await _run_update_team( + update_request=UpdateTeamRequest(team_id="doc-alignment-team", max_budget=900.0), + caller=_team_admin_caller(), + existing_team=_stored_team(organization_id="org-doc-alignment"), + org=mock_org, + caller_is_org_admin=True, + ) + + assert result is not None diff --git a/ui/litellm-dashboard/src/components/ModelSelect/ModelSelect.test.tsx b/ui/litellm-dashboard/src/components/ModelSelect/ModelSelect.test.tsx index e253bc4c0ef..0e84d886d3a 100644 --- a/ui/litellm-dashboard/src/components/ModelSelect/ModelSelect.test.tsx +++ b/ui/litellm-dashboard/src/components/ModelSelect/ModelSelect.test.tsx @@ -578,4 +578,94 @@ describe("ModelSelect", () => { expect(screen.getByText(/\+5 more/)).toBeInTheDocument(); }); }); + describe("restrictToCurrentTeamModels", () => { + const renderRestricted = (teamModels: string[], organizationModels?: string[]) => { + mockUseTeam.mockReturnValue({ + data: { team_id: "team-1", team_alias: "Test Team", models: teamModels }, + isLoading: false, + } as any); + if (organizationModels) { + mockUseOrganization.mockReturnValue({ + data: createMockOrganization(organizationModels), + isLoading: false, + } as any); + } + + renderWithProviders( + , + ); + }; + + it("offers only the models the team already holds", async () => { + renderRestricted(["gpt-4"]); + + await waitFor(() => { + expect(screen.getByRole("option", { name: "gpt-4" })).toBeInTheDocument(); + }); + expect(screen.queryByRole("option", { name: "claude-3" })).not.toBeInTheDocument(); + expect(screen.queryByRole("option", { name: "All Proxy Models" })).not.toBeInTheDocument(); + }); + + it("hides All Proxy Models even when the team's org grants everything", async () => { + renderRestricted(["gpt-4"], ["all-proxy-models"]); + + await waitFor(() => { + expect(screen.getByRole("option", { name: "gpt-4" })).toBeInTheDocument(); + }); + expect(screen.queryByRole("option", { name: "All Proxy Models" })).not.toBeInTheDocument(); + expect(screen.queryByRole("option", { name: "claude-3" })).not.toBeInTheDocument(); + }); + + it("does not restrict a team that already reaches every model", async () => { + renderRestricted(["all-proxy-models"]); + + await waitFor(() => { + expect(screen.getByRole("option", { name: "gpt-4" })).toBeInTheDocument(); + }); + expect(screen.getByRole("option", { name: "claude-3" })).toBeInTheDocument(); + }); + + it("keeps every model a team wildcard already reaches selectable", async () => { + mockUseAllProxyModels.mockReturnValue({ + data: { + data: [ + { id: "openai/gpt-4o", object: "model", created: 1, owned_by: "openai" }, + { id: "openai/*", object: "model", created: 1, owned_by: "openai" }, + { id: "anthropic/claude-opus-4-5", object: "model", created: 1, owned_by: "anthropic" }, + ], + }, + isLoading: false, + } as any); + renderRestricted(["openai/*"]); + + await waitFor(() => { + expect(screen.getByRole("option", { name: "openai/gpt-4o" })).toBeInTheDocument(); + }); + expect(screen.queryByRole("option", { name: "anthropic/claude-opus-4-5" })).not.toBeInTheDocument(); + }); + + it("does not restrict a team holding the bare * grant", async () => { + renderRestricted(["*"]); + + await waitFor(() => { + expect(screen.getByRole("option", { name: "gpt-4" })).toBeInTheDocument(); + }); + expect(screen.getByRole("option", { name: "claude-3" })).toBeInTheDocument(); + }); + + it("does not restrict a team with an empty model list", async () => { + renderRestricted([]); + + await waitFor(() => { + expect(screen.getByRole("option", { name: "gpt-4" })).toBeInTheDocument(); + }); + expect(screen.getByRole("option", { name: "claude-3" })).toBeInTheDocument(); + }); + }); }); diff --git a/ui/litellm-dashboard/src/components/ModelSelect/ModelSelect.tsx b/ui/litellm-dashboard/src/components/ModelSelect/ModelSelect.tsx index 0965683c241..88403de808d 100644 --- a/ui/litellm-dashboard/src/components/ModelSelect/ModelSelect.tsx +++ b/ui/litellm-dashboard/src/components/ModelSelect/ModelSelect.tsx @@ -29,6 +29,7 @@ export interface ModelSelectProps { showAllTeamModelsOption?: boolean; showAllProxyModelsOverride?: boolean; includeSpecialOptions?: boolean; + restrictToCurrentTeamModels?: boolean; }; context: "team" | "organization" | "user" | "global"; dataTestId?: string; @@ -45,6 +46,28 @@ type FilterContextArgs = { options?: ModelSelectProps["options"]; }; +/** + * The team's own models, when the caller may only narrow that list (a team + * admin can drop a model but not grant a new one — /team/update rejects it). + * Returns null when the restriction doesn't apply, including when the team + * already reaches every model, since nothing can widen it further. + */ +const keepOnlyCurrentTeamModels = ( + selectedTeam: Team | undefined, + options: ModelSelectProps["options"], + allProxyModels: string[], +): string[] | null => { + if (!options?.restrictToCurrentTeamModels) return null; + const teamModels = selectedTeam?.models ?? []; + if (teamModels.length === 0) return null; + if (teamModels.includes(MODEL_SELECT_ALL_PROXY_MODELS_SPECIAL_VALUE.value) || teamModels.includes("*")) return null; + const wildcardPrefixes = teamModels.filter((m) => m.endsWith("/*")).map((m) => m.slice(0, -1)); + const reachableProxyModels = allProxyModels.filter((model) => + wildcardPrefixes.some((prefix) => model.startsWith(prefix)), + ); + return Array.from(new Set([...teamModels, ...reachableProxyModels])); +}; + const contextFilters: Record string[]> = { user: ({ allProxyModels, userModels, options }) => { if (!userModels) return []; @@ -52,7 +75,10 @@ const contextFilters: Record { + team: ({ allProxyModels, selectedTeam, selectedOrganization, userModels, options }) => { + const currentTeamModels = keepOnlyCurrentTeamModels(selectedTeam, options, allProxyModels); + if (currentTeamModels) return currentTeamModels; + if (selectedOrganization) { if ( selectedOrganization.models.includes(MODEL_SELECT_ALL_PROXY_MODELS_SPECIAL_VALUE.value) || @@ -107,7 +133,8 @@ export const ModelSelect = (props: ModelSelectProps) => { organization?.models.includes(MODEL_SELECT_ALL_PROXY_MODELS_SPECIAL_VALUE.value) || organization?.models.length === 0; const shouldShowAllProxyModels = - showAllProxyModelsOverride || (organizationHasAllProxyModels && includeSpecialOptions) || context === "global"; + keepOnlyCurrentTeamModels(team, options, []) === null && + (showAllProxyModelsOverride || (organizationHasAllProxyModels && includeSpecialOptions) || context === "global"); if (isLoading) { return ; diff --git a/ui/litellm-dashboard/src/components/team/TeamInfo.test.tsx b/ui/litellm-dashboard/src/components/team/TeamInfo.test.tsx index 712cff80649..27a9d8c0a7a 100644 --- a/ui/litellm-dashboard/src/components/team/TeamInfo.test.tsx +++ b/ui/litellm-dashboard/src/components/team/TeamInfo.test.tsx @@ -3,7 +3,7 @@ import { screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { renderWithProviders, testQueryClient } from "../../../tests/test-utils"; -import TeamInfoView from "./TeamInfo"; +import TeamInfoView, { validateTeamMaxBudget } from "./TeamInfo"; vi.mock("@/components/networking", () => ({ teamInfoCall: vi.fn(), @@ -43,6 +43,21 @@ vi.mock("@/app/(dashboard)/hooks/users/useCurrentUser", () => ({ useCurrentUser: vi.fn(), })); +let mockUserRole = "Admin"; + +vi.mock("@/app/(dashboard)/hooks/useAuthorized", () => ({ + default: () => ({ + token: "123", + accessToken: "test-token", + userId: "user-1", + userEmail: "user@example.com", + userRole: mockUserRole, + premiumUser: false, + disabledPersonalKeyCreation: null, + showSSOBanner: false, + }), +})); + vi.mock("@/components/team/TeamMemberTab", () => ({ default: vi.fn(({ setIsAddMemberModalVisible }) => (
@@ -1193,4 +1208,119 @@ describe("TeamInfoView", () => { }); }); }); + describe("team-admin budget authority", () => { + const teamAdminProps = { ...defaultProps, is_proxy_admin: false, is_team_admin: true }; + + const openSettingsForm = async (user: ReturnType) => { + await waitFor(() => { + expect(screen.queryAllByText("Test Team").length).toBeGreaterThan(0); + }); + await user.click(screen.getByRole("tab", { name: "Settings" })); + await user.click(await screen.findByRole("button", { name: /edit settings/i })); + await screen.findByLabelText("Team Name"); + }; + + beforeEach(() => { + testQueryClient.clear(); + mockUserRole = "Internal User"; + vi.mocked(networking.teamUpdateCall).mockResolvedValue({ data: {}, team_id: "123" } as any); + vi.mocked(networking.teamInfoCall).mockResolvedValue( + createMockTeamData({ models: ["gpt-4"], max_budget: 30 }), + ); + }); + + afterEach(() => { + mockUserRole = "Admin"; + }); + + it("blocks a raise before the request leaves the browser", async () => { + const user = userEvent.setup({ delay: null }); + renderWithProviders(); + await openSettingsForm(user); + + const budgetInput = screen.getByLabelText("Max Budget (USD)"); + await user.clear(budgetInput); + await user.type(budgetInput, "100"); + await user.click(screen.getByRole("button", { name: /save changes/i })); + + expect(await screen.findByText(/Only a proxy admin can raise this team's budget above \$30/)).toBeInTheDocument(); + expect(networking.teamUpdateCall).not.toHaveBeenCalled(); + }); + + it("blocks clearing the cap", async () => { + const user = userEvent.setup({ delay: null }); + renderWithProviders(); + await openSettingsForm(user); + + await user.clear(screen.getByLabelText("Max Budget (USD)")); + await user.click(screen.getByRole("button", { name: /save changes/i })); + + expect(await screen.findByText(/Only a proxy admin can remove this team's budget/)).toBeInTheDocument(); + expect(networking.teamUpdateCall).not.toHaveBeenCalled(); + }); + + it("lets a team admin lower the cap", async () => { + const user = userEvent.setup({ delay: null }); + renderWithProviders(); + await openSettingsForm(user); + + const budgetInput = screen.getByLabelText("Max Budget (USD)"); + await user.clear(budgetInput); + await user.type(budgetInput, "10"); + await user.click(screen.getByRole("button", { name: /save changes/i })); + + await waitFor(() => { + expect(networking.teamUpdateCall).toHaveBeenCalled(); + }); + const [accessToken, payload] = vi.mocked(networking.teamUpdateCall).mock.calls[0]; + expect(accessToken).toBe("test-token"); + expect(payload.team_id).toBe("123"); + expect(Number(payload.max_budget)).toBe(10); + }); + + it("leaves a proxy admin free to raise the cap", async () => { + const user = userEvent.setup({ delay: null }); + mockUserRole = "Admin"; + renderWithProviders(); + await openSettingsForm(user); + + const budgetInput = screen.getByLabelText("Max Budget (USD)"); + await user.clear(budgetInput); + await user.type(budgetInput, "100"); + await user.click(screen.getByRole("button", { name: /save changes/i })); + + await waitFor(() => { + expect(networking.teamUpdateCall).toHaveBeenCalled(); + }); + const [accessToken, payload] = vi.mocked(networking.teamUpdateCall).mock.calls[0]; + expect(accessToken).toBe("test-token"); + expect(payload.team_id).toBe("123"); + expect(Number(payload.max_budget)).toBe(100); + }); + }); + + describe("validateTeamMaxBudget", () => { + it("is inert for callers who hold the grant, or when the team has no cap", async () => { + await expect(validateTeamMaxBudget(true, 30)(null, 1000)).resolves.toBeUndefined(); + await expect(validateTeamMaxBudget(false, null)(null, 1000)).resolves.toBeUndefined(); + await expect(validateTeamMaxBudget(false, undefined)(null, "")).resolves.toBeUndefined(); + }); + + it("allows keeping or lowering, rejects raising or clearing", async () => { + const validate = validateTeamMaxBudget(false, 30); + + await expect(validate(null, 30)).resolves.toBeUndefined(); + await expect(validate(null, 29.99)).resolves.toBeUndefined(); + await expect(validate(null, 30.01)).rejects.toThrow(/raise/); + await expect(validate(null, "")).rejects.toThrow(/remove/); + await expect(validate(null, null)).rejects.toThrow(/remove/); + }); + + it("treats a zero cap as a real ceiling", async () => { + const validate = validateTeamMaxBudget(false, 0); + + await expect(validate(null, 0)).resolves.toBeUndefined(); + await expect(validate(null, 5)).rejects.toThrow(/raise/); + }); + }); }); diff --git a/ui/litellm-dashboard/src/components/team/TeamInfo.tsx b/ui/litellm-dashboard/src/components/team/TeamInfo.tsx index 34570043f52..84544f74432 100644 --- a/ui/litellm-dashboard/src/components/team/TeamInfo.tsx +++ b/ui/litellm-dashboard/src/components/team/TeamInfo.tsx @@ -144,6 +144,25 @@ export interface TeamInfoProps { premiumUser?: boolean; } +/** + * Mirrors the /team/update budget-authority gate so a team admin sees why the + * save is refused before the round-trip: they may keep or lower the team's + * ceiling, never raise or remove it. + */ +export const validateTeamMaxBudget = + (canWidenTeamGrants: boolean, currentMaxBudget: number | null | undefined) => + async (_rule: unknown, value: unknown): Promise => { + if (canWidenTeamGrants || currentMaxBudget == null) return; + if (value === null || value === undefined || value === "") { + throw new Error(`Only a proxy admin can remove this team's budget (currently $${currentMaxBudget})`); + } + const requested = Number(value); + if (Number.isNaN(requested)) return; + if (requested > currentMaxBudget) { + throw new Error(`Only a proxy admin can raise this team's budget above $${currentMaxBudget}`); + } + }; + const getOrganizationModels = (organization: Organization | null, userModels: string[]) => { let tempModelsToPick = []; @@ -234,6 +253,10 @@ const TeamInfoView: React.FC = ({ ); const canEditTeam = is_team_admin || is_proxy_admin || is_org_admin || isOrgAdminForTeam || isTeamAdminFromTeamData; + const isTeamAdminForThisTeam = is_team_admin || isTeamAdminFromTeamData; + const holdsAuthorityOverTeam = + isProxyAdminRole(userRole) || is_proxy_admin || is_org_admin || isOrgAdminForTeam; + const canWidenTeamGrants = holdsAuthorityOverTeam || !isTeamAdminForThisTeam; const visibleTabs = useMemo(() => getTeamInfoVisibleTabs(canEditTeam), [canEditTeam]); const defaultTabKey = useMemo(() => getTeamInfoDefaultTab(editTeam, canEditTeam), [editTeam, canEditTeam]); @@ -1042,6 +1065,7 @@ const TeamInfoView: React.FC = ({ includeUserModels: !teamData?.team_info?.organization_id, showAllProxyModelsOverride: isProxyAdminRole(userRole) && !teamData?.team_info?.organization_id, + restrictToCurrentTeamModels: !canWidenTeamGrants, }} context="team" dataTestId="models-select" @@ -1066,7 +1090,16 @@ const TeamInfoView: React.FC = ({ /> - +