From 6d573247a8beb6e63e55f318627475a23ff5a02a Mon Sep 17 00:00:00 2001 From: Walter Moar Date: Thu, 15 Aug 2024 15:28:28 -0700 Subject: [PATCH] test: FORMS-1444 add public and utils routes tests (#1477) * test: FORMS-1444 add public and utils routes * for consistency check userAccess.currentUser * docs: add some thoughts around the route testing * docs: added brief notes on middleware testing * refactor: updated the proxy routes tests for consistency --- app/tests/unit/README.md | 33 ++++++++ .../forms/form/externalApi/routes.spec.js | 5 ++ app/tests/unit/forms/form/routes.spec.js | 39 ++++++++++ app/tests/unit/forms/proxy/routes.spec.js | 5 +- app/tests/unit/forms/public/routes.spec.js | 77 +++++++++++++++++++ app/tests/unit/forms/user/routes.spec.js | 10 +++ app/tests/unit/forms/utils/routes.spec.js | 43 +++++++++++ 7 files changed, 209 insertions(+), 3 deletions(-) create mode 100644 app/tests/unit/forms/public/routes.spec.js create mode 100644 app/tests/unit/forms/utils/routes.spec.js diff --git a/app/tests/unit/README.md b/app/tests/unit/README.md index 8bc172937..1baf80012 100644 --- a/app/tests/unit/README.md +++ b/app/tests/unit/README.md @@ -53,3 +53,36 @@ describe('my tests', () => { ``` Similar to `.only` is the `.skip` modifier to skip a test or group of tests. + +## Testing Strategy + +The testing strategy for the backend unit tests can be broken down into the different layers of the backend. For all tests we should: + +- ensure that the tests are consistent +- ensure that we have 100% test coverage +- ensure that we have complete test coverage: we should be testing additional corner cases even once we reach 100% test coverage +- test the interface, not the implementation +- test the unit under test, not its dependencies + +### Middleware Testing + +The tests for the middleware files should: + +- mock all services calls used by the middleware, including both exception and minimal valid results +- test all response codes produced by the middleware + +### Route Testing + +The tests for the `route.js` files should: + +- mock all middleware used by the file +- each route test should check that every middleware is called the proper number of times +- each route test should mock the controller function that it calls +- mock controller functions with `res.sendStatus(200)`, as doing something like `next()` will call multiple controller functions when route paths are overloaded +- check that the mocked controller function is called - this will catch when a new route path accidentally overloads an old one +- for consistency and ease of comparison, alphabetize the expect clauses ("alphabetize when possible") + +Note: + +- Some middleware is called when the `routes.js` is loaded, not when the route is called. For example, the parameters to `userAccess.hasFormPermissions` cannot be tested by calling the route using it. Even if we reload the `routes.js` for each route test, it would be hard to tell which call to `hasFormPermissions` was the call for the route under test +- Maybe we should refactor and create a set of standard middleware mocks that live outside the `routes.spec.js` files diff --git a/app/tests/unit/forms/form/externalApi/routes.spec.js b/app/tests/unit/forms/form/externalApi/routes.spec.js index bc8e90451..d31ac02ef 100644 --- a/app/tests/unit/forms/form/externalApi/routes.spec.js +++ b/app/tests/unit/forms/form/externalApi/routes.spec.js @@ -83,6 +83,7 @@ describe(`${basePath}/:formId/externalAPIs`, () => { expect(controller.listExternalAPIs).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateExternalAPIId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); }); @@ -98,6 +99,7 @@ describe(`${basePath}/:formId/externalAPIs`, () => { expect(controller.createExternalAPI).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateExternalAPIId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); }); @@ -125,6 +127,7 @@ describe(`${basePath}/:formId/externalAPIs/:externalAPIId`, () => { expect(controller.deleteExternalAPI).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateExternalAPIId).toBeCalledTimes(1); expect(validateParameter.validateFormId).toBeCalledTimes(1); }); @@ -152,6 +155,7 @@ describe(`${basePath}/:formId/externalAPIs/:externalAPIId`, () => { expect(controller.updateExternalAPI).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateExternalAPIId).toBeCalledTimes(1); expect(validateParameter.validateFormId).toBeCalledTimes(1); }); @@ -219,6 +223,7 @@ describe(`${basePath}/:formId/externalAPIs/statusCodes`, () => { expect(controller.listExternalAPIStatusCodes).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateExternalAPIId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); }); diff --git a/app/tests/unit/forms/form/routes.spec.js b/app/tests/unit/forms/form/routes.spec.js index 6189ba661..1ffae78ba 100644 --- a/app/tests/unit/forms/form/routes.spec.js +++ b/app/tests/unit/forms/form/routes.spec.js @@ -82,6 +82,7 @@ describe(`${basePath}`, () => { expect(controller.listForms).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(0); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -99,6 +100,7 @@ describe(`${basePath}`, () => { expect(controller.createForm).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(0); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -121,6 +123,7 @@ describe(`${basePath}/:formId`, () => { expect(controller.deleteForm).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -138,6 +141,7 @@ describe(`${basePath}/:formId`, () => { expect(controller.readForm).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -155,6 +159,7 @@ describe(`${basePath}/:formId`, () => { expect(controller.updateForm).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -177,6 +182,7 @@ describe(`${basePath}/:formId/apiKey`, () => { expect(controller.deleteApiKey).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -194,6 +200,7 @@ describe(`${basePath}/:formId/apiKey`, () => { expect(controller.readApiKey).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -211,6 +218,7 @@ describe(`${basePath}/:formId/apiKey`, () => { expect(controller.createOrReplaceApiKey).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -233,6 +241,7 @@ describe(`${basePath}/:formId/apiKey/filesApiAccess`, () => { expect(controller.filesApiKeyAccess).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -255,6 +264,7 @@ describe(`${basePath}/:formId/csvexport/fields`, () => { expect(controller.readFieldsForCSVExport).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -277,6 +287,7 @@ describe(`${basePath}/:formId/documentTemplates`, () => { expect(controller.documentTemplateList).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -294,6 +305,7 @@ describe(`${basePath}/:formId/documentTemplates`, () => { expect(controller.documentTemplateCreate).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -317,6 +329,7 @@ describe(`${basePath}/:formId/documentTemplates/:documentTemplateId`, () => { expect(controller.documentTemplateDelete).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(1); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -334,6 +347,7 @@ describe(`${basePath}/:formId/documentTemplates/:documentTemplateId`, () => { expect(controller.documentTemplateRead).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(1); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -356,6 +370,7 @@ describe(`${basePath}/:formId/drafts`, () => { expect(controller.listDrafts).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -373,6 +388,7 @@ describe(`${basePath}/:formId/drafts`, () => { expect(controller.createDraft).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -396,6 +412,7 @@ describe(`${basePath}/:formId/drafts/:formVersionDraftId`, () => { expect(controller.deleteDraft).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(1); @@ -413,6 +430,7 @@ describe(`${basePath}/:formId/drafts/:formVersionDraftId`, () => { expect(controller.readDraft).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(1); @@ -430,6 +448,7 @@ describe(`${basePath}/:formId/drafts/:formVersionDraftId`, () => { expect(controller.updateDraft).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(1); @@ -453,6 +472,7 @@ describe(`${basePath}/:formId/drafts/:formVersionDraftId/publish`, () => { expect(controller.publishDraft).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(1); @@ -475,6 +495,7 @@ describe(`${basePath}/:formId/emailTemplate`, () => { expect(controller.createOrUpdateEmailTemplate).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -497,6 +518,7 @@ describe(`${basePath}/:formId/emailTemplates`, () => { expect(controller.readEmailTemplates).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -519,6 +541,7 @@ describe(`${basePath}/:formId/export`, () => { expect(controller.export).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -541,6 +564,7 @@ describe(`${basePath}/:formId/export/fields`, () => { expect(controller.exportWithFields).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -563,6 +587,7 @@ describe(`${basePath}/:formId/options`, () => { expect(controller.readFormOptions).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -585,6 +610,7 @@ describe(`${basePath}/:formId/statusCodes`, () => { expect(controller.getStatusCodes).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -607,6 +633,7 @@ describe(`${basePath}/:formId/submissions`, () => { expect(controller.listFormSubmissions).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -629,6 +656,7 @@ describe(`${basePath}/:formId/subscriptions`, () => { expect(controller.readFormSubscriptionDetails).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -646,6 +674,7 @@ describe(`${basePath}/:formId/subscriptions`, () => { expect(controller.createOrUpdateSubscriptionDetails).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -668,6 +697,7 @@ describe(`${basePath}/:formId/version`, () => { expect(controller.readPublishedForm).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -691,6 +721,7 @@ describe(`${basePath}/:formId/versions/:formVersionId`, () => { expect(controller.readVersion).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -714,6 +745,7 @@ describe(`${basePath}/:formId/versions/:formVersionId/fields`, () => { expect(controller.readVersionFields).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -737,6 +769,7 @@ describe(`${basePath}/:formId/versions/:formVersionId/multiSubmission`, () => { expect(controller.createMultiSubmission).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -760,6 +793,7 @@ describe(`${basePath}/:formId/versions/:formVersionId/publish`, () => { expect(controller.publishVersion).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -783,6 +817,7 @@ describe(`${basePath}/:formId/versions/:formVersionId/submissions`, () => { expect(controller.listSubmissions).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -800,6 +835,7 @@ describe(`${basePath}/:formId/versions/:formVersionId/submissions`, () => { expect(controller.createSubmission).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -823,6 +859,7 @@ describe(`${basePath}/:formId/versions/:formVersionId/submissions/discover`, () expect(controller.listSubmissionFields).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -845,6 +882,7 @@ describe(`${basePath}/formcomponents/proactivehelp/imageUrl/:componentId`, () => expect(controller.getFCProactiveHelpImageUrl).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(0); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); @@ -866,6 +904,7 @@ describe(`${basePath}/formcomponents/proactivehelp/list`, () => { expect(controller.listFormComponentsProactiveHelp).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); expect(validateParameter.validateFormId).toBeCalledTimes(0); expect(validateParameter.validateFormVersionDraftId).toBeCalledTimes(0); diff --git a/app/tests/unit/forms/proxy/routes.spec.js b/app/tests/unit/forms/proxy/routes.spec.js index 2313f1a2d..dc7bb9bd4 100644 --- a/app/tests/unit/forms/proxy/routes.spec.js +++ b/app/tests/unit/forms/proxy/routes.spec.js @@ -6,15 +6,15 @@ jest.mock('cors', () => }) ); -const request = require('supertest'); const Problem = require('api-problem'); +const request = require('supertest'); const { expressHelper } = require('../../../common/helper'); const jwtService = require('../../../../src/components/jwtService'); const apiAccess = require('../../../../src/forms/auth/middleware/apiAccess'); -const rateLimiter = require('../../../../src/forms/common/middleware/rateLimiter'); const userAccess = require('../../../../src/forms/auth/middleware/userAccess'); +const rateLimiter = require('../../../../src/forms/common/middleware/rateLimiter'); // // Mock out all the middleware - we're testing that the routes are set up @@ -49,7 +49,6 @@ const service = require('../../../../src/forms/proxy/service'); // const router = require('../../../../src/forms/proxy/routes'); - const basePath = '/proxy'; const app = expressHelper(basePath, router); const appRequest = request(app); diff --git a/app/tests/unit/forms/public/routes.spec.js b/app/tests/unit/forms/public/routes.spec.js new file mode 100644 index 000000000..504a09a21 --- /dev/null +++ b/app/tests/unit/forms/public/routes.spec.js @@ -0,0 +1,77 @@ +const request = require('supertest'); +const uuid = require('uuid'); + +const { expressHelper } = require('../../../common/helper'); + +const controller = require('../../../../src/forms/public/controller'); + +// +// Create the router and a simple Express server. +// + +const router = require('../../../../src/forms/public/routes'); +const basePath = '/public'; +const app = expressHelper(basePath, router); +const appRequest = request(app); + +afterEach(() => { + jest.clearAllMocks(); +}); + +// TODO - move code out of the route and into middleware or the controller. The +// tests should continue to pass, but should themselves be moved into the +// middleware or controller tests. + +describe(`${basePath}/reminder`, () => { + const path = `${basePath}/reminder`; + + controller.sendReminderToSubmitter = jest.fn((_req, res) => { + res.sendStatus(200); + }); + + it('200s when the APITOKEN matches apiKey', async () => { + process.env.APITOKEN = uuid.v4(); + + const response = await appRequest.get(path).set({ apikey: process.env.APITOKEN }); + + expect(controller.sendReminderToSubmitter).toBeCalledTimes(1); + expect(response.status).toBe(200); + }); + + // This is testing some strange code. Refactor the code and get rid of this. + it('401s when there is no APITOKEN or apiKey', async () => { + process.env.APITOKEN = ''; + + const response = await appRequest.get(path).set({ apikey: process.env.APITOKEN }); + + expect(controller.sendReminderToSubmitter).toBeCalledTimes(0); + expect(response.status).toBe(401); + }); + + it('401s when the apiKey is empty', async () => { + process.env.APITOKEN = uuid.v4(); + + const response = await appRequest.get(path).set({ apikey: '' }); + + expect(controller.sendReminderToSubmitter).toBeCalledTimes(0); + expect(response.status).toBe(401); + }); + + it('401s when the apiKey exists but does not match', async () => { + process.env.APITOKEN = uuid.v4(); + + const response = await appRequest.get(path).set({ apikey: uuid.v4() }); + + expect(controller.sendReminderToSubmitter).toBeCalledTimes(0); + expect(response.status).toBe(401); + }); + + // Ideally the code should be refactored to not create the 404 and let Express + // do it. Then this test should still pass. + it('404s on a non-GET', async () => { + const response = await appRequest.post(path); + + expect(controller.sendReminderToSubmitter).toBeCalledTimes(0); + expect(response.status).toBe(404); + }); +}); diff --git a/app/tests/unit/forms/user/routes.spec.js b/app/tests/unit/forms/user/routes.spec.js index 02d096ff8..f867d0fbf 100644 --- a/app/tests/unit/forms/user/routes.spec.js +++ b/app/tests/unit/forms/user/routes.spec.js @@ -54,6 +54,7 @@ describe(`${basePath}`, () => { await appRequest.get(path); expect(controller.list).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateFormId).toBeCalledTimes(0); expect(validateParameter.validateUserId).toBeCalledTimes(0); }); @@ -71,6 +72,7 @@ describe(`${basePath}/:userId`, () => { await appRequest.get(path); expect(controller.read).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateFormId).toBeCalledTimes(0); expect(validateParameter.validateUserId).toBeCalledTimes(1); }); @@ -87,6 +89,7 @@ describe(`${basePath}/labels`, () => { await appRequest.get(path); expect(controller.readUserLabels).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateFormId).toBeCalledTimes(0); expect(validateParameter.validateUserId).toBeCalledTimes(0); }); @@ -99,6 +102,7 @@ describe(`${basePath}/labels`, () => { await appRequest.put(path); expect(controller.updateUserLabels).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateFormId).toBeCalledTimes(0); expect(validateParameter.validateUserId).toBeCalledTimes(0); }); @@ -115,6 +119,7 @@ describe(`${basePath}/preferences`, () => { await appRequest.delete(path); expect(controller.deleteUserPreferences).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateFormId).toBeCalledTimes(0); expect(validateParameter.validateUserId).toBeCalledTimes(0); }); @@ -127,6 +132,7 @@ describe(`${basePath}/preferences`, () => { await appRequest.get(path); expect(controller.readUserPreferences).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateFormId).toBeCalledTimes(0); expect(validateParameter.validateUserId).toBeCalledTimes(0); }); @@ -139,6 +145,7 @@ describe(`${basePath}/preferences`, () => { await appRequest.put(path); expect(controller.updateUserPreferences).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateFormId).toBeCalledTimes(0); expect(validateParameter.validateUserId).toBeCalledTimes(0); }); @@ -156,6 +163,7 @@ describe(`${basePath}/preferences/forms/:formId`, () => { await appRequest.delete(path); expect(controller.deleteUserFormPreferences).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateUserId).toBeCalledTimes(0); }); @@ -168,6 +176,7 @@ describe(`${basePath}/preferences/forms/:formId`, () => { await appRequest.get(path); expect(controller.readUserFormPreferences).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateUserId).toBeCalledTimes(0); }); @@ -180,6 +189,7 @@ describe(`${basePath}/preferences/forms/:formId`, () => { await appRequest.put(path); expect(controller.updateUserFormPreferences).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateUserId).toBeCalledTimes(0); }); diff --git a/app/tests/unit/forms/utils/routes.spec.js b/app/tests/unit/forms/utils/routes.spec.js new file mode 100644 index 000000000..e7dd6b100 --- /dev/null +++ b/app/tests/unit/forms/utils/routes.spec.js @@ -0,0 +1,43 @@ +const request = require('supertest'); + +const { expressHelper } = require('../../../common/helper'); + +const userAccess = require('../../../../src/forms/auth/middleware/userAccess'); +const controller = require('../../../../src/forms/submission/controller'); + +// +// Mock out all the middleware - we're testing that the routes are set up +// correctly, not the functionality of the middleware. +// + +userAccess.currentUser = jest.fn((_req, _res, next) => { + next(); +}); + +// +// Create the router and a simple Express server. +// + +const router = require('../../../../src/forms/utils/routes'); +const basePath = '/utils'; +const app = expressHelper(basePath, router); +const appRequest = request(app); + +afterEach(() => { + jest.clearAllMocks(); +}); + +describe(`${basePath}/template/render`, () => { + const path = `${basePath}/template/render`; + + it('should have correct middleware for POST', async () => { + controller.draftTemplateUploadAndRender = jest.fn((_req, res) => { + res.sendStatus(200); + }); + + await appRequest.post(path); + + expect(controller.draftTemplateUploadAndRender).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); + }); +});