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
11 changes: 11 additions & 0 deletions DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion src/features/auth/store/authStore.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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.
Expand Down
11 changes: 11 additions & 0 deletions src/features/auth/store/signOutEntities.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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']);

Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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 () => {
Expand Down
9 changes: 5 additions & 4 deletions src/features/cluster/DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading