Merge pull request #38867 from BerriAI/litellm_hide_admin_tabs_view_only

fix(ui): hide admin write-form tabs on the models page from view-only admins
This commit is contained in:
Mateo Wang 2026-09-12 21:13:34 -07:00 committed by GitHub
commit 386d29ee67
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 90 additions and 18 deletions

View file

@ -48,7 +48,7 @@ const AllModelsTab = ({
setSelectedTeamId,
}: AllModelsTabProps) => {
const { data: modelCostMapData, isLoading: isLoadingModelCostMap } = useModelCostMap();
const { accessToken, userId, userRole } = useAuthorized();
const { accessToken, userId, userRole, isViewOnly } = useAuthorized();
const { data: teams, isLoading: isLoadingTeams } = useTeams();
const queryClient = useQueryClient();
@ -281,6 +281,7 @@ const AllModelsTab = ({
availableModelAccessGroups={availableModelAccessGroups}
userRole={userRole}
userID={userId}
isViewOnly={isViewOnly}
onModelIdClick={setSelectedModelId}
onTeamIdClick={setSelectedTeamId}
onDeleteClick={handleDeleteClick}

View file

@ -59,6 +59,7 @@ const baseProps = {
availableModelAccessGroups: ["sales-team"],
userRole: "Admin",
userID: "alice",
isViewOnly: false,
onModelIdClick: vi.fn(),
onTeamIdClick: vi.fn(),
onDeleteClick: vi.fn(),
@ -254,6 +255,17 @@ describe("AllModelsTable", () => {
expect(onTogglePauseClick).not.toHaveBeenCalled();
});
it("does not let a view-only admin toggle a model", async () => {
const user = userEvent.setup();
const onTogglePauseClick = vi.fn();
render(<AllModelsTable {...baseProps} isViewOnly onTogglePauseClick={onTogglePauseClick} />);
const toggle = screen.getByTestId("model-pause-toggle-model-1");
expect(toggle).toHaveAttribute("data-disabled");
await user.click(toggle);
expect(onTogglePauseClick).not.toHaveBeenCalled();
});
it("does not let anyone toggle a config model", async () => {
const user = userEvent.setup();
const onTogglePauseClick = vi.fn();
@ -309,6 +321,17 @@ describe("AllModelsTable", () => {
expect(onDeleteClick).not.toHaveBeenCalled();
});
it("blocks a view-only admin from deleting a DB model they created", async () => {
const user = userEvent.setup();
const onDeleteClick = vi.fn();
render(<AllModelsTable {...baseProps} isViewOnly onDeleteClick={onDeleteClick} />);
const deleteButton = screen.getByTestId("model-delete-model-1");
expect(deleteButton).toBeDisabled();
await user.click(deleteButton);
expect(onDeleteClick).not.toHaveBeenCalled();
});
it("blocks deleting a config model", async () => {
const user = userEvent.setup();
const onDeleteClick = vi.fn();

View file

@ -73,6 +73,7 @@ interface AllModelsTableProps {
availableModelAccessGroups: string[];
userRole: string;
userID: string;
isViewOnly: boolean;
onModelIdClick: (modelId: string) => void;
onTeamIdClick: (teamId: string) => void;
onDeleteClick: (modelId: string) => void;
@ -120,6 +121,7 @@ export function AllModelsTable({
availableModelAccessGroups,
userRole,
userID,
isViewOnly,
onModelIdClick,
onTeamIdClick,
onDeleteClick,
@ -132,6 +134,7 @@ export function AllModelsTable({
const columnDeps = {
userRole,
userID,
isViewOnly,
onModelIdClick,
onTeamIdClick,
onDeleteClick,
@ -139,7 +142,7 @@ export function AllModelsTable({
pausingModelId,
};
return getModelsTableColumns(columnDeps);
}, [userRole, userID, onModelIdClick, onTeamIdClick, onDeleteClick, onTogglePauseClick, pausingModelId]);
}, [userRole, userID, isViewOnly, onModelIdClick, onTeamIdClick, onDeleteClick, onTogglePauseClick, pausingModelId]);
const modelGroupOptions = useMemo(
() => [

View file

@ -247,6 +247,7 @@ interface ModelRowActionsProps {
model: ModelData;
userRole: string;
userID: string;
isViewOnly: boolean;
isPausing: boolean;
onDeleteClick?: (modelId: string) => void;
onTogglePauseClick?: (modelId: string, blocked: boolean) => void | Promise<void>;
@ -256,14 +257,15 @@ function ModelRowActions({
model,
userRole,
userID,
isViewOnly,
isPausing,
onDeleteClick,
onTogglePauseClick,
}: ModelRowActionsProps) {
const modelId = model.model_info?.id;
const isConfigModel = !model.model_info?.db_model;
const isAdmin = userRole === "Admin";
const canEditModel = isAdmin || model.model_info?.created_by === userID;
const isAdmin = userRole === "Admin" && !isViewOnly;
const canEditModel = !isViewOnly && (isAdmin || model.model_info?.created_by === userID);
const isBlocked = model.model_info?.blocked === true;
const isPauseToggleable = !isConfigModel && isAdmin && Boolean(onTogglePauseClick);
@ -340,6 +342,7 @@ function ModelRowActions({
export interface ModelsTableColumnDeps {
userRole: string;
userID: string;
isViewOnly: boolean;
onModelIdClick: (modelId: string) => void;
onTeamIdClick: (teamId: string) => void;
onDeleteClick?: (modelId: string) => void;
@ -350,6 +353,7 @@ export interface ModelsTableColumnDeps {
export const getModelsTableColumns = ({
userRole,
userID,
isViewOnly,
onModelIdClick,
onTeamIdClick,
onDeleteClick,
@ -479,6 +483,7 @@ export const getModelsTableColumns = ({
model={row.original}
userRole={userRole}
userID={userID}
isViewOnly={isViewOnly}
isPausing={pausingModelId === row.original.model_info?.id}
onDeleteClick={onDeleteClick}
onTogglePauseClick={onTogglePauseClick}

View file

@ -14,6 +14,7 @@ vi.mock("./panels/HealthStatusPanel", () => ({ default: () => <div data-testid="
vi.mock("./panels/ModelRetrySettingsPanel", () => ({ default: () => <div data-testid="panel-retry" /> }));
vi.mock("./panels/ModelGroupAliasPanel", () => ({ default: () => <div data-testid="panel-alias" /> }));
vi.mock("./panels/PriceDataPanel", () => ({ default: () => <div data-testid="panel-price" /> }));
vi.mock("./panels/AccessGroupBudgetsPanel", () => ({ default: () => <div data-testid="panel-budgets" /> }));
const detailState = { modelId: null as string | null, teamId: null as string | null };
vi.mock("./detailNavigation", () => ({
@ -25,7 +26,11 @@ vi.mock("@/components/model_info_view", () => ({
default: ({ modelId }: { modelId: string }) => <div data-testid="model-info">model:{modelId}</div>,
}));
vi.mock("@/components/team/TeamInfo", () => ({
default: ({ teamId }: { teamId: string }) => <div data-testid="team-info">team:{teamId}</div>,
default: ({ teamId, is_team_admin }: { teamId: string; is_team_admin: boolean }) => (
<div data-testid="team-info" data-team-admin={String(is_team_admin)}>
team:{teamId}
</div>
),
}));
const mockUseAuthorized = vi.fn();
@ -95,10 +100,19 @@ describe("ModelsAndEndpointsPage", () => {
expect(screen.queryByRole("tab", { name: "All Models" })).not.toBeInTheDocument();
});
it("renders the team detail overlay from the ?team drill-in", () => {
it("renders the team detail overlay from the ?team drill-in with admin edit rights", () => {
detailState.teamId = "team-9";
renderPage();
expect(screen.getByTestId("team-info")).toHaveTextContent("team:team-9");
expect(screen.getByTestId("team-info")).toHaveAttribute("data-team-admin", "true");
});
it("opens the ?team drill-in without edit rights for a view-only admin", () => {
mockUseAuthorized.mockReturnValue(VIEW_ONLY_ADMIN);
detailState.teamId = "team-9";
renderPage();
expect(screen.getByTestId("team-info")).toHaveTextContent("team:team-9");
expect(screen.getByTestId("team-info")).toHaveAttribute("data-team-admin", "false");
});
it("hides admin-only tabs for a non-admin user", () => {
@ -108,6 +122,35 @@ describe("ModelsAndEndpointsPage", () => {
expect(screen.queryByRole("tab", { name: "Health Status" })).not.toBeInTheDocument();
});
it("keeps the full admin tab order for a real admin", () => {
renderPage();
expect(screen.getAllByRole("tab").map((tab) => tab.textContent)).toEqual([
"All Models",
"Add Model",
"Auto-Routers Beta",
"LLM Credentials",
"Pass-Through Endpoints",
"Health Status",
"Model Retry Settings",
"Model Group Alias",
"Model Access Group Budgets Beta",
"Price Data Reload",
]);
});
it("hides the admin write-form tabs from a view-only admin, keeping the read views", () => {
mockUseAuthorized.mockReturnValue(VIEW_ONLY_ADMIN);
renderPage();
expect(screen.getByRole("tab", { name: "All Models" })).toBeInTheDocument();
expect(screen.getByRole("tab", { name: "Health Status" })).toBeInTheDocument();
expect(screen.queryByRole("tab", { name: "LLM Credentials" })).not.toBeInTheDocument();
expect(screen.queryByRole("tab", { name: "Pass-Through Endpoints" })).not.toBeInTheDocument();
expect(screen.queryByRole("tab", { name: "Model Retry Settings" })).not.toBeInTheDocument();
expect(screen.queryByRole("tab", { name: "Model Group Alias" })).not.toBeInTheDocument();
expect(screen.queryByRole("tab", { name: /Model Access Group Budgets/ })).not.toBeInTheDocument();
expect(screen.queryByRole("tab", { name: "Price Data Reload" })).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);

View file

@ -106,19 +106,16 @@ export default function ModelsAndEndpointsPage() {
"",
...(canCreate ? (["add"] as const) : []),
...(isAdmin || canCreate ? (["auto-routers"] as const) : []),
...(isAdmin
? ([
"llm-credentials",
"pass-through",
"health",
"retry-settings",
"model-group-alias",
"access-group-budgets",
"price-data",
] as const)
// effectiveSessionRole reports proxy_admin_viewer as "Admin", so isAdmin alone would show a
// viewer these write-only panels; only the raw-role isViewOnly separates them. Health Status
// stays: it is the bucket's one read view, and viewers keep read parity with admins.
...(isAdmin && !isViewOnly ? (["llm-credentials", "pass-through"] as const) : []),
...(isAdmin ? (["health"] as const) : []),
...(isAdmin && !isViewOnly
? (["retry-settings", "model-group-alias", "access-group-budgets", "price-data"] as const)
: []),
],
[canCreate, isAdmin],
[canCreate, isAdmin, isViewOnly],
);
const allModelsLabel = isAdmin ? "All Models" : "Your Models";
@ -148,7 +145,7 @@ export default function ModelsAndEndpointsPage() {
teamId={teamId}
onClose={close}
accessToken={accessToken}
is_team_admin={userRole === "Admin"}
is_team_admin={userRole === "Admin" && !isViewOnly}
is_proxy_admin={userRole === "Proxy Admin"}
userModels={allModelsOnProxy}
editTeam={false}