From a6ef8181499b92da02a1545e6764bdb2f363eb8f Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Tue, 3 Nov 2026 23:01:41 +0100 Subject: [PATCH] fix: end a login's push registration when it signs out, is removed or wiped --- lib/__tests__/web-push.test.ts | 52 +++++++++++++++++++ lib/offline/device-wipe.ts | 1 + lib/web-push.ts | 17 ++++++ .../__tests__/auth-store-push-signout.test.ts | 46 ++++++++++++++++ stores/auth-store.ts | 26 ++++++++-- 5 files changed, 137 insertions(+), 5 deletions(-) create mode 100644 stores/__tests__/auth-store-push-signout.test.ts diff --git a/lib/__tests__/web-push.test.ts b/lib/__tests__/web-push.test.ts index 32dafd0a3..f1d8a1c36 100644 --- a/lib/__tests__/web-push.test.ts +++ b/lib/__tests__/web-push.test.ts @@ -7,6 +7,7 @@ import { buildEmailPushConfig, disableWebPush, enableWebPush, + endWebPushForSignOut, isWebPushEnabled, listPushDevices, mailAccountsWithoutPush, @@ -1724,6 +1725,57 @@ describe('disableWebPush', () => { }); }); +describe('endWebPushForSignOut', () => { + it('takes everything down for a login with push on, using the relay it registered with', async () => { + localStorage.setItem(DEVICE_KEY, THIS_DEVICE); + localStorage.setItem(SUB_KEY, 'push-mine'); + localStorage.setItem(RELAY_BASE_KEY, RELAY); + const client = makeClient([sub('push-mine', THIS_DEVICE)]); + const calls = installFetch({}); + + await endWebPushForSignOut(client); + + expect(client.destroyed).toEqual(['push-mine']); + expect(calls).toEqual([{ url: `${RELAY}/api/push/register/${THIS_DEVICE}`, method: 'DELETE' }]); + expect(localStorage.getItem(SUB_KEY)).toBeNull(); + }); + + it('does nothing and calls nobody for a login without push', async () => { + const client = makeClient([]); + const calls = installFetch({}); + + await endWebPushForSignOut(client); + + expect(calls).toEqual([]); + expect(client.listPushSubscriptions).not.toHaveBeenCalled(); + }); + + it('never throws: signing out goes on', async () => { + localStorage.setItem(DEVICE_KEY, THIS_DEVICE); + localStorage.setItem(SUB_KEY, 'push-mine'); + localStorage.setItem(RELAY_BASE_KEY, RELAY); + const client = makeClient([sub('push-mine', THIS_DEVICE)]); + vi.stubGlobal('fetch', vi.fn(async () => { throw new TypeError('offline'); })); + (client.destroyPushSubscription as ReturnType).mockRejectedValue(new Error('offline')); + + await expect(endWebPushForSignOut(client)).resolves.toBeUndefined(); + expect(localStorage.getItem(SUB_KEY)).toBeNull(); + }); + + it('forgets the key material with nobody left', async () => { + localStorage.setItem(DEVICE_KEY, THIS_DEVICE); + localStorage.setItem(SUB_KEY, 'push-mine'); + localStorage.setItem(RELAY_BASE_KEY, RELAY); + const { browserSub } = installPushBrowser(); + const client = makeClient([sub('push-mine', THIS_DEVICE)]); + installFetch({}); + + await endWebPushForSignOut(client); + + expect(browserSub.unsubscribe).toHaveBeenCalledTimes(1); + }); +}); + describe('listPushDevices', () => { it('flags this device and reports the relay status of each registration', async () => { localStorage.setItem(DEVICE_KEY, THIS_DEVICE); diff --git a/lib/offline/device-wipe.ts b/lib/offline/device-wipe.ts index faface568..c20b329c3 100644 --- a/lib/offline/device-wipe.ts +++ b/lib/offline/device-wipe.ts @@ -265,6 +265,7 @@ async function endWebPushIfNobodyLeft(): Promise { const registration = await pushRegistration(); const subscription = await registration?.pushManager?.getSubscription(); await subscription?.unsubscribe(); + removeLocalKeys([], ['bulwark.push.']); } /** With nobody left after this there's nothing to lock for, and a next person shouldn't meet it. */ diff --git a/lib/web-push.ts b/lib/web-push.ts index 9b7ee0473..f611ece59 100644 --- a/lib/web-push.ts +++ b/lib/web-push.ts @@ -1031,6 +1031,23 @@ async function disableWebPushForThisTab(params: DisableWebPushParams): Promise { + let accountId: string; + try { + accountId = client.getAccountId(); + } catch { + return; + } + if (!accountId || typeof localStorage === 'undefined' || !localStorage.getItem(subscriptionIdKey(accountId))) return; + const relayBaseUrl = localStorage.getItem(relayBaseKey(accountId)) ?? undefined; + await disableWebPush({ client, relayBaseUrl }).catch(() => undefined); +} + export interface PushDevice { // The JMAP PushSubscription id - what you destroy to revoke it. id: string; diff --git a/stores/__tests__/auth-store-push-signout.test.ts b/stores/__tests__/auth-store-push-signout.test.ts new file mode 100644 index 000000000..9d98268d9 --- /dev/null +++ b/stores/__tests__/auth-store-push-signout.test.ts @@ -0,0 +1,46 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const endWebPushForSignOut = vi.hoisted(() => vi.fn(async () => {})); +vi.mock('@/lib/web-push', async (original) => ({ ...(await original()), endWebPushForSignOut })); + +import { useAuthStore } from '../auth-store'; + +function fakeClient() { + return { disconnect: vi.fn(), getAccountId: () => 'jmap-a' } as unknown as NonNullable['client']>; +} + +describe('signing out ends the login\'s push registration', () => { + beforeEach(() => { + endWebPushForSignOut.mockClear(); + vi.stubGlobal('fetch', vi.fn(async () => new Response('{}', { status: 200 }))); + }); + + it('when the user signs out', async () => { + const client = fakeClient(); + useAuthStore.setState({ client, isAuthenticated: true, activeAccountId: null, isDemoMode: false }); + await useAuthStore.getState().logout(); + expect(endWebPushForSignOut).toHaveBeenCalledWith(client); + }); + + it('when a wipe signs the login out', async () => { + const client = fakeClient(); + useAuthStore.setState({ client, isAuthenticated: true, activeAccountId: null, isDemoMode: false }); + await useAuthStore.getState().logout({ endProviderSession: false }); + expect(endWebPushForSignOut).toHaveBeenCalledWith(client); + }); + + it('not when the session only ran out: it comes back with the next sign-in', async () => { + const client = fakeClient(); + useAuthStore.setState({ client, isAuthenticated: true, activeAccountId: null, isDemoMode: false }); + await useAuthStore.getState().logout({ endProviderSession: false, keepLocalData: true }); + expect(endWebPushForSignOut).not.toHaveBeenCalled(); + }); + + it('not in demo mode', async () => { + const client = fakeClient(); + useAuthStore.setState({ client, isAuthenticated: true, activeAccountId: null, isDemoMode: true }); + await useAuthStore.getState().logout(); + expect(endWebPushForSignOut).not.toHaveBeenCalled(); + useAuthStore.setState({ isDemoMode: false }); + }); +}); diff --git a/stores/auth-store.ts b/stores/auth-store.ts index 7953d72a2..ee5e6ef21 100644 --- a/stores/auth-store.ts +++ b/stores/auth-store.ts @@ -1625,6 +1625,17 @@ function notifySignedIn(state: { activeAccountId: string | null; username: strin void authHooks.onLogin.emit({ accountId: state.activeAccountId, username: state.username ?? '' }); } +/** Within the sign-out's budget: an unreachable server doesn't hold it up. */ +function endPushForSignOut(signingOut: Iterable): Promise { + return Promise.race([ + (async () => { + const { endWebPushForSignOut } = await import('@/lib/web-push'); + for (const client of signingOut) await endWebPushForSignOut(client); + })(), + new Promise((resolve) => setTimeout(resolve, LOGOUT_HOOK_BUDGET_MS)), + ]); +} + function beforeLogoutHooks(accountId: string, options: SignOutCleanupOptions = {}): Promise { return Promise.race([ Promise.all([authHooks.onBeforeLogout.emit({ accountId }), runBeforeSignOut(accountId, options)]).then(() => {}), @@ -2541,6 +2552,8 @@ export const useAuthStore = create()( if (userInitiated && !(await passSignOutGates(accountId ?? 'all'))) return; await beforeLogoutHooks(accountId ?? 'all', { keepLocalStore: keepLocalData }); + // A session that only ran out comes back with the next sign-in, push included. + if (state.client && !wasDemoMode && !keepLocalData) await endPushForSignOut([state.client]); // From here on nothing may renew this slot's session; the cleanup // below reopens it once the credentials are gone. @@ -2690,12 +2703,14 @@ export const useAuthStore = create()( if (!account) return; const slot = account.cookieSlot ?? 0; const wasOAuth = account.authMode === 'oauth'; - - // Started first: the cleanup marks the slot closing right away, so a - // renewal in flight cannot bring the session back. - void clearSlotCredentials(slot, wasOAuth); - clearRefreshTimer(accountId); const client = clients.get(accountId); + // The login's push goes first, while its session still works. + const pushEnded = client && reason === 'explicit' ? endPushForSignOut([client]) : Promise.resolve(); + + // The cleanup marks the slot closing right away, so a renewal in flight + // cannot bring the session back. + void pushEnded.finally(() => clearSlotCredentials(slot, wasOAuth)); + clearRefreshTimer(accountId); if (client) { forgetCalendarSubscriptions(client); try { client.disconnect(); } catch { /* noop */ } @@ -2717,6 +2732,7 @@ export const useAuthStore = create()( for (const account of useAccountStore.getState().accounts) noteRefusedLogin(account.id, true, 'session'); } await beforeLogoutHooks('all', { keepLocalStore: keepLocalData }); + if (!get().isDemoMode && !keepLocalData) await endPushForSignOut(clients.values()); const accountStore = useAccountStore.getState(); const allAccounts = [...accountStore.accounts];