mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-08 03:08:45 +00:00
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.
This commit is contained in:
parent
8c409006ad
commit
be4f46683a
8 changed files with 330 additions and 3 deletions
|
|
@ -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})
|
||||
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
1
ui/litellm-dashboard/.nvmrc
Normal file
1
ui/litellm-dashboard/.nvmrc
Normal file
|
|
@ -0,0 +1 @@
|
|||
20
|
||||
|
|
@ -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(<VirtualKeysTable {...defaultMockProps} />);
|
||||
|
||||
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(<VirtualKeysTable {...defaultMockProps} />);
|
||||
|
||||
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(<VirtualKeysTable {...defaultMockProps} />);
|
||||
|
||||
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();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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 (
|
||||
<Tag color="green" data-testid={`key-status-${key.token_id}`}>
|
||||
Active
|
||||
</Tag>
|
||||
);
|
||||
}
|
||||
const isScimBlocked =
|
||||
(key.metadata as Record<string, unknown> | 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 (
|
||||
<Tooltip title={reason}>
|
||||
<Tag color="red" data-testid={`key-status-${key.token_id}`}>
|
||||
Blocked
|
||||
</Tag>
|
||||
</Tooltip>
|
||||
);
|
||||
},
|
||||
},
|
||||
{
|
||||
id: "key_name",
|
||||
accessorKey: "key_name",
|
||||
|
|
|
|||
|
|
@ -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 }) => <span className="text-xs">{row.original.user_email || "-"}</span>,
|
||||
},
|
||||
{
|
||||
id: "status",
|
||||
header: "Status",
|
||||
enableSorting: false,
|
||||
cell: ({ row }) => {
|
||||
const isScimInactive =
|
||||
(row.original.metadata as Record<string, unknown> | null | undefined)
|
||||
?.scim_active === false;
|
||||
if (isScimInactive) {
|
||||
return (
|
||||
<Tooltip title="Deactivated via SCIM (external identity provider). The user's virtual keys are blocked.">
|
||||
<Tag color="red" data-testid={`user-status-${row.original.user_id}`}>
|
||||
Inactive
|
||||
</Tag>
|
||||
</Tooltip>
|
||||
);
|
||||
}
|
||||
return (
|
||||
<Tag color="green" data-testid={`user-status-${row.original.user_id}`}>
|
||||
Active
|
||||
</Tag>
|
||||
);
|
||||
},
|
||||
},
|
||||
{
|
||||
header: "Global Proxy Role",
|
||||
accessorKey: "user_role",
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -11,4 +11,5 @@ export interface UserInfo {
|
|||
updated_at: string;
|
||||
sso_user_id: string | null;
|
||||
budget_duration: string | null;
|
||||
metadata?: Record<string, unknown> | null;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue