diff --git a/frontend/src/constants.ts b/frontend/src/constants.ts index 754c063e3..60b122291 100644 --- a/frontend/src/constants.ts +++ b/frontend/src/constants.ts @@ -154,6 +154,9 @@ export const FRONTEND_RETRY_DELAY = 20000 // How long sign out waits for the local backend to come back before giving up and // tearing down the frontend on its own. Short: it's a localhost socket. export const SIGN_OUT_BACKEND_TIMEOUT = 3000 +// How long "Sign out everywhere" waits for the AS to end every session before signing out +// locally regardless — a stalled token mint must never leave the person signed in here. +export const SIGN_OUT_EVERYWHERE_TIMEOUT = 10000 export const MAX_CONNECTION_NAME_LENGTH = 62 export const MAX_DESCRIPTION_LENGTH = 1024 export const SIDEBAR_WIDTH = 250 diff --git a/frontend/src/models/auth.test.ts b/frontend/src/models/auth.test.ts index defe88d81..e233512f4 100644 --- a/frontend/src/models/auth.test.ts +++ b/frontend/src/models/auth.test.ts @@ -6,10 +6,11 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' // the hoisted vi.mock factory runs. `browser` and the live `store` state are hoisted MUTABLE // objects so individual tests can steer the electron/backend branch and what the effects // re-read from the store after a teardown. -const { oidcStart, oidcEndSessionSilently, oidcGrantStale, oidcMcpDetailReady, browser, storeState } = vi.hoisted(() => ({ +const { oidcStart, signOutEverywhere, oidcGrantStale, oidcMcpDetailReady, oidcActor, browser, storeState } = vi.hoisted(() => ({ oidcStart: vi.fn(), - oidcEndSessionSilently: vi.fn(), + signOutEverywhere: vi.fn(), oidcGrantStale: vi.fn(), + oidcActor: vi.fn(), oidcMcpDetailReady: vi.fn(), browser: { isElectron: false, hasBackend: false }, storeState: { auth: {} as Record }, @@ -19,11 +20,12 @@ const { oidcStart, oidcEndSessionSilently, oidcGrantStale, oidcMcpDetailReady, b // (an undefined right-hand side of instanceof throws rather than returning false). vi.mock('../services/oidc', () => ({ oidcStart, - oidcEndSessionSilently, oidcGrantStale, oidcMcpDetailReady, + oidcActor, OidcError: class OidcError extends Error {}, })) +vi.mock('../services/permitteerAccount', () => ({ signOutEverywhere })) vi.mock('../services/Controller', () => ({ default: {}, emit: vi.fn(() => false) })) vi.mock('../services/CloudSync', () => ({ default: {} })) vi.mock('../services/cloudController', () => ({ default: {} })) @@ -36,13 +38,13 @@ vi.mock('../services/remoteit', () => ({ getToken: vi.fn(), apiAuthHeaders: vi.f vi.mock('../selectors/devices', () => ({ selectDeviceModelAttributes: vi.fn() })) vi.mock('../store', () => ({ persistor: { purge: vi.fn() }, store: { getState: () => storeState } })) vi.mock('../i18n', () => ({ default: { t: (k: string) => k } })) -vi.mock('../constants', () => ({ API_URL: '', DEVELOPER_KEY: '', SIGN_OUT_BACKEND_TIMEOUT: 1000 })) +vi.mock('../constants', () => ({ API_URL: '', DEVELOPER_KEY: '', SIGN_OUT_BACKEND_TIMEOUT: 1000, SIGN_OUT_EVERYWHERE_TIMEOUT: 50 })) vi.mock('axios', () => ({ default: {} })) // The effects are `dispatch => ({...})`; build them against a fake dispatch so each auth.* // call is an observable spy rather than a real reducer/effect. function makeDispatch() { - return { auth: { set: vi.fn(), signedOut: vi.fn(), signOut: vi.fn() }, ui: { set: vi.fn() } } + return { auth: { set: vi.fn(), signedOut: vi.fn(), signOut: vi.fn() }, ui: { set: vi.fn() }, chat: { signOut: vi.fn() } } } // The only shape SignInApp renders: it shows a message ONLY while signInFailed is true, and @@ -56,7 +58,8 @@ const effectsFor = (dispatch: any) => (authModel as any).effects(dispatch) beforeEach(() => { oidcStart.mockReset() - oidcEndSessionSilently.mockReset() + signOutEverywhere.mockReset().mockResolvedValue({ status: 200, body: { ended: 1, pool: 'skipped' } }) + oidcActor.mockReset().mockReturnValue(null) oidcGrantStale.mockReset() oidcMcpDetailReady.mockReset().mockResolvedValue('mcp_type') }) @@ -71,25 +74,72 @@ describe('auth model — sign-in always offers the chooser', () => { }) describe('auth model — sign-out is local to the app', () => { - it('signOut does NOT end the AS session (no oidcEndSessionSilently)', async () => { + it('signOut does NOT end the AS sessions (no signOutEverywhere)', async () => { const dispatch = makeDispatch() await effectsFor(dispatch).signOut(undefined, { auth: { backendAuthenticated: false } }) - expect(oidcEndSessionSilently).not.toHaveBeenCalled() + expect(signOutEverywhere).not.toHaveBeenCalled() // Local teardown still happens. expect(dispatch.auth.signedOut).toHaveBeenCalledTimes(1) }) }) -describe('auth model — "Sign out everywhere" stays AS-wide', () => { - it('globalSignOut ends the AS session BEFORE local teardown', async () => { +/* "Sign out everywhere" is ONE call at the AS — every session of the account, this one + included — and it must run while this app still holds a usable token: before the local + teardown, and after the agent's background grant is revoked (that revocation mints from the + very session the call ends). It is best-effort: the person reaching for the panic button + must end up signed out here whatever the AS answered. */ +describe('auth model — "Sign out everywhere" is one AS call, then the local teardown', () => { + it('globalSignOut revokes the background grant, calls sign-out-all, THEN signs out locally', async () => { const dispatch = makeDispatch() await effectsFor(dispatch).globalSignOut() - expect(oidcEndSessionSilently).toHaveBeenCalledTimes(1) + expect(dispatch.chat.signOut).toHaveBeenCalledTimes(1) + expect(signOutEverywhere).toHaveBeenCalledTimes(1) + expect(dispatch.auth.signOut).toHaveBeenCalledTimes(1) + const [grant, everywhere, local] = [ + dispatch.chat.signOut.mock.invocationCallOrder[0], + signOutEverywhere.mock.invocationCallOrder[0], + dispatch.auth.signOut.mock.invocationCallOrder[0], + ] + expect(grant).toBeLessThan(everywhere) + expect(everywhere).toBeLessThan(local) + }) + + it('a refused sign-out-all still signs the app out locally', async () => { + vi.spyOn(console, 'warn').mockImplementation(() => {}) + signOutEverywhere.mockResolvedValue({ status: 403, body: { error: 'insufficient_authorization' } }) + const dispatch = makeDispatch() + await effectsFor(dispatch).globalSignOut() + expect(dispatch.auth.signOut).toHaveBeenCalledTimes(1) + }) + + it('an AS that cannot be reached still signs the app out locally', async () => { + vi.spyOn(console, 'warn').mockImplementation(() => {}) + signOutEverywhere.mockRejectedValue(new Error('network down')) + const dispatch = makeDispatch() + await effectsFor(dispatch).globalSignOut() + expect(dispatch.auth.signOut).toHaveBeenCalledTimes(1) + }) + + /* Audience mints serialize through one shared promise; a mint the grant revoke abandoned + mid-stall would queue the AS call behind it for good. The bound is what keeps the panic + button from leaving the person signed in here. */ + it('a call that never answers is cut off at the bound — the local sign-out still follows', async () => { + vi.spyOn(console, 'warn').mockImplementation(() => {}) + signOutEverywhere.mockReturnValue(new Promise(() => {})) // never settles + const dispatch = makeDispatch() + await effectsFor(dispatch).globalSignOut() + expect(dispatch.auth.signOut).toHaveBeenCalledTimes(1) + }) + + /* A support session (the id_token carries `act`) holds no refresh token and the AS refuses its + writes: there is nothing to call. Straight to the local teardown, no revoke, no AS round trip. */ + it('a support session goes straight to the local sign-out — nothing is asked of the AS', async () => { + oidcActor.mockReturnValue({ sub: 'op_1' }) + const dispatch = makeDispatch() + await effectsFor(dispatch).globalSignOut() + expect(signOutEverywhere).not.toHaveBeenCalled() + expect(dispatch.chat.signOut).not.toHaveBeenCalled() expect(dispatch.auth.signOut).toHaveBeenCalledTimes(1) - // Order matters: the AS logout must precede the local sign-out. - expect(oidcEndSessionSilently.mock.invocationCallOrder[0]).toBeLessThan( - dispatch.auth.signOut.mock.invocationCallOrder[0] - ) }) }) diff --git a/frontend/src/models/auth.ts b/frontend/src/models/auth.ts index 8524f1f68..ae5c1bbab 100644 --- a/frontend/src/models/auth.ts +++ b/frontend/src/models/auth.ts @@ -5,7 +5,7 @@ import network from '../services/Network' import browser from '../services/browser' import analytics from '../services/analytics' import { selectDeviceModelAttributes } from '../selectors/devices' -import { API_URL, DEVELOPER_KEY, SIGN_OUT_BACKEND_TIMEOUT } from '../constants' +import { API_URL, DEVELOPER_KEY, SIGN_OUT_BACKEND_TIMEOUT, SIGN_OUT_EVERYWHERE_TIMEOUT } from '../constants' import { persistor, store } from '../store' import { graphQLLogin } from '../services/graphQLRequest' import { getToken, apiAuthHeaders } from '../services/remoteit' @@ -463,10 +463,9 @@ export default createModel()({ async signOut(_: void, state) { // Sign-out is LOCAL to this app: drop this app's tokens/session (dispatch.auth.signedOut // below). The AS browser session belongs to the user and is NOT ended here — a true - // "sign out everywhere" is a separate, explicit action (oidcEndSessionSilently / - // end_session remain for it). Because signIn always uses prompt=select_account, the next - // sign-in and any reload land on the AS chooser rather than silently SSO-ing back in, so - // no login-prompt guard is needed. + // "sign out everywhere" is a separate, explicit action (globalSignOut). Because signIn + // always uses prompt=select_account, the next sign-in and any reload land on the AS + // chooser rather than silently SSO-ing back in, so no login-prompt guard is needed. // emit returns false when the local socket isn't connected, and // backendAuthenticated can still be true at that moment - the flag is only // cleared once the socket's disconnect event lands. Without checking the @@ -504,7 +503,7 @@ export default createModel()({ await persistor.purge() // LOCAL-ONLY: drop this app's tokens. The AS session is never ended from here — // signing out of the app must not sign the user out of login.* (their browser - // session is theirs; an explicit "sign out everywhere" action can come later). + // session is theirs; the explicit "sign out everywhere" is globalSignOut). oidcClearLocal() /* signInCleared as well as the user: a failure recorded while SIGNED IN — a refused account switch, say — would otherwise survive into the signed-out screen, where @@ -555,13 +554,49 @@ export default createModel()({ Controller.close() }, async globalSignOut() { - // "Sign out everywhere" (SecurityPage) is the EXPLICIT, AS-wide action, distinct from the - // avatar-menu sign-out which is local to this app: end the AS browser session (RP-initiated - // logout) BEFORE the local teardown, so the security control does what it reports. The - // every-device /logout/all lands with Phase 2b. signOut itself stays LOCAL — a failure-path - // or menu sign-out must never end the AS session. - const { oidcEndSessionSilently } = await import('../services/oidc') - await oidcEndSessionSilently() + // "Sign out everywhere" (SecurityPage) is the EXPLICIT, account-wide action, distinct from + // the avatar-menu sign-out which is local to this app. ONE call at the AS ends every session + // of the account — this one included — with each refresh family swept, the resource servers + // told, and on a bridged stage the legacy pool's tokens revoked too (permitteer + // docs/remoteit-desktop-login.md Phase 4e); it runs BEFORE the local teardown, so the + // security control does what it reports, and it needs only the access token this app + // already holds. Best-effort by design: the refusal or outage that a person hits while + // reaching for the panic button must not leave them signed in here, so the local sign-out + // always follows — a miss is logged, never fatal. signOut itself stays LOCAL — a + // failure-path or menu sign-out must never end the AS sessions. + // + // + // A SUPPORT session (an operator viewing as the person) holds no refresh token and can + // mint for nothing but the data plane, and the account API refuses writes from an acted + // token anyway — so there is nothing to call; the control is hidden for it (SecurityPage), + // and this is the backstop: straight to the local teardown. Ending the support session + // itself is the operator's console or the person's account page, never this button. + if (oidcActor()) { + dispatch.auth.signOut() + return + } + // The agent's background grant goes FIRST: chat.signOut revokes it through the agent + // service with a token minted from THIS session, and once the AS has ended the session no + // token can be minted for that call. It revokes once per identity, so the chat.signOut + // inside signedOut() is a real no-op on the far side. + await dispatch.chat.signOut() + // BOUNDED, like the revoke above. Audience mints serialize through one shared promise + // (services/oidc), so a mint the revoke abandoned mid-stall would otherwise queue this call + // behind it indefinitely — and the panic button must never leave the person signed in here + // because the token service was half-open. Past the bound, the local sign-out proceeds and + // the AS is told nothing; that is the failure the mail and the account page can still show. + try { + const { signOutEverywhere } = await import('../services/permitteerAccount') + const r = await Promise.race([ + signOutEverywhere(), + new Promise(resolve => setTimeout(() => resolve(null), SIGN_OUT_EVERYWHERE_TIMEOUT)), + ]) + if (!r) console.warn('SIGN OUT EVERYWHERE timed out — signing out locally') + else if (r.status === 200) console.log('SIGN OUT EVERYWHERE', r.body) + else console.warn('SIGN OUT EVERYWHERE refused', r.status, r.body) + } catch (error) { + console.warn('SIGN OUT EVERYWHERE FAILED', error) + } dispatch.auth.signOut() }, }), diff --git a/frontend/src/models/chat.test.ts b/frontend/src/models/chat.test.ts index bc7bd6482..69f808375 100644 --- a/frontend/src/models/chat.test.ts +++ b/frontend/src/models/chat.test.ts @@ -381,3 +381,33 @@ describe('chat model — syncTranscript discards a response for a conversation n expect(dispatch.chat.set).not.toHaveBeenCalled() }) }) + +/* The background grant is revoked ONCE per identity. "Sign out everywhere" (models/auth) revokes + it before the AS ends the session, and the local teardown that follows calls signOut again — + a second enrollment DELETE, and another bounded wait on a slow agent, for nothing. reset() + ends every teardown and re-arms it for the next identity. */ +describe('chat model — the background grant is revoked once per identity', () => { + const reducers = (chatModel as any).reducers + const signedInAs = (id: string) => ({ auth: { user: { id } } }) + it('a second signOut for the same identity issues no second revoke; reset re-arms it', async () => { + reducers.reset({}) // whatever an earlier test left behind + const dispatch = makeDispatch() + const fx = effectsFor(dispatch) + await fx.signOut(undefined, signedInAs('alice')) + await fx.signOut(undefined, signedInAs('alice')) + expect(backgroundDisable).toHaveBeenCalledTimes(1) + reducers.reset({}) + await fx.signOut(undefined, signedInAs('alice')) + expect(backgroundDisable).toHaveBeenCalledTimes(2) + reducers.reset({}) // leave the module armed for the tests that follow + }) + it('a DIFFERENT identity is never skipped', async () => { + reducers.reset({}) + const dispatch = makeDispatch() + const fx = effectsFor(dispatch) + await fx.signOut(undefined, signedInAs('alice')) + await fx.signOut(undefined, signedInAs('bob')) + expect(backgroundDisable).toHaveBeenCalledTimes(2) + reducers.reset({}) + }) +}) diff --git a/frontend/src/models/chat.ts b/frontend/src/models/chat.ts index 48091ed21..798de1de8 100644 --- a/frontend/src/models/chat.ts +++ b/frontend/src/models/chat.ts @@ -184,6 +184,14 @@ const usageLimitMessage = (e: UsageLimitError): string => { } let abortController: AbortController | null = null +/* The background grant is revoked ONCE per signed-in identity: the id whose revoke this cycle has + already issued. "Sign out everywhere" revokes it before the AS call (models/auth globalSignOut) + and the local teardown that follows runs signOut again — without this the second pass issued a + second enrollment DELETE and could hold the teardown for another bounded wait on a slow agent. + Keyed by identity rather than a bare flag so a different account is never skipped. Module + state, like the controller above: it belongs to the process's sign-in cycle, not to persisted + chat state. Cleared by reset(), which every completed teardown ends with. */ +let backgroundRevokedFor: string | null = null /* The GENERATION of the conversation on screen — the one guard for everything that writes fetched chat content into the store. It advances on every event that makes a load already in flight unwanted: a history pick (the pick itself takes the new ticket), New Chat, a send (the user has @@ -521,7 +529,7 @@ export default createModel()({ chat's end. The transcript reset is dispatched by auth.signedOut alongside the other model resets — dispatching it here would land in the purge-to-reload window and re-persist the pre-signout state. */ - async signOut() { + async signOut(_: void, state) { broadcastChatSignout() // Aborting covers the STREAM; the generation covers every other load in flight. Without it a // slow history pick started under this account passed its own guard after the reset (its @@ -534,7 +542,10 @@ export default createModel()({ // grant BEFORE the session tokens vanish. AWAITED but BOUNDED — an unawaited revoke raced // oidcClearLocal(), so its authenticated DELETE minted no token and background AI access // survived sign-out. Awaiting lets the revoke finish while the tokens are still valid; the - // timeout keeps a slow agent from blocking sign-out. + // timeout keeps a slow agent from blocking sign-out. Once per identity (see the marker). + const who = state?.auth?.user?.id + if (who && backgroundRevokedFor === who) return + backgroundRevokedFor = who ?? null await Promise.race([ backgroundDisable().catch(() => {}), new Promise(resolve => setTimeout(resolve, 3000)), @@ -580,6 +591,7 @@ export default createModel()({ return state }, reset() { + backgroundRevokedFor = null // the next sign-in cycle gets its own revoke return { ...defaultChatState } }, }, diff --git a/frontend/src/pages/SecurityPage.tsx b/frontend/src/pages/SecurityPage.tsx index c5530d8cb..0cd4449f2 100644 --- a/frontend/src/pages/SecurityPage.tsx +++ b/frontend/src/pages/SecurityPage.tsx @@ -9,6 +9,7 @@ import { MFASettings } from '../components/MFA/MFASettings' import { PasskeysSettings } from '../components/MFA/PasskeysSettings' import { Dispatch } from '../store' import { useDispatch } from 'react-redux' +import { oidcActor } from '../services/oidc' export const SecurityPage: React.FC = () => { const { t } = useTranslation() @@ -26,8 +27,17 @@ export const SecurityPage: React.FC = () => { - - + {/* A SUPPORT session (an operator viewing as the person — the id_token says so) has nothing + this button can do: no refresh token to mint the account-API audience with, and the AS + refuses writes from an acted token regardless. Offering a "sign out everywhere" that + could only clear this tab would misdescribe itself; the session ends from the operator's + console or the person's account page. */} + {!oidcActor() && ( + <> + + + + )} ) } diff --git a/frontend/src/services/oidc.ts b/frontend/src/services/oidc.ts index 39b759c4a..66d006222 100644 --- a/frontend/src/services/oidc.ts +++ b/frontend/src/services/oidc.ts @@ -334,7 +334,9 @@ const declared = (): Array<{ resource: string; type: string; actions: string[]; { resource: OAUTH_PASSPORT_RESOURCE, type: 'passport_account', actions: ['profile.read', 'credentials.write'] }, // accounts.read: the OTHER accounts signed in on this browser, served by the account API from // this token's session — first-party apps only (permitteer docs/browser-accounts.md). - { resource: `${OAUTH_ISSUER}/account/api`, type: 'permitteer_account', actions: ['apps.read', 'apps.write', 'accounts.read'] }, + // devices.write: "Sign out everywhere" (SecurityPage) — every session of the account, this + // one included, ended in one call at the AS (permitteer docs/remoteit-desktop-login.md 4e). + { resource: `${OAUTH_ISSUER}/account/api`, type: 'permitteer_account', actions: ['apps.read', 'apps.write', 'accounts.read', 'devices.write'] }, // The AI agent's slice (remoteit-ai-agent.md D5): the stage's MCP detail, delegated // ONWARD to the agent service — `actor` is what stamps may_act into this session's // tokens, which is the exchange's precondition. The slice partitions from any plain @@ -597,23 +599,6 @@ async function refreshOnce(resource: string): Promise { * navigation — the parade of redirect hops was the only thing the front-channel bought * us. Best-effort: an unreachable AS must not block local teardown; the session gate * kills the tokens lazily anyway. */ -export async function oidcEndSessionSilently(): Promise { - const idToken = stored()?.id_token - if (!idToken) return - try { - const d = await discover() - if (!d.end_session_api_endpoint) return - const response = await fetch(d.end_session_api_endpoint, { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ id_token_hint: idToken }), - }) - if (!response.ok && response.status !== 204) console.warn('OIDC SILENT LOGOUT', response.status) - } catch (error) { - console.warn('OIDC SILENT LOGOUT FAILED', error) - } -} - /** Why the last mint for this audience was refused, if it was. */ export const oidcMintError = (resource: string): string | undefined => mintErrors[resource] @@ -626,8 +611,8 @@ export function invalidateOidcToken() { * NOTHING else. App sign-out never ends the AS session (user directive — the browser * session at the AS belongs to the user, not to this app's error handling), and it never * touches the OTHER saved accounts — signing out one identity is not signing out of the - * app's memory of the rest. `oidcSignOut` (RP-initiated end_session) remains for a future - * explicit "sign out everywhere" action only. */ + * app's memory of the rest. The explicit "Sign out everywhere" (models/auth globalSignOut) + * ends the sessions at the AS through the account API before it lands here. */ export function oidcClearLocal() { clearLocal() } diff --git a/frontend/src/services/permitteerAccount.ts b/frontend/src/services/permitteerAccount.ts index d25207f1f..136ad4bb8 100644 --- a/frontend/src/services/permitteerAccount.ts +++ b/frontend/src/services/permitteerAccount.ts @@ -37,6 +37,16 @@ async function call(path: string, init: RequestInit = {}): Promise> { + return await call('/devices/sign-out-all', { method: 'POST' }) +} + /** The person's connected apps — the AS account API's own view rows, unreshaped. */ export async function accountApps(): Promise> { return await call('/apps')