fix(security): stop cross-user offline data leak on shared devices (#1176)

Closes BLOCKER B4 — three reinforcing paths could serve one account's
cached data to the next user on a shared device:

- The Workbox 'api-data' cache keyed trip/user-scoped GETs by URL only
  (cookie-blind). Changed to NetworkOnly; offline reads come from the
  per-user IndexedDB cache via the repo layer instead.
- IndexedDB had no per-user scoping. The Dexie connection is now scoped
  per user (trek-offline-u<id>) behind a Proxy so the ~19 importers keep a
  stable binding; login opens the user DB, logout deletes it and returns
  to the anonymous DB.
- logout() was fire-and-forget and racy: background flush/syncAll could
  re-seed the DB after the wipe. It is now async and ordered — close an
  auth gate, unregister sync triggers, disconnect, clear caches, delete
  the user DB — and flush()/syncAll() bail when the gate is closed.
This commit is contained in:
jubnl
2026-06-15 07:58:20 +02:00
committed by GitHub
parent 0a794583d7
commit 5500405f2f
10 changed files with 223 additions and 28 deletions
+76 -3
View File
@@ -54,6 +54,33 @@ export interface BlobCacheEntry {
// ── Dexie class ──────────────────────────────────────────────────────────────── // ── Dexie class ────────────────────────────────────────────────────────────────
/**
* The offline DB is scoped per user so that one account can never read another
* account's cached data on a shared device. Anonymous (logged-out) state uses
* the base name; a logged-in user uses `trek-offline-u<userId>`.
*/
const ANON_DB_NAME = 'trek-offline';
function userDbName(userId: number | string): string {
return `trek-offline-u${userId}`;
}
/**
* Best-effort read of the persisted auth snapshot so the very first DB opened on
* app load (before loadUser resolves) is already the correct per-user one — the
* PWA can render cached data offline without leaking across users.
*/
function initialDbName(): string {
try {
const raw = typeof localStorage !== 'undefined' ? localStorage.getItem('trek_auth_snapshot') : null;
if (!raw) return ANON_DB_NAME;
const id = JSON.parse(raw)?.state?.user?.id;
return id != null ? userDbName(id) : ANON_DB_NAME;
} catch {
return ANON_DB_NAME;
}
}
class TrekOfflineDb extends Dexie { class TrekOfflineDb extends Dexie {
trips!: Table<Trip, number>; trips!: Table<Trip, number>;
days!: Table<Day, number>; days!: Table<Day, number>;
@@ -71,8 +98,8 @@ class TrekOfflineDb extends Dexie {
syncMeta!: Table<SyncMeta, number>; syncMeta!: Table<SyncMeta, number>;
blobCache!: Table<BlobCacheEntry, string>; blobCache!: Table<BlobCacheEntry, string>;
constructor() { constructor(name: string = ANON_DB_NAME) {
super('trek-offline'); super(name);
this.version(1).stores({ this.version(1).stores({
trips: 'id', trips: 'id',
@@ -97,7 +124,53 @@ class TrekOfflineDb extends Dexie {
} }
} }
export const offlineDb = new TrekOfflineDb(); // The live instance is swapped on login/logout via reopenForUser/reopenAnonymous.
// A Proxy keeps the exported `offlineDb` binding stable for the ~19 modules that
// import it directly, while every access forwards to the current connection.
let _db = new TrekOfflineDb(initialDbName());
export const offlineDb = new Proxy({} as TrekOfflineDb, {
get(_target, prop) {
const value = (_db as unknown as Record<string | symbol, unknown>)[prop];
return typeof value === 'function' ? (value as (...args: unknown[]) => unknown).bind(_db) : value;
},
set(_target, prop, value) {
(_db as unknown as Record<string | symbol, unknown>)[prop] = value;
return true;
},
}) as TrekOfflineDb;
async function switchTo(name: string): Promise<void> {
if (_db.name === name) {
if (!_db.isOpen()) await _db.open();
return;
}
if (_db.isOpen()) _db.close();
_db = new TrekOfflineDb(name);
await _db.open();
}
/** Point the offline DB at a specific user's scoped database (call on login). */
export async function reopenForUser(userId: number | string): Promise<void> {
await switchTo(userDbName(userId));
}
/** Point the offline DB at the anonymous database (call on logout). */
export async function reopenAnonymous(): Promise<void> {
await switchTo(ANON_DB_NAME);
}
/**
* Delete the current user's scoped database entirely and return to the anonymous
* DB. Used on logout so no trace of the account's data remains on the device.
*/
export async function deleteCurrentUserDb(): Promise<void> {
if (_db.name !== ANON_DB_NAME) {
try { await _db.delete(); } catch { /* ignore — fall through to anon */ }
}
_db = new TrekOfflineDb(ANON_DB_NAME);
await _db.open();
}
// ── Bulk upsert helpers ──────────────────────────────────────────────────────── // ── Bulk upsert helpers ────────────────────────────────────────────────────────
+39 -10
View File
@@ -5,7 +5,9 @@ import { connect, disconnect } from '../api/websocket'
import type { User } from '../types' import type { User } from '../types'
import { getApiErrorMessage } from '../types' import { getApiErrorMessage } from '../types'
import { tripSyncManager } from '../sync/tripSyncManager' import { tripSyncManager } from '../sync/tripSyncManager'
import { clearAll } from '../db/offlineDb' import { reopenForUser, deleteCurrentUserDb } from '../db/offlineDb'
import { setAuthed } from '../sync/authGate'
import { unregisterSyncTriggers } from '../sync/syncTriggers'
import { useSystemNoticeStore } from './systemNoticeStore.js' import { useSystemNoticeStore } from './systemNoticeStore.js'
interface AuthResponse { interface AuthResponse {
@@ -40,7 +42,7 @@ interface AuthState {
login: (email: string, password: string) => Promise<LoginResult> login: (email: string, password: string) => Promise<LoginResult>
completeMfaLogin: (mfaToken: string, code: string) => Promise<AuthResponse> completeMfaLogin: (mfaToken: string, code: string) => Promise<AuthResponse>
register: (username: string, email: string, password: string, invite_token?: string) => Promise<AuthResponse> register: (username: string, email: string, password: string, invite_token?: string) => Promise<AuthResponse>
logout: () => void logout: () => Promise<void>
/** Pass `{ silent: true }` to refresh the user without toggling global isLoading (avoids unmounting protected routes). */ /** Pass `{ silent: true }` to refresh the user without toggling global isLoading (avoids unmounting protected routes). */
loadUser: (opts?: { silent?: boolean }) => Promise<void> loadUser: (opts?: { silent?: boolean }) => Promise<void>
updateMapsKey: (key: string | null) => Promise<void> updateMapsKey: (key: string | null) => Promise<void>
@@ -65,6 +67,19 @@ interface AuthState {
// Sequence counter to prevent stale loadUser responses from overwriting fresh auth state // Sequence counter to prevent stale loadUser responses from overwriting fresh auth state
let authSequence = 0 let authSequence = 0
/**
* Mark the session authenticated and point the offline DB at this user's scoped
* database before any background sync runs, so cached data never crosses users.
*/
async function onAuthSuccess(userId: number): Promise<void> {
setAuthed(true)
try {
await reopenForUser(userId)
} catch (err) {
console.error('[auth] failed to open user-scoped offline DB', err)
}
}
export const useAuthStore = create<AuthState>()( export const useAuthStore = create<AuthState>()(
persist( persist(
(set, get) => ({ (set, get) => ({
@@ -99,6 +114,7 @@ export const useAuthStore = create<AuthState>()(
isLoading: false, isLoading: false,
error: null, error: null,
}) })
await onAuthSuccess(data.user.id)
connect() connect()
tripSyncManager.syncAll().catch(console.error) tripSyncManager.syncAll().catch(console.error)
if (!data.user?.must_change_password) { if (!data.user?.must_change_password) {
@@ -123,6 +139,7 @@ export const useAuthStore = create<AuthState>()(
isLoading: false, isLoading: false,
error: null, error: null,
}) })
await onAuthSuccess(data.user.id)
connect() connect()
tripSyncManager.syncAll().catch(console.error) tripSyncManager.syncAll().catch(console.error)
if (!data.user?.must_change_password) { if (!data.user?.must_change_password) {
@@ -147,6 +164,7 @@ export const useAuthStore = create<AuthState>()(
isLoading: false, isLoading: false,
error: null, error: null,
}) })
await onAuthSuccess(data.user.id)
connect() connect()
tripSyncManager.syncAll().catch(console.error) tripSyncManager.syncAll().catch(console.error)
useSystemNoticeStore.getState().fetch() useSystemNoticeStore.getState().fetch()
@@ -158,18 +176,27 @@ export const useAuthStore = create<AuthState>()(
} }
}, },
logout: () => { logout: async () => {
// 1. Gate first so any in-flight flush/syncAll bails before we wipe the DB.
setAuthed(false)
set({ isAuthenticated: false })
// 2. Stop background sync triggers (30s interval, WS pre-reconnect hook, listeners).
unregisterSyncTriggers()
// 3. Tear down the live connection.
disconnect() disconnect()
useSystemNoticeStore.getState().reset() useSystemNoticeStore.getState().reset()
// Tell server to clear the httpOnly cookie // 4. Tell server to clear the httpOnly cookie (best-effort).
fetch('/api/auth/logout', { method: 'POST', credentials: 'include' }).catch(() => {}) await fetch('/api/auth/logout', { method: 'POST', credentials: 'include' }).catch(() => {})
// Clear service worker caches containing sensitive data // 5. Clear service worker caches containing sensitive data.
if ('caches' in window) { if ('caches' in window) {
caches.delete('api-data').catch(() => {}) await Promise.all([
caches.delete('user-uploads').catch(() => {}) caches.delete('api-data').catch(() => {}),
caches.delete('user-uploads').catch(() => {}),
])
} }
// Purge all cached trip data from IndexedDB // 6. Delete this user's scoped IndexedDB and return to the anonymous DB.
clearAll().catch(console.error) await deleteCurrentUserDb().catch(console.error)
// 7. Finish clearing auth state.
set({ set({
user: null, user: null,
isAuthenticated: false, isAuthenticated: false,
@@ -189,6 +216,7 @@ export const useAuthStore = create<AuthState>()(
isAuthenticated: true, isAuthenticated: true,
isLoading: false, isLoading: false,
}) })
await onAuthSuccess(data.user.id)
connect() connect()
} catch (err: unknown) { } catch (err: unknown) {
if (seq !== authSequence) return // stale response — ignore if (seq !== authSequence) return // stale response — ignore
@@ -282,6 +310,7 @@ export const useAuthStore = create<AuthState>()(
demoMode: true, demoMode: true,
error: null, error: null,
}) })
await onAuthSuccess(data.user.id)
connect() connect()
return data return data
} catch (err: unknown) { } catch (err: unknown) {
+18
View File
@@ -0,0 +1,18 @@
/**
* Auth gate — a single boolean the sync layer checks before touching the
* offline DB. It lets logout disable all background sync (flush / syncAll /
* periodic triggers) *before* awaiting the DB swap, so an in-flight loop can't
* re-seed the database after the user has logged out.
*
* Kept separate from authStore to avoid an import cycle
* (authStore → tripSyncManager → authStore).
*/
let _authed = false
export function setAuthed(value: boolean): void {
_authed = value
}
export function isAuthed(): boolean {
return _authed
}
+2 -1
View File
@@ -7,6 +7,7 @@
*/ */
import { offlineDb } from '../db/offlineDb' import { offlineDb } from '../db/offlineDb'
import { apiClient } from '../api/client' import { apiClient } from '../api/client'
import { isAuthed } from './authGate'
import type { QueuedMutation } from '../db/offlineDb' import type { QueuedMutation } from '../db/offlineDb'
import type { Table } from 'dexie' import type { Table } from 'dexie'
@@ -88,7 +89,7 @@ export const mutationQueue = {
* 4xx responses are marked failed and skipped. * 4xx responses are marked failed and skipped.
*/ */
async flush(): Promise<void> { async flush(): Promise<void> {
if (_flushing || !navigator.onLine) return if (_flushing || !navigator.onLine || !isAuthed()) return
_flushing = true _flushing = true
// tempId → realId learned during this flush, so a dependent edit/delete // tempId → realId learned during this flush, so a dependent edit/delete
// queued against an offline-created entity (still holding the negative id) // queued against an offline-created entity (still holding the negative id)
+2 -1
View File
@@ -29,6 +29,7 @@ import {
clearTripData, clearTripData,
} from '../db/offlineDb' } from '../db/offlineDb'
import { prefetchTilesForTrip } from './tilePrefetcher' import { prefetchTilesForTrip } from './tilePrefetcher'
import { isAuthed } from './authGate'
import { useSettingsStore } from '../store/settingsStore' import { useSettingsStore } from '../store/settingsStore'
import type { Trip, Day, Place, PackingItem, TodoItem, BudgetItem, Reservation, TripFile, Accommodation, TripMember } from '../types' import type { Trip, Day, Place, PackingItem, TodoItem, BudgetItem, Reservation, TripFile, Accommodation, TripMember } from '../types'
@@ -134,7 +135,7 @@ export const tripSyncManager = {
* No-ops when offline. * No-ops when offline.
*/ */
async syncAll(): Promise<void> { async syncAll(): Promise<void> {
if (_syncing || !navigator.onLine) return if (_syncing || !navigator.onLine || !isAuthed()) return
_syncing = true _syncing = true
try { try {
const { trips } = await tripsApi.list() as { trips: Trip[] } const { trips } = await tripsApi.list() as { trips: Trip[] }
+37
View File
@@ -23,6 +23,9 @@ import {
upsertReservations, upsertReservations,
upsertTripFiles, upsertTripFiles,
upsertSyncMeta, upsertSyncMeta,
reopenForUser,
reopenAnonymous,
deleteCurrentUserDb,
type QueuedMutation, type QueuedMutation,
type SyncMeta, type SyncMeta,
type BlobCacheEntry, type BlobCacheEntry,
@@ -271,3 +274,37 @@ describe('offlineDb — clearAll', () => {
expect(await offlineDb.places.count()).toBe(0); expect(await offlineDb.places.count()).toBe(0);
}); });
}); });
describe('offlineDb — per-user scoping (B4)', () => {
afterEach(async () => {
// Leave the suite on the anonymous DB so other tests are unaffected.
await reopenAnonymous();
});
it('isolates one user\'s cached data from another', async () => {
await reopenForUser(1);
await upsertPlaces([makePlace(10, 1)]);
expect(await offlineDb.places.count()).toBe(1);
// Switching users must not expose user 1's rows.
await reopenForUser(2);
expect(await offlineDb.places.count()).toBe(0);
// Switching back restores user 1's data (different physical DB).
await reopenForUser(1);
expect(await offlineDb.places.get(10)).toBeDefined();
});
it('deleteCurrentUserDb wipes the user DB and returns to anonymous', async () => {
await reopenForUser(5);
await upsertPlaces([makePlace(20, 1)]);
await deleteCurrentUserDb();
// Now on the anonymous DB — no user data.
expect(await offlineDb.places.count()).toBe(0);
// Re-opening user 5 starts empty (DB was deleted, not just detached).
await reopenForUser(5);
expect(await offlineDb.places.count()).toBe(0);
});
});
+4 -4
View File
@@ -105,10 +105,10 @@ describe('authStore', () => {
}); });
describe('FE-AUTH-006: logout', () => { describe('FE-AUTH-006: logout', () => {
it('calls disconnect() and clears user state', () => { it('calls disconnect() and clears user state', async () => {
useAuthStore.setState({ user: buildUser(), isAuthenticated: true }); useAuthStore.setState({ user: buildUser(), isAuthenticated: true });
useAuthStore.getState().logout(); await useAuthStore.getState().logout();
const state = useAuthStore.getState(); const state = useAuthStore.getState();
expect(disconnect).toHaveBeenCalledOnce(); expect(disconnect).toHaveBeenCalledOnce();
@@ -441,10 +441,10 @@ describe('authStore', () => {
}); });
describe('FE-STORE-AUTH-PERSIST-001: logout resets persisted snapshot', () => { describe('FE-STORE-AUTH-PERSIST-001: logout resets persisted snapshot', () => {
it('snapshot has isAuthenticated:false after logout (PWA offline will redirect to login)', () => { it('snapshot has isAuthenticated:false after logout (PWA offline will redirect to login)', async () => {
useAuthStore.setState({ user: buildUser(), isAuthenticated: true }); useAuthStore.setState({ user: buildUser(), isAuthenticated: true });
useAuthStore.getState().logout(); await useAuthStore.getState().logout();
const snapshot = JSON.parse(localStorage.getItem('trek_auth_snapshot') ?? '{}'); const snapshot = JSON.parse(localStorage.getItem('trek_auth_snapshot') ?? '{}');
expect(snapshot?.state?.isAuthenticated).toBe(false); expect(snapshot?.state?.isAuthenticated).toBe(false);
@@ -8,6 +8,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import 'fake-indexeddb/auto'; import 'fake-indexeddb/auto';
import { server } from '../../helpers/msw/server'; import { server } from '../../helpers/msw/server';
import { http, HttpResponse } from 'msw'; import { http, HttpResponse } from 'msw';
import { setAuthed } from '../../../src/sync/authGate';
import { mutationQueue, generateUUID, nextTempId } from '../../../src/sync/mutationQueue'; import { mutationQueue, generateUUID, nextTempId } from '../../../src/sync/mutationQueue';
import { offlineDb, clearAll } from '../../../src/db/offlineDb'; import { offlineDb, clearAll } from '../../../src/db/offlineDb';
import { placeRepo } from '../../../src/repo/placeRepo'; import { placeRepo } from '../../../src/repo/placeRepo';
@@ -16,11 +17,13 @@ import { buildPlace, buildPackingItem } from '../../helpers/factories';
beforeEach(async () => { beforeEach(async () => {
await clearAll(); await clearAll();
mutationQueue._resetFlushing(); mutationQueue._resetFlushing();
setAuthed(true);
Object.defineProperty(navigator, 'onLine', { value: true, writable: true, configurable: true }); Object.defineProperty(navigator, 'onLine', { value: true, writable: true, configurable: true });
}); });
afterEach(() => { afterEach(() => {
vi.restoreAllMocks(); vi.restoreAllMocks();
setAuthed(false);
}); });
// ── helpers ────────────────────────────────────────────────────────────────── // ── helpers ──────────────────────────────────────────────────────────────────
@@ -215,6 +218,25 @@ describe('mutationQueue.flush — offline guard', () => {
const m = await offlineDb.mutationQueue.get(id); const m = await offlineDb.mutationQueue.get(id);
expect(m!.status).toBe('pending'); expect(m!.status).toBe('pending');
}); });
it('does nothing when logged out (auth gate closed)', async () => {
setAuthed(false);
const id = generateUUID();
await mutationQueue.enqueue(makeMutation({ id }));
let called = false;
server.use(
http.post('/api/trips/1/places', () => {
called = true;
return HttpResponse.json({ place: buildPlace({ trip_id: 1 }) });
}),
);
await mutationQueue.flush();
expect(called).toBe(false);
const m = await offlineDb.mutationQueue.get(id);
expect(m!.status).toBe('pending');
});
}); });
// ── pending / pendingCount ──────────────────────────────────────────────────── // ── pending / pendingCount ────────────────────────────────────────────────────
@@ -9,6 +9,7 @@ import 'fake-indexeddb/auto';
import { server } from '../../helpers/msw/server'; import { server } from '../../helpers/msw/server';
import { http, HttpResponse } from 'msw'; import { http, HttpResponse } from 'msw';
import { tripSyncManager } from '../../../src/sync/tripSyncManager'; import { tripSyncManager } from '../../../src/sync/tripSyncManager';
import { setAuthed } from '../../../src/sync/authGate';
import { offlineDb, clearAll, upsertTrip } from '../../../src/db/offlineDb'; import { offlineDb, clearAll, upsertTrip } from '../../../src/db/offlineDb';
import { import {
buildTrip, buildTrip,
@@ -45,6 +46,7 @@ function makeBundle(tripId: number) {
beforeEach(async () => { beforeEach(async () => {
await clearAll(); await clearAll();
tripSyncManager._resetSyncing(); tripSyncManager._resetSyncing();
setAuthed(true);
Object.defineProperty(navigator, 'onLine', { value: true, writable: true, configurable: true }); Object.defineProperty(navigator, 'onLine', { value: true, writable: true, configurable: true });
// Stub fetch for blob caching (used by cacheFilesForTrip) // Stub fetch for blob caching (used by cacheFilesForTrip)
vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ vi.stubGlobal('fetch', vi.fn().mockResolvedValue({
@@ -56,6 +58,19 @@ beforeEach(async () => {
afterEach(() => { afterEach(() => {
vi.restoreAllMocks(); vi.restoreAllMocks();
vi.unstubAllGlobals(); vi.unstubAllGlobals();
setAuthed(false);
});
describe('tripSyncManager.syncAll — auth gate (B4)', () => {
it('no-ops when logged out (gate closed)', async () => {
setAuthed(false);
let called = false;
server.use(
http.get('/api/trips', () => { called = true; return HttpResponse.json({ trips: [] }); }),
);
await tripSyncManager.syncAll();
expect(called).toBe(false);
});
}); });
// ── offline guard ───────────────────────────────────────────────────────────── // ── offline guard ─────────────────────────────────────────────────────────────
+8 -9
View File
@@ -48,16 +48,15 @@ export default defineConfig({
}, },
}, },
{ {
// API calls — prefer network, fall back to cache // API calls — network only. We deliberately do NOT cache API
// Exclude sensitive endpoints (auth, admin, backup, settings) // responses in the Service Worker: Workbox keys entries by URL and
// cannot vary on the httpOnly session cookie, so a shared device
// could serve one user's cached data to the next (cross-user leak).
// Offline reads are served from the per-user IndexedDB cache via the
// repo layer instead. The urlPattern is kept so these requests still
// bypass the SPA navigation fallback.
urlPattern: /\/api\/(?!auth|admin|backup|settings|health).*/i, urlPattern: /\/api\/(?!auth|admin|backup|settings|health).*/i,
handler: 'NetworkFirst', handler: 'NetworkOnly',
options: {
cacheName: 'api-data',
expiration: { maxEntries: 200, maxAgeSeconds: 24 * 60 * 60 },
networkTimeoutSeconds: 5,
cacheableResponse: { statuses: [200] },
},
}, },
{ {
// Uploaded files (photos, covers — public assets only) // Uploaded files (photos, covers — public assets only)