mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-09 22:31:41 +00:00
fix(ui): let team admins grant a team all proxy models
The team edit form only offered "All Proxy Models" when the dashboard could read the parent organization, and /organization/info 403s for anyone who is not a proxy admin or an admin of that org. A team admin with the internal_user proxy role therefore saw only "No Default Models", which is the opposite of what they wanted, and had no way to grant their team everything on the proxy. /team/info now reports the parent org's model ceiling as organization_models, which the same authorization already admits, and ModelSelect reads the ceiling from the team it is editing before falling back to the organization. That also makes the individual model list respect the org's allow-list instead of listing every proxy model to a caller whose save would be rejected. useTeam was typed as Team while returning the /team/info envelope, so its one other caller unwrapped it behind a cast. It now returns the team itself. Claude-Session: https://claude.ai/code/session_011Tn3657NkV6ojLqewL64Kb
This commit is contained in:
parent
13df85cceb
commit
14a5c15224
10 changed files with 201 additions and 46 deletions
|
|
@ -4313,6 +4313,8 @@ class TeamInfoResponseObjectTeamTable(LiteLLM_TeamTable):
|
|||
access_group_mcp_server_ids: list[str] | None = None
|
||||
access_group_agent_ids: list[str] | None = None
|
||||
access_group_details: tuple[TeamAccessGroupModelGrant, ...] | None = None
|
||||
# Parent org's model ceiling. None = no org; [] or ["all-proxy-models"] = no ceiling.
|
||||
organization_models: list[str] | None = None
|
||||
|
||||
|
||||
class TeamInfoResponseObject(TypedDict):
|
||||
|
|
|
|||
|
|
@ -4325,6 +4325,20 @@ async def _hydrate_member_emails(
|
|||
)
|
||||
|
||||
|
||||
class _OrganizationModelsRow(BaseModel):
|
||||
models: list[str] = [] # mutable-ok: pydantic field default
|
||||
|
||||
|
||||
class _TeamRowWithOrganization(BaseModel):
|
||||
litellm_organization_table: _OrganizationModelsRow | None = None
|
||||
|
||||
|
||||
def _parent_organization_models(team_row: BaseModel) -> list[str] | None:
|
||||
"""Return the parent org's model allow-list, or None when the team has no org."""
|
||||
organization: Final = _TeamRowWithOrganization.model_validate(team_row.model_dump()).litellm_organization_table
|
||||
return organization.models if organization is not None else None
|
||||
|
||||
|
||||
async def _resolve_team_access_group_resources(
|
||||
_team_info: TeamInfoResponseObjectTeamTable,
|
||||
) -> TeamInfoResponseObjectTeamTable:
|
||||
|
|
@ -4396,7 +4410,11 @@ async def team_info(
|
|||
try:
|
||||
team_info: BaseModel | None = await _team_db(prisma_client).find_unique(
|
||||
where={"team_id": team_id},
|
||||
include={"litellm_model_table": True, "object_permission": True},
|
||||
include={
|
||||
"litellm_model_table": True,
|
||||
"object_permission": True,
|
||||
"litellm_organization_table": True,
|
||||
},
|
||||
)
|
||||
if team_info is None:
|
||||
raise Exception
|
||||
|
|
@ -4405,6 +4423,7 @@ async def team_info(
|
|||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
detail={"message": f"Team not found, passed team id: {team_id}."},
|
||||
)
|
||||
organization_models: Final[list[str] | None] = _parent_organization_models(team_info)
|
||||
await validate_membership(
|
||||
user_api_key_dict=user_api_key_dict,
|
||||
team_table=LiteLLM_TeamTable.model_validate(team_info.model_dump()),
|
||||
|
|
@ -4471,7 +4490,8 @@ async def team_info(
|
|||
update={ # mutable-ok: pydantic update payload
|
||||
# list(), not the tuple: model_copy skips validation, so the field has
|
||||
# to be handed the list[Member] the response model declares.
|
||||
"members_with_roles": list(hydrated_members) # mutable-ok: declared list[Member]
|
||||
"members_with_roles": list(hydrated_members), # mutable-ok: declared list[Member]
|
||||
"organization_models": organization_models,
|
||||
}
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -14067,3 +14067,75 @@ async def test_get_team_spend_by_user_rejects_bad_input(mock_db_client, team_ids
|
|||
assert exc_info.value.status_code == 400
|
||||
assert expected_error in str(exc_info.value.detail)
|
||||
mock_db_client.db.query_raw.assert_not_called()
|
||||
|
||||
|
||||
class _TeamRowWithOrganization(LiteLLM_TeamTable):
|
||||
litellm_organization_table: LiteLLM_OrganizationTable | None = None
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"organization, expected_models",
|
||||
[
|
||||
(
|
||||
LiteLLM_OrganizationTable(
|
||||
organization_id="org-1",
|
||||
budget_id="budget-1",
|
||||
models=["all-proxy-models"],
|
||||
created_by="admin",
|
||||
updated_by="admin",
|
||||
),
|
||||
["all-proxy-models"],
|
||||
),
|
||||
(
|
||||
LiteLLM_OrganizationTable(
|
||||
organization_id="org-1",
|
||||
budget_id="budget-1",
|
||||
models=["gpt-4o"],
|
||||
created_by="admin",
|
||||
updated_by="admin",
|
||||
),
|
||||
["gpt-4o"],
|
||||
),
|
||||
(None, None),
|
||||
],
|
||||
)
|
||||
@pytest.mark.asyncio
|
||||
async def test_team_info_returns_parent_organization_models(organization, expected_models):
|
||||
"""/team/info must report the parent org's model ceiling.
|
||||
|
||||
A team admin who is not an org admin gets a 403 from /organization/info, so this
|
||||
is the only read that can tell the Admin UI whether the org allows all proxy
|
||||
models. Without it the team edit form hides the "All Proxy Models" option and a
|
||||
team admin cannot grant their team everything on the proxy.
|
||||
"""
|
||||
from fastapi import Request
|
||||
|
||||
from litellm.proxy.management_endpoints import team_endpoints
|
||||
|
||||
team_row = _TeamRowWithOrganization(
|
||||
team_id="team-1",
|
||||
organization_id="org-1" if organization is not None else None,
|
||||
litellm_organization_table=organization,
|
||||
)
|
||||
|
||||
mock_prisma = MagicMock()
|
||||
mock_prisma.db.litellm_teamtable.find_unique = AsyncMock(return_value=team_row)
|
||||
mock_prisma.get_data = AsyncMock(return_value=[])
|
||||
|
||||
with (
|
||||
patch("litellm.proxy.proxy_server.prisma_client", mock_prisma),
|
||||
patch.object(team_endpoints, "get_all_team_memberships", AsyncMock(return_value=[])),
|
||||
):
|
||||
response = await team_endpoints.team_info(
|
||||
http_request=MagicMock(spec=Request),
|
||||
team_id="team-1",
|
||||
user_api_key_dict=UserAPIKeyAuth(user_role=LitellmUserRoles.PROXY_ADMIN),
|
||||
)
|
||||
|
||||
include = mock_prisma.db.litellm_teamtable.find_unique.await_args.kwargs["include"]
|
||||
assert include["litellm_organization_table"] is True
|
||||
|
||||
team_info = response["team_info"]
|
||||
assert team_info.organization_models == expected_models
|
||||
# the org row itself carries budgets and spend; only its model list may ride along
|
||||
assert "litellm_organization_table" not in team_info.model_dump()
|
||||
|
|
|
|||
|
|
@ -328,7 +328,8 @@ describe("useTeam", () => {
|
|||
});
|
||||
|
||||
it("should return team data when query is successful", async () => {
|
||||
(teamInfoCall as any).mockResolvedValue(mockTeams[0]);
|
||||
// /team/info answers with an envelope; the hook is typed as the team itself.
|
||||
(teamInfoCall as any).mockResolvedValue({ team_id: "team-1", team_info: mockTeams[0], keys: [] });
|
||||
|
||||
const { result } = renderHook(() => useTeam("team-1"), { wrapper });
|
||||
|
||||
|
|
|
|||
|
|
@ -163,7 +163,8 @@ export const useTeam = (teamId?: string) => {
|
|||
throw new Error("Missing auth or teamId");
|
||||
}
|
||||
|
||||
return teamInfoCall(accessToken, teamId);
|
||||
const { team_info } = (await teamInfoCall(accessToken, teamId)) as { team_info: Team };
|
||||
return team_info;
|
||||
},
|
||||
|
||||
initialData: () => {
|
||||
|
|
|
|||
|
|
@ -242,13 +242,11 @@ describe("ProjectDetail", () => {
|
|||
it("should show team information when team data is available", () => {
|
||||
mockUseTeam.mockReturnValue({
|
||||
data: {
|
||||
team_info: {
|
||||
team_id: "team-1",
|
||||
team_alias: "Engineering",
|
||||
models: ["gpt-4"],
|
||||
spend: 50,
|
||||
members_with_roles: [],
|
||||
},
|
||||
team_id: "team-1",
|
||||
team_alias: "Engineering",
|
||||
models: ["gpt-4"],
|
||||
spend: 50,
|
||||
members_with_roles: [],
|
||||
},
|
||||
isLoading: false,
|
||||
});
|
||||
|
|
|
|||
|
|
@ -14,16 +14,6 @@ import { UiLoadingSpinner } from "@/components/ui/ui-loading-spinner";
|
|||
import { EditProjectModal } from "./ProjectModals/EditProjectModal";
|
||||
import { ProjectKeysSection } from "./ProjectKeysSection";
|
||||
|
||||
interface TeamInfoShape {
|
||||
team_id: string;
|
||||
team_alias?: string;
|
||||
models?: string[];
|
||||
max_budget?: number | null;
|
||||
budget_duration?: string | null;
|
||||
spend?: number;
|
||||
members_with_roles?: { user_id: string; role: string }[];
|
||||
}
|
||||
|
||||
interface ProjectDetailProps {
|
||||
projectId: string;
|
||||
onBack: () => void;
|
||||
|
|
@ -33,10 +23,7 @@ const utilisationTone = (percent: number) => (percent >= 90 ? "over" : percent >
|
|||
|
||||
export function ProjectDetail({ projectId, onBack }: ProjectDetailProps) {
|
||||
const { data: project, isLoading } = useProjectDetails(projectId);
|
||||
const { data: teamData } = useTeam(project?.team_id ?? undefined);
|
||||
// teamInfoCall returns { team_id, team_info: {...}, keys, team_memberships }
|
||||
const teamInfo: TeamInfoShape | undefined = ((teamData as unknown as { team_info?: TeamInfoShape })?.team_info ??
|
||||
teamData) as TeamInfoShape | undefined;
|
||||
const { data: teamInfo } = useTeam(project?.team_id ?? undefined);
|
||||
const [isEditModalVisible, setIsEditModalVisible] = useState(false);
|
||||
|
||||
const spend = project?.spend ?? 0;
|
||||
|
|
|
|||
|
|
@ -415,6 +415,87 @@ describe("ModelSelect", () => {
|
|||
}
|
||||
});
|
||||
|
||||
it("should take the org model ceiling from the team when /organization/info is not readable", async () => {
|
||||
const testCases = [
|
||||
{
|
||||
name: "org allows all proxy models",
|
||||
organizationModels: ["all-proxy-models"],
|
||||
shouldShowSentinel: true,
|
||||
offered: ["gpt-4", "claude-3"],
|
||||
notOffered: [] as string[],
|
||||
},
|
||||
{
|
||||
name: "org places no ceiling at all",
|
||||
organizationModels: [],
|
||||
shouldShowSentinel: true,
|
||||
offered: ["gpt-4", "claude-3"],
|
||||
notOffered: [] as string[],
|
||||
},
|
||||
{
|
||||
name: "org restricts the team to one model",
|
||||
organizationModels: ["gpt-4"],
|
||||
shouldShowSentinel: false,
|
||||
offered: ["gpt-4"],
|
||||
notOffered: ["claude-3"],
|
||||
},
|
||||
];
|
||||
|
||||
for (const testCase of testCases) {
|
||||
const user = userEvent.setup();
|
||||
// A team admin gets a 403 from /organization/info, so the org query never resolves.
|
||||
mockUseOrganization.mockReturnValue({ data: undefined, isLoading: false } as any);
|
||||
mockUseTeam.mockReturnValue({
|
||||
data: { team_id: "team-1", organization_models: testCase.organizationModels },
|
||||
isLoading: false,
|
||||
} as any);
|
||||
|
||||
const { unmount } = renderWithProviders(
|
||||
<ModelSelect
|
||||
onChange={mockOnChange}
|
||||
context="team"
|
||||
teamID="team-1"
|
||||
organizationID="org-1"
|
||||
options={{ includeSpecialOptions: true }}
|
||||
/>,
|
||||
);
|
||||
|
||||
await openModelList(user);
|
||||
if (testCase.shouldShowSentinel) {
|
||||
expectOffered("All Proxy Models");
|
||||
} else {
|
||||
expectNotOffered("All Proxy Models");
|
||||
}
|
||||
expectOffered("No Default Models");
|
||||
testCase.offered.forEach(expectOffered);
|
||||
testCase.notOffered.forEach(expectNotOffered);
|
||||
|
||||
unmount();
|
||||
}
|
||||
});
|
||||
|
||||
it("should keep hiding All Proxy Models when neither the team nor the org reports a ceiling", async () => {
|
||||
const user = userEvent.setup();
|
||||
mockUseOrganization.mockReturnValue({ data: undefined, isLoading: false } as any);
|
||||
mockUseTeam.mockReturnValue({
|
||||
data: { team_id: "team-1", organization_models: null },
|
||||
isLoading: false,
|
||||
} as any);
|
||||
|
||||
renderWithProviders(
|
||||
<ModelSelect
|
||||
onChange={mockOnChange}
|
||||
context="team"
|
||||
teamID="team-1"
|
||||
organizationID="org-1"
|
||||
options={{ includeSpecialOptions: true }}
|
||||
/>,
|
||||
);
|
||||
|
||||
await openModelList(user);
|
||||
expectNotOffered("All Proxy Models");
|
||||
expectOffered("No Default Models");
|
||||
});
|
||||
|
||||
it("should use custom dataTestId when provided", async () => {
|
||||
renderWithProviders(
|
||||
<ModelSelect
|
||||
|
|
|
|||
|
|
@ -19,7 +19,6 @@ import {
|
|||
} from "@/components/ui/combobox";
|
||||
import { Skeleton } from "@/components/ui/skeleton";
|
||||
import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/components/ui/tooltip";
|
||||
import { Organization, Team } from "../networking";
|
||||
import { splitWildcardModels } from "./modelUtils";
|
||||
|
||||
const MODEL_SELECT_ALL_PROXY_MODELS_SPECIAL_VALUE = {
|
||||
|
|
@ -69,12 +68,14 @@ type ModelOptionGroup = {
|
|||
|
||||
type FilterContextArgs = {
|
||||
allProxyModels: string[];
|
||||
selectedTeam?: Team;
|
||||
selectedOrganization?: Organization;
|
||||
organizationModels?: string[];
|
||||
userModels?: string[];
|
||||
options?: ModelSelectProps["options"];
|
||||
};
|
||||
|
||||
const isUncappedModelCeiling = (organizationModels: string[]) =>
|
||||
organizationModels.length === 0 || organizationModels.includes(MODEL_SELECT_ALL_PROXY_MODELS_SPECIAL_VALUE.value);
|
||||
|
||||
const contextFilters: Record<ModelSelectProps["context"], (args: FilterContextArgs) => string[]> = {
|
||||
user: ({ allProxyModels, userModels, options }) => {
|
||||
if (!userModels) return [];
|
||||
|
|
@ -82,18 +83,9 @@ const contextFilters: Record<ModelSelectProps["context"], (args: FilterContextAr
|
|||
return [];
|
||||
},
|
||||
|
||||
team: ({ allProxyModels, selectedOrganization, userModels }) => {
|
||||
if (selectedOrganization) {
|
||||
if (
|
||||
selectedOrganization.models.includes(MODEL_SELECT_ALL_PROXY_MODELS_SPECIAL_VALUE.value) ||
|
||||
selectedOrganization.models.length === 0
|
||||
) {
|
||||
return allProxyModels;
|
||||
}
|
||||
return allProxyModels.filter((model) => selectedOrganization.models.includes(model));
|
||||
}
|
||||
|
||||
return allProxyModels ?? [];
|
||||
team: ({ allProxyModels, organizationModels }) => {
|
||||
if (!organizationModels || isUncappedModelCeiling(organizationModels)) return allProxyModels;
|
||||
return allProxyModels.filter((model) => organizationModels.includes(model));
|
||||
},
|
||||
|
||||
organization: ({ allProxyModels }) => {
|
||||
|
|
@ -108,7 +100,7 @@ const contextFilters: Record<ModelSelectProps["context"], (args: FilterContextAr
|
|||
const filterModels = (
|
||||
allProxyModels: ProxyModel[],
|
||||
ctx: ModelSelectProps,
|
||||
extra: { selectedTeam?: Team; selectedOrganization?: Organization; userModels?: string[] },
|
||||
extra: { organizationModels?: string[]; userModels?: string[] },
|
||||
): string[] => {
|
||||
const deduplicatedProxyModels = Array.from(new Map(allProxyModels.map((m) => [m.id, m])).values()).map(
|
||||
(model) => model.id,
|
||||
|
|
@ -133,9 +125,9 @@ export const ModelSelect = (props: ModelSelectProps) => {
|
|||
const isSpecialOption = (value: string) => MODEL_SENTINEL_OPTIONS.some((sv) => sv.value === value);
|
||||
const hasSpecialOptionSelected = value.some(isSpecialOption);
|
||||
const isLoading = isLoadingAllProxyModels || isLoadingTeam || isLoadingOrganization || isCurrentUserLoading;
|
||||
const organizationHasAllProxyModels =
|
||||
organization?.models.includes(MODEL_SELECT_ALL_PROXY_MODELS_SPECIAL_VALUE.value) ||
|
||||
organization?.models.length === 0;
|
||||
// The org's ceiling rides on /team/info, which a team admin may read; /organization/info 403s for them.
|
||||
const organizationModels = team?.organization_models ?? organization?.models;
|
||||
const organizationHasAllProxyModels = organizationModels !== undefined && isUncappedModelCeiling(organizationModels);
|
||||
const shouldShowAllProxyModels =
|
||||
showAllProxyModelsOverride || (organizationHasAllProxyModels && includeSpecialOptions) || context === "global";
|
||||
|
||||
|
|
@ -159,8 +151,7 @@ export const ModelSelect = (props: ModelSelectProps) => {
|
|||
};
|
||||
|
||||
const filteredModels = filterModels(allProxyModels?.data ?? [], props, {
|
||||
selectedTeam: team,
|
||||
selectedOrganization: organization,
|
||||
organizationModels,
|
||||
userModels: currentUser?.models,
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -27,6 +27,8 @@ export interface Team {
|
|||
access_group_models?: string[];
|
||||
access_group_mcp_server_ids?: string[];
|
||||
access_group_agent_ids?: string[];
|
||||
// Parent org's model ceiling. undefined = no org / not loaded; [] or ["all-proxy-models"] = no ceiling.
|
||||
organization_models?: string[] | null;
|
||||
}
|
||||
|
||||
export interface KeyResponse {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue