diff --git a/backend/src/handlers/admin.rs b/backend/src/handlers/admin.rs index b750807..5faa338 100644 --- a/backend/src/handlers/admin.rs +++ b/backend/src/handlers/admin.rs @@ -364,8 +364,10 @@ pub struct DownloadQuery { /// is a top-level navigation so the multi-GB ZIP streams straight to disk instead /// of being buffered in memory by `fetch()` + `blob()` — but a navigation can't /// carry an `Authorization` header, so the client exchanges its Bearer token for -/// an opaque ticket here, then hits `/export/zip?ticket=...`. Reuses the same -/// single-use, 30s-TTL store as the SSE stream. +/// an opaque ticket here, then hits `/export/zip?ticket=...`. Uses the same store as the SSE +/// stream, but NOT the same lifetime: a download ticket lives `DOWNLOAD_TTL` (6 h) and is +/// redeemable up to `MAX_DOWNLOAD_REDEMPTIONS` times, because a multi-GB transfer over venue wifi +/// has to survive being resumed with `Range`. #[derive(serde::Deserialize)] pub struct ExportTicketQuery { /// Which archive the ticket is for — `zip` or `html`. @@ -395,8 +397,11 @@ pub async fn export_ticket( // forever, with no explanation, on the one screen that is the emotional payoff of the app. // Minting is a normal `fetch`, so a 429 here reaches the user as a German message. // - // Moving it does not weaken the limit: tickets are single-use with a 30s TTL and can only be - // obtained from this authenticated endpoint, so one mint is at most one download. + // Moving it does not weaken the limit meaningfully: a ticket can only be obtained from this + // authenticated endpoint, is bound to one archive, and — since downloads must be resumable — + // is worth at most `MAX_DOWNLOAD_REDEMPTIONS` transfers rather than exactly one. The daily + // limit is therefore a bound on mints, not on bytes; see `MAX_DOWNLOAD_REDEMPTIONS` for why + // charging per redemption would re-break resumption. // Confirm the archive actually EXISTS before spending anything on it. // // `export_status` — which is what enables the Download button — reports `done` from @@ -460,14 +465,29 @@ pub async fn export_ticket( ) })?; - enforce_export_rate(&state, auth.user_id).await?; + // A refused mint must not leave its ticket behind. The per-session cap is FOUR tickets of the + // same kind, and a download ticket now lives six hours instead of being consumed on first use — + // so every abandoned one occupies a slot until it expires. A guest whose 1.4 GB transfer looks + // stuck and who taps "Herunterladen" a few more times spends mints 1-3 legitimately, then gets + // a 429 on taps 4 and 5 — but both still minted, and the fifth evicted the OLDEST download + // ticket for the session: the one the running transfer is holding. The next `Range` resume then + // 401s, and re-minting is impossible because they are at the daily limit. The keepsake is gone + // until tomorrow, having done nothing worse than tapping a button that appeared to do nothing. + // + // Discarding here keeps both properties that put the mint first: a store-capacity 503 still + // costs no download, and a refused download costs no slot. + if let Err(e) = enforce_export_rate(&state, auth.user_id).await { + let _ = state + .sse_tickets + .consume(&ticket, TicketKind::Download(export_kind)); + return Err(e); + } Ok(Json(serde_json::json!({ "ticket": ticket }))) } -/// Validate a download ticket (single-use) and confirm its session still exists. -/// Resolve a single-use download ticket to the user who minted it. The caller needs the -/// id to key the export rate limit per-user (see `enforce_export_rate`). +/// Validate a download ticket and confirm its session still exists, resolving it to the user who +/// minted it. Deliberately NOT single-use — see the note on `redeem_download` below. async fn authenticate_download_ticket( state: &AppState, ticket: &str, diff --git a/e2e/specs/06-export/ticket-refused-mint.spec.ts b/e2e/specs/06-export/ticket-refused-mint.spec.ts new file mode 100644 index 0000000..1a9e7af --- /dev/null +++ b/e2e/specs/06-export/ticket-refused-mint.spec.ts @@ -0,0 +1,99 @@ +/** + * A mint that is REFUSED must not cost the guest the download they already have running. + * + * `/export/ticket` issues the ticket before charging the daily limit, deliberately: charging first + * meant a store-capacity 503 — a server-side condition the guest cannot see or cause — still spent + * one of their three daily downloads, with no refund path. + * + * But the ticket a refused mint created was left in the store, and it occupies a slot there for six + * hours, because a download ticket is long-lived so a multi-GB transfer can resume with `Range`. + * The per-session cap is four tickets of the same kind, so: + * + * the 1.4 GB transfer starts on ticket A → the progress bar looks stuck on venue wifi → the guest + * taps "Herunterladen" again → mints 2 and 3 succeed, 4 and 5 are refused with 429 but STILL mint + * → the fifth evicts the oldest download ticket for the session, which is A + * → the transfer drops, resumes with `Range`, and 401s + * → re-minting is impossible: they are at the daily limit + * + * The keepsake is then unreachable until the next day, for tapping a button that appeared to do + * nothing. A refused mint now discards its own ticket. + */ +import { test, expect } from '../../fixtures/test'; +import { seedUpload } from '../../helpers/seed'; +import { BASE } from '../../helpers/env'; + +test.describe('Export — a refused mint does not evict a running download', () => { + test('the first ticket still works after the daily limit has refused later mints', async ({ + host, + api, + adminToken, + }) => { + test.setTimeout(60_000); + const bearer = { Authorization: `Bearer ${host.jwt}` }; + + await seedUpload(host.jwt, { caption: 'keepsake' }); + + const rel = await fetch(`${BASE}/api/v1/host/gallery/release`, { + method: 'POST', + headers: bearer, + }); + expect(rel.status).toBe(204); + + await expect + .poll( + async () => { + const res = await fetch(`${BASE}/api/v1/export/status`, { headers: bearer }); + return (await res.json()).zip?.status; + }, + { timeout: 45_000, intervals: [500] } + ) + .toBe('done'); + + // The e2e reseed forces every limiter OFF, so without this the daily limit never bites and the + // whole test passes vacuously. `export_rate_per_day` is seeded at 3 (migration 005). + await api.patchConfig(adminToken, { + rate_limits_enabled: 'true', + export_rate_enabled: 'true', + }); + + try { + const mint = async () => + fetch(`${BASE}/api/v1/export/ticket?kind=zip`, { method: 'POST', headers: bearer }); + + // 1. The ticket the "running" transfer holds. + const first = await mint(); + expect(first.status).toBe(200); + const ticketA = (await first.json()).ticket as string; + expect(ticketA).toBeTruthy(); + + // 2. Two more legitimate mints, exhausting the day's three. + for (const i of [2, 3]) { + const r = await mint(); + expect(r.status, `mint ${i} is still within the daily allowance`).toBe(200); + } + + // 3. Two refused mints — the impatient taps. This is the CONTROL: if these came back 200 the + // limit was not in force and step 4 would prove nothing. + for (const i of [4, 5]) { + const r = await mint(); + expect(r.status, `mint ${i} must be refused — the daily limit is spent`).toBe(429); + } + + // 4. The running transfer resumes. Its ticket must have survived the refused mints: it is the + // guest's only remaining way to reach the keepsake today. + const resumed = await fetch( + `${BASE}/api/v1/export/zip?ticket=${encodeURIComponent(ticketA)}` + ); + expect( + resumed.status, + 'a refused mint must not evict the ticket an in-flight download is holding' + ).toBe(200); + await resumed.arrayBuffer(); + } finally { + await api.patchConfig(adminToken, { + rate_limits_enabled: 'false', + export_rate_enabled: 'false', + }); + } + }); +});