Merge branch 'fix/rate-limit-shared-nat'

This commit is contained in:
fabi
2026-07-27 21:56:49 +02:00
11 changed files with 341 additions and 86 deletions

View File

@@ -0,0 +1 @@
DELETE FROM config WHERE key IN ('join_ip_rate_per_min', 'admin_login_rate_enabled');

View File

@@ -0,0 +1,18 @@
-- Per-IP flood ceiling for /join, and the `admin_login_rate_enabled` toggle that
-- every prior migration forgot to seed.
--
-- Rationale: /join was throttled at 5 requests per 60s keyed on the client IP. At a
-- venue every guest is behind one NAT, so the whole party shared a single bucket —
-- 12 guests scanning the QR code within a few seconds meant 5 got in and 7 were
-- turned away. The handler now keys the real anti-spam bucket per (ip, name), the
-- same shape as `recover:{ip}:{name}`, and keeps only a loose per-IP ceiling to bound
-- raw volume. 60/min comfortably covers a whole wedding arriving at once while still
-- capping a flood from a single source.
--
-- `admin_login_rate_enabled` is read by auth::handlers::admin_login with a code
-- default of `true`, but no migration ever inserted it, so it was invisible to the
-- admin config UI and to the e2e reseed. Seed it explicitly.
INSERT INTO config (key, value) VALUES
('join_ip_rate_per_min', '60'),
('admin_login_rate_enabled', 'true')
ON CONFLICT (key) DO NOTHING;

View File

@@ -1,11 +1,12 @@
use std::time::Duration; use std::time::Duration;
use axum::Json; use axum::Json;
use axum::extract::State; use axum::extract::{ConnectInfo, State};
use axum::http::{HeaderMap, StatusCode}; use axum::http::{HeaderMap, StatusCode};
use chrono::Utc; use chrono::Utc;
use rand::Rng; use rand::Rng;
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use std::net::SocketAddr;
use uuid::Uuid; use uuid::Uuid;
use crate::auth::jwt; use crate::auth::jwt;
@@ -33,22 +34,32 @@ pub struct JoinResponse {
pub async fn join( pub async fn join(
State(state): State<AppState>, State(state): State<AppState>,
ConnectInfo(peer): ConnectInfo<SocketAddr>,
headers: HeaderMap, headers: HeaderMap,
Json(body): Json<JoinRequest>, Json(body): Json<JoinRequest>,
) -> Result<(StatusCode, Json<JoinResponse>), AppError> { ) -> Result<(StatusCode, Json<JoinResponse>), AppError> {
let ip = client_ip(&headers, "unknown"); let ip = client_ip(&headers, &peer.ip().to_string());
let rate_limits_on = config::get_bool(&state.config_cache, "rate_limits_enabled", true).await; let rate_limits_on = config::get_bool(&state.config_cache, "rate_limits_enabled", true).await;
let join_rate_on = config::get_bool(&state.config_cache, "join_rate_enabled", true).await; let join_rate_on = config::get_bool(&state.config_cache, "join_rate_enabled", true).await;
if rate_limits_on
&& join_rate_on // Coarse per-IP flood ceiling. `/join` is pre-auth so there is no user to key on, and
&& !state // at a venue EVERY guest arrives from one public IP — a tight per-IP bucket meant the
.rate_limiter // 6th person through the door was turned away by the 5 ahead of them. So the per-IP
.check(format!("join:{ip}"), 5, Duration::from_secs(60)) // limit here only bounds raw volume; the real anti-spam bucket is per-name below.
{ // Cheap enough to run before validation, which keeps a flood of malformed bodies from
return Err(AppError::TooManyRequests( // being free.
"Zu viele Anfragen. Bitte warte kurz und versuche es erneut.".into(), if rate_limits_on && join_rate_on {
None, let ip_ceiling = config::get_usize(&state.config_cache, "join_ip_rate_per_min", 60).await;
)); if let Err(retry_after_secs) = state.rate_limiter.check_with_retry(
format!("join_ip:{ip}"),
ip_ceiling,
Duration::from_secs(60),
) {
return Err(AppError::TooManyRequests(
"Zu viele Anfragen. Bitte warte kurz und versuche es erneut.".into(),
Some(retry_after_secs),
));
}
} }
let display_name = body.display_name.trim(); let display_name = body.display_name.trim();
@@ -66,6 +77,23 @@ pub async fn join(
)); ));
} }
// Per-guest bucket, keyed like the `recover:{ip}:{name}` limiter below. This carries
// the original 5/60s anti-spam intent, but one guest retrying can no longer consume
// the allowance of everyone else sharing the venue's NAT.
if rate_limits_on && join_rate_on {
let name_key = display_name.to_lowercase();
if let Err(retry_after_secs) = state.rate_limiter.check_with_retry(
format!("join:{ip}:{name_key}"),
5,
Duration::from_secs(60),
) {
return Err(AppError::TooManyRequests(
"Zu viele Anfragen. Bitte warte kurz und versuche es erneut.".into(),
Some(retry_after_secs),
));
}
}
let event = Event::find_or_create( let event = Event::find_or_create(
&state.pool, &state.pool,
&state.config.event_slug, &state.config.event_slug,
@@ -149,6 +177,7 @@ fn dummy_pin_hash() -> &'static str {
pub async fn recover( pub async fn recover(
State(state): State<AppState>, State(state): State<AppState>,
ConnectInfo(peer): ConnectInfo<SocketAddr>,
headers: HeaderMap, headers: HeaderMap,
Json(body): Json<RecoverRequest>, Json(body): Json<RecoverRequest>,
) -> Result<Json<RecoverResponse>, AppError> { ) -> Result<Json<RecoverResponse>, AppError> {
@@ -159,19 +188,19 @@ pub async fn recover(
// burn through 3 wrong PINs and lock the victim for 15 minutes — repeated // 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) // every 15 minutes, indefinitely. 5 attempts per 15 minutes per (IP, name)
// softens that into a real cost. // softens that into a real cost.
let ip = client_ip(&headers, "unknown"); let ip = client_ip(&headers, &peer.ip().to_string());
let rate_limits_on = config::get_bool(&state.config_cache, "rate_limits_enabled", true).await; let rate_limits_on = config::get_bool(&state.config_cache, "rate_limits_enabled", true).await;
let recover_rate_on = config::get_bool(&state.config_cache, "recover_rate_enabled", true).await; let recover_rate_on = config::get_bool(&state.config_cache, "recover_rate_enabled", true).await;
if rate_limits_on && recover_rate_on { if rate_limits_on && recover_rate_on {
let name_key = display_name.to_lowercase(); let name_key = display_name.to_lowercase();
if !state.rate_limiter.check( if let Err(retry_after_secs) = state.rate_limiter.check_with_retry(
format!("recover:{ip}:{name_key}"), format!("recover:{ip}:{name_key}"),
5, 5,
Duration::from_secs(15 * 60), Duration::from_secs(15 * 60),
) { ) {
return Err(AppError::TooManyRequests( return Err(AppError::TooManyRequests(
"Zu viele Versuche. Bitte warte kurz und versuche es erneut.".into(), "Zu viele Versuche. Bitte warte kurz und versuche es erneut.".into(),
None, Some(retry_after_secs),
)); ));
} }
} }
@@ -200,9 +229,12 @@ pub async fn recover(
// is effectively permanently fragile. // is effectively permanently fragile.
if let Some(locked_until) = user.pin_locked_until { if let Some(locked_until) = user.pin_locked_until {
if Utc::now() < locked_until { if Utc::now() < locked_until {
// The exact deadline is known, so surface it as Retry-After instead of
// making the client guess at the "15 Minuten" in the copy.
let retry_after_secs = (locked_until - Utc::now()).num_seconds().max(1) as u64;
return Err(AppError::TooManyRequests( return Err(AppError::TooManyRequests(
"Zu viele Versuche. Bitte warte 15 Minuten.".into(), "Zu viele Versuche. Bitte warte 15 Minuten.".into(),
None, Some(retry_after_secs),
)); ));
} }
// Lockout window expired — wipe the counter and the timestamp. // Lockout window expired — wipe the counter and the timestamp.
@@ -274,6 +306,7 @@ pub struct AdminLoginResponse {
pub async fn admin_login( pub async fn admin_login(
State(state): State<AppState>, State(state): State<AppState>,
ConnectInfo(peer): ConnectInfo<SocketAddr>,
headers: HeaderMap, headers: HeaderMap,
Json(body): Json<AdminLoginRequest>, Json(body): Json<AdminLoginRequest>,
) -> Result<Json<AdminLoginResponse>, AppError> { ) -> Result<Json<AdminLoginResponse>, AppError> {
@@ -287,19 +320,23 @@ pub async fn admin_login(
// verify) but with no IP-level limit a determined attacker can still mount // 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 // a long-running guess campaign. 5 attempts / minute / IP is plenty for
// honest typos. // honest typos.
let ip = client_ip(&headers, "unknown"); let ip = client_ip(&headers, &peer.ip().to_string());
let rate_limits_on = config::get_bool(&state.config_cache, "rate_limits_enabled", true).await; let rate_limits_on = config::get_bool(&state.config_cache, "rate_limits_enabled", true).await;
let admin_rate_on = let admin_rate_on =
config::get_bool(&state.config_cache, "admin_login_rate_enabled", true).await; config::get_bool(&state.config_cache, "admin_login_rate_enabled", true).await;
// Stays keyed by IP on purpose: this guards a single shared credential, so a per-user
// or per-name key would just hand an attacker a fresh bucket per guess.
if rate_limits_on if rate_limits_on
&& admin_rate_on && admin_rate_on
&& !state && let Err(retry_after_secs) = state.rate_limiter.check_with_retry(
.rate_limiter format!("admin_login:{ip}"),
.check(format!("admin_login:{ip}"), 5, Duration::from_secs(60)) 5,
Duration::from_secs(60),
)
{ {
return Err(AppError::TooManyRequests( return Err(AppError::TooManyRequests(
"Zu viele Anmeldeversuche. Bitte warte kurz und versuche es erneut.".into(), "Zu viele Anmeldeversuche. Bitte warte kurz und versuche es erneut.".into(),
None, Some(retry_after_secs),
)); ));
} }
@@ -390,22 +427,23 @@ pub struct PinResetRequestBody {
/// feed already exposes. /// feed already exposes.
pub async fn request_pin_reset( pub async fn request_pin_reset(
State(state): State<AppState>, State(state): State<AppState>,
ConnectInfo(peer): ConnectInfo<SocketAddr>,
headers: HeaderMap, headers: HeaderMap,
Json(body): Json<PinResetRequestBody>, Json(body): Json<PinResetRequestBody>,
) -> Result<StatusCode, AppError> { ) -> Result<StatusCode, AppError> {
let display_name = body.display_name.trim(); let display_name = body.display_name.trim();
let ip = client_ip(&headers, "unknown"); let ip = client_ip(&headers, &peer.ip().to_string());
let rate_limits_on = config::get_bool(&state.config_cache, "rate_limits_enabled", true).await; let rate_limits_on = config::get_bool(&state.config_cache, "rate_limits_enabled", true).await;
if rate_limits_on { if rate_limits_on {
let name_key = display_name.to_lowercase(); let name_key = display_name.to_lowercase();
if !state.rate_limiter.check( if let Err(retry_after_secs) = state.rate_limiter.check_with_retry(
format!("pin_reset_req:{ip}:{name_key}"), format!("pin_reset_req:{ip}:{name_key}"),
3, 3,
Duration::from_secs(15 * 60), Duration::from_secs(15 * 60),
) { ) {
return Err(AppError::TooManyRequests( return Err(AppError::TooManyRequests(
"Zu viele Anfragen. Bitte warte kurz und versuche es erneut.".into(), "Zu viele Anfragen. Bitte warte kurz und versuche es erneut.".into(),
None, Some(retry_after_secs),
)); ));
} }
} }

View File

@@ -3,13 +3,13 @@ use std::time::Duration;
use axum::Json; use axum::Json;
use axum::extract::{Query, State}; use axum::extract::{Query, State};
use axum::http::{HeaderMap, StatusCode}; use axum::http::StatusCode;
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use uuid::Uuid;
use crate::auth::middleware::RequireAdmin; use crate::auth::middleware::RequireAdmin;
use crate::error::AppError; use crate::error::AppError;
use crate::services::config; use crate::services::config;
use crate::services::rate_limiter::client_ip;
use crate::state::AppState; use crate::state::AppState;
// ── DTOs ───────────────────────────────────────────────────────────────────── // ── DTOs ─────────────────────────────────────────────────────────────────────
@@ -120,6 +120,10 @@ pub async fn patch_config(
("upload_rate_per_hour", true, 1.0, 100_000.0), ("upload_rate_per_hour", true, 1.0, 100_000.0),
("feed_rate_per_min", true, 1.0, 100_000.0), ("feed_rate_per_min", true, 1.0, 100_000.0),
("export_rate_per_day", true, 1.0, 100_000.0), ("export_rate_per_day", true, 1.0, 100_000.0),
// Loose per-IP ceiling on /join. The real anti-spam bucket is per (ip, name); this
// only bounds raw volume from one source, so it must stay well above the size of a
// party arriving at once (see migration 017).
("join_ip_rate_per_min", true, 1.0, 100_000.0),
("quota_tolerance", false, 0.0, 1.0), ("quota_tolerance", false, 0.0, 1.0),
("estimated_guest_count", true, 1.0, 1_000_000.0), ("estimated_guest_count", true, 1.0, 1_000_000.0),
]; ];
@@ -320,25 +324,26 @@ pub async fn export_ticket(
} }
/// Validate a download ticket (single-use) and confirm its session still exists. /// Validate a download ticket (single-use) and confirm its session still exists.
async fn authenticate_download_ticket(state: &AppState, ticket: &str) -> Result<(), AppError> { /// Resolve a single-use download ticket to the user who minted it. The caller needs the
/// id to key the export rate limit per-user (see `enforce_export_rate`).
async fn authenticate_download_ticket(state: &AppState, ticket: &str) -> Result<Uuid, AppError> {
let token_hash = state let token_hash = state
.sse_tickets .sse_tickets
.consume(ticket) .consume(ticket)
.ok_or_else(|| AppError::Unauthorized("Ticket ungültig oder abgelaufen.".into()))?; .ok_or_else(|| AppError::Unauthorized("Ticket ungültig oder abgelaufen.".into()))?;
crate::models::session::Session::find_by_token_hash(&state.pool, &token_hash) let session = crate::models::session::Session::find_by_token_hash(&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.".into()))?; .ok_or_else(|| AppError::Unauthorized("Sitzung nicht gefunden.".into()))?;
Ok(()) Ok(session.user_id)
} }
pub async fn download_zip( pub async fn download_zip(
State(state): State<AppState>, State(state): State<AppState>,
Query(q): Query<DownloadQuery>, Query(q): Query<DownloadQuery>,
headers: HeaderMap,
) -> Result<axum::response::Response, AppError> { ) -> Result<axum::response::Response, AppError> {
authenticate_download_ticket(&state, &q.ticket).await?; let user_id = authenticate_download_ticket(&state, &q.ticket).await?;
enforce_export_rate(&state, &headers).await?; enforce_export_rate(&state, user_id).await?;
let path = let path =
resolve_export_file(&state, "zip", "Der ZIP-Export ist noch nicht verfügbar.").await?; resolve_export_file(&state, "zip", "Der ZIP-Export ist noch nicht verfügbar.").await?;
@@ -389,10 +394,9 @@ async fn resolve_export_file(
pub async fn download_html( pub async fn download_html(
State(state): State<AppState>, State(state): State<AppState>,
Query(q): Query<DownloadQuery>, Query(q): Query<DownloadQuery>,
headers: HeaderMap,
) -> Result<axum::response::Response, AppError> { ) -> Result<axum::response::Response, AppError> {
authenticate_download_ticket(&state, &q.ticket).await?; let user_id = authenticate_download_ticket(&state, &q.ticket).await?;
enforce_export_rate(&state, &headers).await?; enforce_export_rate(&state, user_id).await?;
let path = let path =
resolve_export_file(&state, "html", "Der HTML-Export ist noch nicht verfügbar.").await?; resolve_export_file(&state, "html", "Der HTML-Export ist noch nicht verfügbar.").await?;
@@ -476,21 +480,24 @@ pub async fn export_status(
/// Centralised guard for the export rate limit. Same pattern as upload/feed: master /// Centralised guard for the export rate limit. Same pattern as upload/feed: master
/// switch + per-endpoint switch + numeric value, all stored in `config` and read on /// switch + per-endpoint switch + numeric value, all stored in `config` and read on
/// each request. /// each request.
async fn enforce_export_rate(state: &AppState, headers: &HeaderMap) -> Result<(), AppError> { async fn enforce_export_rate(state: &AppState, user_id: Uuid) -> Result<(), AppError> {
let rate_limits_on = config::get_bool(&state.config_cache, "rate_limits_enabled", true).await; let rate_limits_on = config::get_bool(&state.config_cache, "rate_limits_enabled", true).await;
let export_rate_on = config::get_bool(&state.config_cache, "export_rate_enabled", true).await; let export_rate_on = config::get_bool(&state.config_cache, "export_rate_enabled", true).await;
if !(rate_limits_on && export_rate_on) { if !(rate_limits_on && export_rate_on) {
return Ok(()); return Ok(());
} }
let ip = client_ip(headers, "unknown");
let limit = config::get_usize(&state.config_cache, "export_rate_per_day", 3).await; let limit = config::get_usize(&state.config_cache, "export_rate_per_day", 3).await;
if !state // Keyed per-user. This was the worst of the IP-keyed limiters: 3 downloads per DAY
.rate_limiter // shared across every guest behind the venue's public IP, so the fourth person to
.check(format!("export:{ip}"), limit, Duration::from_secs(86400)) // fetch their keepsake was locked out until the next day.
{ if let Err(retry_after_secs) = state.rate_limiter.check_with_retry(
format!("export:{user_id}"),
limit,
Duration::from_secs(86400),
) {
return Err(AppError::TooManyRequests( return Err(AppError::TooManyRequests(
"Zu viele Anfragen. Bitte warte kurz und versuche es erneut.".into(), "Zu viele Anfragen. Bitte warte kurz und versuche es erneut.".into(),
None, Some(retry_after_secs),
)); ));
} }
Ok(()) Ok(())

View File

@@ -2,7 +2,6 @@ use std::time::Duration;
use axum::Json; use axum::Json;
use axum::extract::{Query, State}; use axum::extract::{Query, State};
use axum::http::HeaderMap;
use chrono::{DateTime, Utc}; use chrono::{DateTime, Utc};
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use uuid::Uuid; use uuid::Uuid;
@@ -10,7 +9,6 @@ use uuid::Uuid;
use crate::auth::middleware::AuthUser; use crate::auth::middleware::AuthUser;
use crate::error::AppError; use crate::error::AppError;
use crate::services::config; use crate::services::config;
use crate::services::rate_limiter::client_ip;
use crate::state::AppState; use crate::state::AppState;
#[derive(Deserialize)] #[derive(Deserialize)]
@@ -61,21 +59,23 @@ struct FeedRow {
pub async fn feed( pub async fn feed(
State(state): State<AppState>, State(state): State<AppState>,
auth: AuthUser, auth: AuthUser,
headers: HeaderMap,
Query(q): Query<FeedQuery>, Query(q): Query<FeedQuery>,
) -> Result<Json<FeedResponse>, AppError> { ) -> Result<Json<FeedResponse>, AppError> {
let ip = client_ip(&headers, "unknown");
let rate_limits_on = config::get_bool(&state.config_cache, "rate_limits_enabled", true).await; let rate_limits_on = config::get_bool(&state.config_cache, "rate_limits_enabled", true).await;
let feed_rate_on = config::get_bool(&state.config_cache, "feed_rate_enabled", true).await; let feed_rate_on = config::get_bool(&state.config_cache, "feed_rate_enabled", true).await;
if rate_limits_on && feed_rate_on { if rate_limits_on && feed_rate_on {
let rate_limit = config::get_usize(&state.config_cache, "feed_rate_per_min", 60).await; let rate_limit = config::get_usize(&state.config_cache, "feed_rate_per_min", 60).await;
if !state // Keyed per-user, exactly like `feed_delta` below: at a venue every guest shares
.rate_limiter // one public IP, so an IP key gave the whole party a single 60/min bucket and the
.check(format!("feed:{ip}"), rate_limit, Duration::from_secs(60)) // fastest scroller starved everyone else.
{ if let Err(retry_after_secs) = state.rate_limiter.check_with_retry(
format!("feed:{}", auth.user_id),
rate_limit,
Duration::from_secs(60),
) {
return Err(AppError::TooManyRequests( return Err(AppError::TooManyRequests(
"Zu viele Anfragen. Bitte warte kurz und versuche es erneut.".into(), "Zu viele Anfragen. Bitte warte kurz und versuche es erneut.".into(),
None, Some(retry_after_secs),
)); ));
} }
} }
@@ -225,14 +225,14 @@ pub async fn feed_delta(
let feed_rate_on = config::get_bool(&state.config_cache, "feed_rate_enabled", true).await; let feed_rate_on = config::get_bool(&state.config_cache, "feed_rate_enabled", true).await;
if rate_limits_on && feed_rate_on { if rate_limits_on && feed_rate_on {
let rate_limit = config::get_usize(&state.config_cache, "feed_rate_per_min", 60).await; let rate_limit = config::get_usize(&state.config_cache, "feed_rate_per_min", 60).await;
if !state.rate_limiter.check( if let Err(retry_after_secs) = state.rate_limiter.check_with_retry(
format!("feed_delta:{}", auth.user_id), format!("feed_delta:{}", auth.user_id),
rate_limit, rate_limit,
Duration::from_secs(60), Duration::from_secs(60),
) { ) {
return Err(AppError::TooManyRequests( return Err(AppError::TooManyRequests(
"Zu viele Anfragen. Bitte warte kurz und versuche es erneut.".into(), "Zu viele Anfragen. Bitte warte kurz und versuche es erneut.".into(),
None, Some(retry_after_secs),
)); ));
} }
} }

View File

@@ -13,9 +13,10 @@ use crate::auth::middleware::RequireAdmin;
use crate::error::AppError; use crate::error::AppError;
use crate::state::AppState; use crate::state::AppState;
/// Truncates every event-scoped table, wipes media on disk, and reseeds the /// Truncates every event-scoped table, wipes media on disk, and reseeds the `config`
/// `config` table from migration defaults. Requires an admin JWT — even with /// table: numeric values from the migration defaults, but every feature toggle forced
/// `EVENTSNAP_TEST_MODE=1` it cannot be hit anonymously. /// OFF (production seeds them ON — see the note at the reseed below). Requires an admin
/// JWT — even with `EVENTSNAP_TEST_MODE=1` it cannot be hit anonymously.
pub async fn truncate_all( pub async fn truncate_all(
State(state): State<AppState>, State(state): State<AppState>,
RequireAdmin(_auth): RequireAdmin, RequireAdmin(_auth): RequireAdmin,
@@ -40,8 +41,19 @@ pub async fn truncate_all(
.execute(&state.pool) .execute(&state.pool)
.await?; .await?;
// Reseed config mirrors migrations 005, 009 and 015. Kept in sync by hand // Reseed config. The NUMERIC values mirror migrations 005/015/016; the BOOLEAN
// because pulling SQL out of the migration files at runtime is fragile. // toggles deliberately do NOT — migration 009 seeds every one of them `true`
// (production), and this forces them `false` so the suite isn't fighting rate limits
// and quotas it isn't testing.
//
// Be aware of what that costs: this runs as an auto-fixture before EVERY test, so no
// test starts from production's config unless it explicitly turns a toggle back on
// (02-upload/rate-limit, 07-adversarial/ddos, 01-auth/rate-limit-nat, …). That blind
// spot is exactly why an entire class of per-IP limiter bugs went unnoticed: the
// limiters were simply off. When adding a limiter or quota, add a spec that enables it.
//
// Kept in sync by hand because pulling SQL out of the migration files at runtime is
// fragile — if you add a config key in a migration, add it here too.
sqlx::query( sqlx::query(
r#"INSERT INTO config (key, value) VALUES r#"INSERT INTO config (key, value) VALUES
('max_image_size_mb', '20'), ('max_image_size_mb', '20'),
@@ -49,6 +61,7 @@ pub async fn truncate_all(
('upload_rate_per_hour', '100'), ('upload_rate_per_hour', '100'),
('feed_rate_per_min', '60'), ('feed_rate_per_min', '60'),
('export_rate_per_day', '3'), ('export_rate_per_day', '3'),
('join_ip_rate_per_min', '60'),
('quota_tolerance', '0.75'), ('quota_tolerance', '0.75'),
('estimated_guest_count', '100'), ('estimated_guest_count', '100'),
('compression_concurrency', '2'), ('compression_concurrency', '2'),
@@ -57,6 +70,7 @@ pub async fn truncate_all(
('feed_rate_enabled', 'false'), ('feed_rate_enabled', 'false'),
('export_rate_enabled', 'false'), ('export_rate_enabled', 'false'),
('join_rate_enabled', 'false'), ('join_rate_enabled', 'false'),
('admin_login_rate_enabled', 'false'),
('quota_enabled', 'false'), ('quota_enabled', 'false'),
('storage_quota_enabled', 'false'), ('storage_quota_enabled', 'false'),
('upload_count_quota_enabled', 'false'), ('upload_count_quota_enabled', 'false'),

View File

@@ -252,9 +252,15 @@ async fn main() -> Result<()> {
let listener = tokio::net::TcpListener::bind(("0.0.0.0", config.app_port)).await?; let listener = tokio::net::TcpListener::bind(("0.0.0.0", config.app_port)).await?;
tracing::info!("listening on {}", listener.local_addr()?); tracing::info!("listening on {}", listener.local_addr()?);
axum::serve(listener, router) // `into_make_service_with_connect_info` is required by the pre-auth handlers, which
.with_graceful_shutdown(shutdown_signal()) // extract `ConnectInfo<SocketAddr>` to use the peer address as the rate-limit key when
.await?; // X-Forwarded-For is absent. Without it those extractors fail at runtime.
axum::serve(
listener,
router.into_make_service_with_connect_info::<std::net::SocketAddr>(),
)
.with_graceful_shutdown(shutdown_signal())
.await?;
Ok(()) Ok(())
} }

View File

@@ -17,13 +17,14 @@ impl RateLimiter {
} }
} }
/// Returns `true` if the request is allowed, `false` if rate-limited.
pub fn check(&self, key: impl Into<String>, max: usize, window: Duration) -> bool {
self.check_with_retry(key, max, window).is_ok()
}
/// Returns `Ok(())` if allowed, `Err(retry_after_secs)` if rate-limited. /// Returns `Ok(())` if allowed, `Err(retry_after_secs)` if rate-limited.
/// `retry_after_secs` is how long until the oldest slot in the window expires. /// `retry_after_secs` is how long until the oldest slot in the window expires.
///
/// This is deliberately the ONLY entry point. There used to be a `check()` wrapper
/// returning a plain bool, and 7 of the 8 call sites used it and then hard-coded
/// `None` for the response's `Retry-After` — so a throttled client was told to back
/// off but never for how long. Forcing every caller through the `Result` makes the
/// retry delay impossible to discard by accident.
pub fn check_with_retry( pub fn check_with_retry(
&self, &self,
key: impl Into<String>, key: impl Into<String>,
@@ -84,6 +85,11 @@ impl RateLimiter {
/// appends is the real client. A client can prepend arbitrary spoofed values to /// 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 /// the left of XFF to dodge throttles — those are ignored here. This assumes
/// exactly one trusted proxy (Caddy); revisit if that changes. /// exactly one trusted proxy (Caddy); revisit if that changes.
///
/// Pass the peer address as `fallback`, never a constant. Every caller used to pass
/// the literal `"unknown"`, so any request that arrived without XFF — i.e. anything
/// reaching the app directly rather than through Caddy — shared ONE bucket with every
/// other such request, turning the limiter into a self-inflicted global throttle.
pub fn client_ip(headers: &axum::http::HeaderMap, fallback: &str) -> String { pub fn client_ip(headers: &axum::http::HeaderMap, fallback: &str) -> String {
headers headers
.get("x-forwarded-for") .get("x-forwarded-for")
@@ -104,29 +110,35 @@ mod tests {
#[test] #[test]
fn allows_up_to_max_then_blocks() { fn allows_up_to_max_then_blocks() {
let rl = RateLimiter::new(); let rl = RateLimiter::new();
assert!(rl.check("k", 3, MIN)); assert!(rl.check_with_retry("k", 3, MIN).is_ok());
assert!(rl.check("k", 3, MIN)); assert!(rl.check_with_retry("k", 3, MIN).is_ok());
assert!(rl.check("k", 3, MIN)); assert!(rl.check_with_retry("k", 3, MIN).is_ok());
assert!(!rl.check("k", 3, MIN), "the 4th request must be blocked"); assert!(
rl.check_with_retry("k", 3, MIN).is_err(),
"the 4th request must be blocked"
);
} }
#[test] #[test]
fn keys_are_independent() { fn keys_are_independent() {
let rl = RateLimiter::new(); let rl = RateLimiter::new();
assert!(rl.check("a", 1, MIN)); assert!(rl.check_with_retry("a", 1, MIN).is_ok());
assert!(!rl.check("a", 1, MIN)); assert!(rl.check_with_retry("a", 1, MIN).is_err());
assert!(rl.check("b", 1, MIN), "a different key has its own window"); assert!(
rl.check_with_retry("b", 1, MIN).is_ok(),
"a different key has its own window"
);
} }
#[test] #[test]
fn window_slides_and_allows_again_after_expiry() { fn window_slides_and_allows_again_after_expiry() {
let rl = RateLimiter::new(); let rl = RateLimiter::new();
let w = Duration::from_millis(40); let w = Duration::from_millis(40);
assert!(rl.check("k", 1, w)); assert!(rl.check_with_retry("k", 1, w).is_ok());
assert!(!rl.check("k", 1, w)); assert!(rl.check_with_retry("k", 1, w).is_err());
std::thread::sleep(Duration::from_millis(55)); std::thread::sleep(Duration::from_millis(55));
assert!( assert!(
rl.check("k", 1, w), rl.check_with_retry("k", 1, w).is_ok(),
"the slot should expire once the window passes" "the slot should expire once the window passes"
); );
} }
@@ -191,10 +203,13 @@ mod tests {
#[test] #[test]
fn clear_resets_every_window() { fn clear_resets_every_window() {
let rl = RateLimiter::new(); let rl = RateLimiter::new();
assert!(rl.check("k", 1, MIN)); assert!(rl.check_with_retry("k", 1, MIN).is_ok());
assert!(!rl.check("k", 1, MIN)); assert!(rl.check_with_retry("k", 1, MIN).is_err());
rl.clear(); rl.clear();
assert!(rl.check("k", 1, MIN), "clear() must free the window"); assert!(
rl.check_with_retry("k", 1, MIN).is_ok(),
"clear() must free the window"
);
} }
/// `prune()` is a memory-leak guard: without it a long-lived process keeps one HashMap /// `prune()` is a memory-leak guard: without it a long-lived process keeps one HashMap
@@ -216,7 +231,7 @@ mod tests {
.insert("stale".to_string(), vec![ancient]); .insert("stale".to_string(), vec![ancient]);
// ...alongside a key that is still inside its window. // ...alongside a key that is still inside its window.
assert!(rl.check("live", 5, MIN)); assert!(rl.check_with_retry("live", 5, MIN).is_ok());
assert_eq!(rl.windows.lock().unwrap().len(), 2); assert_eq!(rl.windows.lock().unwrap().len(), 2);
rl.prune(); rl.prune();
@@ -239,13 +254,13 @@ mod tests {
// prune() dropped live keys, every background sweep would hand attackers a fresh // prune() dropped live keys, every background sweep would hand attackers a fresh
// budget. // budget.
let rl = RateLimiter::new(); let rl = RateLimiter::new();
assert!(rl.check("k", 1, MIN)); assert!(rl.check_with_retry("k", 1, MIN).is_ok());
assert!(!rl.check("k", 1, MIN)); assert!(rl.check_with_retry("k", 1, MIN).is_err());
rl.prune(); rl.prune();
assert!( assert!(
!rl.check("k", 1, MIN), rl.check_with_retry("k", 1, MIN).is_err(),
"prune() must not clear a window that is still active" "prune() must not clear a window that is still active"
); );
} }

View File

@@ -13,7 +13,11 @@ test.describe('Auth — join flow', () => {
const join = new JoinPage(page); const join = new JoinPage(page);
await join.goto(); await join.goto();
await expect(page.getByRole('heading', { name: 'Willkommen!' })).toBeVisible(); // The join form's landing state. There is no "Willkommen!" heading — the wedding
// redesign (f243bfe) split it into a "Willkommen bei" lead-in plus the event name as
// the <h1>, and this assertion was never updated, so it had been failing since.
// Anchor on the testid the markup provides rather than on copy.
await expect(page.getByTestId('join-event-name')).toBeVisible();
const { pin } = await join.joinAs('Alice'); const { pin } = await join.joinAs('Alice');
expect(pin).toMatch(/^\d{4}$/); expect(pin).toMatch(/^\d{4}$/);

View File

@@ -0,0 +1,144 @@
/**
* Regression guard — the door must not close on a venue behind one NAT.
*
* `/join` was throttled 5 per 60s keyed purely on the client IP. Every guest at a venue
* arrives from the same public IP (that is what a NAT is), so the whole party shared one
* bucket: 12 guests scanning the QR code within a few seconds meant 5 got in and 7 were
* turned away — with no Retry-After to tell them when to try again. `/feed` (60/min) and
* `/export` (3/DAY) had the identical defect.
*
* These ran green for the same structural reason every time: the e2e reseed forces every
* limiter toggle OFF before each test, so nothing here was ever exercised. Enable them
* explicitly, exactly as 02-upload/rate-limit does.
*/
import { test, expect } from '../../fixtures/test';
import { BASE } from '../../helpers/env';
test.describe('Rate limits — guests behind a shared NAT', () => {
test('a dozen guests can all join from one IP, and 429s carry Retry-After', async ({
api,
adminToken,
}) => {
await api.patchConfig(adminToken, {
rate_limits_enabled: 'true',
join_rate_enabled: 'true',
});
// Twelve DISTINCT guests, same source IP — the arrival burst at a real party.
const names = Array.from({ length: 12 }, (_, i) => `NatGuest${i}`);
const results = await Promise.all(
names.map((display_name) =>
fetch(`${BASE}/api/v1/join`, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ display_name }),
})
)
);
const rejected = results.filter((r) => r.status === 429);
expect(
rejected.length,
`all 12 guests must get in from one IP; ${rejected.length} were turned away`
).toBe(0);
expect(results.every((r) => r.status === 201)).toBe(true);
});
test('one guest retrying their own name is still throttled, and told for how long', async ({
api,
adminToken,
}) => {
// The per-name bucket must still bite — otherwise the NAT fix would have simply
// removed the anti-spam limit rather than re-keyed it.
await api.patchConfig(adminToken, {
rate_limits_enabled: 'true',
join_rate_enabled: 'true',
});
const attempt = () =>
fetch(`${BASE}/api/v1/join`, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ display_name: 'RepeatOffender' }),
});
// 5 per 60s for the same (ip, name): the first succeeds (201), the next four collide
// with the taken name (409), and the sixth exhausts the bucket.
const codes: number[] = [];
for (let i = 0; i < 6; i++) codes.push((await attempt()).status);
expect(codes[0], 'the first join should succeed').toBe(201);
expect(codes.at(-1), 'the 6th attempt on one name must be throttled').toBe(429);
const throttled = await attempt();
expect(throttled.status).toBe(429);
const retryAfter = throttled.headers.get('retry-after');
expect(retryAfter, '429 must tell the client when to come back').toBeTruthy();
expect(Number(retryAfter)).toBeGreaterThan(0);
expect(Number(retryAfter)).toBeLessThanOrEqual(60);
});
test('the feed limit is per-user, not per-IP', async ({ api, adminToken, guest }) => {
// Two guests, one IP. With a limit of 3/min an IP key would let the first guest's
// three reads starve the second entirely.
await api.patchConfig(adminToken, {
rate_limits_enabled: 'true',
feed_rate_enabled: 'true',
feed_rate_per_min: '3',
});
const a = await guest('FeedHog');
const b = await guest('FeedVictim');
const read = (jwt: string) =>
fetch(`${BASE}/api/v1/feed`, { headers: { Authorization: `Bearer ${jwt}` } });
// Guest A burns their whole allowance.
for (let i = 0; i < 3; i++) expect((await read(a.jwt)).status).toBe(200);
expect((await read(a.jwt)).status, "A's own 4th read is throttled").toBe(429);
// Guest B must be entirely unaffected.
expect((await read(b.jwt)).status, 'B must not inherit As exhausted bucket').toBe(200);
});
test('the export limit is per-user — one guest cannot spend the whole venues quota', async ({
api,
adminToken,
guest,
host,
db,
}) => {
// The sharpest case: 3 downloads per DAY on an IP key meant the 4th guest to fetch
// their keepsake was locked out until tomorrow.
await db.setExportReleased('e2e-test-event', true);
await api.patchConfig(adminToken, {
rate_limits_enabled: 'true',
export_rate_enabled: 'true',
export_rate_per_day: '1',
});
const mintAndFetch = async (jwt: string) => {
const res = await fetch(`${BASE}/api/v1/export/ticket`, {
method: 'POST',
headers: { Authorization: `Bearer ${jwt}` },
});
const { ticket } = await res.json();
return fetch(`${BASE}/api/v1/export/zip?ticket=${encodeURIComponent(ticket)}`);
};
const a = await guest('ExportFirst');
const b = await guest('ExportSecond');
// A spends their single daily allowance. The archive itself may not exist (404) —
// what matters is that the limiter admitted the request rather than 429ing it.
expect((await mintAndFetch(a.jwt)).status).not.toBe(429);
expect((await mintAndFetch(a.jwt)).status, 'As second download is throttled').toBe(429);
// B shares A's IP and must still get their keepsake.
expect((await mintAndFetch(b.jwt)).status, 'B must not be locked out by As download').not.toBe(
429
);
// And the host too, for good measure.
expect((await mintAndFetch(host.jwt)).status).not.toBe(429);
});
});

View File

@@ -15,7 +15,15 @@ test.describe('Adversarial — small-scale abuse', () => {
await api.patchConfig(adminToken, { rate_limits_enabled: 'true', join_rate_enabled: 'true' }); await api.patchConfig(adminToken, { rate_limits_enabled: 'true', join_rate_enabled: 'true' });
}); });
test('20 parallel /join from one IP — rate limiter catches the excess', async () => { test('a /join flood from one IP is caught by the per-IP ceiling', async ({ api, adminToken }) => {
// This used to assert that 20 joins from one IP produced 429s under a 5/min per-IP
// bucket. That "protection" was the bug: at a venue every guest shares one public IP,
// so it turned real arriving guests away (see 01-auth/rate-limit-shared-nat). The
// anti-spam bucket is now per (ip, name); what remains per-IP is a loose ceiling whose
// job is only to bound raw volume. Squeeze the ceiling so a flood is reproducible here
// without firing 60+ requests.
await api.patchConfig(adminToken, { join_ip_rate_per_min: '5' });
const requests = Array.from({ length: 20 }, (_, i) => const requests = Array.from({ length: 20 }, (_, i) =>
fetch(`${BASE}/api/v1/join`, { fetch(`${BASE}/api/v1/join`, {
method: 'POST', method: 'POST',
@@ -24,7 +32,7 @@ test.describe('Adversarial — small-scale abuse', () => {
}) })
); );
const statuses = (await Promise.all(requests)).map((r) => r.status); const statuses = (await Promise.all(requests)).map((r) => r.status);
// 5/min limit → at least some should be 429. // Ceiling of 5 → the excess must be shed.
expect(statuses.filter((s) => s === 429).length).toBeGreaterThan(0); expect(statuses.filter((s) => s === 429).length).toBeGreaterThan(0);
// Server stays up — at least one succeeded. // Server stays up — at least one succeeded.
expect(statuses.some((s) => s === 201 || s === 409)).toBe(true); expect(statuses.some((s) => s === 201 || s === 409)).toBe(true);