feat(web): migrate OAuth store to shared /store API (RemoteOAuthStorage parity) (closes #1548)

Migrate the web client's OAuth storage from BrowserOAuthStorage
(sessionStorage) to RemoteOAuthStorage, which POSTs through the backend's
/api/storage/oauth route to ~/.mcp-inspector/storage/oauth.json (mode 0600).
A credential obtained in the browser is now the same blob the TUI/CLI read
on the same host, giving web ⇄ TUI ⇄ CLI parity (Wave 1 of #1579).

Core (shared with TUI/CLI):
- OAuthStorageBase now drives a single async hydration (`ready()`,
  `getHydrationError()`); every post-redirect read (getClientInformation,
  getTokens, getCodeVerifier, getServerMetadata, getIdpSession) and every
  save awaits it so a late hydration merge cannot clobber a fresh write.
  getCodeVerifier/getServerMetadata become async; the store is created with
  skipHydration:true so there is no auto-hydration to race the explicit one.
- store.ts adds normalizeServerUrl so a token saved under
  https://Example.com/mcp is found when the CLI asks for
  https://example.com/mcp/; getServerState falls back to the raw key.
- remote-storage adapter POSTs with keepalive so the write survives the
  OAuth authorize redirect, surfaces swallowed persist failures, and gives
  richer read/write error messages.
- generateOAuthState throws instead of silently degrading to Math.random.
- NodeOAuthStorage honors MCP_INSPECTOR_OAUTH_STATE_PATH for isolated
  fixtures. Provider codeVerifier()/getServerMetadata() + the EMA idpOidc and
  connection-state readers await the now-async storage.

Web:
- environmentFactory + a new shared lib/remoteOAuthStorage accessor
  (memoized per {baseUrl, authToken}) wire RemoteOAuthStorage into the
  connection path, the EMA IdP hook, and the per-server "clear OAuth" action
  so all three share one in-memory view of oauth.json.

Tests: async-hydration + normalizeServerUrl suite, keepalive/error-path
coverage, and the sync→async migration across existing storage/provider/EMA
tests. Web validate + coverage gate + integration suite green; CLI/TUI/
launcher validate green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

clients/web/src/test/core/auth/connection-state.test.ts

@@ -16,18 +16,19 @@ function createStorage(
     dynamic: Awaited<ReturnType<OAuthStorage["getClientInformation"]>>;
     registrationKind: ReturnType<OAuthStorage["getClientRegistrationKind"]>;
     scope: string | undefined;
-    serverMetadata: ReturnType<OAuthStorage["getServerMetadata"]>;
+    serverMetadata: Awaited<ReturnType<OAuthStorage["getServerMetadata"]>>;
     idpSession: Awaited<ReturnType<OAuthStorage["getIdpSession"]>>;
-    idpMetadata: ReturnType<OAuthStorage["getServerMetadata"]>;
+    idpMetadata: Awaited<ReturnType<OAuthStorage["getServerMetadata"]>>;
   }> = {},
 ): OAuthStorage {
   return {
+    ready: vi.fn().mockResolvedValue(undefined),
     getTokens: vi.fn().mockResolvedValue(overrides.tokens),
     getClientInformation: vi.fn(async (_url, isPreregistered) =>
       isPreregistered ? overrides.preregistered : overrides.dynamic,
     ),
     getScope: vi.fn().mockReturnValue(overrides.scope),
-    getServerMetadata: vi.fn((url: string) => {
+    getServerMetadata: vi.fn(async (url: string) => {
       if (url.startsWith("ema-idp:")) {
         return overrides.idpMetadata ?? null;
       }

clients/web/src/test/core/auth/ema/idpOidc.test.ts

@@ -470,7 +470,7 @@ describe("completeIdpOidcAuthorization", () => {
   });
 
   it("throws when the PKCE verifier is missing", async () => {
-    storage = buildStorage({ getCodeVerifier: vi.fn(() => undefined) });
+    storage = buildStorage({ getCodeVerifier: vi.fn(async () => undefined) });
     await expect(
       completeIdpOidcAuthorization({
         idp,

clients/web/src/test/core/auth/oauth-storage.test.ts

@@ -0,0 +1,224 @@
+import { describe, it, expect, vi } from "vitest";
+import { createJSONStorage } from "zustand/middleware";
+import { OAuthStorageBase } from "@inspector/core/auth/oauth-storage.js";
+import {
+  createOAuthStore,
+  normalizeServerUrl,
+} from "@inspector/core/auth/store.js";
+
+/**
+ * Builds an OAuthStorageBase backed by an *async* in-memory storage adapter
+ * pre-seeded with the given persisted blob. Hydration of the store completes
+ * only after the returned `release()` is called, so tests can assert that
+ * getters wait for it.
+ */
+function makeAsyncBackedStorage(
+  persistedServers: Record<string, unknown> = {},
+) {
+  let mem: string | null = JSON.stringify({
+    state: { servers: persistedServers },
+    version: 0,
+  });
+  let releaseGetItem!: () => void;
+  const gate = new Promise<void>((resolve) => {
+    releaseGetItem = resolve;
+  });
+  const adapter = createJSONStorage(() => ({
+    getItem: async () => {
+      await gate;
+      return mem;
+    },
+    setItem: async (name: string, value: string) => {
+      void name;
+      mem = value;
+    },
+    removeItem: async () => {
+      mem = null;
+    },
+  }));
+  const store = createOAuthStore(adapter);
+  return { storage: new OAuthStorageBase(store), release: releaseGetItem };
+}
+
+const SERVER = "https://example.com/mcp";
+const METADATA = {
+  issuer: SERVER,
+  authorization_endpoint: `${SERVER}/authorize`,
+  token_endpoint: `${SERVER}/token`,
+  response_types_supported: ["code"],
+};
+
+describe("OAuthStorageBase — async hydration", () => {
+  it("getCodeVerifier awaits hydration before reading state", async () => {
+    const { storage, release } = makeAsyncBackedStorage({
+      [SERVER]: { codeVerifier: "persisted-verifier" },
+    });
+    // The store is empty until release() lets the async getItem resolve.
+    let resolvedWith: string | undefined | "pending" = "pending";
+    const p = storage.getCodeVerifier(SERVER).then((v) => {
+      resolvedWith = v;
+    });
+    // Give the microtask queue a chance — the getter should NOT have resolved
+    // because hydration is still gated.
+    await Promise.resolve();
+    expect(resolvedWith).toBe("pending");
+    release();
+    await p;
+    expect(resolvedWith).toBe("persisted-verifier");
+  });
+
+  it("getServerMetadata awaits hydration before reading state", async () => {
+    const { storage, release } = makeAsyncBackedStorage({
+      [SERVER]: { serverMetadata: METADATA },
+    });
+    const p = storage.getServerMetadata(SERVER);
+    release();
+    expect(await p).toEqual(METADATA);
+  });
+
+  it("getTokens and getClientInformation await hydration", async () => {
+    const { storage, release } = makeAsyncBackedStorage({
+      [SERVER]: {
+        tokens: { access_token: "t", token_type: "Bearer" },
+        clientInformation: { client_id: "abc" },
+      },
+    });
+    const tokens = storage.getTokens(SERVER);
+    const client = storage.getClientInformation(SERVER);
+    release();
+    expect(await tokens).toEqual({ access_token: "t", token_type: "Bearer" });
+    expect(await client).toEqual({ client_id: "abc" });
+  });
+
+  it("ready() resolves once hydration completes", async () => {
+    const { storage, release } = makeAsyncBackedStorage();
+    let ready = false;
+    void storage.ready().then(() => {
+      ready = true;
+    });
+    await Promise.resolve();
+    expect(ready).toBe(false);
+    release();
+    await storage.ready();
+    expect(ready).toBe(true);
+  });
+
+  it("ready() resolves promptly when the adapter is synchronous", async () => {
+    let mem: string | null = null;
+    const syncAdapter = createJSONStorage(() => ({
+      getItem: () => mem,
+      setItem: (name: string, value: string) => {
+        void name;
+        mem = value;
+      },
+      removeItem: () => {
+        mem = null;
+      },
+    }));
+    const storage = new OAuthStorageBase(createOAuthStore(syncAdapter));
+    // With a sync adapter the constructor's rehydrate() resolves on the next
+    // microtask, so ready() and the getters settle without any release step.
+    await storage.ready();
+    expect(await storage.getCodeVerifier(SERVER)).toBeUndefined();
+  });
+
+  it("save* awaits hydration so a late merge cannot clobber the write", async () => {
+    const { storage, release } = makeAsyncBackedStorage({
+      [SERVER]: { codeVerifier: "persisted-old" },
+    });
+    // Kick off a save while hydration is still gated. It must not write until
+    // hydration completes, otherwise the persist merge would overwrite "new"
+    // with "persisted-old".
+    const save = storage.saveCodeVerifier(SERVER, "new");
+    let saved = false;
+    void save.then(() => {
+      saved = true;
+    });
+    await Promise.resolve();
+    expect(saved).toBe(false);
+    release();
+    await save;
+    expect(await storage.getCodeVerifier(SERVER)).toBe("new");
+  });
+
+  it("normalizeServerUrl canonicalizes host case, bare-origin trailing slash, default port, and whitespace", () => {
+    expect(normalizeServerUrl("https://Example.COM/mcp")).toBe(
+      "https://example.com/mcp",
+    );
+    expect(normalizeServerUrl("https://example.com")).toBe(
+      "https://example.com/",
+    );
+    expect(normalizeServerUrl("https://example.com:443/mcp")).toBe(
+      "https://example.com/mcp",
+    );
+    expect(normalizeServerUrl("  https://example.com/mcp  ")).toBe(
+      "https://example.com/mcp",
+    );
+    // Non-URL strings (e.g. a stdio server name) are passed through trimmed.
+    expect(normalizeServerUrl("  my-stdio-server  ")).toBe("my-stdio-server");
+  });
+
+  it("store keys are normalized so save and get tolerate cosmetic URL differences", async () => {
+    const { storage, release } = makeAsyncBackedStorage();
+    release();
+    await storage.saveCodeVerifier("https://Example.COM/mcp", "v");
+    expect(await storage.getCodeVerifier("https://example.com/mcp")).toBe("v");
+    // Bare-origin: the web inspector stores `new URL().href` (trailing slash);
+    // a CLI caller passing the slash-less form must still hit it.
+    await storage.saveTokens("https://api.partner.dev/", {
+      access_token: "t",
+      token_type: "Bearer",
+    });
+    expect(await storage.getTokens("https://api.partner.dev")).toEqual({
+      access_token: "t",
+      token_type: "Bearer",
+    });
+  });
+
+  it("getServerState falls back to a pre-normalization persisted key", async () => {
+    // A blob persisted before normalization existed may carry a raw key that
+    // the canonical form differs from (here: bare-origin without slash).
+    const { storage, release } = makeAsyncBackedStorage({
+      "https://legacy.example": { codeVerifier: "legacy" },
+    });
+    release();
+    expect(await storage.getCodeVerifier("https://legacy.example")).toBe(
+      "legacy",
+    );
+  });
+
+  it("ready()/getters resolve (empty) when the adapter throws instead of hanging", async () => {
+    const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
+    try {
+      const throwingAdapter = createJSONStorage(() => ({
+        getItem: async () => {
+          throw new Error("Failed to read store: 500");
+        },
+        setItem: async () => {},
+        removeItem: async () => {},
+      }));
+      const storage = new OAuthStorageBase(createOAuthStore(throwingAdapter));
+      // Race ready() against a short timeout — if the catch path doesn't
+      // resolve `hydrated`, this would hit "TIMEOUT" instead of "ok".
+      const result = await Promise.race([
+        storage.ready().then(() => "ok"),
+        new Promise((r) => setTimeout(() => r("TIMEOUT"), 200)),
+      ]);
+      expect(result).toBe("ok");
+      expect(await storage.getTokens(SERVER)).toBeUndefined();
+      expect(warn).toHaveBeenCalled();
+      // The failure is recorded so callers can distinguish "no token" from
+      // "store unreadable".
+      expect(storage.getHydrationError()).toBeInstanceOf(Error);
+    } finally {
+      warn.mockRestore();
+    }
+  });
+
+  it("getHydrationError() is undefined after a successful hydration", async () => {
+    const { storage, release } = makeAsyncBackedStorage();
+    release();
+    await storage.ready();
+    expect(storage.getHydrationError()).toBeUndefined();
+  });
+});

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

@@ -221,9 +221,9 @@ describe("OAuthNavigation", () => {
         getTokens: vi.fn(async () => undefined),
         saveTokens: vi.fn(async () => undefined),
         saveCodeVerifier: vi.fn(async () => undefined),
-        getCodeVerifier: vi.fn(() => undefined),
+        getCodeVerifier: vi.fn(async () => undefined),
         clear: vi.fn(),
-        getServerMetadata: vi.fn(() => null),
+        getServerMetadata: vi.fn(async () => null),
         saveServerMetadata: vi.fn(async () => undefined),
       } as unknown as OAuthStorage;
     }
@@ -271,21 +271,21 @@ describe("OAuthNavigation", () => {
       expect(await provider.clientInformation()).toEqual({ client_id: "dyn" });
     });
 
-    it("codeVerifier() throws when none is saved", () => {
+    it("codeVerifier() rejects when none is saved", async () => {
       const storage = makeStorage();
       const provider = makeProvider(storage);
 
-      expect(() => provider.codeVerifier()).toThrow(
+      await expect(provider.codeVerifier()).rejects.toThrow(
         /No code verifier saved for session/,
       );
     });
 
-    it("codeVerifier() returns the saved verifier", () => {
+    it("codeVerifier() returns the saved verifier", async () => {
       const storage = makeStorage();
-      vi.mocked(storage.getCodeVerifier).mockReturnValue("cv-1");
+      vi.mocked(storage.getCodeVerifier).mockResolvedValue("cv-1");
       const provider = makeProvider(storage);
 
-      expect(provider.codeVerifier()).toBe("cv-1");
+      expect(await provider.codeVerifier()).toBe("cv-1");
     });
 
     it("clientMetadata reflects the stored scope when present", () => {
@@ -362,7 +362,7 @@ describe("OAuthNavigation", () => {
       expect(storage.saveCodeVerifier).toHaveBeenCalledWith(SERVER, "cv");
       expect(storage.saveServerMetadata).toHaveBeenCalled();
       expect(await provider.tokens()).toBeUndefined();
-      expect(provider.getServerMetadata()).toBeNull();
+      expect(await provider.getServerMetadata()).toBeNull();
       const state = await provider.state();
       expect(typeof state).toBe("string");
       expect(state.length).toBeGreaterThan(0);

clients/web/src/test/core/auth/storage-browser.test.ts

@@ -1,5 +1,8 @@
 import { describe, it, expect, beforeEach, afterEach } from "vitest";
-import { BrowserOAuthStorage } from "@inspector/core/auth/browser/storage.js";
+import {
+  BrowserOAuthStorage,
+  getBrowserOAuthStorage,
+} from "@inspector/core/auth/browser/storage.js";
 import type {
   OAuthClientInformation,
   OAuthTokens,
@@ -41,6 +44,16 @@ const mockSessionStorage = new MockSessionStorage();
 (global as typeof globalThis & { sessionStorage?: Storage }).sessionStorage =
   mockSessionStorage;
 
+describe("getBrowserOAuthStorage", () => {
+  it("returns a memoized singleton across calls", () => {
+    const first = getBrowserOAuthStorage();
+    const second = getBrowserOAuthStorage();
+    expect(first).toBeInstanceOf(BrowserOAuthStorage);
+    // Second call hits the cached-instance branch rather than constructing anew.
+    expect(second).toBe(first);
+  });
+});
+
 describe("BrowserOAuthStorage", () => {
   let storage: BrowserOAuthStorage;
   const testServerUrl = "http://localhost:3000";
@@ -226,8 +239,10 @@ describe("BrowserOAuthStorage", () => {
     it("should return stored scope", async () => {
       const scope = "read write";
 
-      storage.saveScope(testServerUrl, scope);
-      const result = await storage.getScope(testServerUrl);
+      // getScope is intentionally synchronous; await the save (which waits for
+      // hydration) so the value has landed before the sync read.
+      await storage.saveScope(testServerUrl, scope);
+      const result = storage.getScope(testServerUrl);
 
       expect(result).toBe(scope);
     });
@@ -237,8 +252,8 @@ describe("BrowserOAuthStorage", () => {
     it("should save scope", async () => {
       const scope = "read write";
 
-      storage.saveScope(testServerUrl, scope);
-      const result = await storage.getScope(testServerUrl);
+      await storage.saveScope(testServerUrl, scope);
+      const result = storage.getScope(testServerUrl);
 
       expect(result).toBe(scope);
     });
@@ -322,13 +337,13 @@ describe("BrowserOAuthStorage", () => {
 
     it("clearCodeVerifier removes only the PKCE verifier", async () => {
       storage.saveCodeVerifier(testServerUrl, "verifier");
-      expect(storage.getCodeVerifier(testServerUrl)).toBe("verifier");
+      expect(await storage.getCodeVerifier(testServerUrl)).toBe("verifier");
       storage.clearCodeVerifier(testServerUrl);
-      expect(storage.getCodeVerifier(testServerUrl)).toBeUndefined();
+      expect(await storage.getCodeVerifier(testServerUrl)).toBeUndefined();
     });
 
     it("clearScope removes only the scope", async () => {
-      storage.saveScope(testServerUrl, "read");
+      await storage.saveScope(testServerUrl, "read");
       expect(storage.getScope(testServerUrl)).toBe("read");
       storage.clearScope(testServerUrl);
       expect(storage.getScope(testServerUrl)).toBeUndefined();
@@ -342,9 +357,9 @@ describe("BrowserOAuthStorage", () => {
         response_types_supported: ["code"],
       };
       storage.saveServerMetadata(testServerUrl, metadata);
-      expect(storage.getServerMetadata(testServerUrl)).toEqual(metadata);
+      expect(await storage.getServerMetadata(testServerUrl)).toEqual(metadata);
       storage.clearServerMetadata(testServerUrl);
-      expect(storage.getServerMetadata(testServerUrl)).toBeNull();
+      expect(await storage.getServerMetadata(testServerUrl)).toBeNull();
     });
   });
 
@@ -358,10 +373,10 @@ describe("BrowserOAuthStorage", () => {
         token_type: "Bearer",
       };
 
-      storage.saveClientInformation(testServerUrl, clientInfo, {
+      await storage.saveClientInformation(testServerUrl, clientInfo, {
         registrationKind: "dcr",
       });
-      storage.saveTokens(testServerUrl, tokens);
+      await storage.saveTokens(testServerUrl, tokens);
 
       storage.clear(testServerUrl);
 
@@ -375,10 +390,10 @@ describe("BrowserOAuthStorage", () => {
         client_id: "test-client-id",
       };
 
-      storage.saveClientInformation(testServerUrl, clientInfo, {
+      await storage.saveClientInformation(testServerUrl, clientInfo, {
         registrationKind: "dcr",
       });
-      storage.saveClientInformation(otherServerUrl, clientInfo, {
+      await storage.saveClientInformation(otherServerUrl, clientInfo, {
         registrationKind: "dcr",
       });
 

clients/web/src/test/core/auth/storage-remote.test.ts

@@ -72,9 +72,9 @@ describe("RemoteOAuthStorage (unit, mocked fetch)", () => {
 
   it("codeVerifier round-trip and clearCodeVerifier", async () => {
     await storage.saveCodeVerifier(serverUrl, "verifier");
-    expect(storage.getCodeVerifier(serverUrl)).toBe("verifier");
+    expect(await storage.getCodeVerifier(serverUrl)).toBe("verifier");
     storage.clearCodeVerifier(serverUrl);
-    expect(storage.getCodeVerifier(serverUrl)).toBeUndefined();
+    expect(await storage.getCodeVerifier(serverUrl)).toBeUndefined();
   });
 
   it("scope round-trip and clearScope", async () => {
@@ -92,9 +92,9 @@ describe("RemoteOAuthStorage (unit, mocked fetch)", () => {
       response_types_supported: ["code"],
     };
     await storage.saveServerMetadata(serverUrl, md);
-    expect(storage.getServerMetadata(serverUrl)).toEqual(md);
+    expect(await storage.getServerMetadata(serverUrl)).toEqual(md);
     storage.clearServerMetadata(serverUrl);
-    expect(storage.getServerMetadata(serverUrl)).toBeNull();
+    expect(await storage.getServerMetadata(serverUrl)).toBeNull();
   });
 
   it("clear() wipes all state for a server", async () => {
@@ -120,4 +120,65 @@ describe("RemoteOAuthStorage (unit, mocked fetch)", () => {
     // No public accessor; constructing without throwing covers the default-branch.
     expect(s).toBeInstanceOf(RemoteOAuthStorage);
   });
+
+  it("getCodeVerifier waits for the async hydration GET before reading", async () => {
+    // Unlike the other tests in this file (which write-then-read in the same
+    // session), this asserts that a value PERSISTED on the backend before the
+    // store was constructed is still returned — i.e. the getter awaits the
+    // remote storage adapter's hydration GET.
+    let releaseGet!: () => void;
+    const getGate = new Promise<void>((resolve) => {
+      releaseGet = resolve;
+    });
+    const fetchFn = vi.fn(
+      async (url: RequestInfo | URL, init?: RequestInit) => {
+        if ((init?.method ?? "GET") === "GET") {
+          await getGate;
+          return new Response(
+            JSON.stringify({
+              state: {
+                servers: { [serverUrl]: { codeVerifier: "from-disk" } },
+              },
+              version: 0,
+            }),
+            { status: 200 },
+          );
+        }
+        void url;
+        return new Response("{}", { status: 200 });
+      },
+    );
+    const s = new RemoteOAuthStorage({
+      baseUrl: "http://remote.example",
+      storeId: "hydration-test",
+      fetchFn: fetchFn as unknown as typeof fetch,
+    });
+    const p = s.getCodeVerifier(serverUrl);
+    releaseGet();
+    expect(await p).toBe("from-disk");
+  });
+
+  it("persist POST is sent with keepalive so it survives an immediate redirect", async () => {
+    const fetchFn = vi.fn(
+      async (url: RequestInfo | URL, init?: RequestInit) => {
+        void url;
+        void init;
+        return new Response("{}", { status: 200 });
+      },
+    );
+    const s = new RemoteOAuthStorage({
+      baseUrl: "http://remote.example",
+      storeId: "keepalive-test",
+      fetchFn: fetchFn as unknown as typeof fetch,
+    });
+    await s.ready();
+    await s.saveCodeVerifier(serverUrl, "v");
+    // Zustand persist fires setItem on the next microtask after set(); flush
+    // a couple of ticks so the POST has been issued.
+    await Promise.resolve();
+    await Promise.resolve();
+    const post = fetchFn.mock.calls.find((c) => c[1]?.method === "POST");
+    expect(post).toBeDefined();
+    expect(post?.[1]?.keepalive).toBe(true);
+  });
 });

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

@@ -61,6 +61,26 @@ describe("generateOAuthState", () => {
     const s2 = generateOAuthState();
     expect(s1).not.toBe(s2);
   });
+
+  it("throws instead of silently degrading when crypto.getRandomValues is unavailable", () => {
+    const original = globalThis.crypto;
+    // Simulate a runtime without Web Crypto — the CSRF token must not fall back
+    // to a predictable Math.random source.
+    Object.defineProperty(globalThis, "crypto", {
+      value: undefined,
+      configurable: true,
+    });
+    try {
+      expect(() => generateOAuthState()).toThrow(
+        /crypto\.getRandomValues is not available/,
+      );
+    } finally {
+      Object.defineProperty(globalThis, "crypto", {
+        value: original,
+        configurable: true,
+      });
+    }
+  });
 });
 
 describe("parseOAuthState", () => {

clients/web/src/test/core/mcp/oauthManager.test.ts

@@ -38,6 +38,7 @@ function createMockParams(
   const dispatchOAuthError = vi.fn();
 
   const storage = {
+    ready: vi.fn().mockResolvedValue(undefined),
     getScope: vi.fn().mockReturnValue(undefined),
     getClientInformation: vi.fn().mockResolvedValue(undefined),
     getClientRegistrationKind: vi.fn().mockReturnValue(undefined),