From 69d7f69c7f6d02c42b7d26c256273f6fbc4dd8ba Mon Sep 17 00:00:00 2001 From: Walter Moar Date: Mon, 12 Aug 2024 11:00:19 -0700 Subject: [PATCH] fix: FORMS-1237 validate formSubmissionId parameters (#1457) * fix: FORMS-1237 validate formSubmissionId parameters * add tests * added some philosophical docs --- app/src/README.md | 67 ++++ .../common/middleware/validateParameter.js | 19 ++ app/src/forms/submission/routes.js | 9 +- .../forms/auth/middleware/apiAccess.spec.js | 10 +- .../middleware/validateParameter.spec.js | 71 ++++- app/tests/unit/forms/file/controller.spec.js | 4 +- app/tests/unit/forms/file/routes.spec.js | 4 +- app/tests/unit/forms/form/controller.spec.js | 34 +- .../unit/forms/form/exportService.spec.js | 4 +- .../forms/form/externalApi/controller.spec.js | 14 +- .../forms/form/externalApi/routes.spec.js | 6 +- .../forms/form/externalApi/service.spec.js | 18 +- app/tests/unit/forms/form/routes.spec.js | 9 +- app/tests/unit/forms/form/service.spec.js | 12 +- app/tests/unit/forms/proxy/service.spec.js | 6 +- .../unit/forms/submission/routes.spec.js | 299 +++++++++++++++++- .../unit/forms/submission/service.spec.js | 10 +- app/tests/unit/routes/v1/form.spec.js | 8 +- app/tests/unit/routes/v1/submission.spec.js | 26 +- 19 files changed, 527 insertions(+), 103 deletions(-) create mode 100644 app/src/README.md diff --git a/app/src/README.md b/app/src/README.md new file mode 100644 index 000000000..7879b4bd6 --- /dev/null +++ b/app/src/README.md @@ -0,0 +1,67 @@ +# Backend API + +This is the source code for the Express API. It contains all routes (endpoints) needed to run the application. + +## Design + +The code is broken down into layers: + +- The `middleware` layer provides a collection of utility functions that validate requests, check permissions, etc. +- The `route` layer uses `middleware` to validate requests and check permissions, and then performs work by calling a `controller` +- The `controller` layer deals with HTTP requests and responses, and performs work by calling a `service` +- The `service` layer contains business logic and manipulates data by calling the `model` +- The `model` layer is the ORM that deals with the database + +> Note: Between the `controller` and `service` there is a logical division where nothing below the `controller` knows about requests and responses, and nothing above the `service` touches the `model`. + +### Middleware + +Express middleware always has to call `next`, otherwise the request hangs. + +- when called as `next()` it chains to the next piece of middleware +- when called as `next(obj)` execution jumps to the error handler with `obj` as the error object + +We wrap middleware in a `try`/`catch` to make it obvious that `next` is always called: + +```javascript +const middleware = async (req, _res, next) => { + try { + // If possible the helper functions should throw a Problem rather than + // return a response that needs handling logic. + const formId = _getValidFormId(req); + const foo = service.getForm(formId); + + next(); + } catch (error) { + next(error); + } +}; +``` + +#### Testing + +Testing the middleware should include but is not limited to: + +- TODO + +### Routes + +Sets up routes with Middleware and then calls a Controller. + +#### Testing + +Testing the routes should include but is not limited to: + +- TODO + +### Controllers + +Calls Services + +### Services + +Calls ORM + +### ORM + +Calls DB diff --git a/app/src/forms/common/middleware/validateParameter.js b/app/src/forms/common/middleware/validateParameter.js index 2132b2794..d25e0b702 100644 --- a/app/src/forms/common/middleware/validateParameter.js +++ b/app/src/forms/common/middleware/validateParameter.js @@ -122,6 +122,24 @@ const validateFormId = async (_req, _res, next, formId) => { } }; +/** + * Validates that the :formSubmissionId route parameter exists and is a UUID. + * + * @param {*} _req the Express object representing the HTTP request - unused. + * @param {*} _res the Express object representing the HTTP response - unused. + * @param {*} next the Express chaining function. + * @param {*} formSubmissionId the :formSubmissionId value from the route. + */ +const validateFormSubmissionId = async (_req, _res, next, formSubmissionId) => { + try { + _validateUuid(formSubmissionId, 'formSubmissionId'); + + next(); + } catch (error) { + next(error); + } +}; + /** * Validates that the :formVersionDraftId route parameter exists and is a UUID. * This validator requires that the :formId route parameter also exists. @@ -179,6 +197,7 @@ module.exports = { validateExternalAPIId, validateFileId, validateFormId, + validateFormSubmissionId, validateFormVersionDraftId, validateFormVersionId, }; diff --git a/app/src/forms/submission/routes.js b/app/src/forms/submission/routes.js index 2425ef4bf..22622204b 100644 --- a/app/src/forms/submission/routes.js +++ b/app/src/forms/submission/routes.js @@ -1,15 +1,18 @@ -const apiAccess = require('../auth/middleware/apiAccess'); const routes = require('express').Router(); -const controller = require('./controller'); -const P = require('../common/constants').Permissions; +const apiAccess = require('../auth/middleware/apiAccess'); const { currentUser, hasSubmissionPermissions, filterMultipleSubmissions } = require('../auth/middleware/userAccess'); +const P = require('../common/constants').Permissions; const rateLimiter = require('../common/middleware').apiKeyRateLimiter; const validateParameter = require('../common/middleware/validateParameter'); +const controller = require('./controller'); + routes.use(currentUser); routes.param('documentTemplateId', validateParameter.validateDocumentTemplateId); +routes.param('formId', validateParameter.validateFormId); +routes.param('formSubmissionId', validateParameter.validateFormSubmissionId); routes.get('/:formSubmissionId', rateLimiter, apiAccess, hasSubmissionPermissions([P.SUBMISSION_READ]), async (req, res, next) => { await controller.read(req, res, next); diff --git a/app/tests/unit/forms/auth/middleware/apiAccess.spec.js b/app/tests/unit/forms/auth/middleware/apiAccess.spec.js index 2a494bf6c..cb6b9be8c 100644 --- a/app/tests/unit/forms/auth/middleware/apiAccess.spec.js +++ b/app/tests/unit/forms/auth/middleware/apiAccess.spec.js @@ -1,5 +1,5 @@ const { getMockReq, getMockRes } = require('@jest-mock/express'); -const { v4: uuidv4 } = require('uuid'); +const uuid = require('uuid'); const apiAccess = require('../../../../../src/forms/auth/middleware/apiAccess'); const formService = require('../../../../../src/forms/form/service'); @@ -8,10 +8,10 @@ const fileService = require('../../../../../src/forms/file/service'); const { NotFoundError } = require('objection'); describe('apiAccess', () => { - const fileId = uuidv4(); - const formId = uuidv4(); - const formSubmissionId = uuidv4(); - const secret = uuidv4(); + const fileId = uuid.v4(); + const formId = uuid.v4(); + const formSubmissionId = uuid.v4(); + const secret = uuid.v4(); const token = Buffer.from(`${formId}:${secret}`).toString('base64'); const authHeader = `Basic ${token}`; diff --git a/app/tests/unit/forms/common/middleware/validateParameter.spec.js b/app/tests/unit/forms/common/middleware/validateParameter.spec.js index 8aeb0baf3..3aa9076c2 100644 --- a/app/tests/unit/forms/common/middleware/validateParameter.spec.js +++ b/app/tests/unit/forms/common/middleware/validateParameter.spec.js @@ -1,24 +1,24 @@ const { getMockReq, getMockRes } = require('@jest-mock/express'); -const { v4: uuidv4 } = require('uuid'); +const uuid = require('uuid'); const validateParameter = require('../../../../../src/forms/common/middleware/validateParameter'); const externalApiService = require('../../../../../src/forms/form/externalApi/service'); const formService = require('../../../../../src/forms/form/service'); const submissionService = require('../../../../../src/forms/submission/service'); -const fileId = uuidv4(); -const formId = uuidv4(); -const formSubmissionId = uuidv4(); +const fileId = uuid.v4(); +const formId = uuid.v4(); +const formSubmissionId = uuid.v4(); // Various types of invalid UUIDs that we see in API calls. -const invalidUuids = [[''], ['undefined'], ['{{id}}'], ['${id}'], [uuidv4() + '.'], [' ' + uuidv4() + ' ']]; +const invalidUuids = [[''], ['undefined'], ['{{id}}'], ['${id}'], [uuid.v4() + '.'], [' ' + uuid.v4() + ' ']]; afterEach(() => { jest.clearAllMocks(); }); describe('validateDocumentTemplateId', () => { - const documentTemplateId = uuidv4(); + const documentTemplateId = uuid.v4(); const mockReadDocumentTemplateResponse = { formId: formId, @@ -86,7 +86,7 @@ describe('validateDocumentTemplateId', () => { test('formId does not match', async () => { formService.documentTemplateRead.mockReturnValueOnce({ - formId: uuidv4(), + formId: uuid.v4(), id: documentTemplateId, }); const req = getMockReq({ @@ -107,7 +107,7 @@ describe('validateDocumentTemplateId', () => { test('submission formId does not match', async () => { submissionService.read.mockReturnValueOnce({ form: { - id: uuidv4(), + id: uuid.v4(), }, }); const req = getMockReq({ @@ -200,7 +200,7 @@ describe('validateDocumentTemplateId', () => { }); describe('validateExternalApiId', () => { - const externalApiId = uuidv4(); + const externalApiId = uuid.v4(); const mockReadExternalApiResponse = { formId: formId, @@ -261,7 +261,7 @@ describe('validateExternalApiId', () => { test('formId does not match', async () => { externalApiService.readExternalAPI.mockReturnValueOnce({ - formId: uuidv4(), + formId: uuid.v4(), id: externalApiId, }); const req = getMockReq({ @@ -402,8 +402,51 @@ describe('validateFormId', () => { }); }); +describe('validateFormSubmissionId', () => { + describe('400 response when', () => { + const expectedStatus = { status: 400 }; + + test('formSubmissionId is missing', async () => { + const req = getMockReq({ + params: {}, + }); + const { res, next } = getMockRes(); + + await validateParameter.validateFormSubmissionId(req, res, next); + + expect(next).toBeCalledWith(expect.objectContaining(expectedStatus)); + }); + + test.each(invalidUuids)('formSubmissionId is "%s"', async (eachFormSubmissionId) => { + const req = getMockReq({ + params: { formSubmissionId: eachFormSubmissionId }, + }); + const { res, next } = getMockRes(); + + await validateParameter.validateFormSubmissionId(req, res, next, eachFormSubmissionId); + + expect(next).toBeCalledWith(expect.objectContaining(expectedStatus)); + }); + }); + + describe('allows', () => { + test('uuid for formSubmissionId', async () => { + const req = getMockReq({ + params: { + formSubmissionId: formSubmissionId, + }, + }); + const { res, next } = getMockRes(); + + await validateParameter.validateFormSubmissionId(req, res, next, formSubmissionId); + + expect(next).toBeCalledWith(); + }); + }); +}); + describe('validateFormVersionDraftId', () => { - const formVersionDraftId = uuidv4(); + const formVersionDraftId = uuid.v4(); const mockReadDraftResponse = { formId: formId, @@ -461,7 +504,7 @@ describe('validateFormVersionDraftId', () => { test('formId does not match', async () => { formService.readDraft.mockReturnValueOnce({ - formId: uuidv4(), + formId: uuid.v4(), id: formVersionDraftId, }); const req = getMockReq({ @@ -517,7 +560,7 @@ describe('validateFormVersionDraftId', () => { }); describe('validateFormVersionId', () => { - const formVersionId = uuidv4(); + const formVersionId = uuid.v4(); const mockReadVersionResponse = { formId: formId, @@ -575,7 +618,7 @@ describe('validateFormVersionId', () => { test('formId does not match', async () => { formService.readVersion.mockReturnValueOnce({ - formId: uuidv4(), + formId: uuid.v4(), id: formVersionId, }); const req = getMockReq({ diff --git a/app/tests/unit/forms/file/controller.spec.js b/app/tests/unit/forms/file/controller.spec.js index e23c62db5..cbe00f3be 100644 --- a/app/tests/unit/forms/file/controller.spec.js +++ b/app/tests/unit/forms/file/controller.spec.js @@ -1,5 +1,5 @@ const { getMockReq, getMockRes } = require('@jest-mock/express'); -const { v4: uuidv4 } = require('uuid'); +const uuid = require('uuid'); const controller = require('../../../../src/forms/file/controller'); const service = require('../../../../src/forms/file/service'); @@ -12,7 +12,7 @@ const currentUser = { const fileStorage = { createdAt: '2024-06-25T13:53:01-0700', createdBy: currentUser.usernameIdp, - id: uuidv4(), + id: uuid.v4(), mimeType: 'text/plain', originalName: 'testfile.txt', path: '/some/path', diff --git a/app/tests/unit/forms/file/routes.spec.js b/app/tests/unit/forms/file/routes.spec.js index 206ea04a1..69d7f8fa1 100644 --- a/app/tests/unit/forms/file/routes.spec.js +++ b/app/tests/unit/forms/file/routes.spec.js @@ -1,5 +1,5 @@ const request = require('supertest'); -const { v4: uuidv4 } = require('uuid'); +const uuid = require('uuid'); const { expressHelper } = require('../../../common/helper'); @@ -94,7 +94,7 @@ describe(`${basePath}`, () => { }); describe(`${basePath}/:id`, () => { - const fileId = uuidv4(); + const fileId = uuid.v4(); const path = `${basePath}/${fileId}`; it('should have correct middleware for DELETE', async () => { diff --git a/app/tests/unit/forms/form/controller.spec.js b/app/tests/unit/forms/form/controller.spec.js index 78e70f65c..630ba5899 100644 --- a/app/tests/unit/forms/form/controller.spec.js +++ b/app/tests/unit/forms/form/controller.spec.js @@ -1,5 +1,5 @@ const { getMockReq, getMockRes } = require('@jest-mock/express'); -const { v4: uuidv4 } = require('uuid'); +const uuid = require('uuid'); const controller = require('../../../../src/forms/form/controller'); const exportService = require('../../../../src/forms/form/exportService'); @@ -11,15 +11,15 @@ const currentUser = { const documentTemplate = { filename: 'cdogs_template.txt', - formId: uuidv4(), - id: uuidv4(), + formId: uuid.v4(), + id: uuid.v4(), template: 'My Template', }; const error = new Error('error'); // Various strings that should produce 400 errors when used as UUIDs. -const testCases400 = [[''], ['undefined'], ['{{oops}}'], [uuidv4() + '.']]; +const testCases400 = [[''], ['undefined'], ['{{oops}}'], [uuid.v4() + '.']]; // // Mock out all happy-path service calls. @@ -275,7 +275,7 @@ describe('form controller', () => { }); describe('listFormSubmissions', () => { - const uuid = uuidv4(); + const formId = uuid.v4(); const mockResponse = { results: [ { @@ -287,7 +287,7 @@ describe('listFormSubmissions', () => { it('should 200 if the formId is valid', async () => { // Arrange service.listFormSubmissions = jest.fn().mockReturnValue(mockResponse); - const req = getMockReq({ params: { formId: uuid } }); + const req = getMockReq({ params: { formId: formId } }); const { res, next } = getMockRes(); // Act @@ -313,9 +313,9 @@ describe('listFormSubmissions', () => { expect(res.json).toBeCalledWith({ detail: 'Bad formId "undefined".' }); }); - test.each(testCases400)('should 400 if the formId is "%s"', async (formId) => { + test.each(testCases400)('should 400 if the formId is "%s"', async (eachFormId) => { // Arrange - const req = getMockReq({ params: { formId: formId } }); + const req = getMockReq({ params: { formId: eachFormId } }); const { res, next } = getMockRes(); // Act @@ -324,7 +324,7 @@ describe('listFormSubmissions', () => { // Assert expect(service.listFormSubmissions).toBeCalledTimes(0); expect(res.status).toBeCalledWith(400); - expect(res.json).toBeCalledWith({ detail: `Bad formId "${formId}".` }); + expect(res.json).toBeCalledWith({ detail: `Bad formId "${eachFormId}".` }); }); it('should forward service errors for handling elsewhere', async () => { @@ -333,7 +333,7 @@ describe('listFormSubmissions', () => { service.listFormSubmissions = jest.fn(() => { throw error; }); - const req = getMockReq({ params: { formId: uuid } }); + const req = getMockReq({ params: { formId: formId } }); const { res, next } = getMockRes(); // Act @@ -346,17 +346,17 @@ describe('listFormSubmissions', () => { }); describe('readFormOptions', () => { - const uuid = uuidv4(); + const formId = uuid.v4(); const mockReadResponse = { form: { - id: uuid, + id: formId, }, }; it('should 200 if the formId is valid', async () => { // Arrange service.readFormOptions = jest.fn().mockReturnValue(mockReadResponse); - const req = getMockReq({ params: { formId: uuid } }); + const req = getMockReq({ params: { formId: formId } }); const { res, next } = getMockRes(); // Act @@ -382,9 +382,9 @@ describe('readFormOptions', () => { expect(res.json).toBeCalledWith({ detail: 'Bad formId "undefined".' }); }); - test.each(testCases400)('should 400 if the formId is "%s"', async (formId) => { + test.each(testCases400)('should 400 if the formId is "%s"', async (eachFormId) => { // Arrange - const req = getMockReq({ params: { formId: formId } }); + const req = getMockReq({ params: { formId: eachFormId } }); const { res, next } = getMockRes(); // Act @@ -393,7 +393,7 @@ describe('readFormOptions', () => { // Assert expect(service.readFormOptions).toBeCalledTimes(0); expect(res.status).toBeCalledWith(400); - expect(res.json).toBeCalledWith({ detail: `Bad formId "${formId}".` }); + expect(res.json).toBeCalledWith({ detail: `Bad formId "${eachFormId}".` }); }); it('should forward service errors for handling elsewhere', async () => { @@ -402,7 +402,7 @@ describe('readFormOptions', () => { service.readFormOptions = jest.fn(() => { throw error; }); - const req = getMockReq({ params: { formId: uuid } }); + const req = getMockReq({ params: { formId: formId } }); const { res, next } = getMockRes(); // Act diff --git a/app/tests/unit/forms/form/exportService.spec.js b/app/tests/unit/forms/form/exportService.spec.js index 329355431..b57895ee2 100644 --- a/app/tests/unit/forms/form/exportService.spec.js +++ b/app/tests/unit/forms/form/exportService.spec.js @@ -1,4 +1,4 @@ -const { v4: uuidv4 } = require('uuid'); +const uuid = require('uuid'); const exportService = require('../../../../src/forms/form/exportService'); const emailService = require('../../../../src/forms/email/emailService'); @@ -14,7 +14,7 @@ jest.mock('../../../../src/forms/common/models/views/submissionData', () => ({ then: jest.fn().mockReturnThis(), })); -const formId = uuidv4(); +const formId = uuid.v4(); const getCsvRowCount = (result) => { return result.data.split('\n').length; diff --git a/app/tests/unit/forms/form/externalApi/controller.spec.js b/app/tests/unit/forms/form/externalApi/controller.spec.js index 69aa55532..9983bd4b6 100644 --- a/app/tests/unit/forms/form/externalApi/controller.spec.js +++ b/app/tests/unit/forms/form/externalApi/controller.spec.js @@ -1,5 +1,5 @@ const { getMockReq, getMockRes } = require('@jest-mock/express'); -const { v4: uuidv4 } = require('uuid'); +const uuid = require('uuid'); const { ENCRYPTION_ALGORITHMS } = require('../../../../../src/components/encryptionService'); @@ -96,9 +96,9 @@ describe('createExternalAPI', () => { }); }); - const formId = uuidv4(); + const formId = uuid.v4(); const externalApi = { - id: uuidv4(), + id: uuid.v4(), formId: formId, name: 'test_api', endpointUrl: 'http://external.api/', @@ -192,8 +192,8 @@ describe('updateExternalAPI', () => { }); }); - const formId = uuidv4(); - const externalAPIId = uuidv4(); + const formId = uuid.v4(); + const externalAPIId = uuid.v4(); const externalApi = { id: externalAPIId, formId: formId, @@ -289,8 +289,8 @@ describe('deleteExternalAPI', () => { }); }); - const formId = uuidv4(); - const externalAPIId = uuidv4(); + const formId = uuid.v4(); + const externalAPIId = uuid.v4(); const validRequest = { currentUser: currentUser, diff --git a/app/tests/unit/forms/form/externalApi/routes.spec.js b/app/tests/unit/forms/form/externalApi/routes.spec.js index 6a9605a58..94fd983c8 100644 --- a/app/tests/unit/forms/form/externalApi/routes.spec.js +++ b/app/tests/unit/forms/form/externalApi/routes.spec.js @@ -1,5 +1,5 @@ const request = require('supertest'); -const { v4: uuidv4 } = require('uuid'); +const uuid = require('uuid'); const { expressHelper } = require('../../../../common/helper'); @@ -69,7 +69,7 @@ validateParameter.validateFormId = jest.fn((_req, _res, next) => { validateParameter.validateExternalAPIId = jest.fn((_req, _res, next) => { next(); }); -const formId = uuidv4(); +const formId = uuid.v4(); // // Create the router and a simple Express server. @@ -127,7 +127,7 @@ describe(`${basePath}/:formId/externalAPIs`, () => { }); describe(`${basePath}/:formId/externalAPIs/:externalAPIId`, () => { - const externalAPIId = uuidv4(); + const externalAPIId = uuid.v4(); const path = `${basePath}/${formId}/externalAPIs/${externalAPIId}`; controller.updateExternalAPI = jest.fn((_req, _res, next) => { next(); diff --git a/app/tests/unit/forms/form/externalApi/service.spec.js b/app/tests/unit/forms/form/externalApi/service.spec.js index 3d1903064..620429d02 100644 --- a/app/tests/unit/forms/form/externalApi/service.spec.js +++ b/app/tests/unit/forms/form/externalApi/service.spec.js @@ -1,6 +1,6 @@ const { MockModel, MockTransaction } = require('../../../../common/dbHelper'); -const { v4: uuidv4 } = require('uuid'); +const uuid = require('uuid'); const { ExternalAPIStatuses } = require('../../../../../src/forms/common/constants'); const service = require('../../../../../src/forms/form/externalApi/service'); @@ -21,8 +21,8 @@ describe('checkAllowSendUserToken', () => { let validData = null; beforeEach(() => { validData = { - id: uuidv4(), - formId: uuidv4(), + id: uuid.v4(), + formId: uuid.v4(), name: 'test_api', endpointUrl: 'http://external.api/', sendApiKey: true, @@ -65,8 +65,8 @@ describe('validateExternalAPI', () => { let validData = null; beforeEach(() => { validData = { - id: uuidv4(), - formId: uuidv4(), + id: uuid.v4(), + formId: uuid.v4(), name: 'test_api', endpointUrl: 'http://external.api/', sendApiKey: true, @@ -130,8 +130,8 @@ describe('createExternalAPI', () => { MockModel.mockReset(); MockTransaction.mockReset(); validData = { - id: uuidv4(), - formId: uuidv4(), + id: uuid.v4(), + formId: uuid.v4(), name: 'test_api', endpointUrl: 'http://external.api/', sendApiKey: true, @@ -180,8 +180,8 @@ describe('updateExternalAPI', () => { MockModel.mockReset(); MockTransaction.mockReset(); validData = { - id: uuidv4(), - formId: uuidv4(), + id: uuid.v4(), + formId: uuid.v4(), name: 'test_api', endpointUrl: 'http://external.api/', sendApiKey: true, diff --git a/app/tests/unit/forms/form/routes.spec.js b/app/tests/unit/forms/form/routes.spec.js index 242958cd8..2ab130875 100644 --- a/app/tests/unit/forms/form/routes.spec.js +++ b/app/tests/unit/forms/form/routes.spec.js @@ -1,5 +1,5 @@ const request = require('supertest'); -const { v4: uuidv4 } = require('uuid'); +const uuid = require('uuid'); const { expressHelper } = require('../../../common/helper'); @@ -13,9 +13,6 @@ const controller = require('../../../../src/forms/form/controller'); // Mock out all the middleware - we're testing that the routes are set up // correctly, not the functionality of the middleware. // -// -// mock middleware -// const jwtService = require('../../../../src/components/jwtService'); // @@ -70,8 +67,8 @@ validateParameter.validateFormId = jest.fn((_req, _res, next) => { next(); }); -const documentTemplateId = uuidv4(); -const formId = uuidv4(); +const documentTemplateId = uuid.v4(); +const formId = uuid.v4(); // // Create the router and a simple Express server. diff --git a/app/tests/unit/forms/form/service.spec.js b/app/tests/unit/forms/form/service.spec.js index 8e91daa48..9ae660f54 100644 --- a/app/tests/unit/forms/form/service.spec.js +++ b/app/tests/unit/forms/form/service.spec.js @@ -1,6 +1,6 @@ const { MockModel, MockTransaction } = require('../../../common/dbHelper'); -const { v4: uuidv4 } = require('uuid'); +const uuid = require('uuid'); const { EmailTypes } = require('../../../../src/forms/common/constants'); const service = require('../../../../src/forms/form/service'); @@ -9,8 +9,8 @@ jest.mock('../../../../src/forms/common/models/tables/documentTemplate', () => M jest.mock('../../../../src/forms/common/models/tables/formEmailTemplate', () => MockModel); jest.mock('../../../../src/forms/common/models/views/submissionMetadata', () => MockModel); -const documentTemplateId = uuidv4(); -const formId = uuidv4(); +const documentTemplateId = uuid.v4(); +const formId = uuid.v4(); const currentUser = { usernameIdp: 'TESTER', @@ -689,7 +689,7 @@ describe('popFormLevelInfo', () => { describe('_getDefaultEmailTemplate', () => { it('should return a template', async () => { - const formId = uuidv4(); + const formId = uuid.v4(); const template = service._getDefaultEmailTemplate(formId, EmailTypes.SUBMISSION_CONFIRMATION); expect(template.formId).toEqual(formId); @@ -773,7 +773,7 @@ describe('createOrUpdateEmailTemplates', () => { }); it('should update template when it already exists', async () => { - const id = uuidv4(); + const id = uuid.v4(); service.readEmailTemplate = jest.fn().mockReturnValue({ id: id, ...emailTemplate }); service.readEmailTemplates = jest.fn().mockReturnValue([{ id: id, ...emailTemplate }]); @@ -806,7 +806,7 @@ describe('createOrUpdateEmailTemplates', () => { }); it('should rollback when an update error occurs inside transaction', async () => { - const id = uuidv4(); + const id = uuid.v4(); service.readEmailTemplate = jest.fn().mockReturnValue({ id: id, ...emailTemplate }); service.readEmailTemplates = jest.fn().mockReturnValue([{ id: id, ...emailTemplate }]); MockModel.update = jest.fn().mockRejectedValueOnce(new Error('SQL Error')); diff --git a/app/tests/unit/forms/proxy/service.spec.js b/app/tests/unit/forms/proxy/service.spec.js index 2fa294125..7c3d953dc 100644 --- a/app/tests/unit/forms/proxy/service.spec.js +++ b/app/tests/unit/forms/proxy/service.spec.js @@ -1,6 +1,6 @@ const { MockModel, MockTransaction } = require('../../../common/dbHelper'); -const { v4: uuidv4 } = require('uuid'); +const uuid = require('uuid'); const { encryptionService, ENCRYPTION_ALGORITHMS } = require('../../../../src/components/encryptionService'); const service = require('../../../../src/forms/proxy/service'); @@ -33,8 +33,8 @@ const goodProxyHeaderInfo = { delete goodProxyHeaderInfo.idpUserId; const goodExternalApi = { - id: uuidv4(), - formId: uuidv4(), + id: uuid.v4(), + formId: uuid.v4(), name: 'test_api', endpointUrl: 'http://external.api/', sendApiKey: true, diff --git a/app/tests/unit/forms/submission/routes.spec.js b/app/tests/unit/forms/submission/routes.spec.js index d47f04df3..f44af8400 100644 --- a/app/tests/unit/forms/submission/routes.spec.js +++ b/app/tests/unit/forms/submission/routes.spec.js @@ -1,5 +1,5 @@ const request = require('supertest'); -const { v4: uuidv4 } = require('uuid'); +const uuid = require('uuid'); const { expressHelper } = require('../../../common/helper'); @@ -31,9 +31,51 @@ apiAccess.mockImplementation( }) ); +controller.addNote = jest.fn((_req, _res, next) => { + next(); +}); +controller.addStatus = jest.fn((_req, _res, next) => { + next(); +}); +controller.delete = jest.fn((_req, _res, next) => { + next(); +}); +controller.deleteMutipleSubmissions = jest.fn((_req, _res, next) => { + next(); +}); +controller.email = jest.fn((_req, _res, next) => { + next(); +}); +controller.getNotes = jest.fn((_req, _res, next) => { + next(); +}); +controller.getStatus = jest.fn((_req, _res, next) => { + next(); +}); +controller.listEdits = jest.fn((_req, _res, next) => { + next(); +}); +controller.read = jest.fn((_req, _res, next) => { + next(); +}); +controller.readOptions = jest.fn((_req, _res, next) => { + next(); +}); +controller.restore = jest.fn((_req, _res, next) => { + next(); +}); +controller.restoreMutipleSubmissions = jest.fn((_req, _res, next) => { + next(); +}); controller.templateRender = jest.fn((_req, _res, next) => { next(); }); +controller.templateUploadAndRender = jest.fn((_req, _res, next) => { + next(); +}); +controller.update = jest.fn((_req, _res, next) => { + next(); +}); rateLimiter.apiKeyRateLimiter = jest.fn((_req, _res, next) => { next(); @@ -46,7 +88,9 @@ const hasSubmissionPermissionsMock = jest.fn((_req, _res, next) => { userAccess.currentUser = jest.fn((_req, _res, next) => { next(); }); - +userAccess.filterMultipleSubmissions = jest.fn((_req, _res, next) => { + next(); +}); userAccess.hasSubmissionPermissions = jest.fn(() => { return hasSubmissionPermissionsMock; }); @@ -54,9 +98,16 @@ userAccess.hasSubmissionPermissions = jest.fn(() => { validateParameter.validateDocumentTemplateId = jest.fn((_req, _res, next) => { next(); }); +validateParameter.validateFormId = jest.fn((_req, _res, next) => { + next(); +}); +validateParameter.validateFormSubmissionId = jest.fn((_req, _res, next) => { + next(); +}); -const documentTemplateId = uuidv4(); -const formSubmissionId = uuidv4(); +const documentTemplateId = uuid.v4(); +const formId = uuid.v4(); +const formSubmissionId = uuid.v4(); // // Create the router and a simple Express server. @@ -71,16 +122,256 @@ afterEach(() => { jest.clearAllMocks(); }); +describe(`${basePath}/:formSubmissionId`, () => { + const path = `${basePath}/${formSubmissionId}`; + + it('should have correct middleware for DELETE', async () => { + await appRequest.delete(path); + + expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(0); + expect(validateParameter.validateFormSubmissionId).toBeCalledTimes(1); + expect(apiAccess).toBeCalledTimes(1); + expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(hasSubmissionPermissionsMock).toBeCalledTimes(1); + expect(userAccess.filterMultipleSubmissions).toBeCalledTimes(0); + expect(controller.delete).toBeCalledTimes(1); + }); + + it('should have correct middleware for GET', async () => { + await appRequest.get(path); + + expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(0); + expect(validateParameter.validateFormSubmissionId).toBeCalledTimes(1); + expect(apiAccess).toBeCalledTimes(1); + expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(hasSubmissionPermissionsMock).toBeCalledTimes(1); + expect(userAccess.filterMultipleSubmissions).toBeCalledTimes(0); + expect(controller.read).toBeCalledTimes(1); + }); + + it('should have correct middleware for PUT', async () => { + await appRequest.put(path); + + expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(0); + expect(validateParameter.validateFormSubmissionId).toBeCalledTimes(1); + expect(apiAccess).toBeCalledTimes(0); + expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(hasSubmissionPermissionsMock).toBeCalledTimes(1); + expect(userAccess.filterMultipleSubmissions).toBeCalledTimes(0); + expect(controller.update).toBeCalledTimes(1); + }); +}); + +describe(`${basePath}/:formSubmissionId/:formId/submissions`, () => { + const path = `${basePath}/${formSubmissionId}/${formId}/submissions`; + + it('should have correct middleware for DELETE', async () => { + await appRequest.delete(path); + + expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(1); + expect(validateParameter.validateFormSubmissionId).toBeCalledTimes(1); + expect(apiAccess).toBeCalledTimes(0); + expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(hasSubmissionPermissionsMock).toBeCalledTimes(1); + expect(userAccess.filterMultipleSubmissions).toBeCalledTimes(1); + expect(controller.deleteMutipleSubmissions).toBeCalledTimes(1); + }); +}); + +describe(`${basePath}/:formSubmissionId/:formId/submissions/restore`, () => { + const path = `${basePath}/${formSubmissionId}/${formId}/submissions/restore`; + + it('should have correct middleware for PUT', async () => { + await appRequest.put(path); + + expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(1); + expect(validateParameter.validateFormSubmissionId).toBeCalledTimes(1); + expect(apiAccess).toBeCalledTimes(0); + expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(hasSubmissionPermissionsMock).toBeCalledTimes(1); + expect(userAccess.filterMultipleSubmissions).toBeCalledTimes(1); + expect(controller.restoreMutipleSubmissions).toBeCalledTimes(1); + }); +}); + +describe(`${basePath}/:formSubmissionId/edits`, () => { + const path = `${basePath}/${formSubmissionId}/edits`; + + it('should have correct middleware for GET', async () => { + await appRequest.get(path); + + expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(0); + expect(validateParameter.validateFormSubmissionId).toBeCalledTimes(1); + expect(apiAccess).toBeCalledTimes(0); + expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(hasSubmissionPermissionsMock).toBeCalledTimes(1); + expect(userAccess.filterMultipleSubmissions).toBeCalledTimes(0); + expect(controller.listEdits).toBeCalledTimes(1); + }); +}); + +describe(`${basePath}/:formSubmissionId/email`, () => { + const path = `${basePath}/${formSubmissionId}/email`; + + it('should have correct middleware for POST', async () => { + await appRequest.post(path); + + expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(0); + expect(validateParameter.validateFormSubmissionId).toBeCalledTimes(1); + expect(apiAccess).toBeCalledTimes(0); + expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(hasSubmissionPermissionsMock).toBeCalledTimes(1); + expect(userAccess.filterMultipleSubmissions).toBeCalledTimes(0); + expect(controller.email).toBeCalledTimes(1); + }); +}); + +describe(`${basePath}/:formSubmissionId/notes`, () => { + const path = `${basePath}/${formSubmissionId}/notes`; + + it('should have correct middleware for GET', async () => { + await appRequest.get(path); + + expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(0); + expect(validateParameter.validateFormSubmissionId).toBeCalledTimes(1); + expect(apiAccess).toBeCalledTimes(0); + expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(hasSubmissionPermissionsMock).toBeCalledTimes(1); + expect(userAccess.filterMultipleSubmissions).toBeCalledTimes(0); + expect(controller.getNotes).toBeCalledTimes(1); + }); + + it('should have correct middleware for POST', async () => { + await appRequest.post(path); + + expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(0); + expect(validateParameter.validateFormSubmissionId).toBeCalledTimes(1); + expect(apiAccess).toBeCalledTimes(0); + expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(hasSubmissionPermissionsMock).toBeCalledTimes(1); + expect(userAccess.filterMultipleSubmissions).toBeCalledTimes(0); + expect(controller.addNote).toBeCalledTimes(1); + }); +}); + +describe(`${basePath}/:formSubmissionId/options`, () => { + const path = `${basePath}/${formSubmissionId}/options`; + + it('should have correct middleware for GET', async () => { + await appRequest.get(path); + + expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(0); + expect(validateParameter.validateFormSubmissionId).toBeCalledTimes(1); + expect(apiAccess).toBeCalledTimes(0); + expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(hasSubmissionPermissionsMock).toBeCalledTimes(0); + expect(userAccess.filterMultipleSubmissions).toBeCalledTimes(0); + expect(controller.readOptions).toBeCalledTimes(1); + }); +}); + +describe(`${basePath}/:formSubmissionId/restore`, () => { + const path = `${basePath}/${formSubmissionId}/restore`; + + it('should have correct middleware for PUT', async () => { + await appRequest.put(path); + + expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(0); + expect(validateParameter.validateFormSubmissionId).toBeCalledTimes(1); + expect(apiAccess).toBeCalledTimes(0); + expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(hasSubmissionPermissionsMock).toBeCalledTimes(1); + expect(userAccess.filterMultipleSubmissions).toBeCalledTimes(0); + expect(controller.restore).toBeCalledTimes(1); + }); +}); + +describe(`${basePath}/:formSubmissionId/status`, () => { + const path = `${basePath}/${formSubmissionId}/status`; + + it('should have correct middleware for GET', async () => { + await appRequest.get(path); + + expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(0); + expect(validateParameter.validateFormSubmissionId).toBeCalledTimes(1); + expect(apiAccess).toBeCalledTimes(1); + expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(hasSubmissionPermissionsMock).toBeCalledTimes(1); + expect(userAccess.filterMultipleSubmissions).toBeCalledTimes(0); + expect(controller.getStatus).toBeCalledTimes(1); + }); + + it('should have correct middleware for POST', async () => { + await appRequest.post(path); + + expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(0); + expect(validateParameter.validateFormSubmissionId).toBeCalledTimes(1); + expect(apiAccess).toBeCalledTimes(0); + expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(hasSubmissionPermissionsMock).toBeCalledTimes(1); + expect(userAccess.filterMultipleSubmissions).toBeCalledTimes(0); + expect(controller.addStatus).toBeCalledTimes(1); + }); +}); + describe(`${basePath}/:formSubmissionId/template/:documentTemplateId/render`, () => { const path = `${basePath}/${formSubmissionId}/template/${documentTemplateId}/render`; it('should have correct middleware for GET', async () => { await appRequest.get(path); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(1); + expect(validateParameter.validateFormId).toBeCalledTimes(0); + expect(validateParameter.validateFormSubmissionId).toBeCalledTimes(1); expect(apiAccess).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); expect(hasSubmissionPermissionsMock).toBeCalledTimes(1); + expect(userAccess.filterMultipleSubmissions).toBeCalledTimes(0); expect(controller.templateRender).toBeCalledTimes(1); }); }); + +describe(`${basePath}/:formSubmissionId/template/render`, () => { + const path = `${basePath}/${formSubmissionId}/template/render`; + + it('should have correct middleware for POST', async () => { + await appRequest.post(path); + + expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(0); + expect(validateParameter.validateFormSubmissionId).toBeCalledTimes(1); + expect(apiAccess).toBeCalledTimes(1); + expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(hasSubmissionPermissionsMock).toBeCalledTimes(1); + expect(userAccess.filterMultipleSubmissions).toBeCalledTimes(0); + expect(controller.templateUploadAndRender).toBeCalledTimes(1); + }); +}); diff --git a/app/tests/unit/forms/submission/service.spec.js b/app/tests/unit/forms/submission/service.spec.js index 2070af1b1..40a5b8db6 100644 --- a/app/tests/unit/forms/submission/service.spec.js +++ b/app/tests/unit/forms/submission/service.spec.js @@ -1,5 +1,5 @@ const { MockModel, MockTransaction } = require('../../../common/dbHelper'); -const { v4: uuidv4 } = require('uuid'); +const uuid = require('uuid'); jest.mock('../../../../src/forms/common/models/tables/formSubmissionStatus', () => MockModel); jest.mock('../../../../src/forms/common/models/tables/formSubmission', () => MockModel); @@ -54,8 +54,8 @@ describe('createStatus', () => { describe('deleteMutipleSubmissions', () => { it('should delete the selected submissions', async () => { - let submissionId1 = uuidv4(); - let submissionId2 = uuidv4(); + let submissionId1 = uuid.v4(); + let submissionId2 = uuid.v4(); const submissionIds = [submissionId1, submissionId2]; const returnValue = { @@ -85,8 +85,8 @@ describe('deleteMutipleSubmissions', () => { describe('restoreMutipleSubmissions', () => { it('should delete the selected submissions', async () => { - let submissionId1 = uuidv4(); - let submissionId2 = uuidv4(); + let submissionId1 = uuid.v4(); + let submissionId2 = uuid.v4(); const submissionIds = [submissionId1, submissionId2]; const returnValue = { diff --git a/app/tests/unit/routes/v1/form.spec.js b/app/tests/unit/routes/v1/form.spec.js index ffdb78efc..c2a0fa4bf 100644 --- a/app/tests/unit/routes/v1/form.spec.js +++ b/app/tests/unit/routes/v1/form.spec.js @@ -1,6 +1,6 @@ const request = require('supertest'); const Problem = require('api-problem'); -const { v4: uuidv4 } = require('uuid'); +const uuid = require('uuid'); const { expressHelper } = require('../../../common/helper'); @@ -28,9 +28,9 @@ userAccess.hasFormPermissions = jest.fn(() => { }); }); -const formId = uuidv4(); -const formVersionDraftId = uuidv4(); -const formVersionId = uuidv4(); +const formId = uuid.v4(); +const formVersionDraftId = uuid.v4(); +const formVersionId = uuid.v4(); // // we will mock the underlying data service calls... diff --git a/app/tests/unit/routes/v1/submission.spec.js b/app/tests/unit/routes/v1/submission.spec.js index a2871efb7..de4fb20b4 100644 --- a/app/tests/unit/routes/v1/submission.spec.js +++ b/app/tests/unit/routes/v1/submission.spec.js @@ -1,8 +1,12 @@ -const request = require('supertest'); const Problem = require('api-problem'); +const request = require('supertest'); +const uuid = require('uuid'); const { expressHelper } = require('../../../common/helper'); +const formId = uuid.v4(); +const formSubmissionId = uuid.v4(); + // // mock middleware // @@ -49,7 +53,7 @@ afterEach(() => { }); describe(`${basePath}/:formSubmissionId`, () => { - const path = `${basePath}/:formSubmissionId`; + const path = `${basePath}/${formSubmissionId}`; describe('DELETE', () => { it('should return 200', async () => { @@ -161,7 +165,7 @@ describe(`${basePath}/:formSubmissionId`, () => { }); describe(`${basePath}/:formSubmissionId/edits`, () => { - const path = `${basePath}/:formSubmissionId/edits`; + const path = `${basePath}/${formSubmissionId}/edits`; describe('GET', () => { it('should return 200', async () => { @@ -201,7 +205,7 @@ describe(`${basePath}/:formSubmissionId/edits`, () => { }); describe(`${basePath}/:formSubmissionId/email`, () => { - const path = `${basePath}/:formSubmissionId/email`; + const path = `${basePath}/${formSubmissionId}/email`; describe('POST', () => { const submissionResult = { form: { id: '' }, submission: { id: '' }, version: { id: '' } }; @@ -259,7 +263,7 @@ describe(`${basePath}/:formSubmissionId/email`, () => { }); describe(`${basePath}/:formSubmissionId/:formId/submissions`, () => { - const path = `${basePath}/:formSubmissionId/:formId/submissions`; + const path = `${basePath}/${formSubmissionId}/${formId}/submissions`; describe('DELETE', () => { it('should return 200', async () => { @@ -299,7 +303,7 @@ describe(`${basePath}/:formSubmissionId/:formId/submissions`, () => { }); describe(`${basePath}/:formSubmissionId/:formId/submissions/restore`, () => { - const path = `${basePath}/:formSubmissionId/:formId/submissions/restore`; + const path = `${basePath}/${formSubmissionId}/${formId}/submissions/restore`; describe('PUT', () => { it('should return 200', async () => { @@ -339,7 +343,7 @@ describe(`${basePath}/:formSubmissionId/:formId/submissions/restore`, () => { }); describe(`${basePath}/:formSubmissionId/notes`, () => { - const path = `${basePath}/:formSubmissionId/notes`; + const path = `${basePath}/${formSubmissionId}/notes`; describe('GET', () => { it('should return 200', async () => { @@ -416,7 +420,7 @@ describe(`${basePath}/:formSubmissionId/notes`, () => { }); describe(`${basePath}/:formSubmissionId/options`, () => { - const path = `${basePath}/:formSubmissionId/options`; + const path = `${basePath}/${formSubmissionId}/options`; describe('GET', () => { it('should return 200', async () => { @@ -456,7 +460,7 @@ describe(`${basePath}/:formSubmissionId/options`, () => { }); describe(`${basePath}/:formSubmissionId/restore`, () => { - const path = `${basePath}/:formSubmissionId/restore`; + const path = `${basePath}/${formSubmissionId}/restore`; describe('PUT', () => { it('should return 200', async () => { @@ -496,7 +500,7 @@ describe(`${basePath}/:formSubmissionId/restore`, () => { }); describe(`${basePath}/:formSubmissionId/status`, () => { - const path = `${basePath}/:formSubmissionId/status`; + const path = `${basePath}/${formSubmissionId}/status`; describe('GET', () => { it('should return 200', async () => { @@ -577,7 +581,7 @@ describe(`${basePath}/:formSubmissionId/status`, () => { }); describe(`${basePath}/:formSubmissionId/template/render`, () => { - const path = `${basePath}/:formSubmissionId/template/render`; + const path = `${basePath}/${formSubmissionId}/template/render`; describe('POST', () => { it('should return 200', async () => {