mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-13 23:11:40 +00:00
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.
This commit is contained in:
parent
4c3afe3845
commit
7821a7c3be
9 changed files with 362 additions and 102 deletions
|
|
@ -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.
|
||||
|
||||
|
|
|
|||
|
|
@ -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}."},
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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(() => {
|
||||
|
|
|
|||
|
|
@ -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<ModelSelectProps["context"], (args: FilterContextArgs) => string[]> = {
|
||||
|
|
@ -76,8 +71,8 @@ const contextFilters: Record<ModelSelectProps["context"], (args: FilterContextAr
|
|||
},
|
||||
|
||||
team: ({ allProxyModels, selectedTeam, selectedOrganization, userModels, options }) => {
|
||||
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) {
|
||||
|
|
|
|||
|
|
@ -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(<TeamInfoView {...teamAdminProps} />);
|
||||
|
||||
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(<TeamInfoView {...defaultProps} />);
|
||||
|
||||
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", () => {
|
||||
|
|
|
|||
|
|
@ -254,8 +254,7 @@ const TeamInfoView: React.FC<TeamInfoProps> = ({
|
|||
|
||||
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<TeamInfoProps> = ({
|
|||
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"
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue