From 7821a7c3be27d3ede6e2fc488f186b3ff891f1ca Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Sat, 1 Aug 2026 13:10:11 -0700 Subject: [PATCH] fix(team): refuse team-admin grant widening on /team/update Follows the review pass: models:[] / "*" widening, wildcard and access-group narrowing false-403s, the org-admin-who-is-also-team-admin lockout, malformed model and passthrough payloads, and the dashboard reading the wrong shape for the team's model list. --- litellm/proxy/auth/auth_checks.py | 3 +- .../management_endpoints/common_utils.py | 24 ++-- .../management_endpoints/team_endpoints.py | 111 +++++++++++------- .../management_endpoints/test_common_utils.py | 65 ++++++++++ .../test_team_endpoints.py | 109 +++++++++++++++++ .../ModelSelect/ModelSelect.test.tsx | 38 +++--- .../components/ModelSelect/ModelSelect.tsx | 39 +++--- .../src/components/team/TeamInfo.test.tsx | 70 ++++++++++- .../src/components/team/TeamInfo.tsx | 5 +- 9 files changed, 362 insertions(+), 102 deletions(-) diff --git a/litellm/proxy/auth/auth_checks.py b/litellm/proxy/auth/auth_checks.py index 263fec77d12..79b7d2e45d2 100644 --- a/litellm/proxy/auth/auth_checks.py +++ b/litellm/proxy/auth/auth_checks.py @@ -13,6 +13,7 @@ import asyncio import math import re import time +from collections.abc import Sequence from typing import TYPE_CHECKING, Any, Dict, List, Literal, Optional, Type, Union, cast from fastapi import HTTPException, Request, status @@ -4441,7 +4442,7 @@ def is_model_allowed_by_pattern(model: str, allowed_model_pattern: str) -> bool: return False -def _model_matches_any_wildcard_pattern_in_list(model: str, allowed_model_list: list) -> bool: +def _model_matches_any_wildcard_pattern_in_list(model: str, allowed_model_list: Sequence[str]) -> bool: """ Returns True if a model matches any wildcard pattern in a list. diff --git a/litellm/proxy/management_endpoints/common_utils.py b/litellm/proxy/management_endpoints/common_utils.py index cf4623839b5..9a886f5bedf 100644 --- a/litellm/proxy/management_endpoints/common_utils.py +++ b/litellm/proxy/management_endpoints/common_utils.py @@ -86,11 +86,14 @@ def _passthrough_routes_differ(requested: object, existing: Sequence[str] | None """ if requested is None: return False - if not isinstance(requested, (list, tuple)): + if not _is_route_list(requested): return True - if any(not isinstance(route, str) for route in requested): - return True - return frozenset(requested) != frozenset(existing or ()) + stored = existing if _is_route_list(existing) else () + return frozenset(requested) != frozenset(stored) + + +def _is_route_list(value: object) -> bool: + return isinstance(value, (list, tuple)) and all(isinstance(route, str) for route in value) def _check_passthrough_routes_caller_permission( @@ -120,9 +123,16 @@ def _check_passthrough_routes_caller_permission( detail={"error": f"Only proxy admins can set `allowed_passthrough_routes` on a {entity}."}, ) metadata = getattr(data, "metadata", None) - if isinstance(metadata, dict) and _passthrough_routes_differ( - metadata.get("allowed_passthrough_routes"), existing_routes - ): + if not isinstance(metadata, dict) or "allowed_passthrough_routes" not in metadata: + return + requested_in_metadata = metadata["allowed_passthrough_routes"] + # an explicit null wipes the stored routes, so it is a change like any other + metadata_differs = ( + bool(existing_routes) + if requested_in_metadata is None + else _passthrough_routes_differ(requested_in_metadata, existing_routes) + ) + if metadata_differs: 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 a342f5863d4..041872720b5 100644 --- a/litellm/proxy/management_endpoints/team_endpoints.py +++ b/litellm/proxy/management_endpoints/team_endpoints.py @@ -1071,27 +1071,28 @@ def _check_team_budget_update_authority( Setting a finite budget on a team that has no cap is a restriction and is allowed. Org-scoped teams are additionally capped by _check_org_team_limits(). """ - if team_access is not TeamAccessGrant.TEAM_ADMIN: + if team_access in (TeamAccessGrant.PROXY_ADMIN, TeamAccessGrant.ORG_ADMIN): return if existing_team_max_budget is None: return - budget_explicitly_set = "max_budget" in (getattr(data, "model_fields_set", None) or set()) - if budget_explicitly_set and data.max_budget is None: - raise HTTPException( - status_code=403, - detail={ - "error": f"Only a proxy admin can remove a team's max_budget. Team's current max_budget={existing_team_max_budget}." - }, - ) + budget_explicitly_set = "max_budget" in (getattr(data, "model_fields_set", None) or frozenset()) + removing_cap = budget_explicitly_set and data.max_budget is None + raising_cap = data.max_budget is not None and data.max_budget > existing_team_max_budget + if not removing_cap and not raising_cap: + return - if data.max_budget is not None and data.max_budget > existing_team_max_budget: - raise HTTPException( - status_code=403, - detail={ - "error": f"Only a proxy admin can raise a team's max_budget. Team's current max_budget={existing_team_max_budget}, requested={data.max_budget}." - }, - ) + action = "remove" if removing_cap else "raise" + requested = "" if removing_cap else f", requested={data.max_budget}" + raise HTTPException( + status_code=403, + detail={ + "error": ( + f"Only a proxy admin can {action} a team's max_budget. " + f"Team's current max_budget={existing_team_max_budget}{requested}." + ) + }, + ) def _model_list_grants_every_model(models: Sequence[str]) -> bool: @@ -1103,6 +1104,24 @@ def _model_list_grants_every_model(models: Sequence[str]) -> bool: return len(models) == 0 or "*" in models or SpecialModelNames.all_proxy_models.value in models +def _team_models_reachable_today( + existing_team_models: Sequence[str], + llm_router: Optional[Router], +) -> tuple[str, ...]: + """The team's stored models plus the members of any access group it holds. + + `team.models` may name a router access group; the auth path expands those + before deciding what the team can call, so the widening check has to expand + them too or a legitimate narrowing reads as a new grant. + """ + if llm_router is None: + return tuple(existing_team_models) + access_groups = llm_router.get_model_access_groups() + return tuple(existing_team_models) + tuple( + model for name in existing_team_models for model in access_groups.get(name, ()) + ) + + def _check_team_models_update_authority( data: UpdateTeamRequest, team_access: TeamAccessGrant, @@ -1117,40 +1136,43 @@ def _check_team_models_update_authority( 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: + if team_access in (TeamAccessGrant.PROXY_ADMIN, TeamAccessGrant.ORG_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))}." + if model != SpecialModelNames.no_default_models.value + and ( + not isinstance(model, str) + or ( + model not in existing_team_models + and not _model_matches_any_wildcard_pattern_in_list( + model=model, allowed_model_list=existing_team_models ) - }, + ) ) + ) + if not _model_list_grants_every_model(data.models) and not added_models: + return + + widening = ( + "grant a team access to every proxy model" + if _model_list_grants_every_model(data.models) + else f"add models to a team. Models the team cannot already reach: {sorted(frozenset(added_models))}" + ) + raise HTTPException( + status_code=403, + detail={ + "error": ( + f"Only a proxy admin can {widening}. Team's current models={sorted(frozenset(existing_team_models))}." + ) + }, + ) def _should_auto_add_team_creator( @@ -1981,8 +2003,10 @@ async def update_team( user_api_key_dict=user_api_key_dict, ) - existing_metadata = existing_team_row.metadata if isinstance(existing_team_row.metadata, dict) else {} - stored_passthrough_routes = existing_metadata.get("allowed_passthrough_routes") + existing_metadata = existing_team_row.metadata + stored_passthrough_routes = ( + existing_metadata.get("allowed_passthrough_routes") if isinstance(existing_metadata, dict) else None + ) _check_passthrough_routes_caller_permission( data, user_api_key_dict, @@ -1995,7 +2019,10 @@ async def update_team( _check_team_models_update_authority( data=data, team_access=team_access, - existing_team_models=existing_team_row.models or [], + existing_team_models=_team_models_reachable_today( + existing_team_models=existing_team_row.models or (), + llm_router=llm_router, + ), ) if data.soft_budget is not None: 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 5ec6c1319d6..0eca6b31197 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_common_utils.py +++ b/tests/test_litellm/proxy/management_endpoints/test_common_utils.py @@ -1046,3 +1046,68 @@ class TestUpdateMetadataFieldMove: _update_metadata_fields(updated_kv) assert "guardrails" not in updated_kv assert updated_kv["metadata"]["guardrails"] == ["g1"] + + +class TestPassthroughRoutesStoredValueEdgeCases: + """The stored side of the comparison is caller data too, and an explicit null + is a wipe, not an omission.""" + + def _non_admin(self): + return UserAPIKeyAuth(user_id="u1", api_key="sk-x", user_role=LitellmUserRoles.INTERNAL_USER) + + def _data(self, metadata): + from pydantic import BaseModel + + class _RouteData(BaseModel): + allowed_passthrough_routes: list | None = None + metadata: dict | None = None + + return _RouteData(metadata=metadata) + + def test_unhashable_stored_routes_do_not_crash(self): + from fastapi import HTTPException + + from litellm.proxy.management_endpoints.common_utils import ( + _check_passthrough_routes_caller_permission, + ) + + with pytest.raises(HTTPException) as exc_info: + _check_passthrough_routes_caller_permission( + self._data({"allowed_passthrough_routes": ["/v1/foo"]}), + self._non_admin(), + entity="team", + existing_routes=[{"route": "/v1/foo"}], + ) + + assert exc_info.value.status_code == 403 + + def test_explicit_null_that_would_wipe_stored_routes_is_rejected(self): + from fastapi import HTTPException + + from litellm.proxy.management_endpoints.common_utils import ( + _check_passthrough_routes_caller_permission, + ) + + with pytest.raises(HTTPException) as exc_info: + _check_passthrough_routes_caller_permission( + self._data({"allowed_passthrough_routes": None}), + self._non_admin(), + entity="team", + existing_routes=["/v1/foo"], + ) + + assert exc_info.value.status_code == 403 + + def test_explicit_null_with_nothing_stored_is_allowed(self): + from litellm.proxy.management_endpoints.common_utils import ( + _check_passthrough_routes_caller_permission, + ) + + assert ( + _check_passthrough_routes_caller_permission( + self._data({"allowed_passthrough_routes": None}), + self._non_admin(), + entity="team", + ) + is None + ) 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 97e4d01705d..3bf66db8bdb 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py @@ -10990,3 +10990,112 @@ async def test_update_team_budget_raise_allowed_for_team_admin_who_is_also_org_a ) assert result is not None + + +@pytest.mark.asyncio +async def test_update_team_narrowing_a_bare_star_grant_allowed_for_team_admin(): + """A team stored with "*" already reaches everything, so naming models narrows it.""" + from litellm.proxy._types import UpdateTeamRequest + + result = await _run_update_team( + update_request=UpdateTeamRequest(team_id="doc-alignment-team", models=["gpt-4"]), + caller=_team_admin_caller(), + existing_team=_stored_team(models=["*"]), + ) + + assert result is not None + + +@pytest.mark.asyncio +async def test_update_team_no_default_models_allowed_for_team_admin(): + """Handing the team the no-default-models sentinel is the strictest narrowing there is.""" + from litellm.proxy._types import UpdateTeamRequest + + result = await _run_update_team( + update_request=UpdateTeamRequest(team_id="doc-alignment-team", models=["no-default-models"]), + caller=_team_admin_caller(), + existing_team=_stored_team(), + ) + + assert result is not None + + +@pytest.mark.asyncio +async def test_update_team_non_string_model_entry_is_refused_not_crashed(): + """A junk element must land on the 403, not blow up inside the wildcard matcher.""" + 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=[123]), + caller=_team_admin_caller(), + existing_team=_stored_team(models=["openai/*"]), + ) + + assert exc_info.value.code == "403" + + +@pytest.mark.asyncio +async def test_update_team_access_group_member_is_reachable_for_team_admin(): + """team.models may name a router access group; narrowing to one of its members is not a new grant.""" + from litellm.proxy._types import UpdateTeamRequest + + mock_router = MagicMock() + mock_router.get_model_access_groups.return_value = {"beta-models": ["gpt-4o", "o4-mini"]} + + with patch("litellm.proxy.proxy_server.llm_router", mock_router): + result = await _run_update_team( + update_request=UpdateTeamRequest(team_id="doc-alignment-team", models=["gpt-4o"]), + caller=_team_admin_caller(), + existing_team=_stored_team(models=["beta-models"]), + ) + + assert result is not None + + +def test_team_grant_gates_are_fail_closed_on_an_unknown_grant(): + """A grant value that isn't a known enum member must not disable the gates.""" + from litellm.proxy._types import UpdateTeamRequest + from litellm.proxy.management_endpoints.team_endpoints import ( + _check_team_budget_update_authority, + _check_team_models_update_authority, + ) + + request = UpdateTeamRequest(team_id="doc-alignment-team", models=["claude-opus-4-5"], max_budget=999.0) + + with pytest.raises(HTTPException) as exc_info: + _check_team_models_update_authority( + data=request, + team_access="team_admin", # a plain string stands in for a stale or serialized grant + existing_team_models=["gpt-4"], + ) + assert exc_info.value.status_code == 403 + + with pytest.raises(HTTPException) as budget_exc_info: + _check_team_budget_update_authority( + data=request, + team_access="team_admin", + existing_team_max_budget=30.0, + ) + assert budget_exc_info.value.status_code == 403 + + +@pytest.mark.asyncio +async def test_is_org_admin_for_team_or_false_degrades_instead_of_raising(): + """A failed org lookup must withhold authority, never 500 the request.""" + from litellm.proxy._types import LiteLLM_TeamTable + from litellm.proxy.management_endpoints.team_endpoints import _is_org_admin_for_team_or_false + + caller = _team_admin_caller() + standalone_team = LiteLLM_TeamTable(team_id="t", organization_id=None) + org_team = LiteLLM_TeamTable(team_id="t", organization_id="org-1") + + with patch( + "litellm.proxy.management_endpoints.team_endpoints._is_user_org_admin_for_team", + new=AsyncMock(side_effect=ValueError("User doesn't exist in db")), + ) as mock_lookup: + assert await _is_org_admin_for_team_or_false(team_obj=standalone_team, user_api_key_dict=caller) is False + mock_lookup.assert_not_called() + + assert await _is_org_admin_for_team_or_false(team_obj=org_team, user_api_key_dict=caller) is False + mock_lookup.assert_awaited_once() diff --git a/ui/litellm-dashboard/src/components/ModelSelect/ModelSelect.test.tsx b/ui/litellm-dashboard/src/components/ModelSelect/ModelSelect.test.tsx index 0e84d886d3a..a95c85529f0 100644 --- a/ui/litellm-dashboard/src/components/ModelSelect/ModelSelect.test.tsx +++ b/ui/litellm-dashboard/src/components/ModelSelect/ModelSelect.test.tsx @@ -578,12 +578,8 @@ 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); + describe("restrictToModels", () => { + const renderRestricted = (grantedModels: string[], organizationModels?: string[]) => { if (organizationModels) { mockUseOrganization.mockReturnValue({ data: createMockOrganization(organizationModels), @@ -597,12 +593,12 @@ describe("ModelSelect", () => { context="team" teamID="team-1" organizationID={organizationModels ? "org-1" : undefined} - options={{ includeSpecialOptions: true, restrictToCurrentTeamModels: true }} + options={{ includeSpecialOptions: true, restrictToModels: grantedModels }} />, ); }; - it("offers only the models the team already holds", async () => { + it("offers only the models the grant already covers", async () => { renderRestricted(["gpt-4"]); await waitFor(() => { @@ -612,7 +608,7 @@ describe("ModelSelect", () => { expect(screen.queryByRole("option", { name: "All Proxy Models" })).not.toBeInTheDocument(); }); - it("hides All Proxy Models even when the team's org grants everything", async () => { + it("hides All Proxy Models even when the org grants everything", async () => { renderRestricted(["gpt-4"], ["all-proxy-models"]); await waitFor(() => { @@ -622,16 +618,7 @@ describe("ModelSelect", () => { 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 () => { + it("keeps every model a wildcard grant already reaches selectable", async () => { mockUseAllProxyModels.mockReturnValue({ data: { data: [ @@ -650,7 +637,16 @@ describe("ModelSelect", () => { expect(screen.queryByRole("option", { name: "anthropic/claude-opus-4-5" })).not.toBeInTheDocument(); }); - it("does not restrict a team holding the bare * grant", async () => { + it("does not restrict a grant 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("does not restrict a bare * grant", async () => { renderRestricted(["*"]); await waitFor(() => { @@ -659,7 +655,7 @@ describe("ModelSelect", () => { expect(screen.getByRole("option", { name: "claude-3" })).toBeInTheDocument(); }); - it("does not restrict a team with an empty model list", async () => { + it("does not restrict when no grant list is given", async () => { renderRestricted([]); await waitFor(() => { diff --git a/ui/litellm-dashboard/src/components/ModelSelect/ModelSelect.tsx b/ui/litellm-dashboard/src/components/ModelSelect/ModelSelect.tsx index 88403de808d..3542b3bbdf2 100644 --- a/ui/litellm-dashboard/src/components/ModelSelect/ModelSelect.tsx +++ b/ui/litellm-dashboard/src/components/ModelSelect/ModelSelect.tsx @@ -3,6 +3,7 @@ import { useOrganization } from "@/app/(dashboard)/hooks/organizations/useOrgani import { useTeam } from "@/app/(dashboard)/hooks/teams/useTeams"; import { useCurrentUser } from "@/app/(dashboard)/hooks/users/useCurrentUser"; import { Select, Skeleton, Tooltip, type SelectProps } from "antd"; +import { unfurlWildcardModelsInList } from "../key_team_helpers/fetch_available_models_team_key"; import { Organization, Team } from "../networking"; import { splitWildcardModels } from "./modelUtils"; @@ -29,7 +30,7 @@ export interface ModelSelectProps { showAllTeamModelsOption?: boolean; showAllProxyModelsOverride?: boolean; includeSpecialOptions?: boolean; - restrictToCurrentTeamModels?: boolean; + restrictToModels?: string[]; }; context: "team" | "organization" | "user" | "global"; dataTestId?: string; @@ -47,25 +48,19 @@ type FilterContextArgs = { }; /** - * 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. + * The models the caller may pick from when they can only narrow an existing + * grant (a team admin can drop a model but not add one — /team/update rejects + * it). Returns null when the restriction doesn't apply, including when the + * grant 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 restrictedModelOptions = (options: ModelSelectProps["options"], allProxyModels: string[]): string[] | null => { + const grantedModels = options?.restrictToModels; + if (!grantedModels || grantedModels.length === 0) return null; + if (grantedModels.includes(MODEL_SELECT_ALL_PROXY_MODELS_SPECIAL_VALUE.value) || grantedModels.includes("*")) { + return null; + } + const reachable = unfurlWildcardModelsInList(grantedModels, allProxyModels); + return Array.from(new Set([...grantedModels, ...reachable])); }; const contextFilters: Record string[]> = { @@ -76,8 +71,8 @@ const contextFilters: Record { - const currentTeamModels = keepOnlyCurrentTeamModels(selectedTeam, options, allProxyModels); - if (currentTeamModels) return currentTeamModels; + const restrictedModels = restrictedModelOptions(options, allProxyModels); + if (restrictedModels) return restrictedModels; if (selectedOrganization) { if ( @@ -133,7 +128,7 @@ export const ModelSelect = (props: ModelSelectProps) => { organization?.models.includes(MODEL_SELECT_ALL_PROXY_MODELS_SPECIAL_VALUE.value) || organization?.models.length === 0; const shouldShowAllProxyModels = - keepOnlyCurrentTeamModels(team, options, []) === null && + restrictedModelOptions(options, []) === null && (showAllProxyModelsOverride || (organizationHasAllProxyModels && includeSpecialOptions) || context === "global"); if (isLoading) { diff --git a/ui/litellm-dashboard/src/components/team/TeamInfo.test.tsx b/ui/litellm-dashboard/src/components/team/TeamInfo.test.tsx index 27a9d8c0a7a..386773d626d 100644 --- a/ui/litellm-dashboard/src/components/team/TeamInfo.test.tsx +++ b/ui/litellm-dashboard/src/components/team/TeamInfo.test.tsx @@ -1224,9 +1224,7 @@ describe("TeamInfoView", () => { 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 }), - ); + vi.mocked(networking.teamInfoCall).mockResolvedValue(createMockTeamData({ models: ["gpt-4"], max_budget: 30 })); }); afterEach(() => { @@ -1245,7 +1243,7 @@ describe("TeamInfoView", () => { expect(await screen.findByText(/Only a proxy admin can raise this team's budget above \$30/)).toBeInTheDocument(); expect(networking.teamUpdateCall).not.toHaveBeenCalled(); - }); + }, 60000); it("blocks clearing the cap", async () => { const user = userEvent.setup({ delay: null }); @@ -1257,7 +1255,7 @@ describe("TeamInfoView", () => { expect(await screen.findByText(/Only a proxy admin can remove this team's budget/)).toBeInTheDocument(); expect(networking.teamUpdateCall).not.toHaveBeenCalled(); - }); + }, 60000); it("lets a team admin lower the cap", async () => { const user = userEvent.setup({ delay: null }); @@ -1276,7 +1274,7 @@ describe("TeamInfoView", () => { expect(accessToken).toBe("test-token"); expect(payload.team_id).toBe("123"); expect(Number(payload.max_budget)).toBe(10); - }); + }, 60000); it("leaves a proxy admin free to raise the cap", async () => { const user = userEvent.setup({ delay: null }); @@ -1296,7 +1294,67 @@ describe("TeamInfoView", () => { expect(accessToken).toBe("test-token"); expect(payload.team_id).toBe("123"); expect(Number(payload.max_budget)).toBe(100); + }, 60000); + }); + + describe("team-admin model grants", () => { + const teamAdminProps = { ...defaultProps, is_proxy_admin: false, is_team_admin: true }; + + beforeEach(() => { + testQueryClient.clear(); + mockUserRole = "Internal User"; + mockUseAllProxyModels.mockReturnValue({ + data: { + data: [ + { id: "gpt-4", object: "model", created: 1, owned_by: "openai" }, + { id: "claude-opus-4-5", object: "model", created: 1, owned_by: "anthropic" }, + ], + }, + isLoading: false, + } as any); + vi.mocked(networking.teamInfoCall).mockResolvedValue(createMockTeamData({ models: ["gpt-4"] })); }); + + afterEach(() => { + mockUserRole = "Admin"; + }); + + it("offers a team admin only the models the team already holds", async () => { + const user = userEvent.setup({ delay: null }); + renderWithProviders(); + + 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 })); + + const modelsSelect = await screen.findByTestId("models-select"); + await user.click(within(modelsSelect).getByRole("combobox")); + + await waitFor(() => { + expect(screen.getAllByText("gpt-4").length).toBeGreaterThan(0); + }); + expect(screen.queryByText("claude-opus-4-5")).not.toBeInTheDocument(); + expect(screen.queryByText("All Proxy Models")).not.toBeInTheDocument(); + }, 60000); + + it("leaves a proxy admin the full proxy model list", async () => { + const user = userEvent.setup({ delay: null }); + mockUserRole = "Admin"; + renderWithProviders(); + + 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 })); + + const modelsSelect = await screen.findByTestId("models-select"); + await user.click(within(modelsSelect).getByRole("combobox")); + + expect(await screen.findByText("claude-opus-4-5")).toBeInTheDocument(); + }, 60000); }); describe("validateTeamMaxBudget", () => { diff --git a/ui/litellm-dashboard/src/components/team/TeamInfo.tsx b/ui/litellm-dashboard/src/components/team/TeamInfo.tsx index 84544f74432..37c846c84d5 100644 --- a/ui/litellm-dashboard/src/components/team/TeamInfo.tsx +++ b/ui/litellm-dashboard/src/components/team/TeamInfo.tsx @@ -254,8 +254,7 @@ 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 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]); @@ -1065,7 +1064,7 @@ const TeamInfoView: React.FC = ({ includeUserModels: !teamData?.team_info?.organization_id, showAllProxyModelsOverride: isProxyAdminRole(userRole) && !teamData?.team_info?.organization_id, - restrictToCurrentTeamModels: !canWidenTeamGrants, + restrictToModels: canWidenTeamGrants ? undefined : info.models ?? [], }} context="team" dataTestId="models-select"