From d4d0bf0acc078f081e7c0f7628bae3696a48ae10 Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Mon, 3 Aug 2026 18:09:49 -0700 Subject: [PATCH] fix(ui): hide guardrail review buttons from non-admin users (#27535) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(ui): hide guardrail review buttons from non-admin users The team guardrail submissions list rendered Approve/Reject buttons for non-admin users even though the backend correctly rejected the calls. Thread userRole from the page through GuardrailsPanel into TeamGuardrailsTab and gate the row-card and detail-panel review buttons on isAdmin so the UI matches the backend authorization. Defense in depth only — the backend remains the source of truth and is double-gated at both the route admin check and the explicit endpoint role check. Refs LIT-2494 * refactor(ui): read userRole from useAuthorized hook instead of prop drilling Drop the userRole prop chain through GuardrailsPage → GuardrailsPanel → TeamGuardrailsTab. Each component reads userRole directly from the useAuthorized hook, matching the pattern used elsewhere in the dashboard. Tests now mock useAuthorized per case (the same pattern as top_key_view.test.tsx) instead of passing userRole as a prop. Refs LIT-2494 * fix(ui): drop userRole prop on GuardrailsPanel call site in src/app/page.tsx Missed in the earlier refactor — GuardrailsPanel no longer accepts userRole as a prop (reads from useAuthorized hook), so callers must not pass it. The build was failing in production type-check. Refs LIT-2494 * fix(ui): gate guardrail forward-key toggle and header editors on proxy admin * refactor(ui): remove dead app_admin case from user role formatting --- .../_components/TeamGuardrailsTab.test.tsx | 142 +++++++++++ .../_components/TeamGuardrailsTab.tsx | 221 ++++++++++-------- .../(dashboard)/hooks/useAuthorized.test.ts | 6 +- .../src/components/user_dashboard.tsx | 2 - ui/litellm-dashboard/src/utils/roles.ts | 2 - 5 files changed, 269 insertions(+), 104 deletions(-) create mode 100644 ui/litellm-dashboard/src/app/(dashboard)/guardrails/_components/TeamGuardrailsTab.test.tsx diff --git a/ui/litellm-dashboard/src/app/(dashboard)/guardrails/_components/TeamGuardrailsTab.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/guardrails/_components/TeamGuardrailsTab.test.tsx new file mode 100644 index 00000000000..603cddb7b89 --- /dev/null +++ b/ui/litellm-dashboard/src/app/(dashboard)/guardrails/_components/TeamGuardrailsTab.test.tsx @@ -0,0 +1,142 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { renderWithProviders } from "@/../tests/test-utils"; +import { screen, fireEvent } from "@testing-library/react"; +import { TeamGuardrailsTab } from "./TeamGuardrailsTab"; +import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized"; + +vi.mock("@/components/networking", () => ({ + listGuardrailSubmissions: vi.fn(), + approveGuardrailSubmission: vi.fn(), + rejectGuardrailSubmission: vi.fn(), + updateGuardrailCall: vi.fn(), +})); + +vi.mock("@/app/(dashboard)/hooks/guardrails/useRegisterGuardrail", () => ({ + useRegisterGuardrail: () => ({ + mutateAsync: vi.fn(), + isPending: false, + }), +})); + +vi.mock("@/components/common_components/team_dropdown", () => ({ + default: () => null, +})); + +vi.mock("@/app/(dashboard)/hooks/useAuthorized", () => ({ + default: vi.fn(), +})); + +import { listGuardrailSubmissions } from "@/components/networking"; + +const pendingSubmission = { + guardrail_id: "guard-1", + guardrail_name: "test-pending-guardrail", + status: "pending_review", + team_id: "team-1", + team_guardrail: true, + litellm_params: { + guardrail: "generic_guardrail_api", + mode: "pre_call", + api_base: "https://example.com/guard", + headers: { "X-API-Key": "secret" }, + extra_headers: ["x-request-id"], + }, + guardrail_info: {}, + submitted_at: "2026-05-09T00:00:00Z", +}; + +const baseAuth = { + token: "test-token", + accessToken: "test-token", + userId: "user-1", + userEmail: "user@example.com", + premiumUser: false, + disabledPersonalKeyCreation: null, + showSSOBanner: false, +}; + +describe("TeamGuardrailsTab — approve/reject role gate", () => { + const mockUseAuthorized = vi.mocked(useAuthorized); + + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(listGuardrailSubmissions).mockResolvedValue({ + submissions: [pendingSubmission], + summary: { total: 1, pending_review: 1, active: 0, rejected: 0 }, + }); + }); + + it("hides Approve and Reject buttons for an internal user on a pending submission", async () => { + mockUseAuthorized.mockReturnValue({ ...baseAuth, userRole: "Internal User" }); + renderWithProviders(); + + await screen.findByText("test-pending-guardrail"); + + expect(screen.queryByRole("button", { name: /approve/i })).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: /reject/i })).not.toBeInTheDocument(); + }); + + it("hides Approve and Reject buttons for an Admin Viewer, whom the backend rejects with 403", async () => { + mockUseAuthorized.mockReturnValue({ ...baseAuth, userRole: "Admin Viewer" }); + renderWithProviders(); + + await screen.findByText("test-pending-guardrail"); + + expect(screen.queryByRole("button", { name: /approve/i })).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: /reject/i })).not.toBeInTheDocument(); + }); + + it("shows Approve and Reject buttons for an admin on a pending submission", async () => { + mockUseAuthorized.mockReturnValue({ ...baseAuth, userRole: "Admin" }); + renderWithProviders(); + + await screen.findByText("test-pending-guardrail"); + + expect(screen.getByRole("button", { name: /approve/i })).toBeInTheDocument(); + expect(screen.getByRole("button", { name: /reject/i })).toBeInTheDocument(); + }); + + it("hides Approve and Reject buttons when userRole is undefined (defaults to non-admin)", async () => { + mockUseAuthorized.mockReturnValue({ ...baseAuth, userRole: undefined }); + renderWithProviders(); + + await screen.findByText("test-pending-guardrail"); + + expect(screen.queryByRole("button", { name: /approve/i })).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: /reject/i })).not.toBeInTheDocument(); + }); + + it("disables all admin-only write controls for a non-admin, including the detail panel", async () => { + mockUseAuthorized.mockReturnValue({ ...baseAuth, userRole: "Internal User" }); + renderWithProviders(); + + await screen.findByText("test-pending-guardrail"); + expect(screen.getByRole("switch")).toBeDisabled(); + + fireEvent.click(screen.getByRole("button", { name: "Review" })); + await screen.findByText("Forward LiteLLM API Key"); + + expect(screen.queryByRole("button", { name: /approve/i })).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: /reject/i })).not.toBeInTheDocument(); + screen.getAllByRole("switch").forEach((toggle) => expect(toggle).toBeDisabled()); + expect(screen.queryByRole("button", { name: "Add" })).not.toBeInTheDocument(); + expect(screen.queryByLabelText(/^Remove/)).not.toBeInTheDocument(); + expect(screen.queryByPlaceholderText("e.g. x-request-id")).not.toBeInTheDocument(); + }); + + it("keeps all write controls enabled for an admin in the detail panel", async () => { + mockUseAuthorized.mockReturnValue({ ...baseAuth, userRole: "Admin" }); + renderWithProviders(); + + await screen.findByText("test-pending-guardrail"); + + fireEvent.click(screen.getByRole("button", { name: "Review" })); + await screen.findByText("Forward LiteLLM API Key"); + + expect(screen.getAllByRole("button", { name: /approve/i }).length).toBeGreaterThanOrEqual(2); + screen.getAllByRole("switch").forEach((toggle) => expect(toggle).toBeEnabled()); + expect(screen.getAllByRole("button", { name: "Add" })).toHaveLength(2); + expect(screen.getByLabelText("Remove X-API-Key")).toBeInTheDocument(); + expect(screen.getByLabelText("Remove x-request-id")).toBeInTheDocument(); + }); +}); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/guardrails/_components/TeamGuardrailsTab.tsx b/ui/litellm-dashboard/src/app/(dashboard)/guardrails/_components/TeamGuardrailsTab.tsx index 4217a765732..496e1129371 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/guardrails/_components/TeamGuardrailsTab.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/guardrails/_components/TeamGuardrailsTab.tsx @@ -27,6 +27,8 @@ import { import NotificationsManager from "@/components/molecules/notifications_manager"; import TeamDropdown from "@/components/common_components/team_dropdown"; import { useRegisterGuardrail } from "@/app/(dashboard)/hooks/guardrails/useRegisterGuardrail"; +import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized"; +import { isProxyAdminRole } from "@/utils/roles"; type GuardrailStatus = "active" | "pending" | "rejected"; @@ -188,16 +190,25 @@ function StatCard({ label, value, color }: { label: string; value: number; color ); } -function Toggle({ enabled, onToggle }: { enabled: boolean; onToggle: () => void }) { +function Toggle({ + enabled, + onToggle, + disabled = false, +}: { + enabled: boolean; + onToggle: () => void; + disabled?: boolean; +}) { return ( - {g.status === "pending" && ( + {isAdmin && g.status === "pending" && ( <> + {isAdmin && ( + + )} ))} )} -
- setNewStaticHeaderKey(e.target.value)} - placeholder="Header name (e.g. X-API-Key)" - className="flex-1 min-w-0 text-xs font-mono border border-gray-200 rounded-sm px-2 py-1.5 text-gray-700 placeholder-gray-400 focus:outline-hidden focus:ring-1 focus:ring-blue-500" - onKeyDown={(e) => { - if (e.key === "Enter") { - e.preventDefault(); + {isAdmin && ( +
+ setNewStaticHeaderKey(e.target.value)} + placeholder="Header name (e.g. X-API-Key)" + className="flex-1 min-w-0 text-xs font-mono border border-gray-200 rounded-sm px-2 py-1.5 text-gray-700 placeholder-gray-400 focus:outline-hidden focus:ring-1 focus:ring-blue-500" + onKeyDown={(e) => { + if (e.key === "Enter") { + e.preventDefault(); + const key = newStaticHeaderKey.trim(); + const value = newStaticHeaderValue.trim(); + if (key && !g.customHeaders.some((h) => h.key.toLowerCase() === key.toLowerCase())) { + onUpdateCustomHeaders([...g.customHeaders, { key, value }]); + setNewStaticHeaderKey(""); + setNewStaticHeaderValue(""); + } + } + }} + /> + setNewStaticHeaderValue(e.target.value)} + placeholder="Value" + className="flex-1 min-w-0 text-xs font-mono border border-gray-200 rounded-sm px-2 py-1.5 text-gray-700 placeholder-gray-400 focus:outline-hidden focus:ring-1 focus:ring-blue-500" + onKeyDown={(e) => { + if (e.key === "Enter") { + e.preventDefault(); + const key = newStaticHeaderKey.trim(); + const value = newStaticHeaderValue.trim(); + if (key && !g.customHeaders.some((h) => h.key.toLowerCase() === key.toLowerCase())) { + onUpdateCustomHeaders([...g.customHeaders, { key, value }]); + setNewStaticHeaderKey(""); + setNewStaticHeaderValue(""); + } + } + }} + /> + -
+ }} + className="text-xs font-medium text-blue-600 hover:text-blue-700 border border-blue-200 bg-blue-50 hover:bg-blue-100 px-2 py-1.5 rounded-sm transition-colors shrink-0" + > + Add + +
+ )}
@@ -546,50 +565,54 @@ function DetailPanel({ className="flex items-center justify-between gap-2 text-xs font-mono bg-gray-50 border border-gray-200 rounded-sm px-2 py-1.5" > {name} - + {isAdmin && ( + + )} ))} )} -
- setNewExtraHeader(e.target.value)} - placeholder="e.g. x-request-id" - className="flex-1 min-w-0 text-xs font-mono border border-gray-200 rounded-sm px-2 py-1.5 text-gray-700 placeholder-gray-400 focus:outline-hidden focus:ring-1 focus:ring-blue-500" - onKeyDown={(e) => { - if (e.key === "Enter") { - e.preventDefault(); + {isAdmin && ( +
+ setNewExtraHeader(e.target.value)} + placeholder="e.g. x-request-id" + className="flex-1 min-w-0 text-xs font-mono border border-gray-200 rounded-sm px-2 py-1.5 text-gray-700 placeholder-gray-400 focus:outline-hidden focus:ring-1 focus:ring-blue-500" + onKeyDown={(e) => { + if (e.key === "Enter") { + e.preventDefault(); + const name = newExtraHeader.trim().toLowerCase(); + if (name && !g.extraHeaders.map((h) => h.toLowerCase()).includes(name)) { + onUpdateExtraHeaders([...g.extraHeaders, name]); + setNewExtraHeader(""); + } + } + }} + /> + -
+ }} + className="text-xs font-medium text-blue-600 hover:text-blue-700 border border-blue-200 bg-blue-50 hover:bg-blue-100 px-2 py-1.5 rounded-sm transition-colors" + > + Add + +
+ )}
- {g.status === "pending" && ( + {isAdmin && g.status === "pending" && (