Skip to content

Web: migrate auth store to shared /store API (RemoteOAuthStorage parity with TUI/CLI) #1548

Description

@cliffhall

Follow-up split out of #1514 (Auth cleanup), completed by #1531.

#1531 removed guided auth and added connect-time OAuth, install-level CIMD, per-server OAuth clear, and keychain rehydration for TUI/CLI. The remaining piece of #1514's "migrate web client auth store to /store API" scope is still open:

  • Migrate the web client's OAuth storage to the shared /store API (RemoteOAuthStorage) so it reads/writes the same oauth.json as the TUI/CLI file-based store, giving web ⇄ TUI ⇄ CLI parity.
  • Listed explicitly as a follow-up in Guided-auth removal, OAuth smoke testing, and TUI/CLI support #1531 ("Web shared OAuth store (RemoteOAuthStorage / oauth.json parity with TUI/CLI)").

Target branch: v2/main.


Scope & context (added post-hoc — this issue is the Wave-1 home for PR #1510's web-OAuth-storage migration)

This pre-existing issue was terse and cited no files, but it stands in as the Wave-1 owner of the RemoteOAuthStorage + async-hydration work from the #1510 decomposition (see tracking #1579). Delivered by PR #1592 onto the Wave-1 rollup branch 1579-wave-1 (reference implementation: #1510 head 33fac3f). The concrete scope:

Behavior

  • Web OAuth store migrates from BrowserOAuthStorage (sessionStorage) to backend-backed RemoteOAuthStorage (~/.mcp-inspector/storage/oauth.json), shared with TUI/CLI on the same host.
  • Introduces a single async hydration gate (ready() / getHydrationError()); all post-redirect reads and all saves await it. Clears run hydration-safe (defer until hydrated, never clobber the on-disk blob).
  • getCodeVerifier / getServerMetadata become async across the interface, propagated through the provider and consumers.

Files changed (delivered on 1579-wave-1)

  • core/auth/storage.ts — ready() + async hydration gate; async getCodeVerifier/getServerMetadata signatures
  • core/auth/oauth-storage.ts — OAuthStorageBase hydration + hydration-safe clear*
  • core/auth/providers.ts — async codeVerifier/getServerMetadata
  • core/auth/store.ts — skipHydration + normalizeServerUrl keying + raw-key migration
  • core/auth/connection-state.ts — awaits getServerMetadata (this is feat(web/apps): spec-conformant Apps host + CLI/deep-link path for programmatic review #1510's core/auth/state-machine.ts, renamed on current v2/main)
  • core/auth/node/storage-node.ts — honors MCP_INSPECTOR_OAUTH_STATE_PATH
  • core/storage/adapters/remote-storage.ts — keepalive POSTs + surfaces persist failures
  • clients/web/src/lib/environmentFactory.ts — the load-bearing swap: wires web to RemoteOAuthStorage via the memoized getRemoteOAuthStorage(...) accessor
  • clients/web/src/lib/remoteOAuthStorage.ts — new memoized RemoteOAuthStorage accessor
  • clients/web/src/App.tsx — async hydration in the OAuth-callback flow
  • Tests: oauth-storage / storage-browser / storage-remote / oauthManager (async mocks) + integration auth/node/storage and storage/adapters

Verification: exhaustive @claude review to clean, coverage gate ≥90 on all four dims for the gated files, and an independent smoke/audit (backend round-trip through /api/storage/oauth, oauth.json shape matches TUI/CLI, clear-before-hydration preserves sibling tokens) — see the audit on this issue and the rollup verification in #1600.

Activity

  1. cliffhall commented on Jul 1, 2026

    @cliffhall
    MemberAuthor

    Heads-up: PR #1510 contains a complete reference implementation of this issue's scope (plus extras), now decomposed under tracking issue #1579 with this issue folded in as the OAuth-storage foundation item.

    Reference implementation at 33fac3f:

    Note the CLI stored-auth suite (#1575) and deep-link auto-connect (#1576) depend on the oauth.json format and URL normalization established here.

  2. self-assigned this
    on Jul 1, 2026
  3. cliffhall commented on Jul 2, 2026

    @cliffhall
    MemberAuthor

    Smoke test & AGENTS.md audit — PR #1592

    Audited at PR head 588fc3f8 (branch v2/1548-web-remote-oauth-storage), fresh npm install. This is a store/core/lib wiring change (auth storage migrated to RemoteOAuthStorage + async hydration); there is no essential visual surface — the OAuth flow UI is unchanged, so Playwright/browser drive was not run.

    1. Smoke test — method + real output

    Primary proof: the integration + unit suites. The RemoteOAuthStorage/RemoteStorageAdapter and the async-hydration OAuthStorageBase are exercised end-to-end against a real Hono backend and mocked-fetch paths.

    • clients/web npm run test:integration → 38 files / 801 tests passed (the lone "1 error" is a known vitest teardown flake — EnvironmentTeardownError: Closing rpc while "onUserConsoleLog" was pending — not a test failure).
    • Focused verbose runs of the new/updated suites:
    ✓ integration  storage/adapters.test.ts > RemoteStorageAdapter > creates store and persists state via HTTP
    ✓ integration  storage/adapters.test.ts > RemoteStorageAdapter > loads persisted state on initialization
    ✓ integration  storage/adapters.test.ts > RemoteStorageAdapter > handles empty state after clear
    ✓ integration  storage/adapters.test.ts > createRemoteStorageAdapter > setItem sends keepalive so the POST survives an immediate redirect
    ✓ integration  storage/adapters.test.ts > createRemoteStorageAdapter > setItem rethrows a network-level fetch rejection after logging it
    ✓ integration  auth/node/storage.test.ts > OAuth Store (Zustand) > should persist state to file
       (2 files / 54 tests passed)
    
    ✓ unit  core/auth/oauth-storage.test.ts > getCodeVerifier awaits hydration before reading state
    ✓ unit  core/auth/oauth-storage.test.ts > save* awaits hydration so a late merge cannot clobber the write
    ✓ unit  core/auth/oauth-storage.test.ts > clear() before hydration is not resurrected by the late merge
    ✓ unit  core/auth/oauth-storage.test.ts > clearTokens before hydration clears only tokens and survives the merge
    ✓ unit  core/auth/oauth-storage.test.ts > ready()/getters resolve (empty) when the adapter throws instead of hanging
    ✓ unit  core/auth/storage-remote.test.ts > getCodeVerifier waits for the async hydration GET before reading
    ✓ unit  core/auth/storage-remote.test.ts > persist POST is sent with keepalive so it survives an immediate redirect
       (oauth-storage + storage-remote + storage-browser: 3 files / 59 tests passed)
    

    Additional throwaway proof (custom script, /tmp, not committed). Drives the real createRemoteApp Hono backend + RemoteOAuthStorage over HTTP to prove the round-trip, on-disk oauth.json shape, and the pre-hydration clear guard. Real captured output:

    [proof] read-back tokens for A: {"access_token":"tok-A","token_type":"Bearer"}
    [proof] fresh instance sees A after hydration: {"access_token":"tok-A","token_type":"Bearer"}
    [proof] oauth.json path: <tmp>/oauth.json
    [proof] oauth.json shape: {
      "state": {
        "servers": {
          "https://a.example.com/mcp": {
            "tokens": {
              "access_token": "tok-A",
              "token_type": "Bearer"
            }
          }
        },
        "idpSessions": {}
      },
      "version": 0
    }
    [proof] after pre-hydration clear(A): A =            (undefined — cleared)
    [proof] after pre-hydration clear(A): B = {"access_token":"tok-B","token_type":"Bearer"}
    [proof] oauth.json servers after clear: ["https://a.example.com/mcp","https://b.example.com/mcp"]
    

    This confirms: (a) a token saved through RemoteOAuthStorage round-trips through POST/GET /api/storage/oauth and is readable by a fresh instance (post-redirect page-reload semantics) only after hydration; (b) it persists to oauth.json in the exact shape/path the TUI/CLI read (~/.mcp-inspector/storage/oauth.json); (c) a clearTokens(A) issued before hydration lands is deferred and merges onto the real persisted blob — A's token is cleared while sibling B's token survives (no whole-blob clobber).

    2. AGENTS.md compliance audit

    Mantine — N/A for the changed surface. Every changed file is core/store/lib wiring. The one React file (App.tsx) changed only wiring: swap getBrowserOAuthStorage → getRemoteOAuthStorage, a useMemo, and passing an oauthStorage prop to clearServerOAuthState. No JSX/layout/style changes, no inline styles, no raw hex/rgba.

    TypeScript — clean. No any types (the only any token in the source diff is prose in a JSDoc comment: "Resolves once any async hydration…"), no ts-ignore/ts-expect-error/eslint-disable, no new v8 ignore. The async migration is fully typed: OAuthStorage gained ready(): Promise<void> and made getCodeVerifier/getServerMetadata return Promise<…>; BaseOAuthClientProvider.codeVerifier()/getServerMetadata() became async; every caller (connection-state.ts, ema/idpOidc.ts) was updated to await. eslint . passed with no errors; tsc -p tsconfig.app.json --noEmit exited 0.

    Coverage — clients/web npm run test:coverage: PASSED (231 files / 3265 tests, per-file gate ≥90 on all four dims). Key touched files (S / B / F / L):

    File Stmts Branch Funcs Lines
    core/auth/oauth-storage.ts 98.63 92.59 97.29 98.61
    core/auth/store.ts 100 100 100 100
    core/auth/storage.ts 100 100 100 100
    core/auth/providers.ts 100 100 100 100
    core/auth/utils.ts 100 96.66 100 100
    core/auth/connection-state.ts 100 98.38 100 100
    core/auth/ema/idpOidc.ts 98.57 96.66 100 98.57
    core/auth/node/storage-node.ts 100 100 100 100
    core/storage/adapters/remote-storage.ts 100 100 100 100
    clients/web/src/utils/clearServerOAuthState.ts 100 100 100 100

    All ≥ 90 on every dimension.

    ⚠️ Include-glob note. The two new web wiring files — clients/web/src/lib/remoteOAuthStorage.ts and clients/web/src/lib/environmentFactory.ts — live under src/lib/**, which is not in the coverage include globs (only src/components/**, src/utils/**, clients/web/server/**, and core/{mcp,react,auth,storage,logging}/** are included — see clients/web/vite.config.ts). They therefore are not gate-enforced. remoteOAuthStorage.test.ts was added and passes, but its source file's coverage is not measured against the 90 gate. Not a blocker, but worth flagging if src/lib should be brought under the gate.

    Verdict

    ✅ PASS. Integration + unit suites green; RemoteOAuthStorage round-trips through the backend to oauth.json with hydration-safe reads and a pre-hydration clear that does not clobber sibling tokens; TypeScript clean (no any/suppressions, async signatures fully typed); lint + typecheck pass; coverage gate passes with all key touched files ≥90. Only advisory: the src/lib wiring files sit outside the coverage include globs.

    Throwaway proof script (/tmp, not committed) — run via vitest --project integration
    // THROWAWAY smoke proof for PR #1592 — not committed.
    // Drives a REAL Hono backend (createRemoteApp) + RemoteOAuthStorage to prove:
    //   1. save -> await hydration -> read-back round-trips through POST/GET /api/storage/oauth
    //   2. the blob persists to oauth.json on disk (shows path + shape)
    //   3. a clear issued BEFORE hydration lands clears only its own key and does
    //      NOT clobber a sibling server's tokens (the whole-blob clobber guard)
    import { describe, it, expect, afterEach } from "vitest";
    import { serve } from "@hono/node-server";
    import type { ServerType } from "@hono/node-server";
    import { mkdtempSync, readFileSync, rmSync } from "node:fs";
    import { tmpdir } from "node:os";
    import { join } from "node:path";
    import { createRemoteApp } from "@inspector/core/mcp/remote/node/server.js";
    import { RemoteOAuthStorage } from "@inspector/core/auth/remote/index.js";
    import { getStoreFilePath } from "@inspector/core/storage/store-io.js";
    import { waitForRemoteStore } from "@modelcontextprotocol/inspector-test-server";
    
    // Bypass vitest's console interception (which hides logs on passing tests).
    function proofLog(...parts: string[]): void {
      process.stdout.write(parts.join(" ") + "\n");
    }
    
    // The zustand persist POST is fire-and-forget (saveTokens does not await the
    // HTTP write), so wait for the backend to actually have the key before reading
    // it back through a fresh instance.
    function serversHave(...urls: string[]) {
      return (body: unknown): boolean => {
        const servers = (body as { state?: { servers?: Record<string, unknown> } })
          ?.state?.servers;
        return !!servers && urls.every((u) => u in servers);
      };
    }
    
    const SERVER_A = "https://a.example.com/mcp";
    const SERVER_B = "https://b.example.com/mcp";
    const TOKENS_A = { access_token: "tok-A", token_type: "Bearer" as const };
    const TOKENS_B = { access_token: "tok-B", token_type: "Bearer" as const };
    
    let server: ServerType | undefined;
    let dir: string | undefined;
    
    afterEach(() => {
      server?.close();
      if (dir) rmSync(dir, { recursive: true, force: true });
    });
    
    async function start() {
      dir = mkdtempSync(join(tmpdir(), "oauth-proof-"));
      const { app, authToken } = createRemoteApp({
        storageDir: dir,
        initialConfig: { defaultEnvironment: {} },
      });
      const baseUrl: string = await new Promise((resolve) => {
        server = serve({ fetch: app.fetch, port: 0, hostname: "127.0.0.1" }, (i) =>
          resolve(`http://127.0.0.1:${(i as { port: number }).port}`),
        );
      });
      return { baseUrl, authToken };
    }
    
    describe("PR#1592 round-trip + pre-hydration clear proof", () => {
      it("round-trips a token through the backend and persists oauth.json", async () => {
        const { baseUrl, authToken } = await start();
        const writer = new RemoteOAuthStorage({ baseUrl, authToken });
    
        await writer.saveTokens(SERVER_A, TOKENS_A);
        await writer.ready();
        const readBack = await writer.getTokens(SERVER_A);
        proofLog("[proof] read-back tokens for A:", JSON.stringify(readBack));
        expect(readBack).toEqual(TOKENS_A);
    
        // Wait for the fire-and-forget persist POST to actually land on the backend.
        await waitForRemoteStore(baseUrl, "oauth", authToken, serversHave(SERVER_A));
    
        // Fresh instance == post-redirect page reload: empty in-memory store that
        // must hydrate from the backend GET before it can see A.
        const reader = new RemoteOAuthStorage({ baseUrl, authToken });
        const afterHydrate = await reader.getTokens(SERVER_A);
        proofLog(
          "[proof] fresh instance sees A after hydration:",
          JSON.stringify(afterHydrate),
        );
        expect(afterHydrate).toEqual(TOKENS_A);
    
        const filePath = getStoreFilePath(dir!, "oauth");
        const onDisk = JSON.parse(readFileSync(filePath, "utf8"));
        proofLog("[proof] oauth.json path:", filePath.replace(dir!, "<tmp>"));
        proofLog("[proof] oauth.json shape:", JSON.stringify(onDisk, null, 2));
        expect(onDisk.state.servers[SERVER_A].tokens).toEqual(TOKENS_A);
      });
    
      it("a clear issued BEFORE hydration does not clobber a sibling's tokens", async () => {
        const { baseUrl, authToken } = await start();
    
        // Seed BOTH servers via a first instance and flush to disk.
        const seed = new RemoteOAuthStorage({ baseUrl, authToken });
        await seed.saveTokens(SERVER_A, TOKENS_A);
        await seed.saveTokens(SERVER_B, TOKENS_B);
        await seed.ready();
        await waitForRemoteStore(
          baseUrl,
          "oauth",
          authToken,
          serversHave(SERVER_A, SERVER_B),
        );
    
        // Fresh instance (post-redirect reset). Issue clearTokens(A) IMMEDIATELY,
        // before awaiting hydration — the dangerous pre-hydration path.
        const fresh = new RemoteOAuthStorage({ baseUrl, authToken });
        fresh.clearTokens(SERVER_A); // deferred until hydration lands
        await fresh.ready();
        // Let the deferred clear's own persist write settle on the backend.
        await waitForRemoteStore(
          baseUrl,
          "oauth",
          authToken,
          (body) =>
            !serversHave(SERVER_A)(body) ||
            !(
              body as {
                state?: { servers?: Record<string, { tokens?: unknown }> };
              }
            )?.state?.servers?.[SERVER_A]?.tokens,
        );
    
        const a = await fresh.getTokens(SERVER_A);
        const b = await fresh.getTokens(SERVER_B);
        proofLog("[proof] after pre-hydration clear(A): A =", JSON.stringify(a));
        proofLog("[proof] after pre-hydration clear(A): B =", JSON.stringify(b));
        expect(a).toBeUndefined(); // A cleared
        expect(b).toEqual(TOKENS_B); // B survived — no whole-blob clobber
    
        // And it truly landed on disk (B intact, A gone).
        const onDisk = JSON.parse(
          readFileSync(getStoreFilePath(dir!, "oauth"), "utf8"),
        );
        proofLog(
          "[proof] oauth.json servers after clear:",
          JSON.stringify(Object.keys(onDisk.state.servers)),
        );
        expect(onDisk.state.servers[SERVER_B]?.tokens).toEqual(TOKENS_B);
      });
    });

    🤖 Smoke test + AGENTS.md audit performed by Claude Code.

  4. cliffhall commented on Jul 2, 2026

    @cliffhall
    MemberAuthor

    🔗 Implemented by PR #1592. Its code is already integrated into the Wave 1 rollup branch 1579-wave-1 (see rollup PR #1601 / verification #1600). #1592 currently targets v2/main and will auto-show as Merged once the rollup lands in v2/main. The Development panel doesn't link it now because the closing keyword is on a non-default base branch.

  5. added a commit that references this issue on Jul 28, 2026
    c299b6e
  6. added this to the v2.0.0 milestone on Jul 28, 2026
  7. added
    authIssues and PRs related to authorization
    on Aug 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

authIssues and PRs related to authorizationv2Issues and PRs for v2

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions