feat(sms-limiting): check admin sms count on public submission (#2326)

* feat(form.utils): adds util method to check if a form is onboarded

* feat(verification.util): adds util method for checking if an admin has exceed limit

this is duplicated from another PR because they are only tangentially dependent.

* feat(admin-form.service): adds method to send email/disable verification when sms limits are reached

* refactor(verification.controller): adds step to check sms counts for otp generation

* refactor(admin-form.service): simplify check by removing redundant neverthrow usage

* test(admin-form.service.spec): adds unit tests for new methods

* fix(verification.controller): shifted order so that retrieveFullForm is awaited on

* test(verification.controller.spec): adds check for disabling mail

* refactor(admin-form.service): shifted initialization of admin id below

* refactor(verification): updated to use env var rather than const

* refactor(admin-form.service): removed export of helper method; tests now shifted to wrapper method

* refactor(form.utils): removed duplicate method due to PR separation

* refactor(verification): shifted over processing admin sms count to verification module

src/app/modules/form/admin-form/admin-form.service.ts

@@ -63,7 +63,7 @@ import {
   TransferOwnershipError,
 } from '../form.errors'
 import { getFormModelByResponseMode } from '../form.service'
-import { getFormFieldById, getLogicById, isOnboardedForm } from '../form.utils'
+import { getFormFieldById, getLogicById, isFormOnboarded } from '../form.utils'
 
 import { PRESIGNED_POST_EXPIRY_SECS } from './admin-form.constants'
 import {
@@ -1135,7 +1135,7 @@ const isMobileFieldUpdateAllowed = (
   PossibleDatabaseError | SmsLimitExceededError
 > => {
   // Field can always update if it's not a verifiable field or if the form has been onboarded
-  if (!isVerifiableMobileField(mobileField) || isOnboardedForm(form)) {
+  if (!isVerifiableMobileField(mobileField) || isFormOnboarded(form)) {
     return okAsync(form)
   }
 

src/app/modules/form/form.utils.ts

@@ -10,6 +10,7 @@ import {
   Permission,
   ResponseMode,
 } from '../../../types'
+import { smsConfig } from '../../config/features/sms.config'
 import { isMongooseDocumentArray } from '../../utils/mongoose'
 
 // Converts '[email protected], [email protected]' to ['[email protected]', '[email protected]']
@@ -124,9 +125,15 @@ export const getLogicById = (
   return form_logics.find((logic) => logicId === String(logic._id)) ?? null
 }
 
-// Typeguard to check if a form has a message service id
-export const isOnboardedForm = <T extends IForm = IForm>(
-  form: T,
+/**
+ * Checks if a given form is onboarded (the form's message service name is defined and different from the default)
+ * @param form The form to check
+ * @returns boolean indicating if the form is/is not onboarded
+ */
+export const isFormOnboarded = <T extends IForm = IForm>(
+  form: Pick<T, 'msgSrvcName'>,
 ): form is IOnboardedForm<T> => {
-  return !!form.msgSrvcName
+  return form.msgSrvcName
+    ? !(form.msgSrvcName === smsConfig.twilioMsgSrvcSid)
+    : false
 }

src/app/modules/verification/__tests__/verification.controller.spec.ts

@@ -13,7 +13,7 @@ import {
 import { HashingError } from 'src/app/utils/hash'
 import * as OtpUtils from 'src/app/utils/otp'
 import { WAIT_FOR_OTP_SECONDS } from 'src/shared/util/verification'
-import { IFormSchema, IVerificationSchema } from 'src/types'
+import { IFormSchema, IPopulatedForm, IVerificationSchema } from 'src/types'
 
 import dbHandler from 'tests/unit/backend/helpers/jest-db'
 
@@ -614,10 +614,22 @@ describe('Verification controller', () => {
       },
     })
 
+    const MOCK_FORM = {
+      admin: {
+        _id: new ObjectId(),
+      },
+      title: 'i am a form',
+      _id: new ObjectId(),
+      permissionList: [{ email: '[email protected]' }],
+    } as IPopulatedForm
+
     beforeEach(() => {
       MockFormService.retrieveFormById.mockReturnValue(
         okAsync({} as IFormSchema),
       )
+      MockFormService.retrieveFullFormById.mockReturnValueOnce(
+        okAsync(MOCK_FORM),
+      )
 
       MockOtpUtils.generateOtpWithHash.mockReturnValue(
         okAsync({
@@ -631,6 +643,11 @@ describe('Verification controller', () => {
     })
 
     it('should return 201 when params are valid', async () => {
+      // Arrange
+      MockVerificationService.processAdminSmsCounts.mockReturnValueOnce(
+        okAsync(true),
+      )
+
       // Act
       await VerificationController._handleGenerateOtp(
         MOCK_REQ,
@@ -650,6 +667,9 @@ describe('Verification controller', () => {
         hashedOtp: MOCK_HASHED_OTP,
         recipient: MOCK_ANSWER,
       })
+      expect(
+        MockVerificationService.processAdminSmsCounts,
+      ).toHaveBeenCalledWith(MOCK_FORM)
       expect(mockRes.sendStatus).toHaveBeenCalledWith(StatusCodes.CREATED)
     })
 

src/app/modules/verification/__tests__/verification.service.spec.ts

@@ -4,16 +4,23 @@ import mongoose from 'mongoose'
 import { errAsync, okAsync } from 'neverthrow'
 import { mocked } from 'ts-jest/utils'
 
+import { smsConfig } from 'src/app/config/features/sms.config'
 import formsgSdk from 'src/app/config/formsg-sdk'
 import * as FormService from 'src/app/modules/form/form.service'
-import { MailSendError } from 'src/app/services/mail/mail.errors'
+import {
+  MailGenerationError,
+  MailSendError,
+} from 'src/app/services/mail/mail.errors'
 import MailService from 'src/app/services/mail/mail.service'
 import { SmsSendError } from 'src/app/services/sms/sms.errors'
 import { SmsFactory } from 'src/app/services/sms/sms.factory'
+import * as SmsService from 'src/app/services/sms/sms.service'
 import * as HashUtils from 'src/app/utils/hash'
+import { SMS_WARNING_TIERS } from 'src/shared/util/verification'
 import {
   BasicField,
   IFormSchema,
+  IPopulatedForm,
   IVerificationSchema,
   PublicTransaction,
   UpdateFieldData,
@@ -22,7 +29,9 @@ import {
 import dbHandler from 'tests/unit/backend/helpers/jest-db'
 
 import { DatabaseError } from '../../core/core.errors'
+import * as AdminFormService from '../../form/admin-form/admin-form.service'
 import { FormNotFoundError } from '../../form/form.errors'
+import * as FormUtils from '../../form/form.utils'
 import {
   FieldNotFoundInTransactionError,
   MissingHashDataError,
@@ -682,4 +691,152 @@ describe('Verification service', () => {
       expect(result._unsafeUnwrapErr()).toEqual(new WrongOtpError())
     })
   })
+
+  describe('processAdminSmsCounts', () => {
+    const MOCK_FORM = {
+      title: 'some mock form',
+      _id: new ObjectId(),
+      admin: {
+        _id: new ObjectId(),
+      },
+      permissionList: [{ email: '[email protected]' }],
+    } as IPopulatedForm
+    const onboardSpy = jest.spyOn(FormUtils, 'isFormOnboarded')
+    const retrievalSpy = jest.spyOn(SmsService, 'retrieveFreeSmsCounts')
+    const disableSpy = jest.spyOn(
+      AdminFormService,
+      'disableSmsVerificationsForUser',
+    )
+
+    it('should not do anything when the form is onboarded', async () => {
+      // Arrange
+      onboardSpy.mockReturnValueOnce(true)
+
+      // Act
+      const actual = await VerificationService.processAdminSmsCounts(MOCK_FORM)
+
+      // Assert
+      expect(actual._unsafeUnwrap()).toBe(true)
+      expect(retrievalSpy).not.toHaveBeenCalled()
+    })
+
+    it('should disable sms verifications and send email when sms limit is exceeded', async () => {
+      // Arrange
+
+      MockMailService.sendSmsVerificationDisabledEmail.mockReturnValueOnce(
+        okAsync(true),
+      )
+
+      disableSpy.mockReturnValueOnce(okAsync(true))
+      retrievalSpy.mockReturnValueOnce(
+        okAsync(smsConfig.smsVerificationLimit + 1),
+      )
+
+      // Act
+      const actual = await VerificationService.processAdminSmsCounts(MOCK_FORM)
+
+      // Assert
+      expect(actual._unsafeUnwrap()).toBe(true)
+      expect(
+        MockMailService.sendSmsVerificationDisabledEmail,
+      ).toHaveBeenCalledWith(MOCK_FORM)
+      // NOTE: String casting is required so that the test recognises them as equal
+      expect(disableSpy).toHaveBeenCalledWith(String(MOCK_FORM.admin._id))
+    })
+
+    it('should send a warning when the admin has sent out a certain number of sms', async () => {
+      // Arrange
+
+      MockMailService.sendSmsVerificationWarningEmail.mockReturnValueOnce(
+        okAsync(true),
+      )
+      retrievalSpy.mockReturnValueOnce(okAsync(SMS_WARNING_TIERS.LOW))
+
+      // Act
+      const actual = await VerificationService.processAdminSmsCounts(MOCK_FORM)
+
+      // Assert
+      expect(actual._unsafeUnwrap()).toBe(true)
+      expect(disableSpy).not.toHaveBeenCalled()
+      expect(
+        MockMailService.sendSmsVerificationWarningEmail,
+      ).toHaveBeenCalledWith(MOCK_FORM, SMS_WARNING_TIERS.LOW)
+    })
+
+    it('should not do anything when the sms sent by admin is not at any limit', async () => {
+      // Arrange
+
+      retrievalSpy.mockReturnValueOnce(okAsync(SMS_WARNING_TIERS.LOW - 1))
+
+      // Act
+      const actual = await VerificationService.processAdminSmsCounts(MOCK_FORM)
+
+      // Assert
+      expect(actual._unsafeUnwrap()).toBe(true)
+      expect(
+        MockMailService.sendSmsVerificationDisabledEmail,
+      ).not.toHaveBeenCalled()
+      expect(
+        MockMailService.sendSmsVerificationWarningEmail,
+      ).not.toHaveBeenCalled()
+      expect(disableSpy).not.toHaveBeenCalled()
+    })
+
+    it('should propagate any errors encountered during warning mail sending', async () => {
+      // Arrange
+      const expected = new MailGenerationError('big ded')
+      MockMailService.sendSmsVerificationWarningEmail.mockReturnValueOnce(
+        errAsync(expected),
+      )
+      retrievalSpy.mockReturnValueOnce(okAsync(SMS_WARNING_TIERS.LOW))
+
+      // Act
+      const actual = await VerificationService.processAdminSmsCounts(MOCK_FORM)
+
+      // Assert
+      expect(actual._unsafeUnwrapErr()).toBe(expected)
+      expect(disableSpy).not.toHaveBeenCalled()
+      expect(
+        MockMailService.sendSmsVerificationWarningEmail,
+      ).toHaveBeenCalledWith(MOCK_FORM, SMS_WARNING_TIERS.LOW)
+    })
+
+    it('should propagate any errors encountered during disabled mail sending', async () => {
+      // Arrange
+      const expected = new MailGenerationError('big ded')
+      MockMailService.sendSmsVerificationDisabledEmail.mockReturnValueOnce(
+        errAsync(expected),
+      )
+      retrievalSpy.mockReturnValueOnce(
+        okAsync(smsConfig.smsVerificationLimit + 1),
+      )
+
+      // Act
+      const actual = await VerificationService.processAdminSmsCounts(MOCK_FORM)
+
+      // Assert
+      expect(disableSpy).not.toHaveBeenCalled()
+      expect(
+        MockMailService.sendSmsVerificationWarningEmail,
+      ).not.toHaveBeenCalled()
+      expect(actual._unsafeUnwrapErr()).toBe(expected)
+      expect(
+        MockMailService.sendSmsVerificationDisabledEmail,
+      ).toHaveBeenCalledWith(MOCK_FORM)
+    })
+
+    it('should return the error received when retrieval of sms counts fails', async () => {
+      // Arrange
+      const expected = new DatabaseError()
+      onboardSpy.mockReturnValueOnce(false)
+      retrievalSpy.mockReturnValueOnce(errAsync(expected))
+
+      // Act
+      const actual = await VerificationService.processAdminSmsCounts(MOCK_FORM)
+
+      // Assert
+      expect(actual._unsafeUnwrapErr()).toBe(expected)
+      expect(retrievalSpy).toBeCalledWith(String(MOCK_FORM.admin._id))
+    })
+  })
 })

src/app/modules/verification/verification.controller.ts

@@ -221,7 +221,21 @@ export const _handleGenerateOtp: ControllerHandler<
           transactionId,
         }),
       )
-      .map(() => res.sendStatus(StatusCodes.CREATED))
+      .map(() => {
+        res.sendStatus(StatusCodes.CREATED)
+        // NOTE: This is returned because tests require this to avoid async mocks interfering with each other.
+        // However, this is not an issue in reality because express does not require awaiting on the sendStatus call.
+        return FormService.retrieveFullFormById(formId)
+          .andThen((form) => VerificationService.processAdminSmsCounts(form))
+          .mapErr((error) => {
+            logger.error({
+              message:
+                'Error checking sms counts or deactivating OTP verification for admin',
+              meta: logMeta,
+              error,
+            })
+          })
+      })
       .mapErr((error) => {
         logger.error({
           message: 'Error creating new OTP',