diff --git a/packages/fxa-settings/src/lib/channels/firefox.test.ts b/packages/fxa-settings/src/lib/channels/firefox.test.ts index 2bb17b9f5a7..3c3ce5eb88b 100644 --- a/packages/fxa-settings/src/lib/channels/firefox.test.ts +++ b/packages/fxa-settings/src/lib/channels/firefox.test.ts @@ -7,13 +7,14 @@ import { firefox, Firefox, FirefoxCommand, - buildSyncOAuthSearch, + buildOAuthSearch, DEFAULT_SEND_TIMEOUT_LENGTH_MS, FxAOAuthFlowBeginResponse, FxAStatusResponse, PAIR_OAUTH_TIMEOUT_MS, PairOAuthFinishState, PairOAuthStartState, + WebChannelService, } from './firefox'; describe('Firefox pairing WebChannel methods', () => { @@ -79,7 +80,7 @@ describe('Firefox pairing WebChannel methods', () => { }); }); -describe('buildSyncOAuthSearch', () => { +describe('buildOAuthSearch', () => { const MOCK_CODE_VERIFIER = 'au3dqDz2dOB0_vSikXCUf4S8Gc-37dL-F7sGxtxpR3R'; // Mirrors a real fxa_oauth_flow_begin response. Firefox derives the challenge @@ -102,7 +103,7 @@ describe('buildSyncOAuthSearch', () => { // would still pass if the allowlist were replaced by a spread. The response // type has no verifier field, but the payload crosses a trust boundary and // TypeScript is erased, so a compromised Firefox could put one on the wire. - const search = buildSyncOAuthSearch({ + const search = buildOAuthSearch({ ...OAUTH_PARAMS, code_verifier: MOCK_CODE_VERIFIER, sessionToken: 'deadbeef', @@ -125,6 +126,79 @@ describe('buildSyncOAuthSearch', () => { expect(search.get('code_challenge')).toBe(OAUTH_PARAMS.code_challenge); expect(search.toString()).not.toContain(MOCK_CODE_VERIFIER); }); + + // Precedence: the caller argument, then the browser echo, then sync. + it.each([ + { echoed: undefined, passed: undefined, expected: 'sync' }, + { echoed: undefined, passed: 'relay', expected: 'relay' }, + { echoed: 'vpn', passed: undefined, expected: 'vpn' }, + { echoed: 'vpn', passed: 'sync', expected: 'sync' }, + // The echo crosses a trust boundary and the type is erased, so an + // unrecognized name must not reach the sign-in URL. + { + echoed: 'evil' as WebChannelService, + passed: undefined, + expected: 'sync', + }, + ] as const)( + 'sets service=$expected when the browser echoes $echoed and the caller passes $passed', + ({ echoed, passed, expected }) => { + const search = buildOAuthSearch( + { ...OAUTH_PARAMS, service: echoed }, + passed + ); + expect(search.get('service')).toBe(expected); + } + ); +}); + +describe('fxaOAuthFlowBegin', () => { + const SCOPES = ['profile', Constants.OAUTH_OLDSYNC_SCOPE]; + + let ff: Firefox; + let sendSpy: jest.SpyInstance; + const originalRAF = window.requestAnimationFrame; + + beforeEach(() => { + jest.useFakeTimers(); + ff = new Firefox(); + sendSpy = jest.spyOn(ff, 'send').mockImplementation(() => {}); + window.requestAnimationFrame = (cb: FrameRequestCallback) => { + cb(0); + return 0; + }; + }); + + afterEach(() => { + jest.restoreAllMocks(); + window.requestAnimationFrame = originalRAF; + jest.useRealTimers(); + }); + + // Let the request time out so a pending promise cannot leak into the next case. + const settle = async (promise: Promise) => { + jest.runOnlyPendingTimers(); + await promise; + }; + + it.each(['sync', 'relay'] as const)( + 'sends service=%s with the scopes', + async (service) => { + const promise = ff.fxaOAuthFlowBegin(SCOPES, service); + expect(sendSpy).toHaveBeenCalledWith(FirefoxCommand.OAuthFlowBegin, { + scopes: SCOPES, + service, + }); + await settle(promise); + } + ); + + it('omits the service when the caller passes none', async () => { + const promise = ff.fxaOAuthFlowBegin(SCOPES); + // toStrictEqual, because toEqual would pass on a `service: undefined` key. + expect(sendSpy.mock.calls[0][1]).toStrictEqual({ scopes: SCOPES }); + await settle(promise); + }); }); describe('Firefox pairing OAuth WebChannel methods', () => { diff --git a/packages/fxa-settings/src/lib/channels/firefox.ts b/packages/fxa-settings/src/lib/channels/firefox.ts index 4d43d900690..912221548ae 100644 --- a/packages/fxa-settings/src/lib/channels/firefox.ts +++ b/packages/fxa-settings/src/lib/channels/firefox.ts @@ -150,6 +150,25 @@ export type WebChannelServices = vpn: {}; }; +// keyof a union yields only the shared keys, so distribute over it first. +type KeysOfUnion = T extends unknown ? keyof T : never; + +// Service names the WebChannel messages accept, derived from +// WebChannelServices so the two cannot drift apart. +export type WebChannelService = KeysOfUnion; + +// The same names at runtime. The browser echoes a service back over the +// WebChannel, where the type above is erased, so the echo needs a real check. +// The Record makes a missing name a compile error, not a silent rejection. +const WEB_CHANNEL_SERVICES = new Set( + Object.keys({ + sync: true, + relay: true, + smartwindow: true, + vpn: true, + } satisfies Record) +); + // ref: [FxAccounts.sys.mjs](https://searchfox.org/mozilla-central/rev/82828dba9e290914eddd294a0871533875b3a0b5/services/fxaccounts/FxAccounts.sys.mjs#910) export type FxALoginSignedInUserRequest = FxALoginRequest & { authAt: number; @@ -213,17 +232,24 @@ export type FxAOAuthFlowBeginResponse = { code_challenge_method?: string; // Forward to /authorization, otherwise the OAuth code is keyless and Sync never enables. keys_jwk?: string; + // Optional because the browser does not echo the service back yet. + service?: WebChannelService; }; -// Builds the oauth_webchannel_v1 Sync sign-in URL search params from the -// fxa_oauth_flow_begin response. Callers may set additional params on the -// returned URLSearchParams (e.g. entrypoint, email, utm_*). -export function buildSyncOAuthSearch( - oauthParams: FxAOAuthFlowBeginResponse +// Builds the oauth_webchannel_v1 sign-in URL search params from the +// fxa_oauth_flow_begin response. The service comes from the caller, then the +// browser echo, then sync. Callers may set additional params on the returned +// URLSearchParams (e.g. entrypoint, email, utm_*). +export function buildOAuthSearch( + oauthParams: FxAOAuthFlowBeginResponse, + service?: WebChannelService ): URLSearchParams { + const echoed = oauthParams.service; + const resolvedService = + service ?? (echoed && WEB_CHANNEL_SERVICES.has(echoed) ? echoed : 'sync'); const search = new URLSearchParams({ context: 'oauth_webchannel_v1', - service: 'sync', + service: resolvedService, client_id: oauthParams.client_id, state: oauthParams.state, scope: oauthParams.scope, @@ -519,7 +545,8 @@ export class Firefox extends EventTarget { /** Start new OAuth flow in Firefox and get fresh params for recovery. */ async fxaOAuthFlowBegin( - scopes: string[] + scopes: string[], + service?: WebChannelService ): Promise { let timeoutId: number; return Promise.race([ @@ -533,7 +560,12 @@ export class Firefox extends EventTarget { this.addEventListener(FirefoxCommand.OAuthFlowBegin, eventHandler); requestAnimationFrame(() => { - this.send(FirefoxCommand.OAuthFlowBegin, { scopes }); + // Omit the key when the caller has no service, rather than send + // undefined. + this.send(FirefoxCommand.OAuthFlowBegin, { + scopes, + ...(service ? { service } : {}), + }); }); }), new Promise((resolve) => { diff --git a/packages/fxa-settings/src/pages/ConnectAnotherDevice/index.test.tsx b/packages/fxa-settings/src/pages/ConnectAnotherDevice/index.test.tsx index 0e9f45a8635..ebdf44d902a 100644 --- a/packages/fxa-settings/src/pages/ConnectAnotherDevice/index.test.tsx +++ b/packages/fxa-settings/src/pages/ConnectAnotherDevice/index.test.tsx @@ -3,6 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ import { screen, waitFor } from '@testing-library/react'; +import { userEvent } from '@testing-library/user-event'; import * as ReactUtils from 'fxa-react/lib/utils'; import firefox from '../../lib/channels/firefox'; import { MOCK_ACCOUNT, renderWithRouter } from '../../models/mocks'; @@ -160,6 +161,52 @@ describe('ConnectAnotherDevice', () => { expect(screen.getByRole('button', { name: 'Sign in' })).toBeInTheDocument(); }); + describe('sync sign-in', () => { + let hardNavigate: jest.SpyInstance; + beforeEach(() => { + hardNavigate = jest + .spyOn(ReactUtils, 'hardNavigate') + .mockImplementation(() => {}); + }); + afterEach(() => { + jest.restoreAllMocks(); + (firefox.fxaOAuthFlowBegin as jest.Mock).mockReset(); + }); + + it('sends the user to a sync sign-in when they choose Sign in', async () => { + (firefox.fxaOAuthFlowBegin as jest.Mock).mockResolvedValue({ + action: 'email', + response_type: 'code', + access_type: 'offline', + scope: 'profile https://identity.mozilla.com/apps/oldsync', + client_id: 'cid-abc', + state: 'state-xyz', + }); + renderWithRouter( + + ); + await userEvent.click(screen.getByRole('button', { name: 'Sign in' })); + + await waitFor(() => expect(hardNavigate).toHaveBeenCalled()); + const url = new URL(hardNavigate.mock.calls[0][0], 'http://localhost'); + expect(url.searchParams.get('service')).toBe('sync'); + expect(firefox.fxaOAuthFlowBegin).toHaveBeenCalledWith( + ['profile', 'https://identity.mozilla.com/apps/oldsync'], + 'sync' + ); + }); + }); + it('renders device-specific messaging for Android', () => { renderWithRouter( => { const oauthParams = await firefox - .fxaOAuthFlowBegin(['profile', Constants.OAUTH_OLDSYNC_SCOPE]) + .fxaOAuthFlowBegin(['profile', Constants.OAUTH_OLDSYNC_SCOPE], 'sync') .catch(() => null); if (!oauthParams) return false; - const params = buildSyncOAuthSearch(oauthParams); + const params = buildOAuthSearch(oauthParams, 'sync'); // Underscore form: the CMS endpoint validator rejects ':' in entrypoint. params.set('entrypoint', 'fxa_connect_another_device'); if (email) params.set('email', email); 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 e9d92f2d902..f75eed771eb 100644 --- a/packages/fxa-settings/src/pages/Pair/Index/index.test.tsx +++ b/packages/fxa-settings/src/pages/Pair/Index/index.test.tsx @@ -48,8 +48,8 @@ jest.mock('../../../lib/channels/firefox', () => ({ requestSignedInUser: jest.fn(), fxaOAuthFlowBegin: jest.fn(), }, - buildSyncOAuthSearch: jest.requireActual('../../../lib/channels/firefox') - .buildSyncOAuthSearch, + buildOAuthSearch: jest.requireActual('../../../lib/channels/firefox') + .buildOAuthSearch, FirefoxCommand: { PairPreferences: 'fxaccounts:pair_preferences', }, @@ -418,10 +418,10 @@ describe('Pair', () => { requestSignedInUserMock.mockResolvedValue(response); renderWithRouter(); await waitFor(() => - expect(fxaOAuthFlowBeginMock).toHaveBeenCalledWith([ - 'profile', - 'https://identity.mozilla.com/apps/oldsync', - ]) + expect(fxaOAuthFlowBeginMock).toHaveBeenCalledWith( + ['profile', 'https://identity.mozilla.com/apps/oldsync'], + 'sync' + ) ); } ); @@ -900,21 +900,26 @@ describe('parseV2PairingHash', () => { it.each([ ['iOS Safari', IOS_SAFARI, true], ['Android Chrome', ANDROID_CHROME, false], - ])('routes to the download screen on %s', async (_label, ua, iosHandoff) => { - setUserAgent(ua); - renderWithRouter( - , - {}, - v2AppContext({ iosHandoff }) - ); + ])( + 'routes to the download screen on %s', + async (_label, ua, iosHandoff) => { + setUserAgent(ua); + renderWithRouter( + , + {}, + v2AppContext({ iosHandoff }) + ); - await waitFor(() => - expect(mockNavigate).toHaveBeenCalledWith( - '/pair/supplicant/download_firefox', - { state: { channelId: 'chan-1', channelKey: 'key-1', version: '2' } } - ) - ); - }); + await waitFor(() => + expect(mockNavigate).toHaveBeenCalledWith( + '/pair/supplicant/download_firefox', + { + state: { channelId: 'chan-1', channelKey: 'key-1', version: '2' }, + } + ) + ); + } + ); // Firefox iOS cannot finish a pairing that started in another browser, so // the hand-off card would only be a tap in front of the same dead end. diff --git a/packages/fxa-settings/src/pages/Pair/Index/index.tsx b/packages/fxa-settings/src/pages/Pair/Index/index.tsx index 19dd0108706..1fad176f1c0 100644 --- a/packages/fxa-settings/src/pages/Pair/Index/index.tsx +++ b/packages/fxa-settings/src/pages/Pair/Index/index.tsx @@ -26,7 +26,7 @@ import Banner from '../../../components/Banner'; import ButtonBack from '../../../components/ButtonBack'; import { Constants } from '../../../lib/constants'; import firefox, { - buildSyncOAuthSearch, + buildOAuthSearch, FirefoxCommand, } from '../../../lib/channels/firefox'; import { hardNavigate } from 'fxa-react/lib/utils'; @@ -314,14 +314,14 @@ const Pair = ({ return; } const oauthParams = await firefox - .fxaOAuthFlowBegin(['profile', Constants.OAUTH_OLDSYNC_SCOPE]) + .fxaOAuthFlowBegin(['profile', Constants.OAUTH_OLDSYNC_SCOPE], 'sync') .catch(() => null); if (cancelled) return; if (oauthParams) { - // buildSyncOAuthSearch emits OAuth params only, so the attribution + // buildOAuthSearch emits OAuth params only, so the attribution // params would be lost across the sign-in round trip and /pair would // come back without an entrypoint (FXA-14132). - const search = buildSyncOAuthSearch(oauthParams); + const search = buildOAuthSearch(oauthParams, 'sync'); for (const [key, value] of Object.entries(pairingAttribution)) { search.set(key, value); }