From f1383f16faa98bc7bbd72bfd061a1deaaa00e96a Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Tue, 4 Aug 2026 14:15:29 -0700 Subject: [PATCH] refactor(ui): route MCP session tokens through the shared storage helper mcpTokenStore was the only OAuth path writing straight to window.sessionStorage; useMcpOAuthFlow, useToolsOAuthFlow, the callback page and the edit-screen UI state all already go through secureStorage. Align it so the OAuth surface has one storage format instead of two. The stored payload also carried a refresh_token that nothing ever read back. All three read sites take access_token only, and nothing reads the mcp-session-token: keys directly, so the field was write-only. Drop it from the store and from the four callers that populated it. The client-forwarded modes (true_passthrough and oauth_delegate) re-authorize rather than refresh, and authorization_code is unaffected because it persists through storeMCPOAuthUserCredential on the backend, which keeps its own refresh token. Entries written before this change decode to null and are treated as absent, which surfaces the normal Authorize prompt; they are session-scoped and expire in an hour. Add two regression tests that decode the stored value before asserting, so neither can pass merely because the payload is no longer plain text. --- .../_components/CreateMCPServer.tsx | 1 - .../_components/mcp_server_edit.tsx | 2 - .../src/hooks/useToolsOAuthFlow.tsx | 1 - .../src/utils/mcpTokenStore.test.ts | 37 +++++++++++++++++++ .../src/utils/mcpTokenStore.ts | 9 ++--- 5 files changed, 41 insertions(+), 9 deletions(-) diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/CreateMCPServer.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/CreateMCPServer.tsx index 0785dd142ff..be41434c151 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/CreateMCPServer.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/CreateMCPServer.tsx @@ -434,7 +434,6 @@ const CreateMCPServer: React.FC = ({ const browserHeldToken = { access_token: oauthTokenResponse.access_token, expires_in: oauthTokenResponse.expires_in, - refresh_token: oauthTokenResponse.refresh_token, token_type: oauthTokenResponse.token_type, }; setToken(response.server_id, browserHeldToken, userID); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_server_edit.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_server_edit.tsx index acecec105eb..1e96a414859 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_server_edit.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_server_edit.tsx @@ -218,7 +218,6 @@ const MCPServerEdit: React.FC = ({ const browserHeldToken = { access_token: token.access_token, expires_in: token.expires_in, - refresh_token: token.refresh_token, token_type: token.token_type, }; setToken(mcpServer.server_id, browserHeldToken, userID); @@ -977,7 +976,6 @@ const MCPServerEdit: React.FC = ({ const browserHeldToken = { access_token: oauthTokenResponse.access_token, expires_in: oauthTokenResponse.expires_in, - refresh_token: oauthTokenResponse.refresh_token, token_type: oauthTokenResponse.token_type, }; setToken(mcpServer.server_id, browserHeldToken, userID); diff --git a/ui/litellm-dashboard/src/hooks/useToolsOAuthFlow.tsx b/ui/litellm-dashboard/src/hooks/useToolsOAuthFlow.tsx index d21282ffcd7..2cfd6e7737c 100644 --- a/ui/litellm-dashboard/src/hooks/useToolsOAuthFlow.tsx +++ b/ui/litellm-dashboard/src/hooks/useToolsOAuthFlow.tsx @@ -211,7 +211,6 @@ export const useToolsOAuthFlow = ({ { access_token: token.access_token, expires_in: token.expires_in, - refresh_token: token.refresh_token, token_type: token.token_type, }, userId, diff --git a/ui/litellm-dashboard/src/utils/mcpTokenStore.test.ts b/ui/litellm-dashboard/src/utils/mcpTokenStore.test.ts index c2ff59a27e0..0d096677f0d 100644 --- a/ui/litellm-dashboard/src/utils/mcpTokenStore.test.ts +++ b/ui/litellm-dashboard/src/utils/mcpTokenStore.test.ts @@ -1,6 +1,20 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { clearAllMcpTokens, getToken, isTokenValid, removeToken, setToken } from "./mcpTokenStore"; +const decodeMaybeBase64 = (raw: string): string => { + try { + return atob(raw); + } catch { + return raw; + } +}; + +const allStoredValues = (): string => + Array.from({ length: sessionStorage.length }, (_, i) => sessionStorage.key(i) ?? "") + .map((key) => sessionStorage.getItem(key) ?? "") + .flatMap((raw) => [raw, decodeMaybeBase64(raw)]) + .join("\n"); + describe("mcpTokenStore", () => { beforeEach(() => { sessionStorage.clear(); @@ -10,6 +24,29 @@ describe("mcpTokenStore", () => { sessionStorage.clear(); }); + it("never persists a refresh token, even when a caller supplies one", () => { + const callerPayload = { + access_token: "access-value", + expires_in: 3600, + refresh_token: "refresh-value-must-not-persist", + token_type: "bearer", + }; + + setToken("server-a", callerPayload, "user-1"); + + expect(getToken("server-a", "user-1")?.access_token).toBe("access-value"); + expect(allStoredValues()).not.toContain("refresh-value-must-not-persist"); + }); + + it("does not write the token payload as readable text", () => { + setToken("server-a", { access_token: "plain-access-value" }, "user-1"); + + const raw = sessionStorage.getItem("mcp-session-token:user-1:server-a"); + expect(raw).not.toBeNull(); + expect(raw).not.toContain("plain-access-value"); + expect(getToken("server-a", "user-1")?.access_token).toBe("plain-access-value"); + }); + it("scopes tokens by user id", () => { setToken("server-a", { access_token: "user1-token" }, "user-1"); setToken("server-a", { access_token: "user2-token" }, "user-2"); diff --git a/ui/litellm-dashboard/src/utils/mcpTokenStore.ts b/ui/litellm-dashboard/src/utils/mcpTokenStore.ts index 1486e0e26ab..c3987a32d38 100644 --- a/ui/litellm-dashboard/src/utils/mcpTokenStore.ts +++ b/ui/litellm-dashboard/src/utils/mcpTokenStore.ts @@ -4,19 +4,19 @@ * session ends (tab/window close). Never written to localStorage. */ +import { getSecureItem, setSecureItem } from "./secureStorage"; + const KEY_PREFIX = "mcp-session-token:"; interface StoredToken { access_token: string; expires_at: number; - refresh_token?: string; token_type: string; } interface TokenInput { access_token: string; expires_in?: number; - refresh_token?: string; token_type?: string; } @@ -33,10 +33,9 @@ export function setToken(serverId: string, data: TokenInput, userId?: string | n access_token: data.access_token, expires_at: Date.now() + (data.expires_in != null ? data.expires_in * 1000 : DEFAULT_TTL_MS), token_type: data.token_type ?? "bearer", - ...(data.refresh_token ? { refresh_token: data.refresh_token } : {}), }; try { - window.sessionStorage.setItem(storageKey(serverId, userId), JSON.stringify(stored)); + setSecureItem(storageKey(serverId, userId), JSON.stringify(stored)); } catch { // Silently ignore storage errors (private browsing, quota exceeded, etc.) } @@ -45,7 +44,7 @@ export function setToken(serverId: string, data: TokenInput, userId?: string | n export function getToken(serverId: string, userId?: string | null): StoredToken | null { if (typeof window === "undefined") return null; try { - const raw = window.sessionStorage.getItem(storageKey(serverId, userId)); + const raw = getSecureItem(storageKey(serverId, userId)); if (!raw) return null; return JSON.parse(raw) as StoredToken; } catch {