From 279ff98e0d5f0639077f730083b0c3fd498c800e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Sun, 2 Aug 2026 19:46:05 -0400 Subject: [PATCH] fix(ui): stop precaching index.html so logins can't show the wrong user (#5882) * fix(ui): stop precaching index.html so logins can't show the wrong user index.html is rendered per-user by the server: it carries __APP_CONFIG__, including the auth payload (user id, name, role, Subsonic token) when authentication comes from a reverse-proxy header. Workbox precached it, and because precacheAndRoute registers its route before the NetworkOnly NavigationRoute, every navigation to /app/ was answered from Cache Storage with a frozen copy of whoever installed the service worker. With ExtAuth, signing out and back in as a different user therefore kept showing the previous user, along with their Subsonic token. Cache Storage ignores the no-store header serve_index.go already sets, and ignores the browser's "disable cache", so only clearing site data recovered. Excluding index.html from the precache manifest lets the NetworkOnly navigation strategy do the job it was written for. Existing poisoned caches heal themselves: the entry is dropped when the new manifest activates. The same stale document caused the create-admin dialog to reappear (#3613), patched then by calling removeHomeCache() after login. That helper only ran on password login and token refresh, so it never covered the ExtAuth path, and a refetch re-poisoned the cache anyway. Fixing the cause makes it dead code, so it is removed. Also serve the offline page on 5xx: it arrives as a normal response, so the existing catch never saw it, and without a precached shell a restarting server would surface a raw gateway error. The offline copy is reworded to fit both causes. * test(ui): cover the service worker navigation fallback rules The handler lived inside sw.js, which only loads in a service worker where workbox arrives via importScripts, so none of it was reachable from vitest. Moving the decision into its own module pins the rules that matter: a 5xx falls back to the offline page like a thrown network error does, while 4xx and 304 still pass through to the app. --- .gitignore | 1 + ui/public/offline.html | 2 +- ui/src/authProvider.js | 2 - ui/src/dataProvider/httpClient.js | 2 - ui/src/sw.js | 20 +++++----- ui/src/swNavigation.js | 14 +++++++ ui/src/swNavigation.test.js | 62 +++++++++++++++++++++++++++++++ ui/src/utils/removeHomeCache.js | 20 ---------- ui/vite.config.js | 3 ++ 9 files changed, 90 insertions(+), 36 deletions(-) create mode 100644 ui/src/swNavigation.js create mode 100644 ui/src/swNavigation.test.js delete mode 100644 ui/src/utils/removeHomeCache.js diff --git a/.gitignore b/.gitignore index fc8eaac69..3567a7d90 100644 --- a/.gitignore +++ b/.gitignore @@ -40,3 +40,4 @@ openspec/ .agents go.work* .worktrees/ +.playwright-mcp/ diff --git a/ui/public/offline.html b/ui/public/offline.html index 72f2d04ae..e3c44e8d3 100644 --- a/ui/public/offline.html +++ b/ui/public/offline.html @@ -5,6 +5,6 @@

It looks like we are having trouble connecting.
-Please check your internet connection and try again.

+Please check your internet connection, or try again in a moment.

\ No newline at end of file diff --git a/ui/src/authProvider.js b/ui/src/authProvider.js index 813a4f5b4..18badbc9c 100644 --- a/ui/src/authProvider.js +++ b/ui/src/authProvider.js @@ -1,7 +1,6 @@ import { jwtDecode } from 'jwt-decode' import { baseUrl } from './utils' import config from './config' -import { removeHomeCache } from './utils/removeHomeCache' // config sent from server may contain authentication info, for example when the user is authenticated // by a reverse proxy request header @@ -49,7 +48,6 @@ const authProvider = { storeAuthenticationInfo(response) // Avoid "going to create admin" dialog after logout/login without a refresh config.firstTime = false - removeHomeCache() return response }) .catch((error) => { diff --git a/ui/src/dataProvider/httpClient.js b/ui/src/dataProvider/httpClient.js index 3820f658d..706486fa6 100644 --- a/ui/src/dataProvider/httpClient.js +++ b/ui/src/dataProvider/httpClient.js @@ -3,7 +3,6 @@ import { v4 as uuidv4 } from 'uuid' import { baseUrl } from '../utils' import config from '../config' import { jwtDecode } from 'jwt-decode' -import { removeHomeCache } from '../utils/removeHomeCache' const customAuthorizationHeader = 'X-ND-Authorization' export const clientUniqueIdHeader = 'X-ND-Client-Unique-Id' @@ -27,7 +26,6 @@ const httpClient = (url, options = {}) => { localStorage.setItem('userId', decoded.uid) // Avoid going to create admin dialog after logout/login without a refresh config.firstTime = false - removeHomeCache() } return response }) diff --git a/ui/src/sw.js b/ui/src/sw.js index f4a5664b8..edc272850 100644 --- a/ui/src/sw.js +++ b/ui/src/sw.js @@ -1,5 +1,7 @@ /* eslint-disable */ +import { createNavigationHandler } from './swNavigation' + // documentation: https://developers.google.com/web/tools/workbox/modules/workbox-sw importScripts('3rdparty/workbox/workbox-sw.js') @@ -36,17 +38,13 @@ self.addEventListener('install', async (event) => { }) const networkOnly = new workbox.strategies.NetworkOnly() -const navigationHandler = async (params) => { - try { - // Attempt a network request. - return await networkOnly.handle(params) - } catch (error) { - // If it fails, return the cached HTML. - return caches.match(FALLBACK_HTML_URL, { - cacheName: CACHE_NAME, - }) - } -} +const fallbackToCachedHtml = () => + caches.match(FALLBACK_HTML_URL, { cacheName: CACHE_NAME }) + +const navigationHandler = createNavigationHandler( + networkOnly, + fallbackToCachedHtml, +) // self.__WB_MANIFEST is default injection point workbox.precaching.precacheAndRoute(self.__WB_MANIFEST) diff --git a/ui/src/swNavigation.js b/ui/src/swNavigation.js new file mode 100644 index 000000000..28d01eaef --- /dev/null +++ b/ui/src/swNavigation.js @@ -0,0 +1,14 @@ +// Lives outside sw.js so it can be unit tested: sw.js only loads inside a +// service worker, where workbox arrives via importScripts. +export const createNavigationHandler = + (networkOnly, offlineFallback) => async (params) => { + try { + // Attempt a network request. + const response = await networkOnly.handle(params) + // A 5xx reaches us as a normal response, but carries no usable app + return response.status >= 500 ? offlineFallback() : response + } catch (error) { + // If it fails, return the cached HTML. + return offlineFallback() + } + } diff --git a/ui/src/swNavigation.test.js b/ui/src/swNavigation.test.js new file mode 100644 index 000000000..8ab749757 --- /dev/null +++ b/ui/src/swNavigation.test.js @@ -0,0 +1,62 @@ +import { describe, it, expect, vi } from 'vitest' +import { createNavigationHandler } from './swNavigation' + +const OFFLINE = { offlinePage: true } + +const setup = (networkResult) => { + const networkOnly = { + handle: vi.fn(() => + networkResult instanceof Error + ? Promise.reject(networkResult) + : Promise.resolve(networkResult), + ), + } + const offlineFallback = vi.fn(() => Promise.resolve(OFFLINE)) + return { + networkOnly, + offlineFallback, + handler: createNavigationHandler(networkOnly, offlineFallback), + } +} + +describe('createNavigationHandler', () => { + it('serves the response from the network', async () => { + const response = { status: 200 } + const { handler, offlineFallback } = setup(response) + + await expect(handler({})).resolves.toBe(response) + expect(offlineFallback).not.toHaveBeenCalled() + }) + + it('falls back to the offline page when the network fails', async () => { + const { handler } = setup(new Error('Failed to fetch')) + + await expect(handler({})).resolves.toBe(OFFLINE) + }) + + it.each([500, 502, 503])( + 'falls back to the offline page on %i', + async (status) => { + const { handler } = setup({ status }) + + await expect(handler({})).resolves.toBe(OFFLINE) + }, + ) + + it.each([304, 401, 404])('passes %i through untouched', async (status) => { + const response = { status } + const { handler, offlineFallback } = setup(response) + + await expect(handler({})).resolves.toBe(response) + expect(offlineFallback).not.toHaveBeenCalled() + }) + + it('passes the navigation params to the network strategy', async () => { + const params = { request: { url: '/app/' } } + const { handler, networkOnly } = setup({ status: 200 }) + + await handler(params) + + expect(networkOnly.handle).toHaveBeenCalledWith(params) + }) +}) diff --git a/ui/src/utils/removeHomeCache.js b/ui/src/utils/removeHomeCache.js deleted file mode 100644 index 08ed720e0..000000000 --- a/ui/src/utils/removeHomeCache.js +++ /dev/null @@ -1,20 +0,0 @@ -export const removeHomeCache = async () => { - try { - const workboxKey = (await caches.keys()).find((key) => - key.startsWith('workbox-precache'), - ) - if (!workboxKey) return - - const workboxCache = await caches.open(workboxKey) - const indexKey = (await workboxCache.keys()).find((key) => - key.url.includes('app/index.html'), - ) - - if (indexKey) { - await workboxCache.delete(indexKey) - } - } catch (e) { - // eslint-disable-next-line no-console - console.error('error reading cache', e) - } -} diff --git a/ui/vite.config.js b/ui/vite.config.js index 9d9c845f1..182e825b0 100644 --- a/ui/vite.config.js +++ b/ui/vite.config.js @@ -16,6 +16,9 @@ export default defineConfig({ filename: 'sw.js', injectManifest: { maximumFileSizeToCacheInBytes: 3 * 1024 * 1024, // 3 MiB + // index.html is rendered per-user by the server, so a precached copy + // would pin one user's config (and auth payload) across logins + globIgnores: ['index.html'], }, devOptions: { enabled: true,