fix(backend): use generic error message on login failure to prevent user enumeration (fix #92)

[Pratyush-Panda-2006]

Description

This pull request addresses the user enumeration vulnerability in the login endpoint. Previously, the controller returned distinct error messages for "User not found" and "Invalid Credentials", allowing attackers to enumerate registered users.

Changes

• Updated backend/src/controllers/userAuth.js to return "Invalid Credentials" when the requested email does not exist in the database, matching the error returned for invalid passwords.

Fixes #92

Summary by CodeRabbit

• Bug Fixes
  • Updated the login error message for unknown email addresses to use a generic “Invalid Credentials” response, making sign-in feedback consistent across the login flow.

[coderabbitai]

📝 Walkthrough

Walkthrough

The login controller's error message for a nonexistent user lookup was changed from "User not found" to "Invalid Credentials", matching the message used for incorrect passwords elsewhere in the same login flow.

Changes

Login Error Message Unification

Layer / File(s)	Summary
Unify login error message backend/src/controllers/userAuth.js	The not-found branch in login now throws "Invalid Credentials" instead of "User not found", matching the invalid-password error message.

Estimated code review effort: 1 (Trivial) | ~2 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the security fix to use a generic login failure message and matches the changed code.
Linked Issues check	✅ Passed	The change satisfies issue #92 by making missing-user and wrong-password login failures return the same generic error.
Out of Scope Changes check	✅ Passed	The PR is narrowly scoped to the login error message change and introduces no unrelated code changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

backend/src/controllers/userAuth.js (1)

61-67: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Timing side-channel still enables user enumeration.

The message unification is correct and addresses the response-body leak. However, when the user is not found, bcrypt.compare (line 63) is skipped entirely, making that path return significantly faster than the wrong-password path. Attackers can measure response latency to distinguish existing from non-existing emails, defeating the objective of issue #92.

Perform a dummy bcrypt.compare when the user is not found to equalize timing across both failure paths.

🔒 Proposed fix to equalize timing

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

-   if (!user) throw new Error("Invalid Credentials");
+   if (!user) {
+     // Perform a dummy compare to equalize timing with the wrong-password path
+     await bcrypt.compare(password, "$2a$10$N9qo8uLOickgx2ZMRZoMyeIjZAgcfl7p92ldGxad68LJZdL17lhWy");
+     throw new Error("Invalid Credentials");
+   }

    const match = await bcrypt.compare(password, user.password);

    if (!match) {
      throw new Error("Invalid Credentials");
    }

🤖 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/src/controllers/userAuth.js` around lines 61 - 67, The login flow
still leaks user existence through timing because the `bcrypt.compare` call is
skipped when `user` is missing. Update the authentication logic in the
controller handling this block so both failure paths run a `bcrypt.compare`
call, using a dummy password/hash when no user is found, before throwing the
shared “Invalid Credentials” error. Keep the existing message unification, but
ensure the `match` check is driven by the real or dummy compare result so
response timing stays consistent.

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

Outside diff comments:
In `@backend/src/controllers/userAuth.js`:
- Around line 61-67: The login flow still leaks user existence through timing
because the `bcrypt.compare` call is skipped when `user` is missing. Update the
authentication logic in the controller handling this block so both failure paths
run a `bcrypt.compare` call, using a dummy password/hash when no user is found,
before throwing the shared “Invalid Credentials” error. Keep the existing
message unification, but ensure the `match` check is driven by the real or dummy
compare result so response timing stays consistent.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 23a69fde-9bf6-4efb-b92e-511f6bbce1ae

📥 Commits

Reviewing files that changed from the base of the PR and between 87148ba and 9b0ecab.

📒 Files selected for processing (1)

• backend/src/controllers/userAuth.js