From d0dccdeb67acc838ee9d4ed2f9771ae666fe714b Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Sat, 12 Sep 2026 19:46:05 -0400 Subject: [PATCH] fix(auth): OIDC token rotation no longer reloads every other tab MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit useSessionSync reloaded on any out-of-tab session change, so a routine refresh in one tab hard-reloaded the others mid-call. Classify the change: removed → reload, user/device changed → reload, same device with a new token → swap it into the running client (setAccessToken + the shared refresh token) in place. The refresher takes a Web Lock and adopts tokens another tab already rotated instead of racing the issuer. Unit-tested classifier. Fixes #16 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PPmy3tPq869XDW4njjVaKA --- src/app/hooks/useSessionSync.test.ts | 35 +++++++++++++++ src/app/hooks/useSessionSync.ts | 66 ++++++++++++++++++++++------ src/app/pages/client/ClientRoot.tsx | 8 ++-- src/client/oidcTokenRefresher.ts | 52 +++++++++++++++++++++- 4 files changed, 143 insertions(+), 18 deletions(-) create mode 100644 src/app/hooks/useSessionSync.test.ts diff --git a/src/app/hooks/useSessionSync.test.ts b/src/app/hooks/useSessionSync.test.ts new file mode 100644 index 000000000..86bc631cb --- /dev/null +++ b/src/app/hooks/useSessionSync.test.ts @@ -0,0 +1,35 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { classifySessionChange, SessionIdentity } from './useSessionSync'; + +const alice: SessionIdentity = { userId: '@alice:hs', deviceId: 'DEV1', accessToken: 'tok-1' }; + +test('classifySessionChange: session removed elsewhere → removed', () => { + assert.equal(classifySessionChange(alice, null), 'removed'); +}); + +test('classifySessionChange: nothing before or after → none', () => { + assert.equal(classifySessionChange(null, null), 'none'); +}); + +test('classifySessionChange: session appeared → relogin', () => { + assert.equal(classifySessionChange(null, alice), 'relogin'); +}); + +test('classifySessionChange: different user or device → relogin', () => { + assert.equal(classifySessionChange(alice, { ...alice, userId: '@bob:hs' }), 'relogin'); + assert.equal(classifySessionChange(alice, { ...alice, deviceId: 'DEV2' }), 'relogin'); + // Even when the access token also changed, the identity change wins. + assert.equal( + classifySessionChange(alice, { ...alice, deviceId: 'DEV2', accessToken: 'tok-2' }), + 'relogin', + ); +}); + +test('classifySessionChange: same user+device, new access token → rotated (no reload)', () => { + assert.equal(classifySessionChange(alice, { ...alice, accessToken: 'tok-2' }), 'rotated'); +}); + +test('classifySessionChange: identical credentials (metadata-only rewrite) → none', () => { + assert.equal(classifySessionChange(alice, { ...alice }), 'none'); +}); diff --git a/src/app/hooks/useSessionSync.ts b/src/app/hooks/useSessionSync.ts index 1776ea093..aee0055fc 100644 --- a/src/app/hooks/useSessionSync.ts +++ b/src/app/hooks/useSessionSync.ts @@ -1,5 +1,32 @@ import { useEffect } from 'react'; -import { getFallbackSession, subscribeSessionChanges } from '../state/sessions'; +import type { MatrixClient } from 'matrix-js-sdk'; +import { getFallbackSession, Session, subscribeSessionChanges } from '../state/sessions'; + +/** The credential identity this tab is currently running on. */ +export type SessionIdentity = Pick; + +/** + * What an out-of-tab session change means for this tab: + * - `none` — nothing credential-relevant changed (metadata-only rewrite, + * or a duplicate storage event from the dual-write). + * - `removed` — the session disappeared (logout / localStorage.clear()). + * - `relogin` — a different user or device signed in. + * - `rotated` — same user + device, only the access token changed (an OIDC + * refresh performed by another tab). + */ +export type SessionChange = 'none' | 'removed' | 'relogin' | 'rotated'; + +/** Pure: classify a freshly-read session against the one this tab runs on. */ +export const classifySessionChange = ( + current: SessionIdentity | null, + next: SessionIdentity | null, +): SessionChange => { + if (!next) return current ? 'removed' : 'none'; + if (!current) return 'relogin'; + if (next.userId !== current.userId || next.deviceId !== current.deviceId) return 'relogin'; + if (next.accessToken !== current.accessToken) return 'rotated'; + return 'none'; +}; /** * Keep this tab in sync with session changes performed in other tabs/windows. @@ -11,26 +38,39 @@ import { getFallbackSession, subscribeSessionChanges } from '../state/sessions'; * Default action is the safest one for auth-critical state — a full reload: * - session REMOVED elsewhere (logout / localStorage.clear()) → the access * token disappears, so we reload; the router bounces to auth on next boot. - * - session APPEARED or its access token CHANGED elsewhere (a fresh login or - * a token rotation) → we reload so the client re-initialises with the new - * credentials rather than running on a stale/revoked token. + * - session APPEARED or its user/device CHANGED elsewhere (a fresh login) → + * we reload so the client re-initialises with the new credentials rather + * than running on a stale/revoked token. * - * A change that does not alter the access token (e.g. an OIDC metadata-only + * #16 — a same-user, same-device access-token ROTATION (a routine OIDC refresh + * in another tab) must NOT reload: that would drop in-progress calls/uploads + * every few minutes. Instead the new tokens are swapped into the running + * client in place. When no client is available yet we fall back to the reload. + * + * A change that does not alter the credentials (e.g. an OIDC metadata-only * rewrite) is ignored, which also collapses the several storage events emitted - * by a single dual-write into at most one reload. + * by a single dual-write into at most one reaction. */ -export const useSessionSync = (): void => { +export const useSessionSync = (mx?: MatrixClient): void => { useEffect(() => { - // Snapshot the credential this tab booted with; compare against it so we - // only reload on a genuine credential change. - const initialAccessToken = getFallbackSession()?.accessToken ?? null; + // Snapshot the credential this tab runs on; compare against it so we only + // react to a genuine credential change. Updated on an in-place rotation. + let current: SessionIdentity | null = getFallbackSession() ?? null; const unsubscribe = subscribeSessionChanges((session) => { - const nextAccessToken = session?.accessToken ?? null; - if (nextAccessToken === initialAccessToken) return; + const change = classifySessionChange(current, session); + if (change === 'none') return; + if (change === 'rotated' && mx && session) { + // Same opts object backs the SDK's TokenRefresher, so updating the + // refresh token here keeps its next refresh on the rotated token. + mx.setAccessToken(session.accessToken); + if (session.refreshToken) mx.http.opts.refreshToken = session.refreshToken; + current = session; + return; + } window.location.reload(); }); return unsubscribe; - }, []); + }, [mx]); }; diff --git a/src/app/pages/client/ClientRoot.tsx b/src/app/pages/client/ClientRoot.tsx index d42a17ce9..21753d080 100644 --- a/src/app/pages/client/ClientRoot.tsx +++ b/src/app/pages/client/ClientRoot.tsx @@ -208,9 +208,11 @@ export function ClientRoot({ children }: ClientRootProps) { ); useLogoutListener(mx); - // Cross-tab session sync: another tab logging out / in (access token changed - // in localStorage) reloads this tab so it never runs with stale credentials. - useSessionSync(); + // Cross-tab session sync: another tab logging out / in reloads this tab so it + // never runs with stale credentials. A same-device token *rotation* (OIDC + // refresh in another tab) is swapped into the running client instead of + // reloading, which would drop an in-progress call/upload (#16). + useSessionSync(mx); useEffect(() => { if (loadState.status === AsyncStatus.Idle) { diff --git a/src/client/oidcTokenRefresher.ts b/src/client/oidcTokenRefresher.ts index 3e7df256e..5da445f96 100644 --- a/src/client/oidcTokenRefresher.ts +++ b/src/client/oidcTokenRefresher.ts @@ -1,6 +1,23 @@ -import { OidcTokenRefresher } from 'matrix-js-sdk'; +import { AccessTokens, OidcTokenRefresher } from 'matrix-js-sdk'; import type { IdTokenClaims } from 'oidc-client-ts'; -import { OidcSessionMeta, setFallbackSession } from '../app/state/sessions'; +import { getFallbackSession, OidcSessionMeta, setFallbackSession } from '../app/state/sessions'; + +// Web Lock name serialising OIDC refreshes across tabs (Gitea #16). Every tab +// runs its own refresher against the SAME stored refresh token; with rotating +// refresh tokens the second tab to hit the issuer gets `invalid_grant` and is +// signed out. Holding the lock while refreshing (and re-reading storage once +// inside it) makes the loser adopt the winner's tokens instead. +const REFRESH_LOCK_NAME = 'lotus-oidc-refresh'; + +/** + * Run `fn` under the cross-tab refresh lock. Falls back to running it directly + * when the Web Locks API is unavailable (older browsers, non-secure contexts). + */ +export const withRefreshLock = (fn: () => Promise): Promise => { + const locks = typeof navigator !== 'undefined' ? navigator.locks : undefined; + if (!locks) return fn(); + return locks.request(REFRESH_LOCK_NAME, fn); +}; /** * OidcTokenRefresher that persists rotated tokens back to the fallback session, @@ -30,6 +47,37 @@ export class LotusOidcTokenRefresher extends OidcTokenRefresher { this.oidcRef = oidc; } + // #16 — before touching the issuer, check whether another tab already rotated + // the tokens. `refreshToken` is exactly what the SDK currently holds, so a + // DIFFERENT stored refresh token (same user + device) means a sibling tab won + // the race: adopt its tokens instead of burning a possibly-consumed refresh + // token. Comparing refresh tokens (not access tokens) can never adopt the very + // token that just 401'd, so this cannot loop. The whole thing runs under a + // cross-tab Web Lock so concurrent refreshes serialise and the waiter sees + // the winner's write. + public doRefreshAccessToken(refreshToken: string): Promise { + return withRefreshLock(async () => { + const stored = getFallbackSession(); + if ( + stored && + stored.userId === this.userIdRef && + stored.deviceId === this.deviceIdRef && + stored.refreshToken && + stored.refreshToken !== refreshToken + ) { + return { + accessToken: stored.accessToken, + refreshToken: stored.refreshToken, + expiry: + typeof stored.expiresInMs === 'number' + ? new Date(Date.now() + stored.expiresInMs) + : undefined, + }; + } + return super.doRefreshAccessToken(refreshToken); + }); + } + // F5 — persist the new expiry so the stored `expiresAt` stays fresh across // reloads instead of going stale. The SDK invokes persistTokens synchronously // inside the refresh and passes the freshly-refreshed `expiry` (a Date) on the