Merge pull request #38872 from BerriAI/litellm_fix_viewer_add_model_tab

fix(ui): hide model write affordances from view-only admin sessions
This commit is contained in:
Mateo Wang 2026-09-01 14:52:54 -07:00 committed by GitHub
commit bad55da9bf
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 183 additions and 38 deletions

View file

@ -140,6 +140,7 @@ const renderPanel = (canModify = true) =>
accessToken="token"
userRole="Admin"
userID="u-admin"
isViewOnly={false}
teams={null}
createScope={canModify ? "unscoped-ok" : "forbidden"}
/>,

View file

@ -21,12 +21,20 @@ interface AutoRoutersPanelProps {
accessToken: string;
userRole: string;
userID: string | null;
isViewOnly: boolean;
teams: Team[] | null;
/** Owned by the page, which knows how this caller must scope what they create. */
createScope: ModelWriteScope;
}
export function AutoRoutersPanel({ accessToken, userRole, userID, teams, createScope }: AutoRoutersPanelProps) {
export function AutoRoutersPanel({
accessToken,
userRole,
userID,
isViewOnly,
teams,
createScope,
}: AutoRoutersPanelProps) {
const canCreate = createScope !== "forbidden";
const { data: deployments, isLoading } = useAutoRouters();
const invalidateAutoRouters = useInvalidateAutoRouters();
@ -39,8 +47,8 @@ export function AutoRoutersPanel({ accessToken, userRole, userID, teams, createS
const [isDeleting, setIsDeleting] = useState(false);
const routers = useMemo(
() => toAutoRouterRows(deployments ?? [], { userRole, userID }, teams),
[deployments, userRole, userID, teams],
() => toAutoRouterRows(deployments ?? [], { userRole, userID, isViewOnly }, teams),
[deployments, userRole, userID, isViewOnly, teams],
);
const handleCreated = () => {

View file

@ -5,8 +5,9 @@ import { toAutoRouterRow, toAutoRouterRows } from "./autoRouterRows";
// Existing cases assert resource classification, so they run as a proxy admin: the actor
// gate is then a pass-through and canEdit/canDelete still reflect the row itself.
const ADMIN = { userRole: "Admin", userID: "u-admin" };
const TEAM_ADMIN = { userRole: "Internal User", userID: "u-team-admin" };
const ADMIN = { userRole: "Admin", userID: "u-admin", isViewOnly: false };
const TEAM_ADMIN = { userRole: "Internal User", userID: "u-team-admin", isViewOnly: false };
const VIEW_ONLY_ADMIN = { userRole: "Admin", userID: "u-viewer", isViewOnly: true };
const complexityDeployment = {
model_name: "tri-tier-router",
@ -224,7 +225,7 @@ describe("autoRouterRows actor gating", () => {
{ team_id: "team-1", members_with_roles: [{ user_id: "u-team-admin", user_email: "t@t", role: "admin" }] },
] as never;
const rowIn = (actor: { userRole: string; userID: string }, teamId: string | null) =>
const rowIn = (actor: { userRole: string; userID: string; isViewOnly: boolean }, teamId: string | null) =>
toAutoRouterRow(
{ ...complexityDeployment, model_info: { id: "cid-1", db_model: true, team_id: teamId } },
0,
@ -259,4 +260,12 @@ describe("autoRouterRows actor gating", () => {
expect(row.canEdit).toBe(true);
expect(row.canDelete).toBe(true);
});
// A proxy_admin_viewer session reads "Admin" through the masquerade, but PATCH and
// DELETE both 403 it, so its rows must not offer the affordances.
it("hides write affordances from a view-only admin session", () => {
const row = rowIn(VIEW_ONLY_ADMIN, null);
expect(row.canEdit).toBe(false);
expect(row.canDelete).toBe(false);
});
});

View file

@ -38,8 +38,17 @@ vi.mock("./useModelDashboardData", () => ({
useModelDashboardData: () => ({ availableModelAccessGroups: [], allModelsOnProxy: [], availableModelGroups: [] }),
}));
const ADMIN = { accessToken: "at", token: "t", userRole: "Admin", userId: "u1", premiumUser: false };
const NON_ADMIN = { accessToken: "at", token: "t", userRole: "Internal User", userId: "u1", premiumUser: false };
const ADMIN = { accessToken: "at", token: "t", userRole: "Admin", userId: "u1", premiumUser: false, isViewOnly: false };
const NON_ADMIN = {
accessToken: "at",
token: "t",
userRole: "Internal User",
userId: "u1",
premiumUser: false,
isViewOnly: false,
};
// A proxy_admin_viewer session: effectiveSessionRole masquerades the role as "Admin".
const VIEW_ONLY_ADMIN = { ...ADMIN, isViewOnly: true };
const renderPage = () => {
const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false, gcTime: 0 } } });
@ -99,6 +108,22 @@ describe("ModelsAndEndpointsPage", () => {
expect(screen.queryByRole("tab", { name: "Health Status" })).not.toBeInTheDocument();
});
// POST /model/new 403s a proxy_admin_viewer, so the form's tab must not render for one.
it("hides the Add Model tab for a view-only admin session", () => {
mockUseAuthorized.mockReturnValue(VIEW_ONLY_ADMIN);
const { getByRole, queryByRole } = renderPage();
expect(queryByRole("tab", { name: "Add Model" })).not.toBeInTheDocument();
expect(getByRole("tab", { name: "All Models" })).toBeInTheDocument();
});
// Read parity: the Auto-Routers list stays reachable for a view-only admin; only the
// create affordance inside it is withheld, which AutoRoutersTabPanel decides.
it("keeps the Auto-Routers tab for a view-only admin session", () => {
mockUseAuthorized.mockReturnValue(VIEW_ONLY_ADMIN);
const { getByRole } = renderPage();
expect(getByRole("tab", { name: /Auto-Routers/ })).toBeInTheDocument();
});
// Auto-routers are excluded from the All Models table, so this tab is their home: the only
// place in the product to list, create, edit or delete one.
describe("Auto-Routers tab", () => {

View file

@ -80,7 +80,7 @@ const renderPanel = (key: string) => {
};
export default function ModelsAndEndpointsPage() {
const { accessToken, userRole, userId: userID, premiumUser } = useAuthorized();
const { accessToken, userRole, userId: userID, premiumUser, isViewOnly } = useAuthorized();
const { data: teams } = useTeams();
const { data: uiSettings } = useUISettings();
const queryClient = useQueryClient();
@ -92,7 +92,7 @@ export default function ModelsAndEndpointsPage() {
const isInternalUser = userRole && internalUserRoles.includes(userRole);
const canCreate = canCreateModels(
{ userRole, userID },
{ userRole, userID, isViewOnly },
{
teams: teams ?? null,
disabledForInternalUsers:
@ -182,6 +182,7 @@ export default function ModelsAndEndpointsPage() {
accessToken={accessToken}
userID={userID}
userRole={userRole}
isViewOnly={isViewOnly}
onModelUpdate={invalidateModels}
modelAccessGroups={availableModelAccessGroups}
/>

View file

@ -0,0 +1,39 @@
/* @vitest-environment jsdom */
import { render } from "@testing-library/react";
import { describe, expect, it, vi } from "vitest";
import AutoRoutersTabPanel from "./AutoRoutersTabPanel";
const panelProps = vi.fn();
vi.mock("../components/AutoRouters/AutoRoutersPanel", () => ({
AutoRoutersPanel: (props: Record<string, unknown>) => {
panelProps(props);
return <div data-testid="auto-routers-panel" />;
},
}));
const mockUseAuthorized = vi.fn();
vi.mock("@/app/(dashboard)/hooks/useAuthorized", () => ({ default: () => mockUseAuthorized() }));
vi.mock("@/app/(dashboard)/hooks/teams/useTeams", () => ({ useTeams: () => ({ data: [] }) }));
vi.mock("@/app/(dashboard)/hooks/uiSettings/useUISettings", () => ({
useUISettings: () => ({ data: { values: {} } }),
}));
const SESSION = { accessToken: "at", userRole: "Admin", userId: "u1", isViewOnly: false };
const lastProps = () => panelProps.mock.calls.at(-1)?.[0] as { createScope: string };
describe("AutoRoutersTabPanel", () => {
it("grants an unscoped create to a real proxy admin", () => {
mockUseAuthorized.mockReturnValue(SESSION);
render(<AutoRoutersTabPanel />);
expect(lastProps().createScope).toBe("unscoped-ok");
});
// The masqueraded "Admin" a proxy_admin_viewer session carries: POST /model/new 403s it,
// so the panel must not be told it may create.
it("withholds the create affordance from a view-only admin session", () => {
mockUseAuthorized.mockReturnValue({ ...SESSION, isViewOnly: true });
render(<AutoRoutersTabPanel />);
expect(lastProps().createScope).toBe("forbidden");
});
});

View file

@ -15,13 +15,13 @@ import { AutoRoutersPanel } from "../components/AutoRouters/AutoRoutersPanel";
* Viewer roles reach the list without write affordances.
*/
export default function AutoRoutersTabPanel() {
const { accessToken, userRole, userId: userID } = useAuthorized();
const { accessToken, userRole, userId: userID, isViewOnly } = useAuthorized();
const { data: teams } = useTeams();
const { data: uiSettings } = useUISettings();
const isInternalUser = userRole != null && internalUserRoles.includes(userRole);
const scope = modelCreationScope(
{ userRole, userID },
{ userRole, userID, isViewOnly },
{
teams: teams ?? null,
disabledForInternalUsers: isInternalUser && uiSettings?.values?.disable_model_add_for_internal_users === true,
@ -33,6 +33,7 @@ export default function AutoRoutersTabPanel() {
accessToken={accessToken}
userRole={userRole ?? ""}
userID={userID ?? null}
isViewOnly={isViewOnly}
teams={teams ?? null}
createScope={scope}
/>

View file

@ -82,7 +82,7 @@ const AddModelForm: React.FC<AddModelFormProps> = ({
// Using a unique ID to force the ConnectionErrorDisplay to remount and run a fresh test
const [connectionTestId, setConnectionTestId] = useState<string>("");
const { accessToken, userRole, premiumUser, userId } = useAuthorized();
const { accessToken, userRole, premiumUser, userId, isViewOnly } = useAuthorized();
const {
data: providerMetadata,
isLoading: isProviderMetadataLoading,
@ -157,7 +157,10 @@ const AddModelForm: React.FC<AddModelFormProps> = ({
const isTeamAdmin = isUserTeamAdminForAnyTeam(teams, userId);
// Same owner the Auto-Routers tab uses, so the two creation forms cannot disagree about
// who has to name a team. This form is only reachable when creation is allowed at all.
const createScope = modelCreationScope({ userRole, userID: userId }, { teams, disabledForInternalUsers: false });
const createScope = modelCreationScope(
{ userRole, userID: userId, isViewOnly },
{ teams, disabledForInternalUsers: false },
);
const requiresTeamScope = createScope === "team-required";
return (

View file

@ -87,6 +87,7 @@ describe("ModelInfoView", () => {
accessToken: "test-token",
userID: "123",
userRole: "Admin",
isViewOnly: false,
onModelUpdate: vi.fn(),
modelAccessGroups: ["group1", "group2"],
};
@ -328,6 +329,16 @@ describe("ModelInfoView", () => {
});
});
// A proxy_admin_viewer session reads "Admin" through effectiveSessionRole, but the update
// and delete endpoints 403 it, so the write buttons must not be offered.
it("should disable delete and update buttons for a view-only admin session", async () => {
render(<ModelInfoView {...DEFAULT_ADMIN_PROPS} isViewOnly={true} />, { wrapper });
await waitFor(() => {
expect(screen.getByTestId("delete-model-button")).toBeDisabled();
});
expect(screen.getByTestId("update-api-key-button")).toBeDisabled();
});
it("should disable delete button when model is not a DB model", async () => {
const nonDbModelData = {
...defaultModelData,

View file

@ -53,6 +53,7 @@ interface ModelInfoViewProps {
accessToken: string | null;
userID: string | null;
userRole: string | null;
isViewOnly: boolean;
onModelUpdate?: (updatedModel: any) => void;
modelAccessGroups: string[] | null;
}
@ -117,6 +118,7 @@ export default function ModelInfoView({
accessToken,
userID,
userRole,
isViewOnly,
onModelUpdate,
modelAccessGroups,
}: ModelInfoViewProps) {
@ -167,7 +169,7 @@ export default function ModelInfoView({
// Keep modelData variable name for backwards compatibility
const modelData = transformedModelData;
const canEditModel = canModifyModel({ userRole, userID }, teams ?? null, {
const canEditModel = canModifyModel({ userRole, userID, isViewOnly }, teams ?? null, {
teamId: modelData?.model_info?.team_id,
isDbModel: modelData?.model_info?.db_model === true,
});

View file

@ -1,14 +1,16 @@
import { describe, expect, it } from "vitest";
import { Team } from "@/components/networking";
import { canModifyModel, modelCreationScope } from "./modelPermissions";
import { canCreateModels, canModifyModel, modelCreationScope } from "./modelPermissions";
const teamWhere = (userId: string, role: string, teamId = "team-1"): Team[] =>
[{ team_id: teamId, members_with_roles: [{ user_id: userId, user_email: "t@test.com", role }] }] as unknown as Team[];
const PROXY_ADMIN = { userRole: "Admin", userID: "u-admin" };
const TEAM_ADMIN = { userRole: "Internal User", userID: "u-team-admin" };
const MEMBER = { userRole: "Internal User", userID: "u-member" };
const PROXY_ADMIN = { userRole: "Admin", userID: "u-admin", isViewOnly: false };
const TEAM_ADMIN = { userRole: "Internal User", userID: "u-team-admin", isViewOnly: false };
const MEMBER = { userRole: "Internal User", userID: "u-member", isViewOnly: false };
// proxy_admin_viewer sessions: effectiveSessionRole masquerades the role as "Admin".
const VIEW_ONLY_ADMIN = { userRole: "Admin", userID: "u-viewer", isViewOnly: true };
const noLimits = { disabledForInternalUsers: false };
@ -40,9 +42,24 @@ describe("modelCreationScope", () => {
// an unscoped create from them 403s. Treating them as admins here is what let a form submit
// a payload the backend always rejected.
it("does not treat an org admin as able to create unscoped", () => {
const orgAdmin = { userRole: "org_admin", userID: "u-org" };
const orgAdmin = { userRole: "org_admin", userID: "u-org", isViewOnly: false };
expect(modelCreationScope(orgAdmin, { teams: teamWhere("u-org", "admin"), ...noLimits })).toBe("team-required");
});
// Server-side, POST /model/new 403s the viewer roles, so the "Admin" the masquerade
// reports must not read as a proxy admin here.
it("forbids a view-only admin session despite the masqueraded Admin role", () => {
expect(modelCreationScope(VIEW_ONLY_ADMIN, { teams: [], ...noLimits })).toBe("forbidden");
expect(canCreateModels(VIEW_ONLY_ADMIN, { teams: [], ...noLimits })).toBe(false);
});
// _check_proxy_admin_viewer_access (route_checks.py) 403s /model/new on the session role
// alone, before the team-scoped carve-out in ModelManagementAuthChecks can run.
it("forbids a view-only admin even when they admin a team", () => {
expect(modelCreationScope(VIEW_ONLY_ADMIN, { teams: teamWhere("u-viewer", "admin"), ...noLimits })).toBe(
"forbidden",
);
});
});
describe("canModifyModel", () => {
@ -80,6 +97,17 @@ describe("canModifyModel", () => {
});
it("does not treat two absent identities as a match", () => {
expect(canModifyModel({ userRole: "Internal User", userID: null }, null, teamRow)).toBe(false);
expect(canModifyModel({ userRole: "Internal User", userID: null, isViewOnly: false }, null, teamRow)).toBe(false);
});
// PATCH /model/{id}/update and POST /model/delete 403 the viewer roles like /model/new does.
it("refuses a view-only admin session on a DB row", () => {
expect(canModifyModel(VIEW_ONLY_ADMIN, null, teamRow)).toBe(false);
});
// The route RBAC blocks /model/update and /model/delete for the viewer role before the
// team-scoped carve-out runs, so team-admin membership changes nothing here either.
it("refuses a view-only user even when they admin the owning team", () => {
expect(canModifyModel(VIEW_ONLY_ADMIN, teamWhere("u-viewer", "admin"), teamRow)).toBe(false);
});
});

View file

@ -3,20 +3,32 @@ import { Team } from "@/components/networking";
import { isProxyAdminRole, isUserTeamAdminForAnyTeam, isUserTeamAdminForSingleTeam } from "./roles";
/**
* The dashboard's mirror of ModelManagementAuthChecks in
* litellm/proxy/management_endpoints/model_management_endpoints.py.
* The dashboard's mirror of the two server layers that gate model writes: the role-level
* route RBAC (`_check_proxy_admin_viewer_access` in litellm/proxy/auth/route_checks.py),
* which 403s /model/new, /model/update, and /model/delete for every view-only session
* before the endpoint runs, and ModelManagementAuthChecks in
* litellm/proxy/management_endpoints/model_management_endpoints.py behind it.
*
* Both questions below are answered there by exactly two inputs: the caller's role, and
* whether the caller admins the team named in `model_info.team_id`. `created_by` is written
* at creation and never read by an auth check, so it is deliberately absent here; gating on
* it hid controls from team admins the API accepts, and showed controls to former team admins
* the API rejects.
* Past that route gate, both questions below are answered by exactly two inputs: the
* caller's role, and whether the caller admins the team named in `model_info.team_id`.
* `created_by` is written at creation and never read by an auth check, so it is deliberately
* absent here; gating on it hid controls from team admins the API accepts, and showed
* controls to former team admins the API rejects.
*/
export interface ModelActor {
userRole: string | null;
userID: string | null;
/**
* From useAuthorized(). A proxy_admin_viewer session masquerades as "Admin" in userRole
* (effectiveSessionRole, for read parity), yet every management write 403s it, so the role
* alone cannot answer a write question.
*/
isViewOnly: boolean;
}
const isWritableProxyAdmin = ({ userRole, isViewOnly }: ModelActor): boolean =>
!isViewOnly && userRole != null && isProxyAdminRole(userRole);
/** How this actor must scope a deployment they create, or that they may not create one. */
export type ModelWriteScope = "forbidden" | "unscoped-ok" | "team-required";
@ -33,20 +45,25 @@ const isTeamAdminOf = (teams: Team[] | null, userID: string, teamId: string): bo
/**
* POST /model/new takes a proxy admin unconditionally, or a team admin whose payload names a
* team; an unscoped create from anyone else is a 403. Returning the requirement rather than a
* pair of booleans keeps "may not create" and "may create unscoped" from being confused.
* team; an unscoped create from anyone else is a 403. A view-only session is 403d by the
* route RBAC on its role alone, so team-admin membership cannot rescue it. Returning the
* requirement rather than a pair of booleans keeps "may not create" and "may create
* unscoped" from being confused.
*/
export const modelCreationScope = (
{ userRole, userID }: ModelActor,
actor: ModelActor,
{ teams, disabledForInternalUsers }: ModelCreationLimits,
): ModelWriteScope => {
if (userRole != null && isProxyAdminRole(userRole)) {
if (actor.isViewOnly) {
return "forbidden";
}
if (isWritableProxyAdmin(actor)) {
return "unscoped-ok";
}
if (disabledForInternalUsers) {
return "forbidden";
}
if (userID != null && isUserTeamAdminForAnyTeam(teams, userID)) {
if (actor.userID != null && isUserTeamAdminForAnyTeam(teams, actor.userID)) {
return "team-required";
}
return "forbidden";
@ -63,18 +80,18 @@ export interface ModelRowOrigin {
/** May this actor edit or delete this specific deployment? */
export const canModifyModel = (
{ userRole, userID }: ModelActor,
actor: ModelActor,
teams: Team[] | null,
{ teamId, isDbModel }: ModelRowOrigin,
): boolean => {
if (!isDbModel) {
if (actor.isViewOnly || !isDbModel) {
return false;
}
if (userRole != null && isProxyAdminRole(userRole)) {
if (isWritableProxyAdmin(actor)) {
return true;
}
if (userID == null || teamId == null) {
if (actor.userID == null || teamId == null) {
return false;
}
return isTeamAdminOf(teams, userID, teamId);
return isTeamAdminOf(teams, actor.userID, teamId);
};