fix(settings,auth): Fix sumo login issue

packages/fxa-auth-client/lib/client.ts

@@ -149,6 +149,7 @@ export type SessionStatus = {
   uid: string;
   details: {
     accountEmailVerified: boolean;
+    mustVerify: boolean;
     sessionVerificationMethod: string | null;
     sessionVerified: boolean;
     sessionVerificationMeetsMinimumAAL: boolean;

packages/fxa-auth-server/lib/routes/session.js

@@ -316,6 +316,7 @@ module.exports = function (
             uid: isA.string().regex(HEX_STRING).required(),
             details: isA.object({
               accountEmailVerified: isA.boolean(),
+              mustVerify: isA.boolean(),
               sessionVerificationMethod: isA.string().allow(null),
               sessionVerified: isA.boolean(),
               sessionVerificationMeetsMinimumAAL: isA.boolean(),
@@ -359,6 +360,7 @@ module.exports = function (
           uid: sessionToken.uid,
           details: {
             accountEmailVerified,
+            mustVerify: !!sessionToken.mustVerify,
             sessionVerificationMethod,
             sessionVerified,
             sessionVerificationMeetsMinimumAAL,

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

@@ -204,6 +204,7 @@ describe('/session/status', () => {
         state: 'unverified',
         details: {
           accountEmailVerified: false,
+          mustVerify: false,
           sessionVerificationMeetsMinimumAAL: true,
           sessionVerificationMethod: 'totp-2fa',
           sessionVerified: false,
@@ -242,6 +243,7 @@ describe('/session/status', () => {
       state: 'unverified',
       details: {
         accountEmailVerified: false,
+        mustVerify: false,
         sessionVerificationMethod: 'email',
         sessionVerified: false,
         verified: false,
@@ -278,6 +280,7 @@ describe('/session/status', () => {
       state: 'unverified',
       details: {
         accountEmailVerified: false,
+        mustVerify: false,
         sessionVerificationMethod: 'email',
         sessionVerified: false,
         verified: false,
@@ -314,6 +317,7 @@ describe('/session/status', () => {
       state: 'unverified',
       details: {
         accountEmailVerified: true,
+        mustVerify: false,
         sessionVerificationMethod: 'email',
         sessionVerified: false,
         verified: false,
@@ -350,6 +354,7 @@ describe('/session/status', () => {
       state: 'verified',
       details: {
         accountEmailVerified: true,
+        mustVerify: false,
         sessionVerificationMethod: 'email',
         sessionVerified: true,
         verified: true,
@@ -385,6 +390,7 @@ describe('/session/status', () => {
       state: 'verified',
       details: {
         accountEmailVerified: true,
+        mustVerify: false,
         sessionVerificationMethod: 'email',
         sessionVerified: true,
         verified: true,
@@ -421,6 +427,7 @@ describe('/session/status', () => {
       state: 'verified',
       details: {
         accountEmailVerified: true,
+        mustVerify: false,
         sessionVerificationMethod: 'totp-2fa',
         sessionVerified: true,
         verified: true,

packages/fxa-auth-server/test/remote/session_tests.in.spec.ts

@@ -2,13 +2,20 @@
  * 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 { createTestServer, TestServerInstance } from '../support/helpers/test-server';
+import {
+  createTestServer,
+  TestServerInstance,
+} from '../support/helpers/test-server';
 
 const Client = require('../client')();
 
 let server: TestServerInstance;
 
 beforeAll(async () => {
+  // Force sign-in confirmation so the suite is deterministic. With the
+  // default (skipForNewAccounts.enabled=true) brand-new accounts get a
+  // pre-verified session and we lose the `unverified` state the
+  // `reauth` and `status` tests rely on.
   server = await createTestServer({
     configOverrides: {
       signinConfirmation: {
@@ -37,7 +44,11 @@ describe.each(testVersions)(
         const email = server.uniqueEmail();
         const password = 'foobar';
         const client = await Client.createAndVerify(
-          server.publicUrl, email, password, server.mailbox, testOptions
+          server.publicUrl,
+          email,
+          password,
+          server.mailbox,
+          testOptions
         );
 
         await client.sessionStatus();
@@ -57,7 +68,12 @@ describe.each(testVersions)(
       it('deletes a different custom token', async () => {
         const email = server.uniqueEmail();
         const password = 'foobar';
-        const client = await Client.create(server.publicUrl, email, password, testOptions);
+        const client = await Client.create(
+          server.publicUrl,
+          email,
+          password,
+          testOptions
+        );
 
         const sessionTokenCreate = client.sessionToken;
         const sessions = await client.api.sessions(sessionTokenCreate);
@@ -85,7 +101,12 @@ describe.each(testVersions)(
       it('fails with a bad custom token', async () => {
         const email = server.uniqueEmail();
         const password = 'foobar';
-        const client = await Client.create(server.publicUrl, email, password, testOptions);
+        const client = await Client.create(
+          server.publicUrl,
+          email,
+          password,
+          testOptions
+        );
 
         const sessionTokenCreate = client.sessionToken;
         const c = await client.login();
@@ -107,7 +128,9 @@ describe.each(testVersions)(
           expect(err.code).toBe(401);
           expect(err.errno).toBe(110);
           expect(err.error).toBe('Unauthorized');
-          expect(err.message).toBe('The authentication token could not be found');
+          expect(err.message).toBe(
+            'The authentication token could not be found'
+          );
         }
       });
     });
@@ -117,7 +140,11 @@ describe.each(testVersions)(
         const email = server.uniqueEmail();
         const password = 'foobar';
         const client1 = await Client.createAndVerify(
-          server.publicUrl, email, password, server.mailbox, testOptions
+          server.publicUrl,
+          email,
+          password,
+          server.mailbox,
+          testOptions
         );
 
         const client2 = await client1.duplicate();
@@ -150,7 +177,12 @@ describe.each(testVersions)(
       it('creates independent verification state for the new token', async () => {
         const email = server.uniqueEmail();
         const password = 'foobar';
-        const client1 = await Client.create(server.publicUrl, email, password, testOptions);
+        const client1 = await Client.create(
+          server.publicUrl,
+          email,
+          password,
+          testOptions
+        );
         const client2 = await client1.duplicate();
 
         expect(client1.verified).toBeFalsy();
@@ -183,7 +215,10 @@ describe.each(testVersions)(
         const email = server.uniqueEmail();
         const password = 'foobar';
         const client = await Client.createAndVerify(
-          server.publicUrl, email, password, server.mailbox,
+          server.publicUrl,
+          email,
+          password,
+          server.mailbox,
           { ...testOptions, keys: true }
         );
 
@@ -205,7 +240,11 @@ describe.each(testVersions)(
         const email = server.uniqueEmail();
         const password = 'foobar';
         const client = await Client.createAndVerify(
-          server.publicUrl, email, password, server.mailbox, testOptions
+          server.publicUrl,
+          email,
+          password,
+          server.mailbox,
+          testOptions
         );
 
         await client.setupCredentials(email, 'fiibar');
@@ -225,7 +264,12 @@ describe.each(testVersions)(
       it('has sane account-verification behaviour', async () => {
         const email = server.uniqueEmail();
         const password = 'foobar';
-        const client = await Client.create(server.publicUrl, email, password, testOptions);
+        const client = await Client.create(
+          server.publicUrl,
+          email,
+          password,
+          testOptions
+        );
         expect(client.verified).toBeFalsy();
 
         // Clear the verification email, without verifying.
@@ -247,7 +291,10 @@ describe.each(testVersions)(
         const email = server.uniqueEmail();
         const password = 'foobar';
         await Client.createAndVerify(
-          server.publicUrl, email, password, server.mailbox,
+          server.publicUrl,
+          email,
+          password,
+          server.mailbox,
           { ...testOptions, keys: false }
         );
 
@@ -288,7 +335,10 @@ describe.each(testVersions)(
         const email = server.uniqueEmail();
         const password = 'foobar';
         const client = await Client.createAndVerify(
-          server.publicUrl, email, password, server.mailbox,
+          server.publicUrl,
+          email,
+          password,
+          server.mailbox,
           { ...testOptions, keys: true }
         );
 
@@ -307,7 +357,11 @@ describe.each(testVersions)(
         const email = server.uniqueEmail();
         const password = 'testx';
         const c = await Client.createAndVerify(
-          server.publicUrl, email, password, server.mailbox, testOptions
+          server.publicUrl,
+          email,
+          password,
+          server.mailbox,
+          testOptions
         );
         const uid = c.uid;
         await c.login();
@@ -318,6 +372,10 @@ describe.each(testVersions)(
           uid,
           details: {
             accountEmailVerified: true,
+            // With signin confirmation forced (see configOverrides at top of file),
+            // every fresh login on a new account sets mustVerify=true because
+            // the server is going to require this session be verified.
+            mustVerify: true,
             sessionVerificationMeetsMinimumAAL: true,
             sessionVerificationMethod: null,
             sessionVerified: false,
@@ -326,6 +384,21 @@ describe.each(testVersions)(
         });
       });
 
+      it('exposes mustVerify=true for a session that requires verification', async () => {
+        const email = server.uniqueEmail();
+        const password = 'testx';
+        const c = await Client.createAndVerify(
+          server.publicUrl,
+          email,
+          password,
+          server.mailbox,
+          testOptions
+        );
+        await c.login({ keys: true });
+        const status = await c.api.sessionStatus(c.sessionToken);
+        expect(status.details.mustVerify).toBe(true);
+      });
+
       it('errors with invalid token', async () => {
         const client = new Client(server.publicUrl, testOptions);
         try {

packages/fxa-settings/src/models/mocks.tsx

@@ -138,6 +138,7 @@ export function mockAuthClient() {
     details: {
       verified: true,
       accountEmailVerified: true,
+      mustVerify: false,
       sessionVerified: true,
       sessionVerificationMeetsMinimumAAL: true,
       sessionVerificationMethod: 'email',

packages/fxa-settings/src/pages/Signin/container.test.tsx

@@ -401,7 +401,7 @@ function mockSigninModule() {
 }
 
 function mockReactUtilsModule() {
-  jest.spyOn(ReactUtils, 'hardNavigate').mockImplementation(() => { });
+  jest.spyOn(ReactUtils, 'hardNavigate').mockImplementation(() => {});
 }
 
 let mockGetCredentials: jest.SpyInstance;
@@ -1240,7 +1240,7 @@ describe('signin container', () => {
       // doesn't overwrite the initial hasPassword=false from locationState.
       mockAuthClient.accountStatusByEmail = jest
         .fn()
-        .mockReturnValue(new Promise(() => { }));
+        .mockReturnValue(new Promise(() => {}));
       mockLocationState = {
         email: MOCK_ROUTER_STATE_EMAIL,
         hasPassword: false,
@@ -1410,6 +1410,7 @@ describe('signin container', () => {
         details: {
           accountEmailVerified: true,
           sessionVerified: false,
+          mustVerify: true,
         },
       });
 
@@ -1489,6 +1490,7 @@ describe('signin container', () => {
         details: {
           accountEmailVerified: true,
           sessionVerified: false,
+          mustVerify: true,
         },
       });
 
@@ -1511,6 +1513,41 @@ describe('signin container', () => {
       });
     });
 
+    it('skips verification when mustVerify=false even if session is unverified (FXA-12972 SUMO regression)', async () => {
+      // Mirrors the SUMO-style OAuth RP case: the session is unverified but
+      // the auth-server has decided it does not need verification
+      // (mustVerify=false). cachedSignIn must not send a verifyLoginCode
+      // email and must leave verificationMethod/verificationReason undefined
+      // so the user is not stranded on the confirm-code page.
+      mockSession.sendVerificationCode.mockClear();
+
+      mockAuthClient.accountProfile = jest.fn().mockResolvedValue({
+        authenticationMethods: ['pwd', 'email'],
+        authenticatorAssuranceLevel: 1,
+      });
+      mockAuthClient.sessionStatus = jest.fn().mockResolvedValue({
+        details: {
+          accountEmailVerified: true,
+          sessionVerified: false,
+          mustVerify: false,
+        },
+      });
+
+      mockUseValidateModule();
+      render();
+
+      await waitFor(async () => {
+        const handlerResult =
+          await currentSigninProps?.cachedSigninHandler(MOCK_SESSION_TOKEN);
+
+        expect(mockSession.sendVerificationCode).not.toHaveBeenCalled();
+        expect(handlerResult?.data?.verificationMethod).toBeUndefined();
+        expect(handlerResult?.data?.verificationReason).toBeUndefined();
+        expect(handlerResult?.data?.sessionVerified).toEqual(false);
+        expect(handlerResult?.data?.emailVerified).toEqual(true);
+      });
+    });
+
     it('does not return a verification reason with verified 2FA session', async () => {
       // Reset session mock for this test
       mockSession.sendVerificationCode.mockClear();
@@ -1608,7 +1645,7 @@ describe('signin container', () => {
     });
 
     it('calls session.isValid and discards token when session is invalid', async () => {
-      const storedAccount: { sessionToken?: string;[key: string]: any } = {
+      const storedAccount: { sessionToken?: string; [key: string]: any } = {
         ...MOCK_STORED_ACCOUNT,
         email: MOCK_QUERY_PARAM_EMAIL,
         sessionToken: MOCK_SESSION_TOKEN,