diff --git a/apps/api/src/index.ts b/apps/api/src/index.ts index 1934124..3d101d1 100644 --- a/apps/api/src/index.ts +++ b/apps/api/src/index.ts @@ -46,6 +46,16 @@ if (!devBypassActive) { } // Protected API routes (behind oidcAuthMiddleware) + +// GET /api/login — login entry point for the PWA. +// Flow (production): unauthenticated top-level nav hits the OIDC guard above, +// which 302-redirects to Authelia. After login, Authelia POSTs to /callback, +// the middleware sets a `continue` cookie pointing back to /api/login, and the +// browser follows it here — now authenticated. The handler then redirects to / +// so the SPA boots with a valid session. Under DEV_AUTH_BYPASS the guard is not +// mounted, so /api/login reaches this handler directly and still redirects to /. +app.get('/api/login', (c) => c.redirect('/')) + app.route('/api/me', meRouter) app.route('/api/events', eventsRouter) app.route('/api/sse', sseRouter) diff --git a/apps/api/tests/routes/login.test.ts b/apps/api/tests/routes/login.test.ts new file mode 100644 index 0000000..8b5ea4f --- /dev/null +++ b/apps/api/tests/routes/login.test.ts @@ -0,0 +1,109 @@ +/** + * GET /api/login — tests for the top-level-nav login entry point. + * + * Covers: + * 1. DEV_AUTH_BYPASS=true: GET /api/login returns 302 → '/'. + * Guard is not mounted; handler fires directly. + * 2. Without DEV_AUTH_BYPASS: oidcAuthMiddleware passthrough mock lets the + * request through; handler returns 302 → '/' (mirroring me.test.ts OIDC path). + * + * Flow (production): + * Unauthenticated top-level nav → guard 302 → Authelia → /callback → `continue` + * cookie returns browser to /api/login (now authenticated) → handler 302 → /. + * Under DEV_AUTH_BYPASS, guard is not mounted, so /api/login reaches the handler + * directly and redirects to '/'. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' + +// --------------------------------------------------------------------------- +// Shared mock: DB — avoids real DB connections across all tests in this file. +// Hoisted so it applies to every dynamic import below. +// --------------------------------------------------------------------------- +vi.mock('../../src/db/client.js', () => ({ + db: { + execute: vi.fn().mockResolvedValue([[{ '1': 1 }]]), + select: vi.fn().mockReturnValue({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockResolvedValue([]), + innerJoin: vi.fn().mockReturnValue({ + innerJoin: vi.fn().mockReturnValue({ + where: vi.fn().mockResolvedValue([]), + }), + }), + }), + }), + }, +})) + +// --------------------------------------------------------------------------- +// Track whether oidcAuthMiddleware was registered on the app. +// --------------------------------------------------------------------------- +const oidcMiddlewareSpy = vi.fn( + () => async (_c: unknown, next: () => Promise) => next(), +) + +vi.mock('@hono/oidc-auth', () => ({ + oidcAuthMiddleware: () => oidcMiddlewareSpy(), + processOAuthCallback: () => async (c: { json: (v: unknown) => unknown }) => + c.json({ ok: true }), + getAuth: vi.fn().mockResolvedValue(null), +})) + +// --------------------------------------------------------------------------- +// Env snapshot — restored after each test. +// --------------------------------------------------------------------------- +const originalNodeEnv = process.env.NODE_ENV +const originalBypassFlag = process.env.DEV_AUTH_BYPASS + +afterEach(() => { + process.env.NODE_ENV = originalNodeEnv + if (originalBypassFlag === undefined) { + delete process.env.DEV_AUTH_BYPASS + } else { + process.env.DEV_AUTH_BYPASS = originalBypassFlag + } + vi.resetModules() + oidcMiddlewareSpy.mockClear() +}) + +// --------------------------------------------------------------------------- + +describe('GET /api/login — dev-auth bypass (DEV_AUTH_BYPASS=true)', () => { + beforeEach(() => { + process.env.NODE_ENV = 'test' + process.env.DEV_AUTH_BYPASS = 'true' + }) + + it('returns 302 with location "/" (bypass active, guard not mounted)', async () => { + const { app } = await import('../../src/index.js') + + const res = await app.request('/api/login') + expect(res.status).toBe(302) + expect(res.headers.get('location')).toBe('/') + }) + + it('does not invoke oidcAuthMiddleware when bypass is active', async () => { + const { app } = await import('../../src/index.js') + + await app.request('/api/login') + + expect(oidcMiddlewareSpy).not.toHaveBeenCalled() + }) +}) + +describe('GET /api/login — OIDC path (no DEV_AUTH_BYPASS)', () => { + beforeEach(() => { + process.env.NODE_ENV = 'test' + delete process.env.DEV_AUTH_BYPASS + }) + + it('returns 302 with location "/" when OIDC passthrough allows the request', async () => { + const { app } = await import('../../src/index.js') + + // oidcAuthMiddleware is mocked as a passthrough — request reaches the handler. + const res = await app.request('/api/login') + expect(res.status).toBe(302) + expect(res.headers.get('location')).toBe('/') + }) +}) diff --git a/apps/pwa/src/api/client.ts b/apps/pwa/src/api/client.ts index ea2d8ca..cc7d73a 100644 --- a/apps/pwa/src/api/client.ts +++ b/apps/pwa/src/api/client.ts @@ -5,8 +5,12 @@ * every cross-origin request (Vite dev proxy routes to :3000; production is * same-origin via Pangolin). * - * 401 responses mean the session has expired — the browser will follow the - * 302 redirect to Authelia on the next API call automatically (full-page nav). + * Auth note: the OIDC guard's 302 to Authelia is CORS-blocked for fetch/XHR — + * browsers do not follow cross-origin redirects from XHR to an external IdP. + * Re-authentication therefore requires a TOP-LEVEL navigation to /api/login + * (see apps/pwa/src/lib/loginRedirect.ts). fetchMe and other fetch calls here + * are pure data fetches; they throw on non-ok responses and leave the redirect + * decision to the caller (CalendarShell via maybeRedirectToLogin). */ // ── /api/me ──────────────────────────────────────────────────────────────── @@ -27,8 +31,9 @@ export async function fetchMe(): Promise { }) if (!res.ok) { - // 302 → browser follows redirect to Authelia automatically. - // For 4xx/5xx, throw so React Query can surface the error. + // Throw on any non-ok response. The OIDC guard's 302 to Authelia cannot be + // followed by fetch (CORS-blocked for XHR). Callers that handle auth errors + // should use maybeRedirectToLogin() for a top-level navigation to /api/login. throw new Error(`GET /api/me failed: ${res.status}`) } diff --git a/apps/pwa/src/components/CalendarShell.tsx b/apps/pwa/src/components/CalendarShell.tsx index 009f324..3f4ef37 100644 --- a/apps/pwa/src/components/CalendarShell.tsx +++ b/apps/pwa/src/components/CalendarShell.tsx @@ -43,6 +43,7 @@ import { Plus } from 'lucide-react' import { fetchMe, fetchEvents } from '../api/client.js' import { hydrateEvents } from '../lib/hydrateEvents.js' +import { maybeRedirectToLogin, clearLoginRedirect } from '../lib/loginRedirect.js' import { buildCalendarConfig, SX_FIRST_DAY_OF_WEEK } from '../lib/calendarConfig.js' import { useCalendarStore } from '../store/calendarStore.js' import { EventDetailPopover } from './EventDetailPopover.js' @@ -174,6 +175,26 @@ export function CalendarShell() { eventsService.set(sxEvents as Parameters[0]) }, [eventsQuery.data, eventsService]) + // Auth redirect — one-shot full-page nav to /api/login when /api/me fails. + // If this is the first failure, maybeRedirectToLogin() sets a sessionStorage + // flag and navigates the browser to /api/login (top-level nav, no CORS block). + // The page will unmount as the browser navigates. If the flag is already set + // (already bounced through login once and still failing), returns false and the + // existing "Sign-in required" branch below renders. + useEffect(() => { + if (meQuery.isError) { + maybeRedirectToLogin() + } + }, [meQuery.isError]) + + // Clear the one-shot flag on a successful /api/me load so a later session + // expiry can trigger another redirect instead of showing "Sign-in required". + useEffect(() => { + if (meQuery.isSuccess) { + clearLoginRedirect() + } + }, [meQuery.isSuccess]) + // ── Render helpers ──────────────────────────────────────────────────────── // Determine which content to show in the calendar area diff --git a/apps/pwa/src/lib/loginRedirect.test.ts b/apps/pwa/src/lib/loginRedirect.test.ts new file mode 100644 index 0000000..2639c72 --- /dev/null +++ b/apps/pwa/src/lib/loginRedirect.test.ts @@ -0,0 +1,71 @@ +/** + * loginRedirect — one-shot sessionStorage-guarded redirect helper. + * + * Covers: + * - maybeRedirectToLogin() redirects to /api/login on first call and returns true + * - maybeRedirectToLogin() does NOT redirect on second call (one-shot guard) and returns false + * - clearLoginRedirect() removes the flag so a subsequent call redirects again + */ + +import { describe, it, expect, beforeEach, vi } from 'vitest' + +// Stub window.location with a writable href before importing the module. +// jsdom sets window.location to a read-only getter backed by a Location object; +// we need to replace it with a plain object so href assignment is detectable. +const locationStub = { href: '' } +Object.defineProperty(window, 'location', { + value: locationStub, + writable: true, +}) + +import { maybeRedirectToLogin, clearLoginRedirect } from './loginRedirect.js' + +describe('maybeRedirectToLogin', () => { + beforeEach(() => { + sessionStorage.clear() + locationStub.href = '' + vi.clearAllMocks() + }) + + it('sets window.location.href to /api/login and returns true on first call', () => { + const result = maybeRedirectToLogin() + expect(result).toBe(true) + expect(locationStub.href).toBe('/api/login') + }) + + it('sets the sessionStorage flag after first call', () => { + maybeRedirectToLogin() + expect(sessionStorage.getItem('familysync.loginRedirectAttempted')).toBe('1') + }) + + it('does NOT change href on second call and returns false (one-shot guard)', () => { + maybeRedirectToLogin() // first call — redirects + locationStub.href = '' // reset the stub to detect a second assignment + const result = maybeRedirectToLogin() // second call — should NOT redirect + expect(result).toBe(false) + expect(locationStub.href).toBe('') // href must NOT have been reassigned + }) +}) + +describe('clearLoginRedirect', () => { + beforeEach(() => { + sessionStorage.clear() + locationStub.href = '' + vi.clearAllMocks() + }) + + it('removes the sessionStorage flag', () => { + sessionStorage.setItem('familysync.loginRedirectAttempted', '1') + clearLoginRedirect() + expect(sessionStorage.getItem('familysync.loginRedirectAttempted')).toBeNull() + }) + + it('allows maybeRedirectToLogin to redirect again after the flag is cleared', () => { + maybeRedirectToLogin() // first call — sets flag + clearLoginRedirect() // clear the flag + locationStub.href = '' // reset stub + const result = maybeRedirectToLogin() // should redirect again + expect(result).toBe(true) + expect(locationStub.href).toBe('/api/login') + }) +}) diff --git a/apps/pwa/src/lib/loginRedirect.ts b/apps/pwa/src/lib/loginRedirect.ts new file mode 100644 index 0000000..aec41f4 --- /dev/null +++ b/apps/pwa/src/lib/loginRedirect.ts @@ -0,0 +1,64 @@ +/** + * One-shot login-redirect helper. + * + * Problem: the OIDC guard issues a 302 to Authelia, but fetch()/XHR cannot follow a + * cross-origin 302 to Authelia — the browser enforces CORS on XHR redirects, so the + * response is opaque and the SPA never reaches Authelia. Re-auth requires a TOP-LEVEL + * browser navigation to /api/login, where the guard's redirect is followed at the + * document level (no CORS restriction). + * + * Solution: when /api/me fails (indicating an unauthenticated session), perform a + * full-page navigation to /api/login. A sessionStorage flag prevents infinite loops: + * if the navigation completes and /api/me still fails (genuine error), the second + * maybeRedirectToLogin() call is a no-op and the "Sign-in required" UI is shown. + */ + +export const LOGIN_REDIRECT_KEY = 'familysync.loginRedirectAttempted' + +/** + * Navigate to /api/login if this is the first attempt. + * + * Returns true when a redirect was triggered (caller can bail early — the page + * is navigating away). Returns false when the flag was already set (already + * bounced through login once; let the caller render the error UI instead). + * + * Safe under SSR/test environments: no-ops and returns false when window or + * sessionStorage is unavailable. + */ +export function maybeRedirectToLogin(): boolean { + if (typeof window === 'undefined') return false + if (typeof sessionStorage === 'undefined') return false + + try { + if (sessionStorage.getItem(LOGIN_REDIRECT_KEY) !== null) { + // Already attempted — do not redirect again (loop guard). + return false + } + sessionStorage.setItem(LOGIN_REDIRECT_KEY, '1') + window.location.href = '/api/login' + return true + } catch { + // sessionStorage access can throw in private-browsing mode or with storage quota + // exceeded. Fail open: do not redirect, let the caller render the error. + return false + } +} + +/** + * Clear the one-shot flag. + * + * Call on a successful /api/me load so that a later session expiry can trigger + * another redirect instead of silently showing "Sign-in required". + * + * Safe under SSR/test environments (guarded). + */ +export function clearLoginRedirect(): void { + if (typeof window === 'undefined') return + if (typeof sessionStorage === 'undefined') return + + try { + sessionStorage.removeItem(LOGIN_REDIRECT_KEY) + } catch { + // Ignore storage errors — clearing the flag is best-effort. + } +}