From faf7a2504a4ccb251f316dfff8124d1fd4e7c458 Mon Sep 17 00:00:00 2001 From: fabi Date: Tue, 7 Jul 2026 07:28:27 +0200 Subject: [PATCH] fix(audit): restore broken upload pipeline + role-based E2E audit fixes A comprehensive role-based E2E audit (guest/host/admin, across browser sessions) surfaced one critical and several smaller issues; this addresses them and hardens the tests that missed them. Critical - The client upload pipeline was fully broken: the IndexedDB v1->v2 upgrade opened a *new* transaction inside the upgrade callback, which throws during a version-change transaction and aborted the whole upgrade, leaving the queue object store uncreated -- so no UI upload ever fired. Reuse the version-change transaction the callback provides, and bump the DB to v3 with a contains() guard so installs already corrupted by the shipped bug self-heal on next load. Re-enabled the previously-fixme'd UI upload E2E test. High / Medium - Event lock is uploads-only again: likes, comments and browsing stay open while the event is locked (USER_JOURNEYS 9.3 / FEATURES) -- it was wrongly freezing social interaction. Updated the event-lock spec accordingly. - get_original now excludes soft-deleted and ban-hidden uploads, and direct /media/originals/** serving is blocked, so a hidden user's originals can no longer be pulled by UUID (all originals go through the checked alias). - The upload handler reads the file field with an early-abort size cap chosen from the declared content-type, instead of buffering the entire body before the size check. Low - unban_user mirrors the ban role guard (a host can no longer unban a host/admin banned by an admin). - reset_user_pin's UPDATE is event-scoped. - Admin login returns and stores a real identity (user_id + display name) instead of a blank session. - The host user list no longer renders target-actions (ban/promote/demote/PIN) on the caller's own row, where the backend always rejected them. - /diashow gains a client-side auth guard like the other protected routes. - The join page shows the event name via a new public GET /api/v1/event. Verified: backend cargo build clean, frontend svelte-check 0 errors, full Playwright E2E suite 144 passed / 1 skipped. Co-Authored-By: Claude Opus 4.8 --- backend/src/auth/handlers.rs | 10 +++- backend/src/handlers/admin.rs | 4 ++ backend/src/handlers/host.rs | 23 +++++++- backend/src/handlers/mod.rs | 1 + backend/src/handlers/public.rs | 24 ++++++++ backend/src/handlers/social.rs | 22 ++----- backend/src/handlers/upload.rs | 62 +++++++++++++++----- backend/src/main.rs | 11 ++++ backend/src/models/upload.rs | 16 +++++ e2e/specs/02-upload/gallery-path.spec.ts | 17 +++--- e2e/specs/04-host/event-lock.spec.ts | 24 +++++--- frontend/src/lib/upload-queue.ts | 25 +++++--- frontend/src/routes/admin/login/+page.svelte | 11 +++- frontend/src/routes/diashow/+page.svelte | 8 +++ frontend/src/routes/host/+page.svelte | 9 ++- frontend/src/routes/join/+page.svelte | 15 +++++ 16 files changed, 220 insertions(+), 62 deletions(-) create mode 100644 backend/src/handlers/public.rs diff --git a/backend/src/auth/handlers.rs b/backend/src/auth/handlers.rs index ca45d4c..c46cce4 100644 --- a/backend/src/auth/handlers.rs +++ b/backend/src/auth/handlers.rs @@ -238,6 +238,10 @@ pub struct AdminLoginRequest { #[derive(Serialize)] pub struct AdminLoginResponse { pub jwt: String, + /// The admin's user id + display name, so the client can populate a real identity + /// (own-post affordances, a name on the Account page) instead of a blank session. + pub user_id: Uuid, + pub display_name: String, } pub async fn admin_login( @@ -325,7 +329,11 @@ pub async fn admin_login( let expires_at = Utc::now() + chrono::Duration::days(1); Session::create(&state.pool, admin_user.id, &token_hash, expires_at).await?; - Ok(Json(AdminLoginResponse { jwt: token })) + Ok(Json(AdminLoginResponse { + jwt: token, + user_id: admin_user.id, + display_name: admin_user.display_name, + })) } pub async fn logout( diff --git a/backend/src/handlers/admin.rs b/backend/src/handlers/admin.rs index f687255..a2d4098 100644 --- a/backend/src/handlers/admin.rs +++ b/backend/src/handlers/admin.rs @@ -266,6 +266,10 @@ pub async fn export_ticket( State(state): State, auth: crate::auth::middleware::AuthUser, ) -> Json { + // NOTE: intentionally NOT gated on `is_banned`. A banned user keeps *read* access + // by design (USER_JOURNEYS §10.3, FEATURES: "Can still download the export once + // released — Spec design choice"). The export is read-only, so it stays available + // to them, consistent with the read-only-ban model. let ticket = state.sse_tickets.issue(auth.token_hash); Json(serde_json::json!({ "ticket": ticket })) } diff --git a/backend/src/handlers/host.rs b/backend/src/handlers/host.rs index 5fc4d1a..cbae4e3 100644 --- a/backend/src/handlers/host.rs +++ b/backend/src/handlers/host.rs @@ -146,6 +146,26 @@ pub async fn unban_user( RequireHost(auth): RequireHost, Path(user_id): Path, ) -> Result { + // Mirror the ban guard: a host may only lift bans on guests, never on hosts or + // admins. Without this a host could override an admin's ban of another host, + // which is asymmetric with `ban_user` and lets a host escalate a peer back in. + let target = sqlx::query_as::<_, (String,)>( + "SELECT role::text FROM \"user\" WHERE id = $1 AND event_id = $2", + ) + .bind(user_id) + .bind(auth.event_id) + .fetch_optional(&state.pool) + .await? + .ok_or_else(|| AppError::NotFound("Benutzer nicht gefunden.".into()))?; + + if target.0 == "admin" + || (target.0 == "host" && auth.role != crate::models::user::UserRole::Admin) + { + return Err(AppError::Forbidden( + "Du kannst diesen Benutzer nicht entsperren.".into(), + )); + } + let result = sqlx::query( "UPDATE \"user\" SET is_banned = FALSE WHERE id = $1 AND event_id = $2", ) @@ -275,10 +295,11 @@ pub async fn reset_user_pin( SET recovery_pin_hash = $1, failed_pin_attempts = 0, pin_locked_until = NULL - WHERE id = $2", + WHERE id = $2 AND event_id = $3", ) .bind(&pin_hash) .bind(user_id) + .bind(auth.event_id) .execute(&state.pool) .await?; diff --git a/backend/src/handlers/mod.rs b/backend/src/handlers/mod.rs index fbdf47b..830f6de 100644 --- a/backend/src/handlers/mod.rs +++ b/backend/src/handlers/mod.rs @@ -2,6 +2,7 @@ pub mod admin; pub mod feed; pub mod host; pub mod me; +pub mod public; pub mod social; pub mod sse; pub mod test_admin; diff --git a/backend/src/handlers/public.rs b/backend/src/handlers/public.rs new file mode 100644 index 0000000..1131ad7 --- /dev/null +++ b/backend/src/handlers/public.rs @@ -0,0 +1,24 @@ +//! Unauthenticated, read-only endpoints safe to expose before a user has joined. + +use axum::extract::State; +use axum::Json; +use serde::Serialize; + +use crate::state::AppState; + +#[derive(Serialize)] +pub struct PublicEventDto { + pub name: String, + pub slug: String, +} + +/// Public event identity, used by the pre-auth join/recover screens so a guest can +/// see *which* event they're joining. Only the display name and slug are exposed — +/// nothing user-scoped — so this is safe without a token. Served straight from the +/// instance config (no DB round-trip needed). +pub async fn get_public_event(State(state): State) -> Json { + Json(PublicEventDto { + name: state.config.event_name.clone(), + slug: state.config.event_slug.clone(), + }) +} diff --git a/backend/src/handlers/social.rs b/backend/src/handlers/social.rs index c7a6621..7a9fdcc 100644 --- a/backend/src/handlers/social.rs +++ b/backend/src/handlers/social.rs @@ -12,18 +12,6 @@ use crate::models::hashtag::{self, Hashtag}; use crate::models::upload::Upload; use crate::state::AppState; -/// Reject the request when the event's uploads (and, by extension, social -/// interaction) are locked. Mirrors the guard in the upload handler. -async fn require_event_open(state: &AppState) -> Result<(), AppError> { - let event = crate::models::event::Event::find_by_slug(&state.pool, &state.config.event_slug) - .await? - .ok_or_else(|| AppError::NotFound("Event nicht gefunden.".into()))?; - if event.uploads_locked_at.is_some() { - return Err(AppError::Forbidden("Das Event ist geschlossen.".into())); - } - Ok(()) -} - pub async fn toggle_like( State(state): State, auth: AuthUser, @@ -43,8 +31,9 @@ pub async fn toggle_like( .await? .ok_or_else(|| AppError::NotFound("Upload nicht gefunden.".into()))?; - // A closed event freezes social interaction too, matching the upload handler. - require_event_open(&state).await?; + // NOTE: liking is intentionally allowed while the event is locked. Locking + // ("Event schließen") freezes *new uploads* only — likes, comments and + // browsing stay open (USER_JOURNEYS §9.3, FEATURES capability matrix). // Try to insert; if conflict, delete (toggle) let result = sqlx::query( @@ -135,8 +124,9 @@ pub async fn add_comment( .await? .ok_or_else(|| AppError::NotFound("Upload nicht gefunden.".into()))?; - // A closed event freezes social interaction too, matching the upload handler. - require_event_open(&state).await?; + // NOTE: commenting is intentionally allowed while the event is locked. Locking + // freezes *new uploads* only — likes, comments and browsing stay open + // (USER_JOURNEYS §9.3, FEATURES capability matrix). let text = body.body.trim(); let text_chars = text.chars().count(); diff --git a/backend/src/handlers/upload.rs b/backend/src/handlers/upload.rs index 29f05c9..96ad78b 100644 --- a/backend/src/handlers/upload.rs +++ b/backend/src/handlers/upload.rs @@ -90,14 +90,21 @@ pub async fn upload( let name = field.name().unwrap_or_default().to_string(); match name.as_str() { "file" => { - // Note: the client-declared filename and Content-Type are intentionally - // ignored — the stored MIME and extension are derived from the file's - // magic bytes below, so a mislabelled payload can't influence them. - file_data = Some( - field.bytes().await - .map_err(|e| AppError::BadRequest(format!("Datei konnte nicht gelesen werden: {e}")))? - .to_vec(), - ); + // Note: the client-declared filename and Content-Type do NOT determine + // the stored MIME/extension — those come from the file's magic bytes + // below. The declared type is used only to pick a memory cap so an + // oversized body can't be fully buffered before the size check. A + // mislabelled type only makes the cap *stricter* (safe); the + // authoritative per-class check still runs on the detected type. + let declared = field.content_type().unwrap_or("").to_string(); + let cap_bytes = if declared.starts_with("video/") { + (max_video_mb * 1024 * 1024) as usize + } else if declared.starts_with("image/") { + (max_image_mb * 1024 * 1024) as usize + } else { + (max_image_mb.max(max_video_mb) * 1024 * 1024) as usize + }; + file_data = Some(read_field_capped(field, cap_bytes).await?); } "caption" => { caption = Some( @@ -332,6 +339,32 @@ pub async fn delete_upload( Ok(StatusCode::NO_CONTENT) } +/// Read a multipart field into memory, aborting with a 400 the moment it exceeds +/// `max_bytes`. Without this the whole field is buffered (up to the HTTP body cap) +/// before the post-read size check runs, so a request claiming to be a tiny image +/// could still force hundreds of MB of allocation. Streaming with an early abort +/// bounds peak memory to roughly the applicable per-class limit. +async fn read_field_capped( + mut field: axum::extract::multipart::Field<'_>, + max_bytes: usize, +) -> Result, AppError> { + let mut buf: Vec = Vec::new(); + while let Some(chunk) = field + .chunk() + .await + .map_err(|e| AppError::BadRequest(format!("Datei konnte nicht gelesen werden: {e}")))? + { + if buf.len().saturating_add(chunk.len()) > max_bytes { + return Err(AppError::BadRequest(format!( + "Datei ist zu groß. Maximum: {} MB.", + max_bytes / (1024 * 1024) + ))); + } + buf.extend_from_slice(&chunk); + } + Ok(buf) +} + /// Drain a multipart body so the HTTP connection stays clean when returning an early error. /// Without draining, the client may still be sending the body after we've sent our response, /// which can corrupt the keep-alive connection for subsequent requests. @@ -407,16 +440,17 @@ pub async fn compute_storage_quota(state: &AppState) -> QuotaEstimate { /// - `` / `