diff --git a/tests/e2e/coverage_registry/mcp.yaml b/tests/e2e/coverage_registry/mcp.yaml index d477b257cb0..ab644118a47 100644 --- a/tests/e2e/coverage_registry/mcp.yaml +++ b/tests/e2e/coverage_registry/mcp.yaml @@ -7,6 +7,14 @@ assertions: [succeeds] source: "server.py:637" rationale: Core operation; most common auth path; high usage +- id: mcp.list_tools.api_key.access_group_scoped + module: mcp + tier: P1 + operation: list_tools + auth_family: api_key + assertions: [access_group_scoped] + source: "test_mcp_access_group_e2e.py" + rationale: "A key granted an MCP access group sees the tagged server's tools; a key with a different group does not. Access-group-scoped tool selection at key creation" - id: mcp.list_tools.api_key.denied_without_permission module: mcp tier: P0 diff --git a/tests/e2e/mcp/datadog_mcp.py b/tests/e2e/mcp/datadog_mcp.py index f1b9461f23b..d1ea53a0b3b 100644 --- a/tests/e2e/mcp/datadog_mcp.py +++ b/tests/e2e/mcp/datadog_mcp.py @@ -30,7 +30,12 @@ def assert_dd_mcp_creds() -> None: ) -def register_datadog_mcp(client: McpClient, resources: ResourceManager) -> str: +def register_datadog_mcp( + client: McpClient, + resources: ResourceManager, + *, + mcp_access_groups: list[str] | None = None, +) -> str: assert_dd_mcp_creds() name = f"e2e_dd_mcp_{unique_marker()}" server_id = client.register_server( @@ -43,6 +48,7 @@ def register_datadog_mcp(client: McpClient, resources: ResourceManager) -> str: "DD-APPLICATION-KEY": _dd_app_key(), }, allowed_tools=[SEARCH_LOGS_TOOL], + mcp_access_groups=mcp_access_groups, ) resources.defer(lambda: client.delete_server(server_id)) return server_id diff --git a/tests/e2e/mcp/mcp_client.py b/tests/e2e/mcp/mcp_client.py index b0aa4c68e3a..f758a41cae6 100644 --- a/tests/e2e/mcp/mcp_client.py +++ b/tests/e2e/mcp/mcp_client.py @@ -36,6 +36,7 @@ class McpServerNewBody(BaseModel): auth_type: str | None = None static_headers: dict[str, str] | None = None allowed_tools: list[str] | None = None + mcp_access_groups: list[str] | None = None class McpServerNewResponse(BaseModel): @@ -155,6 +156,7 @@ class McpClient: auth_type: str | None = None, static_headers: dict[str, str] | None = None, allowed_tools: list[str] | None = None, + mcp_access_groups: list[str] | None = None, ) -> str: return unwrap( self.proxy.transport.post( @@ -168,6 +170,7 @@ class McpClient: auth_type=auth_type, static_headers=static_headers, allowed_tools=allowed_tools, + mcp_access_groups=mcp_access_groups, ), response_type=McpServerNewResponse, ) @@ -196,10 +199,13 @@ class McpClient: *, user_id: str, mcp_servers: list[str] | None, + mcp_access_groups: list[str] | None = None, models: list[str] | None = None, ) -> str: object_permission = ( - ObjectPermission(mcp_servers=mcp_servers) if mcp_servers is not None else None + ObjectPermission(mcp_servers=mcp_servers, mcp_access_groups=mcp_access_groups) + if mcp_servers is not None or mcp_access_groups is not None + else None ) return self.proxy.generate_key( KeyGenerateBody( diff --git a/tests/e2e/mcp/test_mcp_access_group_e2e.py b/tests/e2e/mcp/test_mcp_access_group_e2e.py new file mode 100644 index 00000000000..d7ff9736896 --- /dev/null +++ b/tests/e2e/mcp/test_mcp_access_group_e2e.py @@ -0,0 +1,58 @@ +"""Live e2e: MCP tool selection via access group at key creation. + +An admin registers the Datadog remote MCP server tagged with a server-side +access group (`mcp_access_groups`). A key minted with that access group +(`object_permission.mcp_access_groups`) sees the server's tools; a key minted +with a different group does not. This exercises access-group-scoped tool +selection, the enterprise MCP surface where keys are granted tool access groups +rather than explicit server ids. + +A tools/list that leaks the server across the access-group boundary fails hard. +Requires DD_API_KEY + DD_APP_KEY (the suite's real MCP upstream). +""" + +import pytest + +from datadog_mcp import SEARCH_LOGS_TOOL, register_datadog_mcp +from e2e_config import unique_marker +from e2e_http import unwrap +from lifecycle import ResourceManager +from mcp_client import McpClient + +pytestmark = pytest.mark.e2e + + +class TestMcpAccessGroupToolSelection: + @pytest.mark.covers("mcp.list_tools.api_key.access_group_scoped") + def test_access_group_scopes_tool_selection( + self, client: McpClient, resources: ResourceManager + ) -> None: + group = f"e2e-mcp-grp-{unique_marker()}" + server_id = register_datadog_mcp(client, resources, mcp_access_groups=[group]) + + granted = client.generate_key( + user_id=f"e2e-mcp-ag-granted-{unique_marker()}", + mcp_servers=None, + mcp_access_groups=[group], + ) + resources.defer(lambda: client.proxy.delete_key(granted)) + + other = client.generate_key( + user_id=f"e2e-mcp-ag-other-{unique_marker()}", + mcp_servers=None, + mcp_access_groups=[f"e2e-mcp-grp-absent-{unique_marker()}"], + ) + resources.defer(lambda: client.proxy.delete_key(other)) + + granted_tools = unwrap(client.list_tools(granted)) + assert granted_tools.tool_name_containing(server_id, SEARCH_LOGS_TOOL) is not None, ( + f"key granted access group {group} did not see the tagged server's tool " + f"(upstream dead or access-group grant not applied): " + f"{granted_tools.tool_names_for_server(server_id)}" + ) + + other_tools = unwrap(client.list_tools(other)).tool_names_for_server(server_id) + assert other_tools == frozenset(), ( + f"key with a different access group saw the server's tools; access-group tool " + f"selection leaked across the boundary: {other_tools}" + ) diff --git a/tests/e2e/models.py b/tests/e2e/models.py index d21920cf848..af695acaa5e 100644 --- a/tests/e2e/models.py +++ b/tests/e2e/models.py @@ -47,6 +47,7 @@ class KeyMetadata(BaseModel): class ObjectPermission(BaseModel): mcp_servers: list[str] | None = None + mcp_access_groups: list[str] | None = None class KeyGenerateBody(BaseModel): diff --git a/ui/litellm-dashboard/eslint-suppressions.json b/ui/litellm-dashboard/eslint-suppressions.json index ce743063309..289012659a1 100644 --- a/ui/litellm-dashboard/eslint-suppressions.json +++ b/ui/litellm-dashboard/eslint-suppressions.json @@ -1088,11 +1088,6 @@ "count": 1 } }, - "src/app/(dashboard)/organizations/_components/OrganizationsPanel.tsx": { - "no-restricted-imports": { - "count": 1 - } - }, "src/app/(dashboard)/playground/components/chat_ui/A2AMetrics.tsx": { "no-restricted-imports": { "count": 1 diff --git a/ui/litellm-dashboard/src/app/(dashboard)/organizations/_components/OrganizationsPanel.tsx b/ui/litellm-dashboard/src/app/(dashboard)/organizations/_components/OrganizationsPanel.tsx index 9f7e029a1d4..b1c026d3904 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/organizations/_components/OrganizationsPanel.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/organizations/_components/OrganizationsPanel.tsx @@ -1,19 +1,14 @@ import { organizationKeys, useOrganizations } from "@/app/(dashboard)/hooks/organizations/useOrganizations"; import { useUserModels } from "@/app/(dashboard)/hooks/models/useModels"; import OrganizationFilters, { FilterState } from "@/app/(dashboard)/organizations/OrganizationFilters"; -import { InfoCircleOutlined } from "@ant-design/icons"; -import { Form, Input, Modal, Select as Select2, Tooltip } from "antd"; import { useQueryClient } from "@tanstack/react-query"; import React, { useState } from "react"; import DeleteResourceModal from "@/components/common_components/DeleteResourceModal"; -import MCPServerSelector from "@/components/mcp_server_management/MCPServerSelector"; -import { ModelSelect } from "@/components/ModelSelect/ModelSelect"; import NotificationsManager from "@/components/molecules/notifications_manager"; -import { organizationCreateCall, organizationDeleteCall } from "@/components/networking"; +import { organizationDeleteCall } from "@/components/networking"; +import { OrgCreateDialog } from "@/components/organization/org-create/OrgCreateDialog"; import OrganizationInfoView from "@/components/organization/organization_view"; -import NumericalInput from "@/components/shared/numerical_input"; import { Button } from "@/components/ui/button"; -import VectorStoreSelector from "@/components/vector_store_management/VectorStoreSelector"; import OrganizationsTable from "./OrganizationsTable"; @@ -30,7 +25,6 @@ const OrganizationsPanel: React.FC = ({ userRole, acces const [orgToDelete, setOrgToDelete] = useState(null); const [isDeleting, setIsDeleting] = useState(false); const [isOrgModalVisible, setIsOrgModalVisible] = useState(false); - const [form] = Form.useForm(); const [showFilters, setShowFilters] = useState(false); const [filters, setFilters] = useState({ org_id: "", org_alias: "" }); @@ -83,48 +77,6 @@ const OrganizationsPanel: React.FC = ({ userRole, acces setOrgToDelete(null); }; - const handleCreate = async (values: any) => { - try { - if (!accessToken) return; - - // Transform allowed_vector_store_ids and allowed_mcp_servers_and_groups into object_permission - if ( - (values.allowed_vector_store_ids && values.allowed_vector_store_ids.length > 0) || - (values.allowed_mcp_servers_and_groups && - (values.allowed_mcp_servers_and_groups.servers?.length > 0 || - values.allowed_mcp_servers_and_groups.accessGroups?.length > 0)) - ) { - values.object_permission = {}; - if (values.allowed_vector_store_ids && values.allowed_vector_store_ids.length > 0) { - values.object_permission.vector_stores = values.allowed_vector_store_ids; - delete values.allowed_vector_store_ids; - } - if (values.allowed_mcp_servers_and_groups) { - if (values.allowed_mcp_servers_and_groups.servers?.length > 0) { - values.object_permission.mcp_servers = values.allowed_mcp_servers_and_groups.servers; - } - if (values.allowed_mcp_servers_and_groups.accessGroups?.length > 0) { - values.object_permission.mcp_access_groups = values.allowed_mcp_servers_and_groups.accessGroups; - } - delete values.allowed_mcp_servers_and_groups; - } - } - - await organizationCreateCall(accessToken, values); - NotificationsManager.success("Organization created successfully"); - setIsOrgModalVisible(false); - form.resetFields(); - await refetchOrganizations(); - } catch (error) { - console.error("Error creating organization:", error); - } - }; - - const handleCancel = () => { - setIsOrgModalVisible(false); - form.resetFields(); - }; - if (!premiumUser) { return (
@@ -190,97 +142,7 @@ const OrganizationsPanel: React.FC = ({ userRole, acces )} - -
- - - - - form.setFieldValue("models", values)} - context="organization" - /> - - - - - - - - daily - weekly - monthly - - - - - - - - - - - Allowed Vector Stores{" "} - - - - - } - name="allowed_vector_store_ids" - className="mt-4" - help="Select vector stores this organization can access. Leave empty for access to all vector stores" - > - form.setFieldValue("allowed_vector_store_ids", values)} - value={form.getFieldValue("allowed_vector_store_ids")} - accessToken={accessToken || ""} - placeholder="Select vector stores (optional)" - /> - - - - Allowed MCP Servers{" "} - - - - - } - name="allowed_mcp_servers_and_groups" - className="mt-4" - help="Select MCP servers and access groups this organization can access." - > - form.setFieldValue("allowed_mcp_servers_and_groups", values)} - value={form.getFieldValue("allowed_mcp_servers_and_groups")} - accessToken={accessToken || ""} - placeholder="Select MCP servers and access groups (optional)" - /> - - - - - - -
- -
-
- + ; -} +export type AgentObjectPermission = components["schemas"]["AgentObjectPermission"]; export interface Agent { agent_id: string; diff --git a/ui/litellm-dashboard/src/components/common_components/KeyLifecycleSettings.test.tsx b/ui/litellm-dashboard/src/components/common_components/KeyLifecycleSettings.test.tsx index 45121013652..896d8a14717 100644 --- a/ui/litellm-dashboard/src/components/common_components/KeyLifecycleSettings.test.tsx +++ b/ui/litellm-dashboard/src/components/common_components/KeyLifecycleSettings.test.tsx @@ -1,357 +1,214 @@ +import React, { useState } from "react"; +// eslint-disable-next-line no-restricted-imports -- exercising KeyLifecycleSettings requires hosting it in a real antd Form (the component it's built on) +import { Form } from "antd"; import userEvent from "@testing-library/user-event"; import { describe, expect, it, vi, beforeEach } from "vitest"; -import { renderWithProviders, screen } from "../../../tests/test-utils"; +import { renderWithProviders, screen, waitFor } from "../../../tests/test-utils"; import KeyLifecycleSettings from "./KeyLifecycleSettings"; -vi.mock("antd", () => { - const Option = ({ children, value }: any) => ; - const Select = ({ children, value, onChange, placeholder }: any) => ( - - ); - Select.Option = Option; - return { - Select, - Tooltip: ({ children, title }: any) => ( -
- {children} -
- ), - Switch: ({ checked, onChange }: any) => ( - onChange(e.target.checked)} /> - ), - Divider: () =>
, - }; -}); +const CREATE_PLACEHOLDER = "e.g., 30d or leave empty to never expire"; +const EDIT_PLACEHOLDER = "e.g., 30d"; -vi.mock("@ant-design/icons", () => ({ - InfoCircleOutlined: () => ℹ, -})); +interface HarnessProps { + isCreateMode?: boolean; + onFinish?: (values: Record) => void; +} -vi.mock("@tremor/react", () => ({ - TextInput: ({ value, onValueChange, onChange, placeholder, name, className }: any) => { - const handleChange = (e: React.ChangeEvent) => { - if (onChange) { - onChange(e); - } - if (onValueChange) { - onValueChange(e.target.value); - } - }; - return ( - = ({ isCreateMode = true, onFinish = () => {} }) => { + const [form] = Form.useForm(); + const [autoRotationEnabled, setAutoRotationEnabled] = useState(false); + const [rotationInterval, setRotationInterval] = useState(""); + const [neverExpire, setNeverExpire] = useState(false); + + return ( +
+ - ); - }, -})); + + + {rotationInterval} + + ); +}; + +const getDurationInput = (isCreateMode = true) => + screen.getByPlaceholderText(isCreateMode ? CREATE_PLACEHOLDER : EDIT_PLACEHOLDER) as HTMLInputElement; describe("KeyLifecycleSettings", () => { - const mockForm = { - getFieldValue: vi.fn(), - setFieldValue: vi.fn(), - setFieldsValue: vi.fn(), - }; - - const defaultProps = { - form: mockForm, - autoRotationEnabled: false, - onAutoRotationChange: vi.fn(), - rotationInterval: "", - onRotationIntervalChange: vi.fn(), - isCreateMode: false, - }; - beforeEach(() => { vi.clearAllMocks(); - mockForm.getFieldValue.mockReturnValue(""); }); - it("should render without crashing", () => { - renderWithProviders(); - + it("renders the expiry and auto-rotation sections", () => { + renderWithProviders(); expect(screen.getByText("Key Expiry Settings")).toBeInTheDocument(); expect(screen.getByText("Auto-Rotation Settings")).toBeInTheDocument(); + expect(getDurationInput()).toBeInTheDocument(); }); - describe("Key Expiry Settings", () => { - it("should render expiry input field", () => { - renderWithProviders(); + it("uses the create-mode placeholder in create mode", () => { + renderWithProviders(); + expect(screen.getByPlaceholderText(CREATE_PLACEHOLDER)).toBeInTheDocument(); + }); - expect(screen.getByText("Expire Key")).toBeInTheDocument(); - expect(screen.getByTestId("duration-input")).toBeInTheDocument(); - }); + it("uses the edit-mode placeholder in edit mode", () => { + renderWithProviders(); + expect(screen.getByPlaceholderText(EDIT_PLACEHOLDER)).toBeInTheDocument(); + }); - it("should show correct placeholder in create mode", () => { - renderWithProviders(); - - const input = screen.getByTestId("duration-input"); - expect(input).toHaveAttribute("placeholder", "e.g., 30d or leave empty to never expire"); - }); - - it("should show correct placeholder in edit mode", () => { - renderWithProviders(); - - const input = screen.getByTestId("duration-input"); - expect(input).toHaveAttribute("placeholder", "e.g., 30d"); - }); - - it("should show correct tooltip in create mode", () => { - renderWithProviders(); - - const tooltips = screen.getAllByTestId("tooltip"); - const expiryTooltip = tooltips.find((tooltip) => - tooltip.getAttribute("title")?.includes("Leave empty to keep the current expiry unchanged"), - ); - expect(expiryTooltip).toBeInTheDocument(); - expect(expiryTooltip).toHaveAttribute( - "title", - "Set when this key should expire. Format: 30s (seconds), 30m (minutes), 30h (hours), 30d (days). Leave empty to keep the current expiry unchanged.", - ); - }); - - it("should show correct tooltip in edit mode", () => { - renderWithProviders(); - - const tooltips = screen.getAllByTestId("tooltip"); - const expiryTooltip = tooltips.find((tooltip) => - tooltip.getAttribute("title")?.includes("Leave empty to keep the current expiry unchanged"), - ); - expect(expiryTooltip).toBeInTheDocument(); - expect(expiryTooltip).toHaveAttribute( - "title", - "Set when this key should expire. Format: 30s (seconds), 30m (minutes), 30h (hours), 30d (days). Leave empty to keep the current expiry unchanged.", - ); - }); - - it("should initialize with form value if present", () => { - mockForm.getFieldValue.mockReturnValue("30d"); - renderWithProviders(); - - const input = screen.getByTestId("duration-input") as HTMLInputElement; - expect(input.value).toBe("30d"); - }); - - it("should update form using setFieldValue when duration changes", async () => { + describe("duration is a single source of truth (regression for pre-filled value dropped on submit)", () => { + it("submits the duration the user typed", async () => { const user = userEvent.setup(); - renderWithProviders(); + const onFinish = vi.fn(); + renderWithProviders(); - const input = screen.getByTestId("duration-input"); - await user.type(input, "60d"); + await user.type(getDurationInput(), "1d"); + await user.click(screen.getByRole("button", { name: "submit" })); - expect(mockForm.setFieldValue).toHaveBeenCalledWith("duration", "60d"); + await waitFor(() => expect(onFinish).toHaveBeenCalledTimes(1)); + expect(onFinish.mock.calls[0][0]).toMatchObject({ duration: "1d" }); }); - it("should update form using setFieldsValue when setFieldValue is not available", async () => { + it("clears the displayed value when the form is reset, so no stale value lingers", async () => { const user = userEvent.setup(); - const formWithoutSetFieldValue = { - getFieldValue: vi.fn().mockReturnValue(""), - setFieldsValue: vi.fn(), - }; - renderWithProviders(); + renderWithProviders(); - const input = screen.getByTestId("duration-input"); - await user.type(input, "90d"); + await user.type(getDurationInput(), "1d"); + expect(getDurationInput().value).toBe("1d"); - expect(formWithoutSetFieldValue.setFieldsValue).toHaveBeenCalledWith({ duration: "90d" }); + await user.click(screen.getByRole("button", { name: "reset" })); + + await waitFor(() => expect(getDurationInput().value).toBe("")); + }); + + it("never submits a value that differs from what is displayed after a reset", async () => { + const user = userEvent.setup(); + const onFinish = vi.fn(); + renderWithProviders(); + + // First create: type "1d" and submit -> "1d" is sent. + await user.type(getDurationInput(), "1d"); + await user.click(screen.getByRole("button", { name: "submit" })); + await waitFor(() => expect(onFinish).toHaveBeenCalledTimes(1)); + expect(onFinish.mock.calls[0][0]).toMatchObject({ duration: "1d" }); + + // Second create: form resets, so the field must show empty AND submit empty. + // The old bug showed a stale "1d" while submitting null/empty. + await user.click(screen.getByRole("button", { name: "reset" })); + await waitFor(() => expect(getDurationInput().value).toBe("")); + + await user.click(screen.getByRole("button", { name: "submit" })); + await waitFor(() => expect(onFinish).toHaveBeenCalledTimes(2)); + expect(onFinish.mock.calls[1][0].duration).not.toBe("1d"); + expect(getDurationInput().value).toBe(onFinish.mock.calls[1][0].duration ?? ""); }); }); - describe("Auto-Rotation Settings", () => { - it("should render auto-rotation switch", () => { - renderWithProviders(); - - expect(screen.getByText("Enable Auto-Rotation")).toBeInTheDocument(); - expect(screen.getByTestId("switch")).toBeInTheDocument(); - }); - - it("should show switch as unchecked when autoRotationEnabled is false", () => { - renderWithProviders(); - - const switchElement = screen.getByTestId("switch") as HTMLInputElement; - expect(switchElement.checked).toBe(false); - }); - - it("should show switch as checked when autoRotationEnabled is true", () => { - renderWithProviders(); - - const switchElement = screen.getByTestId("switch") as HTMLInputElement; - expect(switchElement.checked).toBe(true); - }); - - it("should call onAutoRotationChange when switch is toggled", async () => { + describe("Never Expire", () => { + it("clears and disables the duration input, then submits an empty duration", async () => { const user = userEvent.setup(); - const onAutoRotationChange = vi.fn(); - renderWithProviders(); + const onFinish = vi.fn(); + renderWithProviders(); - const switchElement = screen.getByTestId("switch"); - await user.click(switchElement); + await user.type(getDurationInput(false), "30d"); + expect(getDurationInput(false).value).toBe("30d"); - expect(onAutoRotationChange).toHaveBeenCalledWith(true); + await user.click(screen.getByRole("checkbox", { name: /never expire/i })); + + await waitFor(() => expect(getDurationInput(false).value).toBe("")); + expect(getDurationInput(false)).toBeDisabled(); + + await user.click(screen.getByRole("button", { name: "submit" })); + await waitFor(() => expect(onFinish).toHaveBeenCalledTimes(1)); + expect(onFinish.mock.calls[0][0]).toMatchObject({ duration: "" }); }); + }); - it("should not show rotation interval section when auto-rotation is disabled", () => { - renderWithProviders(); + describe("Auto-Rotation", () => { + it("reveals the rotation interval controls when enabled", async () => { + const user = userEvent.setup(); + renderWithProviders(); expect(screen.queryByText("Rotation Interval")).not.toBeInTheDocument(); - expect(screen.queryByTestId("select")).not.toBeInTheDocument(); + await user.click(screen.getByRole("switch")); + + await waitFor(() => expect(screen.getByText("Rotation Interval")).toBeInTheDocument()); }); - it("should show rotation interval section when auto-rotation is enabled", () => { - renderWithProviders(); - - expect(screen.getByText("Rotation Interval")).toBeInTheDocument(); - expect(screen.getByTestId("select")).toBeInTheDocument(); - }); - - it("should show all predefined interval options", () => { - renderWithProviders(); - - expect(screen.getByText("7 days")).toBeInTheDocument(); - expect(screen.getByText("30 days")).toBeInTheDocument(); - expect(screen.getByText("90 days")).toBeInTheDocument(); - expect(screen.getByText("180 days")).toBeInTheDocument(); - expect(screen.getByText("365 days")).toBeInTheDocument(); - expect(screen.getByText("Custom interval")).toBeInTheDocument(); - }); - - it("should display current rotation interval in select", () => { - renderWithProviders(); - - const select = screen.getByTestId("select") as HTMLSelectElement; - expect(select.value).toBe("90d"); - }); - - it("should call onRotationIntervalChange when predefined interval is selected", async () => { + it("propagates a selected predefined interval", async () => { const user = userEvent.setup(); - const onRotationIntervalChange = vi.fn(); - renderWithProviders( - , - ); + renderWithProviders(); - const select = screen.getByTestId("select"); - await user.selectOptions(select, "30d"); + await user.click(screen.getByRole("switch")); + await waitFor(() => expect(screen.getByText("Rotation Interval")).toBeInTheDocument()); - expect(onRotationIntervalChange).toHaveBeenCalledWith("30d"); + await user.click(screen.getByRole("combobox")); + await user.click(await screen.findByText("90 days")); + + await waitFor(() => expect(document.querySelector(".ant-select-selection-item")?.textContent).toBe("90 days")); + expect(screen.getByTestId("rotation-interval-value")).toHaveTextContent("90d"); }); - it("should show custom input when custom option is selected", async () => { + it("shows the custom interval input when Custom interval is selected, without propagating yet", async () => { const user = userEvent.setup(); - renderWithProviders(); + renderWithProviders(); - const select = screen.getByTestId("select"); - await user.selectOptions(select, "custom"); + await user.click(screen.getByRole("switch")); + await waitFor(() => expect(screen.getByText("Rotation Interval")).toBeInTheDocument()); - expect(screen.getByTestId("custom-interval-input")).toBeInTheDocument(); + await user.click(screen.getByRole("combobox")); + await user.click(await screen.findByText("Custom interval")); + + expect(await screen.findByPlaceholderText("e.g., 1s, 5m, 2h, 14d")).toBeInTheDocument(); expect(screen.getByText("Supported formats: seconds (s), minutes (m), hours (h), days (d)")).toBeInTheDocument(); + expect(screen.getByTestId("rotation-interval-value")).toHaveTextContent(""); }); - it("should hide custom input when predefined interval is selected after custom", async () => { + it("propagates a typed custom interval to the parent", async () => { const user = userEvent.setup(); - const onRotationIntervalChange = vi.fn(); - renderWithProviders( - , - ); + renderWithProviders(); - const select = screen.getByTestId("select"); - await user.selectOptions(select, "7d"); + await user.click(screen.getByRole("switch")); + await waitFor(() => expect(screen.getByText("Rotation Interval")).toBeInTheDocument()); - expect(screen.queryByTestId("custom-interval-input")).not.toBeInTheDocument(); - expect(onRotationIntervalChange).toHaveBeenCalledWith("7d"); - }); + await user.click(screen.getByRole("combobox")); + await user.click(await screen.findByText("Custom interval")); - it("should call onRotationIntervalChange when custom interval is entered", async () => { - const user = userEvent.setup(); - const onRotationIntervalChange = vi.fn(); - renderWithProviders( - , - ); - - const select = screen.getByTestId("select"); - await user.selectOptions(select, "custom"); - - const customInput = screen.getByTestId("custom-interval-input"); + const customInput = await screen.findByPlaceholderText("e.g., 1s, 5m, 2h, 14d"); await user.type(customInput, "14d"); - expect(onRotationIntervalChange).toHaveBeenCalledWith("14d"); + await waitFor(() => expect(screen.getByTestId("rotation-interval-value")).toHaveTextContent("14d")); + expect((customInput as HTMLInputElement).value).toBe("14d"); }); - it("should show info message when auto-rotation is enabled", () => { - renderWithProviders(); - - expect( - screen.getByText( - "When rotation occurs, you'll receive a notification with the new key. The old key will be deactivated after a brief grace period.", - ), - ).toBeInTheDocument(); - }); - - it("should not show info message when auto-rotation is disabled", () => { - renderWithProviders(); - - expect( - screen.queryByText( - "When rotation occurs, you'll receive a notification with the new key. The old key will be deactivated after a brief grace period.", - ), - ).not.toBeInTheDocument(); - }); - - it("should initialize with custom interval input visible when custom interval is provided", () => { - renderWithProviders(); - - expect(screen.getByTestId("custom-interval-input")).toBeInTheDocument(); - const customInput = screen.getByTestId("custom-interval-input") as HTMLInputElement; - expect(customInput.value).toBe("14d"); - }); - - it("should show custom option selected when custom interval is provided", () => { - renderWithProviders(); - - const select = screen.getByTestId("select") as HTMLSelectElement; - expect(select.value).toBe("custom"); - }); - - it("should not call onRotationIntervalChange when selecting custom option", async () => { + it("hides the custom input and propagates the value when switching back to a predefined interval", async () => { const user = userEvent.setup(); - const onRotationIntervalChange = vi.fn(); - renderWithProviders( - , - ); + renderWithProviders(); - const select = screen.getByTestId("select"); - await user.selectOptions(select, "custom"); + await user.click(screen.getByRole("switch")); + await waitFor(() => expect(screen.getByText("Rotation Interval")).toBeInTheDocument()); - expect(onRotationIntervalChange).not.toHaveBeenCalled(); + await user.click(screen.getByRole("combobox")); + await user.click(await screen.findByText("Custom interval")); + const customInput = await screen.findByPlaceholderText("e.g., 1s, 5m, 2h, 14d"); + await user.type(customInput, "14d"); + await waitFor(() => expect(screen.getByTestId("rotation-interval-value")).toHaveTextContent("14d")); + + await user.click(screen.getByRole("combobox")); + await user.click(await screen.findByText("7 days")); + + await waitFor(() => expect(screen.getByTestId("rotation-interval-value")).toHaveTextContent("7d")); + expect(screen.queryByPlaceholderText("e.g., 1s, 5m, 2h, 14d")).not.toBeInTheDocument(); }); }); }); diff --git a/ui/litellm-dashboard/src/components/common_components/KeyLifecycleSettings.tsx b/ui/litellm-dashboard/src/components/common_components/KeyLifecycleSettings.tsx index 7c4738f9ede..8e88fab1095 100644 --- a/ui/litellm-dashboard/src/components/common_components/KeyLifecycleSettings.tsx +++ b/ui/litellm-dashboard/src/components/common_components/KeyLifecycleSettings.tsx @@ -1,5 +1,5 @@ import React, { useState } from "react"; -import { Select, Tooltip, Divider, Switch, Checkbox } from "antd"; +import { Select, Tooltip, Divider, Switch, Checkbox, Form } from "antd"; import { InfoCircleOutlined } from "@ant-design/icons"; import { TextInput } from "@tremor/react"; @@ -34,7 +34,6 @@ const KeyLifecycleSettings: React.FC = ({ const [showCustomInput, setShowCustomInput] = useState(isCustomInterval); const [customInterval, setCustomInterval] = useState(isCustomInterval ? rotationInterval : ""); - const [durationValue, setDurationValue] = useState(form?.getFieldValue?.("duration") || ""); const handleIntervalChange = (value: string) => { if (value === "custom") { @@ -53,14 +52,6 @@ const KeyLifecycleSettings: React.FC = ({ onRotationIntervalChange(value); }; - const handleDurationChange = (value: string) => { - setDurationValue(value); - if (form && typeof form.setFieldValue === "function") { - form.setFieldValue("duration", value); - } else if (form && typeof form.setFieldsValue === "function") { - form.setFieldsValue({ duration: value }); - } - }; return (
{/* Key Expiry Section */} @@ -80,7 +71,6 @@ const KeyLifecycleSettings: React.FC = ({ const checked = e.target.checked; onNeverExpireChange(checked); if (checked) { - setDurationValue(""); if (form && typeof form.setFieldValue === "function") { form.setFieldValue("duration", ""); } else if (form && typeof form.setFieldsValue === "function") { @@ -94,14 +84,13 @@ const KeyLifecycleSettings: React.FC = ({ )} - + + +
diff --git a/ui/litellm-dashboard/src/components/key_team_helpers/key_list.tsx b/ui/litellm-dashboard/src/components/key_team_helpers/key_list.tsx index ceff1809b7b..4b446b0c283 100644 --- a/ui/litellm-dashboard/src/components/key_team_helpers/key_list.tsx +++ b/ui/litellm-dashboard/src/components/key_team_helpers/key_list.tsx @@ -1,6 +1,7 @@ import { Setter } from "@/types"; import { useEffect, useState } from "react"; import { keyListCall, Member, Organization } from "../networking"; +import type { ObjectPermission } from "../object_permission_types"; export interface Team { team_id: string; @@ -90,16 +91,7 @@ export interface KeyResponse { user_tpm_limit: number; user_rpm_limit: number; user_email: string; - object_permission?: { - object_permission_id: string; - mcp_servers: string[]; - mcp_access_groups?: string[]; - mcp_toolsets?: string[] | null; - mcp_tool_permissions?: Record; - vector_stores: string[]; - agents?: string[]; - agent_access_groups?: string[]; - }; + object_permission?: ObjectPermission | null; access_group_ids?: string[]; budget_fallbacks?: Record; budget_limits?: Array<{ budget_duration: string; max_budget: number; reset_at?: string }>; diff --git a/ui/litellm-dashboard/src/components/networking.tsx b/ui/litellm-dashboard/src/components/networking.tsx index 051b83f4e27..576e16cbb37 100644 --- a/ui/litellm-dashboard/src/components/networking.tsx +++ b/ui/litellm-dashboard/src/components/networking.tsx @@ -28,6 +28,7 @@ import { TagNewRequest, TagUpdateRequest, TagListResponse, TagInfoResponse } fro import { Team } from "./key_team_helpers/key_list"; import { EmailEventSettingsResponse, EmailEventSettingsUpdateRequest } from "./email_events/types"; import type { SkillRegisterRequest } from "./claude_code_plugins/types"; +import type { ObjectPermission } from "./object_permission_types"; import { jsonFields } from "./common_components/check_openapi_schema"; import NotificationsManager from "./molecules/notifications_manager"; import type { MCPUserEnvVarsStatus } from "./mcp_tools/types"; @@ -208,13 +209,7 @@ export interface Organization { teams: any[] | null; users: any[] | null; members: any[] | null; - object_permission?: { - object_permission_id: string; - mcp_servers: string[]; - mcp_access_groups?: string[]; - mcp_toolsets?: string[]; - vector_stores: string[]; - }; + object_permission?: ObjectPermission | null; } export interface CredentialItem { @@ -1184,35 +1179,6 @@ export const organizationInfoCall = async (accessToken: string, organizationID: } }; -export const organizationCreateCall = async ( - accessToken: string, - formValues: Record, // Assuming formValues is an object -) => { - try { - if (formValues.metadata) { - // if there's an exception JSON.parse, show it in the message - try { - formValues.metadata = JSON.parse(formValues.metadata); - } catch (error) { - console.error("Failed to parse metadata:", error); - throw new Error("Failed to parse metadata: " + error); - } - } - - const data = await apiClient.post(`/organization/new`, { - accessToken, - body: { - ...formValues, // Include formValues in the request body - }, - }); - return data; - // Handle success - you might want to update some state or UI based on the created key - } catch (error) { - console.error("Failed to create key:", error); - throw error; - } -}; - export const organizationUpdateCall = async ( accessToken: string, formValues: Record, // Assuming formValues is an object diff --git a/ui/litellm-dashboard/src/components/object_permission_types.ts b/ui/litellm-dashboard/src/components/object_permission_types.ts new file mode 100644 index 00000000000..bde7281faec --- /dev/null +++ b/ui/litellm-dashboard/src/components/object_permission_types.ts @@ -0,0 +1,3 @@ +import type { components } from "@/lib/http/schema"; + +export type ObjectPermission = Partial; diff --git a/ui/litellm-dashboard/src/components/object_permissions_view.tsx b/ui/litellm-dashboard/src/components/object_permissions_view.tsx index b0ee38bd834..687d1a5a846 100644 --- a/ui/litellm-dashboard/src/components/object_permissions_view.tsx +++ b/ui/litellm-dashboard/src/components/object_permissions_view.tsx @@ -3,21 +3,10 @@ import { Text } from "@tremor/react"; import VectorStorePermissions from "./permissions/VectorStorePermissions"; import MCPServerPermissions from "./permissions/MCPServerPermissions"; import AgentPermissions from "./permissions/AgentPermissions"; - -interface ObjectPermission { - object_permission_id: string; - mcp_servers: string[]; - mcp_access_groups?: string[]; - mcp_tool_permissions?: Record; - mcp_toolsets?: string[] | null; - vector_stores: string[]; - agents?: string[]; - agent_access_groups?: string[]; - search_tools?: string[]; -} +import type { ObjectPermission } from "./object_permission_types"; interface ObjectPermissionsViewProps { - objectPermission?: ObjectPermission; + objectPermission?: ObjectPermission | null; variant?: "card" | "inline"; className?: string; accessToken?: string | null; diff --git a/ui/litellm-dashboard/src/components/organisms/create_key_button.tsx b/ui/litellm-dashboard/src/components/organisms/create_key_button.tsx index 2a04bad6aa8..711652eb783 100644 --- a/ui/litellm-dashboard/src/components/organisms/create_key_button.tsx +++ b/ui/litellm-dashboard/src/components/organisms/create_key_button.tsx @@ -1644,9 +1644,6 @@ const CreateKey: React.FC = ({ team, teams, data, addKey, autoOp /> - diff --git a/ui/litellm-dashboard/src/components/organization/org-create/OrgCreateDialog.test.tsx b/ui/litellm-dashboard/src/components/organization/org-create/OrgCreateDialog.test.tsx new file mode 100644 index 00000000000..5ec9bb1e633 --- /dev/null +++ b/ui/litellm-dashboard/src/components/organization/org-create/OrgCreateDialog.test.tsx @@ -0,0 +1,203 @@ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import React from "react"; +import { describe, expect, it, vi } from "vitest"; + +vi.mock("@/components/molecules/notifications_manager", () => ({ + __esModule: true, + default: { success: vi.fn(), fromBackend: vi.fn() }, +})); +vi.mock("@/components/ModelSelect/ModelSelect", () => ({ + ModelSelect: ({ onChange }: { onChange: (values: string[]) => void }) => ( + + ), +})); +vi.mock("@/components/vector_store_management/VectorStoreSelector", () => ({ + __esModule: true, + default: ({ onChange }: { onChange: (values: string[]) => void }) => ( + + ), +})); +vi.mock("@/components/mcp_server_management/MCPServerSelector", () => ({ + __esModule: true, + default: ({ + onChange, + }: { + onChange: (values: { servers: string[]; accessGroups: string[]; toolsets: string[] }) => void; + }) => ( + + ), +})); + +import { OrgCreateDialog } from "./OrgCreateDialog"; + +const Harness = ({ createOrganization }: { createOrganization: (body: unknown) => Promise }) => { + const [open, setOpen] = React.useState(true); + return ( + <> + + + + ); +}; + +const renderDialog = (overrides?: { createOrganization?: ReturnType }) => { + const createOrganization = overrides?.createOrganization ?? vi.fn().mockResolvedValue({}); + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + render( + + + , + ); + return { createOrganization }; +}; + +describe("OrgCreateDialog", () => { + it("blocks submit and shows an error when the name is missing", async () => { + const user = userEvent.setup(); + const { createOrganization } = renderDialog(); + + await user.click(screen.getByRole("button", { name: "Create Organization" })); + + expect(await screen.findByRole("alert")).toHaveTextContent("Please input an organization name"); + expect(createOrganization).not.toHaveBeenCalled(); + }); + + it("sends only alias and models for a minimal create and closes the dialog", async () => { + const user = userEvent.setup(); + const { createOrganization } = renderDialog(); + + await user.type(screen.getByLabelText("Organization Name"), "new-org"); + await user.click(screen.getByRole("button", { name: "Create Organization" })); + + await waitFor(() => expect(createOrganization).toHaveBeenCalledTimes(1)); + expect(createOrganization.mock.calls[0][0]).toStrictEqual({ organization_alias: "new-org", models: [] }); + await waitFor(() => expect(screen.queryByLabelText("Organization Name")).not.toBeInTheDocument()); + }); + + it("maps selectors and limits into the create body", async () => { + const user = userEvent.setup(); + const { createOrganization } = renderDialog(); + + await user.type(screen.getByLabelText("Organization Name"), "new-org"); + await user.click(screen.getByRole("button", { name: "set-models" })); + await user.type(screen.getByLabelText("Tokens per minute Limit (TPM)"), "1000"); + await user.click(screen.getByRole("button", { name: "set-vector-stores" })); + await user.click(screen.getByRole("button", { name: "set-mcp" })); + await user.click(screen.getByRole("button", { name: "Create Organization" })); + + await waitFor(() => expect(createOrganization).toHaveBeenCalledTimes(1)); + const expectedBody = { + organization_alias: "new-org", + models: ["gpt-5.2"], + tpm_limit: 1000, + object_permission: { + vector_stores: ["vs-1"], + mcp_servers: ["srv-1"], + mcp_toolsets: ["ts-1"], + }, + }; + expect(createOrganization.mock.calls[0][0]).toStrictEqual(expectedBody); + }); + + it("blocks submit and shows an error for invalid metadata JSON", async () => { + const user = userEvent.setup(); + const { createOrganization } = renderDialog(); + + await user.type(screen.getByLabelText("Organization Name"), "new-org"); + await user.type(screen.getByLabelText("Metadata"), "not json"); + await user.click(screen.getByRole("button", { name: "Create Organization" })); + + expect(await screen.findByRole("alert")).toHaveTextContent("Metadata must be a valid JSON object"); + expect(createOrganization).not.toHaveBeenCalled(); + }); + + it("keeps the dialog open with the entered values when the create fails", async () => { + const user = userEvent.setup(); + const { createOrganization } = renderDialog({ + createOrganization: vi.fn().mockRejectedValue(new Error("boom")), + }); + + await user.type(screen.getByLabelText("Organization Name"), "new-org"); + await user.click(screen.getByRole("button", { name: "Create Organization" })); + + await waitFor(() => expect(createOrganization).toHaveBeenCalledTimes(1)); + expect(screen.getByLabelText("Organization Name")).toHaveValue("new-org"); + }); + + it("resets the form when the dialog is cancelled and reopened", async () => { + const user = userEvent.setup(); + renderDialog(); + + await user.type(screen.getByLabelText("Organization Name"), "abandoned"); + await user.click(screen.getByRole("button", { name: "Cancel" })); + await waitFor(() => expect(screen.queryByLabelText("Organization Name")).not.toBeInTheDocument()); + + await user.click(screen.getByRole("button", { name: "reopen" })); + expect(screen.getByLabelText("Organization Name")).toHaveValue(""); + }); + + it("resets the form when the dialog is dismissed with Escape and reopened", async () => { + const user = userEvent.setup(); + renderDialog(); + + await user.type(screen.getByLabelText("Organization Name"), "abandoned"); + await user.keyboard("{Escape}"); + await waitFor(() => expect(screen.queryByLabelText("Organization Name")).not.toBeInTheDocument()); + + await user.click(screen.getByRole("button", { name: "reopen" })); + expect(screen.getByLabelText("Organization Name")).toHaveValue(""); + }); + + it("cannot be dismissed while a create is pending, then closes once on success", async () => { + const user = userEvent.setup(); + let resolveCreate: (value: unknown) => void = () => {}; + const createOrganization = vi.fn().mockImplementation( + () => + new Promise((resolve) => { + resolveCreate = resolve; + }), + ); + renderDialog({ createOrganization }); + + await user.type(screen.getByLabelText("Organization Name"), "new-org"); + await user.keyboard("{Enter}"); + await waitFor(() => expect(createOrganization).toHaveBeenCalledTimes(1)); + + await user.keyboard("{Escape}"); + expect(screen.getByLabelText("Organization Name")).toHaveValue("new-org"); + + resolveCreate({}); + await waitFor(() => expect(screen.queryByLabelText("Organization Name")).not.toBeInTheDocument()); + }); + + it("does not fire a second create while one is pending", async () => { + const user = userEvent.setup(); + let resolveCreate: (value: unknown) => void = () => {}; + const createOrganization = vi.fn().mockImplementation( + () => + new Promise((resolve) => { + resolveCreate = resolve; + }), + ); + renderDialog({ createOrganization }); + + await user.type(screen.getByLabelText("Organization Name"), "new-org"); + await user.keyboard("{Enter}"); + await waitFor(() => expect(createOrganization).toHaveBeenCalledTimes(1)); + await user.keyboard("{Enter}"); + + expect(createOrganization).toHaveBeenCalledTimes(1); + resolveCreate({}); + await waitFor(() => expect(screen.queryByLabelText("Organization Name")).not.toBeInTheDocument()); + }); +}); diff --git a/ui/litellm-dashboard/src/components/organization/org-create/OrgCreateDialog.tsx b/ui/litellm-dashboard/src/components/organization/org-create/OrgCreateDialog.tsx new file mode 100644 index 00000000000..998d9446365 --- /dev/null +++ b/ui/litellm-dashboard/src/components/organization/org-create/OrgCreateDialog.tsx @@ -0,0 +1,186 @@ +"use client"; + +import { useMutation, useQueryClient } from "@tanstack/react-query"; +import * as React from "react"; + +import { organizationKeys } from "@/app/(dashboard)/hooks/organizations/useOrganizations"; +import { ModelSelect } from "@/components/ModelSelect/ModelSelect"; +import MCPServerSelector from "@/components/mcp_server_management/MCPServerSelector"; +import NotificationsManager from "@/components/molecules/notifications_manager"; +import { FieldGroup } from "@/components/shared/form/field"; +import { FormField } from "@/components/shared/form/FormField"; +import { Button } from "@/components/ui/button"; +import { Dialog, DialogContent, DialogFooter, DialogHeader, DialogTitle } from "@/components/ui/dialog"; +import { Input } from "@/components/ui/input"; +import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select"; +import { Textarea } from "@/components/ui/textarea"; +import VectorStoreSelector from "@/components/vector_store_management/VectorStoreSelector"; +import { useZodForm } from "@/lib/forms/useZodForm"; +import { fetchClient } from "@/lib/http/api"; + +import { BUDGET_DURATION_OPTIONS, NO_RESET } from "../org-settings/OrgSettingsForm"; +import { orgSettingsSchema } from "../org-settings/schema"; +import { buildOrgCreateBody, emptyOrgFormValues, type OrgCreateBody } from "./mapper"; + +const defaultCreateOrganization = async (body: OrgCreateBody): Promise => { + const { data } = await fetchClient.POST("/organization/new", { body }); + return data; +}; + +interface OrgCreateDialogProps { + open: boolean; + onOpenChange: (open: boolean) => void; + accessToken: string; + createOrganization?: (body: OrgCreateBody) => Promise; +} + +export const OrgCreateDialog = ({ + open, + onOpenChange, + accessToken, + createOrganization = defaultCreateOrganization, +}: OrgCreateDialogProps) => { + const queryClient = useQueryClient(); + const form = useZodForm(orgSettingsSchema, { defaultValues: emptyOrgFormValues }); + + const closeAndReset = () => { + form.reset(emptyOrgFormValues); + onOpenChange(false); + }; + + const mutation = useMutation({ + mutationFn: (body: OrgCreateBody) => createOrganization(body), + onSuccess: () => { + NotificationsManager.success("Organization created successfully"); + queryClient.invalidateQueries({ queryKey: organizationKeys.all }); + closeAndReset(); + }, + onError: (error: unknown) => + NotificationsManager.fromBackend(error instanceof Error ? error.message : "Failed to create organization"), + }); + + const handleOpenChange = (nextOpen: boolean) => { + if (!nextOpen && mutation.isPending) return; + if (!nextOpen) { + form.reset(emptyOrgFormValues); + } + onOpenChange(nextOpen); + }; + + const onSubmit = form.handleSubmit((values) => { + if (mutation.isPending) return; + mutation.mutate(buildOrgCreateBody(values)); + }); + + return ( + + + + Create Organization + + +
+ + + {({ ref, ...field }) => } + + + + {(field) => ( + + )} + + + + {({ ref, ...field }) => } + + + + {({ id, value, onChange, "aria-invalid": ariaInvalid, "aria-describedby": ariaDescribedBy }) => ( + + )} + + + + {({ ref, ...field }) => } + + + + {({ ref, ...field }) => } + + + + {(field) => ( + + )} + + + + {(field) => ( + + )} + + + + {({ ref, ...field }) =>