From 335813b3447e259b1147a665ac76ed67b1dbcca1 Mon Sep 17 00:00:00 2001 From: Valerie Pomerleau Date: Fri, 3 Jul 2026 09:32:27 -0700 Subject: [PATCH 1/3] fix(passkeys): build signin metricsContext from flowQueryParams Because: - The passkey sign-in ceremony built its metricsContext from a live location.search read, which can drop flow params the SPA strips after page load. The server login.complete (reason=passkey) then carried a flow_id that no longer matched the client passkey metrics, so the events failed to join and passkey sign-in conversion was unmeasurable. This commit: - Sources the passkey metricsContext from the flowQueryParams snapshot App froze at load and feeds to Glean, mirroring SigninPasskeyFallback, so the server flow_id matches the client passkey metrics. - Threads flowQueryParams through usePasskeySignIn and its four callers. - Adds a regression test asserting metricsContext is built from flowQueryParams, not the live location.search. Closes #FXA-14093 --- .../src/lib/passkeys/signin-flow.test.tsx | 34 +++++++++++++++++++ .../src/lib/passkeys/signin-flow.ts | 11 +++--- .../fxa-settings/src/pages/Index/index.tsx | 1 + .../Signin/SigninPasswordlessCode/index.tsx | 1 + .../SigninAlternativeAuthOptions/index.tsx | 1 + .../fxa-settings/src/pages/Signin/index.tsx | 1 + 6 files changed, 45 insertions(+), 4 deletions(-) diff --git a/packages/fxa-settings/src/lib/passkeys/signin-flow.test.tsx b/packages/fxa-settings/src/lib/passkeys/signin-flow.test.tsx index 45b8fb3befc..dfab426b285 100644 --- a/packages/fxa-settings/src/lib/passkeys/signin-flow.test.tsx +++ b/packages/fxa-settings/src/lib/passkeys/signin-flow.test.tsx @@ -15,6 +15,8 @@ import { getCredential, isWebAuthnLevel3Supported } from './webauthn'; import { storeAccountData } from '../storage-utils'; import { AuthUiErrors } from '../auth-errors/auth-errors'; import GleanMetrics from '../glean'; +import { queryParamsToMetricsContext } from '../metrics'; +import type { QueryParams } from '../..'; import { IntegrationType } from '../../models'; import { OAuthNativeServices } from '@fxa/accounts/oauth'; import { @@ -113,6 +115,11 @@ const MOCK_CREDENTIAL = { response: {}, clientExtensionResults: {}, }; +const MOCK_FLOW_QUERY_PARAMS = { + flowId: 'f'.repeat(64), + flowBeginTime: '1700000000000', + deviceId: 'd'.repeat(32), +} as unknown as QueryParams; const buildArgs = ( overrides: Partial[0]> = {} @@ -814,6 +821,33 @@ describe('usePasskeySignIn', () => { expect(Sentry.captureException as jest.Mock).not.toHaveBeenCalled(); }); + describe('metricsContext flow identity', () => { + it('builds metricsContext from flowQueryParams, not the live location.search', async () => { + // location.search carries a different flow; it must be ignored so the + // server login.complete joins the client passkey metrics on flow_id. + const { args, spies } = buildArgs({ + queryParams: + '?flowId=stale-url-flow&flowBeginTime=1&deviceId=stale-url-device', + flowQueryParams: MOCK_FLOW_QUERY_PARAMS, + }); + + const { result } = renderHook(() => usePasskeySignIn(args)); + await act(async () => { + await result.current.onClick(); + }); + + expect(spies.completePasskeyAuthentication).toHaveBeenCalledWith( + MOCK_CREDENTIAL, + CHALLENGE, + { + service: 'service-id', + keysRequired: false, + metricsContext: queryParamsToMetricsContext(MOCK_FLOW_QUERY_PARAMS), + } + ); + }); + }); + describe('Glean metrics', () => { it.each([ ['emailfirst' as const, 'emailfirst'], diff --git a/packages/fxa-settings/src/lib/passkeys/signin-flow.ts b/packages/fxa-settings/src/lib/passkeys/signin-flow.ts index 01e4ee061e3..20b470780e3 100644 --- a/packages/fxa-settings/src/lib/passkeys/signin-flow.ts +++ b/packages/fxa-settings/src/lib/passkeys/signin-flow.ts @@ -26,7 +26,7 @@ import { } from '../../pages/Signin/utils'; import { resolveServiceOrClientId } from '../../models/integrations/utils'; import { queryParamsToMetricsContext } from '../metrics'; -import { searchParams } from '../utilities'; +import type { QueryParams } from '../..'; import { getCredential, handleWebAuthnError, @@ -210,6 +210,9 @@ export interface UsePasskeySignInArgs { ftlMsgResolver: FtlMsgResolver; navigateWithQuery: ReturnType; queryParams: string; + // Frozen load-time flow snapshot for the metrics join; not the live + // `queryParams` above, which the SPA may strip of flow params after load. + flowQueryParams?: QueryParams; surface: PasskeySignInSurface; isButtonVisible?: boolean; supportsKeysOptionalLogin?: boolean; @@ -228,6 +231,7 @@ export function usePasskeySignIn({ ftlMsgResolver, navigateWithQuery, queryParams, + flowQueryParams = {}, surface, isButtonVisible = false, supportsKeysOptionalLogin, @@ -326,9 +330,7 @@ export function usePasskeySignIn({ } const serviceForRequest = resolvePasskeyService(integration); - const metricsContext = queryParamsToMetricsContext( - searchParams(queryParams) as Record - ); + const metricsContext = queryParamsToMetricsContext(flowQueryParams); const keysRequired = integration.requiresPasswordForLogin( supportsKeysOptionalLogin @@ -484,6 +486,7 @@ export function usePasskeySignIn({ ftlMsgResolver, navigateWithQuery, queryParams, + flowQueryParams, setLocalizedError, surface, supportsKeysOptionalLogin, diff --git a/packages/fxa-settings/src/pages/Index/index.tsx b/packages/fxa-settings/src/pages/Index/index.tsx index 3c6f5e0900f..906e96d50f3 100644 --- a/packages/fxa-settings/src/pages/Index/index.tsx +++ b/packages/fxa-settings/src/pages/Index/index.tsx @@ -66,6 +66,7 @@ export const Index = ({ ftlMsgResolver, navigateWithQuery, queryParams: location.search, + flowQueryParams, surface: 'emailfirst', isButtonVisible: showPasskeySignin, supportsKeysOptionalLogin: useFxAStatusResult.supportsKeysOptionalLogin, diff --git a/packages/fxa-settings/src/pages/Signin/SigninPasswordlessCode/index.tsx b/packages/fxa-settings/src/pages/Signin/SigninPasswordlessCode/index.tsx index 258742ab7ef..c2542c62b44 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninPasswordlessCode/index.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninPasswordlessCode/index.tsx @@ -97,6 +97,7 @@ const SigninPasswordlessCode = ({ ftlMsgResolver, navigateWithQuery, queryParams: location.search, + flowQueryParams, surface: 'login_otp', isButtonVisible: showPasskeySignin, supportsKeysOptionalLogin: useFxAStatusResult.supportsKeysOptionalLogin, diff --git a/packages/fxa-settings/src/pages/Signin/components/SigninAlternativeAuthOptions/index.tsx b/packages/fxa-settings/src/pages/Signin/components/SigninAlternativeAuthOptions/index.tsx index c51d0320636..e85c7c611e5 100644 --- a/packages/fxa-settings/src/pages/Signin/components/SigninAlternativeAuthOptions/index.tsx +++ b/packages/fxa-settings/src/pages/Signin/components/SigninAlternativeAuthOptions/index.tsx @@ -60,6 +60,7 @@ const SigninAlternativeAuthOptions = ({ ftlMsgResolver, navigateWithQuery, queryParams: location.search, + flowQueryParams, surface: 'alternative_auth', isButtonVisible: showPasskeySignin, supportsKeysOptionalLogin, diff --git a/packages/fxa-settings/src/pages/Signin/index.tsx b/packages/fxa-settings/src/pages/Signin/index.tsx index 9a27d3b4b9a..e9adcf1ea01 100644 --- a/packages/fxa-settings/src/pages/Signin/index.tsx +++ b/packages/fxa-settings/src/pages/Signin/index.tsx @@ -76,6 +76,7 @@ const Signin = ({ ftlMsgResolver, navigateWithQuery, queryParams: location.search, + flowQueryParams, surface: 'login', isButtonVisible: showPasskeySignin, supportsKeysOptionalLogin, From 9e7272a0f656a2857b477bdc84f69b880eccc336 Mon Sep 17 00:00:00 2001 From: Valerie Pomerleau Date: Mon, 6 Jul 2026 09:25:06 -0700 Subject: [PATCH 2/3] chore(claude): add flow metrics context rule for fxa-settings Because: - The flowQueryParams-vs-live-location.search trap that broke passkey sign-in metrics (FXA-14093) is easy to reintroduce on any new metrics surface. This commit: - Adds .claude/rules/metrics-flow.md, auto-loaded when editing fxa-settings metrics/glean/passkeys/signin files, documenting that flow identity must come from the frozen flowQueryParams snapshot, not a live URL read. --- .claude/rules/metrics-flow.md | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 .claude/rules/metrics-flow.md diff --git a/.claude/rules/metrics-flow.md b/.claude/rules/metrics-flow.md new file mode 100644 index 00000000000..1b47ae645da --- /dev/null +++ b/.claude/rules/metrics-flow.md @@ -0,0 +1,31 @@ +--- +paths: + - "packages/fxa-settings/src/lib/metrics.ts" + - "packages/fxa-settings/src/lib/metrics-flow.ts" + - "packages/fxa-settings/src/lib/glean/**" + - "packages/fxa-settings/src/lib/passkeys/**" + - "packages/fxa-settings/src/pages/Index/**" + - "packages/fxa-settings/src/pages/Signin/**" +--- + +# FXA Settings — flow metrics context + +When building a `metricsContext` or forwarding flow identity (`flowId`, +`flowBeginTime`, `deviceId`) to the auth-server or Glean, source it from the +frozen `flowQueryParams` snapshot (threaded from `App`, the same value the Glean +SDK is initialized with) — **not** a live `location.search` read. + +The SPA can strip flow params from the URL after load, so a later +`location.search` read yields a `flow_id` that no longer matches the client's +Glean events. The server event (e.g. `login.complete`) then fails to join the +client funnel and conversion analytics silently break. (Incident: FXA-14093.) + +```ts +// Wrong — live URL read; may have lost flow params since load +queryParamsToMetricsContext(searchParams(location.search)); +// Right — frozen snapshot; matches the Glean SDK flow_id +queryParamsToMetricsContext(flowQueryParams); +``` + +`location.search` is still correct for navigation; the trap is metrics only. +Pattern: `lib/passkeys/signin-flow.ts`, `Signin/SigninPasskeyFallback/container.tsx`. From 89bc062ac4d37a0a9a4881f1f18455413bb41393 Mon Sep 17 00:00:00 2001 From: Valerie Pomerleau Date: Mon, 6 Jul 2026 12:16:10 -0700 Subject: [PATCH 3/3] fix(passkeys): accept undefined flowQueryParams in metricsContext Because: - The passkey hook defaulted flowQueryParams to a fresh {} each render, churning the onClick useCallback identity for callers that omit it (raised in PR review). This commit: - Widens queryParamsToMetricsContext to accept undefined (its MetricsContext constructor already treats it as empty), so an optional flowQueryParams forwards without a caller-side default. - Drops the hook's {} default, matching how the shared signin components already pass flowQueryParams through. --- packages/fxa-settings/src/lib/metrics.ts | 10 ++++++---- packages/fxa-settings/src/lib/passkeys/signin-flow.ts | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/fxa-settings/src/lib/metrics.ts b/packages/fxa-settings/src/lib/metrics.ts index 03d534add5a..f63ca57cb96 100644 --- a/packages/fxa-settings/src/lib/metrics.ts +++ b/packages/fxa-settings/src/lib/metrics.ts @@ -518,12 +518,14 @@ export function addExperiment(choice: string, group: string) { /** * Pick out the MetricsContext key-values from URL query params. Accepts either * a raw query-param record or a typed {@link QueryParams} object so callers can - * pass `flowQueryParams` directly. The typed shape declares a few fields as - * number/boolean, but at runtime every value is a string parsed from the URL, - * so it's read through the same string-record contract the constructor expects. + * pass `flowQueryParams` directly, or `undefined` (treated as empty), so an + * optional `flowQueryParams` can be forwarded without a caller-side default. + * The typed shape declares a few fields as number/boolean, but at runtime every + * value is a string parsed from the URL, so it's read through the same + * string-record contract the constructor expects. */ export function queryParamsToMetricsContext( - queryParams: Record | QueryParams + queryParams?: Record | QueryParams ): Partial { const context = new MetricsContext( queryParams as Record diff --git a/packages/fxa-settings/src/lib/passkeys/signin-flow.ts b/packages/fxa-settings/src/lib/passkeys/signin-flow.ts index 20b470780e3..0e5e26b59b6 100644 --- a/packages/fxa-settings/src/lib/passkeys/signin-flow.ts +++ b/packages/fxa-settings/src/lib/passkeys/signin-flow.ts @@ -231,7 +231,7 @@ export function usePasskeySignIn({ ftlMsgResolver, navigateWithQuery, queryParams, - flowQueryParams = {}, + flowQueryParams, surface, isButtonVisible = false, supportsKeysOptionalLogin,