Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions backend/controllers/authController.js
Original file line number Diff line number Diff line change
Expand Up @@ -366,16 +366,13 @@ const resendVerificationEmail = async (req, res) => {

const user = await User.findOne({ email: email.trim().toLowerCase() });

// Return success even if user not found — avoids exposing which emails are registered
if (!user) {
// 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." });
}

Comment on lines +369 to 375

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 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.

// If already verified, no need to resend
if (user.isEmailVerified) {
return res.status(400).json({ success: false, message: "This email is already verified. Please log in." });
}

// Generate a fresh token and reset expiry to 24 hours from now
user.emailVerificationToken = crypto.randomBytes(32).toString("hex");
user.emailVerificationExpires = new Date(Date.now() + 24 * 60 * 60 * 1000);
Expand All @@ -384,7 +381,7 @@ const resendVerificationEmail = async (req, res) => {
const verificationUrl = `${process.env.FRONTEND_URL}/verify-email?token=${user.emailVerificationToken}`;
await sendVerificationEmail(user.email, verificationUrl);

res.json({ success: true, message: "Verification email resent. Please check your inbox." });
res.json({ success: true, message: "If this email is registered, a verification link has been sent." });
} catch (error) {
console.error("Resend verification error:", error);
res.status(500).json({ success: false, message: "Internal server error occurred" });
Expand Down
142 changes: 142 additions & 0 deletions backend/tests/resendVerification.enumeration.unit.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
import { Module } from "node:module";
import { describe, it, expect, beforeAll, beforeEach, afterEach, vi } from "vitest";

// ---------------------------------------------------------------------------
// resend-verification enumeration fix (issue #1437): the endpoint must return
// the identical generic 200 for every input (not found / verified / unverified)
// so it cannot be used as an account oracle.
//
// authController.js is CommonJS, so we shim Node's module loader (same pattern
// as registerEnumeration.unit.test.js).
// ---------------------------------------------------------------------------

const userMock = vi.hoisted(() => ({
findOne: vi.fn(),
}));

const sendEmailMock = vi.hoisted(() => ({
sendVerificationEmail: vi.fn(),
}));

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);
};
Comment on lines +21 to +28

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 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.


const clearRequireCache = () => {
Object.keys(require.cache).forEach((key) => {
if (
key.includes("controllers\\authController") ||
key.includes("controllers/authController") ||
key.includes("models\\User") ||
key.includes("models/User")
) {
delete require.cache[key];
}
});
};

let resendVerificationEmail;

beforeAll(async () => {
clearRequireCache();
testDoubles.set("../models/User", {
findOne: userMock.findOne,
create: vi.fn(),
});
testDoubles.set("../utils/sendEmail", sendEmailMock);

const mod = await import("../controllers/authController.js");
resendVerificationEmail = mod.resendVerificationEmail;
});

beforeEach(() => {
userMock.findOne.mockReset();
sendEmailMock.sendVerificationEmail.mockReset();
sendEmailMock.sendVerificationEmail.mockResolvedValue(undefined);
});

afterEach(() => {
vi.unstubAllEnvs();
});

const mockRes = () => {
const res = { statusCode: 200, body: null };
res.status = (code) => {
res.statusCode = code;
return res;
};
res.json = (body) => {
res.body = body;
return res;
};
return res;
};

const req = (email) => ({ body: { email } });

const GENERIC = {
success: true,
message: "If this email is registered, a verification link has been sent.",
};

describe("resendVerificationEmail — account enumeration", () => {
it("returns the generic 200 and sends nothing when the email is not registered", async () => {
userMock.findOne.mockResolvedValue(null);

const res = mockRes();
await resendVerificationEmail(req("not-registered@example.com"), res);

expect(res.statusCode).toBe(200);
expect(res.body).toEqual(GENERIC);
expect(sendEmailMock.sendVerificationEmail).not.toHaveBeenCalled();
});

it("returns the generic 200 (not a 400) for an already-verified email and sends nothing", async () => {
userMock.findOne.mockResolvedValue({ email: "verified@example.com", isEmailVerified: true });

const res = mockRes();
await resendVerificationEmail(req("verified@example.com"), res);

expect(res.statusCode).toBe(200);
expect(res.body).toEqual(GENERIC);
expect(sendEmailMock.sendVerificationEmail).not.toHaveBeenCalled();
});

it("sends the email and returns the same generic shape for an unverified user", async () => {
const unverified = {
email: "unverified@example.com",
isEmailVerified: false,
emailVerificationToken: null,
emailVerificationExpires: null,
save: vi.fn(async function () {
return this;
}),
};
userMock.findOne.mockResolvedValue(unverified);
vi.stubEnv("FRONTEND_URL", "http://localhost:5173");

const res = mockRes();
await resendVerificationEmail(req("unverified@example.com"), res);

expect(res.statusCode).toBe(200);
expect(res.body).toEqual(GENERIC);
expect(sendEmailMock.sendVerificationEmail).toHaveBeenCalledTimes(1);
expect(sendEmailMock.sendVerificationEmail).toHaveBeenCalledWith(
"unverified@example.com",
expect.stringContaining("http://localhost:5173/verify-email")
);
});

it("returns 400 only when the email field itself is missing", async () => {
const res = mockRes();
await resendVerificationEmail(req(undefined), res);

expect(res.statusCode).toBe(400);
expect(userMock.findOne).not.toHaveBeenCalled();
});
});