Username-Enumeration-Vulnerability

[kingjosmel]

Security Fix: Prevent Email Address Enumeration on Login

close #163

Overview

Standardizes the error response for non-existent users on the login endpoint to prevent email enumeration and mitigates potential timing attacks.

Changes

• Updated loginController to return 401 Unauthorized with { message: "Invalid credentials" } when a user is not found, matching invalid password responses.
• Added a dummy bcrypt.compare execution when user lookup fails to equalize response timing and mitigate timing attacks.
• Updated tests/login.controller.test.ts to assert the updated 401 status code and message.

Summary by CodeRabbit

• Bug Fixes

  • Login attempts for unknown email addresses now return the same generic “Invalid credentials” response as other failed sign-ins.
  • This provides more consistent authentication behavior and avoids revealing whether an account exists.

• Tests

  • Updated login validation coverage to reflect the unified error response.

[drips-wave]

@kingjosmel Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

[coderabbitai]

Warning

Review limit reached

@kingjosmel, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 47 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 98479e68-3a61-4aef-adf3-c0cbbb4236e9

📥 Commits

Reviewing files that changed from the base of the PR and between d5e0335 and 42fbd0b.

📒 Files selected for processing (10)

• src/controllers/login.controller.ts
• src/services/asset-pricing.service.ts
• src/services/event-ticket.service.ts
• src/services/ticket-order.service.ts
• src/utils/pagination-cursor.ts
• tests/event-ticket.controller.test.ts
• tests/event-ticket.service.test.ts
• tests/payment-conversion.service.test.ts
• tests/setup.ts
• tests/ticket-order.service.test.ts

📝 Walkthrough

Walkthrough

The login controller now performs dummy bcrypt verification when no user is found and returns HTTP 401 with a generic invalid-credentials message. The corresponding test expectations were updated.

Changes

Login credential handling

Layer / File(s)	Summary
Generic invalid-credentials response src/controllers/login.controller.ts, tests/login.controller.test.ts	The missing-user path performs bcrypt comparison against a dummy hash and returns 401 { message: "Invalid credentials" }; the unit test verifies the updated response.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: yerimahjr

🚥 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 matches the main security change and clearly indicates the login username-enumeration fix.
Linked Issues check	✅ Passed	The PR satisfies #163 by returning 401 Invalid credentials for missing users and invalid passwords, and updates tests accordingly.
Out of Scope Changes check	✅ Passed	The diff stays focused on the login enumeration fix and related tests, with no unrelated code changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Fix failing CI checks

🧪 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]

🧹 Nitpick comments (1)

tests/login.controller.test.ts (1)

26-34: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Test the timing-equalization behavior as well.

This test would pass even if the dummy bcrypt.compare call were removed. Mock bcrypt.compare and assert it receives the supplied password plus a valid fixed hash, so the security requirement is protected against regression.

🤖 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 `@tests/login.controller.test.ts` around lines 26 - 34, Strengthen the
user-not-found test around loginController by mocking bcrypt.compare and
asserting it is called with the supplied password and a valid fixed hash, while
retaining the existing 401 response assertions. Ensure the test verifies the
dummy comparison used for timing equalization and would fail if that call were
removed.

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

Nitpick comments:
In `@tests/login.controller.test.ts`:
- Around line 26-34: Strengthen the user-not-found test around loginController
by mocking bcrypt.compare and asserting it is called with the supplied password
and a valid fixed hash, while retaining the existing 401 response assertions.
Ensure the test verifies the dummy comparison used for timing equalization and
would fail if that call were removed.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f19275c2-424e-4039-bcf2-d4e9bfb164b9

📥 Commits

Reviewing files that changed from the base of the PR and between 3334a41 and d5e0335.

📒 Files selected for processing (2)

• src/controllers/login.controller.ts
• tests/login.controller.test.ts