Refactor TwilioSmsProvider into two separate modules (messaging, verify)

.env.alfajores

@@ -96,7 +96,7 @@ ATTESTATION_SERVICE_DOCKER_IMAGE_TAG="attestation-service-alfajores"
 
 CELO_PROVIDER_BACKUPS=https://alfajores-forno.celo-testnet.org
 TELEKOM_FROM="+14157389085"
-SMS_PROVIDERS=twilio,nexmo,telekom
+SMS_PROVIDERS=twilioverify,twiliomessaging,nexmo,telekom
 SMS_PROVIDERS_RANDOMIZED="1"
 
 NEXMO_APPLICATIONS=0e8afe80-29f8-4e88-af03-5b5066161944,775fb58b-331b-4040-b1eb-7aed298b5685,07bd882c-5565-4387-a9e8-48fa2de3b3cc,93a3355e-ebed-4eeb-bb93-61f0c752add9,7a194b22-6f3c-488a-b174-933f7e875875,06a4573b-406d-48f6-97db-ef5652ce98c4,90b0915b-eb5f-4d27-942b-7d90d137ec1f,9c572771-f37d-4b02-b783-69c7934fe5fb,d93e193d-367f-4315-95a5-6dfb108288e5,adfeb89a-7dfb-49b8-9e3e-2a1284719986

.env.baklava

@@ -134,7 +134,7 @@ ATTESTATION_SERVICE_DOCKER_IMAGE_REPOSITORY="us.gcr.io/celo-testnet/celo-monorep
 ATTESTATION_SERVICE_DOCKER_IMAGE_TAG="attestation-service-baklava"
 
 CELO_PROVIDER_BACKUPS=https://baklava-forno.celo-testnet.org
-SMS_PROVIDERS=twilio,nexmo
+SMS_PROVIDERS=twilioverify,twiliomessaging,nexmo
 SMS_PROVIDERS_RANDOMIZED="1"
 
 # We only configured 15 nexmo applications. So delivery receipts from the second 15 will always go to the wrong place for now

packages/attestation-service/config/.env.development

@@ -78,3 +78,6 @@ LOG_FORMAT= human
 LOG_LEVEL=info
 
 APP_SIGNATURE=x
+
+# Phone number (prefixed with "+") for test-send-sms-twilio script
+# TEST_SMS_RECIPIENT=x

packages/attestation-service/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@celo/attestation-service",
-  "version": "1.4.0",
+  "version": "1.5.0-dev",
   "description": "Issues attestation messages for Celo's identity protocol",
   "main": "./lib/index.js",
   "types": "./lib/index.d.ts",
@@ -25,7 +25,8 @@
     "db:migrate:undo": "sequelize db:migrate:undo",
     "db:migrate:dev": "sequelize db:migrate",
     "dev": "NODE_ENV=dev CONFIG=config/.env.development nodemon",
-    "lint": "tslint -c tslint.json --project ."
+    "lint": "tslint -c tslint.json --project .",
+    "test-send-sms-twilio": "TS_NODE_FILES=true CONFIG=config/.env.development ts-node scripts/test-send-sms-twilio.ts"
   },
   "dependencies": {
     "@celo/contractkit": "1.3.1-dev",

packages/attestation-service/scripts/test-send-sms-twilio.ts

@@ -0,0 +1,104 @@
+/**
+ * Script for testing TwilioSmsProvider.sendSms
+ * (Verify & Messaging Services) using the real twilio API.
+ * Uses `.env.development` file for sensitive info; set your phone number
+ * as TEST_SMS_RECIPIENT to receive the messages and check against
+ * expected output (logged in console).
+ * Comment out cases under `testCases` as desired.
+ */
+
+import { fetchEnv, fetchEnvOrDefault } from '../src/env'
+import { SmsFields } from '../src/models/attestation'
+import { readUnsupportedRegionsFromEnv } from '../src/sms/base'
+import { TwilioSmsProvider } from '../src/sms/twilio'
+import { TwilioMessagingProvider } from '../src/sms/twilioMessaging'
+import { TwilioVerifyProvider } from '../src/sms/twilioVerify'
+;(async function main() {
+  const twilioSid = fetchEnv('TWILIO_ACCOUNT_SID')
+  const messagingServiceSid = fetchEnv('TWILIO_MESSAGING_SERVICE_SID')
+  const verifyServiceSid = fetchEnvOrDefault('TWILIO_VERIFY_SERVICE_SID', '')
+  const twilioAuthToken = fetchEnv('TWILIO_AUTH_TOKEN')
+  const unsupportedRegionCodes = readUnsupportedRegionsFromEnv(
+    'TWILIO_UNSUPPORTED_REGIONS',
+    'TWILIO_BLACKLIST'
+  )
+  const testPhoneNumber = fetchEnv('TEST_SMS_RECIPIENT')
+  const countryCode = 'DE'
+
+  enum SendMethod {
+    MESSAGE_SERVICE = 'message service',
+    VERIFY = 'verify API',
+  }
+
+  type TestCase = {
+    id: string
+    expectedSendMethod: SendMethod
+  }
+
+  const testCases: TestCase[] = [
+    {
+      id: '000000',
+      expectedSendMethod: SendMethod.MESSAGE_SERVICE,
+    },
+    {
+      id: '111111',
+      expectedSendMethod: SendMethod.VERIFY,
+    },
+  ]
+
+  testCases.map(async (testCase: TestCase) => {
+    const attestation: SmsFields = {
+      account: '0x123',
+      identifier: '0x456',
+      issuer: '0x789',
+      countryCode: countryCode,
+      phoneNumber: testPhoneNumber,
+      message: '',
+      securityCode: '',
+      attestationCode: '123',
+      appSignature: undefined,
+      language: 'en',
+    }
+
+    let twilioSmsProvider: TwilioSmsProvider
+    let expectedMsg: string
+    let expectedSidStart: string
+
+    switch (testCase.expectedSendMethod) {
+      case SendMethod.MESSAGE_SERVICE:
+        twilioSmsProvider = new TwilioMessagingProvider(
+          twilioSid,
+          twilioAuthToken,
+          unsupportedRegionCodes,
+          messagingServiceSid
+        )
+        expectedMsg = attestation.message = 'via-message:' + testCase.id
+        expectedSidStart = 'SM'
+        break
+      case SendMethod.VERIFY:
+        twilioSmsProvider = new TwilioVerifyProvider(
+          twilioSid,
+          twilioAuthToken,
+          unsupportedRegionCodes,
+          verifyServiceSid
+        )
+        expectedMsg = 'Your Celo verification code is: ' + testCase.id
+        attestation.securityCode = testCase.id
+        expectedSidStart = 'VE'
+        break
+    }
+
+    try {
+      await twilioSmsProvider.initialize()
+      const messageSID = await twilioSmsProvider.sendSms(attestation)
+      const messageSIDStart = messageSID.substring(0, 2)
+      console.log(`Message SID for id ${testCase.id}: ${messageSID}`)
+      console.log(`SMS should match: ${expectedMsg}`)
+      if (messageSIDStart !== expectedSidStart) {
+        throw new Error(`Returned message SID did not match expected starting letters`)
+      }
+    } catch (e) {
+      console.error(e)
+    }
+  })
+})()

packages/attestation-service/src/models/attestation.ts

@@ -1,17 +1,25 @@
 import { E164Number } from '@celo/utils/lib/io'
 import { BuildOptions, DataTypes, Model, Sequelize } from 'sequelize'
 
-export interface AttestationModel extends Model {
-  readonly id: number
+// Split out SmsFields from the underlying data model;
+// Contains fields relevant to the message itself, not to meta-info
+// about how/when the message was sent.
+export interface SmsFields {
   account: string
   identifier: string
   issuer: string
   countryCode: string
   phoneNumber: E164Number
   message: string
   securityCode: string | null
-  securityCodeAttempt: number
   attestationCode: string | null
+  appSignature: string | undefined
+  language: string | undefined
+}
+
+export interface AttestationModel extends Model, SmsFields {
+  readonly id: number
+  securityCodeAttempt: number
   ongoingDeliveryId: string | null
   providers: string
   attempt: number
@@ -24,8 +32,6 @@ export interface AttestationModel extends Model {
   recordError: (error: string) => void
   failure: () => boolean
   currentError: () => string | undefined
-  appSignature: string | undefined
-  language: string | undefined
 }
 
 export interface AttestationKey {

packages/attestation-service/src/sms/base.ts

@@ -2,7 +2,7 @@ import { E164Number } from '@celo/utils/lib/io'
 import Logger from 'bunyan'
 import express from 'express'
 import { fetchEnvOrDefault } from '../env'
-import { AttestationModel } from '../models/attestation'
+import { SmsFields } from '../models/attestation'
 
 export abstract class SmsProvider {
   abstract type: SmsProviderType
@@ -12,7 +12,7 @@ export abstract class SmsProvider {
     return !this.unsupportedRegionCodes.includes(countryCode.toUpperCase())
   }
   // Should throw Error when unsuccesful, return if successful
-  abstract sendSms(attestation: AttestationModel): Promise<string>
+  abstract sendSms(attestation: SmsFields): Promise<string>
 
   // if this provider supports delivery status updates to an endpoint delivery_<providername>/, should return 'GET' or 'POST'
   abstract deliveryStatusMethod(): string | null
@@ -27,6 +27,8 @@ export enum SmsProviderType {
   NEXMO = 'nexmo',
   UNKNOWN = 'unknown',
   TWILIO = 'twilio',
+  TWILIO_MESSAGING = 'twiliomessaging',
+  TWILIO_VERIFY = 'twilioverify',
   MESSAGEBIRD = 'messagebird',
   TELEKOM = 'telekom',
 }

packages/attestation-service/src/sms/index.ts

@@ -21,7 +21,7 @@ import { obfuscateNumber, SmsProvider, SmsProviderType } from './base'
 import { MessageBirdSmsProvider } from './messagebird'
 import { NexmoSmsProvider } from './nexmo'
 import { TelekomSmsProvider } from './telekom'
-import { TwilioSmsProvider } from './twilio'
+import { TwilioMessagingProvider, TwilioVerifyProvider } from './twilio'
 
 // Maximum delivery attempts (including first) regardless of provider
 const maxDeliveryAttempts = parseInt(
@@ -69,13 +69,23 @@ function phoneNumberTypeToString(t: PhoneNumberType): string {
   }
 }
 
+function providerNamesToList(providers: string) {
+  return (
+    providers
+      // Backwards compatibility: 'twilio' as syntactic sugar for 'twilioverify,twiliomessaging'
+      .replace(
+        new RegExp(`\\b(${SmsProviderType.TWILIO})\\b`, 'g'),
+        `${SmsProviderType.TWILIO_VERIFY},${SmsProviderType.TWILIO_MESSAGING}`
+      )
+      .split(',')
+      .filter((t) => t != null && t !== '')
+  )
+}
+
 export async function initializeSmsProviders(
   deliveryStatusURLForProviderType: (type: string) => string
 ) {
-  const smsProvidersToConfigure = fetchEnv('SMS_PROVIDERS')
-    .split(',')
-    .filter((t) => t != null && t !== '') as Array<SmsProviderType | string>
-
+  const smsProvidersToConfigure = providerNamesToList(fetchEnv('SMS_PROVIDERS'))
   if (smsProvidersToConfigure.length === 0) {
     throw new Error('You have to specify at least one sms provider')
   }
@@ -105,11 +115,21 @@ export async function initializeSmsProviders(
         smsProviders.push(nexmoProvider)
         smsProvidersByType[SmsProviderType.NEXMO] = nexmoProvider
         break
-      case SmsProviderType.TWILIO:
-        const twilioProvider = TwilioSmsProvider.fromEnv()
-        await twilioProvider.initialize(deliveryStatusURLForProviderType(configuredSmsProvider))
-        smsProviders.push(twilioProvider)
-        smsProvidersByType[SmsProviderType.TWILIO] = twilioProvider
+      case SmsProviderType.TWILIO_VERIFY:
+        const twilioVerifyProvider = TwilioVerifyProvider.fromEnv()
+        await twilioVerifyProvider.initialize(
+          deliveryStatusURLForProviderType(configuredSmsProvider)
+        )
+        smsProviders.push(twilioVerifyProvider)
+        smsProvidersByType[SmsProviderType.TWILIO_VERIFY] = twilioVerifyProvider
+        break
+      case SmsProviderType.TWILIO_MESSAGING:
+        const twilioMessagingProvider = TwilioMessagingProvider.fromEnv()
+        await twilioMessagingProvider.initialize(
+          deliveryStatusURLForProviderType(configuredSmsProvider)
+        )
+        smsProviders.push(twilioMessagingProvider)
+        smsProvidersByType[SmsProviderType.TWILIO_MESSAGING] = twilioMessagingProvider
         break
       default:
         throw new Error(`Unknown sms provider type specified: ${configuredSmsProvider}`)
@@ -124,9 +144,9 @@ function smsProvidersFor(
   phoneNumber: E164Number,
   logger: Logger
 ): SmsProvider[] {
-  const providersForRegion = fetchEnvOrDefault(`SMS_PROVIDERS_${countryCode}`, '')
-    .split(',')
-    .filter((t) => t != null && t !== '')
+  const providersForRegion = providerNamesToList(
+    fetchEnvOrDefault(`SMS_PROVIDERS_${countryCode}`, '')
+  )
   let providers =
     providersForRegion.length > 0
       ? providersForRegion.map((name) => smsProvidersByType[name])
@@ -184,10 +204,9 @@ function providersToCsv(providers: SmsProvider[]) {
 // but cannot require providers not configured or unsupported for this country code.
 function getProvidersFor(attestation: AttestationModel, logger: Logger) {
   const validProviders = smsProvidersFor(attestation.countryCode, attestation.phoneNumber, logger)
-  const attestationProviders = attestation.providers
-    .split(',')
-    .filter((t) => t != null && t !== '')
-    .map((name) => smsProvidersByType[name])
+  const attestationProviders = providerNamesToList(attestation.providers).map(
+    (name) => smsProvidersByType[name]
+  )
 
   for (const p of attestationProviders) {
     if (!validProviders.find((v) => p.type === v.type)) {

packages/attestation-service/src/sms/messagebird.ts

@@ -6,7 +6,7 @@ import initMB, { MessageBird } from 'messagebird'
 import fetch from 'node-fetch'
 import util from 'util'
 import { fetchEnv } from '../env'
-import { AttestationModel, AttestationStatus } from '../models/attestation'
+import { AttestationStatus, SmsFields } from '../models/attestation'
 import { readUnsupportedRegionsFromEnv, SmsProvider, SmsProviderType } from './base'
 import { receivedDeliveryReport } from './index'
 
@@ -97,7 +97,7 @@ export class MessageBirdSmsProvider extends SmsProvider {
     }
   }
 
-  async sendSms(attestation: AttestationModel) {
+  async sendSms(attestation: SmsFields) {
     const reference = randomBytes(8).toString('hex')
     const m = await util.promisify(this.messagebird.messages.create)({
       originator: 'Celo',

packages/attestation-service/src/sms/nexmo.ts

@@ -6,7 +6,7 @@ import Nexmo from 'nexmo'
 import { receivedDeliveryReport } from '.'
 import { fetchEnv, fetchEnvOrDefault, isYes } from '../env'
 import { Gauges } from '../metrics'
-import { AttestationModel, AttestationStatus } from '../models/attestation'
+import { AttestationStatus, SmsFields } from '../models/attestation'
 import { readUnsupportedRegionsFromEnv, SmsProvider, SmsProviderType } from './base'
 
 const phoneUtil = PhoneNumberUtil.getInstance()
@@ -106,7 +106,7 @@ export class NexmoSmsProvider extends SmsProvider {
 
   deliveryStatusHandlers = () => [bodyParser.json()]
 
-  async sendSms(attestation: AttestationModel) {
+  async sendSms(attestation: SmsFields) {
     const nexmoNumber = this.getMatchingNumber(attestation.countryCode)
     return new Promise<string>((resolve, reject) => {
       this.client.message.sendSms(

packages/attestation-service/src/sms/telekom.ts

@@ -4,7 +4,7 @@ import express from 'express'
 import fetch from 'node-fetch'
 import { receivedDeliveryReport } from '.'
 import { fetchEnv, fetchEnvOrDefault } from '../env'
-import { AttestationModel, AttestationStatus } from '../models/attestation'
+import { AttestationStatus, SmsFields } from '../models/attestation'
 import { readUnsupportedRegionsFromEnv, SmsProvider, SmsProviderType } from './base'
 
 export class TelekomSmsProvider extends SmsProvider {
@@ -71,7 +71,7 @@ export class TelekomSmsProvider extends SmsProvider {
     this.deliveryStatusURL = deliveryStatusURL
   }
 
-  async sendSms(attestation: AttestationModel) {
+  async sendSms(attestation: SmsFields) {
     const response = await fetch(this.serviceURL, {
       method: 'POST',
       headers: {