cjw/Oxicloud
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

feat(config): make the per-caller rate limits configurable

Browse Source 
An e2e run emitted 81 × 429 in 763 log lines. The env already set
LOGIN/REGISTER/REFRESH to 36000/hour, and that changed nothing, because
those three are the only rate limiters with env vars — and they are the
wrong ones. They key on the client IP and guard the unauthenticated
front door. The limiters that fired key on the CALLER ID.

The log distinguishes them: all 81 landed on target `http::api`, never
`http::api::auth`, where login/register/refresh live.

The likely culprit is `user_profile_rate_limiter`, 60 lookups/min/caller,
guarding the visibility query behind GET /api/users/{id}. The whole
suite runs as a single `admin`, so every test shares one bucket; admin
views resolve an owner name per row and the run creates 34 users, so a
minute of tests clears 60 easily. Nothing failed, because the SPA
degrades to an unresolved name — which is exactly the problem, since
that noise would hide a real rate-limit regression.

Adds OXICLOUD_RATE_LIMIT_USER_PROFILE_MAX / _WINDOW_SECS and
OXICLOUD_RATE_LIMIT_DELTA_UPLOAD_MAX / _WINDOW_SECS, following the
existing three exactly. Defaults are the literals they replaced (60/60
and 240/60), so an operator who sets nothing sees no change; a unit test
pins that, because the failure is silent in both directions — too low
and real users get 429s on listings, too high and the `access_grants`
query loses the guard that stops an attacker exhausting it with random
UUIDs.

`tests/common/server.env` (shared by the e2e AND hurl suites) sets both
to a 1-hour budget, matching the posture already used for the other
three rather than a raised per-minute rate that would still burst-trip.

The docs now state the IP-vs-caller split, since that is what decides
which knob to reach for — and note that several actors sharing one
identity (CI, a bot, a kiosk) share one caller bucket.

Left alone: the four narrower env files (OIDC, webdav-drive-root) keep
their existing MAX=3600 with default windows. No evidence they trip the
per-caller limits, and adding config on speculation is how these files
drift.

Not fixed here: rate-limit rejections emit NO audit line, which is why
the attribution above reads "likely" rather than "confirmed" — nothing
in the log names the limiter. AGENTS.md requires one for every
rejection; that is a separate change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Edouard Vanbelle
2026-09-05 08:02:23 +02:00
parent 99a230f373
commit 9a83f8c0d1
5 changed files with 155 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/common/config.rs
+73
View File
@@ -1600,6 +1600,33 @@ pub struct RateLimitConfig {
pub lockout_max_failures: u32,
/// Account lockout duration in seconds (default: 900 = 15 min)
pub lockout_duration_secs: u64,
// ── Per-caller limits ────────────────────────────────────────────
//
// Keyed on `caller_id`, not IP: these guard an authenticated user
// against exhausting a shared resource, whereas the three above
// guard the unauthenticated front door against an attacker.
//
// The distinction matters when several actors share one identity —
// an automated test suite, a CI job, an integration bot. They then
// share one bucket, and a ceiling that is generous for one human
// is easily exceeded. That is exactly what made the e2e suite emit
// 81 × 429 in a single run, all as the same `admin`.
/// Max user-profile lookups per caller per window (default: 60).
///
/// Guards the visibility query behind `GET /api/users/{id}`, which
/// touches `access_grants` — the rate check runs BEFORE it so an
/// attacker cannot exhaust it by hammering random UUIDs.
pub user_profile_max_requests: u32,
/// User-profile lookup window in seconds (default: 60).
pub user_profile_window_secs: u64,
/// Max delta-upload requests per caller per window (default: 240).
///
/// Generous for a real client — chunk PUTs carry up to 100 MB each
/// — while stopping pin/negotiate floods.
pub delta_upload_max_requests: u32,
/// Delta-upload window in seconds (default: 60).
pub delta_upload_window_secs: u64,
}
impl Default for RateLimitConfig {
@@ -1613,6 +1640,12 @@ impl Default for RateLimitConfig {
refresh_window_secs: 60,
lockout_max_failures: 5,
lockout_duration_secs: 900,
// Unchanged from the literals these replaced in `di.rs`, so
// an operator who sets nothing sees no behaviour change.
user_profile_max_requests: 60,
user_profile_window_secs: 60,
delta_upload_max_requests: 240,
delta_upload_window_secs: 60,
}
}
}
@@ -3093,6 +3126,28 @@ impl AppConfig {
{
config.auth.rate_limit.refresh_window_secs = val;
}
if let Ok(v) = env::var("OXICLOUD_RATE_LIMIT_USER_PROFILE_MAX").map(|v| v.parse::<u32>())
&& let Ok(val) = v
{
config.auth.rate_limit.user_profile_max_requests = val;
}
if let Ok(v) =
env::var("OXICLOUD_RATE_LIMIT_USER_PROFILE_WINDOW_SECS").map(|v| v.parse::<u64>())
&& let Ok(val) = v
{
config.auth.rate_limit.user_profile_window_secs = val;
}
if let Ok(v) = env::var("OXICLOUD_RATE_LIMIT_DELTA_UPLOAD_MAX").map(|v| v.parse::<u32>())
&& let Ok(val) = v
{
config.auth.rate_limit.delta_upload_max_requests = val;
}
if let Ok(v) =
env::var("OXICLOUD_RATE_LIMIT_DELTA_UPLOAD_WINDOW_SECS").map(|v| v.parse::<u64>())
&& let Ok(val) = v
{
config.auth.rate_limit.delta_upload_window_secs = val;
}
if let Ok(v) = env::var("OXICLOUD_LOCKOUT_MAX_FAILURES").map(|v| v.parse::<u32>())
&& let Ok(val) = v
{
@@ -3959,6 +4014,24 @@ pub fn default_config() -> AppConfig {
mod tests {
use super::*;
/// The per-caller limits moved from hardcoded literals in `di.rs` into
/// config. The whole point was to add a knob, NOT to change behaviour
/// for anyone who does not turn it — so the defaults must still be the
/// values that were compiled in before.
///
/// Worth pinning because the failure is silent and asymmetric: too low
/// and real users get 429s on file listings, too high and the
/// `access_grants` visibility query loses the guard that stops an
/// attacker exhausting it with random UUIDs.
#[test]
fn per_caller_rate_limit_defaults_match_the_previous_literals() {
let rl = RateLimitConfig::default();
assert_eq!(rl.user_profile_max_requests, 60);
assert_eq!(rl.user_profile_window_secs, 60);
assert_eq!(rl.delta_upload_max_requests, 240);
assert_eq!(rl.delta_upload_window_secs, 60);
}
#[test]
fn startup_job_parses_name_and_flags() {
let jobs = parse_startup_jobs(
src/common/di.rs
+20 -10
View File
@@ -2330,19 +2330,29 @@ impl AppServiceFactory {
mock_email_sender: None, // populated below
magic_link_invite_service: None, // populated below
recipient_notification_service: None, // populated below alongside magic_link_invite_service
// 60 lookups / minute / caller; cap at 50 000 tracked
// callers to bound memory. The same limiter instance is
// shared by every clone of AppState since it lives in an
// Arc.
// Per-caller limits, configurable since the hardcoded ceilings
// had no escape hatch for deployments where several actors share
// one identity — a CI suite running as a single `admin` shares
// one bucket and trips a limit sized for one human. Defaults
// match the literals these replaced, so an operator who sets
// nothing sees no change. `OXICLOUD_RATE_LIMIT_USER_PROFILE_*` /
// `OXICLOUD_RATE_LIMIT_DELTA_UPLOAD_*`.
//
// 50 000 tracked callers caps memory in both cases. The limiter
// lives in an Arc, so every clone of AppState shares the counts.
user_profile_rate_limiter: Arc::new(
crate::interfaces::middleware::rate_limit::RateLimiter::new(60, 60, 50_000),
crate::interfaces::middleware::rate_limit::RateLimiter::new(
self.config.auth.rate_limit.user_profile_max_requests,
self.config.auth.rate_limit.user_profile_window_secs,
50_000,
),
),
// Delta upload: 240 requests / minute / caller. Generous for a
// real client (chunk PUTs carry up to 100 MB each) while
// stopping pin/negotiate floods; 50 000 tracked callers bound
// the memory like the other limiters.
delta_upload_rate_limiter: Arc::new(
crate::interfaces::middleware::rate_limit::RateLimiter::new(240, 60, 50_000),
crate::interfaces::middleware::rate_limit::RateLimiter::new(
self.config.auth.rate_limit.delta_upload_max_requests,
self.config.auth.rate_limit.delta_upload_window_secs,
50_000,
),
),
// PR 12 — per-sharer email-invite ceiling: caller_id-keyed.
// Defends against a compromised account spamming external