fix(auth): specific error messages and strong password validation (#67)#77
Open
sricharan12-hub wants to merge 1 commit into
Open
fix(auth): specific error messages and strong password validation (#67)#77sricharan12-hub wants to merge 1 commit into
sricharan12-hub wants to merge 1 commit into
Conversation
…harika-mente#67) - Login: return 404 with a clear message when the email is not registered, and 401 specifically for an incorrect password - Register: detect duplicate emails before creation and return 409 with a clear message; handle the unique-index race condition as a fallback - Enforce a strong password policy (min 8 chars, upper, lower, number, special character) on both the backend validator and the signup form - Stop AuthContext from swallowing API errors so pages can show the backend's specific error message - Normalize emails to lowercase/trimmed for consistent lookups
📝 WalkthroughWalkthroughPassword rules are tightened in both backend and signup UI, emails are normalized during auth, and login/register errors now prefer backend messages while the backend returns explicit duplicate, credential, and server responses. ChangesAuth validation and error flow
Sequence Diagram(s)sequenceDiagram
participant Signup.jsx
participant AuthContext
participant userAuth.js
participant MongoDB
Signup.jsx->>AuthContext: register(email, password)
AuthContext->>userAuth.js: POST /auth/register
userAuth.js->>MongoDB: normalize email and check existing user
MongoDB-->>userAuth.js: existing user or none
userAuth.js->>MongoDB: save new user
MongoDB-->>userAuth.js: success or duplicate key
userAuth.js-->>AuthContext: 201, 409, or 400 response
AuthContext-->>Signup.jsx: true / false or API error
sequenceDiagram
participant Login.jsx
participant AuthContext
participant userAuth.js
participant MongoDB
Login.jsx->>AuthContext: login(email, password)
AuthContext->>userAuth.js: POST /auth/login
userAuth.js->>MongoDB: lookup normalized email
MongoDB-->>userAuth.js: user record or none
userAuth.js->>userAuth.js: compare password
userAuth.js-->>AuthContext: 200, 404, 401, or 500 response
AuthContext-->>Login.jsx: true / false or API error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/src/controllers/userAuth.js`:
- Around line 52-63: The registration error handler in userAuth.js is logging
the full exception object, which can expose user identifiers on expected
duplicate-key failures. Update the catch block in the registration flow to
remove console.log("FULL ERROR:", err) and log only minimal safe fields from err
when needed, while keeping the existing duplicate-email handling in the same
catch path.
- Around line 16-20: The auth endpoints in userAuth should not reveal whether an
email is registered; unify the public failure responses for signup
duplicate-email handling and login 404/401 cases into one generic
message/status. Update the signup and login branches in the relevant controller
methods so anonymous callers always get the same generic auth failure response,
and move any account-specific guidance into separate out-of-band recovery flows.
In `@backend/src/Utils/Validetor.js`:
- Around line 11-15: The email validation in Validate currently checks the raw
input before normalization, so addresses with surrounding spaces or mixed case
can fail even though register later trims/lowercases them. Update Validate in
Validetor.js to normalize the email value before the emailRegex test, or ensure
userAuth.register passes a trimmed/lowercased email into Validate so the
validation matches the lookup behavior. Use the existing Validate and register
flow as the entry points to locate the fix.
In `@frontend/src/pages/Login.jsx`:
- Around line 42-47: The fallback in Login’s error handling is too
credential-specific for failures where Axios has no response. Update the error
path in Login.jsx so the backend-provided message is still used when available,
but the default message in the setError branch becomes a neutral retry message
for non-response failures. Keep the change localized to the Login component’s
error handling logic.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1e13a99f-7077-4e00-8d12-a9c696bf2af6
📒 Files selected for processing (5)
backend/src/Utils/Validetor.jsbackend/src/controllers/userAuth.jsfrontend/src/context/AuthContext.jsxfrontend/src/pages/Login.jsxfrontend/src/pages/Signup.jsx
Comment on lines
+16
to
+20
| return res.status(409).json({ | ||
| success: false, | ||
| error: | ||
| "This email is already registered. Please sign in or use a different email address.", | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Use a generic public auth failure response.
The duplicate-email 409 on signup plus the 404/401 split on login lets anonymous callers verify which email addresses are registered. Keep one generic message/status for public auth failures, and move account-specific guidance to out-of-band flows.
Also applies to: 56-61, 87-100
🤖 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 16 - 20, The auth endpoints
in userAuth should not reveal whether an email is registered; unify the public
failure responses for signup duplicate-email handling and login 404/401 cases
into one generic message/status. Update the signup and login branches in the
relevant controller methods so anonymous callers always get the same generic
auth failure response, and move any account-specific guidance into separate
out-of-band recovery flows.
Comment on lines
52
to
+63
| } catch (err) { | ||
| console.log("FULL ERROR:", err); | ||
|
|
||
| // Handle duplicate email (race condition / unique index violation) | ||
| if (err.code === 11000) { | ||
| return res.status(409).json({ | ||
| success: false, | ||
| error: | ||
| "This email is already registered. Please sign in or use a different email address.", | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Avoid logging the full registration exception.
Duplicate-key errors commonly include the conflicting email in their payload/message, so console.log("FULL ERROR:", err) can leak user identifiers into server logs on a normal path. Log only the minimal fields you need.
🤖 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 52 - 63, The registration
error handler in userAuth.js is logging the full exception object, which can
expose user identifiers on expected duplicate-key failures. Update the catch
block in the registration flow to remove console.log("FULL ERROR:", err) and log
only minimal safe fields from err when needed, while keeping the existing
duplicate-email handling in the same catch path.
Comment on lines
+11
to
15
| // Email validation | ||
| const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; | ||
| if (!emailRegex.test(email)) { | ||
| throw new Error("Invalid email format"); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Normalize the email before this regex check.
register trims/lowercases the address later in backend/src/controllers/userAuth.js, but Validate(req.body) runs first, so " user@example.com " is still rejected here as invalid. Validate the trimmed value here, or normalize before calling Validate, so signup matches the new lookup behavior.
🤖 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/Utils/Validetor.js` around lines 11 - 15, The email validation in
Validate currently checks the raw input before normalization, so addresses with
surrounding spaces or mixed case can fail even though register later
trims/lowercases them. Update Validate in Validetor.js to normalize the email
value before the emailRegex test, or ensure userAuth.register passes a
trimmed/lowercased email into Validate so the validation matches the lookup
behavior. Use the existing Validate and register flow as the entry points to
locate the fix.
Comment on lines
+42
to
47
| // Prefer the specific message returned by the backend (e.g. unregistered | ||
| // email vs. wrong password); fall back to a generic message otherwise. | ||
| if (errorMsg) { | ||
| setError(errorMsg); | ||
| } else { | ||
| setError("Login failed. Please check your credentials."); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use a neutral fallback for non-response failures.
When Axios fails before a response exists, this path shows “Please check your credentials,” even for offline/CORS/timeout errors. A neutral fallback like “Login failed. Please try again.” is less misleading.
Suggested change
- setError("Login failed. Please check your credentials.");
+ setError("Login failed. Please try again.");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Prefer the specific message returned by the backend (e.g. unregistered | |
| // email vs. wrong password); fall back to a generic message otherwise. | |
| if (errorMsg) { | |
| setError(errorMsg); | |
| } else { | |
| setError("Login failed. Please check your credentials."); | |
| // Prefer the specific message returned by the backend (e.g. unregistered | |
| // email vs. wrong password); fall back to a generic message otherwise. | |
| if (errorMsg) { | |
| setError(errorMsg); | |
| } else { | |
| setError("Login failed. Please try again."); |
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 44-44: Avoid using the initial state variable in setState
Context: setError(errorMsg)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
🤖 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 `@frontend/src/pages/Login.jsx` around lines 42 - 47, The fallback in Login’s
error handling is too credential-specific for failures where Axios has no
response. Update the error path in Login.jsx so the backend-provided message is
still used when available, but the default message in the setError branch
becomes a neutral retry message for non-response failures. Keep the change
localized to the Login component’s error handling logic.
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.
Description
Fixes #67
Improves the authentication system by providing clear, specific error messages and enforcing a strong password policy.
Changes
1. Specific error for unregistered email during login
2. Specific error for duplicate email registration
code === 11000fallback to handle the unique-index race condition gracefully.3. Strong password validation
Enforced on both the backend validator and the signup form:
Each rule reports its own clear validation message.
Additional fix
AuthContextpreviously swallowed API errors and returnedfalse, so the specificcatchhandlers inLogin.jsx/Signup.jsxnever ran and users always saw a generic message. Errors now propagate, and both pages display the backend's specific message. Emails are also normalized to lowercase/trimmed for consistent lookups.Testing
node --check.Summary by CodeRabbit