diff --git a/DESIGN.md b/DESIGN.md index 4522c6b2b..0cc685813 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -142,6 +142,17 @@ 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). +- **App sign-out and cluster removal post `logout` only to an instance that holds a credential of the user's.** + `buildLogoutClient`, shared by the app sign-out sweep and by removing a self-hosted cluster (`signOutEntities`), + returns no client for a sign-in that `connectsThroughProxy` (the Disconnect button and `signOutOfInstance` still + post one, HarperFast/studio#1974): the central manager hands a proxied operation to the host manager, which calls the + instance over its socket with no cookie or token, so Harper's `logout` has no session of the user's to end and + answers 500 (RUM: 16 of 24 proxied logouts at app sign-out in the 30 days to 2026-10-15, each a round trip + `/Logout/` waited on). The Fabric Connect flag outlives the tab while the direct token does not, so a second tab sees + another tab's direct sign-in as proxied and would post the same doomed logout. On app sign-out the central-manager + `/Logout/` ends the only session that path has; on cluster removal there was never one to end. Direct, basic-auth + and cookie sign-ins still get their `logout` on both paths (`signOutFromPotentiallyAuthenticatedInstances.test.ts`, + `signOutEntities.test.ts`). - **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 diff --git a/src/features/auth/store/authStore.ts b/src/features/auth/store/authStore.ts index ee612bfcd..e1271d9b0 100644 --- a/src/features/auth/store/authStore.ts +++ b/src/features/auth/store/authStore.ts @@ -1,5 +1,5 @@ import { isLocalStudio, localStudioDevUrl } from '@/config/constants'; -import { getInstanceClient } from '@/config/getInstanceClient'; +import { connectsThroughProxy, getInstanceClient } from '@/config/getInstanceClient'; import { fetchCurrentUser } from '@/features/auth/queries/getCurrentUser'; import { forgetAllEntitySettings, @@ -750,6 +750,10 @@ class AuthStore { return null; } try { + // Through the proxy the instance holds no credential of the user's for `logout` to end. + if (connectsThroughProxy(entityId)) { + return null; + } const instanceClient = getInstanceClient({ id: entityId, operationsUrl }); instanceClient.defaults.timeout = LOGOUT_TIMEOUT_MS; // No gateway-error retries (5s + 10s + 20s) and no token recovery for a best-effort logout. diff --git a/src/features/auth/store/signOutEntities.test.ts b/src/features/auth/store/signOutEntities.test.ts index 0ba335f3c..5f9eba167 100644 --- a/src/features/auth/store/signOutEntities.test.ts +++ b/src/features/auth/store/signOutEntities.test.ts @@ -112,6 +112,17 @@ describe('authStore.signOutEntities', () => { expect(storedSignInMarkers()).toEqual(markers); }); + it('posts no logout for a sign-in reached through the Fabric Connect proxy, and still forgets it', async () => { + authStore.flagForBasicAuth('clu-2', null); + authStore.flagForFabricConnect('clu-2', true); + + await authStore.signOutEntities(['clu-2', 'ins-1']); + + expect(loggedOut().map(({ entityId }) => entityId)).toEqual(['ins-1']); + expect(authStore.checkForFabricConnect('clu-2')).toBe(false); + expect(rememberedEntities()).toEqual(['clu-1']); + }); + it('posts no logout for an id with nothing remembered', async () => { await authStore.signOutEntities(['ins-unknown']); diff --git a/src/features/auth/store/signOutFromPotentiallyAuthenticatedInstances.test.ts b/src/features/auth/store/signOutFromPotentiallyAuthenticatedInstances.test.ts index f7bfd4fde..4f9333cb8 100644 --- a/src/features/auth/store/signOutFromPotentiallyAuthenticatedInstances.test.ts +++ b/src/features/auth/store/signOutFromPotentiallyAuthenticatedInstances.test.ts @@ -1,5 +1,4 @@ /** @vitest-environment jsdom */ -import { apiClient } from '@/config/apiClient'; import { AxiosError, InternalAxiosRequestConfig } from 'axios'; import { afterEach, beforeEach, describe, expect, it, MockInstance, vi } from 'vitest'; import { authStore, OverallAppSignIn } from './authStore'; @@ -102,18 +101,32 @@ describe('authStore.signOutFromPotentiallyAuthenticatedInstances', () => { expect(authStore.getOperationToken('ins-direct')).toBeUndefined(); }); - it('routes a Fabric Connect proxy entity through the central-manager proxy', async () => { + it('posts no logout for a Fabric Connect proxy entity, and still clears it locally', async () => { + // The flag with no operation token is also what another tab's direct sign-in looks like from here. authStore.flagForFabricConnect('ins-proxy', true); signInToInstance('ins-proxy'); + // getInstanceClient lets the flag outrank a stored Basic Auth credential, so this one is proxied too. + authStore.flagForFabricConnect('ins-flagged-basic', true); + authStore.flagForBasicAuth('ins-flagged-basic', { username: 'u', password: 'p' }); + signInToInstance('ins-flagged-basic'); + authStore.flagForBasicAuth('ins-basic', { username: 'u', password: 'p' }); + signInToInstance('ins-basic'); + signInToInstance('ins-cookie'); onInstanceLogoutSubmit.mockResolvedValue({ message: 'ok' }); await authStore.signOutFromPotentiallyAuthenticatedInstances(); - expect(loggedOutTargets()).toEqual([[ - 'ins-proxy', - `${apiClient.defaults.baseURL}/HDBInstance/ins-proxy/operation`, - ]]); - expect(authStore.checkForFabricConnect('ins-proxy')).toBe(false); + expect(loggedOutTargets()).toEqual([['ins-basic', 'https://ins-basic'], ['ins-cookie', 'https://ins-cookie']]); + expect(onInstanceLogoutSubmit.mock.calls[0][0].instanceClient.defaults.auth).toEqual({ + username: 'u', + password: 'p', + }); + for (const id of ['ins-proxy', 'ins-flagged-basic']) { + expect(authStore.checkForFabricConnect(id)).toBe(false); + expect(userOf(id)).toBeNull(); + } + expect(authStore.checkForBasicAuth('ins-flagged-basic')).toBeUndefined(); + expect(consoleDebug).not.toHaveBeenCalled(); }); it('clears every entity locally up front, then waits for every logout', async () => { diff --git a/src/features/cluster/DESIGN.md b/src/features/cluster/DESIGN.md index 734552ef3..f48eebd8a 100644 --- a/src/features/cluster/DESIGN.md +++ b/src/features/cluster/DESIGN.md @@ -91,13 +91,14 @@ organization's cluster list doesn't carry them; with neither read it forgets not cluster was self-hosted. And when any page of a terminated cluster renders, `ClusterLayout` does it, which covers a terminate done in another tab, by another member or by central manager. It skips `TERMINATING`, which is not final. Never from a route guard: guards also run on hover preloads, so a hover would delete stored -credentials. A sign-in left behind is not harmless. Each app sign-out posts a logout to every remembered host, and a -stored Basic Auth credential would stay in localStorage for good. +credentials. A sign-in left behind is not harmless. Each app sign-out posts a logout to every remembered host except +one reached through the Fabric Connect proxy (the root `DESIGN.md` says why), and a stored Basic Auth credential would +stay in localStorage for good. A self-hosted cluster removed from Fabric is never cleared on render. Its servers may still run, and forgetting their sign-ins locally would leave their cookie sessions open. Removing one from Studio sends each remembered sign-in's server a -logout the way app sign-out does (`signOutEntities`: one attempt, a 10s timeout, not awaited by the removal), and forgets -the sign-in without waiting for the answer. So a failed logout still forgets it, and a server unreachable at that moment +logout the way app sign-out does (`signOutEntities`: one attempt, a 10s timeout, not awaited by the removal, none for a +sign-in reached through the Fabric Connect proxy), and forgets the sign-in without waiting for the answer. So a failed logout still forgets it, and a server unreachable at that moment keeps its session. Removed elsewhere, its sign-ins stay until an app sign-out logs them out. None of the hidden pages can do anything there. Central manager's `assertClusterMutable` refuses every update to a cluster