test(auth): add regex to force unverified sessions, fix stray code emails by LZoog · Pull Request #21235 · mozilla/fxa

LZoog · 2026-09-16T16:00:25Z

Because:

The heuristic (non-Sync non-2FA) unverified session is only reachable past skipForNewAccounts.maxAge, so no automated test could produce it
SIGNIN_CONFIRMATION_FORCE_EMAIL_REGEX has set mustVerify since Feb 2026, which blocks OAuth grants, so it no longer simulates that state
On RP pass-through the front end emailed a code no one was asked for

This commit:

Adds forcedHeuristicEmailAddresses, defaulting to the unverifiedsession test-account prefix so no per-environment value is needed
Renames forcedEmailAddresses to forcedSyncEmailAddresses, env var unchanged, and reworks the signinConfirmation docs, including the evaluation order
Gates the front-end code resend on actually showing a verification page
Adds a functional spec for the heuristic unverified session; the Payments Next case is local-only, the rest run on every target
Drops a dead skipForEmailAddresses mock

Partially closes FXA-13740
Closes FXA-12972
Closes FXA-14301

Local testing: this table shows tokenVerificationIds, meaning an unverified session. mustVerify: 1 is the scoped keys flow.

the 3rd row ending in 75 token ID was me on main, setting skipForNewAccounts=false and using an @mozilla.com email, which is what our docs say to do to get into this state. The 2nd row, is me trying with a @gmail.com email on main with skipForNewAccounts=false, which is the local verification noted here. The 1st row is me on this branch, with skipForNewAccounts=true (the default), and using unverifiedsession@restmail.com to test the new regex. They all result in this state:

The next PR that should close 13740:

I'm going to include the fix I have for 14408 in it, because the diff is small and I want to add a functional test for this.
Tweak the Payments Next stuff. Maybe the test I have here, should be moved to tests-payments-next where it's actually spun up, and then we reference that test for a client in the config array and 123done for one that isn't. Also according to Claude, there's two skips in signIn.spec.ts on L131/223 for payments next that shouldn't send PKCE params which causes a hidden 400.
Likely a few more missing tests for more coverage
Doc tweaks in ecosystem-platform

Edit: all of the above should be covered in #21266

I have added other notes mentioned in this PR to a comment in 13180 for further clean up.

LZoog · 2026-09-16T16:08:53Z

+      );
+      expect(login.sessionVerified).toBe(false);
+
+      // /recovery_email/status reports `verified` as the email status unless


I'm not sure if we actually want this to be the case but I'm going to add a note in FXA-13180 about it.

LZoog · 2026-09-16T16:09:24Z

+      ).toBe(1);
+    });
+
+    test('RP in servicesWithEmailVerification, Payments Next, lands on signin_token_code and sends exactly one verifyLoginCode', async ({


I have a note in the PR description about this and that I'll tweak this in the part II for this ticket.

LZoog · 2026-09-16T16:11:05Z

  signinConfirmation: {
-    forcedEmailAddresses: {
-      doc: 'Force sign-in confirmation for email addresses matching this regex for those that do not request scoped keys. Sets "mustVerify: 0" on created session tokens but creates an entry in unverifiedTokens, simulating a non-Sync non-2FA unverified session state',
+    forcedSyncEmailAddresses: {


I'm going to verify this by testing locally, but Claude says this was only accurate for 9 days, and the logic changed in #20071.

I'm going to add a note about that logic we added there also in FXA-13180 because we may not need it.

Okay so I verified this locally. There's a pm2 override here changing this to sync*@restmail.net, so the @moz default never matches, and if you use sync, it gets mustVerify: 1 since #20071.

The only thing that actually put it into the mustVerify: 0 non-2FA state, was skipForNewAccounts=false, so on main if you set that to false and use any email and sign out and sign in you'll be in this state.

For this PR, I'm keeping the logic there as-is, but I changed the return of 'email' to 'syncEmail' just to make it slightly more clear, and I've added forcedHeuristicEmailAddresses for the mustVerify: 0 case so we can test with functional tests. I've updated the docs here and added a comment in 13180 about changing that, because I believe we can remove what we added in that other PR and consolidate.

LZoog · 2026-09-16T16:11:57Z

+      wantsKeys;
+
+    // Case 5 above: the RP flow continues to the grant and the user is never
+    // asked for a code, so none is emailed. The server did not send one either


This is what seems to fix the SUMO issue. The playwright test for this fails on main but passes with this change.

LZoog · 2026-09-16T16:19:22Z

+    },
    skipForEmailRegex: {
-      doc: 'Regex pattern for email addresses that will always skip any non-TOTP sign-in confirmation.',
+      doc: 'Skip sign-in confirmation for matching emails even when scoped keys are requested. A customs suspect verdict, the forced regexes above, and TOTP still apply.',


Another note for the mustVerify clean up issue: I didn't want to specify "A customs suspect verdict" here, but that seems to still be separate from our other heuristics-based checks. "Heuristic-based" should simply mean non-2FA non-Sync for less confusion later (edit: or this naming should at least not be confusing, I added a note in the mustVerify simplification issue).

Copilot

🟡 Changes recommended

Address the missing PKCE parameters in the functional test and preserve verification-code resends for must-verify OAuth flows.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds configurable heuristic unverified-session handling, prevents stray OAuth verification emails, and expands test coverage.

Changes:

Separates Sync-style and heuristic forced-email configuration.
Updates verification navigation and resend behavior.
Adds unit, authentication, and functional test coverage.

File summaries

File	Summary
`packages/fxa-settings/src/pages/Signin/utils.ts`	Controls verification routing and OTP resends.
`packages/fxa-settings/src/pages/Signin/utils.test.ts`	Tests OAuth pass-through email behavior.
`packages/fxa-auth-server/lib/routes/account.ts`	Implements heuristic forced-session handling.
`packages/fxa-auth-server/lib/routes/account.spec.ts`	Tests forced session states.
`packages/fxa-auth-server/config/index.ts`	Defines and documents regex configuration.
`packages/functional-tests/tests/signin/unverifiedSession.spec.ts`	Adds end-to-end unverified-session coverage.
`packages/functional-tests/lib/testAccountTracker.ts`	Adds support for heuristic-session test accounts.

Review details

Suppressed comments (2)

packages/fxa-auth-server/config/index.ts:1838

This says disabling skipForNewAccounts puts any account in the heuristic state, but the earlier evaluation order shows that recognized device/IP, skipForEmailRegex, suspicious/global/forced confirmation, scoped keys, TOTP, and password-change paths can still take precedence. That wording can lead local test setup to assume the wrong session type; narrow it to accounts with none of those conditions.

        doc: 'Skip all sign-in email confirmations for newly-created accounts. Set false locally to put any account into the heuristic (non-Sync non-2FA) unverified session state.',

packages/fxa-auth-server/config/index.ts:1827

The PR's local-testing instructions use unverifiedsession@restmail.com, but this default only matches @restmail.net; that documented address would not enter the forced heuristic state. Please align the instructions with the .net domain used by TestAccountTracker/the local config, or change both the regex and test helper if .com is intended.

      default: /^unverifiedsession.*@restmail\.net$/,

Files reviewed: 7/7 changed files
Comments generated: 2
Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

LZoog · 2026-09-17T22:33:36Z

+        client_id: '32aaeb6f1c21316a',
+        redirect_uri: 'http://localhost:3035/api/auth/callback/fxa',
+        scope: 'https://identity.mozilla.com/account/subscriptions',
+        response_type: 'code',
+        state: 'fakestate',


Seems like a hallucination? I pulled this up, and do not see this marked as a public client in dev.json. I didn't include the params here on this test because of this and have noted in the follow up to remove these from the other tests.

LZoog · 2026-09-17T22:40:14Z

+    const skipsVerificationPage =
+      !requiresVerificationPage && isOAuthWebIntegration(integration);


Sigh this might be a good catch, investigating now

Yeah, so the can sign in to OAuth after abandoning sync confirmation code functional test we have here fails locally. I think I've got the fix now, verifying.

Force pushed. I made a couple of changes in utils to account for this and I went ahead and added another test (slightly different) in the new unverified session functional test for this.

…ails Because: * The heuristic (non-Sync non-2FA) unverified session is only reachable past skipForNewAccounts.maxAge, so no automated test could produce it * SIGNIN_CONFIRMATION_FORCE_EMAIL_REGEX has set mustVerify since Feb 2026, which blocks OAuth grants, so it no longer simulates that state * On RP pass-through the front end emailed a code no one was asked for This commit: * Adds forcedHeuristicEmailAddresses, defaulting to the unverifiedsession test-account prefix so no per-environment value is needed * Renames forcedEmailAddresses to forcedSyncEmailAddresses, env var unchanged, and reworks the signinConfirmation docs, including the evaluation order * Gates the front-end code resend on actually showing a verification page * Adds a functional spec for the heuristic unverified session; the Payments Next case is local-only, the rest run on every target * Drops a dead skipForEmailAddresses mock Partially closes FXA-13740 Closes FXA-12972 Closes FXA-14301

LZoog · 2026-09-18T22:26:05Z

+    it('creates an unverified session without mustVerify for forcedHeuristicEmailAddresses', () => {
+      const email = 'test@mozilla.com';
+      config.signinConfirmation.forcedSyncEmailAddresses = /^$/;
+      config.signinConfirmation.forcedHeuristicEmailAddresses =


Copliot called something out in my part 2 PR that I should fix here: #21266 (comment)

I'll do this with my next push on this branch.

nshirley

A few questions, but I guess I'm mostly curious why you went the route of functional tests for many of these when the logic can or already is tested with an integration or unit test? Not saying it's wrong! Just curious, trying to understand if we have a gap in confidence with our unit and integration tests that we're using functional as a band-aid for

nshirley · 2026-09-21T15:36:16Z

Nice work chasing this state down! One thing I'd like to understand; this PR adds unit tests in Signin/utils.test.ts and Authorization/container.test.tsx and then, for every branch these functional tests walk through the browser, duplicating the logic. What are we hoping the functional versions catch that the unit tests can't?

nshirley · 2026-09-21T15:37:56Z

+// the code is entered.
+test.describe('severity-1 #smoke', () => {
+  test.describe('heuristic unverified session', () => {
+    test('forced unverified session has mustVerify unset', async ({


It doesn't look like this test uses a browser, just authClient calls. Would it make more sense in the auth-server /test/remote tests?

nshirley · 2026-09-21T15:42:24Z

+      expect(status.verified).toBe(true);
+    });
+
+    test('Settings sign-in lands on signin_token_code and sends exactly one verifyLoginCode', async ({


On a Settings signin there's no service, so the server sends the code for both prefixes and the client routes to the code page for both. Is there a branch this reaches that the existing sync-prefix test in signIn.spec.ts doesn't? The mustVerify difference seems to only be observable on the RP path, which there are tests for already in here

LZoog commented Sep 16, 2026

View reviewed changes

LZoog force-pushed the FXA-13740 branch 2 times, most recently from 8d4300f to a40e3c1 Compare September 17, 2026 22:08

LZoog marked this pull request as ready for review September 17, 2026 22:14

LZoog requested a review from a team as a code owner September 17, 2026 22:14

Copilot AI balanced review requested due to automatic review settings September 17, 2026 22:14

Copilot started reviewing on behalf of LZoog September 17, 2026 22:15 View session

Copilot AI reviewed Sep 17, 2026

View reviewed changes

LZoog force-pushed the FXA-13740 branch from a40e3c1 to a942db0 Compare September 17, 2026 23:05

LZoog mentioned this pull request Sep 18, 2026

fix(oauth): refuse prompt=none, add session verification test coverage #21266

Open

LZoog commented Sep 18, 2026

View reviewed changes

nshirley reviewed Sep 21, 2026

View reviewed changes

		const skipsVerificationPage =
		!requiresVerificationPage && isOAuthWebIntegration(integration);

Conversation

LZoog commented Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The next PR that should close 13740:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LZoog Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LZoog Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (2)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LZoog Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nshirley left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

LZoog commented Sep 16, 2026 •

edited

Loading

LZoog Sep 17, 2026 •

edited

Loading

LZoog Sep 16, 2026 •

edited

Loading

LZoog Sep 17, 2026 •

edited

Loading