Merge pull request #18356 from mozilla/FXA-11065

task(recovery-phone): Add check for sms pumping risk

Author: dschom

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

@@ -9,7 +9,6 @@ import {
   RecoveryPhoneFactory,
 } from '@fxa/shared/db/mysql/account';
 import { Test } from '@nestjs/testing';
-import { RecoveryPhoneFactory } from '@fxa/shared/db/mysql/account';
 
 describe('RecoveryPhoneManager', () => {
   let recoveryPhoneManager: RecoveryPhoneManager;

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

@@ -2,11 +2,14 @@
  * 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 { IsArray, IsBoolean, IsObject } from 'class-validator';
+import { IsArray, IsBoolean, IsNumber, IsObject } from 'class-validator';
 
 export class RecoveryPhoneServiceConfig {
   @IsArray()
   public validNumberPrefixes?: Array<string>;
+
+  @IsNumber()
+  public smsPumpingRiskThreshold?: number;
 }
 
 export class RecoveryPhoneConfig {

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

@@ -15,6 +15,7 @@ import {
   RecoveryNumberRemoveMissingBackupCodes,
 } from './recovery-phone.errors';
 import { LOGGER_PROVIDER } from '@fxa/shared/log';
+import { StatsDService } from '@fxa/shared/metrics/statsd';
 
 describe('RecoveryPhoneService', () => {
   const phoneNumber = '+15005551234';
@@ -24,6 +25,10 @@ describe('RecoveryPhoneService', () => {
   const mockLogger = {
     error: jest.fn(),
   };
+  const mockMetrics = {
+    gauge: jest.fn(),
+    increment: jest.fn(),
+  };
   const mockSmsManager = {
     sendSMS: jest.fn(),
     phoneNumberLookup: jest.fn(),
@@ -44,6 +49,7 @@ describe('RecoveryPhoneService', () => {
     allowedRegions: ['US'],
     sms: {
       validNumberPrefixes: ['+1500'],
+      smsPumpingRiskThreshold: 75,
     },
   };
   const mockError = new Error('BOOM');
@@ -68,6 +74,10 @@ describe('RecoveryPhoneService', () => {
           provide: LOGGER_PROVIDER,
           useValue: mockLogger,
         },
+        {
+          provide: StatsDService,
+          useValue: mockMetrics,
+        },
         RecoveryPhoneService,
       ],
     }).compile();
@@ -224,7 +234,36 @@ describe('RecoveryPhoneService', () => {
       expect(mockRecoveryPhoneManager.getUnconfirmed).toBeCalledWith(uid, code);
     });
 
-    it('can handled failed lookup by throwing PhoneNumberNotSupported error', async () => {
+    it('can handle smsPumpingRisk score throwing PhoneNumberNotSupported error', async () => {
+      const phoneNumber = '+15005550000';
+      const smsPumpingRisk = 95;
+
+      mockRecoveryPhoneManager.getUnconfirmed.mockReturnValueOnce({
+        isSetup: true,
+        phoneNumber,
+      });
+      mockSmsManager.phoneNumberLookup.mockReturnValueOnce({
+        phoneNumber,
+        smsPumpingRisk,
+      });
+
+      const promise = service.confirmSetupCode(uid, code);
+      await expect(promise).rejects.toThrow(/Phone number not supported.*/);
+      expect(mockMetrics.gauge).toBeCalledWith(
+        'sim_pumping_risk',
+        smsPumpingRisk
+      );
+      expect(mockMetrics.increment).toBeCalledWith('sim_pumping_risk.denied');
+      expect(mockLogger.error).toBeCalledWith(
+        'RecoveryPhoneService.smsPumpingRisk',
+        {
+          phoneNumber,
+          smsPumpingRisk,
+        }
+      );
+    });
+
+    it('can handle failed lookup by throwing PhoneNumberNotSupported error', async () => {
       mockRecoveryPhoneManager.getUnconfirmed.mockReturnValue({
         isSetup: true,
       });

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

@@ -18,6 +18,7 @@ import {
 } from './recovery-phone.errors';
 import { MessageInstance } from 'twilio/lib/rest/api/v2010/account/message';
 import { LOGGER_PROVIDER } from '@fxa/shared/log';
+import { StatsD, StatsDService } from '@fxa/shared/metrics/statsd';
 
 @Injectable()
 export class RecoveryPhoneService {
@@ -26,6 +27,7 @@ export class RecoveryPhoneService {
     private readonly smsManager: SmsManager,
     private readonly otpCode: OtpManager,
     private readonly config: RecoveryPhoneConfig,
+    @Inject(StatsDService) private readonly metrics: StatsD,
     @Inject(LOGGER_PROVIDER) private readonly log?: LoggerService
   ) {}
 
@@ -140,6 +142,33 @@ export class RecoveryPhoneService {
       }
     })();
 
+    // Reject numbers suspected of sim pumping
+    const smsPumpingRiskThreshold = this.config.sms?.smsPumpingRiskThreshold;
+    const smsPumpingRisk = lookupData?.smsPumpingRisk;
+    if (
+      typeof smsPumpingRiskThreshold === 'number' &&
+      typeof smsPumpingRisk === 'number'
+    ) {
+      this.metrics.gauge('sim_pumping_risk', smsPumpingRisk);
+
+      if (smsPumpingRisk > smsPumpingRiskThreshold) {
+        this.metrics.increment('sim_pumping_risk.denied');
+
+        const error = new RecoveryNumberNotSupportedError(
+          data.phoneNumber,
+          new Error('Sim pumping risk threshold exceeded')
+        );
+        this.log?.error('RecoveryPhoneService.smsPumpingRisk', {
+          phoneNumber: data.phoneNumber,
+          smsPumpingRisk,
+        });
+
+        throw error;
+      } else {
+        this.metrics.increment('sim_pumping_risk.allowed');
+      }
+    }
+
     await this.recoveryPhoneManager.registerPhoneNumber(
       uid,
       data.phoneNumber,

libs/accounts/recovery-phone/src/lib/sms.manager.config.ts

@@ -19,4 +19,7 @@ export class SmsManagerConfig {
 
   @IsArray()
   public readonly validNumberPrefixes!: Array<string>;
+
+  @IsArray()
+  public readonly extraLookupFields!: Array<string>;
 }

libs/accounts/recovery-phone/src/lib/sms.manager.ts

@@ -31,7 +31,7 @@ export class SmsManager {
   public async phoneNumberLookup(phoneNumber: string) {
     const result = await this.client.lookups.v2
       .phoneNumbers(phoneNumber)
-      .fetch();
+      .fetch({ fields: this.config.extraLookupFields.join(',') });
     // Calling toJSON converts PhoneNumberInstance into a
     // object that just holds state and can be serialized.
     return result.toJSON();

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

@@ -246,6 +246,7 @@ async function run(config) {
     smsManager,
     otpCodeManager,
     config.recoveryPhone,
+    statsd,
     log
   );
   Container.set(RecoveryPhoneService, recoveryPhoneService);

packages/fxa-auth-server/config/index.ts

@@ -2213,6 +2213,18 @@ const convictConf = convict({
         env: 'RECOVERY_PHONE__SMS__MAX_RETRIES',
         format: Number,
       },
+      smsPumpingRiskThreshold: {
+        default: 75,
+        doc: 'Max sms pumping risk score allowed. Value is from 0 to 100. e.g. 74-90 is moderate risk. See twilio docs for more info. https://www.twilio.com/docs/lookup/v2-api/sms-pumping-risk',
+        env: 'RECOVERY_PHONE__SMS__SMS_PUMPING_RISK_THRESHOLD',
+        format: Number,
+      },
+      extraLookupFields: {
+        default: ['sms_pumping_risk'],
+        doc: 'Extra data to fetch about phone numbers being registered for sms backup.',
+        env: 'RECOVERY_PHONE__SMS__EXTRA_LOOKUP_FIELDS',
+        format: Array,
+      },
     },
   },
   twilio: {