Merge pull request #18141 from mozilla/FXA-10851.2

mozilla · Dec 13, 2024 · f6a0b36 · f6a0b36
2 parents 5ab55b2 + 9bc1529
commit f6a0b36
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 16 deletions.
diff --git a/libs/shared/metrics/glean/src/lib/metrics.context.ts b/libs/shared/metrics/glean/src/lib/metrics.context.ts
@@ -29,7 +29,7 @@ export class MetricsContext {
   utmSource?: string;
   utmTerm?: string;
 
-  constructor(queryParams?: Record<string, string>) {
+  constructor(queryParams?: Record<string, string | undefined>) {
     queryParams = queryParams || {};
     this.deviceId = queryParams['deviceId'];
     this.entrypoint = queryParams['entrypoint'];

diff --git a/packages/fxa-settings/src/lib/metrics.ts b/packages/fxa-settings/src/lib/metrics.ts
@@ -515,7 +515,7 @@ export function addExperiment(choice: string, group: string) {
  * specific keys but the value is actually a record of all the URL query params.
  */
 export function queryParamsToMetricsContext(
-  queryParams: Record<string, string>
+  queryParams: Record<string, string | undefined>
 ): Partial<MetricsContext> {
   const context = new MetricsContext(queryParams);
   return MetricsContext.prune(context);

diff --git a/packages/fxa-settings/src/pages/PostVerify/ThirdPartyAuthCallback/index.test.tsx b/packages/fxa-settings/src/pages/PostVerify/ThirdPartyAuthCallback/index.test.tsx
@@ -74,7 +74,7 @@ function mockThirdPartyAuthCallbackIntegration({
     data: { redirectTo: undefined },
     getError: () => getError,
     thirdPartyAuthParams: () => ({ code: 'code', provider: 'provider' }),
-    getFxAParams: () => '?param=value',
+    getFxAParams: () => '?flowId=aaaa&flowBeginTime=1734112296000',
     // TODO, fix this type cast
   } as unknown as ModelsModule.ThirdPartyAuthCallbackIntegration;
 }
@@ -91,7 +91,10 @@ function renderWith(
     flowQueryParams?: QueryParams;
     integration: ModelsModule.Integration;
   } = {
-    flowQueryParams: {},
+    flowQueryParams: {
+      flowId: 'bbbb',
+      flowBeginTime: 1734112296874,
+    },
     integration: mockThirdPartyAuthCallbackIntegration(),
   }
 ) {
@@ -152,12 +155,15 @@ describe('ThirdPartyAuthCallback component', () => {
         'code',
         'provider',
         undefined,
-        expect.any(Object)
+        {
+          flowId: 'aaaa',
+          flowBeginTime: 1734112296000,
+        }
       );
     });
 
     expect(hardNavigateSpy).toBeCalledWith(
-      '/post_verify/third_party_auth/callback?param=value'
+      '/post_verify/third_party_auth/callback?flowId=aaaa&flowBeginTime=1734112296000'
     );
   });
 
@@ -176,7 +182,9 @@ describe('ThirdPartyAuthCallback component', () => {
 
     renderWith({ integration });
 
-    expect(hardNavigateSpy).toBeCalledWith('/?param=value');
+    expect(hardNavigateSpy).toBeCalledWith(
+      '/?flowId=aaaa&flowBeginTime=1734112296000'
+    );
   });
   it('redirects to web redirect', async () => {
     const redirectTo = 'surprisinglyValid!';

diff --git a/packages/fxa-settings/src/pages/PostVerify/ThirdPartyAuthCallback/index.tsx b/packages/fxa-settings/src/pages/PostVerify/ThirdPartyAuthCallback/index.tsx
@@ -125,20 +125,26 @@ const ThirdPartyAuthCallback = ({
 
     try {
       const fxaParams = integration.getFxAParams();
-      let originalService;
-      if (fxaParams) {
-        const params = new URLSearchParams(fxaParams);
-        originalService =
-          params.get('service') || params.get('client_id') || undefined;
-      }
+
+      // Extract relayed fxa parameters
+      const params = new URLSearchParams(fxaParams || '');
+      const flowId = params.get('flowId') || undefined;
+      const flowBeginTime = params.get('flowBeginTime') || undefined;
+      const originalService =
+        params.get('service') || params.get('client_id') || undefined;
+
       const linkedAccount: LinkedAccountData =
         await account.verifyAccountThirdParty(
           thirdPartyOAuthCode,
           provider,
           originalService,
-          queryParamsToMetricsContext(
-            flowQueryParams as unknown as Record<string, string>
-          )
+          queryParamsToMetricsContext({
+            ...((flowQueryParams as Record<string, string>) || {}),
+            // Important be sure to overwrite current flowQueryParams with provided fxa parameters.
+            // If we don't do this, we will lose metrics.
+            flowId,
+            flowBeginTime,
+          })
         );
 
       const totpRequired =