diff --git a/packages/fxa-settings/src/lib/glean/index.test.ts b/packages/fxa-settings/src/lib/glean/index.test.ts index c01a0af31d6..5d1668022ef 100644 --- a/packages/fxa-settings/src/lib/glean/index.test.ts +++ b/packages/fxa-settings/src/lib/glean/index.test.ts @@ -412,6 +412,26 @@ describe('lib/glean', () => { ); }); + // The dispatcher case arm is the only thing deciding what reaches Glean + // for this event, so both the populated and empty reason are pinned here + // (FXA-14133). + it('submits a ping with the cad_firefox_choice_view event name and a reason', async () => { + GleanMetrics.cadFireFox.choiceView({ + event: { reason: 'otp_login' }, + }); + await GleanMetrics.isDone(); + sinon.assert.calledOnce(setEventNameStub); + sinon.assert.calledWith(setEventNameStub, 'cad_firefox_choice_view'); + sinon.assert.calledWith(setEventReasonStub, 'otp_login'); + }); + + it('submits a ping with the cad_firefox_choice_view event name and no reason', async () => { + GleanMetrics.cadFireFox.choiceView(); + await GleanMetrics.isDone(); + sinon.assert.calledOnce(setEventNameStub); + sinon.assert.calledWith(setEventNameStub, 'cad_firefox_choice_view'); + }); + it('submits a ping with the email_first_passkey_submit_success event name', async () => { GleanMetrics.emailFirst.passkeySubmitSuccess(); await GleanMetrics.isDone(); diff --git a/packages/fxa-settings/src/lib/glean/index.ts b/packages/fxa-settings/src/lib/glean/index.ts index fd3b690c437..d0b582c291e 100644 --- a/packages/fxa-settings/src/lib/glean/index.ts +++ b/packages/fxa-settings/src/lib/glean/index.ts @@ -510,7 +510,9 @@ const recordEventMetric = ( cadFirefox.view.record(); break; case 'cad_firefox_choice_view': - cadFirefox.choiceView.record(); + cadFirefox.choiceView.record({ + reason: gleanPingMetrics?.event?.['reason'] || '', + }); break; case 'cad_firefox_choice_engage': cadFirefox.choiceEngage.record({ diff --git a/packages/fxa-settings/src/pages/InlineRecoveryKeySetup/container.test.tsx b/packages/fxa-settings/src/pages/InlineRecoveryKeySetup/container.test.tsx index 441ed81cce4..ee4c7567962 100644 --- a/packages/fxa-settings/src/pages/InlineRecoveryKeySetup/container.test.tsx +++ b/packages/fxa-settings/src/pages/InlineRecoveryKeySetup/container.test.tsx @@ -114,7 +114,7 @@ describe('InlineRecoveryKeySetupContainer', () => { ); expect(hardNavigateSpy).toHaveBeenCalledWith( - '/pair?showSuccessMessage=true' + '/pair?showSuccessMessage=true&pairReason=password_login' ); expect(InlineRecoveryKeySetupModule.default).not.toHaveBeenCalled(); }); diff --git a/packages/fxa-settings/src/pages/Pair/Index/index.test.tsx b/packages/fxa-settings/src/pages/Pair/Index/index.test.tsx index d9e5bca2641..9f1deda6ec7 100644 --- a/packages/fxa-settings/src/pages/Pair/Index/index.test.tsx +++ b/packages/fxa-settings/src/pages/Pair/Index/index.test.tsx @@ -13,18 +13,20 @@ import * as ReactUtils from 'fxa-react/lib/utils'; import { MOCK_ERROR } from './mocks'; import { MOCK_CMS_INFO } from '../../mocks'; import Pair, { viewName } from '.'; +import { PAIR_GLEAN_REASONS } from 'fxa-shared/metrics/glean/pair-reasons'; jest.mock('../../../lib/metrics', () => ({ usePageViewEvent: jest.fn(), })); let mockLocationState: unknown = null; +let mockLocationSearch = ''; const mockNavigate = jest.fn(); jest.mock('react-router', () => ({ ...jest.requireActual('react-router'), useLocation: () => ({ pathname: '/pair', - search: '', + search: mockLocationSearch, state: mockLocationState, }), useNavigate: () => mockNavigate, @@ -105,6 +107,7 @@ describe('Pair', () => { afterEach(() => { jest.clearAllMocks(); mockLocationState = null; + mockLocationSearch = ''; }); // Render Pair and wait for the bootstrap spinner to clear before asserting. @@ -139,9 +142,59 @@ describe('Pair', () => { ).toBeInTheDocument(); }); - it('fires choiceView Glean event on render', async () => { + // Helper reads the reason actually handed to Glean rather than pinning the + // whole call shape, so a behaviour-preserving change to how the argument is + // built doesn't break these. + const recordedReason = () => + (GleanMetrics.cadFireFox.choiceView as jest.Mock).mock.calls[0]?.[0] + ?.event?.reason; + + it('fires choiceView with no reason when there is no pairReason', async () => { + await renderPair(); + expect(GleanMetrics.cadFireFox.choiceView).toHaveBeenCalledTimes(1); + expect(recordedReason()).toBeUndefined(); + }); + + it.each(PAIR_GLEAN_REASONS)( + 'fires choiceView with reason %s from location state', + async (pairReason) => { + mockLocationState = { pairReason }; + await renderPair(); + expect(recordedReason()).toBe(pairReason); + } + ); + + // Flows that stop at /signup_confirmed_sync or /inline_recovery_key_setup + // reach /pair by hard navigation, so the reason arrives as a query param. + it.each(PAIR_GLEAN_REASONS)( + 'fires choiceView with reason %s from the query param', + async (pairReason) => { + mockLocationSearch = `?pairReason=${pairReason}`; + await renderPair(); + expect(recordedReason()).toBe(pairReason); + } + ); + + it('prefers location state over the query param', async () => { + mockLocationState = { pairReason: 'otp_login' }; + mockLocationSearch = '?pairReason=password_login'; + await renderPair(); + expect(recordedReason()).toBe('otp_login'); + }); + + it.each(['not-a-real-flow', '', ' otp_login'])( + 'ignores the unrecognized query param %j', + async (pairReason) => { + mockLocationSearch = `?pairReason=${encodeURIComponent(pairReason)}`; + await renderPair(); + expect(recordedReason()).toBeUndefined(); + } + ); + + it('ignores an unrecognized value in location state', async () => { + mockLocationState = { pairReason: 'not-a-real-flow' }; await renderPair(); - expect(GleanMetrics.cadFireFox.choiceView).toHaveBeenCalled(); + expect(recordedReason()).toBeUndefined(); }); it('enables Continue button after selecting a radio', async () => { diff --git a/packages/fxa-settings/src/pages/Pair/Index/index.tsx b/packages/fxa-settings/src/pages/Pair/Index/index.tsx index 30cbeed9446..6a1615f10ff 100644 --- a/packages/fxa-settings/src/pages/Pair/Index/index.tsx +++ b/packages/fxa-settings/src/pages/Pair/Index/index.tsx @@ -2,7 +2,14 @@ * 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 React, { useCallback, useEffect, useRef, useState } from 'react'; +import React, { + useCallback, + useEffect, + useMemo, + useRef, + useState, +} from 'react'; +import { isPairGleanReason } from 'fxa-shared/metrics/glean/pair-reasons'; import { Link, useLocation } from 'react-router'; import { useNavigateWithQuery } from '../../../lib/hooks/useNavigateWithQuery'; import { FtlMsg } from 'fxa-react/lib/utils'; @@ -68,11 +75,7 @@ type PairProps = { }; export const viewName = 'pair'; -const Pair = ({ - error, - cmsInfo: cmsInfoProp, - integration, -}: PairProps) => { +const Pair = ({ error, cmsInfo: cmsInfoProp, integration }: PairProps) => { usePageViewEvent(viewName, REACT_ENTRYPOINT); const ftlMsgResolver = useFtlMsgResolver(); const localizedQRCodeLabel = ftlMsgResolver.getMsg( @@ -161,22 +164,41 @@ const Pair = ({ // eslint-disable-next-line react-hooks/exhaustive-deps }, []); + // Banner variant is driven by router state from getSyncNavigate. + const { origin: pairOrigin } = (location.state ?? {}) as Pick< + SigninLocationState, + 'origin' + >; + + // Router state is the primary channel, but flows that stop at an interstitial + // (/signup_confirmed_sync, /inline_recovery_key_setup) reach here through a + // hard navigation that carries the query param instead, and a reload drops + // router state entirely. Both are validated: `location.state` is untyped at + // runtime, and the param is user-controllable. + const pairReason = useMemo(() => { + const fromState = (location.state as { pairReason?: unknown } | null) + ?.pairReason; + if (isPairGleanReason(fromState)) { + return fromState; + } + const fromQuery = new URLSearchParams(location.search).get('pairReason'); + return isPairGleanReason(fromQuery) ? fromQuery : undefined; + }, [location.state, location.search]); + // Fire Glean view events only after the bootstrap reveals the page; // otherwise users redirected during bootstrap would skew the metric. useEffect(() => { if (bootstrapping) return; if (currentView === 'choice') { - GleanMetrics.cadFireFox.choiceView(); + // Recorded as an empty reason when /pair is reached outside a sign-in or + // sign-up flow, or from a flow with no sanctioned bucket (third-party + // auth) — the dispatcher coerces a missing reason to ''. + GleanMetrics.cadFireFox.choiceView({ event: { reason: pairReason } }); return; } GleanMetrics.cadFireFox.view(); - }, [bootstrapping, currentView]); + }, [bootstrapping, currentView, pairReason]); - // Banner variant is driven by reach-router state from getSyncNavigate. - const { origin: pairOrigin } = (location.state ?? {}) as Pick< - SigninLocationState, - 'origin' - >; const bannerCopy = currentView === 'choice' && pairOrigin ? PAIR_BANNER_FTL[pairOrigin] : null; diff --git a/packages/fxa-settings/src/pages/PostVerify/SetPassword/container.tsx b/packages/fxa-settings/src/pages/PostVerify/SetPassword/container.tsx index 82b3885fc45..2e4fbd6b60c 100644 --- a/packages/fxa-settings/src/pages/PostVerify/SetPassword/container.tsx +++ b/packages/fxa-settings/src/pages/PostVerify/SetPassword/container.tsx @@ -189,6 +189,11 @@ const SetPasswordContainer = ({ handleFxaOAuthLogin: true, showSignupConfirmedSync: true, origin: 'post-verify-set-password', + // Sync needs keys, so every passwordless OTP/passkey sign-in lands + // here before it can reach /pair. This is the only place that still + // knows how the session was established, so it drives the /pair + // `choice_view` reason. + passwordCreationReason, syncEngines: { offeredEngines: offeredSyncEngines, declinedEngines: declinedSyncEngines, diff --git a/packages/fxa-settings/src/pages/Signin/SigninPasskeyFallback/container.test.tsx b/packages/fxa-settings/src/pages/Signin/SigninPasskeyFallback/container.test.tsx index 6c7f1362d9b..051a390ce2f 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninPasskeyFallback/container.test.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninPasskeyFallback/container.test.tsx @@ -225,6 +225,20 @@ describe('SigninPasskeyFallback container', () => { ); }); + // This is the only producer of isPasskeySession, and therefore the only + // source of the passkey_login reason on /pair for accounts that already + // have a password (FXA-14133). + it('marks the session as passkey-established so /pair attributes passkey_login', async () => { + const { getByTestId } = render(); + submitPassword(getByTestId); + + await waitFor(() => { + expect(mockHandleNavigation).toHaveBeenCalledWith( + expect.objectContaining({ isPasskeySession: true }) + ); + }); + }); + it('passes the flow metricsContext to sessionReauth so the deferred account.login is correlated', async () => { const { getByTestId } = render(); submitPassword(getByTestId); diff --git a/packages/fxa-settings/src/pages/Signin/SigninPasskeyFallback/container.tsx b/packages/fxa-settings/src/pages/Signin/SigninPasskeyFallback/container.tsx index c221ffde407..1704274b7ec 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninPasskeyFallback/container.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninPasskeyFallback/container.tsx @@ -113,6 +113,10 @@ const SigninPasskeyFallbackContainer = ({ // messages; navigating the WebView away would interrupt it and leave // Sync paused. Desktop finishes by navigating. performNavigation: !integration.isFirefoxMobileClient(), + // The session was established by the passkey assertion; the password + // entered here only unwraps keys. Keeps /pair's `choice_view` reason + // attributed to the passkey flow rather than password sign-in. + isPasskeySession: true, authClient, }); if (navError) { diff --git a/packages/fxa-settings/src/pages/Signin/SigninPasswordlessCode/index.tsx b/packages/fxa-settings/src/pages/Signin/SigninPasswordlessCode/index.tsx index 43561a7f52b..58e2347c792 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninPasswordlessCode/index.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninPasswordlessCode/index.tsx @@ -370,6 +370,7 @@ const SigninPasswordlessCode = ({ if (isSyncDesktopV3Integration(integration)) { const { to } = getSyncNavigate(location.search, { showSignupConfirmedSync: true, + origin: 'signup', }); navigate(to); } else if (isOAuthIntegration(integration)) { diff --git a/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/index.test.tsx b/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/index.test.tsx index ab80242bfdd..eb72e489110 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/index.test.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/index.test.tsx @@ -389,6 +389,71 @@ describe('PageSigninRecoveryCode', () => { expect.anything() ); }); + + // Without this, /pair tags cad_firefox.choice_view as password_login for a + // user who actually signed in with an email OTP code. + it('forwards isPasswordlessOtpSignin to handleNavigation for an OTP sign-in', async () => { + const user = userEvent.setup(); + const handleNavigationSpy = jest + .spyOn(SigninUtils, 'handleNavigation') + .mockResolvedValue({ error: undefined }); + const integration = createMockSigninOAuthNativeSyncIntegration(); + integration.requiresPasswordForLogin = () => false; + + renderWithLocalizationProvider( + + + + ); + await user.type(screen.getByRole('textbox'), MOCK_BACKUP_CODE); + await user.click(screen.getByRole('button', { name: 'Confirm' })); + + await waitFor(() => { + expect(handleNavigationSpy).toHaveBeenCalledWith( + expect.objectContaining({ isPasswordlessOtpSignin: true }) + ); + }); + }); + + it('leaves isPasswordlessOtpSignin unset for a password sign-in', async () => { + const user = userEvent.setup(); + const handleNavigationSpy = jest + .spyOn(SigninUtils, 'handleNavigation') + .mockResolvedValue({ error: undefined }); + const integration = createMockSigninOAuthNativeSyncIntegration(); + integration.requiresPasswordForLogin = () => false; + + renderWithLocalizationProvider( + + + + ); + await user.type(screen.getByRole('textbox'), MOCK_BACKUP_CODE); + await user.click(screen.getByRole('button', { name: 'Confirm' })); + + await waitFor(() => { + expect(handleNavigationSpy).toHaveBeenCalledWith( + expect.objectContaining({ isPasswordlessOtpSignin: undefined }) + ); + }); + }); }); describe('submit with error', () => { diff --git a/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/index.tsx b/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/index.tsx index 6c9fe8612c4..8fc82bcaf23 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/index.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/index.tsx @@ -138,6 +138,7 @@ const SigninRecoveryCode = ({ handleFxaLogin: true, handleFxaOAuthLogin: true, performNavigation: !integration.isFirefoxMobileClient(), + isPasswordlessOtpSignin: signinState.isPasswordlessOtpSignin, authClient, }; diff --git a/packages/fxa-settings/src/pages/Signin/SigninRecoveryPhone/container.test.tsx b/packages/fxa-settings/src/pages/Signin/SigninRecoveryPhone/container.test.tsx index e5181ecb678..dc2b35adf9d 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninRecoveryPhone/container.test.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninRecoveryPhone/container.test.tsx @@ -325,6 +325,45 @@ describe('SigninRecoveryPhoneContainer', () => { expect.anything() ); }); + + // Without this, /pair tags cad_firefox.choice_view as password_login for a + // user who actually signed in with an email OTP code (FXA-14133). + it('forwards isPasswordlessOtpSignin to handleNavigation for an OTP sign-in', async () => { + const integration = + createMockSigninOAuthNativeSyncIntegration() as Integration; + integration.requiresPasswordForLogin = () => false; + mockReachRouter('/signin_recovery_phone', { + signinState: { + ...mockSigninLocationState, + isPasswordlessOtpSignin: true, + }, + lastFourPhoneDigits: '1234', + }); + renderSigninRecoveryPhoneContainer(integration); + + await currentPageProps?.verifyCode('123456'); + + expect(handleNavigation).toHaveBeenCalledWith( + expect.objectContaining({ isPasswordlessOtpSignin: true }) + ); + }); + + it('leaves isPasswordlessOtpSignin unset for a password sign-in', async () => { + const integration = + createMockSigninOAuthNativeSyncIntegration() as Integration; + integration.requiresPasswordForLogin = () => false; + mockReachRouter('/signin_recovery_phone', { + signinState: mockSigninLocationState, + lastFourPhoneDigits: '1234', + }); + renderSigninRecoveryPhoneContainer(integration); + + await currentPageProps?.verifyCode('123456'); + + expect(handleNavigation).toHaveBeenCalledWith( + expect.objectContaining({ isPasswordlessOtpSignin: undefined }) + ); + }); }); describe('with mobile integration', () => { diff --git a/packages/fxa-settings/src/pages/Signin/SigninRecoveryPhone/container.tsx b/packages/fxa-settings/src/pages/Signin/SigninRecoveryPhone/container.tsx index 204e14f3811..a4ad262baf1 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninRecoveryPhone/container.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninRecoveryPhone/container.tsx @@ -157,6 +157,7 @@ const SigninRecoveryPhoneContainer = ({ handleFxaLogin: true, handleFxaOAuthLogin: true, performNavigation: !integration.isFirefoxMobileClient(), + isPasswordlessOtpSignin: signinState.isPasswordlessOtpSignin, authClient, }; diff --git a/packages/fxa-settings/src/pages/Signin/SigninTokenCode/index.test.tsx b/packages/fxa-settings/src/pages/Signin/SigninTokenCode/index.test.tsx index 7f0d4156f85..7e1f068defe 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninTokenCode/index.test.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninTokenCode/index.test.tsx @@ -15,7 +15,11 @@ import { mockAppContext, mockSession } from '../../../models/mocks'; import { REACT_ENTRYPOINT } from '../../../constants'; import { Session, AppContext } from '../../../models'; import { SigninTokenCodeProps } from './interfaces'; -import { createOAuthNativeIntegration, Subject } from './mocks'; +import { + createMockSigninLocationState, + createOAuthNativeIntegration, + Subject, +} from './mocks'; import { MOCK_SIGNUP_CODE } from '../../Signup/ConfirmSignupCode/mocks'; import { MOCK_CMS_INFO, @@ -381,7 +385,9 @@ describe('SigninTokenCode page', () => { await expectSuccessGleanEvents(); expect(mockOnSessionVerified).toHaveBeenCalledTimes(1); - expect(mockNavigate).toHaveBeenCalledWith('/settings', { replace: false }); + expect(mockNavigate).toHaveBeenCalledWith('/settings', { + replace: false, + }); }); it('when verificationReason is a force password change', async () => { session = mockSession(); @@ -444,6 +450,49 @@ describe('SigninTokenCode page', () => { ); }); }); + + // Keeps /pair's cad_firefox.choice_view reason accurate when this page is + // reached with an OTP session via the unverified_session OAuth error + // branch, matching the other post-2FA pages. + it('forwards isPasswordlessOtpSignin to handleNavigation for an OTP sign-in', async () => { + const handleNavigationSpy = jest.spyOn(SigninUtils, 'handleNavigation'); + session = mockSession(); + const integration = createMockSigninOAuthNativeSyncIntegration(); + render({ + finishOAuthFlowHandler: jest + .fn() + .mockReturnValueOnce(MOCK_OAUTH_FLOW_HANDLER_RESPONSE), + integration, + signinState: { + ...createMockSigninLocationState(integration.wantsKeys()), + isPasswordlessOtpSignin: true, + }, + }); + await submitCode(); + await waitFor(() => { + expect(handleNavigationSpy).toHaveBeenCalledWith( + expect.objectContaining({ isPasswordlessOtpSignin: true }) + ); + }); + }); + + it('leaves isPasswordlessOtpSignin unset for a password sign-in', async () => { + const handleNavigationSpy = jest.spyOn(SigninUtils, 'handleNavigation'); + session = mockSession(); + const integration = createMockSigninOAuthNativeSyncIntegration(); + render({ + finishOAuthFlowHandler: jest + .fn() + .mockReturnValueOnce(MOCK_OAUTH_FLOW_HANDLER_RESPONSE), + integration, + }); + await submitCode(); + await waitFor(() => { + expect(handleNavigationSpy).toHaveBeenCalledWith( + expect.objectContaining({ isPasswordlessOtpSignin: undefined }) + ); + }); + }); }); }); }); diff --git a/packages/fxa-settings/src/pages/Signin/SigninTokenCode/index.tsx b/packages/fxa-settings/src/pages/Signin/SigninTokenCode/index.tsx index 8660b1231b8..48a23eccb12 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninTokenCode/index.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninTokenCode/index.tsx @@ -53,6 +53,7 @@ const SigninTokenCode = ({ sessionToken, verificationReason, showInlineRecoveryKeySetup, + isPasswordlessOtpSignin, } = signinState; const [localizedErrorBannerMessage, setLocalizedErrorBannerMessage] = @@ -190,6 +191,7 @@ const SigninTokenCode = ({ handleFxaLogin: false, handleFxaOAuthLogin: true, performNavigation: !integration.isFirefoxMobileClient(), + isPasswordlessOtpSignin, authClient, }; @@ -232,6 +234,7 @@ const SigninTokenCode = ({ unwrapBKey, verificationReason, showInlineRecoveryKeySetup, + isPasswordlessOtpSignin, onSessionVerified, startThrottle, authClient, diff --git a/packages/fxa-settings/src/pages/Signin/SigninTokenCode/mocks.tsx b/packages/fxa-settings/src/pages/Signin/SigninTokenCode/mocks.tsx index 5bdc72865cc..bec2b020e47 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninTokenCode/mocks.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninTokenCode/mocks.tsx @@ -80,6 +80,7 @@ export const Subject = ({ integration = createMockWebIntegration(), verificationReason = undefined, onSessionVerified = async () => {}, + signinState, }: Partial & { verificationReason?: VerificationReasons; }) => { @@ -90,10 +91,13 @@ export const Subject = ({ integration, onSessionVerified, }} - signinState={createMockSigninLocationState( - integration.wantsKeys(), - verificationReason - )} + signinState={ + signinState ?? + createMockSigninLocationState( + integration.wantsKeys(), + verificationReason + ) + } /> ); }; diff --git a/packages/fxa-settings/src/pages/Signin/SigninTotpCode/index.test.tsx b/packages/fxa-settings/src/pages/Signin/SigninTotpCode/index.test.tsx index 26998191ea3..1425adceb4a 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninTotpCode/index.test.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninTotpCode/index.test.tsx @@ -95,7 +95,11 @@ describe('Sign in with TOTP code page', () => { }); it('renders as expected', () => { - renderWithLocalizationProvider(); + renderWithLocalizationProvider( + + + + ); const headingEl = screen.getByRole('heading', { level: 2 }); expect(headingEl).toHaveTextContent('Enter two-step authentication code'); @@ -108,7 +112,11 @@ describe('Sign in with TOTP code page', () => { }); it('enables submit button when code entered', async () => { - renderWithLocalizationProvider(); + renderWithLocalizationProvider( + + + + ); const inputEl = screen.getByLabelText('Enter 6-digit code'); await waitFor(() => userEvent.type(inputEl, '123456')); @@ -159,7 +167,11 @@ describe('Sign in with TOTP code page', () => { }); it('emits a metrics event on render', () => { - renderWithLocalizationProvider(); + renderWithLocalizationProvider( + + + + ); expect(GleanMetrics.totpForm.view).toHaveBeenCalledTimes(1); expect(GleanMetrics.totpForm.submit).toHaveBeenCalledTimes(0); expect(GleanMetrics.totpForm.success).toHaveBeenCalledTimes(0); @@ -205,7 +217,9 @@ describe('Sign in with TOTP code page', () => { expect(GleanMetrics.totpForm.view).toHaveBeenCalledTimes(1); expect(GleanMetrics.totpForm.submit).toHaveBeenCalledTimes(1); expect(GleanMetrics.totpForm.success).toHaveBeenCalledTimes(1); - expect(mockNavigate).toHaveBeenCalledWith('/settings', { replace: false }); + expect(mockNavigate).toHaveBeenCalledWith('/settings', { + replace: false, + }); }); describe('fxaLogin webchannel message', () => { @@ -224,7 +238,7 @@ describe('Sign in with TOTP code page', () => { ); expect(fxaLoginSpy).toHaveBeenCalled(); expect(hardNavigateSpy).toHaveBeenCalledWith( - '/pair?showSuccessMessage=true', + '/pair?showSuccessMessage=true&pairReason=password_login', undefined, undefined, true @@ -578,6 +592,65 @@ describe('Sign in with TOTP code page', () => { ); }); + // Without this, /pair tags cad_firefox.choice_view as password_login for a + // user who actually signed in with an email OTP code. + it('forwards isPasswordlessOtpSignin to handleNavigation for an OTP sign-in', async () => { + const user = userEvent.setup(); + const handleNavigationSpy = jest + .spyOn(SigninUtils, 'handleNavigation') + .mockResolvedValue({ error: undefined }); + const submitTotpCode = jest.fn().mockResolvedValue({ error: undefined }); + + renderWithLocalizationProvider( + + + + ); + await user.type(screen.getByLabelText('Enter 6-digit code'), '123456'); + await user.click(screen.getByRole('button', { name: 'Confirm' })); + + await waitFor(() => { + expect(handleNavigationSpy).toHaveBeenCalledWith( + expect.objectContaining({ isPasswordlessOtpSignin: true }) + ); + }); + }); + + it('leaves isPasswordlessOtpSignin unset for a password sign-in', async () => { + const user = userEvent.setup(); + const handleNavigationSpy = jest + .spyOn(SigninUtils, 'handleNavigation') + .mockResolvedValue({ error: undefined }); + const submitTotpCode = jest.fn().mockResolvedValue({ error: undefined }); + + renderWithLocalizationProvider( + + + + ); + await user.type(screen.getByLabelText('Enter 6-digit code'), '123456'); + await user.click(screen.getByRole('button', { name: 'Confirm' })); + + await waitFor(() => { + expect(handleNavigationSpy).toHaveBeenCalledWith( + expect.objectContaining({ isPasswordlessOtpSignin: undefined }) + ); + }); + }); + it('redirects a third-party-auth sign-in to set_password when keys are not optional', async () => { const user = userEvent.setup(); const handleNavigationSpy = jest diff --git a/packages/fxa-settings/src/pages/Signin/SigninTotpCode/index.tsx b/packages/fxa-settings/src/pages/Signin/SigninTotpCode/index.tsx index 92dc5f0ad3c..416c51af24a 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninTotpCode/index.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninTotpCode/index.tsx @@ -193,6 +193,7 @@ export const SigninTotpCode = ({ handleFxaLogin: true, handleFxaOAuthLogin: true, performNavigation: !integration.isFirefoxMobileClient(), + isPasswordlessOtpSignin: signinState.isPasswordlessOtpSignin, authClient, }; diff --git a/packages/fxa-settings/src/pages/Signin/index.test.tsx b/packages/fxa-settings/src/pages/Signin/index.test.tsx index 8d9ab6f6f95..41b320f8e94 100644 --- a/packages/fxa-settings/src/pages/Signin/index.test.tsx +++ b/packages/fxa-settings/src/pages/Signin/index.test.tsx @@ -620,7 +620,9 @@ describe('Signin component', () => { await enterPasswordAndSubmit(); await waitFor(() => { expect(navigate).toHaveBeenCalledWith( - '/inline_recovery_key_setup?', + // pairReason rides along so it survives the later hard + // navigation from this interstitial to /pair. + '/inline_recovery_key_setup?pairReason=password_login', { replace: true, state: { @@ -687,7 +689,7 @@ describe('Signin component', () => { }); }); expect(hardNavigateSpy).toHaveBeenCalledWith( - '/pair?showSuccessMessage=true', + '/pair?showSuccessMessage=true&pairReason=password_login', undefined, undefined, false @@ -858,7 +860,7 @@ describe('Signin component', () => { expect(fxaLoginCallOrder).toBeLessThan(fxaOAuthLoginCallOrder); expect(hardNavigateSpy).toHaveBeenCalledWith( - '/pair?showSuccessMessage=true', + '/pair?showSuccessMessage=true&pairReason=password_login', undefined, undefined, true diff --git a/packages/fxa-settings/src/pages/Signin/interfaces.ts b/packages/fxa-settings/src/pages/Signin/interfaces.ts index e9a12c776c4..6031c3b6811 100644 --- a/packages/fxa-settings/src/pages/Signin/interfaces.ts +++ b/packages/fxa-settings/src/pages/Signin/interfaces.ts @@ -15,6 +15,8 @@ import { Integration } from '../../models'; import { QueryParams } from '../..'; import { UseFxAStatusResult } from '../../lib/hooks/useFxAStatus'; import AuthClient from 'fxa-auth-client/browser'; +import type { PairGleanReason } from 'fxa-shared/metrics/glean/pair-reasons'; +import type { PasswordCreationReason } from '../PostVerify/SetPassword/interfaces'; export interface AvatarResponse { account: { @@ -292,6 +294,11 @@ export interface NavigationOptions { // True when the session was established by a passkey assertion; pairs // with accountHasTotp to drive the AAL2-RP TOTP redirect in utils.ts. isPasskeySession?: boolean; + // Set by PostVerify/SetPassword. Because Sync always needs encryption keys, + // passwordless OTP and passkey sign-ins are routed through that page before + // they can reach /pair, so this is the only surviving record of how the + // session was established. Drives the /pair `choice_view` reason. + passwordCreationReason?: PasswordCreationReason; accountHasTotp?: boolean; authClient: Pick; } @@ -314,6 +321,11 @@ export interface SigninLocationState { isSessionAALUpgrade?: boolean; isSignInWithThirdPartyAuth?: boolean; isPasswordlessOtpSignin?: boolean; + /** + * Set by `getSyncNavigate` when routing to the React /pair choice screen. + * Read there to tag `cad_firefox.choice_view` with the originating flow. + */ + pairReason?: PairGleanReason; /** * Sign-in surface the user came from before reaching SigninPasskeyFallback. * Used to populate the `reason` extra on `passkey_enter_password.*` Glean diff --git a/packages/fxa-settings/src/pages/Signin/utils.test.ts b/packages/fxa-settings/src/pages/Signin/utils.test.ts index 954cbf71e01..e0a9983fdf0 100644 --- a/packages/fxa-settings/src/pages/Signin/utils.test.ts +++ b/packages/fxa-settings/src/pages/Signin/utils.test.ts @@ -87,6 +87,61 @@ describe('Signin utils', () => { ...overrides, }) as NavigationOptions; + // End-to-end coverage of the pass-through from NavigationOptions into the + // /pair `choice_view` reason. Without these, the options could be dropped + // anywhere between handleNavigation and getSyncNavigate and every other + // test in this file would still pass (FXA-14133). + describe('pair reason pass-through', () => { + const navigateToPair = async (overrides: Partial) => { + await handleNavigation( + createBaseNavigationOptions({ + integration: createMockSigninOAuthNativeSyncIntegration(), + performNavigation: true, + ...overrides, + }) + ); + return mockNavigate.mock.calls[0]?.[1]?.state?.pairReason; + }; + + it('reports otp_login for a Sync OTP sign-in that created a password', async () => { + expect(await navigateToPair({ passwordCreationReason: 'otp' })).toBe( + 'otp_login' + ); + }); + + it('reports passkey_login for a Sync passkey sign-in that created a password', async () => { + expect( + await navigateToPair({ passwordCreationReason: 'passkey' }) + ).toBe('passkey_login'); + }); + + it('reports no reason for a third-party-auth password creation', async () => { + expect( + await navigateToPair({ passwordCreationReason: 'third_party_auth' }) + ).toBeUndefined(); + }); + + it('reports passkey_login for a passkey session that skipped set_password', async () => { + expect(await navigateToPair({ isPasskeySession: true })).toBe( + 'passkey_login' + ); + }); + + it('reports otp_login for an OTP session that skipped set_password', async () => { + expect(await navigateToPair({ isPasswordlessOtpSignin: true })).toBe( + 'otp_login' + ); + }); + + it('reports password_reg for a sign-up', async () => { + expect(await navigateToPair({ origin: 'signup' })).toBe('password_reg'); + }); + + it('reports password_login for a plain password sign-in', async () => { + expect(await navigateToPair({})).toBe('password_login'); + }); + }); + it('does not navigate if performNavigation is false', async () => { const navigationOptions = createBaseNavigationOptions({ integration: createMockSigninOAuthNativeSyncIntegration({ @@ -397,7 +452,9 @@ describe('Signin utils', () => { const result = await handleNavigation(navigationOptions); expect(result.error).toBeUndefined(); - expect(sessionResendVerifyCode).toHaveBeenCalledWith(MOCK_SESSION_TOKEN); + expect(sessionResendVerifyCode).toHaveBeenCalledWith( + MOCK_SESSION_TOKEN + ); expect(mockNavigate).toHaveBeenCalledWith( '/confirm_signup_code', expect.any(Object) @@ -859,6 +916,18 @@ describe('Signin utils', () => { }); expect(result.to).not.toContain('passwordCreated'); }); + + it('includes pairReason=password_login by default', () => { + const result = getSyncNavigate('?service=sync'); + expect(result.to).toContain('pairReason=password_login'); + }); + + it('includes pairReason=passkey_login for a passkey session', () => { + const result = getSyncNavigate('?service=sync', { + isPasskeySession: true, + }); + expect(result.to).toContain('pairReason=passkey_login'); + }); }); describe('/pair redirect (React path, pairRoutes=true)', () => { @@ -869,7 +938,10 @@ describe('Signin utils', () => { expect(result.to).toContain('/pair?'); expect(result.to).not.toContain('showSuccessMessage'); expect(result.shouldHardNavigate).toBe(false); - expect(result.locationState).toEqual({ origin: 'signin' }); + expect(result.locationState).toEqual({ + origin: 'signin', + pairReason: 'password_login', + }); }); it('soft-navs with origin=signup when signupSuccess', () => { @@ -878,17 +950,22 @@ describe('Signin utils', () => { }); expect(result.to).not.toContain('signupSuccess'); expect(result.shouldHardNavigate).toBe(false); - expect(result.locationState).toEqual({ origin: 'signup' }); + expect(result.locationState).toEqual({ + origin: 'signup', + pairReason: 'password_reg', + }); }); it('soft-navs with origin=post-verify-set-password when set-password flow', () => { const result = getSyncNavigate('?service=sync', { origin: 'post-verify-set-password', + passwordCreationReason: 'otp', }); expect(result.to).not.toContain('passwordCreated'); expect(result.shouldHardNavigate).toBe(false); expect(result.locationState).toEqual({ origin: 'post-verify-set-password', + pairReason: 'otp_login', }); }); @@ -897,6 +974,98 @@ describe('Signin utils', () => { expect(result.to).toBe('/pair'); expect(result.shouldHardNavigate).toBe(false); }); + + // Keeps the user's auth method out of history and server access logs on + // the soft-nav path; router state carries it instead. + it('does not put pairReason in the URL when soft-navigating to /pair', () => { + const result = getSyncNavigate('?service=sync', { + isPasskeySession: true, + }); + expect(result.to).not.toContain('pairReason'); + expect(result.locationState?.pairReason).toBe('passkey_login'); + }); + + describe('pairReason location state', () => { + it('is password_reg for a sign-up, even with a passkey session', () => { + const result = getSyncNavigate('?service=sync', { + origin: 'signup', + isPasskeySession: true, + }); + expect(result.locationState?.pairReason).toBe('password_reg'); + }); + + it.each([ + ['passkey', 'passkey_login'], + ['otp', 'otp_login'], + ] as const)( + 'is %s -> %s from passwordCreationReason', + (passwordCreationReason, expected) => { + const result = getSyncNavigate('?service=sync', { + passwordCreationReason, + }); + expect(result.locationState?.pairReason).toBe(expected); + } + ); + + it('is undefined for third-party-auth password creation', () => { + const result = getSyncNavigate('?service=sync', { + passwordCreationReason: 'third_party_auth', + }); + expect(result.locationState?.pairReason).toBeUndefined(); + }); + + it('is passkey_login for a passkey session', () => { + const result = getSyncNavigate('?service=sync', { + isPasskeySession: true, + }); + expect(result.locationState?.pairReason).toBe('passkey_login'); + }); + + it('is otp_login for a passwordless OTP sign-in', () => { + const result = getSyncNavigate('?service=sync', { + isPasswordlessOtpSignin: true, + }); + expect(result.locationState?.pairReason).toBe('otp_login'); + }); + + it('is passkey_login when both passkey and OTP flags are set', () => { + const result = getSyncNavigate('?service=sync', { + isPasskeySession: true, + isPasswordlessOtpSignin: true, + }); + expect(result.locationState?.pairReason).toBe('passkey_login'); + }); + }); + + // Interstitials reach /pair via hardNavigate('/pair', {}, true), which + // forwards the current query string — so the reason has to be on their + // URL or it is lost for every flow that stops at one (FXA-14133). + describe('interstitial hand-off', () => { + it('carries pairReason onto /signup_confirmed_sync', () => { + const result = getSyncNavigate('?service=sync', { + showSignupConfirmedSync: true, + passwordCreationReason: 'otp', + }); + expect(result.to).toContain('/signup_confirmed_sync?'); + expect(result.to).toContain('pairReason=otp_login'); + }); + + it('carries pairReason onto /inline_recovery_key_setup', () => { + const result = getSyncNavigate('?service=sync', { + showInlineRecoveryKeySetup: true, + }); + expect(result.to).toContain('/inline_recovery_key_setup?'); + expect(result.to).toContain('pairReason=password_login'); + }); + + it('omits pairReason on an interstitial when the flow has no bucket', () => { + const result = getSyncNavigate('?service=sync', { + showSignupConfirmedSync: true, + passwordCreationReason: 'third_party_auth', + }); + expect(result.to).not.toContain('pairReason'); + }); + }); }); }); }); diff --git a/packages/fxa-settings/src/pages/Signin/utils.ts b/packages/fxa-settings/src/pages/Signin/utils.ts index 9ad5c3a5032..f08e6321dbc 100644 --- a/packages/fxa-settings/src/pages/Signin/utils.ts +++ b/packages/fxa-settings/src/pages/Signin/utils.ts @@ -6,7 +6,11 @@ import type { NavigateFunction } from 'react-router'; import VerificationMethods from '../../constants/verification-methods'; import VerificationReasons from '../../constants/verification-reasons'; import { NavigationOptions, SigninLocationState } from './interfaces'; -import type { SetPasswordLocationState } from '../PostVerify/SetPassword/interfaces'; +import type { PairGleanReason } from 'fxa-shared/metrics/glean/pair-reasons'; +import type { + PasswordCreationReason, + SetPasswordLocationState, +} from '../PostVerify/SetPassword/interfaces'; import { AuthUiError, AuthUiErrors } from '../../lib/auth-errors/auth-errors'; import { isOAuthIntegration, @@ -51,6 +55,59 @@ interface SyncNavigateOptions { syncHidePromoAfterLogin?: boolean; signupSuccess?: boolean; origin?: PairOrigin; + isPasswordlessOtpSignin?: boolean; + isPasskeySession?: boolean; + passwordCreationReason?: PasswordCreationReason; +} + +/** + * Resolves the flow that landed the user on the /pair choice screen so + * `cad_firefox.choice_view` can be split by originating flow. + * + * `passwordCreationReason` is the authoritative signal for anything arriving + * via /post_verify/set_password. Sync always requires encryption keys, so a + * passwordless OTP or passkey sign-in is routed through that page to create a + * password before it can ever reach /pair — meaning the raw session flags below + * are only observable on the flows that skip it. + * + * Registration wins over the method flags: a Sync sign-up always ends with a + * password, and there the method flags describe a step *inside* registration. + * + * Returns undefined for flows with no sanctioned bucket (third-party auth), so + * they record an empty reason rather than inflating `password_login`. + */ +function getPairGleanReason({ + signupSuccess, + origin, + isPasskeySession, + isPasswordlessOtpSignin, + isSignInWithThirdPartyAuth, + passwordCreationReason, +}: Pick< + SyncNavigateOptions, + | 'signupSuccess' + | 'origin' + | 'isPasskeySession' + | 'isPasswordlessOtpSignin' + | 'isSignInWithThirdPartyAuth' + | 'passwordCreationReason' +>): PairGleanReason | undefined { + if (signupSuccess || origin === 'signup') { + return 'password_reg'; + } + if (passwordCreationReason === 'passkey' || isPasskeySession) { + return 'passkey_login'; + } + if (passwordCreationReason === 'otp' || isPasswordlessOtpSignin) { + return 'otp_login'; + } + if ( + passwordCreationReason === 'third_party_auth' || + isSignInWithThirdPartyAuth + ) { + return undefined; + } + return 'password_login'; } export function getSyncNavigate( @@ -62,15 +119,41 @@ export function getSyncNavigate( syncHidePromoAfterLogin, signupSuccess, origin, + isPasswordlessOtpSignin, + isPasskeySession, + passwordCreationReason, }: SyncNavigateOptions = {} ): { to: string; shouldHardNavigate: boolean; - locationState?: Pick & + locationState?: Pick & Pick; } { const searchParams = new URLSearchParams(queryParams); + // This is used for pages that reach /pair via `hardNavigate('/pair', {}, true)`, + // which forwards the current query string — without this the reason would be lost. + const pairReason = getPairGleanReason({ + signupSuccess, + origin, + isPasskeySession, + isPasswordlessOtpSignin, + isSignInWithThirdPartyAuth, + passwordCreationReason, + }); + + // Only applied to destinations that /pair reaches by hard navigation, where + // router state cannot survive. The React /pair soft-nav deliberately keeps the + // reason out of the URL. + const withPairReason = () => { + if (!pairReason) { + return searchParams; + } + const params = new URLSearchParams(searchParams); + params.set('pairReason', pairReason); + return params; + }; + if (isSignInWithThirdPartyAuth) { return { to: `/post_verify/set_password?${searchParams}`, @@ -84,14 +167,14 @@ export function getSyncNavigate( if (showInlineRecoveryKeySetup) { return { - to: `/inline_recovery_key_setup?${searchParams}`, + to: `/inline_recovery_key_setup?${withPairReason()}`, shouldHardNavigate: false, }; } if (showSignupConfirmedSync) { return { - to: `/signup_confirmed_sync?${searchParams}`, + to: `/signup_confirmed_sync?${withPairReason()}`, shouldHardNavigate: false, }; } @@ -117,7 +200,7 @@ export function getSyncNavigate( return { to, shouldHardNavigate: false, - locationState: { origin: pairOrigin }, + locationState: { origin: pairOrigin, pairReason }, }; } @@ -128,6 +211,9 @@ export function getSyncNavigate( if (origin === 'post-verify-set-password') { searchParams.set('passwordCreated', 'true'); } + if (pairReason) { + searchParams.set('pairReason', pairReason); + } return { to: `/pair?${searchParams}`, shouldHardNavigate: true, @@ -298,7 +384,8 @@ export async function handleNavigation(navigationOptions: NavigationOptions) { // unverified email) and we know their session isn't fully verified, then send them // an otp code. Sending here couples the email with the actual navigation action. if ( - (to?.includes('signin_token_code') || to?.includes('confirm_signup_code')) && + (to?.includes('signin_token_code') || + to?.includes('confirm_signup_code')) && navigationOptions.signinData.sessionToken && navigationOptions.signinData.verificationMethod === VerificationMethods.EMAIL_OTP @@ -332,7 +419,8 @@ export async function handleNavigation(navigationOptions: NavigationOptions) { return { error }; } if (to) { - performNavigation({ navigate, + performNavigation({ + navigate, to, locationState, shouldHardNavigate, @@ -368,7 +456,8 @@ export async function handleNavigation(navigationOptions: NavigationOptions) { if (navigationOptions.performNavigation !== false) { const { to, locationState, shouldHardNavigate } = await getNonOAuthNavigationTarget(navigationOptions); - performNavigation({ navigate, + performNavigation({ + navigate, to, locationState, shouldHardNavigate, @@ -409,7 +498,8 @@ export async function handleNavigation(navigationOptions: NavigationOptions) { if (to === '/post_verify/service_welcome') { navigate(to, { state: { origin: 'signin' }, replace: true }); } else { - performNavigation({ navigate, + performNavigation({ + navigate, to, locationState, shouldHardNavigate, @@ -539,6 +629,9 @@ const getNonOAuthNavigationTarget = async ( isSignInWithThirdPartyAuth, showSignupConfirmedSync, origin, + isPasswordlessOtpSignin, + isPasskeySession, + passwordCreationReason, } = navigationOptions; if (integration.isSync()) { const syncNav = getSyncNavigate(queryParams, { @@ -546,6 +639,9 @@ const getNonOAuthNavigationTarget = async ( isSignInWithThirdPartyAuth, showSignupConfirmedSync, origin, + isPasswordlessOtpSignin, + isPasskeySession, + passwordCreationReason, }); const locationState = createSigninLocationState(navigationOptions); return { @@ -654,6 +750,9 @@ const getOAuthNavigationTarget = async ( showSignupConfirmedSync: navigationOptions.showSignupConfirmedSync, syncHidePromoAfterLogin: navigationOptions.syncHidePromoAfterLogin, origin: navigationOptions.origin, + isPasswordlessOtpSignin: navigationOptions.isPasswordlessOtpSignin, + isPasskeySession: navigationOptions.isPasskeySession, + passwordCreationReason: navigationOptions.passwordCreationReason, }); return { ...syncNav, diff --git a/packages/fxa-settings/src/pages/Signup/ConfirmSignupCode/index.tsx b/packages/fxa-settings/src/pages/Signup/ConfirmSignupCode/index.tsx index cec4b17d542..961bd3b3851 100644 --- a/packages/fxa-settings/src/pages/Signup/ConfirmSignupCode/index.tsx +++ b/packages/fxa-settings/src/pages/Signup/ConfirmSignupCode/index.tsx @@ -213,6 +213,7 @@ const ConfirmSignupCode = ({ if (isSyncDesktopV3Integration(integration)) { const { to } = getSyncNavigate(location.search, { showSignupConfirmedSync: true, + origin: 'signup', }); navigate(to); } else if (isOAuthIntegration(integration)) { @@ -274,6 +275,10 @@ const ConfirmSignupCode = ({ { showSignupConfirmedSync: !isSendTab, signupSuccess: isSendTab, + // Needed for the !isSendTab branch: signupSuccess is false + // there, so without this the /pair reason would fall through + // to password_login instead of password_reg. + origin: 'signup', } ); if (shouldHardNavigate) { diff --git a/packages/fxa-shared/metrics/glean/fxa-ui-metrics.yaml b/packages/fxa-shared/metrics/glean/fxa-ui-metrics.yaml index 46e02f51bf3..2e2be7d81ea 100644 --- a/packages/fxa-shared/metrics/glean/fxa-ui-metrics.yaml +++ b/packages/fxa-shared/metrics/glean/fxa-ui-metrics.yaml @@ -2294,12 +2294,18 @@ cad_firefox: - fxa-staff@mozilla.com bugs: - https://mozilla-hub.atlassian.net/browse/FXA-9607 + - https://mozilla-hub.atlassian.net/browse/FXA-14133 data_reviews: - https://bugzilla.mozilla.org/show_bug.cgi?id=1830504 - https://bugzilla.mozilla.org/show_bug.cgi?id=1844121 expires: never data_sensitivity: - interaction + extra_keys: + reason: + description: | + The sign-in or registration flow that redirected the user to the choice screen. See PAIR_GLEAN_REASONS in fxa-shared/metrics/glean/pair-reasons.ts for the permitted values. Empty when the screen is reached directly (e.g. the pair link on a success page) or from a flow with no bucket of its own, such as third-party auth. + type: string choice_engage: type: event description: | diff --git a/packages/fxa-shared/metrics/glean/pair-reasons.ts b/packages/fxa-shared/metrics/glean/pair-reasons.ts new file mode 100644 index 00000000000..c518fcb9a2a --- /dev/null +++ b/packages/fxa-shared/metrics/glean/pair-reasons.ts @@ -0,0 +1,31 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * 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/. */ + +/** + * Flows that redirect a user to the /pair choice screen, recorded as the + * `reason` extra on `cad_firefox.choice_view` so the screen's funnel can be + * split by originating flow (FXA-14133). + * + * Single source of truth for both /pair implementations: fxa-settings reads the + * value from router state, and the Backbone view in fxa-content-server + * validates it out of a query param. Adding a value here is all that is needed + * for both to accept it — but remember to update the `reason` description on + * `cad_firefox.choice_view` in fxa-ui-metrics.yaml too. + * + * Flows with no entry here (third-party auth, cached-credential sign-in) + * deliberately record no reason rather than being folded into a bucket they + * don't belong to. + */ +export const PAIR_GLEAN_REASONS = [ + 'password_login', + 'password_reg', + 'otp_login', + 'passkey_login', +] as const; + +export type PairGleanReason = (typeof PAIR_GLEAN_REASONS)[number]; + +export const isPairGleanReason = (value: unknown): value is PairGleanReason => + typeof value === 'string' && + (PAIR_GLEAN_REASONS as readonly string[]).includes(value); diff --git a/packages/fxa-shared/metrics/glean/web/cadFirefox.ts b/packages/fxa-shared/metrics/glean/web/cadFirefox.ts index ddb50510f93..00d5a7fe0fe 100644 --- a/packages/fxa-shared/metrics/glean/web/cadFirefox.ts +++ b/packages/fxa-shared/metrics/glean/web/cadFirefox.ts @@ -68,7 +68,9 @@ export const choiceSubmit = new EventMetricType<{ * * Generated from `cad_firefox.choice_view`. */ -export const choiceView = new EventMetricType( +export const choiceView = new EventMetricType<{ + reason?: string; +}>( { category: 'cad_firefox', name: 'choice_view', @@ -76,7 +78,7 @@ export const choiceView = new EventMetricType( lifetime: 'ping', disabled: false, }, - [] + ['reason'] ); /**