Add email verification OTP flow and update test suite#111
Merged
Akash504-ai merged 13 commits intoJun 26, 2026
Conversation
…mproved error handling
|
@nihar-ux18 is attempting to deploy a commit to the Akash Santra 's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds OTP-based email verification to signup and login. The backend stores hashed verification codes, exposes verify/resend endpoints, the frontend adds a verification page and routing, and tests are updated across auth and supporting suites. ChangesEmail Verification OTP Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
🚥 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: 7
🤖 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/auth.controller.js`:
- Around line 899-909: The resendVerificationOtp flow in auth.controller should
not reveal whether a user exists or is already verified. Update the early checks
around the user lookup and user.isVerified branch so both cases return the same
generic success response used by sendOtp, while only proceeding to generate/send
an OTP when a real unverified user is found. Keep the behavior localized to
resendVerificationOtp and preserve the existing generic message shape.
- Around line 96-117: The signup flow in auth.controller.js reports success
before OTP delivery is even attempted, so move the verification email step into
the main create path around User.create and sendVerificationOtpEmail instead of
setImmediate. In the signup handler, await or otherwise handle
sendVerificationOtpEmail for the newUser.email before returning the 201
response, and surface a failure or roll back the created user when OTP delivery
cannot be started. Keep the response in the same signup/resend transaction so
the success message only returns when email delivery has at least been
attempted.
- Around line 88-91: The email-verification OTP handling in the auth controller
uses a fast SHA-256 hash, which is too cheap for offline brute force; update the
verification flow to use the same slow-hash approach as the password-reset OTP
flow already used in this file, or replace the 6-digit OTP with a longer
high-entropy token. Apply the change in the verification OTP
generation/validation logic referenced by the hashedVerificationOtp handling and
the corresponding compare logic, keeping the implementation consistent with the
existing bcrypt-based OTP flow.
In `@backend/src/models/user.model.js`:
- Around line 121-125: Add a backfill or compatibility path for the new
User.isVerified field so existing documents don’t read as undefined and fail the
auth.controller login gate. Update the user model/default handling and the login
check in auth.controller.js together, or run a migration to set isVerified for
legacy users before enforcing the new gate.
In `@backend/src/routes/auth.route.js`:
- Around line 24-25: The /resend-verification-otp route is missing the same OTP
abuse protection used by the other email-sending endpoints. Update the
auth.route.js router registration for resendVerificationOtp so it also uses
otpRateLimiter, matching the verifyEmail and send-otp routes and keeping the
rate limiting consistent across these handlers.
In `@backend/test/auth/otp.test.js`:
- Around line 329-364: The resend verification OTP test in the `should generate
a new verification OTP and update expiry` case only verifies that
`emailVerificationOtpExpiry` is a Date, which does not confirm the expiry was
refreshed. Update the assertions in this test to capture the previous expiry
from `previousUser` and verify the new `user.emailVerificationOtpExpiry` is
later than the old value after calling `/api/auth/resend-verification-otp`,
using the existing `User` lookup and `emailVerificationOtpExpiry` field names to
keep the check tied to the controller behavior.
In `@backend/test/auth/signup.test.js`:
- Around line 54-56: The signup test is relying on a fixed 20ms sleep before
checking the OTP email mock, which makes the async assertion flaky. In
signup.test.js, replace the hardcoded timeout after the signup flow with a
deterministic wait/poll helper that waits until sendVerificationOtpEmailMock has
been called, then assert on the mock in the existing test case.
🪄 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 Plus
Run ID: 25b94da7-fc86-43ce-b94f-7b23c667a794
📒 Files selected for processing (16)
backend/src/controllers/auth.controller.jsbackend/src/lib/sendEmail.jsbackend/src/models/user.model.jsbackend/src/routes/auth.route.jsbackend/test/admin/admin.test.jsbackend/test/auth/checkAuth.test.jsbackend/test/auth/login.test.jsbackend/test/auth/otp.test.jsbackend/test/auth/signup.test.jsbackend/test/group/group.test.jsbackend/test/integration/integration.test.jsbackend/test/message/message.test.jsbackend/test/middleware/auth.middleware.test.jsbackend/test/security/security.test.jsbackend/test/socket/socket.test.jsbackend/test/utils/testHelpers.js
Comment on lines
+96
to
+117
| // Create unverified user with email verification OTP | ||
| const newUser = await User.create({ | ||
| fullName, | ||
| email: normalizedEmail, | ||
| password: hashedPassword, | ||
| securityQuestions: hashedQuestions, | ||
| role: "user", | ||
| isVerified: false, | ||
| emailVerificationOtp: hashedVerificationOtp, | ||
| emailVerificationOtpExpiry: verificationOtpExpiry, | ||
| }); | ||
|
|
||
| // Generate JWT immediately | ||
| const token = generateToken(newUser._id); | ||
|
|
||
| // Send Welcome Email (non-blocking) | ||
| setImmediate(() => { | ||
| sendWelcomeEmail( | ||
| sendVerificationOtpEmail( | ||
| newUser.email, | ||
| newUser.fullName | ||
| verificationOtp | ||
| ); | ||
| }); | ||
|
|
||
| // Send user response | ||
| res.status(201).json({ | ||
| _id: newUser._id, | ||
| fullName: newUser.fullName, | ||
| email: newUser.email, | ||
| profilePic: newUser.profilePic, | ||
| role: newUser.role, | ||
| token, | ||
| message: | ||
| "Account created successfully. Please verify your email using the OTP sent to your email address.", |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Don’t report signup success before OTP delivery is attempted.
This creates the unverified user and returns 201 before the verification email is even attempted. Since sendVerificationOtpEmail currently returns early for missing placeholder config and only logs Brevo failures, the user can be left unable to verify or log in while the API still says signup succeeded. Treat OTP delivery as part of the signup/resend transaction, or roll back/surface a failure when delivery cannot start.
🤖 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/auth.controller.js` around lines 96 - 117, The signup
flow in auth.controller.js reports success before OTP delivery is even
attempted, so move the verification email step into the main create path around
User.create and sendVerificationOtpEmail instead of setImmediate. In the signup
handler, await or otherwise handle sendVerificationOtpEmail for the
newUser.email before returning the 201 response, and surface a failure or roll
back the created user when OTP delivery cannot be started. Keep the response in
the same signup/resend transaction so the success message only returns when
email delivery has at least been attempted.
Contributor
Author
|
Addressed all review comments:
Verification:
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
backend/src/controllers/auth.controller.js (3)
151-170: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUse generic failure responses in email verification.
verifyEmaildistinguishes nonexistent users, already-verified users, and expired/unverified accounts. A public endpoint can use those differences to enumerate registered emails and verification state; keep failures generic unless verification succeeds.🤖 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/auth.controller.js` around lines 151 - 170, In verifyEmail within auth.controller.js, replace the distinct failure messages for missing user, already verified, and expired/invalid OTP with a single generic response so the endpoint does not reveal account existence or verification state. Keep the success path unchanged, but make every rejection branch return the same generic error from the verifyEmail handler.
258-262: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winCheck the password before revealing verification status.
This branch runs before
bcrypt.compare, so any password submitted for an unverified email gets the 403 verification message. Move the unverified-account check after a successful password match, or return generic credentials errors until the password is proven.🤖 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/auth.controller.js` around lines 258 - 262, Move the unverified-account check in auth.controller.js so it runs only after a successful password match in the login flow. The current branch in the login handler reveals verification status before bcrypt.compare, so relocate the user.isVerified/isInit("isVerified") check to the post-password-validation path, or otherwise return a generic credentials error until the password is confirmed. Use the login logic around the bcrypt.compare call and the existing verification guard as the symbols to update.
137-143: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winValidate email/OTP types before normalizing.
verifyEmailandresendVerificationOtpcallemail.toLowerCase()after only truthiness checks, so object/array inputs can turn a bad request into a 500. Mirror the signup/login string guards, and also requireotpto be a string before comparison.Suggested guard shape
- if (!email || !otp) { + if (typeof email !== "string" || typeof otp !== "string" || !email || !otp) { return res.status(400).json({ message: "Email and OTP are required", }); }- if (!email) { + if (typeof email !== "string" || !email) { return res.status(400).json({ message: "Email is required", }); }Also applies to: 894-901
🤖 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/auth.controller.js` around lines 137 - 143, The email/OTP handling in verifyEmail and resendVerificationOtp only checks truthiness before calling normalizedEmail logic, so non-string inputs can still reach toLowerCase and cause a server error. Add explicit type guards in the auth.controller.js handlers (the verifyEmail and resendVerificationOtp flow) to require email to be a string before normalization and otp to be a string before any comparison, matching the existing signup/login validation style, and keep returning a 400 for invalid input.
♻️ Duplicate comments (1)
backend/src/controllers/auth.controller.js (1)
108-123: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftKeep OTP persistence and delivery in the same failure boundary.
Both signup and resend still mutate account/OTP state before scheduling
sendVerificationOtpEmailinsetImmediate. SincesendVerificationOtpEmailcan return early or only log Brevo failures, these paths can report success while no verification email can be sent, leaving users blocked from login.Also applies to: 927-944
🤖 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/auth.controller.js` around lines 108 - 123, The signup/resend flow in auth.controller currently persists the OTP and then schedules sendVerificationOtpEmail inside setImmediate, so failures can be swallowed and the API still returns success. Move the OTP save and email send into the same awaitable try/catch boundary in the signup and resend handlers, and only return the success response after sendVerificationOtpEmail completes; if it fails, roll back or invalidate the OTP state and return an error instead of logging only. Use the existing sendVerificationOtpEmail path and the signup/resend controller methods to keep both state mutation and delivery tied to the same failure handling.
🤖 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/auth.controller.js`:
- Around line 151-170: In verifyEmail within auth.controller.js, replace the
distinct failure messages for missing user, already verified, and
expired/invalid OTP with a single generic response so the endpoint does not
reveal account existence or verification state. Keep the success path unchanged,
but make every rejection branch return the same generic error from the
verifyEmail handler.
- Around line 258-262: Move the unverified-account check in auth.controller.js
so it runs only after a successful password match in the login flow. The current
branch in the login handler reveals verification status before bcrypt.compare,
so relocate the user.isVerified/isInit("isVerified") check to the
post-password-validation path, or otherwise return a generic credentials error
until the password is confirmed. Use the login logic around the bcrypt.compare
call and the existing verification guard as the symbols to update.
- Around line 137-143: The email/OTP handling in verifyEmail and
resendVerificationOtp only checks truthiness before calling normalizedEmail
logic, so non-string inputs can still reach toLowerCase and cause a server
error. Add explicit type guards in the auth.controller.js handlers (the
verifyEmail and resendVerificationOtp flow) to require email to be a string
before normalization and otp to be a string before any comparison, matching the
existing signup/login validation style, and keep returning a 400 for invalid
input.
---
Duplicate comments:
In `@backend/src/controllers/auth.controller.js`:
- Around line 108-123: The signup/resend flow in auth.controller currently
persists the OTP and then schedules sendVerificationOtpEmail inside
setImmediate, so failures can be swallowed and the API still returns success.
Move the OTP save and email send into the same awaitable try/catch boundary in
the signup and resend handlers, and only return the success response after
sendVerificationOtpEmail completes; if it fails, roll back or invalidate the OTP
state and return an error instead of logging only. Use the existing
sendVerificationOtpEmail path and the signup/resend controller methods to keep
both state mutation and delivery tied to the same failure handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8b14ff2d-e240-4687-bf2f-1adff4bd4b3a
📒 Files selected for processing (7)
backend/src/controllers/auth.controller.jsbackend/src/routes/auth.route.jsbackend/test/auth/login.test.jsbackend/test/auth/otp.test.jsbackend/test/auth/signup.test.jsbackend/test/integration/integration.test.jsbackend/test/security/security.test.js
🚧 Files skipped from review as they are similar to previous changes (4)
- backend/src/routes/auth.route.js
- backend/test/integration/integration.test.js
- backend/test/auth/otp.test.js
- backend/test/auth/signup.test.js
Contributor
Author
|
Hi @Akash504-ai , The requested updates have been completed and all tests are passing. Could you please take a look at the PR and let me know if any further changes are needed? Thanks! |
Akash504-ai
reviewed
Jun 25, 2026
Akash504-ai
left a comment
Akash504-ai
left a comment
Member
There was a problem hiding this comment.
Thanks for the PR @nihar-ux18. Could you please share a short demo video/screen recording of the OTP verification flow before I begin the review?
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
frontend/src/App.jsx (1)
15-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unused
Ribbonimport.Line 15 adds
Ribbon, but the provided file only usesLoaderon Line 53. This keeps the frontend lint signal noisy.🤖 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/App.jsx` at line 15, The App component has an unused Ribbon import, which should be removed to keep the frontend lint output clean. Update the import in App so it only brings in the symbol actually used by the component, and verify the remaining Loader usage still resolves correctly.Source: Linters/SAST tools
🤖 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 `@frontend/src/pages/SignUpPage.jsx`:
- Around line 50-56: The email used for verification is only passed through
navigation state, so add a sessionStorage fallback in the signup flow and use it
in the verification page. In SignUpPage.handleSubmit, after a successful signup,
persist result.email to sessionStorage before calling navigate to
VerifyEmailPage; then in VerifyEmailPage, read the email from location.state
first and fall back to sessionStorage when state is missing, avoiding the
redirect loop on refresh or direct access. Make sure the VerifyEmailPage logic
clears or reuses the stored email appropriately once it is no longer needed.
In `@frontend/src/pages/VerifyEmailPage.jsx`:
- Around line 17-21: Guard OTP submission and resend in VerifyEmailPage by
ensuring the OTP input only accepts six digits, rejecting non-digit characters
before calling verifyEmailOtp, and disabling both verify and resend actions
while either request is pending so overlapping requests can’t race. Update the
handlers and UI state in VerifyEmailPage (including the verification flow around
verifyEmailOtp and the resend action used in the same component) so incomplete
codes are handled explicitly instead of being silently ignored.
In `@frontend/src/store/useAuthStore.js`:
- Line 182: The email verification success toast in useAuthStore’s verification
flow can receive an undefined message when the response omits it, unlike the
resend path. Update the success handling around toast.success(res.data.message)
to use a safe fallback string when res.data.message is missing, keeping the
behavior consistent with the resend verification logic in the same store.
---
Nitpick comments:
In `@frontend/src/App.jsx`:
- Line 15: The App component has an unused Ribbon import, which should be
removed to keep the frontend lint output clean. Update the import in App so it
only brings in the symbol actually used by the component, and verify the
remaining Loader usage still resolves correctly.
🪄 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 Plus
Run ID: a94562f6-6e19-4f08-8994-f4caa8b1f8a0
📒 Files selected for processing (6)
backend/src/controllers/auth.controller.jsbackend/test/auth/otp.test.jsfrontend/src/App.jsxfrontend/src/pages/SignUpPage.jsxfrontend/src/pages/VerifyEmailPage.jsxfrontend/src/store/useAuthStore.js
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/test/auth/otp.test.js
- backend/src/controllers/auth.controller.js
Contributor
Author
paso-otp-verification.mp4@Akash504-ai, I've attached a screen recording demonstrating the complete email verification flow, including signup, OTP verification, and successful login after verification. |
Akash504-ai
requested changes
Jun 26, 2026
Akash504-ai
left a comment
Akash504-ai
left a comment
Member
There was a problem hiding this comment.
After successful OTP verification, can we redirect the user directly to the home/chat page instead of the Sign In page? Since the email is already verified, an auto-login flow might provide a smoother user experience.
Contributor
Author
|
@Akash504-ai,I have implemented the requested change. After successful OTP verification, the user is now automatically logged in and redirected to the home/chat page instead of the Sign In page. This provides a smoother onboarding experience. I've also attached an updated screen recording demonstrating the complete flow. paso_otp_email_verification.mov |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@frontend/src/pages/VerifyEmailPage.jsx`:
- Around line 174-216: Add an accessible name to each OTP input in
VerifyEmailPage’s OTP map so screen readers can distinguish them; update the
individual input elements in the otp.map block to include a per-digit label such
as an aria-label or equivalent accessible text, or wrap them in a labeled group.
Keep the existing handleChange and handleKeyDown behavior unchanged while
ensuring each field is announced as a specific digit out of six.
- Around line 93-97: The success branch in VerifyEmailPage’s email verification
flow is redirecting to the unauthenticated login page even though
verifyEmailOtp() already creates an authenticated session by storing the token,
hydrating authUser, and opening the socket. Update the navigate call in the
success path to send the user to the authenticated landing page instead, or
change verifyEmailOtp() so it does not establish a session if manual login
should remain required.
🪄 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 Plus
Run ID: 8fd7c1a2-a2dd-48df-9569-166032720a43
📒 Files selected for processing (1)
frontend/src/pages/VerifyEmailPage.jsx
Member
|
Akash504-ai
approved these changes
Jun 26, 2026
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
2 participants
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.
Summary
Implemented email verification using OTP.
Changes
Testing
Summary by CodeRabbit
New Features
Bug Fixes
Changes
Tests