diff --git a/frontend/src/hooks/useChatSync.ts b/frontend/src/hooks/useChatSync.ts index 629be5812..fb9514885 100644 --- a/frontend/src/hooks/useChatSync.ts +++ b/frontend/src/hooks/useChatSync.ts @@ -27,11 +27,8 @@ const useAgentHealthOnReconnect = (check: () => void): void => { const currentHandoff = () => toChatHandoff(store.getState().chat) /* The identity the chat is scoped by. auth.user, NOT the persisted `user` model: auth.user is - fetched for the CURRENT tokens at sign-in (it is what lets App mount), while the user model - is restored from storage and only catches up when the cloud sync lands. Activating a saved - account swaps tokens and reloads without purging persisted models, so for that interval - (indefinitely, if the sync stalls) the user model still names the PREVIOUS account — and an - ownership check against it would keep that account's transcript on the new account's screen. */ + fetched for the CURRENT tokens at sign-in (it is what lets App mount), while the rest of the + user model is restored from storage and only catches up when the cloud sync lands. */ const useChatIdentity = (): string => useSelector((state: State) => state.auth.user?.id ?? '') // '' = not signed in: syncIdentity no-ops /* What both chat surfaces do on boot: follow the signed-in identity, clear what must not diff --git a/frontend/src/models/auth.test.ts b/frontend/src/models/auth.test.ts index 9a750564a..b9e70c612 100644 --- a/frontend/src/models/auth.test.ts +++ b/frontend/src/models/auth.test.ts @@ -51,7 +51,7 @@ const { oidcMcpDetailReady: vi.fn(), pushUnregister: vi.fn(), browser: { isElectron: false, hasBackend: false }, - storeState: { auth: {} as Record }, + storeState: { auth: {} as Record, user: { id: '' } }, })) // signInFailure() tests `error instanceof OidcError`, so the mock must export a real class @@ -74,9 +74,11 @@ vi.mock('../services/oidc', () => ({ })) vi.mock('../helpers/stageHelper', () => ({ chooseStage, reloadIfStageChanged })) vi.mock('../services/permitteerAccount', () => ({ signOutEverywhere })) +const { broadcastChatSignout } = vi.hoisted(() => ({ broadcastChatSignout: vi.fn() })) +vi.mock('../services/chatPopout', () => ({ broadcastChatSignout })) vi.mock('../services/accountSecurity', () => ({ changePassword })) vi.mock('../services/Controller', () => ({ - default: { close: controllerClose, emitWithAck, reconnectNow }, + default: { close: controllerClose, emitWithAck, reconnectNow, setupConnection: vi.fn() }, emit: vi.fn(() => false), })) vi.mock('../services/CloudSync', () => ({ default: { reset: vi.fn() } })) @@ -89,8 +91,23 @@ vi.mock('../services/pushNotifications', () => ({ default: { register: vi.fn(), vi.mock('../services/graphQLRequest', () => ({ graphQLLogin: vi.fn() })) vi.mock('../services/remoteit', () => ({ getToken: vi.fn(), apiAuthHeaders: vi.fn() })) vi.mock('../selectors/devices', () => ({ selectDeviceModelAttributes: vi.fn() })) +const persist = vi.hoisted(() => ({ + bootstrapped: true, + persistsState: true, + listener: undefined as undefined | (() => void), +})) vi.mock('../store', () => ({ - persistor: { purge: vi.fn() }, + get persistsState() { + return persist.persistsState + }, + persistor: { + purge: vi.fn(), + getState: () => ({ bootstrapped: persist.bootstrapped }), + subscribe: (listener: () => void) => { + persist.listener = listener + return () => (persist.listener = undefined) + }, + }, store: { getState: () => storeState, dispatch: { auth: { completeCallback } } }, })) vi.mock('../i18n', () => ({ default: { t: (k: string) => k } })) @@ -114,6 +131,18 @@ function makeDispatch() { } } +// Every model and method on demand, as a spy: for effects that fan out across many models. +function spyDispatch() { + const models: Record>> = {} + const dispatch = new Proxy(models, { + get: (all, model: string) => + (all[model] ??= new Proxy({} as Record>, { + get: (calls, fn: string) => (calls[fn] ??= vi.fn()), + })), + }) + return { models, dispatch: dispatch as any } +} + // The only shape SignInApp renders: it shows a message ONLY while signInFailed is true, and // signInFailed is also the brake on auto sign-in — so every failure writer must produce it. const aFailureShowing = (signInError: string) => expect.objectContaining({ signInFailed: true, signInError }) @@ -121,6 +150,8 @@ const aFailureShowing = (signInError: string) => expect.objectContaining({ signI // eslint-disable-next-line @typescript-eslint/no-var-requires import authModel, { liveAuthCallback } from './auth' import { agentOwnedMessage } from '@common/agentOwner' +import { graphQLLogin } from '../services/graphQLRequest' +import { PERSISTED_MODELS } from './persistedModels' const effectsFor = (dispatch: any) => (authModel as any).effects(dispatch) @@ -403,13 +434,7 @@ describe('auth model — a stage switch signs out and reloads onto the new stage }) it('the sign-out teardown reloads onto a changed stage as its last step', async () => { - const fns: Record>> = {} - const dispatch = new Proxy(fns, { - get: (models, model: string) => - (models[model] ??= new Proxy({} as Record>, { - get: (calls, fn: string) => (calls[fn] ??= vi.fn()), - })), - }) + const { dispatch } = spyDispatch() await effectsFor(dispatch).signedOut() expect(reloadIfStageChanged).toHaveBeenCalledTimes(1) expect(controllerClose.mock.invocationCallOrder[0]).toBeLessThan(reloadIfStageChanged.mock.invocationCallOrder[0]) @@ -659,6 +684,88 @@ describe('auth model — the desktop hands the chooser callback to a signed-in w }) }) +describe('auth model — signing in as another account clears the last account’s persisted state', () => { + const signInAs = async (id: string, persistedOwner: string) => { + storeState.user = { id: persistedOwner } + vi.mocked(graphQLLogin).mockResolvedValue({ + data: { data: { login: { id, authhash: 'hash', yoicsId: 'yoics' } } }, + } as any) + const { dispatch } = spyDispatch() + await effectsFor(dispatch).fetchUser() + return dispatch + } + + beforeEach(() => broadcastChatSignout.mockReset()) + + afterEach(() => { + storeState.user = { id: '' } + persist.bootstrapped = true + persist.persistsState = true + }) + + it('resets and closes chat popouts before the new user is set when the persisted state is another account’s', async () => { + const dispatch = await signInAs('USER-B', 'USER-A') + expect(dispatch.auth.resetAccountData).toHaveBeenCalledTimes(1) + expect(broadcastChatSignout).toHaveBeenCalledTimes(1) + expect(dispatch.auth.resetAccountData.mock.invocationCallOrder[0]).toBeLessThan( + dispatch.auth.set.mock.invocationCallOrder[0] + ) + }) + + it('stamps the new owner right after the reset, before any account fetch can persist', async () => { + const dispatch = await signInAs('USER-B', 'USER-A') + expect(dispatch.user.set).toHaveBeenCalledWith({ id: 'USER-B' }) + expect(dispatch.auth.resetAccountData.mock.invocationCallOrder[0]).toBeLessThan( + dispatch.user.set.mock.invocationCallOrder[0] + ) + }) + + it('keeps the persisted state and chat popouts for the same account', async () => { + const dispatch = await signInAs('USER-A', 'USER-A') + expect(dispatch.auth.resetAccountData).not.toHaveBeenCalled() + expect(broadcastChatSignout).not.toHaveBeenCalled() + }) + + it('treats state with no recorded owner as another account’s, popouts included', async () => { + const dispatch = await signInAs('USER-A', '') + expect(dispatch.auth.resetAccountData).toHaveBeenCalledTimes(1) + expect(broadcastChatSignout).toHaveBeenCalledTimes(1) + expect(dispatch.user.set).toHaveBeenCalledWith({ id: 'USER-A' }) + }) + + it('skips the check in a popout or support tab, which persists nothing and must not close other windows’ popouts', async () => { + persist.persistsState = false + const dispatch = await signInAs('USER-A', '') + expect(dispatch.auth.resetAccountData).not.toHaveBeenCalled() + expect(broadcastChatSignout).not.toHaveBeenCalled() + expect(dispatch.user.set).toHaveBeenCalledWith({ id: 'USER-A' }) + }) + + it('waits for rehydration before reading the owner', async () => { + persist.bootstrapped = false + vi.mocked(graphQLLogin).mockResolvedValue({ + data: { data: { login: { id: 'USER-B', authhash: 'h', yoicsId: 'y' } } }, + } as any) + const { dispatch } = spyDispatch() + const signingIn = effectsFor(dispatch).fetchUser() + await vi.waitFor(() => expect(persist.listener).toBeDefined()) + expect(dispatch.auth.set).not.toHaveBeenCalled() + + storeState.user = { id: 'USER-A' } + persist.bootstrapped = true + persist.listener?.() + await signingIn + expect(dispatch.auth.resetAccountData).toHaveBeenCalledTimes(1) + }) + + it('resets every persisted model', () => { + const { models, dispatch } = spyDispatch() + effectsFor(dispatch).resetAccountData() + const reset = Object.keys(models).filter(model => models[model].reset?.mock.calls.length) + expect(reset.sort()).toEqual([...PERSISTED_MODELS].sort()) + }) +}) + describe("auth model — a different account arriving drops this phone's push token", () => { beforeEach(() => { oidcClaims.mockReset().mockReturnValue({ sub: 'sub-a' }) diff --git a/frontend/src/models/auth.ts b/frontend/src/models/auth.ts index 4495a1757..7e121d6e2 100644 --- a/frontend/src/models/auth.ts +++ b/frontend/src/models/auth.ts @@ -14,11 +14,12 @@ import { StageName, } from '../constants' import { chooseStage, reloadIfStageChanged } from '../helpers/stageHelper' -import { persistor, store } from '../store' +import { persistor, persistsState, store } from '../store' import { graphQLLogin } from '../services/graphQLRequest' import { getToken } from '../services/remoteit' import { changePassword as changeAccountPassword } from '../services/accountSecurity' import { signOutEverywhere } from '../services/permitteerAccount' +import { broadcastChatSignout } from '../services/chatPopout' import { oidcConfigured, oidcSignedIn, @@ -116,6 +117,17 @@ const signInFailure = (error: any): Partial => ({ signInRetryAfter: error instanceof OidcError ? error.retryAfter : undefined, }) +// auth.init is not gated on rehydration, and an owner read before it sees the empty default. +const rehydrated = () => + new Promise(resolve => { + if (persistor.getState().bootstrapped) return resolve() + const unsubscribe = persistor.subscribe(() => { + if (!persistor.getState().bootstrapped) return + unsubscribe() + resolve() + }) + }) + const signInCleared = { signInFailed: false, signInError: undefined, @@ -302,6 +314,17 @@ export default createModel()({ const user = response?.data?.data?.login + // Switching accounts reloads without signing out, so a window's persisted state, and any chat popout still paired + // with it, can be the last account's. With no owner recorded neither can be trusted. + await rehydrated() + const owner = store.getState().user.id + if (persistsState && user?.id && owner !== user.id) { + auth.resetAccountData() + broadcastChatSignout() + } + // Stamped here rather than when user.fetch lands, so a second switch before then still sees an owner. + if (user?.id) dispatch.user.set({ id: user.id }) + auth.set({ user, ...signInCleared }) if (user.authhash && user.yoicsId) { Controller.setupConnection({ username: user.yoicsId, authHash: user.authhash, guid: user.id }) @@ -512,29 +535,14 @@ export default createModel()({ a stale error and no redirect, for something that happened in someone else's session. */ await dispatch.auth.set({ user: undefined, ...signInCleared }) - dispatch.chat.reset() + dispatch.auth.resetAccountData() dispatch.agents.reset() - dispatch.user.reset() - dispatch.organization.reset() - dispatch.networks.reset() - dispatch.accounts.reset() - dispatch.connections.reset() - dispatch.devices.reset() - dispatch.sessions.reset() dispatch.logs.reset() dispatch.search.reset() - dispatch.announcements.reset() - dispatch.applicationTypes.reset() - dispatch.plans.reset() - dispatch.contacts.reset() dispatch.billing.reset() dispatch.backend.reset() - dispatch.files.reset() dispatch.keys.reset() - dispatch.jobs.reset() - dispatch.tags.reset() dispatch.ui.reset() - dispatch.products.reset() dispatch.partnerStats.reset() dispatch.adminUsers.reset() dispatch.adminPartners.reset() @@ -556,6 +564,25 @@ export default createModel()({ Controller.close() reloadIfStageChanged() }, + /** Every persisted model: what sign-out clears, and what a sign-in finding another account's state clears. */ + resetAccountData() { + dispatch.chat.reset() + dispatch.user.reset() + dispatch.organization.reset() + dispatch.networks.reset() + dispatch.accounts.reset() + dispatch.connections.reset() + dispatch.devices.reset() + dispatch.sessions.reset() + dispatch.announcements.reset() + dispatch.applicationTypes.reset() + dispatch.plans.reset() + dispatch.contacts.reset() + dispatch.files.reset() + dispatch.jobs.reset() + dispatch.tags.reset() + dispatch.products.reset() + }, /** Test Settings' stage switch. This signs out of the old login server; the reload at the end of * signedOut boots every endpoint on the new stage, where init drops the old stage's accounts. */ async switchStage(stage: StageName, state) { diff --git a/frontend/src/models/persistedModels.ts b/frontend/src/models/persistedModels.ts new file mode 100644 index 000000000..54e416b0f --- /dev/null +++ b/frontend/src/models/persistedModels.ts @@ -0,0 +1,18 @@ +export const PERSISTED_MODELS = [ + 'accounts', + 'announcements', + 'applicationTypes', + 'chat', + 'connections', + 'contacts', + 'devices', + 'files', + 'jobs', + 'networks', + 'organization', + 'plans', + 'products', + 'sessions', + 'tags', + 'user', +] as const diff --git a/frontend/src/store.ts b/frontend/src/store.ts index b89c1bf64..5e1c628c3 100644 --- a/frontend/src/store.ts +++ b/frontend/src/store.ts @@ -2,11 +2,13 @@ import { numericVersion } from './helpers/versionHelper' import { models, RootModel } from './models' import { defaultChatState, IChatState } from './models/chat' import { isChatPopout, popoutScopeId } from './services/chatPopout' +import { oidcIsSupportTab } from './services/oidc' import { createLogger, ReduxLoggerOptions } from 'redux-logger' import { init, RematchDispatch, RematchRootState } from '@rematch/core' import { createTransform, PersistConfig } from 'redux-persist' import persistPlugin, { getPersistor } from '@rematch/persist' import DateTransform from './helpers/DateTransform' +import { PERSISTED_MODELS } from './models/persistedModels' import immerPlugin from '@rematch/immer' import localForage from 'localforage' @@ -43,29 +45,14 @@ const noopStorage = { removeItem: () => Promise.resolve(), } +export const persistsState = !isChatPopout && !oidcIsSupportTab() + const persistConfig: PersistConfig = { key: 'app', version: numericVersion(), - // The popout persists nothing (noopStorage) so it cannot clobber the main window's 'app' key. - storage: isChatPopout ? noopStorage : localForage, - whitelist: [ - 'accounts', - 'announcements', - 'applicationTypes', - 'chat', - 'connections', - 'contacts', - 'devices', - 'files', - 'jobs', - 'networks', - 'organization', - 'plans', - 'products', - 'sessions', - 'tags', - 'user', - ], + // The popout and support tabs persist nothing (noopStorage), so neither can clobber the main window's 'app' key. + storage: persistsState ? localForage : noopStorage, + whitelist: [...PERSISTED_MODELS], throttle: 1000, transforms: [DateTransform, chatTransform], }