From 491dd8327f3df890b5ee0ed02aa747d67784e4b5 Mon Sep 17 00:00:00 2001 From: Johannes Fleck Date: Thu, 8 Oct 2026 09:41:23 +0200 Subject: [PATCH 1/4] fix(auth): return a JSON 401 for unauthenticated API requests API routes redirected unauthenticated requests to the HTML login page, so client-side fetch callers failed with a JSON parse error once the session had expired. Page routes still redirect to the login page. Refs #356 asdf --- src/hooks.server.ts | 7 ++++--- src/lib/server/auth-guard.test.ts | 31 +++++++++++++++++++++++++++++++ src/lib/server/auth-guard.ts | 18 ++++++++++++++++++ 3 files changed, 53 insertions(+), 3 deletions(-) create mode 100644 src/lib/server/auth-guard.test.ts create mode 100644 src/lib/server/auth-guard.ts 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/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}`); +} From a26361843132681a59920b3992de8977e903af2a Mon Sep 17 00:00:00 2001 From: Johannes Fleck Date: Thu, 8 Oct 2026 09:41:31 +0200 Subject: [PATCH 2/4] feat(auth): check the session when an API request returns 401 Wrap window.fetch in the client init hook so that any same-origin 401 triggers a session check. A 401 alone is not proof of an expired session (the storage API uses it for a missing connection), so only an explicit "no session" answer from better-auth marks the session as expired. Refs #356 fix(auth): ignore session check results after the layout unmounts Refs #356 --- src/hooks.client.ts | 28 ++++++++++++ src/lib/client/session.svelte.spec.ts | 66 +++++++++++++++++++++++++++ src/lib/client/session.svelte.ts | 36 +++++++++++++++ 3 files changed, 130 insertions(+) create mode 100644 src/hooks.client.ts create mode 100644 src/lib/client/session.svelte.spec.ts create mode 100644 src/lib/client/session.svelte.ts 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/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; +} From f79a357ef13da08a4558794a2402dca07d13575f Mon Sep 17 00:00:00 2001 From: Johannes Fleck Date: Thu, 8 Oct 2026 09:41:31 +0200 Subject: [PATCH 3/4] feat(auth): block the app with a modal once the session has expired Show a non-dismissible modal with a link to sign in again, returning the user to the current page afterwards. The session is also checked when the tab becomes visible again, so the modal appears before the next API call. Modal gains a dismissible prop and passes through attributes such as aria-labelledby. Refs #356 --- messages/de.json | 3 ++ messages/en.json | 3 ++ src/lib/components/Modal.svelte | 34 ++++++++++-- .../layout/SessionExpiredModal.svelte | 54 +++++++++++++++++++ src/routes/(app)/+layout.svelte | 6 +++ 5 files changed, 96 insertions(+), 4 deletions(-) create mode 100644 src/lib/components/layout/SessionExpiredModal.svelte 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/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 @@ + + + +
+

{m.session_expired_title()}

+

{m.session_expired_message()}

+
+ + + + + +
+
+ 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} From 13cfaa7d7686be392c935e174a44f2665aabe110 Mon Sep 17 00:00:00 2001 From: Johannes Fleck Date: Thu, 8 Oct 2026 09:41:31 +0200 Subject: [PATCH 4/4] test(e2e): cover session expiry handling Closes #356 test(e2e): check the session modal survives a repeated Escape Refs #356 --- e2e/auth/session-expiry.spec.ts | 90 +++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 e2e/auth/session-expiry.spec.ts 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(); + }); +});