Skip to content

Commit

Permalink
feat: sms limiting (#2504)
Browse files Browse the repository at this point in the history
* feat(sms-limiting): email for disabling sms verification (#2133)

* feat(templates): add html template for email when verification is disabled

* feat(mail.utils.ts): added new method for generation of email html

* feat(mail.service): new service method to send mail when sms verification disabled

* chore(sms-verification-disabled.view): updated wording

* test(mail.service.spec): adds tests for verification disabled email sending

* feat(sms limit): sms warning for different limits (#2104)

* feat(templates): adds new sms verification warning template

* feat(mail.service): adds new method to send sms verification warning mail

* chore(templates): added colon for proper punctuation for sms verification template

* chore(sms-verification-warning): updated wording to reflect limit is on admin level

* refactor(mail.service): updated the api for mail service

* feat(verified-sms): disabling sms verifications (#2262)

feat(sms_count.server.model): adds new isOnboardedAcc key and scripts to add/delete the key

chore(sms_count.server.model): remove unrelated code changes

chore(scripts): added extra checks for verifiction counts

style(scripts): updated whitespace for formatting

feat(form.server.model): add new static method for disabling sms verifications for an admin

feat(admin-form.service): adds new service method to disable sms verifications for a certain user

fix(form.server.model): disabling sms verifications does not show as an update

fix(types): updated return type for disabling sms and added extra db types

test(form.server.model): added unit tests for disabling sms on the model scehma

test(admin-form.service.spec): added tests for disabling sms on admin service

chore(form.server.model): addressed PR comments

refactor(sms.config): shifts limiting to be on env var

refactor(sms.config): reworked to use env-vars

* feat(sms-limiting): admin facing frontend (#2280)

* feat(configure-mobile.client.directive): updated modal to use new messages with hard-coded data

* refactor(verification.constants): shifted sms_verification_limit to shared/util/verification

* feat(configure-mobile.client): added frontend ui for toggling on/off

* feat(configure-mobile.client.view): added lock and disabled state when limit exceeded

* refactor(configure-mobile.client.view): changed text message to be error themed

* refactor(edit-form.css): added inline-padding for error message display

* fix(configure-mobile.client.view): updated form twilio account link

* feat(configure-mobile.client.directive): added loading state for toggle

* refactor(configure-mobile.client.view): update wording for error message

* fix(configure-mobile.client.view.html): added ng-show so that error only shows when limit exceeded

* feat(public/services): adds new smsservice

* feat(configure-mobile.client.directive): connected sms counts to fetch from backend

* refactor(pop-up-modal): adds variable to check if we should display red button

\

* test(smsservice): adds tests for sms service

* refactor(configure-mobile.client): adds error reporting and handling on FE

* refactor(smsservice): renamed SmsService to AdminMetaService

* refactor(adminmetaservice): renamed method for greater clarity

* chore(configure-mobile.client): addressed PR comments

* refactor(configure-mobile.client): uses env var instead of using constants

* refactor(mail.service): changes to use env var instead of const

* refactor(adminmetaservice): updated definitions to fit BE

* refactor(configure-mobile.client): changed condition to be same as BE

* refactor(configure-mobile.client.directive): extracts formatting to own function

* feat(sms-limiting): disable sms verifications toggling (#2300)

* feat(verification): added utility method to check if admin has exceeded sms limit

* refactor(admin-form.controller): adds new step to check if admin has exceeded the free sms limit

the service method returns an error on limit being exceeded so that field creation short-circuits
and we hit the mapErr step immediately

* test(admin-form.service): added unit tests for isAdminOverFreeSmsLimit

* refactor(types): added more refined form and field types together with typeguards

added an onboardedForm type so that we can check if a form is onboarded. this is restricted to
subtypes of IForm because the context of msgSrvcName is unique to forms and not just any generic
type. Added a verifiable mobile field type that checks if the field is toggled to have
isVerifiableOn

* refactor(admin-form): adds checks to prevent admin updates when the fe state is inconsistent with be

* refactor(admin-form.service): optmised to avoid unnecessary query

* test(admin-form.serivce): adds unit tests for shouldUpdateFormField

* docs(admin-form.controller): updated documentation for 409s when sms limit is exceeded

* test(admin-form.controller.spec): updated with unit tests for 409s

* test(admin-forms.form.routes.spec): fixed failing test

* test(admin-form.controller): updated faulty tests

* refactor(mail.service): changes to use env var instead of const

* chore(form.utils): addressed PR comments

* test(admin-form.service.spec): updated test structure and description for greater clarity

* refactor(admin-form.service): splits field specific logic from checking field type

* 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

* refactor(configure-mobile.client.directive): updated to account for new BE api

* chore(types): clean up unused types

* test(verification): fixed import path in tests

fix(configure-mobile.client.directive): fixed import path

* fix(mail.service): updated to cc rather than send individual mail

* chore(create-isonboardedacc): updated db script to be more specific

* fix(sms_count.server.model): updated read pref to secondary

* fix(form.server.model): updated disable to only affect forms w/o msg srvc name

* test(form.server.model): updated tests to check that only not onboarded forms are disabled

* feat(sms-limiting): ui changes for modal (#2541)

* refactor(pop-up-modal): removed isImportant prop as it is now unused

* chore(configure-mobile.client): updated modal wording

* refactor(configure-mobile.client): added sms counts figure

* refactor(configure-mobile.client): updated stats to not show if loading or error

* chore(configure-mobile.client): removed extra html property

* feat(sms-limiting): disabled email (#2532)

* feat(form.server.model): added new method to retrieve public ofrms with sms verifications

* feat(form.service): adds new method to retrieve public forms w/ sms verif enabled

* refactor(mail.service): changed warning mails to only be sent to public forms w/ active sms verif

* refactor(mail.service): updated disabled mail to have all forms for admin/single form for collab

* fix(views/templates): updated wording for warning/disabled mails

* fix(verification): fixed lowercase k

* chore(templates): disabled email templates updated to match issue copy

* fix(templates): updated email template for warning emails

chore(template): removed fullstop so it looks more natural

* fix(mail.service): updated mail service + tests so that numbers have commas

* fix(form.server.model): updated read pref to secondary

* fix(form.server.model): updated retrieve method to only affect forms that are not onboarded

* docs(mail.service): updated documentation for using toLocaleString

* test(form.server.model): updated test to match only verifiable public forms that are not onboarded
  • Loading branch information
seaerchin authored Aug 11, 2021
1 parent 920f7d1 commit 6f3c9b6
Show file tree
Hide file tree
Showing 39 changed files with 2,153 additions and 180 deletions.
63 changes: 63 additions & 0 deletions scripts/20210628_create-isOnboardedAcc/create-isOnboardedAcc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/* eslint-disable */

/*
This script creates a new key, isOnboardedAccount, on the smscounts db which indexes the msgSrvcId variable
*/

let formTwilioId = 'insert the form twilio id here'

// == PRE-UPDATE CHECKS ==
// Count the number of verifications (A)
db.getCollection('smscounts').count({ smsType: 'VERIFICATION' })

// Count of forms using our twilio acc (B)
db.getCollection('smscounts').count({
smsType: 'VERIFICATION',
msgSrvcSid: {$eq: formTwilioId},
})

// Count of forms using their own twilio acc (C)
// A === B + C
db.getCollection('smscounts').count({
smsType: 'VERIFICATION',
msgSrvcSid: { $ne: formTwilioId },
})


// == UPDATE ==
// Update verifications which have message service id equal to form twilio id
db.getCollection('smscounts').updateMany(
{ smsType: 'VERIFICATION', msgSrvcSid: { $eq: formTwilioId } },
{
$set: {
isOnboardedAccount: false,
},
}
)

// Update verifications whose message service id is not equal to form twilio id
db.getCollection('smscounts').updateMany(
{ smsType: 'VERIFICATION', msgSrvcSid: { $ne: formTwilioId } },
{
$set: {
isOnboardedAccount: true,
},
}
)

// == POST-UPDATE CHECKS ==
// Check number of verifications updated
// Sum of these two should be equal to the initial count
// Count of forms using our twilio acc
db.getCollection('smscounts').count({
smsType: 'VERIFICATION',
msgSrvcSid: { $eq: formTwilioId },
isOnboardedAccount: false
})

// Count of forms using their own twilio acc
db.getCollection('smscounts').count({
smsType: 'VERIFICATION',
msgSrvcSid: { $ne: formTwilioId },
isOnboardedAccount: true
})
30 changes: 30 additions & 0 deletions scripts/20210628_create-isOnboardedAcc/delete-isOnboardedAcc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/* eslint-disable */

/*
This script creates a new key, isOnboardedAccount, on the smscounts db which indexes the msgSrvcId variable
*/

// == PRE-UPDATE CHECKS ==
// Count the number of verifications we have to update
db.getCollection('smscounts').count({ smsType: 'VERIFICATION' })

// == UPDATE ==
// Update verified smses
db.getCollection('smscounts').updateMany(
{ smsType: 'VERIFICATION' },
{
$unset: {
isOnboardedAccount: false,
},
}
)

// == POST-UPDATE CHECKS ==
// Check number of verifications updated
// SHOULD BE 0
db.getCollection('smscounts').count({
smsType: 'VERIFICATION',
isOnboardedAccount: {
$exists: true
}
})
20 changes: 20 additions & 0 deletions shared/utils/verification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,23 @@ export enum VfnErrors {
TransactionNotFound = 'TRANSACTION_NOT_FOUND',
InvalidMobileNumber = 'INVALID_MOBILE_NUMBER',
}

export enum ADMIN_VERIFIED_SMS_STATES {
limitExceeded = 'LIMIT_EXCEEDED',
belowLimit = 'BELOW_LIMIT',
hasMessageServiceId = 'MESSAGE_SERVICE_ID_OBTAINED',
}

export enum SMS_WARNING_TIERS {
LOW = 2500,
MED = 5000,
HIGH = 7500,
}

export const stringifiedSmsWarningTiers: {
[K in keyof typeof SMS_WARNING_TIERS]: string
} = {
LOW: '2.5K',
MED: '5K',
HIGH: '7.5K',
}
201 changes: 200 additions & 1 deletion src/app/models/__tests__/form.server.model.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint-disable @typescript-eslint/ban-ts-comment */
import { ObjectId } from 'bson-ext'
import { cloneDeep, map, merge, omit, orderBy, pick } from 'lodash'
import { cloneDeep, map, merge, omit, orderBy, pick, range } from 'lodash'
import mongoose, { Types } from 'mongoose'
import {
EMAIL_PUBLIC_FORM_FIELDS,
Expand Down Expand Up @@ -1802,6 +1802,205 @@ describe('Form Model', () => {
await expect(Form.countDocuments()).resolves.toEqual(0)
})
})

describe('disableSmsVerificationsForUser', () => {
const MOCK_MSG_SRVC_NAME = 'mockTwilioId'
it('should disable sms verifications for all forms belonging to a user that are not onboarded successfully', async () => {
// Arrange
const mockFormPromises = range(3).map((_, idx) => {
const isOnboarded = !!(idx % 2)
return Form.create({
admin: populatedAdmin._id,
responseMode: ResponseMode.Email,
title: 'mock mobile form',
emails: [populatedAdmin.email],
...(isOnboarded && { msgSrvcName: MOCK_MSG_SRVC_NAME }),
form_fields: [
generateDefaultField(BasicField.Mobile, { isVerifiable: true }),
],
})
})
await Promise.all(mockFormPromises)

// Act
await Form.disableSmsVerificationsForUser(populatedAdmin._id)

// Assert
// Find all forms that match admin id
// All forms with msgSrvcName have been using their own credentials
// They should not have verifications disabled.
const onboardedForms = await Form.find({
admin: populatedAdmin._id,
msgSrvcName: {
$exists: true,
},
})
onboardedForms.map(({ form_fields }) =>
form_fields!.map((field) => {
expect(field.isVerifiable).toBe(true)
}),
)

// Conversely, forms without msgSrvcName are using our credentials
// And should have their verifications disabled.
const notOnboardedForms = await Form.find({
admin: populatedAdmin._id,
msgSrvcName: {
$exists: false,
},
})

notOnboardedForms.map(({ form_fields }) =>
form_fields!.map((field) => {
expect(field.isVerifiable).toBe(false)
}),
)
})

it('should not disable non mobile fields for a user', async () => {
// Arrange
const mockFormPromises = range(3).map(() => {
return Form.create({
admin: populatedAdmin._id,
responseMode: ResponseMode.Email,
title: 'mock email form',
emails: [populatedAdmin.email],
form_fields: [
generateDefaultField(BasicField.Email, { isVerifiable: true }),
],
})
})
await Promise.all(mockFormPromises)

// Act
await Form.disableSmsVerificationsForUser(populatedAdmin._id)

// Assert
// Find all forms that match admin id
const updatedForms = await Form.find({ admin: populatedAdmin._id })
updatedForms.map(({ form_fields }) =>
form_fields!.map((field) => {
expect(field.isVerifiable).toBe(true)
}),
)
})

it('should only disable sms verifications for a particular user', async () => {
// Arrange
const MOCK_USER_ID = new ObjectId()
await dbHandler.insertFormCollectionReqs({
userId: MOCK_USER_ID,
mailDomain: 'something.com',
})
await Form.create({
admin: populatedAdmin._id,
responseMode: ResponseMode.Email,
title: 'mock email form',
emails: [populatedAdmin.email],
form_fields: [
generateDefaultField(BasicField.Mobile, { isVerifiable: true }),
],
})
await Form.create({
admin: MOCK_USER_ID,
responseMode: ResponseMode.Email,
title: 'mock email form',
emails: [populatedAdmin.email],
form_fields: [
generateDefaultField(BasicField.Email, { isVerifiable: true }),
],
})

// Act
await Form.disableSmsVerificationsForUser(populatedAdmin._id)

// Assert
// Find all forms that match admin id
const updatedMobileForm = await Form.find({ admin: populatedAdmin._id })
expect(updatedMobileForm[0]!.form_fields[0].isVerifiable).toBe(false)
const updatedEmailForm = await Form.find({ admin: MOCK_USER_ID })
expect(updatedEmailForm[0]!.form_fields[0].isVerifiable).toBe(true)
})

it('should not update when a db error occurs', async () => {
// Arrange
const disableSpy = jest.spyOn(Form, 'disableSmsVerificationsForUser')
disableSpy.mockResolvedValueOnce(new Error('tee hee db crashed'))
const mockFormPromises = range(3).map(() => {
return Form.create({
admin: populatedAdmin._id,
responseMode: ResponseMode.Email,
title: 'mock mobile form',
emails: [populatedAdmin.email],
form_fields: [
generateDefaultField(BasicField.Mobile, { isVerifiable: true }),
],
})
})
await Promise.all(mockFormPromises)

// Act
await Form.disableSmsVerificationsForUser(populatedAdmin._id)

// Assert
// Find all forms that match admin id
const updatedForms = await Form.find({ admin: populatedAdmin._id })
updatedForms.map(({ form_fields }) =>
form_fields!.map((field) => {
expect(field.isVerifiable).toBe(true)
}),
)
})
})

describe('retrievePublicFormsWithSmsVerification', () => {
const MOCK_MSG_SRVC_NAME = 'mockTwilioName'
it('should retrieve only public forms with verifiable mobile fields that are not onboarded', async () => {
// Arrange
const mockFormPromises = range(8).map((_, idx) => {
// Extract bits and use them to represent state
const isPublic = !!(idx % 2)
const isVerifiable = !!((idx >> 1) % 2)
const isOnboarded = !!((idx >> 2) % 2)
return Form.create({
admin: populatedAdmin._id,
responseMode: ResponseMode.Email,
title: 'mock mobile form',
emails: [populatedAdmin.email],
status: isPublic ? Status.Public : Status.Private,
...(isOnboarded && { msgSrvcName: MOCK_MSG_SRVC_NAME }),
form_fields: [
generateDefaultField(BasicField.Mobile, { isVerifiable }),
],
})
})
await Promise.all(mockFormPromises)

// Act
const forms = await Form.retrievePublicFormsWithSmsVerification(
populatedAdmin._id,
)

// Assert
expect(forms.length).toBe(1)
expect(forms[0].form_fields[0].isVerifiable).toBe(true)
expect(forms[0].status).toBe(Status.Public)
expect(forms[0].msgSrvcName).toBeUndefined()
})

it('should return an empty array when there are no forms', async () => {
// NOTE: This is an edge case and should never happen in prod as this method is called when
// a public form has a certain amount of verifications

// Act
const forms = await Form.retrievePublicFormsWithSmsVerification(
populatedAdmin._id,
)

// Assert
expect(forms.length).toBe(0)
})
})
})

describe('Methods', () => {
Expand Down
47 changes: 47 additions & 0 deletions src/app/models/form.server.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,53 @@ const compileFormModel = (db: Mongoose): IFormModel => {
).exec()
}

FormSchema.statics.disableSmsVerificationsForUser = async function (
userId: IUserSchema['_id'],
) {
return this.updateMany(
// Filter the collection so that only specified user is selected
// Only update forms without message service name
// As it implies that those forms are using default (our) credentials
{
admin: userId,
msgSrvcName: {
$exists: false,
},
},
// Next, set the isVerifiable property for each field in form_fields
// Refer here for $[identifier] syntax: https://docs.mongodb.com/manual/reference/operator/update/positional-filtered/
{ $set: { 'form_fields.$[field].isVerifiable': false } },
{
// Only set if the field has fieldType equal to mobile
arrayFilters: [{ 'field.fieldType': 'mobile' }],
// NOTE: Not updating the timestamp because we should preserve ordering due to user-level modifications
timestamps: false,
},
).exec()
}

/**
* Retrieves all the public forms for a user which has sms verifications enabled
* This only retrieves forms that are using FormSG credentials
* @param userId The userId to retrieve the forms for
* @returns All public forms that have sms verifications enabled
*/
FormSchema.statics.retrievePublicFormsWithSmsVerification = async function (
userId: IUserSchema['_id'],
) {
return this.find({
admin: userId,
'form_fields.fieldType': BasicField.Mobile,
'form_fields.isVerifiable': true,
status: Status.Public,
msgSrvcName: {
$exists: false,
},
})
.read('secondary')
.exec()
}

// Hooks
FormSchema.pre<IFormSchema>('validate', function (next) {
// Reject save if form document is too large
Expand Down
Loading

0 comments on commit 6f3c9b6

Please sign in to comment.