fix(auth): reset by stored email, trim lookups, preserve anti-enumeration#4185
Open
gorkem-bwl wants to merge 2 commits into
Open
fix(auth): reset by stored email, trim lookups, preserve anti-enumeration#4185gorkem-bwl wants to merge 2 commits into
gorkem-bwl wants to merge 2 commits into
Conversation
Two fixes for a reported case where a user could neither log in nor receive a password-reset email, while the reset screen still reported success. ## Email normalization (backend) UserModel.createNewUser and updateCurrentUser validated the raw email and only trimmed it afterward (create did not trim at all). Because the email validator and the mail layer's address validation are anchored and reject stray whitespace, an email stored with leading/trailing whitespace would pass through creation yet later fail the strict validation in the password-reset send path, which threw and aborted the send. Both methods now trim before validating, so a whitespace-padded address is accepted and stored clean. Login and reset lookups are already case-insensitive. ## Surfaced reset failures (frontend) The Forgot password handler fired the reset request without awaiting it and navigated to the reset screen immediately, so a real failure (network, 500, validation) could never be caught and the user always saw apparent success. The handler now awaits the request, navigates only on success, and trims the email client-side. The backend still returns success for unknown accounts (anti-enumeration), so legitimate sends route the user forward as before. ## Tests - user.model.spec: trims surrounding whitespace from the email on create. - ForgotPassword: navigates only after the request resolves (with the trimmed email) and does not navigate when the request fails.
…tion Addresses code-review findings on the email-normalization branch. The original model-layer trim fixed future inserts but missed the actual root cause and introduced an enumeration regression. ## Reset targets the stored email (root cause) resetPassword resolved the row case-insensitively but then passed the raw request email into resetPasswordQuery, whose UPDATE matches email exactly. When the stored value differed in case/whitespace from what the user typed, the UPDATE matched zero rows and the password silently stayed unchanged. It now passes the canonical _user.email, so reset works for every existing row regardless of how it was created. ## Lookups tolerate stored whitespace getUserByEmailQuery now compares LOWER(TRIM(...)) on both sides. Existing rows with stray whitespace are found by login, the reset send path, and the duplicate-user guard, without a data migration. This also closes a duplicate account path where a whitespace-padded email skipped the pre-insert check. ## Forgot-password preserves anti-enumeration Awaiting the request and navigating only on success re-exposed a found-vs-not- found oracle (200 navigate for unknown accounts vs 500 error when an account exists and SMTP fails). The handler now navigates to the confirmation screen regardless of outcome, keeps the entered email, and guards against double-submit. ## Tests - user.ctrl: resetPassword updates by the stored email, not the raw request email. - ForgotPassword: navigates to the confirmation screen even when the request fails.
MuhammadKhalilzadeh
approved these changes
Jun 26, 2026
MuhammadKhalilzadeh
left a comment
MuhammadKhalilzadeh
left a comment
Collaborator
There was a problem hiding this comment.
Looks mostly good to me @gorkem-bwl 👍🏻
What do you think @HarshP4585 ?
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
3 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
Fixes a case where a user could neither log in nor receive a password-reset email, while the reset screen still reported success.
Root cause
resetPasswordresolved the user row case-insensitively but then passed the raw request email intoresetPasswordQuery, whoseUPDATEmatchesemailexactly. When the stored value differed in case or whitespace from what the user typed, theUPDATEmatched zero rows and the password silently stayed unchanged. The reset request still returned success, so the user saw a confirmation but could never actually log in.Changes
resetPasswordnow passes the canonical_user.emailinto the update instead of the raw request email, so reset works for every existing row regardless of how it was created.getUserByEmailQuerynow comparesLOWER(TRIM(...))on both sides, so existing rows with stray whitespace are found by login, the reset send path, and the duplicate-user guard, with no data migration. This also closes a path where a whitespace-padded email skipped the pre-insert duplicate check.createNewUserandupdateCurrentUsertrim the email before validating and storing, so future rows are canonical.Tests
user.ctrl: reset updates by the stored email, not the raw request email.user.model.spec: surrounding whitespace is trimmed from the email on create.ForgotPassword: trims the email before sending, and navigates to the confirmation screen even when the request fails.