Prevent account enumeration via signup error messages (Issue #697)#702
Prevent account enumeration via signup error messages (Issue #697)#702anshul23102 wants to merge 1 commit into
Conversation
…csLab#697) Replace specific 'User already exists' error message with generic 'Username or email is invalid' message. This prevents attackers from enumerating valid email addresses and usernames in the system by observing different error messages during signup attempts. Changes: - Changed error message on duplicate user detection to generic message - Changed duplicate key error message to match generic message - Prevents account enumeration attacks that rely on error message differences Fixes GitMetricsLab#697
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThe signup endpoint's duplicate user detection now returns a generic error message instead of revealing whether a username or email is already registered. This prevents attackers from enumerating valid email addresses via the signup form. ChangesAccount enumeration prevention
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
github-actions
Bot
added
gssoc:approved
level:intermediate
quality:clean
type:accessibility
labels
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/routes/auth.js (1)
24-30:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove raw internal error leakage from signup responses.
At Line 29, returning
error: err.messageexposes backend internals in a public auth endpoint. This conflicts with the enumeration-hardening goal and can leak DB/runtime details. Return a generic message only, and logerr.messageserver-side instead.Suggested patch
- res.status(500).json({ message: 'Error creating user', error: err.message }); + // Log detailed error internally only + console.error('Signup error:', err); + res.status(500).json({ message: 'Signup failed. Please try again or contact support.' });🤖 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/routes/auth.js` around lines 24 - 30, The catch block in the signup route currently returns raw internal error details (err.message) to clients; instead log the full error server-side and return only a generic message. Update the catch in backend/routes/auth.js so that when not handling the duplicate-key (err.code === 11000) case you call your server logger (e.g., console.error or the app logger) with the err object (for example: log "Error creating user" and err) and then send res.status(500).json({ message: 'Error creating user' }) without including err.message or any error field.
🤖 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/routes/auth.js`:
- Line 19: Update the signup tests to match the new sanitized error message
returned by the signup handler: where the spec (auth.routes.spec.cjs) asserts
the old "User already exists" message, change those expectations to the new
"Username or email is invalid" string to match the res.status(400).json response
in the signup route handler in auth.js (the POST /signup / signup handler that
returns the sanitized error at the two call sites). Ensure both assertions that
expect the old message are updated so tests validate the new contract.
---
Outside diff comments:
In `@backend/routes/auth.js`:
- Around line 24-30: The catch block in the signup route currently returns raw
internal error details (err.message) to clients; instead log the full error
server-side and return only a generic message. Update the catch in
backend/routes/auth.js so that when not handling the duplicate-key (err.code ===
11000) case you call your server logger (e.g., console.error or the app logger)
with the err object (for example: log "Error creating user" and err) and then
send res.status(500).json({ message: 'Error creating user' }) without including
err.message or any error field.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
|
|
||
| if (existingUser) | ||
| return res.status(400).json({ message: 'User already exists' }); | ||
| return res.status(400).json({ message: 'Username or email is invalid' }); |
There was a problem hiding this comment.
Update downstream signup spec to match the new sanitized message contract.
The response contract changed at Line 19 and Line 26, but spec/auth.routes.spec.cjs (lines 71-88 in provided context) still expects "User already exists". This will cause test failures and leaves validation of the new security behavior incomplete.
Also applies to: 26-26
🤖 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/routes/auth.js` at line 19, Update the signup tests to match the new
sanitized error message returned by the signup handler: where the spec
(auth.routes.spec.cjs) asserts the old "User already exists" message, change
those expectations to the new "Username or email is invalid" string to match the
res.status(400).json response in the signup route handler in auth.js (the POST
/signup / signup handler that returns the sanitized error at the two call
sites). Ensure both assertions that expect the old message are updated so tests
validate the new contract.
|
Could the maintainers please add relevant labels to this PR? Suggested labels:
This would help with issue tracking and organization. Thank you! |
1 participant
Summary
Replace specific 'User already exists' error message with generic 'Username or email is invalid' message. This prevents attackers from enumerating valid email addresses and usernames in the system by observing different error messages during signup attempts.
Changes
Testing
Type of Change
Contributor Declaration
This contribution is original work and I have the right to contribute this code to the project. I understand that the project is licensed under its respective license and my contributions will be available under the same license terms.
Closes #697
Summary by CodeRabbit