feat(rate-limit): exempt admins and raise default limits - #612
Conversation
Admin users already bypass the file quota; extend the same treatment to request rate limiting so trusted operators (admin UI polling, bulk scripts) are never throttled. RateLimitMiddleware runs after AuthMiddleware, so request.state.user["is_admin"] is available — short-circuit on it, mirroring check_user_file_quota. Also raise the default limits, which were low for real usage: - RATE_LIMIT_AUTH 20 -> 60/minute: this tier is keyed on client IP (callers are unauthenticated during the login flow), so a shared corporate/NAT egress IP could throttle a legitimate login rush. - RATE_LIMIT_CHAT 60 -> 120/minute: 1 req/s is tight for streaming / agentic clients doing rapid tool-call follow-ups. - RATE_LIMIT_DEFAULT 300 -> 600/minute: a UI that polls job status while loading lists can approach 5 req/s. The IP-keyed auth-failure limiter in AuthMiddleware is unchanged: it runs before authentication and only counts failed attempts, so a successful admin never trips it and identity is unknown at that point.
📝 WalkthroughWalkthroughAdds an admin bypass to the rate-limiting middleware, so authenticated admin requests skip rate-limit checks entirely. Increases default per-minute rate limits for general, auth, and chat paths in code and env example, updates docstrings, and adds unit tests covering the admin bypass behavior. ChangesRate Limit Admin Bypass
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant RateLimitMiddleware
participant Limiter
participant Downstream
Client->>RateLimitMiddleware: HTTP request
RateLimitMiddleware->>RateLimitMiddleware: _is_admin(request)
alt admin user
RateLimitMiddleware->>Downstream: call_next(request)
else non-admin user
RateLimitMiddleware->>RateLimitMiddleware: _limit_for(path), _identity(request)
RateLimitMiddleware->>Limiter: hit / get_window_stats
alt limit exceeded
RateLimitMiddleware->>Client: 429 JSONResponse with Retry-After
else within limit
RateLimitMiddleware->>Downstream: call_next(request)
end
end
Suggested labels: enhancement, security Suggested reviewers: none identified 🐰 A hop, a skip, past limiter gates — 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
hedhoud
left a comment
hedhoud
left a comment
There was a problem hiding this comment.
Looks good to me. I only noticed one small test-only nit: the admin bypass test comment says the same-tier non-admin still gets 429, but the test itself only exercises the admin path. Either trimming that sentence or adding the non-admin assertion would make the coverage clearer. Not blocking from my side.
The comment claimed a non-admin still gets 429, but the test only exercises the admin path. Reword it to describe what the test actually asserts and note that the non-admin side is covered by the _is_admin unit tests, per PR review feedback.
|
Good catch @hedhoud — the comment was over-claiming. Fixed in 4a2b336: reworded it to describe what the test actually asserts (admin path only) and noted that the non-admin side is covered by I deliberately didn't add a live non-admin 429 assertion: exhausting the limit across |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openrag/api/middleware/rate_limit.py`:
- Around line 44-46: The `/auth/*` rate limit in `RateLimitMiddleware` is too
permissive for unauthenticated traffic keyed by client IP, so tighten it to
better resist credential-stuffing. Adjust the `_auth` bucket in the middleware
initialization to a lower default and/or add a dedicated failed-login limiter
for the login path so repeated failures are throttled more aggressively.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 70e0c1dd-9c17-4a54-8bbe-ca91fe65c561
📒 Files selected for processing (3)
infra/compose/.env.exampleopenrag/api/middleware/rate_limit.pytests/unit/api/middleware/test_rate_limit.py
| self._default = parse(os.environ.get("RATE_LIMIT_DEFAULT", "600/minute")) | ||
| self._auth = parse(os.environ.get("RATE_LIMIT_AUTH", "60/minute")) | ||
| self._chat = parse(os.environ.get("RATE_LIMIT_CHAT", "120/minute")) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
recommended rate limit per minute for login endpoints to prevent brute force
💡 Result:
There is no single "correct" rate limit for all login endpoints; the optimal configuration must be tailored to your application's expected user behavior and threat model [1][2][3]. However, authoritative security guidance from sources like OWASP recommends a layered, multi-dimensional approach rather than relying on a single IP-based limit [4][5][2]. Recommended Strategies and Best Practices: 1. Use Layered Buckets (Independent Limits) To effectively stop both targeted brute force and distributed credential stuffing, you must implement independent rate-limiting buckets that are checked separately [4][5]. If a request hits either threshold, it should be rejected [4][5]. * Per-Username Bucket: Limits attempts against any specific account, regardless of the source IP. This defends against distributed attacks where an attacker rotates through many IPs to target one account [4][5]. * Per-IP (or Per-IP+ASN) Bucket: Limits total login attempts originating from a single source (e.g., an IP address). This defends against credential-stuffing sweeps that attempt many different usernames from one location [4][5]. 2. Typical Starting Thresholds While you should tune these based on your specific traffic patterns, common starting points from industry examples include: * Per-IP: Often set between 5–10 attempts per minute [6][2][7]. Some implementations allow for a small burst (e.g., 20) to accommodate legitimate users [8][7]. * Per-Username: Often more conservative, such as 5 attempts per hour, depending on the sensitivity of the service [2][9]. * Adjustment: Start with conservative limits and gradually relax them after monitoring for false positives [6][9]. 3. Implementation Guidelines * Use Edge-Level Filtering: Perform rate limiting at the network edge (CDN, WAF, or load balancer) whenever possible [8][2][7]. This is the most efficient method because it blocks malicious traffic before it consumes application resources (like database connections or PHP workers) [8][2][7]. * Use Appropriate Algorithms: Prefer token-bucket or sliding-window algorithms [5][9][3]. Avoid simple fixed-window counters, which can be easily bypassed by bursting at the window boundary [5][3]. * State Management: In distributed or serverless environments, ensure your rate-limiting state is stored in a shared, high-performance data store like Redis [10]. In-memory counters will fail to protect against distributed attacks in multi-instance deployments [10]. * Consistent Responses: Return a generic 429 Too Many Requests status code [4][10][5]. Avoid including specific diagnostic details (such as which bucket was tripped or the number of remaining attempts) as this helps attackers refine their tools [4][5]. * Supplemental Defenses: Rate limiting alone is insufficient against sophisticated botnets [2][3]. Complement it with bot detection (device fingerprinting), CAPTCHA challenges for suspicious traffic, account lockout policies, and phishing-resistant authentication methods like passkeys [1][8][5][2][11]. Always monitor your authentication logs to distinguish between legitimate user frustration (e.g., mistyping a password) and actual attack traffic, and adjust your thresholds accordingly [6][2][12].
Citations:
- 1: https://cheatsheetseries.owasp.org/cheatsheets/Credential_Stuffing_Prevention_Cheat_Sheet.html
- 2: https://guptadeepak.com/ciam-compass/best-practices/bot-defense-rate-limiting/
- 3: https://queries.cloud/api-rate-limiting-strategies-guide
- 4: https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/Bot_Management_and_Anti-Automation_Cheat_Sheet.md
- 5: https://cheatsheetseries.owasp.org/cheatsheets/Bot_Management_and_Anti-Automation_Cheat_Sheet.html
- 6: https://www.securecodinghub.com/guides/brute-force
- 7: https://jorijn.com/en/knowledge-base/wordpress/security/brute-force-attack-protection-in-wordpress/
- 8: https://developer.wordpress.org/advanced-administration/security/brute-force/
- 9: https://blog.postman.com/what-is-api-rate-limiting/
- 10: https://auditbuffet.com/patterns/ab-002232
- 11: https://owasp.org/www-community/controls/Blocking_Brute_Force_Attacks
- 12: https://docs.nextcloud.com/server/stable/admin%5Fmanual/configuration_server/bruteforce_configuration.html
🏁 Script executed:
sed -n '1,220p' openrag/api/middleware/rate_limit.pyRepository: linagora/openrag
Length of output: 4283
Tighten /auth/* throttling
The /auth/* bucket is keyed by client IP for unauthenticated requests, so 60/min leaves a high ceiling for credential-stuffing from shared egress IPs. Add a separate failed-login sub-limit, or lower this default if brute-force resistance matters here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openrag/api/middleware/rate_limit.py` around lines 44 - 46, The `/auth/*`
rate limit in `RateLimitMiddleware` is too permissive for unauthenticated
traffic keyed by client IP, so tighten it to better resist credential-stuffing.
Adjust the `_auth` bucket in the middleware initialization to a lower default
and/or add a dedicated failed-login limiter for the login path so repeated
failures are throttled more aggressively.
2 participants
Why
Admin users already bypass the per-user file quota (
check_user_file_quotainopenrag/api/dependencies/auth.py), but they were still subject to request rate limiting. Trusted operators — the admin UI polling job status, bulk indexing scripts — shouldn't be throttled. This extends the same admin exemption toRateLimitMiddleware.While there, the default limits were low for real usage, so this raises them.
Changes
Admin bypass (
openrag/api/middleware/rate_limit.py)RateLimitMiddleware._is_admin(request)readsrequest.state.user["is_admin"]— the same dictAuthMiddlewarepopulates.dispatch()short-circuits for admins before touching the limiter, mirroring the file-quota bypass.RateLimitMiddlewareruns afterAuthMiddleware(registration is reverse of execution, seeapi/main.py), sois_adminis available. Unauthenticated/auth/*paths have no user onrequest.state, so this only ever exempts an authenticated admin.Raised default limits (per-identity, per-worker moving window)
RATE_LIMIT_AUTHRATE_LIMIT_CHATRATE_LIMIT_DEFAULT.env.exampleupdated to match, with a note about the IP-keyed/auth/*tier.Not changed
The IP-keyed auth-failure limiter in
AuthMiddlewareis deliberately left as-is: it runs before authentication and only counts failed attempts, so a successful admin never trips it — and the caller's identity is unknown at that point, so an admin bypass isn't possible (or desirable, since it's brute-force protection).Tests
Added to
tests/unit/api/middleware/test_rate_limit.py:_is_admin→ true for admin dict, false for non-admin and unauthenticated.ruff check/ruff formatclean; new tests pass locally.Summary by CodeRabbit
New Features
Bug Fixes