Skip to content
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

test(verification): integration tests for otp verification #1928

Merged
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import bcrypt from 'bcrypt'
import { ObjectId } from 'bson-ext'
import { subMinutes } from 'date-fns'
import { subMinutes, subYears } from 'date-fns'
import { getReasonPhrase, StatusCodes } from 'http-status-codes'
import _ from 'lodash'
import mongoose from 'mongoose'
import { okAsync } from 'neverthrow'
import nodemailer from 'nodemailer'
import Mail from 'nodemailer/lib/mailer'
import session, { Session } from 'supertest-session'
Expand All @@ -16,15 +18,22 @@ import {
MOCK_SIGNED_DATA,
} from 'src/app/modules/verification/__tests__/verification.test.helpers'
import getVerificationModel from 'src/app/modules/verification/verification.model'
import MailService from 'src/app/services/mail/mail.service'
import { SmsSendError } from 'src/app/services/sms/sms.errors'
import { WAIT_FOR_OTP_SECONDS } from 'src/shared/util/verification'
import * as SmsService from 'src/app/services/sms/sms.service'
import * as OtpUtils from 'src/app/utils/otp'
import {
NUM_OTP_RETRIES,
WAIT_FOR_OTP_SECONDS,
} from 'src/shared/util/verification'
import { BasicField, IVerificationSchema } from 'src/types'

import { setupApp } from 'tests/integration/helpers/express-setup'
import MockTwilio from 'tests/integration/helpers/twilio'
import { generateDefaultField } from 'tests/unit/backend/helpers/generate-form-data'
import dbHandler from 'tests/unit/backend/helpers/jest-db'

import { MOCK_OTP } from '../../../../../modules/verification/__tests__/verification.test.helpers'
import { PublicFormsVerificationRouter } from '../public-forms.verification.routes'

const Form = getFormModel(mongoose)
Expand All @@ -46,6 +55,9 @@ describe('public-forms.verification.routes', () => {
let mockEmailFieldId: string
let mockMobileFieldId: string
let request: Session
const MOCK_MOBILE_NUMBER = '+6582990039'
const MOCK_VALID_EMAIL_DOMAIN = 'example.com'
const MOCK_EMAIL = `mock@${MOCK_VALID_EMAIL_DOMAIN}`
let MockTransport: MockedObjectDeep<Mail>

beforeAll(async () => {
Expand Down Expand Up @@ -73,7 +85,7 @@ describe('public-forms.verification.routes', () => {
mockMobileFieldId = String(mobileField._id)
const { form: verifiableForm } = await dbHandler.insertEmailForm({
// Alternative mail domain so as not to clash with emptyForm
mailDomain: 'test2.gov.sg',
mailDomain: MOCK_VALID_EMAIL_DOMAIN,
formOptions: {
form_fields: [emailField, mobileField],
},
Expand Down Expand Up @@ -391,7 +403,7 @@ describe('public-forms.verification.routes', () => {
it('should return 400 when the otp could not be sent and fieldType is email', async () => {
// Arrange
// Retries on failure until limit hit, hence cannot just mock once
MockTransport.sendMail.mockRejectedValue('no')
MockTransport.sendMail.mockRejectedValueOnce('no')
const expectedResponse = {
message:
'Sorry, we were unable to send the email out at this time. Please ensure that the email entered is correct. If this problem persists, please refresh and try again later.',
Expand Down Expand Up @@ -521,7 +533,7 @@ describe('public-forms.verification.routes', () => {

it('should return 500 when there is a database error', async () => {
// Arrange
jest.spyOn(VerificationModel, 'findById').mockReturnValue({
jest.spyOn(VerificationModel, 'findById').mockReturnValueOnce({
exec: () => Promise.reject('no.'),
})
const expectedResponse = {
Expand All @@ -542,4 +554,288 @@ describe('public-forms.verification.routes', () => {
expect(response.body).toEqual(expectedResponse)
})
})

describe('POST /forms/:formId/fieldverifications/:transactionId/fields/:fieldId/otp/verify', () => {
// Mock the generation of otp so that the stored otp is the mock OTP
beforeEach(async () => {
await dbHandler.insertAgency({
mailDomain: MOCK_VALID_EMAIL_DOMAIN,
})
jest.spyOn(OtpUtils, 'generateOtp').mockReturnValue(MOCK_OTP)
})

it('should return 200 when the fieldType is email, request parameters are valid and the otp is correct', async () => {
// Arrange
await requestForMailOtp(mockEmailFieldId, MOCK_EMAIL)

// Act
const response = await request
.post(
`/forms/${mockVerifiableFormId}/fieldverifications/${mockTransactionId}/fields/${mockEmailFieldId}/otp/verify`,
)
.send({ otp: MOCK_OTP })

// Assert
expect(response.status).toBe(StatusCodes.OK)
expect(response.body).toBeString()
})

it('should return 200 when the fieldType is mobile, request parameters are valid and the otp is correct', async () => {
// Arrange
await requestForSmsOtp(mockMobileFieldId, MOCK_MOBILE_NUMBER)

// Act
const response = await request
.post(
`/forms/${mockVerifiableFormId}/fieldverifications/${mockTransactionId}/fields/${mockMobileFieldId}/otp/verify`,
)
.send({ otp: MOCK_OTP })

// Assert
expect(response.status).toBe(StatusCodes.OK)
expect(response.body).toBeString()
})

it('should return 400 when the transaction is expired', async () => {
// Arrange
const { _id: expiredTransactionId } = await VerificationModel.create({
formId: mockVerifiableFormId,
expireAt: Date.now(),
fields: [],
})
const expectedResponse = {
message: 'Your session has expired, please refresh and try again.',
}

// Act
const response = await request
.post(
`/forms/${mockVerifiableFormId}/fieldverifications/${expiredTransactionId}/fields/${mockMobileFieldId}/otp/verify`,
)
.send({ otp: MOCK_OTP })

// Assert
expect(response.status).toBe(StatusCodes.BAD_REQUEST)
expect(response.body).toEqual(expectedResponse)
})

it('should return 400 when the hash data could not be found', async () => {
// Arrange
// Remove the email field and persist to db
await mockTransaction.fields[1].remove()
await mockTransaction.save()

const expectedResponse = {
message: 'Sorry, something went wrong. Please refresh and try again.',
}

// Act
const response = await request
.post(
`/forms/${mockVerifiableFormId}/fieldverifications/${mockTransactionId}/fields/${mockMobileFieldId}/otp/verify`,
)
.send({ otp: MOCK_OTP })

// Assert
expect(response.status).toBe(StatusCodes.NOT_FOUND)
expect(response.body).toEqual(expectedResponse)
})

it('should return 404 when the form could not be found', async () => {
// Arrange
const expectedResponse = {
message: 'Sorry, something went wrong. Please refresh and try again.',
}

// Act
const response = await request
.post(
`/forms/${new ObjectId().toHexString()}/fieldverifications/${mockTransactionId}/fields/${mockMobileFieldId}/otp/verify`,
)
.send({ otp: MOCK_OTP })

// Assert
expect(response.status).toBe(StatusCodes.NOT_FOUND)
expect(response.body).toEqual(expectedResponse)
})

it('should return 404 when the transaction could not be found', async () => {
// Arrange
const expectedResponse = {
message: 'Sorry, something went wrong. Please refresh and try again.',
}

// Act
const response = await request
.post(
`/forms/${mockVerifiableFormId}/fieldverifications/${new ObjectId().toHexString()}/fields/${mockMobileFieldId}/otp/verify`,
)
.send({ otp: MOCK_OTP })

// Assert
expect(response.status).toBe(StatusCodes.NOT_FOUND)
expect(response.body).toEqual(expectedResponse)
})

it('should return 404 when the field could not be found', async () => {
// Arrange
const expectedResponse = {
message: 'Sorry, something went wrong. Please refresh and try again.',
}

// Act
const response = await request
.post(
`/forms/${mockVerifiableFormId}/fieldverifications/${mockTransactionId}/fields/${new ObjectId().toHexString()}/otp/verify`,
)
.send({ otp: MOCK_OTP })

// Assert
expect(response.status).toBe(StatusCodes.NOT_FOUND)
expect(response.body).toEqual(expectedResponse)
})

it('should return 422 when the otp is expired', async () => {
// Arrange
const mockOtpExpiredTransaction = await VerificationModel.create({
formId: mockVerifiableFormId,
fields: [
generateFieldParams({
_id: mockEmailFieldId,
hashCreatedAt: subYears(Date.now(), 1),
fieldType: BasicField.Email,
hashRetries: 0,
hashedOtp: MOCK_HASHED_OTP,
signedData: MOCK_SIGNED_DATA,
}),
],
})
const expectedResponse = {
message: 'Your OTP has expired, please request for a new one.',
}

// Act
const response = await request
.post(
`/forms/${mockVerifiableFormId}/fieldverifications/${mockOtpExpiredTransaction._id}/fields/${mockEmailFieldId}/otp/verify`,
)
.send({ otp: MOCK_OTP })

// Assert
expect(response.status).toBe(StatusCodes.UNPROCESSABLE_ENTITY)
expect(response.body).toEqual(expectedResponse)
})

it('should return 422 when the OTP retry limit is exceeded', async () => {
// Arrange
const requestForOtp = () =>
request
.post(
`/forms/${mockVerifiableFormId}/fieldverifications/${mockTransactionId}/fields/${mockEmailFieldId}/otp/verify`,
)
.send({ otp: MOCK_OTP })
const expectedResponse = {
message:
'You have entered too many invalid OTPs. Please request for a new OTP and try again.',
}

// Act
await Promise.allSettled(
_.range(NUM_OTP_RETRIES).map(() => requestForOtp()),
)
const response = await requestForOtp()

// Assert
expect(response.status).toBe(StatusCodes.UNPROCESSABLE_ENTITY)
expect(response.body).toEqual(expectedResponse)
})

it('should return 422 when the otp is wrong', async () => {
// Arrange
jest.spyOn(bcrypt, 'compare').mockResolvedValueOnce(false)
const expectedResponse = {
message: 'Wrong OTP.',
}

// Act
const response = await request
.post(
`/forms/${mockVerifiableFormId}/fieldverifications/${mockTransactionId}/fields/${mockEmailFieldId}/otp/verify`,
)
.send({ otp: '000000' })

// Assert
expect(response.status).toBe(StatusCodes.UNPROCESSABLE_ENTITY)
expect(response.body).toEqual(expectedResponse)
})

it('should return 500 when hashing error occurs', async () => {
// Arrange
jest.spyOn(bcrypt, 'compare').mockRejectedValueOnce(false)
const expectedResponse = {
message: 'Sorry, something went wrong. Please refresh and try again.',
}

// Act
const response = await request
.post(
`/forms/${mockVerifiableFormId}/fieldverifications/${mockTransactionId}/fields/${mockEmailFieldId}/otp/verify`,
)
.send({ otp: MOCK_OTP })

// Assert
expect(response.status).toBe(StatusCodes.INTERNAL_SERVER_ERROR)
expect(response.body).toEqual(expectedResponse)
})

it('should return 500 when database error occurs', async () => {
// Arrange
jest
.spyOn(VerificationModel, 'findById')
.mockReturnValue({ exec: jest.fn().mockRejectedValue('no') })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, but is it okay that the mockRejectedValue is 'no' instead of an error?

Copy link
Contributor Author

@seaerchin seaerchin May 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeap! the reason this is ok (or not, because of neverthrow hehehe) is due to neverthrow's ResultAsync.fromPromise.

our calls to the database essentially returns Promise<ModelSchema | null>, where the null is used to indicate an expected error when something is off with the query and the requested information could not be found (eg: no model matching the specified options).

however, promises and throwable functions actually have the possibility of an exception, which is an unexpected error that the called function isn't qualified to handle, and returns to teh caller. this is not encoded in the type signature and hence, is a mental overhead that engineers have to keep in the when calling functions, lest it crashes their system. In light of this, promises are actually typed as such: Promise<your_return_type | exception>

The ResultAsync.fromPromise function in neverthrow essentially allows us to sidestep this problem for promises (the corresponding function for throwables is Result.fromThrowable. This allows us to catch a rejecting promise (or an error) and map it back using a function. Because the type of exception is not encoded in ts' type system, this has to transform an unknown error back into a proper error type, which is done by transformMongoError

tl; dr: rejecting the promise causes it to go into error branch, and then a default error is returned from transformMongoError hence it's fine. thx for coming to my ted talk.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh okay thanks for sharing!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chowyiyin fyi the team decided a while ago that integration tests only need to cover Joi validation and success cases, since the controller unit tests should already test for the various error cases. @seaerchin has put in a lot of effort to mock lower layers even in integration tests so that error cases can be tested, which is great, but requires quite a bit of additional work.

const expectedResponse = {
message: 'Sorry, something went wrong. Please refresh and try again.',
}

// Act
const response = await request
.post(
`/forms/${mockVerifiableFormId}/fieldverifications/${mockTransactionId}/fields/${mockEmailFieldId}/otp/verify`,
)
.send({ otp: MOCK_OTP })

// Assert
expect(response.status).toBe(StatusCodes.INTERNAL_SERVER_ERROR)
expect(response.body).toEqual(expectedResponse)
})
})

// Helper functions
const requestForMailOtp = async (fieldId: string, answer: string) => {
// Set that so no real mail is sent.
jest
.spyOn(MailService, 'sendVerificationOtp')
.mockReturnValueOnce(okAsync(true))

const response = await request
.post(
`/forms/${mockVerifiableFormId}/fieldverifications/${mockTransactionId}/fields/${fieldId}/otp/generate`,
)
.send({ answer })
expect(response.status).toBe(StatusCodes.CREATED)
}

const requestForSmsOtp = async (fieldId: string, answer: string) => {
// Set that so no real mail is sent.
jest
.spyOn(SmsService, 'sendVerificationOtp')
.mockReturnValueOnce(okAsync(true))

const response = await request
.post(
`/forms/${mockVerifiableFormId}/fieldverifications/${mockTransactionId}/fields/${fieldId}/otp/generate`,
)
.send({ answer })
expect(response.status).toBe(StatusCodes.CREATED)
}
})