Skip to content

Commit 391d2e0

Browse files
authored
Merge pull request #18339 from mozilla/FXA-11055
task(recovery-phone): Handle error retrieveving Twilio lookup
2 parents 7e18bba + 50417eb commit 391d2e0

5 files changed

Lines changed: 42 additions & 7 deletions

File tree

libs/accounts/recovery-phone/src/lib/recovery-phone.manager.in.spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
testAccountDatabaseSetup,
99
} from '@fxa/shared/db/mysql/account';
1010
import { Test } from '@nestjs/testing';
11+
import { RecoveryPhoneFactory } from '@fxa/shared/db/mysql/account';
1112

1213
describe('RecoveryPhoneManager', () => {
1314
let recoveryPhoneManager: RecoveryPhoneManager;

libs/accounts/recovery-phone/src/lib/recovery-phone.service.spec.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,16 @@ import {
1414
RecoveryPhoneNotEnabled,
1515
RecoveryNumberRemoveMissingBackupCodes,
1616
} from './recovery-phone.errors';
17+
import { LOGGER_PROVIDER } from '@fxa/shared/log';
1718

1819
describe('RecoveryPhoneService', () => {
1920
const phoneNumber = '+15005551234';
2021
const uid = '0123456789abcdef0123456789abcdef';
2122
const code = '000000';
2223

24+
const mockLogger = {
25+
error: jest.fn(),
26+
};
2327
const mockSmsManager = {
2428
sendSMS: jest.fn(),
2529
phoneNumberLookup: jest.fn(),
@@ -60,6 +64,10 @@ describe('RecoveryPhoneService', () => {
6064
provide: RecoveryPhoneConfig,
6165
useValue: mockRecoveryPhoneConfig,
6266
},
67+
{
68+
provide: LOGGER_PROVIDER,
69+
useValue: mockLogger,
70+
},
6371
RecoveryPhoneService,
6472
],
6573
}).compile();
@@ -216,6 +224,18 @@ describe('RecoveryPhoneService', () => {
216224
expect(mockRecoveryPhoneManager.getUnconfirmed).toBeCalledWith(uid, code);
217225
});
218226

227+
it('can handled failed lookup by throwing PhoneNumberNotSupported error', async () => {
228+
mockRecoveryPhoneManager.getUnconfirmed.mockReturnValue({
229+
isSetup: true,
230+
});
231+
mockSmsManager.phoneNumberLookup.mockRejectedValueOnce(new Error('BOOM'));
232+
233+
const promise = service.confirmSetupCode(uid, code);
234+
235+
await expect(promise).rejects.toThrow(/Phone number not supported.*/);
236+
expect(mockLogger.error).toBeCalledTimes(1);
237+
});
238+
219239
it('will not confirm a valid sms code for signin', async () => {
220240
mockRecoveryPhoneManager.getUnconfirmed.mockReturnValue({});
221241

libs/accounts/recovery-phone/src/lib/recovery-phone.service.ts

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,26 +2,31 @@
22
* License, v. 2.0. If a copy of the MPL was not distributed with this
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
44

5-
import { Injectable } from '@nestjs/common';
5+
import { Inject, Injectable, LoggerService } from '@nestjs/common';
66
import { SmsManager } from './sms.manager';
77
import { OtpManager } from '@fxa/shared/otp';
88
import { RecoveryPhoneConfig } from './recovery-phone.service.config';
9-
import { RecoveryPhoneManager } from './recovery-phone.manager';
9+
import {
10+
PhoneNumberLookupData,
11+
RecoveryPhoneManager,
12+
} from './recovery-phone.manager';
1013
import {
1114
RecoveryNumberNotExistsError,
1215
RecoveryNumberNotSupportedError,
1316
RecoveryPhoneNotEnabled,
1417
RecoveryNumberRemoveMissingBackupCodes,
1518
} from './recovery-phone.errors';
1619
import { MessageInstance } from 'twilio/lib/rest/api/v2010/account/message';
20+
import { LOGGER_PROVIDER } from '@fxa/shared/log';
1721

1822
@Injectable()
1923
export class RecoveryPhoneService {
2024
constructor(
2125
private readonly recoveryPhoneManager: RecoveryPhoneManager,
2226
private readonly smsManager: SmsManager,
2327
private readonly otpCode: OtpManager,
24-
private readonly config: RecoveryPhoneConfig
28+
private readonly config: RecoveryPhoneConfig,
29+
@Inject(LOGGER_PROVIDER) private readonly log?: LoggerService
2530
) {}
2631

2732
/**
@@ -125,9 +130,16 @@ export class RecoveryPhoneService {
125130
}
126131

127132
// If this was for a setup operation. Register the phone number to the uid.
128-
const lookupData = await this.smsManager.phoneNumberLookup(
129-
data.phoneNumber
130-
);
133+
const lookupData: PhoneNumberLookupData = await (async () => {
134+
try {
135+
return await this.smsManager.phoneNumberLookup(data.phoneNumber);
136+
} catch (error) {
137+
this.log?.error('RecoveryPhoneService.confirmSetupCode', error);
138+
139+
throw new RecoveryNumberNotSupportedError(data.phoneNumber, error);
140+
}
141+
})();
142+
131143
await this.recoveryPhoneManager.registerPhoneNumber(
132144
uid,
133145
data.phoneNumber,

libs/shared/db/mysql/account/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ export {
99
AccountFactory,
1010
AccountCustomerFactory,
1111
PaypalCustomerFactory,
12+
RecoveryPhoneFactory,
1213
} from './lib/factories';
1314
export { setupAccountDatabase, AccountDbProvider } from './lib/setup';
1415
export { testAccountDatabaseSetup } from './lib/tests';

packages/fxa-auth-server/bin/key_server.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,8 @@ async function run(config) {
245245
recoveryPhoneManager,
246246
smsManager,
247247
otpCodeManager,
248-
config.recoveryPhone
248+
config.recoveryPhone,
249+
log
249250
);
250251
Container.set(RecoveryPhoneService, recoveryPhoneService);
251252

0 commit comments

Comments
 (0)