diff --git a/src/app/factories/sms.factory.js b/src/app/factories/sms.factory.js deleted file mode 100644 index fd07ba659e..0000000000 --- a/src/app/factories/sms.factory.js +++ /dev/null @@ -1,48 +0,0 @@ -const twilio = require('twilio') -const featureManager = require('../../config/feature-manager').default -const smsService = require('../services/sms.service') - -const smsFactory = ({ isEnabled, props }) => { - let twilioClient - if (isEnabled && props) { - twilioClient = twilio(props.twilioApiKey, props.twilioApiSecret, { - accountSid: props.twilioAccountSid, - }) - } - - let twilioConfig = { - msgSrvcSid: props && props.twilioMsgSrvcSid, - client: twilioClient, - } - - return { - async sendVerificationOtp(recipient, otp, formId) { - if (isEnabled) { - return smsService.sendVerificationOtp( - recipient, - otp, - formId, - twilioConfig, - ) - } else { - throw new Error( - `Verification OTP has not been configured to be sent for mobile fields`, - ) - } - }, - async sendAdminContactOtp(recipient, otp, userId) { - if (isEnabled) { - return smsService.sendAdminContactOtp( - recipient, - otp, - userId, - twilioConfig, - ) - } else { - throw new Error(`Send Admin Contact OTP has not been enabled`) - } - }, - } -} - -module.exports = smsFactory(featureManager.get('sms')) diff --git a/src/app/models/form.server.model.ts b/src/app/models/form.server.model.ts index 5beb51b95a..b94d9e749b 100644 --- a/src/app/models/form.server.model.ts +++ b/src/app/models/form.server.model.ts @@ -92,7 +92,7 @@ export interface IFormModel extends Model { deactivateById(formId: string): Promise } -type IEncryptedFormModel = Model +type IEncryptedFormModel = Model & IFormModel const EncryptedFormSchema = new Schema({ publicKey: { @@ -101,7 +101,7 @@ const EncryptedFormSchema = new Schema({ }, }) -type IEmailFormModel = Model +type IEmailFormModel = Model & IFormModel const EmailFormSchema = new Schema({ emails: { diff --git a/src/app/modules/user/user.controller.ts b/src/app/modules/user/user.controller.ts index c35b35cf30..09d80903b2 100644 --- a/src/app/modules/user/user.controller.ts +++ b/src/app/modules/user/user.controller.ts @@ -5,7 +5,7 @@ import { StatusCodes } from 'http-status-codes' import { createLoggerWithLabel } from '../../../config/logger' import { IPopulatedUser } from '../../../types' -import SmsFactory from '../../factories/sms.factory' +import { SmsFactory } from '../../services/sms/sms.factory' import { ApplicationError } from '../core/core.errors' import { diff --git a/src/app/modules/verification/verification.factory.ts b/src/app/modules/verification/verification.factory.ts index 3914e633b9..0e7d4cd8e6 100644 --- a/src/app/modules/verification/verification.factory.ts +++ b/src/app/modules/verification/verification.factory.ts @@ -1,8 +1,7 @@ import { RequestHandler } from 'express' import { StatusCodes } from 'http-status-codes' -import featureManager from '../../../config/feature-manager' -import { FeatureNames } from '../../../config/feature-manager/types' +import featureManager, { FeatureNames } from '../../../config/feature-manager' import * as verification from './verification.controller' diff --git a/src/app/modules/verification/verification.service.ts b/src/app/modules/verification/verification.service.ts index 90272a8662..bf9402c9ef 100644 --- a/src/app/modules/verification/verification.service.ts +++ b/src/app/modules/verification/verification.service.ts @@ -12,10 +12,10 @@ import { IVerificationFieldSchema, IVerificationSchema, } from '../../../types' -import smsFactory from '../../factories/sms.factory' import getFormModel from '../../models/form.server.model' import getVerificationModel from '../../models/verification.server.model' import MailService from '../../services/mail.service' +import { SmsFactory } from '../../services/sms/sms.factory' import { generateOtp } from '../../utils/otp' import { ITransaction } from './verification.types' @@ -257,17 +257,15 @@ const sendOTPForField = async ( field: IVerificationFieldSchema, recipient: string, otp: string, -): Promise => { +): Promise => { const { fieldType } = field switch (fieldType) { case 'mobile': // call sms - it should validate the recipient - await smsFactory.sendVerificationOtp(recipient, otp, formId) - break + return SmsFactory.sendVerificationOtp(recipient, otp, formId) case 'email': // call email - it should validate the recipient - await MailService.sendVerificationOtp(recipient, otp) - break + return MailService.sendVerificationOtp(recipient, otp) default: throw new Error(`sendOTPForField: ${fieldType} is unsupported`) } diff --git a/src/app/services/mail.service.ts b/src/app/services/mail.service.ts index e58d7948cc..93aa35f190 100644 --- a/src/app/services/mail.service.ts +++ b/src/app/services/mail.service.ts @@ -107,7 +107,10 @@ export class MailService { * @param mail Mail data to send with * @param sendOptions Extra options to better identify mail, such as form or mail id. */ - #sendNodeMail = async (mail: MailOptions, sendOptions?: SendMailOptions) => { + #sendNodeMail = async ( + mail: MailOptions, + sendOptions?: SendMailOptions, + ): Promise => { const logMeta = { action: '#sendNodeMail', mailId: sendOptions?.mailId, @@ -141,12 +144,12 @@ export class MailService { }) try { - const response = await this.#transporter.sendMail(mail) + await this.#transporter.sendMail(mail) logger.info({ message: `Mail successfully sent on attempt ${attemptNum}`, meta: logMeta, }) - return response + return true } catch (err) { // Pass errors to the callback logger.error({ @@ -182,7 +185,7 @@ export class MailService { form, submission, index, - }: SendSingleAutoreplyMailArgs) => { + }: SendSingleAutoreplyMailArgs): Promise => { const emailSubject = autoReplyMailData.subject || `Thank you for submitting ${form.title}` // Sender's name appearing after "("" symbol gets truncated. Escaping it @@ -229,7 +232,10 @@ export class MailService { * @param otp the otp to send * @throws error if mail fails, to be handled by the caller */ - sendVerificationOtp = async (recipient: string, otp: string) => { + sendVerificationOtp = async ( + recipient: string, + otp: string, + ): Promise => { // TODO(#42): Remove param guards once whole backend is TypeScript. if (!otp) { throw new Error('OTP is missing.') @@ -269,7 +275,7 @@ export class MailService { recipient: string otp: string ipAddress: string - }): ResultAsync => { + }): ResultAsync => { return generateLoginOtpHtml({ appName: this.#appName, appUrl: this.#appUrl, @@ -326,7 +332,7 @@ export class MailService { bounceType: BounceType | undefined formTitle: string formId: string - }) => { + }): Promise => { const htmlData: BounceNotificationHtmlData = { formTitle, formLink: `${this.#appUrl}/${formId}`, @@ -374,7 +380,7 @@ export class MailService { question: string answer: string | number }[] - }) => { + }): Promise => { const refNo = submission.id const formTitle = form.title const submissionTime = moment(submission.created) @@ -447,7 +453,7 @@ export class MailService { responsesData, autoReplyMailDatas, attachments = [], - }: SendAutoReplyEmailsArgs) => { + }: SendAutoReplyEmailsArgs): Promise[]> => { // Data to render both the submission details mail HTML body PDF. const renderData: AutoreplySummaryRenderData = { refNo: submission.id, diff --git a/src/app/services/sms/__tests__/sms.factory.spec.ts b/src/app/services/sms/__tests__/sms.factory.spec.ts new file mode 100644 index 0000000000..89c59e4626 --- /dev/null +++ b/src/app/services/sms/__tests__/sms.factory.spec.ts @@ -0,0 +1,127 @@ +import Twilio from 'twilio' + +import { + FeatureNames, + ISms, + RegisteredFeature, +} from 'src/config/feature-manager' + +import { createSmsFactory } from '../sms.factory' +import * as SmsService from '../sms.service' +import { TwilioConfig } from '../sms.types' + +// This is hoisted and thus a const cannot be passed in. +jest.mock('twilio', () => + jest.fn().mockImplementation(() => ({ + mocked: 'this is mocked', + })), +) + +const MOCKED_TWILIO = ({ + mocked: 'this is mocked', +} as unknown) as Twilio.Twilio + +describe('sms.factory', () => { + beforeEach(() => jest.clearAllMocks()) + + describe('sms feature disabled', () => { + const MOCK_DISABLED_SMS_FEATURE: RegisteredFeature = { + isEnabled: false, + props: {} as ISms, + } + + const SmsFactory = createSmsFactory(MOCK_DISABLED_SMS_FEATURE) + + it('should throw error when invoking sendAdminContactOtp', () => { + // Act + const invocation = () => + SmsFactory.sendAdminContactOtp('anything', 'anything', 'anything') + + // Assert + expect(invocation).toThrowError( + 'sendAdminContactOtp: SMS feature must be enabled in Feature Manager first', + ) + }) + + it('should throw error when invoking sendVerificationOtp', () => { + // Act + const invocation = () => + SmsFactory.sendVerificationOtp('anything', 'anything', 'anything') + + // Assert + expect(invocation).toThrowError( + 'sendVerificationOtp: SMS feature must be enabled in Feature Manager first', + ) + }) + }) + + describe('sms feature enabled', () => { + const MOCK_ENABLED_SMS_FEATURE: RegisteredFeature = { + isEnabled: true, + props: { + twilioAccountSid: 'ACrandomTwilioSid', + twilioApiKey: 'SKrandomTwilioAPIKEY', + twilioApiSecret: 'this is a super secret', + twilioMsgSrvcSid: 'formsg-is-great-pleasehelpme', + }, + } + + const SmsFactory = createSmsFactory(MOCK_ENABLED_SMS_FEATURE) + + it('should call SmsService counterpart when invoking sendAdminContactOtp', async () => { + // Arrange + const serviceContactSpy = jest + .spyOn(SmsService, 'sendAdminContactOtp') + .mockResolvedValue(true) + + const mockArguments: Parameters = [ + 'mockRecipient', + 'mockOtp', + 'mockFormId', + ] + + // Act + await SmsFactory.sendAdminContactOtp(...mockArguments) + + // Assert + const expectedTwilioConfig: TwilioConfig = { + msgSrvcSid: MOCK_ENABLED_SMS_FEATURE.props.twilioMsgSrvcSid, + client: MOCKED_TWILIO, + } + + expect(serviceContactSpy).toHaveBeenCalledTimes(1) + expect(serviceContactSpy).toHaveBeenCalledWith( + ...mockArguments, + expectedTwilioConfig, + ) + }) + + it('should call SmsService counterpart when invoking sendVerificationOtp', async () => { + // Arrange + const serviceVfnSpy = jest + .spyOn(SmsService, 'sendVerificationOtp') + .mockResolvedValue(true) + + const mockArguments: Parameters = [ + 'mockRecipient', + 'mockOtp', + 'mockUserId', + ] + + // Act + await SmsFactory.sendVerificationOtp(...mockArguments) + + // Assert + const expectedTwilioConfig: TwilioConfig = { + msgSrvcSid: MOCK_ENABLED_SMS_FEATURE.props.twilioMsgSrvcSid, + client: MOCKED_TWILIO, + } + + expect(serviceVfnSpy).toHaveBeenCalledTimes(1) + expect(serviceVfnSpy).toHaveBeenCalledWith( + ...mockArguments, + expectedTwilioConfig, + ) + }) + }) +}) diff --git a/src/app/services/sms/__tests__/sms.service.spec.ts b/src/app/services/sms/__tests__/sms.service.spec.ts new file mode 100644 index 0000000000..0496e118ca --- /dev/null +++ b/src/app/services/sms/__tests__/sms.service.spec.ts @@ -0,0 +1,201 @@ +import mongoose from 'mongoose' +import dbHandler from 'tests/unit/backend/helpers/jest-db' + +import getFormModel from 'src/app/models/form.server.model' +import { VfnErrors } from 'src/shared/util/verification' +import { FormOtpData, IFormSchema, IUserSchema, ResponseMode } from 'src/types' + +import getSmsCountModel from '../sms_count.server.model' +import * as SmsService from '../sms.service' +import { LogType, SmsType, TwilioConfig } from '../sms.types' + +const FormModel = getFormModel(mongoose) +const SmsCountModel = getSmsCountModel(mongoose) + +// Test numbers provided by Twilio: +// https://www.twilio.com/docs/iam/test-credentials +const TWILIO_TEST_NUMBER = '+15005550006' + +const MOCK_MSG_SRVC_SID = 'mockMsgSrvcSid' + +const MOCK_VALID_CONFIG = ({ + msgSrvcSid: MOCK_MSG_SRVC_SID, + client: { + messages: { + create: jest.fn().mockResolvedValue({ + status: 'testStatus', + sid: 'testSid', + }), + }, + }, +} as unknown) as TwilioConfig + +const MOCK_INVALID_CONFIG = ({ + msgSrvcSid: MOCK_MSG_SRVC_SID, + client: { + messages: { + create: jest.fn().mockResolvedValue({ + status: 'testStatus', + sid: undefined, + errorCode: 21211, + }), + }, + }, +} as unknown) as TwilioConfig + +const smsCountSpy = jest.spyOn(SmsCountModel, 'logSms') + +describe('sms.service', () => { + let testUser: IUserSchema + + beforeAll(async () => await dbHandler.connect()) + beforeEach(async () => { + const { user } = await dbHandler.insertFormCollectionReqs() + testUser = user + smsCountSpy.mockClear() + }) + afterEach(async () => await dbHandler.clearDatabase()) + afterAll(async () => await dbHandler.closeDatabase()) + + describe('sendVerificationOtp', () => { + let mockOtpData: FormOtpData + let testForm: IFormSchema + + beforeEach(async () => { + testForm = await FormModel.create({ + title: 'Test Form', + emails: [testUser.email], + admin: testUser._id, + responseMode: ResponseMode.Email, + }) + + mockOtpData = { + form: testForm._id, + formAdmin: { + email: testUser.email, + userId: testUser._id, + }, + } + }) + + it('should throw error when retrieved otpData is null', async () => { + // Arrange + // Return null on Form method + jest.spyOn(FormModel, 'getOtpData').mockResolvedValueOnce(null) + + // Act + const actualPromise = SmsService.sendVerificationOtp( + /* recipient= */ TWILIO_TEST_NUMBER, + /* otp= */ '111111', + /* formId= */ testForm._id, + /* defaultConfig= */ MOCK_VALID_CONFIG, + ) + + await expect(actualPromise).rejects.toThrowError() + }) + + it('should log and send verification OTP when sending has no errors', async () => { + // Arrange + jest.spyOn(FormModel, 'getOtpData').mockResolvedValueOnce(mockOtpData) + + // Act + const actualPromise = SmsService.sendVerificationOtp( + /* recipient= */ TWILIO_TEST_NUMBER, + /* otp= */ '111111', + /* formId= */ testForm._id, + /* defaultConfig= */ MOCK_VALID_CONFIG, + ) + + // Assert + // Should resolve to true + await expect(actualPromise).resolves.toEqual(true) + // Logging should also have happened. + const expectedLogParams = { + otpData: mockOtpData, + msgSrvcSid: MOCK_MSG_SRVC_SID, + smsType: SmsType.verification, + logType: LogType.success, + } + expect(smsCountSpy).toHaveBeenCalledWith(expectedLogParams) + }) + + it('should log failure and throw error when verification OTP fails to send', async () => { + // Arrange + jest.spyOn(FormModel, 'getOtpData').mockResolvedValueOnce(mockOtpData) + + // Act + const actualPromise = SmsService.sendVerificationOtp( + /* recipient= */ TWILIO_TEST_NUMBER, + /* otp= */ '111111', + /* formId= */ testForm._id, + /* defaultConfig= */ MOCK_INVALID_CONFIG, + ) + + // Assert + const expectedError = new Error(VfnErrors.InvalidMobileNumber) + expectedError.name = VfnErrors.SendOtpFailed + + await expect(actualPromise).rejects.toThrow(expectedError) + // Logging should also have happened. + const expectedLogParams = { + otpData: mockOtpData, + msgSrvcSid: MOCK_MSG_SRVC_SID, + smsType: SmsType.verification, + logType: LogType.failure, + } + expect(smsCountSpy).toHaveBeenCalledWith(expectedLogParams) + }) + }) + + describe('sendAdminContactOtp', () => { + it('should log and send contact OTP when sending has no errors', async () => { + // Act + const actualPromise = SmsService.sendAdminContactOtp( + /* recipient= */ TWILIO_TEST_NUMBER, + /* otp= */ '111111', + /* userId= */ testUser._id, + /* defaultConfig= */ MOCK_VALID_CONFIG, + ) + + // Assert + // Should resolve to true + await expect(actualPromise).resolves.toEqual(true) + // Logging should also have happened. + const expectedLogParams = { + otpData: { + admin: testUser._id, + }, + msgSrvcSid: MOCK_MSG_SRVC_SID, + smsType: SmsType.adminContact, + logType: LogType.success, + } + expect(smsCountSpy).toHaveBeenCalledWith(expectedLogParams) + }) + }) + + it('should log failure and throw error when contact OTP fails to send', async () => { + // Act + const actualPromise = SmsService.sendAdminContactOtp( + /* recipient= */ TWILIO_TEST_NUMBER, + /* otp= */ '111111', + /* userId= */ testUser._id, + /* defaultConfig= */ MOCK_INVALID_CONFIG, + ) + + // Assert + const expectedError = new Error(VfnErrors.InvalidMobileNumber) + expectedError.name = VfnErrors.SendOtpFailed + + await expect(actualPromise).rejects.toThrow(expectedError) + // Logging should also have happened. + const expectedLogParams = { + otpData: { + admin: testUser._id, + }, + msgSrvcSid: MOCK_MSG_SRVC_SID, + smsType: SmsType.adminContact, + logType: LogType.failure, + } + expect(smsCountSpy).toHaveBeenCalledWith(expectedLogParams) + }) +}) diff --git a/tests/unit/backend/models/sms_count.server.model.spec.ts b/src/app/services/sms/__tests__/sms_count.server.model.spec.ts similarity index 96% rename from tests/unit/backend/models/sms_count.server.model.spec.ts rename to src/app/services/sms/__tests__/sms_count.server.model.spec.ts index a3faddf2d2..49b51e2525 100644 --- a/tests/unit/backend/models/sms_count.server.model.spec.ts +++ b/src/app/services/sms/__tests__/sms_count.server.model.spec.ts @@ -1,11 +1,11 @@ +/* eslint-disable @typescript-eslint/ban-ts-comment */ import { ObjectId } from 'bson' import { cloneDeep, merge, omit } from 'lodash' import mongoose from 'mongoose' +import dbHandler from 'tests/unit/backend/helpers/jest-db' -import getSmsCountModel from 'src/app/models/sms_count.server.model' -import { IVerificationSmsCount, LogType, SmsType } from 'src/types' - -import dbHandler from '../helpers/jest-db' +import getSmsCountModel from '../sms_count.server.model' +import { IVerificationSmsCount, LogType, SmsType } from '../sms.types' const SmsCount = getSmsCountModel(mongoose) @@ -194,7 +194,7 @@ describe('SmsCount', () => { form: MOCK_FORM_ID, }).lean() - expect(actualLog!._id).toBeDefined() + expect(actualLog?._id).toBeDefined() // Retrieve object and compare to params, remove indeterministic keys const actualSavedObject = omit(actualLog, ['_id', 'createdAt', '__v']) expect(actualSavedObject).toEqual(expectedLog) @@ -220,7 +220,7 @@ describe('SmsCount', () => { form: MOCK_FORM_ID, }).lean() - expect(actualLog!._id).toBeDefined() + expect(actualLog?._id).toBeDefined() // Retrieve object and compare to params, remove indeterministic keys const actualSavedObject = omit(actualLog, ['_id', 'createdAt', '__v']) expect(actualSavedObject).toEqual(expectedLog) diff --git a/src/app/services/sms/sms.errors.ts b/src/app/services/sms/sms.errors.ts new file mode 100644 index 0000000000..a7f11c906b --- /dev/null +++ b/src/app/services/sms/sms.errors.ts @@ -0,0 +1,16 @@ +import { ApplicationError } from '../../modules/core/core.errors' + +export class SmsSendError extends ApplicationError { + code?: number + sendStatus: unknown + + constructor( + message = 'Error sending OTP. Please try again later and if the problem persists, contact us.', + code?: number, + sendStatus?: unknown, + ) { + super(message) + this.code = code + this.sendStatus = sendStatus + } +} diff --git a/src/app/services/sms/sms.factory.ts b/src/app/services/sms/sms.factory.ts new file mode 100644 index 0000000000..430e6767c8 --- /dev/null +++ b/src/app/services/sms/sms.factory.ts @@ -0,0 +1,65 @@ +import Twilio from 'twilio' + +import FeatureManager, { + FeatureNames, + RegisteredFeature, +} from '../../../config/feature-manager' + +import { sendAdminContactOtp, sendVerificationOtp } from './sms.service' +import { TwilioConfig } from './sms.types' + +interface ISmsFactory { + sendVerificationOtp: ( + recipient: string, + otp: string, + formId: string, + ) => ReturnType + sendAdminContactOtp: ( + recipient: string, + otp: string, + userId: string, + ) => ReturnType +} + +const smsFeature = FeatureManager.get(FeatureNames.Sms) + +// Exported for testing. +export const createSmsFactory = ( + smsFeature: RegisteredFeature, +): ISmsFactory => { + if (!smsFeature.isEnabled) { + const errorMessage = 'SMS feature must be enabled in Feature Manager first' + return { + sendAdminContactOtp: () => { + throw new Error(`sendAdminContactOtp: ${errorMessage}`) + }, + sendVerificationOtp: () => { + throw new Error(`sendVerificationOtp: ${errorMessage}`) + }, + } + } + + const { + twilioAccountSid, + twilioApiKey, + twilioApiSecret, + twilioMsgSrvcSid, + } = smsFeature.props + + const twilioClient = Twilio(twilioApiKey, twilioApiSecret, { + accountSid: twilioAccountSid, + }) + const twilioConfig: TwilioConfig = { + msgSrvcSid: twilioMsgSrvcSid, + client: twilioClient, + } + + return { + sendVerificationOtp: (recipient, otp, formId) => + sendVerificationOtp(recipient, otp, formId, twilioConfig), + sendAdminContactOtp: (recipient, otp, userId) => + sendAdminContactOtp(recipient, otp, userId, twilioConfig), + } +} + +export const SmsFactory = createSmsFactory(smsFeature) diff --git a/src/app/services/sms.service.ts b/src/app/services/sms/sms.service.ts similarity index 81% rename from src/app/services/sms.service.ts rename to src/app/services/sms/sms.service.ts index 316a4d31c8..4d7c7f7963 100644 --- a/src/app/services/sms.service.ts +++ b/src/app/services/sms/sms.service.ts @@ -3,19 +3,22 @@ import mongoose from 'mongoose' import NodeCache from 'node-cache' import Twilio from 'twilio' -import config from '../../config/config' -import { createLoggerWithLabel } from '../../config/logger' -import { isPhoneNumber } from '../../shared/util/phone-num-validation' -import { VfnErrors } from '../../shared/util/verification' +import config from '../../../config/config' +import { createLoggerWithLabel } from '../../../config/logger' +import { isPhoneNumber } from '../../../shared/util/phone-num-validation' +import { VfnErrors } from '../../../shared/util/verification' +import { AdminContactOtpData, FormOtpData } from '../../../types' +import getFormModel from '../../models/form.server.model' + +import getSmsCountModel from './sms_count.server.model' +import { SmsSendError } from './sms.errors' import { - AdminContactOtpData, - FormOtpData, LogSmsParams, LogType, SmsType, -} from '../../types' -import getFormModel from '../models/form.server.model' -import getSmsCountModel from '../models/sms_count.server.model' + TwilioConfig, + TwilioCredentials, +} from './sms.types' const logger = createLoggerWithLabel(module) const SmsCount = getSmsCountModel(mongoose) @@ -28,13 +31,6 @@ const secretsManager = new SecretsManager({ region: config.aws.region }) // credentials, or wait 10 seconds before. const twilioClientCache = new NodeCache({ deleteOnExpire: true, stdTTL: 10 }) -type TwilioCredentials = { - accountSid: string - apiKey: string - apiSecret: string - messagingServiceSid: string -} - /** * Retrieves credentials from secrets manager * @param msgSrvcName The name of credential stored in the secret manager. @@ -71,11 +67,6 @@ const getCredentials = async ( return null } -type TwilioConfig = { - client: Twilio.Twilio - msgSrvcSid: string -} - /** * * @param msgSrvcName The name of credential stored in the secret manager @@ -135,17 +126,6 @@ const getTwilio = async ( return defaultConfig } -/** - * Retrieve the relevant data required to send an OTP from given formId - * @param formId The form to retrieve data from - * @returns Relevant OTP data containing the linked messaging service name (if available), and form details such as its id and the admin. - * - */ -const getOtpDataFromForm = async (formId: string) => { - const otpData = await Form.getOtpData(formId) - return otpData -} - /** * Sends a message to a valid phone number * @param twilioConfig The configuration used to send OTPs with @@ -179,7 +159,7 @@ const send = async ( // Sent but with error code. // Throw error to be caught in catch block. if (!sid || errorCode) { - throw new TwilioError(errorMessage, errorCode, status) + throw new SmsSendError(errorMessage, errorCode, status) } // Log success @@ -189,6 +169,7 @@ const send = async ( msgSrvcSid, logType: LogType.success, } + SmsCount.logSms(logParams).catch((err) => { logger.error({ message: 'Error logging sms count to database', @@ -200,6 +181,14 @@ const send = async ( }) }) + logger.info({ + message: 'Successfully sent sms', + meta: { + action: 'send', + otpData, + }, + }) + return true }) .catch((err) => { @@ -232,7 +221,7 @@ const send = async ( // Invalid number error code, throw a more reasonable error for error // handling. // See https://www.twilio.com/docs/api/errors/21211 - if (err.code === 21211) { + if (err?.code === 21211) { const invalidOtpError = new Error(VfnErrors.InvalidMobileNumber) invalidOtpError.name = VfnErrors.SendOtpFailed throw invalidOtpError @@ -249,12 +238,12 @@ const send = async ( * @param formId Form id for logging. * */ -const sendVerificationOtp = async ( +export const sendVerificationOtp = async ( recipient: string, otp: string, formId: string, defaultConfig: TwilioConfig, -) => { +): Promise => { logger.info({ message: `Sending verification OTP for ${formId}`, meta: { @@ -262,7 +251,7 @@ const sendVerificationOtp = async ( formId, }, }) - const otpData = await getOtpDataFromForm(formId) + const otpData = await Form.getOtpData(formId) if (!otpData) { const errMsg = `Unable to retrieve otpData from ${formId}` @@ -282,12 +271,12 @@ const sendVerificationOtp = async ( return send(twilioData, otpData, recipient, message, SmsType.verification) } -const sendAdminContactOtp = async ( +export const sendAdminContactOtp = async ( recipient: string, otp: string, userId: string, defaultConfig: TwilioConfig, -) => { +): Promise => { logger.info({ message: `Sending admin contact verification OTP for ${userId}`, meta: { @@ -304,24 +293,3 @@ const sendAdminContactOtp = async ( return send(defaultConfig, otpData, recipient, message, SmsType.adminContact) } - -class TwilioError extends Error { - code: number - status: string - - constructor(message: string, code: number, status: string) { - super(message) - this.code = code - this.status = status - this.name = this.constructor.name - - // Set the prototype explicitly. - // See https://github.com/facebook/jest/issues/8279 - Object.setPrototypeOf(this, TwilioError.prototype) - } -} - -module.exports = { - sendVerificationOtp, - sendAdminContactOtp, -} diff --git a/src/types/sms_count.ts b/src/app/services/sms/sms.types.ts similarity index 77% rename from src/types/sms_count.ts rename to src/app/services/sms/sms.types.ts index 48ca223996..a5d52222f3 100644 --- a/src/types/sms_count.ts +++ b/src/app/services/sms/sms.types.ts @@ -1,7 +1,12 @@ import { Document, Model } from 'mongoose' +import { Twilio } from 'twilio' -import { FormOtpData, IFormSchema } from './form' -import { AdminContactOtpData, IUserSchema } from './user' +import { + AdminContactOtpData, + FormOtpData, + IFormSchema, + IUserSchema, +} from 'src/types' export enum SmsType { verification = 'VERIFICATION', @@ -50,3 +55,15 @@ export type IAdminContactSmsCountSchema = ISmsCountSchema export interface ISmsCountModel extends Model { logSms: (logParams: LogSmsParams) => Promise } + +export type TwilioCredentials = { + accountSid: string + apiKey: string + apiSecret: string + messagingServiceSid: string +} + +export type TwilioConfig = { + client: Twilio + msgSrvcSid: string +} diff --git a/src/app/models/sms_count.server.model.ts b/src/app/services/sms/sms_count.server.model.ts similarity index 91% rename from src/app/models/sms_count.server.model.ts rename to src/app/services/sms/sms_count.server.model.ts index f702563f2b..c0e2299d96 100644 --- a/src/app/models/sms_count.server.model.ts +++ b/src/app/services/sms/sms_count.server.model.ts @@ -1,5 +1,8 @@ import { Mongoose, Schema } from 'mongoose' +import { FORM_SCHEMA_ID } from '../../models/form.server.model' +import { USER_SCHEMA_ID } from '../../models/user.server.model' + import { IAdminContactSmsCountSchema, ISmsCount, @@ -9,10 +12,7 @@ import { LogSmsParams, LogType, SmsType, -} from '../../types' - -import { FORM_SCHEMA_ID } from './form.server.model' -import { USER_SCHEMA_ID } from './user.server.model' +} from './sms.types' const SMS_COUNT_SCHEMA_NAME = 'SmsCount' @@ -101,7 +101,7 @@ const compileSmsCountModel = (db: Mongoose) => { * @param db The mongoose instance to retrieve the SmsCount model from * @returns The SmsCount model */ -const getSmsCountModel = (db: Mongoose) => { +const getSmsCountModel = (db: Mongoose): ISmsCountModel => { try { return db.model(SMS_COUNT_SCHEMA_NAME) as ISmsCountModel } catch { diff --git a/src/config/feature-manager/index.ts b/src/config/feature-manager/index.ts index cf39394a8a..eaf41a85d4 100644 --- a/src/config/feature-manager/index.ts +++ b/src/config/feature-manager/index.ts @@ -8,6 +8,8 @@ import spcpMyInfo from './spcp-myinfo.config' import verifiedFields from './verified-fields.config' import webhookVerifiedContent from './webhook-verified-content.config' +export * from './types' + const featureManager = new FeatureManager() // Register features and associated middleware/fallbacks diff --git a/src/config/feature-manager/types.ts b/src/config/feature-manager/types.ts index d9e6a98c75..89de1fd588 100644 --- a/src/config/feature-manager/types.ts +++ b/src/config/feature-manager/types.ts @@ -80,6 +80,11 @@ export interface IFeatureManager { [FeatureNames.WebhookVerifiedContent]: IWebhookVerifiedContent } +export interface RegisteredFeature { + isEnabled: boolean + props: IFeatureManager[T] +} + export interface RegisterableFeature { name: K schema: Schema diff --git a/src/config/feature-manager/util/FeatureManager.class.ts b/src/config/feature-manager/util/FeatureManager.class.ts index 09e5aaa84b..77e5f7f146 100644 --- a/src/config/feature-manager/util/FeatureManager.class.ts +++ b/src/config/feature-manager/util/FeatureManager.class.ts @@ -3,16 +3,16 @@ import validator from 'convict-format-with-validator' import _ from 'lodash' import { createLoggerWithLabel } from '../../logger' -import { FeatureNames, IFeatureManager, RegisterableFeature } from '../types' +import { + FeatureNames, + IFeatureManager, + RegisterableFeature, + RegisteredFeature, +} from '../types' const logger = createLoggerWithLabel(module) convict.addFormat(validator.url) -interface RegisteredFeature { - isEnabled: boolean - props: IFeatureManager[T] -} - export default class FeatureManager { public states: Partial> // Map some feature names to some env vars @@ -96,9 +96,9 @@ export default class FeatureManager { * Return props registered for requested feature * @param name the feature to return properties for */ - props(name: K) { + props(name: K): IFeatureManager[K] { if (this.states[name] !== undefined) { - return this.properties[name] + return this.properties[name] as IFeatureManager[K] } // Not enabled or not in state. throw new Error(`A feature called ${name} does not exist`) @@ -109,11 +109,10 @@ export default class FeatureManager { * and whether requested feature is enabled * @param name the name of the feature to return */ - get(name: FeatureNames): RegisteredFeature { + get(name: K): RegisteredFeature { return { isEnabled: this.isEnabled(name), - // TODO (#317): remove usage of non-null assertion - props: this.props(name)!, + props: this.props(name), } } } diff --git a/src/config/formsg-sdk.ts b/src/config/formsg-sdk.ts index 84aa64ea8e..5dda108394 100644 --- a/src/config/formsg-sdk.ts +++ b/src/config/formsg-sdk.ts @@ -3,9 +3,8 @@ import { get } from 'lodash' import * as vfnConstants from '../shared/util/verification' -import { FeatureNames } from './feature-manager/types' import { formsgSdkMode } from './config' -import featureManager from './feature-manager' +import featureManager, { FeatureNames } from './feature-manager' const formsgSdk = formsgSdkPackage({ webhookSecretKey: get( diff --git a/src/loaders/express/helmet.ts b/src/loaders/express/helmet.ts index 008c0b909a..05e6da84a6 100644 --- a/src/loaders/express/helmet.ts +++ b/src/loaders/express/helmet.ts @@ -4,8 +4,7 @@ import { ContentSecurityPolicyOptions } from 'helmet/dist/middlewares/content-se import { get } from 'lodash' import config from '../../config/config' -import featureManager from '../../config/feature-manager' -import { FeatureNames } from '../../config/feature-manager/types' +import featureManager, { FeatureNames } from '../../config/feature-manager' const helmetMiddlewares = () => { // Only add the "Strict-Transport-Security" header if request is https. diff --git a/src/loaders/express/locals.ts b/src/loaders/express/locals.ts index 5439d35184..a3e5fe77f2 100644 --- a/src/loaders/express/locals.ts +++ b/src/loaders/express/locals.ts @@ -2,8 +2,7 @@ import ejs from 'ejs' import { get } from 'lodash' import config from '../../config/config' -import featureManager from '../../config/feature-manager' -import { FeatureNames } from '../../config/feature-manager/types' +import featureManager, { FeatureNames } from '../../config/feature-manager' // Construct js with environment variables needed by frontend const frontendVars = { diff --git a/src/types/index.ts b/src/types/index.ts index b1e4ab0b98..ae47076e39 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -10,7 +10,6 @@ export * from './login' export * from './mail' export * from './myinfo_hash' export * from './response' -export * from './sms_count' export * from './sns' export * from './submission' export * from './token' diff --git a/tests/unit/backend/modules/user/user.controller.spec.ts b/tests/unit/backend/modules/user/user.controller.spec.ts index d057cfdc02..011a021149 100644 --- a/tests/unit/backend/modules/user/user.controller.spec.ts +++ b/tests/unit/backend/modules/user/user.controller.spec.ts @@ -1,16 +1,16 @@ import { StatusCodes } from 'http-status-codes' import { mocked } from 'ts-jest/utils' -import SmsFactory from 'src/app/factories/sms.factory' import * as UserController from 'src/app/modules/user/user.controller' import { InvalidOtpError } from 'src/app/modules/user/user.errors' import * as UserService from 'src/app/modules/user/user.service' +import { SmsFactory } from 'src/app/services/sms/sms.factory' import { IPopulatedUser, IUser, IUserSchema } from 'src/types' import expressHandler from '../../helpers/jest-express' jest.mock('src/app/modules/user/user.service') -jest.mock('src/app/factories/sms.factory') +jest.mock('src/app/services/sms/sms.factory') const MockUserService = mocked(UserService) const MockSmsFactory = mocked(SmsFactory) diff --git a/tests/unit/backend/modules/verification/verification.service.spec.ts b/tests/unit/backend/modules/verification/verification.service.spec.ts index babde3bff3..5df8847826 100644 --- a/tests/unit/backend/modules/verification/verification.service.spec.ts +++ b/tests/unit/backend/modules/verification/verification.service.spec.ts @@ -3,7 +3,6 @@ import { ObjectId } from 'bson' import mongoose from 'mongoose' import { mocked } from 'ts-jest/utils' -import smsFactory from 'src/app/factories/sms.factory' import getFormModel from 'src/app/models/form.server.model' import getVerificationModel from 'src/app/models/verification.server.model' import { @@ -14,6 +13,7 @@ import { verifyOtp, } from 'src/app/modules/verification/verification.service' import MailService from 'src/app/services/mail.service' +import { SmsFactory } from 'src/app/services/sms/sms.factory' import { generateOtp } from 'src/app/utils/otp' import formsgSdk from 'src/config/formsg-sdk' import { BasicField, IUserSchema, IVerificationSchema } from 'src/types' @@ -29,8 +29,8 @@ jest.mock('src/app/utils/otp') const MockGenerateOtp = mocked(generateOtp, true) jest.mock('src/config/formsg-sdk') const MockFormsgSdk = mocked(formsgSdk, true) -jest.mock('src/app/factories/sms.factory') -const MockSmsFactory = mocked(smsFactory, true) +jest.mock('src/app/services/sms/sms.factory') +const MockSmsFactory = mocked(SmsFactory, true) jest.mock('src/app/services/mail.service') const MockMailService = mocked(MailService, true) jest.mock('bcrypt') diff --git a/tests/unit/backend/services/mail.service.spec.ts b/tests/unit/backend/services/mail.service.spec.ts index 2f5811d02e..b517cd0b35 100644 --- a/tests/unit/backend/services/mail.service.spec.ts +++ b/tests/unit/backend/services/mail.service.spec.ts @@ -91,8 +91,7 @@ describe('mail.service', () => { it('should send verification otp successfully', async () => { // Arrange // sendMail should return mocked success response - const mockedResponse = 'mockedSuccessResponse' - sendMailSpy.mockResolvedValueOnce(mockedResponse) + sendMailSpy.mockResolvedValueOnce('mockedSuccessResponse') const expectedArgument = await generateExpectedArg() @@ -103,7 +102,7 @@ describe('mail.service', () => { ) // Assert - await expect(pendingSend).resolves.toEqual(mockedResponse) + await expect(pendingSend).resolves.toEqual(true) // Check arguments passed to sendNodeMail expect(sendMailSpy).toHaveBeenCalledTimes(1) expect(sendMailSpy).toHaveBeenCalledWith(expectedArgument) @@ -126,14 +125,13 @@ describe('mail.service', () => { it('should autoretry when 4xx error is thrown by sendNodeMail and pass if second try passes', async () => { // Arrange // sendMail should return mocked success response - const mockedResponse = 'mockedSuccessResponse' const mock4xxReject = { responseCode: 454, message: 'oh no something went wrong', } sendMailSpy .mockRejectedValueOnce(mock4xxReject) - .mockReturnValueOnce(mockedResponse) + .mockReturnValueOnce('mockedSuccessResponse') const expectedArgument = await generateExpectedArg() @@ -144,7 +142,7 @@ describe('mail.service', () => { ) // Assert - await expect(pendingSendVerification).resolves.toEqual(mockedResponse) + await expect(pendingSendVerification).resolves.toEqual(true) // Check arguments passed to sendNodeMail // Should have been called two times since it rejected the first one and // resolved @@ -232,8 +230,7 @@ describe('mail.service', () => { it('should send login otp successfully', async () => { // Arrange // sendMail should return mocked success response - const mockedResponse = 'mockedSuccessResponse' - sendMailSpy.mockResolvedValueOnce(mockedResponse) + sendMailSpy.mockResolvedValueOnce('mockedSuccessResponse') const expectedArgument = await generateExpectedArg() @@ -246,7 +243,7 @@ describe('mail.service', () => { // Assert expect(actualResult.isOk()).toBe(true) - expect(actualResult._unsafeUnwrap()).toEqual(mockedResponse) + expect(actualResult._unsafeUnwrap()).toEqual(true) // Check arguments passed to sendNodeMail expect(sendMailSpy).toHaveBeenCalledTimes(1) expect(sendMailSpy).toHaveBeenCalledWith(expectedArgument) @@ -270,14 +267,13 @@ describe('mail.service', () => { it('should autoretry when 4xx error is thrown by sendNodeMail and pass if second try passes', async () => { // Arrange // sendMail should return mocked success response - const mockedResponse = 'mockedSuccessResponse' const mock4xxReject = { responseCode: 454, message: 'oh no something went wrong', } sendMailSpy .mockRejectedValueOnce(mock4xxReject) - .mockReturnValueOnce(mockedResponse) + .mockReturnValueOnce('mockedSuccessResponse') const expectedArgument = await generateExpectedArg() @@ -290,7 +286,7 @@ describe('mail.service', () => { // Assert expect(actualResult.isOk()).toBe(true) - expect(actualResult._unsafeUnwrap()).toEqual(mockedResponse) + expect(actualResult._unsafeUnwrap()).toEqual(true) // Check arguments passed to sendNodeMail // Should have been called two times since it rejected the first one and // resolved @@ -430,8 +426,7 @@ describe('mail.service', () => { it('should send submission mail to admin successfully if form.emails is an array with a single string', async () => { // Arrange // sendMail should return mocked success response - const mockedResponse = 'mockedSuccessResponse' - sendMailSpy.mockResolvedValueOnce(mockedResponse) + sendMailSpy.mockResolvedValueOnce('mockedSuccessResponse') const expectedArgument = generateExpectedArgWithToField( MOCK_VALID_SUBMISSION_PARAMS.form.emails, @@ -443,7 +438,7 @@ describe('mail.service', () => { ) // Assert - await expect(pendingSend).resolves.toEqual(mockedResponse) + await expect(pendingSend).resolves.toEqual(true) // Check arguments passed to sendNodeMail expect(sendMailSpy).toHaveBeenCalledTimes(1) expect(sendMailSpy).toHaveBeenCalledWith(expectedArgument) @@ -452,8 +447,7 @@ describe('mail.service', () => { it('should send submission mail to admin successfully if form.emails is an array with a single comma separated string', async () => { // Arrange // sendMail should return mocked success response - const mockedResponse = 'mockedSuccessResponse' - sendMailSpy.mockResolvedValueOnce(mockedResponse) + sendMailSpy.mockResolvedValueOnce('mockedSuccessResponse') const formEmailsCommaSeparated = [ `${MOCK_VALID_EMAIL}, ${MOCK_VALID_EMAIL_2}`, @@ -469,7 +463,7 @@ describe('mail.service', () => { const pendingSend = mailService.sendSubmissionToAdmin(modifiedParams) // Assert - await expect(pendingSend).resolves.toEqual(mockedResponse) + await expect(pendingSend).resolves.toEqual(true) // Check arguments passed to sendNodeMail expect(sendMailSpy).toHaveBeenCalledTimes(1) expect(sendMailSpy).toHaveBeenCalledWith(expectedArgument) @@ -477,8 +471,7 @@ describe('mail.service', () => { it('should send submission mail to admin successfully if form.emails is an array with multiple emails', async () => { // Arrange - const mockedResponse = 'mockedSuccessResponse' - sendMailSpy.mockResolvedValueOnce(mockedResponse) + sendMailSpy.mockResolvedValueOnce('mockedSuccessResponse') const formMultipleEmailsArray = [MOCK_VALID_EMAIL, MOCK_VALID_EMAIL_2] const modifiedParams = cloneDeep(MOCK_VALID_SUBMISSION_PARAMS) @@ -492,7 +485,7 @@ describe('mail.service', () => { const pendingSend = mailService.sendSubmissionToAdmin(modifiedParams) // Assert - await expect(pendingSend).resolves.toEqual(mockedResponse) + await expect(pendingSend).resolves.toEqual(true) // Check arguments passed to sendNodeMail expect(sendMailSpy).toHaveBeenCalledTimes(1) expect(sendMailSpy).toHaveBeenCalledWith(expectedArgument) @@ -500,8 +493,7 @@ describe('mail.service', () => { it('should send submission mail to admin successfully if form.emails is an array with a mixture of emails and comma separated emails strings', async () => { // Arrange - const mockedResponse = 'mockedSuccessResponse' - sendMailSpy.mockResolvedValueOnce(mockedResponse) + sendMailSpy.mockResolvedValueOnce('mockedSuccessResponse') const formEmailsMixture = [ `${MOCK_VALID_EMAIL}, ${MOCK_VALID_EMAIL_2}`, @@ -516,7 +508,7 @@ describe('mail.service', () => { const pendingSend = mailService.sendSubmissionToAdmin(modifiedParams) // Assert - await expect(pendingSend).resolves.toEqual(mockedResponse) + await expect(pendingSend).resolves.toEqual(true) // Check arguments passed to sendNodeMail expect(sendMailSpy).toHaveBeenCalledTimes(1) expect(sendMailSpy).toHaveBeenCalledWith(expectedArgument) @@ -524,8 +516,7 @@ describe('mail.service', () => { it('should send submission mail to admin successfully if form.emails is an array with a mixture of emails, semi-colon, and comma separated emails strings', async () => { // Arrange - const mockedResponse = 'mockedSuccessResponse' - sendMailSpy.mockResolvedValueOnce(mockedResponse) + sendMailSpy.mockResolvedValueOnce('mockedSuccessResponse') const formEmailsMixture = [ `${MOCK_VALID_EMAIL}, ${MOCK_VALID_EMAIL_2}`, @@ -540,7 +531,7 @@ describe('mail.service', () => { const pendingSend = mailService.sendSubmissionToAdmin(modifiedParams) // Assert - await expect(pendingSend).resolves.toEqual(mockedResponse) + await expect(pendingSend).resolves.toEqual(true) // Check arguments passed to sendNodeMail expect(sendMailSpy).toHaveBeenCalledTimes(1) expect(sendMailSpy).toHaveBeenCalledWith(expectedArgument) @@ -576,10 +567,9 @@ describe('mail.service', () => { responseCode: 454, message: 'oh no something went wrong', } - const mockedResponse = 'mockedSuccessResponse' sendMailSpy .mockRejectedValueOnce(mock4xxReject) - .mockReturnValueOnce(mockedResponse) + .mockReturnValueOnce('mockedSuccessResponse') const formEmailsMixture = [ `${MOCK_VALID_EMAIL}, ${MOCK_VALID_EMAIL_2}`, @@ -596,7 +586,7 @@ describe('mail.service', () => { ) // Assert - await expect(pendingSendSubmission).resolves.toEqual(mockedResponse) + await expect(pendingSendSubmission).resolves.toEqual(true) // Check arguments passed to sendNodeMail // Should have been called two times since it rejected the first one and // resolved @@ -731,12 +721,9 @@ describe('mail.service', () => { it('should send single autoreply mail successfully with defaults', async () => { // Arrange - const mockedResponse = 'mockedSuccessResponse' - sendMailSpy.mockResolvedValueOnce(mockedResponse) + sendMailSpy.mockResolvedValueOnce('mockedSuccessResponse') - const expectedResponse = await Promise.allSettled([ - Promise.resolve(mockedResponse), - ]) + const expectedResponse = await Promise.allSettled([Promise.resolve(true)]) // Act const pendingSend = mailService.sendAutoReplyEmails(MOCK_AUTOREPLY_PARAMS) @@ -751,7 +738,7 @@ describe('mail.service', () => { it('should send array of multiple autoreply mails successfully with defaults', async () => { // Arrange const firstMockedResponse = 'mockedSuccessResponse1' - const secondMockedResponse = 'mockedSuccessResponse1' + const secondMockedResponse = 'mockedSuccessResponse2' sendMailSpy .mockResolvedValueOnce(firstMockedResponse) .mockResolvedValueOnce(secondMockedResponse) @@ -765,8 +752,8 @@ describe('mail.service', () => { secondExpectedArg.to = MOCK_VALID_EMAIL_3 const expectedResponse = await Promise.allSettled([ - Promise.resolve(firstMockedResponse), - Promise.resolve(secondMockedResponse), + Promise.resolve(true), + Promise.resolve(true), ]) // Act @@ -783,8 +770,7 @@ describe('mail.service', () => { it('should send single autoreply mail successfully with custom autoreply subject', async () => { // Arrange - const mockedResponse = 'mockedSuccessResponse' - sendMailSpy.mockResolvedValueOnce(mockedResponse) + sendMailSpy.mockResolvedValueOnce('mockedSuccessResponse') const customSubject = 'customSubject' const customDataParams = cloneDeep(MOCK_AUTOREPLY_PARAMS) @@ -792,9 +778,7 @@ describe('mail.service', () => { const expectedArg = { ...defaultExpectedArg, subject: customSubject } - const expectedResponse = await Promise.allSettled([ - Promise.resolve(mockedResponse), - ]) + const expectedResponse = await Promise.allSettled([Promise.resolve(true)]) // Act const pendingSend = mailService.sendAutoReplyEmails(customDataParams) @@ -808,8 +792,7 @@ describe('mail.service', () => { it('should send single autoreply mail successfully with custom autoreply sender', async () => { // Arrange - const mockedResponse = 'mockedSuccessResponse' - sendMailSpy.mockResolvedValueOnce(mockedResponse) + sendMailSpy.mockResolvedValueOnce('mockedSuccessResponse') const customSender = 'customSender@example.com' const customDataParams = cloneDeep(MOCK_AUTOREPLY_PARAMS) @@ -819,9 +802,7 @@ describe('mail.service', () => { ...defaultExpectedArg, from: `${customSender} <${MOCK_SENDER_EMAIL}>`, } - const expectedResponse = await Promise.allSettled([ - Promise.resolve(mockedResponse), - ]) + const expectedResponse = await Promise.allSettled([Promise.resolve(true)]) // Act const pendingSend = mailService.sendAutoReplyEmails(customDataParams) @@ -835,8 +816,7 @@ describe('mail.service', () => { it('should send single autoreply mail with attachment if autoReply.includeFormSummary is true', async () => { // Arrange - const mockedResponse = 'mockedSuccessResponse' - sendMailSpy.mockResolvedValueOnce(mockedResponse) + sendMailSpy.mockResolvedValueOnce('mockedSuccessResponse') const customDataParams = cloneDeep(MOCK_AUTOREPLY_PARAMS) customDataParams.autoReplyMailDatas[0].includeFormSummary = true @@ -867,9 +847,7 @@ describe('mail.service', () => { }, ], } - const expectedResponse = await Promise.allSettled([ - Promise.resolve(mockedResponse), - ]) + const expectedResponse = await Promise.allSettled([Promise.resolve(true)]) // Act const pendingSend = mailService.sendAutoReplyEmails(customDataParams) @@ -903,19 +881,16 @@ describe('mail.service', () => { responseCode: 454, message: 'oh no something went wrong', } - const mockedResponse = 'mockedSuccessResponse' sendMailSpy .mockRejectedValueOnce(mock4xxReject) - .mockReturnValueOnce(mockedResponse) + .mockReturnValueOnce('mockedSuccessResponse') const customSubject = 'customSubject' const customDataParams = cloneDeep(MOCK_AUTOREPLY_PARAMS) customDataParams.autoReplyMailDatas[0].subject = customSubject const expectedArg = { ...defaultExpectedArg, subject: customSubject } - const expectedResponse = await Promise.allSettled([ - Promise.resolve(mockedResponse), - ]) + const expectedResponse = await Promise.allSettled([Promise.resolve(true)]) // Act const pendingSend = mailService.sendAutoReplyEmails(customDataParams) @@ -1024,8 +999,7 @@ describe('mail.service', () => { it('should send permanent bounce notification successfully', async () => { // Arrange // sendMail should return mocked success response - const mockedResponse = 'mockedSuccessResponse' - sendMailSpy.mockResolvedValueOnce(mockedResponse) + sendMailSpy.mockResolvedValueOnce('mockedSuccessResponse') // Act const sent = await mailService.sendBounceNotification({ @@ -1037,7 +1011,7 @@ describe('mail.service', () => { }) const expectedArgs = await generateExpectedArg(BounceType.Permanent) // Assert - expect(sent).toEqual(mockedResponse) + expect(sent).toEqual(true) // Check arguments passed to sendNodeMail expect(sendMailSpy).toHaveBeenCalledTimes(1) expect(sendMailSpy).toHaveBeenCalledWith(expectedArgs) @@ -1046,8 +1020,7 @@ describe('mail.service', () => { it('should send transient bounce notification successfully', async () => { // Arrange // sendMail should return mocked success response - const mockedResponse = 'mockedSuccessResponse' - sendMailSpy.mockResolvedValueOnce(mockedResponse) + sendMailSpy.mockResolvedValueOnce('mockedSuccessResponse') // Act const sent = await mailService.sendBounceNotification({ @@ -1059,7 +1032,7 @@ describe('mail.service', () => { }) const expectedArgs = await generateExpectedArg(BounceType.Transient) // Assert - expect(sent).toEqual(mockedResponse) + expect(sent).toEqual(true) // Check arguments passed to sendNodeMail expect(sendMailSpy).toHaveBeenCalledTimes(1) expect(sendMailSpy).toHaveBeenCalledWith(expectedArgs) @@ -1085,14 +1058,13 @@ describe('mail.service', () => { it('should autoretry when 4xx error is thrown by sendNodeMail and pass if second try passes', async () => { // Arrange // sendMail should return mocked success response - const mockedResponse = 'mockedSuccessResponse' const mock4xxReject = { responseCode: 454, message: 'oh no something went wrong', } sendMailSpy .mockRejectedValueOnce(mock4xxReject) - .mockReturnValueOnce(mockedResponse) + .mockReturnValueOnce('mockedSuccessResponse') // Act const sent = await mailService.sendBounceNotification({ @@ -1105,7 +1077,7 @@ describe('mail.service', () => { const expectedArgs = await generateExpectedArg(MOCK_BOUNCE_TYPE) // Assert - expect(sent).toEqual(mockedResponse) + expect(sent).toEqual(true) // Check arguments passed to sendNodeMail // Should have been called two times since it rejected the first one and // resolved diff --git a/tests/unit/backend/services/sms.service.spec.js b/tests/unit/backend/services/sms.service.spec.js deleted file mode 100644 index b62845fc38..0000000000 --- a/tests/unit/backend/services/sms.service.spec.js +++ /dev/null @@ -1,130 +0,0 @@ -const dbHandler = require('../helpers/db-handler') -const { VfnErrors } = spec('dist/backend/shared/util/verification') -const { SmsType, LogType } = spec('dist/backend/types') - -const Form = dbHandler.makeModel('form.server.model', 'Form') -const SmsCount = dbHandler.makeModel('sms_count.server.model', 'SmsCount') - -const SmsService = spec('dist/backend/app/services/sms.service') - -// Test numbers provided by Twilio: -// https://www.twilio.com/docs/iam/test-credentials -const TWILIO_TEST_NUMBER = '+15005550006' - -const MOCK_MSG_SRVC_SID = 'mockMsgSrvcSid' - -describe('sms.service', () => { - const MOCK_VALID_CONFIG = { - msgSrvcSid: MOCK_MSG_SRVC_SID, - client: { - messages: { - create: jasmine.createSpy().and.resolveTo({ - status: 'testStatus', - sid: 'testSid', - }), - }, - }, - } - - const MOCK_INVALID_CONFIG = { - msgSrvcSid: MOCK_MSG_SRVC_SID, - client: { - messages: { - create: jasmine.createSpy().and.resolveTo({ - status: 'testStatus', - sid: undefined, - errorCode: 21211, - }), - }, - }, - } - - beforeAll(async () => await dbHandler.connect()) - afterEach(async () => await dbHandler.clearDatabase()) - afterAll(async () => await dbHandler.closeDatabase()) - - describe('sendVerificationOtp', () => { - let testForm - let mockOtpData - beforeEach(async () => { - const { form, user } = await dbHandler.preloadCollections() - - mockOtpData = { - form: form._id, - formAdmin: { - email: user.email, - userId: user._id, - }, - } - - testForm = form - }) - it('should throw error if otpData is null', async () => { - // Arrange - // Return null on Form method - spyOn(Form, 'getOtpData').and.returnValue(null) - - await expectAsync( - SmsService.sendVerificationOtp( - /* recipient= */ TWILIO_TEST_NUMBER, - /* otp= */ '111111', - /* formId= */ testForm._id, - /* defaultConfig= */ MOCK_VALID_CONFIG, - ), - ).toBeRejectedWithError() - }) - - it('should log and send verification OTP if twilio has no errors', async () => { - // Arrange - spyOn(Form, 'getOtpData').and.returnValue(mockOtpData) - const smsCountSpy = spyOn(SmsCount, 'logSms').and.callThrough() - // Act + Assert - await expectAsync( - SmsService.sendVerificationOtp( - /* recipient= */ TWILIO_TEST_NUMBER, - /* otp= */ '111111', - /* formId= */ testForm._id, - /* defaultConfig= */ MOCK_VALID_CONFIG, - ), - // Should resolve to true - ).toBeResolvedTo(true) - - // Logging should also have happened. - const expectedLogParams = { - otpData: mockOtpData, - msgSrvcSid: MOCK_MSG_SRVC_SID, - smsType: SmsType.verification, - logType: LogType.success, - } - expect(smsCountSpy).toHaveBeenCalledWith(expectedLogParams) - }) - - it('should log failure and throw error if twilio failed to send', async () => { - // Arrange - spyOn(Form, 'getOtpData').and.returnValue(mockOtpData) - const smsCountSpy = spyOn(SmsCount, 'logSms').and.callThrough() - - // Act + Assert - const expectedError = new Error(VfnErrors.InvalidMobileNumber) - expectedError.name = VfnErrors.SendOtpFailed - - await expectAsync( - SmsService.sendVerificationOtp( - /* recipient= */ TWILIO_TEST_NUMBER, - /* otp= */ '111111', - /* formId= */ testForm._id, - /* defaultConfig= */ MOCK_INVALID_CONFIG, - ), - ).toBeRejectedWith(expectedError) - - // Logging should also have happened. - const expectedLogParams = { - otpData: mockOtpData, - msgSrvcSid: MOCK_MSG_SRVC_SID, - smsType: SmsType.verification, - logType: LogType.failure, - } - expect(smsCountSpy).toHaveBeenCalledWith(expectedLogParams) - }) - }) -})