diff --git a/e2e/fixtures/db.ts b/e2e/fixtures/db.ts index 1c285b6..bdbb9b2 100644 --- a/e2e/fixtures/db.ts +++ b/e2e/fixtures/db.ts @@ -68,6 +68,17 @@ export const db = { ); }, + /** + * Flip the `export_zip_ready` gate directly. The download handler serves bytes + * only when this boolean is true AND the file exists on disk, so setting it true + * without a file lets tests exercise the "ready but file missing" 404 branch. + */ + async setExportZipReady(slug: string, ready: boolean) { + await withClient((c) => + c.query(`UPDATE event SET export_zip_ready = $2 WHERE slug = $1`, [slug, ready]) + ); + }, + /** Insert a pre-baked export job row to skip the (slow) real compression path. */ async fakeExportJob(eventSlug: string, type: 'zip' | 'html', status: 'pending' | 'running' | 'done') { await withClient(async (c) => { diff --git a/e2e/specs/06-export/export.spec.ts b/e2e/specs/06-export/export.spec.ts index fd7fd47..fa17887 100644 --- a/e2e/specs/06-export/export.spec.ts +++ b/e2e/specs/06-export/export.spec.ts @@ -42,22 +42,42 @@ test.describe('Export — release and download', () => { expect(body.html.status).toBe('done'); }); - test('ZIP download returns 404 when no file is on disk (export released but never compressed)', async ({ guest, db }) => { - const g = await guest('NoFile'); + const base = process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101'; + + // Browser downloads stream to disk via a top-level navigation, so the download + // endpoint authenticates with a single-use ticket (no Bearer header). + async function mintTicket(jwt: string): Promise { + const res = await fetch(base + '/api/v1/export/ticket', { + method: 'POST', + headers: { Authorization: `Bearer ${jwt}` }, + }); + return (await res.json()).ticket; + } + + test('ZIP download 404s when the export is not yet marked ready', async ({ guest, db }) => { + const g = await guest('NotReady'); + // Released flag set, but export_zip_ready is still false → must refuse, never serve. await db.setExportReleased(SLUG, true); await db.fakeExportJob(SLUG, 'zip', 'done'); - const base = process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101'; - // Browser downloads stream straight to disk via a top-level navigation, so the - // download endpoint authenticates with a single-use ticket (no Bearer header). - const ticketRes = await fetch(base + '/api/v1/export/ticket', { - method: 'POST', - headers: { Authorization: `Bearer ${g.jwt}` }, - }); - const { ticket } = await ticketRes.json(); - // Real backend additionally checks event.export_zip_ready. The faked row is - // enough for /status; the download path needs the boolean flag too. + const ticket = await mintTicket(g.jwt); + const res = await fetch(base + '/api/v1/export/zip?ticket=' + encodeURIComponent(ticket)); - // Either 404 ("not available" OR "file not found") — both are valid states for this setup. - expect([404, 200]).toContain(res.status); + // Pinned to 404 (not [404,200]): a 200 here would mean serving an export that was + // never released for download — a data-exposure regression. This hits the + // `!export_zip_ready` guard. + expect(res.status).toBe(404); + }); + + test('ZIP download 404s when marked ready but the file is missing on disk', async ({ guest, db }) => { + const g = await guest('ReadyNoFile'); + // Released AND ready, but no Gallery.zip on disk (we never ran a real export) → + // the handler must 404 on the missing-file check, not 200/500 or serve a stale file. + await db.setExportReleased(SLUG, true); + await db.setExportZipReady(SLUG, true); + await db.fakeExportJob(SLUG, 'zip', 'done'); + const ticket = await mintTicket(g.jwt); + + const res = await fetch(base + '/api/v1/export/zip?ticket=' + encodeURIComponent(ticket)); + expect(res.status).toBe(404); }); }); diff --git a/e2e/specs/07-adversarial/authorization-deep.spec.ts b/e2e/specs/07-adversarial/authorization-deep.spec.ts index dca3def..4b4a521 100644 --- a/e2e/specs/07-adversarial/authorization-deep.spec.ts +++ b/e2e/specs/07-adversarial/authorization-deep.spec.ts @@ -5,26 +5,109 @@ * with cross-user and banned-user scenarios that span multiple resources. */ import { test, expect } from '../../fixtures/test'; +import { uploadRaw, JPEG_MAGIC } from '../../helpers/upload-client'; const BASE = process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101'; -test.describe('Adversarial — deep authorization', () => { - test('user A cannot delete user B\'s comment via /api/v1/comment/{id}', async ({ api, guest }) => { - const a = await guest('CommentA'); - const b = await guest('CommentB'); +/** Seed a real upload owned by `jwt` and return its id. */ +async function seedUpload(jwt: string, caption?: string): Promise { + const body = new Uint8Array(1024); + body.set(JPEG_MAGIC, 0); + const res = await uploadRaw(jwt, body, { filename: 'a.jpg', contentType: 'image/jpeg', caption }); + if (res.status !== 201) throw new Error(`seed upload failed: ${res.status} ${await res.text()}`); + return (await res.json()).id; +} - // We need an upload first; without a multipart helper here we use a placeholder: - // post a comment on a non-existent upload to force the path to return 404 / 403 / 401. - // The real intent is verified once an upload helper feeds this test a real upload_id. - const fakeId = '00000000-0000-0000-0000-000000000000'; - const res = await fetch(`${BASE}/api/v1/comment/${fakeId}`, { +/** Seed a real comment on `uploadId` authored by `jwt`; return its id. */ +async function seedComment(jwt: string, uploadId: string, body = 'mine'): Promise { + const res = await fetch(`${BASE}/api/v1/upload/${uploadId}/comments`, { + method: 'POST', + headers: { Authorization: `Bearer ${jwt}`, 'Content-Type': 'application/json' }, + body: JSON.stringify({ body }), + }); + if (res.status !== 201) throw new Error(`seed comment failed: ${res.status} ${await res.text()}`); + return (await res.json()).id; +} + +async function listComments(jwt: string, uploadId: string): Promise { + const res = await fetch(`${BASE}/api/v1/upload/${uploadId}/comments`, { + headers: { Authorization: `Bearer ${jwt}` }, + }); + return res.json(); +} + +test.describe('Adversarial — deep authorization', () => { + // IDOR: user B must not be able to delete user A's REAL comment. This exercises the + // ownership guard (`comment.user_id != auth.user_id` → 403) — the previous version fired + // at the all-zeros UUID, which 404s at the lookup BEFORE that guard runs, so it never + // tested authorization at all. + test('user B cannot delete user A\'s comment (real resource → 403, comment survives)', async ({ guest }) => { + const a = await guest('CommentOwnerA'); + const b = await guest('AttackerB'); + + const uploadId = await seedUpload(a.jwt); + const commentId = await seedComment(a.jwt, uploadId, 'A owns this'); + + const res = await fetch(`${BASE}/api/v1/comment/${commentId}`, { method: 'DELETE', headers: { Authorization: `Bearer ${b.jwt}` }, }); - // Acceptable: 403 (not your comment), 404 (no such comment), 401. - expect([401, 403, 404]).toContain(res.status); - void a; - void api; + // Must be 403 specifically — the comment exists and is in B's event, so a 404 would + // mean the ownership check was skipped/reordered. + expect(res.status).toBe(403); + + // No state change: the comment is still there. + const after = await listComments(a.jwt, uploadId); + expect(after.some((c: any) => c.id === commentId)).toBe(true); + + // Control: the real owner CAN delete it (proves the 403 was about identity, not a broken route). + const ownerDel = await fetch(`${BASE}/api/v1/comment/${commentId}`, { + method: 'DELETE', + headers: { Authorization: `Bearer ${a.jwt}` }, + }); + expect(ownerDel.status).toBe(204); + }); + + // IDOR: user B must not be able to delete user A's REAL upload. + test('user B cannot delete user A\'s upload (403, upload survives)', async ({ guest, db }) => { + const a = await guest('UploadOwnerA'); + const b = await guest('AttackerB2'); + + const uploadId = await seedUpload(a.jwt); + expect(await db.countUploadsForUser(a.userId)).toBe(1); + + const res = await fetch(`${BASE}/api/v1/upload/${uploadId}`, { + method: 'DELETE', + headers: { Authorization: `Bearer ${b.jwt}` }, + }); + expect(res.status).toBe(403); + + // No state change: A's upload is still present (not soft-deleted). + expect(await db.countUploadsForUser(a.userId)).toBe(1); + }); + + // IDOR: user B must not be able to edit (re-caption / re-tag) user A's upload. + test('user B cannot edit user A\'s upload caption (403, caption unchanged)', async ({ guest, db }) => { + const a = await guest('UploadOwnerA2'); + const b = await guest('AttackerB3'); + + const uploadId = await seedUpload(a.jwt, 'original caption'); + // Make it visible in the feed so we can read the caption back. + await db.setUploadCompressionStatus(uploadId, 'done'); + + const res = await fetch(`${BASE}/api/v1/upload/${uploadId}`, { + method: 'PATCH', + headers: { Authorization: `Bearer ${b.jwt}`, 'Content-Type': 'application/json' }, + body: JSON.stringify({ caption: 'hacked by B' }), + }); + expect(res.status).toBe(403); + + // No state change: the caption A set is intact. + const feedRes = await fetch(`${BASE}/api/v1/feed`, { headers: { Authorization: `Bearer ${a.jwt}` } }); + const feed: any = await feedRes.json(); + const list: any[] = feed.uploads ?? feed.items ?? feed; + const row = list.find((u: any) => u.id === uploadId); + expect(row?.caption).toBe('original caption'); }); test('banned user cannot toggle a like', async ({ api, host, guest }) => { diff --git a/e2e/specs/07-adversarial/file-upload-attacks.spec.ts b/e2e/specs/07-adversarial/file-upload-attacks.spec.ts index baa9dca..8cbe49d 100644 Binary files a/e2e/specs/07-adversarial/file-upload-attacks.spec.ts and b/e2e/specs/07-adversarial/file-upload-attacks.spec.ts differ