diff --git a/packages/fxa-settings/src/models/integrations/pairing-authority-integration.test.ts b/packages/fxa-settings/src/models/integrations/pairing-authority-integration.test.ts index c871c1719b0..4e5febee26b 100644 --- a/packages/fxa-settings/src/models/integrations/pairing-authority-integration.test.ts +++ b/packages/fxa-settings/src/models/integrations/pairing-authority-integration.test.ts @@ -598,7 +598,9 @@ describe('PairingAuthorityIntegration', () => { expect(integration.state).toBe(AuthorityState.Failed); expect(onError).toHaveBeenCalledWith( - expect.objectContaining({ errno: OAUTH_ERRORS.INVALID_PARAMETER.errno }) + expect.objectContaining({ + errno: OAUTH_ERRORS.INVALID_PARAMETER.errno, + }) ); }); @@ -606,10 +608,21 @@ describe('PairingAuthorityIntegration', () => { // two only prove the authority is wired to the validator rather than to // its own truthiness checks. it.each([ - { label: 'a client that cannot pair', field: 'client_id', value: '0123456789abcdef' }, - { label: 'a PKCE method other than S256', field: 'code_challenge_method', value: 'plain' }, + { + label: 'a client that cannot pair', + field: 'client_id', + value: '0123456789abcdef', + }, + { + label: 'a PKCE method other than S256', + field: 'code_challenge_method', + value: 'plain', + }, ])('fails on $label', ({ field, value }) => { - emit('remote:pair:supp:request', { ...MOCK_SUPP_REQUEST, [field]: value }); + emit('remote:pair:supp:request', { + ...MOCK_SUPP_REQUEST, + [field]: value, + }); expect(integration.state).toBe(AuthorityState.Failed); }); @@ -758,6 +771,40 @@ describe('PairingAuthorityIntegration', () => { }); }); + describe('cancel', () => { + // The supplicant cannot tell a closed channel from an expired one, so the + // notice has to get out while the channel is still up. + it('tells the supplicant before closing the channel', async () => { + const integration = createIntegration(); + await integration.createChannel(); + + await integration.cancel(); + + expect(mockChannelSend).toHaveBeenCalledWith('pair:auth:cancel', {}); + expect(mockChannelSend.mock.invocationCallOrder[0]).toBeLessThan( + mockChannelClose.mock.invocationCallOrder[0] + ); + }); + + it('closes the channel when the notice cannot be sent', async () => { + const integration = createIntegration(); + await integration.createChannel(); + mockChannelSend.mockRejectedValue(new Error('channel server gone')); + + await integration.cancel(); + + expect(mockChannelClose).toHaveBeenCalled(); + expect(integration.hasChannel()).toBe(false); + }); + + it('resolves when there is no channel to cancel', async () => { + const integration = createIntegration(); + + await expect(integration.cancel()).resolves.toBeUndefined(); + expect(mockChannelSend).not.toHaveBeenCalled(); + }); + }); + describe('destroy', () => { it('cleans up timers and callbacks', async () => { const integration = createIntegration(); diff --git a/packages/fxa-settings/src/models/integrations/pairing-authority-integration.ts b/packages/fxa-settings/src/models/integrations/pairing-authority-integration.ts index 3eb59d58cde..3c76e634562 100644 --- a/packages/fxa-settings/src/models/integrations/pairing-authority-integration.ts +++ b/packages/fxa-settings/src/models/integrations/pairing-authority-integration.ts @@ -137,7 +137,7 @@ export class PairingAuthorityIntegration extends OAuthWebIntegration { async createChannel(): Promise { if (this._channel) { - console.warn('Pairing channel already exists!') + console.warn('Pairing channel already exists!'); return; } @@ -185,7 +185,10 @@ export class PairingAuthorityIntegration extends OAuthWebIntegration { private setState(state: AuthorityState): void { this._state = state; - console.info('Emitting pairing authority state change event.', {id: this._iid, state: this.state}); + console.info('Emitting pairing authority state change event.', { + id: this._iid, + state: this.state, + }); this.onStateChange?.(state); } @@ -261,9 +264,10 @@ export class PairingAuthorityIntegration extends OAuthWebIntegration { this._supRequest = validation.request; - this._channel.send('pair:auth:metadata', {}) - .then(()=>{ - this.setState(AuthorityState.WaitingForAuthorizations); + this._channel + .send('pair:auth:metadata', {}) + .then(() => { + this.setState(AuthorityState.WaitingForAuthorizations); }) .catch((err) => { console.warn('Error sending pair:auth:metadata'); @@ -485,18 +489,18 @@ export class PairingAuthorityIntegration extends OAuthWebIntegration { code_challenge_method: supRequest.code_challenge_method, keys_jwk: supRequest.keys_jwk, scope: supRequest.scope, - state: supRequest.state + state: supRequest.state, }); if (!result?.code || !result.state) { throw new Error('Failed to finalize oauth pair!'); } - console.info('OAuth pair finish success!') + console.info('OAuth pair finish success!'); await this._channel.send('pair:auth:authorize', { code: result.code, - state: result.state - }) + state: result.state, + }); } catch (err) { // pairOauthFinish rejects on its own timeout, on a missing code/state and // on an echoed-state mismatch, and the send can reject too. The approve @@ -506,8 +510,7 @@ export class PairingAuthorityIntegration extends OAuthWebIntegration { this.fail(err); return; } - } - else { + } else { await firefox.pairAuthorize(this.channelId); } @@ -531,6 +534,25 @@ export class PairingAuthorityIntegration extends OAuthWebIntegration { await firefox.pairComplete(this.channelId); } + /** + * Ends the flow at the authority user's request. + * + * A channel the user closed and one that expired look identical from the + * other end, so the supplicant is told before the channel goes away — + * otherwise its dead-end screen blames a timeout for a pairing this user + * deliberately stopped. + */ + async cancel(): Promise { + try { + await this._channel?.send('pair:auth:cancel', {}); + } catch (err) { + // The notice is a courtesy to the other device. A channel that will not + // carry it still has to be torn down. + Sentry.captureException(err); + } + await this.destroy(); + } + /** Clean up timers on unmount. */ async destroy() { this.stopHeartbeat(); @@ -557,7 +579,7 @@ export class PairingAuthorityIntegration extends OAuthWebIntegration { try { await this._channel.close(); } catch (err) { - Sentry.captureException(err) + Sentry.captureException(err); } finally { this._channel = null; } diff --git a/packages/fxa-settings/src/models/integrations/pairing-supplicant-integration.test.ts b/packages/fxa-settings/src/models/integrations/pairing-supplicant-integration.test.ts index 1067e8730db..b9f34c14c73 100644 --- a/packages/fxa-settings/src/models/integrations/pairing-supplicant-integration.test.ts +++ b/packages/fxa-settings/src/models/integrations/pairing-supplicant-integration.test.ts @@ -431,6 +431,42 @@ describe('PairingSupplicantIntegration', () => { }); }); + describe('authority cancel', () => { + async function connectedIntegration() { + const integration = createIntegration(); + await integration.openChannel('wss://ch.example.com', 'c', 'k'); + emit('connected'); + return integration; + } + + it('ends the flow when the authority cancels', async () => { + const integration = await connectedIntegration(); + + emit('remote:pair:auth:cancel'); + + expect(integration.state).toBe(SupplicantState.Failed); + }); + + it('records that the authority cancelled', async () => { + const integration = await connectedIntegration(); + + emit('remote:pair:auth:cancel'); + + expect(integration.canceledByAuthority).toBe(true); + }); + + // Without a notice the close is indistinguishable from the channel running + // out of time, which is what the dead-end screen then has to say. + it('does not treat a bare channel close as a cancel', async () => { + const integration = await connectedIntegration(); + + emit('close'); + + expect(integration.state).toBe(SupplicantState.Failed); + expect(integration.canceledByAuthority).toBe(false); + }); + }); + describe('isPairing', () => { it('returns true', () => { expect(createIntegration().isPairing()).toBe(true); diff --git a/packages/fxa-settings/src/models/integrations/pairing-supplicant-integration.ts b/packages/fxa-settings/src/models/integrations/pairing-supplicant-integration.ts index 17e87043e9c..af8d0db1ec0 100644 --- a/packages/fxa-settings/src/models/integrations/pairing-supplicant-integration.ts +++ b/packages/fxa-settings/src/models/integrations/pairing-supplicant-integration.ts @@ -103,6 +103,7 @@ export class PairingSupplicantIntegration extends OAuthWebIntegration { private _remoteMetadata: RemoteMetadata | null = null; private _oauthCode: string | null = null; private _error: Error | { errno: number; message: string } | null = null; + private _canceledByAuthority = false; private _email = ''; private _deviceName = ''; private _channelId: string | null = null; @@ -171,6 +172,15 @@ export class PairingSupplicantIntegration extends OAuthWebIntegration { return this._error; } + /** + * True once the authority has said it cancelled. The channel closing is the + * same event either way, so this is the only thing that separates a cancel + * from a pairing that ran out of time. + */ + get canceledByAuthority(): boolean { + return this._canceledByAuthority; + } + /** Email address sent by the authority in pair:auth:metadata. */ get email(): string { return this._email; @@ -189,7 +199,10 @@ export class PairingSupplicantIntegration extends OAuthWebIntegration { private setState(state: SupplicantState): void { this._state = state; - console.info(`Emitting pairing supplicant state change event.`, {id: this._iid, state: this.state}); + console.info(`Emitting pairing supplicant state change event.`, { + id: this._iid, + state: this.state, + }); this.onStateChange?.(state); } @@ -220,7 +233,7 @@ export class PairingSupplicantIntegration extends OAuthWebIntegration { this.onError?.(this._error); } - hasChannel(channelId:string) { + hasChannel(channelId: string) { return !!this._channel && this._channel.channelId === channelId; } @@ -232,9 +245,8 @@ export class PairingSupplicantIntegration extends OAuthWebIntegration { channelServerUri: string, channelId: string, channelKey: string, - version:number = 1 + version: number = 1 ): Promise { - if (version === 2 && this._channel) { if (channelId === this._channel.channelId) { console.warn('Pairing channel already open!'); @@ -252,6 +264,7 @@ export class PairingSupplicantIntegration extends OAuthWebIntegration { this._channelId = channelId; this._version = version; + this._canceledByAuthority = false; this._channel = new PairingChannelClient(); // Listen for channel events @@ -268,6 +281,10 @@ export class PairingSupplicantIntegration extends OAuthWebIntegration { 'remote:pair:auth:authorize', this.handleAuthAuthorize ); + this._channel.addEventListener( + 'remote:pair:auth:cancel', + this.handleAuthCancel + ); try { await this._channel.open(channelServerUri, channelId, channelKey); @@ -294,8 +311,7 @@ export class PairingSupplicantIntegration extends OAuthWebIntegration { // In V2, the the UI flow forces the supplicant to approve first. if (this._version === 2) { this.setState(SupplicantState.WaitingForAuthority); - } - else { + } else { if (this._state === SupplicantState.WaitingForAuthorizations) { this.setState(SupplicantState.WaitingForAuthority); } else if (this._state === SupplicantState.WaitingForSupplicant) { @@ -313,11 +329,9 @@ export class PairingSupplicantIntegration extends OAuthWebIntegration { // Send OAuth request to authority if (!this._channel) { - throw new Error('Channel no longe exists!') + throw new Error('Channel no longe exists!'); } - await this._channel - .send('pair:supp:request', oauthParams); - + await this._channel.send('pair:supp:request', oauthParams); })().catch((err: unknown) => { this.fail(err); }); @@ -392,6 +406,16 @@ export class PairingSupplicantIntegration extends OAuthWebIntegration { } }; + /** + * The authority user cancelled. Failing here rather than waiting for the + * channel to close keeps the reason and the failure in the same turn, so the + * dead-end screen cannot be routed to before it is known. + */ + private handleAuthCancel = () => { + this._canceledByAuthority = true; + this.fail(new Error('Pairing was canceled on the other device')); + }; + /** True when a close/error during connect is the FXA-13616 reload, not a real failure. */ private isPostCompletionReconnect(): boolean { return ( @@ -401,7 +425,7 @@ export class PairingSupplicantIntegration extends OAuthWebIntegration { ); } - checkClientInfo () { + checkClientInfo() { // no-op. The supplicant doesn't have client info. } @@ -425,7 +449,7 @@ export class PairingSupplicantIntegration extends OAuthWebIntegration { } if (!clientId) { - console.warn("Could not resolve a valid clientId!") + console.warn('Could not resolve a valid clientId!'); } return clientId; @@ -443,8 +467,7 @@ export class PairingSupplicantIntegration extends OAuthWebIntegration { }; private handleChannelError = (event: Event) => { - - console.warn('Channel error event', event) + console.warn('Channel error event', event); if (this.isPostCompletionReconnect()) { return; @@ -488,7 +511,7 @@ export class PairingSupplicantIntegration extends OAuthWebIntegration { if (this._version === 2) { const data = await firefox.pairOauthStart({}); if (!data) { - throw new Error('Firefox could not provide oauth params.') + throw new Error('Firefox could not provide oauth params.'); } // This following client id check is a bandaide for now, so we at least @@ -496,8 +519,8 @@ export class PairingSupplicantIntegration extends OAuthWebIntegration { // out, since there's not point in proceeding. const client_id = this.data.clientId || this.getClientId(); if (!client_id) { - console.warn('Could not determine clientId!') - throw new Error("Could not determine clientId! Cannot proceed.") + console.warn('Could not determine clientId!'); + throw new Error('Could not determine clientId! Cannot proceed.'); } const scope = [ @@ -515,7 +538,9 @@ export class PairingSupplicantIntegration extends OAuthWebIntegration { }; // Fail fast if something wasn't provided. - const missing = Object.entries(result).filter(([k,v]) => !v && k !== 'client_id').map(([k]) => k); + const missing = Object.entries(result) + .filter(([k, v]) => !v && k !== 'client_id') + .map(([k]) => k); if (missing.length) { throw new Error(`Missing required OAuth params: ${missing.join(', ')}`); } @@ -572,7 +597,7 @@ export class PairingSupplicantIntegration extends OAuthWebIntegration { } async destroy(): Promise { - console.info('Channel destroy') + console.info('Channel destroy'); this.onStateChange = null; this.onError = null; @@ -588,6 +613,10 @@ export class PairingSupplicantIntegration extends OAuthWebIntegration { 'remote:pair:auth:authorize', this.handleAuthAuthorize ); + this._channel.removeEventListener( + 'remote:pair:auth:cancel', + this.handleAuthCancel + ); try { await this._channel.close(); } catch { diff --git a/packages/fxa-settings/src/pages/Pair2/Authority/ContinueOnMobile/container.test.tsx b/packages/fxa-settings/src/pages/Pair2/Authority/ContinueOnMobile/container.test.tsx index 65a6f2b86af..941283a8d2b 100644 --- a/packages/fxa-settings/src/pages/Pair2/Authority/ContinueOnMobile/container.test.tsx +++ b/packages/fxa-settings/src/pages/Pair2/Authority/ContinueOnMobile/container.test.tsx @@ -109,12 +109,14 @@ describe('Pair2/Authority/ContinueOnMobile container', () => { await user.click(screen.getByRole('button', { name: 'Cancel' })); }; - it('closes the channel before leaving for the cancel screen', async () => { + // The supplicant is left waiting on this channel, so it has to be told the + // pairing is over rather than left to infer it from the channel closing. + it('cancels the pairing before leaving for the cancel screen', async () => { renderContainer(); await clickCancel(); - await waitFor(() => expect(integration.destroy).toHaveBeenCalled()); + await waitFor(() => expect(integration.cancel).toHaveBeenCalled()); expect(mockNavigate).toHaveBeenCalledWith( '/pair/authority/timeout_and_cancel', { state: { reason: 'canceled' } } @@ -125,7 +127,7 @@ describe('Pair2/Authority/ContinueOnMobile container', () => { // keep them on a screen that is waiting on a pairing they cancelled. it('still leaves for the cancel screen when the channel cannot be closed', async () => { const err = new Error('channel server unreachable'); - integration.destroy.mockRejectedValue(err); + integration.cancel.mockRejectedValue(err); renderContainer(); await clickCancel(); diff --git a/packages/fxa-settings/src/pages/Pair2/Authority/ContinueOnMobile/container.tsx b/packages/fxa-settings/src/pages/Pair2/Authority/ContinueOnMobile/container.tsx index 34feadf4f08..ce3bb0d3cd6 100644 --- a/packages/fxa-settings/src/pages/Pair2/Authority/ContinueOnMobile/container.tsx +++ b/packages/fxa-settings/src/pages/Pair2/Authority/ContinueOnMobile/container.tsx @@ -54,7 +54,7 @@ const ContinueOnMobileContainer = ({ // Leave even if the channel will not close; the user asked to stop. const onCancel = async () => { try { - await integration.destroy(); + await integration.cancel(); } catch (err) { Sentry.captureException(err); } diff --git a/packages/fxa-settings/src/pages/Pair2/Authority/ContinueOnMobile/mocks.tsx b/packages/fxa-settings/src/pages/Pair2/Authority/ContinueOnMobile/mocks.tsx index 2c22ca33b3f..9569b6c49b6 100644 --- a/packages/fxa-settings/src/pages/Pair2/Authority/ContinueOnMobile/mocks.tsx +++ b/packages/fxa-settings/src/pages/Pair2/Authority/ContinueOnMobile/mocks.tsx @@ -16,6 +16,7 @@ export const Subject = ({ ); export type MockAuthorityIntegration = PairingAuthorityIntegration & { + cancel: jest.Mock; destroy: jest.Mock; }; @@ -32,6 +33,7 @@ export function mockAuthorityIntegration( ) as MockAuthorityIntegration; return Object.assign(integration, { + cancel: jest.fn().mockResolvedValue(undefined), destroy: jest.fn().mockResolvedValue(undefined), onStateChange: null, ...overrides, diff --git a/packages/fxa-settings/src/pages/Pair2/Supplicant/ApproveSignIn/container.test.tsx b/packages/fxa-settings/src/pages/Pair2/Supplicant/ApproveSignIn/container.test.tsx index 70031a9b594..123a6d55725 100644 --- a/packages/fxa-settings/src/pages/Pair2/Supplicant/ApproveSignIn/container.test.tsx +++ b/packages/fxa-settings/src/pages/Pair2/Supplicant/ApproveSignIn/container.test.tsx @@ -33,15 +33,19 @@ type MockSupplicantIntegration = PairingSupplicantIntegration & { */ function mockSupplicantIntegration({ remoteMetadata = MOCK_METADATA_WITH_DEVICE_NAME as RemoteMetadata | null, + canceledByAuthority = false, } = {}): MockSupplicantIntegration { const integration = Object.create( PairingSupplicantIntegration.prototype ) as MockSupplicantIntegration; - // `remoteMetadata` is a getter on the prototype, so it cannot be assigned. + // Both are getters on the prototype, so they cannot be assigned. Object.defineProperty(integration, 'remoteMetadata', { get: () => remoteMetadata, }); + Object.defineProperty(integration, 'canceledByAuthority', { + get: () => canceledByAuthority, + }); return Object.assign(integration, { destroy: jest.fn().mockResolvedValue(undefined), @@ -92,14 +96,29 @@ describe('Pair2/Supplicant/ApproveSignIn container', () => { ); }); - it('navigates to the cancel screen when the flow fails', () => { + it('blames a timeout when the flow fails on its own', () => { renderContainer(); emitState(integration, SupplicantState.Failed); expect(navigateWithQuery).toHaveBeenCalledWith( '/pair/supplicant/timeout_and_cancel', - {}, + { state: { reason: 'timeout' } }, + true + ); + }); + + // The desktop user cancelling is not a wait this user ever made, so the + // dead-end screen has to name the cancel instead of a timeout. + it('names the cancel when the authority cancelled', () => { + integration = mockSupplicantIntegration({ canceledByAuthority: true }); + renderContainer(); + + emitState(integration, SupplicantState.Failed); + + expect(navigateWithQuery).toHaveBeenCalledWith( + '/pair/supplicant/timeout_and_cancel', + { state: { reason: 'canceled' } }, true ); }); diff --git a/packages/fxa-settings/src/pages/Pair2/Supplicant/ApproveSignIn/container.tsx b/packages/fxa-settings/src/pages/Pair2/Supplicant/ApproveSignIn/container.tsx index b13b92cd292..3326bc37a45 100644 --- a/packages/fxa-settings/src/pages/Pair2/Supplicant/ApproveSignIn/container.tsx +++ b/packages/fxa-settings/src/pages/Pair2/Supplicant/ApproveSignIn/container.tsx @@ -2,42 +2,63 @@ * 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 { useEffect } from "react"; -import * as Sentry from "@sentry/browser"; -import ApproveSignIn from "."; -import { Integration, PairingSupplicantIntegration, SupplicantState } from "../../../../models"; -import { navigateWithQuery } from "../../../../lib/utilities"; +import { useEffect } from 'react'; +import * as Sentry from '@sentry/browser'; +import ApproveSignIn from '.'; +import { + Integration, + PairingSupplicantIntegration, + SupplicantState, +} from '../../../../models'; +import { navigateWithQuery } from '../../../../lib/utilities'; - -export const ApproveSignInContainer = ({integration}:{integration:Integration|PairingSupplicantIntegration}) => { +export const ApproveSignInContainer = ({ + integration, +}: { + integration: Integration | PairingSupplicantIntegration; +}) => { if (!(integration instanceof PairingSupplicantIntegration)) { - throw new Error('Invalid integration. Expecting instance of PairingSupplicantIntegration'); + throw new Error( + 'Invalid integration. Expecting instance of PairingSupplicantIntegration' + ); } if (!integration.remoteMetadata) { - throw new Error('Invalid integration state. Remote meta data should be populated.'); + throw new Error( + 'Invalid integration state. Remote meta data should be populated.' + ); } useEffect(() => { integration.onStateChange = (state) => { - switch(state) { + switch (state) { case SupplicantState.Complete: navigateWithQuery('/pair/supplicant/sync_success', {}, true); break; case SupplicantState.Failed: - console.warn('SupplicantState failed', { tags: { state } }) - navigateWithQuery('/pair/supplicant/timeout_and_cancel', {}, true); + console.warn('SupplicantState failed', { tags: { state } }); + navigateWithQuery( + '/pair/supplicant/timeout_and_cancel', + { + state: { + reason: integration.canceledByAuthority + ? 'canceled' + : 'timeout', + }, + }, + true + ); break; default: console.warn('Unexpected state change: ' + state); break; } - } + }; return () => { // Unsubscribe only — the channel outlives this page for sync_success. integration.onStateChange = null; }; - },[integration]) + }, [integration]); // Leave even if the channel will not close; the user asked to stop. The // reason rides along so the dead-end screen says "Canceled" rather than @@ -53,9 +74,13 @@ export const ApproveSignInContainer = ({integration}:{integration:Integration|Pa { state: { reason: 'canceled' } }, true ); - } + }; - return -} + return ( + + ); +}; -export default ApproveSignInContainer +export default ApproveSignInContainer; diff --git a/packages/fxa-settings/src/pages/Pair2/Supplicant/ConnectThisDevice/container.test.tsx b/packages/fxa-settings/src/pages/Pair2/Supplicant/ConnectThisDevice/container.test.tsx index 069448d5bbd..2ada4154a8c 100644 --- a/packages/fxa-settings/src/pages/Pair2/Supplicant/ConnectThisDevice/container.test.tsx +++ b/packages/fxa-settings/src/pages/Pair2/Supplicant/ConnectThisDevice/container.test.tsx @@ -47,16 +47,21 @@ type MockSupplicantIntegration = PairingSupplicantIntegration & { * the real prototype because the container narrows on `instanceof` before it * touches any of it. */ -function mockSupplicantIntegration(): MockSupplicantIntegration { +function mockSupplicantIntegration({ + canceledByAuthority = false, +} = {}): MockSupplicantIntegration { const integration = Object.create( PairingSupplicantIntegration.prototype ) as MockSupplicantIntegration; - // Both are getters on the prototype, so they cannot be assigned. + // All three are getters on the prototype, so they cannot be assigned. Object.defineProperty(integration, 'remoteMetadata', { get: () => MOCK_METADATA_WITH_DEVICE_NAME, }); Object.defineProperty(integration, 'email', { get: () => MOCK_EMAIL }); + Object.defineProperty(integration, 'canceledByAuthority', { + get: () => canceledByAuthority, + }); return Object.assign(integration, { openChannel: jest.fn().mockResolvedValue(undefined), @@ -153,7 +158,7 @@ describe('Pair2/Supplicant/ConnectThisDevice container', () => { ); }); - it('navigates to the cancel screen when pairing fails', async () => { + it('blames a timeout when pairing fails on its own', async () => { renderContainer(); await waitFor(() => expect(integration.onStateChange).toBeTruthy()); @@ -161,7 +166,23 @@ describe('Pair2/Supplicant/ConnectThisDevice container', () => { expect(navigateWithQuery).toHaveBeenCalledWith( '/pair/supplicant/timeout_and_cancel', - {}, + { state: { reason: 'timeout' } }, + true + ); + }); + + // The desktop user cancelling is not a wait this user ever made, so the + // dead-end screen has to name the cancel instead of a timeout. + it('names the cancel when the authority cancelled', async () => { + integration = mockSupplicantIntegration({ canceledByAuthority: true }); + renderContainer(); + await waitFor(() => expect(integration.onStateChange).toBeTruthy()); + + emitState(integration, SupplicantState.Failed); + + expect(navigateWithQuery).toHaveBeenCalledWith( + '/pair/supplicant/timeout_and_cancel', + { state: { reason: 'canceled' } }, true ); }); @@ -193,9 +214,7 @@ describe('Pair2/Supplicant/ConnectThisDevice container', () => { const user = userEvent.setup(); await renderReady(); - await user.click( - screen.getByRole('button', { name: 'Connect' }) - ); + await user.click(screen.getByRole('button', { name: 'Connect' })); expect(integration.supplicantApprove).toHaveBeenCalledTimes(1); await waitFor(() => diff --git a/packages/fxa-settings/src/pages/Pair2/Supplicant/ConnectThisDevice/container.tsx b/packages/fxa-settings/src/pages/Pair2/Supplicant/ConnectThisDevice/container.tsx index d89f4498530..3de6f27d34f 100644 --- a/packages/fxa-settings/src/pages/Pair2/Supplicant/ConnectThisDevice/container.tsx +++ b/packages/fxa-settings/src/pages/Pair2/Supplicant/ConnectThisDevice/container.tsx @@ -17,25 +17,29 @@ import ConnectThisDevice from '.'; import { navigateWithQuery } from '../../../../lib/utilities'; export const ConnectThisDeviceContainer = ({ - integration -}:{ - integration?: Integration|PairingSupplicantIntegration + integration, +}: { + integration?: Integration | PairingSupplicantIntegration; }) => { const location = useLocation(); const navigate = useNavigate(); - const [remoteMetadata, setRemoteMetadata] = useState(null); - const [email, setEmail] = useState(); + const [remoteMetadata, setRemoteMetadata] = useState( + null + ); + const [email, setEmail] = useState(); const [ready, setReady] = useState(false); if (!(integration instanceof PairingSupplicantIntegration)) { - throw new Error('Invalid integration. Expecting instance of PairingSupplicantIntegration'); + throw new Error( + 'Invalid integration. Expecting instance of PairingSupplicantIntegration' + ); } if (location.state?.version !== '2') { throw new Error('Invalid location state. Expecting version of 2.'); } useEffect(() => { - integration.onStateChange = (state:SupplicantState) => { + integration.onStateChange = (state: SupplicantState) => { switch (state) { case SupplicantState.WaitingForAuthorizations: // The following data should have been relayed over the pairing channel @@ -45,9 +49,9 @@ export const ConnectThisDeviceContainer = ({ ipAddress: integration.remoteMetadata?.ipAddress || 'Unknown', city: integration.remoteMetadata?.city || 'Unknown', country: integration.remoteMetadata?.country || 'Unknown', - deviceFamily: integration.remoteMetadata?.deviceFamily || 'Unknown', + deviceFamily: integration.remoteMetadata?.deviceFamily || 'Unknown', deviceName: integration.remoteMetadata?.deviceName || 'Unknown', - deviceOS: integration.remoteMetadata?.deviceOS || 'Unknown', + deviceOS: integration.remoteMetadata?.deviceOS || 'Unknown', region: integration.remoteMetadata?.region || 'Unknown', }); break; @@ -55,14 +59,23 @@ export const ConnectThisDeviceContainer = ({ navigateWithQuery('/pair/supplicant/approve_signin', {}, true); break; case SupplicantState.Failed: - navigateWithQuery('/pair/supplicant/timeout_and_cancel', {}, true); + navigateWithQuery( + '/pair/supplicant/timeout_and_cancel', + { + state: { + reason: integration.canceledByAuthority + ? 'canceled' + : 'timeout', + }, + }, + true + ); break; default: console.warn('Unexpected state change: ' + state); break; } - } - + }; (async () => { await integration.openChannel( @@ -70,7 +83,7 @@ export const ConnectThisDeviceContainer = ({ location.state.channelId, location.state.channelKey, 2 - ) + ); setReady(true); })().catch((err) => { Sentry.captureException(err); @@ -97,27 +110,29 @@ export const ConnectThisDeviceContainer = ({ { state: { reason: 'canceled' } }, true ); - } + }; const onConnect = () => { integration .supplicantApprove() .then(() => { - navigate('/pair/supplicant/approve_signin') + navigate('/pair/supplicant/approve_signin'); }) .catch((err) => { Sentry.captureException(err); - navigate('/pair/supplicant/timeout_and_cancel') + navigate('/pair/supplicant/timeout_and_cancel'); }); }; if (!ready || !remoteMetadata) { // Rendered through AppLayout rather than a bare spinner so the wait for the // channel keeps the page chrome and centering of the card that follows it. - return + return ; } - return -} + return ( + + ); +}; export default ConnectThisDeviceContainer;