From 13be42792d6de4bf38188a7dbdbae163fcdf3425 Mon Sep 17 00:00:00 2001 From: neon <18559658+neonsolstice@users.noreply.github.com> Date: Thu, 2 Jul 2026 23:10:15 +0000 Subject: [PATCH] fix(client): prevent cached state leaking across user sessions Use request generation counters in composables to ignore in-flight fetches that resolve after a state reset. This ensures that user-scoped data like smart scopes, libraries, and collections do not persist across user switches. Closes #510 --- .../composables/__tests__/useAuth.spec.ts | 111 ++++++++++++++++++ .../src/features/auth/composables/useAuth.ts | 6 + .../__tests__/useCollections.spec.ts | 26 ++++ .../collection/composables/useCollections.ts | 22 +++- .../__tests__/useLibraries.spec.ts | 103 ++++++++++++++++ .../library/composables/useLibraries.ts | 20 +++- .../__tests__/useSmartScopes.spec.ts | 46 ++++++++ .../smart-scope/composables/useSmartScopes.ts | 32 ++++- 8 files changed, 355 insertions(+), 11 deletions(-) create mode 100644 client/src/features/auth/composables/__tests__/useAuth.spec.ts create mode 100644 client/src/features/library/composables/__tests__/useLibraries.spec.ts diff --git a/client/src/features/auth/composables/__tests__/useAuth.spec.ts b/client/src/features/auth/composables/__tests__/useAuth.spec.ts new file mode 100644 index 000000000..212bd63d7 --- /dev/null +++ b/client/src/features/auth/composables/__tests__/useAuth.spec.ts @@ -0,0 +1,111 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +type VoidFn = () => void + +const routerPushMock = vi.hoisted(() => vi.fn<(to: string) => void>()) +const setAccessTokenMock = vi.hoisted(() => vi.fn<(token: string | null) => void>()) +const setOnAuthFailureMock = vi.hoisted(() => vi.fn<(fn: VoidFn) => void>()) +const refreshAccessTokenMock = vi.hoisted(() => vi.fn<() => Promise>()) +const apiMock = vi.hoisted(() => vi.fn<(input: RequestInfo | URL, init?: RequestInit) => Promise>()) +const resetLibrariesMock = vi.hoisted(() => vi.fn()) +const resetSmartScopesMock = vi.hoisted(() => vi.fn()) +const resetCollectionsMock = vi.hoisted(() => vi.fn()) +const resetWhatsNewMock = vi.hoisted(() => vi.fn()) +const cancelPendingThemeSyncMock = vi.hoisted(() => vi.fn()) +const cancelPendingDisplaySettingsSyncMock = vi.hoisted(() => vi.fn()) +const disconnectAuthorEnrichmentSocketMock = vi.hoisted(() => vi.fn()) +const disconnectBookMetadataFetchSocketMock = vi.hoisted(() => vi.fn()) + +vi.mock('@/router', () => ({ + default: { + push: routerPushMock, + currentRoute: { value: { query: {} } }, + }, +})) + +vi.mock('@/lib/api', () => ({ + api: apiMock, + refreshAccessToken: refreshAccessTokenMock, + setAccessToken: setAccessTokenMock, + setOnAuthFailure: setOnAuthFailureMock, +})) + +vi.mock('@/composables/useThemeSync', () => ({ + cancelPendingThemeSync: cancelPendingThemeSyncMock, + initThemeSync: vi.fn(), + loadFromServer: vi.fn<() => Promise>(), +})) + +vi.mock('@/composables/useDisplaySettingsSync', () => ({ + cancelPendingDisplaySettingsSync: cancelPendingDisplaySettingsSyncMock, + initDisplaySettingsSync: vi.fn(), + loadDisplaySettingsFromServer: vi.fn<() => Promise>(), +})) + +vi.mock('../useSetupStatus', () => ({ + useSetupStatus: () => ({ + needsSetup: { value: false }, + }), +})) + +vi.mock('@/features/settings/composables/useAuthorEnrichmentStatus', () => ({ + disconnectAuthorEnrichmentSocket: disconnectAuthorEnrichmentSocketMock, +})) + +vi.mock('@/features/book-metadata-fetch/composables/useBookMetadataFetchStatus', () => ({ + disconnectBookMetadataFetchSocket: disconnectBookMetadataFetchSocketMock, +})) + +vi.mock('@/features/whats-new/composables/useWhatsNew', () => ({ + resetWhatsNew: resetWhatsNewMock, +})) + +vi.mock('@/features/library/composables/useLibraries', () => ({ + resetLibraries: resetLibrariesMock, +})) + +vi.mock('@/features/smart-scope/composables/useSmartScopes', () => ({ + resetSmartScopes: resetSmartScopesMock, +})) + +vi.mock('@/features/collection/composables/useCollections', () => ({ + resetCollections: resetCollectionsMock, +})) + +describe('useAuth', () => { + beforeEach(() => { + vi.resetModules() + routerPushMock.mockReset() + setAccessTokenMock.mockReset() + setOnAuthFailureMock.mockReset() + refreshAccessTokenMock.mockReset() + apiMock.mockReset() + resetLibrariesMock.mockReset() + resetSmartScopesMock.mockReset() + resetCollectionsMock.mockReset() + resetWhatsNewMock.mockReset() + cancelPendingThemeSyncMock.mockReset() + cancelPendingDisplaySettingsSyncMock.mockReset() + disconnectAuthorEnrichmentSocketMock.mockReset() + disconnectBookMetadataFetchSocketMock.mockReset() + vi.stubGlobal( + 'fetch', + vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({}), + } as Response), + ) + }) + + it('clears user-scoped sidebar caches on logout', async () => { + const { useAuth } = await import('../useAuth') + + await useAuth().logout() + + expect(resetLibrariesMock).toHaveBeenCalledTimes(1) + expect(resetSmartScopesMock).toHaveBeenCalledTimes(1) + expect(resetCollectionsMock).toHaveBeenCalledTimes(1) + expect(setAccessTokenMock).toHaveBeenCalledWith(null) + expect(routerPushMock).toHaveBeenCalledWith('/login') + }) +}) diff --git a/client/src/features/auth/composables/useAuth.ts b/client/src/features/auth/composables/useAuth.ts index d2bc327df..a6f99d692 100644 --- a/client/src/features/auth/composables/useAuth.ts +++ b/client/src/features/auth/composables/useAuth.ts @@ -8,6 +8,9 @@ import { useSetupStatus } from './useSetupStatus' import { disconnectAuthorEnrichmentSocket } from '@/features/settings/composables/useAuthorEnrichmentStatus' import { disconnectBookMetadataFetchSocket } from '@/features/book-metadata-fetch/composables/useBookMetadataFetchStatus' import { resetWhatsNew } from '@/features/whats-new/composables/useWhatsNew' +import { resetLibraries } from '@/features/library/composables/useLibraries' +import { resetSmartScopes } from '@/features/smart-scope/composables/useSmartScopes' +import { resetCollections } from '@/features/collection/composables/useCollections' const SESSION_REFRESH_INTERVAL_MS = 5 * 60 * 1000 @@ -49,6 +52,9 @@ function clearAuth() { cancelPendingThemeSync() cancelPendingDisplaySettingsSync() stopSessionRefresh() + resetLibraries() + resetSmartScopes() + resetCollections() user.value = null setAccessToken(null) disconnectAuthorEnrichmentSocket() diff --git a/client/src/features/collection/composables/__tests__/useCollections.spec.ts b/client/src/features/collection/composables/__tests__/useCollections.spec.ts index 8f5def7d7..18c08db4a 100644 --- a/client/src/features/collection/composables/__tests__/useCollections.spec.ts +++ b/client/src/features/collection/composables/__tests__/useCollections.spec.ts @@ -111,4 +111,30 @@ describe('useCollections', () => { await expect(removeBooksFromCollection(1, { bookIds: [9] })).rejects.toThrow('Failed to remove books from collection') }) + + it('resets cached collections so the next fetch reloads them', async () => { + const first = makeCollection({ id: 1, name: 'Owner Collection' }) + const second = makeCollection({ id: 2, name: 'Next User Collection' }) + apiMock.mockResolvedValueOnce(makeResponse([first])).mockResolvedValueOnce(makeResponse([second])) + + const { resetCollections, useCollections } = await import('../useCollections') + const { collections, loaded, fetchCollections } = useCollections() + + await fetchCollections() + await fetchCollections() + expect(apiMock).toHaveBeenCalledTimes(1) + expect(collections.value).toEqual([first]) + expect(loaded.value).toBe(true) + + resetCollections() + + expect(collections.value).toEqual([]) + expect(loaded.value).toBe(false) + + await fetchCollections() + + expect(apiMock).toHaveBeenCalledTimes(2) + expect(collections.value).toEqual([second]) + expect(loaded.value).toBe(true) + }) }) diff --git a/client/src/features/collection/composables/useCollections.ts b/client/src/features/collection/composables/useCollections.ts index bd2651a1f..6b952571b 100644 --- a/client/src/features/collection/composables/useCollections.ts +++ b/client/src/features/collection/composables/useCollections.ts @@ -7,6 +7,16 @@ const loaded = ref(false) const loading = ref(false) const error = ref(null) let fetchPromise: Promise | null = null +let requestGeneration = 0 + +export function resetCollections(): void { + requestGeneration += 1 + collections.value = [] + loaded.value = false + loading.value = false + error.value = null + fetchPromise = null +} export function useCollections() { async function fetchCollections(): Promise { @@ -14,20 +24,26 @@ export function useCollections() { if (fetchPromise) return fetchPromise loading.value = true error.value = null + const generation = requestGeneration fetchPromise = api('/api/v1/collections') .then(async (res) => { if (!res.ok) { throw new Error(`HTTP ${res.status}`) } - collections.value = await res.json() + const nextCollections: Collection[] = await res.json() + if (generation !== requestGeneration) return + collections.value = nextCollections loaded.value = true }) .catch((e: unknown) => { + if (generation !== requestGeneration) return error.value = e instanceof Error ? e.message : 'Failed to load collections' }) .finally(() => { - loading.value = false - fetchPromise = null + if (generation === requestGeneration) { + loading.value = false + fetchPromise = null + } }) return fetchPromise } diff --git a/client/src/features/library/composables/__tests__/useLibraries.spec.ts b/client/src/features/library/composables/__tests__/useLibraries.spec.ts new file mode 100644 index 000000000..a558e7328 --- /dev/null +++ b/client/src/features/library/composables/__tests__/useLibraries.spec.ts @@ -0,0 +1,103 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { Library } from '@bookorbit/types' + +const apiMock = vi.hoisted(() => vi.fn<(input: RequestInfo | URL, init?: RequestInit) => Promise>()) + +vi.mock('@/lib/api', () => ({ + api: apiMock, +})) + +function makeLibrary(overrides: Partial = {}): Library { + return { + id: 3, + name: 'Main Library', + icon: null, + displayOrder: 0, + coverAspectRatio: '2/3', + watch: false, + autoScanCronExpression: null, + metadataPrecedence: [], + formatPriority: [], + allowedFormats: [], + organizationMode: 'book_per_file', + excludePatterns: [], + readingThreshold: 10, + markAsFinishedPercentComplete: 95, + fileNamingPattern: null, + fileWriteEnabled: false, + fileWriteWriteCover: false, + fileWriteEpubEnabled: false, + fileWriteEpubMaxFileSizeMb: 50, + fileWritePdfEnabled: false, + fileWritePdfMaxFileSizeMb: 50, + fileWriteCbxEnabled: false, + fileWriteCbxMaxFileSizeMb: 50, + fileWriteAudioEnabled: false, + fileWriteAudioMaxFileSizeMb: 50, + fileRenameEnabled: false, + folders: [], + createdAt: '2026-01-01T00:00:00.000Z', + updatedAt: '2026-01-01T00:00:00.000Z', + ...overrides, + } +} + +function makeResponse(data?: unknown, ok = true): Response { + return { + ok, + json: async () => data, + } as Response +} + +describe('useLibraries', () => { + beforeEach(() => { + vi.resetModules() + apiMock.mockReset() + }) + + it('resets cached libraries so the next fetch reloads them', async () => { + const first = makeLibrary({ id: 1, name: 'Owner Library' }) + const second = makeLibrary({ id: 2, name: 'Next User Library' }) + apiMock.mockResolvedValueOnce(makeResponse([first])).mockResolvedValueOnce(makeResponse([second])) + + const { resetLibraries, useLibraries } = await import('../useLibraries') + const { libraries, loaded, fetchLibraries } = useLibraries() + + await fetchLibraries() + await fetchLibraries() + expect(apiMock).toHaveBeenCalledTimes(1) + expect(libraries.value).toEqual([first]) + expect(loaded.value).toBe(true) + + resetLibraries() + + expect(libraries.value).toEqual([]) + expect(loaded.value).toBe(false) + + await fetchLibraries() + + expect(apiMock).toHaveBeenCalledTimes(2) + expect(libraries.value).toEqual([second]) + expect(loaded.value).toBe(true) + }) + + it('ignores an in-flight fetch after libraries are reset', async () => { + const stale = makeLibrary({ id: 1, name: 'Stale Library' }) + let resolveFetch!: (response: Response) => void + apiMock.mockReturnValueOnce(new Promise((resolve) => (resolveFetch = resolve))) + + const { resetLibraries, useLibraries } = await import('../useLibraries') + const { libraries, loaded, loading, fetchLibraries } = useLibraries() + + const fetchPromise = fetchLibraries() + expect(loading.value).toBe(true) + + resetLibraries() + resolveFetch(makeResponse([stale])) + await fetchPromise + + expect(libraries.value).toEqual([]) + expect(loaded.value).toBe(false) + expect(loading.value).toBe(false) + }) +}) diff --git a/client/src/features/library/composables/useLibraries.ts b/client/src/features/library/composables/useLibraries.ts index 1e76ee0a2..bbddb5ff2 100644 --- a/client/src/features/library/composables/useLibraries.ts +++ b/client/src/features/library/composables/useLibraries.ts @@ -6,6 +6,15 @@ const libraries = ref([]) const loading = ref(false) const loaded = ref(false) let fetchPromise: Promise | null = null +let requestGeneration = 0 + +export function resetLibraries(): void { + requestGeneration += 1 + libraries.value = [] + loading.value = false + loaded.value = false + fetchPromise = null +} export function useLibraries() { async function fetchLibraries(): Promise { @@ -16,15 +25,20 @@ export function useLibraries() { async function refreshLibraries(): Promise { if (fetchPromise) return fetchPromise loading.value = true + const generation = requestGeneration fetchPromise = api('/api/v1/libraries') .then(async (res) => { if (!res.ok) return - libraries.value = await res.json() + const nextLibraries: Library[] = await res.json() + if (generation !== requestGeneration) return + libraries.value = nextLibraries loaded.value = true }) .finally(() => { - fetchPromise = null - loading.value = false + if (generation === requestGeneration) { + fetchPromise = null + loading.value = false + } }) return fetchPromise } diff --git a/client/src/features/smart-scope/composables/__tests__/useSmartScopes.spec.ts b/client/src/features/smart-scope/composables/__tests__/useSmartScopes.spec.ts index 0f5182d56..e633c4453 100644 --- a/client/src/features/smart-scope/composables/__tests__/useSmartScopes.spec.ts +++ b/client/src/features/smart-scope/composables/__tests__/useSmartScopes.spec.ts @@ -65,4 +65,50 @@ describe('useSmartScopes', () => { expect(smartScopes.value).toEqual([updated]) expect(smartScopes.value).not.toBe(previous) }) + + it('resets cached smartScopes so the next fetch reloads them', async () => { + const first = makeSmartScope({ id: 1, name: 'Owner Scope' }) + const second = makeSmartScope({ id: 2, userId: 4, name: 'Next User Scope' }) + apiMock.mockResolvedValueOnce(makeResponse([first])).mockResolvedValueOnce(makeResponse([second])) + + const { resetSmartScopes, useSmartScopes } = await import('../useSmartScopes') + const { smartScopes, loaded, fetchSmartScopes } = useSmartScopes() + + await fetchSmartScopes() + await fetchSmartScopes() + expect(apiMock).toHaveBeenCalledTimes(1) + expect(smartScopes.value).toEqual([first]) + expect(loaded.value).toBe(true) + + resetSmartScopes() + + expect(smartScopes.value).toEqual([]) + expect(loaded.value).toBe(false) + + await fetchSmartScopes() + + expect(apiMock).toHaveBeenCalledTimes(2) + expect(smartScopes.value).toEqual([second]) + expect(loaded.value).toBe(true) + }) + + it('ignores an in-flight fetch after smartScopes are reset', async () => { + const stale = makeSmartScope({ id: 1, name: 'Stale Scope' }) + let resolveFetch!: (response: Response) => void + apiMock.mockReturnValueOnce(new Promise((resolve) => (resolveFetch = resolve))) + + const { resetSmartScopes, useSmartScopes } = await import('../useSmartScopes') + const { smartScopes, loaded, loading, fetchSmartScopes } = useSmartScopes() + + const fetchPromise = fetchSmartScopes() + expect(loading.value).toBe(true) + + resetSmartScopes() + resolveFetch(makeResponse([stale])) + await fetchPromise + + expect(smartScopes.value).toEqual([]) + expect(loaded.value).toBe(false) + expect(loading.value).toBe(false) + }) }) diff --git a/client/src/features/smart-scope/composables/useSmartScopes.ts b/client/src/features/smart-scope/composables/useSmartScopes.ts index 943276761..50a2ef4fc 100644 --- a/client/src/features/smart-scope/composables/useSmartScopes.ts +++ b/client/src/features/smart-scope/composables/useSmartScopes.ts @@ -7,6 +7,16 @@ const loaded = ref(false) const loading = ref(false) const error = ref(null) let fetchPromise: Promise | null = null +let requestGeneration = 0 + +export function resetSmartScopes(): void { + requestGeneration += 1 + smartScopes.value = [] + loaded.value = false + loading.value = false + error.value = null + fetchPromise = null +} export function useSmartScopes() { async function fetchSmartScopes() { @@ -14,20 +24,26 @@ export function useSmartScopes() { if (fetchPromise) return fetchPromise loading.value = true error.value = null + const generation = requestGeneration fetchPromise = api('/api/v1/smart-scopes') .then(async (res) => { if (!res.ok) { throw new Error(`HTTP ${res.status}`) } - smartScopes.value = await res.json() + const nextSmartScopes: SmartScope[] = await res.json() + if (generation !== requestGeneration) return + smartScopes.value = nextSmartScopes loaded.value = true }) .catch((e: unknown) => { + if (generation !== requestGeneration) return error.value = e instanceof Error ? e.message : 'Failed to load smart scopes' }) .finally(() => { - loading.value = false - fetchPromise = null + if (generation === requestGeneration) { + loading.value = false + fetchPromise = null + } }) return fetchPromise } @@ -35,17 +51,23 @@ export function useSmartScopes() { async function refreshSmartScopes() { loading.value = true error.value = null + const generation = requestGeneration try { const res = await api('/api/v1/smart-scopes') if (!res.ok) { throw new Error(`HTTP ${res.status}`) } - smartScopes.value = await res.json() + const nextSmartScopes: SmartScope[] = await res.json() + if (generation !== requestGeneration) return + smartScopes.value = nextSmartScopes loaded.value = true } catch (e: unknown) { + if (generation !== requestGeneration) return error.value = e instanceof Error ? e.message : 'Failed to load smart scopes' } finally { - loading.value = false + if (generation === requestGeneration) { + loading.value = false + } } }