Merge pull request #20308 from mozilla/fxa-13388

fix(settings): pass isPasswordlessFlow through handleNavigation to TOTP

Author: vbudhram

packages/functional-tests/tests/passwordless/signinPasswordless.spec.ts

@@ -1085,7 +1085,7 @@ test.describe('severity-2', () => {
   test.describe('Passwordless authentication - Browser Service (Relay)', () => {
     test('passwordless signin via Relay OAuth flow', async ({
       target,
-      pages: { page, signin, signinPasswordlessCode },
+      syncOAuthBrowserPages: { page, signin, signinPasswordlessCode },
       testAccountTracker,
     }) => {
       const { email } = await testAccountTracker.signUpPasswordless();
@@ -1109,7 +1109,7 @@ test.describe('severity-2', () => {
 
     test('passwordless signup via Relay OAuth flow - service allowed', async ({
       target,
-      pages: { page, signin, signinPasswordlessCode },
+      syncOAuthBrowserPages: { page, signin, signinPasswordlessCode },
       testAccountTracker,
     }, { project }) => {
       test.skip(
@@ -1137,6 +1137,89 @@ test.describe('severity-2', () => {
       await expect(page).not.toHaveURL(/signin_passwordless_code/);
     });
 
+    test('passwordless signin via Relay OAuth flow - account with 2FA proceeds to TOTP verification', async ({
+      target,
+      syncOAuthBrowserPages: { page, signin, signinPasswordlessCode, signinTotpCode },
+      testAccountTracker,
+    }) => {
+      // Create passwordless account and set up TOTP via API
+      const { email, sessionToken } =
+        await testAccountTracker.signUpPasswordless();
+      const account: any = testAccountTracker.accounts.find(
+        (a) => a.email === email
+      );
+      if (!account) {
+        throw new Error(
+          `Account for email ${email} not found in testAccountTracker.accounts`
+        );
+      }
+      const password = account.password;
+
+      const { secret } = await target.authClient.createTotpToken(
+        sessionToken,
+        {}
+      );
+      const totpCode = await getTotpCode(secret);
+      await target.authClient.verifyTotpSetupCode(sessionToken, totpCode);
+      await target.authClient.completeTotpSetup(sessionToken);
+
+      if (account) {
+        account.secret = secret;
+        account.sessionToken = sessionToken;
+      }
+
+      await signin.clearCache();
+
+      // Sign in via Relay OAuth flow
+      const params = new URLSearchParams(relayDesktopOAuthQueryParams);
+      params.set('force_passwordless', 'true');
+      await signin.goto('/authorization', params);
+
+      await signin.fillOutEmailFirstForm(email);
+
+      // Should redirect to passwordless code page
+      await page.waitForURL(/signin_passwordless_code/);
+
+      const passwordlessCode =
+        await target.emailClient.getPasswordlessSigninCode(email);
+      await signinPasswordlessCode.fillOutCodeForm(passwordlessCode);
+
+      // Should redirect to TOTP code entry page
+      await page.waitForURL(/signin_totp_code/);
+
+      const newTotpCode = await getTotpCode(secret);
+      await signinTotpCode.fillOutCodeForm(newTotpCode);
+
+      // Should complete OAuth flow and land on settings
+      await page.waitForURL(/\/settings/);
+
+      // Cleanup: set password so testAccountTracker can destroy the account
+      await target.authClient.passwordlessSendCode(email, {
+        clientId: 'dcdb5ae7add825d2',
+      });
+      const cleanupCode =
+        await target.emailClient.getPasswordlessSigninCode(email);
+      const cleanupResult = await target.authClient.passwordlessConfirmCode(
+        email,
+        cleanupCode,
+        { clientId: 'dcdb5ae7add825d2' }
+      );
+      const cleanupTotpCode = await getTotpCode(secret);
+      await target.authClient.verifyTotpCode(
+        cleanupResult.sessionToken,
+        cleanupTotpCode
+      );
+      await target.authClient.createPassword(
+        cleanupResult.sessionToken,
+        email,
+        password
+      );
+
+      if (account) {
+        account.isPasswordless = false;
+      }
+    });
+
     test('passwordless signup blocked for service not in allowedClientServices', async ({
       target,
       pages: { page, signin },

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

@@ -1200,6 +1200,48 @@ describe('passwordless statsd metrics', () => {
       expect(mockStatsd.increment.args[0][0]).toBe(
         'passwordless.sendCode.success'
       );
+      expect(mockStatsd.increment.args[0][1]).toMatchObject({
+        isResend: 'false',
+      });
+    });
+  });
+
+  it('should increment statsd counter when OTP is resent', () => {
+    mockDB.accountRecord = sinon.spy(() =>
+      Promise.resolve({
+        uid,
+        email: TEST_EMAIL,
+        verifierSetAt: 0,
+        emails: [{ email: TEST_EMAIL, isPrimary: true }],
+      })
+    );
+
+    routes = makeRoutes({
+      log: mockLog,
+      db: mockDB,
+      customs: mockCustoms,
+      statsd: mockStatsd,
+      config: {
+        passwordlessOtp: {
+          enabled: true,
+          ttl: 300,
+          digits: 6,
+          allowedClientServices: {
+            'test-client-id': { allowedServices: ['*'] },
+          },
+        },
+      },
+    });
+    route = getRoute(routes, '/account/passwordless/resend_code', 'POST');
+
+    return runTest(route, mockRequest, () => {
+      expect(mockStatsd.increment.callCount).toBe(1);
+      expect(mockStatsd.increment.args[0][0]).toBe(
+        'passwordless.sendCode.success'
+      );
+      expect(mockStatsd.increment.args[0][1]).toMatchObject({
+        isResend: 'true',
+      });
     });
   });
 

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

@@ -181,7 +181,7 @@ class PasswordlessHandler {
       request
     );
 
-    return this.generateAndSendOtp(request, email, account, isNewAccount);
+    return this.generateAndSendOtp(request, email, account, isNewAccount, false);
   }
 
   async confirmCode(request: AuthRequest) {
@@ -349,7 +349,7 @@ class PasswordlessHandler {
     const otpKey = account ? account.uid : email;
     await this.otpManager.delete(otpKey);
 
-    return this.generateAndSendOtp(request, email, account, isNewAccount);
+    return this.generateAndSendOtp(request, email, account, isNewAccount, true);
   }
 
   /**
@@ -361,7 +361,8 @@ class PasswordlessHandler {
     request: AuthRequest,
     email: string,
     account: any,
-    isNewAccount: boolean
+    isNewAccount: boolean,
+    isResend: boolean
   ) {
     const otpKey = account ? account.uid : email;
     const code = await this.otpManager.create(otpKey);
@@ -443,10 +444,10 @@ class PasswordlessHandler {
       });
     }
 
-    this.statsd.increment(
-      'passwordless.sendCode.success',
-      getClientServiceTags(request)
-    );
+    this.statsd.increment('passwordless.sendCode.success', {
+      ...getClientServiceTags(request),
+      isResend: String(isResend),
+    });
 
     // Record security event
     await recordSecurityEvent('account.passwordless_login_otp_sent', {

packages/fxa-settings/src/pages/Signin/SigninPasswordlessCode/index.tsx

@@ -236,6 +236,7 @@ const SigninPasswordlessCode = ({
         performNavigation: !(
           integration.isFirefoxMobileClient() && isSessionVerified
         ),
+        isPasswordlessFlow: true,
       };
 
       // For existing users signing into Sync (signin flow), show merge warning

packages/fxa-settings/src/pages/Signin/SigninTotpCode/index.tsx

@@ -137,7 +137,9 @@ export const SigninTotpCode = ({
       // Passwordless Sync flow: after TOTP, redirect to set_password
       // for key derivation. The session is now verified so
       // /password/create (which requires verifiedSessionToken) will work.
-      if (signinState.isPasswordlessFlow) {
+      // Only redirect to set_password for Sync integrations, non-Sync
+      // OAuth flows (e.g. Relay) should continue through handleNavigation.
+      if (signinState.isPasswordlessFlow && integration.isSync()) {
         navigateWithQuery('/post_verify/third_party_auth/set_password', {
           replace: true,
           state: {

packages/fxa-settings/src/pages/Signin/interfaces.ts

@@ -246,6 +246,7 @@ export interface NavigationOptions {
   // If false, skip actually navigating. Still sends web channel messages etc.
   performNavigation?: boolean;
   isServiceWithEmailVerification?: boolean;
+  isPasswordlessFlow?: boolean;
 }
 
 export interface OAuthSigninResult {

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

@@ -367,6 +367,7 @@ const createSigninLocationState = (
     },
     showInlineRecoveryKeySetup,
     isSignInWithThirdPartyAuth,
+    isPasswordlessFlow,
     origin,
   } = navigationOptions;
   return {
@@ -379,6 +380,7 @@ const createSigninLocationState = (
     verificationReason,
     showInlineRecoveryKeySetup,
     isSignInWithThirdPartyAuth,
+    isPasswordlessFlow,
     origin,
   };
 };