diff --git a/e2e/auth/session-expiry.spec.ts b/e2e/auth/session-expiry.spec.ts new file mode 100644 index 00000000..6fdd8c65 --- /dev/null +++ b/e2e/auth/session-expiry.spec.ts @@ -0,0 +1,90 @@ +import { test, expect, type Page } from '@playwright/test'; +import { waitForHydration } from '../support/helpers.js'; + +/** Issue a same-origin fetch from the page and wait for the session check it triggers. */ +async function fetchAndAwaitSessionCheck(page: Page, url: string) { + const sessionCheck = page.waitForResponse((res) => res.url().includes('/api/auth/get-session')); + const status = await page.evaluate(async (u) => (await fetch(u)).status, url); + await sessionCheck; + return status; +} + +test.describe('Session expiry', () => { + test.use({ locale: 'en-US' }); + + test.describe('without a session', () => { + test.use({ storageState: { cookies: [], origins: [] } }); + + test('API routes return a JSON 401', async ({ request }) => { + const res = await request.get('/api/trino/catalog?level=catalogs', { maxRedirects: 0 }); + expect(res.status()).toBe(401); + expect(await res.json()).toEqual({ message: 'Authentication required' }); + }); + }); + + test('shows a blocking modal when an API call fails after the session is gone', async ({ + page + }) => { + await page.goto('/trino?tab=1'); + await waitForHydration(page); + + const dialog = page.getByRole('dialog', { name: 'Session expired' }); + await expect(dialog).toBeHidden(); + + await page.context().clearCookies(); + const status = await fetchAndAwaitSessionCheck(page, '/api/trino/catalog?level=catalogs'); + expect(status).toBe(401); + + await expect(dialog).toBeVisible(); + await expect(dialog.getByText('Your session has expired.')).toBeVisible(); + + const signIn = dialog.getByRole('link', { name: 'Sign in again' }); + + // The modal cannot be dismissed. Press Escape twice: browsers may skip the + // cancellable `cancel` event on a repeated Escape and close the dialog. + await page.keyboard.press('Escape'); + await page.keyboard.press('Escape'); + await expect(dialog).toBeVisible(); + await expect(signIn).toBeFocused(); + + await expect(signIn).toHaveAttribute( + 'href', + '/auth/login?redirectTo=' + encodeURIComponent('/trino?tab=1') + ); + + // Signing in again returns the user to where they were. This relies on the + // mock OIDC provider signing in without a login form. + await signIn.click(); + await expect(page).toHaveURL(/\/auth\/login/); + await waitForHydration(page); + await page.getByRole('button', { name: /sign in with sso/i }).click(); + await expect(page).toHaveURL('/trino?tab=1'); + await expect(dialog).toBeHidden(); + }); + + test('shows the modal when the user returns to the tab after the session is gone', async ({ + page + }) => { + await page.goto('/'); + await waitForHydration(page); + + await page.context().clearCookies(); + const sessionCheck = page.waitForResponse((res) => res.url().includes('/api/auth/get-session')); + await page.evaluate(() => document.dispatchEvent(new Event('visibilitychange'))); + await sessionCheck; + + await expect(page.getByRole('dialog', { name: 'Session expired' })).toBeVisible(); + }); + + test('does not show the modal for a 401 while the session is valid', async ({ page }) => { + await page.goto('/'); + await waitForHydration(page); + + // The storage API answers 401 when no storage connection header is sent + // (requires the storage browser to be enabled, as in .env.test). + const status = await fetchAndAwaitSessionCheck(page, '/api/storage/buckets'); + expect(status).toBe(401); + + await expect(page.getByRole('dialog', { name: 'Session expired' })).toBeHidden(); + }); +}); diff --git a/messages/de.json b/messages/de.json index 8431a757..54b1d0ee 100644 --- a/messages/de.json +++ b/messages/de.json @@ -44,6 +44,9 @@ "auth_login_subtitle": "Verwenden Sie den SSO-Anbieter Ihrer Organisation, um auf die Plattform zuzugreifen.", "auth_login_button": "Mit SSO anmelden", "auth_login_error": "Anmeldung fehlgeschlagen. Bitte versuchen Sie es erneut.", + "session_expired_title": "Sitzung abgelaufen", + "session_expired_message": "Ihre Sitzung ist abgelaufen. Melden Sie sich erneut an, um fortzufahren.", + "session_expired_sign_in": "Erneut anmelden", "header_sign_out": "Abmelden", "trino_results_empty": "Keine Ergebnisse", "trino_query_error": "Abfragefehler", diff --git a/messages/en.json b/messages/en.json index 92a586a0..3cd59ee7 100644 --- a/messages/en.json +++ b/messages/en.json @@ -44,6 +44,9 @@ "auth_login_subtitle": "Use your organisation's SSO provider to access the platform.", "auth_login_button": "Sign in with SSO", "auth_login_error": "Sign-in failed. Please try again.", + "session_expired_title": "Session expired", + "session_expired_message": "Your session has expired. Sign in again to continue.", + "session_expired_sign_in": "Sign in again", "header_sign_out": "Sign out", "trino_results_empty": "No results", "trino_query_error": "Query error", diff --git a/src/hooks.client.ts b/src/hooks.client.ts new file mode 100644 index 00000000..4de7dab2 --- /dev/null +++ b/src/hooks.client.ts @@ -0,0 +1,28 @@ +import type { ClientInit } from '@sveltejs/kit'; +import { checkSession } from '$lib/client/session.svelte'; + +/** + * Wrap `window.fetch` so that any same-origin 401 triggers a session check. + * A 401 does not always mean the session has expired (e.g. the storage API + * uses it for a missing connection), so the response itself is passed through + * unchanged and `checkSession` decides whether the session is really gone. + */ +export const init: ClientInit = () => { + const originalFetch = window.fetch; + window.fetch = async (input, requestInit) => { + const response = await originalFetch(input, requestInit); + if (response.status === 401 && isSameOrigin(response.url)) { + void checkSession(); + } + return response; + }; +}; + +function isSameOrigin(url: string): boolean { + if (!url) return false; + try { + return new URL(url).origin === window.location.origin; + } catch { + return false; + } +} diff --git a/src/hooks.server.ts b/src/hooks.server.ts index 0cd63943..a6cc19e6 100644 --- a/src/hooks.server.ts +++ b/src/hooks.server.ts @@ -1,10 +1,11 @@ import { paraglideMiddleware } from '$lib/paraglide/server'; import { httpRequestDuration } from '$lib/server/metrics'; import { building, dev } from '$app/environment'; -import { error, redirect, type Handle, type HandleServerError } from '@sveltejs/kit'; +import { error, type Handle, type HandleServerError } from '@sveltejs/kit'; import { sequence } from '@sveltejs/kit/hooks'; import { svelteKitHandler } from 'better-auth/svelte-kit'; import { auth, oidcEnabled } from '$lib/server/auth'; +import { unauthenticatedResponse } from '$lib/server/auth-guard'; import { requestLogger, logger } from '$lib/server/logging'; import { getConnectionFromHeader } from '$lib/server/storage/connection.js'; import { storageBrowserEnabled } from '$lib/server/feature-flags.js'; @@ -53,8 +54,8 @@ const handleAuthGuard: Handle = async ({ event, resolve }) => { const isPublic = PUBLIC_PATHS.some((p) => event.url.pathname.startsWith(p)); if (!isPublic && !event.locals.user) { - const redirectTo = encodeURIComponent(event.url.pathname + event.url.search); - throw redirect(302, `/auth/login?redirectTo=${redirectTo}`); + event.locals.logger.debug({ path: event.url.pathname }, 'Rejecting unauthenticated request'); + return unauthenticatedResponse(event.url); } return resolve(event); diff --git a/src/lib/client/session.svelte.spec.ts b/src/lib/client/session.svelte.spec.ts new file mode 100644 index 00000000..3f894f4a --- /dev/null +++ b/src/lib/client/session.svelte.spec.ts @@ -0,0 +1,66 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { checkSession, sessionState } from './session.svelte.js'; + +const { getSession } = vi.hoisted(() => ({ getSession: vi.fn() })); +vi.mock('$lib/auth-client', () => ({ authClient: { getSession } })); + +describe('checkSession', () => { + beforeEach(() => { + getSession.mockReset(); + sessionState.active = true; + sessionState.expired = false; + }); + + it('marks the session expired when the server reports no session', async () => { + getSession.mockResolvedValue({ data: null, error: null }); + + await checkSession(); + + expect(sessionState.expired).toBe(true); + expect(getSession).toHaveBeenCalledWith({ query: { disableRefresh: true } }); + }); + + it('keeps the session when the server returns one', async () => { + getSession.mockResolvedValue({ data: { user: {}, session: {} }, error: null }); + + await checkSession(); + + expect(sessionState.expired).toBe(false); + }); + + it('clears the flag once the session is restored', async () => { + sessionState.expired = true; + getSession.mockResolvedValue({ data: { user: {}, session: {} }, error: null }); + + await checkSession(); + + expect(sessionState.expired).toBe(false); + }); + + it('ignores server and network errors', async () => { + getSession.mockResolvedValueOnce({ data: null, error: { status: 500 } }); + getSession.mockRejectedValueOnce(new TypeError('Failed to fetch')); + + await checkSession(); + await checkSession(); + + expect(sessionState.expired).toBe(false); + }); + + it('shares one request between concurrent checks', async () => { + getSession.mockResolvedValue({ data: { user: {}, session: {} }, error: null }); + + await Promise.all([checkSession(), checkSession(), checkSession()]); + + expect(getSession).toHaveBeenCalledTimes(1); + }); + + it('does nothing while inactive', async () => { + sessionState.active = false; + + await checkSession(); + + expect(getSession).not.toHaveBeenCalled(); + expect(sessionState.expired).toBe(false); + }); +}); diff --git a/src/lib/client/session.svelte.ts b/src/lib/client/session.svelte.ts new file mode 100644 index 00000000..02b21baf --- /dev/null +++ b/src/lib/client/session.svelte.ts @@ -0,0 +1,36 @@ +import { authClient } from '$lib/auth-client'; + +/** + * Client-side view of the user's session. `active` is set while a signed-in + * layout is mounted; without it (e.g. OIDC disabled) no checks are made. + */ +export const sessionState = $state({ active: false, expired: false }); + +let pending: Promise | null = null; + +/** + * Ask the server whether the current session is still valid and update + * `sessionState.expired`. Concurrent calls share one request. A session that + * is restored (e.g. by signing in again in another tab) clears the flag. + * + * Network and server errors are ignored: only an explicit "no session" answer + * marks the session as expired. + * + * Note: `disableRefresh` only prevents extending a database-backed session. + * In stateless mode (no database, as configured today) a check made shortly + * before the session cookie expires still renews it, like any other request. + */ +export function checkSession(): Promise { + if (!sessionState.active) return Promise.resolve(); + pending ??= authClient + .getSession({ query: { disableRefresh: true } }) + .then(({ data, error }) => { + // Drop the result if the signed-in layout unmounted in the meantime. + if (!error && sessionState.active) sessionState.expired = !data; + }) + .catch(() => {}) + .finally(() => { + pending = null; + }); + return pending; +} diff --git a/src/lib/components/Modal.svelte b/src/lib/components/Modal.svelte index 99558c69..6130a725 100644 --- a/src/lib/components/Modal.svelte +++ b/src/lib/components/Modal.svelte @@ -1,12 +1,21 @@ - + {#if open} {@render children()} {/if} diff --git a/src/lib/components/layout/SessionExpiredModal.svelte b/src/lib/components/layout/SessionExpiredModal.svelte new file mode 100644 index 00000000..1eec6651 --- /dev/null +++ b/src/lib/components/layout/SessionExpiredModal.svelte @@ -0,0 +1,54 @@ + + + + + diff --git a/src/lib/server/auth-guard.test.ts b/src/lib/server/auth-guard.test.ts new file mode 100644 index 00000000..4052be07 --- /dev/null +++ b/src/lib/server/auth-guard.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, it } from 'vitest'; +import { isRedirect } from '@sveltejs/kit'; +import { unauthenticatedResponse } from './auth-guard.js'; + +describe('unauthenticatedResponse', () => { + it('returns a JSON 401 for API routes', async () => { + const res = unauthenticatedResponse( + new URL('http://localhost/api/trino/catalog?level=catalogs') + ); + + expect(res.status).toBe(401); + expect(res.headers.get('content-type')).toContain('application/json'); + expect(await res.json()).toEqual({ message: 'Authentication required' }); + }); + + it('redirects page routes to the login page, preserving the target', () => { + try { + unauthenticatedResponse(new URL('http://localhost/trino?tab=1')); + expect.unreachable('expected a redirect to be thrown'); + } catch (err) { + expect(isRedirect(err)).toBe(true); + if (!isRedirect(err)) return; + expect(err.status).toBe(302); + expect(err.location).toBe('/auth/login?redirectTo=%2Ftrino%3Ftab%3D1'); + } + }); + + it('does not treat paths that merely start with "api" as API routes', () => { + expect(() => unauthenticatedResponse(new URL('http://localhost/apis'))).toThrow(); + }); +}); diff --git a/src/lib/server/auth-guard.ts b/src/lib/server/auth-guard.ts new file mode 100644 index 00000000..70d679e5 --- /dev/null +++ b/src/lib/server/auth-guard.ts @@ -0,0 +1,18 @@ +import { json, redirect } from '@sveltejs/kit'; + +/** + * Build the response for a request that requires authentication but has no + * valid session. + * + * API routes get a JSON 401 so that client-side `fetch` callers receive a + * parseable error. + * The client treats a 401 as a hint to re-check the session (see + * `src/hooks.client.ts`). All other routes redirect to the login page. + */ +export function unauthenticatedResponse(url: URL): Response { + if (url.pathname.startsWith('/api/')) { + return json({ message: 'Authentication required' }, { status: 401 }); + } + const redirectTo = encodeURIComponent(url.pathname + url.search); + throw redirect(302, `/auth/login?redirectTo=${redirectTo}`); +} diff --git a/src/routes/(app)/+layout.svelte b/src/routes/(app)/+layout.svelte index f538f0f6..944903b7 100644 --- a/src/routes/(app)/+layout.svelte +++ b/src/routes/(app)/+layout.svelte @@ -5,6 +5,7 @@ import Sidebar from '$lib/components/layout/sidebar/Sidebar.svelte'; import Header from '$lib/components/layout/header/Header.svelte'; import ToastHost from '$lib/components/ToastHost.svelte'; + import SessionExpiredModal from '$lib/components/layout/SessionExpiredModal.svelte'; let { children, data } = $props(); @@ -64,3 +65,8 @@ + + +{#if data.user} + +{/if}