From c552a92ce7323d5f55af38eeea364631169f3697 Mon Sep 17 00:00:00 2001 From: seaerchin <44049504+seaerchin@users.noreply.github.com> Date: Wed, 28 Jul 2021 13:15:58 +0800 Subject: [PATCH] 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 --- .../__tests__/admin-form.service.spec.ts | 336 +++++++++++------- .../form/admin-form/admin-form.service.ts | 4 +- src/app/modules/form/form.utils.ts | 15 +- .../__tests__/verification.controller.spec.ts | 22 +- .../__tests__/verification.service.spec.ts | 159 ++++++++- .../verification/verification.controller.ts | 16 +- .../verification/verification.service.ts | 74 +++- src/shared/util/verification.ts | 6 + 8 files changed, 485 insertions(+), 147 deletions(-) diff --git a/src/app/modules/form/admin-form/__tests__/admin-form.service.spec.ts b/src/app/modules/form/admin-form/__tests__/admin-form.service.spec.ts index 9bcc88bf35..688932390a 100644 --- a/src/app/modules/form/admin-form/__tests__/admin-form.service.spec.ts +++ b/src/app/modules/form/admin-form/__tests__/admin-form.service.spec.ts @@ -68,32 +68,7 @@ import { FieldNotFoundError, InvalidFileTypeError, } from '../admin-form.errors' -import { - archiveForm, - createForm, - createFormField, - createFormLogic, - createPresignedPostUrlForImages, - createPresignedPostUrlForLogos, - deleteFormField, - deleteFormLogic, - disableSmsVerificationsForUser, - duplicateForm, - duplicateFormField, - editFormFields, - getDashboardForms, - getFormField, - reorderFormField, - shouldUpdateFormField, - transferFormOwnership, - updateEndPage, - updateForm, - updateFormCollaborators, - updateFormField, - updateFormLogic, - updateFormSettings, - updateStartPage, -} from '../admin-form.service' +import * as AdminFormService from '../admin-form.service' import { OverrideProps } from '../admin-form.types' import * as AdminFormUtils from '../admin-form.utils' @@ -138,7 +113,7 @@ describe('admin-form.service', () => { .mockResolvedValueOnce(mockDashboardForms) // Act - const actualResult = await getDashboardForms(mockUserId) + const actualResult = await AdminFormService.getDashboardForms(mockUserId) // Assert expect(getSpy).toHaveBeenCalledWith(mockUserId, mockUser.email) @@ -152,7 +127,7 @@ describe('admin-form.service', () => { MockUserService.findUserById.mockReturnValueOnce(errAsync(expectedError)) // Act - const actualResult = await getDashboardForms('any') + const actualResult = await AdminFormService.getDashboardForms('any') // Assert expect(actualResult.isErr()).toEqual(true) @@ -176,7 +151,7 @@ describe('admin-form.service', () => { .mockRejectedValueOnce(new Error('some error')) // Act - const actualResult = await getDashboardForms(mockUserId) + const actualResult = await AdminFormService.getDashboardForms(mockUserId) // Assert expect(getSpy).toHaveBeenCalledWith(mockUserId, mockUser.email) @@ -205,11 +180,12 @@ describe('admin-form.service', () => { }) // Act - const actualResult = await createPresignedPostUrlForImages({ - fileId: 'any id', - fileMd5Hash: 'any hash', - fileType: VALID_UPLOAD_FILE_TYPES[0], - }) + const actualResult = + await AdminFormService.createPresignedPostUrlForImages({ + fileId: 'any id', + fileMd5Hash: 'any hash', + fileType: VALID_UPLOAD_FILE_TYPES[0], + }) // Assert // Check that the correct bucket was used. @@ -229,11 +205,12 @@ describe('admin-form.service', () => { expect(VALID_UPLOAD_FILE_TYPES.includes(invalidFileType)).toEqual(false) // Act - const actualResult = await createPresignedPostUrlForImages({ - fileId: 'any id', - fileMd5Hash: 'any hash', - fileType: invalidFileType, - }) + const actualResult = + await AdminFormService.createPresignedPostUrlForImages({ + fileId: 'any id', + fileMd5Hash: 'any hash', + fileType: invalidFileType, + }) // Assert expect(actualResult.isErr()).toEqual(true) @@ -254,11 +231,12 @@ describe('admin-form.service', () => { }) // Act - const actualResult = await createPresignedPostUrlForImages({ - fileId: 'any id', - fileMd5Hash: 'any hash', - fileType: VALID_UPLOAD_FILE_TYPES[0], - }) + const actualResult = + await AdminFormService.createPresignedPostUrlForImages({ + fileId: 'any id', + fileMd5Hash: 'any hash', + fileType: VALID_UPLOAD_FILE_TYPES[0], + }) // Assert // Check that the correct bucket was used. @@ -295,11 +273,12 @@ describe('admin-form.service', () => { }) // Act - const actualResult = await createPresignedPostUrlForLogos({ - fileId: 'any id', - fileMd5Hash: 'any hash', - fileType: VALID_UPLOAD_FILE_TYPES[0], - }) + const actualResult = + await AdminFormService.createPresignedPostUrlForLogos({ + fileId: 'any id', + fileMd5Hash: 'any hash', + fileType: VALID_UPLOAD_FILE_TYPES[0], + }) // Assert // Check that the correct bucket was used. @@ -319,11 +298,12 @@ describe('admin-form.service', () => { expect(VALID_UPLOAD_FILE_TYPES.includes(invalidFileType)).toEqual(false) // Act - const actualResult = await createPresignedPostUrlForLogos({ - fileId: 'any id', - fileMd5Hash: 'any hash', - fileType: invalidFileType, - }) + const actualResult = + await AdminFormService.createPresignedPostUrlForLogos({ + fileId: 'any id', + fileMd5Hash: 'any hash', + fileType: invalidFileType, + }) // Assert expect(actualResult.isErr()).toEqual(true) @@ -344,11 +324,12 @@ describe('admin-form.service', () => { }) // Act - const actualResult = await createPresignedPostUrlForLogos({ - fileId: 'any id', - fileMd5Hash: 'any hash', - fileType: VALID_UPLOAD_FILE_TYPES[0], - }) + const actualResult = + await AdminFormService.createPresignedPostUrlForLogos({ + fileId: 'any id', + fileMd5Hash: 'any hash', + fileType: VALID_UPLOAD_FILE_TYPES[0], + }) // Assert // Check that the correct bucket was used. @@ -379,7 +360,7 @@ describe('admin-form.service', () => { } as unknown as IPopulatedForm // Act - const actual = await archiveForm(mockInitialForm) + const actual = await AdminFormService.archiveForm(mockInitialForm) // Assert expect(actual.isOk()).toEqual(true) @@ -397,7 +378,7 @@ describe('admin-form.service', () => { } as unknown as IPopulatedForm // Act - const actual = await archiveForm(mockInitialForm) + const actual = await AdminFormService.archiveForm(mockInitialForm) // Assert expect(actual.isErr()).toEqual(true) @@ -460,7 +441,7 @@ describe('admin-form.service', () => { .mockResolvedValueOnce(expectedForm as never) // Act - const actualResult = await duplicateForm( + const actualResult = await AdminFormService.duplicateForm( mockForm, mockNewAdminId, MOCK_EMAIL_OVERRIDE_PARAMS, @@ -509,7 +490,7 @@ describe('admin-form.service', () => { .mockResolvedValueOnce(expectedForm as never) // Act - const actualResult = await duplicateForm( + const actualResult = await AdminFormService.duplicateForm( mockForm, mockNewAdminId, MOCK_EMAIL_OVERRIDE_PARAMS, @@ -550,7 +531,7 @@ describe('admin-form.service', () => { .mockReturnValueOnce(expectedOverrideProps) // Act - const actualResult = await duplicateForm( + const actualResult = await AdminFormService.duplicateForm( mockForm, mockNewAdminId, MOCK_EMAIL_OVERRIDE_PARAMS, @@ -616,7 +597,7 @@ describe('admin-form.service', () => { ) // Act - const actualResult = await transferFormOwnership( + const actualResult = await AdminFormService.transferFormOwnership( mockValidForm, MOCK_NEW_OWNER_EMAIL, ) @@ -647,7 +628,7 @@ describe('admin-form.service', () => { } as unknown as IPopulatedForm // Act - const actualResult = await transferFormOwnership( + const actualResult = await AdminFormService.transferFormOwnership( mockValidForm, MOCK_NEW_OWNER_EMAIL, ) @@ -675,7 +656,7 @@ describe('admin-form.service', () => { ) // Act - const actualResult = await transferFormOwnership( + const actualResult = await AdminFormService.transferFormOwnership( mockValidForm, MOCK_NEW_OWNER_EMAIL, ) @@ -699,7 +680,7 @@ describe('admin-form.service', () => { ) // Act - const actualResult = await transferFormOwnership( + const actualResult = await AdminFormService.transferFormOwnership( mockValidForm, MOCK_NEW_OWNER_EMAIL, ) @@ -726,7 +707,7 @@ describe('admin-form.service', () => { } as unknown as IPopulatedForm // Act - const actualResult = await transferFormOwnership( + const actualResult = await AdminFormService.transferFormOwnership( mockValidForm, MOCK_NEW_OWNER_EMAIL, ) @@ -750,7 +731,7 @@ describe('admin-form.service', () => { } as unknown as IPopulatedForm // Act - const actualResult = await transferFormOwnership( + const actualResult = await AdminFormService.transferFormOwnership( mockValidForm, // Should trigger error since new owner email is the same as current. MOCK_CURRENT_OWNER.email, @@ -796,7 +777,7 @@ describe('admin-form.service', () => { ) // Act - const actualResult = await transferFormOwnership( + const actualResult = await AdminFormService.transferFormOwnership( mockValidForm, MOCK_NEW_OWNER_EMAIL, ) @@ -817,7 +798,7 @@ describe('admin-form.service', () => { describe('createForm', () => { it('should successfully create form', async () => { // Arrange - const formParams: Parameters[0] = { + const formParams: Parameters[0] = { title: 'create form title', admin: new ObjectId().toHexString(), responseMode: ResponseMode.Email, @@ -832,7 +813,7 @@ describe('admin-form.service', () => { .mockResolvedValueOnce(expectedForm as never) // Act - const actualResult = await createForm(formParams) + const actualResult = await AdminFormService.createForm(formParams) // Assert expect(actualResult._unsafeUnwrap()).toEqual(expectedForm) @@ -841,7 +822,7 @@ describe('admin-form.service', () => { it('should return DatabaseValidationError on invalid form params whilst creating form', async () => { // Arrange - const formParams: Parameters[0] = { + const formParams: Parameters[0] = { title: 'create form title', admin: new ObjectId().toHexString(), responseMode: ResponseMode.Encrypt, @@ -854,7 +835,7 @@ describe('admin-form.service', () => { .mockRejectedValueOnce(new mongoose.Error.ValidationError() as never) // Act - const actualResult = await createForm(formParams) + const actualResult = await AdminFormService.createForm(formParams) // Assert expect(actualResult._unsafeUnwrapErr()).toBeInstanceOf( @@ -865,7 +846,7 @@ describe('admin-form.service', () => { it('should return DatabaseConflictError on mongoose version error', async () => { // Arrange - const formParams: Parameters[0] = { + const formParams: Parameters[0] = { title: 'create form title', admin: new ObjectId().toHexString(), responseMode: ResponseMode.Encrypt, @@ -877,7 +858,7 @@ describe('admin-form.service', () => { ) // Act - const actualResult = await createForm(formParams) + const actualResult = await AdminFormService.createForm(formParams) // Assert expect(actualResult._unsafeUnwrapErr()).toBeInstanceOf( @@ -888,7 +869,7 @@ describe('admin-form.service', () => { it('should return DatabasePayloadError on form size error', async () => { // Arrange - const formParams: Parameters[0] = { + const formParams: Parameters[0] = { title: 'create form title', admin: new ObjectId().toHexString(), responseMode: ResponseMode.Encrypt, @@ -903,7 +884,7 @@ describe('admin-form.service', () => { .mockRejectedValueOnce(expectedError as never) // Act - const actualResult = await createForm(formParams) + const actualResult = await AdminFormService.createForm(formParams) // Assert expect(actualResult._unsafeUnwrapErr()).toEqual( @@ -916,7 +897,7 @@ describe('admin-form.service', () => { it('should return DatabaseError on database error whilst creating form', async () => { // Arrange - const formParams: Parameters[0] = { + const formParams: Parameters[0] = { title: 'create form title', admin: new ObjectId().toHexString(), responseMode: ResponseMode.Encrypt, @@ -928,7 +909,7 @@ describe('admin-form.service', () => { .mockRejectedValueOnce(new Error(mockErrorString) as never) // Act - const actualResult = await createForm(formParams) + const actualResult = await AdminFormService.createForm(formParams) // Assert expect(actualResult._unsafeUnwrapErr()).toEqual( @@ -973,7 +954,10 @@ describe('admin-form.service', () => { } // Act - const actualResult = await editFormFields(MOCK_INTIAL_FORM, createParams) + const actualResult = await AdminFormService.editFormFields( + MOCK_INTIAL_FORM, + createParams, + ) // Assert expect(actualResult._unsafeUnwrap()).toEqual(MOCK_UPDATED_FORM) @@ -995,7 +979,10 @@ describe('admin-form.service', () => { } // Act - const actualResult = await editFormFields(MOCK_INTIAL_FORM, reorderParams) + const actualResult = await AdminFormService.editFormFields( + MOCK_INTIAL_FORM, + reorderParams, + ) // Assert expect(actualResult._unsafeUnwrapErr()).toEqual(mockError) @@ -1021,7 +1008,10 @@ describe('admin-form.service', () => { } // Act - const actualResult = await editFormFields(MOCK_INTIAL_FORM, deleteParams) + const actualResult = await AdminFormService.editFormFields( + MOCK_INTIAL_FORM, + deleteParams, + ) // Assert expect(actualResult._unsafeUnwrapErr()).toBeInstanceOf( @@ -1053,12 +1043,17 @@ describe('admin-form.service', () => { it('should successfully update given form keys', async () => { // Arrange - const formUpdateParams: Parameters[1] = { + const formUpdateParams: Parameters< + typeof AdminFormService.updateForm + >[1] = { status: Status.Private, } // Act - const actualResult = await updateForm(MOCK_INITIAL_FORM, formUpdateParams) + const actualResult = await AdminFormService.updateForm( + MOCK_INITIAL_FORM, + formUpdateParams, + ) // Assert expect(actualResult._unsafeUnwrap()).toEqual(MOCK_UPDATED_FORM) @@ -1071,7 +1066,9 @@ describe('admin-form.service', () => { it('should return DatabaseError when error occurs whilst updating form', async () => { // Arrange - const formUpdateParams: Parameters[1] = { + const formUpdateParams: Parameters< + typeof AdminFormService.updateForm + >[1] = { esrvcId: 'MOCK-ESRVCID', } // Mock database failure. @@ -1080,7 +1077,10 @@ describe('admin-form.service', () => { ) // Act - const actualResult = await updateForm(MOCK_INITIAL_FORM, formUpdateParams) + const actualResult = await AdminFormService.updateForm( + MOCK_INITIAL_FORM, + formUpdateParams, + ) // Assert expect(actualResult._unsafeUnwrapErr()).toBeInstanceOf(DatabaseError) @@ -1149,7 +1149,7 @@ describe('admin-form.service', () => { } // Act - const actualResult = await updateFormSettings( + const actualResult = await AdminFormService.updateFormSettings( MOCK_EMAIL_FORM, settingsToUpdate, ) @@ -1172,7 +1172,7 @@ describe('admin-form.service', () => { }, } // Act - const actualResult = await updateFormSettings( + const actualResult = await AdminFormService.updateFormSettings( MOCK_ENCRYPT_FORM, settingsToUpdate, ) @@ -1203,7 +1203,7 @@ describe('admin-form.service', () => { }) // Act - const actualResult = await updateFormSettings( + const actualResult = await AdminFormService.updateFormSettings( MOCK_ENCRYPT_FORM, settingsToUpdate, ) @@ -1243,7 +1243,7 @@ describe('admin-form.service', () => { } as unknown as IPopulatedForm // Act - const actual = await updateFormField( + const actual = await AdminFormService.updateFormField( mockForm, fieldToUpdate._id, mockNewField, @@ -1271,7 +1271,7 @@ describe('admin-form.service', () => { ) as FieldUpdateDto // Act - const actual = await updateFormField( + const actual = await AdminFormService.updateFormField( mockForm, invalidFieldId, mockNewField, @@ -1298,7 +1298,7 @@ describe('admin-form.service', () => { ) as FieldUpdateDto // Act - const actual = await updateFormField( + const actual = await AdminFormService.updateFormField( mockForm, invalidFieldId, mockNewField, @@ -1335,7 +1335,10 @@ describe('admin-form.service', () => { ]) as FieldCreateDto // Act - const actual = await createFormField(mockForm, formCreateParams) + const actual = await AdminFormService.createFormField( + mockForm, + formCreateParams, + ) // Assert // Should return last element in form_field @@ -1362,7 +1365,10 @@ describe('admin-form.service', () => { } as FieldCreateDto // Act - const actual = await createFormField(mockForm, formCreateParams) + const actual = await AdminFormService.createFormField( + mockForm, + formCreateParams, + ) // Assert expect(actual._unsafeUnwrapErr()).toBeInstanceOf(DatabaseValidationError) @@ -1410,7 +1416,10 @@ describe('admin-form.service', () => { }) // Act - const actualResult = await deleteFormLogic(mockEmailForm, logicId) + const actualResult = await AdminFormService.deleteFormLogic( + mockEmailForm, + logicId, + ) // Assert expect(actualResult.isOk()).toEqual(true) @@ -1444,7 +1453,10 @@ describe('admin-form.service', () => { }) // Act - const actualResult = await deleteFormLogic(mockEncryptForm, logicId) + const actualResult = await AdminFormService.deleteFormLogic( + mockEncryptForm, + logicId, + ) // Assert expect(actualResult.isOk()).toEqual(true) @@ -1470,7 +1482,10 @@ describe('admin-form.service', () => { it('should return LogicNotFoundError if logic does not exist on form', async () => { // Act const wrongLogicId = new ObjectId().toHexString() - const actualResult = await deleteFormLogic(mockEmailForm, wrongLogicId) + const actualResult = await AdminFormService.deleteFormLogic( + mockEmailForm, + wrongLogicId, + ) // Assert expect(actualResult.isErr()).toEqual(true) @@ -1497,7 +1512,7 @@ describe('admin-form.service', () => { } as unknown as IPopulatedForm // Act - const actual = await duplicateFormField( + const actual = await AdminFormService.duplicateFormField( mockForm, String(fieldToDuplicate._id), ) @@ -1527,7 +1542,10 @@ describe('admin-form.service', () => { } as unknown as IPopulatedForm // Act - const actual = await duplicateFormField(mockForm, fieldToDuplicate._id) + const actual = await AdminFormService.duplicateFormField( + mockForm, + fieldToDuplicate._id, + ) // Assert expect(actual._unsafeUnwrapErr()).toEqual(new FormNotFoundError()) @@ -1546,7 +1564,10 @@ describe('admin-form.service', () => { } as unknown as IPopulatedForm // Act - const actual = await duplicateFormField(mockForm, initialFields[0]._id) + const actual = await AdminFormService.duplicateFormField( + mockForm, + initialFields[0]._id, + ) // Assert expect(actual._unsafeUnwrapErr()).toBeInstanceOf(DatabaseValidationError) @@ -1571,7 +1592,7 @@ describe('admin-form.service', () => { const newPosition = 1 // Act - const actual = await reorderFormField( + const actual = await AdminFormService.reorderFormField( mockForm, fieldToReorder, newPosition, @@ -1595,7 +1616,7 @@ describe('admin-form.service', () => { const newPosition = 2 // Act - const actual = await reorderFormField( + const actual = await AdminFormService.reorderFormField( mockForm, fieldToReorder, newPosition, @@ -1622,7 +1643,7 @@ describe('admin-form.service', () => { const newPosition = 2 // Act - const actual = await reorderFormField( + const actual = await AdminFormService.reorderFormField( mockForm, fieldToReorder, newPosition, @@ -1656,7 +1677,10 @@ describe('admin-form.service', () => { } as unknown as IPopulatedForm // Act - const actual = await updateFormCollaborators(mockForm, newCollaborators) + const actual = await AdminFormService.updateFormCollaborators( + mockForm, + newCollaborators, + ) // Assert expect(mockForm.updateFormCollaborators).toHaveBeenCalledWith( @@ -1681,7 +1705,10 @@ describe('admin-form.service', () => { } as unknown as IPopulatedForm // Act - const actual = await updateFormCollaborators(mockForm, newCollaborators) + const actual = await AdminFormService.updateFormCollaborators( + mockForm, + newCollaborators, + ) // Assert expect(mockForm.updateFormCollaborators).toHaveBeenCalledWith( @@ -1767,7 +1794,10 @@ describe('admin-form.service', () => { CREATE_SPY.mockResolvedValue(mockEmailFormUpdated as IFormDocument) // Act - const actualResult = await createFormLogic(mockEmailForm, createLogicBody) + const actualResult = await AdminFormService.createFormLogic( + mockEmailForm, + createLogicBody, + ) // Assert expect(actualResult.isOk()).toEqual(true) @@ -1786,7 +1816,7 @@ describe('admin-form.service', () => { CREATE_SPY.mockResolvedValue(mockEncryptFormUpdated as IFormDocument) // Act - const actualResult = await createFormLogic( + const actualResult = await AdminFormService.createFormLogic( mockEncryptForm, createLogicBody, ) @@ -1808,7 +1838,7 @@ describe('admin-form.service', () => { CREATE_SPY.mockResolvedValue(undefined as unknown as IFormDocument) // Act - const actualResult = await createFormLogic( + const actualResult = await AdminFormService.createFormLogic( mockEncryptForm, createLogicBody, ) @@ -1832,7 +1862,7 @@ describe('admin-form.service', () => { CREATE_SPY.mockResolvedValue(updatedFormWithoutLogic as IFormDocument) // Act - const actualResult = await createFormLogic( + const actualResult = await AdminFormService.createFormLogic( mockEncryptForm, createLogicBody, ) @@ -1856,7 +1886,7 @@ describe('admin-form.service', () => { CREATE_SPY.mockResolvedValue(updatedFormWithEmptyLogic as IFormDocument) // Act - const actualResult = await createFormLogic( + const actualResult = await AdminFormService.createFormLogic( mockEncryptForm, createLogicBody, ) @@ -1898,7 +1928,10 @@ describe('admin-form.service', () => { deleteSpy.mockResolvedValueOnce(mockUpdatedForm) // Act - const actual = await deleteFormField(mockForm, String(fieldToDelete._id)) + const actual = await AdminFormService.deleteFormField( + mockForm, + String(fieldToDelete._id), + ) // Assert expect(actual._unsafeUnwrap()).toEqual(mockUpdatedForm) @@ -1917,7 +1950,7 @@ describe('admin-form.service', () => { } as unknown as IPopulatedForm // Act - const actual = await deleteFormField( + const actual = await AdminFormService.deleteFormField( mockForm, new ObjectId().toHexString(), ) @@ -1938,7 +1971,10 @@ describe('admin-form.service', () => { deleteSpy.mockResolvedValueOnce(null) // Act - const actual = await deleteFormField(mockForm, fieldToDelete._id) + const actual = await AdminFormService.deleteFormField( + mockForm, + fieldToDelete._id, + ) // Assert expect(actual._unsafeUnwrapErr()).toEqual(new FormNotFoundError()) @@ -1967,7 +2003,10 @@ describe('admin-form.service', () => { updateSpy.mockResolvedValueOnce(mockUpdatedForm) // Act - const actual = await updateEndPage(MOCK_FORM_ID, MOCK_NEW_END_PAGE) + const actual = await AdminFormService.updateEndPage( + MOCK_FORM_ID, + MOCK_NEW_END_PAGE, + ) // Assert expect(actual._unsafeUnwrap()).toEqual(MOCK_NEW_END_PAGE) @@ -1978,7 +2017,10 @@ describe('admin-form.service', () => { updateSpy.mockResolvedValueOnce(null) // Act - const actual = await updateEndPage(MOCK_FORM_ID, MOCK_NEW_END_PAGE) + const actual = await AdminFormService.updateEndPage( + MOCK_FORM_ID, + MOCK_NEW_END_PAGE, + ) // Assert expect(actual._unsafeUnwrapErr()).toEqual(new FormNotFoundError()) @@ -1990,7 +2032,10 @@ describe('admin-form.service', () => { updateSpy.mockRejectedValueOnce(new Error(expectedErrorMsg)) // Act - const actual = await updateEndPage(MOCK_FORM_ID, MOCK_NEW_END_PAGE) + const actual = await AdminFormService.updateEndPage( + MOCK_FORM_ID, + MOCK_NEW_END_PAGE, + ) // Assert const actualError = actual._unsafeUnwrapErr() @@ -2022,7 +2067,10 @@ describe('admin-form.service', () => { updateSpy.mockResolvedValueOnce(mockUpdatedForm) // Act - const actual = await updateStartPage(MOCK_FORM_ID, MOCK_NEW_START_PAGE) + const actual = await AdminFormService.updateStartPage( + MOCK_FORM_ID, + MOCK_NEW_START_PAGE, + ) // Assert expect(actual._unsafeUnwrap()).toEqual(MOCK_NEW_START_PAGE) @@ -2033,7 +2081,10 @@ describe('admin-form.service', () => { updateSpy.mockResolvedValueOnce(null) // Act - const actual = await updateStartPage(MOCK_FORM_ID, MOCK_NEW_START_PAGE) + const actual = await AdminFormService.updateStartPage( + MOCK_FORM_ID, + MOCK_NEW_START_PAGE, + ) // Assert expect(actual._unsafeUnwrapErr()).toEqual(new FormNotFoundError()) @@ -2045,7 +2096,10 @@ describe('admin-form.service', () => { updateSpy.mockRejectedValueOnce(new Error(expectedErrorMsg)) // Act - const actual = await updateStartPage(MOCK_FORM_ID, MOCK_NEW_START_PAGE) + const actual = await AdminFormService.updateStartPage( + MOCK_FORM_ID, + MOCK_NEW_START_PAGE, + ) // Assert const actualError = actual._unsafeUnwrapErr() @@ -2134,7 +2188,7 @@ describe('admin-form.service', () => { UPDATE_SPY.mockResolvedValue(mockEmailFormUpdated as IFormSchema) // Act - const actualResult = await updateFormLogic( + const actualResult = await AdminFormService.updateFormLogic( mockEmailForm, logicId1.toHexString(), updateLogicBody, @@ -2156,7 +2210,7 @@ describe('admin-form.service', () => { UPDATE_SPY.mockResolvedValue(mockEncryptFormUpdated as IFormSchema) // Act - const actualResult = await updateFormLogic( + const actualResult = await AdminFormService.updateFormLogic( mockEncryptForm, logicId1.toHexString(), updateLogicBody, @@ -2176,7 +2230,7 @@ describe('admin-form.service', () => { it('should return LogicNotFoundError if logic does not exist on form', async () => { // Act const wrongLogicId = new ObjectId().toHexString() - const actualResult = await updateFormLogic( + const actualResult = await AdminFormService.updateFormLogic( mockEmailForm, wrongLogicId, updateLogicBody, @@ -2198,10 +2252,13 @@ describe('admin-form.service', () => { // Append created field to end of form_fields. form_fields: [MOCK_FIELD], _id: new ObjectId(), - } as IFormDocument + } as IPopulatedForm // Act - const actual = await getFormField(MOCK_FORM, String(MOCK_FIELD._id)) + const actual = await AdminFormService.getFormField( + MOCK_FORM, + String(MOCK_FIELD._id), + ) // Assert expect(actual._unsafeUnwrap()).toEqual(MOCK_FIELD) @@ -2215,13 +2272,13 @@ describe('admin-form.service', () => { // Append created field to end of form_fields. form_fields: [], _id: new ObjectId(), - } as unknown as IFormDocument + } as unknown as IPopulatedForm const expectedError = new FieldNotFoundError( `Attempted to retrieve field ${MOCK_ID} from ${MOCK_FORM._id} but field was not present`, ) // Act - const actual = await getFormField(MOCK_FORM, MOCK_ID) + const actual = await AdminFormService.getFormField(MOCK_FORM, MOCK_ID) // Assert expect(actual._unsafeUnwrapErr()).toEqual(expectedError) @@ -2236,7 +2293,9 @@ describe('admin-form.service', () => { disableSpy.mockResolvedValueOnce({ n: 0, nModified: 0, ok: 0 }) // Act - const expected = await disableSmsVerificationsForUser(MOCK_ADMIN_ID) + const expected = await AdminFormService.disableSmsVerificationsForUser( + MOCK_ADMIN_ID, + ) // Assert expect(disableSpy).toHaveBeenCalledWith(MOCK_ADMIN_ID) @@ -2250,7 +2309,9 @@ describe('admin-form.service', () => { disableSpy.mockRejectedValueOnce('whoops') // Act - const expected = await disableSmsVerificationsForUser(MOCK_ADMIN_ID) + const expected = await AdminFormService.disableSmsVerificationsForUser( + MOCK_ADMIN_ID, + ) // Assert expect(disableSpy).toHaveBeenCalledWith(MOCK_ADMIN_ID) @@ -2271,7 +2332,10 @@ describe('admin-form.service', () => { const MOCK_RATING_FIELD = generateDefaultField(BasicField.Rating) it('should return the form without doing anything', async () => { // Act - const actual = await shouldUpdateFormField(MOCK_FORM, MOCK_RATING_FIELD) + const actual = await AdminFormService.shouldUpdateFormField( + MOCK_FORM, + MOCK_RATING_FIELD, + ) // Assert expect(actual._unsafeUnwrap()).toEqual(MOCK_FORM) @@ -2293,7 +2357,7 @@ describe('admin-form.service', () => { countSpy.mockReturnValueOnce(okAsync(smsConfig.smsVerificationLimit)) // Act - const actual = await shouldUpdateFormField( + const actual = await AdminFormService.shouldUpdateFormField( MOCK_FORM, MOCK_VERIFIABLE_MOBILE_FIELD, ) @@ -2307,7 +2371,7 @@ describe('admin-form.service', () => { const MOCK_ONBOARDED_FORM = { ...MOCK_FORM, msgSrvcName: 'form a form' } // Act - const actual = await shouldUpdateFormField( + const actual = await AdminFormService.shouldUpdateFormField( MOCK_ONBOARDED_FORM, MOCK_VERIFIABLE_MOBILE_FIELD, ) @@ -2319,7 +2383,7 @@ describe('admin-form.service', () => { it('should return the given form when mobile field is not verifiable', async () => { // Act - const actual = await shouldUpdateFormField( + const actual = await AdminFormService.shouldUpdateFormField( MOCK_FORM, MOCK_UNVERIFIABLE_MOBILE_FIELD, ) @@ -2336,7 +2400,7 @@ describe('admin-form.service', () => { ) // Act - const actual = await shouldUpdateFormField( + const actual = await AdminFormService.shouldUpdateFormField( MOCK_FORM, MOCK_VERIFIABLE_MOBILE_FIELD, ) @@ -2352,7 +2416,7 @@ describe('admin-form.service', () => { countSpy.mockReturnValueOnce(errAsync(expectedError)) // Act - const actual = await shouldUpdateFormField( + const actual = await AdminFormService.shouldUpdateFormField( MOCK_FORM, MOCK_VERIFIABLE_MOBILE_FIELD, ) diff --git a/src/app/modules/form/admin-form/admin-form.service.ts b/src/app/modules/form/admin-form/admin-form.service.ts index 5f786665f2..cf3095e57b 100644 --- a/src/app/modules/form/admin-form/admin-form.service.ts +++ b/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) } diff --git a/src/app/modules/form/form.utils.ts b/src/app/modules/form/form.utils.ts index e26ddbc1ae..7e4a8d9532 100644 --- a/src/app/modules/form/form.utils.ts +++ b/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 'test@hotmail.com, test@gmail.com' to ['test@hotmail.com', 'test@gmail.com'] @@ -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 = ( - 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 = ( + form: Pick, ): form is IOnboardedForm => { - return !!form.msgSrvcName + return form.msgSrvcName + ? !(form.msgSrvcName === smsConfig.twilioMsgSrvcSid) + : false } diff --git a/src/app/modules/verification/__tests__/verification.controller.spec.ts b/src/app/modules/verification/__tests__/verification.controller.spec.ts index 4f7ba37aed..30df475714 100644 --- a/src/app/modules/verification/__tests__/verification.controller.spec.ts +++ b/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: 'former@forms.sg' }], + } 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) }) diff --git a/src/app/modules/verification/__tests__/verification.service.spec.ts b/src/app/modules/verification/__tests__/verification.service.spec.ts index 8a2cf5b428..112d3d878d 100644 --- a/src/app/modules/verification/__tests__/verification.service.spec.ts +++ b/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: 'some@user.gov.sg' }], + } 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)) + }) + }) }) diff --git a/src/app/modules/verification/verification.controller.ts b/src/app/modules/verification/verification.controller.ts index aac2173c02..473688ea2b 100644 --- a/src/app/modules/verification/verification.controller.ts +++ b/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', diff --git a/src/app/modules/verification/verification.service.ts b/src/app/modules/verification/verification.service.ts index d1f48041d8..1beff04d8d 100644 --- a/src/app/modules/verification/verification.service.ts +++ b/src/app/modules/verification/verification.service.ts @@ -1,19 +1,28 @@ import mongoose from 'mongoose' import { err, errAsync, ok, okAsync, Result, ResultAsync } from 'neverthrow' -import { NUM_OTP_RETRIES } from '../../../shared/util/verification' +import { + NUM_OTP_RETRIES, + SMS_WARNING_TIERS, +} from '../../../shared/util/verification' import { BasicField, + IPopulatedForm, IVerificationFieldSchema, IVerificationSchema, PublicTransaction, } from '../../../types' import formsgSdk from '../../config/formsg-sdk' import { createLoggerWithLabel } from '../../config/logger' -import { MailSendError } from '../../services/mail/mail.errors' +import * as AdminFormService from '../../modules/form/admin-form/admin-form.service' +import { + MailGenerationError, + MailSendError, +} from '../../services/mail/mail.errors' import MailService from '../../services/mail/mail.service' import { InvalidNumberError, SmsSendError } from '../../services/sms/sms.errors' import { SmsFactory } from '../../services/sms/sms.factory' +import * as SmsService from '../../services/sms/sms.service' import { transformMongoError } from '../../utils/handle-mongo-error' import { compareHash, HashingError } from '../../utils/hash' import { @@ -23,6 +32,7 @@ import { } from '../core/core.errors' import { FormNotFoundError } from '../form/form.errors' import * as FormService from '../form/form.service' +import { isFormOnboarded } from '../form/form.utils' import { FieldNotFoundInTransactionError, @@ -38,6 +48,7 @@ import { import getVerificationModel from './verification.model' import { SendOtpParams } from './verification.types' import { + hasAdminExceededFreeSmsLimit, isOtpExpired, isOtpWaitTimeElapsed, isTransactionExpired, @@ -476,3 +487,62 @@ const sendOtpForField = ( return errAsync(new NonVerifiedFieldTypeError(fieldType)) } } + +/** + * Checks the number of free smses sent by the admin of the form and deactivates verification or sends mail as required + * @param form The form whose admin's sms counts needs to be checked + * @returns ok(true) when the verification has been deactivated successfully or no action is required + * @returns err(MailGenerationError) when an error occurred on creating the HTML template for the email + * @returns err(MailSendError) when an error occurred on sending the email + * @returns err(PossibleDatabaseError) when an error occurred while retrieving the counts from the database + */ +export const processAdminSmsCounts = ( + form: IPopulatedForm, +): ResultAsync< + true, + MailGenerationError | MailSendError | PossibleDatabaseError +> => { + if (isFormOnboarded(form)) { + return okAsync(true) + } + + // Convert to string because it's typed as any + const formAdminId = String(form.admin._id) + + return SmsService.retrieveFreeSmsCounts(formAdminId).andThen((freeSmsSent) => + checkSmsCountAndPerformAction(form, freeSmsSent), + ) +} + +/** + * Checks the number of free smses sent by the admin of a form and performs the appropriate action + * @param form The form whose admin's sms counts needs to be checked + * @returns ok(true) when the action has been performed successfully + * @returns err(MailGenerationError) when an error occurred on creating the HTML template for the email + * @returns err(MailSendError) when an error occurred on sending the email + * @returns err(PossibleDatabaseError) when an error occurred while retrieving the counts from the database + */ +const checkSmsCountAndPerformAction = ( + form: Pick, + freeSmsSent: number, +): ResultAsync< + true, + MailGenerationError | MailSendError | PossibleDatabaseError +> => { + // Convert to string because it's typed as any + const formAdminId = String(form.admin._id) + + // NOTE: Because the admin has exceeded their allowable limit of free sms, + // the sms verifications for their forms also need to be disabled. + if (hasAdminExceededFreeSmsLimit(freeSmsSent)) { + return MailService.sendSmsVerificationDisabledEmail(form).andThen(() => + AdminFormService.disableSmsVerificationsForUser(formAdminId), + ) + } + + if (freeSmsSent in SMS_WARNING_TIERS) { + return MailService.sendSmsVerificationWarningEmail(form, freeSmsSent) + } + + return okAsync(true) +} diff --git a/src/shared/util/verification.ts b/src/shared/util/verification.ts index 233701cc4f..8f09fbec52 100644 --- a/src/shared/util/verification.ts +++ b/src/shared/util/verification.ts @@ -27,3 +27,9 @@ export enum ADMIN_VERIFIED_SMS_STATES { belowLimit = 'BELOW_LIMIT', hasMessageServiceId = 'MESSAGE_SERVICE_ID_OBTAINED', } + +export enum SMS_WARNING_TIERS { + LOW = 2500, + MED = 5000, + HIGH = 7500, +}