extra config changes for test creds & api keys

.circleci/config.yml

@@ -776,6 +776,9 @@ jobs:
           command: ./packages/functional-tests/scripts/start-services.sh
           environment:
             NODE_ENV: test
+            # Functional tests use 'real' numbers.
+            # Switch out of the default 'test' credential mode.
+            RECOVERY_PHONE__TWILIO__CREDENTIAL_MODE: 'apiKeys'
       - run-playwright-tests:
           project: local
       - store-artifacts

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

@@ -84,6 +84,7 @@ describe('RecoveryPhoneService', () => {
   const mockKeys = createNewFxaKeyPair();
 
   const mockTwilioConfig: TwilioConfig = {
+    credentialMode: 'default',
     accountSid: mockAccountSid,
     authToken: undefined,
     webhookUrl: mockWebhookUrl,

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

@@ -8,6 +8,21 @@ import { IsBoolean, IsString } from 'class-validator';
  * Configuration for twilio client. See twilio SDK docs for more details.
  */
 export class TwilioConfig {
+  /**
+   * Determines which credential set to use. Options are
+   * - test - uses the testing account sid and testing auth token
+   * - default - uses the account sid and auth token
+   * - apiKeys - ues the account sid, apiKey and apiSecret
+   */
+  @IsString()
+  credentialMode!: 'test' | 'default' | 'apiKeys';
+
+  @IsString()
+  testAuthToken?: string;
+
+  @IsString()
+  testAccountSid?: string;
+
   /**
    * The twilio account sid
    */
@@ -16,34 +31,34 @@ export class TwilioConfig {
 
   /**
    * The twilio auth token. Note that this, or an apiKey/apiSecret must be set!
-   * Using the auth token is not preferred.
+   * Using the auth token is not preferred, unless it's the 'testing' auth token.
    */
   @IsString()
   authToken?: string;
 
   /**
-   * The webhook url that twilio will deliver status updates about messages to.
+   * A twilio apiKey. Works in conjunction with apiSecret. This is the preferred path for client authentication.
    */
   @IsString()
-  webhookUrl!: string;
+  apiKey?: string;
 
   /**
-   * Flag that toggles on / off webhook validation.
+   * A twilio apiSecret. Works in conjunction with apiKey. This is the preferred path for client authentication.
    */
-  @IsBoolean()
-  validateWebhookCalls!: boolean;
+  @IsString()
+  apiSecret?: string;
 
   /**
-   * A twilio apiKey. Works in conjunction with apiSecret. This is the preferred path for client authentication.
+   * The webhook url that twilio will deliver status updates about messages to.
    */
   @IsString()
-  apiKey?: string;
+  webhookUrl!: string;
 
   /**
-   * A twilio apiSecret. Works in conjunction with apiKey. This is the preferred path for client authentication.
+   * Flag that toggles on / off webhook validation.
    */
-  @IsString()
-  apiSecret?: string;
+  @IsBoolean()
+  validateWebhookCalls!: boolean;
 
   /**
    * A public key for validating webhook messages. Works in conjunction with fxaPrivateKey.

libs/accounts/recovery-phone/src/lib/twilio.provider.spec.ts

@@ -13,6 +13,7 @@ describe('TwilioFactory', () => {
   let client: Twilio;
 
   const MockTwilioConfig = {
+    credentialMode: 'apiKeys',
     accountSid: 'AC',
     apiKey: 'SK',
     apiSecret: 'SHHH',

libs/accounts/recovery-phone/src/lib/twilio.provider.ts

@@ -22,8 +22,34 @@ export const TwilioProvider = Symbol('TwilioProvider');
 export const TwilioFactory: Provider<Twilio> = {
   provide: TwilioProvider,
   useFactory: (config: TwilioConfig) => {
-    const { accountSid, authToken } = config;
-    return new Twilio(accountSid, authToken);
+    const {
+      credentialMode,
+      testAccountSid,
+      testAuthToken,
+      accountSid,
+      authToken,
+      apiKey,
+      apiSecret,
+    } = config;
+
+    // Okay for test, dev, and CI when using real phone numbers
+    if (credentialMode === 'default' && accountSid && authToken) {
+      return new Twilio(accountSid, authToken);
+    }
+
+    // For test and dev when using magic Twilio phone numbers.
+    if (credentialMode === 'test' && testAccountSid && testAuthToken) {
+      return new Twilio(testAccountSid, testAuthToken);
+    }
+
+    // The preferred way for deployments
+    if (credentialMode === 'apiKeys' && accountSid && apiKey && apiSecret) {
+      return new Twilio(apiKey, apiSecret, { accountSid });
+    }
+
+    throw new Error(
+      'Invalid configuration state. Check docs for TwilioConfig, a value is probably missing.'
+    );
   },
   inject: [TwilioConfig],
 };

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

@@ -2234,6 +2234,24 @@ const convictConf = convict({
     },
   },
   twilio: {
+    credentialMode: {
+      default: 'test',
+      doc: 'Which credential set to use. Options are test, default, or apiKeys.',
+      env: 'RECOVERY_PHONE__TWILIO__CREDENTIAL_MODE',
+      format: String,
+    },
+    testAccountSid: {
+      default: 'AC_REPLACEMEWITHKEY',
+      doc: 'Twilio Testing Account ID. Note must be used for tests leveraging Twilio magic phone numbers.',
+      env: 'RECOVERY_PHONE__TWILIO__TEST_ACCOUNT_SID',
+      format: String,
+    },
+    testAuthToken: {
+      default: '',
+      doc: 'Twilio Testing Account Auth Token. Note must be used for tests leverage Twilio magic phone numbers.',
+      env: 'RECOVERY_PHONE__TWILIO__TEST_AUTH_TOKEN',
+      format: String,
+    },
     accountSid: {
       default: 'AC_REPLACEMEWITHKEY',
       doc: 'Twilio Account ID',
@@ -2245,16 +2263,6 @@ const convictConf = convict({
       doc: 'Twilio Auth Token to access api. Note, using apiKey/apiSecret is preferred.',
       env: 'RECOVERY_PHONE__TWILIO__AUTH_TOKEN',
     },
-    webhookUrl: {
-      default: '',
-      doc: 'Webhook url registered with twilio for message status updates',
-      env: 'RECOVERY_PHONE__TWILIO__WEBHOOK_URL',
-    },
-    validateWebhookCalls: {
-      default: true,
-      doc: 'Controls if twilio signature is validated during webhook calls from twilio',
-      env: 'RECOVERY_PHONE__TWILIO__VALIDATE_WEBHOOK_CALLS',
-    },
     apiKey: {
       default: '',
       doc: 'An api key used to access the twilio rest api. Note, when provided the authToken is no longer needed.',
@@ -2265,6 +2273,16 @@ const convictConf = convict({
       doc: 'A secret used in conjunction with the apiKey to access the twilio rest api.',
       env: 'RECOVERY_PHONE__TWILIO__API_SECRET',
     },
+    webhookUrl: {
+      default: '',
+      doc: 'Webhook url registered with twilio for message status updates',
+      env: 'RECOVERY_PHONE__TWILIO__WEBHOOK_URL',
+    },
+    validateWebhookCalls: {
+      default: true,
+      doc: 'Controls if twilio signature is validated during webhook calls from twilio',
+      env: 'RECOVERY_PHONE__TWILIO__VALIDATE_WEBHOOK_CALLS',
+    },
     fxaPublicKey: {
       default: '',
       doc: 'A key used to to for validating signature in webhook calls.',

packages/fxa-auth-server/test/remote/recovery_phone_tests.js

@@ -48,10 +48,14 @@ const redisUtil = {
   },
 };
 
-const isTwilioConfigured =
-  config.twilio.accountSid?.length >= 24 &&
-  config.twilio.accountSid?.startsWith('AC') &&
-  config.twilio.authToken?.length >= 24;
+// Note we have to use the 'test' credentials since these integration tests
+// require that we send messages to 'magic' phone numbers, which are only
+// supported by the twilio testing credentials.
+const isTwilioConfiguredForTest =
+  config.twilio.testAccountSid?.length >= 24 &&
+  config.twilio.testAccountSid?.startsWith('AC') &&
+  config.twilio.testAuthToken?.length >= 24 &&
+  config.twilio.credentialMode === 'test';
 
 describe(`#integration - recovery phone`, function () {
   // TODO: Something flakes... figure out where the slowdown is.
@@ -123,7 +127,7 @@ describe(`#integration - recovery phone`, function () {
   });
 
   it('sets up a recovery phone', async function () {
-    if (!isTwilioConfigured) {
+    if (!isTwilioConfiguredForTest) {
       this.skip('Invalid twilio accountSid or authToken. Check env / config!');
     }
     const createResp = await client.recoveryPhoneCreate(phoneNumber);
@@ -139,7 +143,7 @@ describe(`#integration - recovery phone`, function () {
   });
 
   it('can send, confirm code, verify session, and remove totp', async function () {
-    if (!isTwilioConfigured) {
+    if (!isTwilioConfiguredForTest) {
       this.skip('Invalid twilio accountSid or authToken. Check env / config!');
     }
 
@@ -179,7 +183,7 @@ describe(`#integration - recovery phone`, function () {
   });
 
   it('can remove recovery phone', async function () {
-    if (!isTwilioConfigured) {
+    if (!isTwilioConfiguredForTest) {
       this.skip('Invalid twilio accountSid or authToken. Check env / config!');
     }
     await client.recoveryPhoneCreate(phoneNumber);
@@ -196,7 +200,7 @@ describe(`#integration - recovery phone`, function () {
   });
 
   it('fails to set up invalid phone number', async function () {
-    if (!isTwilioConfigured) {
+    if (!isTwilioConfiguredForTest) {
       this.skip('Invalid twilio accountSid or authToken. Check env / config!');
     }
 
@@ -213,7 +217,7 @@ describe(`#integration - recovery phone`, function () {
   });
 
   it('can recreate recovery phone number', async function () {
-    if (!isTwilioConfigured) {
+    if (!isTwilioConfiguredForTest) {
       this.skip('Invalid twilio accountSid or authToken. Check env / config!');
     }
     await client.recoveryPhoneCreate(phoneNumber);
@@ -223,7 +227,7 @@ describe(`#integration - recovery phone`, function () {
   });
 
   it('fails to send a code to an unregistered phone number', async function () {
-    if (!isTwilioConfigured) {
+    if (!isTwilioConfiguredForTest) {
       this.skip('Invalid twilio accountSid or authToken. Check env / config!');
     }
 
@@ -240,7 +244,7 @@ describe(`#integration - recovery phone`, function () {
   });
 
   it('fails to register the same phone number again', async function () {
-    if (!isTwilioConfigured) {
+    if (!isTwilioConfiguredForTest) {
       this.skip('Invalid twilio accountSid or authToken. Check env / config!');
     }
     await client.recoveryPhoneCreate(phoneNumber);
@@ -260,7 +264,7 @@ describe(`#integration - recovery phone`, function () {
   });
 
   it('fails to use the same code again', async function () {
-    if (!isTwilioConfigured) {
+    if (!isTwilioConfiguredForTest) {
       this.skip('Invalid twilio accountSid or authToken. Check env / config!');
     }
     await client.recoveryPhoneCreate(phoneNumber);
@@ -362,7 +366,7 @@ describe(`#integration - recovery phone - customs checks`, function () {
   });
 
   it('prevents excessive calls to /recovery_phone/create', async function () {
-    if (!isTwilioConfigured) {
+    if (!isTwilioConfiguredForTest) {
       this.skip('Invalid twilio accountSid or authToken. Check env / config!');
     }
 
@@ -382,7 +386,7 @@ describe(`#integration - recovery phone - customs checks`, function () {
   });
 
   it('prevents excessive calls to /recovery_phone/confirm', async function () {
-    if (!isTwilioConfigured) {
+    if (!isTwilioConfiguredForTest) {
       this.skip('Invalid twilio accountSid or authToken. Check env / config!');
     }
 
@@ -408,7 +412,7 @@ describe(`#integration - recovery phone - customs checks`, function () {
   });
 
   it('prevents excessive calls to /recovery_phone/signin/send_code', async function () {
-    if (!isTwilioConfigured) {
+    if (!isTwilioConfiguredForTest) {
       this.skip('Invalid twilio accountSid or authToken. Check env / config!');
     }