Fix: allow single CC/BCC recipient to be sent via form-data (#2286)

* fix cc and bcc to allow stringified array

* add tests for cc

* abstract email validator, update test cases

* revert to 2d array for test inputs

backend/src/email/routes/email-transactional.routes.ts

@@ -21,6 +21,45 @@ export const InitEmailTransactionalRoute = (
   const router = Router({ mergeParams: true })
 
   // Validators
+  const emailValidator = Joi.string()
+    .trim()
+    .email()
+    .options({ convert: true })
+    .lowercase()
+
+  const emailArrayValidation = (fieldName: 'cc' | 'bcc') => {
+    return Joi.alternatives().try(
+      // array
+      Joi.array().unique().items(emailValidator),
+
+      // stringified array (form-data)
+      Joi.string().custom((value: string) => {
+        let parsed
+        try {
+          parsed = JSON.parse(value)
+        } catch {
+          throw new Error(
+            `${fieldName} must be a valid array or stringified array.`
+          )
+        }
+
+        if (!Array.isArray(parsed)) {
+          throw new Error(`${fieldName} must be a valid stringified array`)
+        }
+        const { value: validatedEmails, error } = Joi.array()
+          .unique()
+          .items(emailValidator)
+          .validate(parsed)
+
+        if (error) {
+          throw new Error(`${fieldName} ${error.message}`)
+        }
+
+        return validatedEmails
+      })
+    )
+  }
+
   const sendValidator = {
     [Segments.BODY]: Joi.object({
       recipient: Joi.string()
@@ -43,27 +82,15 @@ export const InitEmailTransactionalRoute = (
           )
         ),
       from: fromAddressValidator,
-      reply_to: Joi.string()
-        .trim()
-        .email()
-        .options({ convert: true })
-        .lowercase(),
+      reply_to: emailValidator,
       attachments: Joi.array().items(Joi.object().keys().required()),
       classification: Joi.string()
         .uppercase()
         .valid(...Object.values(TransactionalEmailClassification))
         .optional(),
       tag: Joi.string().max(255).optional(),
-      cc: Joi.array()
-        .unique()
-        .items(
-          Joi.string().trim().email().options({ convert: true }).lowercase()
-        ),
-      bcc: Joi.array()
-        .unique()
-        .items(
-          Joi.string().trim().email().options({ convert: true }).lowercase()
-        ),
+      cc: emailArrayValidation('cc'),
+      bcc: emailArrayValidation('bcc'),
       disable_tracking: Joi.boolean().default(false),
     }),
   }

backend/src/email/routes/tests/email-transactional.routes.test.ts

@@ -1,4 +1,5 @@
 import request from 'supertest'
+import { Op } from 'sequelize'
 import { Sequelize } from 'sequelize-typescript'
 
 import { User } from '@core/models'
@@ -15,6 +16,7 @@ import {
 import {
   EmailFromAddress,
   EmailMessageTransactional,
+  EmailMessageTransactionalCc,
   TransactionalEmailMessageStatus,
 } from '@email/models'
 import {
@@ -863,6 +865,166 @@ describe(`${emailTransactionalRoute}/send`, () => {
     ])
   })
 
+  const ccValidTests = [
+    ['[email protected]'],
+    ['[email protected]', '[email protected]'],
+    JSON.stringify(['[email protected]']),
+    JSON.stringify([
+      '[email protected]',
+      '[email protected]',
+    ]),
+  ]
+  test.each(ccValidTests)(
+    'Should send email with cc from valid array or stringified array - JSON payload',
+    async (cc) => {
+      mockSendEmail = jest
+        .spyOn(EmailService, 'sendEmail')
+        .mockResolvedValue(true)
+      const res = await request(app)
+        .post(endpoint)
+        .set('Authorization', `Bearer ${apiKey}`)
+        .send({
+          ...validApiCall,
+          cc,
+          reply_to: user.email,
+        })
+
+      const arrayToCheck = Array.isArray(cc) ? cc : JSON.parse(cc)
+
+      expect(res.status).toBe(201)
+      expect(res.body.cc.sort()).toStrictEqual(arrayToCheck.sort())
+      expect(mockSendEmail).toBeCalledTimes(1)
+      const transactionalEmail = await EmailMessageTransactional.findOne({
+        where: { id: res.body.id },
+      })
+      expect(transactionalEmail).not.toBeNull()
+      expect(transactionalEmail).toMatchObject({
+        recipient: validApiCall.recipient,
+        from: validApiCall.from,
+        status: TransactionalEmailMessageStatus.Accepted,
+        errorCode: null,
+      })
+    }
+  )
+
+  test.each(ccValidTests)(
+    'Should send email with cc from valid array or stringified array - form-data',
+    async (cc) => {
+      mockSendEmail = jest
+        .spyOn(EmailService, 'sendEmail')
+        .mockResolvedValue(true)
+      // in the case where single cc is sent, stringify the cc list
+      const ccSend =
+        Array.isArray(cc) && cc.length === 1 ? JSON.stringify(cc) : cc
+
+      const res = await request(app)
+        .post(endpoint)
+        .set('Authorization', `Bearer ${apiKey}`)
+        .field('recipient', validApiCall.recipient)
+        .field('subject', validApiCall.subject)
+        .field('body', validApiCall.body)
+        .field('from', 'Postman <[email protected]>')
+        .field('reply_to', validApiCall.reply_to)
+        .field('cc', ccSend)
+
+      const arrayToCheck = Array.isArray(cc) ? cc : JSON.parse(cc)
+
+      expect(res.status).toBe(201)
+      expect(res.body.cc.sort()).toStrictEqual(arrayToCheck.sort())
+      expect(mockSendEmail).toBeCalledTimes(1)
+      const transactionalEmail = await EmailMessageTransactional.findOne({
+        where: { id: res.body.id },
+        include: [
+          {
+            model: EmailMessageTransactionalCc,
+            attributes: ['email', 'ccType'],
+            where: { errorCode: { [Op.eq]: null } },
+            required: false,
+          },
+        ],
+      })
+
+      expect(transactionalEmail).not.toBeNull()
+      expect(transactionalEmail).toMatchObject({
+        recipient: validApiCall.recipient,
+        from: validApiCall.from,
+        status: TransactionalEmailMessageStatus.Accepted,
+        errorCode: null,
+      })
+      const transactionalCcEmails =
+        transactionalEmail?.emailMessageTransactionalCc.map(
+          (item) => item.email
+        )
+      expect(transactionalCcEmails?.sort()).toStrictEqual(arrayToCheck.sort())
+    }
+  )
+
+  const ccInvalidTests = [
+    {
+      cc: '[email protected]',
+      errMsg:
+        '"cc" failed custom validation because cc must be a valid array or stringified array.',
+    },
+    {
+      cc: JSON.stringify('[email protected]'),
+      errMsg:
+        '"cc" failed custom validation because cc must be a valid stringified array',
+    },
+    {
+      cc: JSON.stringify({ key: 'cc', email: '[email protected]' }),
+      errMsg:
+        '"cc" failed custom validation because cc must be a valid stringified array',
+    },
+  ]
+  test.each(ccInvalidTests)(
+    'Should throw api validation error if cc is not valid array or stringified array - JSON payload',
+    async ({ cc, errMsg }) => {
+      mockSendEmail = jest.spyOn(EmailService, 'sendEmail')
+
+      const res = await request(app)
+        .post(endpoint)
+        .set('Authorization', `Bearer ${apiKey}`)
+        .send({
+          ...validApiCall,
+          cc,
+          reply_to: user.email,
+        })
+
+      expect(res.status).toBe(400)
+      expect(mockSendEmail).not.toBeCalled()
+
+      expect(res.body).toStrictEqual({
+        code: 'api_validation',
+        message: errMsg,
+      })
+    }
+  )
+
+  test.each(ccInvalidTests)(
+    'Should throw api validation error if cc is not valid array or stringified array - form-data',
+    async ({ cc, errMsg }) => {
+      mockSendEmail = jest.spyOn(EmailService, 'sendEmail')
+
+      const res = await request(app)
+        .post(endpoint)
+        .set('Authorization', `Bearer ${apiKey}`)
+        .field('recipient', validApiCall.recipient)
+        .field('subject', validApiCall.subject)
+        .field('body', validApiCall.body)
+        .field('from', 'Postman <[email protected]>')
+        .field('reply_to', validApiCall.reply_to)
+        .field('cc', cc)
+
+      expect(res.status).toBe(400)
+      expect(mockSendEmail).not.toBeCalled()
+
+      expect(res.body).toStrictEqual({
+        code: 'api_validation',
+        message: errMsg,
+      })
+    }
+  )
+
   test('Requests should be rate limited and metadata and error code is saved correctly in db', async () => {
     mockSendEmail = jest
       .spyOn(EmailService, 'sendEmail')