From 4eedb67979a4886c58ea3d18c856218cf6727e8e Mon Sep 17 00:00:00 2001 From: Lauren Zugai Date: Tue, 16 Dec 2025 18:52:00 -0600 Subject: [PATCH] fix(sync): Only send fxaStatus if UA is likely Fx, update unsupported context list Because: * We have some Sentry events showing users not using Firefox but with valid Sync query params. We should not send fxaStatus in this case. Other events show some users on fx_ios_v1 context, which is unsupported This commit: * Moves the existing isProbablyFirefox check into a helper to reference also in the useFxaStatus hook * Adds fx_ios_v1 to unsupported contexts closes FXA-12760 --- .../fxa-settings/src/components/App/index.tsx | 7 ++--- .../src/lib/hooks/useFxAStatus/index.test.tsx | 28 +++++++++++++++++-- .../src/lib/hooks/useFxAStatus/index.tsx | 3 +- .../src/models/integrations/index.ts | 1 + .../src/models/integrations/utils.ts | 11 ++++++++ 5 files changed, 42 insertions(+), 8 deletions(-) diff --git a/packages/fxa-settings/src/components/App/index.tsx b/packages/fxa-settings/src/components/App/index.tsx index db24655c50..a116583f9f 100644 --- a/packages/fxa-settings/src/components/App/index.tsx +++ b/packages/fxa-settings/src/components/App/index.tsx @@ -30,6 +30,7 @@ import { useIntegration, useLocalSignedInQueryState, useSession, + isProbablyFirefox, } from '../../models'; import { initializeSettingsContext, @@ -216,12 +217,8 @@ export const App = ({ // Request and update account data/state to match the browser state. // If there is a user actively signed into the browser, // we should try to use that user's account when possible. - const ua = navigator.userAgent.toLowerCase(); - // This may not catch all Firefox browsers notably iOS devices, see FXA-11520 for alternate approach - const isProbablyFirefox = ua.includes('firefox') || ua.includes('fxios'); - let userFromBrowser; - if (isProbablyFirefox) { + if (isProbablyFirefox()) { userFromBrowser = await firefox.requestSignedInUser( integration.data.context || '', // TODO with React pairing flow, update this if pairing flow diff --git a/packages/fxa-settings/src/lib/hooks/useFxAStatus/index.test.tsx b/packages/fxa-settings/src/lib/hooks/useFxAStatus/index.test.tsx index 472ca8d346..f690a85444 100644 --- a/packages/fxa-settings/src/lib/hooks/useFxAStatus/index.test.tsx +++ b/packages/fxa-settings/src/lib/hooks/useFxAStatus/index.test.tsx @@ -6,7 +6,7 @@ import { renderHook } from '@testing-library/react-hooks'; import { useFxAStatus } from '.'; import { Constants } from '../../constants'; import firefox from '../../channels/firefox'; -import { IntegrationType } from '../../../models'; +import { IntegrationType, isProbablyFirefox } from '../../../models'; jest.mock('../../channels/firefox', () => ({ __esModule: true, @@ -15,9 +15,18 @@ jest.mock('../../channels/firefox', () => ({ }, })); +jest.mock('../../../models', () => { + const actual = jest.requireActual('../../../models'); + return { + ...actual, + isProbablyFirefox: jest.fn(() => true), + }; +}); + describe('useFxAStatus', () => { beforeEach(() => { - jest.resetAllMocks(); + jest.clearAllMocks(); + (isProbablyFirefox as jest.Mock).mockImplementation(() => true); }); describe('SyncDesktopV3 integration', () => { @@ -149,4 +158,19 @@ describe('useFxAStatus', () => { expect(firefox.fxaStatus).not.toHaveBeenCalled(); }); }); + + describe('Non-Firefox browser', () => { + it('does not call fxaStatus when isProbablyFirefox returns false', () => { + (isProbablyFirefox as jest.Mock).mockReturnValueOnce(false); + + const integration = { + type: IntegrationType.SyncDesktopV3, + isSync: () => true, + isFirefoxNonSync: () => false, + }; + + renderHook(() => useFxAStatus(integration)); + expect(firefox.fxaStatus).not.toHaveBeenCalled(); + }); + }); }); diff --git a/packages/fxa-settings/src/lib/hooks/useFxAStatus/index.tsx b/packages/fxa-settings/src/lib/hooks/useFxAStatus/index.tsx index df23b24516..0b68073215 100644 --- a/packages/fxa-settings/src/lib/hooks/useFxAStatus/index.tsx +++ b/packages/fxa-settings/src/lib/hooks/useFxAStatus/index.tsx @@ -8,6 +8,7 @@ import { isOAuthIntegration, isSyncDesktopV3Integration, isOAuthNativeIntegration, + isProbablyFirefox, } from '../../../models'; import { defaultDesktopV3SyncEngineConfigs, @@ -45,7 +46,7 @@ export function useFxAStatus(integration: FxAStatusIntegration) { useEffect(() => { // This sends a web channel message to the browser to prompt a response // that we listen for. - if (isSync || isOAuthNative) { + if ((isSync || isOAuthNative) && isProbablyFirefox()) { (async () => { const status = await firefox.fxaStatus({ // TODO: Improve getting 'context', probably set this on the integration diff --git a/packages/fxa-settings/src/models/integrations/index.ts b/packages/fxa-settings/src/models/integrations/index.ts index 3c155d0a56..5df158967f 100644 --- a/packages/fxa-settings/src/models/integrations/index.ts +++ b/packages/fxa-settings/src/models/integrations/index.ts @@ -14,3 +14,4 @@ export * from './sync-desktop-v3-integration'; export * from './web-integration'; export * from './third-party-auth-callback-integration'; export * from './relier-interfaces'; +export * from './utils'; diff --git a/packages/fxa-settings/src/models/integrations/utils.ts b/packages/fxa-settings/src/models/integrations/utils.ts index e09cde7887..a9afaf2276 100644 --- a/packages/fxa-settings/src/models/integrations/utils.ts +++ b/packages/fxa-settings/src/models/integrations/utils.ts @@ -12,6 +12,7 @@ export function isFirefoxService(service?: string) { } const NO_LONGER_SUPPORTED_CONTEXTS = new Set([ + 'fx_ios_v1', 'fx_desktop_v1', 'fx_desktop_v2', 'fx_firstrun_v2', @@ -20,3 +21,13 @@ const NO_LONGER_SUPPORTED_CONTEXTS = new Set([ export function isUnsupportedContext(context?: string): boolean { return !!context && NO_LONGER_SUPPORTED_CONTEXTS.has(context); } + +/** + * Checks if the browser is probably Firefox based on the user agent string. + * This may not catch all Firefox browsers, notably iOS devices. + * See FXA-11520 for alternate approach. + */ +export function isProbablyFirefox(): boolean { + const ua = navigator.userAgent.toLowerCase(); + return ua.includes('firefox') || ua.includes('fxios'); +}