feat(ui): show MCP allowed clients as cards edited in a dialog

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
yassin 2026-09-19 00:13:44 +00:00
parent da603c629b
commit c32309fb2d
2 changed files with 201 additions and 80 deletions

View file

@ -1,4 +1,4 @@
import { fireEvent, render, screen, waitFor } from "@testing-library/react";
import { fireEvent, render, screen, waitFor, within } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { describe, it, expect, vi, beforeEach } from "vitest";
import MCPNetworkSettings from "./MCPNetworkSettings";
@ -26,12 +26,20 @@ const renderSettings = () => render(<MCPNetworkSettings accessToken="tok" />);
const ANTIGRAVITY = { alias: "Antigravity CLI", value: "antigravity-cli" };
const CODEX = { alias: "Codex", value: "codex-mcp-client" };
const clientCard = (alias: string) => screen.getByRole("button", { name: new RegExp(`^${alias}`) });
const fillClientDialog = async (alias: string, value: string) => {
const dialog = await screen.findByRole("dialog");
fireEvent.change(within(dialog).getByRole("textbox", { name: "Alias" }), { target: { value: alias } });
fireEvent.change(within(dialog).getByRole("textbox", { name: "Value" }), { target: { value } });
return dialog;
};
const addClient = async (alias: string, value: string) => {
await userEvent.click(screen.getByRole("button", { name: "Add client" }));
const aliases = screen.getAllByRole("textbox", { name: /^Client \d+ alias$/ });
const values = screen.getAllByRole("textbox", { name: /^Client \d+ value$/ });
fireEvent.change(aliases[aliases.length - 1], { target: { value: alias } });
fireEvent.change(values[values.length - 1], { target: { value } });
const dialog = await fillClientDialog(alias, value);
await userEvent.click(within(dialog).getByRole("button", { name: "Add" }));
await waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument());
};
describe("MCPNetworkSettings", () => {
@ -139,7 +147,7 @@ describe("MCPNetworkSettings", () => {
expect(updateConfigFieldSetting).not.toHaveBeenCalled();
});
it("labels the section Allowed Clients and renders each stored client as an alias and value row", async () => {
it("labels the section Allowed Clients and renders each stored client as a card showing alias and value", async () => {
vi.mocked(getGeneralSettingsCall).mockResolvedValue([
{ field_name: "mcp_allowed_clients", field_value: [ANTIGRAVITY, CODEX] },
]);
@ -148,10 +156,24 @@ describe("MCPNetworkSettings", () => {
expect(await screen.findByText("Allowed Clients")).toBeVisible();
expect(screen.queryByText(/Allowed Client IDs/)).not.toBeInTheDocument();
expect(screen.getByRole("textbox", { name: "Client 1 alias" })).toHaveValue("Antigravity CLI");
expect(screen.getByRole("textbox", { name: "Client 1 value" })).toHaveValue("antigravity-cli");
expect(screen.getByRole("textbox", { name: "Client 2 alias" })).toHaveValue("Codex");
expect(screen.getByRole("textbox", { name: "Client 2 value" })).toHaveValue("codex-mcp-client");
expect(screen.queryByText(/Allowed Client Applications/)).not.toBeInTheDocument();
expect(clientCard("Antigravity CLI")).toHaveTextContent("antigravity-cli");
expect(clientCard("Codex")).toHaveTextContent("codex-mcp-client");
expect(screen.queryByRole("dialog")).not.toBeInTheDocument();
});
it("opens an edit dialog when a client card is clicked, prefilled with that client's alias and value", async () => {
vi.mocked(getGeneralSettingsCall).mockResolvedValue([
{ field_name: "mcp_allowed_clients", field_value: [ANTIGRAVITY, CODEX] },
]);
renderSettings();
await screen.findByText("Allowed Clients");
await userEvent.click(clientCard("Codex"));
const dialog = await screen.findByRole("dialog", { name: "Edit client" });
expect(within(dialog).getByRole("textbox", { name: "Alias" })).toHaveValue("Codex");
expect(within(dialog).getByRole("textbox", { name: "Value" })).toHaveValue("codex-mcp-client");
});
it("warns that a stored allowlist in the old plain-string shape denies every client and lets Save remove it", async () => {
@ -162,7 +184,7 @@ describe("MCPNetworkSettings", () => {
renderSettings();
expect(await screen.findByText(/stored allowlist is not a list of alias and value pairs/)).toBeVisible();
expect(screen.queryByRole("textbox", { name: "Client 1 value" })).not.toBeInTheDocument();
expect(screen.queryByText("antigravity-cli")).not.toBeInTheDocument();
await userEvent.click(screen.getByRole("button", { name: /Save/ }));
@ -203,15 +225,22 @@ describe("MCPNetworkSettings", () => {
expect(deleteConfigFieldSetting).not.toHaveBeenCalledWith("tok", "mcp_allowed_clients");
});
it("edits a stored client's value in place and saves the new value", async () => {
it("edits a stored client's value through its dialog and saves the new value", async () => {
vi.mocked(getGeneralSettingsCall).mockResolvedValue([
{ field_name: "mcp_allowed_clients", field_value: [ANTIGRAVITY] },
]);
renderSettings();
fireEvent.change(await screen.findByRole("textbox", { name: "Client 1 value" }), {
target: { value: "0oa1b2c3d4e5f6g7h8i9" },
await screen.findByText("Allowed Clients");
await userEvent.click(clientCard("Antigravity CLI"));
const dialog = await screen.findByRole("dialog");
fireEvent.change(within(dialog).getByRole("textbox", { name: "Value" }), {
target: { value: " 0oa1b2c3d4e5f6g7h8i9 " },
});
await userEvent.click(within(dialog).getByRole("button", { name: "Done" }));
await waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument());
expect(clientCard("Antigravity CLI")).toHaveTextContent("0oa1b2c3d4e5f6g7h8i9");
await userEvent.click(screen.getByRole("button", { name: /Save/ }));
await waitFor(() =>
@ -221,22 +250,37 @@ describe("MCPNetworkSettings", () => {
);
});
it("refuses to save a client that has an alias but no value, and reports why", async () => {
it("keeps a stored client untouched when its dialog is cancelled", async () => {
vi.mocked(getGeneralSettingsCall).mockResolvedValue([
{ field_name: "mcp_allowed_clients", field_value: [ANTIGRAVITY] },
]);
renderSettings();
await screen.findByText("Allowed Clients");
await userEvent.click(clientCard("Antigravity CLI"));
const dialog = await screen.findByRole("dialog");
fireEvent.change(within(dialog).getByRole("textbox", { name: "Value" }), { target: { value: "changed" } });
await userEvent.click(within(dialog).getByRole("button", { name: "Cancel" }));
await waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument());
expect(clientCard("Antigravity CLI")).toHaveTextContent("antigravity-cli");
await userEvent.click(screen.getByRole("button", { name: /Save/ }));
await waitFor(() => expect(toast.success).toHaveBeenCalledWith("MCP network settings saved"));
expect(updateConfigFieldSetting).not.toHaveBeenCalled();
});
it("will not add a client that has an alias but no value", async () => {
renderSettings();
await screen.findByText("Allowed Clients");
await addClient("Antigravity CLI", "");
await userEvent.click(screen.getByRole("button", { name: /Save/ }));
await userEvent.click(screen.getByRole("button", { name: "Add client" }));
const dialog = await fillClientDialog("Antigravity CLI", " ");
await waitFor(() =>
expect(toast.fromError).toHaveBeenCalledWith(new Error("Every allowed client needs both an alias and a value")),
);
expect(updateConfigFieldSetting).not.toHaveBeenCalled();
expect(deleteConfigFieldSetting).not.toHaveBeenCalled();
expect(toast.success).not.toHaveBeenCalled();
expect(within(dialog).getByRole("button", { name: "Add" })).toBeDisabled();
});
it("drops rows left completely blank instead of saving or failing on them", async () => {
it("adds nothing when the add dialog is cancelled", async () => {
vi.mocked(getGeneralSettingsCall).mockResolvedValue([
{ field_name: "mcp_allowed_clients", field_value: [ANTIGRAVITY] },
]);
@ -244,6 +288,11 @@ describe("MCPNetworkSettings", () => {
renderSettings();
await screen.findByText("Allowed Clients");
await userEvent.click(screen.getByRole("button", { name: "Add client" }));
const dialog = await fillClientDialog("Codex", "codex-mcp-client");
await userEvent.click(within(dialog).getByRole("button", { name: "Cancel" }));
await waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument());
expect(screen.queryByText("Codex")).not.toBeInTheDocument();
await userEvent.click(screen.getByRole("button", { name: /Save/ }));
await waitFor(() => expect(toast.success).toHaveBeenCalledWith("MCP network settings saved"));
@ -260,13 +309,17 @@ describe("MCPNetworkSettings", () => {
]);
renderSettings();
await userEvent.click(await screen.findByRole("button", { name: "Remove client Claude Code" }));
await screen.findByText("Allowed Clients");
await userEvent.click(clientCard("Claude Code"));
await userEvent.click(within(await screen.findByRole("dialog")).getByRole("button", { name: "Remove client" }));
await waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument());
expect(screen.queryByText("claude-code")).not.toBeInTheDocument();
await userEvent.click(screen.getByRole("button", { name: /Save/ }));
await waitFor(() =>
expect(updateConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_allowed_clients", [ANTIGRAVITY, CODEX]),
);
expect(screen.queryByDisplayValue("claude-code")).not.toBeInTheDocument();
});
it("removes a client and clears the setting when the list becomes empty", async () => {
@ -275,9 +328,12 @@ describe("MCPNetworkSettings", () => {
]);
renderSettings();
await userEvent.click(await screen.findByRole("button", { name: "Remove client Claude Code" }));
await screen.findByText("Allowed Clients");
await userEvent.click(clientCard("Claude Code"));
await userEvent.click(within(await screen.findByRole("dialog")).getByRole("button", { name: "Remove client" }));
expect(screen.queryByDisplayValue("claude-code")).not.toBeInTheDocument();
await waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument());
expect(screen.queryByText("claude-code")).not.toBeInTheDocument();
await userEvent.click(screen.getByRole("button", { name: /Save/ }));
@ -304,7 +360,7 @@ describe("MCPNetworkSettings", () => {
renderSettings();
await screen.findByText("Allowed Client Applications");
await screen.findByText("Allowed Clients");
expect(screen.queryByText(/every client is denied/)).not.toBeInTheDocument();
});

View file

@ -1,9 +1,18 @@
import React, { useState, useEffect } from "react";
import React, { useState, useEffect, useId } from "react";
import { Save, Plus, X } from "lucide-react";
import { Badge } from "@/components/ui/badge";
import { Button } from "@/components/ui/button";
import { Card } from "@/components/ui/card";
import {
Dialog,
DialogContent,
DialogDescription,
DialogFooter,
DialogHeader,
DialogTitle,
} from "@/components/ui/dialog";
import { Input } from "@/components/ui/input";
import { Label } from "@/components/ui/label";
import { UiLoadingSpinner } from "@/components/ui/ui-loading-spinner";
import { DeprecationBanner } from "@/components/DeprecationBanner";
import { toast } from "@/lib/toast";
@ -36,6 +45,10 @@ interface AllowedClientRow extends AllowedClient {
readonly key: string;
}
interface ClientDraft extends AllowedClient {
readonly key: string | null;
}
const isAllowedClient = (entry: unknown): entry is AllowedClient => {
if (typeof entry !== "object" || entry === null) return false;
const { alias, value } = entry as Partial<Record<keyof AllowedClient, unknown>>;
@ -58,14 +71,10 @@ const parseStoredClients = (fieldValue: unknown): StoredAllowlist => {
};
let nextRowKey = 0;
const newRow = (client: AllowedClient = { alias: "", value: "" }): AllowedClientRow => ({
...client,
key: `client-${nextRowKey++}`,
});
const newRow = (client: AllowedClient): AllowedClientRow => ({ ...client, key: `client-${nextRowKey++}` });
const trimClient = ({ alias, value }: AllowedClient): AllowedClient => ({ alias: alias.trim(), value: value.trim() });
const isBlank = ({ alias, value }: AllowedClient) => alias === "" && value === "";
const isIncomplete = ({ alias, value }: AllowedClient) => alias === "" || value === "";
const sameList = (a: string[], b: string[]) => a.length === b.length && a.every((value, i) => value === b[i]);
@ -90,6 +99,67 @@ const clientsUnchangedSinceLoad = (value: AllowedClient[], stored: StoredAllowli
const headerUnchangedSinceLoad = (value: string, stored: string | null) =>
stored === null ? value === "" : value !== "" && value === stored;
interface AllowedClientDialogProps {
readonly draft: ClientDraft | null;
readonly onChange: (draft: ClientDraft) => void;
readonly onCommit: () => void;
readonly onRemove: () => void;
readonly onClose: () => void;
}
const AllowedClientDialog: React.FC<AllowedClientDialogProps> = ({ draft, onChange, onCommit, onRemove, onClose }) => {
const aliasId = useId();
const valueId = useId();
if (draft === null) return null;
return (
<Dialog open onOpenChange={(open) => !open && onClose()}>
<DialogContent>
<DialogHeader>
<DialogTitle>{draft.key === null ? "Add client" : "Edit client"}</DialogTitle>
<DialogDescription>
The alias is the name shown in the dashboard and gateway logs. The value is the exact JWT claim or header
value that identifies the client, such as the OAuth client ID your identity provider issues.
</DialogDescription>
</DialogHeader>
<div className="grid gap-4">
<div className="grid gap-2">
<Label htmlFor={aliasId}>Alias</Label>
<Input
id={aliasId}
value={draft.alias}
placeholder="e.g. Coding CLI"
onChange={(e) => onChange({ ...draft, alias: e.target.value })}
/>
</div>
<div className="grid gap-2">
<Label htmlFor={valueId}>Value</Label>
<Input
id={valueId}
value={draft.value}
placeholder="e.g. 0oa1b2c3d4e5f6g7h8i9"
className="font-mono"
onChange={(e) => onChange({ ...draft, value: e.target.value })}
/>
</div>
</div>
<DialogFooter>
{draft.key !== null && (
<Button type="button" variant="destructive" className="sm:mr-auto" onClick={onRemove}>
Remove client
</Button>
)}
<Button type="button" variant="outline" onClick={onClose}>
Cancel
</Button>
<Button type="button" disabled={isIncomplete(trimClient(draft))} onClick={onCommit}>
{draft.key === null ? "Add" : "Done"}
</Button>
</DialogFooter>
</DialogContent>
</Dialog>
);
};
const MCPNetworkSettings: React.FC<MCPNetworkSettingsProps> = ({ accessToken }) => {
const [loading, setLoading] = useState(true);
const [saving, setSaving] = useState(false);
@ -101,6 +171,7 @@ const MCPNetworkSettings: React.FC<MCPNetworkSettingsProps> = ({ accessToken })
const [storedClientIdHeader, setStoredClientIdHeader] = useState<string | null>(null);
const [currentIp, setCurrentIp] = useState<string | null>(null);
const [rangeDraft, setRangeDraft] = useState("");
const [clientDraft, setClientDraft] = useState<ClientDraft | null>(null);
useEffect(() => {
loadSettings();
@ -154,10 +225,7 @@ const MCPNetworkSettings: React.FC<MCPNetworkSettingsProps> = ({ accessToken })
};
const persistAllowedClients = async (token: string) => {
const clients = allowedClients.map(trimClient).filter((client) => !isBlank(client));
if (clients.some(isIncomplete)) {
throw new Error("Every allowed client needs both an alias and a value");
}
const clients = allowedClients.map(({ alias, value }) => ({ alias, value }));
if (clientsUnchangedSinceLoad(clients, storedClients)) return;
if (clients.length > 0) {
await updateConfigFieldSetting(token, "mcp_allowed_clients", clients);
@ -218,10 +286,22 @@ const MCPNetworkSettings: React.FC<MCPNetworkSettingsProps> = ({ accessToken })
setRangeDraft("");
};
const updateClient = (key: string, patch: Partial<AllowedClient>) =>
setAllowedClients(allowedClients.map((row) => (row.key === key ? { ...row, ...patch } : row)));
const commitClientDraft = () => {
if (clientDraft === null) return;
const client = trimClient(clientDraft);
setAllowedClients(
clientDraft.key === null
? [...allowedClients, newRow(client)]
: allowedClients.map((row) => (row.key === clientDraft.key ? { ...row, ...client } : row)),
);
setClientDraft(null);
};
const removeClient = (key: string) => setAllowedClients(allowedClients.filter((row) => row.key !== key));
const removeDraftedClient = () => {
if (clientDraft === null) return;
setAllowedClients(allowedClients.filter((row) => row.key !== clientDraft.key));
setClientDraft(null);
};
if (loading) {
return (
@ -307,7 +387,7 @@ const MCPNetworkSettings: React.FC<MCPNetworkSettingsProps> = ({ accessToken })
</Card>
<div>
<p className="text-lg font-semibold">Allowed Client Applications</p>
<p className="text-lg font-semibold">Allowed Clients</p>
<p className="mt-1 text-sm text-muted-foreground">
Only the MCP client applications listed here can use the gateway. Leave empty to allow every client. A client
that authenticates with a JWT is identified by the claim named in litellm_jwtauth.mcp_client_id_jwt_field in
@ -317,9 +397,6 @@ const MCPNetworkSettings: React.FC<MCPNetworkSettingsProps> = ({ accessToken })
</div>
<Card className="p-6">
<div className="mb-2 flex items-center">
<p className="text-sm font-medium">Allowed Clients</p>
</div>
{storedAllowlistIsMalformed && (
<p className="mb-2 text-sm text-destructive">
The stored allowlist is not a list of alias and value pairs, so every client is denied. Add the clients you
@ -333,35 +410,17 @@ const MCPNetworkSettings: React.FC<MCPNetworkSettingsProps> = ({ accessToken })
</p>
)}
{allowedClients.length > 0 && (
<div className="mb-2 grid grid-cols-[1fr_1fr_auto] items-center gap-2">
<p className="text-xs text-muted-foreground">Alias</p>
<p className="text-xs text-muted-foreground">Value</p>
<span />
{allowedClients.map((row, index) => (
<React.Fragment key={row.key}>
<Input
aria-label={`Client ${index + 1} alias`}
value={row.alias}
placeholder="e.g. Coding CLI"
onChange={(e) => updateClient(row.key, { alias: e.target.value })}
/>
<Input
aria-label={`Client ${index + 1} value`}
value={row.value}
placeholder="e.g. 0oa1b2c3d4e5f6g7h8i9"
className="font-mono"
onChange={(e) => updateClient(row.key, { value: e.target.value })}
/>
<Button
type="button"
variant="ghost"
size="icon"
aria-label={`Remove client ${row.alias.trim() || index + 1}`}
onClick={() => removeClient(row.key)}
>
<X className="size-4" />
</Button>
</React.Fragment>
<div className="mb-3 grid gap-2 sm:grid-cols-2 lg:grid-cols-3">
{allowedClients.map((row) => (
<button
key={row.key}
type="button"
className="flex min-w-0 flex-col items-start gap-1 rounded-lg border border-border bg-background p-3 text-left hover:bg-muted focus-visible:ring-2 focus-visible:ring-ring focus-visible:outline-none"
onClick={() => setClientDraft(row)}
>
<span className="w-full truncate text-sm font-medium">{row.alias}</span>
<span className="w-full truncate font-mono text-xs text-muted-foreground">{row.value}</span>
</button>
))}
</div>
)}
@ -369,16 +428,14 @@ const MCPNetworkSettings: React.FC<MCPNetworkSettingsProps> = ({ accessToken })
type="button"
variant="outline"
size="sm"
onClick={() => setAllowedClients([...allowedClients, newRow()])}
onClick={() => setClientDraft({ key: null, alias: "", value: "" })}
>
<Plus />
Add client
</Button>
<p className="mt-2 text-xs text-muted-foreground">
The alias is the name shown here and in gateway logs. The value is the exact JWT claim or header value that
identifies the client, such as the OAuth client ID your identity provider issues. Leave the list empty to
allow every client. Every MCP request from an unlisted client, or from one with no resolvable identity, gets a
403.
Click a client to edit or remove it. Leave the list empty to allow every client. Every MCP request from an
unlisted client, or from one with no resolvable identity, gets a 403.
</p>
<div className="mt-6 mb-2 flex items-center">
@ -403,6 +460,14 @@ const MCPNetworkSettings: React.FC<MCPNetworkSettingsProps> = ({ accessToken })
Save
</Button>
</div>
<AllowedClientDialog
draft={clientDraft}
onChange={setClientDraft}
onCommit={commitClientDraft}
onRemove={removeDraftedClient}
onClose={() => setClientDraft(null)}
/>
</div>
);
};