test(auth): add regex to force unverified sessions, fix stray code emails

packages/functional-tests/lib/testAccountTracker.ts

@@ -23,6 +23,12 @@ enum EmailPrefix {
   SIGNIN = 'signin',
   SIGNUP = 'signup',
   SYNC = 'sync',
+  /**
+   * Matches the auth-server's forcedHeuristicEmailAddresses default: the
+   * session is created unverified but not `mustVerify`, the non-Sync, non-2FA
+   * heuristic state that RP redirect flows can pass through without a code.
+   */
+  UNVERIFIED_SESSION = 'unverifiedsession',
 }
 
 const SUPPORTED_SERVICE = 'smoketests';
@@ -260,6 +266,16 @@ export class TestAccountTracker {
     return await this.signUp(options, EmailPrefix.SYNC);
   }
 
+  /**
+   * Signs up an account whose sign-ins land in the heuristic unverified session
+   * state (unverified, not `mustVerify`). See EmailPrefix.UNVERIFIED_SESSION.
+   * @param options AuthClient signup options
+   * @returns Credentials
+   */
+  async signUpUnverifiedSession(options?: any): Promise<Credentials> {
+    return await this.signUp(options, EmailPrefix.UNVERIFIED_SESSION);
+  }
+
   /**
    * Creates a passwordless account via API (verifierSetAt: 0, no password).
    * The returned `password` is generated for tests that subsequently drive

packages/functional-tests/tests/signin/unverifiedSession.spec.ts

@@ -0,0 +1,156 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+import { EmailHeader, EmailType } from '../../lib/email';
+import { expect, test } from '../../lib/fixtures/standard';
+
+// The non-Sync, non-2FA unverified session: the primary email is verified, the
+// session is unverified, and `mustVerify` is unset. Real users reach it when the
+// auth-server's sign-in heuristics decline to pre-verify (account must be older
+// than `skipForNewAccounts.maxAge`). Test accounts are always new, so the
+// auth-server's `forcedHeuristicEmailAddresses` default forces it for the
+// `unverifiedsession` prefix instead. The `sync` prefix is a different state:
+// `forcedSyncEmailAddresses` sets `mustVerify`, which blocks OAuth grants until
+// the code is entered.
+test.describe('severity-1 #smoke', () => {
+  test.describe('heuristic unverified session', () => {
+    test('RP outside servicesWithEmailVerification grants without a code; Settings then asks for it and only one verifyLoginCode is sent', async ({
+      target,
+      pages: { page, relier, settings, signin, signinTokenCode },
+      testAccountTracker,
+    }) => {
+      const credentials = await testAccountTracker.signUpUnverifiedSession();
+      await target.emailClient.clear(credentials.email);
+
+      await relier.goto();
+      await relier.clickEmailFirst();
+      await signin.fillOutEmailFirstForm(credentials.email);
+      await signin.fillOutPasswordForm(credentials.password);
+
+      // Straight to the RP: no code page, and the session stays unverified.
+      expect(await relier.isLoggedIn()).toBe(true);
+
+      // The unverified session cannot use Settings, which bounces to the
+      // cached sign-in and from there to the code page.
+      await settings.goto();
+      await expect(signin.cachedSigninSubmitButton).toBeVisible();
+      await signin.cachedSigninSubmitButton.click();
+      await expect(page).toHaveURL(/signin_token_code/);
+
+      const code = await target.emailClient.waitForEmail(
+        credentials.email,
+        EmailType.verifyLoginCode,
+        EmailHeader.signinCode
+      );
+      await signinTokenCode.fillOutCodeForm(code);
+      await expect(settings.settingsHeading).toBeVisible();
+
+      // The local inbox blocks reads on an empty mailbox, so the RP step
+      // cannot be counted on its own. `newDeviceLogin` follows verification
+      // and bounds both possible senders of the code email, on both steps.
+      await target.emailClient.waitForEmail(
+        credentials.email,
+        EmailType.newDeviceLogin
+      );
+      expect(
+        await target.emailClient.countEmailsByType(
+          credentials.email,
+          EmailType.verifyLoginCode
+        ),
+        'the RP pass-through must not email a code no one is asked for'
+      ).toBe(1);
+    });
+
+    test('RP in servicesWithEmailVerification, Payments Next, lands on signin_token_code and sends exactly one verifyLoginCode', async ({
+      target,
+      pages: { page, signin, signinTokenCode },
+      testAccountTracker,
+    }) => {
+      test.skip(
+        target.name !== 'local',
+        'uses the local Payments Next client id and redirect_uri'
+      );
+      const credentials = await testAccountTracker.signUpUnverifiedSession();
+      await target.emailClient.clear(credentials.email);
+
+      const params = new URLSearchParams({
+        client_id: '32aaeb6f1c21316a',
+        redirect_uri: 'http://localhost:3035/api/auth/callback/fxa',
+        scope: 'https://identity.mozilla.com/account/subscriptions',
+        response_type: 'code',
+        state: 'fakestate',
+      });
+      await page.goto(`${target.contentServerUrl}/authorization?${params}`);
+      await signin.fillOutEmailFirstForm(credentials.email);
+      await signin.fillOutPasswordForm(credentials.password);
+      await expect(page).toHaveURL(/signin_token_code/);
+
+      const code = await target.emailClient.waitForEmail(
+        credentials.email,
+        EmailType.verifyLoginCode,
+        EmailHeader.signinCode
+      );
+      // The backend guard for listed services releases the session once the
+      // code is accepted. The grant response is the assertion; the redirect
+      // to Payments Next then fails locally because it is not running.
+      const grant = page.waitForResponse(
+        (response) =>
+          response.url().endsWith('/v1/oauth/authorization') &&
+          response.status() === 200
+      );
+      await signinTokenCode.fillOutCodeForm(code);
+      await grant;
+
+      await target.emailClient.waitForEmail(
+        credentials.email,
+        EmailType.newDeviceLogin
+      );
+      expect(
+        await target.emailClient.countEmailsByType(
+          credentials.email,
+          EmailType.verifyLoginCode
+        )
+      ).toBe(1);
+    });
+  });
+
+  // The Sync-style state: the same forced confirmation, but `mustVerify` is
+  // set (the `sync` prefix), so the server refuses the OAuth grant and the
+  // front end falls back to the code page. The code email must follow it.
+  test.describe('mustVerify unverified session (scoped keys)', () => {
+    test('RP outside servicesWithEmailVerification is still forced to verify', async ({
+      target,
+      pages: { page, relier, signin, signinTokenCode },
+      testAccountTracker,
+    }) => {
+      const credentials = await testAccountTracker.signUpSync();
+      await target.emailClient.clear(credentials.email);
+
+      await relier.goto();
+      await relier.clickEmailFirst();
+      await signin.fillOutEmailFirstForm(credentials.email);
+      await signin.fillOutPasswordForm(credentials.password);
+      await expect(page).toHaveURL(/signin_token_code/);
+
+      const code = await target.emailClient.waitForEmail(
+        credentials.email,
+        EmailType.verifyLoginCode,
+        EmailHeader.signinCode
+      );
+      await signinTokenCode.fillOutCodeForm(code);
+      expect(await relier.isLoggedIn()).toBe(true);
+
+      await target.emailClient.waitForEmail(
+        credentials.email,
+        EmailType.newDeviceLogin
+      );
+      expect(
+        await target.emailClient.countEmailsByType(
+          credentials.email,
+          EmailType.verifyLoginCode
+        )
+      ).toBe(1);
+    });
+  });
+});

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

@@ -1809,22 +1809,33 @@ const convictConf = convict({
     env: 'REMOTE_ADDRESS_CHAIN_OVERRIDE',
     default: '',
   },
+  // Sign-in confirmation is decided in this order: a suspicious request,
+  // forceGlobally, forcedSyncEmailAddresses and forcedHeuristicEmailAddresses
+  // each force it and skip every bypass; otherwise a recognized device, a
+  // recently verified IP, a new account, and skipForEmailRegex each bypass it.
+  // TOTP is required regardless.
   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: {
+      doc: 'Force Sync-style sign-in confirmation (mustVerify: 1) for matching emails. The session cannot complete an OAuth grant until the emailed code is entered, as when scoped keys are requested. Overrides the skip settings below.',
       format: RegExp,
       default: /.+@mozilla\.com$/,
       env: 'SIGNIN_CONFIRMATION_FORCE_EMAIL_REGEX',
     },
+    forcedHeuristicEmailAddresses: {
+      doc: 'Force the heuristic (non-Sync non-2FA) unverified session state for matching emails: sign-in confirmation with mustVerify: 0. The session is created unverified, RP redirect flows outside servicesWithEmailVerification continue without a code, and Settings asks for it later. This is the state the heuristics produce for older accounts.',
+      format: RegExp,
+      default: /^unverifiedsession.*@restmail\.net$/,
+      env: 'SIGNIN_CONFIRMATION_FORCE_HEURISTIC_EMAIL_REGEX',
+    },
     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.',
       format: RegExp,
       default: /^$/,
       env: 'SIGNIN_CONFIRMATION_SKIP_FOR_EMAIL_REGEX',
     },
     skipForNewAccounts: {
       enabled: {
-        doc: 'Skip all sign-in email confirmations for newly-created accounts',
+        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.',
         default: true,
         env: 'SIGNIN_CONFIRMATION_SKIP_FOR_NEW_ACCOUNTS',
       },
@@ -1849,7 +1860,7 @@ const convictConf = convict({
       },
     },
     forceGlobally: {
-      doc: 'Force sign-in confirmation for all accounts. Sets "mustVerify: 1" on created session tokens and creates an entry in unverifiedTokens, simulating a suspicious request or requesting scoped keys',
+      doc: 'Force the Sync-style state (mustVerify: 1) for every account, as a suspicious request or a scoped-key request would. Per-email equivalent: forcedSyncEmailAddresses.',
       format: Boolean,
       default: false,
       env: 'SIGNIN_CONFIRMATION_FORCE_GLOBALLY',

packages/fxa-auth-server/lib/routes/account.spec.ts

@@ -728,7 +728,6 @@ describe('deleteAccountIfUnverified', () => {
   const mockConfig: any = {};
   mockConfig.oauth = {};
   mockConfig.signinConfirmation = {};
-  mockConfig.signinConfirmation.skipForEmailAddresses = [];
   mockConfig.signinConfirmation.skipForEmailRegex = /^$/;
   const emailRecord: any = {
     isPrimary: true,
@@ -2560,7 +2559,7 @@ describe('/account/login', () => {
 
   describe('sign-in confirmation', () => {
     beforeAll(() => {
-      config.signinConfirmation.forcedEmailAddresses = /.+@mozilla\.com$/;
+      config.signinConfirmation.forcedSyncEmailAddresses = /.+@mozilla\.com$/;
 
       mockDB.accountRecord = function () {
         return Promise.resolve({
@@ -2765,6 +2764,50 @@ describe('/account/login', () => {
       });
     });
 
+    it('creates an unverified session without mustVerify for forcedHeuristicEmailAddresses', () => {
+      const email = 'test@mozilla.com';
+      const { forcedSyncEmailAddresses, forcedHeuristicEmailAddresses } =
+        config.signinConfirmation;
+      config.signinConfirmation.forcedSyncEmailAddresses = /^$/;
+      config.signinConfirmation.forcedHeuristicEmailAddresses =
+        /.+@mozilla\.com$/;
+      mockDB.accountRecord = function () {
+        return Promise.resolve({
+          authSalt: hexString(32),
+          data: hexString(32),
+          email: email,
+          emailVerified: true,
+          primaryEmail: {
+            normalizedEmail: normalizeEmail(email),
+            email: email,
+            isVerified: true,
+            isPrimary: true,
+          },
+          kA: hexString(32),
+          lastAuthAt: function () {
+            return Date.now();
+          },
+          uid: uid,
+          wrapWrapKb: hexString(32),
+        });
+      };
+
+      return runTest(route, mockRequestNoKeys, (response: any) => {
+        expect(mockDB.createSessionToken).toHaveBeenCalledTimes(1);
+        const tokenData = mockDB.createSessionToken.mock.calls[0][0];
+        expect(tokenData.mustVerify).toBeFalsy();
+        expect(tokenData.tokenVerificationId).toBeTruthy();
+        expect(response.sessionVerified).toBeFalsy();
+        expect(response.verificationMethod).toBe('email');
+        expect(response.verificationReason).toBe('login');
+      }).finally(() => {
+        config.signinConfirmation.forcedSyncEmailAddresses =
+          forcedSyncEmailAddresses;
+        config.signinConfirmation.forcedHeuristicEmailAddresses =
+          forcedHeuristicEmailAddresses;
+      });
+    });
+
     it('requires change password verification when the lockedAt field is set', () => {
       const email = 'test@mozilla.com';
       mockDB.accountRecord = function () {

packages/fxa-auth-server/lib/routes/account.ts

@@ -1007,11 +1007,22 @@ export class AccountHandler {
       // If it's an email address used for testing etc,
       // we should force token verification.
       if (
-        this.config.signinConfirmation?.forcedEmailAddresses?.test(
+        this.config.signinConfirmation?.forcedSyncEmailAddresses?.test(
           account.primaryEmail.email
         )
       ) {
-        return 'email';
+        return 'syncEmail';
+      }
+      // Same forced confirmation, but the session is not `mustVerify`, so RP
+      // redirect flows outside `servicesWithEmailVerification` pass through
+      // without a code. This is the heuristic (non-Sync non-2FA) state that
+      // `skipTokenVerification` would otherwise pre-verify for new accounts.
+      if (
+        this.config.signinConfirmation?.forcedHeuristicEmailAddresses?.test(
+          account.primaryEmail.email
+        )
+      ) {
+        return 'heuristicEmail';
       }
 
       return false;
@@ -1202,7 +1213,7 @@ export class AccountHandler {
         needsVerificationId &&
         (verificationForced === 'suspect' ||
           verificationForced === 'global' ||
-          verificationForced === 'email' ||
+          verificationForced === 'syncEmail' ||
           requestHelper.wantsKeys(request));
 
       // For accounts with TOTP, we always force verifying a session.

packages/fxa-settings/src/pages/Signin/utils.test.ts

@@ -463,6 +463,61 @@ describe('Signin utils', () => {
         );
       });
 
+      it('does not resend the email OTP code when an OAuth RP outside servicesWithEmailVerification continues without the code page', async () => {
+        const sessionResendVerifyCode = jest.fn().mockResolvedValue({});
+        const navigationOptions = createBaseNavigationOptions({
+          signinData: {
+            ...createBaseNavigationOptions().signinData,
+            emailVerified: true,
+            sessionVerified: false,
+            verificationMethod: VerificationMethods.EMAIL_OTP,
+            verificationReason: VerificationReasons.SIGN_IN,
+          },
+          isServiceWithEmailVerification: false,
+          integration: createMockSigninOAuthIntegration(),
+          authClient: { sessionResendVerifyCode },
+        });
+
+        const result = await handleNavigation(navigationOptions);
+
+        expect(result.error).toBeUndefined();
+        expect(sessionResendVerifyCode).not.toHaveBeenCalled();
+        // Straight to the RP: no in-app navigation to a code page.
+        expect(mockNavigate).not.toHaveBeenCalled();
+        expect(hardNavigateSpy).toHaveBeenCalledTimes(1);
+      });
+
+      it('resends the email OTP code when the OAuth grant is refused for an unverified session and falls back to /signin_token_code', async () => {
+        const sessionResendVerifyCode = jest.fn().mockResolvedValue({});
+        const navigationOptions = createBaseNavigationOptions({
+          signinData: {
+            ...createBaseNavigationOptions().signinData,
+            emailVerified: true,
+            sessionVerified: false,
+            verificationMethod: VerificationMethods.EMAIL_OTP,
+            verificationReason: VerificationReasons.SIGN_IN,
+          },
+          isServiceWithEmailVerification: false,
+          integration: createMockSigninOAuthIntegration(),
+          authClient: { sessionResendVerifyCode },
+          finishOAuthFlowHandler: jest
+            .fn()
+            .mockResolvedValue({ error: AuthUiErrors.UNVERIFIED_SESSION }),
+        });
+
+        const result = await handleNavigation(navigationOptions);
+
+        expect(result.error).toBeUndefined();
+        expect(sessionResendVerifyCode).toHaveBeenCalledTimes(1);
+        expect(sessionResendVerifyCode).toHaveBeenCalledWith(
+          MOCK_SESSION_TOKEN
+        );
+        expect(mockNavigate).toHaveBeenCalledWith(
+          '/signin_token_code',
+          expect.objectContaining({ replace: true })
+        );
+      });
+
       it('does not resend the email OTP code when the verification method is not EMAIL_OTP', async () => {
         const sessionResendVerifyCode = jest.fn().mockResolvedValue({});
         const navigationOptions = createBaseNavigationOptions({