Skip to content

bug(auth-server): Let FxA's front-end turn off the server-sent signin OTP - #20865

Open
dschom wants to merge 1 commit into
mainfrom
FXA-14109
Open

bug(auth-server): Let FxA's front-end turn off the server-sent signin OTP#20865
dschom wants to merge 1 commit into
mainfrom
FXA-14109

Conversation

@dschom

@dschom dschom commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

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 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 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.

Issue that this pull request solves

Closes: FXA-14109, FXA-12972

Checklist

Put an x in the boxes that apply

  • My commit is GPG signed.
  • If applicable, I have modified or added tests which pass locally.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).
  • I have manually reviewed all AI generated code.

How to review (Optional)

To test manually, do the following comparison:

  1. Check out main
  2. Set your dev.json in auth server follows
image
  1. Go to http://localhost:30303, and create an account. Be sure to set a password!
  2. Run yarn firefox to spin up a Firefox instance where sync is pointed at local host.
  3. Wait at least 1 minute since you created that account in step 3.
  4. Try signing in. You'll see two emails. The trigger here is the fact the account was created more than 1 minute ago.

Now checkout this branch and repeat the steps. You'll see only one email is sent.

  • Key files: 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 in servicesWithEmailVerification, passwordChangeRequired) maps to a handleNavigation branch that routes to a code screen. service=vpn is the one to watch, since it is in servicesWithEmailVerification."

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 sendSigninVerificationEmail flag. 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_code page will now trigger an email send from the front end as this navigation decision is made.

@dschom
dschom requested a review from a team as a code owner July 14, 2026 20:58
@dschom
dschom marked this pull request as draft July 14, 2026 20:58
@dschom
dschom force-pushed the FXA-14109 branch 2 times, most recently from 563e854 to 2c74911 Compare August 3, 2026 18:40
@dschom dschom changed the title bug(settings): Stop duplicate sign-in verification emails for dormant accounts bug(auth-server): Let clients opt out of the server-sent signin OTP Aug 3, 2026
@dschom dschom changed the title bug(auth-server): Let clients opt out of the server-sent signin OTP bug(auth-server): Let FxA's front-end opt out of the server-sent signin OTP Aug 3, 2026
@dschom dschom changed the title bug(auth-server): Let FxA's front-end opt out of the server-sent signin OTP bug(auth-server): Let FxA's front-end turn off the server-sent signin OTP Aug 3, 2026
@dschom
dschom force-pushed the FXA-14109 branch 2 times, most recently from b53ef55 to a0ef925 Compare August 3, 2026 23:50
… 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
dschom marked this pull request as ready for review August 4, 2026 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant