fix: harden validate_password_compliance against credential stuffing#1687
fix: harden validate_password_compliance against credential stuffing#1687
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
✨ Finishing Touches🧪 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88bb7d0fbc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!body.captcha_token) { | ||
| throw simpleError('invalid_request', 'Captcha token is required') | ||
| } |
There was a problem hiding this comment.
Keep captcha optional for existing compliance clients
When CAPTCHA_SECRET_KEY is configured, this now hard-fails requests that omit captcha_token, but the existing caller in src/pages/settings/account/ChangePassword.vue still sends only email, password, and org_id; that means users in captcha-enabled environments will receive invalid_request and cannot complete the password-compliance remediation flow. This change needs a compatibility path (or coordinated caller update) to avoid breaking current production clients.
Useful? React with 👍 / 👎.
| return quickError(401, 'invalid_credentials', 'Invalid email or password') | ||
| } | ||
|
|
||
| await clearFailedAuth(c) |
There was a problem hiding this comment.
Keep failed-auth throttling from resetting on any success
Clearing the per-IP failed-auth counter after every successful login allows a credential-stuffing attacker to bypass the new limit by alternating failed guesses with one known-valid account from the same IP, which repeatedly resets the counter before it reaches the block threshold. This undermines the protection added in this commit; the counter should not be globally zeroed on unrelated successful authentications.
Useful? React with 👍 / 👎.
6f1aca1 to
d9eb0b0
Compare
|



Summary (AI generated)
verifyCaptchaTokenhelper insupabase/functions/_backend/utils/captcha.tsused by bothinvite_new_user_to_organdvalidate_password_compliance.validate_password_complianceflow compatible for authenticated callers by skipping CAPTCHA enforcement when a valid session token is present inauthorization.Motivation (AI generated)
The endpoint accepts public credentials checks with only an API key and had no robust abuse controls. The compatibility and throttling comments identified gaps that could block legitimate clients or weaken anti-abuse protections, so this follow-up tightens behavior without regressing existing app flows.
Business Impact (AI generated)
This keeps abuse protection in place for unauthenticated calls while avoiding false failures for established authenticated sessions, reducing risk of support regressions and maintaining reliable password compliance remediation for users.
Test Plan (AI generated)
bun lintbun lint:backendCAPTCHA_SECRET_KEY-enabled client path invalidate_password_compliance(authenticated and unauthenticated).src/pages/settings/account/ChangePassword.vue.Generated with AI