diff --git a/backend/src/handlers/test_admin.rs b/backend/src/handlers/test_admin.rs index b6c40ce..917ccbf 100644 --- a/backend/src/handlers/test_admin.rs +++ b/backend/src/handlers/test_admin.rs @@ -98,6 +98,13 @@ pub async fn truncate_all( // surviving ticket is a dangling reference to a user that no longer exists. state.sse_tickets.clear(); + // Invalidate any in-flight/queued compression task spawned by the previous test. Without this a + // task still waiting on the concurrency semaphore wakes AFTER this wipe, fails to find its + // (now-deleted) file, and broadcasts upload-error/upload-deleted into the NEXT test's SSE + // stream. (Export workers are already inert across a truncate: they are epoch-guarded on the + // event row, and truncate gives the event a fresh random UUID, so their writes match nothing.) + state.compression.bump_generation(); + Ok(StatusCode::NO_CONTENT) } diff --git a/backend/src/handlers/upload.rs b/backend/src/handlers/upload.rs index 0edcc5a..e0fdbd2 100644 --- a/backend/src/handlers/upload.rs +++ b/backend/src/handlers/upload.rs @@ -416,6 +416,15 @@ pub async fn edit_upload( // Caption update + hashtag wipe-then-relink in one transaction, so a crash // mid-relink can't leave the upload with its hashtags stripped. + // + // Editing is intentionally allowed while uploads are locked or the gallery is released — like + // comments and likes, the lock freezes *new uploads* only (USER_JOURNEYS §9.3). But a caption + // is embedded in the HTML viewer keepsake (the ZIP holds media only — see export.rs), so an + // edit AFTER release must regenerate the viewer, or the downloadable keepsake keeps showing the + // old caption forever while the live feed shows the new one. Same atomicity as delete_upload: + // the edit and its invalidation share one tx so a dropped handler can't leave them disagreeing. + // `Affects::ViewerOnly` carries the finished ZIP forward (the media didn't change); when the + // gallery isn't released, `invalidate_and_arm` returns None and this is a no-op. let mut tx = state.pool.begin().await?; if let Some(ref caption) = body.caption { Upload::update_caption(&mut *tx, upload_id, Some(caption)).await?; @@ -427,7 +436,16 @@ pub async fn edit_upload( Hashtag::link_to_upload(&mut *tx, upload_id, h.id).await?; } } + let regen = crate::services::export::invalidate_and_arm( + &mut tx, + &state.config.event_slug, + crate::services::export::Affects::ViewerOnly, + ) + .await?; tx.commit().await?; + if let Some(r) = regen { + crate::handlers::host::start_regen(&state, r); + } Ok(StatusCode::OK) } diff --git a/backend/src/services/compression.rs b/backend/src/services/compression.rs index 44813c0..29fcb1b 100644 --- a/backend/src/services/compression.rs +++ b/backend/src/services/compression.rs @@ -1,4 +1,5 @@ use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::Arc; use anyhow::{Context, Result}; @@ -15,6 +16,10 @@ pub struct CompressionWorker { pool: PgPool, media_path: PathBuf, sse_tx: broadcast::Sender, + /// Bumped whenever the underlying data is reset out from under in-flight work (only the e2e + /// TRUNCATE does this today). A task captures the value at spawn and abandons itself if it has + /// changed by the time it runs — see `process`. + generation: Arc, } impl CompressionWorker { @@ -24,14 +29,32 @@ impl CompressionWorker { pool, media_path, sse_tx, + generation: Arc::new(AtomicU64::new(0)), } } + /// Invalidate all in-flight and queued compression work. Called by the e2e TRUNCATE endpoint: + /// truncating deletes the upload rows and wipes `media/`, so a worker that was queued on the + /// semaphore when the wipe happened would otherwise wake in the NEXT test, fail to find its + /// file, and broadcast `upload-error` / `upload-deleted` into that test's live SSE stream — + /// corrupting any test that asserts on toasts or feed contents. Bumping the generation makes + /// those stale tasks return silently instead. A no-op in production (never called there). + pub fn bump_generation(&self) { + self.generation.fetch_add(1, Ordering::SeqCst); + } + /// Spawn a background task to process an uploaded file. pub fn process(&self, upload_id: Uuid, original_path: String, mime_type: String) { let worker = self.clone(); + let born_at = worker.generation.load(Ordering::SeqCst); tokio::spawn(async move { let _permit = worker.semaphore.acquire().await; + // The data this task was queued against may have been reset while it waited for a permit + // (e2e TRUNCATE). If so, its file and row are gone; doing anything — including + // broadcasting a failure — would leak into an unrelated test. Abandon quietly. + if worker.generation.load(Ordering::SeqCst) != born_at { + return; + } match worker.do_process(upload_id, &original_path, &mime_type).await { Ok(_) => { tracing::info!("compression completed for upload {upload_id}"); diff --git a/e2e/fixtures/api-client.ts b/e2e/fixtures/api-client.ts index 238ae0b..c0b35b0 100644 --- a/e2e/fixtures/api-client.ts +++ b/e2e/fixtures/api-client.ts @@ -137,6 +137,11 @@ export class ApiClient { }); } + async listPinResetRequests(token: string): Promise { + const { body } = await this.request('GET', '/host/pin-reset-requests', { token }); + return body; + } + async closeEvent(token: string) { return this.request('POST', '/host/event/close', { token, expectedStatus: [200, 204] }); } diff --git a/e2e/fixtures/db.ts b/e2e/fixtures/db.ts index f881632..6a001fc 100644 --- a/e2e/fixtures/db.ts +++ b/e2e/fixtures/db.ts @@ -59,6 +59,26 @@ export const db = { }); }, + async countSessionsForUser(userId: string): Promise { + return withClient(async (c) => { + const r = await c.query<{ count: string }>( + `SELECT COUNT(*)::text AS count FROM session WHERE user_id = $1`, + [userId] + ); + return Number(r.rows[0].count); + }); + }, + + async countPinResetRequestsForUser(userId: string): Promise { + return withClient(async (c) => { + const r = await c.query<{ count: string }>( + `SELECT COUNT(*)::text AS count FROM pin_reset_request WHERE user_id = $1`, + [userId] + ); + return Number(r.rows[0].count); + }); + }, + async setExportReleased(slug: string, released: boolean) { await withClient((c) => c.query(`UPDATE event SET export_released_at = $2 WHERE slug = $1`, [ diff --git a/e2e/specs/01-auth/logout-everywhere.spec.ts b/e2e/specs/01-auth/logout-everywhere.spec.ts new file mode 100644 index 0000000..9451c00 --- /dev/null +++ b/e2e/specs/01-auth/logout-everywhere.spec.ts @@ -0,0 +1,58 @@ +/** + * "Sign out everywhere" — DELETE /api/v1/sessions. A security control (revoke every device after a + * lost/stolen phone) that had ZERO coverage. Sessions are validated per-request against the DB + * (auth middleware resolves the token hash to a live session row), so revocation is observable: a + * revoked token must stop authenticating. + */ +import { test, expect } from '../../fixtures/test'; + +const BASE = process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101'; + +const ctx = (jwt: string) => + fetch(`${BASE}/api/v1/me/context`, { headers: { Authorization: `Bearer ${jwt}` } }); + +test.describe('Auth — sign out everywhere', () => { + test('DELETE /sessions revokes ALL of the caller\'s sessions, not just the current one', async ({ + api, + guest, + db, + }) => { + // Two devices for one guest: the join session, plus a second session from a recover login. + const g = await guest('MultiDevice'); + const second = await api.recover(g.displayName, g.pin); + const secondJwt = second.body.jwt; + + // Both tokens work, and there really are two distinct sessions. + expect((await ctx(g.jwt)).status).toBe(200); + expect((await ctx(secondJwt)).status).toBe(200); + expect(await db.countSessionsForUser(g.userId)).toBeGreaterThanOrEqual(2); + + // Sign out everywhere, authenticated as device one. + const res = await fetch(`${BASE}/api/v1/sessions`, { + method: 'DELETE', + headers: { Authorization: `Bearer ${g.jwt}` }, + }); + expect(res.status).toBe(204); + + // BOTH devices are now logged out — the whole point of the control. If it only killed the + // caller's own session, device two would still be live and a stolen phone would keep access. + expect((await ctx(g.jwt)).status, 'the calling device is signed out').toBe(401); + expect((await ctx(secondJwt)).status, 'the OTHER device is signed out too').toBe(401); + expect(await db.countSessionsForUser(g.userId)).toBe(0); + }); + + test('one user signing out everywhere does not touch another user\'s sessions', async ({ + guest, + }) => { + const a = await guest('SignsOut'); + const b = await guest('StaysIn'); + + await fetch(`${BASE}/api/v1/sessions`, { + method: 'DELETE', + headers: { Authorization: `Bearer ${a.jwt}` }, + }); + + expect((await ctx(a.jwt)).status).toBe(401); + expect((await ctx(b.jwt)).status, "another user's session must be untouched").toBe(200); + }); +}); diff --git a/e2e/specs/01-auth/pin-reset-lifecycle.spec.ts b/e2e/specs/01-auth/pin-reset-lifecycle.spec.ts new file mode 100644 index 0000000..7579a06 --- /dev/null +++ b/e2e/specs/01-auth/pin-reset-lifecycle.spec.ts @@ -0,0 +1,119 @@ +/** + * USER_JOURNEYS §4 — the "I forgot my PIN" in-app request lifecycle, end to end. + * + * This whole journey had ZERO functional coverage (only the 403 gate, from the authz sweep). The + * invariants below are all security-relevant and none of them were tested: + * - the always-204 non-enumeration contract (an unknown name must look identical to a known one) + * - the per-IP+name throttle (3 / 15 min) + * - admins are EXCLUDED from the request queue (they recover via password, not PIN) + * - a host reset REVOKES the target's sessions (the forgotten/compromised device is logged out) + */ +import { test, expect } from '../../fixtures/test'; + +const BASE = process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101'; + +const requestReset = (displayName: string) => + fetch(`${BASE}/api/v1/recover/request`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ display_name: displayName }), + }); + +test.describe('PIN reset — in-app request lifecycle', () => { + test('a request for a real guest queues exactly one request the host can see', async ({ + api, + host, + guest, + db, + }) => { + const g = await guest('ForgetfulFrida'); + + expect((await requestReset('ForgetfulFrida')).status).toBe(204); + + // The host sees it... + const list = await api.listPinResetRequests(host.jwt); + expect(list.some((r: any) => r.user_id === g.userId)).toBe(true); + // ...and it's deduped: a second request does not stack a duplicate (ON CONFLICT DO NOTHING). + expect((await requestReset('ForgetfulFrida')).status).toBe(204); + expect(await db.countPinResetRequestsForUser(g.userId)).toBe(1); + }); + + test('an UNKNOWN name is indistinguishable from a known one (204, no queue, no enumeration)', async ({ + host, + api, + guest, + }) => { + // Same status code and no error body as the real path — the only observable difference must be + // in the host's queue, which the requester cannot see. + const known = await guest('KnownKarl'); + expect((await requestReset('KnownKarl')).status).toBe(204); + expect((await requestReset('no-such-guest-' + Date.now())).status).toBe(204); + + // The unknown name queued nothing; the known one queued exactly one. + const list = await api.listPinResetRequests(host.jwt); + expect(list).toHaveLength(1); + expect(list[0].user_id).toBe(known.userId); + }); + + test('an ADMIN never gets a reset request queued (admins recover via password)', async ({ + api, + host, + db, + adminToken, + }) => { + // The admin user exists (adminToken logged them in). Their display name is "Admin" by + // convention; request a reset for it and confirm the queue stays empty — the INSERT filters + // `role <> 'admin'`, so queuing a reset for an admin would be a privilege-relevant leak. + const admin = (await api.listUsers(host.jwt)).find((u: any) => u.role === 'admin'); + expect(admin, 'an admin user must exist').toBeTruthy(); + + expect((await requestReset(admin.display_name)).status).toBe(204); + + expect(await db.countPinResetRequestsForUser(admin.id)).toBe(0); + expect(await api.listPinResetRequests(host.jwt)).toHaveLength(0); + }); + + test('requests are throttled to 3 per 15 min per IP+name', async ({ api, adminToken, guest }) => { + await api.patchConfig(adminToken, { rate_limits_enabled: 'true' }); + await guest('ThrottleTarget'); + + const statuses: number[] = []; + for (let i = 0; i < 5; i++) statuses.push((await requestReset('ThrottleTarget')).status); + + // First 3 accepted, the rest throttled — proving the limiter is real and keyed. + expect(statuses.filter((s) => s === 204)).toHaveLength(3); + expect(statuses.filter((s) => s === 429).length).toBeGreaterThan(0); + + // Restore so the next test's fixtures aren't affected (rate limits share the backend). + await api.patchConfig(adminToken, { rate_limits_enabled: 'false' }); + }); + + test('a host PIN reset revokes the targets sessions and clears the request', async ({ + api, + host, + guest, + db, + }) => { + const g = await guest('CompromisedCarl'); + await requestReset('CompromisedCarl'); + + // The guest has a live session (the join minted one) and a queued request. + expect(await db.countSessionsForUser(g.userId)).toBeGreaterThan(0); + expect(await db.countPinResetRequestsForUser(g.userId)).toBe(1); + + // Host resets the PIN. + await api.resetUserPin(host.jwt, g.userId); + + // The old device is logged out: the guest's pre-reset JWT no longer authenticates. This is the + // security point of a reset — sessions are token-bound, so without the revoke the compromised + // device would stay logged in with full access despite the new PIN. + const stale = await fetch(`${BASE}/api/v1/me/context`, { + headers: { Authorization: `Bearer ${g.jwt}` }, + }); + expect(stale.status, 'the pre-reset session must be revoked').toBe(401); + expect(await db.countSessionsForUser(g.userId)).toBe(0); + + // And the pending request was resolved. + expect(await db.countPinResetRequestsForUser(g.userId)).toBe(0); + }); +}); diff --git a/e2e/specs/07-adversarial/media-gating.spec.ts b/e2e/specs/07-adversarial/media-gating.spec.ts index 84f11ae..47b2f04 100644 --- a/e2e/specs/07-adversarial/media-gating.spec.ts +++ b/e2e/specs/07-adversarial/media-gating.spec.ts @@ -56,4 +56,42 @@ test.describe('Media gating — moderation revokes preview access (F2)', () => { const afterDelete = await fetch(`${BASE}/api/v1/upload/${id}/preview`); expect(afterDelete.status, 'moderation must revoke preview access').toBe(404); }); + + test('the preview is revoked when the UPLOADER is banned (not just on delete)', async ({ + host, + api, + guest, + }) => { + test.setTimeout(30_000); + // The header of this file claims delete AND ban-hide both revoke access, but only delete was + // ever exercised. A ban hides the user's content everywhere (the visibility check filters + // `is_banned` inside `find_by_id_visible`, which gates preview AND thumbnail identically), and + // its whole point is that a direct-URL holder loses the image — so it must 404 the gated + // preview too, exactly like a delete. + const offender = await guest('BannedUploader'); + const id = await seedUpload(offender.jwt, { caption: 'to be hidden' }); + + // Wait for the compression worker to produce the preview. + await expect + .poll( + async () => { + const row = (await api.getFeed(host.jwt)).uploads?.find((u: any) => u.id === id); + return row?.preview_url; + }, + { timeout: 20_000, intervals: [500] } + ) + .toBe(`/api/v1/upload/${id}/preview`); + + // Served while the uploader is in good standing. + expect((await fetch(`${BASE}/api/v1/upload/${id}/preview`)).status).toBe(200); + + // Ban the uploader (default: hide their uploads). + await api.banUser(host.jwt, offender.userId); + + // Must now 404 — the direct-URL holder loses the image, same as a takedown. + expect( + (await fetch(`${BASE}/api/v1/upload/${id}/preview`)).status, + "a banned uploader's preview must be revoked" + ).toBe(404); + }); }); diff --git a/e2e/specs/10-flow-review/export-reopen-rerelease.spec.ts b/e2e/specs/10-flow-review/export-reopen-rerelease.spec.ts index 8a0a8e2..7257124 100644 --- a/e2e/specs/10-flow-review/export-reopen-rerelease.spec.ts +++ b/e2e/specs/10-flow-review/export-reopen-rerelease.spec.ts @@ -518,6 +518,43 @@ test.describe('Flow re-review — reopen→re-release export integrity + stale-k expect(await zipJobEpoch()).toBe(await eventEpoch()); }); + test('editing a caption AFTER release regenerates the viewer but carries the ZIP forward', async ({ + host, + }) => { + // edit_upload used to update the caption and NOTHING else — no keepsake invalidation. A caption + // lives in the HTML viewer (the ZIP holds media only), so after release the downloadable viewer + // kept showing the OLD caption forever while the live feed showed the new one. Editing is + // intentionally allowed post-release (like comments), so the fix is to regenerate, not forbid: + // invalidate_and_arm(ViewerOnly) — rebuild the viewer, carry the finished ZIP forward untouched. + const uploadId = await seedUpload(host.jwt, { caption: 'tippfehlr' }); + expect((await post('/api/v1/host/gallery/release', host.jwt)).status).toBe(204); + await waitExportDone(host.jwt); + + const zipEpochBefore = await zipJobEpoch(); + const eventEpochBefore = await eventEpoch(); + expect(zipEpochBefore).toBe(eventEpochBefore); + + // The edit is allowed (not blocked by the release lock) ... + const res = await fetch(BASE + `/api/v1/upload/${uploadId}`, { + method: 'PATCH', + headers: { Authorization: `Bearer ${host.jwt}`, 'Content-Type': 'application/json' }, + body: JSON.stringify({ caption: 'korrigiert' }), + }); + expect(res.status).toBe(200); + + // ... and it bumped the epoch (the viewer must rebuild) ... + await expect.poll(async () => await eventEpoch(), { timeout: 10_000 }).toBeGreaterThan(eventEpochBefore); + + // ... while the ZIP was CARRIED FORWARD, not rebuilt: its row rides the new epoch (the media + // didn't change, so a full ZIP rebuild would be wasted work). + await waitExportDone(host.jwt); + expect(await zipJobEpoch(), 'the ZIP must be carried forward to the new epoch').toBe( + await eventEpoch() + ); + // The keepsake is still downloadable with its single photo intact. + expect(await downloadZipEntries(host.jwt)).toHaveLength(1); + }); + test('a comment deleted while the ZIP is still BUILDING does not strand the ZIP', async ({ host, guest,