feat(auth): let an unverified account get a new OTP - #78
Conversation
users.email had no unique index, so the findOne check in registerController was the only thing between two concurrent registrations and two accounts on the same address. It is a read followed by a write, so both requests win. Adds the constraint and everything it needs around it: - email is unique on the schema, and registerController translates the resulting E11000 into the same "User already exists" response the pre-check returns, so the race does not just turn into a 500. - accountIdentity.js normalises addresses in one place. Registration stored them lowercased while every lookup afterwards used the raw request value, so signing in as User@Example.com missed an account created as user@example.com. Login, verify-otp, forgot-password and reset-password now all resolve through the same filter. - ensureIndexes() builds declared indexes at startup instead of leaving Mongoose's background builder to fail silently, and names the fix in the error when duplicates are blocking the build. - scripts/dedupeUserEmails.js merges pre-existing duplicates: keeps the verified/oldest row, re-points enrolments, payments, reviews, bookmarks, logs and authored courses at it, and drops rows that would violate a compound unique key. Supports --dry-run. Also indexes coursePayments by userId/courseId/createdAt and courses by userId/createdAt/enrolled; both were collection scans on every request. Closes udaycodespace#72
The verification code expires after ten minutes and nothing issued a second one. Registering again answered "User already exists", logging in answered "Email is not verified", and there was no resend route, so a missed code burned the address until a maintainer deleted the row by hand. Backend: - POST /api/user/resend-otp, with a one-minute-per-address cooldown derived from a new otpLastSentAt field rather than from anything the client sends. An unknown address, an already-verified account and a successful send all return the same body, so the route is not an enumeration oracle. - registerController stops collapsing two different cases. A verified account still answers "User already exists"; an unverified row is a registration nobody completed, so the details are taken from the new attempt and a fresh code goes out. - otpCodes.js holds the length, lifetime, cooldown and mail copy so registration and resend cannot drift apart. generateOtp moves off Math.random, which is not a CSPRNG and is the only thing guarding the account. - verifyOtpController clears otpLastSentAt and reports canResend so the UI can tell whether the button will do anything. Frontend: - VerifyEmailPanel is shared by Register and Login and keeps the pending address in sessionStorage. Refreshing the verify step used to discard both the step and the address, leaving a valid code in the inbox with nothing to enter it against. - Login handles notVerified, which the API has always sent and nothing read. - Register swaps its alert() calls for the existing Toast, disables submit while in flight, and stops labelling its password field current-password. Closes udaycodespace#73
d44faef to
4b43535
Compare
|
Heads up on merge order: this branch and #77 both rewrite The resolution keeps both fixes rather than picking one:
So this PR currently shows #77's commits in its diff. Merge #77 first and this one collapses to just its own changes. If you would rather they stayed independent, say so and I will unstack it — it will just mean resolving the same conflict at merge time. Verified: |
Three conflicts, all unions: - userControllers.js require block: this branch carried a require for buildPaymentSummary, formatPaymentMessage and isFreeCourse, which main has since moved into enrollmentController (udaycodespace#62) and removed from here. Nothing in this file uses them any more, so only the accountIdentity require is kept. - Register.jsx imports: main added ROLES/roleLabel (udaycodespace#84), this branch added Toast and VerifyEmailPanel. Both. - Register.jsx handleSelect: main rewrote it so the stored role and the label on the toggle stop being the same string; this branch added the toast helpers and the pending-email effect immediately above it. Main's implementation is kept, with this branch's additions in front of it.
Overlap with #100, worth deciding before either landsBoth merge cleanly into
Whichever lands second has to take on the other's shape. Concretely:
I am happy to rebase whichever one you prefer to take second — just say which order you want and I will push the resolution. My suggestion is #100 first, then this one: adapting this resend handler to |
Four conflicts, all from udaycodespace#63's credential throttling and udaycodespace#72's email normalisation landing on main while this branch reworked the same two files. registerController: this branch's version supersedes main's on both hunks. Its verified/unverified split replaces the unconditional "User already exists", and issueVerificationOtp saves the document and mails the code in one helper — it already carries main's isDuplicateOn catch, in the right place, because the save now happens inside it. userRoutes: main's rate limiter and failure throttle are kept on verify-otp, forgot-password and reset-password, and /resend-otp is given the same treatment. It is rate limited because it sends mail, and has no failure throttle for the reason /forgot-password has none — it answers the same way for known and unknown addresses, so there is no failure to count. The per-address cooldown stays in the controller so it applies however the code is requested.
|
@MOHITKOURAV01 Everything looks good from my side. Merge conflicts are resolved, and the changes are aligned with the intended OTP resend flow. The updated auth handling and edge cases look good as well. Approved to merge. |
Summary
The verification code expires after 10 minutes and nothing anywhere issued a
second one. Registering again answered
"User already exists", logging inanswered
"Email is not verified", and there was no resend route — so a missedcode burned the address until a maintainer deleted the row by hand.
The UI made it easy to land there.
Register.jsxheld the verify step inuseState, so a refresh — or reading the code in another tab and coming back —threw away both
showOtpInputanddata.email. The code sat in the inbox withnothing left that knew which address it belonged to.
Related Issue
Closes #73
What changed
Backend
POST /api/user/resend-otp, with a one-minute-per-address cooldown derivedfrom a new
otpLastSentAtfield on the user rather than from anything theclient sends. An unknown address, an already-verified account and a
successful send all return an identical body — a resend route takes an
address from an anonymous caller, and one that says "no such account" is an
enumeration oracle.
registerControllerstops collapsing two different cases. A verifiedaccount still answers
"User already exists". An unverified row is aregistration nobody completed, so nobody owns the address yet: the details
are taken from the new attempt (the user may be retrying because they
mistyped something) and a fresh code goes out.
utils/otpCodes.jsholds the length, lifetime, cooldown and mail copy in oneplace so registration and resend cannot drift apart.
generateOtpmoves offMath.floor(100000 + Math.random() * 900000)—Math.randomis not a CSPRNGand this value is the only thing guarding the account. Same range, so the
code is still exactly six digits.
verifyOtpControllerclearsotpLastSentAt, trims the submitted code, andreturns
canResendso the UI knows whether offering the button will doanything or only answer 429.
Frontend
VerifyEmailPanelis shared by Register and Login and keeps the pendingaddress in
sessionStorage, which is what makes a refresh survivable. Thevalue is an address the user just typed into a form on the same page.
notVerified: true— the API has always sent it and nothingread it — and offers the verify step instead of a dead end.
alert()calls for theToastReplace native browser alerts with a Toast/notification component in Login #36 introduced,disables submit while in flight, checks the 6-character password rule before
sending, and stops labelling its password field
current-password.retryAfterSecondsrather than guessing.Type
Areas touched
Testing
npm testinbackend/: 143 passing (128 before, 15 added).npm run buildin
frontend/passes.Test steps
POST /api/user/resend-otpwith that address → 200 and a new code.retryAfterSeconds.is sent.
with the address intact.
of just an error message.
Screenshots
Edge cases checked
Other edge case details
secondsUntilResendtreats a timestamp in the future — clock skew between appservers — as one cooldown rather than a lockout until that time passes. A
missing address is a 400 before any lookup happens. A database failure is a 500
that does not put the driver's message in the response body.
sessionStoragebeing refused (private browsing) degrades to the previous in-memory behaviour
rather than throwing.
Checklist
CONTRIBUTING.mdNotes
This is the missing recovery path, not throttling — #63 covers rate limiting on
the auth endpoints generally. The cooldown here exists because a resend route
without one is a free mail relay pointed at any address an attacker picks, and
it belongs on the route regardless of what #63 lands.
The re-registration behaviour is the part worth a second opinion: taking the
name/password/type from the new attempt is deliberate, on the grounds that
nobody has proven ownership of that address yet, so there is nothing to
protect. Happy to make it keep the original details if you would rather.
docs/issue-73-otp-resend.mdhas the full reasoning.