Repository navigation
feat: rate limit password and email change mutations (JEF-43) - #156
Conversation
|
Warning Review limit reached
Next review available in: 51 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
WalkthroughAuthentication rate limiting now covers password updates and email-change requests. Dedicated limiter configurations are registered in the container, enforced before downstream use-case work, and covered by updated tests. ChangesAuthentication rate limiting
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant UpdatePasswordUseCase
participant IRateLimiter
participant userRepository
Client->>UpdatePasswordUseCase: execute(userId)
UpdatePasswordUseCase->>IRateLimiter: consume(update-password:user:userId)
IRateLimiter-->>UpdatePasswordUseCase: allowed or false
UpdatePasswordUseCase->>userRepository: findById(userId)
Possibly related PRs
Poem
🚥 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 |
|
Preview deployments for this PR: |
![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 `@apps/api/src/http/container.ts`:
- Around line 361-372: Replace the production registrations for
updatePasswordRateLimiter and requestEmailChangeRateLimiter with the shared
atomic Redis-backed sliding-window limiter, using the existing production
limiter configuration and dependency wiring. Keep the current process-local
RateLimiter available only under the established local-development/testing path,
and preserve each limiter’s existing RATE_LIMIT thresholds and windows.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f6afe325-861b-4888-b58d-6c8d378e13b3
📒 Files selected for processing (7)
apps/api/src/__tests__/application/user/RequestEmailChangeUseCase.test.tsapps/api/src/__tests__/application/user/UpdatePasswordUseCase.test.tsapps/api/src/__tests__/security/authorizationGuards.test.tsapps/api/src/constants.tsapps/api/src/http/container.tsapps/api/src/use-cases/user/RequestEmailChangeUseCase.tsapps/api/src/use-cases/user/UpdatePasswordUseCase.ts
| updatePasswordRateLimiter: asValue( | ||
| new RateLimiter( | ||
| RATE_LIMIT.UPDATE_PASSWORD.MAX_ATTEMPTS, | ||
| RATE_LIMIT.UPDATE_PASSWORD.WINDOW_MS, | ||
| ), | ||
| ), | ||
| requestEmailChangeRateLimiter: asValue( | ||
| new RateLimiter( | ||
| RATE_LIMIT.REQUEST_EMAIL_CHANGE.MAX_ATTEMPTS, | ||
| RATE_LIMIT.REQUEST_EMAIL_CHANGE.WINDOW_MS, | ||
| ), | ||
| ), |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Use a shared sliding-window backend for these production limiters.
The supplied RateLimiter is a process-local Map with a { count, resetAt } bucket, so these registrations are neither Redis-backed nor sliding-window. In a multi-instance deployment, attempts can be spread across instances (or cleared by restart), weakening both new brute-force protections. Register a shared, atomic production limiter and retain the in-memory implementation only for local development/testing.
🤖 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 `@apps/api/src/http/container.ts` around lines 361 - 372, Replace the
production registrations for updatePasswordRateLimiter and
requestEmailChangeRateLimiter with the shared atomic Redis-backed sliding-window
limiter, using the existing production limiter configuration and dependency
wiring. Keep the current process-local RateLimiter available only under the
established local-development/testing path, and preserve each limiter’s existing
RATE_LIMIT thresholds and windows.
Add sliding-window rate limiting to UPDATE_PASSWORD and REQUEST_EMAIL_CHANGE mutations to prevent brute-force attacks. Uses fixed-window counters with Redis-backed storage in production.
…ate-limit keys Moves assertValidPassword above the updatePassword rate-limit check so that invalid passwords fail fast without consuming the rate-limit budget. Adds assertions for the exact keys passed to the rate limiter in both UpdatePasswordUseCase and RequestEmailChangeUseCase tests.
251a90e to
9fcce0c
Compare
1 participant
Summary
Add sliding-window rate limiting to UPDATE_PASSWORD and REQUEST_EMAIL_CHANGE mutations to prevent brute-force attacks.
Changes
Test Plan
Closes JEF-43
Summary by CodeRabbit