fix(auth): key in-flight PKCE verifiers by flow, not server URL

clients/tui/__tests__/App.test.tsx

@@ -108,7 +108,11 @@ const h = vi.hoisted(() => {
   // Captured options from the most recent callbackServer.start(), so a test can
   // drive the onCallback / onError handlers the OAuth flows register.
   interface CallbackOpts {
-    onCallback: (p: { code: string; iss?: string }) => Promise<void> | void;
+    onCallback: (p: {
+      code: string;
+      iss?: string;
+      state?: string;
+    }) => Promise<void> | void;
     onError: (p: { error?: string; error_description?: string }) => void;
   }
   const cb: { opts: CallbackOpts | null } = { opts: null };
@@ -1551,11 +1555,18 @@ describe("App (OAuth flows)", () => {
     const r = await mount(oneHttp());
     await press(r, ["c"]);
     await waitUntil(() => h.cb.opts !== null);
-    await h.cb.opts!.onCallback({ code: "abc", iss: "https://as.example" });
+    // #2726: the redirect's `state` selects the flow's PKCE verifier.
+    const state = "d".repeat(64);
+    await h.cb.opts!.onCallback({
+      code: "abc",
+      iss: "https://as.example",
+      state,
+    });
     await tick();
     expect(h.clientSpies.completeOAuthFlow).toHaveBeenCalledWith(
       "abc",
       "https://as.example",
+      state,
     );
   });
 

clients/web/src/hooks/useOAuthRecovery.ts

@@ -1381,6 +1381,8 @@ export function useOAuthRecovery({
         await client.resumeAfterOAuth(params.code, {
           remoteSessionId: resumeSnapshot?.remoteSessionId,
           iss: params.iss,
+          // #2726: selects this flow's PKCE verifier from the tab's store.
+          state: stateParam ?? undefined,
         });
       } catch (err) {
         connectStartRef.current = undefined;

clients/web/src/lib/environmentFactory.ts

@@ -6,6 +6,7 @@ import {
 } from "@inspector/core/mcp/remote/index.js";
 import { BrowserNavigation } from "@inspector/core/auth/browser/index.js";
 import type { RedirectUrlProvider } from "@inspector/core/auth/index.js";
+import { createWebStoragePkceFlowStore } from "@inspector/core/auth/pkce-flow-store.js";
 import { getWebRemoteOAuthStorage } from "./remoteOAuthStorage.js";
 
 export interface WebEnvironmentResult {
@@ -20,6 +21,10 @@ export interface WebEnvironmentResult {
  *     dev-backend wires this in `/api/*`).
  *   - OAuth storage uses `RemoteOAuthStorage` (shared `oauth.json` via
  *     `/api/storage/oauth`); navigation uses `BrowserNavigation`.
+ *   - In-flight PKCE verifiers stay in this tab's `sessionStorage`, keyed by
+ *     the flow's OAuth `state`, so overlapping flows for one server — two tabs,
+ *     or two people on one deployed Inspector — cannot overwrite each other's
+ *     in the shared store (#2726).
  *
  * Returns both the assembled environment and the logger so callers can share
  * the same pino instance for any direct logging they need to do, instead of
@@ -69,6 +74,7 @@ export function createWebEnvironment(
       storage: getWebRemoteOAuthStorage(authToken),
       navigation: new BrowserNavigation(undefined, onBeforeOAuthRedirect),
       redirectUrlProvider,
+      flowStore: createWebStoragePkceFlowStore(),
     },
   };
 

clients/web/src/test/core/auth/pkce-flow-store.test.ts

@@ -0,0 +1,102 @@
+import { describe, it, expect, afterEach, vi } from "vitest";
+import {
+  createWebStoragePkceFlowStore,
+  PKCE_FLOW_STORAGE_KEY_PREFIX,
+  type PkceFlowStorageArea,
+} from "@inspector/core/auth/pkce-flow-store.js";
+
+const SERVER = "https://mcp.example.com";
+const FLOW = "f".repeat(64);
+
+function makeArea(): PkceFlowStorageArea & { items: Map<string, string> } {
+  const items = new Map<string, string>();
+  return {
+    items,
+    getItem: (key) => items.get(key) ?? null,
+    setItem: (key, value) => void items.set(key, value),
+    removeItem: (key) => void items.delete(key),
+  };
+}
+
+describe("createWebStoragePkceFlowStore (#2726)", () => {
+  afterEach(() => {
+    vi.unstubAllGlobals();
+  });
+
+  it("round-trips a verifier under a per-flow key", async () => {
+    const area = makeArea();
+    const store = createWebStoragePkceFlowStore(() => area);
+
+    await store.saveCodeVerifier(FLOW, SERVER, "cv");
+
+    expect([...area.items.keys()]).toEqual([
+      `${PKCE_FLOW_STORAGE_KEY_PREFIX}${FLOW}`,
+    ]);
+    expect(await store.getCodeVerifier(FLOW, SERVER)).toBe("cv");
+  });
+
+  it("keeps flows for the same server apart", async () => {
+    const otherTab = createWebStoragePkceFlowStore(() => makeArea());
+    const area = makeArea();
+    const shared = createWebStoragePkceFlowStore(() => area);
+    const other = "e".repeat(64);
+
+    await shared.saveCodeVerifier(FLOW, SERVER, "cv-1");
+    await shared.saveCodeVerifier(other, SERVER, "cv-2");
+
+    expect(await shared.getCodeVerifier(FLOW, SERVER)).toBe("cv-1");
+    expect(await shared.getCodeVerifier(other, SERVER)).toBe("cv-2");
+    // A separate storage area (another tab) sees neither.
+    expect(await otherTab.getCodeVerifier(FLOW, SERVER)).toBeUndefined();
+  });
+
+  it("does not answer for a different server than the flow was started for", async () => {
+    const area = makeArea();
+    const store = createWebStoragePkceFlowStore(() => area);
+    await store.saveCodeVerifier(FLOW, SERVER, "cv");
+
+    // Same area, so only the server binding can make this read miss.
+    expect(await store.getCodeVerifier(FLOW, SERVER)).toBe("cv");
+    expect(
+      await store.getCodeVerifier(FLOW, "https://other.example.com"),
+    ).toBeUndefined();
+  });
+
+  it("clears a flow's verifier", async () => {
+    const area = makeArea();
+    const store = createWebStoragePkceFlowStore(() => area);
+    await store.saveCodeVerifier(FLOW, SERVER, "cv");
+
+    await store.clearCodeVerifier(FLOW);
+
+    expect(area.items.size).toBe(0);
+    expect(await store.getCodeVerifier(FLOW, SERVER)).toBeUndefined();
+  });
+
+  it.each([
+    ["not JSON", "{"],
+    ["not an object", "42"],
+    ["null", "null"],
+    ["missing the verifier", JSON.stringify({ serverUrl: SERVER })],
+    [
+      "a non-string verifier",
+      JSON.stringify({ serverUrl: SERVER, codeVerifier: 1 }),
+    ],
+  ])("treats a corrupt entry (%s) as absent", async (_label, raw) => {
+    const area = makeArea();
+    area.items.set(`${PKCE_FLOW_STORAGE_KEY_PREFIX}${FLOW}`, raw);
+    const store = createWebStoragePkceFlowStore(() => area);
+
+    expect(await store.getCodeVerifier(FLOW, SERVER)).toBeUndefined();
+  });
+
+  it("defaults to sessionStorage", async () => {
+    const area = makeArea();
+    vi.stubGlobal("sessionStorage", area);
+    const store = createWebStoragePkceFlowStore();
+
+    await store.saveCodeVerifier(FLOW, SERVER, "cv");
+
+    expect(area.items.has(`${PKCE_FLOW_STORAGE_KEY_PREFIX}${FLOW}`)).toBe(true);
+  });
+});

clients/web/src/test/core/auth/providers.test.ts

@@ -11,6 +11,10 @@ import { OAuthStorageBase } from "@inspector/core/auth/oauth-storage.js";
 import { OAuthMemoryStore } from "@inspector/core/auth/store.js";
 import type { OAuthPersistBackend } from "@inspector/core/auth/oauth-persist.js";
 import { ensureCimdClientRegistration } from "@inspector/core/auth/cimd.js";
+import {
+  createWebStoragePkceFlowStore,
+  type PkceFlowStore,
+} from "@inspector/core/auth/pkce-flow-store.js";
 import {
   BrowserNavigation,
   BrowserOAuthClientProvider,
@@ -1108,5 +1112,118 @@ describe("OAuthNavigation", () => {
         expect(storage.clearDiscoveryState).toHaveBeenCalledWith(SERVER);
       });
     });
+
+    describe("per-flow PKCE verifier (#2726)", () => {
+      function makeFlowStore(): PkceFlowStore {
+        const store = new Map<string, string>();
+        return createWebStoragePkceFlowStore(() => ({
+          getItem: (key) => store.get(key) ?? null,
+          setItem: (key, value) => void store.set(key, value),
+          removeItem: (key) => void store.delete(key),
+        }));
+      }
+
+      it("keys the verifier by the state it generated, not the shared slot", async () => {
+        const storage = makeStorage();
+        const flowStore = makeFlowStore();
+        const provider = makeProvider(storage, vi.fn(), { flowStore });
+
+        const state = await provider.state();
+        await provider.saveCodeVerifier("cv-a");
+
+        expect(storage.saveCodeVerifier).not.toHaveBeenCalled();
+        expect(await flowStore.getCodeVerifier(state, SERVER)).toBe("cv-a");
+        expect(await provider.codeVerifier()).toBe("cv-a");
+      });
+
+      it("gives two overlapping flows for one server their own verifiers", async () => {
+        const storage = makeStorage();
+        const flowStore = makeFlowStore();
+        const providerA = makeProvider(storage, vi.fn(), { flowStore });
+        const providerB = makeProvider(storage, vi.fn(), { flowStore });
+
+        const stateA = await providerA.state();
+        await providerA.saveCodeVerifier("cv-a");
+        const stateB = await providerB.state();
+        await providerB.saveCodeVerifier("cv-b");
+
+        // The callback legs run on fresh providers, as after a page reload.
+        const callbackB = makeProvider(storage, vi.fn(), { flowStore });
+        callbackB.resumeAuthorizationFlow(stateB);
+        expect(await callbackB.codeVerifier()).toBe("cv-b");
+        await callbackB.clearAuthorizationFlow();
+
+        const callbackA = makeProvider(storage, vi.fn(), { flowStore });
+        callbackA.resumeAuthorizationFlow(stateA);
+        expect(await callbackA.codeVerifier()).toBe("cv-a");
+      });
+
+      it("fails clearly for a state with no stored verifier instead of using another flow's", async () => {
+        const storage = makeStorage();
+        vi.mocked(storage.getCodeVerifier).mockResolvedValue("someone-elses");
+        const flowStore = makeFlowStore();
+        const other = makeProvider(storage, vi.fn(), { flowStore });
+        await other.state();
+        await other.saveCodeVerifier("cv-other");
+
+        const provider = makeProvider(storage, vi.fn(), { flowStore });
+        provider.resumeAuthorizationFlow("a".repeat(64));
+
+        await expect(provider.codeVerifier()).rejects.toThrow(
+          /No code verifier saved for this authorization flow/,
+        );
+        expect(storage.getCodeVerifier).not.toHaveBeenCalled();
+      });
+
+      it("binds no flow for a state that is not one of ours", async () => {
+        const storage = makeStorage();
+        const provider = makeProvider(storage, vi.fn(), {
+          flowStore: makeFlowStore(),
+        });
+
+        provider.resumeAuthorizationFlow("not-our-state");
+
+        await expect(provider.codeVerifier()).rejects.toThrow(
+          /No code verifier saved for this authorization flow/,
+        );
+      });
+
+      it("invalidateCredentials('verifier') clears only the bound flow", async () => {
+        const storage = makeStorage();
+        const flowStore = makeFlowStore();
+        const provider = makeProvider(storage, vi.fn(), { flowStore });
+        const state = await provider.state();
+        await provider.saveCodeVerifier("cv");
+
+        await provider.invalidateCredentials("verifier");
+
+        expect(await flowStore.getCodeVerifier(state, SERVER)).toBeUndefined();
+        expect(storage.clearCodeVerifier).not.toHaveBeenCalled();
+      });
+
+      it("clearAuthorizationFlow() is a no-op without a flow store or a bound flow", async () => {
+        const storage = makeStorage();
+        await makeProvider(storage).clearAuthorizationFlow();
+
+        const flowStore = makeFlowStore();
+        const clear = vi.spyOn(flowStore, "clearCodeVerifier");
+        await makeProvider(storage, vi.fn(), {
+          flowStore,
+        }).clearAuthorizationFlow();
+
+        expect(clear).not.toHaveBeenCalled();
+        expect(storage.clearCodeVerifier).not.toHaveBeenCalled();
+      });
+
+      it("keeps the shared slot when no flow store is configured", async () => {
+        const storage = makeStorage();
+        const provider = makeProvider(storage);
+
+        await provider.state();
+        await provider.saveCodeVerifier("cv");
+
+        expect(storage.saveCodeVerifier).toHaveBeenCalledWith(SERVER, "cv");
+      });
+    });
   });
 });

clients/web/src/test/core/auth/runner-interactive-oauth.test.ts

@@ -58,6 +58,8 @@ function createMockCallbackServer(handlers: {
 }
 
 const CALLBACK_ISS = "https://as.example";
+/** The OAuth `state` the redirect echoes; it selects the flow's verifier (#2726). */
+const CALLBACK_STATE = "c".repeat(64);
 
 /**
  * Drive the loopback callback the way a browser redirect would, including the
@@ -71,7 +73,7 @@ async function simulateCallback(
   if (!handlers.onCallback) {
     throw new Error("onCallback not registered");
   }
-  await handlers.onCallback({ code, iss });
+  await handlers.onCallback({ code, iss, state: CALLBACK_STATE });
 }
 
 describe("runRunnerInteractiveOAuth", () => {
@@ -155,6 +157,7 @@ describe("runRunnerInteractiveOAuth", () => {
     expect(client.completeOAuthFlow).toHaveBeenCalledWith(
       "auth-code-123",
       CALLBACK_ISS,
+      CALLBACK_STATE,
     );
     expect(redirectUrlProvider.redirectUrl).toBe(
       "http://127.0.0.1:6276/oauth/callback",
@@ -192,6 +195,7 @@ describe("runRunnerInteractiveOAuth", () => {
     expect(client.completeOAuthFlow).toHaveBeenCalledWith(
       "step-up-code",
       CALLBACK_ISS,
+      CALLBACK_STATE,
     );
   });