From 8dc3feba0fc9e337f4b483c6de6b81266ad8768f Mon Sep 17 00:00:00 2001 From: seaerchin Date: Mon, 5 Apr 2021 02:06:39 +0800 Subject: [PATCH 1/5] feat(helpers/jest-db): adds util methods to get/update form while testing --- tests/unit/backend/helpers/jest-db.ts | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/tests/unit/backend/helpers/jest-db.ts b/tests/unit/backend/helpers/jest-db.ts index 356c5aa3c9..5e9ce1a90e 100644 --- a/tests/unit/backend/helpers/jest-db.ts +++ b/tests/unit/backend/helpers/jest-db.ts @@ -11,9 +11,11 @@ import { IAgencySchema, IEmailFormSchema, IEncryptedFormSchema, + IPopulatedForm, IUserSchema, ResponseMode, } from 'src/types' +import { SettingsUpdateDto } from 'src/types/api' import MemoryDatabaseServer from 'tests/database' @@ -153,7 +155,6 @@ const insertEmailForm = async ({ }) const EmailFormModel = getEmailFormModel(mongoose) - const form = await EmailFormModel.create({ title: 'example form title', admin: user._id, @@ -214,6 +215,21 @@ const insertEncryptForm = async ({ } } +const getFullFormById = async ( + formId: string, +): Promise => + await getEmailFormModel(mongoose).getFullFormById(formId) + +// 1. Get full form by id +// 2. Patch in new settings +const updateForm = async ( + formId: string, + settingsToUpdate: SettingsUpdateDto, +): Promise => + await getEmailFormModel(mongoose) + .findByIdAndUpdate(formId, settingsToUpdate) + .exec() + const dbHandler = { connect, closeDatabase, @@ -224,6 +240,8 @@ const dbHandler = { clearCollection, insertEmailForm, insertEncryptForm, + getFullFormById, + updateForm, } export default dbHandler From 9ffe5a1d7a1b6bcd2316f215ba0d5cb7a9f854eb Mon Sep 17 00:00:00 2001 From: seaerchin Date: Wed, 7 Apr 2021 12:49:32 +0800 Subject: [PATCH 2/5] test(public-form/routes): adds integration tests for public-form --- .../__tests__/public-form.routes.spec.ts | 279 ++++++++++++++++++ 1 file changed, 279 insertions(+) create mode 100644 src/app/modules/form/public-form/__tests__/public-form.routes.spec.ts diff --git a/src/app/modules/form/public-form/__tests__/public-form.routes.spec.ts b/src/app/modules/form/public-form/__tests__/public-form.routes.spec.ts new file mode 100644 index 0000000000..1c2eb1ac36 --- /dev/null +++ b/src/app/modules/form/public-form/__tests__/public-form.routes.spec.ts @@ -0,0 +1,279 @@ +import SPCPAuthClient from '@opengovsg/spcp-auth-client' +import _ from 'lodash' +import { errAsync } from 'neverthrow' +import supertest, { Session } from 'supertest-session' +import { mocked } from 'ts-jest/utils' + +import { DatabaseError } from 'src/app/modules/core/core.errors' +import { MYINFO_COOKIE_NAME } from 'src/app/modules/myinfo/myinfo.constants' +import { MyInfoCookieState } from 'src/app/modules/myinfo/myinfo.types' +import { AuthType, Status } from 'src/types' + +import { setupApp } from 'tests/integration/helpers/express-setup' +import dbHandler from 'tests/unit/backend/helpers/jest-db' + +import * as AuthService from '../../../auth/auth.service' +import { PublicFormRouter } from '../public-form.routes' + +jest.mock('@opengovsg/myinfo-gov-client', () => ({ + MyInfoGovClient: jest.fn().mockImplementation(() => ({ + getPerson: jest.fn().mockReturnValue({ uinFin: 'S1234567A' }), + extractUinFin: jest.fn().mockReturnValue('S1234567A'), + })), + MyInfoMode: jest.requireActual('@opengovsg/myinfo-gov-client').MyInfoMode, + MyInfoSource: jest.requireActual('@opengovsg/myinfo-gov-client').MyInfoSource, + MyInfoAddressType: jest.requireActual('@opengovsg/myinfo-gov-client') + .MyInfoAddressType, + MyInfoAttribute: jest.requireActual('@opengovsg/myinfo-gov-client') + .MyInfoAttribute, +})) + +jest.mock('@opengovsg/spcp-auth-client') +const MockSpcpAuthClient = mocked(SPCPAuthClient, true) + +const app = setupApp('/', PublicFormRouter, { + setupWithAuth: false, +}) + +describe('public-form.routes', () => { + let request: Session + const mockSpClient = mocked(MockSpcpAuthClient.mock.instances[0], true) + const mockCpClient = mocked(MockSpcpAuthClient.mock.instances[1], true) + + beforeAll(async () => await dbHandler.connect()) + beforeEach(async () => { + request = supertest(app) + }) + afterEach(async () => { + await dbHandler.clearDatabase() + jest.restoreAllMocks() + }) + afterAll(async () => await dbHandler.closeDatabase()) + + describe('GET /:formId/publicform', () => { + const MOCK_COOKIE_PAYLOAD = { + userName: 'mock', + rememberMe: false, + } + + it('should return 200 with public form when form has AuthType.NIL and valid formId', async () => { + // Arrange + const { form } = await dbHandler.insertEmailForm({ + formOptions: { status: Status.Public }, + }) + // NOTE: This is needed to inject admin info into the form + const fullForm = await dbHandler.getFullFormById(form._id) + const expectedResponseBody = JSON.parse( + JSON.stringify({ + form: fullForm.getPublicView(), + isIntranetUser: false, + }), + ) + + // Act + const actualResponse = await request.get(`/${form._id}/publicform`) + + // Assert + expect(actualResponse.status).toEqual(200) + expect(actualResponse.body).toEqual(expectedResponseBody) + }) + + it('should return 200 with public form when form has AuthType.SP and valid formId', async () => { + // Arrange + mockSpClient.verifyJWT.mockImplementationOnce((_jwt, cb) => + cb(null, { + userName: MOCK_COOKIE_PAYLOAD.userName, + }), + ) + const { form } = await dbHandler.insertEmailForm({ + formOptions: { + esrvcId: 'mockEsrvcId', + authType: AuthType.SP, + hasCaptcha: false, + status: Status.Public, + }, + }) + const formId = form._id + // NOTE: This is needed to inject admin info into the form + const fullForm = await dbHandler.getFullFormById(formId) + const expectedResponseBody = JSON.parse( + JSON.stringify({ + form: fullForm?.getPublicView(), + spcpSession: { userName: MOCK_COOKIE_PAYLOAD.userName }, + isIntranetUser: false, + }), + ) + + // Act + // Set cookie on request + const actualResponse = await request + .get(`/${formId}/publicform`) + .set('Cookie', ['jwtSp=mockJwt']) + + // Assert + expect(actualResponse.status).toEqual(200) + expect(actualResponse.body).toEqual(expectedResponseBody) + }) + it('should return 200 with public form when form has AuthType.CP and valid formId', async () => { + // Arrange + mockCpClient.verifyJWT.mockImplementationOnce((_jwt, cb) => + cb(null, { + userName: MOCK_COOKIE_PAYLOAD.userName, + userInfo: 'MyCorpPassUEN', + }), + ) + const { form } = await dbHandler.insertEmailForm({ + formOptions: { + esrvcId: 'mockEsrvcId', + authType: AuthType.CP, + hasCaptcha: false, + status: Status.Public, + }, + }) + const formId = form._id + // NOTE: This is needed to inject admin info into the form + const fullForm = await dbHandler.getFullFormById(formId) + const expectedResponseBody = JSON.parse( + JSON.stringify({ + form: fullForm?.getPublicView(), + spcpSession: { userName: MOCK_COOKIE_PAYLOAD.userName }, + isIntranetUser: false, + }), + ) + + // Act + // Set cookie on request + const actualResponse = await request + .get(`/${formId}/publicform`) + .set('Cookie', ['jwtCp=mockJwt']) + + // Assert + expect(actualResponse.status).toEqual(200) + expect(actualResponse.body).toEqual(expectedResponseBody) + }) + it('should return 200 with public form when form has AuthType.MyInfo and valid formId', async () => { + // Arrange + const { form } = await dbHandler.insertEmailForm({ + formOptions: { + esrvcId: 'mockEsrvcId', + authType: AuthType.MyInfo, + hasCaptcha: false, + status: Status.Public, + }, + }) + // NOTE: This is needed to inject admin info into the form + const fullForm = await dbHandler.getFullFormById(form._id) + const expectedResponseBody = JSON.parse( + JSON.stringify({ + form: fullForm.getPublicView(), + spcpSession: { userName: 'S1234567A' }, + isIntranetUser: false, + }), + ) + const cookie = JSON.stringify({ + accessToken: 'mockAccessToken', + usedCount: 0, + state: MyInfoCookieState.Success, + }) + + // Act + const actualResponse = await request + .get(`/${form._id}/publicform`) + .set('Cookie', [ + // The j: indicates that the cookie is in JSON + `${MYINFO_COOKIE_NAME}=j:${encodeURIComponent(cookie)}`, + ]) + + // Assert + expect(actualResponse.status).toEqual(200) + expect(actualResponse.body).toEqual(expectedResponseBody) + }) + + it('should return 404 if the form does not exist', async () => { + // Arrange + const cookie = JSON.stringify({ + accessToken: 'mockAccessToken', + usedCount: 0, + state: MyInfoCookieState.Success, + }) + const MOCK_FORM_ID = _.pad('', 24, '1') + const expectedResponseBody = JSON.parse( + JSON.stringify({ + message: 'Form not found', + }), + ) + + // Act + const actualResponse = await request + .get(`/${MOCK_FORM_ID}/publicform`) + .set('Cookie', [ + // The j: indicates that the cookie is in JSON + `${MYINFO_COOKIE_NAME}=j:${encodeURIComponent(cookie)}`, + ]) + + // Assert + expect(actualResponse.status).toEqual(404) + expect(actualResponse.body).toEqual(expectedResponseBody) + }) + + it('should return 404 if the form is private', async () => { + // Arrange + const { form } = await dbHandler.insertEmailForm({ + formOptions: { status: Status.Private }, + }) + const expectedResponseBody = JSON.parse( + JSON.stringify({ + message: + 'If you think this is a mistake, please contact the agency that gave you the form link.', + }), + ) + + // Act + const actualResponse = await request.get(`/${form._id}/publicform`) + + // Assert + expect(actualResponse.status).toEqual(404) + expect(actualResponse.body).toEqual(expectedResponseBody) + }) + + it('should return 410 if the form has been archived', async () => { + // Arrange + const { form } = await dbHandler.insertEmailForm({ + formOptions: { status: Status.Archived }, + }) + const expectedResponseBody = JSON.parse( + JSON.stringify({ + message: 'Gone', + }), + ) + + // Act + const actualResponse = await request.get(`/${form._id}/publicform`) + + // Assert + expect(actualResponse.status).toEqual(410) + expect(actualResponse.body).toEqual(expectedResponseBody) + }) + + it('should return 500 if a database error occurs', async () => { + // Arrange + const { form } = await dbHandler.insertEmailForm({ + formOptions: { status: Status.Public }, + }) + const expectedError = new DatabaseError('all your base are belong to us') + const expectedResponseBody = JSON.parse( + JSON.stringify({ message: expectedError.message }), + ) + jest + .spyOn(AuthService, 'getFormIfPublic') + .mockReturnValueOnce(errAsync(expectedError)) + + // Act + const actualResponse = await request.get(`/${form._id}/publicform`) + + // Assert + expect(actualResponse.status).toEqual(500) + expect(actualResponse.body).toEqual(expectedResponseBody) + }) + }) +}) From 94d1a2569fa1da97332930b438e1b6c69dd3172a Mon Sep 17 00:00:00 2001 From: seaerchin Date: Wed, 7 Apr 2021 15:42:06 +0800 Subject: [PATCH 3/5] chore(helpers/jest-db): removed unused helper method --- tests/unit/backend/helpers/jest-db.ts | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/tests/unit/backend/helpers/jest-db.ts b/tests/unit/backend/helpers/jest-db.ts index 5e9ce1a90e..751ca8d169 100644 --- a/tests/unit/backend/helpers/jest-db.ts +++ b/tests/unit/backend/helpers/jest-db.ts @@ -15,7 +15,6 @@ import { IUserSchema, ResponseMode, } from 'src/types' -import { SettingsUpdateDto } from 'src/types/api' import MemoryDatabaseServer from 'tests/database' @@ -220,16 +219,6 @@ const getFullFormById = async ( ): Promise => await getEmailFormModel(mongoose).getFullFormById(formId) -// 1. Get full form by id -// 2. Patch in new settings -const updateForm = async ( - formId: string, - settingsToUpdate: SettingsUpdateDto, -): Promise => - await getEmailFormModel(mongoose) - .findByIdAndUpdate(formId, settingsToUpdate) - .exec() - const dbHandler = { connect, closeDatabase, @@ -241,7 +230,6 @@ const dbHandler = { insertEmailForm, insertEncryptForm, getFullFormById, - updateForm, } export default dbHandler From 0ae0623aff85243480d1ddf3b8c49e49374220b9 Mon Sep 17 00:00:00 2001 From: seaerchin Date: Wed, 7 Apr 2021 16:17:13 +0800 Subject: [PATCH 4/5] fix(public-form/controller): fixed regression issues where certain fields were missed --- .../form/public-form/public-form.controller.ts | 17 +++++++++++++++-- src/types/api/core.ts | 7 ++++++- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/app/modules/form/public-form/public-form.controller.ts b/src/app/modules/form/public-form/public-form.controller.ts index 52d0fab2f0..97237031bb 100644 --- a/src/app/modules/form/public-form/public-form.controller.ts +++ b/src/app/modules/form/public-form/public-form.controller.ts @@ -4,7 +4,7 @@ import querystring from 'querystring' import { UnreachableCaseError } from 'ts-essentials' import { AuthType } from '../../../../types' -import { ErrorDto } from '../../../../types/api' +import { ErrorDto, PrivateFormErrorDto } from '../../../../types/api' import { createLoggerWithLabel } from '../../../config/logger' import { isMongoError } from '../../../utils/handle-mongo-error' import { createReqMeta, getRequestIp } from '../../../utils/request' @@ -188,7 +188,7 @@ export const handleRedirect: RequestHandler< */ export const handleGetPublicForm: RequestHandler< { formId: string }, - PublicFormViewDto | ErrorDto + PublicFormViewDto | ErrorDto | PrivateFormErrorDto > = async (req, res) => { const { formId } = req.params const logMeta = { @@ -214,6 +214,19 @@ export const handleGetPublicForm: RequestHandler< }) } const { errorMessage, statusCode } = mapRouteError(error) + + // Specialized error response for PrivateFormError. + // This is to maintain backwards compatibility with the middleware implementation + if (error instanceof PrivateFormError) { + return res.status(statusCode).json({ + message: error.message, + // Flag to prevent default 404 subtext ("please check link") from + // showing. + isPageFound: true, + formTitle: error.formTitle, + }) + } + return res.status(statusCode).json({ message: errorMessage }) } diff --git a/src/types/api/core.ts b/src/types/api/core.ts index e12011e17f..3aec7b4572 100644 --- a/src/types/api/core.ts +++ b/src/types/api/core.ts @@ -1,3 +1,8 @@ -export type ErrorDto = { +export interface ErrorDto { message: string } + +export interface PrivateFormErrorDto extends ErrorDto { + isPageFound: true + formTitle: string +} From e4790c5603daa42b5ed991d2d12acc74c8411176 Mon Sep 17 00:00:00 2001 From: seaerchin Date: Wed, 7 Apr 2021 16:18:38 +0800 Subject: [PATCH 5/5] test(public-form): fixed tests so that privateFormError is checked correctly --- .../public-form/__tests__/public-form.controller.spec.ts | 5 ++++- .../public-form/__tests__/public-form.routes.spec.ts | 9 +++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/app/modules/form/public-form/__tests__/public-form.controller.spec.ts b/src/app/modules/form/public-form/__tests__/public-form.controller.spec.ts index 2d592f65e1..980fa978ed 100644 --- a/src/app/modules/form/public-form/__tests__/public-form.controller.spec.ts +++ b/src/app/modules/form/public-form/__tests__/public-form.controller.spec.ts @@ -952,10 +952,11 @@ describe('public-form.controller', () => { // Arrange // 1. Mock the response const mockRes = expressHandler.mockResponse() + const MOCK_FORM_TITLE = 'private form' // 2. Mock the call to retrieve the form MockAuthService.getFormIfPublic.mockReturnValueOnce( - errAsync(new PrivateFormError(MOCK_ERROR_STRING, 'private form')), + errAsync(new PrivateFormError(MOCK_ERROR_STRING, MOCK_FORM_TITLE)), ) // Act @@ -977,6 +978,8 @@ describe('public-form.controller', () => { expect(mockRes.status).toHaveBeenCalledWith(404) expect(mockRes.json).toHaveBeenCalledWith({ message: MOCK_ERROR_STRING, + formTitle: MOCK_FORM_TITLE, + isPageFound: true, }) }) }) diff --git a/src/app/modules/form/public-form/__tests__/public-form.routes.spec.ts b/src/app/modules/form/public-form/__tests__/public-form.routes.spec.ts index 1c2eb1ac36..ddc33e7bc2 100644 --- a/src/app/modules/form/public-form/__tests__/public-form.routes.spec.ts +++ b/src/app/modules/form/public-form/__tests__/public-form.routes.spec.ts @@ -1,5 +1,5 @@ import SPCPAuthClient from '@opengovsg/spcp-auth-client' -import _ from 'lodash' +import { ObjectId } from 'bson-ext' import { errAsync } from 'neverthrow' import supertest, { Session } from 'supertest-session' import { mocked } from 'ts-jest/utils' @@ -196,7 +196,7 @@ describe('public-form.routes', () => { usedCount: 0, state: MyInfoCookieState.Success, }) - const MOCK_FORM_ID = _.pad('', 24, '1') + const MOCK_FORM_ID = new ObjectId().toHexString() const expectedResponseBody = JSON.parse( JSON.stringify({ message: 'Form not found', @@ -223,8 +223,9 @@ describe('public-form.routes', () => { }) const expectedResponseBody = JSON.parse( JSON.stringify({ - message: - 'If you think this is a mistake, please contact the agency that gave you the form link.', + message: form.inactiveMessage, + formTitle: form.title, + isPageFound: true, }), )