Merge branch 'fix/audit-followup-2026-07-13'

This commit is contained in:
fabi
2026-07-13 21:24:03 +02:00
6 changed files with 157 additions and 16 deletions

View File

@@ -0,0 +1,51 @@
/**
* Regression guard — privilege escalation via mixed auth storage.
*
* The admin JWT lives in sessionStorage; guest JWTs in localStorage; reads are
* sessionStorage-first. So a guest login MUST displace any resident admin token — otherwise
* a leftover admin session on a shared "event laptop" would shadow the guest's own token and
* silently hand the next person who joins full admin rights. This exercises the real UI flow.
*/
import { test, expect } from '../../fixtures/test';
import { JoinPage } from '../../page-objects';
function roleOf(page: import('@playwright/test').Page) {
return page.evaluate(() => {
const t = sessionStorage.getItem('eventsnap_jwt') ?? localStorage.getItem('eventsnap_jwt');
if (!t) return null;
try {
return JSON.parse(atob(t.split('.')[1])).role;
} catch {
return null;
}
});
}
test.describe('AuthZ — guest join displaces a resident admin session', () => {
test('joining as a guest with an admin token resident in sessionStorage yields GUEST rights', async ({
page,
api,
}) => {
// Simulate an admin who logged in on a shared device and walked away without closing the
// tab — their token sits in sessionStorage.
const adminJwt = await api.adminLogin();
await page.goto('/');
await page.evaluate((jwt) => sessionStorage.setItem('eventsnap_jwt', jwt), adminJwt);
// A different person now joins as a guest in that same tab.
const join = new JoinPage(page);
await join.goto();
await join.joinAs('DisplacementGuest');
await join.continueToFeed();
// The effective identity must be the GUEST — not the leftover admin.
expect(await roleOf(page)).toBe('guest');
// The admin token must have been cleared from sessionStorage by the guest login.
expect(await page.evaluate(() => sessionStorage.getItem('eventsnap_jwt'))).toBeNull();
// And the guest must NOT be able to reach the admin dashboard.
await page.goto('/admin');
await page.waitForURL(/admin\/login|join|feed/, { timeout: 5_000 });
expect(page.url()).not.toMatch(/\/admin(\/)?$/);
});
});

View File

@@ -5,7 +5,7 @@ import { describe, it, expect, beforeEach, vi } from 'vitest';
// jsdom localStorage is actually used (the shared mock stubs it to false). // jsdom localStorage is actually used (the shared mock stubs it to false).
vi.mock('$app/environment', () => ({ browser: true, dev: false, building: false, version: 'test' })); vi.mock('$app/environment', () => ({ browser: true, dev: false, building: false, version: 'test' }));
import { setAuth, getToken, getPin, getExpiry, getRole, clearAuth, clearPin } from './auth'; import { setAuth, setAdminAuth, getToken, getPin, getExpiry, getRole, getUserId, clearAuth, clearPin } from './auth';
/** Build a JWT-shaped string (header.payload.sig) with the given claims. */ /** Build a JWT-shaped string (header.payload.sig) with the given claims. */
function makeJwt(claims: object): string { function makeJwt(claims: object): string {
@@ -13,7 +13,10 @@ function makeJwt(claims: object): string {
return `${seg({ alg: 'HS256', typ: 'JWT' })}.${seg(claims)}.sig`; return `${seg({ alg: 'HS256', typ: 'JWT' })}.${seg(claims)}.sig`;
} }
beforeEach(() => localStorage.clear()); beforeEach(() => {
localStorage.clear();
sessionStorage.clear();
});
describe('auth — token storage', () => { describe('auth — token storage', () => {
it('setAuth then getToken/getPin round-trips', () => { it('setAuth then getToken/getPin round-trips', () => {
@@ -42,6 +45,37 @@ describe('auth — token storage', () => {
}); });
}); });
describe('auth — admin (sessionStorage) vs guest (localStorage) identity displacement', () => {
// Regression guard for a privilege-escalation: reads are sessionStorage-first, so a guest
// login MUST displace any resident admin token (else the guest silently runs as admin on a
// shared device), and symmetrically an admin login must displace a resident guest token.
it('a guest login displaces a resident admin session (no privilege escalation)', () => {
setAdminAuth(makeJwt({ role: 'admin' }), 'admin-uid', 'Admin');
expect(getRole()).toBe('admin');
setAuth(makeJwt({ role: 'guest' }), '1234', 'guest-uid', 'Guest');
expect(getRole()).toBe('guest'); // NOT admin
expect(getUserId()).toBe('guest-uid');
expect(sessionStorage.getItem('eventsnap_jwt')).toBeNull(); // admin token cleared
});
it('an admin login displaces a resident guest session but keeps the guest PIN', () => {
setAuth(makeJwt({ role: 'guest' }), '1234', 'guest-uid', 'Guest');
setAdminAuth(makeJwt({ role: 'admin' }), 'admin-uid', 'Admin');
expect(getRole()).toBe('admin');
expect(getUserId()).toBe('admin-uid');
expect(localStorage.getItem('eventsnap_jwt')).toBeNull(); // guest token cleared
expect(getPin()).toBe('1234'); // PIN preserved for later recovery
});
it('clearAuth wipes both stores', () => {
setAdminAuth(makeJwt({ role: 'admin' }), 'admin-uid', 'Admin');
clearAuth();
expect(getToken()).toBeNull();
expect(sessionStorage.getItem('eventsnap_jwt')).toBeNull();
});
});
describe('auth — JWT claim decode', () => { describe('auth — JWT claim decode', () => {
it('getExpiry decodes the exp claim (seconds → ms Date)', () => { it('getExpiry decodes the exp claim (seconds → ms Date)', () => {
const exp = 2_000_000_000; // far-future unix seconds const exp = 2_000_000_000; // far-future unix seconds

View File

@@ -67,6 +67,12 @@ export function getExpiry(): Date | null {
export function setAuth(jwt: string, pin: string | null, userId: string, displayName?: string): void { export function setAuth(jwt: string, pin: string | null, userId: string, displayName?: string): void {
if (!browser) return; if (!browser) return;
// DISPLACE any resident admin session: reads are sessionStorage-first, so a leftover
// admin token there would otherwise shadow this guest login and hand the guest admin
// rights on a shared device. Clear the sessionStorage identity so exactly one is resident.
sessionStorage.removeItem(TOKEN_KEY);
sessionStorage.removeItem(USER_ID_KEY);
sessionStorage.removeItem(DISPLAY_NAME_KEY);
localStorage.setItem(TOKEN_KEY, jwt); localStorage.setItem(TOKEN_KEY, jwt);
if (pin) { if (pin) {
localStorage.setItem(PIN_KEY, pin); localStorage.setItem(PIN_KEY, pin);
@@ -85,6 +91,12 @@ export function setAuth(jwt: string, pin: string | null, userId: string, display
*/ */
export function setAdminAuth(jwt: string, userId: string, displayName?: string): void { export function setAdminAuth(jwt: string, userId: string, displayName?: string): void {
if (!browser) return; if (!browser) return;
// DISPLACE any resident guest session so exactly one identity is resident (symmetric with
// setAuth). Keep the guest PIN — it's deliberately preserved for later recovery, and the
// admin has none. Reads are sessionStorage-first, so the admin token now wins cleanly.
localStorage.removeItem(TOKEN_KEY);
localStorage.removeItem(USER_ID_KEY);
localStorage.removeItem(DISPLAY_NAME_KEY);
sessionStorage.setItem(TOKEN_KEY, jwt); sessionStorage.setItem(TOKEN_KEY, jwt);
sessionStorage.setItem(USER_ID_KEY, userId); sessionStorage.setItem(USER_ID_KEY, userId);
if (displayName) sessionStorage.setItem(DISPLAY_NAME_KEY, displayName); if (displayName) sessionStorage.setItem(DISPLAY_NAME_KEY, displayName);

View File

@@ -15,6 +15,12 @@ export interface EventState {
export const eventState = writable<EventState>({ uploadsLocked: false, galleryReleased: false }); export const eventState = writable<EventState>({ uploadsLocked: false, galleryReleased: false });
// Monotonic sequence bumped by every synchronous state transition (markClosed/markOpened).
// `refreshEventState` captures it before its async fetch and only applies the result if no
// newer transition happened meanwhile — so a slow `/me/context` reflecting a pre-reopen
// snapshot can't clobber a subsequent `event-opened` with stale `locked=true`.
let stateSeq = 0;
/** True when uploads are closed for any reason (event locked or gallery released). */ /** True when uploads are closed for any reason (event locked or gallery released). */
export function uploadsClosed(s: EventState): boolean { export function uploadsClosed(s: EventState): boolean {
return s.uploadsLocked || s.galleryReleased; return s.uploadsLocked || s.galleryReleased;
@@ -22,8 +28,12 @@ export function uploadsClosed(s: EventState): boolean {
/** Refresh from the server. Non-fatal on failure — the next SSE event reconciles. */ /** Refresh from the server. Non-fatal on failure — the next SSE event reconciles. */
export async function refreshEventState(): Promise<void> { export async function refreshEventState(): Promise<void> {
const seq = stateSeq;
try { try {
const ctx = await api.get<MeContextDto>('/me/context'); const ctx = await api.get<MeContextDto>('/me/context');
// A close/reopen landed while this was in flight — its result is now authoritative;
// don't overwrite it with our possibly-stale snapshot.
if (seq !== stateSeq) return;
eventState.set({ eventState.set({
uploadsLocked: ctx.uploads_locked, uploadsLocked: ctx.uploads_locked,
galleryReleased: ctx.gallery_released galleryReleased: ctx.gallery_released
@@ -35,10 +45,12 @@ export async function refreshEventState(): Promise<void> {
/** Apply an `event-closed` SSE event (uploads just locked). */ /** Apply an `event-closed` SSE event (uploads just locked). */
export function markClosed(): void { export function markClosed(): void {
stateSeq++;
eventState.update((s) => ({ ...s, uploadsLocked: true })); eventState.update((s) => ({ ...s, uploadsLocked: true }));
} }
/** Apply an `event-opened` SSE event (uploads reopened → release also cleared). */ /** Apply an `event-opened` SSE event (uploads reopened → release also cleared). */
export function markOpened(): void { export function markOpened(): void {
stateSeq++;
eventState.set({ uploadsLocked: false, galleryReleased: false }); eventState.set({ uploadsLocked: false, galleryReleased: false });
} }

View File

@@ -55,18 +55,26 @@ function bindOnline(): void {
bindOnline(); bindOnline();
// Resume the queue when the host reopens the event. A queued upload that hit a locked/ // Resume the queue when the host reopens the event. A queued upload that hit a locked/
// released event kept its blob and parked as a retryable `error` (LockedError); the // released event kept its blob and parked as a retryable `error` (LockedError); flipping it
// `event-opened` SSE flips it back to pending and re-drains, so a photo staged during a // back to pending and re-draining recovers it so a photo staged during a lock isn't lost.
// lock isn't lost across a reopen. One-way import (sse.ts never imports this module). // We resume on TWO signals:
// - `event-opened`: the live reopen, when the SSE happens to be connected.
// - `feed-delta`: fired on every SSE (re)connect (see sse.ts `deltaFetchAndFan`). Because
// `event-opened` has no server-side replay and the SSE is disconnected on non-feed pages
// / backgrounded tabs, a reopen is often MISSED live — this catches up on the next
// reconnect. Retrying while still locked just re-parks the item (bounded, one per event).
// One-way import (sse.ts never imports this module).
let sseBound = false; let sseBound = false;
function bindSse(): void { function bindSse(): void {
if (sseBound || typeof window === 'undefined') return; if (sseBound || typeof window === 'undefined') return;
onSseEvent('event-opened', () => { const resume = () => {
void (async () => { void (async () => {
await requeueRetriable(); await requeueRetriable();
await processQueue(); await processQueue();
})(); })();
}); };
onSseEvent('event-opened', resume);
onSseEvent('feed-delta', resume);
sseBound = true; sseBound = true;
} }
bindSse(); bindSse();
@@ -266,12 +274,15 @@ export async function addToQueue(
if (dup) return 'duplicate'; if (dup) return 'duplicate';
// Cap the queue so stuck/blocked blobs can't grow IndexedDB without bound. When full, // Cap the queue so stuck/blocked blobs can't grow IndexedDB without bound. When full,
// evict the oldest terminal item (done/error/blocked) to make room; if none exist, // evict the oldest item whose blob is already gone or unrecoverable — ONLY `done` (blob
// refuse and tell the caller so it can surface a "queue full" message (silent loss of a // deleted on success) or `blocked` (blob purged, terminal). NEVER evict `error`: those are
// photo the user believed was queued is the worst outcome here). // retryable and still hold a live blob (a network blip, or a locked/released upload that
// resumes on reopen) — evicting one would silently lose a photo the fix deliberately kept.
// If nothing is evictable, refuse and tell the caller so it can surface a "queue full"
// message rather than dropping a photo the user believed was queued.
if (mine.length >= MAX_QUEUE_ITEMS) { if (mine.length >= MAX_QUEUE_ITEMS) {
const evictable = get(queueItems).find( const evictable = get(queueItems).find(
(i) => i.userId === userId && (i.status === 'done' || i.status === 'error' || i.status === 'blocked') (i) => i.userId === userId && (i.status === 'done' || i.status === 'blocked')
); );
if (evictable) { if (evictable) {
await database.delete(STORE_NAME, evictable.id); await database.delete(STORE_NAME, evictable.id);
@@ -486,7 +497,14 @@ async function uploadItem(id: string): Promise<void> {
// A REVERSIBLE lock (event closed / gallery released) is tagged // A REVERSIBLE lock (event closed / gallery released) is tagged
// `uploads_locked` by the backend — keep the blob and park it retryable so // `uploads_locked` by the backend — keep the blob and park it retryable so
// a host reopen resumes it, instead of purging it like a permanent 4xx. // a host reopen resumes it, instead of purging it like a permanent 4xx.
if (body?.error === 'uploads_locked') { // Also treat ANY 403 we can't positively identify as permanent (`forbidden`
// = banned) as reversible: an unparseable 403 body (proxy/WAF/captive
// portal) must NOT purge the blob — losing a photo is the worst outcome, and
// 403 is the reversible-lock status here.
if (
body?.error === 'uploads_locked' ||
(xhr.status === 403 && body?.error !== 'forbidden')
) {
reject(new LockedError(body?.message || 'Event ist geschlossen.')); reject(new LockedError(body?.message || 'Event ist geschlossen.'));
break; break;
} }

View File

@@ -56,9 +56,12 @@
let exportReady = $derived( let exportReady = $derived(
exportInfo?.zip?.status === 'done' && exportInfo?.html?.status === 'done' exportInfo?.zip?.status === 'done' && exportInfo?.html?.status === 'done'
); );
// The keepsake needs BOTH halves, so EITHER failing means the whole thing failed — use
// `&&` (not `||`), else a one-half failure stays stuck on "wird erstellt…" forever and the
// host never sees the "re-release" recovery hint.
let exportGenerating = $derived( let exportGenerating = $derived(
!!exportInfo?.released && !exportReady && !!exportInfo?.released && !exportReady &&
(exportInfo?.zip?.status !== 'failed' || exportInfo?.html?.status !== 'failed') exportInfo?.zip?.status !== 'failed' && exportInfo?.html?.status !== 'failed'
); );
let exportProgress = $derived( let exportProgress = $derived(
Math.min(exportInfo?.zip?.progress_pct ?? 0, exportInfo?.html?.progress_pct ?? 0) Math.min(exportInfo?.zip?.progress_pct ?? 0, exportInfo?.html?.progress_pct ?? 0)
@@ -148,6 +151,12 @@
} }
await reload(); await reload();
// The awaits above mean the component can already be destroyed by the time we get
// here (user navigated away mid-load). Svelte doesn't cancel an async onMount, so
// without this guard onDestroy would have run with an empty `sseOff` and we'd leak
// these handlers into the module-global SSE map forever.
if (destroyed) return;
// Live updates so the dashboard doesn't need a manual reload: // Live updates so the dashboard doesn't need a manual reload:
// - pin-reset-requested: a guest just asked for a reset → refresh the badge/list. // - pin-reset-requested: a guest just asked for a reset → refresh the badge/list.
// - pin-reset: some host resolved a reset → drop it from the list (two-host race: // - pin-reset: some host resolved a reset → drop it from the list (two-host race:
@@ -161,7 +170,9 @@
]; ];
}); });
let destroyed = false;
onDestroy(() => { onDestroy(() => {
destroyed = true;
for (const off of sseOff) off(); for (const off of sseOff) off();
}); });
@@ -169,17 +180,20 @@
loading = true; loading = true;
error = null; error = null;
try { try {
[event, users, pinResetRequests, exportInfo] = await Promise.all([ [event, users, pinResetRequests] = await Promise.all([
api.get<EventStatus>('/host/event'), api.get<EventStatus>('/host/event'),
api.get<UserSummary[]>('/host/users'), api.get<UserSummary[]>('/host/users'),
api.get<PinResetRequest[]>('/host/pin-reset-requests'), api.get<PinResetRequest[]>('/host/pin-reset-requests')
api.get<ExportStatusDto>('/export/status')
]); ]);
} catch (e: unknown) { } catch (e: unknown) {
error = e instanceof Error ? e.message : 'Fehler beim Laden.'; error = e instanceof Error ? e.message : 'Fehler beim Laden.';
} finally { } finally {
loading = false; loading = false;
} }
// Export status is a secondary widget — fetch it OUTSIDE the all-or-nothing block above
// (and error-swallowing) so a transient /export/status failure can't blank the whole
// dashboard (users, moderation, settings).
void refreshExportStatus();
} }
/** Refetch just the pending PIN-reset requests (live badge; avoids a full-page reload). */ /** Refetch just the pending PIN-reset requests (live badge; avoids a full-page reload). */