From 2068e8c1f3cb2b7aa15c1f3dc74cd6620174119c Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Sat, 27 Jun 2026 15:14:22 +0200 Subject: [PATCH] fix(auth): reconcile role/ban from DB, revoke sessions, fix pin/comment bugs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit H1: AuthUser now JOINs session→user and sources role/ban/event from the live DB row instead of trusting JWT claims. Bans take effect on the next request; demotion/promotion is immediate. Adds Session::find_auth_context and rejects tokens whose sub/event_id disagree with the session row. This also closes M2 (banned export) and M3 (banned edit/delete) globally — banned users can no longer pass the AuthUser extractor. Adds Session::delete_by_user_id and revokes all of a user's sessions on ban_user, set_role, and reset_user_pin so existing JWTs die immediately. ban_user also emits a user-banned SSE for forced client logout. H9: reset_user_pin used a non-existent column pin_failed_attempts (runtime sqlx, so it 500'd in prod). Corrected to failed_pin_attempts — the only account-recovery path now works. C4: LightboxModal posted comments to /comment (singular); backend only registers /comments. One-character route fix re-enables the comment feature. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/src/auth/middleware.rs | 28 +++++++++--- backend/src/handlers/host.rs | 32 +++++++++++++- backend/src/models/session.rs | 44 +++++++++++++++++++ .../src/lib/components/LightboxModal.svelte | 2 +- 4 files changed, 99 insertions(+), 7 deletions(-) diff --git a/backend/src/auth/middleware.rs b/backend/src/auth/middleware.rs index 1511141..e0786aa 100644 --- a/backend/src/auth/middleware.rs +++ b/backend/src/auth/middleware.rs @@ -38,16 +38,32 @@ impl FromRequestParts for AuthUser { let token_hash = jwt::hash_token(token); - let session = Session::find_by_token_hash(&state.pool, &token_hash) + // Reconcile the session against the live `user` row. We do NOT trust the + // JWT claims for role/ban/event — a token can outlive a ban or demotion + // (default lifetime 30 days). The single JOIN query is the same number + // of round-trips as the old existence check. + let ctx = Session::find_auth_context(&state.pool, &token_hash) .await .map_err(|e| AppError::Internal(e.into()))? .ok_or_else(|| AppError::Unauthorized("Sitzung nicht gefunden oder abgelaufen.".into()))?; + // Ban takes effect immediately on the next request, regardless of the + // token's remaining lifetime. + if ctx.is_banned { + return Err(AppError::Forbidden("Du bist gesperrt.".into())); + } + + // Defend against a JWT that was issued for a different user/event than + // the session's DB row points at (e.g. a swapped or replayed token). + if claims.sub != ctx.user_id || claims.event_id != ctx.event_id { + return Err(AppError::Unauthorized("Token passt nicht zur Sitzung.".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. let pool = state.pool.clone(); - let session_id = session.id; + let session_id = ctx.session_id; tokio::spawn(async move { if let Err(e) = Session::touch(&pool, session_id).await { tracing::warn!(error = ?e, session_id = %session_id, "session touch failed"); @@ -55,9 +71,11 @@ impl FromRequestParts for AuthUser { }); Ok(Self { - user_id: claims.sub, - event_id: claims.event_id, - role: claims.role, + user_id: ctx.user_id, + event_id: ctx.event_id, + // Live role from the DB, not the claim — demotion/promotion takes + // effect on the next request. + role: ctx.role, token_hash, }) } diff --git a/backend/src/handlers/host.rs b/backend/src/handlers/host.rs index f9ea788..6cad627 100644 --- a/backend/src/handlers/host.rs +++ b/backend/src/handlers/host.rs @@ -9,10 +9,21 @@ use crate::auth::middleware::RequireHost; use crate::error::AppError; use crate::models::comment::Comment; use crate::models::event::Event; +use crate::models::session::Session; use crate::models::upload::Upload; use crate::models::user::UserRole; use crate::state::{AppState, SseEvent}; +/// Revoke all of a user's sessions (best-effort). A failure here is logged but +/// never fails the surrounding admin action — the live-role/ban reconciliation +/// in `AuthUser` is the authoritative gate; revocation is defense-in-depth. +async fn revoke_sessions(pool: &sqlx::PgPool, user_id: Uuid, action: &str) { + match Session::delete_by_user_id(pool, user_id).await { + Ok(n) => tracing::info!(target_user_id = %user_id, revoked = n, action, "sessions revoked"), + Err(e) => tracing::warn!(error = ?e, target_user_id = %user_id, action, "session revoke failed"), + } +} + // ── DTOs ───────────────────────────────────────────────────────────────────── #[derive(Serialize, sqlx::FromRow)] @@ -120,6 +131,14 @@ pub async fn ban_user( .execute(&state.pool) .await?; + revoke_sessions(&state.pool, user_id, "ban_user").await; + + // Tell the banned user's online devices to log out immediately. + let _ = state.sse_tx.send(SseEvent::new( + "user-banned", + serde_json::json!({ "user_id": user_id }).to_string(), + )); + tracing::info!( actor_user_id = %auth.user_id, target_user_id = %user_id, @@ -201,6 +220,13 @@ pub async fn set_role( .bind(auth.event_id) .execute(&state.pool) .await?; + + // Force a clean re-auth so the new role can't be cached client-side and so + // any in-flight tokens carrying the old role are invalidated. The live-role + // reconciliation in AuthUser already prevents privilege escalation, but + // revoking is cheaper to reason about. + revoke_sessions(&state.pool, user_id, "set_role").await; + tracing::info!( actor_user_id = %auth.user_id, target_user_id = %user_id, @@ -263,7 +289,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", ) @@ -272,6 +298,10 @@ pub async fn reset_user_pin( .execute(&state.pool) .await?; + // A PIN reset must invalidate existing sessions so a compromised/old token + // can't outlive the reset. + revoke_sessions(&state.pool, user_id, "reset_user_pin").await; + // Notify the *recipient* device(s) if they happen to be online so they can clear // their cached local PIN. They'll save the new one on the next /recover. let _ = state.sse_tx.send(SseEvent::new( diff --git a/backend/src/models/session.rs b/backend/src/models/session.rs index 280048f..b8832ce 100644 --- a/backend/src/models/session.rs +++ b/backend/src/models/session.rs @@ -2,6 +2,8 @@ use chrono::{DateTime, Utc}; use sqlx::PgPool; use uuid::Uuid; +use crate::models::user::UserRole; + #[derive(Debug, sqlx::FromRow)] pub struct Session { pub id: Uuid, @@ -12,6 +14,18 @@ pub struct Session { pub created_at: DateTime, } +/// Live identity reconciled from the DB for a valid, unexpired session. Used by +/// the `AuthUser` extractor so role/ban/event are sourced from the `user` row +/// rather than trusted from (potentially stale) JWT claims. +#[derive(Debug, sqlx::FromRow)] +pub struct AuthContext { + pub session_id: Uuid, + pub user_id: Uuid, + pub event_id: Uuid, + pub role: UserRole, + pub is_banned: bool, +} + impl Session { pub async fn create( pool: &PgPool, @@ -43,6 +57,25 @@ impl Session { .await } + /// Reconcile a session against the live `user` row in a single round-trip. + /// Returns `None` when the session is missing/expired. The PK join to + /// `"user"` is cheap and lets the extractor read the *current* role/ban + /// state instead of the JWT claim. + pub async fn find_auth_context( + pool: &PgPool, + token_hash: &str, + ) -> Result, sqlx::Error> { + sqlx::query_as::<_, AuthContext>( + "SELECT s.id AS session_id, u.id AS user_id, u.event_id, u.role, u.is_banned + FROM session s + JOIN \"user\" u ON u.id = s.user_id + WHERE s.token_hash = $1 AND s.expires_at > NOW()", + ) + .bind(token_hash) + .fetch_optional(pool) + .await + } + pub async fn touch(pool: &PgPool, id: Uuid) -> Result<(), sqlx::Error> { sqlx::query("UPDATE session SET last_seen_at = NOW() WHERE id = $1") .bind(id) @@ -61,4 +94,15 @@ impl Session { .await?; Ok(()) } + + /// Revoke every session belonging to a user — used on ban, role change, and + /// PIN reset so existing JWTs stop working immediately. Returns the number + /// of sessions removed. Best-effort: callers log but do not fail on error. + pub async fn delete_by_user_id(pool: &PgPool, user_id: Uuid) -> Result { + let result = sqlx::query("DELETE FROM session WHERE user_id = $1") + .bind(user_id) + .execute(pool) + .await?; + Ok(result.rows_affected()) + } } diff --git a/frontend/src/lib/components/LightboxModal.svelte b/frontend/src/lib/components/LightboxModal.svelte index 314c700..b20f67f 100644 --- a/frontend/src/lib/components/LightboxModal.svelte +++ b/frontend/src/lib/components/LightboxModal.svelte @@ -59,7 +59,7 @@ if (!newComment.trim()) return; loading = true; try { - const comment = await api.post(`/upload/${upload.id}/comment`, { + const comment = await api.post(`/upload/${upload.id}/comments`, { body: newComment.trim() }); comments = [...comments, comment];