fix(auth): make one email mean one account - #77
Merged
udaycodespace merged 2 commits intoAug 24, 2026
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
This was referenced Aug 16, 2026
udaycodespace
self-requested a review
August 17, 2026 09:46
One conflict, in the require block of userControllers.js. 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 the resolution keeps only this branch's own accountIdentity require rather than reinstating the three.
Owner
|
@MOHITKOURAV01 Looks good. The unique email handling, normalization, duplicate cleanup, and index changes are properly addressed. |
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
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
users.emailhad no unique index, so thefindOnecheck at the top ofregisterControllerwas the only thing between two concurrent registrationsand two accounts on the same address. It is a read followed by a write with
nothing in between, so both requests win.
Once two rows share an address,
loginControllerauthenticates againstwhichever one the storage engine returns first, and a reset token written onto
one row can be read from the other.
While adding the constraint I found the quieter half of the same problem:
registration stores addresses lowercased (
validateRegistrationnormalises,and the schema has
lowercase: true) but every lookup afterwards used the rawrequest value. Signing in as
User@Example.comdid not find the accountcreated as
user@example.com.Related Issue
Closes #72
What changed
emailisunique: true, andregisterControllertranslates the resultingE11000into the same"User already exists"response the pre-check gives.Without that the race just turns from a duplicate row into an opaque 500.
utils/accountIdentity.jsowns normalisation and duplicate-key reading.buildEmailFilterdeliberately never returnsnull—findOne(null)isfindOne({})in Mongoose and would hand back an arbitrary account — so anunusable value yields
{ email: "" }, which cannot match a required field./verify-otp,/forgot-passwordand/reset-passwordall resolvethrough that one filter.
config/ensureIndexes.jsbuilds declared indexes at startup. Mongoose buildsthem in the background and emits failures on the model; nothing was
listening, so on a database with duplicates the server would come up looking
healthy with the constraint silently absent. It now reports the failure once,
and names the command that fixes it.
scripts/dedupeUserEmails.js(npm run db:dedupe-emails) mergespre-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.
-- --dry-runreports withoutwriting.
coursePayments(userId,courseId,createdAt) andcourses(userId,createdAt,enrolled). Both were collection scans.docs/issue-72-unique-email.mdhas the reasoning and the order of operationsfor an existing deployment.
Type
Areas touched
Testing
npm testinbackend/: 143 passing (128 before, 15 added).Test steps
cd backend && npm test"User already exists".user@example.com, verify, then sign in asUser@Example.com.Before:
"User not found". After: signed in.npm run db:dedupe-emails -- --dry-run, then without the flag, then restart.Screenshots
Edge cases checked
Other edge case details
A non-string
email(an object posted in place of a string) used to reachfindOneand could match nothing predictable; it now produces an unmatchablefilter.
duplicateKeyFieldshandles all three shapes the driver uses —keyPattern,keyValue, and older versions that only name the index in themessage.
Checklist
CONTRIBUTING.mdNotes
Deploy order matters. On a database that already holds duplicates the index
build will fail until
db:dedupe-emailshas run. That is not silent — startupprints the collision and the command — but the constraint will not exist until
the duplicates are gone.
The dedupe keeps the oldest row when two are equally verified, because that
is the id enrolments and payments already reference.
courseModel.userIdis aStringwhile every other reference is anObjectId, so authored courses gettheir own pass in the script; matching on the ObjectId there silently finds
nothing.