diff --git a/app/package-lock.json b/app/package-lock.json index a78848312..d0c1738b1 100644 --- a/app/package-lock.json +++ b/app/package-lock.json @@ -13,8 +13,8 @@ "@json2csv/transforms": "^6.1.3", "api-problem": "^9.0.2", "aws-sdk": "^2.1376.0", - "axios": "^0.28.1", - "axios-oauth-client": "^1.5.0", + "axios": "^1.7.4", + "axios-oauth-client": "^2.2.0", "axios-token-interceptor": "^0.2.0", "bytes": "^3.1.2", "compression": "^1.7.4", @@ -2165,11 +2165,11 @@ } }, "node_modules/axios": { - "version": "0.28.1", - "resolved": "https://registry.npmjs.org/axios/-/axios-0.28.1.tgz", - "integrity": "sha512-iUcGA5a7p0mVb4Gm/sy+FSECNkPFT4y7wt6OM/CDpO/OnNCvSs3PoMG8ibrC9jRoGYU0gUK5pXVC4NPXq6lHRQ==", + "version": "1.7.4", + "resolved": "https://registry.npmjs.org/axios/-/axios-1.7.4.tgz", + "integrity": "sha512-DukmaFRnY6AzAALSH4J2M3k6PkaC+MfaAGdEERRWcC9q3/TWQwLpHR8ZRLKTdQ3aBDL64EdluRDjJqKw+BPZEw==", "dependencies": { - "follow-redirects": "^1.15.0", + "follow-redirects": "^1.15.6", "form-data": "^4.0.0", "proxy-from-env": "^1.1.0" } @@ -2188,14 +2188,14 @@ } }, "node_modules/axios-oauth-client": { - "version": "1.5.0", - "resolved": "https://registry.npmjs.org/axios-oauth-client/-/axios-oauth-client-1.5.0.tgz", - "integrity": "sha512-CFuTfK9KdRnDDR6LQlUJ0GNKUZ3tHRSFdKPM9WqeCtUdcuKDgWt9aDFH7Xl87VpUcfNt5qRVl4iHdayqtXVv7g==", - "dependencies": { - "qs": "^6.10.1" - }, + "version": "2.2.0", + "resolved": "https://registry.npmjs.org/axios-oauth-client/-/axios-oauth-client-2.2.0.tgz", + "integrity": "sha512-rRaDtK6CuPs+4lDqpAEc4s0c0/Zc2SF4HjVq5DDUL3FkwE/khlxZfnts3MKu1HANODBJ3YOViZVmevx/OUDS2Q==", "engines": { - "node": ">=10" + "node": ">= 14" + }, + "peerDependencies": { + "axios": "^1.2.1" } }, "node_modules/axios-token-interceptor": { @@ -9436,6 +9436,7 @@ "version": "6.11.1", "resolved": "https://registry.npmjs.org/qs/-/qs-6.11.1.tgz", "integrity": "sha512-0wsrzgTz/kAVIeuxSjnpGC56rzYtr6JT/2BwEvMaPhFIoYa1aGO8LbzuU1R0uUYQkLpWBTOj0l/CLAJB64J6nQ==", + "dev": true, "dependencies": { "side-channel": "^1.0.4" }, @@ -12803,11 +12804,11 @@ } }, "axios": { - "version": "0.28.1", - "resolved": "https://registry.npmjs.org/axios/-/axios-0.28.1.tgz", - "integrity": "sha512-iUcGA5a7p0mVb4Gm/sy+FSECNkPFT4y7wt6OM/CDpO/OnNCvSs3PoMG8ibrC9jRoGYU0gUK5pXVC4NPXq6lHRQ==", + "version": "1.7.4", + "resolved": "https://registry.npmjs.org/axios/-/axios-1.7.4.tgz", + "integrity": "sha512-DukmaFRnY6AzAALSH4J2M3k6PkaC+MfaAGdEERRWcC9q3/TWQwLpHR8ZRLKTdQ3aBDL64EdluRDjJqKw+BPZEw==", "requires": { - "follow-redirects": "^1.15.0", + "follow-redirects": "^1.15.6", "form-data": "^4.0.0", "proxy-from-env": "^1.1.0" } @@ -12823,12 +12824,10 @@ } }, "axios-oauth-client": { - "version": "1.5.0", - "resolved": "https://registry.npmjs.org/axios-oauth-client/-/axios-oauth-client-1.5.0.tgz", - "integrity": "sha512-CFuTfK9KdRnDDR6LQlUJ0GNKUZ3tHRSFdKPM9WqeCtUdcuKDgWt9aDFH7Xl87VpUcfNt5qRVl4iHdayqtXVv7g==", - "requires": { - "qs": "^6.10.1" - } + "version": "2.2.0", + "resolved": "https://registry.npmjs.org/axios-oauth-client/-/axios-oauth-client-2.2.0.tgz", + "integrity": "sha512-rRaDtK6CuPs+4lDqpAEc4s0c0/Zc2SF4HjVq5DDUL3FkwE/khlxZfnts3MKu1HANODBJ3YOViZVmevx/OUDS2Q==", + "requires": {} }, "axios-token-interceptor": { "version": "0.2.0", @@ -18223,6 +18222,7 @@ "version": "6.11.1", "resolved": "https://registry.npmjs.org/qs/-/qs-6.11.1.tgz", "integrity": "sha512-0wsrzgTz/kAVIeuxSjnpGC56rzYtr6JT/2BwEvMaPhFIoYa1aGO8LbzuU1R0uUYQkLpWBTOj0l/CLAJB64J6nQ==", + "dev": true, "requires": { "side-channel": "^1.0.4" } diff --git a/app/package.json b/app/package.json index 3553d38fe..25d34d8e7 100644 --- a/app/package.json +++ b/app/package.json @@ -51,8 +51,8 @@ "@json2csv/transforms": "^6.1.3", "api-problem": "^9.0.2", "aws-sdk": "^2.1376.0", - "axios": "^0.28.1", - "axios-oauth-client": "^1.5.0", + "axios": "^1.7.4", + "axios-oauth-client": "^2.2.0", "axios-token-interceptor": "^0.2.0", "bytes": "^3.1.2", "compression": "^1.7.4", diff --git a/app/src/components/clientConnection.js b/app/src/components/clientConnection.js index 6b33853a0..60b2bdc57 100755 --- a/app/src/components/clientConnection.js +++ b/app/src/components/clientConnection.js @@ -4,6 +4,22 @@ const tokenProvider = require('axios-token-interceptor'); const log = require('./log')(module.filename); +// axios-oauth-client removed the "interceptor" in v2.2.0. Replicate it here. + +const getMaxAge = (res) => { + return res.expires_in * 1e3; +}; + +const headerFormatter = (res) => { + return 'Bearer ' + res.access_token; +}; + +const interceptor = (tokenProvider, authenticate) => { + const getToken = tokenProvider.tokenCache(authenticate, { getMaxAge }); + + return tokenProvider({ getToken, headerFormatter }); +}; + class ClientConnection { constructor({ tokenUrl, clientId, clientSecret }) { log.verbose(`Constructed with ${tokenUrl}, ${clientId}, clientSecret`, { function: 'constructor' }); @@ -19,15 +35,7 @@ class ClientConnection { // Wraps axios-token-interceptor with oauth-specific configuration, // fetches the token using the desired claim method, and caches // until the token expires - oauth.interceptor( - tokenProvider, - oauth.client(axios.create(), { - url: this.tokenUrl, - grant_type: 'client_credentials', - client_id: clientId, - client_secret: clientSecret, - }) - ) + interceptor(tokenProvider, oauth.clientCredentials(axios.create(), tokenUrl, clientId, clientSecret)) ); } } 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); + }); +});