fix(sync): Adjust fxaLogin / fxaOAuthLogin web channel data for oauth…

… desktop Because: * We want to send service: { relay: {} } in fxaLogin instead of fxaOAuthLogin in case users start with the Relay flow and then click the browser profile icon to finish the flow; this will prevent 'surprise sync' when patch lands on desktop side * For sync, we don't want the browser to forget which sync engines were sent up if users drop off after signing up but don't confirm their account This commit: * Sends these pieces of data up in fxaLogin instead of fxaOAuthLogin, but keeps sending sync engines in ConfirmSignupCode around for mobile clients until we can get issues filed and worked on their side closes FXA-10614, closes (on our side) FXA-10596
mozilla · Oct 25, 2024 · 1f9f779 · 1f9f779
1 parent 57f2123
commit 1f9f779
Show file tree

Hide file tree

Showing 7 changed files with 26 additions and 33 deletions.
diff --git a/packages/fxa-settings/src/lib/channels/firefox.ts b/packages/fxa-settings/src/lib/channels/firefox.ts
@@ -84,12 +84,17 @@ export type FxALoginRequest = {
   keyFetchToken?: hexstring;
   unwrapBKey?: string;
   verifiedCanLinkAccount?: boolean;
-  services?: {
-    sync: {
-      offeredEngines?: string[];
-      declinedEngines?: string[];
-    };
-  };
+  services?:
+    | {
+        sync: {
+          offeredEngines?: string[];
+          declinedEngines?: string[];
+        };
+      }
+    // For sync optional flows (currently only Relay)
+    | {
+        relay: {};
+      };
 };
 
 // ref: [FxAccounts.sys.mjs](https://searchfox.org/mozilla-central/rev/82828dba9e290914eddd294a0871533875b3a0b5/services/fxaccounts/FxAccounts.sys.mjs#910)
@@ -114,13 +119,11 @@ export type FxAOAuthLogin = {
   code: string;
   redirect: string;
   state: string;
-  // For sync oauth signup
+  // OAuth desktop looks at the syc engine list in fxaLogin.
+  // OAuth mobile currently looks at fxaOAuthLogin, but should
+  // eventually move to look at fxaLogin as well to prevent FXA-10596.
   declinedSyncEngines?: string[];
   offeredSyncEngines?: string[];
-  // For sync optional flows (currently only Relay)
-  services?: {
-    relay: {};
-  };
 };
 
 // ref: https://searchfox.org/mozilla-central/rev/82828dba9e290914eddd294a0871533875b3a0b5/services/fxaccounts/FxAccountsWebChannel.sys.mjs#230

diff --git a/packages/fxa-settings/src/pages/Signin/index.test.tsx b/packages/fxa-settings/src/pages/Signin/index.test.tsx
@@ -572,15 +572,14 @@ describe('Signin', () => {
               });
               enterPasswordAndSubmit();
               await waitFor(() => {
-                // Ensure it's not called with keyFetchToken or unwrapBKey
+                // Ensure it's not called with keyFetchToken, unwrapBKey, or { relay: {} }
                 expect(fxaLoginSpy).toHaveBeenCalledWith({
                   email: MOCK_EMAIL,
                   sessionToken: MOCK_SESSION_TOKEN,
                   uid: MOCK_UID,
                   verified: true,
                   services: { sync: {} },
                 });
-                // Ensure it's not called with services: { relay: {} }
                 expect(fxaOAuthLoginSpy).toHaveBeenCalledWith({
                   action: 'signin',
                   ...MOCK_OAUTH_FLOW_HANDLER_RESPONSE,
@@ -622,11 +621,11 @@ describe('Signin', () => {
                   sessionToken: MOCK_SESSION_TOKEN,
                   uid: MOCK_UID,
                   verified: true,
+                  services: { relay: {} },
                 });
                 expect(fxaOAuthLoginSpy).toHaveBeenCalledWith({
                   action: 'signin',
                   ...MOCK_OAUTH_FLOW_HANDLER_RESPONSE,
-                  services: { relay: {} },
                 });
 
                 const fxaLoginCallOrder =

diff --git a/packages/fxa-settings/src/pages/Signin/utils.ts b/packages/fxa-settings/src/pages/Signin/utils.ts
@@ -130,12 +130,6 @@ export async function handleNavigation(
         code: oauthData.code,
         redirect: oauthData.redirect,
         state: oauthData.state,
-        // OAuth desktop sync optional flow sends service in fxaOAuthLogin
-        ...(integration.isDesktopRelay() && {
-          services: {
-            relay: {},
-          },
-        }),
       });
     }
     performNavigation({ to, locationState, shouldHardNavigate });
@@ -187,12 +181,9 @@ function sendFxaLogin(navigationOptions: NavigationOptions) {
       keyFetchToken: navigationOptions.signinData.keyFetchToken!,
       unwrapBKey: navigationOptions.unwrapBKey!,
     }),
-    // OAuth desktop sync optional flow sends service in fxaOAuthLogin
-    ...(!navigationOptions.integration.isDesktopRelay() && {
-      services: {
-        sync: {},
-      },
-    }),
+    services: navigationOptions.integration.isDesktopRelay()
+      ? { relay: {} }
+      : { sync: {} },
   });
 }
 

diff --git a/packages/fxa-settings/src/pages/Signup/ConfirmSignupCode/index.test.tsx b/packages/fxa-settings/src/pages/Signup/ConfirmSignupCode/index.test.tsx
@@ -298,9 +298,6 @@ describe('ConfirmSignupCode page', () => {
         expect(fxaOAuthLoginSpy).toHaveBeenCalledWith({
           action: 'signup',
           ...MOCK_OAUTH_FLOW_HANDLER_RESPONSE,
-          services: {
-            relay: {},
-          },
         });
       });
     });

diff --git a/packages/fxa-settings/src/pages/Signup/ConfirmSignupCode/index.tsx b/packages/fxa-settings/src/pages/Signup/ConfirmSignupCode/index.tsx
@@ -212,6 +212,9 @@ const ConfirmSignupCode = ({
 
           if (integration.isSync()) {
             firefox.fxaOAuthLogin({
+              // OAuth desktop looks at the syc engine list in fxaLogin. Oauth
+              // mobile currently looks at the engines provided here, but should
+              // eventually move to look at fxaLogin as well to prevent FXA-10596.
               declinedSyncEngines,
               offeredSyncEngines,
               action: 'signup',
@@ -229,9 +232,6 @@ const ConfirmSignupCode = ({
               code,
               redirect,
               state,
-              services: {
-                relay: {},
-              },
             });
             goToSettingsWithAlertSuccess();
           } else {

diff --git a/packages/fxa-settings/src/pages/Signup/index.test.tsx b/packages/fxa-settings/src/pages/Signup/index.test.tsx
@@ -792,6 +792,7 @@ describe('Signup page', () => {
           // Does not send services: { sync: {...} }
           expect(fxaLoginSpy).toBeCalledWith({
             ...oauthCommonFxaLoginOptions,
+            services: { relay: {} },
           });
         });
 

diff --git a/packages/fxa-settings/src/pages/Signup/index.tsx b/packages/fxa-settings/src/pages/Signup/index.tsx
@@ -282,7 +282,9 @@ export const Signup = ({
             sessionToken: data.signUp.sessionToken,
             uid: data.signUp.uid,
             verified: false,
-            // OAuth desktop sync optional flow sends service in fxaOAuthLogin
+            services: {
+              relay: {},
+            },
           });
         } else {
           GleanMetrics.registration.marketing({