From 6c9b36df995de12c25b4b080ebb9bb32a8db9bd9 Mon Sep 17 00:00:00 2001 From: Walter Moar Date: Tue, 20 Aug 2024 13:04:08 -0700 Subject: [PATCH 01/23] fix: FORMS-1448 validate permission and role codes (#1481) --- app/src/forms/common/constants.js | 22 +-- .../common/middleware/validateParameter.js | 47 ++++++ app/src/forms/file/routes.js | 10 +- app/src/forms/form/routes.js | 6 +- app/src/forms/permission/routes.js | 6 +- app/src/forms/proxy/routes.js | 11 +- app/src/forms/public/routes.js | 1 + app/src/forms/rbac/routes.js | 4 +- app/src/forms/role/routes.js | 6 +- app/src/forms/submission/routes.js | 1 - app/src/forms/utils/routes.js | 2 +- .../middleware/validateParameter.spec.js | 146 +++++++++++++++++- .../unit/forms/permission/routes.spec.js | 9 ++ app/tests/unit/forms/role/routes.spec.js | 9 ++ app/tests/unit/routes/v1/permission.spec.js | 8 +- app/tests/unit/routes/v1/role.spec.js | 10 +- 16 files changed, 249 insertions(+), 49 deletions(-) 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 f9f709fab..e917cb61e 100644 --- a/app/src/forms/common/middleware/validateParameter.js +++ b/app/src/forms/common/middleware/validateParameter.js @@ -1,6 +1,7 @@ const Problem = require('api-problem'); const uuid = require('uuid'); +const constants = require('../../common/constants'); const externalApiService = require('../../form/externalApi/service'); const formService = require('../../form/service'); const submissionService = require('../../submission/service'); @@ -210,6 +211,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. * @@ -237,5 +282,7 @@ module.exports = { validateFormSubmissionId, validateFormVersionDraftId, validateFormVersionId, + validatePermissionCode, + validateRoleCode, validateUserId, }; 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/public/routes.js b/app/src/forms/public/routes.js index c72618a40..d8f31ae3d 100644 --- a/app/src/forms/public/routes.js +++ b/app/src/forms/public/routes.js @@ -1,4 +1,5 @@ const routes = require('express').Router(); + const controller = require('./controller'); routes.use('/reminder', (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/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/permission/routes.spec.js b/app/tests/unit/forms/permission/routes.spec.js index d64f5afa9..dc77ea400 100644 --- a/app/tests/unit/forms/permission/routes.spec.js +++ b/app/tests/unit/forms/permission/routes.spec.js @@ -4,6 +4,7 @@ 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'); // @@ -22,6 +23,10 @@ userAccess.currentUser = jest.fn((_req, _res, next) => { next(); }); +validateParameter.validatePermissionCode = jest.fn((_req, _res, next) => { + next(); +}); + // // Create the router and a simple Express server. // @@ -67,6 +72,7 @@ describe(`${basePath}`, () => { 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 () => { @@ -79,6 +85,7 @@ describe(`${basePath}`, () => { expect(controller.create).toBeCalledTimes(1); expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validatePermissionCode).toBeCalledTimes(0); }); }); @@ -95,6 +102,7 @@ describe(`${basePath}/:code`, () => { 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 () => { @@ -107,5 +115,6 @@ describe(`${basePath}/:code`, () => { 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/role/routes.spec.js b/app/tests/unit/forms/role/routes.spec.js index 2a5c92267..f9eba8c83 100644 --- a/app/tests/unit/forms/role/routes.spec.js +++ b/app/tests/unit/forms/role/routes.spec.js @@ -4,6 +4,7 @@ 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'); // @@ -22,6 +23,10 @@ userAccess.currentUser = jest.fn((_req, _res, next) => { next(); }); +validateParameter.validateRoleCode = jest.fn((_req, _res, next) => { + next(); +}); + // // Create the router and a simple Express server. // @@ -67,6 +72,7 @@ describe(`${basePath}`, () => { 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 () => { @@ -79,6 +85,7 @@ describe(`${basePath}`, () => { expect(controller.create).toBeCalledTimes(1); expect(mockJwtServiceProtect).toBeCalledTimes(1); expect(userAccess.currentUser).toBeCalledTimes(1); + expect(validateParameter.validateRoleCode).toBeCalledTimes(0); }); }); @@ -95,6 +102,7 @@ describe(`${basePath}/:code`, () => { 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 () => { @@ -107,5 +115,6 @@ describe(`${basePath}/:code`, () => { 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/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 () => { From d7f978045945eb0dc3402c5b40b6b904fa7f89ed Mon Sep 17 00:00:00 2001 From: Walter Moar Date: Tue, 20 Aug 2024 16:07:20 -0700 Subject: [PATCH 02/23] refactor: FORMS-1488 new middleware for reminder route (#1482) * fix: FORMS-1448 new middleware for reminder route * fix test comments * fixed a comment that could cause confusion --- app/src/forms/public/middleware/apiAccess.js | 35 ++++++++++ app/src/forms/public/routes.js | 23 +------ .../forms/public/middleware/apiAccess.spec.js | 66 +++++++++++++++++++ app/tests/unit/forms/public/routes.spec.js | 55 ++++------------ 4 files changed, 115 insertions(+), 64 deletions(-) create mode 100644 app/src/forms/public/middleware/apiAccess.js create mode 100644 app/tests/unit/forms/public/middleware/apiAccess.spec.js 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 d8f31ae3d..431c235c3 100644 --- a/app/src/forms/public/routes.js +++ b/app/src/forms/public/routes.js @@ -1,28 +1,9 @@ const routes = require('express').Router(); +const apiAccess = require('./middleware/apiAccess'); 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 }); - } -}); - -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/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); }); }); From b7411d68f24cb47734da478e421ea7896ec9e410 Mon Sep 17 00:00:00 2001 From: Walter Moar Date: Wed, 21 Aug 2024 08:20:50 -0700 Subject: [PATCH 03/23] build: FORMS-887 update backup images (#1469) * build: FORMS-887 update backup images * fix: 2.9.0 is buggy so use 2.8.0 * docs: update backup image version --- openshift/README.md | 2 +- openshift/redash/README.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) 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 - ``` From 22dbda9923d65649e3e8c3961986395cddc8a316 Mon Sep 17 00:00:00 2001 From: Walter Moar Date: Thu, 22 Aug 2024 09:04:14 -0700 Subject: [PATCH 04/23] ci: FORMS-1449 code climate test coverage (#1483) --- .codeclimate.yml | 10 +++++----- .github/workflows/unit-tests.yaml | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.codeclimate.yml b/.codeclimate.yml index c81eb7960..fd52aa6bf 100644 --- a/.codeclimate.yml +++ b/.codeclimate.yml @@ -1,18 +1,18 @@ version: "2" exclude_patterns: - config/ - - "**/db/" - dist/ - features/ - - "**/node_modules/" - script/ + - Tests/ + - "**/*.d.ts" + - "**/*_test.go" + - "**/db/" + - "**/node_modules/" - "**/spec/" - "**/test/" - "**/tests/" - - Tests/ - "**/vendor/" - - "**/*_test.go" - - "**/*.d.ts" plugins: csslint: enabled: true diff --git a/.github/workflows/unit-tests.yaml b/.github/workflows/unit-tests.yaml index 9e03ebfea..407a63a58 100644 --- a/.github/workflows/unit-tests.yaml +++ b/.github/workflows/unit-tests.yaml @@ -142,5 +142,5 @@ jobs: CC_TEST_REPORTER_ID: ${{ secrets.CC_TEST_REPORTER_ID }} with: coverageLocations: | - ${{ github.workspace }}/**/lcov.info:lcov + ${{ github.workspace }}/**/clover.xml:clover prefix: ${{ github.workplace }} From fdd1ff57a407cbe34fe307c3c8e5b8fb83afc5f8 Mon Sep 17 00:00:00 2001 From: Vijaivir Dhaliwal <91633223+vijaivir@users.noreply.github.com> Date: Thu, 22 Aug 2024 11:06:04 -0700 Subject: [PATCH 05/23] Fix: Frontend spacing issues (#1470) * fix spacing issues * run npm * added spacing test * remove console log * make spacing more obvious * added tests * fixed spacing for submission period and repeat period * fix another spacing issue --- .../settings/FormScheduleSettings.vue | 20 ++--- .../forms/manage/TeamManagement.vue | 1 + .../submission/ManageSubmissionUsers.vue | 2 + .../settings/FormScheduleSettings.spec.js | 74 +++++++++++++++++++ .../forms/manage/TeamManagement.spec.js | 2 +- .../submission/ManageSubmissionUsers.spec.js | 2 +- 6 files changed, 90 insertions(+), 11 deletions(-) diff --git a/app/frontend/src/components/designer/settings/FormScheduleSettings.vue b/app/frontend/src/components/designer/settings/FormScheduleSettings.vue index 872bc187d..68909117f 100644 --- a/app/frontend/src/components/designer/settings/FormScheduleSettings.vue +++ b/app/frontend/src/components/designer/settings/FormScheduleSettings.vue @@ -555,7 +555,7 @@ defineExpose({ cols="12" md="12" > - + {{ $t('trans.formSettings.submissionsOpenDateRange') }} {{ form.schedule.openSubmissionDateTime }} {{ $t('trans.formSettings.to') }} @@ -567,9 +567,7 @@ defineExpose({ AVAILABLE_DATES[0]['closeDate'] && AVAILABLE_DATES[0]['closeDate'].split(' ')[0] : '' - }} - - {{ + }}{{ form.schedule.scheduleType === SCHEDULE_TYPE.CLOSINGDATE ? form.schedule.closeSubmissionDateTime : '' @@ -577,11 +575,13 @@ defineExpose({ - {{ + {{ form.schedule.allowLateSubmissions.enabled && form.schedule.allowLateSubmissions.forNext.intervalType && form.schedule.allowLateSubmissions.forNext.term - ? $t('trans.formSettings.allowLateSubmissnInterval') + + ? ' ' + + $t('trans.formSettings.allowLateSubmissnInterval') + + ' ' + form.schedule.allowLateSubmissions.forNext.term + ' ' + form.schedule.allowLateSubmissions.forNext.intervalType + @@ -599,9 +599,11 @@ defineExpose({ AVAILABLE_DATES[1] " :lang="locale" - >{{ $t('trans.formSettings.scheduleRepetition') }} - {{ form.schedule.repeatSubmission.everyTerm }} - {{ form.schedule.repeatSubmission.everyIntervalType }} + >{{ ' ' + $t('trans.formSettings.scheduleRepetition') }} + + {{ form.schedule.repeatSubmission.everyTerm }} + {{ form.schedule.repeatSubmission.everyIntervalType }} + {{ $t('trans.formSettings.until') }} {{ form.schedule.repeatSubmission.repeatUntil }}. diff --git a/app/frontend/src/components/forms/manage/TeamManagement.vue b/app/frontend/src/components/forms/manage/TeamManagement.vue index 335e9b1a4..bd9a8d927 100644 --- a/app/frontend/src/components/forms/manage/TeamManagement.vue +++ b/app/frontend/src/components/forms/manage/TeamManagement.vue @@ -295,6 +295,7 @@ function addNewUsers(users, roles) { notificationStore.addNotification({ text: `${user.username}@${user.idpCode}` + + ' ' + t('trans.teamManagement.idpMessage'), }); } diff --git a/app/frontend/src/components/forms/submission/ManageSubmissionUsers.vue b/app/frontend/src/components/forms/submission/ManageSubmissionUsers.vue index d14b40971..b62ce74e7 100644 --- a/app/frontend/src/components/forms/submission/ManageSubmissionUsers.vue +++ b/app/frontend/src/components/forms/submission/ManageSubmissionUsers.vue @@ -174,8 +174,10 @@ async function modifyPermissions(userId, permissions) { ...NotificationTypes.SUCCESS, text: permissions.length ? t('trans.manageSubmissionUsers.sentInviteEmailTo') + + ' ' + `${selectedEmail}` : t('trans.manageSubmissionUsers.sentUninvitedEmailTo') + + ' ' + `${selectedEmail}`, }); } diff --git a/app/frontend/tests/unit/components/designer/settings/FormScheduleSettings.spec.js b/app/frontend/tests/unit/components/designer/settings/FormScheduleSettings.spec.js index 2fd74f621..bb5a72f2b 100644 --- a/app/frontend/tests/unit/components/designer/settings/FormScheduleSettings.spec.js +++ b/app/frontend/tests/unit/components/designer/settings/FormScheduleSettings.spec.js @@ -4,6 +4,8 @@ import { setActivePinia } from 'pinia'; import moment from 'moment'; import { beforeEach, describe, expect, it } from 'vitest'; import { ref } from 'vue'; +import { useI18n } from 'vue-i18n'; +const { t } = useI18n({ useScope: 'global' }); import { useFormStore } from '~/store/form'; import FormScheduleSettings from '~/components/designer/settings/FormScheduleSettings.vue'; @@ -746,4 +748,76 @@ describe('FormScheduleSettings.vue', () => { expect(wrapper.vm.AVAILABLE_PERIOD_OPTIONS).toEqual(['quarters', 'years']); }); + + it('renders submission schedule and late submission text with correct spacing', async () => { + const TODAY = moment().format('YYYY-MM-DD HH:MM:SS'); + const wrapper = mount(FormScheduleSettings, { + global: { + plugins: [pinia], + stubs: { + BaseInfoCard: { + name: 'BaseInfoCard', + template: '
', + }, + BasePanel: { + name: 'BasePanel', + template: '
', + }, + }, + }, + }); + + // Set up the form store with necessary data + const formStore = useFormStore(); + formStore.form = { + schedule: { + enabled: true, + openSubmissionDateTime: TODAY, + closeSubmissionDateTime: moment(TODAY).add(2, 'days').format('YYYY-MM-DD HH:MM:SS'), + scheduleType: ScheduleType.CLOSINGDATE, + allowLateSubmissions: { + enabled: true, + forNext: { + term: 2, + intervalType: 'days' + } + } + } + }; + + await flushPromises(); + + // Check submission schedule text + const submissionScheduleText = wrapper.find('[data-test="submission-schedule-text"]'); + expect(submissionScheduleText.exists()).toBe(true); + + const scheduleTextContent = submissionScheduleText.text(); + expect(scheduleTextContent).toContain(t('trans.formSettings.submissionsOpenDateRange')); + expect(scheduleTextContent).toContain(TODAY); + expect(scheduleTextContent).toContain(t('trans.formSettings.to')); + expect(scheduleTextContent).toContain(formStore.form.schedule.closeSubmissionDateTime); + + // Check that there's exactly one space before and after the "to" text + const toIndex = scheduleTextContent.indexOf(t('trans.formSettings.to')); + expect(scheduleTextContent[toIndex - 1]).toBe(' '); + expect(scheduleTextContent[toIndex + t('trans.formSettings.to').length]).toBe(' '); + + // Check late submission text + const lateSubmissionText = wrapper.find('[data-test="late-submission-text"]'); + expect(lateSubmissionText.exists()).toBe(true); + + const lateTextContent = lateSubmissionText.text(); + const expectedLateText = `${t('trans.formSettings.allowLateSubmissnInterval')} 2 days.`; + expect(lateTextContent).toBe(expectedLateText); + + // Check that there's exactly one space before and after the term and interval type + const termIndex = lateTextContent.indexOf('2'); + expect(lateTextContent[termIndex - 1]).toBe(' '); + expect(lateTextContent[termIndex + 1]).toBe(' '); + + const intervalTypeIndex = lateTextContent.indexOf('days'); + expect(lateTextContent[intervalTypeIndex - 1]).toBe(' '); + expect(lateTextContent[intervalTypeIndex + 4]).toBe('.'); + }); + }); diff --git a/app/frontend/tests/unit/components/forms/manage/TeamManagement.spec.js b/app/frontend/tests/unit/components/forms/manage/TeamManagement.spec.js index c5f210b1b..1d077238e 100644 --- a/app/frontend/tests/unit/components/forms/manage/TeamManagement.spec.js +++ b/app/frontend/tests/unit/components/forms/manage/TeamManagement.spec.js @@ -898,7 +898,7 @@ describe('TeamManagement.vue', () => { expect(addNotificationSpy).toBeCalledTimes(1); expect(addNotificationSpy).toBeCalledWith({ - text: `${IDIR_USER.username}@${IDIR_USER.idpCode}trans.teamManagement.idpMessage`, + text: `${IDIR_USER.username}@${IDIR_USER.idpCode} trans.teamManagement.idpMessage`, }); }); diff --git a/app/frontend/tests/unit/components/forms/submission/ManageSubmissionUsers.spec.js b/app/frontend/tests/unit/components/forms/submission/ManageSubmissionUsers.spec.js index 6d08b408c..66c8aede1 100644 --- a/app/frontend/tests/unit/components/forms/submission/ManageSubmissionUsers.spec.js +++ b/app/frontend/tests/unit/components/forms/submission/ManageSubmissionUsers.spec.js @@ -291,7 +291,7 @@ describe('ManageSubmissionUsers.vue', () => { expect(addNotificationSpy).toHaveBeenCalledWith({ color: 'success', icon: 'mdi:mdi-check-circle', - text: 'trans.manageSubmissionUsers.sentInviteEmailTojohn@email.com', + text: 'trans.manageSubmissionUsers.sentInviteEmailTo john@email.com', type: 'success', }); }); From 0d49e8da712f00f5e22d86c1915374fc03200a0e Mon Sep 17 00:00:00 2001 From: Walter Moar Date: Thu, 22 Aug 2024 11:43:44 -0700 Subject: [PATCH 06/23] ci: FORMS-1449 code climate exclude components (#1484) --- .codeclimate.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.codeclimate.yml b/.codeclimate.yml index fd52aa6bf..9b1220682 100644 --- a/.codeclimate.yml +++ b/.codeclimate.yml @@ -1,5 +1,6 @@ version: "2" exclude_patterns: + - components/ - config/ - dist/ - features/ From 0790db2c3eb20829a1135f007beed194767b62bd Mon Sep 17 00:00:00 2001 From: usingtechnology <39388115+usingtechnology@users.noreply.github.com> Date: Thu, 22 Aug 2024 15:09:23 -0700 Subject: [PATCH 07/23] Adjust external api id middleware for admin routes (#1486) Signed-off-by: Jason Sherman --- .../common/middleware/validateParameter.js | 13 ++++++++++--- .../common/middleware/validateParameter.spec.js | 17 ++++++++++++++++- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/app/src/forms/common/middleware/validateParameter.js b/app/src/forms/common/middleware/validateParameter.js index e917cb61e..af884f5a0 100644 --- a/app/src/forms/common/middleware/validateParameter.js +++ b/app/src/forms/common/middleware/validateParameter.js @@ -93,12 +93,19 @@ const validateExternalAPIId = async (req, _res, next, externalAPIId) => { _validateUuid(externalAPIId, 'externalAPIId'); const externalApi = await externalApiService.readExternalAPI(externalAPIId); - if (!externalApi || externalApi.formId !== req.params.formId) { + if (!externalApi) { throw new Problem(404, { - detail: 'externalAPIId does not exist on this form', + detail: 'externalAPIId does not exist', }); } - + // perform this check only if there is a formId (admin routes don't have form id) + if (req.params.formId) { + if (!externalApi || externalApi.formId !== req.params.formId) { + throw new Problem(404, { + detail: 'externalAPIId does not exist on this form', + }); + } + } next(); } catch (error) { next(error); diff --git a/app/tests/unit/forms/common/middleware/validateParameter.spec.js b/app/tests/unit/forms/common/middleware/validateParameter.spec.js index c5bef0af5..42fcace49 100644 --- a/app/tests/unit/forms/common/middleware/validateParameter.spec.js +++ b/app/tests/unit/forms/common/middleware/validateParameter.spec.js @@ -290,7 +290,8 @@ describe('validateExternalApiId', () => { describe('404 response when', () => { const expectedStatus = { status: 404 }; - test('formId is missing', async () => { + test('externalApiId not found', async () => { + externalApiService.readExternalAPI.mockReturnValueOnce(null); const req = getMockReq({ params: { externalAPIId: externalApiId, @@ -358,6 +359,20 @@ describe('validateExternalApiId', () => { expect(externalApiService.readExternalAPI).toBeCalledTimes(1); expect(next).toBeCalledWith(); }); + + test('external api id only', async () => { + const req = getMockReq({ + params: { + externalAPIId: externalApiId, + }, + }); + const { res, next } = getMockRes(); + + await validateParameter.validateExternalAPIId(req, res, next, externalApiId); + + expect(externalApiService.readExternalAPI).toBeCalledTimes(1); + expect(next).toBeCalledWith(); + }); }); }); From 5b8bd970daa35d5327043053ceccf5d7ae4673ee Mon Sep 17 00:00:00 2001 From: jasonchung1871 <101672465+jasonchung1871@users.noreply.github.com> Date: Fri, 23 Aug 2024 09:45:17 -0700 Subject: [PATCH 08/23] refactor: Forms 1123 composition forms (#1460) * Update ExportSubmissions ExportSubmissions updated to the composition API and almost all coverage is added.. missing the watch variables * Update FormSubmission FormSubmission updated to the composition API with almost full coverage. missing a child component check * PrintOptions Updated PrintOptions is updated to the composition api and has most of the script covered. it is missing watch variables and the print browser functionality as i didn't look deeply into how we can spy on the browsers print functionality. some functions in PrintOptions is moved to a composable or the transformUtils as it can be reused elsewhere and allowed for easier testing. * RequestReceipt Updated RequestReceipt updated to the composition api with full coverage * SubmissionsTable updated SubmissionsTable updated to the composition API but test coverage needs to be added * SubmissionsTable test coverage SubmissionsTable has almost full coverage.. just missing some checking for an error because we actually skip it. and another for checking if the lodash debounce works. --- .../components/forms/ExportSubmissions.vue | 488 +++---- .../src/components/forms/FormSubmission.vue | 150 ++- .../src/components/forms/PrintOptions.vue | 637 +++++---- .../src/components/forms/RequestReceipt.vue | 135 +- .../src/components/forms/SubmissionsTable.vue | 1050 +++++++-------- app/frontend/src/composables/printOptions.js | 10 + app/frontend/src/utils/transformUtils.js | 22 + .../forms/ExportSubmissions.spec.js | 417 +++++- .../components/forms/FormSubmission.spec.js | 256 ++++ .../components/forms/PrintOptions.spec.js | 580 +++++++++ .../components/forms/RequestReceipt.spec.js | 154 +++ .../components/forms/SubmissionsTable.spec.js | 1133 ++++++++++++++++- .../unit/composables/printOptions.spec.js | 16 + app/frontend/tests/unit/fixtures/form.json | 87 ++ .../tests/unit/fixtures/permissions.json | 25 + app/frontend/tests/unit/stubs.js | 40 + 16 files changed, 3930 insertions(+), 1270 deletions(-) create mode 100644 app/frontend/src/composables/printOptions.js create mode 100644 app/frontend/tests/unit/components/forms/FormSubmission.spec.js create mode 100644 app/frontend/tests/unit/components/forms/PrintOptions.spec.js create mode 100644 app/frontend/tests/unit/components/forms/RequestReceipt.spec.js create mode 100644 app/frontend/tests/unit/composables/printOptions.spec.js create mode 100644 app/frontend/tests/unit/fixtures/form.json create mode 100644 app/frontend/tests/unit/fixtures/permissions.json create mode 100644 app/frontend/tests/unit/stubs.js diff --git a/app/frontend/src/components/forms/ExportSubmissions.vue b/app/frontend/src/components/forms/ExportSubmissions.vue index 870a25cb8..fd0f8b1f3 100644 --- a/app/frontend/src/components/forms/ExportSubmissions.vue +++ b/app/frontend/src/components/forms/ExportSubmissions.vue @@ -1,7 +1,8 @@ -