diff --git a/.env.example b/.env.example index 6cfa300..5493161 100644 --- a/.env.example +++ b/.env.example @@ -29,6 +29,13 @@ COOKIE_DOMAIN= # get reaped lazily. SESSION_TTL_DAYS=30 +# ----- Auth brute-force rate limits ----- +# Token-bucket budget shared across /auth/login, /auth/register, and +# /auth/me/password. Set per_sec=0 to disable (e.g. behind a +# rate-limiting reverse proxy that already enforces a budget). +AUTH_RATE_PER_SEC=5 +AUTH_RATE_BURST=10 + # ----- CORS ----- # Comma-separated origins allowed to call the API with credentials. # Default is empty: same-origin only. Set when frontend and backend live diff --git a/backend/Cargo.lock b/backend/Cargo.lock index 5642fcc..e848cca 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -1470,7 +1470,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4" [[package]] name = "mangalord" -version = "0.34.0" +version = "0.35.0" dependencies = [ "anyhow", "argon2", diff --git a/backend/Cargo.toml b/backend/Cargo.toml index c091570..1209b66 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mangalord" -version = "0.34.0" +version = "0.35.0" edition = "2021" default-run = "mangalord" diff --git a/backend/src/api/auth.rs b/backend/src/api/auth.rs index 3dd5cc2..ba09c99 100644 --- a/backend/src/api/auth.rs +++ b/backend/src/api/auth.rs @@ -80,6 +80,7 @@ async fn register( jar: CookieJar, Json(input): Json, ) -> AppResult { + check_auth_rate_limit(&state, "register")?; let username = input.username.trim(); validate_username(username)?; validate_password(&input.password)?; @@ -95,6 +96,7 @@ async fn login( jar: CookieJar, Json(input): Json, ) -> AppResult { + check_auth_rate_limit(&state, "login")?; let username = input.username.trim(); if username.is_empty() || input.password.is_empty() { return Err(AppError::InvalidInput( @@ -149,6 +151,7 @@ async fn change_password( jar: CookieJar, Json(input): Json, ) -> AppResult { + check_auth_rate_limit(&state, "change_password")?; if !verify_password(&input.current_password, &user.password_hash) { return Err(AppError::Unauthenticated); } @@ -293,6 +296,33 @@ fn build_expired_cookie(cfg: &AuthConfig) -> Cookie<'static> { builder.build() } +/// Consume one token from the shared auth rate limiter. Called at the +/// start of `register`, `login`, and `change_password` so credential +/// stuffing / spraying / username-probe loops are throttled by the +/// configured budget (default 5/sec with a 10-request burst). +/// +/// All three endpoints share one bucket — they all expose the same +/// argon2-verify-or-create work and the same enumeration channels, so +/// any one of them in a tight loop should trip the limit. `endpoint` +/// is included in the rate-limit-hit log line so operators can tell +/// which endpoint is being probed. +fn check_auth_rate_limit(state: &AppState, endpoint: &'static str) -> AppResult<()> { + use crate::auth::rate_limit::AcquireResult; + match state.auth_limiter.try_acquire() { + AcquireResult::Allowed => Ok(()), + AcquireResult::Denied { retry_after_secs } => { + tracing::warn!( + endpoint, + retry_after_secs, + "auth rate limit hit; returning 429" + ); + Err(AppError::TooManyRequests { + retry_after_secs: Some(retry_after_secs), + }) + } + } +} + fn validate_username(u: &str) -> AppResult<()> { if u.is_empty() { return Err(AppError::InvalidInput("username is required".into())); diff --git a/backend/src/app.rs b/backend/src/app.rs index 27734bb..3a47315 100644 --- a/backend/src/app.rs +++ b/backend/src/app.rs @@ -12,6 +12,7 @@ use tokio_util::sync::CancellationToken; use tower_http::cors::{AllowOrigin, CorsLayer}; use tower_http::trace::TraceLayer; +use crate::auth::rate_limit::AuthRateLimiter; use crate::config::{AuthConfig, Config, CrawlerConfig, CrawlerModePref, UploadConfig}; use crate::crawler::browser_manager::{self, BrowserManager}; use crate::crawler::content::{self, SyncOutcome}; @@ -30,6 +31,10 @@ pub struct AppState { pub storage: Arc, pub auth: AuthConfig, pub upload: UploadConfig, + /// Shared rate limiter guarding the `/auth/*` mutation endpoints. + /// One instance per AppState so tests stay isolated across the + /// same process. + pub auth_limiter: Arc, } /// Bundle returned by [`build`]. The router is what `axum::serve` consumes; @@ -64,11 +69,13 @@ pub async fn build(config: Config) -> anyhow::Result { None }; + let auth_limiter = Arc::new(AuthRateLimiter::new(config.auth.rate_limit)); let state = AppState { db, storage, auth: config.auth.clone(), upload: config.upload.clone(), + auth_limiter, }; let router = router(state).layer(cors_layer(&config.cors_allowed_origins)); Ok(AppHandle { router, daemon }) diff --git a/backend/src/auth/mod.rs b/backend/src/auth/mod.rs index 4574379..0c5b09f 100644 --- a/backend/src/auth/mod.rs +++ b/backend/src/auth/mod.rs @@ -7,4 +7,5 @@ pub mod extractor; pub mod password; +pub mod rate_limit; pub mod token; diff --git a/backend/src/auth/rate_limit.rs b/backend/src/auth/rate_limit.rs new file mode 100644 index 0000000..1212651 --- /dev/null +++ b/backend/src/auth/rate_limit.rs @@ -0,0 +1,179 @@ +//! Per-process token-bucket rate limiter for the auth endpoints. +//! +//! Protects `/auth/login`, `/auth/register`, and `/auth/me/password` +//! from credential stuffing / password spraying / username probing. +//! +//! The current deploy puts SvelteKit's hooks.server.ts proxy in front +//! of axum without forwarding the original client IP (no +//! `X-Forwarded-For`), so per-IP buckets would all collapse to the +//! proxy container's address. Until the proxy learns to forward the +//! peer address, a single global bucket gives equivalent protection +//! against mass-attack patterns and trades a small DoS surface +//! (legitimate users sharing the limit) for simplicity. +//! +//! Each `AppState` carries its own [`AuthRateLimiter`] instance, so +//! tests run in isolated buckets and won't bleed across `#[sqlx::test]` +//! cases that share a process. + +use std::sync::Mutex; +use std::time::Instant; + +/// Tunable limits. `per_sec == 0` disables the limiter — used by the +/// test harness and by anyone who wants to opt out via env config. +#[derive(Clone, Copy, Debug)] +pub struct RateLimitConfig { + pub per_sec: u32, + pub burst: u32, +} + +impl Default for RateLimitConfig { + /// Disabled by default. The production `AuthConfig::from_env` + /// overrides to a real limit; the test harness keeps the default + /// so existing tests don't flake against shared buckets. + fn default() -> Self { + Self { + per_sec: 0, + burst: 0, + } + } +} + +/// Production defaults: 5 requests/sec sustained, 10-request burst. +/// Tight enough to make brute force impractical, loose enough that a +/// real user mistyping their password three times in a row doesn't +/// hit it. +pub const PRODUCTION_PER_SEC: u32 = 5; +pub const PRODUCTION_BURST: u32 = 10; + +struct Bucket { + tokens: f64, + last_refill: Instant, +} + +/// Outcome of [`AuthRateLimiter::try_acquire`]. When `Denied`, the +/// caller can use `retry_after_secs` for a `Retry-After: N` header +/// (RFC 6585 §4) so well-behaved clients back off correctly rather +/// than retrying in a tight loop. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum AcquireResult { + Allowed, + Denied { retry_after_secs: u64 }, +} + +/// Single-bucket token-bucket limiter. `try_acquire` is cheap (one +/// mutex acquire, no allocations) so the auth path doesn't pay a real +/// cost for the check. +pub struct AuthRateLimiter { + cfg: RateLimitConfig, + bucket: Mutex, +} + +impl AuthRateLimiter { + pub fn new(cfg: RateLimitConfig) -> Self { + Self { + cfg, + bucket: Mutex::new(Bucket { + tokens: cfg.burst as f64, + last_refill: Instant::now(), + }), + } + } + + /// Consume one token if available. Returns `Denied` with a + /// rounded-up seconds-until-refill so the caller can emit a + /// `Retry-After` header. + pub fn try_acquire(&self) -> AcquireResult { + if self.cfg.per_sec == 0 { + return AcquireResult::Allowed; + } + let now = Instant::now(); + let mut bucket = self.bucket.lock().expect("rate limiter mutex poisoned"); + let elapsed = now.duration_since(bucket.last_refill).as_secs_f64(); + bucket.tokens = + (bucket.tokens + elapsed * f64::from(self.cfg.per_sec)).min(f64::from(self.cfg.burst)); + bucket.last_refill = now; + if bucket.tokens >= 1.0 { + bucket.tokens -= 1.0; + AcquireResult::Allowed + } else { + // ceil((1 - tokens) / per_sec), minimum 1 — a `Retry-After: 0` + // would tell clients to retry immediately, which is what we're + // actively trying to discourage. + let deficit = 1.0 - bucket.tokens; + let wait_secs = (deficit / f64::from(self.cfg.per_sec)).ceil() as u64; + AcquireResult::Denied { + retry_after_secs: wait_secs.max(1), + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn disabled_limiter_always_allows() { + let rl = AuthRateLimiter::new(RateLimitConfig { + per_sec: 0, + burst: 0, + }); + for _ in 0..1000 { + assert_eq!(rl.try_acquire(), AcquireResult::Allowed); + } + } + + #[test] + fn burst_lets_through_initial_window_then_blocks() { + // 0 refill, burst 3 → first three pass, fourth blocks. + let rl = AuthRateLimiter::new(RateLimitConfig { + per_sec: 1, + burst: 3, + }); + assert_eq!(rl.try_acquire(), AcquireResult::Allowed); + assert_eq!(rl.try_acquire(), AcquireResult::Allowed); + assert_eq!(rl.try_acquire(), AcquireResult::Allowed); + match rl.try_acquire() { + AcquireResult::Denied { retry_after_secs } => { + // Bucket is at ~0 tokens, refill rate 1/sec → ~1s wait. + assert!( + retry_after_secs >= 1, + "retry_after must be at least 1s, got {retry_after_secs}" + ); + } + AcquireResult::Allowed => panic!("fourth request must be denied"), + } + } + + #[test] + fn tokens_refill_over_time() { + // 10/sec → after ~120ms we should have at least one token back. + let rl = AuthRateLimiter::new(RateLimitConfig { + per_sec: 10, + burst: 1, + }); + assert_eq!(rl.try_acquire(), AcquireResult::Allowed); + assert!(matches!(rl.try_acquire(), AcquireResult::Denied { .. })); + std::thread::sleep(std::time::Duration::from_millis(150)); + assert_eq!( + rl.try_acquire(), + AcquireResult::Allowed, + "token should have refilled" + ); + } + + #[test] + fn retry_after_scales_inversely_with_refill_rate() { + // 1/sec → wait ~1s after burst exhausted. + // 10/sec → wait <1s, but we clamp to a minimum of 1s. + let slow = AuthRateLimiter::new(RateLimitConfig { + per_sec: 1, + burst: 1, + }); + slow.try_acquire(); + match slow.try_acquire() { + AcquireResult::Denied { retry_after_secs } => assert_eq!(retry_after_secs, 1), + _ => panic!("expected Denied"), + } + } +} diff --git a/backend/src/config.rs b/backend/src/config.rs index 7f0180a..d0a9a8f 100644 --- a/backend/src/config.rs +++ b/backend/src/config.rs @@ -21,6 +21,7 @@ pub struct AuthConfig { pub cookie_secure: bool, pub cookie_domain: Option, pub session_ttl_days: i64, + pub rate_limit: crate::auth::rate_limit::RateLimitConfig, } impl Default for AuthConfig { @@ -29,6 +30,11 @@ impl Default for AuthConfig { cookie_secure: true, cookie_domain: None, session_ttl_days: 30, + // Disabled by default so the test harness inherits a + // non-throttling limiter. Production `from_env` overrides + // to the [`PRODUCTION_PER_SEC`]/[`PRODUCTION_BURST`] + // defaults. + rate_limit: crate::auth::rate_limit::RateLimitConfig::default(), } } } @@ -135,6 +141,16 @@ impl Config { .ok() .filter(|s| !s.is_empty()), session_ttl_days: env_i64("SESSION_TTL_DAYS", 30), + rate_limit: crate::auth::rate_limit::RateLimitConfig { + per_sec: env_u64( + "AUTH_RATE_PER_SEC", + crate::auth::rate_limit::PRODUCTION_PER_SEC.into(), + ) as u32, + burst: env_u64( + "AUTH_RATE_BURST", + crate::auth::rate_limit::PRODUCTION_BURST.into(), + ) as u32, + }, }, upload: UploadConfig { max_request_bytes: env_usize("MAX_REQUEST_BYTES", 200 * 1024 * 1024), diff --git a/backend/src/error.rs b/backend/src/error.rs index 1eea2ff..5b60dd1 100644 --- a/backend/src/error.rs +++ b/backend/src/error.rs @@ -21,6 +21,11 @@ pub enum AppError { PayloadTooLarge(String), #[error("unsupported media type: {0}")] UnsupportedMediaType(String), + /// 429 with an optional `Retry-After` header value (in seconds). + #[error("too many requests")] + TooManyRequests { + retry_after_secs: Option, + }, /// Semantic per-field validation failure. `details` is rendered into the /// envelope so the client can highlight the bad field(s). #[error("validation failed")] @@ -51,6 +56,7 @@ impl AppError { AppError::Conflict(_) => "conflict", AppError::PayloadTooLarge(_) => "payload_too_large", AppError::UnsupportedMediaType(_) => "unsupported_media_type", + AppError::TooManyRequests { .. } => "too_many_requests", AppError::ValidationFailed { .. } => "validation_failed", AppError::Database(sqlx::Error::RowNotFound) => "not_found", AppError::Database(_) => "internal_error", @@ -79,6 +85,31 @@ impl IntoResponse for AppError { AppError::UnsupportedMediaType(msg) => { (StatusCode::UNSUPPORTED_MEDIA_TYPE, msg.clone(), None) } + AppError::TooManyRequests { retry_after_secs } => { + // Emit `Retry-After: N` (RFC 6585 §4) so a well-behaved + // client can back off correctly. Done by building the + // response by hand below — the `(status, headers, + // body)` tuple shape doesn't fit the standard + // `(status, body)` IntoResponse path for the other + // variants. + let body = json!({ + "error": { + "code": code, + "message": "too many requests; slow down", + } + }); + let mut resp = (StatusCode::TOO_MANY_REQUESTS, Json(body)).into_response(); + if let Some(secs) = retry_after_secs { + // `HeaderValue: From` skips both the + // intermediate `String` allocation and the + // fallible-by-shape `from_str` path. + resp.headers_mut().insert( + axum::http::header::RETRY_AFTER, + axum::http::HeaderValue::from(*secs), + ); + } + return resp; + } AppError::ValidationFailed { message, details } => ( StatusCode::UNPROCESSABLE_ENTITY, message.clone(), diff --git a/backend/tests/api_auth.rs b/backend/tests/api_auth.rs index 8ded505..892b1fc 100644 --- a/backend/tests/api_auth.rs +++ b/backend/tests/api_auth.rs @@ -567,6 +567,81 @@ async fn user_a_cannot_delete_user_b_token(pool: PgPool) { assert_eq!(resp.status(), StatusCode::NO_CONTENT); } +/// Brute-force / spray protection: at default production limits, a +/// tight loop of /auth/login attempts should burst through the bucket +/// and then 429 every subsequent request until the bucket refills. +#[sqlx::test(migrations = "./migrations")] +async fn login_rate_limited_under_burst_pressure(pool: PgPool) { + let h = common::harness_with_auth_rate_limit(pool, 1, 3); + + // Register a victim so the wrong-password branch is real work. + let _ = h + .app + .clone() + .oneshot(common::post_json("/api/v1/auth/register", creds("victim"))) + .await + .unwrap(); + + // Register consumed one token from the burst-3 bucket. Fire 30 + // wrong-password logins back-to-back; with per_sec=1 the refill + // is too slow to keep up and at least one must come back 429. + let mut saw_429 = false; + for _ in 0..30 { + let resp = h + .app + .clone() + .oneshot(common::post_json( + "/api/v1/auth/login", + json!({ "username": "victim", "password": "wrong" }), + )) + .await + .unwrap(); + if resp.status() == StatusCode::TOO_MANY_REQUESTS { + // RFC 6585 §4: 429 SHOULD include a Retry-After header. The + // value is in seconds; with per_sec=1 the bucket needs ~1s + // to refill, so the header should be 1 or 2. + let retry_after = resp + .headers() + .get(axum::http::header::RETRY_AFTER) + .and_then(|v| v.to_str().ok()) + .and_then(|s| s.parse::().ok()) + .expect("Retry-After header present and numeric"); + assert!( + retry_after >= 1, + "Retry-After must be at least 1s, got {retry_after}" + ); + let body = common::body_json(resp).await; + assert_eq!(body["error"]["code"], "too_many_requests"); + saw_429 = true; + break; + } + } + assert!( + saw_429, + "expected at least one 429 within 30 rapid login attempts" + ); +} + +/// Default (test-harness) limits are disabled, so existing tests that +/// fire multiple auth requests don't start failing. +#[sqlx::test(migrations = "./migrations")] +async fn default_test_harness_does_not_rate_limit(pool: PgPool) { + let h = common::harness(pool); + for i in 0..50 { + let resp = h + .app + .clone() + .oneshot(common::post_json( + "/api/v1/auth/login", + json!({ "username": format!("nobody-{i}"), "password": "x" }), + )) + .await + .unwrap(); + // None of these should be 429 — only 401. + assert_eq!(resp.status(), StatusCode::UNAUTHORIZED, "iter {i}"); + } +} + #[sqlx::test(migrations = "./migrations")] async fn delete_unknown_token_is_404(pool: PgPool) { let h = common::harness(pool); diff --git a/backend/tests/common/mod.rs b/backend/tests/common/mod.rs index f49507c..dc0de86 100644 --- a/backend/tests/common/mod.rs +++ b/backend/tests/common/mod.rs @@ -15,6 +15,7 @@ use tempfile::TempDir; use tower::ServiceExt; use mangalord::app::{router, AppState}; +use mangalord::auth::rate_limit::AuthRateLimiter; use mangalord::config::{AuthConfig, UploadConfig}; use mangalord::storage::{LocalStorage, Storage, StorageError, StreamingFile}; @@ -49,20 +50,51 @@ fn harness_inner( storage: Arc, storage_dir: TempDir, ) -> Harness { + harness_with_auth_config(pool, storage, storage_dir, AuthConfig { + cookie_secure: false, + ..AuthConfig::default() + }) +} + +fn harness_with_auth_config( + pool: PgPool, + storage: Arc, + storage_dir: TempDir, + auth: AuthConfig, +) -> Harness { + let auth_limiter = Arc::new(AuthRateLimiter::new(auth.rate_limit)); let state = AppState { db: pool, storage, - auth: AuthConfig { cookie_secure: false, ..AuthConfig::default() }, + auth, upload: UploadConfig { // Keep file caps small in tests so the size-cap path is cheap to // exercise without producing tens of MBs of bytes. max_request_bytes: 4 * 1024 * 1024, max_file_bytes: 256 * 1024, }, + auth_limiter, }; Harness { app: router(state), _storage_dir: storage_dir } } +/// Like [`harness`] but configures a tight auth rate limit. Used by +/// the brute-force-rate-limiting test. +pub fn harness_with_auth_rate_limit( + pool: PgPool, + per_sec: u32, + burst: u32, +) -> Harness { + let storage_dir = tempfile::tempdir().expect("tempdir"); + let storage = Arc::new(LocalStorage::new(storage_dir.path())); + let auth = AuthConfig { + cookie_secure: false, + rate_limit: mangalord::auth::rate_limit::RateLimitConfig { per_sec, burst }, + ..AuthConfig::default() + }; + harness_with_auth_config(pool, storage, storage_dir, auth) +} + /// Wraps a real `Storage` and fails on the N-th `put` call so tests can /// assert that handlers roll their DB writes back when storage errors /// mid-upload. Reads and other operations delegate to `inner`. diff --git a/frontend/package.json b/frontend/package.json index 159dad9..6a36df7 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mangalord-frontend", - "version": "0.34.0", + "version": "0.35.0", "private": true, "type": "module", "scripts": {