fix: prevent account enumeration via resend-verification endpoint - #1455
Open
ionfwsrijan wants to merge 1 commit into
Open
fix: prevent account enumeration via resend-verification endpoint#1455ionfwsrijan wants to merge 1 commit into
ionfwsrijan wants to merge 1 commit into
Conversation
…meration Return an identical generic 200 for every input (unregistered, already verified, or unverified) so the endpoint can no longer be used as an account oracle. The verification email is only sent when a matching unverified user actually exists. Closes Canopus-Labs#1437
📝 WalkthroughWalkthroughThe resend-verification endpoint now uses one generic success response for nonexistent, verified, and unverified accounts. Tests cover response consistency, email side effects, and missing-email validation. ChangesResend verification anti-enumeration
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@backend/controllers/authController.js`:
- Around line 369-375: Update the unverified-user flow after the account-state
check in the authentication controller to catch failures from user.save() and
sendVerificationEmail(), log the error, and return the same generic HTTP 200
response used for nonexistent and verified accounts. Prevent these
delivery-operation errors from reaching the outer catch and exposing account
status.
In `@backend/tests/resendVerification.enumeration.unit.test.js`:
- Around line 21-28: Restore the process-global Module._load after this test
file finishes by assigning originalLoad back to Module._load in the test
teardown or cleanup path. Keep the testDoubles interception behavior unchanged
during execution, and ensure restoration occurs even when the tests fail.
🪄 Autofix
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 Plus
Run ID: 9ffe7b58-8df2-49de-bfd3-03f38089d44e
📒 Files selected for processing (2)
backend/controllers/authController.jsbackend/tests/resendVerification.enumeration.unit.test.js
Comment on lines
+369
to
375
| // Return the exact same response whether the account does not exist, | ||
| // is already verified, or is unverified. This prevents the endpoint | ||
| // from being used to enumerate which emails are registered. | ||
| if (!user || user.isEmailVerified) { | ||
| return res.json({ success: true, message: "If this email is registered, a verification link has been sent." }); | ||
| } | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Return the generic response when delivery operations fail.
Lines 369-375 require the same response for all account states. For an unverified user, user.save() or sendVerificationEmail() can throw and the outer catch returns HTTP 500. Nonexistent and verified users return HTTP 200 before these operations. This can reveal unverified accounts during a database-write or email-provider failure.
Catch and log failures after the user lookup, then return the generic HTTP 200 response.
Proposed fix
- await user.save();
-
- const verificationUrl = `${process.env.FRONTEND_URL}/verify-email?token=${user.emailVerificationToken}`;
- await sendVerificationEmail(user.email, verificationUrl);
+ try {
+ await user.save();
+ const verificationUrl = `${process.env.FRONTEND_URL}/verify-email?token=${user.emailVerificationToken}`;
+ await sendVerificationEmail(user.email, verificationUrl);
+ } catch (error) {
+ console.error("Resend verification delivery failed:", error);
+ }
- res.json({ success: true, message: "If this email is registered, a verification link has been sent." });
+ return res.json({ success: true, message: "If this email is registered, a verification link has been sent." });Also applies to: 384-384
🤖 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 `@backend/controllers/authController.js` around lines 369 - 375, Update the
unverified-user flow after the account-state check in the authentication
controller to catch failures from user.save() and sendVerificationEmail(), log
the error, and return the same generic HTTP 200 response used for nonexistent
and verified accounts. Prevent these delivery-operation errors from reaching the
outer catch and exposing account status.
Comment on lines
+21
to
+28
| const testDoubles = new Map(); | ||
| const originalLoad = Module._load; | ||
| Module._load = function (request, parent, isMain) { | ||
| if (testDoubles.has(request)) { | ||
| return testDoubles.get(request); | ||
| } | ||
| return originalLoad.call(this, request, parent, isMain); | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Restore Module._load after this test file.
This file replaces the process-global CommonJS loader. It never restores originalLoad. Later tests in the same process can receive testDoubles and become order-dependent.
Proposed fix
-import { describe, it, expect, beforeAll, beforeEach, afterEach, vi } from "vitest";
+import { describe, it, expect, beforeAll, beforeEach, afterEach, afterAll, vi } from "vitest";
+afterAll(() => {
+ Module._load = originalLoad;
+ testDoubles.clear();
+ clearRequireCache();
+});🤖 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 `@backend/tests/resendVerification.enumeration.unit.test.js` around lines 21 -
28, Restore the process-global Module._load after this test file finishes by
assigning originalLoad back to Module._load in the test teardown or cleanup
path. Keep the testDoubles interception behavior unchanged during execution, and
ensure restoration occurs even when the tests fail.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
POST /api/auth/resend-verificationbehaves as an account oracle. A request for an unregistered email returns a generic 200 "If this email is registered...", while a request for a registered-but-already-verified email returns a distinct400with "This email is already verified. Please log in." The distinct status code and message let an attacker enumerate which emails are registered on PrepPilot.Fix
resendVerificationEmailnow returns the identical generic200response shape for every input (unregistered, already verified, or unverified). The verification email is only sent when a matching unverified user actually exists. A400is now returned only when theemailfield itself is missing from the request body.Files changed
backend/controllers/authController.js— uniform response for all email states.backend/tests/resendVerification.enumeration.unit.test.js— new unit tests covering unregistered, verified, and unverified emails.Testing
npx vitest run tests/resendVerification.enumeration.unit.test.js— 4/4 passing.Closes #1437
The
resend-verificationendpoint now prevents account enumeration by returning the same generic200response for all email registration states. It sends verification emails only for matching unverified users and returns400only whenemailis missing.Added unit tests for all scenarios. All 4 tests pass.
Ready to merge.