diff --git a/packages/attestation-service/migrations/20211109200901-attestation-v8.js b/packages/attestation-service/migrations/20211109200901-attestation-v8.js new file mode 100644 index 00000000000..60095627459 --- /dev/null +++ b/packages/attestation-service/migrations/20211109200901-attestation-v8.js @@ -0,0 +1,68 @@ +'use strict' + +module.exports = { + up: async (queryInterface, Sequelize) => { + const transaction = await queryInterface.sequelize.transaction() + + try { + await queryInterface.removeIndex( + 'Attestations', + ['ongoingDeliveryId'], + { fields: ['ongoingDeliveryId'], unique: true }, + { transaction } + ) + await queryInterface.addIndex( + 'Attestations', + ['ongoingDeliveryId'], + { fields: ['ongoingDeliveryId'], unique: false }, + { transaction } + ) + await transaction.commit() + } catch (error) { + await transaction.rollback() + throw error + } + }, + down: async (queryInterface, Sequelize) => { + const transaction = await queryInterface.sequelize.transaction() + try { + await queryInterface.removeIndex( + 'Attestations', + ['ongoingDeliveryId'], + { fields: ['ongoingDeliveryId'], unique: false }, + { transaction } + ) + await queryInterface.addIndex( + 'Attestations', + ['ongoingDeliveryId'], + { fields: ['ongoingDeliveryId'], unique: true }, + { transaction } + ) + } catch (error) { + await transaction.rollback() + if ( + error.errors.length == 1 && + error.errors[0].path == 'ongoingDeliveryId' && + error.errors[0].type == 'unique violation' + ) { + try { + console.warn('Duplicates in ongoingDeliveryId; nulling out column and retrying.') + await queryInterface.removeColumn('Attestations', 'ongoingDeliveryId') + await queryInterface.addColumn('Attestations', 'ongoingDeliveryId', { + type: Sequelize.STRING, + allowNull: true, + }) + await queryInterface.addIndex( + 'Attestations', + ['ongoingDeliveryId'], + { fields: ['ongoingDeliveryId'], unique: true }, + { transaction } + ) + } catch (error) { + await transaction.rollback() + throw error + } + } + } + }, +} diff --git a/packages/attestation-service/src/db.ts b/packages/attestation-service/src/db.ts index ddee3204dcf..066e718ca22 100644 --- a/packages/attestation-service/src/db.ts +++ b/packages/attestation-service/src/db.ts @@ -322,6 +322,8 @@ export async function findAttestationByDeliveryId( ): Promise { return (await getAttestationTable()).findOne({ where: { ongoingDeliveryId }, + // Ensure deterministic result, since deliveryId uniqueness is not enforced + order: [['createdAt', 'DESC']], ...options, }) } diff --git a/packages/attestation-service/src/sms/twilioVerify.ts b/packages/attestation-service/src/sms/twilioVerify.ts index cfe4772526c..77fe228f75e 100644 --- a/packages/attestation-service/src/sms/twilioVerify.ts +++ b/packages/attestation-service/src/sms/twilioVerify.ts @@ -108,6 +108,10 @@ export class TwilioVerifyProvider extends TwilioSmsProvider { requestParams.locale = locale } } + + // 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) diff --git a/packages/attestation-service/test/__mocks__/twilio.ts b/packages/attestation-service/test/__mocks__/twilio.ts index d45160dc419..49cb41174f5 100644 --- 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 index 9f8d564c946..2f3215c6072 100644 --- 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' 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', @@ -22,34 +26,29 @@ describe('TwilioSmsProvider tests', () => { appSignature: undefined, language: 'en', } - - beforeEach(() => { - jest.clearAllMocks() - }) - - it('should initialize and send SMS via TwilioVerifyProvider', 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 TwilioMessagingProvider', async () => { - const twilioMessagingProvider = new TwilioMessagingProvider( - twilioSid, - twilioAuthToken, - unsupportedRegionCodes, - messagingServiceSid - ) - await twilioMessagingProvider.initialize('fake-delivery-status-url') - await twilioMessagingProvider.sendSms(attestation) - expect(mockMessagesCreate).toBeCalledTimes(1) - expect(mockVerifyCreate).not.toBeCalled() - }) + }) + it('should initialize and send SMS via TwilioVerifyProvider', 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 TwilioMessagingProvider', async () => { + const twilioMessagingProvider = new TwilioMessagingProvider( + twilioSid, + twilioAuthToken, + unsupportedRegionCodes, + messagingServiceSid + ) + await twilioMessagingProvider.initialize('fake-delivery-status-url') + await twilioMessagingProvider.sendSms(attestation) + expect(mockMessagesCreate).toBeCalledTimes(1) + expect(mockVerifyCreate).not.toBeCalled() }) })