From a8ec327a73d9b90adc249cab2bbcb28544cb7e1d Mon Sep 17 00:00:00 2001 From: Yuan Ruo Date: Fri, 11 Dec 2020 13:01:58 +0800 Subject: [PATCH] refactor/is-form-public: migrate isFormPublic middleware to TypeScript (#791) --- .../public-forms.server.controller.js | 24 ------- .../__tests__/public-form.middlewares.spec.ts | 70 +++++++++++++++++++ .../public-form/public-form.middlewares.ts | 29 ++++++++ src/app/routes/public-forms.server.routes.js | 8 +-- .../public-forms.server.controller.spec.js | 48 ------------- 5 files changed, 103 insertions(+), 76 deletions(-) delete mode 100644 src/app/controllers/public-forms.server.controller.js create mode 100644 src/app/modules/form/public-form/__tests__/public-form.middlewares.spec.ts create mode 100644 src/app/modules/form/public-form/public-form.middlewares.ts delete mode 100644 tests/unit/backend/controllers/public-forms.server.controller.spec.js diff --git a/src/app/controllers/public-forms.server.controller.js b/src/app/controllers/public-forms.server.controller.js deleted file mode 100644 index ab18e5f9c7..0000000000 --- a/src/app/controllers/public-forms.server.controller.js +++ /dev/null @@ -1,24 +0,0 @@ -'use strict' - -const { StatusCodes } = require('http-status-codes') - -/** - * Checks if form is public - * @param {Object} req - Express request object - * @param {Object} res - Express response object - * @param {Object} next - Express next middleware function - */ -exports.isFormPublic = function (req, res, next) { - switch (req.form.status) { - case 'PUBLIC': - return next() - case 'ARCHIVED': - return res.sendStatus(StatusCodes.GONE) - default: - return res.status(StatusCodes.NOT_FOUND).json({ - message: req.form.inactiveMessage, - isPageFound: true, // Flag to prevent default 404 subtext ("please check link") from showing - formTitle: req.form.title, - }) - } -} diff --git a/src/app/modules/form/public-form/__tests__/public-form.middlewares.spec.ts b/src/app/modules/form/public-form/__tests__/public-form.middlewares.spec.ts new file mode 100644 index 0000000000..e57e669db1 --- /dev/null +++ b/src/app/modules/form/public-form/__tests__/public-form.middlewares.spec.ts @@ -0,0 +1,70 @@ +import { Request } from 'express' +import { StatusCodes } from 'http-status-codes' + +import expressHandler from 'tests/unit/backend/helpers/jest-express' + +import { Status, WithForm } from '../../../../../types' +import { isFormPublicCheck } from '../public-form.middlewares' + +describe('public-form.middlewares', () => { + it('should call next middleware function if form is public', () => { + const mockReq = Object.assign(expressHandler.mockRequest(), { + form: { status: Status.Public }, + }) as WithForm + const mockRes = expressHandler.mockResponse() + const mockNext = jest.fn() + + isFormPublicCheck(mockReq, mockRes, mockNext) + + expect(mockNext).toBeCalled() + expect(mockRes.sendStatus).not.toBeCalled() + expect(mockRes.status).not.toBeCalled() + expect(mockRes.json).not.toBeCalled() + }) + + it('should return HTTP 404 Not Found if form is private', () => { + const title = 'My private form' + const inactiveMessage = 'The form is not available.' + const form = { + status: Status.Private, + title, + inactiveMessage, + } + + const mockReq = Object.assign(expressHandler.mockRequest(), { + form, + }) as WithForm + const mockRes = expressHandler.mockResponse() + const mockNext = jest.fn() + + isFormPublicCheck(mockReq, mockRes, mockNext) + + expect(mockNext).not.toBeCalled() + expect(mockRes.sendStatus).not.toBeCalled() + expect(mockRes.status).toBeCalledWith(StatusCodes.NOT_FOUND) + expect(mockRes.json).toBeCalledWith({ + message: inactiveMessage, + isPageFound: true, + formTitle: title, + }) + }) + + it('should return HTTP 410 Gone if form is archived', () => { + const form = { + status: Status.Archived, + } + + const mockReq = Object.assign(expressHandler.mockRequest(), { + form, + }) as WithForm + const mockRes = expressHandler.mockResponse() + const mockNext = jest.fn() + + isFormPublicCheck(mockReq, mockRes, mockNext) + + expect(mockNext).not.toBeCalled() + expect(mockRes.sendStatus).toBeCalledWith(StatusCodes.GONE) + expect(mockRes.status).not.toBeCalled() + expect(mockRes.json).not.toBeCalledWith() + }) +}) diff --git a/src/app/modules/form/public-form/public-form.middlewares.ts b/src/app/modules/form/public-form/public-form.middlewares.ts new file mode 100644 index 0000000000..016d266a9c --- /dev/null +++ b/src/app/modules/form/public-form/public-form.middlewares.ts @@ -0,0 +1,29 @@ +import { RequestHandler } from 'express' +import { StatusCodes } from 'http-status-codes' + +import { WithForm } from '../../../../types' +import { FormDeletedError } from '../form.errors' +import { isFormPublic } from '../form.service' + +/** + * Express middleware function that checks if a form attached to the Express request handler is public. + * before allowing downstream middleware to handle the request. Otherwise, it returns a HTTP 404 if the + * form is private, or HTTP 410 if the form has been archived. + * @param req - Express request object + * @param res - Express response object + * @param next - Express next middleware function + */ +export const isFormPublicCheck: RequestHandler = (req, res, next) => { + const { form } = req as WithForm + return isFormPublic(form) + .map(() => next()) + .mapErr((error) => { + return error instanceof FormDeletedError + ? res.sendStatus(StatusCodes.GONE) + : res.status(StatusCodes.NOT_FOUND).json({ + message: form.inactiveMessage, + isPageFound: true, // Flag to prevent default 404 subtext ("please check link") from showing + formTitle: form.title, + }) + }) +} diff --git a/src/app/routes/public-forms.server.routes.js b/src/app/routes/public-forms.server.routes.js index 1ee5ea7c0e..fa78ce01d3 100644 --- a/src/app/routes/public-forms.server.routes.js +++ b/src/app/routes/public-forms.server.routes.js @@ -4,7 +4,7 @@ * Module dependencies. */ const forms = require('../../app/controllers/forms.server.controller') -const publicForms = require('../../app/controllers/public-forms.server.controller') +const publicForms = require('../modules/form/public-form/public-form.middlewares') const submissions = require('../../app/controllers/submissions.server.controller') const encryptSubmissions = require('../../app/controllers/encrypt-submissions.server.controller') const myInfoController = require('../../app/controllers/myinfo.server.controller') @@ -128,7 +128,7 @@ module.exports = function (app) { .route('/:formId([a-fA-F0-9]{24})/publicform') .get( forms.formById, - publicForms.isFormPublic, + publicForms.isFormPublicCheck, SpcpController.addSpcpSessionInfo, myInfoController.addMyInfo, forms.read(forms.REQUEST_TYPE.PUBLIC), @@ -162,7 +162,7 @@ module.exports = function (app) { limitRate({ max: rateLimitConfig.submissions }), CaptchaFactory.validateCaptchaParams, forms.formById, - publicForms.isFormPublic, + publicForms.isFormPublicCheck, CaptchaMiddleware.checkCaptchaResponse, SpcpController.isSpcpAuthenticated, EmailSubmissionsMiddleware.receiveEmailSubmission, @@ -269,7 +269,7 @@ module.exports = function (app) { }), }), forms.formById, - publicForms.isFormPublic, + publicForms.isFormPublicCheck, CaptchaMiddleware.checkCaptchaResponse, encryptSubmissions.validateEncryptSubmission, SpcpController.isSpcpAuthenticated, diff --git a/tests/unit/backend/controllers/public-forms.server.controller.spec.js b/tests/unit/backend/controllers/public-forms.server.controller.spec.js deleted file mode 100644 index 5af455455d..0000000000 --- a/tests/unit/backend/controllers/public-forms.server.controller.spec.js +++ /dev/null @@ -1,48 +0,0 @@ -const { StatusCodes } = require('http-status-codes') - -const Controller = spec( - 'dist/backend/app/controllers/public-forms.server.controller', -) - -describe('Public-Forms Controller', () => { - // Declare global variables - let req - let res - - beforeEach(async () => { - req = { - query: {}, - params: {}, - body: {}, - headers: {}, - url: '', - ip: '127.0.0.1', - get: () => '', - } - - res = jasmine.createSpyObj('res', ['status', 'send', 'json']) - }) - - describe('isFormPublic', () => { - it('should pass on to the next middleware if form is public', () => { - req.form = { - status: 'PUBLIC', - } - let next = jasmine.createSpy() - Controller.isFormPublic(req, res, next) - expect(next).toHaveBeenCalled() - }) - - it('should return a 404 error if form is not public', (done) => { - req.form = { - status: 'PRIVATE', - } - res.status.and.callFake(() => { - expect(res.status).toHaveBeenCalledWith(StatusCodes.NOT_FOUND) - done() - return res - }) - Controller.isFormPublic(req, res, () => {}) - }) - }) -})