From be4f46683af81a02d4164cf16830e90c982ca5fa Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Fri, 1 May 2026 20:20:39 -0700 Subject: [PATCH] fix(scim): cascade FK cleanup on user delete and surface block status in UI SCIM DELETE /Users/{id} previously called litellm_usertable.delete without clearing rows that FK back to the user, so Postgres rejected the delete with LiteLLM_InvitationLink_user_id_fkey and the SCIM caller saw a 500. Add a helper to drop invitation_link, organization_membership, and team_membership rows before the user delete (mirrors /user/delete in internal_user_endpoints). Also add a Status column to the Virtual Keys and Internal Users tables so admins can see at a glance which keys are blocked and which users SCIM has deactivated. SCIM-blocked keys carry a tooltip explaining the origin. Pin the dashboard's Node version to 20 via .nvmrc to match CI. --- .../management_endpoints/scim/scim_v2.py | 26 ++++++ .../scim/test_scim_key_deactivation.py | 65 +++++++++++++ ui/litellm-dashboard/.nvmrc | 1 + .../VirtualKeysPage/VirtualKeysTable.test.tsx | 92 ++++++++++++++++++- .../VirtualKeysPage/VirtualKeysTable.tsx | 31 ++++++- .../src/components/view_users/columns.tsx | 26 +++++- .../src/components/view_users/table.test.tsx | 91 ++++++++++++++++++ .../src/components/view_users/types.ts | 1 + 8 files changed, 330 insertions(+), 3 deletions(-) create mode 100644 ui/litellm-dashboard/.nvmrc diff --git a/litellm/proxy/management_endpoints/scim/scim_v2.py b/litellm/proxy/management_endpoints/scim/scim_v2.py index 173acd41b35..5692ed5bc5c 100644 --- a/litellm/proxy/management_endpoints/scim/scim_v2.py +++ b/litellm/proxy/management_endpoints/scim/scim_v2.py @@ -420,6 +420,30 @@ async def _set_user_keys_blocked(user_id: str, blocked: bool) -> int: return len(affected_keys) +async def _delete_rows_referencing_user(prisma_client: Any, *, user_id: str) -> None: + """Drop rows whose foreign keys reference ``LiteLLM_UserTable.user_id``. + + Required before deleting the user row itself, otherwise Postgres rejects + the user delete with an FK constraint violation (e.g. + ``LiteLLM_InvitationLink_user_id_fkey``). + """ + await prisma_client.db.litellm_invitationlink.delete_many( + where={ + "OR": [ + {"user_id": user_id}, + {"created_by": user_id}, + {"updated_by": user_id}, + ] + } + ) + await prisma_client.db.litellm_organizationmembership.delete_many( + where={"user_id": user_id} + ) + await prisma_client.db.litellm_teammembership.delete_many( + where={"user_id": user_id} + ) + + def _scim_active_value(metadata: Optional[Dict[str, Any]]) -> Optional[bool]: """Read the SCIM active flag from a user's metadata dict, if present.""" if not metadata: @@ -1118,6 +1142,8 @@ async def delete_user( # missing owner. await _set_user_keys_blocked(user_id=user_id, blocked=True) + await _delete_rows_referencing_user(prisma_client, user_id=user_id) + # Delete user await prisma_client.db.litellm_usertable.delete(where={"user_id": user_id}) diff --git a/tests/test_litellm/proxy/management_endpoints/scim/test_scim_key_deactivation.py b/tests/test_litellm/proxy/management_endpoints/scim/test_scim_key_deactivation.py index e14209ce068..ad02c0e2a83 100644 --- a/tests/test_litellm/proxy/management_endpoints/scim/test_scim_key_deactivation.py +++ b/tests/test_litellm/proxy/management_endpoints/scim/test_scim_key_deactivation.py @@ -47,6 +47,9 @@ def _build_prisma_with_keys(user_keys, mock_user=None, updated_user=None): mock_db.litellm_verificationtoken.find_many = AsyncMock(return_value=user_keys) mock_db.litellm_verificationtoken.update_many = AsyncMock(return_value=None) mock_db.litellm_verificationtoken.update = AsyncMock(return_value=None) + mock_db.litellm_invitationlink.delete_many = AsyncMock(return_value=None) + mock_db.litellm_organizationmembership.delete_many = AsyncMock(return_value=None) + mock_db.litellm_teammembership.delete_many = AsyncMock(return_value=None) return mock_client, mock_db @@ -185,6 +188,68 @@ async def test_scim_delete_user_blocks_keys_before_deleting_user(): ) +@pytest.mark.asyncio +async def test_scim_delete_user_clears_fk_referenced_rows_before_user_delete(): + user_id = "user-with-invite" + mock_user = LiteLLM_UserTable( + user_id=user_id, + user_email="x@example.com", + user_alias=None, + teams=[], + metadata={}, + ) + mock_client, mock_db = _build_prisma_with_keys(user_keys=[], mock_user=mock_user) + + call_order: list = [] + mock_db.litellm_invitationlink.delete_many = AsyncMock( + side_effect=lambda **kw: call_order.append(("invitation", kw)) or None + ) + mock_db.litellm_organizationmembership.delete_many = AsyncMock( + side_effect=lambda **kw: call_order.append(("orgmembership", kw)) or None + ) + mock_db.litellm_teammembership.delete_many = AsyncMock( + side_effect=lambda **kw: call_order.append(("teammembership", kw)) or None + ) + mock_db.litellm_usertable.delete = AsyncMock( + side_effect=lambda **kw: call_order.append(("user", kw)) or None + ) + + with ( + patch("litellm.proxy.proxy_server.prisma_client", mock_client), + patch("litellm.proxy.proxy_server.user_api_key_cache", MagicMock()), + patch("litellm.proxy.proxy_server.proxy_logging_obj", MagicMock()), + patch( + "litellm.proxy.management_endpoints.scim.scim_v2._delete_cache_key_object", + AsyncMock(), + ), + ): + response = await delete_user(user_id=user_id) + + assert response.status_code == 204 + + mock_db.litellm_invitationlink.delete_many.assert_awaited_once() + inv_kwargs = mock_db.litellm_invitationlink.delete_many.await_args.kwargs + assert inv_kwargs == { + "where": { + "OR": [ + {"user_id": user_id}, + {"created_by": user_id}, + {"updated_by": user_id}, + ] + } + } + mock_db.litellm_organizationmembership.delete_many.assert_awaited_once_with( + where={"user_id": user_id} + ) + mock_db.litellm_teammembership.delete_many.assert_awaited_once_with( + where={"user_id": user_id} + ) + + stages = [stage for stage, _ in call_order] + assert stages.index("user") > stages.index("invitation") + assert stages.index("user") > stages.index("orgmembership") + + @pytest.mark.asyncio async def test_scim_patch_user_active_false_blocks_keys(): user_id = "scim-user" diff --git a/ui/litellm-dashboard/.nvmrc b/ui/litellm-dashboard/.nvmrc new file mode 100644 index 00000000000..209e3ef4b62 --- /dev/null +++ b/ui/litellm-dashboard/.nvmrc @@ -0,0 +1 @@ +20 diff --git a/ui/litellm-dashboard/src/components/VirtualKeysPage/VirtualKeysTable.test.tsx b/ui/litellm-dashboard/src/components/VirtualKeysPage/VirtualKeysTable.test.tsx index e6d144df65a..98813f84625 100644 --- a/ui/litellm-dashboard/src/components/VirtualKeysPage/VirtualKeysTable.test.tsx +++ b/ui/litellm-dashboard/src/components/VirtualKeysPage/VirtualKeysTable.test.tsx @@ -1,4 +1,4 @@ -import { screen, waitFor, fireEvent } from "@testing-library/react"; +import { act, screen, waitFor, fireEvent } from "@testing-library/react"; import { vi, it, expect, beforeEach, MockedFunction } from "vitest"; import { renderWithProviders } from "../../../tests/test-utils"; import { VirtualKeysTable } from "./VirtualKeysTable"; @@ -974,3 +974,93 @@ describe("refetch button", () => { expect(screen.getByText("Fetch")).toBeInTheDocument(); }); }); + +describe("Status column reflects key.blocked / scim_blocked metadata", () => { + it("should render Active for a non-blocked key", async () => { + mockUseFilterLogic.mockReturnValue({ + filters: { + "Team ID": "", + "Organization ID": "", + "Key Alias": "", + "User ID": "", + "Sort By": "created_at", + "Sort Order": "desc", + }, + filteredKeys: [{ ...mockKey, blocked: false, metadata: {} }], + filteredTotalCount: null, + allTeams: [mockTeam], + allOrganizations: [mockOrganization], + handleFilterChange: vi.fn(), + handleFilterReset: vi.fn(), + }); + + renderWithProviders(); + + await waitFor(() => { + expect(screen.getByTestId(`key-status-${mockKey.token_id}`)).toHaveTextContent( + "Active", + ); + }); + }); + + it("should render Blocked when key.blocked is true", async () => { + mockUseFilterLogic.mockReturnValue({ + filters: { + "Team ID": "", + "Organization ID": "", + "Key Alias": "", + "User ID": "", + "Sort By": "created_at", + "Sort Order": "desc", + }, + filteredKeys: [{ ...mockKey, blocked: true, metadata: {} }], + filteredTotalCount: null, + allTeams: [mockTeam], + allOrganizations: [mockOrganization], + handleFilterChange: vi.fn(), + handleFilterReset: vi.fn(), + }); + + renderWithProviders(); + + await waitFor(() => { + expect(screen.getByTestId(`key-status-${mockKey.token_id}`)).toHaveTextContent( + "Blocked", + ); + }); + expect(screen.queryByText(/Blocked by SCIM/i)).not.toBeInTheDocument(); + }); + + it("should mark a SCIM-blocked key with the SCIM tooltip reason", async () => { + mockUseFilterLogic.mockReturnValue({ + filters: { + "Team ID": "", + "Organization ID": "", + "Key Alias": "", + "User ID": "", + "Sort By": "created_at", + "Sort Order": "desc", + }, + filteredKeys: [ + { ...mockKey, blocked: true, metadata: { scim_blocked: true } }, + ], + filteredTotalCount: null, + allTeams: [mockTeam], + allOrganizations: [mockOrganization], + handleFilterChange: vi.fn(), + handleFilterReset: vi.fn(), + }); + + renderWithProviders(); + + const tag = await screen.findByTestId(`key-status-${mockKey.token_id}`); + expect(tag).toHaveTextContent("Blocked"); + + act(() => { + fireEvent.mouseEnter(tag); + }); + await waitFor(() => { + expect(screen.getByText(/Blocked by SCIM/i)).toBeInTheDocument(); + }); + }); +}); diff --git a/ui/litellm-dashboard/src/components/VirtualKeysPage/VirtualKeysTable.tsx b/ui/litellm-dashboard/src/components/VirtualKeysPage/VirtualKeysTable.tsx index b30d4b6ce5b..b48f0285f6e 100644 --- a/ui/litellm-dashboard/src/components/VirtualKeysPage/VirtualKeysTable.tsx +++ b/ui/litellm-dashboard/src/components/VirtualKeysPage/VirtualKeysTable.tsx @@ -26,7 +26,7 @@ import { Text, } from "@tremor/react"; import { InfoCircleOutlined, SyncOutlined } from "@ant-design/icons"; -import { Button as AntButton, Popover, Skeleton, Tooltip, Typography } from "antd"; +import { Button as AntButton, Popover, Skeleton, Tag, Tooltip, Typography } from "antd"; import React, { useEffect, useDeferredValue, useMemo, useState } from "react"; import { getModelDisplayName } from "../key_team_helpers/fetch_available_models_team_key"; import { useFilterLogic } from "../key_team_helpers/filter_logic"; @@ -183,6 +183,35 @@ export function VirtualKeysTable({ teams, organizations, onSortChange, currentSo ); }, }, + { + id: "status", + header: "Status", + size: 100, + enableSorting: false, + cell: ({ row }) => { + const key = row.original; + if (key.blocked !== true) { + return ( + + Active + + ); + } + const isScimBlocked = + (key.metadata as Record | null | undefined) + ?.scim_blocked === true; + const reason = isScimBlocked + ? "Blocked by SCIM (external identity provider deactivated or deleted the owning user)." + : "Blocked. Requests using this key will be rejected with 401."; + return ( + + + Blocked + + + ); + }, + }, { id: "key_name", accessorKey: "key_name", diff --git a/ui/litellm-dashboard/src/components/view_users/columns.tsx b/ui/litellm-dashboard/src/components/view_users/columns.tsx index 4b9fad8f23c..fa3661067ff 100644 --- a/ui/litellm-dashboard/src/components/view_users/columns.tsx +++ b/ui/litellm-dashboard/src/components/view_users/columns.tsx @@ -1,6 +1,6 @@ import { ColumnDef } from "@tanstack/react-table"; import { Badge, Grid, Icon } from "@tremor/react"; -import { Tooltip, Checkbox } from "antd"; +import { Tooltip, Checkbox, Tag } from "antd"; import { UserInfo } from "./types"; import { PencilAltIcon, TrashIcon, InformationCircleIcon, RefreshIcon } from "@heroicons/react/outline"; import { CopyOutlined } from "@ant-design/icons"; @@ -54,6 +54,30 @@ export const columns = ( enableSorting: true, cell: ({ row }) => {row.original.user_email || "-"}, }, + { + id: "status", + header: "Status", + enableSorting: false, + cell: ({ row }) => { + const isScimInactive = + (row.original.metadata as Record | null | undefined) + ?.scim_active === false; + if (isScimInactive) { + return ( + + + Inactive + + + ); + } + return ( + + Active + + ); + }, + }, { header: "Global Proxy Role", accessorKey: "user_role", diff --git a/ui/litellm-dashboard/src/components/view_users/table.test.tsx b/ui/litellm-dashboard/src/components/view_users/table.test.tsx index 82f49d9618e..783b99329bd 100644 --- a/ui/litellm-dashboard/src/components/view_users/table.test.tsx +++ b/ui/litellm-dashboard/src/components/view_users/table.test.tsx @@ -1,6 +1,8 @@ import { act, fireEvent, render, screen } from "@testing-library/react"; import { describe, expect, it, vi } from "vitest"; +import { columns } from "./columns"; import { UserDataTable } from "./table"; +import { UserInfo } from "./types"; const defaultFilters = { email: "", @@ -100,6 +102,7 @@ describe("UserDataTable", () => { [ "User ID", "Email", + "Status", "Global Proxy Role", "User Alias", "Spend (USD)", @@ -113,4 +116,92 @@ describe("UserDataTable", () => { expect(screen.getByRole("columnheader", { name: header })).toBeInTheDocument(); }); }); + + it("should render the user-row Status cell as Active when scim_active is not set to false", () => { + const possibleUIRoles = { admin: { ui_label: "Admin" } }; + const handlers = { edit: vi.fn(), del: vi.fn(), reset: vi.fn(), click: vi.fn() }; + const cols = columns( + possibleUIRoles, + handlers.edit, + handlers.del, + handlers.reset, + handlers.click, + ); + const statusCol = cols.find((c) => (c as { id?: string }).id === "status"); + expect(statusCol).toBeDefined(); + + const baseUser: UserInfo = { + user_id: "u-active", + user_email: "active@example.com", + user_alias: null, + user_role: "admin", + spend: 0, + max_budget: null, + models: [], + key_count: 0, + created_at: "", + updated_at: "", + sso_user_id: null, + budget_duration: null, + }; + + const cellNoMetadata = (statusCol as any).cell({ row: { original: baseUser } }); + render(<>{cellNoMetadata}); + expect(screen.getByText("Active")).toBeInTheDocument(); + expect(screen.queryByText("Inactive")).not.toBeInTheDocument(); + }); + + it("should render the user-row Status cell as Inactive when scim_active is false", () => { + const possibleUIRoles = { admin: { ui_label: "Admin" } }; + const cols = columns(possibleUIRoles, vi.fn(), vi.fn(), vi.fn(), vi.fn()); + const statusCol = cols.find((c) => (c as { id?: string }).id === "status")!; + + const inactiveUser: UserInfo = { + user_id: "u-inactive", + user_email: "alex@acme.io", + user_alias: null, + user_role: "internal_user", + spend: 0, + max_budget: null, + models: [], + key_count: 1, + created_at: "", + updated_at: "", + sso_user_id: null, + budget_duration: null, + metadata: { scim_active: false }, + }; + + const cell = (statusCol as any).cell({ row: { original: inactiveUser } }); + render(<>{cell}); + expect(screen.getByText("Inactive")).toBeInTheDocument(); + expect(screen.queryByText("Active")).not.toBeInTheDocument(); + }); + + it("should treat scim_active=true as Active (not Inactive)", () => { + const possibleUIRoles = { admin: { ui_label: "Admin" } }; + const cols = columns(possibleUIRoles, vi.fn(), vi.fn(), vi.fn(), vi.fn()); + const statusCol = cols.find((c) => (c as { id?: string }).id === "status")!; + + const reactivated: UserInfo = { + user_id: "u-rehired", + user_email: "alex@acme.io", + user_alias: null, + user_role: "internal_user", + spend: 0, + max_budget: null, + models: [], + key_count: 1, + created_at: "", + updated_at: "", + sso_user_id: null, + budget_duration: null, + metadata: { scim_active: true }, + }; + + const cell = (statusCol as any).cell({ row: { original: reactivated } }); + render(<>{cell}); + expect(screen.getByText("Active")).toBeInTheDocument(); + expect(screen.queryByText("Inactive")).not.toBeInTheDocument(); + }); }); diff --git a/ui/litellm-dashboard/src/components/view_users/types.ts b/ui/litellm-dashboard/src/components/view_users/types.ts index 744aa00a88b..7e1bae82845 100644 --- a/ui/litellm-dashboard/src/components/view_users/types.ts +++ b/ui/litellm-dashboard/src/components/view_users/types.ts @@ -11,4 +11,5 @@ export interface UserInfo { updated_at: string; sso_user_id: string | null; budget_duration: string | null; + metadata?: Record | null; }