From 9fd25b22280f97e1fdc31a5f3adb5bb8f0bd9d27 Mon Sep 17 00:00:00 2001 From: "devin-ai-integration[bot]" <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 23 Sep 2026 11:51:05 -0700 Subject: [PATCH] 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 Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../tests/integrationCritical/expected.json | 8 +- .../mcpUserEnvVars.spec.ts | 416 ++++++++++++++++++ .../integration/mcp/test_mcp_user_env_vars.py | 359 +++++++++++++++ .../_components/MCPServerCard.test.tsx | 47 +- .../mcp-servers/_components/MCPServerCard.tsx | 29 +- .../UserEnvVarsModal.integration.test.tsx | 86 +++- .../_components/UserEnvVarsModal.tsx | 81 +++- .../mcp-servers/_components/mcp_servers.tsx | 7 + .../src/components/networking.tsx | 4 + 9 files changed, 1025 insertions(+), 12 deletions(-) create mode 100644 tests/e2e/ui/tests/integrationCritical/mcpUserEnvVars.spec.ts create mode 100644 tests/integration/mcp/test_mcp_user_env_vars.py diff --git a/tests/e2e/ui/tests/integrationCritical/expected.json b/tests/e2e/ui/tests/integrationCritical/expected.json index 1ce8e539975..0354c9f729c 100644 --- a/tests/e2e/ui/tests/integrationCritical/expected.json +++ b/tests/e2e/ui/tests/integrationCritical/expected.json @@ -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" ] diff --git a/tests/e2e/ui/tests/integrationCritical/mcpUserEnvVars.spec.ts b/tests/e2e/ui/tests/integrationCritical/mcpUserEnvVars.spec.ts new file mode 100644 index 00000000000..3572e520ae9 --- /dev/null +++ b/tests/e2e/ui/tests/integrationCritical/mcpUserEnvVars.spec.ts @@ -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; + remove: () => Promise; +}; + +async function createServer( + request: APIRequestContext, + variables: string[], +): Promise { + 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; + }, + 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 { + 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, +): Promise { + 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(); + } +}); diff --git a/tests/integration/mcp/test_mcp_user_env_vars.py b/tests/integration/mcp/test_mcp_user_env_vars.py new file mode 100644 index 00000000000..d9cecaccadb --- /dev/null +++ b/tests/integration/mcp/test_mcp_user_env_vars.py @@ -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) diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPServerCard.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPServerCard.test.tsx index 8c7fe46a1a2..71c2e107774 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPServerCard.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPServerCard.test.tsx @@ -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(); + 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(); + }); +}); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPServerCard.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPServerCard.tsx index cf3c2863e83..42fb95d5951 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPServerCard.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPServerCard.tsx @@ -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 = ({ server, missingUserFields, + hasUserFields, isLoadingHealth, isRechecking, onClick, @@ -100,6 +102,7 @@ const MCPServerCard: FC = ({ } const handleKeyDown = (e: KeyboardEvent) => { + if (e.target !== e.currentTarget) return; if (e.key === "Enter" || e.key === " ") { e.preventDefault(); onClick(); @@ -256,9 +259,10 @@ const MCPServerCard: FC = ({ )} - {(server.is_byok || needsAttention) && ( + {(server.is_byok || hasUserFields || needsAttention) && (
{server.is_byok && } + {hasUserFields && !needsAttention && } {needsAttention && (
@@ -365,6 +369,29 @@ const HealthChip: FC = ({ ); }; +const UserFieldsRow: FC<{ onUpdate?: () => void }> = ({ onUpdate }) => ( +
+ Per-user credentials +
+ + Set + + {onUpdate && ( + + )} +
+
+); + interface ByokRowProps { connected: boolean; onConnect?: () => void; diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/UserEnvVarsModal.integration.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/UserEnvVarsModal.integration.test.tsx index daf53ed9291..1a564d898e6 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/UserEnvVarsModal.integration.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/UserEnvVarsModal.integration.test.tsx @@ -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")); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/UserEnvVarsModal.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/UserEnvVarsModal.tsx index 5d871664314..b5867c5e7fe 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/UserEnvVarsModal.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/UserEnvVarsModal.tsx @@ -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) => void; } @@ -41,7 +50,7 @@ const buildSchema = (required: readonly MCPUserEnvVarSpec[]) => const emptyValues = (required: readonly MCPUserEnvVarSpec[]): Record => Object.fromEntries(required.map((spec) => [spec.name, ""])); -const UserEnvVarsForm: React.FC = ({ required, isSaving, onCancel, onSubmit }) => { +const UserEnvVarsForm: React.FC = ({ required, isSaving, onCancel, onClear, onSubmit }) => { const form = useZodForm(buildSchema(required), { defaultValues: emptyValues(required) }); return ( @@ -73,6 +82,11 @@ const UserEnvVarsForm: React.FC = ({ required, isSaving, o ))}
+ {onClear && ( + + )} @@ -93,12 +107,19 @@ const UserEnvVarsForm: React.FC = ({ required, isSaving, o * description as the placeholder. */ const UserEnvVarsModal: React.FC = ({ 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({ - queryKey: ["mcpUserEnvVars", server?.server_id], + queryKey, queryFn: () => getMCPUserEnvVars(accessToken!, server!.server_id), enabled: open && !!server && !!accessToken, }); @@ -106,15 +127,29 @@ const UserEnvVarsModal: React.FC = ({ server, open, acces const saveMutation = useMutation({ mutationFn: (values: Record) => 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) => { if (!server || !accessToken) return; const trimmed: Record = {}; @@ -126,10 +161,15 @@ const UserEnvVarsModal: React.FC = ({ 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 ( - !opened && onClose()}> + !opened && close()}>
@@ -161,10 +201,35 @@ const UserEnvVarsModal: React.FC = ({ 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. - + setConfirmingClear(true) : undefined} + onSubmit={handleSave} + /> )}
+ !opened && setConfirmingClear(false)}> + + + Clear saved credentials + + This deletes every per-user value you saved for {displayName}. Your next MCP request to this server + fails until you set them again. + + + + + + + + ); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_servers.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_servers.tsx index 818b5150650..738409e28c2 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_servers.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_servers.tsx @@ -241,6 +241,12 @@ const MCPServers: React.FC = ({ 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= — 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 = ({ 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={() => { diff --git a/ui/litellm-dashboard/src/components/networking.tsx b/ui/litellm-dashboard/src/components/networking.tsx index 3f674ea3328..c2f4fa80634 100644 --- a/ui/litellm-dashboard/src/components/networking.tsx +++ b/ui/litellm-dashboard/src/components/networking.tsx @@ -7907,6 +7907,10 @@ export const storeMCPUserEnvVars = async ( }); }; +export const clearMCPUserEnvVars = async (accessToken: string, serverId: string): Promise => { + return apiClient.delete(`/v1/mcp/server/${serverId}/user-env-vars`, { accessToken }); +}; + export const listMCPUserEnvVarStatus = async (accessToken: string): Promise => { // 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.