Add rate limiting to activation, activation code request, and TOTP setup endpoints#6232
Conversation
|
@cnkk I have started the AI code review. It will take a few minutes to complete. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughThis PR renames the public super-admin predicate function from ChangesSuper Admin Rename and Rate Limiting
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@lib/plausible_web/controllers/auth_controller.ex`:
- Around line 424-435: The rate-limit error branch currently conflates the IP
1-minute and user 5-minute throttles (Auth.rate_limit(:totp_setup_ip, conn) and
Auth.rate_limit(:totp_setup_user, user)) and renders a misleading message;
update the {:error, {:rate_limit, _}} branch to use a neutral message such as
"Too many attempts. Wait a few minutes before trying again." or, if you prefer
more accuracy, pattern-match the returned limit type from Auth.rate_limit
(inspect the {:error, {:rate_limit, limit_type}} value) and call render_error
with "Wait a minute..." for :totp_setup_ip and "Wait a few minutes..." for
:totp_setup_user so the render_error call reflects the actual throttle.
In `@lib/plausible/auth/auth.ex`:
- Around line 28-35: The test env block sets `@activation_ip_limit`,
`@totp_setup_ip_limit`, and `@activation_request_limit` to 100_000 which disables
IP-based throttling branches; change those three module attributes in the env
when env in [:test, :ce_test] branch to small limits (e.g., 5 or 10) so tests
exercise IP-based paths, and if any specific tests require higher headroom,
override the limits only within those tests or use a tagged setup to temporarily
increase them.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9be5c5dd-7e30-45a6-b107-06fa7b3937e4
📒 Files selected for processing (11)
extra/lib/plausible_web/live/verification.exlib/plausible/auth/auth.exlib/plausible/sites.exlib/plausible_web/controllers/api/internal_controller.exlib/plausible_web/controllers/auth_controller.exlib/plausible_web/controllers/stats_controller.exlib/plausible_web/plugs/authorize_public_api.exlib/plausible_web/plugs/authorize_site_access.exlib/plausible_web/plugs/super_admin_only_plug.exlib/plausible_web/templates/layout/_header.html.heextest/plausible_web/controllers/auth_controller_test.exs
Adds rate limiting to previously unprotected authentication endpoints:
POST /activate— brute-force of 4-digit codes (1000–9999)POST /activate/request-code— activation email spam preventionPOST /2fa/setup/verify— TOTP code brute-force during enrolment