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,6 +1,8 @@
import bcrypt from 'bcrypt'
import { ObjectId } from 'bson-ext'
import { subMinutes } from 'date-fns'
import { subMinutes, subYears } from 'date-fns'
import { StatusCodes } from 'http-status-codes'
import _ from 'lodash'
import mongoose from 'mongoose'
import session, { Session } from 'supertest-session'

Expand All @@ -10,12 +12,14 @@ import {
MOCK_SIGNED_DATA,
} from 'src/app/modules/verification/__tests__/verification.test.helpers'
import getVerificationModel from 'src/app/modules/verification/verification.model'
import { NUM_OTP_RETRIES } from 'src/shared/util/verification'
import { BasicField, IVerificationSchema } from 'src/types'

import { setupApp } from 'tests/integration/helpers/express-setup'
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 verificationApp = setupApp('/forms', PublicFormsVerificationRouter)
Expand Down Expand Up @@ -231,4 +235,249 @@ describe('public-forms.verification.routes', () => {
expect(response.body).toEqual(expectedResponse)
})
})

describe('POST /forms/:formId/fieldverifications/:transactionId/fields/:fieldId/otp/verify', () => {
beforeAll(() => {
jest.spyOn(bcrypt, 'compare').mockResolvedValue(true)
Copy link
Contributor

Choose a reason for hiding this comment

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

does this need to be mocked? we should try to only mock things we cannot simulate in integration tests; we should instead call the generate end point for real and see if verify returns success.

see our auth integration tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point! will do! will wait for otp generation to be merged first because this change is dependent on that

})

it('should return 200 when the fieldType is email, request parameters are valid and the otp is correct', async () => {
// 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).toBe(MOCK_SIGNED_DATA)
})

it('should return 200 when the fieldType is mobile, request parameters are valid and the otp is correct', async () => {
// 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).toBe(MOCK_SIGNED_DATA)
})

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 the form could not be found', async () => {
seaerchin marked this conversation as resolved.
Show resolved Hide resolved
// 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 the transaction could not be found', async () => {
seaerchin marked this conversation as resolved.
Show resolved Hide resolved
// 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)
})
})
})