From 397aa7b0e04d52d1fc3dddfccc623bcab0471f5d Mon Sep 17 00:00:00 2001 From: eugenioenko Date: Fri, 8 May 2026 17:10:10 -0700 Subject: [PATCH 1/3] fix: redirect before clearing state in logout to prevent race MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, logout() cleared auth state (isAuthenticated: false) before redirecting to the end_session_endpoint. This triggered RequireAuth to call login(), which raced with and won over the logout redirect. Now logout redirects first and returns immediately — state is only cleared locally when no end_session_endpoint is available. Also removes the Kasper adapter's unsubscribe-before-logout workaround since the race is fixed in the client itself. Closes #71 Co-Authored-By: Claude Opus 4.6 --- packages/client/src/client.ts | 18 ++++++----- packages/client/src/tests/client.test.ts | 38 ++++++++++++++++++++++- packages/kasper/src/context.ts | 4 --- packages/kasper/src/tests/context.test.ts | 12 ++----- 4 files changed, 49 insertions(+), 23 deletions(-) diff --git a/packages/client/src/client.ts b/packages/client/src/client.ts index 242e743..4f9259f 100644 --- a/packages/client/src/client.ts +++ b/packages/client/src/client.ts @@ -226,22 +226,16 @@ export class OidcClient { } /** - * Logs the user out by clearing local auth state and redirecting to the OP's end-session endpoint. + * Logs the user out by redirecting to the OP's end-session endpoint. * * If the discovery document has an `end_session_endpoint`, the browser is redirected there * with the current ID token hint and `postLogoutRedirectUri` from config. + * State is only cleared locally when no end-session redirect is available. */ logout(): void { this.stopAutoRefresh(); const idToken = this._state.tokens.id; - this.setState({ - user: null, - tokens: EMPTY_TOKENS, - isAuthenticated: false, - error: null, - }); - if (this.discovery) { const logoutUrl = buildLogoutUrl( this.discovery, @@ -250,8 +244,16 @@ export class OidcClient { ); if (logoutUrl) { window.location.href = logoutUrl; + return; } } + + this.setState({ + user: null, + tokens: EMPTY_TOKENS, + isAuthenticated: false, + error: null, + }); } /** diff --git a/packages/client/src/tests/client.test.ts b/packages/client/src/tests/client.test.ts index 0197f55..78d1034 100644 --- a/packages/client/src/tests/client.test.ts +++ b/packages/client/src/tests/client.test.ts @@ -258,7 +258,7 @@ describe("OidcClient", () => { }); describe("logout", () => { - it("clears state", async () => { + it("redirects to end_session_endpoint without clearing state", async () => { Object.defineProperty(window, "location", { value: { href: "http://localhost:3000?code=auth_code&state=test-state", @@ -288,6 +288,42 @@ describe("OidcClient", () => { client.logout(); + expect(window.location.href).toContain("https://auth.example.com/logout"); + expect(client.state.isAuthenticated).toBe(true); + }); + + it("clears state when no end_session_endpoint", async () => { + const discoveryNoLogout = { ...DISCOVERY, end_session_endpoint: undefined }; + + Object.defineProperty(window, "location", { + value: { + href: "http://localhost:3000?code=auth_code&state=test-state", + search: "?code=auth_code&state=test-state", + pathname: "/", + hash: "", + }, + writable: true, + configurable: true, + }); + + sessionStorage.setItem( + "oidc-js:auth-state", + JSON.stringify({ + codeVerifier: "test-verifier", + state: "test-state", + nonce: "test-nonce", + redirectUri: "http://localhost:3000/callback", + }), + ); + + mockFetchResponses(discoveryNoLogout, TOKEN_RESPONSE); + const client = new OidcClient({ ...CONFIG, fetchProfile: false }); + + await client.init(); + expect(client.state.isAuthenticated).toBe(true); + + client.logout(); + expect(client.state.isAuthenticated).toBe(false); expect(client.state.user).toBeNull(); expect(client.state.tokens).toEqual({ access: null, id: null, refresh: null, expiresAt: null }); diff --git a/packages/kasper/src/context.ts b/packages/kasper/src/context.ts index 951158b..e7c45b0 100644 --- a/packages/kasper/src/context.ts +++ b/packages/kasper/src/context.ts @@ -30,10 +30,6 @@ export function _initAuth( client.login(options); }, logout: () => { - // Unsubscribe before logout to prevent RequireAuth from reacting - // to the intermediate unauthenticated state and racing with the - // logout redirect by triggering a login redirect. - _unsub?.(); client.logout(); }, refresh: () => client.refresh(), diff --git a/packages/kasper/src/tests/context.test.ts b/packages/kasper/src/tests/context.test.ts index 5bcdf3b..14d1cb1 100644 --- a/packages/kasper/src/tests/context.test.ts +++ b/packages/kasper/src/tests/context.test.ts @@ -153,21 +153,13 @@ describe("_destroyAuth", () => { }); }); -describe("logout unsubscribes before calling client", () => { - it("unsubscribes from state changes before logout redirect", () => { - const unsubFn = vi.fn(); - mockClientInstance.subscribe.mockReturnValueOnce(unsubFn); - +describe("logout", () => { + it("calls client.logout()", () => { _initAuth(CONFIG); const auth = useAuth(); auth.actions.logout(); - expect(unsubFn).toHaveBeenCalled(); expect(mockClientInstance.logout).toHaveBeenCalled(); - - const unsubOrder = unsubFn.mock.invocationCallOrder[0]; - const logoutOrder = mockClientInstance.logout.mock.invocationCallOrder[0]; - expect(unsubOrder).toBeLessThan(logoutOrder); }); }); From 8bc69cfcf7d150f65e575dda7fbd964572d7f549 Mon Sep 17 00:00:00 2001 From: eugenioenko Date: Fri, 8 May 2026 17:33:45 -0700 Subject: [PATCH 2/3] docs: add decision 028 for logout redirect ordering Co-Authored-By: Claude Opus 4.6 --- decisions.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/decisions.md b/decisions.md index ce8d0a7..6a31621 100644 --- a/decisions.md +++ b/decisions.md @@ -328,3 +328,16 @@ Architectural and design decisions for the oidc-js project. Each entry captures **Decision**: Option 2. A `setInterval` in `OidcClient` polls every `autoRefreshInterval` seconds (default 10) and triggers `refresh()` when `isExpiredAt(expiresAt, expiryBuffer)` returns true. Enabled by default (`autoRefresh: true`). On refresh failure, the interval stops (no endpoint hammering); a successful manual refresh or new login restarts it. **Rationale**: This is the same approach `oidc-client-ts` uses (their `Timer` class polls every 5s). A short interval is drift-proof (always compares real time), sleep-proof (first tick after wake catches expiration), and cheap (one number comparison per tick). The implementation lives in `OidcClient`, so all framework adapters benefit via the existing subscribe/notify mechanism. The `expiryBuffer` config (already existed) controls how early the refresh fires — the token is still valid during the refresh request, so `RequireAuth` never sees an expired token and children stay mounted. + +### 028 - Redirect before clearing state in logout (2026-05-08) + +**Context**: `OidcClient.logout()` cleared auth state (`isAuthenticated: false`) before redirecting to the `end_session_endpoint`. In frameworks with synchronous reactivity (e.g. Kasper signals), this triggered `RequireAuth` to call `login()` before the logout redirect executed, winning the redirect race. In frameworks with batched updates (React, Vue), the race was probabilistic but still possible. + +**Alternatives considered**: +1. Adapter-level workaround — unsubscribe from state changes before calling `logout()` (what Kasper did) +2. Redirect first, only clear state when no `end_session_endpoint` is available +3. Use `batch()` or similar to defer subscriber notifications until after the redirect + +**Decision**: Option 2. `logout()` now redirects to the `end_session_endpoint` and returns immediately without clearing state. State is only cleared locally when no redirect is available. The Kasper adapter's unsubscribe workaround was removed. + +**Rationale**: The page is navigating away — clearing in-memory state before a redirect is unnecessary since a fresh `init()` on return starts with clean state. Option 1 was a framework-specific bandaid that didn't fix the root cause for other adapters. Option 3 wouldn't help because `batch()` groups signal writes within a single `setState` call, but `logout()` does one `setState` followed by a redirect — the batch completes before the redirect, so subscribers still fire. Fixing in the client means all adapters benefit without per-framework workarounds. From 515204e7bb7c927c72930902741b395702d8bb32 Mon Sep 17 00:00:00 2001 From: eugenioenko Date: Fri, 8 May 2026 18:13:01 -0700 Subject: [PATCH 3/3] fix: update React and Preact logout tests to match redirect-first behavior Co-Authored-By: Claude Opus 4.6 --- packages/preact/src/tests/context.test.tsx | 6 +++--- packages/react/src/tests/context.test.tsx | 7 +++---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/preact/src/tests/context.test.tsx b/packages/preact/src/tests/context.test.tsx index 0798f11..3225b7a 100644 --- a/packages/preact/src/tests/context.test.tsx +++ b/packages/preact/src/tests/context.test.tsx @@ -215,7 +215,7 @@ describe("AuthProvider", () => { expect(result!.error).not.toBeNull(); }); - it("actions.logout clears state", async () => { + it("actions.logout redirects to end_session_endpoint without clearing state", async () => { Object.defineProperty(window, "location", { value: { href: "http://localhost:3000?code=auth_code&state=test-state", @@ -256,7 +256,7 @@ describe("AuthProvider", () => { result!.actions.logout(); }); - expect(result!.isAuthenticated).toBe(false); - expect(result!.user).toBeNull(); + expect(window.location.href).toContain("https://auth.example.com/logout"); + expect(result!.isAuthenticated).toBe(true); }); }); diff --git a/packages/react/src/tests/context.test.tsx b/packages/react/src/tests/context.test.tsx index f5cd675..55df600 100644 --- a/packages/react/src/tests/context.test.tsx +++ b/packages/react/src/tests/context.test.tsx @@ -339,7 +339,7 @@ describe("AuthProvider", () => { expect(onError).toHaveBeenCalledWith(expect.any(Error)); }); - it("actions.logout clears state", async () => { + it("actions.logout redirects to end_session_endpoint without clearing state", async () => { Object.defineProperty(window, "location", { value: { href: "http://localhost:3000?code=auth_code&state=test-state", @@ -372,8 +372,7 @@ describe("AuthProvider", () => { result.current.actions.logout(); }); - expect(result.current.isAuthenticated).toBe(false); - expect(result.current.user).toBeNull(); - expect(result.current.tokens).toEqual({ access: null, id: null, refresh: null, expiresAt: null }); + expect(window.location.href).toContain("https://auth.example.com/logout"); + expect(result.current.isAuthenticated).toBe(true); }); });