Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions decisions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
18 changes: 10 additions & 8 deletions packages/client/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
});
}

/**
Expand Down
38 changes: 37 additions & 1 deletion packages/client/src/tests/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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 });
Expand Down
4 changes: 0 additions & 4 deletions packages/kasper/src/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
12 changes: 2 additions & 10 deletions packages/kasper/src/tests/context.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
6 changes: 3 additions & 3 deletions packages/preact/src/tests/context.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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);
});
});
7 changes: 3 additions & 4 deletions packages/react/src/tests/context.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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);
});
});
Loading