Merge pull request #20750 from mozilla/FXA-13947

fix(mobile): Redirect passwordless users to set_password in OAuthNative flows when Sync is not decoupled

Author: LZoog

packages/functional-tests/lib/query-params.ts

@@ -5,7 +5,12 @@
 // This file contains query params that don't reflect states that can be reached from 123done.
 import uaStrings from './ua-strings';
 import { FF_OAUTH_CLIENT_ID } from './channels';
-import { SYNC_SESSION_TOKEN_SCOPE, OLDSYNC_SCOPE, VPN_SCOPE } from './scopes';
+import {
+  SYNC_SESSION_TOKEN_SCOPE,
+  OLDSYNC_SCOPE,
+  VPN_SCOPE,
+  PROFILE_SCOPE,
+} from './scopes';
 
 export const oauthWebchannelV1 = new URLSearchParams({
   context: 'oauth_webchannel_v1',
@@ -87,6 +92,22 @@ export const vpnMobileOAuthQueryParams = new URLSearchParams({
   service: 'vpn',
 });
 
+// When Sync is not decoupled on Android, signing up for VPN on Android means
+// signing up for Sync as well; all scopes are included in the request
+export const vpnSyncMobileOAuthFenixQueryParams = new URLSearchParams({
+  ...Object.fromEntries(oauthWebchannelV1.entries()),
+  client_id: 'a2270f727f45f648', // Fenix (Android)
+  code_challenge_method: 'S256',
+  code_challenge: '2oc_C4v1qHeefWAGu5LI5oDG1oX4FV_Itc148D8_oQI',
+  keys_jwk:
+    'eyJrdHkiOiJFQyIsImNydiI6IlAtMjU2IiwieCI6ImdUejVIWFJfa2pxSFRtMG43ZjhxcDMybVZFaHZ1cGo1dXNUV1h5TWZsb1kiLCJ5IjoiVER5TlhkalhibHZld1pWLVc5MXNDZU9fRWd0NU9WYXhpblBzOEFTQ3owZyJ9',
+  scope: `${PROFILE_SCOPE} ${OLDSYNC_SCOPE} ${VPN_SCOPE}`,
+  state: 'fakestate',
+  automatedBrowser: 'true',
+  service: 'vpn',
+  entrypoint: 'protection_panel',
+});
+
 export const syncDesktopOAuthQueryParamsNoScope = new URLSearchParams({
   ...Object.fromEntries(oauthWebchannelV1.entries()),
   client_id: FF_OAUTH_CLIENT_ID, // Firefox Desktop

packages/functional-tests/tests/misc/vpnIntegration.spec.ts

@@ -7,6 +7,7 @@ import { expect, test } from '../../lib/fixtures/standard';
 import {
   syncMobileOAuthFenixQueryParams,
   vpnMobileOAuthQueryParams,
+  vpnSyncMobileOAuthFenixQueryParams,
 } from '../../lib/query-params';
 import { PROFILE_SCOPE, VPN_SCOPE } from '../../lib/scopes';
 
@@ -53,4 +54,30 @@ test.describe('vpn integration', () => {
       });
     }
   );
+
+  test('passwordless VPN + Sync signin on Android routes to set password when Sync is not decoupled', async ({
+    target,
+    syncOAuthBrowserPages: { page, signin, signinPasswordlessCode },
+    testAccountTracker,
+  }) => {
+    const { email, password } = await testAccountTracker.signUpPasswordless();
+    await signin.goto('/authorization', vpnSyncMobileOAuthFenixQueryParams);
+    await signin.fillOutEmailFirstForm(email);
+
+    await page.waitForURL(/signin_passwordless_code/);
+    const code = await target.emailClient.getPasswordlessSigninCode(email);
+    await signinPasswordlessCode.fillOutCodeForm(code);
+
+    await page.waitForURL(/set_password/);
+    await expect(
+      page.getByRole('heading', { name: 'Create password to sync' })
+    ).toBeVisible();
+
+    await page.getByLabel('Password', { exact: true }).fill(password);
+    await page.getByLabel('Repeat password').fill(password);
+    await page.getByRole('button', { name: 'Start syncing' }).click();
+
+    await signin.checkWebChannelMessage(FirefoxCommand.OAuthLogin);
+    await signin.checkWebChannelMessage(FirefoxCommand.Login);
+  });
 });

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

@@ -1364,6 +1364,7 @@ test.describe('severity-2', () => {
       },
       testAccountTracker,
     }) => {
+      test.fixme(true, 'TODO in FXA-13647');
       // Create passwordless account and set up TOTP via API
       const { email, sessionToken } =
         await testAccountTracker.signUpPasswordless();

packages/fxa-settings/src/components/App/index.tsx

@@ -585,7 +585,11 @@ const AuthAndAccountSetupRoutes = ({
         {/* Post verify */}
         <ThirdPartyAuthCallback
           path="/post_verify/third_party_auth/callback/*"
-          {...{ flowQueryParams, integration }}
+          {...{
+            flowQueryParams,
+            integration,
+            useFxAStatusResult,
+          }}
         />
         <SetPasswordContainer
           path="/post_verify/set_password/*"
@@ -678,6 +682,7 @@ const AuthAndAccountSetupRoutes = ({
             flowQueryParams,
             isSignedIntoFirefox,
             setCurrentSplitLayout,
+            useFxAStatusResult,
           }}
         />
         <SigninPasswordlessCodeContainer
@@ -688,6 +693,7 @@ const AuthAndAccountSetupRoutes = ({
             flowQueryParams,
             isSignedIntoFirefox,
             setCurrentSplitLayout,
+            useFxAStatusResult,
           }}
         />
         <SigninContainer
@@ -741,7 +747,12 @@ const AuthAndAccountSetupRoutes = ({
         />
         <SigninTotpCodeContainer
           path="/signin_totp_code/*"
-          {...{ integration, serviceName, setCurrentSplitLayout }}
+          {...{
+            integration,
+            serviceName,
+            setCurrentSplitLayout,
+            useFxAStatusResult,
+          }}
         />
         <SigninConfirmed
           path="/signin_verified/*"

packages/fxa-settings/src/components/FormSetupAccount/index.tsx

@@ -16,6 +16,7 @@ export const FormSetupAccount = ({
   onSubmit,
   loading,
   isSync,
+  requirePasswordConfirmation,
   offeredSyncEngineConfigs,
   submitButtonGleanId,
   passwordFormType = 'signup',
@@ -45,8 +46,8 @@ export const FormSetupAccount = ({
         submitButtonGleanId,
         passwordFormType,
         cmsButton,
+        requirePasswordConfirmation,
       }}
-      requirePasswordConfirmation={isSync}
     />
   );
 };

packages/fxa-settings/src/components/FormSetupAccount/interfaces.ts

@@ -22,6 +22,7 @@ export type FormSetupAccountProps = {
   onSubmit: (e?: React.BaseSyntheticEvent) => Promise<void>;
   loading: boolean;
   isSync: boolean;
+  requirePasswordConfirmation?: boolean;
   offeredSyncEngineConfigs?: typeof syncEngineConfigs;
   submitButtonGleanId?: string;
   passwordFormType?: 'signup' | 'post-verify-set-password';

packages/fxa-settings/src/lib/hooks/useFxAStatus/index.test.tsx

@@ -154,6 +154,7 @@ describe('useFxAStatus', () => {
       const integration = {
         type: IntegrationType.Web,
         isSync: () => false,
+        isFirefoxNonSync: () => false,
       };
 
       renderHook(() => useFxAStatus(integration));

packages/fxa-settings/src/lib/hooks/useFxAStatus/index.tsx

@@ -19,7 +19,10 @@ import {
 import firefox from '../../channels/firefox';
 import { Constants } from '../../constants';
 
-type FxAStatusIntegration = Pick<Integration, 'type' | 'isSync'>;
+type FxAStatusIntegration = Pick<
+  Integration,
+  'type' | 'isSync' | 'isFirefoxNonSync'
+>;
 
 /**
  * If integration.isSync or integration is OAuthNative, sends firefox.fxaStatus to retrieve

packages/fxa-settings/src/lib/hooks/useFxAStatus/mocks.tsx

@@ -3,6 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 import { getSyncEngineIds, syncEngineConfigs } from '../../sync-engines';
+import type { UseFxAStatusResult } from '.';
 
 export function mockUseFxAStatus({
   offeredSyncEnginesOverride,
@@ -36,7 +37,7 @@ export function mockUseFxAStatus({
     selectedEnginesForGlean,
     supportsKeysOptionalLogin,
     supportsCanLinkAccountUid,
-  };
+  } satisfies UseFxAStatusResult;
 }
 
 export default mockUseFxAStatus;

packages/fxa-settings/src/models/integrations/integration.test.ts

@@ -41,6 +41,38 @@ describe('Integration Model', function () {
     });
   });
 
+  describe('requiresPasswordForLogin', function () {
+    it('is true when keys are required (Sync), regardless of keys-optional support', () => {
+      jest.spyOn(model, 'requiresKeys').mockReturnValue(true);
+      jest.spyOn(model, 'wantsKeysIfPasswordEntered').mockReturnValue(false);
+      expect(model.requiresPasswordForLogin(true)).toBe(true);
+    });
+
+    it('is true for a non-Sync client that wants keys when keys are not optional', () => {
+      jest.spyOn(model, 'requiresKeys').mockReturnValue(false);
+      jest.spyOn(model, 'wantsKeysIfPasswordEntered').mockReturnValue(true);
+      expect(model.requiresPasswordForLogin(false)).toBe(true);
+    });
+
+    it('is false for a non-Sync client that wants keys when the browser supports keys-optional login', () => {
+      jest.spyOn(model, 'requiresKeys').mockReturnValue(false);
+      jest.spyOn(model, 'wantsKeysIfPasswordEntered').mockReturnValue(true);
+      expect(model.requiresPasswordForLogin(true)).toBe(false);
+    });
+
+    it('is false when the client does not want keys', () => {
+      jest.spyOn(model, 'requiresKeys').mockReturnValue(false);
+      jest.spyOn(model, 'wantsKeysIfPasswordEntered').mockReturnValue(false);
+      expect(model.requiresPasswordForLogin(false)).toBe(false);
+    });
+
+    it('treats an omitted keys-optional flag as "not supported"', () => {
+      jest.spyOn(model, 'requiresKeys').mockReturnValue(false);
+      jest.spyOn(model, 'wantsKeysIfPasswordEntered').mockReturnValue(true);
+      expect(model.requiresPasswordForLogin()).toBe(true);
+    });
+  });
+
   describe('isTrusted', function () {
     it('returns `true`', () => {
       expect(model.isTrusted()).toBeTruthy();