From eb90363143cb7c147b80a560681b0c14b99612f7 Mon Sep 17 00:00:00 2001 From: Shao Wei Date: Tue, 5 May 2020 15:29:49 +0800 Subject: [PATCH 1/5] chore: allow empty params in template on server and worker --- backend/src/core/services/template.service.ts | 21 +++++++++++-------- worker/src/core/services/template.service.ts | 21 +++++++++++-------- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/backend/src/core/services/template.service.ts b/backend/src/core/services/template.service.ts index e808ec6d4..d4d3b530f 100644 --- a/backend/src/core/services/template.service.ts +++ b/backend/src/core/services/template.service.ts @@ -64,18 +64,21 @@ const parseTemplate = (templateBody: string, params?: { [key: string]: string }) throw new TemplateError(`Invalid characters in param named {{${key}}}. Only alphanumeric characters allowed.`) } + // add key regardless, note that this is also returned in lowercase + variables.push(key) + + // if no params continue with the loop + if (!params) return + // if params provided == attempt to carry out templating - if (params) { - if (dict[key]) { - const templated = dict[key] - tokens.push(templated) - } else { - throw new TemplateError(`Param ${templateObject.c} not found`) - } + if (dict[key]) { + const templated = dict[key] + tokens.push(templated) + return } - // add key regardless, note that this is also returned in lowercase - variables.push(key) + // recipient key must have param + if (key === 'recipient') throw new TemplateError(`Param ${templateObject.c} not found`) } else { // I have not found an edge case that trips this yet logger.error(`Templating error: templateObject.c of ${templateObject} is undefined.`) diff --git a/worker/src/core/services/template.service.ts b/worker/src/core/services/template.service.ts index ddeece1f9..26c392cd1 100644 --- a/worker/src/core/services/template.service.ts +++ b/worker/src/core/services/template.service.ts @@ -58,18 +58,21 @@ const parseTemplate = (templateBody: string, params?: { [key: string]: string }) throw new Error(`Invalid characters in param named {{${key}}}. Only alphanumeric characters allowed.`) } + // add key regardless, note that this is also returned in lowercase + variables.push(key) + + // if no params continue with the loop + if (!params) return + // if params provided == attempt to carry out templating - if (params) { - if (dict[key]) { - const templated = dict[key] - tokens.push(templated) - } else { - throw new Error(`Param ${templateObject.c} not found`) - } + if (dict[key]) { + const templated = dict[key] + tokens.push(templated) + return } - // add key regardless, note that this is also returned in lowercase - variables.push(key) + // recipient key must have param + if (key === 'recipient') throw new Error(`Param ${templateObject.c} not found`) } else { // I have not found an edge case that trips this yet logger.error(`Templating error: templateObject.c of ${templateObject} is undefined.`) From c7cebddb6baf9dcd462ff2a47186a49eef3841d3 Mon Sep 17 00:00:00 2001 From: Shao Wei Date: Wed, 6 May 2020 16:13:42 +0800 Subject: [PATCH 2/5] chore: validate email recipients for upload --- backend/src/core/errors/template.errors.ts | 8 ++++++++ backend/src/email/routes/email.routes.ts | 8 ++++++-- backend/src/email/services/email.service.ts | 11 +++++++---- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/backend/src/core/errors/template.errors.ts b/backend/src/core/errors/template.errors.ts index 3cf35e661..be5e466ec 100644 --- a/backend/src/core/errors/template.errors.ts +++ b/backend/src/core/errors/template.errors.ts @@ -22,4 +22,12 @@ export class TemplateError extends Error { Object.setPrototypeOf(this, new.target.prototype) // restore prototype chain Error.captureStackTrace(this) } +} + +export class InvalidRecipientError extends Error { + constructor() { + super('There are invalid recipient(s) in the uploaded recipient list.') + Object.setPrototypeOf(this, new.target.prototype) // restore prototype chain + Error.captureStackTrace(this) + } } \ No newline at end of file diff --git a/backend/src/email/routes/email.routes.ts b/backend/src/email/routes/email.routes.ts index 0bbc4eef6..dc8285207 100644 --- a/backend/src/email/routes/email.routes.ts +++ b/backend/src/email/routes/email.routes.ts @@ -10,7 +10,7 @@ import { testHydration, extractS3Key, } from '@core/services' -import { populateEmailTemplate, upsertEmailTemplate, getEmailStats } from '@email/services' +import { populateEmailTemplate, upsertEmailTemplate, getEmailStats, hasInvalidEmailRecipient } from '@email/services' import { uploadStartHandler, sendCampaign, @@ -25,6 +25,7 @@ import { HydrationError, RecipientColumnMissing, TemplateError, + InvalidRecipientError, } from '@core/errors' import { isSuperSet } from '@core/utils' import logger from '@core/logger' @@ -257,6 +258,8 @@ const uploadCompleteHandler = async (req: Request, res: Response, next: NextFunc templateParams: emailTemplate.params, }) + if (hasInvalidEmailRecipient(records)) throw new InvalidRecipientError() + const recipientCount: number = records.length // START populate template // TODO: is actually populate message logs @@ -272,7 +275,8 @@ const uploadCompleteHandler = async (req: Request, res: Response, next: NextFunc throw err } } catch (err) { - if (err instanceof RecipientColumnMissing || err instanceof MissingTemplateKeysError) { + if (err instanceof RecipientColumnMissing || err instanceof MissingTemplateKeysError + || err instanceof InvalidRecipientError) { return res.status(400).json({ message: err.message }) } return next(err) diff --git a/backend/src/email/services/email.service.ts b/backend/src/email/services/email.service.ts index c5bd2b0c1..c231040bd 100644 --- a/backend/src/email/services/email.service.ts +++ b/backend/src/email/services/email.service.ts @@ -1,12 +1,11 @@ import { chunk } from 'lodash' +import validator from 'validator' import { Campaign, JobQueue } from '@core/models' import { EmailMessage, EmailTemplate, EmailOp } from '@email/models' import logger from '@core/logger' import { CampaignStats } from '@core/interfaces' import { getStatsFromTable } from '@core/services' - - const upsertEmailTemplate = async ({ subject, body, campaignId }: {subject: string; body: string; campaignId: number}): Promise => { let transaction try { @@ -97,6 +96,10 @@ const getEmailStats = async (campaignId: number): Promise => { const stats = await getStatsFromTable(EmailMessage, campaignId) return { error: stats.error, unsent: stats.unsent, sent: stats.sent, status: job.status } -} +} + +const hasInvalidEmailRecipient = (records: MessageBulkInsertInterface[]): boolean => { + return records.some((record) => !validator.isEmail(record.recipient)) +} -export { populateEmailTemplate, upsertEmailTemplate, getEmailStats } \ No newline at end of file +export { populateEmailTemplate, upsertEmailTemplate, getEmailStats, hasInvalidEmailRecipient } \ No newline at end of file From 931cfdb754dff6fd892770c9369d5bb4d31afd65 Mon Sep 17 00:00:00 2001 From: Shao Wei Date: Wed, 6 May 2020 16:20:36 +0800 Subject: [PATCH 3/5] chore: validate sms recipients for upload --- backend/src/sms/routes/sms.routes.ts | 8 ++++++-- backend/src/sms/services/sms.service.ts | 7 ++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/backend/src/sms/routes/sms.routes.ts b/backend/src/sms/routes/sms.routes.ts index 90375b7a8..12cbba654 100644 --- a/backend/src/sms/routes/sms.routes.ts +++ b/backend/src/sms/routes/sms.routes.ts @@ -10,7 +10,7 @@ import { testHydration, extractS3Key, } from '@core/services' -import { populateSmsTemplate, upsertSmsTemplate, getSmsStats } from '@sms/services' +import { populateSmsTemplate, upsertSmsTemplate, getSmsStats, hasInvalidSmsRecipient } from '@sms/services' import { uploadStartHandler, sendCampaign, @@ -23,6 +23,7 @@ import { HydrationError, RecipientColumnMissing, TemplateError, + InvalidRecipientError, } from '@core/errors' import { isSuperSet } from '@core/utils' import { storeCredentials, getCampaignDetails, previewFirstMessage } from '@sms/middlewares' @@ -260,6 +261,8 @@ const uploadCompleteHandler = async (req: Request, res: Response, next: NextFunc templateParams: smsTemplate.params, }) + if (hasInvalidSmsRecipient(records)) throw new InvalidRecipientError() + const recipientCount: number = records.length // START populate template // TODO: is actually populate message logs @@ -275,7 +278,8 @@ const uploadCompleteHandler = async (req: Request, res: Response, next: NextFunc throw err } } catch (err) { - if (err instanceof RecipientColumnMissing || err instanceof MissingTemplateKeysError) { + if (err instanceof RecipientColumnMissing || err instanceof MissingTemplateKeysError + || err instanceof InvalidRecipientError) { return res.status(400).json({ message: err.message }) } return next(err) diff --git a/backend/src/sms/services/sms.service.ts b/backend/src/sms/services/sms.service.ts index 956785003..6e7cba1c5 100644 --- a/backend/src/sms/services/sms.service.ts +++ b/backend/src/sms/services/sms.service.ts @@ -96,4 +96,9 @@ const getSmsStats = async (campaignId: number): Promise => { return { error: stats.error, unsent: stats.unsent, sent: stats.sent, status: job.status } } -export { populateSmsTemplate, upsertSmsTemplate, getSmsStats } \ No newline at end of file +const hasInvalidSmsRecipient = (records: MessageBulkInsertInterface[]): boolean => { + const re = /^[0-9+]+$/ + return records.some((record) => !record.recipient.match(re)) +} + +export { populateSmsTemplate, upsertSmsTemplate, getSmsStats, hasInvalidSmsRecipient } \ No newline at end of file From ba36ccf64aa151a9cfecd6469f9baa28a669780a Mon Sep 17 00:00:00 2001 From: Shao Wei Date: Wed, 6 May 2020 16:44:14 +0800 Subject: [PATCH 4/5] chore: Improve missing template keys error message --- backend/src/core/errors/template.errors.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/core/errors/template.errors.ts b/backend/src/core/errors/template.errors.ts index be5e466ec..bf51bc0ac 100644 --- a/backend/src/core/errors/template.errors.ts +++ b/backend/src/core/errors/template.errors.ts @@ -1,7 +1,7 @@ export class MissingTemplateKeysError extends Error { public readonly missingKeys: string[] constructor(missingKeys: string[]) { - super(`Template contains keys [${missingKeys}] that are not present in uploaded recipient list.`) + super(`The attribute(s) { ${missingKeys} } are not present in uploaded recipient list.`) this.missingKeys = missingKeys Object.setPrototypeOf(this, new.target.prototype) // restore prototype chain Error.captureStackTrace(this) From 9c5086766762c86657c5c6a22584482470f57217 Mon Sep 17 00:00:00 2001 From: Shao Wei Date: Mon, 11 May 2020 15:01:00 +0800 Subject: [PATCH 5/5] chore: improve regex and use test instead of match --- backend/src/sms/services/sms.service.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/src/sms/services/sms.service.ts b/backend/src/sms/services/sms.service.ts index 6e7cba1c5..37b42ade0 100644 --- a/backend/src/sms/services/sms.service.ts +++ b/backend/src/sms/services/sms.service.ts @@ -97,8 +97,8 @@ const getSmsStats = async (campaignId: number): Promise => { } const hasInvalidSmsRecipient = (records: MessageBulkInsertInterface[]): boolean => { - const re = /^[0-9+]+$/ - return records.some((record) => !record.recipient.match(re)) + const re = /^\+?[0-9]+$/ + return records.some((record) => !re.test(record.recipient)) } export { populateSmsTemplate, upsertSmsTemplate, getSmsStats, hasInvalidSmsRecipient } \ No newline at end of file