diff --git a/clients/web/src/test/core/auth/oauth-persist-browser-safe.test.ts b/clients/web/src/test/core/auth/oauth-persist-browser-safe.test.ts new file mode 100644 index 00000000..b689adaa --- /dev/null +++ b/clients/web/src/test/core/auth/oauth-persist-browser-safe.test.ts @@ -0,0 +1,47 @@ +import { describe, it, expect } from "vitest"; +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import { dirname, resolve } from "node:path"; + +/** + * Regression guard for the browser bundle. `core/auth/oauth-persist.ts` is + * imported by browser code (App.tsx -> RemoteOAuthStorage -> oauth-persist), + * so it must stay Node-free. It previously imported `store-io.js`, which pulls + * `node:fs` + `atomically` (-> `stubborn-fs` -> `node:process.getuid`) into the + * browser bundle and blanked the app at runtime. The Node-only file backend + * now lives in `core/auth/node/oauth-persist-file.ts`; the isomorphic module + * must only import the Node-free `store-serialize` helpers. + */ + +const here = dirname(fileURLToPath(import.meta.url)); +const coreAuth = resolve(here, "../../../../../../core/auth"); + +function importSpecifiers(source: string): string[] { + const specs: string[] = []; + const re = /(?:import|export)[^;]*?from\s*["']([^"']+)["']/g; + let m: RegExpExecArray | null; + while ((m = re.exec(source)) !== null) { + specs.push(m[1]); + } + return specs; +} + +describe("oauth-persist.ts stays browser-safe", () => { + const source = readFileSync(resolve(coreAuth, "oauth-persist.ts"), "utf-8"); + const specs = importSpecifiers(source); + + it("does not import the Node-only store-io module", () => { + expect(specs).not.toContain("../storage/store-io.js"); + }); + + it("does not import node: builtins or atomically", () => { + const forbidden = specs.filter( + (s) => s.startsWith("node:") || s === "atomically", + ); + expect(forbidden).toEqual([]); + }); + + it("gets its (de)serializers from the Node-free store-serialize module", () => { + expect(specs).toContain("../storage/store-serialize.js"); + }); +}); diff --git a/clients/web/src/test/core/auth/oauth-persist.test.ts b/clients/web/src/test/core/auth/oauth-persist.test.ts index 62f8eadd..ac0c42ab 100644 --- a/clients/web/src/test/core/auth/oauth-persist.test.ts +++ b/clients/web/src/test/core/auth/oauth-persist.test.ts @@ -1,8 +1,21 @@ -import { describe, it, expect } from "vitest"; +import { describe, it, expect, vi } from "vitest"; import { parseOAuthPersistBlob, serializeOAuthPersistBlob, + createRemoteOAuthPersistBackend, + createSessionOAuthPersistBackend, + OAUTH_PERSIST_STORAGE_KEY, } from "@inspector/core/auth/oauth-persist.js"; +import type { OAuthPersistSnapshot } from "@inspector/core/auth/oauth-persist.js"; + +const SNAPSHOT: OAuthPersistSnapshot = { + servers: { "http://s": { scope: "read" } }, + idpSessions: {}, +}; + +function jsonResponse(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { status }); +} describe("parseOAuthPersistBlob", () => { it("returns null for empty input", () => { @@ -69,3 +82,141 @@ describe("serializeOAuthPersistBlob", () => { expect(raw).not.toMatch(/"state"\s*:/); }); }); + +describe("createRemoteOAuthPersistBackend", () => { + const baseUrl = "http://remote.example/"; + const storeId = "oauth"; + const url = "http://remote.example/api/storage/oauth"; + + it("read() returns the parsed snapshot and sends the auth header", async () => { + const fetchFn = vi.fn(async () => jsonResponse(SNAPSHOT)); + const backend = createRemoteOAuthPersistBackend({ + baseUrl, + storeId, + authToken: "tok", + fetchFn: fetchFn as unknown as typeof fetch, + }); + expect(await backend.read()).toEqual(SNAPSHOT); + expect(fetchFn).toHaveBeenCalledWith(url, { + method: "GET", + headers: { "x-mcp-remote-auth": "Bearer tok" }, + }); + }); + + it("read() returns null for the empty-object missing-file response", async () => { + const backend = createRemoteOAuthPersistBackend({ + baseUrl, + storeId, + fetchFn: (async () => jsonResponse({})) as unknown as typeof fetch, + }); + expect(await backend.read()).toBeNull(); + }); + + it("read() returns null on 404 and throws on other errors", async () => { + const notFound = createRemoteOAuthPersistBackend({ + baseUrl, + storeId, + fetchFn: (async () => + new Response("", { status: 404 })) as unknown as typeof fetch, + }); + expect(await notFound.read()).toBeNull(); + + const failing = createRemoteOAuthPersistBackend({ + baseUrl, + storeId, + fetchFn: (async () => + new Response("", { status: 500 })) as unknown as typeof fetch, + }); + await expect(failing.read()).rejects.toThrow(/Failed to read store: 500/); + }); + + it("write() POSTs the serialized snapshot and throws on failure", async () => { + const ok = vi.fn(async () => new Response("", { status: 200 })); + const backend = createRemoteOAuthPersistBackend({ + baseUrl, + storeId, + fetchFn: ok as unknown as typeof fetch, + }); + await backend.write(SNAPSHOT); + expect(ok).toHaveBeenCalledWith( + url, + expect.objectContaining({ method: "POST" }), + ); + const body = ok.mock.calls[0][1].body; + expect(JSON.parse(body)).toEqual(SNAPSHOT); + + const failing = createRemoteOAuthPersistBackend({ + baseUrl, + storeId, + fetchFn: (async () => + new Response("", { status: 500 })) as unknown as typeof fetch, + }); + await expect(failing.write(SNAPSHOT)).rejects.toThrow( + /Failed to write store: 500/, + ); + }); + + it("remove() DELETEs, tolerates 404, and throws on other errors", async () => { + const ok = createRemoteOAuthPersistBackend({ + baseUrl, + storeId, + authToken: "tok", + fetchFn: (async () => + new Response("", { status: 200 })) as unknown as typeof fetch, + }); + await expect(ok.remove!()).resolves.toBeUndefined(); + + const gone = createRemoteOAuthPersistBackend({ + baseUrl, + storeId, + fetchFn: (async () => + new Response("", { status: 404 })) as unknown as typeof fetch, + }); + await expect(gone.remove!()).resolves.toBeUndefined(); + + const failing = createRemoteOAuthPersistBackend({ + baseUrl, + storeId, + fetchFn: (async () => + new Response("", { status: 500 })) as unknown as typeof fetch, + }); + await expect(failing.remove!()).rejects.toThrow( + /Failed to delete store: 500/, + ); + }); +}); + +describe("createSessionOAuthPersistBackend", () => { + function fakeStorage() { + const map = new Map(); + return { + getItem: (k: string) => map.get(k) ?? null, + setItem: (k: string, v: string) => void map.set(k, v), + removeItem: (k: string) => void map.delete(k), + map, + } as unknown as Storage & { map: Map }; + } + + it("round-trips a snapshot through the default storage key", async () => { + const storage = fakeStorage(); + const backend = createSessionOAuthPersistBackend({ + getStorage: () => storage, + }); + expect(await backend.read()).toBeNull(); + await backend.write(SNAPSHOT); + expect(storage.map.has(OAUTH_PERSIST_STORAGE_KEY)).toBe(true); + expect(await backend.read()).toEqual(SNAPSHOT); + await backend.remove!(); + expect(await backend.read()).toBeNull(); + }); + + it("honors a custom storage key", async () => { + const storage = fakeStorage(); + const backend = createSessionOAuthPersistBackend({ + storageKey: "custom-key", + getStorage: () => storage, + }); + await backend.write(SNAPSHOT); + expect(storage.map.has("custom-key")).toBe(true); + }); +}); diff --git a/clients/web/src/test/core/auth/store.test.ts b/clients/web/src/test/core/auth/store.test.ts new file mode 100644 index 00000000..131a5ab8 --- /dev/null +++ b/clients/web/src/test/core/auth/store.test.ts @@ -0,0 +1,84 @@ +import { describe, it, expect } from "vitest"; +import { OAuthMemoryStore } from "@inspector/core/auth/store.js"; + +describe("OAuthMemoryStore", () => { + it("returns empty defaults for unknown server / issuer", () => { + const store = new OAuthMemoryStore(); + expect(store.getState().getServerState("http://x")).toEqual({}); + expect(store.getState().getIdpSession("http://idp")).toEqual({}); + }); + + it("merges partial updates immutably via setServerState / setIdpSession", () => { + const store = new OAuthMemoryStore(); + const s = store.getState(); + s.setServerState("http://x", { scope: "read" }); + s.setServerState("http://x", { codeVerifier: "v" }); + expect(store.getState().getServerState("http://x")).toEqual({ + scope: "read", + codeVerifier: "v", + }); + s.setIdpSession("http://idp", { idToken: "t" }); + expect(store.getState().getIdpSession("http://idp")).toEqual({ + idToken: "t", + }); + }); + + it("clears a single server and a single idp session", () => { + const store = new OAuthMemoryStore({ + servers: { "http://x": { scope: "read" } }, + idpSessions: { "http://idp": { idToken: "t" } }, + }); + store.getState().clearServerState("http://x"); + store.getState().clearIdpSession("http://idp"); + expect(store.getState().getServerState("http://x")).toEqual({}); + expect(store.getState().getIdpSession("http://idp")).toEqual({}); + }); + + it("clears only enterprise-managed servers", () => { + const store = new OAuthMemoryStore({ + servers: { + "http://managed": { enterpriseManaged: true }, + "http://plain": { scope: "read" }, + }, + idpSessions: {}, + }); + store.getState().clearEnterpriseManagedResourceServers(); + expect(store.getState().getServerState("http://managed")).toEqual({}); + expect(store.getState().getServerState("http://plain")).toEqual({ + scope: "read", + }); + }); + + it("snapshot() returns a shallow copy of current state", () => { + const store = new OAuthMemoryStore({ + servers: { "http://x": { scope: "read" } }, + idpSessions: {}, + }); + const snap = store.snapshot(); + expect(snap).toEqual({ + servers: { "http://x": { scope: "read" } }, + idpSessions: {}, + }); + // Mutating the store afterward must not change the earlier snapshot. + store.getState().setServerState("http://y", { scope: "write" }); + expect(snap.servers["http://y"]).toBeUndefined(); + }); + + it("replace() tolerates a snapshot missing servers or idpSessions", () => { + // Exercises both `?? {}` fallbacks in replace(). + const noServers = new OAuthMemoryStore({ + idpSessions: { "http://idp": { idToken: "t" } }, + } as never); + expect(noServers.snapshot()).toEqual({ + servers: {}, + idpSessions: { "http://idp": { idToken: "t" } }, + }); + const noSessions = new OAuthMemoryStore({ + servers: { "http://x": { scope: "read" } }, + } as never); + expect(noSessions.snapshot()).toEqual({ + servers: { "http://x": { scope: "read" } }, + idpSessions: {}, + }); + }); +}); diff --git a/clients/web/src/test/integration/auth/node/storage.test.ts b/clients/web/src/test/integration/auth/node/storage.test.ts index 9eea463d..0bb328ad 100644 --- a/clients/web/src/test/integration/auth/node/storage.test.ts +++ b/clients/web/src/test/integration/auth/node/storage.test.ts @@ -579,6 +579,18 @@ describe("NodeOAuthStorage with custom storagePath", () => { resetNodeOAuthStorageCache(); }); + it("clearAllOAuthClientState is a no-op when no store file exists", async () => { + // Exercises the null-snapshot branch: read() returns null, so + // `snapshot?.servers ?? {}` yields no urls and nothing is cleared. + const filePath = getStateFilePath(); + resetNodeOAuthStorageCache(); + await fs.unlink(filePath).catch(() => {}); + + await expect(clearAllOAuthClientState()).resolves.toBeUndefined(); + + resetNodeOAuthStorageCache(); + }); + it("should isolate state from default store", async () => { const customPath = path.join( os.tmpdir(), diff --git a/clients/web/src/test/integration/storage/adapters.test.ts b/clients/web/src/test/integration/storage/adapters.test.ts index a48663a9..998e60e7 100644 --- a/clients/web/src/test/integration/storage/adapters.test.ts +++ b/clients/web/src/test/integration/storage/adapters.test.ts @@ -12,7 +12,7 @@ import type { ServerType } from "@hono/node-server"; import { NodeOAuthStorage } from "@inspector/core/auth/node/storage-node.js"; import { RemoteOAuthStorage } from "@inspector/core/auth/remote/storage-remote.js"; import { OAuthMemoryStore } from "@inspector/core/auth/store.js"; -import { createFileOAuthPersistBackend } from "@inspector/core/auth/oauth-persist.js"; +import { createFileOAuthPersistBackend } from "@inspector/core/auth/node/oauth-persist-file.js"; import { createRemoteApp } from "@inspector/core/mcp/remote/node/server.js"; import { writeStoreFile, diff --git a/core/auth/node/oauth-persist-file.ts b/core/auth/node/oauth-persist-file.ts new file mode 100644 index 00000000..dc0714fb --- /dev/null +++ b/core/auth/node/oauth-persist-file.ts @@ -0,0 +1,42 @@ +/** + * Node-only file backend for OAuth persistence. Kept out of the isomorphic + * `core/auth/oauth-persist.ts` because it imports `store-io.js` (which pulls + * `node:fs`/`atomically`); the browser must never load that. Node consumers + * (e.g. `NodeOAuthStorage`) import the file backend from here, while the + * shared blob (de)serialization and browser/remote backends stay isomorphic. + */ + +import { + readStoreFile, + writeStoreFile, + deleteStoreFile, +} from "../../storage/store-io.js"; +import { + parseOAuthPersistBlob, + serializeOAuthPersistBlob, + type OAuthPersistBackend, +} from "../oauth-persist.js"; + +export interface FileOAuthPersistBackendOptions { + filePath: string; +} + +export function createFileOAuthPersistBackend( + options: FileOAuthPersistBackendOptions, +): OAuthPersistBackend { + return { + async read() { + const raw = await readStoreFile(options.filePath); + return parseOAuthPersistBlob(raw); + }, + async write(snapshot) { + await writeStoreFile( + options.filePath, + serializeOAuthPersistBlob(snapshot), + ); + }, + async remove() { + await deleteStoreFile(options.filePath); + }, + }; +} diff --git a/core/auth/node/storage-node.ts b/core/auth/node/storage-node.ts index 49bb1f34..e46e20a2 100644 --- a/core/auth/node/storage-node.ts +++ b/core/auth/node/storage-node.ts @@ -1,6 +1,6 @@ import { OAuthStorageBase } from "../oauth-storage.js"; import { OAuthMemoryStore } from "../store.js"; -import { createFileOAuthPersistBackend } from "../oauth-persist.js"; +import { createFileOAuthPersistBackend } from "./oauth-persist-file.js"; import { getDefaultStorageDir, getStoreFilePath, diff --git a/core/auth/oauth-persist.ts b/core/auth/oauth-persist.ts index 66d36c7d..133dc419 100644 --- a/core/auth/oauth-persist.ts +++ b/core/auth/oauth-persist.ts @@ -1,17 +1,15 @@ /** - * OAuth persistence format and backends (file, remote HTTP, sessionStorage). - * Writes plain JSON `{ servers, idpSessions }`. On read, accepts legacy - * persist envelopes `{ state: { servers, idpSessions }, version }` and - * promotes the inner payload. + * OAuth persistence format and isomorphic backends (remote HTTP, + * sessionStorage). Writes plain JSON `{ servers, idpSessions }`. On read, + * accepts legacy persist envelopes `{ state: { servers, idpSessions }, + * version }` and promotes the inner payload. + * + * This module must stay browser-safe: it imports only the Node-free + * `store-serialize` helpers, never `store-io` (which pulls `node:fs`). The + * Node-only file backend lives in `./node/oauth-persist-file.ts`. */ -import { - readStoreFile, - writeStoreFile, - deleteStoreFile, - serializeStore, - parseStore, -} from "../storage/store-io.js"; +import { serializeStore, parseStore } from "../storage/store-serialize.js"; import type { IdpSessionState } from "./storage.js"; import type { ServerOAuthState } from "./store.js"; @@ -76,30 +74,6 @@ export interface OAuthPersistBackend { remove?(): Promise; } -export interface FileOAuthPersistBackendOptions { - filePath: string; -} - -export function createFileOAuthPersistBackend( - options: FileOAuthPersistBackendOptions, -): OAuthPersistBackend { - return { - async read() { - const raw = await readStoreFile(options.filePath); - return parseOAuthPersistBlob(raw); - }, - async write(snapshot) { - await writeStoreFile( - options.filePath, - serializeOAuthPersistBlob(snapshot), - ); - }, - async remove() { - await deleteStoreFile(options.filePath); - }, - }; -} - export interface RemoteOAuthPersistBackendOptions { baseUrl: string; storeId: string; diff --git a/core/storage/store-io.ts b/core/storage/store-io.ts index 5e0273b4..db04fe2e 100644 --- a/core/storage/store-io.ts +++ b/core/storage/store-io.ts @@ -12,6 +12,10 @@ import { readFile, writeFile } from "atomically"; // isomorphic code can reuse it without pulling Node deps into the browser. export { validateStoreId } from "./store-id.js"; +// Likewise re-export the pure JSON (de)serializers from the Node-free +// `store-serialize` module so existing `store-io` importers are unaffected. +export { serializeStore, parseStore } from "./store-serialize.js"; + /** * Default storage directory (~/.mcp-inspector/storage or %USERPROFILE%\.mcp-inspector\storage on Windows). */ @@ -120,17 +124,3 @@ export async function deleteStoreFile(filePath: string): Promise { } } } - -/** - * Serialize store data to JSON string (consistent format for server writes). - */ -export function serializeStore(data: unknown): string { - return JSON.stringify(data, null, 2); -} - -/** - * Parse store JSON string. Use after readStoreFile when returning parsed object. - */ -export function parseStore(raw: string): unknown { - return JSON.parse(raw); -} diff --git a/core/storage/store-serialize.ts b/core/storage/store-serialize.ts new file mode 100644 index 00000000..83c435be --- /dev/null +++ b/core/storage/store-serialize.ts @@ -0,0 +1,16 @@ +/** + * Pure, Node-free JSON (de)serialization for store files. Split from + * `store-io.ts` (which imports `node:fs`/`node:path`/`atomically`) so + * isomorphic code — e.g. the browser OAuth persistence flow in + * `core/auth/oauth-persist.ts` — can (de)serialize store blobs without + * pulling Node deps into the browser bundle. `store-io.ts` re-exports these + * so existing importers are unaffected. + */ + +export function serializeStore(data: unknown): string { + return JSON.stringify(data, null, 2); +} + +export function parseStore(raw: string): unknown { + return JSON.parse(raw); +}