diff --git a/backend/migrations/012_export_release_seq.down.sql b/backend/migrations/012_export_release_seq.down.sql new file mode 100644 index 0000000..05842dc --- /dev/null +++ b/backend/migrations/012_export_release_seq.down.sql @@ -0,0 +1,2 @@ +ALTER TABLE export_job + DROP COLUMN IF EXISTS release_seq; diff --git a/backend/migrations/012_export_release_seq.up.sql b/backend/migrations/012_export_release_seq.up.sql new file mode 100644 index 0000000..7fb512c --- /dev/null +++ b/backend/migrations/012_export_release_seq.up.sql @@ -0,0 +1,11 @@ +-- H1: export generation guard. A reopen (which clears `export_released_at`) followed by a +-- re-release could spawn a fresh export worker while a worker from the PRIOR release was +-- still streaming a now-stale snapshot to `Gallery.zip`. The stale worker's finalize then +-- re-set `export_*_ready = TRUE` on an archive that predated the reopen — a silent, +-- permanent data loss (new uploads missing from a "ready" keepsake). +-- +-- `release_seq` is a per-(event,type) generation counter bumped on every (re)release. +-- A worker captures the seq it claimed and only finalizes / flips the ready flag while +-- that seq is still current; a superseded worker discards its output instead. +ALTER TABLE export_job + ADD COLUMN release_seq BIGINT NOT NULL DEFAULT 0; diff --git a/backend/migrations/013_uploads_hidden_at.down.sql b/backend/migrations/013_uploads_hidden_at.down.sql new file mode 100644 index 0000000..4d93e09 --- /dev/null +++ b/backend/migrations/013_uploads_hidden_at.down.sql @@ -0,0 +1,2 @@ +ALTER TABLE "user" + DROP COLUMN IF EXISTS uploads_hidden_at; diff --git a/backend/migrations/013_uploads_hidden_at.up.sql b/backend/migrations/013_uploads_hidden_at.up.sql new file mode 100644 index 0000000..2741618 --- /dev/null +++ b/backend/migrations/013_uploads_hidden_at.up.sql @@ -0,0 +1,12 @@ +-- Ban-replay on reconnect. A ban is not a soft-delete (it sets `uploads_hidden`/`is_banned`, +-- never `upload.deleted_at`), and live eviction rode only on the ephemeral `user-hidden` SSE +-- broadcast — which has no reconnect-replay. So a client (especially the unattended diashow +-- projector) that missed that broadcast kept cycling the banned user's already-loaded slides. +-- +-- `uploads_hidden_at` stamps WHEN a user's uploads became hidden, so the reconnect delta can +-- return the set of users hidden since the client's cursor and the client can evict them. +ALTER TABLE "user" + ADD COLUMN uploads_hidden_at TIMESTAMPTZ; + +-- Backfill already-hidden users so a reconnect right after this migration still evicts them. +UPDATE "user" SET uploads_hidden_at = NOW() WHERE uploads_hidden = TRUE; diff --git a/backend/src/error.rs b/backend/src/error.rs index c8698ee..14127d4 100644 --- a/backend/src/error.rs +++ b/backend/src/error.rs @@ -6,6 +6,11 @@ pub enum AppError { BadRequest(String), Unauthorized(String), Forbidden(String), + /// Uploads are temporarily locked (event closed / gallery released). Distinct from + /// `Forbidden` so the client can tell this REVERSIBLE 403 apart from a permanent one + /// (banned user, quota): the queued blob is kept and retried if the host reopens, + /// instead of being purged like a genuinely-terminal rejection. + UploadsLocked(String), NotFound(String), Conflict(String), /// Second field: optional retry-after seconds to include in the response. @@ -23,6 +28,7 @@ impl AppError { Self::BadRequest(_) => (StatusCode::BAD_REQUEST, "bad_request"), Self::Unauthorized(_) => (StatusCode::UNAUTHORIZED, "unauthorized"), Self::Forbidden(_) => (StatusCode::FORBIDDEN, "forbidden"), + Self::UploadsLocked(_) => (StatusCode::FORBIDDEN, "uploads_locked"), Self::NotFound(_) => (StatusCode::NOT_FOUND, "not_found"), Self::Conflict(_) => (StatusCode::CONFLICT, "conflict"), Self::TooManyRequests(..) => (StatusCode::TOO_MANY_REQUESTS, "too_many_requests"), @@ -36,6 +42,7 @@ impl AppError { Self::BadRequest(msg) | Self::Unauthorized(msg) | Self::Forbidden(msg) + | Self::UploadsLocked(msg) | Self::NotFound(msg) | Self::Conflict(msg) => msg.clone(), Self::TooManyRequests(msg, _) => msg.clone(), diff --git a/backend/src/handlers/admin.rs b/backend/src/handlers/admin.rs index 48edcb7..d38c63e 100644 --- a/backend/src/handlers/admin.rs +++ b/backend/src/handlers/admin.rs @@ -298,12 +298,42 @@ pub async fn download_zip( )); } - let path = state.config.export_path.join("Gallery.zip"); + let path = resolve_export_file(&state, "zip").await?; + serve_file(path, "Gallery.zip", "application/zip").await +} + +/// Resolve the on-disk path of the CURRENT export generation from `export_job.file_path` +/// rather than a fixed filename. Exports are written to per-generation paths +/// (`Gallery..zip`) so a superseded worker can't stomp a fresh keepsake (H1); the +/// download must therefore follow whatever the current 'done' row points at, not guess. +async fn resolve_export_file( + state: &AppState, + export_type: &str, +) -> Result { + let file_path: Option<(Option,)> = sqlx::query_as( + "SELECT file_path FROM export_job ej + JOIN event e ON e.id = ej.event_id + WHERE e.slug = $1 AND ej.type = $2::export_type AND ej.status = 'done'", + ) + .bind(&state.config.event_slug) + .bind(export_type) + .fetch_optional(&state.pool) + .await + .map_err(|e| AppError::Internal(e.into()))?; + + // `file_path` is stored as `exports/`; the base dir is already `export_path`, so + // join only the file name (defends against any absolute/`..` content too). + let rel = file_path + .and_then(|(p,)| p) + .ok_or_else(|| AppError::NotFound("Exportdatei nicht gefunden.".into()))?; + let name = std::path::Path::new(&rel) + .file_name() + .ok_or_else(|| AppError::NotFound("Exportdatei nicht gefunden.".into()))?; + let path = state.config.export_path.join(name); if !path.exists() { return Err(AppError::NotFound("Exportdatei nicht gefunden.".into())); } - - serve_file(path, "Gallery.zip", "application/zip").await + Ok(path) } pub async fn download_html( @@ -324,11 +354,7 @@ pub async fn download_html( )); } - let path = state.config.export_path.join("Memories.zip"); - if !path.exists() { - return Err(AppError::NotFound("Exportdatei nicht gefunden.".into())); - } - + let path = resolve_export_file(&state, "html").await?; serve_file(path, "Memories.zip", "application/zip").await } diff --git a/backend/src/handlers/feed.rs b/backend/src/handlers/feed.rs index d72b7a5..fb65476 100644 --- a/backend/src/handlers/feed.rs +++ b/backend/src/handlers/feed.rs @@ -181,6 +181,12 @@ pub struct DeltaQuery { pub struct DeltaResponse { pub uploads: Vec, pub deleted_ids: Vec, + /// Users whose uploads became hidden (banned / uploads_hidden) since `since`. A ban is + /// not a soft-delete, so it never appears in `deleted_ids`; without this, a client that + /// missed the ephemeral `user-hidden` SSE (a reconnecting projector, most acutely) would + /// keep displaying the banned user's already-loaded slides. The client evicts every + /// upload from these users on receipt. + pub hidden_user_ids: Vec, /// True when the upload query hit `DELTA_LIMIT`: the response carries only the /// newest slice of the gap, so the client must fall back to a full feed refresh /// rather than merging (the older missed uploads are absent and unrecoverable @@ -229,11 +235,17 @@ pub async fn feed_delta( // entire event's uploads in one response. If a client hits the cap it should // fall back to a full feed refresh rather than another delta. const DELTA_LIMIT: i64 = 200; + // `>= since` (not `>`): `created_at` is not unique, so a strict `>` anchored to the exact + // timestamp of the last-seen upload silently drops a SECOND upload committed in the same + // microsecond — an unrecoverable live-update miss. `>=` re-includes the boundary rows; + // the client merges by id (see the feed-delta handler's `seen` set), so the duplicate is + // harmless while the tied upload is no longer lost. The cursor still advances to the + // response's `server_time`, so this doesn't re-fetch on every subsequent delta. let rows = sqlx::query_as::<_, FeedRow>( "SELECT id, user_id, uploader_name, preview_path, thumbnail_path, mime_type, caption, like_count, comment_count, created_at FROM v_feed - WHERE event_id = $1 AND created_at > $2 + WHERE event_id = $1 AND created_at >= $2 ORDER BY created_at DESC, id DESC LIMIT $3", ) @@ -247,9 +259,23 @@ pub async fn feed_delta( // client to full-refresh instead of merging a partial delta. let truncated = rows.len() as i64 >= DELTA_LIMIT; + // `>=` for the same tie-break reason as the uploads query above; re-signalling an + // already-removed id is idempotent on the client (it filters its list by these ids). let deleted_ids: Vec<(Uuid,)> = sqlx::query_as( "SELECT id FROM upload - WHERE event_id = $1 AND deleted_at IS NOT NULL AND deleted_at > $2", + WHERE event_id = $1 AND deleted_at IS NOT NULL AND deleted_at >= $2", + ) + .bind(auth.event_id) + .bind(q.since) + .fetch_all(&state.pool) + .await?; + + // Users hidden since the cursor (ban / uploads_hidden). Uncapped like `deleted_ids` and + // for the same reason — an eviction the client misses is unrecoverable via a later delta. + let hidden_user_ids: Vec<(Uuid,)> = sqlx::query_as( + "SELECT id FROM \"user\" + WHERE event_id = $1 AND uploads_hidden = TRUE + AND uploads_hidden_at IS NOT NULL AND uploads_hidden_at >= $2", ) .bind(auth.event_id) .bind(q.since) @@ -285,6 +311,7 @@ pub async fn feed_delta( Ok(Json(DeltaResponse { uploads, deleted_ids: deleted_ids.into_iter().map(|r| r.0).collect(), + hidden_user_ids: hidden_user_ids.into_iter().map(|r| r.0).collect(), truncated, server_time, })) diff --git a/backend/src/handlers/host.rs b/backend/src/handlers/host.rs index f4a2e48..194788b 100644 --- a/backend/src/handlers/host.rs +++ b/backend/src/handlers/host.rs @@ -149,7 +149,9 @@ pub async fn ban_user( // contradict that model, break the documented "banned guest can still download the // keepsake" flow, and be ineffective anyway (the user could just /recover a new session). sqlx::query( - "UPDATE \"user\" SET is_banned = TRUE, uploads_hidden = TRUE WHERE id = $1 AND event_id = $2", + "UPDATE \"user\" + SET is_banned = TRUE, uploads_hidden = TRUE, uploads_hidden_at = NOW() + WHERE id = $1 AND event_id = $2", ) .bind(user_id) .bind(auth.event_id) @@ -201,9 +203,12 @@ pub async fn unban_user( } // Unban restores visibility too: ban set `uploads_hidden = TRUE`, so clearing only - // `is_banned` would leave their content invisible. Clear both. + // `is_banned` would leave their content invisible. Clear all three (the timestamp too, + // so a future ban stamps a fresh `uploads_hidden_at` the reconnect delta will replay). let result = sqlx::query( - "UPDATE \"user\" SET is_banned = FALSE, uploads_hidden = FALSE WHERE id = $1 AND event_id = $2", + "UPDATE \"user\" + SET is_banned = FALSE, uploads_hidden = FALSE, uploads_hidden_at = NULL + WHERE id = $1 AND event_id = $2", ) .bind(user_id) .bind(auth.event_id) @@ -242,10 +247,11 @@ pub async fn set_role( } }; - // Look up the current role so we can apply the host-vs-admin guard. Hosts may - // promote guests and demote *other* hosts (the user explicitly requested this - // expansion). Hosts may not touch admins. Admins may do anything (except change - // themselves, blocked above). + // Look up the current role so we can apply the host-vs-admin guard. A plain host may + // promote/demote GUESTS only; it may not change any host's or admin's role (see the + // guard below — this closes the demote-a-peer-host→ban/PIN-reset takeover chain, F1). + // Only an admin may change a host's role. Admins may do anything except change + // themselves (blocked above). let target = sqlx::query_as::<_, (String,)>( "SELECT role::text FROM \"user\" WHERE id = $1 AND event_id = $2", ) @@ -356,8 +362,13 @@ pub async fn reset_user_pin( .await?; // A PIN reset means the old credential is compromised/forgotten — revoke every - // existing session so old devices must re-authenticate with the new PIN. - let _ = Session::delete_all_for_user(&state.pool, user_id).await; + // existing session so old devices must re-authenticate with the new PIN. This is a + // security-relevant revoke: if it fails, the old sessions stay valid (sessions are + // token- not PIN-bound), so surface the error in logs rather than swallowing it + // silently while reporting success to the host. + if let Err(e) = Session::delete_all_for_user(&state.pool, user_id).await { + tracing::error!(error = ?e, user_id = %user_id, "PIN reset: failed to revoke sessions"); + } // Resolve any pending in-app "I forgot my PIN" request for this user. let _ = sqlx::query("DELETE FROM pin_reset_request WHERE user_id = $1") diff --git a/backend/src/handlers/sse.rs b/backend/src/handlers/sse.rs index e049eeb..31b71f0 100644 --- a/backend/src/handlers/sse.rs +++ b/backend/src/handlers/sse.rs @@ -82,11 +82,11 @@ pub async fn stream( // The session is only checked once at open. Re-validate it periodically so a // logged-out, expired, OR banned session's stream is closed rather than kept alive - // until the client happens to disconnect. Bounds a stale stream to ~60s. Banning - // already revokes the user's sessions (so the row is gone), but re-reading the live - // user row and dropping on `is_banned` is a cheap defense-in-depth. Only a - // *definitive* gone/expired/banned state ends the stream; a transient DB error just - // retries next tick. + // until the client happens to disconnect. Bounds a stale stream to ~60s. NOTE: a ban is + // deliberately read-only and does NOT revoke sessions (see `ban_user`), so the + // `is_banned` re-read below is LOAD-BEARING — it is the only thing that stops a banned + // user's live push stream. Do not remove it. Only a *definitive* gone/expired/banned + // state ends the stream; a transient DB error just retries next tick. let pool = state.pool.clone(); let session_hash = token_hash.clone(); let session_gone = async move { diff --git a/backend/src/handlers/upload.rs b/backend/src/handlers/upload.rs index 32f62e2..34081d7 100644 --- a/backend/src/handlers/upload.rs +++ b/backend/src/handlers/upload.rs @@ -75,14 +75,19 @@ pub async fn upload( .ok_or_else(|| AppError::NotFound("Event nicht gefunden.".into()))?; if event.uploads_locked_at.is_some() { drain_multipart(multipart).await; - return Err(AppError::Forbidden("Uploads sind gesperrt.".into())); + // Reversible: a host can reopen the event, so the client keeps the queued blob and + // retries on `event-opened` rather than purging it (UploadsLocked, not Forbidden). + return Err(AppError::UploadsLocked("Uploads sind gesperrt.".into())); } // Belt-and-suspenders on top of the lock (release ⇒ lock): once the gallery is // released the export has been snapshotted, so a late upload could never make it into // the keepsake. Reject it explicitly rather than silently diverging the live feed. + // Also reversible (reopen clears `export_released_at`), so likewise UploadsLocked. if event.export_released_at.is_some() { drain_multipart(multipart).await; - return Err(AppError::Forbidden("Galerie wurde bereits freigegeben.".into())); + return Err(AppError::UploadsLocked( + "Galerie wurde bereits freigegeben.".into(), + )); } // Read config limits from DB diff --git a/backend/src/services/export.rs b/backend/src/services/export.rs index df074bc..6b82c71 100644 --- a/backend/src/services/export.rs +++ b/backend/src/services/export.rs @@ -96,30 +96,45 @@ pub async fn enqueue_and_spawn_exports( export_path: PathBuf, sse_tx: broadcast::Sender, ) -> Result<()> { - for export_type in ["zip", "html"] { - // Reset a prior 'done'/'failed' row to 'pending' so this (re)release regenerates — - // but DO NOT touch a row that's still 'running'. If a worker from an earlier - // release is mid-export, leaving it 'running' makes the worker we spawn below bail - // its claim (WHERE status='pending' → 0 rows), so the two never race the temp file. + // Only (re)generate a type that isn't ALREADY complete. A (re)release always arrives + // with both ready flags false (a fresh release starts false; `open_event` clears them + // before a re-release), so both regenerate as intended. But startup recovery of a + // half-finished export (e.g. ZIP done+ready, HTML crashed) must leave the good half + // alone — resetting it would 404 an already-downloadable keepsake until it needlessly + // regenerates. Skip the ready ones; their still-`done` rows keep their `file_path`, and + // the worker we spawn for them below simply bails its claim (no `pending` row to grab). + let (zip_ready, html_ready): (bool, bool) = sqlx::query_as( + "SELECT export_zip_ready, export_html_ready FROM event WHERE id = $1", + ) + .bind(event_id) + .fetch_one(&pool) + .await?; + + for (export_type, ready) in [("zip", zip_ready), ("html", html_ready)] { + if ready { + continue; + } + // Reset the row to a clean 'pending' and BUMP `release_seq` — unconditionally, even + // if a worker from a prior release is still 'running'. That worker captured the old + // seq at claim time; bumping it here supersedes its generation, so when it finishes + // its guarded finalize (`WHERE release_seq = `) matches nothing and it + // discards its now-stale output instead of overwriting a fresh keepsake. The worker + // we spawn below then claims this new generation and regenerates from a current + // snapshot. Per-seq temp/final paths (see `run_zip_export`/`run_html_export`) keep + // the old and new workers from racing the same file on disk. sqlx::query( - "INSERT INTO export_job (event_id, type, status, progress_pct) - VALUES ($1, $2::export_type, 'pending', 0) + "INSERT INTO export_job (event_id, type, status, progress_pct, release_seq) + VALUES ($1, $2::export_type, 'pending', 0, 1) ON CONFLICT (event_id, type) DO UPDATE SET status = 'pending', progress_pct = 0, file_path = NULL, - error_message = NULL, completed_at = NULL - WHERE export_job.status <> 'running'", + error_message = NULL, completed_at = NULL, + release_seq = export_job.release_seq + 1", ) .bind(event_id) .bind(export_type) .execute(&pool) .await?; } - // Clear readiness so /export downloads 404 until the fresh run completes, instead of - // serving a stale keepsake that predates a reopen→re-release. - sqlx::query("UPDATE event SET export_zip_ready = FALSE, export_html_ready = FALSE WHERE id = $1") - .bind(event_id) - .execute(&pool) - .await?; spawn_export_jobs(event_id, event_name, pool, media_path, export_path, sse_tx); Ok(()) @@ -182,9 +197,10 @@ pub fn spawn_export_jobs( let event_name2 = event_name.clone(); tokio::spawn(async move { + // Failure marking happens INSIDE the worker (guarded by the claimed `release_seq`), + // so a superseded worker's error can't clobber the fresh generation's row. if let Err(e) = run_zip_export(event_id, &pool, &media_path, &export_path, &sse_tx).await { tracing::error!("ZIP export failed for event {event_id}: {e:#}"); - mark_failed(&pool, event_id, "zip", &e.to_string()).await; } maybe_broadcast_complete(&pool, event_id, &sse_tx).await; }); @@ -194,7 +210,6 @@ pub fn spawn_export_jobs( run_html_export(event_id, &event_name2, &pool2, &media_path2, &export_path2, &sse_tx2).await { tracing::error!("HTML export failed for event {event_id}: {e:#}"); - mark_failed(&pool2, event_id, "html", &e.to_string()).await; } maybe_broadcast_complete(&pool2, event_id, &sse_tx2).await; }); @@ -209,11 +224,29 @@ async fn run_zip_export( export_path: &Path, sse_tx: &broadcast::Sender, ) -> Result<()> { - if !claim_job(pool, event_id, "zip").await { + let seq = match claim_job(pool, event_id, "zip").await { + Some(seq) => seq, // Another worker already owns this ZIP export — bail rather than race the temp file. - return Ok(()); - } + None => return Ok(()), + }; + // Run the body under the captured `seq`; on error mark THIS generation failed (a no-op + // if a re-release already superseded us). + let res = run_zip_export_inner(seq, event_id, pool, media_path, export_path, sse_tx).await; + if let Err(e) = &res { + mark_failed(pool, event_id, "zip", seq, &e.to_string()).await; + } + res +} + +async fn run_zip_export_inner( + seq: i64, + event_id: Uuid, + pool: &PgPool, + media_path: &Path, + export_path: &Path, + sse_tx: &broadcast::Sender, +) -> Result<()> { let uploads = query_uploads(pool, event_id).await?; let total = uploads.len().max(1) as f32; @@ -221,8 +254,13 @@ async fn run_zip_export( let exports_dir = export_path.to_path_buf(); tokio::fs::create_dir_all(&exports_dir).await?; - let tmp_path = exports_dir.join("Gallery.zip.tmp"); - let out_path = exports_dir.join("Gallery.zip"); + // Per-generation paths: a superseded worker (older `seq`) and the fresh worker never + // share a file on disk, so neither can truncate or interleave the other's bytes. The + // final name embeds the seq too; the download handler serves whatever `file_path` the + // current 'done' row points at, so an orphaned older generation is never served. + let tmp_path = exports_dir.join(format!("Gallery.zip.{seq}.tmp")); + let out_name = format!("Gallery.{seq}.zip"); + let out_path = exports_dir.join(&out_name); { let file = tokio::fs::File::create(&tmp_path).await?; @@ -247,7 +285,7 @@ async fn run_zip_export( entry.close().await?; let pct = ((i + 1) as f32 / total * 100.0) as i16; - update_progress(pool, event_id, "zip", pct.min(99)).await; + update_progress(pool, event_id, "zip", seq, pct.min(99)).await; } zip.close().await?; @@ -255,19 +293,31 @@ async fn run_zip_export( tokio::fs::rename(&tmp_path, &out_path).await?; + // Guarded finalize: commit ONLY if our generation is still current. If a reopen→ + // re-release bumped `release_seq` while we were streaming, we lost — discard our (now + // stale) archive and let the fresh worker own the keepsake. This is the fix for the + // stale-keepsake data loss: a superseded worker can no longer flip `export_zip_ready`. + if !finalize_job(pool, event_id, "zip", seq, &format!("exports/{out_name}")).await { + let _ = tokio::fs::remove_file(&out_path).await; + tracing::info!("ZIP export for event {event_id} superseded by a newer release; discarded"); + return Ok(()); + } + + // Flip the ready flag only while still current (a re-release between finalize and here + // would have cleared it; don't resurrect it for a superseded generation). sqlx::query( - "UPDATE export_job SET status = 'done', progress_pct = 100, file_path = $2, completed_at = NOW() - WHERE event_id = $1 AND type = 'zip'::export_type", + "UPDATE event SET export_zip_ready = TRUE + WHERE id = $1 + AND EXISTS (SELECT 1 FROM export_job + WHERE event_id = $1 AND type = 'zip'::export_type + AND release_seq = $2 AND status = 'done')", ) .bind(event_id) - .bind("exports/Gallery.zip") + .bind(seq) .execute(pool) .await?; - sqlx::query("UPDATE event SET export_zip_ready = TRUE WHERE id = $1") - .bind(event_id) - .execute(pool) - .await?; + prune_stale_export_files(&exports_dir, "Gallery", event_id, seq).await; let _ = sse_tx.send(SseEvent { event_type: "export-progress".to_string(), @@ -305,25 +355,43 @@ async fn run_html_export( export_path: &Path, sse_tx: &broadcast::Sender, ) -> Result<()> { - if !claim_job(pool, event_id, "html").await { + let seq = match claim_job(pool, event_id, "html").await { + Some(seq) => seq, // Another worker already owns this HTML export — bail rather than race the temp dir. - return Ok(()); - } + None => return Ok(()), + }; + let res = + run_html_export_inner(seq, event_id, event_name, pool, media_path, export_path, sse_tx).await; + if let Err(e) = &res { + mark_failed(pool, event_id, "html", seq, &e.to_string()).await; + } + res +} + +async fn run_html_export_inner( + seq: i64, + event_id: Uuid, + event_name: &str, + pool: &PgPool, + media_path: &Path, + export_path: &Path, + sse_tx: &broadcast::Sender, +) -> Result<()> { // 1. Query data let uploads = query_uploads(pool, event_id).await?; let comments = query_comments(pool, event_id).await?; let hashtags_per_upload = query_hashtags(pool, event_id).await?; let total = uploads.len().max(1) as f32; - update_progress(pool, event_id, "html", 5).await; + update_progress(pool, event_id, "html", seq, 5).await; // Written OUTSIDE media_path: the public /media ServeDir must never reach these. let exports_dir = export_path.to_path_buf(); tokio::fs::create_dir_all(&exports_dir).await?; - // 2. Create temp directory for media processing - let tmp_dir = exports_dir.join(format!("viewer_tmp_{event_id}")); + // 2. Create temp directory for media processing (per-generation — see run_zip_export). + let tmp_dir = exports_dir.join(format!("viewer_tmp_{event_id}_{seq}")); let media_tmp = tmp_dir.join("media"); tokio::fs::create_dir_all(&media_tmp).await?; @@ -485,7 +553,7 @@ async fn run_html_export( }); let pct = 10 + ((i + 1) as f32 / total * 60.0) as i16; - update_progress(pool, event_id, "html", pct.min(69)).await; + update_progress(pool, event_id, "html", seq, pct.min(69)).await; } // 4. Build data.json @@ -499,11 +567,12 @@ async fn run_html_export( let data_json = serde_json::to_string_pretty(&viewer_data).context("failed to serialize data.json")?; - update_progress(pool, event_id, "html", 72).await; + update_progress(pool, event_id, "html", seq, 72).await; - // 5. Create ZIP - let tmp_path = exports_dir.join("Memories.zip.tmp"); - let out_path = exports_dir.join("Memories.zip"); + // 5. Create ZIP (per-generation paths — see run_zip_export) + let tmp_path = exports_dir.join(format!("Memories.zip.{seq}.tmp")); + let out_name = format!("Memories.{seq}.zip"); + let out_path = exports_dir.join(&out_name); { let file = tokio::fs::File::create(&tmp_path).await?; @@ -512,7 +581,7 @@ async fn run_html_export( // Write embedded viewer assets (index.html, _app/*, etc.) write_dir_to_zip(&VIEWER_DIR, &mut zip).await?; - update_progress(pool, event_id, "html", 75).await; + update_progress(pool, event_id, "html", seq, 75).await; // Write data.json { @@ -532,7 +601,7 @@ async fn run_html_export( entry.close().await?; } - update_progress(pool, event_id, "html", 78).await; + update_progress(pool, event_id, "html", seq, 78).await; // Write media files from the manifest built in step 3. Thumbnails and derived // image variants stream from temp; video and small-image full variants stream @@ -556,7 +625,7 @@ async fn run_html_export( files_written += 1; let pct = 78 + (files_written as f32 / file_total * 20.0) as i16; - update_progress(pool, event_id, "html", pct.min(98)).await; + update_progress(pool, event_id, "html", seq, pct.min(98)).await; } zip.close().await?; @@ -568,19 +637,26 @@ async fn run_html_export( // Clean up temp directory let _ = tokio::fs::remove_dir_all(&tmp_dir).await; + // Guarded finalize — discard if a re-release superseded us mid-export (see run_zip_export). + if !finalize_job(pool, event_id, "html", seq, &format!("exports/{out_name}")).await { + let _ = tokio::fs::remove_file(&out_path).await; + tracing::info!("HTML export for event {event_id} superseded by a newer release; discarded"); + return Ok(()); + } + sqlx::query( - "UPDATE export_job SET status = 'done', progress_pct = 100, file_path = $2, completed_at = NOW() - WHERE event_id = $1 AND type = 'html'::export_type", + "UPDATE event SET export_html_ready = TRUE + WHERE id = $1 + AND EXISTS (SELECT 1 FROM export_job + WHERE event_id = $1 AND type = 'html'::export_type + AND release_seq = $2 AND status = 'done')", ) .bind(event_id) - .bind("exports/Memories.zip") + .bind(seq) .execute(pool) .await?; - sqlx::query("UPDATE event SET export_html_ready = TRUE WHERE id = $1") - .bind(event_id) - .execute(pool) - .await?; + prune_stale_export_files(&exports_dir, "Memories", event_id, seq).await; let _ = sse_tx.send(SseEvent { event_type: "export-progress".to_string(), @@ -640,45 +716,138 @@ async fn query_hashtags(pool: &PgPool, event_id: Uuid) -> Result bool { - sqlx::query( +/// Atomically claim a pending export job for this worker and return the generation +/// (`release_seq`) it claimed. Returns `Some(seq)` only if we won (status flipped +/// `pending`→`running` in one statement); `None` when another worker already owns it — e.g. +/// a reopen→re-release spawned a second worker while a run from a prior release is still in +/// flight. The row-level lock serializes the two UPDATEs, so exactly one sees +/// `status = 'pending'`. The caller threads the returned seq into per-generation temp/final +/// paths and into the guarded finalize, so a superseded worker never corrupts or resurrects +/// a stale keepsake. +async fn claim_job(pool: &PgPool, event_id: Uuid, export_type: &str) -> Option { + sqlx::query_as::<_, (i64,)>( "UPDATE export_job SET status = 'running' - WHERE event_id = $1 AND type = $2::export_type AND status = 'pending'", + WHERE event_id = $1 AND type = $2::export_type AND status = 'pending' + RETURNING release_seq", ) .bind(event_id) .bind(export_type) + .fetch_optional(pool) + .await + .ok() + .flatten() + .map(|(seq,)| seq) +} + +/// Guarded finalize: mark this export `done` and record its file path ONLY if our +/// generation is still current (`release_seq = seq` and still `running`). Returns `true` if +/// we won the finalize, `false` if a re-release superseded us mid-export (in which case the +/// caller must discard its output — the fresh worker owns the keepsake now). Atomic, so it +/// can't race a concurrent re-release. +async fn finalize_job( + pool: &PgPool, + event_id: Uuid, + export_type: &str, + seq: i64, + file_path: &str, +) -> bool { + sqlx::query( + "UPDATE export_job + SET status = 'done', progress_pct = 100, file_path = $3, completed_at = NOW() + WHERE event_id = $1 AND type = $2::export_type + AND release_seq = $4 AND status = 'running'", + ) + .bind(event_id) + .bind(export_type) + .bind(file_path) + .bind(seq) .execute(pool) .await .map(|r| r.rows_affected() > 0) .unwrap_or(false) } -async fn mark_failed(pool: &PgPool, event_id: Uuid, export_type: &str, msg: &str) { +/// Parse the trailing generation number out of `` (e.g. +/// `Gallery..zip`, `Gallery.zip..tmp`, `viewer_tmp__`). Returns None if the +/// name doesn't fit the shape, so unrelated files are left untouched. +fn parse_gen_seq(name: &str, prefix: &str, suffix: &str) -> Option { + name.strip_prefix(prefix)?.strip_suffix(suffix)?.parse::().ok() +} + +/// Best-effort removal of stale per-generation export artifacts for one export type. Deletes +/// ONLY strictly-older generations (`n < keep_seq`) — never `keep_seq`'s own current file, +/// and never a NEWER generation that a concurrent re-release may already be producing (that +/// "delete all but mine" would let a lagging older winner nuke a newer live file). Covers the +/// finished archive (`..zip`), its temp (`.zip..tmp`), and — for html — +/// the `viewer_tmp__` staging dir, so crash-orphaned per-generation temps don't +/// accumulate. Runs after a worker wins its finalize; older generations can never be served +/// again (their `file_path` was nulled by the re-release that superseded them). Tolerates +/// races and IO errors — purely disk hygiene. +async fn prune_stale_export_files(exports_dir: &Path, prefix: &str, event_id: Uuid, keep_seq: i64) { + let final_prefix = format!("{prefix}."); // Gallery. / Memories. + let temp_prefix = format!("{prefix}.zip."); // Gallery.zip. / Memories.zip. + let viewer_prefix = format!("viewer_tmp_{event_id}_"); + let mut rd = match tokio::fs::read_dir(exports_dir).await { + Ok(rd) => rd, + Err(_) => return, + }; + while let Ok(Some(entry)) = rd.next_entry().await { + let name = entry.file_name(); + let name = name.to_string_lossy(); + // Try each artifact shape; a strictly-older seq in any of them marks it for deletion. + // Check the temp shape before the final shape: `Gallery.zip..tmp` also starts with + // `Gallery.` but isn't a `.zip`, so its final-shape parse returns None anyway. + let seq = parse_gen_seq(&name, &temp_prefix, ".tmp") + .or_else(|| parse_gen_seq(&name, &final_prefix, ".zip")) + .or_else(|| { + if prefix == "Memories" { + parse_gen_seq(&name, &viewer_prefix, "") + } else { + None + } + }); + if seq.is_some_and(|n| n < keep_seq) { + let path = entry.path(); + let is_dir = entry.file_type().await.map(|t| t.is_dir()).unwrap_or(false); + let _ = if is_dir { + tokio::fs::remove_dir_all(&path).await + } else { + tokio::fs::remove_file(&path).await + }; + } + } +} + +/// Mark this worker's export `failed` — but ONLY for the generation it was running +/// (`release_seq = seq` and still `running`). Without the seq guard, a superseded worker +/// erroring out after a re-release would clobber the FRESH worker's `running`/`pending` row +/// to `failed`, stranding the new keepsake. A superseded worker's failure is a no-op here. +async fn mark_failed(pool: &PgPool, event_id: Uuid, export_type: &str, seq: i64, msg: &str) { let _ = sqlx::query( "UPDATE export_job SET status = 'failed', error_message = $3 - WHERE event_id = $1 AND type = $2::export_type", + WHERE event_id = $1 AND type = $2::export_type + AND release_seq = $4 AND status = 'running'", ) .bind(event_id) .bind(export_type) .bind(msg) + .bind(seq) .execute(pool) .await; } -async fn update_progress(pool: &PgPool, event_id: Uuid, export_type: &str, pct: i16) { +/// Update the progress bar for THIS generation only (`release_seq = seq`). Seq-guarded so a +/// superseded worker still streaming can't overwrite the fresh generation's progress and make +/// the bar jump backwards. Cosmetic, but keeps the displayed percent monotonic per release. +async fn update_progress(pool: &PgPool, event_id: Uuid, export_type: &str, seq: i64, pct: i16) { let _ = sqlx::query( - "UPDATE export_job SET progress_pct = $3 WHERE event_id = $1 AND type = $2::export_type", + "UPDATE export_job SET progress_pct = $3 + WHERE event_id = $1 AND type = $2::export_type AND release_seq = $4", ) .bind(event_id) .bind(export_type) .bind(pct) + .bind(seq) .execute(pool) .await; } diff --git a/docs/USER_JOURNEYS.md b/docs/USER_JOURNEYS.md index ae70f27..e738a76 100644 --- a/docs/USER_JOURNEYS.md +++ b/docs/USER_JOURNEYS.md @@ -132,15 +132,20 @@ the Host can clean up later). - **Sperren** opens a confirmation modal. Banning **always hides** the user's existing uploads (a banned user's content is "gone" everywhere) — there is no opt-out. Submitting calls `POST /host/users/{id}/ban` (no body). - - **Entsperren** lifts the ban. - - **Host** promotes a guest to host. - - **Degradieren** — visible on Host rows. A Host can demote *other* Hosts back to - guest (planned). The button is hidden on the Host's own row to prevent self-lockout; - only an Admin can demote themselves out of moderation. Admins see Degradieren on - every Host row. - - **PIN zurücksetzen** (planned) — generates a new PIN and shows it once in a modal. - See journey §4. Hosts see this on Guest rows only; Admins see it on Guest + Host - rows. + - **Entsperren** lifts the ban. Same authority boundary as ban (below): a plain Host may + only unban Guests; only an Admin may unban a Host. + - **Host** promotes a guest to host (Hosts and Admins may do this). + - **Degradieren** — demote a Host back to guest. **Only an Admin may change a Host's + role.** A plain Host may *not* demote a peer Host: doing so would let them then ban or + PIN-reset (→ `/recover` account-takeover) that ex-peer, since those guards key off the + target's *current* role. So the backend rejects it (403) and the button is hidden for + non-admin Hosts. Nobody may change their own role (self-lockout / self-escalation guard), + and Admins are un-demotable and un-bannable by anyone. (This tightens an earlier + "Hosts may demote other Hosts" design — see the F1 security fix.) + - **Sperren / PIN zurücksetzen** — a plain Host may act on Guests only; an Admin may act on + Guests + Hosts; nobody may act on an Admin. The buttons are hidden where they'd 403. + PIN reset generates a new PIN, shows it once in a modal, and revokes the target's + existing sessions (forcing re-auth with the new PIN). See journey §4. 6. **Deleting content** — Host can delete any upload or comment via the moderation routes (`DELETE /host/upload/{id}`, `DELETE /host/comment/{id}`). On mobile this is also reachable by long-pressing the content (planned, see §15). diff --git a/e2e/specs/01-auth/admin-login.spec.ts b/e2e/specs/01-auth/admin-login.spec.ts index 9eb0d01..dc62e19 100644 --- a/e2e/specs/01-auth/admin-login.spec.ts +++ b/e2e/specs/01-auth/admin-login.spec.ts @@ -13,17 +13,24 @@ test.describe('Auth — admin login', () => { await login.login(ADMIN_PASSWORD); await page.waitForURL('**/admin', { timeout: 10_000 }); - // Admin JWT should now be in localStorage (admin uses the same key, just with role=admin in the payload) - const role = await page.evaluate(() => { - const token = localStorage.getItem('eventsnap_jwt'); - if (!token) return null; - try { - return JSON.parse(atob(token.split('.')[1])).role; - } catch { - return null; - } + // The admin JWT lands in sessionStorage (USER_JOURNEYS §11.1) — NOT localStorage — so the + // elevated credential doesn't survive a tab/browser close on a shared "event laptop". + const stores = await page.evaluate(() => { + const decodeRole = (t: string | null) => { + if (!t) return null; + try { + return JSON.parse(atob(t.split('.')[1])).role; + } catch { + return null; + } + }; + return { + sessionRole: decodeRole(sessionStorage.getItem('eventsnap_jwt')), + localToken: localStorage.getItem('eventsnap_jwt') + }; }); - expect(role).toBe('admin'); + expect(stores.sessionRole).toBe('admin'); + expect(stores.localToken, 'admin token must not persist in localStorage').toBeNull(); }); test('wrong password → error, no token written', async ({ page }) => { diff --git a/e2e/specs/10-flow-review/ban-replay.spec.ts b/e2e/specs/10-flow-review/ban-replay.spec.ts new file mode 100644 index 0000000..b8553a5 --- /dev/null +++ b/e2e/specs/10-flow-review/ban-replay.spec.ts @@ -0,0 +1,58 @@ +/** + * Regression guard — a ban must replay in the reconnect delta so a client that missed the + * ephemeral `user-hidden` SSE (most acutely the unattended diashow projector) still evicts + * the banned user's already-loaded slides instead of cycling them all night. + * + * A ban is NOT a soft-delete (no `upload.deleted_at`), so it never appears in the delta's + * `deleted_ids`. The fix: `uploads_hidden_at` stamps when a user became hidden, and + * `/feed/delta` returns `hidden_user_ids` for users hidden since the client's cursor. The + * client (feed + diashow) evicts all uploads from those users. + */ +import { test, expect } from '../../fixtures/test'; +import { seedUpload } from '../../helpers/seed'; + +const BASE = process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101'; + +async function feedDelta(jwt: string, since: string): Promise { + const res = await fetch(`${BASE}/api/v1/feed/delta?since=${encodeURIComponent(since)}`, { + headers: { Authorization: `Bearer ${jwt}` }, + }); + expect(res.status).toBe(200); + return res.json(); +} + +test.describe('Realtime — ban replays in the reconnect delta (H1 audit fix)', () => { + test('a ban since the cursor is in hidden_user_ids; a ban before it is not', async ({ + host, + guest, + api, + }) => { + const g = await guest('BanReplayGuest'); + await seedUpload(g.jwt, { caption: 'to be hidden' }); + + // Cursor BEFORE the ban — this is the "last-seen" point a disconnected client holds. + const before = await feedDelta(host.jwt, '2000-01-01T00:00:00Z'); + const cursorBefore: string = before.server_time; + expect(before.hidden_user_ids).not.toContain(g.userId); // not hidden yet + + // Ban the guest (read-only ban; content hidden everywhere). + await api.banUser(host.jwt, g.userId); + + // A delta from the pre-ban cursor MUST surface the ban so the client can evict. + const afterBan = await feedDelta(host.jwt, cursorBefore); + expect(afterBan.hidden_user_ids, 'ban replayed to a client that missed the live event').toContain( + g.userId + ); + + // And a delta from a cursor AFTER the ban must NOT re-surface it (the `>= since` filter + // means a client already past the ban isn't told again forever). + const cursorAfter: string = afterBan.server_time; + const afterCursor = await feedDelta(host.jwt, cursorAfter); + expect(afterCursor.hidden_user_ids).not.toContain(g.userId); + + // Unban clears the timestamp, so a fresh ban later would replay again. + await api.unbanUser(host.jwt, g.userId); + const afterUnban = await feedDelta(host.jwt, '2000-01-01T00:00:00Z'); + expect(afterUnban.hidden_user_ids).not.toContain(g.userId); + }); +}); 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 b956c9c..949a93c 100644 --- a/e2e/specs/10-flow-review/export-reopen-rerelease.spec.ts +++ b/e2e/specs/10-flow-review/export-reopen-rerelease.spec.ts @@ -1,36 +1,37 @@ /** - * Re-review regression guard — Chain #2 ("export corruption on reopen→re-release"). + * Regression guard — reopen→re-release export integrity + the stale-keepsake data-loss fix (H1). * - * Bug that was fixed: `spawn_export_jobs` ran the worker unconditionally and the - * worker wrote a FIXED temp path (`Gallery.zip.tmp` / `viewer_tmp_{event}`). - * The new reopen→re-release flow could therefore spawn a second worker while the - * first was still running — two workers stomping the same temp file → a corrupt - * keepsake ZIP served to guests. + * Two related bugs on this path: * - * The fix: `claim_job` is an atomic `UPDATE ... WHERE status='pending'`; a worker - * that doesn't win the claim bails without touching the temp file. The re-enqueue - * `ON CONFLICT DO UPDATE ... WHERE status <> 'running'` leaves an in-flight job - * alone. Net: exactly one worker per (event,type). + * (1) Temp-file race: a second worker spawned by a re-release could stomp the first + * worker's FIXED temp path → a corrupt/truncated keepsake ZIP. * - * This test seeds real uploads, releases, then churns reopen→re-release (stressing - * the claim guard), waits for the export to settle, and asserts the produced ZIP is - * INTACT (passes `unzip -t`) with exactly one entry per upload — plus that no - * export_job row is left stuck `running`. A temp-file race would corrupt/truncate - * the archive or drop entries, failing these assertions. + * (2) Stale keepsake (silent data loss): a worker from the PRIOR release, still streaming a + * snapshot taken BEFORE a reopen, would finish and unconditionally re-flip + * `export_*_ready = TRUE` on an archive that predated the reopen — so uploads added + * during the reopen window were permanently missing from a "ready" keepsake. * - * Note on scope: a Dockerised export over small fixtures completes in well under a - * second, so this cannot *deterministically* force two workers to overlap in time. - * It stresses the claim/ON-CONFLICT guard under rapid churn and hard-checks archive - * integrity; the atomic single-worker guarantee itself is additionally proven by - * code review + SQL PREPARE. Together they cover the regression. + * The fix: a per-(event,type) generation counter `release_seq`, bumped on every (re)release. + * A worker captures the seq it claimed and writes per-generation temp/final paths + * (`Gallery..zip`); its finalize and ready-flag flip are guarded by `release_seq`, so a + * superseded worker discards its output instead of resurrecting a stale keepsake. The + * download follows the current `done` row's `file_path`, never a fixed name. + * + * Coverage: + * - churn integrity: rapid reopen→re-release yields exactly one INTACT ZIP, no stuck jobs. + * - completeness: an upload added during the reopen window IS present in the re-released + * keepsake (the direct data-loss regression). + * - supersession: a re-release SUPERSEDES an in-flight (running) job — it bumps the + * generation and regenerates, rather than leaving the stale worker to finalize. */ import { test, expect } from '../../fixtures/test'; import { Client } from 'pg'; import { execFileSync } from 'node:child_process'; -import { writeFileSync, mkdtempSync } from 'node:fs'; +import { writeFileSync, mkdtempSync, readFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { seedUpload } from '../../helpers/seed'; +import { uploadRaw } from '../../helpers/upload-client'; const BASE = process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101'; const SLUG = 'e2e-test-event'; @@ -68,9 +69,43 @@ async function mintTicket(jwt: string): Promise { return (await res.json()).ticket; } -test.describe('Flow re-review — reopen→re-release export integrity (#2)', () => { +async function waitExportDone(jwt: string) { + await expect + .poll(async () => { + const s = await exportStatus(jwt); + return s.released === true && s.zip?.status === 'done' && s.html?.status === 'done'; + }, { timeout: 60_000, intervals: [500] }) + .toBe(true); +} + +/** Download the current ZIP keepsake and return its (non-directory) entry names. */ +async function downloadZipEntries(jwt: string): Promise { + const ticket = await mintTicket(jwt); + const res = await fetch(BASE + '/api/v1/export/zip?ticket=' + encodeURIComponent(ticket)); + expect(res.status).toBe(200); + const bytes = Buffer.from(await res.arrayBuffer()); + expect(bytes.subarray(0, 2).toString('latin1')).toBe('PK'); // ZIP magic — not a stomped file + const dir = mkdtempSync(join(tmpdir(), 'es-zip-')); + const zipPath = join(dir, 'Gallery.zip'); + writeFileSync(zipPath, bytes); + execFileSync('unzip', ['-t', zipPath], { stdio: 'pipe' }); // CRC integrity; throws on corruption + return execFileSync('unzip', ['-Z1', zipPath], { encoding: 'utf8' }) + .split('\n') + .map((l) => l.trim()) + .filter((l) => l.length > 0 && !l.endsWith('/')); +} + +async function zipReleaseSeq(): Promise { + const [row] = await pgQuery<{ release_seq: string }>( + `SELECT ej.release_seq FROM export_job ej JOIN event e ON e.id = ej.event_id + WHERE e.slug = '${SLUG}' AND ej.type = 'zip'` + ); + return Number(row.release_seq); +} + +test.describe('Flow re-review — reopen→re-release export integrity + stale-keepsake (H1)', () => { // The real export job runs image processing; give it head-room over the tiny fixtures. - test.setTimeout(90_000); + test.setTimeout(120_000); test('rapid release/reopen/re-release yields exactly one INTACT ZIP and no stuck jobs', async ({ host, @@ -84,8 +119,6 @@ test.describe('Flow re-review — reopen→re-release export integrity (#2)', () expect((await post('/api/v1/host/gallery/release', host.jwt)).status).toBe(204); // Post-release uploads must be rejected (belt-and-suspenders on top of the lock). - const { uploadRaw } = await import('../../helpers/upload-client'); - const { readFileSync } = await import('node:fs'); const rejected = await uploadRaw(host.jwt, readFileSync(join(process.cwd(), 'fixtures', 'media', 'sample.jpg')), { filename: 'late.jpg', contentType: 'image/jpeg', @@ -101,16 +134,9 @@ test.describe('Flow re-review — reopen→re-release export integrity (#2)', () expect(rel.status).toBe(204); } - // Wait for the export to settle: both jobs done and the ZIP marked ready. - await expect - .poll(async () => { - const s = await exportStatus(host.jwt); - return s.released === true && s.zip?.status === 'done' && s.html?.status === 'done'; - }, { timeout: 60_000, intervals: [500] }) - .toBe(true); + await waitExportDone(host.jwt); - // No export_job may be left stuck 'running' (would mean a worker that never - // completed — the failure mode the claim guard exists to avoid). + // No export_job may be left stuck 'running'. const jobs = await pgQuery<{ type: string; status: string }>( `SELECT ej.type::text AS type, ej.status::text AS status FROM export_job ej JOIN event e ON e.id = ej.event_id @@ -119,80 +145,74 @@ test.describe('Flow re-review — reopen→re-release export integrity (#2)', () expect(jobs.length).toBe(2); for (const j of jobs) expect(j.status, `${j.type} job status`).toBe('done'); - // Download the ZIP and prove it is INTACT — a temp-file race would corrupt or - // truncate it. `unzip -t` verifies every entry's CRC; a non-zero exit throws. - const ticket = await mintTicket(host.jwt); - const res = await fetch(BASE + '/api/v1/export/zip?ticket=' + encodeURIComponent(ticket)); - expect(res.status).toBe(200); - const bytes = Buffer.from(await res.arrayBuffer()); - // ZIP local-file-header magic — a stomped/half-written file fails this immediately. - expect(bytes.subarray(0, 2).toString('latin1')).toBe('PK'); - - const dir = mkdtempSync(join(tmpdir(), 'es-zip-')); - const zipPath = join(dir, 'Gallery.zip'); - writeFileSync(zipPath, bytes); - - // Integrity: `unzip -t` exits 0 only if all CRCs check out and the archive is whole. - execFileSync('unzip', ['-t', zipPath], { stdio: 'pipe' }); - - // Completeness: exactly one entry per seeded upload — no entry lost to a stomped - // temp file, none duplicated by a second worker. - const listing = execFileSync('unzip', ['-Z1', zipPath], { encoding: 'utf8' }) - .split('\n') - .map((l) => l.trim()) - .filter((l) => l.length > 0 && !l.endsWith('/')); + // Exactly one entry per seeded upload — no entry lost to a stomped temp file, none + // duplicated by a second worker. + const listing = await downloadZipEntries(host.jwt); expect(listing.length, `zip entries:\n${listing.join('\n')}`).toBe(N_UPLOADS); }); - test('re-release does NOT disturb an in-flight (running) export job — the anti-race guard', async ({ + test('an upload added during the reopen window IS present in the re-released keepsake (H1)', async ({ host, }) => { - // Deterministic proof of the fix, independent of export timing. We simulate a worker - // that is still mid-export by pinning the zip job to `running` with a sentinel - // progress value, then drive reopen→re-release. The fix's - // ON CONFLICT ... DO UPDATE ... WHERE export_job.status <> 'running' - // must leave that row untouched (and the freshly-spawned worker's - // claim_job: UPDATE ... WHERE status = 'pending' - // finds nothing to claim, so it bails without racing the temp file). + // Release an initial keepsake of 3 uploads. + for (let i = 0; i < 3; i++) await seedUpload(host.jwt, { caption: `orig ${i}` }); + expect((await post('/api/v1/host/gallery/release', host.jwt)).status).toBe(204); + await waitExportDone(host.jwt); + expect((await downloadZipEntries(host.jwt)).length).toBe(3); + + // Reopen (invalidates the release) → add a 4th upload while unlocked → re-release. + expect((await post('/api/v1/host/event/open', host.jwt)).status).toBe(204); + await seedUpload(host.jwt, { caption: 'added-during-reopen' }); + expect((await post('/api/v1/host/gallery/release', host.jwt)).status).toBe(204); + await waitExportDone(host.jwt); + + // The re-released keepsake MUST contain all 4 — the stale-keepsake bug would have left + // the download at the pre-reopen snapshot of 3, silently dropping the new upload. + const listing = await downloadZipEntries(host.jwt); + expect(listing.length, `zip entries:\n${listing.join('\n')}`).toBe(4); + }); + + test('re-release SUPERSEDES an in-flight (running) export — bumps the generation', async ({ + host, + }) => { + // Deterministic proof of the generation guard, independent of export timing. We pin the + // zip job to `running` with a sentinel to mimic a worker from a prior release that is + // still mid-export, then drive reopen→re-release. // - // On the OLD code (unconditional ON CONFLICT DO UPDATE) the row would be reset to - // pending/progress=0 and a second worker would claim and run concurrently — so the - // sentinel below would be wiped. This assertion therefore fails on the bug and passes - // on the fix. + // OLD behavior (the bug): the re-enqueue left a `running` row alone, so the stale worker + // would eventually finalize and re-flip the ready flag on its pre-reopen snapshot. + // NEW behavior (the fix): the re-enqueue bumps `release_seq` and resets the row, so the + // stale generation is superseded — its sentinel is wiped and a fresh worker regenerates. await seedUpload(host.jwt, { caption: 'a' }); await seedUpload(host.jwt, { caption: 'b' }); - // Release and let the first export fully finish so no real worker is active. expect((await post('/api/v1/host/gallery/release', host.jwt)).status).toBe(204); - await expect - .poll(async () => { - const s = await exportStatus(host.jwt); - return s.zip?.status === 'done' && s.html?.status === 'done'; - }, { timeout: 60_000, intervals: [500] }) - .toBe(true); + await waitExportDone(host.jwt); + const seqBefore = await zipReleaseSeq(); - // Simulate an in-flight worker owning the zip job, with a sentinel we can check. + // Simulate an in-flight worker owning the zip job at the current generation. await pgQuery( `UPDATE export_job ej SET status = 'running', progress_pct = 77 FROM event e WHERE e.id = ej.event_id AND e.slug = '${SLUG}' AND ej.type = 'zip'` ); - // Reopen (clears release + ready flags, does NOT touch job rows) then re-release. + // Reopen (clears release + ready flags; does NOT touch job rows) then re-release. expect((await post('/api/v1/host/event/open', host.jwt)).status).toBe(204); expect((await post('/api/v1/host/gallery/release', host.jwt)).status).toBe(204); - // Give any spawned worker a beat to attempt (and, correctly, bail) its claim. - await new Promise((r) => setTimeout(r, 750)); - - // The guard must have preserved the 'running' sentinel — untouched by both the - // re-enqueue and the bailing worker. - const [row] = await pgQuery<{ status: string; progress_pct: number }>( - `SELECT ej.status::text AS status, ej.progress_pct + // The fresh generation must settle to done at a HIGHER release_seq — proving the stale + // running row was superseded (not left to finalize) and the sentinel is gone. + await waitExportDone(host.jwt); + const [row] = await pgQuery<{ status: string; progress_pct: number; release_seq: string }>( + `SELECT ej.status::text AS status, ej.progress_pct, ej.release_seq FROM export_job ej JOIN event e ON e.id = ej.event_id WHERE e.slug = '${SLUG}' AND ej.type = 'zip'` ); - expect(row.status, 'zip job status').toBe('running'); - expect(Number(row.progress_pct), 'zip job sentinel progress').toBe(77); + expect(row.status, 'zip job status').toBe('done'); + expect(Number(row.progress_pct), 'zip job progress').toBe(100); + expect(Number(row.release_seq), 'release_seq advanced past the superseded generation').toBeGreaterThan( + seqBefore + ); }); }); diff --git a/e2e/specs/10-flow-review/upload-lock-code.spec.ts b/e2e/specs/10-flow-review/upload-lock-code.spec.ts new file mode 100644 index 0000000..0110565 --- /dev/null +++ b/e2e/specs/10-flow-review/upload-lock-code.spec.ts @@ -0,0 +1,54 @@ +/** + * Regression guard — a locked/released event rejects uploads with the DISTINCT error code + * `uploads_locked` (not the generic `forbidden`), so the offline upload queue can tell this + * REVERSIBLE 403 apart from a permanent one (banned / quota). On `uploads_locked` the client + * KEEPS the queued blob and retries when the host reopens; a permanent 403 purges it. Before + * this, a photo staged during a lock was purged and lost the moment the host reopened. + */ +import { test, expect } from '../../fixtures/test'; +import { uploadRaw } from '../../helpers/upload-client'; +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; + +const SAMPLE = () => readFileSync(join(process.cwd(), 'fixtures', 'media', 'sample.jpg')); + +test.describe('Upload — locked event uses a distinct, reversible 403 (audit fix)', () => { + test('closed event → 403 uploads_locked; reopen → upload succeeds', async ({ host, api }) => { + // Close the event: uploads are locked for everyone. + await api.closeEvent(host.jwt); + + const locked = await uploadRaw(host.jwt, SAMPLE(), { + filename: 'during-lock.jpg', + contentType: 'image/jpeg', + }); + expect(locked.status).toBe(403); + const lockedBody = await locked.json(); + // The distinct code is what tells the client to KEEP the blob (reversible), not purge it. + expect(lockedBody.error).toBe('uploads_locked'); + + // Reopen → the same upload now goes through (the queued blob would have survived). + await api.openEvent(host.jwt); + const ok = await uploadRaw(host.jwt, SAMPLE(), { + filename: 'after-reopen.jpg', + contentType: 'image/jpeg', + }); + expect(ok.status, 'upload succeeds once the host reopens').toBeLessThan(300); + }); + + test('released gallery → 403 uploads_locked (also reversible via reopen)', async ({ host }) => { + const BASE = process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101'; + const rel = await fetch(`${BASE}/api/v1/host/gallery/release`, { + method: 'POST', + headers: { Authorization: `Bearer ${host.jwt}` }, + }); + expect(rel.status).toBe(204); + + const rejected = await uploadRaw(host.jwt, SAMPLE(), { + filename: 'after-release.jpg', + contentType: 'image/jpeg', + }); + expect(rejected.status).toBe(403); + const body = await rejected.json(); + expect(body.error).toBe('uploads_locked'); + }); +}); diff --git a/frontend/src/lib/auth.ts b/frontend/src/lib/auth.ts index 252a35f..a048146 100644 --- a/frontend/src/lib/auth.ts +++ b/frontend/src/lib/auth.ts @@ -15,9 +15,19 @@ export const isAuthenticated = writable(false); */ export const currentPin = writable(null); -export function getToken(): string | null { +// Guest auth lives in localStorage (persists across sessions — a guest returning to the +// event days later stays signed in). The ADMIN token lives in sessionStorage instead +// (USER_JOURNEYS §11.1): its tighter 1-day lifetime is meant to bound exposure on a shared +// "event laptop", and sessionStorage clears on tab/browser close, which localStorage defeats. +// Reads check sessionStorage FIRST so an active admin token wins over any leftover guest +// token on the same device. +function readAuth(key: string): string | null { if (!browser) return null; - return localStorage.getItem(TOKEN_KEY); + return sessionStorage.getItem(key) ?? localStorage.getItem(key); +} + +export function getToken(): string | null { + return readAuth(TOKEN_KEY); } export function getPin(): string | null { @@ -37,13 +47,11 @@ export function clearPin(): void { } export function getUserId(): string | null { - if (!browser) return null; - return localStorage.getItem(USER_ID_KEY); + return readAuth(USER_ID_KEY); } export function getDisplayName(): string | null { - if (!browser) return null; - return localStorage.getItem(DISPLAY_NAME_KEY); + return readAuth(DISPLAY_NAME_KEY); } export function getExpiry(): Date | null { @@ -69,6 +77,20 @@ export function setAuth(jwt: string, pin: string | null, userId: string, display isAuthenticated.set(true); } +/** + * Persist an ADMIN session in sessionStorage (USER_JOURNEYS §11.1) rather than localStorage, + * so the elevated credential does not survive a tab/browser close on a shared device — the + * point of the tighter 1-day admin token. Admins have no recovery PIN. `getToken` reads + * sessionStorage first, so this wins over any leftover guest token on the same device. + */ +export function setAdminAuth(jwt: string, userId: string, displayName?: string): void { + if (!browser) return; + sessionStorage.setItem(TOKEN_KEY, jwt); + sessionStorage.setItem(USER_ID_KEY, userId); + if (displayName) sessionStorage.setItem(DISPLAY_NAME_KEY, displayName); + isAuthenticated.set(true); +} + // Hook registry: cross-cutting stores (export-status, etc.) register a callback // here at import-time so they get reset on every clearAuth path — both the // explicit "Event verlassen" button and the api.ts 401 auto-clear. Keeps @@ -81,11 +103,15 @@ export function onClearAuth(fn: () => void): void { export function clearAuth(): void { if (!browser) return; - localStorage.removeItem(TOKEN_KEY); - localStorage.removeItem(USER_ID_KEY); - // Clear the display name too — on a shared device the next user shouldn't see - // the previous guest's name pre-filled on /join. - localStorage.removeItem(DISPLAY_NAME_KEY); + // Clear from BOTH stores — a guest token lives in localStorage, an admin token in + // sessionStorage; either could be present, and a stale one must not linger. + for (const store of [localStorage, sessionStorage]) { + store.removeItem(TOKEN_KEY); + store.removeItem(USER_ID_KEY); + // Clear the display name too — on a shared device the next user shouldn't see + // the previous guest's name pre-filled on /join. + store.removeItem(DISPLAY_NAME_KEY); + } // PIN is intentionally kept so the user can recover isAuthenticated.set(false); // Hooks fire in registration order. Keep them dependency-free of each other — diff --git a/frontend/src/lib/sse.ts b/frontend/src/lib/sse.ts index d48f2e3..436f33c 100644 --- a/frontend/src/lib/sse.ts +++ b/frontend/src/lib/sse.ts @@ -12,7 +12,7 @@ // delta into its in-memory list. import { getToken } from './auth'; -import { api } from './api'; +import { api, ApiError } from './api'; import type { DeltaResponse } from './types'; type StreamTicketResponse = { ticket: string; server_time: string }; @@ -45,7 +45,9 @@ const KNOWN_EVENTS = [ 'event-updated', 'export-progress', 'export-available', - 'pin-reset' + 'pin-reset', + // A guest asked a host to reset their PIN — hosts refresh their pending-request badge. + 'pin-reset-requested' ] as const; /** @@ -197,7 +199,7 @@ function extractCreatedAt(data: string): string | undefined { * in-memory list. Swallows errors — a failed delta is non-fatal; the next live * SSE event will keep the feed moving. */ -async function deltaFetchAndFan(since: string): Promise { +async function deltaFetchAndFan(since: string, attempt = 0): Promise { try { const response = await api.get( `/feed/delta?since=${encodeURIComponent(since)}` @@ -206,11 +208,24 @@ async function deltaFetchAndFan(since: string): Promise { // reconnect resumes exactly where the server left off (no browser-clock skew). lastEventTime = response.server_time; dispatch('feed-delta', JSON.stringify(response)); - } catch { - // non-fatal + } catch (e) { + // A throttled delta (429) must NOT be silently dropped: live events keep advancing + // `lastEventTime`, so the next reconnect would resume PAST this un-fetched gap and + // lose it. Retry the SAME `since` with backoff so the gap [since, now] is still + // covered regardless of how the live cursor moves in the meantime. Bounded, and only + // reachable by a rapidly flapping EventSource hitting the per-user delta limit. + if (e instanceof ApiError && e.status === 429 && attempt < MAX_DELTA_RETRIES) { + const delayMs = DELTA_RETRY_BASE_MS * 2 ** attempt; + setTimeout(() => void deltaFetchAndFan(since, attempt + 1), delayMs); + } + // Other errors are non-fatal — the next live SSE event keeps the feed moving. } } +/** Bounded backoff for a rate-limited reconnect delta (see `deltaFetchAndFan`). */ +const MAX_DELTA_RETRIES = 4; +const DELTA_RETRY_BASE_MS = 2000; + // Page Visibility API: close while hidden, reopen on focus. On reopen `connectSse`'s // `onopen` runs the delta fetch. function handleVisibilityChange() { diff --git a/frontend/src/lib/types.ts b/frontend/src/lib/types.ts index 702b8ae..13a4338 100644 --- a/frontend/src/lib/types.ts +++ b/frontend/src/lib/types.ts @@ -27,6 +27,10 @@ export interface FeedResponse { export interface DeltaResponse { uploads: FeedUpload[]; deleted_ids: string[]; + // Users hidden (banned / uploads_hidden) since the cursor. A ban is not a soft-delete so + // it's absent from deleted_ids; the client evicts all uploads from these users, which + // replays a ban the client missed while disconnected (esp. the unattended projector). + hidden_user_ids: string[]; // True when the delta hit the backend cap and is only the newest slice of the // gap — the client must full-refresh rather than merge (see feed-delta handler). truncated: boolean; diff --git a/frontend/src/lib/upload-queue.test.ts b/frontend/src/lib/upload-queue.test.ts new file mode 100644 index 0000000..70e0961 --- /dev/null +++ b/frontend/src/lib/upload-queue.test.ts @@ -0,0 +1,42 @@ +import { describe, it, expect } from 'vitest'; +import { classifyUploadStatus } from './upload-queue'; + +/** + * Regression guard for the upload-queue retry policy (H2 + M1). The bug being locked out: + * every 4xx except 429 was classified terminal, and a terminal item has its blob PURGED + * from IndexedDB. A 401 (a sliding session that lapsed, or a host PIN-reset) is a 4xx, so a + * guest's queued photos were irrecoverably destroyed the moment the `online` auto-resume + * fired against a dead session. 401 must be `auth` (blob kept, re-auth), NEVER `terminal`. + */ +describe('classifyUploadStatus', () => { + it('2xx → success', () => { + expect(classifyUploadStatus(200)).toBe('success'); + expect(classifyUploadStatus(201)).toBe('success'); + expect(classifyUploadStatus(299)).toBe('success'); + }); + + it('401 → auth, NOT terminal (never purge the blob on a dead session)', () => { + expect(classifyUploadStatus(401)).toBe('auth'); + }); + + it('429 → rate_limit (back off, auto-resume)', () => { + expect(classifyUploadStatus(429)).toBe('rate_limit'); + }); + + it('408 → transient (request timeout is retryable, not terminal)', () => { + expect(classifyUploadStatus(408)).toBe('transient'); + }); + + it('genuinely permanent 4xx → terminal (locked / banned / released / quota)', () => { + expect(classifyUploadStatus(403)).toBe('terminal'); // banned / locked / released + expect(classifyUploadStatus(413)).toBe('terminal'); // quota exhausted + expect(classifyUploadStatus(400)).toBe('terminal'); + expect(classifyUploadStatus(404)).toBe('terminal'); + }); + + it('5xx / unexpected → transient (retryable, blob kept)', () => { + expect(classifyUploadStatus(500)).toBe('transient'); + expect(classifyUploadStatus(502)).toBe('transient'); + expect(classifyUploadStatus(503)).toBe('transient'); + }); +}); diff --git a/frontend/src/lib/upload-queue.ts b/frontend/src/lib/upload-queue.ts index 20a3a28..99624cd 100644 --- a/frontend/src/lib/upload-queue.ts +++ b/frontend/src/lib/upload-queue.ts @@ -1,6 +1,7 @@ import { openDB, type IDBPDatabase } from 'idb'; import { writable, get } from 'svelte/store'; -import { getToken, getUserId } from './auth'; +import { getToken, getUserId, clearAuth } from './auth'; +import { onSseEvent } from './sse'; import { refreshQuota } from './quota-store'; export interface QueueItem { @@ -8,6 +9,9 @@ export interface QueueItem { userId: string; fileName: string; fileSize: number; + /** Source file's last-modified ms; part of the dedup key so two distinct photos that + * happen to share a name and byte length don't collapse into one. */ + lastModified?: number; mimeType: string; caption: string; hashtags: string; @@ -50,6 +54,23 @@ function bindOnline(): void { } bindOnline(); +// 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 +// `event-opened` SSE flips it back to pending and re-drains, so a photo staged during a +// lock isn't lost across a reopen. One-way import (sse.ts never imports this module). +let sseBound = false; +function bindSse(): void { + if (sseBound || typeof window === 'undefined') return; + onSseEvent('event-opened', () => { + void (async () => { + await requeueRetriable(); + await processQueue(); + })(); + }); + sseBound = true; +} +bindSse(); + /** * Flip transient `error` items (5xx / a network drop that got marked before we could * reclassify it) back to `pending` so a resume actually retries them. Terminal `blocked` @@ -141,6 +162,47 @@ class TerminalError extends Error { */ class NetworkError extends Error {} +/** + * The session is gone/expired (HTTP 401). This is emphatically NOT terminal: the blob is + * KEPT (deleting it — the old behavior for any 4xx — destroyed a guest's staged photos the + * instant their sliding session lapsed or a host reset their PIN, exactly when auto-resume + * fires). We mark the item retryable and route to re-auth; once the user signs back in with + * the same identity (via /recover), `loadQueue` re-associates these entries by userId and + * they resume. Distinct from `TerminalError` so the 4xx branch can't purge the blob. + */ +class AuthError extends Error {} + +/** + * A REVERSIBLE 403 — the event is closed or the gallery was released, but a host can reopen + * it. Backend tags these with code `uploads_locked` (distinct from a permanent `forbidden` + * like a banned user). The blob is KEPT and the item parks as retryable; the `event-opened` + * SSE (or a manual retry) resumes it. Without this, a photo staged during a lock was purged + * as terminal and lost the moment the host reopened. + */ +class LockedError extends Error {} + +/** Retry policy for an upload response status. */ +export type UploadOutcome = 'success' | 'rate_limit' | 'auth' | 'transient' | 'terminal'; + +/** + * Classify an upload HTTP status into a retry policy. Pure + exported so the + * data-loss-critical rules are unit-testable without an XHR/IndexedDB harness: + * - 401 → `auth` (session gone: KEEP the blob, re-auth — NEVER purge it) + * - 408 → `transient` (request timeout: retryable) + * - 429 → `rate_limit`(back off, auto-resume) + * - other 4xx → `terminal` (locked/banned/released/quota — the only case that purges) + * - 2xx → `success`; 5xx/other → `transient` + * The one rule that must never regress: a 401 is `auth`, not `terminal`. + */ +export function classifyUploadStatus(status: number): UploadOutcome { + if (status >= 200 && status < 300) return 'success'; + if (status === 429) return 'rate_limit'; + if (status === 401) return 'auth'; + if (status === 408) return 'transient'; + if (status >= 400 && status < 500) return 'terminal'; + return 'transient'; +} + export async function loadQueue(): Promise { const database = await getDb(); const myUserId = getUserId(); @@ -173,29 +235,40 @@ export async function loadQueue(): Promise { })(); } +/** Outcome of an `addToQueue` call, so the caller can tell the user when a file was NOT + * actually queued (deduped, or the queue is full of un-evictable in-flight items). */ +export type EnqueueResult = 'queued' | 'duplicate' | 'full'; + export async function addToQueue( file: File, caption: string, hashtags: string -): Promise { +): Promise { const database = await getDb(); const userId = getUserId(); - if (!userId) return; // not authenticated — nothing to do + // Not authenticated — nothing to queue. Return the silent 'duplicate' outcome rather than + // 'full' so the caller doesn't show a misleading "queue full" toast. Practically + // unreachable: the upload view sits behind a token guard. + if (!userId) return 'duplicate'; // Dedup: don't queue the same file twice while an identical one is still unsent - // (double-tap, re-added after a flaky reconnect). Matches on name+size+user. + // (double-tap, re-added after a flaky reconnect). Matches on name+size+lastModified+user + // — including lastModified so two genuinely different photos sharing a name and byte + // length aren't silently collapsed into one. const mine = get(queueItems).filter((i) => i.userId === userId); const dup = mine.some( (i) => i.fileName === file.name && i.fileSize === file.size && + (i.lastModified ?? 0) === file.lastModified && (i.status === 'pending' || i.status === 'uploading') ); - if (dup) return; + if (dup) return 'duplicate'; // 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, - // refuse silently rather than pile on. + // refuse and tell the caller so it can surface a "queue full" message (silent loss of a + // photo the user believed was queued is the worst outcome here). if (mine.length >= MAX_QUEUE_ITEMS) { const evictable = get(queueItems).find( (i) => i.userId === userId && (i.status === 'done' || i.status === 'error' || i.status === 'blocked') @@ -204,7 +277,7 @@ export async function addToQueue( await database.delete(STORE_NAME, evictable.id); queueItems.update((items) => items.filter((it) => it.id !== evictable.id)); } else { - return; + return 'full'; } } @@ -214,6 +287,7 @@ export async function addToQueue( userId, fileName: file.name, fileSize: file.size, + lastModified: file.lastModified, mimeType: file.type, caption, hashtags, @@ -229,6 +303,7 @@ export async function addToQueue( userId, fileName: file.name, fileSize: file.size, + lastModified: file.lastModified, mimeType: file.type, caption, hashtags, @@ -238,6 +313,7 @@ export async function addToQueue( ]); processQueue(); + return 'queued'; } export async function retryItem(id: string): Promise { @@ -305,9 +381,20 @@ async function processQueue(): Promise { }, e.retryAfterSecs * 1000); break; } + if (e instanceof AuthError) { + // Dead session — stop the batch (every further item would 401 too). Items + // stay retryable with blobs intact; the re-auth flow (clearAuth) takes over. + break; + } + if (e instanceof LockedError) { + // Event locked — stop the batch (every item would hit the same lock). Items + // stay retryable with blobs intact; the `event-opened` SSE resumes them. + break; + } if (e instanceof NetworkError) { - // Connectivity dropped mid-flight — the item is back to 'pending'. Stop the - // loop; the `online` listener resumes it. No error state, no manual tap. + // Connectivity dropped mid-flight. If offline the item is back to 'pending' + // and the `online` listener resumes it; if the failure hit while nominally + // online it's now a retryable 'error'. Either way, stop hammering here. break; } // Other errors are already handled inside uploadItem (marked 'error'/'blocked') @@ -364,37 +451,50 @@ async function uploadItem(id: string): Promise { }); xhr.addEventListener('load', () => { - if (xhr.status >= 200 && xhr.status < 300) { - resolve(); - } else if (xhr.status === 429) { - // Rate limit only (quota-full is now a distinct 413, handled below as - // terminal) — back off and auto-resume when the window lifts. + const body = (() => { try { - const body = JSON.parse(xhr.responseText); - const secs = typeof body.retry_after_secs === 'number' ? body.retry_after_secs : 60; + return JSON.parse(xhr.responseText); + } catch { + return null; + } + })(); + switch (classifyUploadStatus(xhr.status)) { + case 'success': + resolve(); + break; + case 'rate_limit': { + // Back off and auto-resume when the window lifts (quota-full is a distinct + // 413, classified 'terminal' below). + const secs = typeof body?.retry_after_secs === 'number' ? body.retry_after_secs : 60; reject(new RateLimitError(secs)); - } catch { - reject(new RateLimitError(60)); + break; } - } else if (xhr.status >= 400 && xhr.status < 500) { - // Any other 4xx is permanent for this blob (locked/banned/released/quota). - // Retrying just re-hits the same rejection forever, so mark it terminal. - let msg = 'Upload nicht möglich.'; - try { - const body = JSON.parse(xhr.responseText); - if (body.message) msg = body.message; - else if (xhr.status === 413) msg = 'Speicher-Limit erreicht.'; - } catch { - if (xhr.status === 413) msg = 'Speicher-Limit erreicht.'; - } - reject(new TerminalError(msg)); - } else { - // 5xx / unexpected — transient, keep it retryable. - try { - const body = JSON.parse(xhr.responseText); - reject(new Error(body.message || `HTTP ${xhr.status}`)); - } catch { - reject(new Error(`HTTP ${xhr.status}`)); + case 'auth': + // Session expired/revoked. NOT terminal — keep the blob and re-auth. Lumping + // this into the terminal bucket (which purges the blob) irrecoverably + // destroyed queued photos whenever a sliding session lapsed or a host reset + // the PIN — precisely when the `online` auto-resume kicks in. + reject(new AuthError('Sitzung abgelaufen. Bitte melde dich erneut an.')); + break; + case 'transient': + // 408 (request timeout) behaves like a network blip so it stays retryable; + // 5xx is a generic retryable error. Neither purges the blob. + if (xhr.status === 408) reject(new NetworkError('Zeitüberschreitung')); + else reject(new Error(body?.message || `HTTP ${xhr.status}`)); + break; + case 'terminal': { + // A REVERSIBLE lock (event closed / gallery released) is tagged + // `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. + if (body?.error === 'uploads_locked') { + reject(new LockedError(body?.message || 'Event ist geschlossen.')); + break; + } + // Any other 4xx the server will keep rejecting (banned / quota). + let msg = body?.message || 'Upload nicht möglich.'; + if (!body?.message && xhr.status === 413) msg = 'Speicher-Limit erreicht.'; + reject(new TerminalError(msg)); + break; } } }); @@ -420,12 +520,48 @@ async function uploadItem(id: string): Promise { updateItemStatus(id, 'pending'); throw e; // Propagate to processQueue for scheduling } - if (e instanceof NetworkError) { - // No connectivity — keep the item pending and propagate so processQueue stops - // the loop (no point hammering while offline). The `online` listener resumes it. - entry.status = 'pending'; + if (e instanceof LockedError) { + // Event closed / gallery released, but a host can reopen — KEEP the blob and park + // the item as retryable so it survives until reopen. The `event-opened` SSE + // (bindSse) auto-resumes it; a manual "Erneut" also works. Never purge here. + entry.status = 'error'; + entry.error = e.message; await database.put(STORE_NAME, entry); - updateItemStatus(id, 'pending'); + updateItemStatus(id, 'error', e.message); + throw e; + } + if (e instanceof AuthError) { + // Dead session — KEEP the blob (never purge on auth failure) and mark retryable so + // the file survives re-auth. Route through the app's single de-auth path (matches + // api.ts's 401 handling). The blob persists in IndexedDB keyed by userId; once the + // user signs back in with the SAME identity (via /recover), the next time the upload + // view loads `loadQueue` re-associates this entry and it can be retried/resumed. + entry.status = 'error'; + entry.error = e.message; + await database.put(STORE_NAME, entry); + updateItemStatus(id, 'error', e.message); + clearAuth(); + throw e; + } + if (e instanceof NetworkError) { + const offline = typeof navigator !== 'undefined' && navigator.onLine === false; + if (offline) { + // Genuinely offline — keep the item pending; the `online` listener resumes it + // automatically with no user action. + entry.status = 'pending'; + await database.put(STORE_NAME, entry); + updateItemStatus(id, 'pending'); + } else { + // Network-level failure while the OS still reports online (server down, + // connection refused, TLS error, captive portal). The `online` event will + // NEVER fire in this case, so leaving it 'pending' would strand the item with + // no retry path and no spinner. Mark it retryable 'error' so the user gets a + // working "Erneut" button and a later real reconnect requeues it. + entry.status = 'error'; + entry.error = 'Netzwerkfehler. Erneut versuchen.'; + await database.put(STORE_NAME, entry); + updateItemStatus(id, 'error', 'Netzwerkfehler. Erneut versuchen.'); + } throw e; } if (e instanceof TerminalError) { diff --git a/frontend/src/routes/+layout.svelte b/frontend/src/routes/+layout.svelte index 5673674..edf369d 100644 --- a/frontend/src/routes/+layout.svelte +++ b/frontend/src/routes/+layout.svelte @@ -15,7 +15,7 @@ import { onSseEvent } from '$lib/sse'; import { api } from '$lib/api'; import type { MeContextDto } from '$lib/types'; - import { eventState, markClosed, markOpened } from '$lib/event-state-store'; + import { eventState, markClosed, markOpened, refreshEventState } from '$lib/event-state-store'; let { children } = $props(); @@ -69,7 +69,14 @@ }), // Reflect a host closing/reopening uploads live, so the composer switches to a // locked state immediately instead of a guest finding out via a rejected upload. - onSseEvent('event-closed', () => markClosed()), + // `event-closed` fires for BOTH a plain lock and a gallery release (release ⇒ lock), + // which markClosed can't tell apart — follow it with a server refresh so a release + // correctly flips `galleryReleased` too (else the composer briefly mislabels it as a + // plain lock until the next navigation). + onSseEvent('event-closed', () => { + markClosed(); + void refreshEventState(); + }), onSseEvent('event-opened', () => markOpened()) ); }); diff --git a/frontend/src/routes/admin/login/+page.svelte b/frontend/src/routes/admin/login/+page.svelte index ed2ffa3..72e7554 100644 --- a/frontend/src/routes/admin/login/+page.svelte +++ b/frontend/src/routes/admin/login/+page.svelte @@ -1,7 +1,7 @@