Conversation
dschom
marked this pull request as draft
July 14, 2026 20:58
dschom
force-pushed
the
FXA-14109
branch
2 times, most recently
from
August 3, 2026 18:40
563e854 to
2c74911
Compare
dschom
force-pushed
the
FXA-14109
branch
2 times, most recently
from
August 3, 2026 23:50
b53ef55 to
a0ef925
Compare
… OTP Because: - Dormant accounts signing in through Settings received two identical OTP verification emails. The auth-server sends one from `sendSigninNotifications` during `/account/login`, and `handleNavigation` sends a second via `sessionResendVerifyCode` as it routes to the code screen. - The decision was made in #20601 to move the code sending logic to front end. In theory this stopped the code from being sent when a user was never prompted for one. In practice we had this 'dormant' account state that wasn't accounted for, and had no test coverage. As a result, the change wasn't full proof. This commit: - Adds an optional `sendSigninVerificationEmail` payload flag to `/account/login` and `/session/reauth`. **It defaults to `true` when omitted**, so the safe default is the existing behavior — a client (ie front end) has to opt out deliberately. This makes the API level change non-breaking and backwards compatible. - For this `sendSigninVerificationEmail` flag, FxA will now provide `false` — the fxa-settings password and unblock sign-in paths, which reach `handleNavigation` and send the code themselves. `handleNavigation` is restored to sending unconditionally and is once again the sole sender on `/signin_token_code` for our web UI. Closes #FXA-14109
dschom
marked this pull request as ready for review
August 4, 2026 17:46
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.
Because:
Dormant accounts signing in through Settings received two identical OTP verification emails. The auth-server sends one from
sendSigninNotificationsduring/account/login, andhandleNavigationsends a second viasessionResendVerifyCodeas it routes to the code screen.The decision was made in fix(settings,auth): Fix sumo login issue #20601 to move the code sending logic to front end. In theory this stopped the code from being sent when a user was never prompted for one. In practice we had this 'dormant' account state that wasn't accounted for, and had no test coverage. As a result, the change wasn't full proof.
This PR:
Adds an optional
sendSigninVerificationEmailpayload flag to/account/loginand/session/reauth. It defaults totruewhen omitted, so the safe default is the existing behavior — a client (ie front end) has to opt out deliberately. This makes the API level change non-breaking and backwards compatible.For this
sendSigninVerificationEmailflag, FxA will now providefalse— the fxa-settings password and unblock sign-in paths, which reachhandleNavigationand send the code themselves.handleNavigationis restored to sending unconditionally and is once again the sole sender on/signin_token_codefor our web UI.Issue that this pull request solves
Closes: FXA-14109, FXA-12972
Checklist
Put an
xin the boxes that applyHow to review (Optional)
To test manually, do the following comparison:
yarn firefoxto spin up a Firefox instance where sync is pointed at local host.Now checkout this branch and repeat the steps. You'll see only one email is sent.
lib/routes/utils/signin.js(sendVerifySessionEmail, where the gate sits),pages/Signin/utils.ts(handleNavigation).Manual testing is a good idea for this PR, because automated testing is not possible at the time of writing due to the requirement that we have a 'dormant' account. The back to back comparison described above is the best way to ensure this is actually working.
Screenshots (Optional)
N/A — no user interface change.
Other information (Optional)
Claude Called this out specifically as the risky part! "A flow where the server would have sent but our front-end does not land on a code screen would leave the user with no email. Each server-send branch (
wantsKeys, no service, service inservicesWithEmailVerification,passwordChangeRequired) maps to ahandleNavigationbranch that routes to a code screen.service=vpnis the one to watch, since it is inservicesWithEmailVerification."As far as I can tell, this not an issue. Legacy clients like the VPN app itself will fallback to the old behavior, because of the nature of the
sendSigninVerificationEmailflag. When it is not defined, we default to true state, which ensures the original behavior is preserved. And all web flows that direct users to the/signin_token_codepage will now trigger an email send from the front end as this navigation decision is made.