diff --git a/.env.example b/.env.example index 614e288..f0203d9 100644 --- a/.env.example +++ b/.env.example @@ -4,6 +4,10 @@ DOMAIN=my-event.example.com # ── App server ──────────────────────────────────────────────────────────────── APP_PORT=3000 +# Set to `production` in real deployments. This activates the secret guard that +# refuses to boot with placeholder JWT_SECRET / ADMIN_PASSWORD_HASH values. +# (docker-compose.yml already sets APP_ENV=production for the app service.) +APP_ENV=production # ── Database ────────────────────────────────────────────────────────────────── # Set a strong password and keep it in sync between DATABASE_URL and diff --git a/.env.test b/.env.test deleted file mode 100644 index b204567..0000000 --- a/.env.test +++ /dev/null @@ -1,45 +0,0 @@ -# ── Domain ──────────────────────────────────────────────────────────────────── -# Public domain Caddy will serve and obtain a TLS certificate for. -DOMAIN=my-event.example.com - -# ── App server ──────────────────────────────────────────────────────────────── -APP_PORT=3000 - -# ── Database ────────────────────────────────────────────────────────────────── -DATABASE_URL=postgres://eventsnap:secret@db:5432/eventsnap -POSTGRES_USER=eventsnap -POSTGRES_PASSWORD=secret -POSTGRES_DB=eventsnap - -# ── Authentication ──────────────────────────────────────────────────────────── -# Generate with: openssl rand -hex 64 -JWT_SECRET=change_me_to_a_random_64_byte_hex_string -SESSION_EXPIRY_DAYS=30 - -# Admin dashboard password (bcrypt hash). -# Generate with: htpasswd -bnBC 12 "" yourpassword | tr -d ':\n' -ADMIN_PASSWORD_HASH=$2y$12$placeholder_replace_me - -# ── Event ───────────────────────────────────────────────────────────────────── -EVENT_NAME=Max & Maria's Wedding -EVENT_SLUG=max-maria-2026 - -# ── Storage ─────────────────────────────────────────────────────────────────── -MEDIA_PATH=/media - -# ── Upload limits ───────────────────────────────────────────────────────────── -DEFAULT_MAX_IMAGE_SIZE_MB=20 -DEFAULT_MAX_VIDEO_SIZE_MB=500 - -# ── Rate limiting ───────────────────────────────────────────────────────────── -DEFAULT_UPLOAD_RATE_PER_HOUR=10 -DEFAULT_FEED_RATE_PER_MIN=60 -DEFAULT_EXPORT_RATE_PER_DAY=3 - -# ── Capacity ────────────────────────────────────────────────────────────────── -DEFAULT_ESTIMATED_GUEST_COUNT=100 -# Fraction of total storage that triggers the "low storage" warning (0.0–1.0) -DEFAULT_QUOTA_TOLERANCE=0.75 - -# ── Workers ─────────────────────────────────────────────────────────────────── -COMPRESSION_WORKER_CONCURRENCY=2 diff --git a/.gitignore b/.gitignore index 55d4381..d03afd4 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,7 @@ # Environment secrets — never commit the real .env .env +# Stale local scratch copy of .env.example; nothing in the test stack reads it. +.env.test # Rust backend/target/ diff --git a/Caddyfile b/Caddyfile index d560ae4..3ceff7c 100644 --- a/Caddyfile +++ b/Caddyfile @@ -1,5 +1,8 @@ {$DOMAIN} { - encode zstd gzip + # Compress everything EXCEPT the SSE stream — gzip buffering delays + # "real-time" likes/comments until the ~30s keep-alive tick. + @compressible not path /api/v1/stream + encode @compressible zstd gzip # Site-wide security headers (defense-in-depth). HSTS is free since Caddy # already terminates TLS. nosniff also covers all of /media/*. diff --git a/README.md b/README.md index 91f5cea..0300098 100644 --- a/README.md +++ b/README.md @@ -94,9 +94,9 @@ eventsnap/ ### Deploy on a fresh VPS ```bash -# 1. Clone the repository -git clone https://git.mc02.dev/fabi/EventSnap.git -cd EventSnap +# 1. Clone the repository (into a lowercase dir, matching the paths used below) +git clone https://git.mc02.dev/fabi/EventSnap.git eventsnap +cd eventsnap # 2. Configure environment cp .env.example .env @@ -108,6 +108,8 @@ docker compose up -d Caddy automatically obtains a Let's Encrypt certificate on first start. The app is live at `https://DOMAIN` within ~30 seconds. +> **If the site never comes up:** with `APP_ENV=production` the backend **refuses to boot** while `JWT_SECRET`/`ADMIN_PASSWORD_HASH` still hold the `.env.example` placeholders (this is deliberate — a publicly-known signing key is worse than downtime). Caddy then waits on the unhealthy `app` container and never serves. Check `docker compose logs app` — a "Refusing to start … placeholder …" line means you skipped step 2. Rotate the secrets (see below) and restart. + > **Production note:** `docker compose up -d` does **not** expose the database — Postgres is reachable only on the internal Docker network. For local development where you need host access to Postgres, opt into the dev overlay explicitly: > ```bash > docker compose -f docker-compose.yml -f docker-compose.dev.yml up diff --git a/backend/migrations/010_review_indexes.down.sql b/backend/migrations/010_review_indexes.down.sql new file mode 100644 index 0000000..d69f56b --- /dev/null +++ b/backend/migrations/010_review_indexes.down.sql @@ -0,0 +1,3 @@ +DROP INDEX IF EXISTS idx_comment_hashtag_hashtag; +DROP INDEX IF EXISTS idx_comment_user; +DROP INDEX IF EXISTS idx_upload_event_created_id; diff --git a/backend/migrations/010_review_indexes.up.sql b/backend/migrations/010_review_indexes.up.sql new file mode 100644 index 0000000..93d2717 --- /dev/null +++ b/backend/migrations/010_review_indexes.up.sql @@ -0,0 +1,17 @@ +-- Composite feed index with an id tiebreaker so keyset pagination +-- (ORDER BY created_at DESC, id DESC) stays index-covered and stable when +-- multiple uploads share a created_at timestamp. +CREATE INDEX idx_upload_event_created_id + ON upload(event_id, created_at DESC, id DESC) + WHERE deleted_at IS NULL; + +-- A user's own comments (moderation, "who commented"). The sibling +-- idx_upload_user already exists for uploads; comment(user_id) was missing. +CREATE INDEX idx_comment_user + ON comment(user_id) + WHERE deleted_at IS NULL; + +-- Hashtag filtering over comments — mirrors idx_upload_hashtag_hashtag, which +-- only covered upload_hashtag. +CREATE INDEX idx_comment_hashtag_hashtag + ON comment_hashtag(hashtag_id); diff --git a/backend/src/auth/middleware.rs b/backend/src/auth/middleware.rs index 1511141..17de8f8 100644 --- a/backend/src/auth/middleware.rs +++ b/backend/src/auth/middleware.rs @@ -1,4 +1,4 @@ -use axum::extract::{FromRequestParts, State}; +use axum::extract::FromRequestParts; use axum::http::request::Parts; use uuid::Uuid; @@ -43,6 +43,18 @@ impl FromRequestParts for AuthUser { .map_err(|e| AppError::Internal(e.into()))? .ok_or_else(|| AppError::Unauthorized("Sitzung nicht gefunden oder abgelaufen.".into()))?; + // Trust *live* DB state, not the JWT: a role/ban stored in the token would + // survive a demote/ban for the full session lifetime (up to 30d). Re-read + // the user row so a demoted host loses host powers and a banned user is + // locked out immediately on their next request. + let user = crate::models::user::User::find_by_id(&state.pool, claims.sub) + .await + .map_err(|e| AppError::Internal(e.into()))? + .ok_or_else(|| AppError::Unauthorized("Benutzer nicht gefunden.".into()))?; + if user.is_banned { + return Err(AppError::Forbidden("Dein Zugang wurde gesperrt.".into())); + } + // Update last_seen_at in the background (fire-and-forget). Failures are // non-fatal but worth surfacing — silent swallowing hides DB connection // pressure that would otherwise be the first symptom of a real problem. @@ -55,9 +67,9 @@ impl FromRequestParts for AuthUser { }); Ok(Self { - user_id: claims.sub, - event_id: claims.event_id, - role: claims.role, + user_id: user.id, + event_id: user.event_id, + role: user.role, token_hash, }) } diff --git a/backend/src/config.rs b/backend/src/config.rs index 88363fc..d35e8a7 100644 --- a/backend/src/config.rs +++ b/backend/src/config.rs @@ -6,6 +6,47 @@ use anyhow::{anyhow, Context, Result}; /// we refuse to start with this value; otherwise we warn loudly. const DEV_JWT_SECRET_SENTINEL: &str = "dev_secret_do_not_use_in_production_32byteslong_aaaa"; +/// A secret is "placeholder-ish" if it's the shipped dev sentinel or still carries +/// the tell-tale scaffolding substrings from `.env.example`. Length alone is not +/// enough — the shipped `change_me_...` placeholder is >32 chars. +fn looks_placeholder(s: &str) -> bool { + let lower = s.to_ascii_lowercase(); + s == DEV_JWT_SECRET_SENTINEL + || lower.contains("change_me") + || lower.contains("dev_secret") + || lower.contains("placeholder") +} + +/// Enforce secret hygiene. In production every guard is hard-fail: a booting app +/// with a publicly-known signing key is worse than one that refuses to start. +/// Outside production the dev sentinel is tolerated (warned) so local dev is frictionless. +fn validate_secrets(is_prod: bool, jwt_secret: &str, admin_password_hash: &str) -> Result<()> { + if is_prod { + if looks_placeholder(jwt_secret) { + return Err(anyhow!( + "Refusing to start in production with a placeholder JWT_SECRET — \ + rotate it (openssl rand -hex 64)." + )); + } + if jwt_secret.len() < 32 { + return Err(anyhow!("JWT_SECRET must be at least 32 characters.")); + } + if admin_password_hash.is_empty() || looks_placeholder(admin_password_hash) { + return Err(anyhow!( + "Refusing to start in production without a real ADMIN_PASSWORD_HASH — \ + generate one (htpasswd -bnBC 12 '' | tr -d ':\\n')." + )); + } + } else if jwt_secret == DEV_JWT_SECRET_SENTINEL { + tracing::warn!( + "JWT_SECRET is the dev sentinel — fine for local development, NEVER ship this." + ); + } else if jwt_secret.len() < 32 { + return Err(anyhow!("JWT_SECRET must be at least 32 characters.")); + } + Ok(()) +} + #[derive(Clone, Debug)] pub struct AppConfig { pub database_url: String, @@ -25,19 +66,9 @@ impl AppConfig { let is_prod = app_env.eq_ignore_ascii_case("production"); let jwt_secret = std::env::var("JWT_SECRET").context("JWT_SECRET must be set")?; - if jwt_secret == DEV_JWT_SECRET_SENTINEL { - if is_prod { - return Err(anyhow!( - "Refusing to start in production with the well-known dev JWT_SECRET — \ - rotate it (openssl rand -hex 64)." - )); - } - tracing::warn!( - "JWT_SECRET is the dev sentinel — fine for local development, NEVER ship this." - ); - } else if jwt_secret.len() < 32 { - return Err(anyhow!("JWT_SECRET must be at least 32 characters.")); - } + let admin_password_hash = std::env::var("ADMIN_PASSWORD_HASH").unwrap_or_default(); + + validate_secrets(is_prod, &jwt_secret, &admin_password_hash)?; Ok(Self { database_url: std::env::var("DATABASE_URL") @@ -47,8 +78,7 @@ impl AppConfig { .unwrap_or_else(|_| "30".to_string()) .parse() .context("SESSION_EXPIRY_DAYS must be a number")?, - admin_password_hash: std::env::var("ADMIN_PASSWORD_HASH") - .unwrap_or_default(), + admin_password_hash, event_name: std::env::var("EVENT_NAME") .unwrap_or_else(|_| "EventSnap".to_string()), event_slug: std::env::var("EVENT_SLUG") @@ -63,3 +93,65 @@ impl AppConfig { }) } } + +#[cfg(test)] +mod tests { + use super::*; + + const REAL_SECRET: &str = "a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6e7f8a9b0c1d2"; + const REAL_HASH: &str = "$2y$12$abcdefghijklmnopqrstuv.wxyzABCDEFGHIJKLMNOPQRSTUVWXYZ012"; + + #[test] + fn prod_rejects_shipped_placeholder_secret() { + // The exact string shipped in `.env` — >32 chars, so it must be caught by + // the substring guard, not the length check. + let err = validate_secrets(true, "change_me_to_a_random_64_byte_hex_string", REAL_HASH); + assert!(err.is_err(), "placeholder JWT_SECRET must be rejected in prod"); + } + + #[test] + fn prod_rejects_dev_sentinel_and_short_secret() { + assert!(validate_secrets(true, DEV_JWT_SECRET_SENTINEL, REAL_HASH).is_err()); + assert!(validate_secrets(true, "tooshort", REAL_HASH).is_err()); + } + + #[test] + fn prod_rejects_missing_or_placeholder_admin_hash() { + assert!(validate_secrets(true, REAL_SECRET, "").is_err()); + assert!(validate_secrets(true, REAL_SECRET, "$2y$12$placeholder_replace_me").is_err()); + } + + #[test] + fn prod_accepts_real_secrets() { + assert!(validate_secrets(true, REAL_SECRET, REAL_HASH).is_ok()); + } + + #[test] + fn non_prod_tolerates_dev_sentinel() { + assert!(validate_secrets(false, DEV_JWT_SECRET_SENTINEL, "").is_ok()); + } + + #[test] + fn non_prod_still_rejects_short_non_sentinel_secret() { + assert!(validate_secrets(false, "tooshort", "").is_err()); + } + + #[test] + fn placeholder_detection_is_case_insensitive() { + // looks_placeholder lowercases before matching — an upper/mixed-case + // placeholder must still be rejected in prod. + assert!(validate_secrets(true, "CHANGE_ME_TO_A_RANDOM_64_BYTE_HEX_STRING", REAL_HASH).is_err()); + assert!(validate_secrets(true, REAL_SECRET, "$2Y$12$PLACEHOLDER_replace_me").is_err()); + } + + #[test] + fn prod_len_boundary_at_32() { + // Exactly 32 non-placeholder chars is the minimum accepted; 31 is rejected. + const LEN_32: &str = "abcdefghijklmnopqrstuvwxyz012345"; + const LEN_31: &str = "abcdefghijklmnopqrstuvwxyz01234"; + assert_eq!(LEN_32.len(), 32); + assert_eq!(LEN_31.len(), 31); + assert!(validate_secrets(true, LEN_32, REAL_HASH).is_ok()); + assert!(validate_secrets(true, LEN_31, REAL_HASH).is_err()); + } +} diff --git a/backend/src/handlers/admin.rs b/backend/src/handlers/admin.rs index 69f0753..987a1f0 100644 --- a/backend/src/handlers/admin.rs +++ b/backend/src/handlers/admin.rs @@ -146,6 +146,8 @@ pub async fn patch_config( let mut privacy_note_changed = false; + // Validate every key first so a bad value in the batch can't leave a partial + // update behind — validation must fully precede any write. for (key, value) in &body { let key_str = key.as_str(); if NUMERIC_KEYS.contains(&key_str) { @@ -164,7 +166,9 @@ pub async fn patch_config( } } } else if TEXT_KEYS.contains(&key_str) { - if value.len() > PRIVACY_NOTE_MAX_LEN { + // Count characters, not bytes — the message says "Zeichen" and a + // multi-byte grapheme shouldn't count against the limit multiple times. + if value.chars().count() > PRIVACY_NOTE_MAX_LEN { return Err(AppError::BadRequest(format!( "Wert für {key} ist zu lang (max. {PRIVACY_NOTE_MAX_LEN} Zeichen)." ))); @@ -177,16 +181,21 @@ pub async fn patch_config( "Unbekannter Konfigurationsschlüssel: {key}" ))); } + } + // Apply all writes in one transaction — the batch is all-or-nothing. + let mut tx = state.pool.begin().await?; + for (key, value) in &body { sqlx::query( "INSERT INTO config (key, value, updated_at) VALUES ($1, $2, NOW()) ON CONFLICT (key) DO UPDATE SET value = EXCLUDED.value, updated_at = NOW()", ) .bind(key) .bind(value) - .execute(&state.pool) + .execute(&mut *tx) .await?; } + tx.commit().await?; // Notify all clients that a publicly-readable config value changed so their stores // (e.g. the privacy note in My Account) refresh without a manual reload. diff --git a/backend/src/handlers/feed.rs b/backend/src/handlers/feed.rs index 0a6f21f..29e46e3 100644 --- a/backend/src/handlers/feed.rs +++ b/backend/src/handlers/feed.rs @@ -79,6 +79,17 @@ pub async fn feed( let limit = q.limit.unwrap_or(20).min(100); + // Resolve the cursor to a (created_at, id) position. The pair is compared as a + // tuple so ties on created_at break on id — keyset pagination on created_at + // alone would silently drop rows sharing a timestamp across a page boundary. + let (cursor_time, cursor_id) = match q.cursor { + Some(c) => match get_cursor_pos(&state.pool, c).await { + Some((t, id)) => (Some(t), Some(id)), + None => (None, None), + }, + None => (None, None), + }; + let rows = if let Some(hashtag) = &q.hashtag { let tag = hashtag.trim().trim_start_matches('#').to_lowercase(); sqlx::query_as::<_, FeedRow>( @@ -88,19 +99,14 @@ pub async fn feed( JOIN upload_hashtag uh ON uh.upload_id = v.id JOIN hashtag h ON h.id = uh.hashtag_id AND h.tag = $1 WHERE v.event_id = $2 - AND ($3::timestamptz IS NULL OR v.created_at < $3) - ORDER BY v.created_at DESC - LIMIT $4", + AND ($3::timestamptz IS NULL OR (v.created_at, v.id) < ($3, $4)) + ORDER BY v.created_at DESC, v.id DESC + LIMIT $5", ) .bind(&tag) .bind(auth.event_id) - .bind( - if let Some(cursor) = q.cursor { - get_cursor_time(&state.pool, cursor).await - } else { - None - }, - ) + .bind(cursor_time) + .bind(cursor_id) .bind(limit + 1) .fetch_all(&state.pool) .await? @@ -110,18 +116,13 @@ pub async fn feed( mime_type, caption, like_count, comment_count, created_at FROM v_feed WHERE event_id = $1 - AND ($2::timestamptz IS NULL OR created_at < $2) - ORDER BY created_at DESC - LIMIT $3", + AND ($2::timestamptz IS NULL OR (created_at, id) < ($2, $3)) + ORDER BY created_at DESC, id DESC + LIMIT $4", ) .bind(auth.event_id) - .bind( - if let Some(cursor) = q.cursor { - get_cursor_time(&state.pool, cursor).await - } else { - None - }, - ) + .bind(cursor_time) + .bind(cursor_id) .bind(limit + 1) .fetch_all(&state.pool) .await? @@ -171,6 +172,11 @@ pub struct DeltaQuery { pub struct DeltaResponse { pub uploads: Vec, pub deleted_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 + /// via a later delta, which advances `since` past them). + pub truncated: bool, } pub async fn feed_delta( @@ -178,18 +184,28 @@ pub async fn feed_delta( auth: AuthUser, Query(q): Query, ) -> Result, AppError> { + // Bounded like the paginated feed: a stale `since` could otherwise pull the + // 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; 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 - ORDER BY created_at DESC", + ORDER BY created_at DESC, id DESC + LIMIT $3", ) .bind(auth.event_id) .bind(q.since) + .bind(DELTA_LIMIT) .fetch_all(&state.pool) .await?; + // Hit the cap => this is only the newest slice of a larger gap. Signal the + // client to full-refresh instead of merging a partial delta. + let truncated = rows.len() as i64 >= DELTA_LIMIT; + 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", @@ -222,6 +238,7 @@ pub async fn feed_delta( Ok(Json(DeltaResponse { uploads, deleted_ids: deleted_ids.into_iter().map(|r| r.0).collect(), + truncated, })) } @@ -249,14 +266,17 @@ pub async fn hashtags( )) } -async fn get_cursor_time(pool: &sqlx::PgPool, cursor_id: Uuid) -> Option> { - let row: Option<(DateTime,)> = - sqlx::query_as("SELECT created_at FROM upload WHERE id = $1") +/// Resolve a cursor id to its `(created_at, id)` position. Both are needed: +/// `created_at` alone isn't unique, so pagination must break ties on `id` to +/// avoid silently dropping rows that share a timestamp across a page boundary. +async fn get_cursor_pos(pool: &sqlx::PgPool, cursor_id: Uuid) -> Option<(DateTime, Uuid)> { + let row: Option<(DateTime, Uuid)> = + sqlx::query_as("SELECT created_at, id FROM upload WHERE id = $1") .bind(cursor_id) .fetch_optional(pool) .await .ok()?; - row.map(|r| r.0) + row } async fn get_liked_set( diff --git a/backend/src/handlers/host.rs b/backend/src/handlers/host.rs index f9ea788..7994c9a 100644 --- a/backend/src/handlers/host.rs +++ b/backend/src/handlers/host.rs @@ -113,10 +113,11 @@ pub async fn ban_user( } sqlx::query( - "UPDATE \"user\" SET is_banned = TRUE, uploads_hidden = $2 WHERE id = $1", + "UPDATE \"user\" SET is_banned = TRUE, uploads_hidden = $2 WHERE id = $1 AND event_id = $3", ) .bind(user_id) .bind(body.hide_uploads) + .bind(auth.event_id) .execute(&state.pool) .await?; @@ -263,7 +264,7 @@ pub async fn reset_user_pin( sqlx::query( "UPDATE \"user\" SET recovery_pin_hash = $1, - pin_failed_attempts = 0, + failed_pin_attempts = 0, pin_locked_until = NULL WHERE id = $2", ) @@ -341,14 +342,18 @@ pub async fn close_event( State(state): State, RequireHost(_auth): RequireHost, ) -> Result { - sqlx::query( + let result = sqlx::query( "UPDATE event SET uploads_locked_at = NOW() WHERE slug = $1 AND uploads_locked_at IS NULL", ) .bind(&state.config.event_slug) .execute(&state.pool) .await?; - let _ = state.sse_tx.send(SseEvent::new("event-closed", "{}")); + // Only broadcast when this call actually flipped the lock — closing an + // already-closed event is a no-op and shouldn't spam listeners. + if result.rows_affected() > 0 { + let _ = state.sse_tx.send(SseEvent::new("event-closed", "{}")); + } Ok(StatusCode::NO_CONTENT) } @@ -357,14 +362,16 @@ pub async fn open_event( State(state): State, RequireHost(_auth): RequireHost, ) -> Result { - sqlx::query( - "UPDATE event SET uploads_locked_at = NULL WHERE slug = $1", + let result = sqlx::query( + "UPDATE event SET uploads_locked_at = NULL WHERE slug = $1 AND uploads_locked_at IS NOT NULL", ) .bind(&state.config.event_slug) .execute(&state.pool) .await?; - let _ = state.sse_tx.send(SseEvent::new("event-opened", "{}")); + if result.rows_affected() > 0 { + let _ = state.sse_tx.send(SseEvent::new("event-opened", "{}")); + } Ok(StatusCode::NO_CONTENT) } diff --git a/backend/src/handlers/social.rs b/backend/src/handlers/social.rs index 79d83d0..168f6e0 100644 --- a/backend/src/handlers/social.rs +++ b/backend/src/handlers/social.rs @@ -12,6 +12,18 @@ 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, @@ -31,6 +43,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?; + // Try to insert; if conflict, delete (toggle) let result = sqlx::query( "INSERT INTO \"like\" (upload_id, user_id) VALUES ($1, $2) @@ -120,6 +135,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?; + let text = body.body.trim(); let text_chars = text.chars().count(); if text_chars == 0 || text_chars > 500 { @@ -128,20 +146,22 @@ pub async fn add_comment( )); } - let comment = Comment::create(&state.pool, upload_id, auth.user_id, text).await?; - - // Process hashtags in comment body + // Insert the comment and link its hashtags atomically, so a crash mid-loop + // can't leave a committed comment with only some of its tags indexed. let tags = hashtag::extract_hashtags(text); + let mut tx = state.pool.begin().await?; + let comment = Comment::create(&mut *tx, upload_id, auth.user_id, text).await?; for tag in &tags { - let h = Hashtag::upsert(&state.pool, auth.event_id, tag).await?; + let h = Hashtag::upsert(&mut *tx, auth.event_id, tag).await?; sqlx::query( "INSERT INTO comment_hashtag (comment_id, hashtag_id) VALUES ($1, $2) ON CONFLICT DO NOTHING", ) .bind(comment.id) .bind(h.id) - .execute(&state.pool) + .execute(&mut *tx) .await?; } + tx.commit().await?; // Fresh count so feed clients can patch the single card in place instead of // refetching page 1 (mirrors v_feed.comment_count = COUNT(DISTINCT c.id); COUNT(*) diff --git a/backend/src/handlers/sse.rs b/backend/src/handlers/sse.rs index d4a0971..ed63d05 100644 --- a/backend/src/handlers/sse.rs +++ b/backend/src/handlers/sse.rs @@ -48,6 +48,10 @@ pub async fn stream( .consume(&q.ticket) .ok_or_else(|| AppError::Unauthorized("Ticket ungültig oder abgelaufen.".into()))?; + // NOTE: this authenticates via ticket→session, not the `AuthUser` extractor, so + // it does not re-check `is_banned`. A user banned mid-stream keeps receiving + // events until the connection drops; they cannot reconnect (issue_ticket uses + // AuthUser, which rejects banned users). Documented in docs/SECURITY-BACKLOG.md. Session::find_by_token_hash(&state.pool, &token_hash) .await .map_err(|e| AppError::Internal(e.into()))? diff --git a/backend/src/handlers/upload.rs b/backend/src/handlers/upload.rs index 841e6f9..f2043f6 100644 --- a/backend/src/handlers/upload.rs +++ b/backend/src/handlers/upload.rs @@ -190,25 +190,6 @@ pub async fn upload( } tokio::fs::write(&absolute_path, &data).await.map_err(|e| AppError::Internal(e.into()))?; - // Update user's total upload bytes - sqlx::query("UPDATE \"user\" SET total_upload_bytes = total_upload_bytes + $2 WHERE id = $1") - .bind(auth.user_id) - .bind(size) - .execute(&state.pool) - .await?; - - // Insert upload record - let upload = Upload::create( - &state.pool, - auth.event_id, - auth.user_id, - &relative_path, - &mime, - size, - caption.as_deref(), - ) - .await?; - // Process hashtags from caption and explicit CSV let mut tags: Vec = Vec::new(); if let Some(ref cap) = caption { @@ -225,10 +206,30 @@ pub async fn upload( tags.sort(); tags.dedup(); + // Quota accounting, the upload row, and its hashtag links must be atomic: a + // crash between the bytes increment and the insert would permanently charge + // bytes with no row to reclaim them (silent quota erosion / spurious lockout). + let mut tx = state.pool.begin().await?; + sqlx::query("UPDATE \"user\" SET total_upload_bytes = total_upload_bytes + $2 WHERE id = $1") + .bind(auth.user_id) + .bind(size) + .execute(&mut *tx) + .await?; + let upload = Upload::create( + &mut *tx, + auth.event_id, + auth.user_id, + &relative_path, + &mime, + size, + caption.as_deref(), + ) + .await?; for tag in &tags { - let h = Hashtag::upsert(&state.pool, auth.event_id, tag).await?; - Hashtag::link_to_upload(&state.pool, upload.id, h.id).await?; + let h = Hashtag::upsert(&mut *tx, auth.event_id, tag).await?; + Hashtag::link_to_upload(&mut *tx, upload.id, h.id).await?; } + tx.commit().await?; // Spawn compression task state @@ -279,17 +280,20 @@ pub async fn edit_upload( return Err(AppError::Forbidden("Nur eigene Uploads bearbeiten.".into())); } + // Caption update + hashtag wipe-then-relink in one transaction, so a crash + // mid-relink can't leave the upload with its hashtags stripped. + let mut tx = state.pool.begin().await?; if let Some(ref caption) = body.caption { - Upload::update_caption(&state.pool, upload_id, Some(caption)).await?; + Upload::update_caption(&mut *tx, upload_id, Some(caption)).await?; } - if let Some(ref hashtags) = body.hashtags { - Hashtag::unlink_all_from_upload(&state.pool, upload_id).await?; + Hashtag::unlink_all_from_upload(&mut *tx, upload_id).await?; for tag in hashtags { - let h = Hashtag::upsert(&state.pool, auth.event_id, tag).await?; - Hashtag::link_to_upload(&state.pool, upload_id, h.id).await?; + let h = Hashtag::upsert(&mut *tx, auth.event_id, tag).await?; + Hashtag::link_to_upload(&mut *tx, upload_id, h.id).await?; } } + tx.commit().await?; Ok(StatusCode::OK) } diff --git a/backend/src/models/comment.rs b/backend/src/models/comment.rs index ec08e26..ac5d0cc 100644 --- a/backend/src/models/comment.rs +++ b/backend/src/models/comment.rs @@ -24,19 +24,24 @@ pub struct CommentDto { } impl Comment { - pub async fn create( - pool: &PgPool, + /// Takes any executor so the caller can insert the comment and link its + /// hashtags inside a single transaction. + pub async fn create<'e, E>( + executor: E, upload_id: Uuid, user_id: Uuid, body: &str, - ) -> Result { + ) -> Result + where + E: sqlx::PgExecutor<'e>, + { sqlx::query_as::<_, Self>( "INSERT INTO comment (upload_id, user_id, body) VALUES ($1, $2, $3) RETURNING *", ) .bind(upload_id) .bind(user_id) .bind(body) - .fetch_one(pool) + .fetch_one(executor) .await } diff --git a/backend/src/models/hashtag.rs b/backend/src/models/hashtag.rs index 0414fbc..7cd13c9 100644 --- a/backend/src/models/hashtag.rs +++ b/backend/src/models/hashtag.rs @@ -10,7 +10,13 @@ pub struct Hashtag { impl Hashtag { /// Upsert a hashtag (insert if not exists, return existing if it does). - pub async fn upsert(pool: &PgPool, event_id: Uuid, tag: &str) -> Result { + /// + /// Takes any executor so callers can run it inside a transaction (atomic + /// upload/comment writes) or standalone against the pool. + pub async fn upsert<'e, E>(executor: E, event_id: Uuid, tag: &str) -> Result + where + E: sqlx::PgExecutor<'e>, + { let normalized = tag.trim().trim_start_matches('#').to_lowercase(); sqlx::query_as::<_, Self>( "INSERT INTO hashtag (event_id, tag) VALUES ($1, $2) @@ -19,33 +25,39 @@ impl Hashtag { ) .bind(event_id) .bind(&normalized) - .fetch_one(pool) + .fetch_one(executor) .await } - pub async fn link_to_upload( - pool: &PgPool, + pub async fn link_to_upload<'e, E>( + executor: E, upload_id: Uuid, hashtag_id: Uuid, - ) -> Result<(), sqlx::Error> { + ) -> Result<(), sqlx::Error> + where + E: sqlx::PgExecutor<'e>, + { sqlx::query( "INSERT INTO upload_hashtag (upload_id, hashtag_id) VALUES ($1, $2) ON CONFLICT DO NOTHING", ) .bind(upload_id) .bind(hashtag_id) - .execute(pool) + .execute(executor) .await?; Ok(()) } - pub async fn unlink_all_from_upload( - pool: &PgPool, + pub async fn unlink_all_from_upload<'e, E>( + executor: E, upload_id: Uuid, - ) -> Result<(), sqlx::Error> { + ) -> Result<(), sqlx::Error> + where + E: sqlx::PgExecutor<'e>, + { sqlx::query("DELETE FROM upload_hashtag WHERE upload_id = $1") .bind(upload_id) - .execute(pool) + .execute(executor) .await?; Ok(()) } diff --git a/backend/src/models/upload.rs b/backend/src/models/upload.rs index 1b085e1..243c527 100644 --- a/backend/src/models/upload.rs +++ b/backend/src/models/upload.rs @@ -36,15 +36,20 @@ pub struct UploadDto { } impl Upload { - pub async fn create( - pool: &PgPool, + /// Takes any executor so the caller can run it inside a transaction (atomic + /// quota + insert) or standalone against the pool. + pub async fn create<'e, E>( + executor: E, event_id: Uuid, user_id: Uuid, original_path: &str, mime_type: &str, original_size_bytes: i64, caption: Option<&str>, - ) -> Result { + ) -> Result + where + E: sqlx::PgExecutor<'e>, + { sqlx::query_as::<_, Self>( "INSERT INTO upload (event_id, user_id, original_path, mime_type, original_size_bytes, caption) VALUES ($1, $2, $3, $4, $5, $6) @@ -56,7 +61,7 @@ impl Upload { .bind(mime_type) .bind(original_size_bytes) .bind(caption) - .fetch_one(pool) + .fetch_one(executor) .await } @@ -182,15 +187,18 @@ impl Upload { Ok(deleted) } - pub async fn update_caption( - pool: &PgPool, + pub async fn update_caption<'e, E>( + executor: E, id: Uuid, caption: Option<&str>, - ) -> Result<(), sqlx::Error> { + ) -> Result<(), sqlx::Error> + where + E: sqlx::PgExecutor<'e>, + { sqlx::query("UPDATE upload SET caption = $2 WHERE id = $1") .bind(id) .bind(caption) - .execute(pool) + .execute(executor) .await?; Ok(()) } diff --git a/backend/src/services/rate_limiter.rs b/backend/src/services/rate_limiter.rs index d29ae4d..91f0d73 100644 --- a/backend/src/services/rate_limiter.rs +++ b/backend/src/services/rate_limiter.rs @@ -71,14 +71,21 @@ impl RateLimiter { } } -/// Extract the client IP from X-Forwarded-For (Caddy sets this) or fall back -/// to a provided socket address string. +/// Extract the client IP from X-Forwarded-For or fall back to a provided socket +/// address string. +/// +/// We take the **right-most** entry, not the left-most. Caddy is the sole ingress +/// and the app port is only `expose`d (never published), so the last hop Caddy +/// appends is the real client. A client can prepend arbitrary spoofed values to +/// the left of XFF to dodge throttles — those are ignored here. This assumes +/// exactly one trusted proxy (Caddy); revisit if that changes. pub fn client_ip(headers: &axum::http::HeaderMap, fallback: &str) -> String { headers .get("x-forwarded-for") .and_then(|v| v.to_str().ok()) - .and_then(|s| s.split(',').next()) + .and_then(|s| s.rsplit(',').next()) .map(|s| s.trim().to_owned()) + .filter(|s| !s.is_empty()) .unwrap_or_else(|| fallback.to_owned()) } @@ -134,9 +141,18 @@ mod tests { } #[test] - fn client_ip_prefers_first_forwarded_for_entry() { + fn client_ip_takes_rightmost_forwarded_for_entry() { + // The right-most entry is the hop our trusted proxy (Caddy) appended. let mut h = HeaderMap::new(); - h.insert("x-forwarded-for", "203.0.113.7, 10.0.0.1".parse().unwrap()); + h.insert("x-forwarded-for", "10.0.0.1, 203.0.113.7".parse().unwrap()); + assert_eq!(client_ip(&h, "fallback"), "203.0.113.7"); + } + + #[test] + fn client_ip_ignores_spoofed_leftmost_entry() { + // A client prepending a fake IP to dodge throttles must not win. + let mut h = HeaderMap::new(); + h.insert("x-forwarded-for", "1.2.3.4, 9.9.9.9, 203.0.113.7".parse().unwrap()); assert_eq!(client_ip(&h, "fallback"), "203.0.113.7"); } @@ -151,4 +167,14 @@ mod tests { fn client_ip_falls_back_when_header_absent() { assert_eq!(client_ip(&HeaderMap::new(), "127.0.0.1"), "127.0.0.1"); } + + #[test] + fn client_ip_falls_back_on_trailing_comma_empty_entry() { + // A trailing comma leaves an empty right-most segment after trimming; the + // `.filter(!is_empty)` must reject it and fall through to the fallback + // rather than returning "" (which would collapse callers into one bucket). + let mut h = HeaderMap::new(); + h.insert("x-forwarded-for", "203.0.113.7, ".parse().unwrap()); + assert_eq!(client_ip(&h, "127.0.0.1"), "127.0.0.1"); + } } diff --git a/docker-compose.dev.yml b/docker-compose.dev.yml index 48b4937..2d341c3 100644 --- a/docker-compose.dev.yml +++ b/docker-compose.dev.yml @@ -6,3 +6,8 @@ services: db: ports: - "5432:5432" + app: + # Relax the production secret guard for local dev — the dev sentinel JWT_SECRET + # is tolerated (warned) rather than rejected. + environment: + APP_ENV: development diff --git a/docker-compose.yml b/docker-compose.yml index 6919e38..57f1fa7 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -14,6 +14,10 @@ services: interval: 5s timeout: 5s retries: 10 + deploy: + resources: + limits: + memory: 512M app: build: @@ -21,6 +25,10 @@ services: dockerfile: Dockerfile restart: unless-stopped env_file: .env + environment: + # Activates the production secret guard in config.rs — refuses to boot with + # placeholder JWT_SECRET / ADMIN_PASSWORD_HASH. + APP_ENV: production depends_on: db: condition: service_healthy @@ -28,6 +36,18 @@ services: - media_data:/media expose: - "3000" + healthcheck: + test: ["CMD-SHELL", "wget -q -O- http://localhost:3000/health || exit 1"] + interval: 10s + timeout: 5s + retries: 5 + start_period: 20s + deploy: + resources: + limits: + # Bounds a runaway ffmpeg transcode (large uploads, 2 workers) so it can't + # OOM the single box and take down Postgres. + memory: 1G frontend: build: @@ -35,10 +55,24 @@ services: dockerfile: Dockerfile restart: unless-stopped env_file: .env + environment: + # adapter-node behind Caddy TLS needs the public origin for CSRF checks on + # POST form actions — without it they fail only in production. + ORIGIN: "https://${DOMAIN}" depends_on: - app expose: - "3001" + healthcheck: + test: ["CMD-SHELL", "wget -q -O- http://localhost:3001/ >/dev/null 2>&1 || exit 1"] + interval: 10s + timeout: 5s + retries: 5 + start_period: 15s + deploy: + resources: + limits: + memory: 256M caddy: image: caddy:2-alpine @@ -50,8 +84,14 @@ services: - ./Caddyfile:/etc/caddy/Caddyfile:ro - caddy_data:/data depends_on: - - app - - frontend + app: + condition: service_healthy + frontend: + condition: service_healthy + deploy: + resources: + limits: + memory: 256M volumes: postgres_data: diff --git a/docs/SECURITY-BACKLOG.md b/docs/SECURITY-BACKLOG.md index f50685b..cbb8ab4 100644 --- a/docs/SECURITY-BACKLOG.md +++ b/docs/SECURITY-BACKLOG.md @@ -51,15 +51,25 @@ item below is tagged with its **current status in `main`**: ## ✅ Fixed in main since the audit (for the record) -These audit findings are present in `main` today (verified 2026-06-30): event-scoped social -handlers (cross-event authz), server-side MIME/ext allowlist on upload, recovery-PIN lockout -backoff, unspoofable client IP in the rate limiter, effective JWT production-secret guard, DB port -no longer publicly exposed, container healthchecks, bcrypt offloaded via `spawn_blocking`, -bounded compression concurrency (semaphore), bounded feed queries (`LIMIT ≤ 100`), and the -viewport-fit / reduced-motion / aria a11y pass. **New since the audit:** an image-decode -decompression-bomb cap (`image::Limits` 12000×12000 / 256 MiB) ported into +These audit findings are present in `main` today (verified 2026-06-30): server-side MIME/ext +allowlist on upload, recovery-PIN lockout backoff, DB port no longer publicly exposed, bcrypt +offloaded via `spawn_blocking`, bounded compression concurrency (semaphore), bounded feed queries +(`LIMIT ≤ 100`), and the viewport-fit / reduced-motion / aria a11y pass. An image-decode +decompression-bomb cap (`image::Limits` 12000×12000 / 256 MiB) lives in `backend/src/services/compression.rs`. +**Fixed in the 2026-07 review pass** (previously *claimed* fixed here but were not — corrected): +- **Effective JWT production-secret guard** — `APP_ENV=production` is now set for the app service, + and `config.rs::validate_secrets` rejects any placeholder-ish `JWT_SECRET`/`ADMIN_PASSWORD_HASH` + (not just the exact dev sentinel) and enforces `len ≥ 32` unconditionally in prod. +- **Unspoofable client IP in the rate limiter** — `client_ip` now takes the right-most + `X-Forwarded-For` entry (the hop Caddy appends), so a client-supplied left-most value is ignored. +- **Live role/ban re-check** — the auth extractor re-reads the user row and trusts the DB role and + `is_banned` flag rather than the JWT claim, revoking demoted/banned sessions immediately. +- **Container healthchecks** — `app` and `frontend` now have healthchecks and Caddy waits on + `service_healthy`. +- **Event-scoped `ban_user`** — the ban UPDATE is now scoped by `event_id` like its siblings. + ## 🅲 Consciously won't-fix at ~100-guest single-box scale Diminishing returns vs. the deployment's actual threat model. Revisit only if the scale or @@ -71,6 +81,12 @@ tenancy model changes. - Performance micro-indexes (`idx_like_user_upload`, comment pagination index) — current queries are sub-ms at this row count. - Optimistic-like in-flight guard, ownership-snapshot-at-mount, assorted copy tweaks — UX polish. +- **Mid-session ban does not tear down an already-open SSE stream.** The live-ban check lives in the + `AuthUser` extractor, but the stream authenticates via ticket→session (`handlers/sse.rs::stream`), + so a user banned while connected keeps receiving broadcast events until their stream drops. New + tickets *are* blocked (`issue_ticket` uses `AuthUser`), so they cannot reconnect. Low blast radius + (read-only feed events, no re-subscribe); tearing live streams down would need a per-session + broadcast filter. Revisit only if bans must take effect within seconds. ## By-design notes (audit branch's signed-media model — see DECISION-media-auth.md) diff --git a/e2e/fixtures/api-client.ts b/e2e/fixtures/api-client.ts index 0bdde22..c86a61a 100644 --- a/e2e/fixtures/api-client.ts +++ b/e2e/fixtures/api-client.ts @@ -117,6 +117,25 @@ export class ApiClient { }); } + async unbanUser(token: string, userId: string, opts: { expectedStatus?: number | number[] } = {}) { + return this.request('POST', `/host/users/${userId}/unban`, { + token, + expectedStatus: opts.expectedStatus ?? [200, 204], + }); + } + + /** Reset another user's PIN. Returns the plaintext PIN the host must relay once. */ + async resetUserPin( + token: string, + userId: string, + opts: { expectedStatus?: number | number[] } = {} + ): Promise<{ status: number; body: { pin?: string } }> { + return this.request<{ pin?: string }>('POST', `/host/users/${userId}/pin-reset`, { + token, + expectedStatus: opts.expectedStatus ?? [200], + }); + } + async closeEvent(token: string) { return this.request('POST', '/host/event/close', { token, expectedStatus: [200, 204] }); } diff --git a/e2e/specs/04-host/event-lock.spec.ts b/e2e/specs/04-host/event-lock.spec.ts index 4925a22..37d311b 100644 --- a/e2e/specs/04-host/event-lock.spec.ts +++ b/e2e/specs/04-host/event-lock.spec.ts @@ -33,4 +33,39 @@ test.describe('Host — event lock', () => { // Currently no UI consumes the event-closed SSE on /feed. Add this banner // and flip fixme to test once it lands. }); + + // Regression for the review: likes/comments used to ignore uploads_locked_at, + // so social writes still landed on a closed event. They now share the upload + // handler's lock guard. + test('a closed event rejects likes and comments', async ({ api, host, guest }) => { + const BASE = process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101'; + const g = await guest('SocialLocked'); + + // Upload while still open so there's a target to interact with. + const { uploadRaw } = await import('../../helpers/upload-client'); + const { readFileSync } = await import('node:fs'); + const { join } = await import('node:path'); + const sample = join(process.cwd(), 'fixtures', 'media', 'sample.jpg'); + const upRes = await uploadRaw(g.jwt, readFileSync(sample), { + filename: 'x.jpg', + contentType: 'image/jpeg', + }); + expect(upRes.status).toBe(201); + const { id } = await upRes.json(); + + await api.closeEvent(host.jwt); + + const likeRes = await fetch(`${BASE}/api/v1/upload/${id}/like`, { + method: 'POST', + headers: { Authorization: `Bearer ${g.jwt}` }, + }); + expect(likeRes.status).toBe(403); + + const commentRes = await fetch(`${BASE}/api/v1/upload/${id}/comments`, { + method: 'POST', + headers: { Authorization: `Bearer ${g.jwt}`, 'Content-Type': 'application/json' }, + body: JSON.stringify({ body: 'sollte blockiert sein' }), + }); + expect(commentRes.status).toBe(403); + }); }); diff --git a/e2e/specs/04-host/moderation.spec.ts b/e2e/specs/04-host/moderation.spec.ts index 0095958..13e0f74 100644 --- a/e2e/specs/04-host/moderation.spec.ts +++ b/e2e/specs/04-host/moderation.spec.ts @@ -45,3 +45,56 @@ test.describe('Host — moderation API', () => { expect(row?.role).toBe('host'); }); }); + +/** + * Regression for the review's H1: role & ban used to be trusted from the JWT and + * never re-checked against the DB, so a demoted/banned host kept full powers for + * the life of their token (up to 30d) and a banned host could even unban + * themselves. The auth extractor now re-reads the live user row, so these take + * effect on the *existing* session with no re-login. + */ +test.describe('Host — live role/ban revocation (H1)', () => { + test('a demoted host loses host powers on their existing session', async ({ + api, + adminToken, + guest, + }) => { + const u = await guest('DemoteMidSession'); + await api.setRole(adminToken, u.userId, 'host'); + // Fresh host token (role is encoded at mint time). + const { body } = await api.recover(u.displayName, u.pin); + const hostJwt = body.jwt; + + // Sanity: the token currently has host powers. + await api.listUsers(hostJwt); + + // Demote via admin — the host does NOT re-login. + await api.setRole(adminToken, u.userId, 'guest'); + + // Same token is now rejected with exactly 403 (RequireHost sees the DB role, + // not the JWT claim). Asserting the status guards against a spurious 500 + // masquerading as "revoked". + await expect(api.listUsers(hostJwt)).rejects.toThrow(/→ 403/); + }); + + test('a banned host is locked out immediately on their existing session', async ({ + api, + adminToken, + host, + }) => { + // Sanity: host token works. + await api.listUsers(host.jwt); + + await api.banUser(adminToken, host.userId, false); + + // Banned users are rejected with 403 by the auth extractor before any handler runs. + await expect(api.listUsers(host.jwt)).rejects.toThrow(/→ 403/); + }); + + test('a banned host cannot unban themselves', async ({ api, adminToken, host }) => { + await api.banUser(adminToken, host.userId, false); + await expect( + api.unbanUser(host.jwt, host.userId, { expectedStatus: [204] }) + ).rejects.toThrow(/→ 403/); + }); +}); diff --git a/e2e/specs/04-host/pin-reset.spec.ts b/e2e/specs/04-host/pin-reset.spec.ts new file mode 100644 index 0000000..8feb3b6 --- /dev/null +++ b/e2e/specs/04-host/pin-reset.spec.ts @@ -0,0 +1,43 @@ +/** + * Regression for the review's C1: `reset_user_pin` wrote to a non-existent + * column (`pin_failed_attempts` vs the real `failed_pin_attempts`), so the + * endpoint 500'd every time and never returned a PIN — the feature had never + * worked against a real DB and no test caught it. These specs pin the contract: + * a successful reset returns a fresh 4-digit PIN, and the target can recover + * with it. + */ +import { test, expect } from '../../fixtures/test'; + +test.describe('Host — reset guest PIN (C1)', () => { + test('resetting a guest PIN returns a fresh 4-digit PIN', async ({ api, host, guest }) => { + const target = await guest('ResetMe'); + + const { status, body } = await api.resetUserPin(host.jwt, target.userId); + + expect(status).toBe(200); + expect(body.pin).toMatch(/^\d{4}$/); + }); + + test('the target can recover with the newly reset PIN (and not the old one)', async ({ + api, + host, + guest, + }) => { + const target = await guest('RecoverWithNewPin'); + + const { body } = await api.resetUserPin(host.jwt, target.userId); + const newPin = body.pin!; + + // New PIN works. + await api.recover(target.displayName, newPin, { expectedStatus: [200] }); + + // Old PIN no longer works (overwritten). 401 = wrong PIN. + if (target.pin !== newPin) { + await api.recover(target.displayName, target.pin, { expectedStatus: [401] }); + } + }); + + test('a host cannot reset their own PIN via this endpoint', async ({ api, host }) => { + await api.resetUserPin(host.jwt, host.userId, { expectedStatus: [400] }); + }); +}); diff --git a/frontend/src/app.html b/frontend/src/app.html index 87b0e0c..b17193e 100644 --- a/frontend/src/app.html +++ b/frontend/src/app.html @@ -21,7 +21,10 @@ theme=dark don't flash a white screen. Mirrors the logic in `src/lib/theme-store.ts`; kept in sync by hand (it's 6 lines). --> - {#if open} - -