diff --git a/client/src/features/auth/ResetPasswordPage.vue b/client/src/features/auth/ResetPasswordPage.vue index 46113be7b..3cd8983bb 100644 --- a/client/src/features/auth/ResetPasswordPage.vue +++ b/client/src/features/auth/ResetPasswordPage.vue @@ -4,6 +4,7 @@ import { useI18n } from 'vue-i18n' import { useRoute, useRouter } from 'vue-router' import { Moon, Sun, Wallpaper } from '@lucide/vue' import { ACCENT_OPTIONS, ACCENT_ROWS, RADIUS_OPTIONS, BACKGROUND_OPTIONS, useThemeStore } from '@/stores/theme' +import { fetchWithAuthProxyRecovery } from '@/lib/api' import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' import PublicLegalNotices from '@/components/legal/PublicLegalNotices.vue' @@ -74,7 +75,7 @@ async function handleSubmit() { loading.value = true try { - const res = await fetch('/api/v1/auth/reset-password', { + const res = await fetchWithAuthProxyRecovery('/api/v1/auth/reset-password', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ token: token.value, newPassword: newPassword.value }), diff --git a/client/src/features/auth/__tests__/useAuth.register.spec.ts b/client/src/features/auth/__tests__/useAuth.register.spec.ts index b53cdd7b9..f5a9d7470 100644 --- a/client/src/features/auth/__tests__/useAuth.register.spec.ts +++ b/client/src/features/auth/__tests__/useAuth.register.spec.ts @@ -12,6 +12,7 @@ vi.mock('@/router', () => ({ vi.mock('@/lib/api', () => ({ api: vi.fn<() => void>(), + fetchWithAuthProxyRecovery: (input: RequestInfo | URL, init?: RequestInit) => fetch(input, { ...init, redirect: 'manual' }), refreshAccessToken: vi.fn<() => void>(), setAccessToken: vi.fn<() => void>(), setOnAuthFailure: vi.fn<() => void>(), @@ -59,6 +60,7 @@ describe('useAuth.register', () => { method: 'POST', headers: { 'Content-Type': 'application/json' }, credentials: 'include', + redirect: 'manual', body: JSON.stringify(PAYLOAD), }) }) diff --git a/client/src/features/auth/__tests__/useSetupStatus.spec.ts b/client/src/features/auth/__tests__/useSetupStatus.spec.ts index 6874a9884..f5c34b761 100644 --- a/client/src/features/auth/__tests__/useSetupStatus.spec.ts +++ b/client/src/features/auth/__tests__/useSetupStatus.spec.ts @@ -24,7 +24,7 @@ describe('useSetupStatus', () => { const { fetchSetupStatus, needsSetup, allowRegistration } = await loadComposable() await fetchSetupStatus() - expect(fetchMock).toHaveBeenCalledWith('/api/v1/auth/setup-status', { credentials: 'include' }) + expect(fetchMock).toHaveBeenCalledWith('/api/v1/auth/setup-status', { credentials: 'include', redirect: 'manual' }) expect(needsSetup.value).toBe(false) expect(allowRegistration.value).toBe(true) }) diff --git a/client/src/features/auth/composables/__tests__/useAuth.spec.ts b/client/src/features/auth/composables/__tests__/useAuth.spec.ts index 3a21bcaf4..755f7bef3 100644 --- a/client/src/features/auth/composables/__tests__/useAuth.spec.ts +++ b/client/src/features/auth/composables/__tests__/useAuth.spec.ts @@ -26,6 +26,7 @@ vi.mock('@/router', () => ({ vi.mock('@/lib/api', () => ({ api: apiMock, + fetchWithAuthProxyRecovery: (input: RequestInfo | URL, init?: RequestInit) => fetch(input, { ...init, redirect: 'manual' }), refreshAccessToken: refreshAccessTokenMock, setAccessToken: setAccessTokenMock, setOnAuthFailure: setOnAuthFailureMock, diff --git a/client/src/features/auth/composables/__tests__/useLoginOptions.spec.ts b/client/src/features/auth/composables/__tests__/useLoginOptions.spec.ts new file mode 100644 index 000000000..ec3a46c0a --- /dev/null +++ b/client/src/features/auth/composables/__tests__/useLoginOptions.spec.ts @@ -0,0 +1,49 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import type { useLoginOptions as UseLoginOptions } from '../useLoginOptions' + +describe('useLoginOptions', () => { + const originalFetch = globalThis.fetch + let reload: ReturnType void>> + let useLoginOptions: typeof UseLoginOptions + + beforeEach(async () => { + // The composable keeps its cache and in-flight request at module level; a fresh module per test + // keeps one test's state from leaking into the next. + vi.resetModules() + ;({ useLoginOptions } = await import('../useLoginOptions')) + sessionStorage.clear() + reload = vi.fn<() => void>() + vi.stubGlobal('location', { ...window.location, reload }) + }) + + afterEach(() => { + globalThis.fetch = originalFetch + vi.unstubAllGlobals() + vi.restoreAllMocks() + }) + + it('reloads the page when an edge auth proxy redirects the request, instead of reporting an error', async () => { + const res = new Response(null) + Object.defineProperty(res, 'type', { value: 'opaqueredirect' }) + const fetchMock = vi.fn(() => Promise.resolve(res)) + globalThis.fetch = fetchMock as never + + const { fetchLoginOptions, loginOptionsError } = useLoginOptions() + void fetchLoginOptions(true) + + await vi.waitFor(() => expect(reload).toHaveBeenCalledTimes(1)) + expect(fetchMock.mock.calls[0]![1]?.redirect).toBe('manual') + expect(loginOptionsError.value).toBeNull() + }) + + it('still loads the options normally when there is no proxy in the way', async () => { + const options = { localAuthEnabled: true, oidcProviders: [] } + globalThis.fetch = vi.fn(() => Promise.resolve(new Response(JSON.stringify(options), { status: 200 }))) as never + + const { fetchLoginOptions } = useLoginOptions() + + await expect(fetchLoginOptions(true)).resolves.toEqual(options) + expect(reload).not.toHaveBeenCalled() + }) +}) diff --git a/client/src/features/auth/composables/useAuth.ts b/client/src/features/auth/composables/useAuth.ts index 33b09c24a..446924386 100644 --- a/client/src/features/auth/composables/useAuth.ts +++ b/client/src/features/auth/composables/useAuth.ts @@ -1,6 +1,6 @@ import { ref } from 'vue' import type { AuthUser, AuthResponse } from '@bookorbit/types' -import { api, refreshAccessToken, setAccessToken, setOnAuthFailure } from '@/lib/api' +import { api, fetchWithAuthProxyRecovery, refreshAccessToken, setAccessToken, setOnAuthFailure } from '@/lib/api' import router from '@/router' import { cancelPendingDisplaySettingsSync, initDisplaySettingsSync, loadDisplaySettingsFromServer } from '@/composables/useDisplaySettingsSync' import { cancelPendingThemeSync, initThemeSync, loadFromServer } from '@/composables/useThemeSync' @@ -137,7 +137,7 @@ export function useAuth() { } async function login(username: string, password: string): Promise { - const res = await fetch('/api/v1/auth/login', { + const res = await fetchWithAuthProxyRecovery('/api/v1/auth/login', { method: 'POST', headers: { 'Content-Type': 'application/json' }, credentials: 'include', @@ -169,7 +169,7 @@ export function useAuth() { headers['x-setup-token'] = payload.setupToken } - const res = await fetch('/api/v1/auth/setup', { + const res = await fetchWithAuthProxyRecovery('/api/v1/auth/setup', { method: 'POST', headers, credentials: 'include', @@ -197,7 +197,7 @@ export function useAuth() { } async function register(payload: { username: string; name: string; email: string; password: string }): Promise { - const res = await fetch('/api/v1/auth/register', { + const res = await fetchWithAuthProxyRecovery('/api/v1/auth/register', { method: 'POST', headers: { 'Content-Type': 'application/json' }, credentials: 'include', @@ -214,7 +214,7 @@ export function useAuth() { async function logout(): Promise { try { - await fetch('/api/v1/auth/logout', { method: 'POST', credentials: 'include' }).catch(() => undefined) + await fetchWithAuthProxyRecovery('/api/v1/auth/logout', { method: 'POST', credentials: 'include' }).catch(() => undefined) } finally { clearAuth() router.push('/login') @@ -222,7 +222,7 @@ export function useAuth() { } async function loginWithMagicLink(token: string): Promise { - const res = await fetch('/api/v1/auth/magic-links/login', { + const res = await fetchWithAuthProxyRecovery('/api/v1/auth/magic-links/login', { method: 'POST', headers: { 'Content-Type': 'application/json' }, credentials: 'include', diff --git a/client/src/features/auth/composables/useLoginOptions.ts b/client/src/features/auth/composables/useLoginOptions.ts index bca406a13..683f6b165 100644 --- a/client/src/features/auth/composables/useLoginOptions.ts +++ b/client/src/features/auth/composables/useLoginOptions.ts @@ -1,5 +1,6 @@ import { ref } from 'vue' import type { LoginOptionsResponse } from '@bookorbit/types' +import { fetchWithAuthProxyRecovery } from '@/lib/api' const loginOptions = ref(null) const loginOptionsError = ref(null) @@ -12,7 +13,7 @@ export function useLoginOptions() { inFlight = (async () => { try { - const res = await fetch('/api/v1/auth/login-options', { credentials: 'include' }) + const res = await fetchWithAuthProxyRecovery('/api/v1/auth/login-options', { credentials: 'include' }) if (!res.ok) throw new Error('Failed to load sign-in options') const data = (await res.json()) as LoginOptionsResponse loginOptions.value = data diff --git a/client/src/features/auth/composables/useOidc.ts b/client/src/features/auth/composables/useOidc.ts index 49ecab0e8..ac8076414 100644 --- a/client/src/features/auth/composables/useOidc.ts +++ b/client/src/features/auth/composables/useOidc.ts @@ -1,4 +1,5 @@ import type { OidcCallbackResponse, OidcProviderPublic } from '@bookorbit/types' +import { fetchWithAuthProxyRecovery } from '@/lib/api' export async function generatePkce(): Promise<{ codeVerifier: string; codeChallenge: string }> { const array = new Uint8Array(32) @@ -52,7 +53,7 @@ export function useOidc() { sessionStorage.setItem('oidc_redirect', redirectTarget) } - const stateRes = await fetch(`/api/v1/auth/oidc/${provider.slug}/state`, { method: 'POST', credentials: 'include' }) + const stateRes = await fetchWithAuthProxyRecovery(`/api/v1/auth/oidc/${provider.slug}/state`, { method: 'POST', credentials: 'include' }) if (!stateRes.ok) throw new OidcLoginError(undefined, 'Failed to generate state') const { state, authorizationEndpoint } = (await stateRes.json()) as { state: string; authorizationEndpoint: string } @@ -87,7 +88,7 @@ export function useOidc() { const redirectUri = `${window.location.origin}/oauth2-callback` - const res = await fetch('/api/v1/auth/oidc/callback', { + const res = await fetchWithAuthProxyRecovery('/api/v1/auth/oidc/callback', { method: 'POST', headers: { 'Content-Type': 'application/json' }, credentials: 'include', diff --git a/client/src/features/auth/composables/useSetupStatus.ts b/client/src/features/auth/composables/useSetupStatus.ts index 350c29f43..29edaeb69 100644 --- a/client/src/features/auth/composables/useSetupStatus.ts +++ b/client/src/features/auth/composables/useSetupStatus.ts @@ -1,4 +1,5 @@ import { ref } from 'vue' +import { fetchWithAuthProxyRecovery } from '@/lib/api' const needsSetup = ref(null) const allowRegistration = ref(false) @@ -12,7 +13,7 @@ export function useSetupStatus() { inFlight = (async () => { try { - const res = await fetch('/api/v1/auth/setup-status', { credentials: 'include' }) + const res = await fetchWithAuthProxyRecovery('/api/v1/auth/setup-status', { credentials: 'include' }) if (!res.ok) throw new Error('Failed to load setup status') const data = (await res.json()) as { needsSetup?: boolean; allowRegistration?: boolean } setupStatusError.value = null diff --git a/client/src/lib/__tests__/api.spec.ts b/client/src/lib/__tests__/api.spec.ts index bab9fe046..49f04db91 100644 --- a/client/src/lib/__tests__/api.spec.ts +++ b/client/src/lib/__tests__/api.spec.ts @@ -1,6 +1,15 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { api, getValidToken, onAuthRecovered, refreshAccessToken, setAccessToken, setOnAuthFailure } from '@/lib/api' +import { + api, + fetchWithAuthProxyRecovery, + getValidToken, + NetworkError, + onAuthRecovered, + refreshAccessToken, + setAccessToken, + setOnAuthFailure, +} from '@/lib/api' /** A token shaped like a real JWT, so the client can read `exp` out of it. Never verified here. */ function signedToken(expiresInSeconds: number): string { @@ -259,4 +268,106 @@ describe('api wrapper', () => { expect(recovered).not.toHaveBeenCalled() }) }) + + describe('edge auth proxy redirects', () => { + /** What `fetch` hands back for a 3xx when the request used `redirect: 'manual'`. */ + function opaqueRedirect(): Response { + const res = new Response(null) + Object.defineProperty(res, 'type', { value: 'opaqueredirect' }) + return res + } + + let reload: ReturnType void>> + + beforeEach(() => { + sessionStorage.clear() + reload = vi.fn<() => void>() + vi.stubGlobal('location', { ...window.location, reload }) + }) + + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('asks fetch not to follow redirects, so a proxy redirect is observable', async () => { + const fetchMock = vi.fn(() => Promise.resolve(new Response('{}', { status: 200 }))) + globalThis.fetch = fetchMock as never + + await api('/api/v1/books/1') + + expect(fetchMock.mock.calls[0]![1]?.redirect).toBe('manual') + }) + + it('keeps redirects manual even when a caller asks to follow them', async () => { + const fetchMock = vi.fn(() => Promise.resolve(new Response('{}', { status: 200 }))) + globalThis.fetch = fetchMock as never + + await api('/api/v1/books/1', { redirect: 'follow' }) + + expect(fetchMock.mock.calls[0]![1]?.redirect).toBe('manual') + }) + + it('reloads for a direct sign-in request redirected by the proxy', async () => { + const fetchMock = vi.fn(() => Promise.resolve(opaqueRedirect())) + globalThis.fetch = fetchMock as never + + const settled = vi.fn<() => void>() + void fetchWithAuthProxyRecovery('/api/v1/auth/login', { method: 'POST', redirect: 'follow' }).then(settled, settled) + + await vi.waitFor(() => expect(reload).toHaveBeenCalledTimes(1)) + expect(fetchMock.mock.calls[0]![1]?.redirect).toBe('manual') + expect(settled).not.toHaveBeenCalled() + }) + + it('does not reload when storage is unavailable, since a loop could not be detected', async () => { + globalThis.fetch = vi.fn(() => Promise.resolve(opaqueRedirect())) as never + vi.spyOn(Storage.prototype, 'getItem').mockImplementation(() => { + throw new Error('SecurityError') + }) + + await expect(api('/api/v1/books/1')).rejects.toBeInstanceOf(NetworkError) + expect(reload).not.toHaveBeenCalled() + }) + + it('reloads the page instead of treating a proxy redirect as a logged-out session', async () => { + globalThis.fetch = vi.fn(() => Promise.resolve(opaqueRedirect())) as never + const onAuthFailure = vi.fn<() => void>() + setOnAuthFailure(onAuthFailure) + + const settled = vi.fn<() => void>() + void api('/api/v1/books/1').then(settled, settled) + await vi.waitFor(() => expect(reload).toHaveBeenCalledTimes(1)) + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(onAuthFailure).not.toHaveBeenCalled() + expect(settled).not.toHaveBeenCalled() + }) + + it('reloads when the session refresh itself is redirected by the proxy', async () => { + globalThis.fetch = vi.fn(() => Promise.resolve(opaqueRedirect())) as never + + void refreshAccessToken().catch(() => null) + + await vi.waitFor(() => expect(reload).toHaveBeenCalledTimes(1)) + }) + + it('gives up with an error instead of looping when the reload is redirected again right away', async () => { + globalThis.fetch = vi.fn(() => Promise.resolve(opaqueRedirect())) as never + + void api('/api/v1/books/1') + await vi.waitFor(() => expect(reload).toHaveBeenCalledTimes(1)) + + await expect(api('/api/v1/books/1')).rejects.toBeInstanceOf(NetworkError) + expect(reload).toHaveBeenCalledTimes(1) + }) + + it('reloads again once the loop window has passed', async () => { + globalThis.fetch = vi.fn(() => Promise.resolve(opaqueRedirect())) as never + sessionStorage.setItem('bookorbit:auth-proxy-reload-at', String(Date.now() - 60_000)) + + void api('/api/v1/books/1') + + await vi.waitFor(() => expect(reload).toHaveBeenCalledTimes(1)) + }) + }) }) diff --git a/client/src/lib/api.ts b/client/src/lib/api.ts index f7e7b429d..7549659f6 100644 --- a/client/src/lib/api.ts +++ b/client/src/lib/api.ts @@ -10,6 +10,9 @@ const EXPIRY_SKEW_MS = 30_000 /** After a failed proactive refresh, stop trying for this long. Reactive 401 refreshes ignore it. */ const REFRESH_COOLDOWN_MS = 5_000 +const AUTH_PROXY_RELOAD_KEY = 'bookorbit:auth-proxy-reload-at' +const AUTH_PROXY_RELOAD_WINDOW_MS = 15_000 + let _accessToken: string | null = null let _accessTokenExpiresAt: number | null = null let _onAuthFailure: (() => void) | null = null @@ -110,11 +113,51 @@ export class NetworkError extends Error { } } +/** + * True when a same-origin request came back redirected elsewhere. With `redirect: 'manual'` this is + * how an edge auth proxy (Cloudflare Access, an OAuth2 gateway) shows up once its session cookie has + * expired: it can't be told apart from a logged-out app any other way, since `fetch` either follows the + * redirect into a CORS failure or throws before the caller ever sees a status code. + */ +export function isAuthProxyRedirect(res: Response): boolean { + return res.type === 'opaqueredirect' +} + +/** + * A fetch can't complete a proxy's redirect to its login page itself. A full navigation can, so that's + * the only way through. Normally returns a promise that never settles: the reload is about to tear down this + * page, and letting the caller's `.then`/`.catch` run first (e.g. routing to the app's own login screen) + * would just flash the wrong UI for a moment. + */ +export function reloadForAuthProxy(): Promise { + // If the reload is answered from the service worker cache again (e.g. the network failed), a second + // reload would loop forever. Give up and let the caller surface an error instead. Without storage + // the loop can't be detected, so don't risk the reload at all. + const now = Date.now() + let lastReload: number + try { + lastReload = Number(sessionStorage.getItem(AUTH_PROXY_RELOAD_KEY)) || 0 + sessionStorage.setItem(AUTH_PROXY_RELOAD_KEY, String(now)) + } catch { + return Promise.reject(new NetworkError('auth proxy redirect')) + } + if (now - lastReload < AUTH_PROXY_RELOAD_WINDOW_MS) { + return Promise.reject(new NetworkError('auth proxy redirect')) + } + window.location.reload() + return new Promise(() => {}) +} + +export async function fetchWithAuthProxyRecovery(input: RequestInfo | URL, init?: RequestInit): Promise { + const res = await fetch(input, { ...init, redirect: 'manual' }) + return isAuthProxyRedirect(res) ? reloadForAuthProxy() : res +} + async function rawFetch(input: RequestInfo | URL, init?: RequestInit): Promise { const headers = new Headers(init?.headers) if (_accessToken) headers.set('Authorization', `Bearer ${_accessToken}`) try { - return await fetch(input, { ...init, headers, credentials: 'include' }) + return await fetchWithAuthProxyRecovery(input, { ...init, headers, credentials: 'include' }) } catch (reason) { if (reason instanceof TypeError) throw new NetworkError(reason.message) throw reason diff --git a/client/vite.config.ts b/client/vite.config.ts index 96d0f1727..937d5e02d 100644 --- a/client/vite.config.ts +++ b/client/vite.config.ts @@ -10,6 +10,7 @@ import { VitePWA } from 'vite-plugin-pwa' const apiAgent = new Agent({ keepAlive: true }) /** Lets a second dev client point at a throwaway API instance, so restart testing leaves the main stack alone. */ const apiTarget = process.env.BOOKORBIT_API_TARGET ?? 'http://localhost:6262' +const offlineShellUrl = '__bookorbit_offline_shell' // https://vite.dev/config/ export default defineConfig({ @@ -90,9 +91,38 @@ export default defineConfig({ workbox: { globPatterns: ['**/*.{js,css,html,ico,png,svg,woff,woff2}'], globIgnores: ['**/assets/foliate/**'], - navigateFallback: 'index.html', - navigateFallbackDenylist: [/^\/api\//], + // Keep the offline shell at a separate path. Workbox maps '/' and its query variants to + // precached index.html before the NetworkFirst route can contact the auth proxy. + manifestTransforms: [ + (entries) => ({ + manifest: entries.map((entry) => (entry.url === 'index.html' ? { ...entry, url: offlineShellUrl } : entry)), + }), + ], + navigateFallback: null, runtimeCaching: [ + { + // Page loads must hit the network first: an edge auth proxy (e.g. Cloudflare Access) needs to + // see every navigation to redirect an expired session to its login page. Serving the precached + // shell unconditionally (the old `navigateFallback` behavior) hid the request from the proxy + // entirely and left the app stuck logged out with no way back in short of clearing site data. + urlPattern: ({ request, url }) => request.mode === 'navigate' && !url.pathname.startsWith('/api/'), + handler: 'NetworkFirst', + options: { + cacheName: 'app-shell', + // No networkTimeoutSeconds: a slow proxy redirect must win over the cached page. + expiration: { + maxEntries: 50, + maxAgeSeconds: 60 * 60 * 24 * 7, + }, + precacheFallback: { + fallbackURL: offlineShellUrl, + }, + // Status 0 here would include an auth proxy's opaqueredirect and cache it as the page. + cacheableResponse: { + statuses: [200], + }, + }, + }, { urlPattern: /^.*\/api\/v1\/books\/\d+\/cover\?(?:[^#]*&)?t=[^#]*$/, handler: 'CacheFirst',