From 422e88e6de4ea99ee23ceb386147db988b883974 Mon Sep 17 00:00:00 2001 From: KishenKumarrrrr Date: Wed, 24 Jan 2024 13:37:07 +0800 Subject: [PATCH 1/6] feat: disable tracking for transactional emails --- backend/src/core/config.ts | 6 +++ backend/src/core/services/mail.service.ts | 3 +- .../email-transactional.middleware.ts | 3 ++ .../routes/email-transactional.routes.ts | 1 + .../services/email-transactional.service.ts | 3 ++ shared/src/clients/mail-client.class/index.ts | 43 ++++++++++++++----- 6 files changed, 47 insertions(+), 12 deletions(-) diff --git a/backend/src/core/config.ts b/backend/src/core/config.ts index 832d0b0eb..8343529bb 100644 --- a/backend/src/core/config.ts +++ b/backend/src/core/config.ts @@ -83,6 +83,7 @@ interface ConfigSchema { } mailFrom: string mailConfigurationSet: string + noTrackingMailConfigurationSet: string mailVia: string mailDefaultRate: number transactionalEmail: { @@ -468,6 +469,11 @@ const config: Config = convict({ default: 'postman-email-open', env: 'BACKEND_SES_CONFIGURATION_SET', }, + noTrackingMailConfigurationSet: { + doc: 'AWS SES Configuration set that does not include open and read tracking', + default: 'postman-email-no-tracking', + env: 'BACKEND_SES_NO_TRACKING_CONFIGURATION_SET', + }, mailVia: { doc: 'Text to appended to custom sender name', default: 'via Postman', diff --git a/backend/src/core/services/mail.service.ts b/backend/src/core/services/mail.service.ts index 2d72b3810..7ed68c84c 100644 --- a/backend/src/core/services/mail.service.ts +++ b/backend/src/core/services/mail.service.ts @@ -5,7 +5,8 @@ const mailClient = new MailClient( config.get('mailOptions'), config.get('emailCallback.hashSecret'), config.get('emailFallback.activate') ? config.get('mailFrom') : undefined, - config.get('mailConfigurationSet') + config.get('mailConfigurationSet'), + config.get('noTrackingMailConfigurationSet') ) export const MailService = { diff --git a/backend/src/email/middlewares/email-transactional.middleware.ts b/backend/src/email/middlewares/email-transactional.middleware.ts index 5d5726b32..15ebf3294 100644 --- a/backend/src/email/middlewares/email-transactional.middleware.ts +++ b/backend/src/email/middlewares/email-transactional.middleware.ts @@ -67,6 +67,7 @@ export const InitEmailTransactionalMiddleware = ( tag?: string cc?: string[] bcc?: string[] + disable_tracking?: boolean } type ReqBodyWithId = ReqBody & { emailMessageTransactionalId: string } @@ -210,6 +211,7 @@ export const InitEmailTransactionalMiddleware = ( cc, bcc, emailMessageTransactionalId, // added by saveMessage middleware + disable_tracking: disableTracking, } = req.body try { @@ -275,6 +277,7 @@ export const InitEmailTransactionalMiddleware = ( ? bcc.filter((c) => !blacklistedRecipients.includes(c)) : undefined, emailMessageTransactionalId, + disableTracking, }) emailMessageTransactional.set( 'status', diff --git a/backend/src/email/routes/email-transactional.routes.ts b/backend/src/email/routes/email-transactional.routes.ts index 9421a825a..f87c04b0d 100644 --- a/backend/src/email/routes/email-transactional.routes.ts +++ b/backend/src/email/routes/email-transactional.routes.ts @@ -62,6 +62,7 @@ export const InitEmailTransactionalRoute = ( .items( Joi.string().trim().email().options({ convert: true }).lowercase() ), + disable_tracking: Joi.boolean().optional(), }), } const getByIdValidator = { diff --git a/backend/src/email/services/email-transactional.service.ts b/backend/src/email/services/email-transactional.service.ts index f299d3d5d..fb7afb79d 100644 --- a/backend/src/email/services/email-transactional.service.ts +++ b/backend/src/email/services/email-transactional.service.ts @@ -41,6 +41,7 @@ async function sendMessage({ cc, bcc, emailMessageTransactionalId, + disableTracking, }: { subject: string body: string @@ -51,6 +52,7 @@ async function sendMessage({ cc?: string[] bcc?: string[] emailMessageTransactionalId: string + disableTracking?: boolean }): Promise { // TODO: flagging this coupling for future refactoring: // currently, we are using EmailTemplateService to sanitize both tx emails and campaign emails @@ -99,6 +101,7 @@ async function sendMessage({ // receive from SES, but not saving to DB const isEmailSent = await EmailService.sendEmail(mailToSend, { extraSmtpHeaders: { isTransactional: true }, + disableTracking, }) if (!isEmailSent) { throw new Error('Failed to send transactional email') diff --git a/shared/src/clients/mail-client.class/index.ts b/shared/src/clients/mail-client.class/index.ts index 3cb22cfee..48d899ce5 100644 --- a/shared/src/clients/mail-client.class/index.ts +++ b/shared/src/clients/mail-client.class/index.ts @@ -10,19 +10,22 @@ export * from './interfaces' export type SendEmailOpts = { extraSmtpHeaders: Record + disableTracking?: boolean } export default class MailClient { private mailer: nodemailer.Transporter private email: string private hashSecret: string - private configSet: string | undefined + private defaultConfigSet: string | undefined + private noTrackingConfigSet: string | undefined constructor( credentials: MailCredentials, hashSecret: string, email?: string, - configSet?: string + defaultConfigSet?: string, + noTrackingConfigSet?: string ) { const { host, port, auth } = credentials this.hashSecret = hashSecret @@ -35,7 +38,8 @@ export default class MailClient { pass: auth.pass, }, }) - this.configSet = configSet + this.defaultConfigSet = defaultConfigSet + this.noTrackingConfigSet = noTrackingConfigSet } public sendMail( @@ -61,14 +65,7 @@ export default class MailClient { let headers: any = { [REFERENCE_ID_HEADER]: JSON.stringify(xSmtpHeader), } - if (this.configSet) { - headers = { - ...headers, - // Specify this to configure callback endpoint for notifications other - // than delivery and bounce through SES configuration set - [CONFIGURATION_SET_HEADER]: this.configSet, - } - } + headers = this.setSesConfigurationHeader(headers, option?.disableTracking) if (input.unsubLink) { headers = { ...headers, @@ -96,4 +93,28 @@ export default class MailClient { }) }) } + + private setSesConfigurationHeader( + headers: object, + disableTracking: boolean | undefined + ): object { + // 1. If there is no default config set, we will not set any configuration header + if (!this.defaultConfigSet) { + return headers + } + // 2. If the user wants to disable tracking and there is a no tracking configuration, we set it + if (disableTracking && this.noTrackingConfigSet) { + return { + ...headers, + // Configuration header does not include open and read notification + [CONFIGURATION_SET_HEADER]: this.noTrackingConfigSet, + } + } + // 3. Otherwise, we will use the default tracking SES configuration set + return { + ...headers, + // Configuration header includes open and read notification + [CONFIGURATION_SET_HEADER]: this.defaultConfigSet, + } + } } From a57ae9a1b1bae8d7a8a5780574c8b480247b0d40 Mon Sep 17 00:00:00 2001 From: KishenKumarrrrr Date: Wed, 24 Jan 2024 13:53:49 +0800 Subject: [PATCH 2/6] chore: update dockerfile --- backend/Dockerfile | 2 +- worker/Dockerfile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/Dockerfile b/backend/Dockerfile index 7127ef23c..c5972cf40 100644 --- a/backend/Dockerfile +++ b/backend/Dockerfile @@ -21,7 +21,7 @@ RUN apk update && apk upgrade && apk add --no-cache --virtual builds-deps build- RUN apk add jq -RUN python3 -m pip install awscli +RUN apk add --no-cache aws-cli RUN aws configure set default.region ap-southeast-1 diff --git a/worker/Dockerfile b/worker/Dockerfile index f043b93bc..3ea15995c 100644 --- a/worker/Dockerfile +++ b/worker/Dockerfile @@ -21,7 +21,7 @@ RUN apk update && apk upgrade && apk add --no-cache --virtual builds-deps build- RUN apk add jq -RUN python3 -m pip install awscli +RUN apk add --no-cache aws-cli RUN aws configure set default.region ap-southeast-1 From f21ea3da35c7395c24f9d7aa6f12da88908b003d Mon Sep 17 00:00:00 2001 From: KishenKumarrrrr Date: Thu, 25 Jan 2024 09:07:43 +0800 Subject: [PATCH 3/6] chore: set default value of disable_tracking to false --- backend/src/email/routes/email-transactional.routes.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/email/routes/email-transactional.routes.ts b/backend/src/email/routes/email-transactional.routes.ts index f87c04b0d..ff930bd45 100644 --- a/backend/src/email/routes/email-transactional.routes.ts +++ b/backend/src/email/routes/email-transactional.routes.ts @@ -62,7 +62,7 @@ export const InitEmailTransactionalRoute = ( .items( Joi.string().trim().email().options({ convert: true }).lowercase() ), - disable_tracking: Joi.boolean().optional(), + disable_tracking: Joi.boolean().default(false), }), } const getByIdValidator = { From faf29b0d2c65504500105439126634c2b8147ffa Mon Sep 17 00:00:00 2001 From: KishenKumarrrrr Date: Thu, 25 Jan 2024 09:22:57 +0800 Subject: [PATCH 4/6] chore: add comments to Dockerfile --- backend/Dockerfile | 3 +++ worker/Dockerfile | 3 +++ 2 files changed, 6 insertions(+) diff --git a/backend/Dockerfile b/backend/Dockerfile index c5972cf40..ffd68de25 100644 --- a/backend/Dockerfile +++ b/backend/Dockerfile @@ -21,6 +21,9 @@ RUN apk update && apk upgrade && apk add --no-cache --virtual builds-deps build- RUN apk add jq +# There was a breaking change in the base image used that prevents us from installing via pip +# Instead of activating a virtual env, this is a simpler workaround +# https://github.com/python/cpython/issues/102134 RUN apk add --no-cache aws-cli RUN aws configure set default.region ap-southeast-1 diff --git a/worker/Dockerfile b/worker/Dockerfile index 3ea15995c..e7667fbb5 100644 --- a/worker/Dockerfile +++ b/worker/Dockerfile @@ -21,6 +21,9 @@ RUN apk update && apk upgrade && apk add --no-cache --virtual builds-deps build- RUN apk add jq +# There was a breaking change in the base image used that prevents us from installing via pip +# Instead of activating a virtual env, this is a simpler workaround +# https://github.com/python/cpython/issues/102134 RUN apk add --no-cache aws-cli RUN aws configure set default.region ap-southeast-1 From d3cb6b4b328022c7d2a47806978c2ac469850a05 Mon Sep 17 00:00:00 2001 From: KishenKumarrrrr Date: Thu, 25 Jan 2024 11:46:32 +0800 Subject: [PATCH 5/6] chore: add comment to explain use of noTrackingConfigSet --- shared/src/clients/mail-client.class/index.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/shared/src/clients/mail-client.class/index.ts b/shared/src/clients/mail-client.class/index.ts index 48d899ce5..552b38d9b 100644 --- a/shared/src/clients/mail-client.class/index.ts +++ b/shared/src/clients/mail-client.class/index.ts @@ -18,6 +18,13 @@ export default class MailClient { private email: string private hashSecret: string private defaultConfigSet: string | undefined + /* + The AWS SES events to be tracked are defined in configuration sets within the AWS console. + When an email is sent, we specify the configuration set to be used by setting "X-SES-CONFIGURATION-SET" in the API call header. + + There is no option to turn off tracking via parameters in the API call, it can only be configured through a configuration set. + Thus, we need multiple configuration sets to toggle the tracking feature for read and open receipts. + */ private noTrackingConfigSet: string | undefined constructor( From 26ed6e7e6d190459364772e969ddefec1a0895e5 Mon Sep 17 00:00:00 2001 From: KishenKumarrrrr Date: Thu, 25 Jan 2024 12:15:30 +0800 Subject: [PATCH 6/6] test: update failing tests --- .../email/routes/tests/email-transactional.routes.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/backend/src/email/routes/tests/email-transactional.routes.test.ts b/backend/src/email/routes/tests/email-transactional.routes.test.ts index 68e40ea4f..107b33dfc 100644 --- a/backend/src/email/routes/tests/email-transactional.routes.test.ts +++ b/backend/src/email/routes/tests/email-transactional.routes.test.ts @@ -368,7 +368,7 @@ describe(`${emailTransactionalRoute}/send`, () => { ).id.toString(), attachments: undefined, }, - { extraSmtpHeaders: { isTransactional: true } } + { disableTracking: false, extraSmtpHeaders: { isTransactional: true } } ) }) test('Should throw a 400 error if the body size is too large (JSON payload)', async () => { @@ -616,6 +616,7 @@ describe(`${emailTransactionalRoute}/send`, () => { ], }, { + disableTracking: false, extraSmtpHeaders: { isTransactional: true }, } ) @@ -692,6 +693,7 @@ describe(`${emailTransactionalRoute}/send`, () => { ], }, { + disableTracking: false, extraSmtpHeaders: { isTransactional: true }, } ) @@ -825,6 +827,7 @@ describe(`${emailTransactionalRoute}/send`, () => { ], }, { + disableTracking: false, extraSmtpHeaders: { isTransactional: true }, } )