fix(user-flow): persona-audit fixes — ban replay, locked-upload data loss, host UX, admin session
Follows the perf + security + user-flow work with a role/persona audit (guest, host, admin, projector) and fixes across three review rounds. Highlights: HIGH - Ban now replays on reconnect. A ban isn't a soft-delete, and the `user-hidden` SSE has no replay, so a client that missed it (esp. the unattended diashow) kept cycling a banned user's slides. New `uploads_hidden_at` (migration 013) + `hidden_user_ids` in /feed/delta; feed + diashow evict those users. Applied even on a truncated delta. MEDIUM - Locked-upload data loss: a photo staged offline during a lock/release was purged as a terminal 4xx and lost when the host reopened. New reversible `uploads_locked` error code; the queue keeps the blob and auto-resumes on the `event-opened` SSE. - Reopen after release now warns (ConfirmSheet) that it revokes the published keepsake. - Host "forgotten-PIN" badge updates live (`pin-reset-requested` was broadcast but never in KNOWN_EVENTS / subscribed); host page also refetches on `pin-reset` so a two-host race can't hand out a conflicting PIN. - Ban modal copy fixed (read-only ban, not "session ended"); Degradieren/Sperren/Entsperren hidden on peer-host rows for non-admins (they always 403'd). - Host dashboard shows live keepsake generation progress / ready state + link to /export. - Admin JWT moved to sessionStorage (§11.1) to bound exposure on shared devices. Export generation guard (H1 from the prior round, hardened): per-(event,type) `release_seq` (migration 012) with seq-guarded claim/finalize/mark_failed/update_progress, per-generation temp/final paths, download follows `file_path`, prune only strictly-older generations. LOW: diashow coalesces upload-processed (avoids self-rate-limit); event-closed reconciles galleryReleased; /recover gains a forgot-PIN request + drops a stale cached PIN on 401; delta `>=` tie-break + 429 retry; misc copy/labels. Adds e2e: ban-replay, upload-lock-code, and rewrites export-reopen-rerelease with a data-completeness test. Reconciles USER_JOURNEYS §9/§11. Verified: cargo build clean, 40 unit tests, svelte-check 0 errors, 33 frontend unit tests, 155 e2e passing on chromium-desktop. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -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.<seq>.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<std::path::PathBuf, AppError> {
|
||||
let file_path: Option<(Option<String>,)> = 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/<name>`; 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
|
||||
}
|
||||
|
||||
|
||||
@@ -181,6 +181,12 @@ pub struct DeltaQuery {
|
||||
pub struct DeltaResponse {
|
||||
pub uploads: Vec<FeedUpload>,
|
||||
pub deleted_ids: Vec<Uuid>,
|
||||
/// 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<Uuid>,
|
||||
/// 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,
|
||||
}))
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user