diff --git a/app/src/forms/admin/controller.js b/app/src/forms/admin/controller.js index a3dc39d02..d25b1ea66 100644 --- a/app/src/forms/admin/controller.js +++ b/app/src/forms/admin/controller.js @@ -100,7 +100,7 @@ module.exports = { }, updateExternalAPI: async (req, res, next) => { try { - const response = await service.updateExternalAPI(req.params.id, req.body); + const response = await service.updateExternalAPI(req.params.externalApiId, req.body); res.status(200).json(response); } catch (error) { next(error); diff --git a/app/src/forms/admin/routes.js b/app/src/forms/admin/routes.js index 0a1698232..638075ac4 100644 --- a/app/src/forms/admin/routes.js +++ b/app/src/forms/admin/routes.js @@ -1,22 +1,28 @@ const routes = require('express').Router(); +const jwtService = require('../../components/jwtService'); const currentUser = require('../auth/middleware/userAccess').currentUser; - -const controller = require('./controller'); +const validateParameter = require('../common/middleware/validateParameter'); const userController = require('../user/controller'); -const jwtService = require('../../components/jwtService'); +const controller = require('./controller'); -// Always have this applied to all routes here +// Routes under /admin fetch data without doing form permission checks. All +// routes in this file should remain under the "admin" role check, with the +// "admin" role only given to people who have permission to read all data. routes.use(jwtService.protect('admin')); + routes.use(currentUser); -// Routes under the /admin pathing will fetch data without doing Form permission checks in the database -// As such, this should ALWAYS remain under the :admin role check and that KC role should not be given out -// other than to people who have permission to read all data +routes.param('componentId', validateParameter.validateComponentId); +routes.param('externalApiId', validateParameter.validateExternalAPIId); +routes.param('formId', validateParameter.validateFormId); +routes.param('formVersionId', validateParameter.validateFormVersionId); +routes.param('userId', validateParameter.validateUserId); // // Forms // + routes.get('/forms', async (req, res, next) => { await controller.listForms(req, res, next); }); @@ -52,6 +58,7 @@ routes.put('/forms/:formId/addUser', async (req, res, next) => { // // Users // + routes.get('/users', async (req, res, next) => { await controller.getUsers(req, res, next); }); @@ -61,21 +68,23 @@ routes.get('/users/:userId', async (req, res, next) => { }); // -// APIs +// External APIs // + routes.get('/externalAPIs', async (req, res, next) => { await controller.getExternalAPIs(req, res, next); }); -routes.put('/externalAPIs/:id', async (req, res, next) => { +routes.put('/externalAPIs/:externalApiId', async (req, res, next) => { await controller.updateExternalAPI(req, res, next); }); routes.get('/externalAPIs/statusCodes', async (req, res, next) => { await controller.getExternalAPIStatusCodes(req, res, next); }); + // -//Form componets help info +// Form Components Help // routes.post('/formcomponents/proactivehelp/object', async (req, res, next) => { diff --git a/app/src/forms/common/constants.js b/app/src/forms/common/constants.js index fc5641143..7d377bedf 100644 --- a/app/src/forms/common/constants.js +++ b/app/src/forms/common/constants.js @@ -14,38 +14,38 @@ module.exports = Object.freeze({ REMINDER_FORM_NOT_FILL: 'formNotFill', }, Permissions: { + DESIGN_CREATE: 'design_create', + DESIGN_DELETE: 'design_delete', + DESIGN_READ: 'design_read', + DESIGN_UPDATE: 'design_update', DOCUMENT_TEMPLATE_CREATE: 'document_template_create', DOCUMENT_TEMPLATE_DELETE: 'document_template_delete', DOCUMENT_TEMPLATE_READ: 'document_template_read', EMAIL_TEMPLATE_READ: 'email_template_read', EMAIL_TEMPLATE_UPDATE: 'email_template_update', FORM_API_CREATE: 'form_api_create', + FORM_API_DELETE: 'form_api_delete', FORM_API_READ: 'form_api_read', FORM_API_UPDATE: 'form_api_update', - FORM_API_DELETE: 'form_api_delete', + FORM_DELETE: 'form_delete', FORM_READ: 'form_read', + FORM_SUBMITTER: ['form_read', 'submission_create'], FORM_UPDATE: 'form_update', - FORM_DELETE: 'form_delete', SUBMISSION_CREATE: 'submission_create', + SUBMISSION_DELETE: 'submission_delete', SUBMISSION_READ: 'submission_read', SUBMISSION_REVIEW: 'submission_review', SUBMISSION_UPDATE: 'submission_update', - SUBMISSION_DELETE: 'submission_delete', - DESIGN_CREATE: 'design_create', - DESIGN_READ: 'design_read', - DESIGN_UPDATE: 'design_update', - DESIGN_DELETE: 'design_delete', TEAM_READ: 'team_read', TEAM_UPDATE: 'team_update', - FORM_SUBMITTER: ['form_read', 'submission_create'], }, Roles: { - OWNER: 'owner', - TEAM_MANAGER: 'team_manager', FORM_DESIGNER: 'form_designer', + FORM_SUBMITTER: 'form_submitter', + OWNER: 'owner', SUBMISSION_APPROVER: 'submission_approver', SUBMISSION_REVIEWER: 'submission_reviewer', - FORM_SUBMITTER: 'form_submitter', + TEAM_MANAGER: 'team_manager', }, Regex: { CONFIRMATION_ID: '^[0-9A-Fa-f]{8}$', diff --git a/app/src/forms/common/middleware/validateParameter.js b/app/src/forms/common/middleware/validateParameter.js index baeda9e20..bd1712872 100644 --- a/app/src/forms/common/middleware/validateParameter.js +++ b/app/src/forms/common/middleware/validateParameter.js @@ -1,10 +1,11 @@ const Problem = require('api-problem'); const uuid = require('uuid'); -const encryptionKeyService = require('../../form/encryptionKey/service'); +const constants = require('../../common/constants'); const externalApiService = require('../../form/externalApi/service'); const formService = require('../../form/service'); const submissionService = require('../../submission/service'); +const encryptionKeyService = require('../../form/encryptionKey/service'); /** * Throws a 400 problem if the parameter is not a valid UUID. @@ -21,6 +22,24 @@ const _validateUuid = (parameter, parameterName) => { } }; +/** + * Validates that the :componentId 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 {*} componentId the :componentId value from the route. + */ +const validateComponentId = async (_req, _res, next, componentId) => { + try { + _validateUuid(componentId, 'componentId'); + + next(); + } catch (error) { + next(error); + } +}; + /** * Validates that the :documentTemplateId route parameter exists and is a UUID. * This validator requires that either the :formId or :formSubmissionId route @@ -193,6 +212,50 @@ const validateFormVersionId = async (req, _res, next, formVersionId) => { } }; +/** + * Validates that the :code route parameter for permissions is valid. + * + * @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 {*} code the :code value from the route. + */ +const validatePermissionCode = async (_req, _res, next, code) => { + try { + if (!Object.values(constants.Permissions).includes(code)) { + throw new Problem(400, { + detail: 'Bad permission code', + }); + } + + next(); + } catch (error) { + next(error); + } +}; + +/** + * Validates that the :code route parameter for roles is valid. + * + * @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 {*} code the :code value from the route. + */ +const validateRoleCode = async (_req, _res, next, code) => { + try { + if (!Object.values(constants.Roles).includes(code)) { + throw new Problem(400, { + detail: 'Bad role code', + }); + } + + next(); + } catch (error) { + next(error); + } +}; + /** * Validates that the :userId route parameter exists and is a UUID. * @@ -236,6 +299,7 @@ const validateFormEncryptionKeyId = async (req, _res, next, formEncryptionKeyId) }; module.exports = { + validateComponentId, validateDocumentTemplateId, validateExternalAPIId, validateFileId, @@ -243,6 +307,8 @@ module.exports = { validateFormSubmissionId, validateFormVersionDraftId, validateFormVersionId, + validatePermissionCode, + validateRoleCode, validateUserId, validateFormEncryptionKeyId, }; diff --git a/app/src/forms/file/routes.js b/app/src/forms/file/routes.js index ea3faf36a..0f302a56c 100644 --- a/app/src/forms/file/routes.js +++ b/app/src/forms/file/routes.js @@ -1,13 +1,13 @@ const routes = require('express').Router(); -const controller = require('./controller'); -const apiAccess = require('../auth/middleware/apiAccess'); -const validateParameter = require('../common/middleware/validateParameter'); +const apiAccess = require('../auth/middleware/apiAccess'); +const { currentUser } = 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'); const { currentFileRecord, hasFileCreate, hasFilePermissions } = require('./middleware/filePermissions'); const fileUpload = require('./middleware/upload').fileUpload; -const { currentUser } = require('../auth/middleware/userAccess'); -const rateLimiter = require('../common/middleware').apiKeyRateLimiter; routes.use(currentUser); diff --git a/app/src/forms/form/routes.js b/app/src/forms/form/routes.js index 0dfff16d0..7c2a8deaa 100644 --- a/app/src/forms/form/routes.js +++ b/app/src/forms/form/routes.js @@ -1,11 +1,11 @@ const routes = require('express').Router(); + +const jwtService = require('../../components/jwtService'); const apiAccess = require('../auth/middleware/apiAccess'); const { currentUser, hasFormPermissions } = require('../auth/middleware/userAccess'); -const validateParameter = require('../common/middleware/validateParameter'); const P = require('../common/constants').Permissions; const rateLimiter = require('../common/middleware').apiKeyRateLimiter; - -const jwtService = require('../../components/jwtService'); +const validateParameter = require('../common/middleware/validateParameter'); const controller = require('./controller'); routes.use(currentUser); diff --git a/app/src/forms/permission/routes.js b/app/src/forms/permission/routes.js index 83627e4ff..02a43e39f 100644 --- a/app/src/forms/permission/routes.js +++ b/app/src/forms/permission/routes.js @@ -1,13 +1,15 @@ const routes = require('express').Router(); +const jwtService = require('../../components/jwtService'); const currentUser = require('../auth/middleware/userAccess').currentUser; - +const validateParameter = require('../common/middleware/validateParameter'); const controller = require('./controller'); -const jwtService = require('../../components/jwtService'); routes.use(jwtService.protect('admin')); routes.use(currentUser); +routes.param('code', validateParameter.validatePermissionCode); + routes.get('/', async (req, res, next) => { await controller.list(req, res, next); }); diff --git a/app/src/forms/proxy/routes.js b/app/src/forms/proxy/routes.js index e48e2475c..9b4043e88 100644 --- a/app/src/forms/proxy/routes.js +++ b/app/src/forms/proxy/routes.js @@ -1,21 +1,20 @@ const cors = require('cors'); +const routes = require('express').Router(); const { currentUser } = require('../auth/middleware/userAccess'); const controller = require('./controller'); -const routes = require('express').Router(); - // need to allow cors for OPTIONS call // formio component will call OPTIONS pre-flight routes.options('/external', cors()); // called with encrypted headers, no current user!!! -routes.get('/external', cors(), async (_req, res, next) => { - await controller.callExternalApi(_req, res, next); +routes.get('/external', cors(), async (req, res, next) => { + await controller.callExternalApi(req, res, next); }); -routes.post('/headers', currentUser, async (_req, res, next) => { - await controller.generateProxyHeaders(_req, res, next); +routes.post('/headers', currentUser, async (req, res, next) => { + await controller.generateProxyHeaders(req, res, next); }); module.exports = routes; diff --git a/app/src/forms/proxy/service.js b/app/src/forms/proxy/service.js index f6cd334a1..c624fc738 100644 --- a/app/src/forms/proxy/service.js +++ b/app/src/forms/proxy/service.js @@ -1,7 +1,7 @@ const { encryptionService } = require('../../components/encryptionService'); const jwtService = require('../../components/jwtService'); const ProxyServiceError = require('./error'); -const { ExternalAPI } = require('../../forms/common/models'); +const { ExternalAPI, SubmissionMetadata } = require('../../forms/common/models'); const headerValue = (headers, key) => { if (headers && key) { @@ -19,15 +19,38 @@ const trimLeadingSlashes = (str) => str.replace(/^\/+|\$/g, ''); const trimTrailingSlashes = (str) => str.replace(/\/+$/g, ''); const service = { + _getIds: async (payload) => { + let formId = payload['formId']; + let versionId = payload['versionId']; + let submissionId = payload['submissionId']; + // when we are provided with a submission id (ex. submitting a draft submission) + // we need to fetch the related form and version id (if not provided) + if (submissionId) { + const meta = await SubmissionMetadata.query().where('submissionId', submissionId).first(); + if (meta) { + formId = meta.formId; + versionId = meta.formVersionId; + submissionId = meta.submissionId; + } + } + return { + formId: formId, + versionId: versionId, + submissionId: submissionId, + }; + }, + generateProxyHeaders: async (payload, currentUser, token) => { if (!payload || !currentUser || !currentUser.idp) { throw new ProxyServiceError('Cannot generate proxy headers with missing or incomplete parameters'); } + const { formId, versionId, submissionId } = await service._getIds(payload); + const headerData = { - formId: payload['formId'], - versionId: payload['versionId'], - submissionId: payload['submissionId'], + formId: formId, + versionId: versionId, + submissionId: submissionId, userId: currentUser.idpUserId, username: currentUser.username, firstName: currentUser.firstName, diff --git a/app/src/forms/public/middleware/apiAccess.js b/app/src/forms/public/middleware/apiAccess.js new file mode 100644 index 000000000..478d4a3ef --- /dev/null +++ b/app/src/forms/public/middleware/apiAccess.js @@ -0,0 +1,35 @@ +const Problem = require('api-problem'); + +/** + * Checks that the API Key in the request headers matches the API Key in the + * process environment variables. + * + * @param {*} req the Express object representing the HTTP request. + * @param {*} _res the Express object representing the HTTP response - unused. + * @param {*} next the Express chaining function. + */ +const checkApiKey = async (req, _res, next) => { + try { + const requestApikey = req.headers.apikey; + if (requestApikey === undefined || requestApikey === '') { + throw new Problem(401, { + detail: 'No API key provided', + }); + } + + const systemApikey = process.env.APITOKEN; + if (requestApikey !== systemApikey) { + throw new Problem(401, { + detail: 'Invalid API key', + }); + } + + next(); + } catch (error) { + next(error); + } +}; + +module.exports = { + checkApiKey, +}; diff --git a/app/src/forms/public/routes.js b/app/src/forms/public/routes.js index c72618a40..431c235c3 100644 --- a/app/src/forms/public/routes.js +++ b/app/src/forms/public/routes.js @@ -1,27 +1,9 @@ const routes = require('express').Router(); -const controller = require('./controller'); -routes.use('/reminder', (req, res, next) => { - // eslint-disable-next-line no-empty - try { - if (req.method == 'GET') { - const apikeyEnv = process.env.APITOKEN; - const apikeyIncome = req.headers.apikey; - if (apikeyEnv == apikeyIncome && (apikeyIncome == undefined || apikeyIncome == '')) return res.status(401).json({ message: 'No API key provided' }); - if (apikeyIncome === apikeyEnv) { - next(); - } else { - return res.status(401).json({ message: 'Invalid API key' }); - } - } else { - return res.status(404).json({ message: 'Only GET request is accepted' }); - } - } catch (err) { - return res.status(500).json({ message: err.message }); - } -}); +const apiAccess = require('./middleware/apiAccess'); +const controller = require('./controller'); -routes.get('/reminder', async (req, res, next) => { +routes.get('/reminder', apiAccess.checkApiKey, async (req, res, next) => { await controller.sendReminderToSubmitter(req, res, next); }); diff --git a/app/src/forms/rbac/routes.js b/app/src/forms/rbac/routes.js index aacb85618..a2df13f4e 100644 --- a/app/src/forms/rbac/routes.js +++ b/app/src/forms/rbac/routes.js @@ -1,10 +1,10 @@ const routes = require('express').Router(); -const controller = require('./controller'); const jwtService = require('../../components/jwtService'); +const { currentUser, hasFormPermissions, hasFormRoles, hasRoleDeletePermissions, hasRoleModifyPermissions, hasSubmissionPermissions } = require('../auth/middleware/userAccess'); const P = require('../common/constants').Permissions; const R = require('../common/constants').Roles; -const { currentUser, hasFormPermissions, hasFormRoles, hasRoleDeletePermissions, hasRoleModifyPermissions, hasSubmissionPermissions } = require('../auth/middleware/userAccess'); +const controller = require('./controller'); routes.use(currentUser); diff --git a/app/src/forms/role/routes.js b/app/src/forms/role/routes.js index 4bb8c83cd..86540f0b2 100644 --- a/app/src/forms/role/routes.js +++ b/app/src/forms/role/routes.js @@ -1,12 +1,14 @@ const routes = require('express').Router(); +const jwtService = require('../../components/jwtService'); const currentUser = require('../auth/middleware/userAccess').currentUser; - +const validateParameter = require('../common/middleware/validateParameter'); const controller = require('./controller'); -const jwtService = require('../../components/jwtService'); routes.use(currentUser); +routes.param('code', validateParameter.validateRoleCode); + routes.get('/', jwtService.protect(), async (req, res, next) => { await controller.list(req, res, next); }); diff --git a/app/src/forms/submission/routes.js b/app/src/forms/submission/routes.js index f18cab3ce..887d5268b 100644 --- a/app/src/forms/submission/routes.js +++ b/app/src/forms/submission/routes.js @@ -5,7 +5,6 @@ const { currentUser, hasSubmissionPermissions, filterMultipleSubmissions } = req 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); diff --git a/app/src/forms/utils/routes.js b/app/src/forms/utils/routes.js index df3121726..ac7b32ada 100644 --- a/app/src/forms/utils/routes.js +++ b/app/src/forms/utils/routes.js @@ -1,7 +1,7 @@ const routes = require('express').Router(); -const controller = require('../submission/controller'); const { currentUser } = require('../auth/middleware/userAccess'); +const controller = require('../submission/controller'); routes.use(currentUser); diff --git a/app/tests/unit/README.md b/app/tests/unit/README.md index 1baf80012..ae2c15b5f 100644 --- a/app/tests/unit/README.md +++ b/app/tests/unit/README.md @@ -1,6 +1,6 @@ # Unit Tests -The backend unit tests can be run in VSCode by going to `Terminal` -> `Run Task...` -> `Unit Tests - API`. +The backend unit tests are run in VSCode by going to `Terminal` -> `Run Task...` -> `Unit Tests - API`. Once the tests complete, the backend code coverage report is in `/app/coverage/lcov-report/index.html`. ## Running on the Command Line @@ -84,5 +84,6 @@ The tests for the `route.js` files should: 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 +- The order that middleware is called is very important, but we are not testing this. Perhaps integration tests are the best solution for this +- Some middleware takes parameters and is created when a route is created. This means that, for example, the parameters to `jwtService.protect` or `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 create the middleware 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/admin/routes.spec.js b/app/tests/unit/forms/admin/routes.spec.js index 849d19d54..bc886272d 100644 --- a/app/tests/unit/forms/admin/routes.spec.js +++ b/app/tests/unit/forms/admin/routes.spec.js @@ -6,6 +6,7 @@ const { expressHelper } = require('../../../common/helper'); const jwtService = require('../../../../src/components/jwtService'); const userAccess = require('../../../../src/forms/auth/middleware/userAccess'); const controller = require('../../../../src/forms/admin/controller'); +const validateParameter = require('../../../../src/forms/common/middleware/validateParameter'); const userController = require('../../../../src/forms/user/controller'); // @@ -13,9 +14,6 @@ const userController = require('../../../../src/forms/user/controller'); // correctly, not the functionality of the middleware. // -// TODO: Add a test the confirms that jwtService.protect is called with "admin" -// when the file is loaded. - const mockJwtServiceProtect = jest.fn((_req, _res, next) => { next(); }); @@ -27,6 +25,22 @@ userAccess.currentUser = jest.fn((_req, _res, next) => { next(); }); +validateParameter.validateComponentId = jest.fn((_req, _res, next) => { + next(); +}); +validateParameter.validateExternalAPIId = jest.fn((_req, _res, next) => { + next(); +}); +validateParameter.validateFormId = jest.fn((_req, _res, next) => { + next(); +}); +validateParameter.validateFormVersionId = jest.fn((_req, _res, next) => { + next(); +}); +validateParameter.validateUserId = jest.fn((_req, _res, next) => { + next(); +}); + // // Create the router and a simple Express server. // @@ -40,6 +54,25 @@ afterEach(() => { jest.clearAllMocks(); }); +// jwtService.protect is tricky to test. This test is fragile as the file could +// change and have routes that need admin and others that don't. This test only +// works when we use(protect) at the file level, not individually in the routes. +// However, this does test that we don't accidentally turn off the protection. +describe('jwtService.protect', () => { + it('should be called with admin', () => { + jest.resetModules(); + const jwtService = require('../../../../src/components/jwtService'); + jwtService.protect = jest.fn(() => { + return jest.fn((_req, _res, next) => { + next(); + }); + }); + require('../../../../src/forms/admin/routes'); + + expect(jwtService.protect).toBeCalledWith('admin'); + }); +}); + describe(`${basePath}/externalAPIs`, () => { const path = `${basePath}/externalAPIs`; @@ -53,6 +86,11 @@ describe(`${basePath}/externalAPIs`, () => { expect(controller.getExternalAPIs).toBeCalledTimes(1); expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateComponentId).toBeCalledTimes(0); + expect(validateParameter.validateExternalAPIId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(0); + expect(validateParameter.validateFormVersionId).toBeCalledTimes(0); + expect(validateParameter.validateUserId).toBeCalledTimes(0); }); }); @@ -70,6 +108,11 @@ describe(`${basePath}/externalAPIs/:externalApiId`, () => { expect(controller.updateExternalAPI).toBeCalledTimes(1); expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateComponentId).toBeCalledTimes(0); + expect(validateParameter.validateExternalAPIId).toBeCalledTimes(1); + expect(validateParameter.validateFormId).toBeCalledTimes(0); + expect(validateParameter.validateFormVersionId).toBeCalledTimes(0); + expect(validateParameter.validateUserId).toBeCalledTimes(0); }); }); @@ -86,6 +129,11 @@ describe(`${basePath}/externalAPIs/statusCodes`, () => { expect(controller.getExternalAPIStatusCodes).toBeCalledTimes(1); expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateComponentId).toBeCalledTimes(0); + expect(validateParameter.validateExternalAPIId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(0); + expect(validateParameter.validateFormVersionId).toBeCalledTimes(0); + expect(validateParameter.validateUserId).toBeCalledTimes(0); }); }); @@ -103,6 +151,11 @@ describe(`${basePath}/formcomponents/proactivehelp/:publishStatus/:componentId`, expect(controller.updateFormComponentsProactiveHelp).toBeCalledTimes(1); expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateComponentId).toBeCalledTimes(1); + expect(validateParameter.validateExternalAPIId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(0); + expect(validateParameter.validateFormVersionId).toBeCalledTimes(0); + expect(validateParameter.validateUserId).toBeCalledTimes(0); }); }); @@ -120,6 +173,11 @@ describe(`${basePath}/formcomponents/proactivehelp/imageUrl/:componentId`, () => expect(controller.getFCProactiveHelpImageUrl).toBeCalledTimes(1); expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateComponentId).toBeCalledTimes(1); + expect(validateParameter.validateExternalAPIId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(0); + expect(validateParameter.validateFormVersionId).toBeCalledTimes(0); + expect(validateParameter.validateUserId).toBeCalledTimes(0); }); }); @@ -136,6 +194,11 @@ describe(`${basePath}/formcomponents/proactivehelp/list`, () => { expect(controller.listFormComponentsProactiveHelp).toBeCalledTimes(1); expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateComponentId).toBeCalledTimes(0); + expect(validateParameter.validateExternalAPIId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(0); + expect(validateParameter.validateFormVersionId).toBeCalledTimes(0); + expect(validateParameter.validateUserId).toBeCalledTimes(0); }); }); @@ -152,6 +215,11 @@ describe(`${basePath}/formcomponents/proactivehelp/object`, () => { expect(controller.createFormComponentsProactiveHelp).toBeCalledTimes(1); expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateComponentId).toBeCalledTimes(0); + expect(validateParameter.validateExternalAPIId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(0); + expect(validateParameter.validateFormVersionId).toBeCalledTimes(0); + expect(validateParameter.validateUserId).toBeCalledTimes(0); }); }); @@ -168,6 +236,11 @@ describe(`${basePath}/forms`, () => { expect(controller.listForms).toBeCalledTimes(1); expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateComponentId).toBeCalledTimes(0); + expect(validateParameter.validateExternalAPIId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(0); + expect(validateParameter.validateFormVersionId).toBeCalledTimes(0); + expect(validateParameter.validateUserId).toBeCalledTimes(0); }); }); @@ -185,6 +258,11 @@ describe(`${basePath}/forms/:formId`, () => { expect(controller.readForm).toBeCalledTimes(1); expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateComponentId).toBeCalledTimes(0); + expect(validateParameter.validateExternalAPIId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(1); + expect(validateParameter.validateFormVersionId).toBeCalledTimes(0); + expect(validateParameter.validateUserId).toBeCalledTimes(0); }); }); @@ -202,6 +280,11 @@ describe(`${basePath}/forms/:formId/addUser`, () => { expect(controller.setFormUserRoles).toBeCalledTimes(1); expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateComponentId).toBeCalledTimes(0); + expect(validateParameter.validateExternalAPIId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(1); + expect(validateParameter.validateFormVersionId).toBeCalledTimes(0); + expect(validateParameter.validateUserId).toBeCalledTimes(0); }); }); @@ -219,6 +302,11 @@ describe(`${basePath}/forms/:formId/apiKey`, () => { expect(controller.deleteApiKey).toBeCalledTimes(1); expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateComponentId).toBeCalledTimes(0); + expect(validateParameter.validateExternalAPIId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(1); + expect(validateParameter.validateFormVersionId).toBeCalledTimes(0); + expect(validateParameter.validateUserId).toBeCalledTimes(0); }); it('should have correct middleware for GET', async () => { @@ -231,6 +319,11 @@ describe(`${basePath}/forms/:formId/apiKey`, () => { expect(controller.readApiDetails).toBeCalledTimes(1); expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateComponentId).toBeCalledTimes(0); + expect(validateParameter.validateExternalAPIId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(1); + expect(validateParameter.validateFormVersionId).toBeCalledTimes(0); + expect(validateParameter.validateUserId).toBeCalledTimes(0); }); }); @@ -248,6 +341,11 @@ describe(`${basePath}/forms/:formId/formUsers`, () => { expect(controller.getFormUserRoles).toBeCalledTimes(1); expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateComponentId).toBeCalledTimes(0); + expect(validateParameter.validateExternalAPIId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(1); + expect(validateParameter.validateFormVersionId).toBeCalledTimes(0); + expect(validateParameter.validateUserId).toBeCalledTimes(0); }); }); @@ -265,6 +363,11 @@ describe(`${basePath}/forms/:formId/restore`, () => { expect(controller.restoreForm).toBeCalledTimes(1); expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateComponentId).toBeCalledTimes(0); + expect(validateParameter.validateExternalAPIId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(1); + expect(validateParameter.validateFormVersionId).toBeCalledTimes(0); + expect(validateParameter.validateUserId).toBeCalledTimes(0); }); }); @@ -283,6 +386,11 @@ describe(`${basePath}/forms/:formId/versions/:formVersionId`, () => { expect(controller.readVersion).toBeCalledTimes(1); expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateComponentId).toBeCalledTimes(0); + expect(validateParameter.validateExternalAPIId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(1); + expect(validateParameter.validateFormVersionId).toBeCalledTimes(1); + expect(validateParameter.validateUserId).toBeCalledTimes(0); }); }); @@ -299,6 +407,11 @@ describe(`${basePath}/users`, () => { expect(controller.getUsers).toBeCalledTimes(1); expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateComponentId).toBeCalledTimes(0); + expect(validateParameter.validateExternalAPIId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(0); + expect(validateParameter.validateFormVersionId).toBeCalledTimes(0); + expect(validateParameter.validateUserId).toBeCalledTimes(0); }); }); @@ -316,5 +429,10 @@ describe(`${basePath}/users/:userId`, () => { expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(userController.read).toBeCalledTimes(1); + expect(validateParameter.validateComponentId).toBeCalledTimes(0); + expect(validateParameter.validateExternalAPIId).toBeCalledTimes(0); + expect(validateParameter.validateFormId).toBeCalledTimes(0); + expect(validateParameter.validateFormVersionId).toBeCalledTimes(0); + expect(validateParameter.validateUserId).toBeCalledTimes(1); }); }); diff --git a/app/tests/unit/forms/common/middleware/validateParameter.spec.js b/app/tests/unit/forms/common/middleware/validateParameter.spec.js index 4abebaeda..c5bef0af5 100644 --- a/app/tests/unit/forms/common/middleware/validateParameter.spec.js +++ b/app/tests/unit/forms/common/middleware/validateParameter.spec.js @@ -1,16 +1,12 @@ const { getMockReq, getMockRes } = require('@jest-mock/express'); const uuid = require('uuid'); +const constants = require('../../../../../src/forms/common/constants'); 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 = uuid.v4(); -const formId = uuid.v4(); -const formSubmissionId = uuid.v4(); -const userId = uuid.v4(); - // Various types of invalid UUIDs that we see in API calls. const invalidUuids = [[''], ['undefined'], ['{{id}}'], ['${id}'], [uuid.v4() + '.'], [' ' + uuid.v4() + ' ']]; @@ -18,8 +14,55 @@ afterEach(() => { jest.clearAllMocks(); }); +describe('validateComponentId', () => { + const componentId = uuid.v4(); + + describe('400 response when', () => { + const expectedStatus = { status: 400 }; + + test('componentId is missing', async () => { + const req = getMockReq({ + params: {}, + }); + const { res, next } = getMockRes(); + + await validateParameter.validateComponentId(req, res, next); + + expect(next).toBeCalledWith(expect.objectContaining(expectedStatus)); + }); + + test.each(invalidUuids)('componentId is "%s"', async (eachComponentId) => { + const req = getMockReq({ + params: { componentId: eachComponentId }, + }); + const { res, next } = getMockRes(); + + await validateParameter.validateComponentId(req, res, next, eachComponentId); + + expect(next).toBeCalledWith(expect.objectContaining(expectedStatus)); + }); + }); + + describe('allows', () => { + test('uuid for componentId', async () => { + const req = getMockReq({ + params: { + componentId: componentId, + }, + }); + const { res, next } = getMockRes(); + + await validateParameter.validateComponentId(req, res, next, componentId); + + expect(next).toBeCalledWith(); + }); + }); +}); + describe('validateDocumentTemplateId', () => { const documentTemplateId = uuid.v4(); + const formId = uuid.v4(); + const formSubmissionId = uuid.v4(); const mockReadDocumentTemplateResponse = { formId: formId, @@ -202,6 +245,7 @@ describe('validateDocumentTemplateId', () => { describe('validateExternalApiId', () => { const externalApiId = uuid.v4(); + const formId = uuid.v4(); const mockReadExternalApiResponse = { formId: formId, @@ -346,6 +390,7 @@ describe('validateFileId', () => { describe('allows', () => { test('uuid for fileId', async () => { + const fileId = uuid.v4(); const req = getMockReq({ params: { fileId: fileId, @@ -389,6 +434,7 @@ describe('validateFormId', () => { describe('allows', () => { test('uuid for formId', async () => { + const formId = uuid.v4(); const req = getMockReq({ params: { formId: formId, @@ -432,6 +478,7 @@ describe('validateFormSubmissionId', () => { describe('allows', () => { test('uuid for formSubmissionId', async () => { + const formSubmissionId = uuid.v4(); const req = getMockReq({ params: { formSubmissionId: formSubmissionId, @@ -447,6 +494,7 @@ describe('validateFormSubmissionId', () => { }); describe('validateFormVersionDraftId', () => { + const formId = uuid.v4(); const formVersionDraftId = uuid.v4(); const mockReadDraftResponse = { @@ -561,6 +609,7 @@ describe('validateFormVersionDraftId', () => { }); describe('validateFormVersionId', () => { + const formId = uuid.v4(); const formVersionId = uuid.v4(); const mockReadVersionResponse = { @@ -674,6 +723,92 @@ describe('validateFormVersionId', () => { }); }); +describe('validatePermissionCode', () => { + describe('400 response when', () => { + const expectedStatus = { status: 400 }; + + test('code is missing', async () => { + const req = getMockReq({ + params: {}, + }); + const { res, next } = getMockRes(); + + await validateParameter.validatePermissionCode(req, res, next); + + expect(next).toBeCalledWith(expect.objectContaining(expectedStatus)); + }); + + test('code is invalid', async () => { + const req = getMockReq({ + params: { + code: 'this-is-not-valid', + }, + }); + const { res, next } = getMockRes(); + + await validateParameter.validatePermissionCode(req, res, next); + + expect(next).toBeCalledWith(expect.objectContaining(expectedStatus)); + }); + }); + + describe('allows', () => { + test.each(Object.values(constants.Permissions))('code is "%s"', async (eachCode) => { + const req = getMockReq({ + params: { code: eachCode }, + }); + const { res, next } = getMockRes(); + + await validateParameter.validatePermissionCode(req, res, next, eachCode); + + expect(next).toBeCalledWith(); + }); + }); +}); + +describe('validateRoleCode', () => { + describe('400 response when', () => { + const expectedStatus = { status: 400 }; + + test('code is missing', async () => { + const req = getMockReq({ + params: {}, + }); + const { res, next } = getMockRes(); + + await validateParameter.validateRoleCode(req, res, next); + + expect(next).toBeCalledWith(expect.objectContaining(expectedStatus)); + }); + + test('code is invalid', async () => { + const req = getMockReq({ + params: { + code: 'this-is-not-valid', + }, + }); + const { res, next } = getMockRes(); + + await validateParameter.validateRoleCode(req, res, next); + + expect(next).toBeCalledWith(expect.objectContaining(expectedStatus)); + }); + }); + + describe('allows', () => { + test.each(Object.values(constants.Roles))('code is "%s"', async (eachCode) => { + const req = getMockReq({ + params: { code: eachCode }, + }); + const { res, next } = getMockRes(); + + await validateParameter.validateRoleCode(req, res, next, eachCode); + + expect(next).toBeCalledWith(); + }); + }); +}); + describe('validateUserId', () => { describe('400 response when', () => { const expectedStatus = { status: 400 }; @@ -703,6 +838,7 @@ describe('validateUserId', () => { describe('allows', () => { test('uuid for userId', async () => { + const userId = uuid.v4(); const req = getMockReq({ params: { userId: userId, diff --git a/app/tests/unit/forms/form/routes.spec.js b/app/tests/unit/forms/form/routes.spec.js index 1ffae78ba..def2835b8 100644 --- a/app/tests/unit/forms/form/routes.spec.js +++ b/app/tests/unit/forms/form/routes.spec.js @@ -22,10 +22,11 @@ apiAccess.mockImplementation( }) ); +const mockJwtServiceProtect = jest.fn((_req, _res, next) => { + next(); +}); jwtService.protect = jest.fn(() => { - return jest.fn((_req, _res, next) => { - next(); - }); + return mockJwtServiceProtect; }); rateLimiter.apiKeyRateLimiter = jest.fn((_req, _res, next) => { @@ -81,6 +82,7 @@ describe(`${basePath}`, () => { expect(apiAccess).toBeCalledTimes(0); expect(controller.listForms).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(0); + expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -99,6 +101,7 @@ describe(`${basePath}`, () => { expect(apiAccess).toBeCalledTimes(0); expect(controller.createForm).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(0); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -122,6 +125,7 @@ describe(`${basePath}/:formId`, () => { expect(apiAccess).toBeCalledTimes(1); expect(controller.deleteForm).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -140,6 +144,7 @@ describe(`${basePath}/:formId`, () => { expect(apiAccess).toBeCalledTimes(1); expect(controller.readForm).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -158,6 +163,7 @@ describe(`${basePath}/:formId`, () => { expect(apiAccess).toBeCalledTimes(1); expect(controller.updateForm).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -181,6 +187,7 @@ describe(`${basePath}/:formId/apiKey`, () => { expect(apiAccess).toBeCalledTimes(0); expect(controller.deleteApiKey).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -199,6 +206,7 @@ describe(`${basePath}/:formId/apiKey`, () => { expect(apiAccess).toBeCalledTimes(0); expect(controller.readApiKey).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -217,6 +225,7 @@ describe(`${basePath}/:formId/apiKey`, () => { expect(apiAccess).toBeCalledTimes(0); expect(controller.createOrReplaceApiKey).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -240,6 +249,7 @@ describe(`${basePath}/:formId/apiKey/filesApiAccess`, () => { expect(apiAccess).toBeCalledTimes(0); expect(controller.filesApiKeyAccess).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -263,6 +273,7 @@ describe(`${basePath}/:formId/csvexport/fields`, () => { expect(apiAccess).toBeCalledTimes(1); expect(controller.readFieldsForCSVExport).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -286,6 +297,7 @@ describe(`${basePath}/:formId/documentTemplates`, () => { expect(apiAccess).toBeCalledTimes(1); expect(controller.documentTemplateList).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -304,6 +316,7 @@ describe(`${basePath}/:formId/documentTemplates`, () => { expect(apiAccess).toBeCalledTimes(1); expect(controller.documentTemplateCreate).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -328,6 +341,7 @@ describe(`${basePath}/:formId/documentTemplates/:documentTemplateId`, () => { expect(apiAccess).toBeCalledTimes(1); expect(controller.documentTemplateDelete).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(1); @@ -346,6 +360,7 @@ describe(`${basePath}/:formId/documentTemplates/:documentTemplateId`, () => { expect(apiAccess).toBeCalledTimes(1); expect(controller.documentTemplateRead).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(1); @@ -369,6 +384,7 @@ describe(`${basePath}/:formId/drafts`, () => { expect(apiAccess).toBeCalledTimes(1); expect(controller.listDrafts).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -387,6 +403,7 @@ describe(`${basePath}/:formId/drafts`, () => { expect(apiAccess).toBeCalledTimes(1); expect(controller.createDraft).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -411,6 +428,7 @@ describe(`${basePath}/:formId/drafts/:formVersionDraftId`, () => { expect(apiAccess).toBeCalledTimes(1); expect(controller.deleteDraft).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -429,6 +447,7 @@ describe(`${basePath}/:formId/drafts/:formVersionDraftId`, () => { expect(apiAccess).toBeCalledTimes(1); expect(controller.readDraft).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -447,6 +466,7 @@ describe(`${basePath}/:formId/drafts/:formVersionDraftId`, () => { expect(apiAccess).toBeCalledTimes(1); expect(controller.updateDraft).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -471,6 +491,7 @@ describe(`${basePath}/:formId/drafts/:formVersionDraftId/publish`, () => { expect(apiAccess).toBeCalledTimes(1); expect(controller.publishDraft).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -494,6 +515,7 @@ describe(`${basePath}/:formId/emailTemplate`, () => { expect(apiAccess).toBeCalledTimes(0); expect(controller.createOrUpdateEmailTemplate).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -517,6 +539,7 @@ describe(`${basePath}/:formId/emailTemplates`, () => { expect(apiAccess).toBeCalledTimes(0); expect(controller.readEmailTemplates).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -540,6 +563,7 @@ describe(`${basePath}/:formId/export`, () => { expect(apiAccess).toBeCalledTimes(1); expect(controller.export).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -563,6 +587,7 @@ describe(`${basePath}/:formId/export/fields`, () => { expect(apiAccess).toBeCalledTimes(1); expect(controller.exportWithFields).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -586,6 +611,7 @@ describe(`${basePath}/:formId/options`, () => { expect(apiAccess).toBeCalledTimes(0); expect(controller.readFormOptions).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(0); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -609,6 +635,7 @@ describe(`${basePath}/:formId/statusCodes`, () => { expect(apiAccess).toBeCalledTimes(1); expect(controller.getStatusCodes).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -632,6 +659,7 @@ describe(`${basePath}/:formId/submissions`, () => { expect(apiAccess).toBeCalledTimes(1); expect(controller.listFormSubmissions).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -655,6 +683,7 @@ describe(`${basePath}/:formId/subscriptions`, () => { expect(apiAccess).toBeCalledTimes(0); expect(controller.readFormSubscriptionDetails).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -673,6 +702,7 @@ describe(`${basePath}/:formId/subscriptions`, () => { expect(apiAccess).toBeCalledTimes(0); expect(controller.createOrUpdateSubscriptionDetails).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -696,6 +726,7 @@ describe(`${basePath}/:formId/version`, () => { expect(apiAccess).toBeCalledTimes(1); expect(controller.readPublishedForm).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -720,6 +751,7 @@ describe(`${basePath}/:formId/versions/:formVersionId`, () => { expect(apiAccess).toBeCalledTimes(1); expect(controller.readVersion).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -744,6 +776,7 @@ describe(`${basePath}/:formId/versions/:formVersionId/fields`, () => { expect(apiAccess).toBeCalledTimes(1); expect(controller.readVersionFields).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -768,6 +801,7 @@ describe(`${basePath}/:formId/versions/:formVersionId/multiSubmission`, () => { expect(apiAccess).toBeCalledTimes(1); expect(controller.createMultiSubmission).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -792,6 +826,7 @@ describe(`${basePath}/:formId/versions/:formVersionId/publish`, () => { expect(apiAccess).toBeCalledTimes(1); expect(controller.publishVersion).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -816,6 +851,7 @@ describe(`${basePath}/:formId/versions/:formVersionId/submissions`, () => { expect(apiAccess).toBeCalledTimes(1); expect(controller.listSubmissions).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -834,6 +870,7 @@ describe(`${basePath}/:formId/versions/:formVersionId/submissions`, () => { expect(apiAccess).toBeCalledTimes(1); expect(controller.createSubmission).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -858,6 +895,7 @@ describe(`${basePath}/:formId/versions/:formVersionId/submissions/discover`, () expect(apiAccess).toBeCalledTimes(1); expect(controller.listSubmissionFields).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -881,6 +919,7 @@ describe(`${basePath}/formcomponents/proactivehelp/imageUrl/:componentId`, () => expect(apiAccess).toBeCalledTimes(0); expect(controller.getFCProactiveHelpImageUrl).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(0); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); @@ -903,6 +942,7 @@ describe(`${basePath}/formcomponents/proactivehelp/list`, () => { expect(apiAccess).toBeCalledTimes(0); expect(controller.listFormComponentsProactiveHelp).toBeCalledTimes(1); expect(hasFormPermissionsMock).toBeCalledTimes(0); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateDocumentTemplateId).toBeCalledTimes(0); diff --git a/app/tests/unit/forms/permission/routes.spec.js b/app/tests/unit/forms/permission/routes.spec.js new file mode 100644 index 000000000..dc77ea400 --- /dev/null +++ b/app/tests/unit/forms/permission/routes.spec.js @@ -0,0 +1,120 @@ +const request = require('supertest'); + +const { expressHelper } = require('../../../common/helper'); + +const jwtService = require('../../../../src/components/jwtService'); +const userAccess = require('../../../../src/forms/auth/middleware/userAccess'); +const validateParameter = require('../../../../src/forms/common/middleware/validateParameter'); +const controller = require('../../../../src/forms/permission/controller'); + +// +// Mock out all the middleware - we're testing that the routes are set up +// correctly, not the functionality of the middleware. +// + +const mockJwtServiceProtect = jest.fn((_req, _res, next) => { + next(); +}); +jwtService.protect = jest.fn(() => { + return mockJwtServiceProtect; +}); + +userAccess.currentUser = jest.fn((_req, _res, next) => { + next(); +}); + +validateParameter.validatePermissionCode = jest.fn((_req, _res, next) => { + next(); +}); + +// +// Create the router and a simple Express server. +// + +const router = require('../../../../src/forms/permission/routes'); +const basePath = '/permission'; +const app = expressHelper(basePath, router); +const appRequest = request(app); + +afterEach(() => { + jest.clearAllMocks(); +}); + +// jwtService.protect is tricky to test. This test is fragile as the file could +// change and have routes that need admin and others that don't. This test only +// works when we use(protect) at the file level, not individually in the routes. +// However, this does test that we don't accidentally turn off the protection. +describe('jwtService.protect', () => { + it('should be called with admin', () => { + jest.resetModules(); + const jwtService = require('../../../../src/components/jwtService'); + jwtService.protect = jest.fn(() => { + return jest.fn((_req, _res, next) => { + next(); + }); + }); + require('../../../../src/forms/permission/routes'); + + expect(jwtService.protect).toBeCalledWith('admin'); + }); +}); + +describe(`${basePath}`, () => { + const path = `${basePath}`; + + it('should have correct middleware for GET', async () => { + controller.list = jest.fn((_req, res) => { + res.sendStatus(200); + }); + + await appRequest.get(path); + + expect(controller.list).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validatePermissionCode).toBeCalledTimes(0); + }); + + it('should have correct middleware for POST', async () => { + controller.create = jest.fn((_req, res) => { + res.sendStatus(200); + }); + + await appRequest.post(path); + + expect(controller.create).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validatePermissionCode).toBeCalledTimes(0); + }); +}); + +describe(`${basePath}/:code`, () => { + const path = `${basePath}/code`; + + it('should have correct middleware for GET', async () => { + controller.read = jest.fn((_req, res) => { + res.sendStatus(200); + }); + + await appRequest.get(path); + + expect(controller.read).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validatePermissionCode).toBeCalledTimes(1); + }); + + it('should have correct middleware for PUT', async () => { + controller.update = jest.fn((_req, res) => { + res.sendStatus(200); + }); + + await appRequest.put(path); + + expect(controller.update).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validatePermissionCode).toBeCalledTimes(1); + }); +}); diff --git a/app/tests/unit/forms/proxy/controller.spec.js b/app/tests/unit/forms/proxy/controller.spec.js index d52ad6c68..8d2eec8bf 100644 --- a/app/tests/unit/forms/proxy/controller.spec.js +++ b/app/tests/unit/forms/proxy/controller.spec.js @@ -4,6 +4,7 @@ const { getMockReq, getMockRes } = require('@jest-mock/express'); const controller = require('../../../../src/forms/proxy/controller'); const service = require('../../../../src/forms/proxy/service'); const jwtService = require('../../../../src/components/jwtService'); + const { NotFoundError } = require('objection'); const bearerToken = Math.random().toString(36).substring(2); @@ -29,6 +30,7 @@ describe('generateProxyHeaders', () => { }; it('should generate headers', async () => { + service._getIds = jest.fn().mockReturnValue({ formId: '1234', versionId: '5678', submissionId: null }); service.generateProxyHeaders = jest.fn().mockReturnValue({}); await controller.generateProxyHeaders(req, {}, jest.fn()); diff --git a/app/tests/unit/forms/proxy/routes.spec.js b/app/tests/unit/forms/proxy/routes.spec.js index dc7bb9bd4..1d65140e1 100644 --- a/app/tests/unit/forms/proxy/routes.spec.js +++ b/app/tests/unit/forms/proxy/routes.spec.js @@ -11,7 +11,6 @@ const request = require('supertest'); const { expressHelper } = require('../../../common/helper'); -const jwtService = require('../../../../src/components/jwtService'); const apiAccess = require('../../../../src/forms/auth/middleware/apiAccess'); const userAccess = require('../../../../src/forms/auth/middleware/userAccess'); const rateLimiter = require('../../../../src/forms/common/middleware/rateLimiter'); @@ -28,12 +27,6 @@ apiAccess.mockImplementation( }) ); -jwtService.protect = jest.fn(() => { - return jest.fn((_req, _res, next) => { - next(); - }); -}); - rateLimiter.apiKeyRateLimiter = jest.fn((_req, _res, next) => { next(); }); diff --git a/app/tests/unit/forms/proxy/service.spec.js b/app/tests/unit/forms/proxy/service.spec.js index 7c3d953dc..266966c53 100644 --- a/app/tests/unit/forms/proxy/service.spec.js +++ b/app/tests/unit/forms/proxy/service.spec.js @@ -6,9 +6,11 @@ const { encryptionService, ENCRYPTION_ALGORITHMS } = require('../../../../src/co const service = require('../../../../src/forms/proxy/service'); const { ExternalAPI } = require('../../../../src/forms/common/models'); +jest.mock('../../../../src/forms/common/models/views/submissionMetadata', () => MockModel); + const goodPayload = { formId: '123', - submissionId: '456', + submissionId: null, versionId: '789', }; @@ -60,9 +62,42 @@ afterEach(() => { }); describe('Proxy Service', () => { + describe('_getIds', () => { + beforeEach(() => { + MockModel.mockReset(); + }); + afterEach(() => { + jest.restoreAllMocks(); + }); + it('should return all ids', async () => { + const payload = { formId: null, versionId: null, submissionId: '1' }; + const meta = { formId: '2', formVersionId: '3', submissionId: '1' }; // meta query result + const ids = { formId: '2', versionId: '3', submissionId: '1' }; // _getIds result + MockModel.mockResolvedValue(meta); + const result = await service._getIds(payload); + await expect(result).toStrictEqual(ids); + expect(MockModel.query).toBeCalledTimes(1); // submission id means we query + expect(MockModel.first).toBeCalledTimes(1); + }); + it('should return form and version id', async () => { + const payload = { formId: '2', versionId: '3', submissionId: null }; + const ids = { formId: '2', versionId: '3', submissionId: null }; // _getIds result + const result = await service._getIds(payload); + await expect(result).toStrictEqual(ids); + expect(MockModel.query).toBeCalledTimes(0); // no submission id, no query + }); + it('should return form and version id when submission id not found', async () => { + const payload = { formId: null, versionId: null, submissionId: '1' }; + const meta = null; // meta query result + const ids = { formId: null, versionId: null, submissionId: '1' }; // _getIds result + MockModel.mockResolvedValue(meta); + const result = await service._getIds(payload); + await expect(result).toStrictEqual(ids); + expect(MockModel.query).toBeCalledTimes(1); // submission id means we query + expect(MockModel.first).toBeCalledTimes(1); + }); + }); describe('generateProxyHeaders', () => { - beforeEach(() => {}); - it('should throw error with no payload', async () => { await expect(service.generateProxyHeaders(undefined, goodCurrentUser, token)).rejects.toThrow(); }); @@ -85,14 +120,33 @@ describe('Proxy Service', () => { const decrypted = encryptionService.decryptProxy(result['X-CHEFS-PROXY-DATA']); expect(JSON.parse(decrypted)).toMatchObject(goodProxyHeaderInfo); }); + it('should find formId and versionId when given submissionId', async () => { + const submissionPayload = { submissionId: '456' }; + const idsPaylad = { ...goodPayload, ...submissionPayload }; + service._getIds = jest.fn().mockResolvedValueOnce(idsPaylad); + const submissionProxyHeaderInfo = { + ...idsPaylad, + ...goodCurrentUser, + userId: goodCurrentUser.idpUserId, + token: token, + }; + delete submissionProxyHeaderInfo.idpUserId; + + const result = await service.generateProxyHeaders(submissionPayload, goodCurrentUser, token); + expect(result).toBeTruthy(); + expect(result['X-CHEFS-PROXY-DATA']).toBeTruthy(); + // check the encryption... + const decrypted = encryptionService.decryptProxy(result['X-CHEFS-PROXY-DATA']); + expect(JSON.parse(decrypted)).toMatchObject(submissionProxyHeaderInfo); + }); }); describe('readProxyHeaders', () => { - beforeEach(() => {}); - it('should throw error with no headers', async () => { await expect(service.readProxyHeaders(undefined)).rejects.toThrow(); }); - + it('should throw error with bad headers', async () => { + await expect(service.readProxyHeaders('string-not-object')).rejects.toThrow(); + }); it('should throw error with wrong header name', async () => { await expect(service.readProxyHeaders({ 'X-CHEFS-WRONG_HEADER_NAME': 'headers' })).rejects.toThrow(); }); @@ -108,6 +162,8 @@ describe('Proxy Service', () => { }); it('should return decrypted header data', async () => { + service._getIds = jest.fn().mockResolvedValueOnce({ ...goodPayload, submissionId: null }); + const headers = await service.generateProxyHeaders(goodPayload, goodCurrentUser, token); const decrypted = await service.readProxyHeaders(headers); expect(decrypted).toBeTruthy(); @@ -137,6 +193,12 @@ describe('Proxy Service', () => { await expect(service.getExternalAPI(headers, goodProxyHeaderInfo)).rejects.toThrow(); }); + it('should throw error with bad header', async () => { + // set the external api name... + const headers = 'string-not-object'; + await expect(service.getExternalAPI(headers, goodProxyHeaderInfo)).rejects.toThrow(); + }); + it('should throw error with no proxy header info', async () => { // set the external api name... const headers = { 'X-CHEFS-EXTERNAL-API-NAME': 'testapi' }; @@ -194,8 +256,6 @@ describe('Proxy Service', () => { }); }); describe('createExternalAPIHeaders', () => { - beforeEach(() => {}); - it('should throw error with no headers', async () => { const externalAPI = undefined; const proxyHeaderInfo = goodProxyHeaderInfo; diff --git a/app/tests/unit/forms/public/middleware/apiAccess.spec.js b/app/tests/unit/forms/public/middleware/apiAccess.spec.js new file mode 100644 index 000000000..6255aa217 --- /dev/null +++ b/app/tests/unit/forms/public/middleware/apiAccess.spec.js @@ -0,0 +1,66 @@ +const { getMockReq, getMockRes } = require('@jest-mock/express'); +const uuid = require('uuid'); + +const apiAccess = require('../../../../../src/forms/public/middleware/apiAccess'); + +afterEach(() => { + jest.clearAllMocks(); +}); + +describe('checkApiKey', () => { + process.env.APITOKEN = uuid.v4(); + + describe('401 response when', () => { + const expectedStatus = { status: 401 }; + + test('there is no APITOKEN or apikey', async () => { + const req = getMockReq(); + const { res, next } = getMockRes(); + + await apiAccess.checkApiKey(req, res, next); + + expect(next).toBeCalledTimes(1); + expect(next).toBeCalledWith(expect.objectContaining(expectedStatus)); + }); + + test('the apikey is empty', async () => { + const req = getMockReq({ + headers: { apikey: '' }, + }); + const { res, next } = getMockRes(); + + await apiAccess.checkApiKey(req, res, next); + + expect(next).toBeCalledTimes(1); + expect(next).toBeCalledWith(expect.objectContaining(expectedStatus)); + }); + + test('the apikey exists but does not match', async () => { + const req = getMockReq({ + headers: { apikey: uuid.v4() }, + }); + const { res, next } = getMockRes(); + + await apiAccess.checkApiKey(req, res, next); + + expect(next).toBeCalledTimes(1); + expect(next).toBeCalledWith(expect.objectContaining(expectedStatus)); + }); + }); + + describe('allows', () => { + test('matching APITOKEN and apikey', async () => { + const req = getMockReq({ + headers: { + apikey: process.env.APITOKEN, + }, + }); + const { res, next } = getMockRes(); + + await apiAccess.checkApiKey(req, res, next); + + expect(next).toBeCalledTimes(1); + expect(next).toBeCalledWith(); + }); + }); +}); diff --git a/app/tests/unit/forms/public/routes.spec.js b/app/tests/unit/forms/public/routes.spec.js index 504a09a21..640fa5f63 100644 --- a/app/tests/unit/forms/public/routes.spec.js +++ b/app/tests/unit/forms/public/routes.spec.js @@ -4,6 +4,16 @@ const uuid = require('uuid'); const { expressHelper } = require('../../../common/helper'); const controller = require('../../../../src/forms/public/controller'); +const apiAccess = require('../../../../src/forms/public/middleware/apiAccess'); + +// +// Mock out all the middleware - we're testing that the routes are set up +// correctly, not the functionality of the middleware. +// + +apiAccess.checkApiKey = jest.fn((_req, _res, next) => { + next(); +}); // // Create the router and a simple Express server. @@ -18,10 +28,6 @@ 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`; @@ -32,46 +38,9 @@ describe(`${basePath}/reminder`, () => { it('200s when the APITOKEN matches apiKey', async () => { process.env.APITOKEN = uuid.v4(); - const response = await appRequest.get(path).set({ apikey: process.env.APITOKEN }); + await appRequest.get(path).set({ apikey: process.env.APITOKEN }); + expect(apiAccess.checkApiKey).toBeCalledTimes(1); 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/rbac/routes.spec.js b/app/tests/unit/forms/rbac/routes.spec.js index 80d6e9734..91ceb2b8b 100644 --- a/app/tests/unit/forms/rbac/routes.spec.js +++ b/app/tests/unit/forms/rbac/routes.spec.js @@ -11,10 +11,11 @@ const controller = require('../../../../src/forms/rbac/controller'); // correctly, not the functionality of the middleware. // +const mockJwtServiceProtect = jest.fn((_req, _res, next) => { + next(); +}); jwtService.protect = jest.fn(() => { - return jest.fn((_req, _res, next) => { - next(); - }); + return mockJwtServiceProtect; }); const hasFormPermissionsMock = jest.fn((_req, _res, next) => { @@ -75,6 +76,7 @@ describe(`${basePath}/current`, () => { expect(hasFormPermissionsMock).toBeCalledTimes(0); expect(hasFormRolesMock).toBeCalledTimes(0); expect(hasSubmissionPermissionsMock).toBeCalledTimes(0); + expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(userAccess.hasRoleDeletePermissions).toBeCalledTimes(0); expect(userAccess.hasRoleModifyPermissions).toBeCalledTimes(0); @@ -95,6 +97,7 @@ describe(`${basePath}/current/submissions`, () => { expect(hasFormPermissionsMock).toBeCalledTimes(0); expect(hasFormRolesMock).toBeCalledTimes(0); expect(hasSubmissionPermissionsMock).toBeCalledTimes(0); + expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(userAccess.hasRoleDeletePermissions).toBeCalledTimes(0); expect(userAccess.hasRoleModifyPermissions).toBeCalledTimes(0); @@ -115,6 +118,7 @@ describe(`${basePath}/forms`, () => { expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(hasFormRolesMock).toBeCalledTimes(0); expect(hasSubmissionPermissionsMock).toBeCalledTimes(0); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(userAccess.currentUser).toBeCalledTimes(1); expect(userAccess.hasRoleDeletePermissions).toBeCalledTimes(0); expect(userAccess.hasRoleModifyPermissions).toBeCalledTimes(0); @@ -131,6 +135,7 @@ describe(`${basePath}/forms`, () => { expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(hasFormRolesMock).toBeCalledTimes(0); expect(hasSubmissionPermissionsMock).toBeCalledTimes(0); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(userAccess.currentUser).toBeCalledTimes(1); expect(userAccess.hasRoleDeletePermissions).toBeCalledTimes(0); expect(userAccess.hasRoleModifyPermissions).toBeCalledTimes(0); @@ -151,6 +156,7 @@ describe(`${basePath}/idps`, () => { expect(hasFormPermissionsMock).toBeCalledTimes(0); expect(hasFormRolesMock).toBeCalledTimes(0); expect(hasSubmissionPermissionsMock).toBeCalledTimes(0); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(userAccess.currentUser).toBeCalledTimes(1); expect(userAccess.hasRoleDeletePermissions).toBeCalledTimes(0); expect(userAccess.hasRoleModifyPermissions).toBeCalledTimes(0); @@ -171,6 +177,7 @@ describe(`${basePath}/submissions`, () => { expect(hasFormPermissionsMock).toBeCalledTimes(0); expect(hasFormRolesMock).toBeCalledTimes(0); expect(hasSubmissionPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(userAccess.currentUser).toBeCalledTimes(1); expect(userAccess.hasRoleDeletePermissions).toBeCalledTimes(0); expect(userAccess.hasRoleModifyPermissions).toBeCalledTimes(0); @@ -187,6 +194,7 @@ describe(`${basePath}/submissions`, () => { expect(hasFormPermissionsMock).toBeCalledTimes(0); expect(hasFormRolesMock).toBeCalledTimes(0); expect(hasSubmissionPermissionsMock).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(userAccess.currentUser).toBeCalledTimes(1); expect(userAccess.hasRoleDeletePermissions).toBeCalledTimes(0); expect(userAccess.hasRoleModifyPermissions).toBeCalledTimes(0); @@ -207,6 +215,7 @@ describe(`${basePath}/users`, () => { expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(hasFormRolesMock).toBeCalledTimes(1); expect(hasSubmissionPermissionsMock).toBeCalledTimes(0); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(userAccess.currentUser).toBeCalledTimes(1); expect(userAccess.hasRoleDeletePermissions).toBeCalledTimes(1); expect(userAccess.hasRoleModifyPermissions).toBeCalledTimes(0); @@ -223,6 +232,7 @@ describe(`${basePath}/users`, () => { expect(hasFormPermissionsMock).toBeCalledTimes(0); expect(hasFormRolesMock).toBeCalledTimes(0); expect(hasSubmissionPermissionsMock).toBeCalledTimes(0); + expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(userAccess.hasRoleDeletePermissions).toBeCalledTimes(0); expect(userAccess.hasRoleModifyPermissions).toBeCalledTimes(0); @@ -239,6 +249,7 @@ describe(`${basePath}/users`, () => { expect(hasFormPermissionsMock).toBeCalledTimes(1); expect(hasFormRolesMock).toBeCalledTimes(1); expect(hasSubmissionPermissionsMock).toBeCalledTimes(0); + expect(mockJwtServiceProtect).toBeCalledTimes(0); expect(userAccess.currentUser).toBeCalledTimes(1); expect(userAccess.hasRoleDeletePermissions).toBeCalledTimes(0); expect(userAccess.hasRoleModifyPermissions).toBeCalledTimes(1); diff --git a/app/tests/unit/forms/role/routes.spec.js b/app/tests/unit/forms/role/routes.spec.js new file mode 100644 index 000000000..f9eba8c83 --- /dev/null +++ b/app/tests/unit/forms/role/routes.spec.js @@ -0,0 +1,120 @@ +const request = require('supertest'); + +const { expressHelper } = require('../../../common/helper'); + +const jwtService = require('../../../../src/components/jwtService'); +const userAccess = require('../../../../src/forms/auth/middleware/userAccess'); +const validateParameter = require('../../../../src/forms/common/middleware/validateParameter'); +const controller = require('../../../../src/forms/role/controller'); + +// +// Mock out all the middleware - we're testing that the routes are set up +// correctly, not the functionality of the middleware. +// + +const mockJwtServiceProtect = jest.fn((_req, _res, next) => { + next(); +}); +jwtService.protect = jest.fn(() => { + return mockJwtServiceProtect; +}); + +userAccess.currentUser = jest.fn((_req, _res, next) => { + next(); +}); + +validateParameter.validateRoleCode = jest.fn((_req, _res, next) => { + next(); +}); + +// +// Create the router and a simple Express server. +// + +const router = require('../../../../src/forms/role/routes'); +const basePath = '/role'; +const app = expressHelper(basePath, router); +const appRequest = request(app); + +afterEach(() => { + jest.clearAllMocks(); +}); + +// jwtService.protect is tricky to test. This test is fragile as the file could +// change and have routes that need admin and others that don't. This test only +// works when we use(protect) at the file level, not individually in the routes. +// However, this does test that we don't accidentally turn off the protection. +describe('jwtService.protect', () => { + it('should be called with admin', () => { + jest.resetModules(); + const jwtService = require('../../../../src/components/jwtService'); + jwtService.protect = jest.fn(() => { + return jest.fn((_req, _res, next) => { + next(); + }); + }); + require('../../../../src/forms/role/routes'); + + expect(jwtService.protect).toBeCalledWith('admin'); + }); +}); + +describe(`${basePath}`, () => { + const path = `${basePath}`; + + it('should have correct middleware for GET', async () => { + controller.list = jest.fn((_req, res) => { + res.sendStatus(200); + }); + + await appRequest.get(path); + + expect(controller.list).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateRoleCode).toBeCalledTimes(0); + }); + + it('should have correct middleware for POST', async () => { + controller.create = jest.fn((_req, res) => { + res.sendStatus(200); + }); + + await appRequest.post(path); + + expect(controller.create).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateRoleCode).toBeCalledTimes(0); + }); +}); + +describe(`${basePath}/:code`, () => { + const path = `${basePath}/code`; + + it('should have correct middleware for GET', async () => { + controller.read = jest.fn((_req, res) => { + res.sendStatus(200); + }); + + await appRequest.get(path); + + expect(controller.read).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateRoleCode).toBeCalledTimes(1); + }); + + it('should have correct middleware for PUT', async () => { + controller.update = jest.fn((_req, res) => { + res.sendStatus(200); + }); + + await appRequest.put(path); + + expect(controller.update).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(1); + expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateRoleCode).toBeCalledTimes(1); + }); +}); diff --git a/app/tests/unit/forms/user/routes.spec.js b/app/tests/unit/forms/user/routes.spec.js index f867d0fbf..891374970 100644 --- a/app/tests/unit/forms/user/routes.spec.js +++ b/app/tests/unit/forms/user/routes.spec.js @@ -13,10 +13,11 @@ const controller = require('../../../../src/forms/user/controller'); // correctly, not the functionality of the middleware. // +const mockJwtServiceProtect = jest.fn((_req, _res, next) => { + next(); +}); jwtService.protect = jest.fn(() => { - return jest.fn((_req, _res, next) => { - next(); - }); + return mockJwtServiceProtect; }); userAccess.currentUser = jest.fn((_req, _res, next) => { @@ -43,6 +44,25 @@ afterEach(() => { jest.clearAllMocks(); }); +// jwtService.protect is tricky to test. This test is fragile as the file could +// change and have routes that need admin and others that don't. This test only +// works when we use(protect) at the file level, not individually in the routes. +// However, this does test that we don't accidentally turn off the protection. +describe('jwtService.protect', () => { + it('should be called with no argument', () => { + jest.resetModules(); + const jwtService = require('../../../../src/components/jwtService'); + jwtService.protect = jest.fn(() => { + return jest.fn((_req, _res, next) => { + next(); + }); + }); + require('../../../../src/forms/user/routes'); + + expect(jwtService.protect).toBeCalledWith(); + }); +}); + describe(`${basePath}`, () => { const path = `${basePath}`; @@ -54,6 +74,7 @@ describe(`${basePath}`, () => { await appRequest.get(path); expect(controller.list).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateFormId).toBeCalledTimes(0); expect(validateParameter.validateUserId).toBeCalledTimes(0); @@ -72,6 +93,7 @@ describe(`${basePath}/:userId`, () => { await appRequest.get(path); expect(controller.read).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateFormId).toBeCalledTimes(0); expect(validateParameter.validateUserId).toBeCalledTimes(1); @@ -89,6 +111,7 @@ describe(`${basePath}/labels`, () => { await appRequest.get(path); expect(controller.readUserLabels).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateFormId).toBeCalledTimes(0); expect(validateParameter.validateUserId).toBeCalledTimes(0); @@ -102,6 +125,7 @@ describe(`${basePath}/labels`, () => { await appRequest.put(path); expect(controller.updateUserLabels).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateFormId).toBeCalledTimes(0); expect(validateParameter.validateUserId).toBeCalledTimes(0); @@ -119,6 +143,7 @@ describe(`${basePath}/preferences`, () => { await appRequest.delete(path); expect(controller.deleteUserPreferences).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateFormId).toBeCalledTimes(0); expect(validateParameter.validateUserId).toBeCalledTimes(0); @@ -132,6 +157,7 @@ describe(`${basePath}/preferences`, () => { await appRequest.get(path); expect(controller.readUserPreferences).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateFormId).toBeCalledTimes(0); expect(validateParameter.validateUserId).toBeCalledTimes(0); @@ -145,6 +171,7 @@ describe(`${basePath}/preferences`, () => { await appRequest.put(path); expect(controller.updateUserPreferences).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateFormId).toBeCalledTimes(0); expect(validateParameter.validateUserId).toBeCalledTimes(0); @@ -163,6 +190,7 @@ describe(`${basePath}/preferences/forms/:formId`, () => { await appRequest.delete(path); expect(controller.deleteUserFormPreferences).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateUserId).toBeCalledTimes(0); @@ -176,6 +204,7 @@ describe(`${basePath}/preferences/forms/:formId`, () => { await appRequest.get(path); expect(controller.readUserFormPreferences).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateUserId).toBeCalledTimes(0); @@ -189,6 +218,7 @@ describe(`${basePath}/preferences/forms/:formId`, () => { await appRequest.put(path); expect(controller.updateUserFormPreferences).toBeCalledTimes(1); + expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); expect(validateParameter.validateFormId).toBeCalledTimes(1); expect(validateParameter.validateUserId).toBeCalledTimes(0); diff --git a/app/tests/unit/routes/v1/admin.spec.js b/app/tests/unit/routes/v1/admin.spec.js index 2160fe638..74de623df 100644 --- a/app/tests/unit/routes/v1/admin.spec.js +++ b/app/tests/unit/routes/v1/admin.spec.js @@ -1,16 +1,16 @@ -const request = require('supertest'); const Problem = require('api-problem'); +const request = require('supertest'); +const uuid = require('uuid'); const { expressHelper } = require('../../../common/helper'); -// -// mock middleware -// const jwtService = require('../../../../src/components/jwtService'); +const validateParameter = require('../../../../src/forms/common/middleware/validateParameter'); // -// test assumes that caller has appropriate token, we are not testing middleware here... +// mock middleware // + jwtService.protect = jest.fn(() => { return jest.fn((_req, _res, next) => { next(); @@ -22,6 +22,10 @@ userAccess.currentUser = jest.fn((_req, _res, next) => { next(); }); +validateParameter.validateFormVersionId = jest.fn((_req, _res, next) => { + next(); +}); + // // we will mock the underlying data service calls... // @@ -46,7 +50,8 @@ afterEach(() => { }); describe(`${basePath}/formcomponents/proactivehelp/imageUrl/:componentId`, () => { - const path = `${basePath}/formcomponents/proactivehelp/imageUrl/:componentId`; + const componentId = uuid.v4(); + const path = `${basePath}/formcomponents/proactivehelp/imageUrl/${componentId}`; describe('GET', () => { it('should return 200', async () => { @@ -176,7 +181,8 @@ describe(`${basePath}/formcomponents/proactivehelp/object`, () => { }); describe(`${basePath}/formcomponents/proactivehelp/:publishStatus/:componentId`, () => { - const path = `${basePath}/formcomponents/proactivehelp/:publishStatus/:componentId`; + const componentId = uuid.v4(); + const path = `${basePath}/formcomponents/proactivehelp/:publishStatus/${componentId}`; describe('PUT', () => { it('should return 200', async () => { @@ -266,7 +272,8 @@ describe(`${basePath}/forms`, () => { }); describe(`${basePath}/forms/:formId`, () => { - const path = `${basePath}/forms/:formId`; + const formId = uuid.v4(); + const path = `${basePath}/forms/${formId}`; describe('GET', () => { it('should return 200', async () => { @@ -306,7 +313,8 @@ describe(`${basePath}/forms/:formId`, () => { }); describe(`${basePath}/forms/:formId/addUser`, () => { - const path = `${basePath}/forms/:formId/addUser`; + const formId = uuid.v4(); + const path = `${basePath}/forms/${formId}/addUser`; describe('PUT', () => { it('should return 200', async () => { @@ -315,7 +323,7 @@ describe(`${basePath}/forms/:formId/addUser`, () => { const response = await appRequest.put(path).query({ userId: '123' }).send({ userId: '123' }); - expect(rbacService.setFormUsers).toBeCalledWith(':formId', '123', { userId: '123' }, undefined); + expect(rbacService.setFormUsers).toBeCalledWith(formId, '123', { userId: '123' }, undefined); expect(response.statusCode).toBe(200); expect(response.body).toBeTruthy(); }); @@ -357,7 +365,8 @@ describe(`${basePath}/forms/:formId/addUser`, () => { }); describe(`${basePath}/forms/:formId/apiKey`, () => { - const path = `${basePath}/forms/:formId/apiKey`; + const formId = uuid.v4(); + const path = `${basePath}/forms/${formId}/apiKey`; describe('DELETE', () => { it('should return 204', async () => { @@ -433,7 +442,8 @@ describe(`${basePath}/forms/:formId/apiKey`, () => { }); describe(`${basePath}/forms/:formId/formUsers`, () => { - const path = `${basePath}/forms/:formId/formUsers`; + const formId = uuid.v4(); + const path = `${basePath}/forms/${formId}/formUsers`; describe('GET', () => { it('should return 200', async () => { @@ -473,7 +483,8 @@ describe(`${basePath}/forms/:formId/formUsers`, () => { }); describe(`${basePath}/forms/:formId/restore`, () => { - const path = `${basePath}/forms/:formId/restore`; + const formId = uuid.v4(); + const path = `${basePath}/forms/${formId}/restore`; describe('PUT', () => { it('should return 200', async () => { @@ -513,7 +524,9 @@ describe(`${basePath}/forms/:formId/restore`, () => { }); describe(`${basePath}/forms/:formId/versions/:formVersionId`, () => { - const path = `${basePath}/forms/:formId/versions/:formVersionId`; + const formId = uuid.v4(); + const formVersionId = uuid.v4(); + const path = `${basePath}/forms/${formId}/versions/${formVersionId}`; describe('GET', () => { it('should return 200', async () => { @@ -593,7 +606,8 @@ describe(`${basePath}/users`, () => { }); describe(`${basePath}/users/:userId`, () => { - const path = `${basePath}/users/:userId`; + const userId = uuid.v4(); + const path = `${basePath}/users/${userId}`; describe('GET', () => { it('should return 200', async () => { diff --git a/app/tests/unit/routes/v1/form.spec.js b/app/tests/unit/routes/v1/form.spec.js index c2a0fa4bf..750f6363f 100644 --- a/app/tests/unit/routes/v1/form.spec.js +++ b/app/tests/unit/routes/v1/form.spec.js @@ -1,5 +1,5 @@ -const request = require('supertest'); const Problem = require('api-problem'); +const request = require('supertest'); const uuid = require('uuid'); const { expressHelper } = require('../../../common/helper'); diff --git a/app/tests/unit/routes/v1/permission.spec.js b/app/tests/unit/routes/v1/permission.spec.js index ebcc1038b..b81d4f99b 100644 --- a/app/tests/unit/routes/v1/permission.spec.js +++ b/app/tests/unit/routes/v1/permission.spec.js @@ -1,16 +1,14 @@ const request = require('supertest'); const Problem = require('api-problem'); +const jwtService = require('../../../../src/components/jwtService'); const { expressHelper } = require('../../../common/helper'); +const constants = require('../../../../src/forms/common/constants'); // // mock middleware // -const jwtService = require('../../../../src/components/jwtService'); -// -// test assumes that caller has appropriate token, we are not testing middleware here... -// jwtService.protect = jest.fn(() => { return jest.fn((_req, _res, next) => { next(); @@ -118,7 +116,7 @@ describe(`${basePath}`, () => { }); describe(`${basePath}/:code`, () => { - const path = `${basePath}/:code`; + const path = `${basePath}/${constants.Permissions.DESIGN_CREATE}`; describe('GET', () => { it('should return 200', async () => { diff --git a/app/tests/unit/routes/v1/role.spec.js b/app/tests/unit/routes/v1/role.spec.js index 83c107f16..12a75985c 100644 --- a/app/tests/unit/routes/v1/role.spec.js +++ b/app/tests/unit/routes/v1/role.spec.js @@ -1,16 +1,14 @@ const request = require('supertest'); const Problem = require('api-problem'); +const jwtService = require('../../../../src/components/jwtService'); const { expressHelper } = require('../../../common/helper'); +const constants = require('../../../../src/forms/common/constants'); // // mock middleware // -const jwtService = require('../../../../src/components/jwtService'); -// -// test assumes that caller has appropriate token, we are not testing middleware here... -// jwtService.protect = jest.fn(() => { return jest.fn((_req, _res, next) => { next(); @@ -117,8 +115,8 @@ describe(`${basePath}`, () => { }); }); -describe(`${basePath}/role`, () => { - const path = `${basePath}/role`; +describe(`${basePath}/:code`, () => { + const path = `${basePath}/${constants.Roles.FORM_DESIGNER}`; describe('GET', () => { it('should return 200', async () => { diff --git a/openshift/README.md b/openshift/README.md index fb633ae36..58c8362f6 100644 --- a/openshift/README.md +++ b/openshift/README.md @@ -255,6 +255,6 @@ oc process -f backup-cronjob-verify.yaml \ -p JOB_NAME=backup-postgres-verify \ -p JOB_PERSISTENT_STORAGE_NAME=$PVC \ -p SCHEDULE="0 9 * * *" \ - -p TAG_NAME=2.6.1 \ + -p TAG_NAME=2.8.0 \ | oc -n $NAMESPACE apply -f - ``` diff --git a/openshift/redash/README.md b/openshift/redash/README.md index d723f51bd..ce19c7fa8 100644 --- a/openshift/redash/README.md +++ b/openshift/redash/README.md @@ -26,7 +26,7 @@ oc process -f backup-cronjob.yaml \ -p JOB_PERSISTENT_STORAGE_NAME=backup-chefs-redash-postgresql \ -p MONTHLY_BACKUPS=3 \ -p SCHEDULE="0 8 * * *" \ - -p TAG_NAME=2.6.1 \ + -p TAG_NAME=2.8.0 \ -p WEEKLY_BACKUPS=8 \ | oc -n a12c97-tools apply -f - ``` @@ -42,7 +42,7 @@ oc process -f backup-cronjob-verify.yaml \ -p JOB_NAME=backup-chefs-redash-postgres-verify \ -p JOB_PERSISTENT_STORAGE_NAME=backup-chefs-redash-postgresql \ -p SCHEDULE="0 9 * * *" \ - -p TAG_NAME=2.6.1 \ + -p TAG_NAME=2.8.0 \ | oc -n a12c97-tools apply -f - ``` diff --git a/tests/functional/cypress/e2e/form-apikey-cdogs.cy.js b/tests/functional/cypress/e2e/form-apikey-cdogs.cy.js index ca4ced762..aed9c8c8a 100644 --- a/tests/functional/cypress/e2e/form-apikey-cdogs.cy.js +++ b/tests/functional/cypress/e2e/form-apikey-cdogs.cy.js @@ -119,6 +119,9 @@ describe('Form Designer', () => { cy.get('[data-test="canRemoveForm"]').click(); cy.get('[data-test="continue-btn-continue"]').click(); + cy.waitForLoad(); + cy.waitForLoad(); + cy.get('#logoutButton > .v-btn__content > span').click(); }) diff --git a/tests/functional/cypress/e2e/form-design-advanceddata.cy.js b/tests/functional/cypress/e2e/form-design-advanceddata.cy.js index b5a5b3bcf..522e32b29 100644 --- a/tests/functional/cypress/e2e/form-design-advanceddata.cy.js +++ b/tests/functional/cypress/e2e/form-design-advanceddata.cy.js @@ -227,6 +227,7 @@ it('Checks the Container component', () => { //cy.get('.mdi-delete').click(); cy.get(':nth-child(5) > .v-btn > .v-btn__content > .mdi-delete').click(); cy.get('[data-test="continue-btn-continue"]').click(); + cy.get('#logoutButton > .v-btn__content > span').click(); }) diff --git a/tests/functional/cypress/e2e/form-design-advancedfield.cy.js b/tests/functional/cypress/e2e/form-design-advancedfield.cy.js index 1530de35c..eecdb660d 100644 --- a/tests/functional/cypress/e2e/form-design-advancedfield.cy.js +++ b/tests/functional/cypress/e2e/form-design-advancedfield.cy.js @@ -320,6 +320,7 @@ describe('Form Designer', () => { cy.waitForLoad(); cy.get('.mdi-delete').click(); cy.get('[data-test="continue-btn-continue"]').click(); + cy.get('#logoutButton > .v-btn__content > span').click(); }) diff --git a/tests/functional/cypress/e2e/form-design-basicfields.cy.js b/tests/functional/cypress/e2e/form-design-basicfields.cy.js index e6a0854a3..822fa4146 100644 --- a/tests/functional/cypress/e2e/form-design-basicfields.cy.js +++ b/tests/functional/cypress/e2e/form-design-basicfields.cy.js @@ -243,14 +243,15 @@ describe('Form Designer', () => { cy.get('button').contains('Save').click(); }); - cy.get('[ref=removeComponent]').then($el => { + //cy.get('[ref=removeComponent]').then($el => { - const rem=$el[11]; - rem.click(); + // const rem=$el[11]; + //rem.click(); - }); - + //}); + cy.waitForLoad(); + cy.waitForLoad(); cy.get('[data-cy=saveButton]').click(); cy.waitForLoad(); @@ -288,9 +289,11 @@ describe('Form Designer', () => { cy.visit(`/${depEnv}/form/design?d=${arrayValues[0]}&f=${dval[0]}`); cy.waitForLoad(); cy.waitForLoad(); - //cy.get('[data-cy="settingsRouterLink"] > .text').click(); + cy.get('[data-cy="settingsRouterLink"] > .v-btn').click(); - cy.get('.mt-6 > :nth-child(2) > div > :nth-child(5) > .v-btn').click(); + cy.waitForLoad(); + cy.get('[data-test="canRemoveForm"]').click(); + cy.get('[data-test="continue-btn-continue"]').click(); }); diff --git a/tests/functional/cypress/e2e/form-design-basiclayout-advanced-layout.cy.js b/tests/functional/cypress/e2e/form-design-basiclayout-advanced-layout.cy.js index 91b889e2c..1f9b63ab5 100644 --- a/tests/functional/cypress/e2e/form-design-basiclayout-advanced-layout.cy.js +++ b/tests/functional/cypress/e2e/form-design-basiclayout-advanced-layout.cy.js @@ -257,6 +257,7 @@ it('Checks the Table', () => { //cy.get('.mdi-delete').click(); cy.get(':nth-child(5) > .v-btn > .v-btn__content > .mdi-delete').click(); cy.get('[data-test="continue-btn-continue"]').click(); + cy.get('#logoutButton > .v-btn__content > span').click(); }) diff --git a/tests/functional/cypress/e2e/form-edit-submission-data.cy.js b/tests/functional/cypress/e2e/form-edit-submission-data.cy.js new file mode 100644 index 000000000..88f429f37 --- /dev/null +++ b/tests/functional/cypress/e2e/form-edit-submission-data.cy.js @@ -0,0 +1,210 @@ +import 'cypress-keycloak-commands'; +import 'cypress-drag-drop'; +import { formsettings } from '../support/login.js'; + +const depEnv = Cypress.env('depEnv'); + + +Cypress.Commands.add('waitForLoad', () => { + const loaderTimeout = 60000; + + cy.get('.nprogress-busy', { timeout: loaderTimeout }).should('not.exist'); +}); +describe('Form Designer', () => { + + beforeEach(()=>{ + + + cy.on('uncaught:exception', (err, runnable) => { + // Form.io throws an uncaught exception for missing projectid + // Cypress catches it as undefined: undefined so we can't get the text + console.log(err); + return false; + }); + }); + it('Visits the form settings page', () => { + + + cy.viewport(1000, 1100); + cy.waitForLoad(); + formsettings(); + + }); + it('Add some fields for submission', () => { + + cy.viewport(1000, 1800); + cy.waitForLoad(); + cy.get('button').contains('Basic Fields').click(); + let textFields = ["First Name", "Middle Name", "Last Name"]; + + for(let i=0; i { + const bounds = $el[0].getBoundingClientRect(); + cy.get('span.btn').contains('Text Field') + .trigger('mousedown', { which: 1}, { force: true }) + .trigger('mousemove', bounds.x, -100, { force: true }) + .trigger('mouseup', { force: true }); + cy.get('p').contains('Text Field Component'); + cy.get('input[name="data[label]"]').clear(); + cy.get('input[name="data[label]"]').clear(); + cy.get('input[name="data[label]"]').type(textFields[i]); + cy.get('button').contains('Save').click(); + }); + } + + + }); + it('Form Submission and Updation', () => { + cy.viewport(1000, 1100); + cy.waitForLoad(); + cy.waitForLoad(); + cy.get('div.formio-builder-form').then($el => { + const coords2 = $el[0].getBoundingClientRect(); + cy.get('span.btn').contains('Checkbox') + + .trigger('mousedown', { which: 1}, { force: true }) + .trigger('mousemove', coords2.x, -50, { force: true }) + .trigger('mouseup', { force: true }); + cy.get('p').contains('Checkbox Component'); + cy.get('input[name="data[label]"]').clear(); + cy.get('input[name="data[label]"]').clear(); + cy.get('input[name="data[label]"]').type('Applying for self'); + cy.get('button').contains('Save').click(); + }); + cy.intercept('GET', `/${depEnv}/api/v1/forms/*`).as('getForm'); + // Form saving + let savedButton = cy.get('[data-cy=saveButton]'); + expect(savedButton).to.not.be.null; + savedButton.trigger('click'); + cy.waitForLoad(); + + + + // Go to My forms + cy.wait('@getForm').then(()=>{ + let userFormsLinks = cy.get('[data-cy=userFormsLinks]'); + expect(userFormsLinks).to.not.be.null; + userFormsLinks.trigger('click'); + }); + // Filter the newly created form + cy.location('search').then(search => { + //let pathName = fullUrl.pathname + let arr = search.split('='); + let arrayValues = arr[1].split('&'); + cy.log(arrayValues[0]); + //cy.log(arrayValues[1]); + //cy.log(arrayValues[2]); + cy.visit(`/${depEnv}/form/manage?f=${arrayValues[0]}`); + cy.waitForLoad(); + + + //Publish the form + cy.get('.v-label > span').click(); + + cy.get('span').contains('Publish Version 1'); + + cy.contains('Continue').should('be.visible'); + cy.contains('Continue').trigger('click'); + //Submit the form + cy.visit(`/${depEnv}/form/submit?f=${arrayValues[0]}`); + cy.waitForLoad(); + cy.waitForLoad(); + cy.waitForLoad(); + cy.get('button').contains('Submit').should('be.visible'); + cy.waitForLoad(); + cy.waitForLoad(); + cy.get('input[name="data[simpletextfield1]"').click(); + cy.get('input[name="data[simpletextfield1]"').type('Alex'); + cy.get('input[name="data[simpletextfield2]"').click(); + cy.get('input[name="data[simpletextfield2]"').type('Smith'); + //cy.get('.form-check-input').click(); + //form submission + cy.get('button').contains('Submit').click(); + cy.waitForLoad(); + cy.get('button').contains('Submit').click(); + cy.waitForLoad(); + cy.get('button').contains('Submit').click(); + cy.waitForLoad(); + cy.waitForLoad(); + cy.waitForLoad(); + cy.get('label').contains('First Name').should('be.visible'); + cy.get('label').contains('Last Name').should('be.visible'); + cy.get('label').contains('Applying for self').should('be.visible'); + + + //Update submission + cy.visit(`/${depEnv}/form/manage?f=${arrayValues[0]}`); + cy.waitForLoad(); + cy.waitForLoad(); + cy.visit(`/${depEnv}/form/submit?f=${arrayValues[0]}`); + cy.waitForLoad(); + cy.waitForLoad(); + cy.waitForLoad(); + cy.get('button').contains('Submit').should('be.visible'); + cy.get('input[name="data[simpletextfield1]"').click(); + cy.get('input[name="data[simpletextfield1]"').type('Alex'); + cy.get('input[name="data[simpletextfield2]"').click(); + cy.get('input[name="data[simpletextfield2]"').type('Smith'); + cy.get('button').contains('Submit').click(); + cy.waitForLoad(); + cy.get('button').contains('Submit').click(); + cy.get('[data-test="continue-btn-continue"]').click(); + cy.get('label').contains('First Name').should('be.visible'); + cy.get('label').contains('Last Name').should('be.visible'); + cy.get('label').contains('Applying for self').should('be.visible') + //view submission + cy.visit(`/${depEnv}/form/manage?f=${arrayValues[0]}`); + cy.waitForLoad(); + cy.waitForLoad(); + + cy.get('.mdi-list-box-outline').click(); + cy.waitForLoad(); + cy.get(':nth-child(1) > :nth-child(6) > a > .v-btn > .v-btn__content > .mdi-eye').click(); + //Edit submission + cy.get('.mdi-pencil').click(); + //check visibility of cancel button + cy.get('.v-col-2 > .v-btn').should('be.visible'); + cy.get('button').contains('Submit').should('be.visible'); + + //Edit submission data + cy.get('input[name="data[simpletextfield1]"').click(); + cy.get('input[name="data[simpletextfield1]"').clear(); + cy.get('input[name="data[simpletextfield1]"').type('Nancy'); + cy.get('input[name="data[simpletextfield2]"').click(); + cy.get('input[name="data[simpletextfield2]"').type('Smith'); + cy.get('button').contains('Submit').click(); + cy.waitForLoad(); + cy.get('[data-test="continue-btn-continue"]').click(); + cy.waitForLoad(); + cy.waitForLoad(); + cy.get('label').contains('First Name').should('be.visible'); + cy.get('label').contains('Last Name').should('be.visible'); + //Adding notes to submission + cy.get('.mdi-plus').click(); + cy.get('div').find('textarea').then($el => { + + const rem=$el[0]; + rem.click(); + cy.get(rem).type('some notes'); + + + }); + cy.get('[data-test="canCancelNote"]').should('be.visible'); + cy.get('[data-test="btn-add-note"]').click(); + cy.get('.notes-text').contains('1'); + //Delete form after test run + cy.visit(`/${depEnv}/form/manage?f=${arrayValues[0]}`); + cy.waitForLoad(); + cy.waitForLoad(); + cy.get('.mdi-delete').click(); + cy.get('[data-test="continue-btn-continue"]').click(); + cy.get('#logoutButton > .v-btn__content > span').click(); + + + }) + + + }); + +}); \ No newline at end of file diff --git a/tests/functional/cypress/e2e/form-manage-form.cy.js b/tests/functional/cypress/e2e/form-manage-form.cy.js index 03da3a0c2..297fe5a84 100644 --- a/tests/functional/cypress/e2e/form-manage-form.cy.js +++ b/tests/functional/cypress/e2e/form-manage-form.cy.js @@ -202,6 +202,7 @@ describe('Form Designer', () => { //Delete form after test run cy.get('.mdi-delete').click(); cy.get('[data-test="continue-btn-continue"]').click(); + cy.get('#logoutButton > .v-btn__content > span').click(); diff --git a/tests/functional/cypress/e2e/form-simple-form-publish.cy.js b/tests/functional/cypress/e2e/form-simple-form-publish.cy.js index 0eb43f789..269bcf60e 100644 --- a/tests/functional/cypress/e2e/form-simple-form-publish.cy.js +++ b/tests/functional/cypress/e2e/form-simple-form-publish.cy.js @@ -105,6 +105,7 @@ describe('Form Designer', () => { //cy.get('.mdi-delete').click(); cy.get(':nth-child(5) > .v-btn > .v-btn__content > .mdi-delete').click(); cy.get('[data-test="continue-btn-continue"]').click(); + cy.get('#logoutButton > .v-btn__content > span').click(); }); diff --git a/tests/functional/cypress/e2e/form-team-management.cy.js b/tests/functional/cypress/e2e/form-team-management.cy.js index 93e2be68c..8575e261b 100644 --- a/tests/functional/cypress/e2e/form-team-management.cy.js +++ b/tests/functional/cypress/e2e/form-team-management.cy.js @@ -176,6 +176,7 @@ describe('Form Designer', () => { //Delete form after test run cy.get('.mdi-delete').click(); cy.get('[data-test="continue-btn-continue"]').click(); + cy.get('#logoutButton > .v-btn__content > span').click(); }) });