-
Notifications
You must be signed in to change notification settings - Fork 87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(sms-limiting): email for disabling sms verification #2133
Changes from all commits
3eae2f4
0c79836
ba35d1f
afa8335
f1dfdfb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export const SMS_VERIFICATION_LIMIT = 10000 | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,13 @@ | ||
import ejs from 'ejs' | ||
import { cloneDeep } from 'lodash' | ||
import moment from 'moment-timezone' | ||
import { err, ok, okAsync } from 'neverthrow' | ||
import Mail, { Attachment } from 'nodemailer/lib/mailer' | ||
|
||
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 { | ||
AutoreplySummaryRenderData, | ||
|
@@ -13,6 +17,8 @@ import { | |
import * as MailUtils from 'src/app/services/mail/mail.utils' | ||
import { BounceType, IPopulatedForm, ISubmissionSchema } from 'src/types' | ||
|
||
import { SMS_VERIFICATION_LIMIT } from '../../../modules/verification/verification.constants' | ||
|
||
const MOCK_VALID_EMAIL = '[email protected]' | ||
const MOCK_VALID_EMAIL_2 = '[email protected]' | ||
const MOCK_VALID_EMAIL_3 = '[email protected]' | ||
|
@@ -1242,4 +1248,109 @@ describe('mail.service', () => { | |
expect(sendMailSpy).toHaveBeenCalledWith(expectedArgs) | ||
}) | ||
}) | ||
|
||
describe('sendSmsVerificationDisabledEmail', () => { | ||
const MOCK_FORM_ID = 'mockFormId' | ||
const MOCK_FORM_TITLE = 'You are all individuals!' | ||
const MOCK_INVALID_EMAIL = 'something wrong@a' | ||
|
||
const MOCK_FORM: IPopulatedForm = { | ||
permissionList: [ | ||
{ email: MOCK_VALID_EMAIL }, | ||
{ email: MOCK_VALID_EMAIL_2 }, | ||
], | ||
admin: { | ||
email: MOCK_VALID_EMAIL_3, | ||
}, | ||
title: MOCK_FORM_TITLE, | ||
_id: MOCK_FORM_ID, | ||
} as unknown as IPopulatedForm | ||
|
||
const MOCK_INVALID_EMAIL_FORM: IPopulatedForm = { | ||
permissionList: [], | ||
admin: { | ||
email: MOCK_INVALID_EMAIL, | ||
}, | ||
title: MOCK_FORM_TITLE, | ||
_id: MOCK_FORM_ID, | ||
} as unknown as IPopulatedForm | ||
|
||
const generateExpectedMailOptions = async ( | ||
count: number, | ||
emailRecipients: string | string[], | ||
) => { | ||
const result = await MailUtils.generateSmsVerificationDisabledHtml({ | ||
formTitle: MOCK_FORM_TITLE, | ||
formLink: `${MOCK_APP_URL}/${MOCK_FORM_ID}`, | ||
smsVerificationLimit: SMS_VERIFICATION_LIMIT, | ||
}).map((emailHtml) => { | ||
return { | ||
to: emailRecipients, | ||
from: MOCK_SENDER_STRING, | ||
html: emailHtml, | ||
subject: '[FormSG] SMS Verification - Free Tier Limit Reached', | ||
replyTo: MOCK_SENDER_EMAIL, | ||
bcc: MOCK_SENDER_EMAIL, | ||
} | ||
}) | ||
return result._unsafeUnwrap() | ||
} | ||
|
||
it('should send verified sms disabled emails successfully', async () => { | ||
// Arrange | ||
// sendMail should return mocked success response | ||
sendMailSpy.mockResolvedValueOnce('mockedSuccessResponse') | ||
|
||
// Act | ||
const actualResult = await mailService.sendSmsVerificationDisabledEmail( | ||
MOCK_FORM, | ||
) | ||
const expectedMailOptions = await generateExpectedMailOptions(1000, [ | ||
MOCK_VALID_EMAIL, | ||
MOCK_VALID_EMAIL_2, | ||
MOCK_VALID_EMAIL_3, | ||
]) | ||
|
||
// Assert | ||
expect(actualResult._unsafeUnwrap()).toEqual(true) | ||
// Check arguments passed to sendNodeMail | ||
expect(sendMailSpy).toHaveBeenCalledTimes(1) | ||
expect(sendMailSpy).toHaveBeenCalledWith(expectedMailOptions) | ||
}) | ||
|
||
it('should return MailSendError when the provided email is invalid', async () => { | ||
// Act | ||
const actualResult = await mailService.sendSmsVerificationDisabledEmail( | ||
MOCK_INVALID_EMAIL_FORM, | ||
) | ||
|
||
// Assert | ||
expect(actualResult).toEqual( | ||
err(new MailSendError('Invalid email error')), | ||
) | ||
// Check arguments passed to sendNodeMail | ||
expect(sendMailSpy).toHaveBeenCalledTimes(0) | ||
}) | ||
|
||
it('should return MailGenerationError when the html template could not be created', async () => { | ||
// Arrange | ||
jest.spyOn(ejs, 'renderFile').mockRejectedValueOnce('no.') | ||
|
||
// Act | ||
const actualResult = await mailService.sendSmsVerificationDisabledEmail( | ||
MOCK_INVALID_EMAIL_FORM, | ||
) | ||
|
||
// Assert | ||
expect(actualResult).toEqual( | ||
err( | ||
new MailGenerationError( | ||
'Error occurred whilst rendering mail template', | ||
), | ||
), | ||
) | ||
// Check arguments passed to sendNodeMail | ||
expect(sendMailSpy).toHaveBeenCalledTimes(0) | ||
}) | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,10 +10,12 @@ import { | |
BounceType, | ||
EmailAdminDataField, | ||
IEmailFormSchema, | ||
IPopulatedForm, | ||
ISubmissionSchema, | ||
} from '../../../types' | ||
import config from '../../config/config' | ||
import { createLoggerWithLabel } from '../../config/logger' | ||
import { SMS_VERIFICATION_LIMIT } from '../../modules/verification/verification.constants' | ||
|
||
import { EMAIL_HEADERS, EmailType } from './mail.constants' | ||
import { MailGenerationError, MailSendError } from './mail.errors' | ||
|
@@ -25,12 +27,14 @@ import { | |
SendAutoReplyEmailsArgs, | ||
SendMailOptions, | ||
SendSingleAutoreplyMailArgs, | ||
SmsVerificationDisabledData, | ||
} from './mail.types' | ||
import { | ||
generateAutoreplyHtml, | ||
generateAutoreplyPdf, | ||
generateBounceNotificationHtml, | ||
generateLoginOtpHtml, | ||
generateSmsVerificationDisabledHtml, | ||
generateSubmissionToAdminHtml, | ||
generateVerificationOtpHtml, | ||
isToFieldValid, | ||
|
@@ -572,6 +576,41 @@ export class MailService { | |
}), | ||
) | ||
} | ||
|
||
/** | ||
* Sends a email to the admin and collaborators of the form when the verified sms feature will be disabled. | ||
* This happens only when the admin has hit a certain limit of sms verifications on his account | ||
* @param form The form whose admin and collaborators will be issued the email | ||
* @returns ok(true) when mail sending is successful | ||
* @returns err(MailGenerationError) when there was an error in generating the html data for the mail | ||
* @returns err(MailSendError) when there was an error in sending the mail | ||
*/ | ||
sendSmsVerificationDisabledEmail = ( | ||
form: Pick<IPopulatedForm, 'permissionList' | 'admin' | 'title' | '_id'>, | ||
): ResultAsync<true, MailGenerationError | MailSendError> => { | ||
const htmlData: SmsVerificationDisabledData = { | ||
formTitle: form.title, | ||
formLink: `${this.#appUrl}/${form._id}`, | ||
smsVerificationLimit: SMS_VERIFICATION_LIMIT, | ||
} | ||
|
||
return generateSmsVerificationDisabledHtml(htmlData).andThen((mailHtml) => { | ||
const emailRecipients = form.permissionList | ||
.map(({ email }) => email) | ||
.concat(form.admin.email) | ||
|
||
const mailOptions: MailOptions = { | ||
to: emailRecipients, | ||
from: this.#senderFromString, | ||
html: mailHtml, | ||
subject: '[FormSG] SMS Verification - Free Tier Limit Reached', | ||
replyTo: this.#senderMail, | ||
bcc: this.#senderMail, | ||
} | ||
|
||
return this.#sendNodeMail(mailOptions, { formId: form._id }) | ||
Comment on lines
+598
to
+611
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be good to extract this portion? I think this is repeated in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeap! the reason is cos i want them to be independent and avoid rebasing ;--; but ya, i'll add it in after they get merged into the base branch! |
||
}) | ||
} | ||
} | ||
|
||
export default new MailService() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
<!DOCTYPE html> | ||
<html lang="en" xmlns="http://www.w3.org/1999/xhtml"> | ||
<head> </head> | ||
<body> | ||
<p>Dear form admin(s),</p> | ||
<p> | ||
You are receiving this as you have enabled SMS OTP verification on a | ||
Mobile Number field of your form: | ||
<a href="<%= formLink %>"> '<%= formTitle %>' </a>. | ||
</p> | ||
<p> | ||
SMS OTP verification has been <b>automatically disabled</b> on your form | ||
as you have reached the free tier limit of <%= smsVerificationLimit %> | ||
responses. | ||
</p> | ||
|
||
<p> | ||
We would have previously notified all form admins upon your account | ||
reaching 2500, 5000 and 7500 responses while free SMS OTP verification was | ||
enabled. | ||
</p> | ||
<p> | ||
If you require SMS OTP verification for more than <%= smsVerificationLimit | ||
%> responses, please | ||
<a | ||
href="https://guide.form.gov.sg/AdvancedGuide.html#how-do-i-arrange-payment-for-verified-sms" | ||
> | ||
arrange advance billing with us. | ||
</a> | ||
</p> | ||
<p> | ||
<b>Important: </b> Please refrain from creating multiple forms for the | ||
same use case in order to avoid this limit, as such forms risk being | ||
flagged for abuse and blacklisted. | ||
</p> | ||
<p>The FormSG Team</p> | ||
</body> | ||
</html> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can put this in
shared/util/verification
to keep all the numbers in one placeThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Will update in base branch