From ad9307382c41df1ef3572387897e522a26536319 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Tue, 13 Oct 2026 10:35:14 -0400 Subject: [PATCH 1/3] fix(auth): sign out every tab when one tab signs out A tab left open on a cluster page while another tab signed out kept polling central manager on the ended session. Central manager answers a request with no session 403 "Not allowed" (checkUser -> notAllowed(), HarperFast/central-manager#938), which the 401 interceptor ignores and the global query error handler toasts as "ClientError: Not allowed". Nothing told the open tab about the sign-out: Studio's only cross-tab listeners were the API explorer's credential cleanup. signOutAllLocally, the local clear that runs once the shared session has ended (after /Logout/ succeeds, on a 401, after a password change), now rewrites `Studio:AppSignOut`. Every other tab holding the app sign-in follows that storage event with the same clear minus the shared localStorage, so its route guards send it to sign-in before its next poll. The sign-out sweep does not announce: it runs before /Logout/, which can fail, and a tab sent to sign-in that early could sign in again only for that /Logout/ to end the new session. The follower keys off a dedicated key rather than the marker record, because a tab also drops the app sign-in from that record when its check merely fails (a timeout or a 5xx on /User/current). And it writes no localStorage: its copy of the markers is a snapshot from its own page load, and writing it back could undo the other tab's next sign-in. Co-Authored-By: Claude Opus 5.5 --- DESIGN.md | 9 + .../auth/handlers/clearAuthStateLocally.ts | 34 +- .../auth/handlers/signOutHandlers.test.ts | 35 +- .../auth/store/appSignOutElsewhere.test.ts | 127 ++++++ src/features/auth/store/authStore.ts | 58 +++ src/lib/installCrossTabSignOut.ts | 18 + src/main.tsx | 3 + src/router/__tests__/crossTabSignOut.test.tsx | 378 ++++++++++++++++++ 8 files changed, 652 insertions(+), 10 deletions(-) create mode 100644 src/features/auth/store/appSignOutElsewhere.test.ts create mode 100644 src/lib/installCrossTabSignOut.ts create mode 100644 src/router/__tests__/crossTabSignOut.test.tsx diff --git a/DESIGN.md b/DESIGN.md index ed45af41a..e7dacccfc 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -117,6 +117,15 @@ Central manager only ever sends the boolean, never the other organizations' ids; an expired session — and let still-mounted pages rebuild their queries after the sign-out's cache clear and fetch again on the dead session. The guard redirects every signed-out dashboard load, so the spinner only covers the moment before it does (`endedSessionRouteFailures.test.tsx`, HarperFast/studio#1546). +- **Signing out in one tab signs out every tab.** `signOutAllLocally` rewrites `Studio:AppSignOut`, and each other tab + holding the app sign-in runs `clearAuthStateAfterSignOutElsewhere` on that `storage` event (`installCrossTabSignOut`). + Without it, a tab learns the session ended only from its next central-manager request, whose 403 toasts as a + permission refusal. `signOutAllLocally` runs once the shared session has ended: after `/Logout/` succeeds, on a 401, + after a password change. The sign-out sweep does not announce, because it runs before `/Logout/`: a failed logout + would sign the other tabs out of a live session, and one signed back in meanwhile would be ended by that `/Logout/`. + Tabs follow that key, not the marker record, because a tab also drops the app sign-in from the record when its own + check merely fails (a timeout, a 5xx). A tab that follows writes no `localStorage`: its copy of the markers is a + snapshot from its own load, and writing it back could undo the other tab's next sign-in (`crossTabSignOut.test.tsx`). - **Data fetching**: TanStack React Query 5. Query keys must be instance-scoped when the request targets a specific instance. - **Polling**: every `refetchInterval` on a query that can reject against an instance or a cluster goes through [`pollUnlessForbidden`](src/react-query/pollUnlessForbidden.ts), which stops the timer on a 403 and backs off a sustained 400 or 5xx; a raw number polls a failing endpoint at full rate for as long as the page is open. When the interval depends on the data, compute it and hand it over per tick: `(query) => pollUnlessForbidden(base(query))(query)` (the back-off state is per query, not per wrapper). - **Forms**: react-hook-form with a zod `zodResolver`. A form whose submit button waits on `formState.isValid` must not use the default `onSubmit` mode, which keeps `isValid` current but shows errors only after a submit the disabled button never allows, so the user is blocked with no reason given (HarperFast/studio#1550). Use `mode: 'onTouched'`: a field's error appears once it is first left and then follows each change, while `isValid` still tracks every keystroke (`onBlur` recomputes `isValid` only on blur, so the button lags the text). Enter neither submits past a disabled button nor leaves the field, so give the `
` `onKeyDown={revealFieldErrorOnEnter(form)}`, which treats Enter in a text input as leaving it. A refine that compares two fields reports on one of them; re-check that field from the other's `onChange` once it has been touched (`getFieldState(name).isTouched`, then `trigger(name)`), because `rules={{ deps }}` fires on the other field's first blur, before the user reaches it. react-hook-form only updates the `formState` fields a render has read, and `disabled={!isDirty || !isValid}` skips `isValid` while the form is pristine, so where one change can complete the form (a single field, a paste, a checkbox) the button stays disabled on a valid form. Read the fields before the JSX (`const { isDirty, isValid } = form.formState;`) and pin it with a test that makes one change and checks the button. diff --git a/src/features/auth/handlers/clearAuthStateLocally.ts b/src/features/auth/handlers/clearAuthStateLocally.ts index c70a54680..ce6b05009 100644 --- a/src/features/auth/handlers/clearAuthStateLocally.ts +++ b/src/features/auth/handlers/clearAuthStateLocally.ts @@ -9,15 +9,31 @@ import { queryClient } from '@/react-query/queryClient'; * same-tab sign-in as another user. */ export function clearAuthStateLocally(): void { - for ( - const step of [ - () => authStore.signOutAllLocally(), - () => queryClient.getMutationCache().clear(), - () => queryClient.getQueryCache().clear(), - clearLocalStorage, - clearSessionStorage, - ] - ) { + runEvery([ + () => authStore.signOutAllLocally(), + () => queryClient.getMutationCache().clear(), + () => queryClient.getQueryCache().clear(), + clearLocalStorage, + clearSessionStorage, + ]); +} + +/** + * This tab's half of a sign-out another tab made: the same clears as {@link clearAuthStateLocally}, except + * the `localStorage` both tabs share, which the tab that signed out clears and this one must not write + * (`authStore.forgetAppSignInEndedElsewhere`). + */ +export function clearAuthStateAfterSignOutElsewhere(): void { + runEvery([ + () => authStore.forgetAppSignInEndedElsewhere(), + () => queryClient.getMutationCache().clear(), + () => queryClient.getQueryCache().clear(), + clearSessionStorage, + ]); +} + +function runEvery(steps: Array<() => void>): void { + for (const step of steps) { try { step(); } catch (error) { diff --git a/src/features/auth/handlers/signOutHandlers.test.ts b/src/features/auth/handlers/signOutHandlers.test.ts index 56440a143..36a8db922 100644 --- a/src/features/auth/handlers/signOutHandlers.test.ts +++ b/src/features/auth/handlers/signOutHandlers.test.ts @@ -2,7 +2,7 @@ import { authStore, OverallAppSignIn } from '@/features/auth/store/authStore'; import { queryClient } from '@/react-query/queryClient'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { clearAuthStateLocally } from './clearAuthStateLocally'; +import { clearAuthStateAfterSignOutElsewhere, clearAuthStateLocally } from './clearAuthStateLocally'; import { logoutOnSuccess } from './logoutOnSuccess'; type SetArgs = Parameters; @@ -105,3 +105,36 @@ describe.each( expect(consoleError).toHaveBeenCalledWith(failure); }); }); + +describe('clearAuthStateAfterSignOutElsewhere', () => { + it('signs this tab out and wipes both caches and its sessionStorage, but leaves the shared localStorage', () => { + seedSignedInTab(); + // The tab that signed out cleared localStorage; what is there now is that tab's, a new sign-in perhaps. + localStorage.clear(); + localStorage.setItem('Studio:PotentiallyAuthenticated', JSON.stringify({ OverallAppSignIn })); + + clearAuthStateAfterSignOutElsewhere(); + + expect(authStore.getConnectionById(OverallAppSignIn).user).toBeNull(); + expect(authStore.getConnectionById('ins-a').user).toBeNull(); + expectCachesEmpty(); + expect(sessionStorage.length).toBe(0); + expect(localStorage.length).toBe(1); + expect(localStorage.getItem('Studio:PotentiallyAuthenticated')).toBe(JSON.stringify({ OverallAppSignIn })); + }); + + it('still wipes the caches and sessionStorage when the store teardown throws', () => { + seedSignedInTab(); + const failure = new Error('teardown failed'); + vi.spyOn(authStore, 'forgetAppSignInEndedElsewhere').mockImplementation(() => { + throw failure; + }); + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => undefined); + + clearAuthStateAfterSignOutElsewhere(); + + expectCachesEmpty(); + expect(sessionStorage.length).toBe(0); + expect(consoleError).toHaveBeenCalledWith(failure); + }); +}); diff --git a/src/features/auth/store/appSignOutElsewhere.test.ts b/src/features/auth/store/appSignOutElsewhere.test.ts new file mode 100644 index 000000000..dcfdd2959 --- /dev/null +++ b/src/features/auth/store/appSignOutElsewhere.test.ts @@ -0,0 +1,127 @@ +/** @vitest-environment jsdom */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const { createInstanceAuthenticationTokens, getInstanceUserInfo } = vi.hoisted(() => ({ + createInstanceAuthenticationTokens: vi.fn(), + getInstanceUserInfo: vi.fn(), +})); +vi.mock('@/config/apiClient', () => ({ apiClient: { get: vi.fn(), defaults: { baseURL: 'https://cm.example.com' } } })); +vi.mock('@/integrations/api/instance/auth/createInstanceAuthenticationTokens', () => ({ + createInstanceAuthenticationTokens, + refreshInstanceOperationToken: vi.fn(), +})); +vi.mock('@/integrations/api/instance/status/getInstanceUserInfo', () => ({ getInstanceUserInfo })); + +import { authStore, OverallAppSignIn } from './authStore'; + +type SetArgs = Parameters; + +const MARKER = 'Studio:PotentiallyAuthenticated'; +const SIGNAL = 'Studio:AppSignOut'; +const DIRECT_URL = 'https://ins-direct.example.com:9925/'; + +function signIn() { + authStore.setUserForIdAndKey(OverallAppSignIn, OverallAppSignIn, { id: 'usr_a' } as SetArgs[2]); +} + +function storageEventFromAnotherTab(key: string | null, newValue: string | null) { + window.dispatchEvent(new StorageEvent('storage', { key, newValue, storageArea: localStorage })); +} + +beforeEach(() => { + localStorage.clear(); + sessionStorage.clear(); + vi.spyOn(console, 'debug').mockImplementation(() => undefined); +}); + +afterEach(() => { + authStore.signOutAllLocally(); + vi.resetAllMocks(); + vi.restoreAllMocks(); +}); + +describe('authStore.onAppSignedOutElsewhere', () => { + it('fires for another tab’s app sign-out, and for nothing else it writes', () => { + signIn(); + const signedOut = vi.fn(); + const stop = authStore.onAppSignedOutElsewhere(signedOut); + + // A cluster sign-out, a failed sign-in check dropping the app marker, an explorer clear, a storage wipe. + storageEventFromAnotherTab(MARKER, JSON.stringify({ OverallAppSignIn })); + storageEventFromAnotherTab(MARKER, '{}'); + storageEventFromAnotherTab('Studio:ExplorerAuthEpoch', JSON.stringify({ '*': 1 })); + storageEventFromAnotherTab(null, null); + expect(signedOut).not.toHaveBeenCalled(); + + storageEventFromAnotherTab(SIGNAL, '1'); + expect(signedOut).toHaveBeenCalledOnce(); + stop(); + }); + + it('does nothing in a tab that holds no app sign-in', () => { + const signedOut = vi.fn(); + const stop = authStore.onAppSignedOutElsewhere(signedOut); + + storageEventFromAnotherTab(SIGNAL, '1'); + + expect(signedOut).not.toHaveBeenCalled(); + stop(); + }); + + it('is announced by every local sign-out of an ended session, and not by the sweep before `/Logout/`', async () => { + signIn(); + await authStore.signOutFromPotentiallyAuthenticatedInstances(); + expect(localStorage.getItem(SIGNAL)).toBeNull(); + + const values = new Set(); + for (let i = 0; i < 2; i++) { + signIn(); + authStore.signOutAllLocally(); + values.add(localStorage.getItem(SIGNAL)); + } + values.delete(null); + expect(values.size).toBe(2); + }); +}); + +describe('authStore.forgetAppSignInEndedElsewhere', () => { + it('signs every entity out in memory without writing the storage the tabs share', () => { + signIn(); + authStore.setUserForIdAndKey('ins-a', 'https://ins-a.example.com', { username: 'u' } as SetArgs[2]); + // What the other tab wrote after signing out: a sign-in, perhaps as someone else. + localStorage.clear(); + localStorage.setItem(MARKER, JSON.stringify({ OverallAppSignIn })); + const writes = [ + vi.spyOn(Storage.prototype, 'setItem'), + vi.spyOn(Storage.prototype, 'removeItem'), + vi.spyOn(Storage.prototype, 'clear'), + ]; + + authStore.forgetAppSignInEndedElsewhere(); + + expect(authStore.isKnownSignedOut(OverallAppSignIn)).toBe(true); + expect(authStore.getConnectionById('ins-a').user).toBeNull(); + expect(authStore.getOperationsUrl('ins-a')).toBeUndefined(); + const localWrites = writes.flatMap((spy) => spy.mock.contexts).filter((storage) => storage === localStorage); + expect(localWrites).toEqual([]); + expect(localStorage.getItem(MARKER)).toBe(JSON.stringify({ OverallAppSignIn })); + }); + + it('drops a direct-connect token, and a sign-in check still in flight', async () => { + signIn(); + createInstanceAuthenticationTokens.mockResolvedValue({ operationToken: 'jwt', refreshToken: 'refresh' }); + getInstanceUserInfo.mockResolvedValueOnce({ username: 'direct' }); + await authStore.establishFabricConnectAuth({ id: 'ins-direct', operationsUrl: DIRECT_URL }); + expect(authStore.getOperationToken('ins-direct')).toBe('jwt'); + let answerCheck: (user: { username: string }) => void = () => undefined; + getInstanceUserInfo.mockReturnValueOnce(new Promise((resolve) => (answerCheck = resolve))); + const check = authStore.reloadUser('ins-direct'); + + authStore.forgetAppSignInEndedElsewhere(); + answerCheck({ username: 'direct' }); + + await expect(check).resolves.toBeNull(); + expect(authStore.getOperationToken('ins-direct')).toBeUndefined(); + expect(authStore.getConnectionById('ins-direct').user).toBeNull(); + }); +}); diff --git a/src/features/auth/store/authStore.ts b/src/features/auth/store/authStore.ts index c4005c46a..c3cbe58d8 100644 --- a/src/features/auth/store/authStore.ts +++ b/src/features/auth/store/authStore.ts @@ -99,6 +99,11 @@ class AuthStore { /** Same-tab epoch writes fire no `storage` event, so in-process subscribers are notified directly. */ private readonly explorerInvalidationListeners = new Set<() => void>(); + // Rewritten once the shared central-manager session is known to have ended, so the other tabs follow + // (`onAppSignedOutElsewhere`). Not the marker record: a tab also drops the app sign-in from that when its + // check merely failed, a timeout or a 5xx on `/User/current`, and that must not sign every other tab out. + private readonly appSignOutKey = 'Studio:AppSignOut'; + constructor() { this.potentiallyAuthenticated = JSON.parse(localStorage.getItem(this.potentiallyAuthenticatedKey) || '{}'); } @@ -730,6 +735,59 @@ class AuthStore { // Broadcast a global clear so other tabs' explorers drop their credentials even for entities not // in this tab's potentiallyAuthenticated set. this.bumpExplorerAuthEpochAll(); + this.announceAppSignOut(); + } + + /** + * Tells the other tabs the session ended. Only from `signOutAllLocally`, which runs once it has: after + * `/Logout/` succeeds, on a 401, after a password change. The sign-out sweep runs before `/Logout/`, which + * can fail, and a tab sent to sign-in that early could sign in again only for that `/Logout/` to end it. + * Best effort: a tab that misses it learns of it from its next request, as every tab did before. + */ + private announceAppSignOut(): void { + try { + // A new value every time, since storage fires an event only for a change. + localStorage.setItem(this.appSignOutKey, `${Date.now()}:${Math.random()}`); + } catch (error) { + console.debug('Could not tell the other tabs about this sign-out', error); + } + } + + /** + * Calls `onSignedOut` when another tab signs the app out while this tab holds the app sign-in. The + * central-manager cookie is shared, so the session that tab ended was this one's too, and nothing else + * would tell this tab: central manager answers a request with no session 403, as it answers a request + * the user is not permitted to make. + */ + public onAppSignedOutElsewhere(onSignedOut: () => void): () => void { + const handler = (event: StorageEvent) => { + if (event.key === this.appSignOutKey && this.potentiallyAuthenticated[OverallAppSignIn]) { + onSignedOut(); + } + }; + window.addEventListener('storage', handler); + return () => window.removeEventListener('storage', handler); + } + + /** + * Another tab ended the app sign-in: drop what this tab holds in memory for it, as `signOutAllLocally` + * does, but write no shared storage. The other tab clears that, and this tab's copy of the markers is a + * snapshot from its own page load, so writing it back could undo that tab's next write, a sign-in as + * someone else among them. + */ + public forgetAppSignInEndedElsewhere(): void { + this.bumpSignOutGeneration('*'); + forgetAllEntitySettings(); + const entityIds = new Set([...Object.keys(this.potentiallyAuthenticated), ...Object.keys(this.allConnections)]); + entityIds.delete(OverallAppSignIn); + for (const id of Object.keys(this.potentiallyAuthenticated)) { + delete this.potentiallyAuthenticated[id]; + } + this.dropFabricConnectState(); + for (const id of entityIds) { + this.updateConnectionIfChanged(id, false, null); + } + this.updateConnectionIfChanged(OverallAppSignIn, false, null); } private calculateKeyFromEntity(entity: EntityTypes): AuthenticatedConnectionKey | undefined { diff --git a/src/lib/installCrossTabSignOut.ts b/src/lib/installCrossTabSignOut.ts new file mode 100644 index 000000000..43e2e6b9c --- /dev/null +++ b/src/lib/installCrossTabSignOut.ts @@ -0,0 +1,18 @@ +import { clearAuthStateAfterSignOutElsewhere } from '@/features/auth/handlers/clearAuthStateLocally'; +import { authStore } from '@/features/auth/store/authStore'; + +let installed = false; + +/** + * Sign this tab out when another tab signs the app out, the way `installApiUnauthorizedRedirect` does on a + * central-manager 401: the cleared state re-runs the route guards, which redirect to /sign-in. Without it the + * tab stays on its page until a request fails on the ended session and toasts central manager's 403. Call + * once at startup; a second call (HMR) is a no-op. + */ +export function installCrossTabSignOut(): void { + if (installed) { + return; + } + installed = true; + authStore.onAppSignedOutElsewhere(clearAuthStateAfterSignOutElsewhere); +} diff --git a/src/main.tsx b/src/main.tsx index f29b2a000..7beb02944 100644 --- a/src/main.tsx +++ b/src/main.tsx @@ -4,6 +4,7 @@ import { scrubLegacySettings } from '@/features/instance/apis/explorer/settings' import { reactRootErrorHandlers } from '@/integrations/datadog/reportReactError'; import { installApiUnauthorizedRedirect } from '@/lib/installApiUnauthorizedRedirect'; import { installBrowserTranslationDomGuard } from '@/lib/installBrowserTranslationDomGuard'; +import { installCrossTabSignOut } from '@/lib/installCrossTabSignOut'; import { installStaleDeployReload } from '@/lib/installStaleDeployReload'; import { StrictMode } from 'react'; import { createRoot } from 'react-dom/client'; @@ -18,6 +19,8 @@ installStaleDeployReload(); // Redirect to /sign-in when a CM call returns 401 (lost/expired session), instead // of leaving the SPA on a stale user while every data call fails. installApiUnauthorizedRedirect(); +// Follow a sign-out made in another tab, which this tab otherwise learns of only as central manager's 403. +installCrossTabSignOut(); // Remove the pre-sessionStorage API-explorer credential map (plaintext Basic passwords / Bearer // tokens) at boot, so an upgraded user who never reopens the explorer is still scrubbed. scrubLegacySettings(); diff --git a/src/router/__tests__/crossTabSignOut.test.tsx b/src/router/__tests__/crossTabSignOut.test.tsx new file mode 100644 index 000000000..4291f2868 --- /dev/null +++ b/src/router/__tests__/crossTabSignOut.test.tsx @@ -0,0 +1,378 @@ +// @vitest-environment jsdom +import { type QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { cleanup, render, screen, waitFor } from '@testing-library/react'; +import { AxiosError, type InternalAxiosRequestConfig } from 'axios'; +import { toast } from 'sonner'; +import { afterEach, beforeEach, describe, expect, it, MockInstance, vi } from 'vitest'; + +const ORGANIZATION_ID = 'org-qmvwv3tdm45a9d4a'; +const CLUSTER_ID = 'clu-crosstab'; +const PAGE = `/${ORGANIZATION_ID}/cluster`; +const cloudUser = { id: 'usr_a', email: 'a@example.com', roles: {} }; + +const { routers } = vi.hoisted(() => ({ + routers: new Set<{ history: { destroy: () => void } }>(), +})); + +vi.mock('sonner', () => ({ toast: { error: vi.fn(), dismiss: vi.fn() } })); +vi.mock('@tanstack/react-router-devtools', () => ({ TanStackRouterDevtools: () => null })); +vi.mock('@/features/notifications/NotificationsSubscriptionManager', () => ({ + NotificationsSubscriptionManager: () => null, +})); +vi.mock('@/components/NotificationBanner', () => ({ NotificationBanner: () => null })); +vi.mock('@/components/Navbar', () => ({ Navbar: () => null })); +vi.mock('@/features/search/GlobalSearch', () => ({ + GlobalSearch: ({ children }: { children: unknown }) => children, + GlobalSearchTrigger: () => null, +})); +vi.mock('@/features/auth/AuthLayout', async () => { + const { Outlet } = await import('@tanstack/react-router'); + return { AuthLayout: () => }; +}); +vi.mock('@/features/auth/SignIn', () => ({ SignIn: () =>

sign-in page

})); + +/** Registered again after each `resetModules`, which a `vi.mock` factory's result outlives. */ +function mockAppModules() { + vi.doMock('@/integrations/datadog/datadog', async (importOriginal) => ({ + ...await importOriginal(), + useOnRouteLoadTracker: () => undefined, + })); + vi.doMock('@/router/useNewRouter', async (importOriginal) => { + const { useNewRouter } = await importOriginal(); + return { + useNewRouter: (...args: Parameters) => { + const router = useNewRouter(...args); + routers.add(router); + return router; + }, + }; + }); + // The real root, sign-in, dashboard and organization routes, under a page that polls its cluster the way + // the cluster pages do. + vi.doMock('@/router/rootRouteTree', async () => { + const { createRoute } = await import('@tanstack/react-router'); + const { useQuery } = await import('@tanstack/react-query'); + const { useCloudAuth } = await import('@/hooks/useAuth'); + const { getClusterInfoQueryOptions } = await import('@/features/cluster/queries/getClusterInfoQuery'); + const { rootRoute } = await import('@/router/rootRoute'); + const { authRouteTree } = await import('@/features/auth/routes'); + const { dashboardLayout } = await import('@/router/dashboardRoute'); + const { orgsLayoutRoute } = await import('@/features/organizations/routes'); + const { orgLayoutRoute } = await import('@/features/organization/routes'); + + function ClusterPage() { + useCloudAuth(); + const { data } = useQuery(getClusterInfoQueryOptions(CLUSTER_ID, true)); + return

{data ? `cluster ${data.name}` : 'loading cluster'}

; + } + const clusterRoute = createRoute({ + getParentRoute: () => orgLayoutRoute, + path: '/cluster', + component: ClusterPage, + }); + return { + rootRouteTree: rootRoute.addChildren([ + authRouteTree, + dashboardLayout.addChildren([orgsLayoutRoute.addChildren([orgLayoutRoute.addChildren([clusterRoute])])]), + ]), + }; + }); +} + +mockAppModules(); +await Promise.all([import('@/lib/installApiUnauthorizedRedirect'), import('@/AppRouted')]); + +/** + * The central-manager session every tab shares through its cookie. Central manager answers a request with no + * session 403 `Not allowed` (`checkUser` → `notAllowed()`, HarperFast/central-manager#938), exactly as it + * answers one the user may not make; only `/User/current` says 401. + */ +let session: 'live' | 'ended' | 'ended-with-401' | 'cluster-forbidden'; +const requests: string[] = []; +const requestsAfterSessionEnded: string[] = []; + +function respond(config: InternalAxiosRequestConfig, status: number, data: unknown) { + const response = { status, statusText: String(status), data, headers: {}, config }; + if (status < 300) { + return response; + } + throw new AxiosError( + `Request failed with status code ${status}`, + AxiosError.ERR_BAD_RESPONSE, + config, + undefined, + response, + ); +} + +function notAllowed(config: InternalAxiosRequestConfig) { + return respond(config, 403, { + type: 'error:ClientError', + code: 'ClientError', + title: 'Not allowed', + status: 403, + instance: config.url, + }); +} + +function centralManager(config: InternalAxiosRequestConfig) { + requests.push(config.url ?? ''); + if (session !== 'live' && session !== 'cluster-forbidden') { + requestsAfterSessionEnded.push(config.url ?? ''); + return session === 'ended-with-401' || config.url === '/User/current' + ? respond(config, 401, { code: 'ClientError', title: 'Invalid cookie' }) + : notAllowed(config); + } + switch (config.url) { + case '/User/current': + return respond(config, 200, cloudUser); + case `/Organization/${ORGANIZATION_ID}`: + return respond(config, 200, { id: ORGANIZATION_ID, name: 'Acme' }); + case `/Cluster/${CLUSTER_ID}`: + return session === 'cluster-forbidden' + ? notAllowed(config) + : respond(config, 200, { id: CLUSTER_ID, name: 'Prod', status: 'RUNNING' }); + } + throw new Error(`unexpected request ${config.url}`); +} + +/** This tab: the cluster page, signed in, wired as `main.tsx` wires it. */ +async function openClusterPage() { + vi.resetModules(); + mockAppModules(); + const [ + { apiClient }, + { authStore }, + { installApiUnauthorizedRedirect }, + { installCrossTabSignOut }, + { queryClient }, + { AppRouted }, + ] = await Promise.all([ + import('@/config/apiClient'), + import('@/features/auth/store/authStore'), + import('@/lib/installApiUnauthorizedRedirect'), + import('@/lib/installCrossTabSignOut'), + import('@/react-query/queryClient'), + import('@/AppRouted'), + ]); + apiClient.defaults.adapter = async (config) => centralManager(config); + installApiUnauthorizedRedirect(); + installCrossTabSignOut(); + render( + + + , + ); + await screen.findByText('cluster Prod'); + return { authStore, queryClient }; +} + +type StorageChange = Pick; + +/** + * Another tab of the same browser: its own copy of the app's modules, sharing this tab's `localStorage` and + * central-manager session but not its `sessionStorage`. `run` returns once that tab has done `step`, having + * delivered each change it made to `localStorage` here as the `storage` event a browser fires in every other tab. + */ +async function openAnotherTab() { + vi.resetModules(); + const [{ authStore }, { apiClient }, { logoutOnSuccess }, { clearAuthStateLocally }] = await Promise.all([ + import('@/features/auth/store/authStore'), + import('@/config/apiClient'), + import('@/features/auth/handlers/logoutOnSuccess'), + import('@/features/auth/handlers/clearAuthStateLocally'), + ]); + apiClient.defaults.adapter = async (config) => centralManager(config); + return { + authStore, + apiClient, + logoutOnSuccess, + clearAuthStateLocally, + run: async (step: () => unknown) => { + for (const change of await localStorageChanges(step)) { + window.dispatchEvent(new StorageEvent('storage', { ...change, storageArea: localStorage })); + } + }, + }; +} + +async function localStorageChanges(step: () => unknown): Promise { + const changes: StorageChange[] = []; + const { clear, removeItem, setItem } = Storage.prototype; + const spies = [ + vi.spyOn(Storage.prototype, 'setItem').mockImplementation(function(this: Storage, key, value) { + if (this !== localStorage) { + return; + } + const oldValue = this.getItem(key); + setItem.call(this, key, value); + if (oldValue !== value) { + changes.push({ key, oldValue, newValue: value }); + } + }), + vi.spyOn(Storage.prototype, 'removeItem').mockImplementation(function(this: Storage, key) { + if (this !== localStorage) { + return; + } + const oldValue = this.getItem(key); + removeItem.call(this, key); + if (oldValue !== null) { + changes.push({ key, oldValue, newValue: null }); + } + }), + vi.spyOn(Storage.prototype, 'clear').mockImplementation(function(this: Storage) { + if (this !== localStorage) { + return; + } + const hadItems = this.length > 0; + clear.call(this); + if (hadItems) { + changes.push({ key: null, oldValue: null, newValue: null }); + } + }), + ]; + try { + await step(); + } finally { + for (const spy of spies) { + spy.mockRestore(); + } + } + return changes; +} + +/** This tab's next poll of its page data, as its 10s `refetchInterval` or a refocus would send it. */ +async function nextPoll(queryClient: QueryClient) { + await queryClient.refetchQueries({ type: 'active' }); + await waitFor(() => expect(queryClient.isFetching()).toBe(0)); +} + +/** Needs no wait: a sign-out reaches the store synchronously, and its cache clear leaves the next poll nothing to send. */ +async function expectStillSignedIn(thisTab: Awaited>) { + expect(thisTab.authStore.getConnectionById('OverallAppSignIn').user).toEqual(cloudUser); + const polled = requests.length; + await nextPoll(thisTab.queryClient); + expect(requests.slice(polled)).toContain(`/Cluster/${CLUSTER_ID}`); + expect(screen.getByText('cluster Prod')).toBeTruthy(); + expect(toast.error).not.toHaveBeenCalled(); +} + +let consoleError: MockInstance; + +function reportedAuthorizationFailures() { + return consoleError.mock.calls.filter(([error]) => error instanceof AxiosError && error.response?.status === 403); +} + +beforeEach(() => { + localStorage.clear(); + sessionStorage.clear(); + localStorage.setItem('Studio:PotentiallyAuthenticated', JSON.stringify({ OverallAppSignIn: 'OverallAppSignIn' })); + window.history.replaceState(null, '', `#${PAGE}`); + session = 'live'; + requests.length = 0; + requestsAfterSessionEnded.length = 0; + consoleError = vi.spyOn(console, 'error').mockImplementation(() => undefined); + vi.spyOn(console, 'debug').mockImplementation(() => undefined); +}); + +afterEach(() => { + cleanup(); + for (const router of routers) { + router.history.destroy(); + } + routers.clear(); + vi.clearAllMocks(); + vi.restoreAllMocks(); +}); + +describe('a sign-out in another tab', () => { + it('signs this tab out once that tab’s `/Logout/` succeeds, before its next poll can toast', async () => { + const { queryClient } = await openClusterPage(); + const otherTab = await openAnotherTab(); + + // What `useLogoutMutation` runs there: the sweep, central manager's `/Logout/`, then `logoutOnSuccess`. + await otherTab.run(() => otherTab.authStore.signOutFromPotentiallyAuthenticatedInstances()); + session = 'ended'; + await otherTab.run(() => otherTab.logoutOnSuccess()); + // The poll, or the refocus, that toasted "ClientError: Not allowed" before. + await nextPoll(queryClient); + + expect(toast.error).not.toHaveBeenCalled(); + expect(requestsAfterSessionEnded).toEqual([]); + expect(reportedAuthorizationFailures()).toEqual([]); + await screen.findByText('sign-in page'); + }); + + it('leaves this tab signed in when that tab’s `/Logout/` fails', async () => { + const thisTab = await openClusterPage(); + const otherTab = await openAnotherTab(); + + // The sweep runs first; a failed `/Logout/` skips `logoutOnSuccess`, and the session stays live. + await otherTab.run(() => otherTab.authStore.signOutFromPotentiallyAuthenticatedInstances()); + + await expectStillSignedIn(thisTab); + }); + + it('signs this tab out when another tab found the session ended', async () => { + const { queryClient } = await openClusterPage(); + const otherTab = await openAnotherTab(); + + // The session expired; the other tab heard first, from a 401, and cleared it as the 401 handler does. + session = 'ended'; + await otherTab.run(() => otherTab.clearAuthStateLocally()); + await nextPoll(queryClient); + + expect(toast.error).not.toHaveBeenCalled(); + expect(requestsAfterSessionEnded).toEqual([]); + await screen.findByText('sign-in page'); + }); + + it('leaves this tab signed in when another tab only signs in or out of a cluster', async () => { + const thisTab = await openClusterPage(); + const otherTab = await openAnotherTab(); + + await otherTab.run(() => + otherTab.authStore.setUserForIdAndKey('clu-other', 'https://clu-other.example.com', { username: 'u' } as never) + ); + await otherTab.run(() => otherTab.authStore.signOutLocally('clu-other')); + + await expectStillSignedIn(thisTab); + }); + + it('leaves this tab signed in when another tab’s sign-in check merely fails', async () => { + const thisTab = await openClusterPage(); + const otherTab = await openAnotherTab(); + // A reload during a central-manager blip: that tab reads the failed check as signed out, and drops the app + // sign-in from the marker record both tabs share. + otherTab.apiClient.defaults.adapter = async (config) => { + throw new AxiosError('timeout of 60000ms exceeded', AxiosError.ECONNABORTED, config); + }; + + await otherTab.run(() => otherTab.authStore.reloadUser('OverallAppSignIn')); + expect(localStorage.getItem('Studio:PotentiallyAuthenticated')).toBe('{}'); + + await expectStillSignedIn(thisTab); + }); +}); + +describe('a session that ends with no other tab signing out', () => { + it('still toasts a refusal central manager gives this tab while it is signed in', async () => { + const { queryClient } = await openClusterPage(); + + session = 'cluster-forbidden'; + await nextPoll(queryClient); + + expect(toast.error).toHaveBeenCalledWith('ClientError', expect.objectContaining({ description: 'Not allowed' })); + expect(screen.getByText('cluster Prod')).toBeTruthy(); + }); + + it('signs this tab out without a toast when central manager answers its poll 401', async () => { + const { queryClient } = await openClusterPage(); + + session = 'ended-with-401'; + await nextPoll(queryClient); + + await screen.findByText('sign-in page'); + expect(toast.error).not.toHaveBeenCalled(); + }); +}); From 529d1fc187bf052fdafd0f75fc9f44b8b1953bb2 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Tue, 13 Oct 2026 12:19:49 -0400 Subject: [PATCH 2/3] fix(auth): follow another tab's sign-out only for this tab's own user The `Studio:AppSignOut` announcement carried no identity, so a tab followed any of them while it held an app sign-in. A 401 that ended user U's session in one tab, or U's sign-out reaching a tab after it had signed in as V, signed out a tab holding a live V session. The announcement now names the user whose session ended, and a tab follows only when that is its own app user. The sign-out sweep clears the user before `/Logout/`, so it remembers them for the announcement `signOutAllLocally` makes afterwards. A tab whose sign-in check has not answered yet leaves it to that check. Also sets the installer's flag only once the listener is registered, and drops comments that restated DESIGN.md. Co-Authored-By: Claude Opus 5.5 --- DESIGN.md | 3 + .../auth/handlers/clearAuthStateLocally.ts | 5 -- .../auth/store/appSignOutElsewhere.test.ts | 61 ++++++++++++++---- src/features/auth/store/authStore.ts | 55 ++++++++-------- src/lib/installCrossTabSignOut.ts | 8 +-- src/main.tsx | 1 - src/router/__tests__/crossTabSignOut.test.tsx | 63 +++++++++++++++---- 7 files changed, 135 insertions(+), 61 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index e7dacccfc..c3688d9c9 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -123,6 +123,9 @@ Central manager only ever sends the boolean, never the other organizations' ids; permission refusal. `signOutAllLocally` runs once the shared session has ended: after `/Logout/` succeeds, on a 401, after a password change. The sign-out sweep does not announce, because it runs before `/Logout/`: a failed logout would sign the other tabs out of a live session, and one signed back in meanwhile would be ended by that `/Logout/`. + The value names the user whose session ended (the sweep remembers them, since it clears the user before `/Logout/`), + and a tab follows only when that is its own app user: a late 401 or event from user U's session must not sign out a + tab that has since signed in as V, and a tab whose check has not answered yet waits for that check instead. Tabs follow that key, not the marker record, because a tab also drops the app sign-in from the record when its own check merely fails (a timeout, a 5xx). A tab that follows writes no `localStorage`: its copy of the markers is a snapshot from its own load, and writing it back could undo the other tab's next sign-in (`crossTabSignOut.test.tsx`). diff --git a/src/features/auth/handlers/clearAuthStateLocally.ts b/src/features/auth/handlers/clearAuthStateLocally.ts index ce6b05009..f909c06d1 100644 --- a/src/features/auth/handlers/clearAuthStateLocally.ts +++ b/src/features/auth/handlers/clearAuthStateLocally.ts @@ -18,11 +18,6 @@ export function clearAuthStateLocally(): void { ]); } -/** - * This tab's half of a sign-out another tab made: the same clears as {@link clearAuthStateLocally}, except - * the `localStorage` both tabs share, which the tab that signed out clears and this one must not write - * (`authStore.forgetAppSignInEndedElsewhere`). - */ export function clearAuthStateAfterSignOutElsewhere(): void { runEvery([ () => authStore.forgetAppSignInEndedElsewhere(), diff --git a/src/features/auth/store/appSignOutElsewhere.test.ts b/src/features/auth/store/appSignOutElsewhere.test.ts index dcfdd2959..898a8382c 100644 --- a/src/features/auth/store/appSignOutElsewhere.test.ts +++ b/src/features/auth/store/appSignOutElsewhere.test.ts @@ -12,6 +12,7 @@ vi.mock('@/integrations/api/instance/auth/createInstanceAuthenticationTokens', ( })); vi.mock('@/integrations/api/instance/status/getInstanceUserInfo', () => ({ getInstanceUserInfo })); +import { apiClient } from '@/config/apiClient'; import { authStore, OverallAppSignIn } from './authStore'; type SetArgs = Parameters; @@ -28,6 +29,15 @@ function storageEventFromAnotherTab(key: string | null, newValue: string | null) window.dispatchEvent(new StorageEvent('storage', { key, newValue, storageArea: localStorage })); } +function signOutOf(user: unknown) { + return JSON.stringify({ user, at: '1' }); +} + +function announcedUser() { + const value = localStorage.getItem(SIGNAL); + return value === null ? undefined : (JSON.parse(value) as { user: unknown }).user; +} + beforeEach(() => { localStorage.clear(); sessionStorage.clear(); @@ -41,7 +51,7 @@ afterEach(() => { }); describe('authStore.onAppSignedOutElsewhere', () => { - it('fires for another tab’s app sign-out, and for nothing else it writes', () => { + it('fires for another tab’s sign-out of this tab’s user, and for nothing else it writes', () => { signIn(); const signedOut = vi.fn(); const stop = authStore.onAppSignedOutElsewhere(signedOut); @@ -51,9 +61,15 @@ describe('authStore.onAppSignedOutElsewhere', () => { storageEventFromAnotherTab(MARKER, '{}'); storageEventFromAnotherTab('Studio:ExplorerAuthEpoch', JSON.stringify({ '*': 1 })); storageEventFromAnotherTab(null, null); + // Someone else's session, a sign-out that knew no user, and values no sign-out writes. + storageEventFromAnotherTab(SIGNAL, signOutOf('usr_v')); + storageEventFromAnotherTab(SIGNAL, signOutOf(null)); + storageEventFromAnotherTab(SIGNAL, '1'); + storageEventFromAnotherTab(SIGNAL, 'null'); + storageEventFromAnotherTab(SIGNAL, '{'); expect(signedOut).not.toHaveBeenCalled(); - storageEventFromAnotherTab(SIGNAL, '1'); + storageEventFromAnotherTab(SIGNAL, signOutOf('usr_a')); expect(signedOut).toHaveBeenCalledOnce(); stop(); }); @@ -62,25 +78,46 @@ describe('authStore.onAppSignedOutElsewhere', () => { const signedOut = vi.fn(); const stop = authStore.onAppSignedOutElsewhere(signedOut); - storageEventFromAnotherTab(SIGNAL, '1'); + storageEventFromAnotherTab(SIGNAL, signOutOf('usr_a')); + storageEventFromAnotherTab(SIGNAL, signOutOf(null)); expect(signedOut).not.toHaveBeenCalled(); stop(); }); - it('is announced by every local sign-out of an ended session, and not by the sweep before `/Logout/`', async () => { + it('leaves a tab whose sign-in check has not answered to that check', () => { + signIn(); + vi.mocked(apiClient.get).mockReturnValue(new Promise(() => undefined)); + void authStore.reloadUser(OverallAppSignIn); + expect(authStore.getConnectionById(OverallAppSignIn)).toEqual({ isLoading: true, user: null }); + const signedOut = vi.fn(); + const stop = authStore.onAppSignedOutElsewhere(signedOut); + + storageEventFromAnotherTab(SIGNAL, signOutOf('usr_a')); + + expect(signedOut).not.toHaveBeenCalled(); + stop(); + }); + + it('names the ended user from every local sign-out, and is not written by the sweep before `/Logout/`', async () => { signIn(); await authStore.signOutFromPotentiallyAuthenticatedInstances(); expect(localStorage.getItem(SIGNAL)).toBeNull(); + // `logoutOnSuccess`, after the sweep has already cleared the user. + authStore.signOutAllLocally(); + expect(announcedUser()).toBe('usr_a'); + + signIn(); + authStore.signOutAllLocally(); + const first = localStorage.getItem(SIGNAL); + signIn(); + authStore.signOutAllLocally(); + expect(announcedUser()).toBe('usr_a'); + expect(localStorage.getItem(SIGNAL)).not.toBe(first); - const values = new Set(); - for (let i = 0; i < 2; i++) { - signIn(); - authStore.signOutAllLocally(); - values.add(localStorage.getItem(SIGNAL)); - } - values.delete(null); - expect(values.size).toBe(2); + // A tab with no user to name: nothing for another tab to match. + authStore.signOutAllLocally(); + expect(announcedUser()).toBeNull(); }); }); diff --git a/src/features/auth/store/authStore.ts b/src/features/auth/store/authStore.ts index c3cbe58d8..768902e24 100644 --- a/src/features/auth/store/authStore.ts +++ b/src/features/auth/store/authStore.ts @@ -99,10 +99,9 @@ class AuthStore { /** Same-tab epoch writes fire no `storage` event, so in-process subscribers are notified directly. */ private readonly explorerInvalidationListeners = new Set<() => void>(); - // Rewritten once the shared central-manager session is known to have ended, so the other tabs follow - // (`onAppSignedOutElsewhere`). Not the marker record: a tab also drops the app sign-in from that when its - // check merely failed, a timeout or a 5xx on `/User/current`, and that must not sign every other tab out. private readonly appSignOutKey = 'Studio:AppSignOut'; + // The sweep clears the app user before `/Logout/`, and the announcement after it must still name them. + private sweptAppUser: string | null = null; constructor() { this.potentiallyAuthenticated = JSON.parse(localStorage.getItem(this.potentiallyAuthenticatedKey) || '{}'); @@ -638,6 +637,7 @@ class AuthStore { } public async signOutFromPotentiallyAuthenticatedInstances() { + this.sweptAppUser = this.appUser() ?? this.sweptAppUser; // Dooms connects for entities not yet in `potentiallyAuthenticated`, which the loop skips. this.bumpSignOutGeneration('*'); forgetAllEntitySettings(); @@ -724,6 +724,8 @@ class AuthStore { * per-instance logouts. */ public signOutAllLocally(): void { + const endedUser = this.appUser() ?? this.sweptAppUser; + this.sweptAppUser = null; this.bumpSignOutGeneration('*'); forgetAllEntitySettings(); // Snapshot keys first — signOutLocally mutates the flags as it goes. @@ -735,33 +737,34 @@ class AuthStore { // Broadcast a global clear so other tabs' explorers drop their credentials even for entities not // in this tab's potentiallyAuthenticated set. this.bumpExplorerAuthEpochAll(); - this.announceAppSignOut(); + this.announceAppSignOut(endedUser); } - /** - * Tells the other tabs the session ended. Only from `signOutAllLocally`, which runs once it has: after - * `/Logout/` succeeds, on a 401, after a password change. The sign-out sweep runs before `/Logout/`, which - * can fail, and a tab sent to sign-in that early could sign in again only for that `/Logout/` to end it. - * Best effort: a tab that misses it learns of it from its next request, as every tab did before. - */ - private announceAppSignOut(): void { + /** The signed-in app user's id, or a local instance user's name. */ + private appUser(): string | null { + const user = this.allConnections[OverallAppSignIn]?.user; + if (!user) { + return null; + } + return 'id' in user ? user.id : user.username; + } + + private announceAppSignOut(user: string | null): void { try { // A new value every time, since storage fires an event only for a change. - localStorage.setItem(this.appSignOutKey, `${Date.now()}:${Math.random()}`); + localStorage.setItem(this.appSignOutKey, JSON.stringify({ user, at: `${Date.now()}:${Math.random()}` })); } catch (error) { console.debug('Could not tell the other tabs about this sign-out', error); } } - /** - * Calls `onSignedOut` when another tab signs the app out while this tab holds the app sign-in. The - * central-manager cookie is shared, so the session that tab ended was this one's too, and nothing else - * would tell this tab: central manager answers a request with no session 403, as it answers a request - * the user is not permitted to make. - */ public onAppSignedOutElsewhere(onSignedOut: () => void): () => void { const handler = (event: StorageEvent) => { - if (event.key === this.appSignOutKey && this.potentiallyAuthenticated[OverallAppSignIn]) { + if (event.key !== this.appSignOutKey || !event.newValue) { + return; + } + const user = this.appUser(); + if (user !== null && signedOutUser(event.newValue) === user) { onSignedOut(); } }; @@ -769,12 +772,6 @@ class AuthStore { return () => window.removeEventListener('storage', handler); } - /** - * Another tab ended the app sign-in: drop what this tab holds in memory for it, as `signOutAllLocally` - * does, but write no shared storage. The other tab clears that, and this tab's copy of the markers is a - * snapshot from its own page load, so writing it back could undo that tab's next write, a sign-in as - * someone else among them. - */ public forgetAppSignInEndedElsewhere(): void { this.bumpSignOutGeneration('*'); forgetAllEntitySettings(); @@ -930,6 +927,14 @@ class SignedOutWhileConnectingError extends Error { } } +function signedOutUser(announcement: string): unknown { + try { + return (JSON.parse(announcement) as { user?: unknown } | null)?.user; + } catch { + return undefined; + } +} + // `console.debug`, never `console.error`: the RUM SDK reports `console.error` as an error, which would // put a failure the sign-out deliberately carries past into Error Tracking. Message only: the full // Axios error carries the request credentials. diff --git a/src/lib/installCrossTabSignOut.ts b/src/lib/installCrossTabSignOut.ts index 43e2e6b9c..7b11c9e00 100644 --- a/src/lib/installCrossTabSignOut.ts +++ b/src/lib/installCrossTabSignOut.ts @@ -3,16 +3,10 @@ import { authStore } from '@/features/auth/store/authStore'; let installed = false; -/** - * Sign this tab out when another tab signs the app out, the way `installApiUnauthorizedRedirect` does on a - * central-manager 401: the cleared state re-runs the route guards, which redirect to /sign-in. Without it the - * tab stays on its page until a request fails on the ended session and toasts central manager's 403. Call - * once at startup; a second call (HMR) is a no-op. - */ export function installCrossTabSignOut(): void { if (installed) { return; } - installed = true; authStore.onAppSignedOutElsewhere(clearAuthStateAfterSignOutElsewhere); + installed = true; } diff --git a/src/main.tsx b/src/main.tsx index 7beb02944..ae16180fe 100644 --- a/src/main.tsx +++ b/src/main.tsx @@ -19,7 +19,6 @@ installStaleDeployReload(); // Redirect to /sign-in when a CM call returns 401 (lost/expired session), instead // of leaving the SPA on a stale user while every data call fails. installApiUnauthorizedRedirect(); -// Follow a sign-out made in another tab, which this tab otherwise learns of only as central manager's 403. installCrossTabSignOut(); // Remove the pre-sessionStorage API-explorer credential map (plaintext Basic passwords / Bearer // tokens) at boot, so an upgraded user who never reopens the explorer is still scrubbed. diff --git a/src/router/__tests__/crossTabSignOut.test.tsx b/src/router/__tests__/crossTabSignOut.test.tsx index 4291f2868..f9e56fc0d 100644 --- a/src/router/__tests__/crossTabSignOut.test.tsx +++ b/src/router/__tests__/crossTabSignOut.test.tsx @@ -170,32 +170,42 @@ async function openClusterPage() { type StorageChange = Pick; /** - * Another tab of the same browser: its own copy of the app's modules, sharing this tab's `localStorage` and - * central-manager session but not its `sessionStorage`. `run` returns once that tab has done `step`, having - * delivered each change it made to `localStorage` here as the `storage` event a browser fires in every other tab. + * Another tab of the same browser, signed in as its page load would sign it in: its own copy of the app's + * modules, sharing this tab's `localStorage` and central-manager session but not its `sessionStorage`. + * `record` returns the changes `step` made to `localStorage`; `deliver` hands them to this tab as the + * `storage` events a browser fires in every other tab; `run` does both. */ async function openAnotherTab() { vi.resetModules(); - const [{ authStore }, { apiClient }, { logoutOnSuccess }, { clearAuthStateLocally }] = await Promise.all([ + const [{ authStore }, { apiClient }, { installApiUnauthorizedRedirect }, { logoutOnSuccess }, { + clearAuthStateLocally, + }] = await Promise.all([ import('@/features/auth/store/authStore'), import('@/config/apiClient'), + import('@/lib/installApiUnauthorizedRedirect'), import('@/features/auth/handlers/logoutOnSuccess'), import('@/features/auth/handlers/clearAuthStateLocally'), ]); apiClient.defaults.adapter = async (config) => centralManager(config); + installApiUnauthorizedRedirect(); + await authStore.reloadUser('OverallAppSignIn'); + const record = (step: () => unknown) => localStorageChanges(step); return { authStore, apiClient, logoutOnSuccess, clearAuthStateLocally, - run: async (step: () => unknown) => { - for (const change of await localStorageChanges(step)) { - window.dispatchEvent(new StorageEvent('storage', { ...change, storageArea: localStorage })); - } - }, + record, + run: async (step: () => unknown) => deliver(await record(step)), }; } +function deliver(changes: StorageChange[]) { + for (const change of changes) { + window.dispatchEvent(new StorageEvent('storage', { ...change, storageArea: localStorage })); + } +} + async function localStorageChanges(step: () => unknown): Promise { const changes: StorageChange[] = []; const { clear, removeItem, setItem } = Storage.prototype; @@ -248,8 +258,8 @@ async function nextPoll(queryClient: QueryClient) { } /** Needs no wait: a sign-out reaches the store synchronously, and its cache clear leaves the next poll nothing to send. */ -async function expectStillSignedIn(thisTab: Awaited>) { - expect(thisTab.authStore.getConnectionById('OverallAppSignIn').user).toEqual(cloudUser); +async function expectStillSignedIn(thisTab: Awaited>, user: object = cloudUser) { + expect(thisTab.authStore.getConnectionById('OverallAppSignIn').user).toEqual(user); const polled = requests.length; await nextPoll(thisTab.queryClient); expect(requests.slice(polled)).toContain(`/Cluster/${CLUSTER_ID}`); @@ -327,6 +337,37 @@ describe('a sign-out in another tab', () => { await screen.findByText('sign-in page'); }); + it('leaves this tab signed in when a 401 in another tab ends a different user’s session', async () => { + const thisTab = await openClusterPage(); + const otherTab = await openAnotherTab(); + // That tab still holds user U from before this tab signed in as the current user. + otherTab.authStore.setUserForIdAndKey('OverallAppSignIn', 'OverallAppSignIn', { id: 'usr_u' } as never); + otherTab.apiClient.defaults.adapter = async (config) => + respond(config, 401, { code: 'ClientError', title: 'Invalid cookie' }); + + await otherTab.run(() => + otherTab.apiClient.get(`/Organization/${ORGANIZATION_ID}` as '/Organization/{id}').catch(() => undefined) + ); + expect(otherTab.authStore.isKnownSignedOut('OverallAppSignIn')).toBe(true); + + await expectStillSignedIn(thisTab); + }); + + it('leaves this tab signed in when a sign-out reaches it after it signed in as someone else', async () => { + const thisTab = await openClusterPage(); + const otherTab = await openAnotherTab(); + const signedInSince = { id: 'usr_v', email: 'v@example.com', roles: {} }; + + const changes = [ + ...await otherTab.record(() => otherTab.authStore.signOutFromPotentiallyAuthenticatedInstances()), + ...await otherTab.record(() => otherTab.logoutOnSuccess()), + ]; + thisTab.authStore.setUserForIdAndKey('OverallAppSignIn', 'OverallAppSignIn', signedInSince as never); + deliver(changes); + + await expectStillSignedIn(thisTab, signedInSince); + }); + it('leaves this tab signed in when another tab only signs in or out of a cluster', async () => { const thisTab = await openClusterPage(); const otherTab = await openAnotherTab(); From c25e21b81c1e69b648cf2c6e85dfa0db0d32098d Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Tue, 13 Oct 2026 12:58:18 -0400 Subject: [PATCH 3/3] fix(auth): re-check a sign-in that was out when another tab signed out A tab whose app sign-in check was in flight when another tab announced the end of its user's session ignored the announcement, because it had no user to compare yet. If central manager had answered that check just before the `/Logout/`, the tab then signed in on the ended session and its next poll toasted the 403 this branch removes. The tab now remembers the announced user, and if its check comes back as them, asks central manager again. signOutAllLocally now announces from a `finally`, so a throwing step in its teardown, such as an explorer listener, cannot cost the other tabs the announcement after the ended user's identity was already taken. The mounted test removes each test's `storage` listeners afterwards, so none acts on a later test's events, and drops comments that only narrated the next step. Co-Authored-By: Claude Opus 5.5 --- DESIGN.md | 3 +- .../auth/store/appSignOutElsewhere.test.ts | 45 +++++++++++++-- src/features/auth/store/authStore.ts | 44 +++++++++----- src/router/__tests__/crossTabSignOut.test.tsx | 57 +++++++++++++++++-- 4 files changed, 123 insertions(+), 26 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index c3688d9c9..ad6c61eb5 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -125,7 +125,8 @@ Central manager only ever sends the boolean, never the other organizations' ids; would sign the other tabs out of a live session, and one signed back in meanwhile would be ended by that `/Logout/`. The value names the user whose session ended (the sweep remembers them, since it clears the user before `/Logout/`), and a tab follows only when that is its own app user: a late 401 or event from user U's session must not sign out a - tab that has since signed in as V, and a tab whose check has not answered yet waits for that check instead. + tab that has since signed in as V. A tab whose check is still out remembers the user, and if the check comes back as + them, asks again: central manager may have confirmed them just before the session ended. Tabs follow that key, not the marker record, because a tab also drops the app sign-in from the record when its own check merely fails (a timeout, a 5xx). A tab that follows writes no `localStorage`: its copy of the markers is a snapshot from its own load, and writing it back could undo the other tab's next sign-in (`crossTabSignOut.test.tsx`). diff --git a/src/features/auth/store/appSignOutElsewhere.test.ts b/src/features/auth/store/appSignOutElsewhere.test.ts index 898a8382c..ee1e6cabd 100644 --- a/src/features/auth/store/appSignOutElsewhere.test.ts +++ b/src/features/auth/store/appSignOutElsewhere.test.ts @@ -85,20 +85,55 @@ describe('authStore.onAppSignedOutElsewhere', () => { stop(); }); - it('leaves a tab whose sign-in check has not answered to that check', () => { + it.each([ + ['the user it named', 'usr_a', 2], + ['someone else', 'usr_v', 1], + ])('asks again when a check that was out during a sign-out of %s comes back as them', async (_, ended, checks) => { signIn(); - vi.mocked(apiClient.get).mockReturnValue(new Promise(() => undefined)); - void authStore.reloadUser(OverallAppSignIn); - expect(authStore.getConnectionById(OverallAppSignIn)).toEqual({ isLoading: true, user: null }); + let answer: (response: { data: { id: string } }) => void = () => undefined; + vi.mocked(apiClient.get) + .mockReturnValueOnce(new Promise((resolve) => (answer = resolve))) + .mockReturnValue(new Promise(() => undefined)); + const check = authStore.reloadUser(OverallAppSignIn); const signedOut = vi.fn(); const stop = authStore.onAppSignedOutElsewhere(signedOut); - storageEventFromAnotherTab(SIGNAL, signOutOf('usr_a')); + storageEventFromAnotherTab(SIGNAL, signOutOf(ended)); + answer({ data: { id: 'usr_a' } }); + await check; expect(signedOut).not.toHaveBeenCalled(); + expect(apiClient.get).toHaveBeenCalledTimes(checks); + stop(); + }); + + it('does not ask again on a later check in a tab that was signed out when it heard', async () => { + const stop = authStore.onAppSignedOutElsewhere(vi.fn()); + storageEventFromAnotherTab(SIGNAL, signOutOf('usr_a')); + signIn(); + vi.mocked(apiClient.get).mockResolvedValueOnce({ data: { id: 'usr_a' } }).mockReturnValue( + new Promise(() => undefined), + ); + + await authStore.reloadUser(OverallAppSignIn); + + expect(apiClient.get).toHaveBeenCalledOnce(); stop(); }); + it('still names the ended user when the teardown throws', () => { + signIn(); + const failure = new Error('explorer listener failed'); + const stop = authStore.onExplorerAuthInvalidated(OverallAppSignIn, () => { + throw failure; + }); + + expect(() => authStore.signOutAllLocally()).toThrow(failure); + stop(); + + expect(announcedUser()).toBe('usr_a'); + }); + it('names the ended user from every local sign-out, and is not written by the sweep before `/Logout/`', async () => { signIn(); await authStore.signOutFromPotentiallyAuthenticatedInstances(); diff --git a/src/features/auth/store/authStore.ts b/src/features/auth/store/authStore.ts index 768902e24..c9cf5cb36 100644 --- a/src/features/auth/store/authStore.ts +++ b/src/features/auth/store/authStore.ts @@ -102,6 +102,8 @@ class AuthStore { private readonly appSignOutKey = 'Studio:AppSignOut'; // The sweep clears the app user before `/Logout/`, and the announcement after it must still name them. private sweptAppUser: string | null = null; + // Central manager may have confirmed that user before the session ended, so the check is asked again. + private signOutHeardDuringCheck: string | null = null; constructor() { this.potentiallyAuthenticated = JSON.parse(localStorage.getItem(this.potentiallyAuthenticatedKey) || '{}'); @@ -726,21 +728,23 @@ class AuthStore { public signOutAllLocally(): void { const endedUser = this.appUser() ?? this.sweptAppUser; this.sweptAppUser = null; - this.bumpSignOutGeneration('*'); - forgetAllEntitySettings(); - // Snapshot keys first — signOutLocally mutates the flags as it goes. - for (const id of Object.keys(this.potentiallyAuthenticated)) { - this.signOutLocally(id); + try { + this.bumpSignOutGeneration('*'); + forgetAllEntitySettings(); + // Snapshot keys first — signOutLocally mutates the flags as it goes. + for (const id of Object.keys(this.potentiallyAuthenticated)) { + this.signOutLocally(id); + } + this.dropFabricConnectState(); + this.setUserForEntity(OverallAppSignIn, null); + // Broadcast a global clear so other tabs' explorers drop their credentials even for entities not + // in this tab's potentiallyAuthenticated set. + this.bumpExplorerAuthEpochAll(); + } finally { + this.announceAppSignOut(endedUser); } - this.dropFabricConnectState(); - this.setUserForEntity(OverallAppSignIn, null); - // Broadcast a global clear so other tabs' explorers drop their credentials even for entities not - // in this tab's potentiallyAuthenticated set. - this.bumpExplorerAuthEpochAll(); - this.announceAppSignOut(endedUser); } - /** The signed-in app user's id, or a local instance user's name. */ private appUser(): string | null { const user = this.allConnections[OverallAppSignIn]?.user; if (!user) { @@ -763,9 +767,14 @@ class AuthStore { if (event.key !== this.appSignOutKey || !event.newValue) { return; } + const ended = signedOutUser(event.newValue); const user = this.appUser(); - if (user !== null && signedOutUser(event.newValue) === user) { - onSignedOut(); + if (user !== null) { + if (ended === user) { + onSignedOut(); + } + } else if (typeof ended === 'string' && this.allConnections[OverallAppSignIn]?.isLoading) { + this.signOutHeardDuringCheck = ended; } }; window.addEventListener('storage', handler); @@ -907,6 +916,10 @@ class AuthStore { console.error(error); user = null; } + const signOutHeard = id === OverallAppSignIn ? this.signOutHeardDuringCheck : null; + if (id === OverallAppSignIn) { + this.signOutHeardDuringCheck = null; + } if (this.getSignOutGeneration(id) !== signOutGeneration) { return null; } @@ -916,6 +929,9 @@ class AuthStore { this.flagKeyAsSignedOut(id); } this.updateConnectionIfChanged(id, false, user); + if (signOutHeard !== null && signOutHeard === this.appUser()) { + void this.reloadUser(id); + } return user; } } diff --git a/src/router/__tests__/crossTabSignOut.test.tsx b/src/router/__tests__/crossTabSignOut.test.tsx index f9e56fc0d..a14771ea5 100644 --- a/src/router/__tests__/crossTabSignOut.test.tsx +++ b/src/router/__tests__/crossTabSignOut.test.tsx @@ -90,6 +90,8 @@ await Promise.all([import('@/lib/installApiUnauthorizedRedirect'), import('@/App let session: 'live' | 'ended' | 'ended-with-401' | 'cluster-forbidden'; const requests: string[] = []; const requestsAfterSessionEnded: string[] = []; +/** While set, this tab's `/User/current` answers are decided when sent but delivered only once it settles. */ +let userCurrentHeldUntil: Promise | null = null; function respond(config: InternalAxiosRequestConfig, status: number, data: unknown) { const response = { status, statusText: String(status), data, headers: {}, config }; @@ -155,7 +157,23 @@ async function openClusterPage() { import('@/react-query/queryClient'), import('@/AppRouted'), ]); - apiClient.defaults.adapter = async (config) => centralManager(config); + apiClient.defaults.adapter = async (config) => { + const held = config.url === '/User/current' ? userCurrentHeldUntil : null; + if (!held) { + return centralManager(config); + } + let answer: () => ReturnType; + try { + const response = centralManager(config); + answer = () => response; + } catch (error) { + answer = () => { + throw error; + }; + } + await held; + return answer(); + }; installApiUnauthorizedRedirect(); installCrossTabSignOut(); render( @@ -268,6 +286,7 @@ async function expectStillSignedIn(thisTab: Awaited; +let addEventListener: MockInstance; function reportedAuthorizationFailures() { return consoleError.mock.calls.filter(([error]) => error instanceof AxiosError && error.response?.status === 403); @@ -281,6 +300,9 @@ beforeEach(() => { session = 'live'; requests.length = 0; requestsAfterSessionEnded.length = 0; + userCurrentHeldUntil = null; + // Recorded so each test's `storage` listeners come down with it, rather than acting on its modules later. + addEventListener = vi.spyOn(window, 'addEventListener'); consoleError = vi.spyOn(console, 'error').mockImplementation(() => undefined); vi.spyOn(console, 'debug').mockImplementation(() => undefined); }); @@ -291,6 +313,11 @@ afterEach(() => { router.history.destroy(); } routers.clear(); + for (const [type, listener, options] of addEventListener.mock.calls) { + if (type === 'storage' && listener) { + window.removeEventListener(type, listener, options); + } + } vi.clearAllMocks(); vi.restoreAllMocks(); }); @@ -300,11 +327,9 @@ describe('a sign-out in another tab', () => { const { queryClient } = await openClusterPage(); const otherTab = await openAnotherTab(); - // What `useLogoutMutation` runs there: the sweep, central manager's `/Logout/`, then `logoutOnSuccess`. await otherTab.run(() => otherTab.authStore.signOutFromPotentiallyAuthenticatedInstances()); session = 'ended'; await otherTab.run(() => otherTab.logoutOnSuccess()); - // The poll, or the refocus, that toasted "ClientError: Not allowed" before. await nextPoll(queryClient); expect(toast.error).not.toHaveBeenCalled(); @@ -317,7 +342,6 @@ describe('a sign-out in another tab', () => { const thisTab = await openClusterPage(); const otherTab = await openAnotherTab(); - // The sweep runs first; a failed `/Logout/` skips `logoutOnSuccess`, and the session stays live. await otherTab.run(() => otherTab.authStore.signOutFromPotentiallyAuthenticatedInstances()); await expectStillSignedIn(thisTab); @@ -327,7 +351,6 @@ describe('a sign-out in another tab', () => { const { queryClient } = await openClusterPage(); const otherTab = await openAnotherTab(); - // The session expired; the other tab heard first, from a 401, and cleared it as the 401 handler does. session = 'ended'; await otherTab.run(() => otherTab.clearAuthStateLocally()); await nextPoll(queryClient); @@ -337,10 +360,32 @@ describe('a sign-out in another tab', () => { await screen.findByText('sign-in page'); }); + it('asks again when its own check, out during the sign-out, comes back as the user it ended', async () => { + const { authStore, queryClient } = await openClusterPage(); + const otherTab = await openAnotherTab(); + let releaseUserCurrent: () => void = () => undefined; + userCurrentHeldUntil = new Promise((resolve) => (releaseUserCurrent = resolve)); + const checksBefore = requests.filter((url) => url === '/User/current').length; + + // Central manager answers this check before the other tab's `/Logout/`; the answer arrives after it. + const check = authStore.reloadUser('OverallAppSignIn'); + await waitFor(() => expect(requests.filter((url) => url === '/User/current')).toHaveLength(checksBefore + 1)); + await otherTab.run(() => otherTab.authStore.signOutFromPotentiallyAuthenticatedInstances()); + session = 'ended'; + await otherTab.run(() => otherTab.logoutOnSuccess()); + userCurrentHeldUntil = null; + releaseUserCurrent(); + await check; + await screen.findByText('sign-in page'); + await nextPoll(queryClient); + + expect(toast.error).not.toHaveBeenCalled(); + expect(requestsAfterSessionEnded).toEqual(['/User/current']); + }); + it('leaves this tab signed in when a 401 in another tab ends a different user’s session', async () => { const thisTab = await openClusterPage(); const otherTab = await openAnotherTab(); - // That tab still holds user U from before this tab signed in as the current user. otherTab.authStore.setUserForIdAndKey('OverallAppSignIn', 'OverallAppSignIn', { id: 'usr_u' } as never); otherTab.apiClient.defaults.adapter = async (config) => respond(config, 401, { code: 'ClientError', title: 'Invalid cookie' });