From c48d43f5b3b01140d30d955ae6c92b3de255fd66 Mon Sep 17 00:00:00 2001 From: fabi Date: Wed, 15 Jul 2026 19:48:31 +0200 Subject: [PATCH] test(e2e): cover the untested security lifecycles (PIN reset, logout-all, ban gating, caption) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the coverage gaps the audit flagged and I verified were real: pin-reset-lifecycle.spec.ts (new): the whole "I forgot my PIN" journey (§4) had only its 403 gate tested. Now: the always-204 non-enumeration contract (unknown name looks identical to known); dedup (ON CONFLICT); admins are EXCLUDED from the queue; the 3-per-15-min throttle; and a host reset REVOKES the target's sessions (the pre-reset JWT 401s afterward) and clears the request. Sessions are validated per-request against the DB, so the revoke is observable. logout-everywhere.spec.ts (new): DELETE /sessions ("sign out everywhere") had ZERO tests. Proves it revokes ALL of the caller's sessions across two devices (both tokens 401 after), not just the current one, and doesn't touch another user's sessions. media-gating: the file claimed delete AND ban-hide both revoke preview access, but only delete was exercised. Add the ban case — a banned uploader's gated preview 404s, same as a takedown. (Thumbnail shares the identical find_by_id_visible gate; the seed fixture produces no thumbnail derivative, so preview is the honest thing to assert.) export caption test: an edit after release regenerates the viewer (epoch bumps) while the ZIP is carried forward, not rebuilt — guards the fix in the previous commit. Helpers: api.listPinResetRequests; db.countSessionsForUser / countPinResetRequestsForUser. Co-Authored-By: Claude Opus 4.8 --- e2e/fixtures/api-client.ts | 5 + e2e/fixtures/db.ts | 20 +++ e2e/specs/01-auth/logout-everywhere.spec.ts | 58 +++++++++ e2e/specs/01-auth/pin-reset-lifecycle.spec.ts | 119 ++++++++++++++++++ e2e/specs/07-adversarial/media-gating.spec.ts | 38 ++++++ .../export-reopen-rerelease.spec.ts | 37 ++++++ 6 files changed, 277 insertions(+) create mode 100644 e2e/specs/01-auth/logout-everywhere.spec.ts create mode 100644 e2e/specs/01-auth/pin-reset-lifecycle.spec.ts 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,