fix: per-IP auth rate limiting instead of one global bucket

A single global token bucket let one attacker at the sustained rate 429
every user's login/register/change-password. Key buckets by client IP: the
SvelteKit proxy now stamps the real peer address onto X-Forwarded-For
(overriding any client-supplied value, anti-spoof), and axum reads it via a
ClientIp extractor — but only when AUTH_TRUSTED_PROXY is set, else it falls
back to the shared bucket (today's behavior). The per-IP map is bounded
(10k IPs, idle buckets pruned) so a spoofed-IP spray can't grow it.

AUTH_TRUSTED_PROXY defaults false (safe for a directly-exposed backend);
compose sets it true since the proxy is the single trusted hop.

Bump to 0.124.12.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

backend/src/api/auth.rs

@@ -17,7 +17,7 @@ use serde::{Deserialize, Serialize};
 use uuid::Uuid;
 
 use crate::app::AppState;
-use crate::auth::extractor::{CurrentUser, SESSION_COOKIE_NAME};
+use crate::auth::extractor::{ClientIp, CurrentUser, SESSION_COOKIE_NAME};
 use crate::auth::password::{hash_password_async, verify_password_async};
 use crate::auth::token::{generate_token, hash_token};
 use crate::config::AuthConfig;
@@ -107,6 +107,7 @@ pub struct CreatedTokenResponse {
 
 async fn register(
     State(state): State<AppState>,
+    ClientIp(client_ip): ClientIp,
     jar: CookieJar,
     Json(input): Json<Credentials>,
 ) -> AppResult<impl IntoResponse> {
@@ -114,7 +115,7 @@ async fn register(
     // the toggle can't be probed for the toggle state via timing —
     // disabled and enabled paths both consume a token, and disabled
     // returns 403 instead of running argon2.
-    check_auth_rate_limit(&state, "register")?;
+    check_auth_rate_limit(&state, "register", client_ip)?;
     // Private mode force-blocks self-registration regardless of
     // ALLOW_SELF_REGISTER — operators of locked-down instances mint
     // accounts via `POST /admin/users` instead.
@@ -133,10 +134,11 @@ async fn register(
 
 async fn login(
     State(state): State<AppState>,
+    ClientIp(client_ip): ClientIp,
     jar: CookieJar,
     Json(input): Json<Credentials>,
 ) -> AppResult<impl IntoResponse> {
-    check_auth_rate_limit(&state, "login")?;
+    check_auth_rate_limit(&state, "login", client_ip)?;
     let username = input.username.trim();
     if username.is_empty() || input.password.is_empty() {
         return Err(AppError::InvalidInput(
@@ -214,10 +216,11 @@ async fn me(CurrentUser(user): CurrentUser) -> AppResult<Json<AuthResponse>> {
 async fn change_password(
     State(state): State<AppState>,
     CurrentUser(user): CurrentUser,
+    ClientIp(client_ip): ClientIp,
     jar: CookieJar,
     Json(input): Json<ChangePassword>,
 ) -> AppResult<impl IntoResponse> {
-    check_auth_rate_limit(&state, "change_password")?;
+    check_auth_rate_limit(&state, "change_password", client_ip)?;
     // Cap current_password before verify_password runs argon2 (same DoS
     // vector as login). new_password is bounded by validate_password below.
     reject_oversized_password(&input.current_password)?;
@@ -417,9 +420,13 @@ fn build_expired_cookie(cfg: &AuthConfig) -> Cookie<'static> {
 /// 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<()> {
+fn check_auth_rate_limit(
+    state: &AppState,
+    endpoint: &'static str,
+    client_ip: Option<std::net::IpAddr>,
+) -> AppResult<()> {
     use crate::auth::rate_limit::AcquireResult;
-    match state.auth_limiter.try_acquire() {
+    match state.auth_limiter.try_acquire(client_ip) {
         AcquireResult::Allowed => Ok(()),
         AcquireResult::Denied { retry_after_secs } => {
             tracing::warn!(

backend/src/auth/extractor.rs

@@ -70,6 +70,45 @@ impl FromRequestParts<AppState> for CurrentUser {
     }
 }
 
+/// Client IP for per-IP auth rate limiting. Resolves to the first hop of
+/// `X-Forwarded-For` **only** when [`crate::config::AuthConfig::trusted_proxy`]
+/// is set (the backend is behind a proxy that overrides the header — the
+/// compose deploy). Otherwise `None`, so the limiter uses its shared bucket.
+/// Never fails: a missing or malformed header simply yields `None`.
+pub struct ClientIp(pub Option<std::net::IpAddr>);
+
+/// Parse the client IP from an `X-Forwarded-For` value: the left-most hop is
+/// the original client (later hops are intermediary proxies). Factored out so
+/// the parsing is unit-testable without constructing a request.
+pub(crate) fn first_forwarded_ip(header: &str) -> Option<std::net::IpAddr> {
+    header
+        .split(',')
+        .next()
+        .map(str::trim)
+        .filter(|s| !s.is_empty())
+        .and_then(|s| s.parse::<std::net::IpAddr>().ok())
+}
+
+#[async_trait]
+impl FromRequestParts<AppState> for ClientIp {
+    type Rejection = std::convert::Infallible;
+
+    async fn from_request_parts(
+        parts: &mut Parts,
+        state: &AppState,
+    ) -> Result<Self, Self::Rejection> {
+        if !state.auth.trusted_proxy {
+            return Ok(ClientIp(None));
+        }
+        let ip = parts
+            .headers
+            .get("x-forwarded-for")
+            .and_then(|v| v.to_str().ok())
+            .and_then(first_forwarded_ip);
+        Ok(ClientIp(ip))
+    }
+}
+
 /// Cookie-only authentication. Bot/API tokens are explicitly NOT accepted
 /// here — this is the substrate for [`RequireAdmin`] and exists precisely
 /// to keep admin authority out of bearer-token reach.
@@ -120,3 +159,34 @@ impl FromRequestParts<AppState> for RequireAdmin {
         Ok(RequireAdmin(user))
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::first_forwarded_ip;
+    use std::net::IpAddr;
+
+    #[test]
+    fn parses_left_most_client_hop() {
+        // The client is the first entry; later entries are intermediary proxies.
+        assert_eq!(
+            first_forwarded_ip("203.0.113.5, 10.0.0.1, 10.0.0.2"),
+            Some("203.0.113.5".parse::<IpAddr>().unwrap())
+        );
+        assert_eq!(
+            first_forwarded_ip("  198.51.100.9  "),
+            Some("198.51.100.9".parse::<IpAddr>().unwrap())
+        );
+        assert_eq!(
+            first_forwarded_ip("2001:db8::1, 10.0.0.1"),
+            Some("2001:db8::1".parse::<IpAddr>().unwrap())
+        );
+    }
+
+    #[test]
+    fn rejects_empty_or_garbage() {
+        assert_eq!(first_forwarded_ip(""), None);
+        assert_eq!(first_forwarded_ip("   "), None);
+        assert_eq!(first_forwarded_ip("not-an-ip"), None);
+        assert_eq!(first_forwarded_ip(", 10.0.0.1"), None);
+    }
+}

backend/src/auth/rate_limit.rs

@@ -15,9 +15,17 @@
 //! tests run in isolated buckets and won't bleed across `#[sqlx::test]`
 //! cases that share a process.
 
+use std::collections::HashMap;
+use std::net::IpAddr;
 use std::sync::Mutex;
 use std::time::Instant;
 
+/// Upper bound on distinct client IPs tracked at once, so a spray from many
+/// spoofed/rotating source IPs can't grow the map without limit. When full,
+/// idle (refilled-to-burst) buckets are pruned first; if none are idle a new
+/// IP falls back to the shared global bucket for that request.
+const MAX_TRACKED_IPS: usize = 10_000;
+
 /// 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)]
@@ -50,6 +58,42 @@ struct Bucket {
     last_refill: Instant,
 }
 
+impl Bucket {
+    fn new(burst: u32) -> Self {
+        Self {
+            tokens: f64::from(burst),
+            last_refill: Instant::now(),
+        }
+    }
+
+    /// Refill by elapsed time then try to consume one token.
+    fn try_take(&mut self, cfg: &RateLimitConfig, now: Instant) -> AcquireResult {
+        let elapsed = now.duration_since(self.last_refill).as_secs_f64();
+        self.tokens =
+            (self.tokens + elapsed * f64::from(cfg.per_sec)).min(f64::from(cfg.burst));
+        self.last_refill = now;
+        if self.tokens >= 1.0 {
+            self.tokens -= 1.0;
+            AcquireResult::Allowed
+        } else {
+            let deficit = 1.0 - self.tokens;
+            let wait_secs = (deficit / f64::from(cfg.per_sec)).ceil() as u64;
+            AcquireResult::Denied {
+                retry_after_secs: wait_secs.max(1),
+            }
+        }
+    }
+
+    /// Whether this bucket has fully refilled (i.e. the client has been idle).
+    /// Such buckets carry no state worth keeping — dropping and lazily
+    /// recreating one yields an identical full bucket — so they're the safe
+    /// eviction target under memory pressure.
+    fn is_idle(&self, cfg: &RateLimitConfig, now: Instant) -> bool {
+        let elapsed = now.duration_since(self.last_refill).as_secs_f64();
+        (self.tokens + elapsed * f64::from(cfg.per_sec)) >= f64::from(cfg.burst)
+    }
+}
+
 /// 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
@@ -60,51 +104,64 @@ pub enum AcquireResult {
     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.
+/// Token-bucket limiter keyed by client IP, with a shared global bucket for
+/// requests whose IP is unknown (no trusted proxy).
+///
+/// The old design was a single global bucket. That let one attacker at the
+/// sustained rate deny auth to *every* user (the bucket stayed drained). With
+/// the SvelteKit proxy now forwarding the peer IP (`X-Forwarded-For`, honoured
+/// only when `AUTH_TRUSTED_PROXY` is set), each source IP gets its own bucket,
+/// so an attacker only throttles themselves. When no trustworthy IP is
+/// available the caller passes `None` and the shared `global` bucket applies —
+/// exactly the previous behaviour.
 pub struct AuthRateLimiter {
     cfg: RateLimitConfig,
-    bucket: Mutex<Bucket>,
+    global: Mutex<Bucket>,
+    per_ip: Mutex<HashMap<IpAddr, Bucket>>,
 }
 
 impl AuthRateLimiter {
     pub fn new(cfg: RateLimitConfig) -> Self {
         Self {
             cfg,
-            bucket: Mutex::new(Bucket {
-                tokens: cfg.burst as f64,
-                last_refill: Instant::now(),
-            }),
+            global: Mutex::new(Bucket::new(cfg.burst)),
+            per_ip: Mutex::new(HashMap::new()),
         }
     }
 
-    /// 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 {
+    /// Consume one token from the bucket for `key` (per-IP when `Some`, the
+    /// shared global bucket when `None`). Returns `Denied` with a rounded-up
+    /// seconds-until-refill so the caller can emit a `Retry-After` header.
+    pub fn try_acquire(&self, key: Option<IpAddr>) -> 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),
+        let Some(ip) = key else {
+            return self
+                .global
+                .lock()
+                .expect("rate limiter mutex poisoned")
+                .try_take(&self.cfg, now);
+        };
+        let mut map = self.per_ip.lock().expect("rate limiter mutex poisoned");
+        if map.len() >= MAX_TRACKED_IPS && !map.contains_key(&ip) {
+            map.retain(|_, b| !b.is_idle(&self.cfg, now));
+            if map.len() >= MAX_TRACKED_IPS {
+                // Still saturated with active attackers — degrade to the shared
+                // bucket rather than grow unbounded. Worst case is the old
+                // global-bucket behaviour under an extreme distributed flood.
+                drop(map);
+                return self
+                    .global
+                    .lock()
+                    .expect("rate limiter mutex poisoned")
+                    .try_take(&self.cfg, now);
             }
         }
+        map.entry(ip)
+            .or_insert_with(|| Bucket::new(self.cfg.burst))
+            .try_take(&self.cfg, now)
     }
 }
 
@@ -119,7 +176,7 @@ mod tests {
             burst: 0,
         });
         for _ in 0..1000 {
-            assert_eq!(rl.try_acquire(), AcquireResult::Allowed);
+            assert_eq!(rl.try_acquire(None), AcquireResult::Allowed);
         }
     }
 
@@ -130,10 +187,10 @@ mod tests {
             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() {
+        assert_eq!(rl.try_acquire(None), AcquireResult::Allowed);
+        assert_eq!(rl.try_acquire(None), AcquireResult::Allowed);
+        assert_eq!(rl.try_acquire(None), AcquireResult::Allowed);
+        match rl.try_acquire(None) {
             AcquireResult::Denied { retry_after_secs } => {
                 // Bucket is at ~0 tokens, refill rate 1/sec → ~1s wait.
                 assert!(
@@ -152,11 +209,11 @@ mod tests {
             per_sec: 10,
             burst: 1,
         });
-        assert_eq!(rl.try_acquire(), AcquireResult::Allowed);
-        assert!(matches!(rl.try_acquire(), AcquireResult::Denied { .. }));
+        assert_eq!(rl.try_acquire(None), AcquireResult::Allowed);
+        assert!(matches!(rl.try_acquire(None), AcquireResult::Denied { .. }));
         std::thread::sleep(std::time::Duration::from_millis(150));
         assert_eq!(
-            rl.try_acquire(),
+            rl.try_acquire(None),
             AcquireResult::Allowed,
             "token should have refilled"
         );
@@ -170,10 +227,64 @@ mod tests {
             per_sec: 1,
             burst: 1,
         });
-        slow.try_acquire();
-        match slow.try_acquire() {
+        slow.try_acquire(None);
+        match slow.try_acquire(None) {
             AcquireResult::Denied { retry_after_secs } => assert_eq!(retry_after_secs, 1),
             _ => panic!("expected Denied"),
         }
     }
+
+    fn ip(s: &str) -> Option<IpAddr> {
+        Some(s.parse().unwrap())
+    }
+
+    #[test]
+    fn per_ip_buckets_are_independent() {
+        // The whole point of the fix: one IP draining its bucket must not deny
+        // a different IP.
+        let rl = AuthRateLimiter::new(RateLimitConfig {
+            per_sec: 1,
+            burst: 2,
+        });
+        let a = ip("203.0.113.7");
+        let b = ip("198.51.100.9");
+        assert_eq!(rl.try_acquire(a), AcquireResult::Allowed);
+        assert_eq!(rl.try_acquire(a), AcquireResult::Allowed);
+        assert!(matches!(rl.try_acquire(a), AcquireResult::Denied { .. }));
+        // b is untouched.
+        assert_eq!(rl.try_acquire(b), AcquireResult::Allowed);
+        assert_eq!(rl.try_acquire(b), AcquireResult::Allowed);
+        assert!(matches!(rl.try_acquire(b), AcquireResult::Denied { .. }));
+    }
+
+    #[test]
+    fn none_key_shares_the_global_bucket_independent_of_per_ip() {
+        let rl = AuthRateLimiter::new(RateLimitConfig {
+            per_sec: 1,
+            burst: 2,
+        });
+        // Drain the global (None) bucket.
+        assert_eq!(rl.try_acquire(None), AcquireResult::Allowed);
+        assert_eq!(rl.try_acquire(None), AcquireResult::Allowed);
+        assert!(matches!(rl.try_acquire(None), AcquireResult::Denied { .. }));
+        // A real IP is on its own bucket, unaffected by the drained global one.
+        assert_eq!(rl.try_acquire(ip("203.0.113.1")), AcquireResult::Allowed);
+    }
+
+    #[test]
+    fn tracked_ip_map_is_bounded() {
+        let rl = AuthRateLimiter::new(RateLimitConfig {
+            per_sec: 1,
+            burst: 1,
+        });
+        // Distinct IPs beyond the cap must not grow the map without bound —
+        // excess requests fall back to the shared bucket instead.
+        for i in 0..(MAX_TRACKED_IPS as u64 + 50) {
+            let octet_a = (i >> 8) as u8;
+            let octet_b = (i & 0xff) as u8;
+            let addr = format!("10.20.{octet_a}.{octet_b}");
+            let _ = rl.try_acquire(Some(addr.parse().unwrap()));
+        }
+        assert!(rl.per_ip.lock().unwrap().len() <= MAX_TRACKED_IPS);
+    }
 }

backend/src/config.rs

@@ -27,6 +27,14 @@ pub struct AuthConfig {
     /// so a private instance is locked down with a single switch.
     /// Defaults to `false` (current public behaviour).
     pub private_mode: bool,
+    /// Whether to trust a proxy-supplied `X-Forwarded-For` header as the
+    /// client IP for per-IP auth rate limiting. Enable ONLY when the backend
+    /// sits behind a trusted reverse proxy that overrides the header (the
+    /// compose deploy: SvelteKit's hooks.server.ts sets it from the real peer
+    /// address). When `false` (default), the header is ignored and the auth
+    /// limiter uses a single shared bucket — a directly-exposed backend must
+    /// keep this off or clients could spoof their IP to dodge the limit.
+    pub trusted_proxy: bool,
 }
 
 impl Default for AuthConfig {
@@ -42,6 +50,7 @@ impl Default for AuthConfig {
             rate_limit: crate::auth::rate_limit::RateLimitConfig::default(),
             allow_self_register: true,
             private_mode: false,
+            trusted_proxy: false,
         }
     }
 }
@@ -584,6 +593,7 @@ impl Config {
                 },
                 allow_self_register: env_bool("ALLOW_SELF_REGISTER", true),
                 private_mode: env_bool("PRIVATE_MODE", false),
+                trusted_proxy: env_bool("AUTH_TRUSTED_PROXY", false),
             },
             upload: UploadConfig {
                 max_request_bytes: env_usize("MAX_REQUEST_BYTES", 200 * 1024 * 1024),