Fix/mail

package.json

@@ -39,7 +39,6 @@
     "@sentry/webpack-plugin": "^1.15.1",
     "@types/express-rate-limit": "^5.1.3",
     "aws-sdk": "^2.1101.0",
-    "axios": "^0.27.2",
     "babel-polyfill": "^6.26.0",
     "bcrypt": "^5.0.1",
     "body-parser": "^1.18.3",

src/server/modules/auth/services/AuthService.ts

@@ -54,20 +54,21 @@ export class AuthService implements interfaces.AuthService {
         logger.error(`Could not save OTP hash:\t${saveError}`)
         throw new Error('Could not save OTP hash.')
       }
-      // Email out the otp (nodemailer)
-      try {
-        await this.mailer.mailOTP(email, otp, ip)
-      } catch (error) {
-        logger.error(`Error mailing OTP: ${error}`)
-        throw new Error('Error mailing OTP, please try again later.')
-      }
       logger.info(`OTP generation successful for ${email}`)
       dogstatsd.increment('otp.success', 1, 1)
     } catch (error) {
       logger.error(`OTP generation failed unexpectedly for ${email}:\t${error}`)
       dogstatsd.increment('otp.failure', 1, 1)
       throw new Error('OTP generation failed unexpectedly.')
     }
+
+    // Email out the otp
+    try {
+      await this.mailer.mailOTP(email, otp, ip)
+    } catch (error) {
+      logger.error(`Error mailing OTP: ${error}`)
+      throw new Error('Error mailing OTP, please try again later.')
+    }
   }
 
   public verifyOtp: (email: string, otp: string) => Promise<StorableUser> =

src/server/services/__tests__/email.test.ts

@@ -0,0 +1,149 @@
+import { MailBody } from '../email'
+/* eslint-disable global-require */
+
+/**
+ * Unit tests for Mailer.
+ */
+describe('Mailer tests', () => {
+  const testMailBody = {
+    to: '[email protected]',
+    body: 'Hi',
+    subject: 'Hi from postman',
+    senderDomain: 'go.gov.sg',
+  } as MailBody
+
+  const mockFetch = jest.fn()
+
+  beforeEach(() => {
+    jest.resetModules()
+    mockFetch.mockReset()
+  })
+
+  afterAll(jest.resetModules)
+
+  describe('sendPostmanMail', () => {
+    it('should throw error if postman api key is undefined', async () => {
+      jest.mock('../../config', () => ({
+        logger: console,
+        postmanApiKey: '',
+      }))
+      const { MailerNode } = require('../email')
+      const service = new MailerNode()
+
+      await expect(service.sendPostmanMail(testMailBody)).rejects.toThrowError()
+      expect(mockFetch).not.toHaveBeenCalled()
+    })
+
+    it('should throw error if postman api url is undefined', async () => {
+      jest.mock('../../config', () => ({
+        logger: console,
+        postmanApiUrl: '',
+        postmanApiKey: 'hey',
+      }))
+      const { MailerNode } = require('../email')
+      const service = new MailerNode()
+
+      await expect(service.sendPostmanMail(testMailBody)).rejects.toThrowError()
+      expect(mockFetch).not.toHaveBeenCalled()
+    })
+
+    it('should throw error if postman fails to send mail', async () => {
+      jest.mock('cross-fetch', () => mockFetch)
+      jest.mock('../../config', () => ({
+        logger: console,
+        postmanApiKey: 'key',
+        postmanApiUrl: 'url',
+      }))
+      const { MailerNode } = require('../email')
+      const service = new MailerNode()
+
+      mockFetch.mockResolvedValue({ ok: false })
+
+      await expect(service.sendPostmanMail(testMailBody)).rejects.toThrowError()
+      expect(mockFetch).toHaveBeenCalled()
+    })
+  })
+
+  describe('sendTransporterMail', () => {
+    it('should throw error if transporter fails to send mail', async () => {
+      const sendMailMock = jest.fn((_, callback) => {
+        const err = new Error('error')
+        callback(err, null)
+      })
+      jest.mock('nodemailer', () => ({
+        createTransport: jest.fn().mockImplementation(() => ({
+          sendMail: sendMailMock,
+        })),
+      }))
+
+      const { MailerNode } = require('../email')
+      const service = new MailerNode()
+      service.initMailer()
+      await expect(
+        service.sendTransporterMail(testMailBody),
+      ).rejects.toThrowError()
+      expect(sendMailMock).toHaveBeenCalled()
+    })
+  })
+
+  describe('sendMail', () => {
+    it('should send via nodemailer by default', async () => {
+      const sendMailMock = jest.fn((_, callback) => callback())
+      jest.mock('nodemailer', () => ({
+        createTransport: jest.fn().mockImplementation(() => ({
+          sendMail: sendMailMock,
+        })),
+      }))
+
+      const { MailerNode } = require('../email')
+      const service = new MailerNode()
+      service.initMailer()
+      await service.sendMail(testMailBody)
+      expect(sendMailMock).toHaveBeenCalled()
+    })
+
+    it('should send via Postman if activatePostmanFallback is true', async () => {
+      jest.mock('cross-fetch', () => mockFetch)
+      jest.mock('../../config', () => ({
+        logger: console,
+        activatePostmanFallback: true,
+        postmanApiKey: 'key',
+        postmanApiUrl: 'url',
+      }))
+      const { MailerNode } = require('../email')
+      const service = new MailerNode()
+
+      const postmanSpy = jest.spyOn(service, 'sendPostmanMail')
+      mockFetch.mockResolvedValue({ ok: true })
+
+      await service.sendMail(testMailBody)
+      expect(postmanSpy).toHaveBeenCalledWith(testMailBody)
+      expect(mockFetch).toHaveBeenCalled()
+    })
+  })
+
+  describe('mailOTP', () => {
+    const { MailerNode } = require('../email')
+    const service = new MailerNode()
+
+    it('should not send mail if otp is undefined', async () => {
+      const testEmail = '[email protected]'
+      const testOtp = undefined
+      const testIp = '1.1.1.1'
+
+      const sendMailSpy = jest.spyOn(service, 'sendMail')
+      await service.mailOTP(testEmail, testOtp, testIp)
+      expect(sendMailSpy).not.toHaveBeenCalled()
+    })
+
+    it('should not send mail if email is undefined', async () => {
+      const testEmail = undefined
+      const testOtp = '111111'
+      const testIp = '1.1.1.1'
+
+      const sendMailSpy = jest.spyOn(service, 'sendMail')
+      await service.mailOTP(testEmail, testOtp, testIp)
+      expect(sendMailSpy).not.toHaveBeenCalled()
+    })
+  })
+})

src/server/services/email.ts

@@ -3,7 +3,7 @@
 
 import { injectable } from 'inversify'
 import nodemailer from 'nodemailer'
-import axios from 'axios'
+import fetch from 'cross-fetch'
 import assetVariant from '../../shared/util/asset-variant'
 import {
   activatePostmanFallback,
@@ -19,28 +19,31 @@ const domainVariantMap = {
   gov: 'go.gov.sg',
   edu: 'for.edu.sg',
   health: 'for.sg',
-}
+} as const
 const domainVariant = domainVariantMap[assetVariant]
 
-interface MailBody {
+type SenderDomain = typeof domainVariantMap[keyof typeof domainVariantMap]
+export interface MailBody {
   to: string
   body: string
   subject: string
-  senderDomain?: string
+  senderDomain: SenderDomain
 }
 
 let transporter: nodemailer.Transport
 export interface Mailer {
   initMailer(): void
 
   /**
-   * Sends email to SES / MailDev to send out.
+   * Sends email to SES / MailDev to send out. Falls back to Postman.
    */
   mailOTP(email: string, otp: string, ip: string): Promise<void>
 }
 
 @injectable()
 export class MailerNode implements Mailer {
+  public aFetch: any = fetch
+
   initMailer() {
     transporter = nodemailer.createTransport(transporterOptions)
   }
@@ -58,20 +61,23 @@ export class MailerNode implements Mailer {
       body,
     }
 
-    try {
-      await axios.post(postmanApiUrl, mail, {
-        headers: {
-          Authorization: `Bearer ${postmanApiKey}`,
-        },
-      })
-    } catch (e: unknown) {
-      if (e instanceof Error) {
-        logger.error(e.message)
-      }
-      // TODO: please help to check FE error handling before throwing the exception
-      // throw e
+    const response = await fetch(postmanApiUrl, {
+      method: 'POST',
+      headers: {
+        'Content-Type': 'application/json',
+        Authorization: `Bearer ${postmanApiKey}`,
+      },
+      body: JSON.stringify(mail),
+    })
+    if (!response.ok) {
+      const error = new Error(
+        `Failed to send Postman mail:\tError: ${
+          response.statusText
+        }\thttpResponse: ${response.status}\t body:${JSON.stringify(response)}`,
+      )
+      logger.error(error.message)
+      throw error
     }
-
     return
   }