fix(security): cross-event authz, SSE ticket flow, account hardening, audit logs
Follow-up to the comprehensive code review. Five batches: 1. Cross-event authorization: host_delete_upload, unban_user, and host_delete_comment now scope by auth.event_id. Adds Upload::find_by_id_and_event / soft_delete_in_event and a Comment::soft_delete_in_event variant that joins through upload. 2. Token exposure: SSE auth no longer puts the JWT in the URL. New /api/v1/stream/ticket endpoint mints a short-lived single-use ticket bound to the session; the EventSource passes ?ticket=... instead. Refuse to start in APP_ENV=production with the dev JWT sentinel; warn loudly otherwise. 3. Account hardening: per-IP+name rate limit on /recover (mitigates targeted lockout DoS), per-IP rate limit on /admin/login, random 32-char admin recovery PIN (replaces "0000"), structured tracing events for wrong PIN, lockout, failed admin login, ban/unban/role change/pin-reset/host-delete. 4. DoS / correctness: comment listing paginated (LIMIT 50 + ?before= cursor), hashtag extraction whitelisted to ASCII alnum+underscore (≤40 chars) with unit tests, display_name / caption / comment body length validated in chars rather than bytes. 5. Cleanup: session-touch failures now logged, DATABASE_MAX_CONNECTIONS env var (default 10). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -49,7 +49,8 @@ pub async fn join(
|
||||
}
|
||||
|
||||
let display_name = body.display_name.trim();
|
||||
if display_name.is_empty() || display_name.len() > 50 {
|
||||
let name_chars = display_name.chars().count();
|
||||
if name_chars == 0 || name_chars > 50 {
|
||||
return Err(AppError::BadRequest(
|
||||
"Name muss zwischen 1 und 50 Zeichen lang sein.".into(),
|
||||
));
|
||||
@@ -122,10 +123,33 @@ pub struct RecoverResponse {
|
||||
|
||||
pub async fn recover(
|
||||
State(state): State<AppState>,
|
||||
headers: HeaderMap,
|
||||
Json(body): Json<RecoverRequest>,
|
||||
) -> Result<Json<RecoverResponse>, AppError> {
|
||||
let display_name = body.display_name.trim();
|
||||
|
||||
// Per-IP+name throttle BEFORE the per-user 3-strike counter. Without this
|
||||
// an attacker who knows a display name (they're visible on the feed) can
|
||||
// burn through 3 wrong PINs and lock the victim for 15 minutes — repeated
|
||||
// every 15 minutes, indefinitely. 5 attempts per 15 minutes per (IP, name)
|
||||
// softens that into a real cost.
|
||||
let ip = client_ip(&headers, "unknown");
|
||||
let rate_limits_on = config::get_bool(&state.pool, "rate_limits_enabled", true).await;
|
||||
let recover_rate_on = config::get_bool(&state.pool, "recover_rate_enabled", true).await;
|
||||
if rate_limits_on && recover_rate_on {
|
||||
let name_key = display_name.to_lowercase();
|
||||
if !state.rate_limiter.check(
|
||||
format!("recover:{ip}:{name_key}"),
|
||||
5,
|
||||
Duration::from_secs(15 * 60),
|
||||
) {
|
||||
return Err(AppError::TooManyRequests(
|
||||
"Zu viele Versuche. Bitte warte kurz und versuche es erneut.".into(),
|
||||
None,
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
let event = Event::find_by_slug(&state.pool, &state.config.event_slug)
|
||||
.await?
|
||||
.ok_or_else(|| AppError::NotFound("Event nicht gefunden.".into()))?;
|
||||
@@ -184,9 +208,22 @@ pub async fn recover(
|
||||
|
||||
// Wrong PIN — increment failure count
|
||||
let attempts = User::increment_failed_pin(&state.pool, user.id).await?;
|
||||
tracing::warn!(
|
||||
user_id = %user.id,
|
||||
event_id = %event.id,
|
||||
ip = %ip,
|
||||
attempts,
|
||||
"recover: wrong PIN"
|
||||
);
|
||||
if attempts >= 3 {
|
||||
let lockout = Utc::now() + chrono::Duration::minutes(15);
|
||||
User::lock_pin(&state.pool, user.id, lockout).await?;
|
||||
tracing::warn!(
|
||||
user_id = %user.id,
|
||||
event_id = %event.id,
|
||||
ip = %ip,
|
||||
"recover: account locked for 15 minutes"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -205,6 +242,7 @@ pub struct AdminLoginResponse {
|
||||
|
||||
pub async fn admin_login(
|
||||
State(state): State<AppState>,
|
||||
headers: HeaderMap,
|
||||
Json(body): Json<AdminLoginRequest>,
|
||||
) -> Result<Json<AdminLoginResponse>, AppError> {
|
||||
if state.config.admin_password_hash.is_empty() {
|
||||
@@ -213,10 +251,31 @@ pub async fn admin_login(
|
||||
));
|
||||
}
|
||||
|
||||
// Throttle password attempts. The admin password is bcrypt-hashed (slow to
|
||||
// verify) but with no IP-level limit a determined attacker can still mount
|
||||
// a long-running guess campaign. 5 attempts / minute / IP is plenty for
|
||||
// honest typos.
|
||||
let ip = client_ip(&headers, "unknown");
|
||||
let rate_limits_on = config::get_bool(&state.pool, "rate_limits_enabled", true).await;
|
||||
let admin_rate_on = config::get_bool(&state.pool, "admin_login_rate_enabled", true).await;
|
||||
if rate_limits_on && admin_rate_on
|
||||
&& !state.rate_limiter.check(
|
||||
format!("admin_login:{ip}"),
|
||||
5,
|
||||
Duration::from_secs(60),
|
||||
)
|
||||
{
|
||||
return Err(AppError::TooManyRequests(
|
||||
"Zu viele Anmeldeversuche. Bitte warte kurz und versuche es erneut.".into(),
|
||||
None,
|
||||
));
|
||||
}
|
||||
|
||||
let valid = bcrypt::verify(&body.password, &state.config.admin_password_hash)
|
||||
.unwrap_or(false);
|
||||
|
||||
if !valid {
|
||||
tracing::warn!(ip = %ip, "admin_login: wrong password");
|
||||
return Err(AppError::Unauthorized("Falsches Passwort.".into()));
|
||||
}
|
||||
|
||||
@@ -233,8 +292,13 @@ pub async fn admin_login(
|
||||
let admin_user = if let Some(u) = users.into_iter().find(|u| u.role == UserRole::Admin) {
|
||||
u
|
||||
} else {
|
||||
// Create admin user with a dummy PIN (admin authenticates via password)
|
||||
let dummy_hash = bcrypt::hash("0000", 4)
|
||||
// Admin authenticates via password, but the schema still requires a PIN
|
||||
// hash. Generate a random unguessable PIN so the recovery path remains
|
||||
// unusable as an escalation route even if the role flag ever got cleared.
|
||||
let dummy_pin: String = (0..32)
|
||||
.map(|_| rand::rng().random_range(b'a'..=b'z') as char)
|
||||
.collect();
|
||||
let dummy_hash = bcrypt::hash(&dummy_pin, 4)
|
||||
.map_err(|e| AppError::Internal(anyhow::anyhow!(e)))?;
|
||||
let user = User::create(&state.pool, event.id, admin_name, &dummy_hash).await?;
|
||||
sqlx::query("UPDATE \"user\" SET role = 'admin' WHERE id = $1")
|
||||
@@ -246,6 +310,8 @@ pub async fn admin_login(
|
||||
.ok_or_else(|| AppError::Internal(anyhow::anyhow!("admin user creation failed")))?
|
||||
};
|
||||
|
||||
tracing::info!(user_id = %admin_user.id, event_id = %event.id, ip = %ip, "admin_login: success");
|
||||
|
||||
let token = jwt::create_token(
|
||||
admin_user.id,
|
||||
event.id,
|
||||
|
||||
@@ -43,11 +43,15 @@ impl FromRequestParts<AppState> for AuthUser {
|
||||
.map_err(|e| AppError::Internal(e.into()))?
|
||||
.ok_or_else(|| AppError::Unauthorized("Sitzung nicht gefunden oder abgelaufen.".into()))?;
|
||||
|
||||
// Update last_seen_at in the background (fire-and-forget)
|
||||
// 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;
|
||||
tokio::spawn(async move {
|
||||
let _ = 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");
|
||||
}
|
||||
});
|
||||
|
||||
Ok(Self {
|
||||
|
||||
Reference in New Issue
Block a user