Implement cancel-verify method

celo-org · Nov 11, 2021 · 9b89c59 · 9b89c59
1 parent 80f6ce6
commit 9b89c59
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 21 deletions.
diff --git a/packages/attestation-service/src/models/attestation.ts b/packages/attestation-service/src/models/attestation.ts
@@ -15,14 +15,14 @@ export interface SmsFields {
   attestationCode: string | null
   appSignature: string | undefined
   language: string | undefined
+  attempt: number
 }
 
 export interface AttestationModel extends Model, SmsFields {
   readonly id: number
   securityCodeAttempt: number
   ongoingDeliveryId: string | null
   providers: string
-  attempt: number
   status: AttestationStatus
   errors: string | null
   createdAt: Date

diff --git a/packages/attestation-service/src/sms/twilioVerify.ts b/packages/attestation-service/src/sms/twilioVerify.ts
@@ -108,7 +108,34 @@ export class TwilioVerifyProvider extends TwilioSmsProvider {
         requestParams.locale = locale
       }
     }
+
+    // If there's a previous verification
+    // if (attestation.attempt) {
+    // console.log('hellooooo')
+    // const y = new Date()
+    // try {
+    //   // const x = await this.client.verify
+    //   await this.client.verify
+    //     .services(this.verifyServiceSid)
+    //     .verifications(attestation.phoneNumber)
+    //     .update({ status: 'canceled' })
+    //   // console.log(x)
+    // } catch (e) {
+    //   // let errorMsg = 'no error message'
+    //   // if (e instanceof Error) {
+    //   //   errorMsg = e.message
+    //   // }
+    //   // throw new Error(`Unable to cancel previous verification: ${errorMsg}`)
+    // }
+    // const z = new Date()
+    // console.log('total time: ', z.getTime() - y.getTime())
+    // // }
+
     let deliveryId: string
+
+    // Note: SID returned by Verify API is not unique for a phone number (within a 10 min interval)
+    // i.e. re-requests to the same phone number in <10 min will return an exisiting SID
+
     try {
       const m = await this.client.verify
         .services(this.verifyServiceSid)
@@ -138,9 +165,8 @@ export class TwilioVerifyProvider extends TwilioSmsProvider {
         .update({ status: 'canceled' })
     } catch {
       // This shouldn't throw a hard error though as this is to prevent a tiny edge case:
-      // >5 Verify requests to the same phone number in <10 min.
-      // At this point, the text has been sent; .
-      // throw new Error(`Canceling Verify SID ${deliveryId} failed with message ${e}`)
+      // 2 Verify requests for the same issuer + phone number in <10 min.
+      // At this point, the text has already been sent.
     } finally {
       return deliveryId
     }

diff --git a/packages/attestation-service/test/__mocks__/twilio.ts b/packages/attestation-service/test/__mocks__/twilio.ts
@@ -6,6 +6,7 @@ export const mockVerifyCreate = jest.fn((_obj: Object) => {
     sid: undefined,
   }
 })
+export const mockVerifyUpdate = jest.fn()
 export const mockMessagesCreate = jest.fn((_obj: Object) => {
   return {
     sid: undefined,
@@ -28,9 +29,16 @@ const twilio = jest.fn().mockImplementation((_twilioSid, _twilioAuthToken) => {
       services: Object.assign(
         (_sid: string) => {
           return {
-            verifications: {
-              create: mockVerifyCreate,
-            },
+            verifications: Object.assign(
+              (_sid: string) => {
+                return {
+                  update: mockVerifyUpdate,
+                }
+              },
+              {
+                create: mockVerifyCreate,
+              }
+            ),
           }
         },
         {

diff --git a/packages/attestation-service/test/sms/twilio.test.ts b/packages/attestation-service/test/sms/twilio.test.ts
@@ -1,16 +1,20 @@
+import { SmsFields } from '../../src/models/attestation'
 import { TwilioMessagingProvider, TwilioVerifyProvider } from '../../src/sms/twilio'
-import { mockMessagesCreate, mockVerifyCreate } from '../__mocks__/twilio'
+import { mockMessagesCreate, mockVerifyCreate, mockVerifyUpdate } from '../__mocks__/twilio'
 
 jest.mock('../__mocks__/twilio')
 
 describe('TwilioSmsProvider tests', () => {
-  describe('sendSms', () => {
-    const twilioSid = 'twilioSid-123!'
-    const verifyServiceSid = 'verify-sid-123!'
-    const twilioAuthToken = 'fakeAuth-123!'
-    const unsupportedRegionCodes = ['GH', 'IJ', 'KL']
-    const messagingServiceSid = 'messagingId-123!'
-    const attestation = {
+  const twilioSid = 'twilioSid-123!'
+  const verifyServiceSid = 'verify-sid-123!'
+  const twilioAuthToken = 'fakeAuth-123!'
+  const unsupportedRegionCodes = ['GH', 'IJ', 'KL']
+  const messagingServiceSid = 'messagingId-123!'
+  let attestation: SmsFields
+
+  beforeEach(() => {
+    jest.clearAllMocks()
+    attestation = {
       account: '0x123',
       identifier: '0x456',
       issuer: '0x789',
@@ -21,13 +25,23 @@ describe('TwilioSmsProvider tests', () => {
       attestationCode: '56789',
       appSignature: undefined,
       language: 'en',
+      attempt: 0,
     }
-
-    beforeEach(() => {
-      jest.clearAllMocks()
+  })
+  describe('TwilioVerifyProvider tests', () => {
+    it('should initialize and send SMS', async () => {
+      const twilioVerifyProvider = new TwilioVerifyProvider(
+        twilioSid,
+        twilioAuthToken,
+        unsupportedRegionCodes,
+        verifyServiceSid
+      )
+      await twilioVerifyProvider.initialize('fake-delivery-status-url')
+      await twilioVerifyProvider.sendSms(attestation)
+      expect(mockVerifyCreate).toBeCalledTimes(1)
+      expect(mockMessagesCreate).not.toBeCalled()
     })
-
-    it('should initialize and send SMS via TwilioVerifyProvider', async () => {
+    it('should create new SID on second attempt', async () => {
       const twilioVerifyProvider = new TwilioVerifyProvider(
         twilioSid,
         twilioAuthToken,
@@ -37,9 +51,12 @@ describe('TwilioSmsProvider tests', () => {
       await twilioVerifyProvider.initialize('fake-delivery-status-url')
       await twilioVerifyProvider.sendSms(attestation)
       expect(mockVerifyCreate).toBeCalledTimes(1)
+      expect(mockVerifyUpdate).toBeCalledTimes(1)
       expect(mockMessagesCreate).not.toBeCalled()
     })
-    it('should initialize and send SMS via TwilioMessagingProvider', async () => {
+  })
+  describe('TwilioMessagingProvider tests', () => {
+    it('should initialize and send SMS', async () => {
       const twilioMessagingProvider = new TwilioMessagingProvider(
         twilioSid,
         twilioAuthToken,