diff --git a/clients/tui/src/App.tsx b/clients/tui/src/App.tsx index 01b05bc57..0d1264141 100644 --- a/clients/tui/src/App.tsx +++ b/clients/tui/src/App.tsx @@ -896,7 +896,7 @@ function App({ const handleClearOAuth = useCallback(async () => { if (!selectedInspectorClient) return; - selectedInspectorClient.clearOAuthTokens(); + await selectedInspectorClient.clearOAuthTokens(); setOauthStatus("idle"); setOauthMessage(null); setConnectError(null); diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index 1999cd1cb..c02dc763d 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -86,7 +86,7 @@ import { useServers } from "@inspector/core/react/useServers.js"; import { useSettingsDraft } from "@inspector/core/react/useSettingsDraft.js"; import { useClientSettingsDraft } from "@inspector/core/react/useClientSettingsDraft.js"; import { useEmaIdpLoginState } from "@inspector/core/react/useEmaIdpLoginState.js"; -import { getBrowserOAuthStorage } from "@inspector/core/auth/browser/index.js"; +import { getWebRemoteOAuthStorage } from "./lib/remoteOAuthStorage"; import { useManagedTools } from "@inspector/core/react/useManagedTools.js"; import { useManagedPrompts } from "@inspector/core/react/useManagedPrompts.js"; import { useManagedResources } from "@inspector/core/react/useManagedResources.js"; @@ -1428,6 +1428,13 @@ function App() { [messages], ); + // Shared OAuth runtime store (oauth.json via /api/storage/oauth). Memoized so + // connect, EMA IdP session, and per-server clear share one in-memory view. + const webOAuthStorage = useMemo( + () => getWebRemoteOAuthStorage(getAuthToken()), + [], + ); + // Backend-backed session storage used to carry the fetch (Network) log // across the OAuth full-page redirect. The auth handshake's first half — // protected-resource + auth-server discovery and Dynamic Client @@ -2098,9 +2105,10 @@ function App() { // `onToggleConnection` unloaded the previous one), so all React state is // reset and we recover the initiating server from sessionStorage. We wait for // `servers` to hydrate before acting; the ref guard keeps the exchange to a - // single run. The persisted PKCE verifier + DCR client info live in - // `BrowserOAuthStorage` and survive the redirect, so `completeOAuthFlow` - // exchanges the code without needing the original in-memory state machine. + // single run. The persisted PKCE verifier + DCR client info live in shared + // `RemoteOAuthStorage` (`oauth.json`) and survive the redirect, so + // `completeOAuthFlow` exchanges the code without needing the original + // in-memory state machine. useEffect(() => { if (typeof window === "undefined") return; if (window.location.pathname !== OAUTH_CALLBACK_PATH) return; @@ -2183,6 +2191,15 @@ function App() { } void (async () => { + try { + await webOAuthStorage.load(); + } catch (err) { + connectStartRef.current = undefined; + queueMicrotask(() => { + showReAuthBanner(server.id, err instanceof Error ? err : String(err)); + }); + return; + } const client = setupClientForServer(server, sessionId); setActiveServerId(server.id); try { @@ -2238,7 +2255,7 @@ function App() { }); } })(); - }, [servers, setupClientForServer, showReAuthBanner]); + }, [servers, setupClientForServer, showReAuthBanner, webOAuthStorage]); const onToggleConnection = useCallback( async (id: string) => { @@ -3239,10 +3256,9 @@ function App() { const clientSettingsModalValue = clientSettingsDraft ?? EMPTY_CLIENT_SETTINGS; - const emaOAuthStorage = useMemo(() => getBrowserOAuthStorage(), []); const { loginState: emaIdpLoginState, logout: logoutEmaIdp } = useEmaIdpLoginState( - emaOAuthStorage, + webOAuthStorage, clientSettingsModalValue.emaEnabled ? clientSettingsModalValue.issuer : undefined, @@ -3266,10 +3282,11 @@ function App() { const clearServerOAuthAndDisconnect = useCallback( async (server: { id: string; name: string; config: MCPServerConfig }) => { const isActive = server.id === activeServerId; - const cleared = clearServerOAuthState({ + const cleared = await clearServerOAuthState({ config: server.config, inspectorClient: isActive ? inspectorClient : null, isActiveConnection: isActive, + oauthStorage: webOAuthStorage, }); if (!cleared) return; @@ -3295,6 +3312,7 @@ function App() { [ activeServerId, inspectorClient, + webOAuthStorage, finalizeExplicitDisconnect, clearOAuthResumeOnExplicitDisconnect, ], diff --git a/clients/web/src/lib/environmentFactory.test.ts b/clients/web/src/lib/environmentFactory.test.ts new file mode 100644 index 000000000..c3c4e70e6 --- /dev/null +++ b/clients/web/src/lib/environmentFactory.test.ts @@ -0,0 +1,124 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { BrowserNavigation } from "@inspector/core/auth/browser/index.js"; +import { MutableRedirectUrlProvider } from "@inspector/core/auth/providers.js"; +import { createWebEnvironment } from "./environmentFactory"; +import { + getWebRemoteOAuthStorage, + resetWebRemoteOAuthStorageCacheForTests, +} from "./remoteOAuthStorage"; + +describe("createWebEnvironment", () => { + beforeEach(() => { + vi.stubGlobal("window", { + location: { + protocol: "http:", + host: "127.0.0.1:6299", + }, + }); + }); + + afterEach(() => { + resetWebRemoteOAuthStorageCacheForTests(); + vi.unstubAllGlobals(); + }); + + it("wires RemoteOAuthStorage shared with getWebRemoteOAuthStorage", () => { + const redirectUrlProvider = new MutableRedirectUrlProvider(); + const first = createWebEnvironment("unit-test-token", redirectUrlProvider); + const second = createWebEnvironment("unit-test-token", redirectUrlProvider); + + expect(first.environment.oauth).toBeDefined(); + expect(second.environment.oauth).toBeDefined(); + expect(second.environment.oauth!.storage).toBe( + first.environment.oauth!.storage, + ); + expect(first.environment.oauth!.storage).toBe( + getWebRemoteOAuthStorage("unit-test-token"), + ); + }); + + it("uses BrowserNavigation for oauth.navigation", () => { + const { environment } = createWebEnvironment( + undefined, + new MutableRedirectUrlProvider(), + ); + expect(environment.oauth).toBeDefined(); + expect(environment.oauth!.navigation).toBeInstanceOf(BrowserNavigation); + }); + + it("returns the same logger instance as environment.logger", () => { + const { environment, logger } = createWebEnvironment( + "tok", + new MutableRedirectUrlProvider(), + ); + expect(logger).toBe(environment.logger); + }); + + it("passes redirectUrlProvider into oauth config", () => { + const redirectUrlProvider = new MutableRedirectUrlProvider(); + redirectUrlProvider.redirectUrl = "http://127.0.0.1:6299/oauth/callback"; + const { environment } = createWebEnvironment("tok", redirectUrlProvider); + if (!environment.oauth) { + throw new Error("expected oauth config"); + } + const { redirectUrlProvider: oauthRedirect } = environment.oauth; + if (!oauthRedirect) { + throw new Error("expected redirectUrlProvider"); + } + expect(oauthRedirect.getRedirectUrl()).toBe( + "http://127.0.0.1:6299/oauth/callback", + ); + }); + + it("forwards onBeforeOAuthRedirect to BrowserNavigation", () => { + const onBeforeOAuthRedirect = vi.fn<(authorizationUrl: URL) => void>(); + const { environment } = createWebEnvironment( + "tok", + new MutableRedirectUrlProvider(), + onBeforeOAuthRedirect, + ); + if (!environment.oauth) { + throw new Error("expected oauth config"); + } + const { navigation } = environment.oauth; + if (!navigation) { + throw new Error("expected navigation"); + } + const authUrl = new URL("https://idp.example/authorize?state=abc"); + navigation.navigateToAuthorization(authUrl); + expect(onBeforeOAuthRedirect).toHaveBeenCalledWith(authUrl); + }); + + it("routes fetch through the wrapped global fetch at the window origin", async () => { + const remoteBody = { + ok: true, + status: 200, + statusText: "OK", + headers: { "content-type": "text/plain" }, + body: "echoed", + }; + const fetchMock = vi.fn().mockImplementation( + async () => + new Response(JSON.stringify(remoteBody), { + status: 200, + headers: { "content-type": "application/json" }, + }), + ); + vi.stubGlobal("fetch", fetchMock); + + const { environment } = createWebEnvironment( + "api-tok", + new MutableRedirectUrlProvider(), + ); + fetchMock.mockClear(); + + const res = await environment.fetch!("http://upstream.test/mcp"); + + expect(res.status).toBe(200); + expect(await res.text()).toBe("echoed"); + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(String(fetchMock.mock.calls[0]?.[0])).toBe( + "http://127.0.0.1:6299/api/fetch", + ); + }); +}); diff --git a/clients/web/src/lib/environmentFactory.ts b/clients/web/src/lib/environmentFactory.ts index e64cadef5..3b08460bb 100644 --- a/clients/web/src/lib/environmentFactory.ts +++ b/clients/web/src/lib/environmentFactory.ts @@ -4,11 +4,9 @@ import { createRemoteFetch, createRemoteLogger, } from "@inspector/core/mcp/remote/index.js"; -import { - getBrowserOAuthStorage, - BrowserNavigation, -} from "@inspector/core/auth/browser/index.js"; +import { BrowserNavigation } from "@inspector/core/auth/browser/index.js"; import type { RedirectUrlProvider } from "@inspector/core/auth/index.js"; +import { getWebRemoteOAuthStorage } from "./remoteOAuthStorage.js"; export interface WebEnvironmentResult { environment: InspectorClientEnvironment; @@ -20,16 +18,17 @@ export interface WebEnvironmentResult { * - transport / fetch / logger all routed through the in-process Hono * backend at `window.location.origin` (the `clients/web/server` * dev-backend wires this in `/api/*`). - * - OAuth storage + navigation use the `BrowserOAuthStorage` (sessionStorage) - * and `BrowserNavigation` (full-page redirect) adapters. + * - OAuth storage uses `RemoteOAuthStorage` (shared `oauth.json` via + * `/api/storage/oauth`); navigation uses `BrowserNavigation`. * * 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 * reaching back through the client. * - * `authToken` is read from a higher level (currently unused in this app since - * v2 has no auth-token UI yet, but kept in the signature so the wiring is - * ready when token plumbing lands). + * `authToken` is supplied by `App.tsx` (`getAuthToken()`) and forwarded to + * remote transport/fetch/logger and `getWebRemoteOAuthStorage` so every + * `/api/*` call (including `/api/storage/oauth`) carries `x-mcp-remote-auth` + * when the backend requires it. * * `onBeforeOAuthRedirect` runs synchronously immediately before the OAuth * full-page redirect (see `BrowserNavigation`). The app uses it to flush the @@ -67,7 +66,7 @@ export function createWebEnvironment( }), logger, oauth: { - storage: getBrowserOAuthStorage(), + storage: getWebRemoteOAuthStorage(authToken), navigation: new BrowserNavigation(undefined, onBeforeOAuthRedirect), redirectUrlProvider, }, diff --git a/clients/web/src/lib/remoteOAuthStorage.test.ts b/clients/web/src/lib/remoteOAuthStorage.test.ts new file mode 100644 index 000000000..0077480c2 --- /dev/null +++ b/clients/web/src/lib/remoteOAuthStorage.test.ts @@ -0,0 +1,66 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { + getRemoteOAuthStorage, + getWebRemoteOAuthStorage, + resetWebRemoteOAuthStorageCacheForTests, +} from "./remoteOAuthStorage"; + +describe("remoteOAuthStorage", () => { + afterEach(() => { + resetWebRemoteOAuthStorageCacheForTests(); + }); + + it("returns one RemoteOAuthStorage instance per cache key", () => { + const a = getRemoteOAuthStorage({ + baseUrl: "http://127.0.0.1:6277", + authToken: "tok-a", + }); + const aAgain = getRemoteOAuthStorage({ + baseUrl: "http://127.0.0.1:6277", + authToken: "tok-a", + }); + const b = getRemoteOAuthStorage({ + baseUrl: "http://127.0.0.1:6277", + authToken: "tok-b", + }); + + expect(aAgain).toBe(a); + expect(b).not.toBe(a); + }); + + it("getWebRemoteOAuthStorage uses window.location origin", () => { + vi.stubGlobal("window", { + location: { + protocol: "http:", + host: "127.0.0.1:6299", + }, + }); + + const storage = getWebRemoteOAuthStorage("smoke-web-token"); + const again = getWebRemoteOAuthStorage("smoke-web-token"); + + expect(again).toBe(storage); + vi.unstubAllGlobals(); + }); + + it("throws when window is unavailable", () => { + vi.stubGlobal("window", undefined); + expect(() => getWebRemoteOAuthStorage()).toThrow( + "getWebRemoteOAuthStorage requires a browser environment", + ); + vi.unstubAllGlobals(); + }); + + it("creates a new instance after the test cache reset", () => { + const first = getRemoteOAuthStorage({ + baseUrl: "http://127.0.0.1:6277", + authToken: "reset-me", + }); + resetWebRemoteOAuthStorageCacheForTests(); + const second = getRemoteOAuthStorage({ + baseUrl: "http://127.0.0.1:6277", + authToken: "reset-me", + }); + expect(second).not.toBe(first); + }); +}); diff --git a/clients/web/src/lib/remoteOAuthStorage.ts b/clients/web/src/lib/remoteOAuthStorage.ts new file mode 100644 index 000000000..0d95806b0 --- /dev/null +++ b/clients/web/src/lib/remoteOAuthStorage.ts @@ -0,0 +1,55 @@ +import { RemoteOAuthStorage } from "@inspector/core/auth/remote/storage-remote.js"; + +export interface WebRemoteOAuthStorageOptions { + baseUrl: string; + authToken?: string; +} + +let cached: { cacheKey: string; storage: RemoteOAuthStorage } | undefined; + +function buildCacheKey(options: WebRemoteOAuthStorageOptions): string { + return `${options.baseUrl}\0${options.authToken ?? ""}`; +} + +const defaultFetch: typeof fetch = (...args) => globalThis.fetch(...args); + +/** + * Shared web OAuth store: `RemoteOAuthStorage` → `GET/POST /api/storage/oauth` + * → `~/.mcp-inspector/storage/oauth.json` (same file as CLI/TUI). + * + * Memoized by `{ baseUrl, authToken }` so connect, EMA IdP session, and + * per-server clear all mutate the same in-memory view. + */ +export function getRemoteOAuthStorage( + options: WebRemoteOAuthStorageOptions, +): RemoteOAuthStorage { + const cacheKey = buildCacheKey(options); + if (cached?.cacheKey === cacheKey) { + return cached.storage; + } + cached = { + cacheKey, + storage: new RemoteOAuthStorage({ + baseUrl: options.baseUrl, + authToken: options.authToken, + fetchFn: defaultFetch, + }), + }; + return cached.storage; +} + +/** Current origin + optional API token (see `getAuthToken()` in App.tsx). */ +export function getWebRemoteOAuthStorage( + authToken?: string, +): RemoteOAuthStorage { + if (typeof window === "undefined") { + throw new Error("getWebRemoteOAuthStorage requires a browser environment"); + } + const baseUrl = `${window.location.protocol}//${window.location.host}`; + return getRemoteOAuthStorage({ baseUrl, authToken }); +} + +/** @internal Vitest isolation */ +export function resetWebRemoteOAuthStorageCacheForTests(): void { + cached = undefined; +} diff --git a/clients/web/src/test/core/auth/connection-state.test.ts b/clients/web/src/test/core/auth/connection-state.test.ts index 62716e476..d060439b5 100644 --- a/clients/web/src/test/core/auth/connection-state.test.ts +++ b/clients/web/src/test/core/auth/connection-state.test.ts @@ -22,6 +22,7 @@ function createStorage( }> = {}, ): OAuthStorage { return { + load: vi.fn().mockResolvedValue(undefined), getTokens: vi.fn().mockResolvedValue(overrides.tokens), getClientInformation: vi.fn(async (_url, isPreregistered) => isPreregistered ? overrides.preregistered : overrides.dynamic, diff --git a/clients/web/src/test/core/auth/ema/emaFlow.test.ts b/clients/web/src/test/core/auth/ema/emaFlow.test.ts index 37ef84a12..d88109cb8 100644 --- a/clients/web/src/test/core/auth/ema/emaFlow.test.ts +++ b/clients/web/src/test/core/auth/ema/emaFlow.test.ts @@ -43,6 +43,7 @@ function createMemoryStorage( const clientInfoByKey: Record = {}; const codeVerifierByKey: Record = {}; return { + load: vi.fn().mockResolvedValue(undefined), getIdpSession: vi.fn(async (issuer: string) => idpSessions[issuer]), saveIdpSession: vi.fn(async (issuer: string, updates) => { idpSessions[issuer] = { ...idpSessions[issuer], ...updates }; diff --git a/clients/web/src/test/core/auth/ema/idpOidc.test.ts b/clients/web/src/test/core/auth/ema/idpOidc.test.ts index 0831b4db7..baebb8453 100644 --- a/clients/web/src/test/core/auth/ema/idpOidc.test.ts +++ b/clients/web/src/test/core/auth/ema/idpOidc.test.ts @@ -29,6 +29,7 @@ describe("idpOidc refresh", () => { beforeEach(() => { Object.keys(idpSessions).forEach((k) => delete idpSessions[k]); storage = { + load: vi.fn().mockResolvedValue(undefined), getIdpSession: vi.fn(async (issuer: string) => idpSessions[issuer]), saveIdpSession: vi.fn(async (issuer: string, updates) => { idpSessions[issuer] = { ...idpSessions[issuer], ...updates }; @@ -264,6 +265,8 @@ describe("startIdpOidcAuthorization", () => { beforeEach(() => { Object.keys(saved).forEach((k) => delete saved[k]); storage = { + load: vi.fn().mockResolvedValue(undefined), + getServerMetadata: vi.fn(() => null), saveCodeVerifier: vi.fn(async (key: string, value: string) => { saved[`cv:${key}`] = value; }), @@ -331,6 +334,7 @@ describe("completeIdpOidcAuthorization", () => { function buildStorage(overrides: Partial = {}): OAuthStorage { return { + load: vi.fn().mockResolvedValue(undefined), getServerMetadata: vi.fn(() => minimalOAuthAsMetadata(IDP_ISSUER)), getClientInformation: vi.fn(async () => ({ client_id: "idp-client", diff --git a/clients/web/src/test/core/auth/ema/idpSession.test.ts b/clients/web/src/test/core/auth/ema/idpSession.test.ts index df7eb6163..11ef63e72 100644 --- a/clients/web/src/test/core/auth/ema/idpSession.test.ts +++ b/clients/web/src/test/core/auth/ema/idpSession.test.ts @@ -19,6 +19,7 @@ describe("idpSession", () => { beforeEach(() => { storage = { + load: vi.fn().mockResolvedValue(undefined), getIdpSession: vi.fn(), saveIdpSession: vi.fn(), clearIdpSession: vi.fn(), @@ -73,15 +74,15 @@ describe("idpSession", () => { ); }); - it("clearEmaIdpSession clears idp session, leg-1 key, and tagged resource servers", () => { - clearEmaIdpSession(storage, "https://idp.test/"); + it("clearEmaIdpSession clears idp session, leg-1 key, and tagged resource servers", async () => { + await clearEmaIdpSession(storage, "https://idp.test/"); expect(storage.clearIdpSession).toHaveBeenCalledWith("https://idp.test"); expect(storage.clear).toHaveBeenCalledWith("ema-idp:https://idp.test"); expect(storage.clearEnterpriseManagedResourceServers).toHaveBeenCalled(); }); - it("clearEmaIdpSession no-ops when issuer normalizes to empty", () => { - clearEmaIdpSession(storage, ""); + it("clearEmaIdpSession no-ops when issuer normalizes to empty", async () => { + await clearEmaIdpSession(storage, ""); expect(storage.clearIdpSession).not.toHaveBeenCalled(); expect(storage.clear).not.toHaveBeenCalled(); expect( diff --git a/clients/web/src/test/core/auth/providers.test.ts b/clients/web/src/test/core/auth/providers.test.ts index d20f1b544..d7270584c 100644 --- a/clients/web/src/test/core/auth/providers.test.ts +++ b/clients/web/src/test/core/auth/providers.test.ts @@ -213,6 +213,7 @@ describe("OAuthNavigation", () => { function makeStorage(): OAuthStorage { return { + load: vi.fn().mockResolvedValue(undefined), getScope: vi.fn(() => undefined), getClientInformation: vi.fn(async () => undefined), saveClientInformation: vi.fn(async () => undefined), @@ -240,11 +241,11 @@ describe("OAuthNavigation", () => { return new BaseOAuthClientProvider(SERVER, config); } - it("clear() delegates to storage.clear with the server url", () => { + it("clear() delegates to storage.clear with the server url", async () => { const storage = makeStorage(); const provider = makeProvider(storage); - provider.clear(); + await provider.clear(); expect(storage.clear).toHaveBeenCalledWith(SERVER); }); diff --git a/clients/web/src/test/core/auth/storage-browser.test.ts b/clients/web/src/test/core/auth/storage-browser.test.ts index f009b4bd4..52afc0348 100644 --- a/clients/web/src/test/core/auth/storage-browser.test.ts +++ b/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,14 @@ const mockSessionStorage = new MockSessionStorage(); (global as typeof globalThis & { sessionStorage?: Storage }).sessionStorage = mockSessionStorage; +describe("getBrowserOAuthStorage", () => { + it("returns the same singleton on repeated calls", () => { + const first = getBrowserOAuthStorage(); + const second = getBrowserOAuthStorage(); + expect(second).toBe(first); + }); +}); + describe("BrowserOAuthStorage", () => { let storage: BrowserOAuthStorage; const testServerUrl = "http://localhost:3000"; @@ -66,7 +77,7 @@ describe("BrowserOAuthStorage", () => { client_secret: "test-secret", }; - storage.saveClientInformation(testServerUrl, clientInfo, { + await storage.saveClientInformation(testServerUrl, clientInfo, { registrationKind: "dcr", }); const result = await storage.getClientInformation(testServerUrl); @@ -82,7 +93,7 @@ describe("BrowserOAuthStorage", () => { // Use the storage API instead of manually setting sessionStorage // since BrowserOAuthStorage now uses Zustand with a different storage format - storage.savePreregisteredClientInformation( + await storage.savePreregisteredClientInformation( testServerUrl, preregisteredInfo, ); @@ -102,7 +113,7 @@ describe("BrowserOAuthStorage", () => { client_id: "test-client-id", }; - storage.saveClientInformation(testServerUrl, clientInfo, { + await storage.saveClientInformation(testServerUrl, clientInfo, { registrationKind: "dcr", }); const result = await storage.getClientInformation(testServerUrl); @@ -119,10 +130,10 @@ describe("BrowserOAuthStorage", () => { client_id: "second-id", }; - storage.saveClientInformation(testServerUrl, firstInfo, { + await storage.saveClientInformation(testServerUrl, firstInfo, { registrationKind: "dcr", }); - storage.saveClientInformation(testServerUrl, secondInfo, { + await storage.saveClientInformation(testServerUrl, secondInfo, { registrationKind: "cimd", }); const result = await storage.getClientInformation(testServerUrl); @@ -152,7 +163,7 @@ describe("BrowserOAuthStorage", () => { expires_in: 3600, }; - storage.saveTokens(testServerUrl, tokens); + await storage.saveTokens(testServerUrl, tokens); const result = await storage.getTokens(testServerUrl); expect(result).toEqual(tokens); @@ -166,7 +177,7 @@ describe("BrowserOAuthStorage", () => { token_type: "Bearer", }; - storage.saveTokens(testServerUrl, tokens); + await storage.saveTokens(testServerUrl, tokens); const result = await storage.getTokens(testServerUrl); expect(result).toEqual(tokens); @@ -183,7 +194,7 @@ describe("BrowserOAuthStorage", () => { await storage.saveTokens(emaUrl, tokens, { enterpriseManaged: true }); await storage.saveTokens(standardUrl, tokens); - storage.clearEnterpriseManagedResourceServers(); + await storage.clearEnterpriseManagedResourceServers(); expect(await storage.getTokens(emaUrl)).toBeUndefined(); expect(await storage.getTokens(standardUrl)).toEqual(tokens); @@ -199,7 +210,7 @@ describe("BrowserOAuthStorage", () => { it("should return stored code verifier", async () => { const codeVerifier = "test-code-verifier"; - storage.saveCodeVerifier(testServerUrl, codeVerifier); + await storage.saveCodeVerifier(testServerUrl, codeVerifier); const result = await storage.getCodeVerifier(testServerUrl); expect(result).toBe(codeVerifier); @@ -210,7 +221,7 @@ describe("BrowserOAuthStorage", () => { it("should save code verifier", async () => { const codeVerifier = "test-code-verifier"; - storage.saveCodeVerifier(testServerUrl, codeVerifier); + await storage.saveCodeVerifier(testServerUrl, codeVerifier); const result = await storage.getCodeVerifier(testServerUrl); expect(result).toBe(codeVerifier); @@ -226,7 +237,7 @@ describe("BrowserOAuthStorage", () => { it("should return stored scope", async () => { const scope = "read write"; - storage.saveScope(testServerUrl, scope); + await storage.saveScope(testServerUrl, scope); const result = await storage.getScope(testServerUrl); expect(result).toBe(scope); @@ -237,7 +248,7 @@ describe("BrowserOAuthStorage", () => { it("should save scope", async () => { const scope = "read write"; - storage.saveScope(testServerUrl, scope); + await storage.saveScope(testServerUrl, scope); const result = await storage.getScope(testServerUrl); expect(result).toBe(scope); @@ -258,7 +269,7 @@ describe("BrowserOAuthStorage", () => { response_types_supported: ["code"], }; - storage.saveServerMetadata(testServerUrl, metadata); + await storage.saveServerMetadata(testServerUrl, metadata); const result = await storage.getServerMetadata(testServerUrl); expect(result).toEqual(metadata); @@ -274,7 +285,7 @@ describe("BrowserOAuthStorage", () => { response_types_supported: ["code"], }; - storage.saveServerMetadata(testServerUrl, metadata); + await storage.saveServerMetadata(testServerUrl, metadata); const result = await storage.getServerMetadata(testServerUrl); expect(result).toEqual(metadata); @@ -283,7 +294,7 @@ describe("BrowserOAuthStorage", () => { describe("clearClientInformation", () => { it("removes the dynamically-registered client info by default", async () => { - storage.saveClientInformation( + await storage.saveClientInformation( testServerUrl, { client_id: "dyn" }, { registrationKind: "dcr" }, @@ -291,18 +302,18 @@ describe("BrowserOAuthStorage", () => { expect(await storage.getClientInformation(testServerUrl)).toEqual({ client_id: "dyn", }); - storage.clearClientInformation(testServerUrl); + await storage.clearClientInformation(testServerUrl); expect(await storage.getClientInformation(testServerUrl)).toBeUndefined(); }); it("removes the preregistered client info when isPreregistered=true", async () => { - storage.savePreregisteredClientInformation(testServerUrl, { + await storage.savePreregisteredClientInformation(testServerUrl, { client_id: "pre", }); expect(await storage.getClientInformation(testServerUrl, true)).toEqual({ client_id: "pre", }); - storage.clearClientInformation(testServerUrl, true); + await storage.clearClientInformation(testServerUrl, true); expect( await storage.getClientInformation(testServerUrl, true), ).toBeUndefined(); @@ -311,26 +322,26 @@ describe("BrowserOAuthStorage", () => { describe("individual clear methods", () => { it("clearTokens removes only tokens", async () => { - storage.saveTokens(testServerUrl, { + await storage.saveTokens(testServerUrl, { access_token: "t", token_type: "Bearer", }); expect(await storage.getTokens(testServerUrl)).toBeDefined(); - storage.clearTokens(testServerUrl); + await storage.clearTokens(testServerUrl); expect(await storage.getTokens(testServerUrl)).toBeUndefined(); }); it("clearCodeVerifier removes only the PKCE verifier", async () => { - storage.saveCodeVerifier(testServerUrl, "verifier"); + await storage.saveCodeVerifier(testServerUrl, "verifier"); expect(storage.getCodeVerifier(testServerUrl)).toBe("verifier"); - storage.clearCodeVerifier(testServerUrl); + await storage.clearCodeVerifier(testServerUrl); expect(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); + await storage.clearScope(testServerUrl); expect(storage.getScope(testServerUrl)).toBeUndefined(); }); @@ -341,9 +352,9 @@ describe("BrowserOAuthStorage", () => { token_endpoint: "http://localhost:3000/token", response_types_supported: ["code"], }; - storage.saveServerMetadata(testServerUrl, metadata); + await storage.saveServerMetadata(testServerUrl, metadata); expect(storage.getServerMetadata(testServerUrl)).toEqual(metadata); - storage.clearServerMetadata(testServerUrl); + await storage.clearServerMetadata(testServerUrl); expect(storage.getServerMetadata(testServerUrl)).toBeNull(); }); }); @@ -358,12 +369,12 @@ 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); + await storage.clear(testServerUrl); expect(await storage.getClientInformation(testServerUrl)).toBeUndefined(); expect(await storage.getTokens(testServerUrl)).toBeUndefined(); @@ -375,14 +386,14 @@ 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", }); - storage.clear(testServerUrl); + await storage.clear(testServerUrl); expect(await storage.getClientInformation(testServerUrl)).toBeUndefined(); expect(await storage.getClientInformation(otherServerUrl)).toEqual( @@ -404,10 +415,10 @@ describe("BrowserOAuthStorage", () => { client_id: "client-2", }; - storage.saveClientInformation(server1Url, clientInfo1, { + await storage.saveClientInformation(server1Url, clientInfo1, { registrationKind: "dcr", }); - storage.saveClientInformation(server2Url, clientInfo2, { + await storage.saveClientInformation(server2Url, clientInfo2, { registrationKind: "dcr", }); diff --git a/clients/web/src/test/core/auth/storage-remote.test.ts b/clients/web/src/test/core/auth/storage-remote.test.ts index c20617efc..be8536a52 100644 --- a/clients/web/src/test/core/auth/storage-remote.test.ts +++ b/clients/web/src/test/core/auth/storage-remote.test.ts @@ -1,5 +1,9 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; import { RemoteOAuthStorage } from "@inspector/core/auth/remote/storage-remote.js"; +import { + type OAuthStore, + waitForOAuthStorePersistLoad, +} from "@inspector/core/auth/store.js"; const NOOP_FETCH = vi.fn( async () => @@ -50,7 +54,7 @@ describe("RemoteOAuthStorage (unit, mocked fetch)", () => { { client_id: "dyn" }, { registrationKind: "dcr" }, ); - storage.clearClientInformation(serverUrl); + await storage.clearClientInformation(serverUrl); expect(await storage.getClientInformation(serverUrl)).toBeUndefined(); }); @@ -58,7 +62,7 @@ describe("RemoteOAuthStorage (unit, mocked fetch)", () => { await storage.savePreregisteredClientInformation(serverUrl, { client_id: "pre", }); - storage.clearClientInformation(serverUrl, true); + await storage.clearClientInformation(serverUrl, true); expect(await storage.getClientInformation(serverUrl, true)).toBeUndefined(); }); @@ -66,21 +70,21 @@ describe("RemoteOAuthStorage (unit, mocked fetch)", () => { const tokens = { access_token: "t", token_type: "Bearer" }; await storage.saveTokens(serverUrl, tokens); expect(await storage.getTokens(serverUrl)).toEqual(tokens); - storage.clearTokens(serverUrl); + await storage.clearTokens(serverUrl); expect(await storage.getTokens(serverUrl)).toBeUndefined(); }); it("codeVerifier round-trip and clearCodeVerifier", async () => { await storage.saveCodeVerifier(serverUrl, "verifier"); expect(storage.getCodeVerifier(serverUrl)).toBe("verifier"); - storage.clearCodeVerifier(serverUrl); + await storage.clearCodeVerifier(serverUrl); expect(storage.getCodeVerifier(serverUrl)).toBeUndefined(); }); it("scope round-trip and clearScope", async () => { await storage.saveScope(serverUrl, "read write"); expect(storage.getScope(serverUrl)).toBe("read write"); - storage.clearScope(serverUrl); + await storage.clearScope(serverUrl); expect(storage.getScope(serverUrl)).toBeUndefined(); }); @@ -93,7 +97,7 @@ describe("RemoteOAuthStorage (unit, mocked fetch)", () => { }; await storage.saveServerMetadata(serverUrl, md); expect(storage.getServerMetadata(serverUrl)).toEqual(md); - storage.clearServerMetadata(serverUrl); + await storage.clearServerMetadata(serverUrl); expect(storage.getServerMetadata(serverUrl)).toBeNull(); }); @@ -107,7 +111,7 @@ describe("RemoteOAuthStorage (unit, mocked fetch)", () => { access_token: "t", token_type: "Bearer", }); - storage.clear(serverUrl); + await storage.clear(serverUrl); expect(await storage.getClientInformation(serverUrl)).toBeUndefined(); expect(await storage.getTokens(serverUrl)).toBeUndefined(); }); @@ -120,4 +124,61 @@ describe("RemoteOAuthStorage (unit, mocked fetch)", () => { // No public accessor; constructing without throwing covers the default-branch. expect(s).toBeInstanceOf(RemoteOAuthStorage); }); + + it("waitForOAuthStorePersistLoad resolves for an unregistered store handle", async () => { + await expect( + waitForOAuthStorePersistLoad({} as OAuthStore), + ).resolves.toBeUndefined(); + }); + + it("load() waits for remote GET before sync reads see persisted state", async () => { + const persisted = { + state: { + servers: { + [serverUrl]: { codeVerifier: "persisted-verifier" }, + }, + idpSessions: {}, + }, + version: 0, + }; + let resolveFetch!: (value: Response) => void; + const delayedFetch = vi.fn( + () => + new Promise((resolve) => { + resolveFetch = resolve; + }), + ) as unknown as typeof fetch; + + const delayedStorage = new RemoteOAuthStorage({ + baseUrl: "http://remote.example", + storeId: `delayed-${Math.random().toString(36).slice(2)}`, + fetchFn: delayedFetch, + }); + + const loadPromise = delayedStorage.load(); + expect(delayedStorage.getCodeVerifier(serverUrl)).toBeUndefined(); + + resolveFetch(new Response(JSON.stringify(persisted), { status: 200 })); + await loadPromise; + + expect(delayedStorage.getCodeVerifier(serverUrl)).toBe( + "persisted-verifier", + ); + }); + + it("load() rejects when remote GET fails instead of hanging", async () => { + const failingFetch = vi.fn( + async () => new Response("server error", { status: 500 }), + ) as unknown as typeof fetch; + + const failingStorage = new RemoteOAuthStorage({ + baseUrl: "http://remote.example", + storeId: `fail-${Math.random().toString(36).slice(2)}`, + fetchFn: failingFetch, + }); + + await expect(failingStorage.load()).rejects.toThrow( + "Failed to read store: 500", + ); + }); }); diff --git a/clients/web/src/test/core/mcp/oauthManager.test.ts b/clients/web/src/test/core/mcp/oauthManager.test.ts index 02d764f25..b4bfa7bb8 100644 --- a/clients/web/src/test/core/mcp/oauthManager.test.ts +++ b/clients/web/src/test/core/mcp/oauthManager.test.ts @@ -35,6 +35,7 @@ function createMockParams( const dispatchOAuthError = vi.fn(); const storage = { + load: vi.fn().mockResolvedValue(undefined), getScope: vi.fn().mockReturnValue(undefined), getClientInformation: vi.fn().mockResolvedValue(undefined), getClientRegistrationKind: vi.fn().mockReturnValue(undefined), @@ -45,7 +46,7 @@ function createMockParams( saveTokens: vi.fn().mockResolvedValue(undefined), getCodeVerifier: vi.fn().mockReturnValue("verifier"), saveCodeVerifier: vi.fn().mockResolvedValue(undefined), - clear: vi.fn(), + clear: vi.fn().mockResolvedValue(undefined), clearClientInformation: vi.fn(), clearTokens: vi.fn(), clearCodeVerifier: vi.fn(), @@ -126,10 +127,10 @@ describe("OAuthManager", () => { }); describe("clearOAuthTokens", () => { - it("calls storage.clear(serverUrl) when storage is configured", () => { + it("calls storage.clear(serverUrl) when storage is configured", async () => { const params = createMockParams(); const manager = new OAuthManager(params); - manager.clearOAuthTokens(); + await manager.clearOAuthTokens(); expect(params.initialConfig.storage!.clear).toHaveBeenCalledWith( SERVER_URL, ); @@ -137,7 +138,7 @@ describe("OAuthManager", () => { expect(manager.getOAuthFlowStep()).toBeUndefined(); }); - it("no-ops when storage is not configured", () => { + it("no-ops when storage is not configured", async () => { const params = createMockParams({ initialConfig: { redirectUrlProvider: { @@ -147,7 +148,7 @@ describe("OAuthManager", () => { } as OAuthManagerConfig, }); const manager = new OAuthManager(params); - manager.clearOAuthTokens(); + await manager.clearOAuthTokens(); expect(params.getServerUrl).not.toHaveBeenCalled(); }); }); diff --git a/clients/web/src/test/core/react/useEmaIdpLoginState.test.tsx b/clients/web/src/test/core/react/useEmaIdpLoginState.test.tsx index f8b9c0e60..bc7f1d6b7 100644 --- a/clients/web/src/test/core/react/useEmaIdpLoginState.test.tsx +++ b/clients/web/src/test/core/react/useEmaIdpLoginState.test.tsx @@ -8,10 +8,13 @@ describe("useEmaIdpLoginState", () => { beforeEach(() => { storage = { + load: vi.fn().mockResolvedValue(undefined), getIdpSession: vi.fn().mockResolvedValue(undefined), - clearIdpSession: vi.fn(), - clear: vi.fn(), - clearEnterpriseManagedResourceServers: vi.fn(), + clearIdpSession: vi.fn().mockResolvedValue(undefined), + clear: vi.fn().mockResolvedValue(undefined), + clearEnterpriseManagedResourceServers: vi + .fn() + .mockResolvedValue(undefined), } as unknown as OAuthStorage; }); @@ -56,10 +59,12 @@ describe("useEmaIdpLoginState", () => { result.current.logout(); }); - expect(storage.clearIdpSession).toHaveBeenCalledWith("https://idp.test"); - expect(storage.clear).toHaveBeenCalledWith("ema-idp:https://idp.test"); - expect(storage.clearEnterpriseManagedResourceServers).toHaveBeenCalled(); - expect(result.current.loginState).toBe("none"); + await waitFor(() => { + expect(storage.clearIdpSession).toHaveBeenCalledWith("https://idp.test"); + expect(storage.clear).toHaveBeenCalledWith("ema-idp:https://idp.test"); + expect(storage.clearEnterpriseManagedResourceServers).toHaveBeenCalled(); + expect(result.current.loginState).toBe("none"); + }); }); it("reports 'expired' for an expired token with no refresh token", async () => { 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 2c72f8aa3..eb43eab3e 100644 --- a/clients/web/src/test/integration/auth/node/storage.test.ts +++ b/clients/web/src/test/integration/auth/node/storage.test.ts @@ -126,10 +126,10 @@ describe("NodeOAuthStorage", () => { client_id: "second-id", }; - storage.saveClientInformation(testServerUrl, firstInfo, { + await storage.saveClientInformation(testServerUrl, firstInfo, { registrationKind: "dcr", }); - storage.saveClientInformation(testServerUrl, secondInfo, { + await storage.saveClientInformation(testServerUrl, secondInfo, { registrationKind: "dcr", }); const result = await storage.getClientInformation(testServerUrl); @@ -310,7 +310,7 @@ describe("NodeOAuthStorage", () => { expect(await storage.getClientInformation(testServerUrl)).toEqual({ client_id: "dyn", }); - storage.clearClientInformation(testServerUrl); + await storage.clearClientInformation(testServerUrl); expect(await storage.getClientInformation(testServerUrl)).toBeUndefined(); }); @@ -321,7 +321,7 @@ describe("NodeOAuthStorage", () => { expect(await storage.getClientInformation(testServerUrl, true)).toEqual({ client_id: "pre", }); - storage.clearClientInformation(testServerUrl, true); + await storage.clearClientInformation(testServerUrl, true); expect( await storage.getClientInformation(testServerUrl, true), ).toBeUndefined(); @@ -335,21 +335,21 @@ describe("NodeOAuthStorage", () => { token_type: "Bearer", }); expect(await storage.getTokens(testServerUrl)).toBeDefined(); - storage.clearTokens(testServerUrl); + await storage.clearTokens(testServerUrl); expect(await storage.getTokens(testServerUrl)).toBeUndefined(); }); it("clearCodeVerifier removes only the PKCE verifier", async () => { await storage.saveCodeVerifier(testServerUrl, "verifier"); expect(storage.getCodeVerifier(testServerUrl)).toBe("verifier"); - storage.clearCodeVerifier(testServerUrl); + await storage.clearCodeVerifier(testServerUrl); expect(storage.getCodeVerifier(testServerUrl)).toBeUndefined(); }); it("clearScope removes only the scope", async () => { await storage.saveScope(testServerUrl, "read write"); expect(storage.getScope(testServerUrl)).toBe("read write"); - storage.clearScope(testServerUrl); + await storage.clearScope(testServerUrl); expect(storage.getScope(testServerUrl)).toBeUndefined(); }); @@ -362,7 +362,7 @@ describe("NodeOAuthStorage", () => { }; await storage.saveServerMetadata(testServerUrl, metadata); expect(storage.getServerMetadata(testServerUrl)).toEqual(metadata); - storage.clearServerMetadata(testServerUrl); + await storage.clearServerMetadata(testServerUrl); expect(storage.getServerMetadata(testServerUrl)).toBeNull(); }); }); @@ -382,7 +382,7 @@ describe("NodeOAuthStorage", () => { }); await storage.saveTokens(testServerUrl, tokens); - storage.clear(testServerUrl); + await storage.clear(testServerUrl); expect(await storage.getClientInformation(testServerUrl)).toBeUndefined(); expect(await storage.getTokens(testServerUrl)).toBeUndefined(); @@ -401,7 +401,7 @@ describe("NodeOAuthStorage", () => { registrationKind: "dcr", }); - storage.clear(testServerUrl); + await storage.clear(testServerUrl); expect(await storage.getClientInformation(testServerUrl)).toBeUndefined(); const otherResult = await storage.getClientInformation(otherServerUrl); @@ -424,10 +424,10 @@ describe("NodeOAuthStorage", () => { client_id: "client-2", }; - storage.saveClientInformation(server1Url, clientInfo1, { + await storage.saveClientInformation(server1Url, clientInfo1, { registrationKind: "dcr", }); - storage.saveClientInformation(server2Url, clientInfo2, { + await storage.saveClientInformation(server2Url, clientInfo2, { registrationKind: "dcr", }); @@ -547,7 +547,7 @@ describe("NodeOAuthStorage idpSessions (EMA)", () => { expect(session?.idToken).toBe("eyJ.id.token"); expect(session?.refreshToken).toBe("rt-1"); - storage.clearIdpSession(issuer); + await storage.clearIdpSession(issuer); await flushStoreFileWrites(testStatePath); expect(await storage.getIdpSession(issuer)).toBeUndefined(); }); @@ -635,12 +635,10 @@ describe("NodeOAuthStorage with custom storagePath", () => { ); try { - const defaultStore = getOAuthStore(); - defaultStore.getState().setServerState(testServerUrl, { - tokens: { - access_token: "default-token", - token_type: "Bearer", - }, + const defaultStorage = new NodeOAuthStorage(); + await defaultStorage.saveTokens(testServerUrl, { + access_token: "default-token", + token_type: "Bearer", }); const customStorage = new NodeOAuthStorage(customPath); @@ -652,11 +650,10 @@ describe("NodeOAuthStorage with custom storagePath", () => { const fromCustom = await customStorage.getTokens(testServerUrl); expect(fromCustom?.access_token).toBe("custom-token"); - const defaultStorage = new NodeOAuthStorage(); const fromDefault = await defaultStorage.getTokens(testServerUrl); expect(fromDefault?.access_token).toBe("default-token"); - defaultStore.getState().clearServerState(testServerUrl); + getOAuthStore().getState().clearServerState(testServerUrl); } finally { try { await fs.unlink(customPath); diff --git a/clients/web/src/test/integration/mcp/inspectorClient-oauth-e2e.test.ts b/clients/web/src/test/integration/mcp/inspectorClient-oauth-e2e.test.ts index b2ed94b01..f0aa91ce2 100644 --- a/clients/web/src/test/integration/mcp/inspectorClient-oauth-e2e.test.ts +++ b/clients/web/src/test/integration/mcp/inspectorClient-oauth-e2e.test.ts @@ -630,7 +630,7 @@ describe("InspectorClient OAuth E2E", () => { expect(client.getStatus()).toBe("connected"); await client.disconnect(); - client.clearOAuthTokens(); + await client.clearOAuthTokens(); const authUrlSecond = await client.authenticate(); if (!authUrlSecond) throw new Error("Expected authorization URL"); @@ -995,7 +995,7 @@ describe("InspectorClient OAuth E2E", () => { expect(tokens?.access_token).toBeDefined(); expect(await client.isOAuthAuthorized()).toBe(true); - client.clearOAuthTokens(); + await client.clearOAuthTokens(); expect(await client.isOAuthAuthorized()).toBe(false); expect(await client.getOAuthTokens()).toBeUndefined(); }); diff --git a/clients/web/src/test/integration/mcp/inspectorClient-oauth.test.ts b/clients/web/src/test/integration/mcp/inspectorClient-oauth.test.ts index 86d439fce..edab9e771 100644 --- a/clients/web/src/test/integration/mcp/inspectorClient-oauth.test.ts +++ b/clients/web/src/test/integration/mcp/inspectorClient-oauth.test.ts @@ -173,8 +173,8 @@ describe("InspectorClient OAuth", () => { expect(tokens).toBeUndefined(); }); - it("should clear OAuth tokens", () => { - client.clearOAuthTokens(); + it("should clear OAuth tokens", async () => { + await client.clearOAuthTokens(); // Should not throw expect(client).toBeDefined(); }); diff --git a/clients/web/src/test/integration/mcp/inspectorClient.test.ts b/clients/web/src/test/integration/mcp/inspectorClient.test.ts index 6b23727c9..c75d258f9 100644 --- a/clients/web/src/test/integration/mcp/inspectorClient.test.ts +++ b/clients/web/src/test/integration/mcp/inspectorClient.test.ts @@ -4949,7 +4949,7 @@ describe("InspectorClient", () => { expect(c.getOAuthFlowState()).toBeUndefined(); await expect(c.getOAuthState()).resolves.toBeUndefined(); // clearOAuthTokens is a no-op when there is no manager - expect(() => c.clearOAuthTokens()).not.toThrow(); + await expect(c.clearOAuthTokens()).resolves.toBeUndefined(); }); it("setOAuthConfig throws when oauthManager is unset", () => { diff --git a/clients/web/src/utils/clearServerOAuthState.test.ts b/clients/web/src/utils/clearServerOAuthState.test.ts index 6cdbfeec9..137bc9c31 100644 --- a/clients/web/src/utils/clearServerOAuthState.test.ts +++ b/clients/web/src/utils/clearServerOAuthState.test.ts @@ -1,22 +1,26 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; -import { getBrowserOAuthStorage } from "@inspector/core/auth/browser/index.js"; +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { BrowserOAuthStorage } from "@inspector/core/auth/browser/storage.js"; +import type { InspectorClient } from "@inspector/core/mcp/inspectorClient.js"; import { clearServerOAuthState } from "./clearServerOAuthState"; describe("clearServerOAuthState", () => { - beforeEach(() => { - getBrowserOAuthStorage().clear("https://mcp.example.com/mcp"); + let storage: BrowserOAuthStorage; + + beforeEach(async () => { + storage = new BrowserOAuthStorage(); + await storage.clear("https://mcp.example.com/mcp"); }); it("clears storage by server URL when not the active connection", async () => { - const storage = getBrowserOAuthStorage(); await storage.saveTokens("https://mcp.example.com/mcp", { access_token: "tok", token_type: "Bearer", }); - const cleared = clearServerOAuthState({ + const cleared = await clearServerOAuthState({ config: { type: "streamable-http", url: "https://mcp.example.com/mcp" }, isActiveConnection: false, + oauthStorage: storage, }); expect(cleared).toBe(true); @@ -25,27 +29,28 @@ describe("clearServerOAuthState", () => { ).toBeUndefined(); }); - it("uses the live client when clearing the active connection", () => { - const inspectorClient = { - clearOAuthTokens: vi.fn(), - }; + it("uses the live client when clearing the active connection", async () => { + const clearOAuthTokens = vi.fn(); + const inspectorClient = { clearOAuthTokens }; - const cleared = clearServerOAuthState({ + const cleared = await clearServerOAuthState({ config: { type: "streamable-http", url: "https://mcp.example.com/mcp" }, - inspectorClient: inspectorClient as never, + inspectorClient, isActiveConnection: true, + oauthStorage: storage, }); expect(cleared).toBe(true); - expect(inspectorClient.clearOAuthTokens).toHaveBeenCalledTimes(1); + expect(clearOAuthTokens).toHaveBeenCalledTimes(1); }); - it("returns false for stdio servers", () => { - expect( + it("returns false for stdio servers", async () => { + await expect( clearServerOAuthState({ config: { type: "stdio", command: "node", args: [] }, isActiveConnection: false, + oauthStorage: storage, }), - ).toBe(false); + ).resolves.toBe(false); }); }); diff --git a/clients/web/src/utils/clearServerOAuthState.ts b/clients/web/src/utils/clearServerOAuthState.ts index a3fd9d7cc..f7eb5233f 100644 --- a/clients/web/src/utils/clearServerOAuthState.ts +++ b/clients/web/src/utils/clearServerOAuthState.ts @@ -1,4 +1,4 @@ -import { getBrowserOAuthStorage } from "@inspector/core/auth/browser/index.js"; +import type { OAuthStorage } from "@inspector/core/auth/storage.js"; import { getOAuthServerUrl } from "@inspector/core/mcp/config.js"; import type { InspectorClient } from "@inspector/core/mcp/inspectorClient.js"; import type { MCPServerConfig } from "@inspector/core/mcp/types.js"; @@ -6,8 +6,10 @@ import type { MCPServerConfig } from "@inspector/core/mcp/types.js"; export interface ClearServerOAuthStateParams { config: MCPServerConfig; /** When set and this server is the active connection, clear via the live client. */ - inspectorClient?: InspectorClient | null; + inspectorClient?: Pick | null; isActiveConnection: boolean; + /** Shared web OAuth store; required so clear hits the same blob as connect. */ + oauthStorage: OAuthStorage; } /** @@ -15,18 +17,18 @@ export interface ClearServerOAuthStateParams { * HTTP MCP server. When clearing the active connection, uses the live client so * in-memory flow state is reset too. */ -export function clearServerOAuthState( +export async function clearServerOAuthState( params: ClearServerOAuthStateParams, -): boolean { +): Promise { const serverUrl = getOAuthServerUrl(params.config); if (!serverUrl) { return false; } if (params.isActiveConnection && params.inspectorClient) { - params.inspectorClient.clearOAuthTokens(); + await params.inspectorClient.clearOAuthTokens(); } else { - getBrowserOAuthStorage().clear(serverUrl); + await params.oauthStorage.clear(serverUrl); } return true; } diff --git a/clients/web/vite.config.ts b/clients/web/vite.config.ts index d3b99f939..e59e1f7d0 100644 --- a/clients/web/vite.config.ts +++ b/clients/web/vite.config.ts @@ -100,6 +100,7 @@ export default defineConfig(({ command }) => { reporter: ['text', 'html', 'json-summary'], include: [ 'src/components/**/*.{ts,tsx}', + 'src/lib/**/*.{ts,tsx}', 'src/utils/**/*.{ts,tsx}', 'clients/web/server/**/*.{ts,tsx}', path.join(repoRoot, 'core/mcp/**/*.{ts,tsx}'), diff --git a/core/auth/ema/idpOidc.ts b/core/auth/ema/idpOidc.ts index 8dff2ae6a..e860d7e0f 100644 --- a/core/auth/ema/idpOidc.ts +++ b/core/auth/ema/idpOidc.ts @@ -34,6 +34,7 @@ async function resolveIdpMetadata( storage: OAuthStorage, fetchFn?: typeof fetch, ): Promise { + await storage.load(); const storageKey = idpOAuthStorageKey(issuer); const cached = storage.getServerMetadata(storageKey); if (cached?.token_endpoint) { @@ -63,7 +64,7 @@ export async function startIdpOidcAuthorization(params: { fetchFn?: typeof fetch; }): Promise<{ authorizationUrl: URL }> { const issuer = normalizeIdpIssuer(params.idp.issuer); - const metadata = await discoverIdpMetadata(issuer, params.fetchFn); + const metadata = await resolveIdpMetadata(issuer, params.storage, params.fetchFn); const clientInformation = idpClientInformation(params.idp); const storageKey = idpOAuthStorageKey(issuer); const state = generateOAuthState(); @@ -101,6 +102,7 @@ export async function completeIdpOidcAuthorization(params: { }> { const issuer = normalizeIdpIssuer(params.idp.issuer); const storageKey = idpOAuthStorageKey(issuer); + await params.storage.load(); const metadata = params.storage.getServerMetadata(storageKey); if (!metadata) { throw new Error("IdP OAuth metadata not found — restart EMA IdP login"); @@ -131,7 +133,7 @@ export async function completeIdpOidcAuthorization(params: { throw new Error("IdP token response did not include an ID Token"); } - params.storage.clearCodeVerifier(storageKey); + await params.storage.clearCodeVerifier(storageKey); const idTokenExpiresAt = jwtExpiresAtMs(idToken); await params.storage.saveIdpSession(issuer, { idToken, diff --git a/core/auth/ema/idpSession.ts b/core/auth/ema/idpSession.ts index 21b475782..d736549ae 100644 --- a/core/auth/ema/idpSession.ts +++ b/core/auth/ema/idpSession.ts @@ -21,10 +21,13 @@ export async function getEmaIdpLoginState( return "expired"; } -export function clearEmaIdpSession(storage: OAuthStorage, issuer: string): void { +export async function clearEmaIdpSession( + storage: OAuthStorage, + issuer: string, +): Promise { const normalized = normalizeIdpIssuer(issuer); if (!normalized) return; - storage.clearIdpSession(normalized); - storage.clear(idpOAuthStorageKey(normalized)); - storage.clearEnterpriseManagedResourceServers(); + await storage.clearIdpSession(normalized); + await storage.clear(idpOAuthStorageKey(normalized)); + await storage.clearEnterpriseManagedResourceServers(); } diff --git a/core/auth/oauth-storage.ts b/core/auth/oauth-storage.ts index f2246112a..64c295345 100644 --- a/core/auth/oauth-storage.ts +++ b/core/auth/oauth-storage.ts @@ -8,7 +8,11 @@ import { OAuthTokensSchema, } from "@modelcontextprotocol/sdk/shared/auth.js"; import type { OAuthStorage } from "./storage.js"; -import { type createOAuthStore, type ServerOAuthState } from "./store.js"; +import { + type OAuthStore, + type ServerOAuthState, + waitForOAuthStorePersistLoad, +} from "./store.js"; import type { IdpSessionState, OAuthClientRegistrationKind, @@ -16,22 +20,46 @@ import type { SaveTokensOptions, } from "./storage.js"; +type OAuthStoreWithPersist = OAuthStore & { + persist: { + hasHydrated: () => boolean; + }; +}; + /** * Concrete OAuthStorage implementation parameterized on a Zustand store. * The store carries the storage adapter (sessionStorage, file, remote HTTP, …), * so the same body works for browser, Node, and remote environments. */ export class OAuthStorageBase implements OAuthStorage { - private readonly store: ReturnType; + private readonly store: OAuthStoreWithPersist; + private loadedPromise: Promise | undefined; + + constructor(store: OAuthStore) { + this.store = store as OAuthStoreWithPersist; + } + + load(): Promise { + if (!this.loadedPromise) { + this.loadedPromise = (async () => { + if (this.store.persist.hasHydrated()) { + return; + } + await waitForOAuthStorePersistLoad(this.store); + })(); + } + return this.loadedPromise; + } - constructor(store: ReturnType) { - this.store = store; + private async ensureLoaded(): Promise { + await this.load(); } async getClientInformation( serverUrl: string, isPreregistered?: boolean, ): Promise { + await this.ensureLoaded(); const state = this.store.getState().getServerState(serverUrl); const clientInfo = isPreregistered ? state.preregisteredClientInformation @@ -56,6 +84,7 @@ export class OAuthStorageBase implements OAuthStorage { clientInformation: OAuthClientInformation, options: SaveClientInformationOptions, ): Promise { + await this.ensureLoaded(); this.store.getState().setServerState(serverUrl, { clientInformation, clientRegistrationKind: options.registrationKind, @@ -66,13 +95,18 @@ export class OAuthStorageBase implements OAuthStorage { serverUrl: string, clientInformation: OAuthClientInformation, ): Promise { + await this.ensureLoaded(); this.store.getState().setServerState(serverUrl, { preregisteredClientInformation: clientInformation, clientRegistrationKind: "static", }); } - clearClientInformation(serverUrl: string, isPreregistered?: boolean): void { + async clearClientInformation( + serverUrl: string, + isPreregistered?: boolean, + ): Promise { + await this.ensureLoaded(); const updates: Partial = {}; if (isPreregistered) { @@ -86,6 +120,7 @@ export class OAuthStorageBase implements OAuthStorage { } async getTokens(serverUrl: string): Promise { + await this.ensureLoaded(); const state = this.store.getState().getServerState(serverUrl); if (!state.tokens) { return undefined; @@ -99,13 +134,15 @@ export class OAuthStorageBase implements OAuthStorage { tokens: OAuthTokens, options?: SaveTokensOptions, ): Promise { + await this.ensureLoaded(); this.store.getState().setServerState(serverUrl, { tokens, ...(options?.enterpriseManaged === true && { enterpriseManaged: true }), }); } - clearTokens(serverUrl: string): void { + async clearTokens(serverUrl: string): Promise { + await this.ensureLoaded(); this.store.getState().setServerState(serverUrl, { tokens: undefined }); } @@ -118,10 +155,12 @@ export class OAuthStorageBase implements OAuthStorage { serverUrl: string, codeVerifier: string, ): Promise { + await this.ensureLoaded(); this.store.getState().setServerState(serverUrl, { codeVerifier }); } - clearCodeVerifier(serverUrl: string): void { + async clearCodeVerifier(serverUrl: string): Promise { + await this.ensureLoaded(); this.store .getState() .setServerState(serverUrl, { codeVerifier: undefined }); @@ -133,10 +172,12 @@ export class OAuthStorageBase implements OAuthStorage { } async saveScope(serverUrl: string, scope: string | undefined): Promise { + await this.ensureLoaded(); this.store.getState().setServerState(serverUrl, { scope }); } - clearScope(serverUrl: string): void { + async clearScope(serverUrl: string): Promise { + await this.ensureLoaded(); this.store.getState().setServerState(serverUrl, { scope: undefined }); } @@ -149,22 +190,26 @@ export class OAuthStorageBase implements OAuthStorage { serverUrl: string, metadata: OAuthMetadata, ): Promise { + await this.ensureLoaded(); this.store .getState() .setServerState(serverUrl, { serverMetadata: metadata }); } - clearServerMetadata(serverUrl: string): void { + async clearServerMetadata(serverUrl: string): Promise { + await this.ensureLoaded(); this.store .getState() .setServerState(serverUrl, { serverMetadata: undefined }); } - clear(serverUrl: string): void { + async clear(serverUrl: string): Promise { + await this.ensureLoaded(); this.store.getState().clearServerState(serverUrl); } async getIdpSession(issuer: string): Promise { + await this.ensureLoaded(); const session = this.store.getState().getIdpSession(issuer); if ( !session.idToken && @@ -180,14 +225,17 @@ export class OAuthStorageBase implements OAuthStorage { issuer: string, session: Partial, ): Promise { + await this.ensureLoaded(); this.store.getState().setIdpSession(issuer, session); } - clearIdpSession(issuer: string): void { + async clearIdpSession(issuer: string): Promise { + await this.ensureLoaded(); this.store.getState().clearIdpSession(issuer); } - clearEnterpriseManagedResourceServers(): void { + async clearEnterpriseManagedResourceServers(): Promise { + await this.ensureLoaded(); this.store.getState().clearEnterpriseManagedResourceServers(); } } diff --git a/core/auth/providers.ts b/core/auth/providers.ts index 4b3007283..6e4c46ad3 100644 --- a/core/auth/providers.ts +++ b/core/auth/providers.ts @@ -247,8 +247,8 @@ export class BaseOAuthClientProvider implements OAuthClientProvider { return verifier; } - clear(): void { - this.storage.clear(this.serverUrl); + async clear(): Promise { + await this.storage.clear(this.serverUrl); } getServerMetadata(): OAuthMetadata | null { diff --git a/core/auth/storage.ts b/core/auth/storage.ts index f8b46874a..441821e8a 100644 --- a/core/auth/storage.ts +++ b/core/auth/storage.ts @@ -21,6 +21,13 @@ export interface SaveClientInformationOptions { } export interface OAuthStorage { + /** + * Load persisted OAuth state into memory. Resolves when the backing store + * (file, remote API, sessionStorage, …) has been read. Call before relying on + * sync reads, or await mutating methods which call this internally. + */ + load(): Promise; + /** * Get client information (preregistered or dynamically registered) */ @@ -56,7 +63,10 @@ export interface OAuthStorage { /** * Clear client information */ - clearClientInformation(serverUrl: string, isPreregistered?: boolean): void; + clearClientInformation( + serverUrl: string, + isPreregistered?: boolean, + ): Promise; /** * Get OAuth tokens @@ -75,7 +85,7 @@ export interface OAuthStorage { /** * Clear OAuth tokens */ - clearTokens(serverUrl: string): void; + clearTokens(serverUrl: string): Promise; /** * Get code verifier (for PKCE) @@ -90,7 +100,7 @@ export interface OAuthStorage { /** * Clear code verifier */ - clearCodeVerifier(serverUrl: string): void; + clearCodeVerifier(serverUrl: string): Promise; /** * Get scope @@ -105,7 +115,7 @@ export interface OAuthStorage { /** * Clear scope */ - clearScope(serverUrl: string): void; + clearScope(serverUrl: string): Promise; /** * Get server metadata discovered during OAuth @@ -120,12 +130,12 @@ export interface OAuthStorage { /** * Clear server metadata */ - clearServerMetadata(serverUrl: string): void; + clearServerMetadata(serverUrl: string): Promise; /** * Clear all OAuth data for a server */ - clear(serverUrl: string): void; + clear(serverUrl: string): Promise; /** * Get cached IdP OIDC session for EMA (keyed by issuer). @@ -143,12 +153,12 @@ export interface OAuthStorage { /** * Clear cached IdP session for an issuer. */ - clearIdpSession(issuer: string): void; + clearIdpSession(issuer: string): Promise; /** * Remove per-server OAuth state for MCP servers whose tokens were minted via EMA. */ - clearEnterpriseManagedResourceServers(): void; + clearEnterpriseManagedResourceServers(): Promise; } /** diff --git a/core/auth/store.ts b/core/auth/store.ts index a54bfb309..52d966c3c 100644 --- a/core/auth/store.ts +++ b/core/auth/store.ts @@ -43,6 +43,23 @@ export interface OAuthStoreState { clearEnterpriseManagedResourceServers: () => void; } +export type OAuthStore = ReturnType; + +const persistLoadByStore = new WeakMap>(); + +/** + * Wait until the store's initial persist read has finished (success or failure). + * Used by {@link OAuthStorageBase.load}; resolves on success, rejects when the + * storage adapter's getItem fails (Zustand does not fire onFinishHydration on error). + */ +export function waitForOAuthStorePersistLoad(store: OAuthStore): Promise { + const promise = persistLoadByStore.get(store); + if (!promise) { + return Promise.resolve(); + } + return promise; +} + /** * Creates a Zustand store for OAuth state with the given storage adapter. * The storage adapter handles persistence (file, remote HTTP, sessionStorage, etc.). @@ -53,7 +70,29 @@ export interface OAuthStoreState { export function createOAuthStore( storage: ReturnType, ) { - return createStore()( + let resolvePersistLoad!: () => void; + let rejectPersistLoad!: (error: unknown) => void; + let persistLoadSettled = false; + + const persistLoadPromise = new Promise((resolve, reject) => { + resolvePersistLoad = resolve; + rejectPersistLoad = reject; + }); + + const settlePersistLoad = (error?: unknown) => { + if (persistLoadSettled) { + /* v8 ignore next -- idempotent if persist signals completion more than once */ + return; + } + persistLoadSettled = true; + if (error) { + rejectPersistLoad(error); + return; + } + resolvePersistLoad(); + }; + + const store = createStore()( persist( (set, get) => ({ servers: {}, @@ -118,7 +157,13 @@ export function createOAuthStore( { name: "mcp-inspector-oauth", storage, + onRehydrateStorage: () => (_state, error) => { + settlePersistLoad(error); + }, }, ), ); + + persistLoadByStore.set(store, persistLoadPromise); + return store; } diff --git a/core/mcp/inspectorClient.ts b/core/mcp/inspectorClient.ts index 601fc35b3..e51d98877 100644 --- a/core/mcp/inspectorClient.ts +++ b/core/mcp/inspectorClient.ts @@ -3064,8 +3064,8 @@ export class InspectorClient extends InspectorClientEventTarget { /** * Clears OAuth tokens and client information */ - clearOAuthTokens(): void { - this.oauthManager?.clearOAuthTokens(); + async clearOAuthTokens(): Promise { + await this.oauthManager?.clearOAuthTokens(); } /** diff --git a/core/mcp/oauthManager.ts b/core/mcp/oauthManager.ts index 878495fac..ef8da8bb3 100644 --- a/core/mcp/oauthManager.ts +++ b/core/mcp/oauthManager.ts @@ -282,6 +282,8 @@ export class OAuthManager { iss?: string, ): Promise { try { + await this.requireStorage().load(); + if (this.isEnterpriseManaged()) { const scopeForMint = this.pendingAuthorizationScope ?? this.getEmaFlowConfig().scope; @@ -387,13 +389,13 @@ export class OAuthManager { } } - clearOAuthTokens(): void { + async clearOAuthTokens(): Promise { if (!this.oauthConfig?.storage) { return; } const serverUrl = this.getServerUrl(); - this.oauthConfig.storage.clear(serverUrl); + await this.oauthConfig.storage.clear(serverUrl); this.oauthFlowState = null; this.pendingAuthorizationScope = undefined; diff --git a/core/react/useEmaIdpLoginState.ts b/core/react/useEmaIdpLoginState.ts index a9a65393c..19f698274 100644 --- a/core/react/useEmaIdpLoginState.ts +++ b/core/react/useEmaIdpLoginState.ts @@ -40,8 +40,9 @@ export function useEmaIdpLoginState( const logout = useCallback(() => { if (!normalizedIssuer) return; - clearEmaIdpSession(storage, normalizedIssuer); - setLoginState("none"); + void clearEmaIdpSession(storage, normalizedIssuer).then(() => { + setLoginState("none"); + }); }, [storage, normalizedIssuer]); return { loginState, refresh, logout }; diff --git a/specification/v2_auth_ema.md b/specification/v2_auth_ema.md index 2c38693f3..da4d8f021 100644 --- a/specification/v2_auth_ema.md +++ b/specification/v2_auth_ema.md @@ -111,12 +111,11 @@ Until **client profiles** (see below) land, install-level client config — star **Runtime auth state** (standard OAuth tokens, PKCE, **and** EMA runtime state: cached IdP ID Token / refresh token, leg-1 in-flight PKCE under `ema-idp:{issuer}`, and per-server resource tokens tagged `enterpriseManaged: true` when minted via EMA legs 2–3) uses the existing **`OAuthStorage`** interface (`core/auth/storage.ts`) — whatever adapter each client already passes to `InspectorClient`. ID-JAG is **not** cached — legs 2–3 re-mint on each connect or 401 refresh. EMA does **not** mandate a specific backing store; it extends the serialized auth state that adapter already persists. -No EMA-specific migration of web OAuth persistence is required for the initial ship — sessionStorage-backed web sessions work for EMA today. **Follow-up:** shared file-backed OAuth via `RemoteOAuthStorage` → `/api/storage/oauth` so web matches CLI/TUI (see §Follow-up work). +Web uses shared file-backed OAuth via `RemoteOAuthStorage` → `/api/storage/oauth` (same `oauth.json` as CLI/TUI). Existing **sessionStorage** OAuth blobs from before that wiring are not migrated automatically — users start fresh or re-authorize once. | Client | Config (`client.json`) | Runtime auth state (`OAuthStorage`) | | ------ | ---------------------- | ----------------------------------- | -| **Web (today)** | `RemoteStorage` adapter → `/api/storage/client` | `BrowserOAuthStorage` → sessionStorage | -| **Web (target)** | same | `RemoteOAuthStorage` → `/api/storage/oauth` → `~/.mcp-inspector/storage/oauth.json` | +| **Web** | `RemoteStorage` adapter → `/api/storage/client` | `RemoteOAuthStorage` → `/api/storage/oauth` → `~/.mcp-inspector/storage/oauth.json` (via `getWebRemoteOAuthStorage()` in `clients/web/src/lib/remoteOAuthStorage.ts`) | | **CLI / TUI** | `NodeClientStorage` (file adapter) | `NodeOAuthStorage` → `oauth.json` | On-disk shape (initial): @@ -299,7 +298,7 @@ Install-level IdP credentials are edited in **Client Settings**, separate from p 2. Clears leg-1 in-flight state at `servers["ema-idp:{issuer}"]` 3. Calls `OAuthStorage.clearEnterpriseManagedResourceServers()` — scans the shared OAuth blob and removes `servers[url]` entries where `enterpriseManaged === true` (set when EMA legs 2–3 saved tokens via `saveTokens(url, tokens, { enterpriseManaged: true })` in `emaFlow.ts` / `EmaTransportOAuthProvider`). Standard OAuth entries in the same blob are **not** cleared. Untagged legacy EMA tokens (saved before tagging landed) are not removed until the next EMA connect re-saves with the tag. The next connect to a cleared EMA server has no cached access token, so a **401** triggers leg 1 (IdP login) via `authenticate()`. -- **Web OAuth store singleton:** web uses `getBrowserOAuthStorage()` (`core/auth/browser/storage.ts`) so Client Settings sign-out updates the same in-memory Zustand store as the active `InspectorClient` (not a separate `BrowserOAuthStorage` instance per consumer). +- **Web OAuth store singleton:** web uses `getWebRemoteOAuthStorage()` (`clients/web/src/lib/remoteOAuthStorage.ts`) — memoized `RemoteOAuthStorage` backed by `/api/storage/oauth` — so Client Settings sign-out, EMA IdP session, connect, and per-server clear all mutate the same in-memory Zustand view as the active `InspectorClient`. **Future (not implemented):** explicit **Sign out** may additionally invoke the IdP's OIDC **end-session** / logout endpoint (RP-initiated logout) so the IdP SSO cookie is cleared — not just inspector-local IdP/resource token state. Today sign-out is **local-only** (clear `idpSessions`, tagged EMA resource entries, and leg-1 PKCE); the IdP may still treat the browser as signed in and skip the login prompt on the next authorize redirect until the IdP session expires or the user signs out at the IdP. @@ -347,7 +346,7 @@ Leg 1 is **OIDC authorization-code login** against the enterprise IdP using cred | Client | Leg 1 mechanism (same stack as standard OAuth in that client) | | ------ | ------------------------------------------------------------- | -| **Web** | Browser redirect via `navigation`; callback at `/oauth/callback`; `InspectorClient.completeOAuthFlow(code)`; `BrowserOAuthStorage` | +| **Web** | Browser redirect via `navigation`; callback at `/oauth/callback`; `InspectorClient.completeOAuthFlow(code)`; `RemoteOAuthStorage` → shared `oauth.json` | | **TUI / CLI** | `OAuthCallbackServer` opens local callback; system browser for authorize URL; `completeOAuthFlow(code)`; `NodeOAuthStorage` | **Silent connect:** if a valid cached ID Token exists for the configured issuer, skip the interactive redirect and proceed to leg 2. @@ -422,7 +421,8 @@ Inspector touchpoints to extend: - `core/mcp/oauthManager.ts` — branch on `enterpriseManaged`; EMA connect via `emaFlow`; `getOAuthState()`; `createOAuthProvider()` returns `EmaTransportOAuthProvider` for EMA servers (401 re-auth); standard OAuth unchanged - `core/mcp/inspectorClient.ts` — declare EMA extension in `initialize` capabilities when `enterpriseManaged`; `getOAuthState()` - `core/react/useEmaIdpLoginState.ts` — Client Settings IdP session status; calls `clearEmaIdpSession` on sign-out (no catalog wiring) -- `core/auth/browser/storage.ts` — `getBrowserOAuthStorage()` singleton (web) +- `clients/web/src/lib/remoteOAuthStorage.ts` — `getWebRemoteOAuthStorage()` singleton (web OAuth runtime store) +- `core/auth/browser/storage.ts` — `BrowserOAuthStorage` / `getBrowserOAuthStorage()` (sessionStorage; reference provider only, not wired in v2 web app) - `clients/web` — **Client Settings** modal; **Connection Info** OAuth snapshot; friendly EMA connect toasts; load client config at startup; `ServerSettingsForm` OAuth section (HTTP/SSE only); web callback flow tagging for IdP vs resource OAuth - `clients/cli`, `clients/tui` — load `client.json` at startup; pass into `InspectorClient`; leg 1 via same `OAuthCallbackServer` + `NodeOAuthStorage` stack as TUI standard OAuth today @@ -515,7 +515,7 @@ Design decisions for EMA are complete. Remaining work is the phased plan and che ### Later - [ ] **Remove Zustand from OAuth persistence** — direct read/write of OAuth blob; see §Follow-up work -- [ ] **Web shared OAuth store** — `RemoteOAuthStorage` / `oauth.json` for web + CLI + TUI parity; see §Follow-up work +- [x] **Web shared OAuth store** — `RemoteOAuthStorage` / `oauth.json` for web + CLI + TUI parity (#1548); see §Follow-up work - [ ] Client profile persistence (migrate from `client.json`; may extend or replace web Client Settings) - [ ] Optional: adopt `@modelcontextprotocol/client` v2 Layer-2 helpers for legs 2–3 (replace `wire.ts`) or full v2 transport for EMA - [ ] Optional: IdP **end-session** / RP-initiated logout on explicit **Sign out** (today sign-out clears inspector-local IdP session and tagged EMA resource tokens only; IdP browser SSO may remain active) @@ -558,24 +558,26 @@ These items came out of EMA staging and apply beyond EMA. They are **not** requi Today `OAuthStorage` is backed by a **Zustand** `persist` store (`core/auth/store.ts`) with adapters for sessionStorage (`BrowserOAuthStorage`), file (`NodeOAuthStorage` via `file-storage.ts`), and HTTP (`RemoteOAuthStorage` via `remote-storage.ts`). The Zustand layer wraps a simple `{ servers, idpSessions }` blob and adds: - `{ state, version }` envelope on file/remote adapters (see [Servers file](v2_servers_file.md) — intentionally avoided for `mcp.json`) -- A second in-memory copy and async persist semantics that complicate sign-out, callback rebuild, and multi-consumer sharing (`getBrowserOAuthStorage()` singleton exists partly to paper over this) +- A second in-memory copy and async persist semantics that complicate sign-out, callback rebuild, and multi-consumer sharing (web uses `getWebRemoteOAuthStorage()`; legacy `getBrowserOAuthStorage()` existed partly to paper over this for sessionStorage) **Direction:** refactor `OAuthStorage` implementations to read/write the serialized shape **directly** (file I/O, `/api/storage/oauth`, or in-memory for tests) without Zustand. Keep the `OAuthStorage` interface stable for `InspectorClient` / `OAuthManager`. ### Shared file-backed OAuth state (web + CLI + TUI) -Today: +**Status (July 2026):** Web is wired through `RemoteOAuthStorage` (`environmentFactory.ts`, `App.tsx`, `getWebRemoteOAuthStorage()`). CLI/TUI continue to use `NodeOAuthStorage` on the same on-disk file when using the default local backend. | Client | OAuth runtime store | | ------ | ------------------- | -| **Web** | `BrowserOAuthStorage` → **sessionStorage** (tab-local; lost on new tab / callback page load relies on same tab) | +| **Web** | `RemoteOAuthStorage` → `/api/storage/oauth` → `~/.mcp-inspector/storage/oauth.json` | | **CLI / TUI** | `NodeOAuthStorage` → `~/.mcp-inspector/storage/oauth.json` | -`RemoteOAuthStorage` already exists (`core/auth/remote/storage-remote.ts`) and talks to **`GET/POST/DELETE /api/storage/oauth`** on the local Hono backend — the same generic storage API that persists to disk under `~/.mcp-inspector/storage/`. Integration tests use it (`inspectorClient-oauth-remote-storage-e2e.test.ts`). +`RemoteOAuthStorage` (`core/auth/remote/storage-remote.ts`) talks to **`GET/POST/DELETE /api/storage/oauth`** on the local Hono backend — the same generic storage API that persists to disk under `~/.mcp-inspector/storage/`. Integration tests use it (`inspectorClient-oauth-remote-storage-e2e.test.ts`). -**Direction:** wire the **web app** through `RemoteOAuthStorage` (via `environmentFactory.ts` / `App.tsx`) instead of `BrowserOAuthStorage`, so web, CLI, and TUI share one **`oauth.json`** when using the default local backend. Benefits: IdP session and EMA resource tokens survive browser refresh; sign-out in Client Settings and connect use the same store without a sessionStorage singleton; EMA testing matches how CLI/TUI will behave in Phase 4. +**Benefits:** IdP session and EMA resource tokens survive browser refresh; sign-out in Client Settings and connect use the same store; EMA testing matches CLI/TUI behavior. -**Migration:** one-time import or “start fresh” for existing sessionStorage OAuth blobs; document that enabling shared storage is the default for local dev. +**Migration:** no automatic import from old sessionStorage OAuth blobs; document that local dev uses shared `oauth.json` by default. + +**Optional follow-up:** `navigator.locks` for cross-tab single-flight on silent refresh (see [Mid-session auth](v2_auth_mid_session.md)). --- diff --git a/specification/v2_auth_mid_session.md b/specification/v2_auth_mid_session.md index 7b1cf3a97..0fdc5b56f 100644 --- a/specification/v2_auth_mid_session.md +++ b/specification/v2_auth_mid_session.md @@ -338,7 +338,7 @@ Snapshots **per app tab UI**, not message logs, tool results, or network bodies. ### Multiple browser tabs -Each browser tab has its own `RemoteSession` and (today) `BrowserOAuthStorage` in `sessionStorage`. SSE `auth_challenge` is scoped to that session. +Each browser tab has its own `RemoteSession`. OAuth runtime state is shared file-backed (`RemoteOAuthStorage` → `oauth.json`) across tabs on the same backend; SSE `auth_challenge` is scoped to that tab's session. When a tab is **hidden**, **interactive** OAuth must not steal focus: @@ -413,7 +413,7 @@ Legacy **SSE** transport: **401 only** on mid-session `send()` (no 403 step-up). | ------- | --- | --- | --- | | Challenge detection | Inline send + SSE ambient | SDK transport + intercept | Same as TUI | | Auth execution | Browser `OAuthManager` | Node `OAuthManager` | Node `OAuthManager` | -| OAuth storage | `BrowserOAuthStorage` (sessionStorage) | `NodeOAuthStorage` (file) | Same file as TUI | +| OAuth storage | `RemoteOAuthStorage` → `oauth.json` (via `/api/storage/oauth`) | `NodeOAuthStorage` (file) | Same file as TUI | | Silent recovery | `auth-state` push + send retry | Reconnect / SDK retry (v2) | One-shot RPC retry | | Interactive recovery | Modal + full-page redirect + snapshot | Auth tab + callback | stderr **y/N** + callback | | Step-up confirm | `StepUpAuthModal` | Auth tab **A** / **C** | **y/N** |