mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-26 01:12:21 +00:00
fix(ui): keep per-user MCP credentials updatable and clearable after setup (#42652)
* fix(ui): keep per-user MCP credentials updatable and clearable after setup Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(ui): keep card keyboard activation off nested credential buttons Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(ui): confirm before clearing saved per-user MCP credentials Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(ui): reset the clear confirmation when the credentials modal closes Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(mcp): cover per-user env var edge paths in browser and integration contracts Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(integration): wait for the peer process to grant the key before listing its MCP tools Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(e2e): drop the mutable removed flag from the deleted-server browser contract Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: ryan <ryan@berri.ai> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
devin-ai-integration[bot]
GitHub
parent
1c289e5ecd
commit
9fd25b2228
9 changed files with 1025 additions and 12 deletions
|
|
@ -1,3 +1,9 @@
|
|||
[
|
||||
"tests/e2e/ui/tests/integrationCritical/projectDetachment.spec.ts::project creation and explicit detachment preserve saved scope and restore serving"
|
||||
"tests/e2e/ui/tests/integrationCritical/projectDetachment.spec.ts::project creation and explicit detachment preserve saved scope and restore serving",
|
||||
"tests/e2e/ui/tests/integrationCritical/mcpUserEnvVars.spec.ts::per-user MCP env var stays updatable and clearable from the card after it is set",
|
||||
"tests/e2e/ui/tests/integrationCritical/mcpUserEnvVars.spec.ts::cancelling the clear confirmation keeps the stored value and sends no delete",
|
||||
"tests/e2e/ui/tests/integrationCritical/mcpUserEnvVars.spec.ts::pressing Enter on Update opens the credentials modal instead of the server editor",
|
||||
"tests/e2e/ui/tests/integrationCritical/mcpUserEnvVars.spec.ts::a server with two per-user variables reports the remaining gap until both are saved",
|
||||
"tests/e2e/ui/tests/integrationCritical/mcpUserEnvVars.spec.ts::a server without per-user variables shows no credential row",
|
||||
"tests/e2e/ui/tests/integrationCritical/mcpUserEnvVars.spec.ts::clearing credentials for a server deleted underneath the modal reports the failure without losing the page"
|
||||
]
|
||||
|
|
|
|||
416
tests/e2e/ui/tests/integrationCritical/mcpUserEnvVars.spec.ts
Normal file
416
tests/e2e/ui/tests/integrationCritical/mcpUserEnvVars.spec.ts
Normal file
|
|
@ -0,0 +1,416 @@
|
|||
import {
|
||||
test,
|
||||
expect,
|
||||
APIRequestContext,
|
||||
Locator,
|
||||
Page as PlaywrightPage,
|
||||
} from "@playwright/test";
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { Page } from "../../fixtures/pages";
|
||||
import { navigateToPage } from "../../helpers/navigation";
|
||||
import { captureRequestBody } from "../../helpers/roundTrip";
|
||||
|
||||
const master = process.env.LITELLM_MASTER_KEY ?? "sk-integration-master";
|
||||
const headers = { Authorization: `Bearer ${master}` };
|
||||
const TOKEN = "USER_TOKEN";
|
||||
|
||||
type EnvVarStatus = {
|
||||
missing_count: number;
|
||||
required: { name: string; is_set: boolean }[];
|
||||
};
|
||||
|
||||
type Server = {
|
||||
name: string;
|
||||
id: string;
|
||||
statusUrl: string;
|
||||
status: () => Promise<EnvVarStatus>;
|
||||
remove: () => Promise<void>;
|
||||
};
|
||||
|
||||
async function createServer(
|
||||
request: APIRequestContext,
|
||||
variables: string[],
|
||||
): Promise<Server> {
|
||||
const name = `int_mcp_${randomUUID().replace(/-/g, "").slice(0, 12)}`;
|
||||
const created = await request.post("/v1/mcp/server", {
|
||||
headers,
|
||||
data: {
|
||||
server_name: name,
|
||||
url: `${process.env.INTEGRATION_UPSTREAM_URL}/mcp`,
|
||||
transport: "http",
|
||||
auth_type: "none",
|
||||
env_vars: variables.map((variable) => ({
|
||||
name: variable,
|
||||
scope: "user",
|
||||
description: `Per-user ${variable}`,
|
||||
})),
|
||||
static_headers: Object.fromEntries(
|
||||
variables.map((variable, index) => [
|
||||
`X-User-${index}`,
|
||||
`\${${variable}}`,
|
||||
]),
|
||||
),
|
||||
},
|
||||
});
|
||||
expect(created.ok(), await created.text()).toBe(true);
|
||||
const id = (await created.json()).server_id as string;
|
||||
const statusUrl = `/v1/mcp/server/${id}/user-env-vars`;
|
||||
return {
|
||||
name,
|
||||
id,
|
||||
statusUrl,
|
||||
status: async () => {
|
||||
const response = await request.get(statusUrl, { headers });
|
||||
expect(response.ok(), await response.text()).toBe(true);
|
||||
return response.json() as Promise<EnvVarStatus>;
|
||||
},
|
||||
remove: async () => {
|
||||
const removed = await request.delete(`/v1/mcp/server/${id}`, { headers });
|
||||
expect(
|
||||
removed.ok() || removed.status() === 404,
|
||||
await removed.text(),
|
||||
).toBe(true);
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
async function openMcpServers(page: PlaywrightPage): Promise<void> {
|
||||
await page.goto("/ui/login");
|
||||
await page.getByPlaceholder("Enter your username").fill("admin");
|
||||
await page.getByPlaceholder("Enter your password").fill(master);
|
||||
await page.getByRole("button", { name: "Login", exact: true }).click();
|
||||
await expect(page).toHaveURL(
|
||||
(url) => url.pathname.startsWith("/ui") && !url.pathname.includes("login"),
|
||||
);
|
||||
await navigateToPage(page, Page.McpServers);
|
||||
}
|
||||
|
||||
function cardFor(page: PlaywrightPage, server: Server): Locator {
|
||||
return page.getByRole("button").filter({ hasText: server.name }).first();
|
||||
}
|
||||
|
||||
function credentialsDialog(page: PlaywrightPage): Locator {
|
||||
return page.getByRole("dialog").filter({ hasText: "Set your credentials" });
|
||||
}
|
||||
|
||||
async function saveValues(
|
||||
page: PlaywrightPage,
|
||||
server: Server,
|
||||
values: Record<string, string>,
|
||||
): Promise<void> {
|
||||
const dialog = credentialsDialog(page);
|
||||
for (const [variable, value] of Object.entries(values)) {
|
||||
await dialog.getByLabel(variable).fill(value);
|
||||
}
|
||||
const body = await captureRequestBody(
|
||||
page,
|
||||
{ method: "POST", urlIncludes: server.statusUrl },
|
||||
async () => {
|
||||
await dialog.getByRole("button", { name: "Save Credentials" }).click();
|
||||
},
|
||||
);
|
||||
expect(body).toEqual({ values });
|
||||
await expect(dialog).toHaveCount(0);
|
||||
}
|
||||
|
||||
test("per-user MCP env var stays updatable and clearable from the card after it is set", async ({
|
||||
page,
|
||||
request,
|
||||
}) => {
|
||||
const server = await createServer(request, [TOKEN]);
|
||||
try {
|
||||
await openMcpServers(page);
|
||||
const card = cardFor(page, server);
|
||||
const dialog = credentialsDialog(page);
|
||||
await expect(
|
||||
card.getByText("1 user field missing", { exact: true }),
|
||||
).toBeVisible();
|
||||
|
||||
await card.getByRole("button", { name: "Set", exact: true }).click();
|
||||
await saveValues(page, server, { [TOKEN]: "first-token" });
|
||||
expect(await server.status()).toMatchObject({
|
||||
missing_count: 0,
|
||||
required: [{ name: TOKEN, is_set: true }],
|
||||
});
|
||||
|
||||
await expect(
|
||||
card.getByText("1 user field missing", { exact: true }),
|
||||
).toHaveCount(0);
|
||||
await page.reload();
|
||||
const update = card.getByRole("button", { name: "Update", exact: true });
|
||||
await expect(
|
||||
update,
|
||||
"a set per-user variable must keep an update entry point on the card",
|
||||
).toBeVisible();
|
||||
await update.click();
|
||||
await expect(dialog.getByText("Set", { exact: true })).toBeVisible();
|
||||
await saveValues(page, server, { [TOKEN]: "rotated-token" });
|
||||
expect(await server.status()).toMatchObject({
|
||||
missing_count: 0,
|
||||
required: [{ name: TOKEN, is_set: true }],
|
||||
});
|
||||
|
||||
await update.click();
|
||||
const cleared = page.waitForResponse(
|
||||
(response) =>
|
||||
response.request().method() === "DELETE" &&
|
||||
response.url().includes(server.statusUrl),
|
||||
);
|
||||
await dialog.getByRole("button", { name: "Clear", exact: true }).click();
|
||||
const confirm = page.getByRole("alertdialog", {
|
||||
name: "Clear saved credentials",
|
||||
});
|
||||
await expect(confirm).toContainText(server.name);
|
||||
await confirm
|
||||
.getByRole("button", { name: "Clear credentials", exact: true })
|
||||
.click();
|
||||
const clearResponse = await cleared;
|
||||
expect(clearResponse.ok(), await clearResponse.text()).toBe(true);
|
||||
await expect(dialog).toHaveCount(0);
|
||||
expect(await server.status()).toMatchObject({
|
||||
missing_count: 1,
|
||||
required: [{ name: TOKEN, is_set: false }],
|
||||
});
|
||||
await expect(
|
||||
card.getByText("1 user field missing", { exact: true }),
|
||||
).toBeVisible();
|
||||
await expect(
|
||||
card.getByRole("button", { name: "Set", exact: true }),
|
||||
).toBeVisible();
|
||||
} finally {
|
||||
await server.remove();
|
||||
}
|
||||
});
|
||||
|
||||
test("cancelling the clear confirmation keeps the stored value and sends no delete", async ({
|
||||
page,
|
||||
request,
|
||||
}) => {
|
||||
const server = await createServer(request, [TOKEN]);
|
||||
try {
|
||||
const stored = await request.post(server.statusUrl, {
|
||||
headers,
|
||||
data: { values: { [TOKEN]: "keep-me" } },
|
||||
});
|
||||
expect(stored.ok(), await stored.text()).toBe(true);
|
||||
await openMcpServers(page);
|
||||
const card = cardFor(page, server);
|
||||
const dialog = credentialsDialog(page);
|
||||
const deletes: string[] = [];
|
||||
page.on("request", (sent) => {
|
||||
if (sent.method() === "DELETE" && sent.url().includes(server.statusUrl))
|
||||
deletes.push(sent.url());
|
||||
});
|
||||
await card.getByRole("button", { name: "Update", exact: true }).click();
|
||||
await dialog.getByRole("button", { name: "Clear", exact: true }).click();
|
||||
const confirm = page.getByRole("alertdialog", {
|
||||
name: "Clear saved credentials",
|
||||
});
|
||||
await expect(confirm).toBeVisible();
|
||||
await confirm.getByRole("button", { name: "Cancel", exact: true }).click();
|
||||
await expect(confirm).toHaveCount(0);
|
||||
await expect(
|
||||
dialog,
|
||||
"cancelling the confirmation must leave the credentials modal open",
|
||||
).toBeVisible();
|
||||
await dialog.getByRole("button", { name: "Cancel", exact: true }).click();
|
||||
await expect(dialog).toHaveCount(0);
|
||||
await card.getByRole("button", { name: "Update", exact: true }).click();
|
||||
await expect(
|
||||
confirm,
|
||||
"a cancelled confirmation must not reappear on reopen",
|
||||
).toHaveCount(0);
|
||||
await page.keyboard.press("Escape");
|
||||
await expect(dialog).toHaveCount(0);
|
||||
expect(deletes).toEqual([]);
|
||||
expect(await server.status()).toMatchObject({
|
||||
missing_count: 0,
|
||||
required: [{ name: TOKEN, is_set: true }],
|
||||
});
|
||||
await expect(
|
||||
card.getByRole("button", { name: "Update", exact: true }),
|
||||
).toBeVisible();
|
||||
} finally {
|
||||
await server.remove();
|
||||
}
|
||||
});
|
||||
|
||||
test("pressing Enter on Update opens the credentials modal instead of the server editor", async ({
|
||||
page,
|
||||
request,
|
||||
}) => {
|
||||
const server = await createServer(request, [TOKEN]);
|
||||
try {
|
||||
const stored = await request.post(server.statusUrl, {
|
||||
headers,
|
||||
data: { values: { [TOKEN]: "keyboard" } },
|
||||
});
|
||||
expect(stored.ok(), await stored.text()).toBe(true);
|
||||
await openMcpServers(page);
|
||||
const card = cardFor(page, server);
|
||||
const update = card.getByRole("button", { name: "Update", exact: true });
|
||||
await update.focus();
|
||||
await page.keyboard.press("Enter");
|
||||
const dialog = credentialsDialog(page);
|
||||
await expect(dialog).toBeVisible();
|
||||
await expect(
|
||||
page.getByRole("button", { name: "Back to All Servers" }),
|
||||
).toHaveCount(0);
|
||||
await saveValues(page, server, { [TOKEN]: "keyboard-rotated" });
|
||||
await expect(
|
||||
page.getByRole("button", { name: "Back to All Servers" }),
|
||||
).toHaveCount(0);
|
||||
await expect(card).toBeVisible();
|
||||
expect(await server.status()).toMatchObject({
|
||||
missing_count: 0,
|
||||
required: [{ name: TOKEN, is_set: true }],
|
||||
});
|
||||
await card.click();
|
||||
await expect(
|
||||
page.getByRole("button", { name: "Back to All Servers" }),
|
||||
).toBeVisible();
|
||||
} finally {
|
||||
await server.remove();
|
||||
}
|
||||
});
|
||||
|
||||
test("a server with two per-user variables reports the remaining gap until both are saved", async ({
|
||||
page,
|
||||
request,
|
||||
}) => {
|
||||
const second = "WORKSPACE";
|
||||
const server = await createServer(request, [TOKEN, second]);
|
||||
try {
|
||||
await openMcpServers(page);
|
||||
const card = cardFor(page, server);
|
||||
const dialog = credentialsDialog(page);
|
||||
await expect(
|
||||
card.getByText("2 user fields missing", { exact: true }),
|
||||
).toBeVisible();
|
||||
await card.getByRole("button", { name: "Set", exact: true }).click();
|
||||
await dialog.getByLabel(TOKEN).fill("only-token");
|
||||
const posts: string[] = [];
|
||||
page.on("request", (sent) => {
|
||||
if (sent.method() === "POST" && sent.url().includes(server.statusUrl))
|
||||
posts.push(sent.url());
|
||||
});
|
||||
await dialog.getByRole("button", { name: "Save Credentials" }).click();
|
||||
await expect(dialog.getByRole("alert")).toHaveText(`${second} is required`);
|
||||
expect(posts, "a missing required field must block the save").toEqual([]);
|
||||
await dialog.getByRole("button", { name: "Cancel", exact: true }).click();
|
||||
await expect(dialog).toHaveCount(0);
|
||||
|
||||
const partial = await request.post(server.statusUrl, {
|
||||
headers,
|
||||
data: { values: { [TOKEN]: "only-token" } },
|
||||
});
|
||||
expect(partial.ok(), await partial.text()).toBe(true);
|
||||
await page.reload();
|
||||
await expect(
|
||||
card.getByText("1 user field missing", { exact: true }),
|
||||
).toBeVisible();
|
||||
await expect(
|
||||
card.getByRole("button", { name: "Update", exact: true }),
|
||||
).toHaveCount(0);
|
||||
await card.getByRole("button", { name: "Set", exact: true }).click();
|
||||
await expect(dialog.getByText("Set", { exact: true })).toHaveCount(1);
|
||||
await saveValues(page, server, {
|
||||
[TOKEN]: "",
|
||||
[second]: "workspace-value",
|
||||
});
|
||||
expect(await server.status()).toMatchObject({
|
||||
missing_count: 0,
|
||||
required: [
|
||||
{ name: TOKEN, is_set: true },
|
||||
{ name: second, is_set: true },
|
||||
],
|
||||
});
|
||||
await expect(
|
||||
card.getByRole("button", { name: "Update", exact: true }),
|
||||
).toBeVisible();
|
||||
await expect(card.getByText(/user fields? missing/)).toHaveCount(0);
|
||||
} finally {
|
||||
await server.remove();
|
||||
}
|
||||
});
|
||||
|
||||
test("a server without per-user variables shows no credential row", async ({
|
||||
page,
|
||||
request,
|
||||
}) => {
|
||||
const server = await createServer(request, []);
|
||||
const withVariable = await createServer(request, [TOKEN]);
|
||||
try {
|
||||
await openMcpServers(page);
|
||||
const plain = cardFor(page, server);
|
||||
await expect(plain).toBeVisible();
|
||||
await expect(
|
||||
cardFor(page, withVariable).getByRole("button", {
|
||||
name: "Set",
|
||||
exact: true,
|
||||
}),
|
||||
).toBeVisible();
|
||||
await expect(plain.getByText("Per-user credentials")).toHaveCount(0);
|
||||
await expect(
|
||||
plain.getByRole("button", { name: "Set", exact: true }),
|
||||
).toHaveCount(0);
|
||||
await expect(
|
||||
plain.getByRole("button", { name: "Update", exact: true }),
|
||||
).toHaveCount(0);
|
||||
await expect(plain.getByText(/user fields? missing/)).toHaveCount(0);
|
||||
} finally {
|
||||
await server.remove();
|
||||
await withVariable.remove();
|
||||
}
|
||||
});
|
||||
|
||||
test("clearing credentials for a server deleted underneath the modal reports the failure without losing the page", async ({
|
||||
page,
|
||||
request,
|
||||
}) => {
|
||||
const server = await createServer(request, [TOKEN]);
|
||||
const survivor = await createServer(request, [TOKEN]);
|
||||
try {
|
||||
const stored = await request.post(server.statusUrl, {
|
||||
headers,
|
||||
data: { values: { [TOKEN]: "doomed" } },
|
||||
});
|
||||
expect(stored.ok(), await stored.text()).toBe(true);
|
||||
await openMcpServers(page);
|
||||
const card = cardFor(page, server);
|
||||
const dialog = credentialsDialog(page);
|
||||
await card.getByRole("button", { name: "Update", exact: true }).click();
|
||||
await expect(dialog).toBeVisible();
|
||||
await server.remove();
|
||||
const cleared = page.waitForResponse(
|
||||
(response) =>
|
||||
response.request().method() === "DELETE" &&
|
||||
response.url().includes(server.statusUrl),
|
||||
);
|
||||
await dialog.getByRole("button", { name: "Clear", exact: true }).click();
|
||||
await page
|
||||
.getByRole("alertdialog", { name: "Clear saved credentials" })
|
||||
.getByRole("button", {
|
||||
name: "Clear credentials",
|
||||
exact: true,
|
||||
})
|
||||
.click();
|
||||
const clearResponse = await cleared;
|
||||
expect(clearResponse.status()).toBe(404);
|
||||
await expect(page.getByText(/Failed to clear env vars/)).toBeVisible();
|
||||
await expect(
|
||||
dialog,
|
||||
"a failed clear must keep the modal open for the user",
|
||||
).toBeVisible();
|
||||
await page.keyboard.press("Escape");
|
||||
await expect(dialog).toHaveCount(0);
|
||||
await page.reload();
|
||||
await expect(
|
||||
cardFor(page, survivor).getByRole("button", { name: "Set", exact: true }),
|
||||
).toBeVisible();
|
||||
await expect(page.getByText(server.name)).toHaveCount(0);
|
||||
} finally {
|
||||
await server.remove();
|
||||
await survivor.remove();
|
||||
}
|
||||
});
|
||||
359
tests/integration/mcp/test_mcp_user_env_vars.py
Normal file
359
tests/integration/mcp/test_mcp_user_env_vars.py
Normal file
|
|
@ -0,0 +1,359 @@
|
|||
import signal
|
||||
import uuid
|
||||
from collections.abc import Mapping
|
||||
from concurrent.futures import ThreadPoolExecutor
|
||||
from dataclasses import dataclass
|
||||
from functools import partial
|
||||
from pathlib import Path
|
||||
from typing import Final
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
from integration._support.client import Gateway, Scenario, eventually, object_value, string_value
|
||||
from integration._support.database import read_rows
|
||||
from integration._support.mcp import McpPeer, call_tool, mcp_peer, register_mcp, tool_names
|
||||
from integration._support.process import owned_proxy_process
|
||||
from pydantic import JsonValue, TypeAdapter
|
||||
|
||||
TOKEN: Final = "USER_TOKEN"
|
||||
WORKSPACE: Final = "WORKSPACE"
|
||||
METHODS: Final = ("GET", "POST", "DELETE")
|
||||
|
||||
|
||||
@dataclass(frozen=True, slots=True)
|
||||
class UpstreamCall:
|
||||
body: dict[str, JsonValue]
|
||||
headers: dict[bytes, bytes]
|
||||
|
||||
|
||||
UPSTREAM_CALLS: Final = TypeAdapter(tuple[UpstreamCall, ...])
|
||||
STATUS_LISTING: Final = TypeAdapter(list[dict[str, JsonValue]])
|
||||
JSON_BODY: Final = TypeAdapter(dict[str, JsonValue])
|
||||
|
||||
|
||||
def body(response: httpx.Response) -> dict[str, JsonValue]:
|
||||
return JSON_BODY.validate_json(response.content)
|
||||
|
||||
|
||||
def register_user_var_server(scenario: Scenario, peer: McpPeer, *names: str) -> str:
|
||||
return register_mcp(
|
||||
scenario,
|
||||
peer,
|
||||
"integration" + uuid.uuid4().hex,
|
||||
auth_type="none",
|
||||
env_vars=[{"name": name, "scope": "user", "description": f"per-user {name}"} for name in names],
|
||||
static_headers={
|
||||
"Authorization": f"Bearer ${{{TOKEN}}}",
|
||||
**({"X-Workspace": f"${{{WORKSPACE}}}"} if WORKSPACE in names else {}),
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
def grants(*identities: str) -> JsonValue:
|
||||
return {"mcp_servers": list(identities)}
|
||||
|
||||
|
||||
def user_key(scenario: Scenario, identity: str) -> str:
|
||||
return scenario.key(user_id=scenario.user(), object_permission=grants(identity))
|
||||
|
||||
|
||||
def env_status(gateway: Gateway, key: str, identity: str) -> httpx.Response:
|
||||
return gateway.request("GET", f"/v1/mcp/server/{identity}/user-env-vars", key=key)
|
||||
|
||||
|
||||
def store(gateway: Gateway, key: str, identity: str, values: Mapping[str, str]) -> httpx.Response:
|
||||
return gateway.request("POST", f"/v1/mcp/server/{identity}/user-env-vars", {"values": dict(values)}, key=key)
|
||||
|
||||
|
||||
def clear(gateway: Gateway, key: str, identity: str) -> httpx.Response:
|
||||
return gateway.request("DELETE", f"/v1/mcp/server/{identity}/user-env-vars", key=key)
|
||||
|
||||
|
||||
def set_names(response: httpx.Response) -> dict[str, bool]:
|
||||
assert response.status_code == 200, response.text
|
||||
status: Final = body(response)
|
||||
assert isinstance(status["required"], list)
|
||||
return {
|
||||
string_value(object_value(spec)["name"]): object_value(spec)["is_set"] is True for spec in status["required"]
|
||||
}
|
||||
|
||||
|
||||
def tool_calls(peer: McpPeer) -> tuple[UpstreamCall, ...]:
|
||||
return tuple(
|
||||
call for call in UPSTREAM_CALLS.validate_python(peer.drain()) if call.body.get("method") == "tools/call"
|
||||
)
|
||||
|
||||
|
||||
def add_upstream_headers(gateway: Gateway, peer: McpPeer, key: str, identity: str, a: int = 2) -> dict[bytes, bytes]:
|
||||
peer.drain()
|
||||
response: Final = call_tool(gateway, key, identity, tool_names(gateway, key, identity)["add"], {"a": a, "b": 3})
|
||||
assert response.status_code == 200, response.text
|
||||
calls: Final = tool_calls(peer)
|
||||
assert len(calls) == 1, calls
|
||||
return calls[0].headers
|
||||
|
||||
|
||||
def add_upstream_authorization(gateway: Gateway, peer: McpPeer, key: str, identity: str) -> bytes:
|
||||
return add_upstream_headers(gateway, peer, key, identity)[b"authorization"]
|
||||
|
||||
|
||||
def list_tools_status(target: Gateway, key: str, identity: str) -> int:
|
||||
return target.client.get(
|
||||
"/mcp-rest/tools/list", headers={"x-litellm-api-key": key}, params={"server_id": identity}
|
||||
).status_code
|
||||
|
||||
|
||||
def wait_for_tools(target: Gateway, key: str, identity: str) -> dict[str, str]:
|
||||
eventually(lambda: list_tools_status(target, key, identity), lambda status: status == 200, seconds=60)
|
||||
return eventually(lambda: tool_names(target, key, identity), lambda names: "add" in names, seconds=60)
|
||||
|
||||
|
||||
def assert_forwarded_eventually(target: Gateway, upstream: McpPeer, key: str, identity: str, expected: bytes) -> None:
|
||||
observed: Final = eventually(
|
||||
lambda: add_upstream_authorization(target, upstream, key, identity), lambda value: value == expected, seconds=75
|
||||
)
|
||||
assert observed == expected
|
||||
|
||||
|
||||
def assert_precondition_failed(gateway: Gateway, key: str, identity: str, *missing: str) -> None:
|
||||
response: Final = call_tool(gateway, key, identity, tool_names(gateway, key, identity)["add"], {"a": 2, "b": 3})
|
||||
assert response.status_code == 412, response.text
|
||||
detail: Final = object_value(body(response)["detail"])
|
||||
assert detail["error"] == "missing_user_env_vars"
|
||||
assert detail["server_id"] == identity
|
||||
assert isinstance(detail["missing"], list)
|
||||
assert sorted(string_value(name) for name in detail["missing"]) == sorted(missing)
|
||||
assert string_value(detail["setup_url"]).endswith(f"fill_env_vars={identity}")
|
||||
|
||||
|
||||
def stored_user_ids(identity: str) -> tuple[JsonValue, ...]:
|
||||
return tuple(
|
||||
row["user_id"]
|
||||
for row in read_rows('SELECT user_id FROM "LiteLLM_MCPUserEnvVars" WHERE server_id = %s', (identity,))
|
||||
)
|
||||
|
||||
|
||||
def missing_count(response: httpx.Response) -> JsonValue:
|
||||
return body(response)["missing_count"]
|
||||
|
||||
|
||||
def test_stored_value_is_forwarded_rotated_and_cleared(gateway: Gateway) -> None:
|
||||
with mcp_peer() as peer, gateway.scenario() as scenario:
|
||||
identity: Final = register_user_var_server(scenario, peer, TOKEN)
|
||||
key: Final = user_key(scenario, identity)
|
||||
before: Final = env_status(gateway, key, identity)
|
||||
assert set_names(before) == {TOKEN: False}
|
||||
assert missing_count(before) == 1
|
||||
assert string_value(body(before)["setup_url"]).endswith(f"fill_env_vars={identity}")
|
||||
assert_precondition_failed(gateway, key, identity, TOKEN)
|
||||
first: Final = store(gateway, key, identity, {TOKEN: "first-secret"})
|
||||
assert set_names(first) == {TOKEN: True}
|
||||
assert missing_count(first) == 0
|
||||
assert add_upstream_authorization(gateway, peer, key, identity) == b"Bearer first-secret"
|
||||
rotated: Final = store(gateway, key, identity, {TOKEN: "second-secret"})
|
||||
assert set_names(rotated) == {TOKEN: True}
|
||||
assert add_upstream_authorization(gateway, peer, key, identity) == b"Bearer second-secret"
|
||||
assert len(stored_user_ids(identity)) == 1
|
||||
cleared: Final = clear(gateway, key, identity)
|
||||
assert set_names(cleared) == {TOKEN: False}
|
||||
assert missing_count(cleared) == 1
|
||||
assert stored_user_ids(identity) == ()
|
||||
assert set_names(env_status(gateway, key, identity)) == {TOKEN: False}
|
||||
assert_precondition_failed(gateway, key, identity, TOKEN)
|
||||
assert set_names(clear(gateway, key, identity)) == {TOKEN: False}
|
||||
|
||||
|
||||
def test_store_merges_per_variable_and_drops_undeclared_or_empty_values(gateway: Gateway) -> None:
|
||||
with mcp_peer() as peer, gateway.scenario() as scenario:
|
||||
identity: Final = register_user_var_server(scenario, peer, TOKEN, WORKSPACE)
|
||||
key: Final = user_key(scenario, identity)
|
||||
assert set_names(env_status(gateway, key, identity)) == {TOKEN: False, WORKSPACE: False}
|
||||
assert_precondition_failed(gateway, key, identity, TOKEN, WORKSPACE)
|
||||
partial: Final = store(gateway, key, identity, {TOKEN: "tok", "NOT_DECLARED": "x", "": "y"})
|
||||
assert set_names(partial) == {TOKEN: True, WORKSPACE: False}
|
||||
assert missing_count(partial) == 1
|
||||
assert_precondition_failed(gateway, key, identity, WORKSPACE)
|
||||
long_value: Final = "w" * 5120
|
||||
complete: Final = store(gateway, key, identity, {WORKSPACE: long_value})
|
||||
assert set_names(complete) == {TOKEN: True, WORKSPACE: True}
|
||||
forwarded: Final = add_upstream_headers(gateway, peer, key, identity)
|
||||
assert forwarded[b"authorization"] == b"Bearer tok"
|
||||
assert forwarded[b"x-workspace"] == long_value.encode()
|
||||
kept: Final = store(gateway, key, identity, {TOKEN: "", WORKSPACE: ""})
|
||||
assert set_names(kept) == {TOKEN: True, WORKSPACE: True}
|
||||
assert add_upstream_authorization(gateway, peer, key, identity) == b"Bearer tok"
|
||||
assert set_names(store(gateway, key, identity, {TOKEN: "tok"})) == {TOKEN: True, WORKSPACE: True}
|
||||
assert len(stored_user_ids(identity)) == 1
|
||||
|
||||
|
||||
def test_malformed_bodies_missing_users_and_foreign_servers_are_rejected(gateway: Gateway) -> None:
|
||||
with mcp_peer() as peer, gateway.scenario() as scenario:
|
||||
identity: Final = register_user_var_server(scenario, peer, TOKEN)
|
||||
other: Final = register_user_var_server(scenario, peer, TOKEN)
|
||||
key: Final = user_key(scenario, identity)
|
||||
userless: Final = scenario.key(object_permission=grants(identity))
|
||||
path: Final = f"/v1/mcp/server/{identity}/user-env-vars"
|
||||
payload: Final[dict[str, JsonValue]] = {"values": {TOKEN: "x"}}
|
||||
malformed: Final[tuple[dict[str, JsonValue], ...]] = ({"values": {TOKEN: 7}}, {"values": ["a"]}, {})
|
||||
assert [gateway.request("POST", path, body, key=key).status_code for body in malformed] == [422, 422, 422]
|
||||
assert set_names(env_status(gateway, key, identity)) == {TOKEN: False}
|
||||
assert [gateway.client.request(method, path, json=payload).status_code for method in METHODS] == [401, 401, 401]
|
||||
no_user: Final = tuple(gateway.request(method, path, payload, key=userless) for method in METHODS)
|
||||
assert [response.status_code for response in no_user] == [400, 400, 400], [r.text for r in no_user]
|
||||
assert [object_value(body(r)["detail"])["error"] for r in no_user] == ["User ID not found in token"] * 3
|
||||
foreign: Final = f"/v1/mcp/server/{other}/user-env-vars"
|
||||
assert [gateway.request(method, foreign, payload, key=key).status_code for method in METHODS] == [403, 403, 403]
|
||||
unknown: Final = f"/v1/mcp/server/{uuid.uuid4()}/user-env-vars"
|
||||
assert [gateway.request(method, unknown, payload).status_code for method in METHODS] == [404, 404, 404]
|
||||
assert stored_user_ids(identity) == () and stored_user_ids(other) == ()
|
||||
|
||||
|
||||
def test_status_list_keeps_fully_set_servers_and_is_scoped_to_the_caller(gateway: Gateway) -> None:
|
||||
with mcp_peer() as peer, gateway.scenario() as scenario:
|
||||
per_user: Final = register_user_var_server(scenario, peer, TOKEN)
|
||||
global_only: Final = register_mcp(
|
||||
scenario,
|
||||
peer,
|
||||
"integration" + uuid.uuid4().hex,
|
||||
env_vars=[{"name": "GLOBAL_TOKEN", "scope": "global", "description": "shared"}],
|
||||
)
|
||||
plain: Final = register_mcp(scenario, peer, "integration" + uuid.uuid4().hex)
|
||||
first_user: Final = scenario.key(
|
||||
user_id=scenario.user(), object_permission=grants(per_user, global_only, plain)
|
||||
)
|
||||
second_user: Final = scenario.key(
|
||||
user_id=scenario.user(), object_permission=grants(per_user, global_only, plain)
|
||||
)
|
||||
|
||||
def listing(key: str) -> dict[str, JsonValue]:
|
||||
response: Final = gateway.request("GET", "/v1/mcp/user-env-vars/status", key=key)
|
||||
assert response.status_code == 200, response.text
|
||||
return {
|
||||
string_value(entry["server_id"]): entry["missing_count"]
|
||||
for entry in STATUS_LISTING.validate_json(response.content)
|
||||
if entry["server_id"] in {per_user, global_only, plain}
|
||||
}
|
||||
|
||||
assert listing(first_user) == {per_user: 1}
|
||||
assert set_names(store(gateway, first_user, per_user, {TOKEN: "mine"})) == {TOKEN: True}
|
||||
assert listing(first_user) == {per_user: 0}
|
||||
assert listing(second_user) == {per_user: 1}
|
||||
assert set_names(env_status(gateway, second_user, per_user)) == {TOKEN: False}
|
||||
assert add_upstream_authorization(gateway, peer, first_user, per_user) == b"Bearer mine"
|
||||
assert_precondition_failed(gateway, second_user, per_user, TOKEN)
|
||||
assert set_names(clear(gateway, second_user, per_user)) == {TOKEN: False}
|
||||
assert listing(first_user) == {per_user: 0}
|
||||
assert add_upstream_authorization(gateway, peer, first_user, per_user) == b"Bearer mine"
|
||||
|
||||
|
||||
def test_store_and_clear_on_one_process_are_honored_by_the_other(gateway: Gateway, peer: Gateway) -> None:
|
||||
with mcp_peer() as upstream, gateway.scenario() as scenario:
|
||||
identity: Final = register_user_var_server(scenario, upstream, TOKEN)
|
||||
key: Final = user_key(scenario, identity)
|
||||
assert_precondition_failed(gateway, key, identity, TOKEN)
|
||||
wait_for_tools(peer, key, identity)
|
||||
assert_precondition_failed(peer, key, identity, TOKEN)
|
||||
assert set_names(store(gateway, key, identity, {TOKEN: "from-a"})) == {TOKEN: True}
|
||||
assert set_names(env_status(peer, key, identity)) == {TOKEN: True}
|
||||
assert add_upstream_authorization(peer, upstream, key, identity) == b"Bearer from-a"
|
||||
assert set_names(store(peer, key, identity, {TOKEN: "from-b"})) == {TOKEN: True}
|
||||
assert_forwarded_eventually(gateway, upstream, key, identity, b"Bearer from-b")
|
||||
assert set_names(clear(gateway, key, identity)) == {TOKEN: False}
|
||||
assert set_names(env_status(peer, key, identity)) == {TOKEN: False}
|
||||
assert stored_user_ids(identity) == ()
|
||||
|
||||
def peer_status() -> int:
|
||||
names: Final = tool_names(peer, key, identity)
|
||||
return call_tool(peer, key, identity, names["add"], {"a": 1, "b": 1}).status_code
|
||||
|
||||
assert eventually(peer_status, lambda code: code == 412, seconds=75) == 412
|
||||
assert_precondition_failed(gateway, key, identity, TOKEN)
|
||||
|
||||
|
||||
@pytest.mark.timeout(240)
|
||||
def test_concurrent_users_across_processes_never_leak_and_survive_a_killed_process(
|
||||
gateway: Gateway, peer: Gateway, tmp_path: Path
|
||||
) -> None:
|
||||
with mcp_peer() as upstream, gateway.scenario() as scenario:
|
||||
identity: Final = register_user_var_server(scenario, upstream, TOKEN)
|
||||
users: Final = tuple(scenario.user() for _ in range(4))
|
||||
keys: Final = {user: scenario.key(user_id=user, object_permission=grants(identity)) for user in users}
|
||||
assert [set_names(store(gateway, keys[user], identity, {TOKEN: f"seed-{user}"})) for user in users] == [
|
||||
{TOKEN: True}
|
||||
] * len(users)
|
||||
names: Final = wait_for_tools(gateway, keys[users[0]], identity)
|
||||
wait_for_tools(peer, keys[users[0]], identity)
|
||||
|
||||
def operation(target: Gateway, user: str, index: int) -> httpx.Response:
|
||||
if index % 4 == 1:
|
||||
return env_status(target, keys[user], identity)
|
||||
if index % 4 == 2:
|
||||
return call_tool(target, keys[user], identity, names["add"], {"a": users.index(user), "b": 0})
|
||||
return store(target, keys[user], identity, {TOKEN: f"{user}-{index}"})
|
||||
|
||||
def outcome(targets: tuple[Gateway, ...], job: tuple[str, int]) -> tuple[int, int]:
|
||||
return job[1], operation(targets[job[1] % len(targets)], job[0], job[1]).status_code
|
||||
|
||||
def burst(pool: ThreadPoolExecutor, targets: tuple[Gateway, ...]) -> tuple[tuple[int, int], ...]:
|
||||
jobs: Final = tuple((user, index) for user in users for index in range(6))
|
||||
return tuple(pool.map(partial(outcome, targets), jobs))
|
||||
|
||||
def allowed_authorizations(item: UpstreamCall) -> tuple[str, frozenset[bytes]]:
|
||||
arguments: Final = object_value(object_value(item.body["params"])["arguments"])
|
||||
owner: Final = users[int(string_value(str(arguments["a"])))]
|
||||
return owner, frozenset(
|
||||
{f"Bearer seed-{owner}".encode()} | {f"Bearer {owner}-{i}".encode() for i in range(6)}
|
||||
)
|
||||
|
||||
with owned_proxy_process(gateway, tmp_path, {}) as doomed, ThreadPoolExecutor(max_workers=8) as pool:
|
||||
wait_for_tools(doomed.gateway, keys[users[0]], identity)
|
||||
upstream.drain()
|
||||
outcomes: Final = burst(pool, (gateway, peer, doomed.gateway))
|
||||
assert all(code in {200, 412} for _, code in outcomes), outcomes
|
||||
assert all(code == 200 for index, code in outcomes if index % 4 != 2), outcomes
|
||||
doomed.process.send_signal(signal.SIGKILL)
|
||||
doomed.process.wait(timeout=10)
|
||||
after_kill: Final = burst(pool, (gateway, peer))
|
||||
assert all(code in {200, 412} for _, code in after_kill), after_kill
|
||||
assert all(code == 200 for index, code in after_kill if index % 4 != 2), after_kill
|
||||
forwarded: Final = tool_calls(upstream)
|
||||
assert forwarded
|
||||
leaked: Final = tuple(
|
||||
(owner, item.headers[b"authorization"])
|
||||
for item in forwarded
|
||||
for owner, allowed in (allowed_authorizations(item),)
|
||||
if item.headers[b"authorization"] not in allowed
|
||||
)
|
||||
assert leaked == ()
|
||||
assert [set_names(env_status(gateway, keys[user], identity)) for user in users] == [{TOKEN: True}] * len(users)
|
||||
assert [set_names(env_status(peer, keys[user], identity)) for user in users] == [{TOKEN: True}] * len(users)
|
||||
assert [set_names(store(gateway, keys[user], identity, {TOKEN: f"final-{user}"})) for user in users] == [
|
||||
{TOKEN: True}
|
||||
] * len(users)
|
||||
for user in users:
|
||||
assert_forwarded_eventually(peer, upstream, keys[user], identity, f"Bearer final-{user}".encode())
|
||||
assert sorted(string_value(user_id) for user_id in stored_user_ids(identity)) == sorted(users)
|
||||
|
||||
|
||||
def test_concurrent_stores_of_different_variables_do_not_lose_an_update(gateway: Gateway, peer: Gateway) -> None:
|
||||
with mcp_peer() as upstream, gateway.scenario() as scenario:
|
||||
identity: Final = register_user_var_server(scenario, upstream, TOKEN, WORKSPACE)
|
||||
key: Final = user_key(scenario, identity)
|
||||
wait_for_tools(peer, key, identity)
|
||||
|
||||
def race_once(pool: ThreadPoolExecutor) -> None:
|
||||
assert set_names(clear(gateway, key, identity)) == {TOKEN: False, WORKSPACE: False}
|
||||
first: Final = pool.submit(store, gateway, key, identity, {TOKEN: "racing-token"})
|
||||
second: Final = pool.submit(store, peer, key, identity, {WORKSPACE: "racing-workspace"})
|
||||
assert first.result().status_code == 200, first.result().text
|
||||
assert second.result().status_code == 200, second.result().text
|
||||
assert set_names(env_status(gateway, key, identity)) == {TOKEN: True, WORKSPACE: True}
|
||||
assert set_names(env_status(peer, key, identity)) == {TOKEN: True, WORKSPACE: True}
|
||||
assert len(stored_user_ids(identity)) == 1
|
||||
forwarded: Final = add_upstream_headers(gateway, upstream, key, identity, a=1)
|
||||
assert forwarded[b"authorization"] == b"Bearer racing-token"
|
||||
assert forwarded[b"x-workspace"] == b"racing-workspace"
|
||||
|
||||
with ThreadPoolExecutor(max_workers=2) as pool:
|
||||
for _ in range(5):
|
||||
race_once(pool)
|
||||
|
|
@ -1,5 +1,5 @@
|
|||
import React from "react";
|
||||
import { render, screen } from "@testing-library/react";
|
||||
import { fireEvent, render, screen } from "@testing-library/react";
|
||||
import { describe, it, expect, vi, afterEach } from "vitest";
|
||||
import MCPServerCard from "./MCPServerCard";
|
||||
import type { MCPServer } from "@/components/mcp_tools/types";
|
||||
|
|
@ -67,3 +67,48 @@ describe("MCPServerCard logo", () => {
|
|||
expect(screen.getByText("DE")).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
describe("MCPServerCard per-user credentials", () => {
|
||||
const renderUserFields = (props: { missingUserFields?: string[]; hasUserFields?: boolean }) => {
|
||||
const onOpenFillFields = vi.fn();
|
||||
const onClick = vi.fn();
|
||||
render(<MCPServerCard server={baseServer} onClick={onClick} onOpenFillFields={onOpenFillFields} {...props} />);
|
||||
return { onOpenFillFields, onClick };
|
||||
};
|
||||
|
||||
it("offers Set while a field is missing", () => {
|
||||
const { onOpenFillFields, onClick } = renderUserFields({ missingUserFields: ["USER_TOKEN"], hasUserFields: true });
|
||||
expect(screen.getByText("1 user field missing")).toBeInTheDocument();
|
||||
expect(screen.queryByRole("button", { name: "Update" })).not.toBeInTheDocument();
|
||||
fireEvent.click(screen.getByRole("button", { name: "Set" }));
|
||||
expect(onOpenFillFields).toHaveBeenCalledTimes(1);
|
||||
expect(onClick).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("keeps an Update entry point once every field is set", () => {
|
||||
const { onOpenFillFields, onClick } = renderUserFields({ missingUserFields: [], hasUserFields: true });
|
||||
expect(screen.getByText("Per-user credentials")).toBeInTheDocument();
|
||||
expect(screen.getByText("Set")).toBeInTheDocument();
|
||||
expect(screen.queryByRole("button", { name: "Set" })).not.toBeInTheDocument();
|
||||
expect(screen.queryByText(/user field/)).not.toBeInTheDocument();
|
||||
fireEvent.click(screen.getByRole("button", { name: "Update" }));
|
||||
expect(onOpenFillFields).toHaveBeenCalledTimes(1);
|
||||
expect(onClick).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("keeps Enter on the Update button away from the card's open handler", () => {
|
||||
const { onClick } = renderUserFields({ missingUserFields: [], hasUserFields: true });
|
||||
const update = screen.getByRole("button", { name: "Update" });
|
||||
expect(fireEvent.keyDown(update, { key: "Enter" }), "default activation must survive").toBe(true);
|
||||
expect(onClick).not.toHaveBeenCalled();
|
||||
fireEvent.keyDown(screen.getAllByRole("button")[0], { key: "Enter" });
|
||||
expect(onClick).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("renders no credential row for a server without per-user fields", () => {
|
||||
renderUserFields({ missingUserFields: [], hasUserFields: false });
|
||||
expect(screen.queryByText("Per-user credentials")).not.toBeInTheDocument();
|
||||
expect(screen.queryByRole("button", { name: "Update" })).not.toBeInTheDocument();
|
||||
expect(screen.queryByRole("button", { name: "Set" })).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -21,6 +21,7 @@ interface MCPServerCardProps {
|
|||
// Computed by the parent from the bulk /user-env-vars/status response, so
|
||||
// the card never issues a per-row request (no N+1).
|
||||
missingUserFields?: string[];
|
||||
hasUserFields?: boolean;
|
||||
isLoadingHealth?: boolean;
|
||||
isRechecking?: boolean;
|
||||
onClick: () => void;
|
||||
|
|
@ -42,6 +43,7 @@ const stop = (e: MouseEvent | KeyboardEvent) => e.stopPropagation();
|
|||
const MCPServerCard: FC<MCPServerCardProps> = ({
|
||||
server,
|
||||
missingUserFields,
|
||||
hasUserFields,
|
||||
isLoadingHealth,
|
||||
isRechecking,
|
||||
onClick,
|
||||
|
|
@ -100,6 +102,7 @@ const MCPServerCard: FC<MCPServerCardProps> = ({
|
|||
}
|
||||
|
||||
const handleKeyDown = (e: KeyboardEvent<HTMLDivElement>) => {
|
||||
if (e.target !== e.currentTarget) return;
|
||||
if (e.key === "Enter" || e.key === " ") {
|
||||
e.preventDefault();
|
||||
onClick();
|
||||
|
|
@ -256,9 +259,10 @@ const MCPServerCard: FC<MCPServerCardProps> = ({
|
|||
)}
|
||||
</div>
|
||||
|
||||
{(server.is_byok || needsAttention) && (
|
||||
{(server.is_byok || hasUserFields || needsAttention) && (
|
||||
<div className="mt-auto flex flex-col gap-2">
|
||||
{server.is_byok && <ByokRow connected={!!server.has_user_credential} onConnect={onByokConnect} />}
|
||||
{hasUserFields && !needsAttention && <UserFieldsRow onUpdate={onOpenFillFields} />}
|
||||
{needsAttention && (
|
||||
<div className="flex items-center justify-between gap-2 text-xs">
|
||||
<Tooltip>
|
||||
|
|
@ -365,6 +369,29 @@ const HealthChip: FC<HealthChipProps> = ({
|
|||
);
|
||||
};
|
||||
|
||||
const UserFieldsRow: FC<{ onUpdate?: () => void }> = ({ onUpdate }) => (
|
||||
<div className="flex items-center justify-between gap-2 text-xs">
|
||||
<span className="text-muted-foreground">Per-user credentials</span>
|
||||
<div className="flex items-center gap-2">
|
||||
<Badge variant="outline">
|
||||
<Check /> Set
|
||||
</Badge>
|
||||
{onUpdate && (
|
||||
<Button
|
||||
variant="link"
|
||||
size="sm"
|
||||
onClick={(e) => {
|
||||
stop(e);
|
||||
onUpdate();
|
||||
}}
|
||||
>
|
||||
Update
|
||||
</Button>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
|
||||
interface ByokRowProps {
|
||||
connected: boolean;
|
||||
onConnect?: () => void;
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import React from "react";
|
||||
import { act, fireEvent, render, screen, waitFor } from "@testing-library/react";
|
||||
import { act, fireEvent, render, screen, waitFor, within } from "@testing-library/react";
|
||||
import userEvent, { PointerEventsCheckLevel } from "@testing-library/user-event";
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
|
|
@ -10,6 +10,7 @@ import { MCPServer, MCPUserEnvVarsStatus } from "@/components/mcp_tools/types";
|
|||
vi.mock("@/components/networking", () => ({
|
||||
getMCPUserEnvVars: vi.fn(),
|
||||
storeMCPUserEnvVars: vi.fn(),
|
||||
clearMCPUserEnvVars: vi.fn(),
|
||||
}));
|
||||
|
||||
const createQueryClient = () => new QueryClient({ defaultOptions: { queries: { retry: false, gcTime: 0 } } });
|
||||
|
|
@ -216,6 +217,89 @@ describe("UserEnvVarsModal", () => {
|
|||
expect(networking.storeMCPUserEnvVars).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("clears every stored value through the delete endpoint once the user confirms", async () => {
|
||||
const user = setup();
|
||||
const cleared = statusWith([{ name: "API_KEY", description: null, is_set: false }]);
|
||||
vi.mocked(networking.clearMCPUserEnvVars).mockResolvedValue(cleared);
|
||||
const { onSaved, onClose } = renderModal(statusWith([{ name: "API_KEY", description: null, is_set: true }]));
|
||||
|
||||
await fieldAfterOpen(/^API_KEY/);
|
||||
await user.click(screen.getByRole("button", { name: "Clear" }));
|
||||
expect(networking.clearMCPUserEnvVars).not.toHaveBeenCalled();
|
||||
const confirm = await screen.findByRole("alertdialog", { name: "Clear saved credentials" });
|
||||
await user.click(within(confirm).getByRole("button", { name: "Clear credentials" }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(onSaved).toHaveBeenCalledWith(cleared);
|
||||
});
|
||||
expect(networking.clearMCPUserEnvVars).toHaveBeenCalledWith("sk-test", "srv-1");
|
||||
expect(networking.storeMCPUserEnvVars).not.toHaveBeenCalled();
|
||||
expect(onClose).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("keeps every stored value when the clear confirmation is cancelled", async () => {
|
||||
const user = setup();
|
||||
const { onSaved, onClose } = renderModal(statusWith([{ name: "API_KEY", description: null, is_set: true }]));
|
||||
|
||||
await fieldAfterOpen(/^API_KEY/);
|
||||
await user.click(screen.getByRole("button", { name: "Clear" }));
|
||||
const confirm = await screen.findByRole("alertdialog", { name: "Clear saved credentials" });
|
||||
await user.click(within(confirm).getByRole("button", { name: "Cancel" }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.queryByRole("alertdialog")).not.toBeInTheDocument();
|
||||
});
|
||||
expect(networking.clearMCPUserEnvVars).not.toHaveBeenCalled();
|
||||
expect(onSaved).not.toHaveBeenCalled();
|
||||
expect(onClose).not.toHaveBeenCalled();
|
||||
expect(screen.getByRole("button", { name: "Clear" })).toBeEnabled();
|
||||
});
|
||||
|
||||
it("drops a pending clear confirmation when the modal is closed and reopened", async () => {
|
||||
const user = setup();
|
||||
const { onClose, setOpen } = renderModal(statusWith([{ name: "API_KEY", description: null, is_set: true }]));
|
||||
|
||||
await fieldAfterOpen(/^API_KEY/);
|
||||
await user.click(screen.getByRole("button", { name: "Clear" }));
|
||||
await screen.findByRole("alertdialog", { name: "Clear saved credentials" });
|
||||
|
||||
await user.click(screen.getByRole("button", { name: "Close", hidden: true }));
|
||||
expect(onClose).toHaveBeenCalledTimes(1);
|
||||
setOpen(false);
|
||||
await waitFor(() => {
|
||||
expect(screen.queryByRole("alertdialog")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
setOpen(true);
|
||||
await fieldAfterOpen(/^API_KEY/);
|
||||
expect(screen.queryByRole("alertdialog")).not.toBeInTheDocument();
|
||||
expect(networking.clearMCPUserEnvVars).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("offers Clear only when a value is stored", async () => {
|
||||
renderModal(statusWith([{ name: "API_KEY", description: null, is_set: false }]));
|
||||
|
||||
await fieldAfterOpen(/^API_KEY/);
|
||||
expect(screen.queryByRole("button", { name: "Clear" })).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("surfaces a clear failure without closing", async () => {
|
||||
const user = setup();
|
||||
vi.mocked(networking.clearMCPUserEnvVars).mockRejectedValue(new Error("boom"));
|
||||
const { onSaved, onClose } = renderModal(statusWith([{ name: "API_KEY", description: null, is_set: true }]));
|
||||
|
||||
await fieldAfterOpen(/^API_KEY/);
|
||||
await user.click(screen.getByRole("button", { name: "Clear" }));
|
||||
const confirm = await screen.findByRole("alertdialog", { name: "Clear saved credentials" });
|
||||
await user.click(within(confirm).getByRole("button", { name: "Clear credentials" }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(networking.clearMCPUserEnvVars).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
expect(onSaved).not.toHaveBeenCalled();
|
||||
expect(onClose).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("surfaces a save failure without closing", async () => {
|
||||
const user = setup();
|
||||
vi.mocked(networking.storeMCPUserEnvVars).mockRejectedValue(new Error("boom"));
|
||||
|
|
|
|||
|
|
@ -1,13 +1,21 @@
|
|||
import React from "react";
|
||||
import { CircleAlert, Info } from "lucide-react";
|
||||
import { useMutation, useQuery } from "@tanstack/react-query";
|
||||
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
|
||||
import { z } from "zod/v4";
|
||||
import { MCPServer, MCPUserEnvVarsStatus, MCPUserEnvVarSpec } from "@/components/mcp_tools/types";
|
||||
import { getMCPUserEnvVars, storeMCPUserEnvVars } from "@/components/networking";
|
||||
import { clearMCPUserEnvVars, getMCPUserEnvVars, storeMCPUserEnvVars } from "@/components/networking";
|
||||
import { toast } from "@/lib/toast";
|
||||
import { FieldGroup } from "@/components/ui/field";
|
||||
import { FormField } from "@/components/shared/form/FormField";
|
||||
import { Alert, AlertTitle } from "@/components/shared/Alert";
|
||||
import {
|
||||
AlertDialog,
|
||||
AlertDialogContent,
|
||||
AlertDialogDescription,
|
||||
AlertDialogFooter,
|
||||
AlertDialogHeader,
|
||||
AlertDialogTitle,
|
||||
} from "@/components/ui/alert-dialog";
|
||||
import { PasswordInput } from "@/components/shared/PasswordInput";
|
||||
import { Badge } from "@/components/ui/badge";
|
||||
import { StatusBadge } from "@/components/shared/table_cells/status_badge";
|
||||
|
|
@ -28,6 +36,7 @@ interface UserEnvVarsFormProps {
|
|||
required: readonly MCPUserEnvVarSpec[];
|
||||
isSaving: boolean;
|
||||
onCancel: () => void;
|
||||
onClear?: () => void;
|
||||
onSubmit: (values: Record<string, string>) => void;
|
||||
}
|
||||
|
||||
|
|
@ -41,7 +50,7 @@ const buildSchema = (required: readonly MCPUserEnvVarSpec[]) =>
|
|||
const emptyValues = (required: readonly MCPUserEnvVarSpec[]): Record<string, string> =>
|
||||
Object.fromEntries(required.map((spec) => [spec.name, ""]));
|
||||
|
||||
const UserEnvVarsForm: React.FC<UserEnvVarsFormProps> = ({ required, isSaving, onCancel, onSubmit }) => {
|
||||
const UserEnvVarsForm: React.FC<UserEnvVarsFormProps> = ({ required, isSaving, onCancel, onClear, onSubmit }) => {
|
||||
const form = useZodForm(buildSchema(required), { defaultValues: emptyValues(required) });
|
||||
|
||||
return (
|
||||
|
|
@ -73,6 +82,11 @@ const UserEnvVarsForm: React.FC<UserEnvVarsFormProps> = ({ required, isSaving, o
|
|||
))}
|
||||
</FieldGroup>
|
||||
<div className="mt-6 flex items-center justify-end gap-2 border-t border-border pt-2">
|
||||
{onClear && (
|
||||
<Button type="button" variant="destructive" className="mr-auto" onClick={onClear} disabled={isSaving}>
|
||||
Clear
|
||||
</Button>
|
||||
)}
|
||||
<Button type="button" variant="outline" onClick={onCancel} disabled={isSaving}>
|
||||
Cancel
|
||||
</Button>
|
||||
|
|
@ -93,12 +107,19 @@ const UserEnvVarsForm: React.FC<UserEnvVarsFormProps> = ({ required, isSaving, o
|
|||
* description as the placeholder.
|
||||
*/
|
||||
const UserEnvVarsModal: React.FC<UserEnvVarsModalProps> = ({ server, open, accessToken, onClose, onSaved }) => {
|
||||
const queryClient = useQueryClient();
|
||||
const [confirmingClear, setConfirmingClear] = React.useState(false);
|
||||
const close = () => {
|
||||
setConfirmingClear(false);
|
||||
onClose();
|
||||
};
|
||||
const queryKey = ["mcpUserEnvVars", server?.server_id];
|
||||
const {
|
||||
data: status,
|
||||
isLoading,
|
||||
isError,
|
||||
} = useQuery<MCPUserEnvVarsStatus>({
|
||||
queryKey: ["mcpUserEnvVars", server?.server_id],
|
||||
queryKey,
|
||||
queryFn: () => getMCPUserEnvVars(accessToken!, server!.server_id),
|
||||
enabled: open && !!server && !!accessToken,
|
||||
});
|
||||
|
|
@ -106,15 +127,29 @@ const UserEnvVarsModal: React.FC<UserEnvVarsModalProps> = ({ server, open, acces
|
|||
const saveMutation = useMutation({
|
||||
mutationFn: (values: Record<string, string>) => storeMCPUserEnvVars(accessToken!, server!.server_id, values),
|
||||
onSuccess: (saved) => {
|
||||
queryClient.setQueryData(queryKey, saved);
|
||||
toast.success("Credentials saved");
|
||||
onSaved?.(saved);
|
||||
onClose();
|
||||
close();
|
||||
},
|
||||
onError: (err) => {
|
||||
toast.fromError(`Failed to save env vars: ${err instanceof Error ? err.message : String(err)}`);
|
||||
},
|
||||
});
|
||||
|
||||
const clearMutation = useMutation({
|
||||
mutationFn: () => clearMCPUserEnvVars(accessToken!, server!.server_id),
|
||||
onSuccess: (cleared) => {
|
||||
queryClient.setQueryData(queryKey, cleared);
|
||||
toast.success("Credentials cleared");
|
||||
onSaved?.(cleared);
|
||||
close();
|
||||
},
|
||||
onError: (err) => {
|
||||
toast.fromError(`Failed to clear env vars: ${err instanceof Error ? err.message : String(err)}`);
|
||||
},
|
||||
});
|
||||
|
||||
const handleSave = (values: Record<string, string>) => {
|
||||
if (!server || !accessToken) return;
|
||||
const trimmed: Record<string, string> = {};
|
||||
|
|
@ -126,10 +161,15 @@ const UserEnvVarsModal: React.FC<UserEnvVarsModalProps> = ({ server, open, acces
|
|||
|
||||
const displayName = server?.server_name || server?.alias || server?.server_id || "MCP Server";
|
||||
const required = status?.required ?? [];
|
||||
const isSaving = saveMutation.isPending;
|
||||
const isSaving = saveMutation.isPending || clearMutation.isPending;
|
||||
const canClear = !!server && !!accessToken && required.some((spec) => spec.is_set);
|
||||
const confirmClear = () => {
|
||||
setConfirmingClear(false);
|
||||
clearMutation.mutate();
|
||||
};
|
||||
|
||||
return (
|
||||
<Dialog open={open} onOpenChange={(opened) => !opened && onClose()}>
|
||||
<Dialog open={open} onOpenChange={(opened) => !opened && close()}>
|
||||
<DialogContent className="max-h-[calc(100dvh-2rem)] overflow-y-auto sm:max-w-[520px]">
|
||||
<DialogHeader>
|
||||
<div className="flex items-center gap-2">
|
||||
|
|
@ -161,10 +201,35 @@ const UserEnvVarsModal: React.FC<UserEnvVarsModalProps> = ({ server, open, acces
|
|||
credentials. Saved values are never shown back; leave an already-set field blank to keep it, or enter a
|
||||
value to set or change it.
|
||||
</span>
|
||||
<UserEnvVarsForm required={required} isSaving={isSaving} onCancel={onClose} onSubmit={handleSave} />
|
||||
<UserEnvVarsForm
|
||||
required={required}
|
||||
isSaving={isSaving}
|
||||
onCancel={close}
|
||||
onClear={canClear ? () => setConfirmingClear(true) : undefined}
|
||||
onSubmit={handleSave}
|
||||
/>
|
||||
</>
|
||||
)}
|
||||
</div>
|
||||
<AlertDialog open={confirmingClear} onOpenChange={(opened) => !opened && setConfirmingClear(false)}>
|
||||
<AlertDialogContent>
|
||||
<AlertDialogHeader>
|
||||
<AlertDialogTitle>Clear saved credentials</AlertDialogTitle>
|
||||
<AlertDialogDescription>
|
||||
This deletes every per-user value you saved for {displayName}. Your next MCP request to this server
|
||||
fails until you set them again.
|
||||
</AlertDialogDescription>
|
||||
</AlertDialogHeader>
|
||||
<AlertDialogFooter>
|
||||
<Button variant="outline" onClick={() => setConfirmingClear(false)}>
|
||||
Cancel
|
||||
</Button>
|
||||
<Button variant="destructive" onClick={confirmClear}>
|
||||
Clear credentials
|
||||
</Button>
|
||||
</AlertDialogFooter>
|
||||
</AlertDialogContent>
|
||||
</AlertDialog>
|
||||
</DialogContent>
|
||||
</Dialog>
|
||||
);
|
||||
|
|
|
|||
|
|
@ -241,6 +241,12 @@ const MCPServers: React.FC<MCPServerProps> = ({ accessToken, userRole, userID, i
|
|||
return map;
|
||||
}, [envVarStatuses]);
|
||||
|
||||
const serversWithUserFields = useMemo(
|
||||
() =>
|
||||
new Set((envVarStatuses ?? []).filter((status) => (status.required ?? []).length > 0).map((s) => s.server_id)),
|
||||
[envVarStatuses],
|
||||
);
|
||||
|
||||
// Deep-link via ?fill_env_vars=<server_id> — the link users follow from the
|
||||
// friendly error the proxy returns when a per-user var is missing. The id is
|
||||
// captured into state above and resolved to a server below; here we only strip
|
||||
|
|
@ -730,6 +736,7 @@ const MCPServers: React.FC<MCPServerProps> = ({ accessToken, userRole, userID, i
|
|||
key={server.server_id}
|
||||
server={server}
|
||||
missingUserFields={missingFieldsByServer[server.server_id]}
|
||||
hasUserFields={serversWithUserFields.has(server.server_id)}
|
||||
isLoadingHealth={isLoadingHealth}
|
||||
isRechecking={recheckingServerIds?.has(server.server_id)}
|
||||
onClick={() => {
|
||||
|
|
|
|||
|
|
@ -7907,6 +7907,10 @@ export const storeMCPUserEnvVars = async (
|
|||
});
|
||||
};
|
||||
|
||||
export const clearMCPUserEnvVars = async (accessToken: string, serverId: string): Promise<MCPUserEnvVarsStatus> => {
|
||||
return apiClient.delete<MCPUserEnvVarsStatus>(`/v1/mcp/server/${serverId}/user-env-vars`, { accessToken });
|
||||
};
|
||||
|
||||
export const listMCPUserEnvVarStatus = async (accessToken: string): Promise<MCPUserEnvVarsStatus[]> => {
|
||||
// Best-effort status badges: a failure here must not break the page, so fall
|
||||
// back to an empty list rather than surfacing the error to the caller.
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue