fix(auth): reconcile role/ban from DB, revoke sessions, fix pin/comment bugs
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) <noreply@anthropic.com>
This commit is contained in:
@@ -38,16 +38,32 @@ impl FromRequestParts<AppState> for AuthUser {
|
|||||||
|
|
||||||
let token_hash = jwt::hash_token(token);
|
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
|
.await
|
||||||
.map_err(|e| AppError::Internal(e.into()))?
|
.map_err(|e| AppError::Internal(e.into()))?
|
||||||
.ok_or_else(|| AppError::Unauthorized("Sitzung nicht gefunden oder abgelaufen.".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
|
// Update last_seen_at in the background (fire-and-forget). Failures are
|
||||||
// non-fatal but worth surfacing — silent swallowing hides DB connection
|
// non-fatal but worth surfacing — silent swallowing hides DB connection
|
||||||
// pressure that would otherwise be the first symptom of a real problem.
|
// pressure that would otherwise be the first symptom of a real problem.
|
||||||
let pool = state.pool.clone();
|
let pool = state.pool.clone();
|
||||||
let session_id = session.id;
|
let session_id = ctx.session_id;
|
||||||
tokio::spawn(async move {
|
tokio::spawn(async move {
|
||||||
if let Err(e) = Session::touch(&pool, session_id).await {
|
if let Err(e) = Session::touch(&pool, session_id).await {
|
||||||
tracing::warn!(error = ?e, session_id = %session_id, "session touch failed");
|
tracing::warn!(error = ?e, session_id = %session_id, "session touch failed");
|
||||||
@@ -55,9 +71,11 @@ impl FromRequestParts<AppState> for AuthUser {
|
|||||||
});
|
});
|
||||||
|
|
||||||
Ok(Self {
|
Ok(Self {
|
||||||
user_id: claims.sub,
|
user_id: ctx.user_id,
|
||||||
event_id: claims.event_id,
|
event_id: ctx.event_id,
|
||||||
role: claims.role,
|
// Live role from the DB, not the claim — demotion/promotion takes
|
||||||
|
// effect on the next request.
|
||||||
|
role: ctx.role,
|
||||||
token_hash,
|
token_hash,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -9,10 +9,21 @@ use crate::auth::middleware::RequireHost;
|
|||||||
use crate::error::AppError;
|
use crate::error::AppError;
|
||||||
use crate::models::comment::Comment;
|
use crate::models::comment::Comment;
|
||||||
use crate::models::event::Event;
|
use crate::models::event::Event;
|
||||||
|
use crate::models::session::Session;
|
||||||
use crate::models::upload::Upload;
|
use crate::models::upload::Upload;
|
||||||
use crate::models::user::UserRole;
|
use crate::models::user::UserRole;
|
||||||
use crate::state::{AppState, SseEvent};
|
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 ─────────────────────────────────────────────────────────────────────
|
// ── DTOs ─────────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
#[derive(Serialize, sqlx::FromRow)]
|
#[derive(Serialize, sqlx::FromRow)]
|
||||||
@@ -120,6 +131,14 @@ pub async fn ban_user(
|
|||||||
.execute(&state.pool)
|
.execute(&state.pool)
|
||||||
.await?;
|
.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!(
|
tracing::info!(
|
||||||
actor_user_id = %auth.user_id,
|
actor_user_id = %auth.user_id,
|
||||||
target_user_id = %user_id,
|
target_user_id = %user_id,
|
||||||
@@ -201,6 +220,13 @@ pub async fn set_role(
|
|||||||
.bind(auth.event_id)
|
.bind(auth.event_id)
|
||||||
.execute(&state.pool)
|
.execute(&state.pool)
|
||||||
.await?;
|
.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!(
|
tracing::info!(
|
||||||
actor_user_id = %auth.user_id,
|
actor_user_id = %auth.user_id,
|
||||||
target_user_id = %user_id,
|
target_user_id = %user_id,
|
||||||
@@ -263,7 +289,7 @@ pub async fn reset_user_pin(
|
|||||||
sqlx::query(
|
sqlx::query(
|
||||||
"UPDATE \"user\"
|
"UPDATE \"user\"
|
||||||
SET recovery_pin_hash = $1,
|
SET recovery_pin_hash = $1,
|
||||||
pin_failed_attempts = 0,
|
failed_pin_attempts = 0,
|
||||||
pin_locked_until = NULL
|
pin_locked_until = NULL
|
||||||
WHERE id = $2",
|
WHERE id = $2",
|
||||||
)
|
)
|
||||||
@@ -272,6 +298,10 @@ pub async fn reset_user_pin(
|
|||||||
.execute(&state.pool)
|
.execute(&state.pool)
|
||||||
.await?;
|
.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
|
// 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.
|
// their cached local PIN. They'll save the new one on the next /recover.
|
||||||
let _ = state.sse_tx.send(SseEvent::new(
|
let _ = state.sse_tx.send(SseEvent::new(
|
||||||
|
|||||||
@@ -2,6 +2,8 @@ use chrono::{DateTime, Utc};
|
|||||||
use sqlx::PgPool;
|
use sqlx::PgPool;
|
||||||
use uuid::Uuid;
|
use uuid::Uuid;
|
||||||
|
|
||||||
|
use crate::models::user::UserRole;
|
||||||
|
|
||||||
#[derive(Debug, sqlx::FromRow)]
|
#[derive(Debug, sqlx::FromRow)]
|
||||||
pub struct Session {
|
pub struct Session {
|
||||||
pub id: Uuid,
|
pub id: Uuid,
|
||||||
@@ -12,6 +14,18 @@ pub struct Session {
|
|||||||
pub created_at: DateTime<Utc>,
|
pub created_at: DateTime<Utc>,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// 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 {
|
impl Session {
|
||||||
pub async fn create(
|
pub async fn create(
|
||||||
pool: &PgPool,
|
pool: &PgPool,
|
||||||
@@ -43,6 +57,25 @@ impl Session {
|
|||||||
.await
|
.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<Option<AuthContext>, 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> {
|
pub async fn touch(pool: &PgPool, id: Uuid) -> Result<(), sqlx::Error> {
|
||||||
sqlx::query("UPDATE session SET last_seen_at = NOW() WHERE id = $1")
|
sqlx::query("UPDATE session SET last_seen_at = NOW() WHERE id = $1")
|
||||||
.bind(id)
|
.bind(id)
|
||||||
@@ -61,4 +94,15 @@ impl Session {
|
|||||||
.await?;
|
.await?;
|
||||||
Ok(())
|
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<u64, sqlx::Error> {
|
||||||
|
let result = sqlx::query("DELETE FROM session WHERE user_id = $1")
|
||||||
|
.bind(user_id)
|
||||||
|
.execute(pool)
|
||||||
|
.await?;
|
||||||
|
Ok(result.rows_affected())
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -59,7 +59,7 @@
|
|||||||
if (!newComment.trim()) return;
|
if (!newComment.trim()) return;
|
||||||
loading = true;
|
loading = true;
|
||||||
try {
|
try {
|
||||||
const comment = await api.post<CommentDto>(`/upload/${upload.id}/comment`, {
|
const comment = await api.post<CommentDto>(`/upload/${upload.id}/comments`, {
|
||||||
body: newComment.trim()
|
body: newComment.trim()
|
||||||
});
|
});
|
||||||
comments = [...comments, comment];
|
comments = [...comments, comment];
|
||||||
|
|||||||
Reference in New Issue
Block a user