fix(auth): hash one-time codes and stop confirming which emails exist

[MOHITKOURAV01]

Summary

Every pending verification and password-reset code was stored in the users collection as the six digits the endpoint accepts, and /verify-otp and /reset-password answered differently for an unknown address than for a wrong code. This hashes the credentials, makes every failure indistinguishable, and bounds guessing per account.

Related Issue

Closes #95

What changed?

backend/utils/otpCredentials.js (new)

function	does
generateCode()	six digits from crypto.randomInt, not Math.random()
issueCredential()	{ code, hash, expiresAt, attempts: 0 } — the plaintext is returned once for the email and never persisted
verifyCredential(stored, candidate)	{ status, attempts, shouldClear }
isWellFormedCode()	shape check before any comparison
burnComparison()	one bcrypt comparison against a decoy hash
isFailure(status)	everything except OK — fails closed for a status added later

verifyCredential returns OK, INVALID, EXPIRED, LOCKED or MISSING. The callers answer all four failures identically; the distinction exists for the code path, never for the client.

shouldClear is the invalidation rule in one place: a credential dies when it is used, when it expires, and when it reaches MAX_ATTEMPTS. That last one matters — a locked-but-live code is a code somebody can keep working on. A missing, null, NaN or non-numeric expiresAt counts as expired rather than valid.

burnComparison is the part that is easy to omit. Uniform responses are only uniform if they take the same time; without a decoy comparison on the "no such account" path, the absence of a bcrypt call becomes the new oracle. The decoy hash is computed once at module load.

Schema — otp and resetToken hold a bcrypt hash. otpAttempts and resetTokenAttempts are new, select: false, and deliberately carry no default: an absent counter means no pending code, and $unset has to mean gone. Both are added to toJSON and to adminController.SENSITIVE_USER_FIELDS, and the admin reset-password $unset clears the counter with the token.

Controllers

• register stores credential.hash; the plaintext reaches the mailbox and nothing else.
• verify-otp answers 400 "Invalid or expired OTP" for an unknown address, a wrong code, an expired code, a locked account and a missing code — same status, same body, same work. A verified account has any leftover code cleared. On success, isVerified and the $unset are one atomic update.
• forgot-password stores the hash and resets the attempt counter, so an account that hit the limit can recover by asking for a new code. Its uniform response is unchanged.
• reset-password answers 400 for an unknown address where it used to answer 404 "User not found", and on success clears the reset credential and any pending verification code. It still sets isVerified — that was already the behaviour and is now stated in a comment: holding the code proves control of the mailbox.
• The 500 path no longer echoes error.message to the client.

Type

• Bug fix
• New feature
• Refactor
• Docs only
• Tests
• Config / workflow
• Security
• Breaking change

Areas touched

• Frontend
• Backend
• Database
• Docs
• Workflow / GitHub Actions
• Config / environment

Migration: none needed. A plaintext code left over from before this ships simply fails bcrypt.compare, the user is told the code is invalid — which it now is — and requests a new one. This fails closed, which is the right direction for a credential change.

Testing

• Tested locally
• Build passes
• Lint passes
• Tests added or updated
• Docs only, no runtime testing needed

npm test in backend/ — 257 passing, up from 234 on main. 23 new tests, including integration tests against mongodb-memory-server.

npm run lint is a frontend-only script and this PR touches no frontend files; it has never passed on main (69 problems, almost all pre-existing no-unused-vars on React imports).

Test steps

1. Register a@example.com.
2. db.users.findOne({email:"a@example.com"}, {otp:1}) → a $2a$10$… hash. On main this is the six digits that were just emailed.
3. curl -s -o /dev/null -w '%{http_code}\n' -X POST localhost:5000/api/user/verify-otp -H 'Content-Type: application/json' -d '{"email":"nobody@example.com","otp":"000000"}' → 400. On main, 404.
4. The same call for a@example.com → 400, with an identical body. The oracle is gone.
5. Send five wrong codes, then the correct one → still refused, and otp is gone from the document.
6. POST /api/user/forgot-password → a fresh hash and resetTokenAttempts back to 0.

Screenshots

• Not needed
• Added below

Edge cases checked

• Empty or missing data
• Loading / slow response
• API failure / server error
• Rate limit / throttling
• Invalid or unexpected input
• Permission / access denied
• Partial or inconsistent data
• Mobile / small screen behavior
• Other

Other edge case details

• A malformed candidate — wrong length, letters, a number, null, an object — costs an attempt and never matches.
• A missing, null, NaN or non-numeric expiry is treated as expired, never as valid.
• A credential already at the limit is refused even for the correct code.
• A code left behind on an account that is already verified is cleared rather than left live.
• Replaying a spent reset code leaves the first new password in place.
• An account locked out of a reset can recover by requesting a new code.

Checklist

• Read CONTRIBUTING.md
• Linked the issue
• Assigned before starting or approved by maintainer
• Changes are focused on one issue
• No debug logs or unused code
• Documentation updated if needed
• No new warnings or console errors
• Changes are meaningful, not trivial

Notes

docs/issue-95-otp-credential-storage.md has the full write-up, including why the decoy comparison is load-bearing.

This is complementary to #63, not a substitute for it. MAX_ATTEMPTS bounds guesses against one account regardless of how many source addresses they come from; the limiter in #63 bounds one source regardless of how many accounts it targets. Both are wanted.

[MOHITKOURAV01]

Overlap with #78, worth deciding before either lands

Both merge cleanly into main as they stand, but they conflict with each other on backend/controllers/userControllers.js and backend/schemas/userModel.js, and the overlap is substantive rather than textual:

• feat(auth): let an unverified account get a new OTP #78 adds a resend endpoint that issues a new OTP for an unverified account.
• this PR makes every OTP a bcrypt hash with an expiry, an attempt counter and a single-use rule.

Whichever lands second has to take on the other's shape. Concretely:

• If this PR lands first, feat(auth): let an unverified account get a new OTP #78's resend path must call issueCredential() and store credential.hash, and it should reset otpAttempts to 0 the way forgot-password does — otherwise an account that hit the attempt limit can never recover, which is the exact failure mode feat(auth): let an unverified account get a new OTP #78 exists to fix.
• If feat(auth): let an unverified account get a new OTP #78 lands first, this PR must additionally convert the resend path, or it leaves one endpoint still writing a plaintext code and the guarantee is only partial.

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 this PR first: #78's resend is a small handler and adapting it to issueCredential() is a few lines, whereas adapting this PR to a resend endpoint that does not exist yet is not possible to review properly.