diff --git a/clients/tui/src/App.tsx b/clients/tui/src/App.tsx index 01b05bc5..0d126414 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 4ce5ffe2..077a0cf1 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -2191,6 +2191,7 @@ function App() { } void (async () => { + await webOAuthStorage.load(); const client = setupClientForServer(server, sessionId); setActiveServerId(server.id); try { @@ -2246,7 +2247,7 @@ function App() { }); } })(); - }, [servers, setupClientForServer, showReAuthBanner]); + }, [servers, setupClientForServer, showReAuthBanner, webOAuthStorage]); const onToggleConnection = useCallback( async (id: string) => { @@ -3273,7 +3274,7 @@ 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, 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 62716e47..d060439b 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 37ef84a1..d88109cb 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 0831b4db..baebb845 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 df7eb616..11ef63e7 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 d20f1b54..d7270584 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 6429811c..52afc034 100644 --- a/clients/web/src/test/core/auth/storage-browser.test.ts +++ b/clients/web/src/test/core/auth/storage-browser.test.ts @@ -77,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); @@ -93,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, ); @@ -113,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); @@ -130,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); @@ -163,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); @@ -177,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); @@ -194,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); @@ -210,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); @@ -221,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); @@ -237,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); @@ -248,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); @@ -269,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); @@ -285,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); @@ -294,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" }, @@ -302,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(); @@ -322,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(); }); @@ -352,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(); }); }); @@ -369,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(); @@ -386,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( @@ -415,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 c20617ef..04175845 100644 --- a/clients/web/src/test/core/auth/storage-remote.test.ts +++ b/clients/web/src/test/core/auth/storage-remote.test.ts @@ -50,7 +50,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 +58,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 +66,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 +93,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 +107,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 +120,39 @@ describe("RemoteOAuthStorage (unit, mocked fetch)", () => { // No public accessor; constructing without throwing covers the default-branch. expect(s).toBeInstanceOf(RemoteOAuthStorage); }); + + 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", + ); + }); }); diff --git a/clients/web/src/test/core/mcp/oauthManager.test.ts b/clients/web/src/test/core/mcp/oauthManager.test.ts index 02d764f2..b4bfa7bb 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 f8b9c0e6..bc7f1d6b 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 2c72f8aa..eb43eab3 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 b2ed94b0..f0aa91ce 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 86d439fc..edab9e77 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 6b23727c..c75d258f 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 47f44e5d..137bc9c3 100644 --- a/clients/web/src/utils/clearServerOAuthState.test.ts +++ b/clients/web/src/utils/clearServerOAuthState.test.ts @@ -1,13 +1,14 @@ 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", () => { let storage: BrowserOAuthStorage; - beforeEach(() => { + beforeEach(async () => { storage = new BrowserOAuthStorage(); - storage.clear("https://mcp.example.com/mcp"); + await storage.clear("https://mcp.example.com/mcp"); }); it("clears storage by server URL when not the active connection", async () => { @@ -16,7 +17,7 @@ describe("clearServerOAuthState", () => { token_type: "Bearer", }); - const cleared = clearServerOAuthState({ + const cleared = await clearServerOAuthState({ config: { type: "streamable-http", url: "https://mcp.example.com/mcp" }, isActiveConnection: false, oauthStorage: storage, @@ -28,11 +29,11 @@ describe("clearServerOAuthState", () => { ).toBeUndefined(); }); - it("uses the live client when clearing the active connection", () => { - const clearOAuthTokens = vi.fn<() => void>(); + 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, isActiveConnection: true, @@ -43,13 +44,13 @@ describe("clearServerOAuthState", () => { 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 0e22dd5a..f7eb5233 100644 --- a/clients/web/src/utils/clearServerOAuthState.ts +++ b/clients/web/src/utils/clearServerOAuthState.ts @@ -17,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 { - params.oauthStorage.clear(serverUrl); + await params.oauthStorage.clear(serverUrl); } return true; } diff --git a/core/auth/ema/idpOidc.ts b/core/auth/ema/idpOidc.ts index 8dff2ae6..e860d7e0 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 21b47578..d736549a 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 f2246112..0332adf7 100644 --- a/core/auth/oauth-storage.ts +++ b/core/auth/oauth-storage.ts @@ -16,22 +16,48 @@ import type { SaveTokensOptions, } from "./storage.js"; +type OAuthStoreWithPersist = ReturnType & { + persist: { + hasHydrated: () => boolean; + onFinishHydration: (fn: () => void) => void; + }; +}; + /** * 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: ReturnType) { - this.store = store; + this.store = store as OAuthStoreWithPersist; + } + + load(): Promise { + if (!this.loadedPromise) { + this.loadedPromise = new Promise((resolve) => { + if (this.store.persist.hasHydrated()) { + resolve(); + return; + } + this.store.persist.onFinishHydration(() => resolve()); + }); + } + return this.loadedPromise; + } + + 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 +82,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 +93,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 +118,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 +132,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 +153,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 +170,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 +188,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 +223,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 4b300728..6e4c46ad 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 f8b46874..441821e8 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/mcp/inspectorClient.ts b/core/mcp/inspectorClient.ts index 601fc35b..e51d9887 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 878495fa..ef8da8bb 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 a9a65393..19f69827 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 };