From 604dca35b1e1d62829ee64d7b925943eac7842f2 Mon Sep 17 00:00:00 2001 From: david-rocca Date: Mon, 13 Nov 2023 15:18:29 -0500 Subject: [PATCH 01/14] #944 Created middleware to check for invalid characters, then added middleware to all query parameters, then wrote test --- src/controller/cve-id.controller/index.js | 6 ++++ src/controller/cve.controller/index.js | 2 ++ src/controller/org.controller/index.js | 3 ++ src/controller/user.controller/index.js | 1 + src/middleware/middleware.js | 15 +++++++- .../middleware/checkForInvalidCharacters.js | 36 +++++++++++++++++++ 6 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 test/unit-tests/middleware/checkForInvalidCharacters.js diff --git a/src/controller/cve-id.controller/index.js b/src/controller/cve-id.controller/index.js index 737a383d..1d021856 100644 --- a/src/controller/cve-id.controller/index.js +++ b/src/controller/cve-id.controller/index.js @@ -86,6 +86,8 @@ router.get('/cve-id', */ mw.validateUser, query().custom((query) => { return mw.validateQueryParameterNames(query, ['page', 'state', 'cve_id_year', 'time_reserved.lt', 'time_reserved.gt', 'time_modified.lt', 'time_modified.gt']) }), + // The below containsNoInvalidCharacters function does not need to be aware of the parameters that it is being passed. So the iteration of the query parameters is being handeled by the "query" function instead of the containsNoInvalidCharacters function. + query(['page', 'state', 'cve_id_year', 'time_reserved.lt', 'time_reserved.gt', 'time_modified.lt', 'time_modified.gt']).custom((query) => { return mw.containsNoInvalidCharacters(query) }), query(['page']).optional().isInt({ min: CONSTANTS.PAGINATOR_PAGE }), query(['state']).optional().isString().trim().escape().customSanitizer(val => { return val.toUpperCase() }).isIn(CHOICES).withMessage(errorMsgs.ID_STATES), query(['cve_id_year']).optional().isNumeric().matches(/^[0-9]{4}$/), @@ -177,6 +179,7 @@ router.post('/cve-id', mw.validateUser, mw.onlyCnas, query().custom((query) => { return mw.validateQueryParameterNames(query, ['amount', 'batch_type', 'short_name', 'cve_year']) }), + query(['amount', 'batch_type', 'short_name', 'cve_year']).custom((query) => { return mw.containsNoInvalidCharacters(query) }), query(['amount']).isInt(), query(['batch_type']).optional().isString().trim().escape().customSanitizer(val => { return val.toLowerCase() }), query(['short_name']).isString().trim().escape().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), @@ -263,6 +266,7 @@ router.get('/cve-id/:id', */ mw.rateLimiter, mw.optionallyValidateUser, + query(['id']).custom((query) => { return mw.containsNoInvalidCharacters(query) }), param(['id']).isString().matches(CONSTANTS.CVE_ID_REGEX), parseError, parseGetParams, @@ -340,6 +344,7 @@ router.put('/cve-id/:id', mw.onlyCnas, param(['id']).isString().matches(CONSTANTS.CVE_ID_REGEX), query().custom((query) => { return mw.validateQueryParameterNames(query, ['state', 'org']) }), + query(['state', 'org', 'id']).custom((query) => { return mw.containsNoInvalidCharacters(query) }), query(['state']).optional().isString().trim().escape().customSanitizer(val => { return val.toUpperCase() }).isIn(MODIFYTARGETS).withMessage(errorMsgs.ID_MODIFY_STATES), query(['org']).optional().isString().trim().escape(), parseError, @@ -415,6 +420,7 @@ router.post('/cve-id-range/:year', mw.validateUser, mw.onlySecretariat, param(['year']).isNumeric().matches(/^[0-9]{4}$/), + query(['year']).custom((query) => { return mw.containsNoInvalidCharacters(query) }), parseError, parsePostParams, controller.CVEID_RANGE_CREATE) diff --git a/src/controller/cve.controller/index.js b/src/controller/cve.controller/index.js index 2e89fe49..4783b5b3 100644 --- a/src/controller/cve.controller/index.js +++ b/src/controller/cve.controller/index.js @@ -157,6 +157,7 @@ router.get('/cve', mw.validateUser, mw.onlySecretariatOrBulkDownload, query().custom((query) => { return mw.validateQueryParameterNames(query, ['page', 'time_modified.lt', 'time_modified.gt', 'state', 'count_only', 'assigner_short_name', 'assigner', 'cna_modified', 'adp_short_name']) }), + query(['page', 'time_modified.lt', 'time_modified.gt', 'state', 'count_only', 'assigner_short_name', 'assigner', 'cna_modified', 'adp_short_name']).custom((query) => { return mw.containsNoInvalidCharacters(query) }), query(['page']).optional().isInt({ min: CONSTANTS.PAGINATOR_PAGE }), query(['time_modified.lt']).optional().isString().trim().escape().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), query(['time_modified.gt']).optional().isString().trim().escape().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), @@ -244,6 +245,7 @@ router.get('/cve_cursor', mw.validateUser, mw.onlySecretariatOrBulkDownload, query().custom((query) => { return mw.validateQueryParameterNames(query, ['time_modified.lt', 'time_modified.gt', 'state', 'count_only', 'assigner_short_name', 'assigner', 'cna_modified', 'adp_short_name', 'next_page', 'previous_page', 'limit']) }), + query(['time_modified.lt', 'time_modified.gt', 'state', 'count_only', 'assigner_short_name', 'assigner', 'cna_modified', 'adp_short_name', 'next_page', 'previous_page', 'limit']).custom((query) => { return mw.containsNoInvalidCharacters(query) }), query(['time_modified.lt']).optional().isString().trim().escape().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), query(['time_modified.gt']).optional().isString().trim().escape().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), query(['state']).optional().isString().trim().escape().customSanitizer(val => { return val.toUpperCase() }).isIn(CHOICES).withMessage(errorMsgs.CVE_FILTERED_STATES), diff --git a/src/controller/org.controller/index.js b/src/controller/org.controller/index.js index b1630aa9..520b3063 100644 --- a/src/controller/org.controller/index.js +++ b/src/controller/org.controller/index.js @@ -77,6 +77,7 @@ router.get('/org', mw.validateUser, mw.onlySecretariat, query().custom((query) => { return mw.validateQueryParameterNames(query, ['page']) }), + query(['page']).custom((query) => { return mw.containsNoInvalidCharacters(query) }), query(['page']).optional().isInt({ min: CONSTANTS.PAGINATOR_PAGE }), parseError, parseGetParams, @@ -310,6 +311,7 @@ router.put('/org/:shortname', mw.validateUser, mw.onlySecretariat, query().custom((query) => { return mw.validateQueryParameterNames(query, ['new_short_name', 'id_quota', 'name', 'active_roles.add', 'active_roles.remove']) }), + query(['new_short_name', 'id_quota', 'name', 'active_roles.add', 'active_roles.remove']).custom((query) => { return mw.containsNoInvalidCharacters(query) }), param(['shortname']).isString().trim().escape().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), query(['new_short_name']).optional().isString().trim().escape().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), query(['id_quota']).optional().not().isArray().isInt({ min: CONSTANTS.MONGOOSE_VALIDATION.Org_policies_id_quota_min, max: CONSTANTS.MONGOOSE_VALIDATION.Org_policies_id_quota_max }).withMessage(errorMsgs.ID_QUOTA), @@ -717,6 +719,7 @@ router.put('/org/:shortname/user/:username', return mw.validateQueryParameterNames(query, ['active', 'new_username', 'org_short_name', 'name.first', 'name.last', 'name.middle', 'name.suffix', 'active_roles.add', 'active_roles.remove']) }), + query(['active', 'new_username', 'org_short_name', 'active_roles.add', 'active_roles.remove']).custom((query) => { return mw.containsNoInvalidCharacters(query) }), param(['shortname']).isString().trim().escape().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), param(['username']).isString().trim().escape().notEmpty().custom(isValidUsername), query(['active']).optional().isBoolean({ loose: true }), diff --git a/src/controller/user.controller/index.js b/src/controller/user.controller/index.js index fb57a720..458aa1e2 100644 --- a/src/controller/user.controller/index.js +++ b/src/controller/user.controller/index.js @@ -75,6 +75,7 @@ router.get('/users', mw.validateUser, mw.onlySecretariat, query(['page']).optional().isInt({ min: CONSTANTS.PAGINATOR_PAGE }), + query(['page']).custom((query) => { return mw.containsNoInvalidCharacters(query) }), parseError, parseGetParams, controller.ALL_USERS) diff --git a/src/middleware/middleware.js b/src/middleware/middleware.js index 8e1ff53f..b9856cfe 100644 --- a/src/middleware/middleware.js +++ b/src/middleware/middleware.js @@ -439,6 +439,18 @@ function toUpperCaseArray (val) { return newArr } +// Check for the invalid characters <, >, and " +function containsNoInvalidCharacters (parameters) { + const invalidCharacterList = ['<', '>', '"'] + for (const key in parameters) { + for (const invalidCharacter of invalidCharacterList) { + if (parameters[key].includes(invalidCharacter)) { + throw new Error('contains invalid character: ' + invalidCharacter) + } + } + } +} + module.exports = { setCacheControl, optionallyValidateUser, @@ -457,5 +469,6 @@ module.exports = { validateJsonSyntax, rateLimiter: limiter, isFlatStringArray, - toUpperCaseArray + toUpperCaseArray, + containsNoInvalidCharacters } diff --git a/test/unit-tests/middleware/checkForInvalidCharacters.js b/test/unit-tests/middleware/checkForInvalidCharacters.js new file mode 100644 index 00000000..bf828110 --- /dev/null +++ b/test/unit-tests/middleware/checkForInvalidCharacters.js @@ -0,0 +1,36 @@ +const chai = require('chai') +const expect = chai.expect + +const { containsNoInvalidCharacters } = require('../../../src/middleware/middleware') +const { async } = require('crypto-random-string') + +describe.only('Testing Invalid Character checker', () => { + context('negative tests', () => { + it('Should fail when detecting a >', async () => { + try { + containsNoInvalidCharacters({ key: 'sa>fe' }) + } catch (err) { + expect(err.message).to.contain('contains invalid character: >') + } + }) + it('Should fail when detecting a <', async () => { + try { + containsNoInvalidCharacters({ key: 'sa { + try { + containsNoInvalidCharacters({ key: 'sa"fe' }) + } catch (err) { + expect(err.message).to.contain('contains invalid character: "') + } + }) + }) + context('positive test', () => { + it('Should pass if no invalid characters', async () => { + containsNoInvalidCharacters({ key: 'safe' }) + }) + }) +}) From 817cd12922fe2f1b4426266e28ca94e6293baa42 Mon Sep 17 00:00:00 2001 From: david-rocca Date: Mon, 13 Nov 2023 15:20:58 -0500 Subject: [PATCH 02/14] #944 sigh, intellisense strikes again... --- test/unit-tests/middleware/checkForInvalidCharacters.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/unit-tests/middleware/checkForInvalidCharacters.js b/test/unit-tests/middleware/checkForInvalidCharacters.js index bf828110..07e0ce9f 100644 --- a/test/unit-tests/middleware/checkForInvalidCharacters.js +++ b/test/unit-tests/middleware/checkForInvalidCharacters.js @@ -2,7 +2,6 @@ const chai = require('chai') const expect = chai.expect const { containsNoInvalidCharacters } = require('../../../src/middleware/middleware') -const { async } = require('crypto-random-string') describe.only('Testing Invalid Character checker', () => { context('negative tests', () => { From 5962dc891cfbe55cb84835d519bc60d6b8dc4baa Mon Sep 17 00:00:00 2001 From: david-rocca Date: Mon, 13 Nov 2023 15:27:43 -0500 Subject: [PATCH 03/14] #944 remove an 'only' that I forgot about, whoops --- test/unit-tests/middleware/checkForInvalidCharacters.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit-tests/middleware/checkForInvalidCharacters.js b/test/unit-tests/middleware/checkForInvalidCharacters.js index 07e0ce9f..550b1878 100644 --- a/test/unit-tests/middleware/checkForInvalidCharacters.js +++ b/test/unit-tests/middleware/checkForInvalidCharacters.js @@ -3,7 +3,7 @@ const expect = chai.expect const { containsNoInvalidCharacters } = require('../../../src/middleware/middleware') -describe.only('Testing Invalid Character checker', () => { +describe('Testing Invalid Character checker', () => { context('negative tests', () => { it('Should fail when detecting a >', async () => { try { From faf7d2e90566c50b8b98f15c5a7010d8342b3f04 Mon Sep 17 00:00:00 2001 From: david-rocca Date: Thu, 16 Nov 2023 11:18:52 -0500 Subject: [PATCH 04/14] Fixed a bug in the middleware that was causing tests to fail --- src/controller/cve-id.controller/index.js | 9 +++------ src/controller/cve.controller/index.js | 4 ++-- src/controller/org.controller/index.js | 6 +++--- src/controller/user.controller/index.js | 2 +- src/middleware/middleware.js | 3 ++- 5 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/controller/cve-id.controller/index.js b/src/controller/cve-id.controller/index.js index 1d021856..fa3f93be 100644 --- a/src/controller/cve-id.controller/index.js +++ b/src/controller/cve-id.controller/index.js @@ -86,8 +86,7 @@ router.get('/cve-id', */ mw.validateUser, query().custom((query) => { return mw.validateQueryParameterNames(query, ['page', 'state', 'cve_id_year', 'time_reserved.lt', 'time_reserved.gt', 'time_modified.lt', 'time_modified.gt']) }), - // The below containsNoInvalidCharacters function does not need to be aware of the parameters that it is being passed. So the iteration of the query parameters is being handeled by the "query" function instead of the containsNoInvalidCharacters function. - query(['page', 'state', 'cve_id_year', 'time_reserved.lt', 'time_reserved.gt', 'time_modified.lt', 'time_modified.gt']).custom((query) => { return mw.containsNoInvalidCharacters(query) }), + query().custom((query) => { return mw.containsNoInvalidCharacters(query) }), query(['page']).optional().isInt({ min: CONSTANTS.PAGINATOR_PAGE }), query(['state']).optional().isString().trim().escape().customSanitizer(val => { return val.toUpperCase() }).isIn(CHOICES).withMessage(errorMsgs.ID_STATES), query(['cve_id_year']).optional().isNumeric().matches(/^[0-9]{4}$/), @@ -179,7 +178,7 @@ router.post('/cve-id', mw.validateUser, mw.onlyCnas, query().custom((query) => { return mw.validateQueryParameterNames(query, ['amount', 'batch_type', 'short_name', 'cve_year']) }), - query(['amount', 'batch_type', 'short_name', 'cve_year']).custom((query) => { return mw.containsNoInvalidCharacters(query) }), + query().custom((query) => { return mw.containsNoInvalidCharacters(query) }), query(['amount']).isInt(), query(['batch_type']).optional().isString().trim().escape().customSanitizer(val => { return val.toLowerCase() }), query(['short_name']).isString().trim().escape().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), @@ -266,7 +265,6 @@ router.get('/cve-id/:id', */ mw.rateLimiter, mw.optionallyValidateUser, - query(['id']).custom((query) => { return mw.containsNoInvalidCharacters(query) }), param(['id']).isString().matches(CONSTANTS.CVE_ID_REGEX), parseError, parseGetParams, @@ -344,7 +342,7 @@ router.put('/cve-id/:id', mw.onlyCnas, param(['id']).isString().matches(CONSTANTS.CVE_ID_REGEX), query().custom((query) => { return mw.validateQueryParameterNames(query, ['state', 'org']) }), - query(['state', 'org', 'id']).custom((query) => { return mw.containsNoInvalidCharacters(query) }), + query().custom((query) => { return mw.containsNoInvalidCharacters(query) }), query(['state']).optional().isString().trim().escape().customSanitizer(val => { return val.toUpperCase() }).isIn(MODIFYTARGETS).withMessage(errorMsgs.ID_MODIFY_STATES), query(['org']).optional().isString().trim().escape(), parseError, @@ -420,7 +418,6 @@ router.post('/cve-id-range/:year', mw.validateUser, mw.onlySecretariat, param(['year']).isNumeric().matches(/^[0-9]{4}$/), - query(['year']).custom((query) => { return mw.containsNoInvalidCharacters(query) }), parseError, parsePostParams, controller.CVEID_RANGE_CREATE) diff --git a/src/controller/cve.controller/index.js b/src/controller/cve.controller/index.js index 4783b5b3..ad0039b5 100644 --- a/src/controller/cve.controller/index.js +++ b/src/controller/cve.controller/index.js @@ -157,7 +157,7 @@ router.get('/cve', mw.validateUser, mw.onlySecretariatOrBulkDownload, query().custom((query) => { return mw.validateQueryParameterNames(query, ['page', 'time_modified.lt', 'time_modified.gt', 'state', 'count_only', 'assigner_short_name', 'assigner', 'cna_modified', 'adp_short_name']) }), - query(['page', 'time_modified.lt', 'time_modified.gt', 'state', 'count_only', 'assigner_short_name', 'assigner', 'cna_modified', 'adp_short_name']).custom((query) => { return mw.containsNoInvalidCharacters(query) }), + query().custom((query) => { return mw.containsNoInvalidCharacters(query) }), query(['page']).optional().isInt({ min: CONSTANTS.PAGINATOR_PAGE }), query(['time_modified.lt']).optional().isString().trim().escape().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), query(['time_modified.gt']).optional().isString().trim().escape().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), @@ -245,7 +245,7 @@ router.get('/cve_cursor', mw.validateUser, mw.onlySecretariatOrBulkDownload, query().custom((query) => { return mw.validateQueryParameterNames(query, ['time_modified.lt', 'time_modified.gt', 'state', 'count_only', 'assigner_short_name', 'assigner', 'cna_modified', 'adp_short_name', 'next_page', 'previous_page', 'limit']) }), - query(['time_modified.lt', 'time_modified.gt', 'state', 'count_only', 'assigner_short_name', 'assigner', 'cna_modified', 'adp_short_name', 'next_page', 'previous_page', 'limit']).custom((query) => { return mw.containsNoInvalidCharacters(query) }), + query().custom((query) => { return mw.containsNoInvalidCharacters(query) }), query(['time_modified.lt']).optional().isString().trim().escape().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), query(['time_modified.gt']).optional().isString().trim().escape().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), query(['state']).optional().isString().trim().escape().customSanitizer(val => { return val.toUpperCase() }).isIn(CHOICES).withMessage(errorMsgs.CVE_FILTERED_STATES), diff --git a/src/controller/org.controller/index.js b/src/controller/org.controller/index.js index 520b3063..db322940 100644 --- a/src/controller/org.controller/index.js +++ b/src/controller/org.controller/index.js @@ -77,7 +77,7 @@ router.get('/org', mw.validateUser, mw.onlySecretariat, query().custom((query) => { return mw.validateQueryParameterNames(query, ['page']) }), - query(['page']).custom((query) => { return mw.containsNoInvalidCharacters(query) }), + query().custom((query) => { return mw.containsNoInvalidCharacters(query) }), query(['page']).optional().isInt({ min: CONSTANTS.PAGINATOR_PAGE }), parseError, parseGetParams, @@ -311,7 +311,7 @@ router.put('/org/:shortname', mw.validateUser, mw.onlySecretariat, query().custom((query) => { return mw.validateQueryParameterNames(query, ['new_short_name', 'id_quota', 'name', 'active_roles.add', 'active_roles.remove']) }), - query(['new_short_name', 'id_quota', 'name', 'active_roles.add', 'active_roles.remove']).custom((query) => { return mw.containsNoInvalidCharacters(query) }), + query().custom((query) => { return mw.containsNoInvalidCharacters(query) }), param(['shortname']).isString().trim().escape().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), query(['new_short_name']).optional().isString().trim().escape().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), query(['id_quota']).optional().not().isArray().isInt({ min: CONSTANTS.MONGOOSE_VALIDATION.Org_policies_id_quota_min, max: CONSTANTS.MONGOOSE_VALIDATION.Org_policies_id_quota_max }).withMessage(errorMsgs.ID_QUOTA), @@ -719,7 +719,7 @@ router.put('/org/:shortname/user/:username', return mw.validateQueryParameterNames(query, ['active', 'new_username', 'org_short_name', 'name.first', 'name.last', 'name.middle', 'name.suffix', 'active_roles.add', 'active_roles.remove']) }), - query(['active', 'new_username', 'org_short_name', 'active_roles.add', 'active_roles.remove']).custom((query) => { return mw.containsNoInvalidCharacters(query) }), + query().custom((query) => { return mw.containsNoInvalidCharacters(query) }), param(['shortname']).isString().trim().escape().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), param(['username']).isString().trim().escape().notEmpty().custom(isValidUsername), query(['active']).optional().isBoolean({ loose: true }), diff --git a/src/controller/user.controller/index.js b/src/controller/user.controller/index.js index 458aa1e2..75256a88 100644 --- a/src/controller/user.controller/index.js +++ b/src/controller/user.controller/index.js @@ -75,7 +75,7 @@ router.get('/users', mw.validateUser, mw.onlySecretariat, query(['page']).optional().isInt({ min: CONSTANTS.PAGINATOR_PAGE }), - query(['page']).custom((query) => { return mw.containsNoInvalidCharacters(query) }), + query().custom((query) => { return mw.containsNoInvalidCharacters(query) }), parseError, parseGetParams, controller.ALL_USERS) diff --git a/src/middleware/middleware.js b/src/middleware/middleware.js index b9856cfe..f77ae537 100644 --- a/src/middleware/middleware.js +++ b/src/middleware/middleware.js @@ -442,13 +442,14 @@ function toUpperCaseArray (val) { // Check for the invalid characters <, >, and " function containsNoInvalidCharacters (parameters) { const invalidCharacterList = ['<', '>', '"'] - for (const key in parameters) { + for (const key of Object.keys(parameters)) { for (const invalidCharacter of invalidCharacterList) { if (parameters[key].includes(invalidCharacter)) { throw new Error('contains invalid character: ' + invalidCharacter) } } } + return true } module.exports = { From 68c4a119ec98d3a781ffa0b007f61d5dde39396a Mon Sep 17 00:00:00 2001 From: david-rocca Date: Fri, 17 Nov 2023 10:06:02 -0500 Subject: [PATCH 05/14] #944 a middle ground, slightly worse on the eyes for calling, but provides better failure information --- src/controller/cve-id.controller/index.js | 6 +++--- src/controller/cve.controller/index.js | 4 ++-- src/controller/org.controller/index.js | 7 ++++--- src/controller/user.controller/index.js | 2 +- src/middleware/middleware.js | 6 +++--- 5 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/controller/cve-id.controller/index.js b/src/controller/cve-id.controller/index.js index fa3f93be..23c48eb4 100644 --- a/src/controller/cve-id.controller/index.js +++ b/src/controller/cve-id.controller/index.js @@ -86,7 +86,7 @@ router.get('/cve-id', */ mw.validateUser, query().custom((query) => { return mw.validateQueryParameterNames(query, ['page', 'state', 'cve_id_year', 'time_reserved.lt', 'time_reserved.gt', 'time_modified.lt', 'time_modified.gt']) }), - query().custom((query) => { return mw.containsNoInvalidCharacters(query) }), + query(['page', 'state', 'cve_id_year', 'time_reserved.lt', 'time_reserved.gt', 'time_modified.lt', 'time_modified.gt']).custom((val) => { return mw.containsNoInvalidCharacters(val) }), query(['page']).optional().isInt({ min: CONSTANTS.PAGINATOR_PAGE }), query(['state']).optional().isString().trim().escape().customSanitizer(val => { return val.toUpperCase() }).isIn(CHOICES).withMessage(errorMsgs.ID_STATES), query(['cve_id_year']).optional().isNumeric().matches(/^[0-9]{4}$/), @@ -178,7 +178,7 @@ router.post('/cve-id', mw.validateUser, mw.onlyCnas, query().custom((query) => { return mw.validateQueryParameterNames(query, ['amount', 'batch_type', 'short_name', 'cve_year']) }), - query().custom((query) => { return mw.containsNoInvalidCharacters(query) }), + query(['amount', 'batch_type', 'short_name', 'cve_year']).custom((val) => { return mw.containsNoInvalidCharacters(val) }), query(['amount']).isInt(), query(['batch_type']).optional().isString().trim().escape().customSanitizer(val => { return val.toLowerCase() }), query(['short_name']).isString().trim().escape().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), @@ -342,7 +342,7 @@ router.put('/cve-id/:id', mw.onlyCnas, param(['id']).isString().matches(CONSTANTS.CVE_ID_REGEX), query().custom((query) => { return mw.validateQueryParameterNames(query, ['state', 'org']) }), - query().custom((query) => { return mw.containsNoInvalidCharacters(query) }), + query(['state', 'org']).custom((val) => { return mw.containsNoInvalidCharacters(val) }), query(['state']).optional().isString().trim().escape().customSanitizer(val => { return val.toUpperCase() }).isIn(MODIFYTARGETS).withMessage(errorMsgs.ID_MODIFY_STATES), query(['org']).optional().isString().trim().escape(), parseError, diff --git a/src/controller/cve.controller/index.js b/src/controller/cve.controller/index.js index ad0039b5..8c75ddf6 100644 --- a/src/controller/cve.controller/index.js +++ b/src/controller/cve.controller/index.js @@ -157,7 +157,7 @@ router.get('/cve', mw.validateUser, mw.onlySecretariatOrBulkDownload, query().custom((query) => { return mw.validateQueryParameterNames(query, ['page', 'time_modified.lt', 'time_modified.gt', 'state', 'count_only', 'assigner_short_name', 'assigner', 'cna_modified', 'adp_short_name']) }), - query().custom((query) => { return mw.containsNoInvalidCharacters(query) }), + query(['page', 'time_modified.lt', 'time_modified.gt', 'state', 'count_only', 'assigner_short_name', 'assigner', 'cna_modified', 'adp_short_name']).custom((val) => { return mw.containsNoInvalidCharacters(val) }), query(['page']).optional().isInt({ min: CONSTANTS.PAGINATOR_PAGE }), query(['time_modified.lt']).optional().isString().trim().escape().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), query(['time_modified.gt']).optional().isString().trim().escape().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), @@ -245,7 +245,7 @@ router.get('/cve_cursor', mw.validateUser, mw.onlySecretariatOrBulkDownload, query().custom((query) => { return mw.validateQueryParameterNames(query, ['time_modified.lt', 'time_modified.gt', 'state', 'count_only', 'assigner_short_name', 'assigner', 'cna_modified', 'adp_short_name', 'next_page', 'previous_page', 'limit']) }), - query().custom((query) => { return mw.containsNoInvalidCharacters(query) }), + query(['time_modified.lt', 'time_modified.gt', 'state', 'count_only', 'assigner_short_name', 'assigner', 'cna_modified', 'adp_short_name', 'next_page', 'previous_page', 'limit']).custom((val) => { return mw.containsNoInvalidCharacters(val) }), query(['time_modified.lt']).optional().isString().trim().escape().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), query(['time_modified.gt']).optional().isString().trim().escape().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), query(['state']).optional().isString().trim().escape().customSanitizer(val => { return val.toUpperCase() }).isIn(CHOICES).withMessage(errorMsgs.CVE_FILTERED_STATES), diff --git a/src/controller/org.controller/index.js b/src/controller/org.controller/index.js index db322940..9bec6464 100644 --- a/src/controller/org.controller/index.js +++ b/src/controller/org.controller/index.js @@ -77,7 +77,7 @@ router.get('/org', mw.validateUser, mw.onlySecretariat, query().custom((query) => { return mw.validateQueryParameterNames(query, ['page']) }), - query().custom((query) => { return mw.containsNoInvalidCharacters(query) }), + query(['page']).custom((val) => { return mw.containsNoInvalidCharacters(val) }), query(['page']).optional().isInt({ min: CONSTANTS.PAGINATOR_PAGE }), parseError, parseGetParams, @@ -311,7 +311,7 @@ router.put('/org/:shortname', mw.validateUser, mw.onlySecretariat, query().custom((query) => { return mw.validateQueryParameterNames(query, ['new_short_name', 'id_quota', 'name', 'active_roles.add', 'active_roles.remove']) }), - query().custom((query) => { return mw.containsNoInvalidCharacters(query) }), + query(['new_short_name', 'id_quota', 'name', 'active_roles.add', 'active_roles.remove']).custom((val) => { return mw.containsNoInvalidCharacters(val) }), param(['shortname']).isString().trim().escape().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), query(['new_short_name']).optional().isString().trim().escape().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), query(['id_quota']).optional().not().isArray().isInt({ min: CONSTANTS.MONGOOSE_VALIDATION.Org_policies_id_quota_min, max: CONSTANTS.MONGOOSE_VALIDATION.Org_policies_id_quota_max }).withMessage(errorMsgs.ID_QUOTA), @@ -719,7 +719,8 @@ router.put('/org/:shortname/user/:username', return mw.validateQueryParameterNames(query, ['active', 'new_username', 'org_short_name', 'name.first', 'name.last', 'name.middle', 'name.suffix', 'active_roles.add', 'active_roles.remove']) }), - query().custom((query) => { return mw.containsNoInvalidCharacters(query) }), + query(['active', 'new_username', 'org_short_name', 'name.first', 'name.last', 'name.middle', + 'name.suffix', 'active_roles.add', 'active_roles.remove']).custom((val) => { return mw.containsNoInvalidCharacters(val) }), param(['shortname']).isString().trim().escape().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), param(['username']).isString().trim().escape().notEmpty().custom(isValidUsername), query(['active']).optional().isBoolean({ loose: true }), diff --git a/src/controller/user.controller/index.js b/src/controller/user.controller/index.js index 75256a88..5d153266 100644 --- a/src/controller/user.controller/index.js +++ b/src/controller/user.controller/index.js @@ -75,7 +75,7 @@ router.get('/users', mw.validateUser, mw.onlySecretariat, query(['page']).optional().isInt({ min: CONSTANTS.PAGINATOR_PAGE }), - query().custom((query) => { return mw.containsNoInvalidCharacters(query) }), + query(['page']).custom((val) => { return mw.containsNoInvalidCharacters(val) }), parseError, parseGetParams, controller.ALL_USERS) diff --git a/src/middleware/middleware.js b/src/middleware/middleware.js index f77ae537..066743c3 100644 --- a/src/middleware/middleware.js +++ b/src/middleware/middleware.js @@ -440,11 +440,11 @@ function toUpperCaseArray (val) { } // Check for the invalid characters <, >, and " -function containsNoInvalidCharacters (parameters) { +function containsNoInvalidCharacters (val) { const invalidCharacterList = ['<', '>', '"'] - for (const key of Object.keys(parameters)) { + if (val) { for (const invalidCharacter of invalidCharacterList) { - if (parameters[key].includes(invalidCharacter)) { + if (val.includes(invalidCharacter)) { throw new Error('contains invalid character: ' + invalidCharacter) } } From 44ce92be683d7711e58800c7c0bbc8a6664c601f Mon Sep 17 00:00:00 2001 From: david-rocca Date: Fri, 17 Nov 2023 10:12:08 -0500 Subject: [PATCH 06/14] #944 Fixed unit test to take a string not a dictonary --- test/unit-tests/middleware/checkForInvalidCharacters.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/unit-tests/middleware/checkForInvalidCharacters.js b/test/unit-tests/middleware/checkForInvalidCharacters.js index 550b1878..a03a94cd 100644 --- a/test/unit-tests/middleware/checkForInvalidCharacters.js +++ b/test/unit-tests/middleware/checkForInvalidCharacters.js @@ -7,21 +7,21 @@ describe('Testing Invalid Character checker', () => { context('negative tests', () => { it('Should fail when detecting a >', async () => { try { - containsNoInvalidCharacters({ key: 'sa>fe' }) + containsNoInvalidCharacters('sa>fe') } catch (err) { expect(err.message).to.contain('contains invalid character: >') } }) it('Should fail when detecting a <', async () => { try { - containsNoInvalidCharacters({ key: 'sa { try { - containsNoInvalidCharacters({ key: 'sa"fe' }) + containsNoInvalidCharacters('sa"fe') } catch (err) { expect(err.message).to.contain('contains invalid character: "') } @@ -29,7 +29,7 @@ describe('Testing Invalid Character checker', () => { }) context('positive test', () => { it('Should pass if no invalid characters', async () => { - containsNoInvalidCharacters({ key: 'safe' }) + containsNoInvalidCharacters('safe') }) }) }) From 9b5e26286f3e5d31fb09dfdb19e280de49646dc8 Mon Sep 17 00:00:00 2001 From: "Daigneau, Jeremy T" Date: Tue, 21 Nov 2023 12:47:49 -0500 Subject: [PATCH 07/14] #962 #728 Replaced instances of "escape" with "encodeURI" which encodes a different set of characters. Also removed "decodeEntities" --- package-lock.json | 11 ---- package.json | 3 +- src/controller/cve-id.controller/index.js | 18 +++--- src/controller/cve.controller/index.js | 26 ++++----- src/controller/org.controller/index.js | 56 +++++++++---------- .../org.controller/org.controller.js | 25 ++++----- 6 files changed, 63 insertions(+), 76 deletions(-) diff --git a/package-lock.json b/package-lock.json index 7d86518d..e2b632f0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -21,7 +21,6 @@ "express-rate-limit": "^6.5.2", "express-validator": "^6.14.2", "helmet": "^7.0.0", - "html-entities": "^2.3.3", "jsonschema": "^1.4.0", "JSONStream": "^1.3.5", "kleur": "^4.1.4", @@ -4504,11 +4503,6 @@ "integrity": "sha512-mxIDAb9Lsm6DoOJ7xH+5+X4y1LU/4Hi50L9C5sIswK3JzULS4bwk1FvjdBgvYR4bzT4tuUQiC15FE2f5HbLvYw==", "dev": true }, - "node_modules/html-entities": { - "version": "2.3.3", - "resolved": "https://registry.npmjs.org/html-entities/-/html-entities-2.3.3.tgz", - "integrity": "sha512-DV5Ln36z34NNTDgnz0EWGBLZENelNAtkiFA4kyNOG2tDI6Mz1uSWiq1wAKdyjnJwyDiDO7Fa2SO1CTxPXL8VxA==" - }, "node_modules/html-escaper": { "version": "2.0.2", "resolved": "https://registry.npmjs.org/html-escaper/-/html-escaper-2.0.2.tgz", @@ -13736,11 +13730,6 @@ "integrity": "sha512-mxIDAb9Lsm6DoOJ7xH+5+X4y1LU/4Hi50L9C5sIswK3JzULS4bwk1FvjdBgvYR4bzT4tuUQiC15FE2f5HbLvYw==", "dev": true }, - "html-entities": { - "version": "2.3.3", - "resolved": "https://registry.npmjs.org/html-entities/-/html-entities-2.3.3.tgz", - "integrity": "sha512-DV5Ln36z34NNTDgnz0EWGBLZENelNAtkiFA4kyNOG2tDI6Mz1uSWiq1wAKdyjnJwyDiDO7Fa2SO1CTxPXL8VxA==" - }, "html-escaper": { "version": "2.0.2", "resolved": "https://registry.npmjs.org/html-escaper/-/html-escaper-2.0.2.tgz", diff --git a/package.json b/package.json index 03c65c26..e85f8f2b 100644 --- a/package.json +++ b/package.json @@ -39,7 +39,6 @@ "express-rate-limit": "^6.5.2", "express-validator": "^6.14.2", "helmet": "^7.0.0", - "html-entities": "^2.3.3", "jsonschema": "^1.4.0", "JSONStream": "^1.3.5", "kleur": "^4.1.4", @@ -101,4 +100,4 @@ "test:coverage-html": "NODE_ENV=test nyc --reporter=html mocha src/* --recursive --exit || true", "test:scripts": "NODE_ENV=development node-dev src/scripts/templateScript.js" } -} \ No newline at end of file +} diff --git a/src/controller/cve-id.controller/index.js b/src/controller/cve-id.controller/index.js index 23c48eb4..a8cfa7ab 100644 --- a/src/controller/cve-id.controller/index.js +++ b/src/controller/cve-id.controller/index.js @@ -88,12 +88,12 @@ router.get('/cve-id', query().custom((query) => { return mw.validateQueryParameterNames(query, ['page', 'state', 'cve_id_year', 'time_reserved.lt', 'time_reserved.gt', 'time_modified.lt', 'time_modified.gt']) }), query(['page', 'state', 'cve_id_year', 'time_reserved.lt', 'time_reserved.gt', 'time_modified.lt', 'time_modified.gt']).custom((val) => { return mw.containsNoInvalidCharacters(val) }), query(['page']).optional().isInt({ min: CONSTANTS.PAGINATOR_PAGE }), - query(['state']).optional().isString().trim().escape().customSanitizer(val => { return val.toUpperCase() }).isIn(CHOICES).withMessage(errorMsgs.ID_STATES), + query(['state']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }).customSanitizer(val => { return val.toUpperCase() }).isIn(CHOICES).withMessage(errorMsgs.ID_STATES), query(['cve_id_year']).optional().isNumeric().matches(/^[0-9]{4}$/), - query(['time_reserved.lt']).optional().isString().trim().escape().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), - query(['time_reserved.gt']).optional().isString().trim().escape().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), - query(['time_modified.lt']).optional().isString().trim().escape().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), - query(['time_modified.gt']).optional().isString().trim().escape().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), + query(['time_reserved.lt']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }).customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), + query(['time_reserved.gt']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }).customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), + query(['time_modified.lt']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }).customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), + query(['time_modified.gt']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }).customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), parseError, parseGetParams, controller.CVEID_GET_FILTER) @@ -180,8 +180,8 @@ router.post('/cve-id', query().custom((query) => { return mw.validateQueryParameterNames(query, ['amount', 'batch_type', 'short_name', 'cve_year']) }), query(['amount', 'batch_type', 'short_name', 'cve_year']).custom((val) => { return mw.containsNoInvalidCharacters(val) }), query(['amount']).isInt(), - query(['batch_type']).optional().isString().trim().escape().customSanitizer(val => { return val.toLowerCase() }), - query(['short_name']).isString().trim().escape().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), + query(['batch_type']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }).customSanitizer(val => { return val.toLowerCase() }), + query(['short_name']).isString().trim().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), query(['cve_year']).isNumeric().matches(/^[0-9]{4}$/), parseError, parsePostParams, @@ -343,8 +343,8 @@ router.put('/cve-id/:id', param(['id']).isString().matches(CONSTANTS.CVE_ID_REGEX), query().custom((query) => { return mw.validateQueryParameterNames(query, ['state', 'org']) }), query(['state', 'org']).custom((val) => { return mw.containsNoInvalidCharacters(val) }), - query(['state']).optional().isString().trim().escape().customSanitizer(val => { return val.toUpperCase() }).isIn(MODIFYTARGETS).withMessage(errorMsgs.ID_MODIFY_STATES), - query(['org']).optional().isString().trim().escape(), + query(['state']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }).customSanitizer(val => { return val.toUpperCase() }).isIn(MODIFYTARGETS).withMessage(errorMsgs.ID_MODIFY_STATES), + query(['org']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }), parseError, parsePostParams, mw.cnaMustOwnID, diff --git a/src/controller/cve.controller/index.js b/src/controller/cve.controller/index.js index 8c75ddf6..4a1f9cb9 100644 --- a/src/controller/cve.controller/index.js +++ b/src/controller/cve.controller/index.js @@ -159,14 +159,14 @@ router.get('/cve', query().custom((query) => { return mw.validateQueryParameterNames(query, ['page', 'time_modified.lt', 'time_modified.gt', 'state', 'count_only', 'assigner_short_name', 'assigner', 'cna_modified', 'adp_short_name']) }), query(['page', 'time_modified.lt', 'time_modified.gt', 'state', 'count_only', 'assigner_short_name', 'assigner', 'cna_modified', 'adp_short_name']).custom((val) => { return mw.containsNoInvalidCharacters(val) }), query(['page']).optional().isInt({ min: CONSTANTS.PAGINATOR_PAGE }), - query(['time_modified.lt']).optional().isString().trim().escape().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), - query(['time_modified.gt']).optional().isString().trim().escape().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), - query(['state']).optional().isString().trim().escape().customSanitizer(val => { return val.toUpperCase() }).isIn(CHOICES).withMessage(errorMsgs.CVE_FILTERED_STATES), + query(['time_modified.lt']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }).customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), + query(['time_modified.gt']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }).customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), + query(['state']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }).customSanitizer(val => { return val.toUpperCase() }).isIn(CHOICES).withMessage(errorMsgs.CVE_FILTERED_STATES), query(['count_only']).optional().isBoolean({ loose: true }).withMessage(errorMsgs.COUNT_ONLY), - query(['assigner_short_name']).optional().isString().trim().escape().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), - query(['assigner']).optional().isString().trim().escape().notEmpty(), + query(['assigner_short_name']).optional().isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), + query(['assigner']).optional().isString().trim().notEmpty().customSanitizer(val => { return encodeURI(val) }), query(['cna_modified']).optional().isBoolean({ loose: true }).withMessage(errorMsgs.CNA_MODIFIED), - query(['adp_short_name']).optional().isString().trim().escape().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), + query(['adp_short_name']).optional().isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), parseError, parseGetParams, controller.CVE_GET_FILTERED) @@ -246,15 +246,15 @@ router.get('/cve_cursor', mw.onlySecretariatOrBulkDownload, query().custom((query) => { return mw.validateQueryParameterNames(query, ['time_modified.lt', 'time_modified.gt', 'state', 'count_only', 'assigner_short_name', 'assigner', 'cna_modified', 'adp_short_name', 'next_page', 'previous_page', 'limit']) }), query(['time_modified.lt', 'time_modified.gt', 'state', 'count_only', 'assigner_short_name', 'assigner', 'cna_modified', 'adp_short_name', 'next_page', 'previous_page', 'limit']).custom((val) => { return mw.containsNoInvalidCharacters(val) }), - query(['time_modified.lt']).optional().isString().trim().escape().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), - query(['time_modified.gt']).optional().isString().trim().escape().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), - query(['state']).optional().isString().trim().escape().customSanitizer(val => { return val.toUpperCase() }).isIn(CHOICES).withMessage(errorMsgs.CVE_FILTERED_STATES), + query(['time_modified.lt']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }).customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), + query(['time_modified.gt']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }).customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), + query(['state']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }).customSanitizer(val => { return val.toUpperCase() }).isIn(CHOICES).withMessage(errorMsgs.CVE_FILTERED_STATES), query(['count_only']).optional().isBoolean({ loose: true }).withMessage(errorMsgs.COUNT_ONLY), - query(['assigner_short_name']).optional().isString().trim().escape().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), - query(['assigner']).optional().isString().trim().escape().notEmpty(), + query(['assigner_short_name']).optional().isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), + query(['assigner']).optional().isString().trim().notEmpty().customSanitizer(val => { return encodeURI(val) }), query(['cna_modified']).optional().isBoolean({ loose: true }).withMessage(errorMsgs.CNA_MODIFIED), - query(['adp_short_name']).optional().isString().trim().escape().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), - query(['limit']).optional().isString().trim().escape().notEmpty().isLength({ min: 1, max: CONSTANTS.PAGINATOR_OPTIONS.limit }), + query(['adp_short_name']).optional().isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), + query(['limit']).optional().isString().trim().notEmpty().isLength({ min: 1, max: CONSTANTS.PAGINATOR_OPTIONS.limit }).customSanitizer(val => { return encodeURI(val) }), parseError, parseGetParams, controller.CVE_GET_FILTERED_CURSOR) diff --git a/src/controller/org.controller/index.js b/src/controller/org.controller/index.js index 9bec6464..735c9faa 100644 --- a/src/controller/org.controller/index.js +++ b/src/controller/org.controller/index.js @@ -157,8 +157,8 @@ router.post('/org', */ mw.validateUser, mw.onlySecretariat, - body(['short_name']).isString().trim().escape().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), - body(['name']).isString().trim().escape().notEmpty(), + body(['short_name']).isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), + body(['name']).isString().trim().notEmpty().customSanitizer(val => { return encodeURI(val) }), body(['authority.active_roles']).optional() .custom(isFlatStringArray) .customSanitizer(toUpperCaseArray) @@ -234,7 +234,7 @@ router.get('/org/:identifier', } */ mw.validateUser, - param(['identifier']).isString().trim().escape(), + param(['identifier']).isString().trim().customSanitizer(val => { return encodeURI(val) }), parseError, parseGetParams, controller.ORG_SINGLE) @@ -312,10 +312,10 @@ router.put('/org/:shortname', mw.onlySecretariat, query().custom((query) => { return mw.validateQueryParameterNames(query, ['new_short_name', 'id_quota', 'name', 'active_roles.add', 'active_roles.remove']) }), query(['new_short_name', 'id_quota', 'name', 'active_roles.add', 'active_roles.remove']).custom((val) => { return mw.containsNoInvalidCharacters(val) }), - param(['shortname']).isString().trim().escape().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), - query(['new_short_name']).optional().isString().trim().escape().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), + param(['shortname']).isString().trim().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), + query(['new_short_name']).optional().isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), query(['id_quota']).optional().not().isArray().isInt({ min: CONSTANTS.MONGOOSE_VALIDATION.Org_policies_id_quota_min, max: CONSTANTS.MONGOOSE_VALIDATION.Org_policies_id_quota_max }).withMessage(errorMsgs.ID_QUOTA), - query(['name']).optional().isString().trim().escape().notEmpty(), + query(['name']).optional().isString().trim().notEmpty().customSanitizer(val => { return encodeURI(val) }), query(['active_roles.add']).optional().toArray() .custom(isFlatStringArray) .customSanitizer(toUpperCaseArray) @@ -394,7 +394,7 @@ router.get('/org/:shortname/id_quota', } */ mw.validateUser, - param(['shortname']).isString().trim().escape().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), + param(['shortname']).isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), parseError, parseGetParams, controller.ORG_ID_QUOTA) @@ -466,7 +466,7 @@ router.get('/org/:shortname/users', } */ mw.validateUser, - param(['shortname']).isString().trim().escape().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), + param(['shortname']).isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), query(['page']).optional().isInt({ min: CONSTANTS.PAGINATOR_PAGE }), parseError, parseGetParams, @@ -548,14 +548,14 @@ router.post('/org/:shortname/user', mw.validateUser, mw.onlySecretariatOrAdmin, mw.onlyOrgWithPartnerRole, - param(['shortname']).isString().trim().escape().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), - body(['username']).isString().trim().escape().notEmpty().custom(isValidUsername), - body(['org_uuid']).optional().isString().trim().escape(), - body(['uuid']).optional().isString().trim().escape(), - body(['name.first']).optional().isString().trim().escape().isLength({ max: CONSTANTS.MAX_FIRSTNAME_LENGTH }).withMessage(errorMsgs.FIRSTNAME_LENGTH), - body(['name.last']).optional().isString().trim().escape().isLength({ max: CONSTANTS.MAX_LASTNAME_LENGTH }).withMessage(errorMsgs.LASTNAME_LENGTH), - body(['name.middle']).optional().isString().trim().escape().isLength({ max: CONSTANTS.MAX_MIDDLENAME_LENGTH }).withMessage(errorMsgs.MIDDLENAME_LENGTH), - body(['name.suffix']).optional().isString().trim().escape().isLength({ max: CONSTANTS.MAX_SUFFIX_LENGTH }).withMessage(errorMsgs.SUFFIX_LENGTH), + param(['shortname']).isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), + body(['username']).isString().trim().notEmpty().custom(isValidUsername).customSanitizer(val => { return encodeURI(val) }), + body(['org_uuid']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }), + body(['uuid']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }), + body(['name.first']).optional().isString().trim().isLength({ max: CONSTANTS.MAX_FIRSTNAME_LENGTH }).withMessage(errorMsgs.FIRSTNAME_LENGTH).customSanitizer(val => { return encodeURI(val) }), + body(['name.last']).optional().isString().trim().isLength({ max: CONSTANTS.MAX_LASTNAME_LENGTH }).withMessage(errorMsgs.LASTNAME_LENGTH).customSanitizer(val => { return encodeURI(val) }), + body(['name.middle']).optional().isString().trim().isLength({ max: CONSTANTS.MAX_MIDDLENAME_LENGTH }).withMessage(errorMsgs.MIDDLENAME_LENGTH).customSanitizer(val => { return encodeURI(val) }), + body(['name.suffix']).optional().isString().trim().isLength({ max: CONSTANTS.MAX_SUFFIX_LENGTH }).withMessage(errorMsgs.SUFFIX_LENGTH).customSanitizer(val => { return encodeURI(val) }), body(['authority.active_roles']).optional() .custom(mw.isFlatStringArray) .customSanitizer(toUpperCaseArray) @@ -631,8 +631,8 @@ router.get('/org/:shortname/user/:username', } */ mw.validateUser, - param(['shortname']).isString().trim().escape().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), - param(['username']).isString().trim().escape().notEmpty().custom(isValidUsername), + param(['shortname']).isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), + param(['username']).isString().trim().notEmpty().custom(isValidUsername).customSanitizer(val => { return encodeURI(val) }), parseError, parseGetParams, controller.USER_SINGLE) @@ -721,15 +721,15 @@ router.put('/org/:shortname/user/:username', }), query(['active', 'new_username', 'org_short_name', 'name.first', 'name.last', 'name.middle', 'name.suffix', 'active_roles.add', 'active_roles.remove']).custom((val) => { return mw.containsNoInvalidCharacters(val) }), - param(['shortname']).isString().trim().escape().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), - param(['username']).isString().trim().escape().notEmpty().custom(isValidUsername), + param(['shortname']).isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), + param(['username']).isString().trim().notEmpty().custom(isValidUsername).customSanitizer(val => { return encodeURI(val) }), query(['active']).optional().isBoolean({ loose: true }), - query(['new_username']).optional().isString().trim().escape().notEmpty().custom(isValidUsername), - query(['org_short_name']).optional().isString().trim().escape().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), - body(['name.first']).optional().isString().trim().escape().isLength({ max: CONSTANTS.MAX_FIRSTNAME_LENGTH }).withMessage(errorMsgs.FIRSTNAME_LENGTH), - body(['name.last']).optional().isString().trim().escape().isLength({ max: CONSTANTS.MAX_LASTNAME_LENGTH }).withMessage(errorMsgs.LASTNAME_LENGTH), - body(['name.middle']).optional().isString().trim().escape().isLength({ max: CONSTANTS.MAX_MIDDLENAME_LENGTH }).withMessage(errorMsgs.MIDDLENAME_LENGTH), - body(['name.suffix']).optional().isString().trim().escape().isLength({ max: CONSTANTS.MAX_SUFFIX_LENGTH }).withMessage(errorMsgs.SUFFIX_LENGTH), + query(['new_username']).optional().isString().trim().notEmpty().custom(isValidUsername).customSanitizer(val => { return encodeURI(val) }), + query(['org_short_name']).optional().isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), + body(['name.first']).optional().isString().trim().isLength({ max: CONSTANTS.MAX_FIRSTNAME_LENGTH }).withMessage(errorMsgs.FIRSTNAME_LENGTH).customSanitizer(val => { return encodeURI(val) }), + body(['name.last']).optional().isString().trim().isLength({ max: CONSTANTS.MAX_LASTNAME_LENGTH }).withMessage(errorMsgs.LASTNAME_LENGTH).customSanitizer(val => { return encodeURI(val) }), + body(['name.middle']).optional().isString().trim().isLength({ max: CONSTANTS.MAX_MIDDLENAME_LENGTH }).withMessage(errorMsgs.MIDDLENAME_LENGTH).customSanitizer(val => { return encodeURI(val) }), + body(['name.suffix']).optional().isString().trim().isLength({ max: CONSTANTS.MAX_SUFFIX_LENGTH }).withMessage(errorMsgs.SUFFIX_LENGTH).customSanitizer(val => { return encodeURI(val) }), query(['active_roles.add']).optional().toArray() .custom(isFlatStringArray) .customSanitizer(toUpperCaseArray) @@ -811,8 +811,8 @@ router.put('/org/:shortname/user/:username/reset_secret', */ mw.validateUser, mw.onlyOrgWithPartnerRole, - param(['shortname']).isString().trim().escape().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), - param(['username']).isString().trim().escape().notEmpty().custom(isValidUsername), + param(['shortname']).isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), + param(['username']).isString().trim().notEmpty().custom(isValidUsername).customSanitizer(val => { return encodeURI(val) }), parseError, parsePostParams, controller.USER_RESET_SECRET) diff --git a/src/controller/org.controller/org.controller.js b/src/controller/org.controller/org.controller.js index 62e38fe3..e60c76a5 100644 --- a/src/controller/org.controller/org.controller.js +++ b/src/controller/org.controller/org.controller.js @@ -9,7 +9,6 @@ const uuid = require('uuid') const errors = require('./error') const error = new errors.OrgControllerError() const validateUUID = require('uuid').validate -const decodeEntities = require('html-entities').decode const booleanIsTrue = require('../../utils/utils').booleanIsTrue /** @@ -246,11 +245,11 @@ async function createOrg (req, res, next) { switch (key) { case 'short_name': - newOrg.short_name = decodeEntities(req.ctx.body.short_name) + newOrg.short_name = req.ctx.body.short_name break case 'name': - newOrg.name = decodeEntities(req.ctx.body.name) + newOrg.name = req.ctx.body.name break case 'authority': @@ -342,10 +341,10 @@ async function updateOrg (req, res, next) { const key = k.toLowerCase() if (key === 'new_short_name') { - newOrg.short_name = decodeEntities(req.ctx.query.new_short_name) + newOrg.short_name = req.ctx.query.new_short_name agt = setAggregateOrgObj({ short_name: newOrg.short_name }) } else if (key === 'name') { - newOrg.name = decodeEntities(req.ctx.query.name) + newOrg.name = req.ctx.query.name } else if (key === 'id_quota') { newOrg.policies.id_quota = req.ctx.query.id_quota } else if (key === 'active_roles.add') { @@ -462,16 +461,16 @@ async function createUser (req, res, next) { } } else if (key === 'name') { if (req.ctx.body.name.first) { - newUser.name.first = decodeEntities(req.ctx.body.name.first) + newUser.name.first = req.ctx.body.name.first } if (req.ctx.body.name.last) { - newUser.name.last = decodeEntities(req.ctx.body.name.last) + newUser.name.last = req.ctx.body.name.last } if (req.ctx.body.name.middle) { - newUser.name.middle = decodeEntities(req.ctx.body.name.middle) + newUser.name.middle = req.ctx.body.name.middle } if (req.ctx.body.name.suffix) { - newUser.name.suffix = decodeEntities(req.ctx.body.name.suffix) + newUser.name.suffix = req.ctx.body.name.suffix } } else if (key === 'org_uuid') { return res.status(400).json(error.uuidProvided('org')) @@ -599,13 +598,13 @@ async function updateUser (req, res, next) { return res.status(403).json(error.notAllowedToChangeOrganization()) } } else if (key === 'name.first') { - newUser.name.first = decodeEntities(req.ctx.query['name.first']) + newUser.name.first = req.ctx.query['name.first'] } else if (key === 'name.last') { - newUser.name.last = decodeEntities(req.ctx.query['name.last']) + newUser.name.last = req.ctx.query['name.last'] } else if (key === 'name.middle') { - newUser.name.middle = decodeEntities(req.ctx.query['name.middle']) + newUser.name.middle = req.ctx.query['name.middle'] } else if (key === 'name.suffix') { - newUser.name.suffix = decodeEntities(req.ctx.query['name.suffix']) + newUser.name.suffix = req.ctx.query['name.suffix'] } else if (key === 'active') { newUser.active = booleanIsTrue(req.ctx.query.active) changesRequirePrivilegedRole = true From bef1c6218fc534423f0ccbf118f9f33919df371b Mon Sep 17 00:00:00 2001 From: "Daigneau, Jeremy T" Date: Tue, 21 Nov 2023 14:12:06 -0500 Subject: [PATCH 08/14] #962 removed encodeURI, we don't want to encode whitespace --- src/controller/cve-id.controller/index.js | 18 ++++---- src/controller/cve.controller/index.js | 26 +++++------ src/controller/org.controller/index.js | 56 +++++++++++------------ test/integration-tests/constants.js | 17 ++++++- test/integration-tests/org/postOrgTest.js | 44 ++++++++++++++++++ 5 files changed, 110 insertions(+), 51 deletions(-) create mode 100644 test/integration-tests/org/postOrgTest.js diff --git a/src/controller/cve-id.controller/index.js b/src/controller/cve-id.controller/index.js index a8cfa7ab..0ab114d0 100644 --- a/src/controller/cve-id.controller/index.js +++ b/src/controller/cve-id.controller/index.js @@ -88,12 +88,12 @@ router.get('/cve-id', query().custom((query) => { return mw.validateQueryParameterNames(query, ['page', 'state', 'cve_id_year', 'time_reserved.lt', 'time_reserved.gt', 'time_modified.lt', 'time_modified.gt']) }), query(['page', 'state', 'cve_id_year', 'time_reserved.lt', 'time_reserved.gt', 'time_modified.lt', 'time_modified.gt']).custom((val) => { return mw.containsNoInvalidCharacters(val) }), query(['page']).optional().isInt({ min: CONSTANTS.PAGINATOR_PAGE }), - query(['state']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }).customSanitizer(val => { return val.toUpperCase() }).isIn(CHOICES).withMessage(errorMsgs.ID_STATES), + query(['state']).optional().isString().trim().customSanitizer(val => { return val.toUpperCase() }).isIn(CHOICES).withMessage(errorMsgs.ID_STATES), query(['cve_id_year']).optional().isNumeric().matches(/^[0-9]{4}$/), - query(['time_reserved.lt']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }).customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), - query(['time_reserved.gt']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }).customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), - query(['time_modified.lt']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }).customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), - query(['time_modified.gt']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }).customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), + query(['time_reserved.lt']).optional().isString().trim().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), + query(['time_reserved.gt']).optional().isString().trim().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), + query(['time_modified.lt']).optional().isString().trim().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), + query(['time_modified.gt']).optional().isString().trim().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), parseError, parseGetParams, controller.CVEID_GET_FILTER) @@ -180,8 +180,8 @@ router.post('/cve-id', query().custom((query) => { return mw.validateQueryParameterNames(query, ['amount', 'batch_type', 'short_name', 'cve_year']) }), query(['amount', 'batch_type', 'short_name', 'cve_year']).custom((val) => { return mw.containsNoInvalidCharacters(val) }), query(['amount']).isInt(), - query(['batch_type']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }).customSanitizer(val => { return val.toLowerCase() }), - query(['short_name']).isString().trim().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), + query(['batch_type']).optional().isString().trim().customSanitizer(val => { return val.toLowerCase() }), + query(['short_name']).isString().trim().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), query(['cve_year']).isNumeric().matches(/^[0-9]{4}$/), parseError, parsePostParams, @@ -343,8 +343,8 @@ router.put('/cve-id/:id', param(['id']).isString().matches(CONSTANTS.CVE_ID_REGEX), query().custom((query) => { return mw.validateQueryParameterNames(query, ['state', 'org']) }), query(['state', 'org']).custom((val) => { return mw.containsNoInvalidCharacters(val) }), - query(['state']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }).customSanitizer(val => { return val.toUpperCase() }).isIn(MODIFYTARGETS).withMessage(errorMsgs.ID_MODIFY_STATES), - query(['org']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }), + query(['state']).optional().isString().trim().customSanitizer(val => { return val.toUpperCase() }).isIn(MODIFYTARGETS).withMessage(errorMsgs.ID_MODIFY_STATES), + query(['org']).optional().isString().trim(), parseError, parsePostParams, mw.cnaMustOwnID, diff --git a/src/controller/cve.controller/index.js b/src/controller/cve.controller/index.js index 4a1f9cb9..eba11d17 100644 --- a/src/controller/cve.controller/index.js +++ b/src/controller/cve.controller/index.js @@ -159,14 +159,14 @@ router.get('/cve', query().custom((query) => { return mw.validateQueryParameterNames(query, ['page', 'time_modified.lt', 'time_modified.gt', 'state', 'count_only', 'assigner_short_name', 'assigner', 'cna_modified', 'adp_short_name']) }), query(['page', 'time_modified.lt', 'time_modified.gt', 'state', 'count_only', 'assigner_short_name', 'assigner', 'cna_modified', 'adp_short_name']).custom((val) => { return mw.containsNoInvalidCharacters(val) }), query(['page']).optional().isInt({ min: CONSTANTS.PAGINATOR_PAGE }), - query(['time_modified.lt']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }).customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), - query(['time_modified.gt']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }).customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), - query(['state']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }).customSanitizer(val => { return val.toUpperCase() }).isIn(CHOICES).withMessage(errorMsgs.CVE_FILTERED_STATES), + query(['time_modified.lt']).optional().isString().trim().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), + query(['time_modified.gt']).optional().isString().trim().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), + query(['state']).optional().isString().trim().customSanitizer(val => { return val.toUpperCase() }).isIn(CHOICES).withMessage(errorMsgs.CVE_FILTERED_STATES), query(['count_only']).optional().isBoolean({ loose: true }).withMessage(errorMsgs.COUNT_ONLY), - query(['assigner_short_name']).optional().isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), - query(['assigner']).optional().isString().trim().notEmpty().customSanitizer(val => { return encodeURI(val) }), + query(['assigner_short_name']).optional().isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), + query(['assigner']).optional().isString().trim().notEmpty(), query(['cna_modified']).optional().isBoolean({ loose: true }).withMessage(errorMsgs.CNA_MODIFIED), - query(['adp_short_name']).optional().isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), + query(['adp_short_name']).optional().isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), parseError, parseGetParams, controller.CVE_GET_FILTERED) @@ -246,15 +246,15 @@ router.get('/cve_cursor', mw.onlySecretariatOrBulkDownload, query().custom((query) => { return mw.validateQueryParameterNames(query, ['time_modified.lt', 'time_modified.gt', 'state', 'count_only', 'assigner_short_name', 'assigner', 'cna_modified', 'adp_short_name', 'next_page', 'previous_page', 'limit']) }), query(['time_modified.lt', 'time_modified.gt', 'state', 'count_only', 'assigner_short_name', 'assigner', 'cna_modified', 'adp_short_name', 'next_page', 'previous_page', 'limit']).custom((val) => { return mw.containsNoInvalidCharacters(val) }), - query(['time_modified.lt']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }).customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), - query(['time_modified.gt']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }).customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), - query(['state']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }).customSanitizer(val => { return val.toUpperCase() }).isIn(CHOICES).withMessage(errorMsgs.CVE_FILTERED_STATES), + query(['time_modified.lt']).optional().isString().trim().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), + query(['time_modified.gt']).optional().isString().trim().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT), + query(['state']).optional().isString().trim().customSanitizer(val => { return val.toUpperCase() }).isIn(CHOICES).withMessage(errorMsgs.CVE_FILTERED_STATES), query(['count_only']).optional().isBoolean({ loose: true }).withMessage(errorMsgs.COUNT_ONLY), - query(['assigner_short_name']).optional().isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), - query(['assigner']).optional().isString().trim().notEmpty().customSanitizer(val => { return encodeURI(val) }), + query(['assigner_short_name']).optional().isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), + query(['assigner']).optional().isString().trim().notEmpty(), query(['cna_modified']).optional().isBoolean({ loose: true }).withMessage(errorMsgs.CNA_MODIFIED), - query(['adp_short_name']).optional().isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), - query(['limit']).optional().isString().trim().notEmpty().isLength({ min: 1, max: CONSTANTS.PAGINATOR_OPTIONS.limit }).customSanitizer(val => { return encodeURI(val) }), + query(['adp_short_name']).optional().isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), + query(['limit']).optional().isString().trim().notEmpty().isLength({ min: 1, max: CONSTANTS.PAGINATOR_OPTIONS.limit }), parseError, parseGetParams, controller.CVE_GET_FILTERED_CURSOR) diff --git a/src/controller/org.controller/index.js b/src/controller/org.controller/index.js index 735c9faa..1c94e1b5 100644 --- a/src/controller/org.controller/index.js +++ b/src/controller/org.controller/index.js @@ -157,8 +157,8 @@ router.post('/org', */ mw.validateUser, mw.onlySecretariat, - body(['short_name']).isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), - body(['name']).isString().trim().notEmpty().customSanitizer(val => { return encodeURI(val) }), + body(['short_name']).isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), + body(['name']).isString().trim().notEmpty(), body(['authority.active_roles']).optional() .custom(isFlatStringArray) .customSanitizer(toUpperCaseArray) @@ -234,7 +234,7 @@ router.get('/org/:identifier', } */ mw.validateUser, - param(['identifier']).isString().trim().customSanitizer(val => { return encodeURI(val) }), + param(['identifier']).isString().trim(), parseError, parseGetParams, controller.ORG_SINGLE) @@ -312,10 +312,10 @@ router.put('/org/:shortname', mw.onlySecretariat, query().custom((query) => { return mw.validateQueryParameterNames(query, ['new_short_name', 'id_quota', 'name', 'active_roles.add', 'active_roles.remove']) }), query(['new_short_name', 'id_quota', 'name', 'active_roles.add', 'active_roles.remove']).custom((val) => { return mw.containsNoInvalidCharacters(val) }), - param(['shortname']).isString().trim().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), - query(['new_short_name']).optional().isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), + param(['shortname']).isString().trim().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), + query(['new_short_name']).optional().isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), query(['id_quota']).optional().not().isArray().isInt({ min: CONSTANTS.MONGOOSE_VALIDATION.Org_policies_id_quota_min, max: CONSTANTS.MONGOOSE_VALIDATION.Org_policies_id_quota_max }).withMessage(errorMsgs.ID_QUOTA), - query(['name']).optional().isString().trim().notEmpty().customSanitizer(val => { return encodeURI(val) }), + query(['name']).optional().isString().trim().notEmpty(), query(['active_roles.add']).optional().toArray() .custom(isFlatStringArray) .customSanitizer(toUpperCaseArray) @@ -394,7 +394,7 @@ router.get('/org/:shortname/id_quota', } */ mw.validateUser, - param(['shortname']).isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), + param(['shortname']).isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), parseError, parseGetParams, controller.ORG_ID_QUOTA) @@ -466,7 +466,7 @@ router.get('/org/:shortname/users', } */ mw.validateUser, - param(['shortname']).isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), + param(['shortname']).isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), query(['page']).optional().isInt({ min: CONSTANTS.PAGINATOR_PAGE }), parseError, parseGetParams, @@ -548,14 +548,14 @@ router.post('/org/:shortname/user', mw.validateUser, mw.onlySecretariatOrAdmin, mw.onlyOrgWithPartnerRole, - param(['shortname']).isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), - body(['username']).isString().trim().notEmpty().custom(isValidUsername).customSanitizer(val => { return encodeURI(val) }), - body(['org_uuid']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }), - body(['uuid']).optional().isString().trim().customSanitizer(val => { return encodeURI(val) }), - body(['name.first']).optional().isString().trim().isLength({ max: CONSTANTS.MAX_FIRSTNAME_LENGTH }).withMessage(errorMsgs.FIRSTNAME_LENGTH).customSanitizer(val => { return encodeURI(val) }), - body(['name.last']).optional().isString().trim().isLength({ max: CONSTANTS.MAX_LASTNAME_LENGTH }).withMessage(errorMsgs.LASTNAME_LENGTH).customSanitizer(val => { return encodeURI(val) }), - body(['name.middle']).optional().isString().trim().isLength({ max: CONSTANTS.MAX_MIDDLENAME_LENGTH }).withMessage(errorMsgs.MIDDLENAME_LENGTH).customSanitizer(val => { return encodeURI(val) }), - body(['name.suffix']).optional().isString().trim().isLength({ max: CONSTANTS.MAX_SUFFIX_LENGTH }).withMessage(errorMsgs.SUFFIX_LENGTH).customSanitizer(val => { return encodeURI(val) }), + param(['shortname']).isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), + body(['username']).isString().trim().notEmpty().custom(isValidUsername), + body(['org_uuid']).optional().isString().trim(), + body(['uuid']).optional().isString().trim(), + body(['name.first']).optional().isString().trim().isLength({ max: CONSTANTS.MAX_FIRSTNAME_LENGTH }).withMessage(errorMsgs.FIRSTNAME_LENGTH), + body(['name.last']).optional().isString().trim().isLength({ max: CONSTANTS.MAX_LASTNAME_LENGTH }).withMessage(errorMsgs.LASTNAME_LENGTH), + body(['name.middle']).optional().isString().trim().isLength({ max: CONSTANTS.MAX_MIDDLENAME_LENGTH }).withMessage(errorMsgs.MIDDLENAME_LENGTH), + body(['name.suffix']).optional().isString().trim().isLength({ max: CONSTANTS.MAX_SUFFIX_LENGTH }).withMessage(errorMsgs.SUFFIX_LENGTH), body(['authority.active_roles']).optional() .custom(mw.isFlatStringArray) .customSanitizer(toUpperCaseArray) @@ -631,8 +631,8 @@ router.get('/org/:shortname/user/:username', } */ mw.validateUser, - param(['shortname']).isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), - param(['username']).isString().trim().notEmpty().custom(isValidUsername).customSanitizer(val => { return encodeURI(val) }), + param(['shortname']).isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), + param(['username']).isString().trim().notEmpty().custom(isValidUsername), parseError, parseGetParams, controller.USER_SINGLE) @@ -721,15 +721,15 @@ router.put('/org/:shortname/user/:username', }), query(['active', 'new_username', 'org_short_name', 'name.first', 'name.last', 'name.middle', 'name.suffix', 'active_roles.add', 'active_roles.remove']).custom((val) => { return mw.containsNoInvalidCharacters(val) }), - param(['shortname']).isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), - param(['username']).isString().trim().notEmpty().custom(isValidUsername).customSanitizer(val => { return encodeURI(val) }), + param(['shortname']).isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), + param(['username']).isString().trim().notEmpty().custom(isValidUsername), query(['active']).optional().isBoolean({ loose: true }), - query(['new_username']).optional().isString().trim().notEmpty().custom(isValidUsername).customSanitizer(val => { return encodeURI(val) }), - query(['org_short_name']).optional().isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), - body(['name.first']).optional().isString().trim().isLength({ max: CONSTANTS.MAX_FIRSTNAME_LENGTH }).withMessage(errorMsgs.FIRSTNAME_LENGTH).customSanitizer(val => { return encodeURI(val) }), - body(['name.last']).optional().isString().trim().isLength({ max: CONSTANTS.MAX_LASTNAME_LENGTH }).withMessage(errorMsgs.LASTNAME_LENGTH).customSanitizer(val => { return encodeURI(val) }), - body(['name.middle']).optional().isString().trim().isLength({ max: CONSTANTS.MAX_MIDDLENAME_LENGTH }).withMessage(errorMsgs.MIDDLENAME_LENGTH).customSanitizer(val => { return encodeURI(val) }), - body(['name.suffix']).optional().isString().trim().isLength({ max: CONSTANTS.MAX_SUFFIX_LENGTH }).withMessage(errorMsgs.SUFFIX_LENGTH).customSanitizer(val => { return encodeURI(val) }), + query(['new_username']).optional().isString().trim().notEmpty().custom(isValidUsername), + query(['org_short_name']).optional().isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), + body(['name.first']).optional().isString().trim().isLength({ max: CONSTANTS.MAX_FIRSTNAME_LENGTH }).withMessage(errorMsgs.FIRSTNAME_LENGTH), + body(['name.last']).optional().isString().trim().isLength({ max: CONSTANTS.MAX_LASTNAME_LENGTH }).withMessage(errorMsgs.LASTNAME_LENGTH), + body(['name.middle']).optional().isString().trim().isLength({ max: CONSTANTS.MAX_MIDDLENAME_LENGTH }).withMessage(errorMsgs.MIDDLENAME_LENGTH), + body(['name.suffix']).optional().isString().trim().isLength({ max: CONSTANTS.MAX_SUFFIX_LENGTH }).withMessage(errorMsgs.SUFFIX_LENGTH), query(['active_roles.add']).optional().toArray() .custom(isFlatStringArray) .customSanitizer(toUpperCaseArray) @@ -811,8 +811,8 @@ router.put('/org/:shortname/user/:username/reset_secret', */ mw.validateUser, mw.onlyOrgWithPartnerRole, - param(['shortname']).isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }).customSanitizer(val => { return encodeURI(val) }), - param(['username']).isString().trim().notEmpty().custom(isValidUsername).customSanitizer(val => { return encodeURI(val) }), + param(['shortname']).isString().trim().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), + param(['username']).isString().trim().notEmpty().custom(isValidUsername), parseError, parsePostParams, controller.USER_RESET_SECRET) diff --git a/test/integration-tests/constants.js b/test/integration-tests/constants.js index 4b8b954b..8eba9fe3 100644 --- a/test/integration-tests/constants.js +++ b/test/integration-tests/constants.js @@ -241,6 +241,20 @@ const testAdp2 = { } } +const testOrg = { + + short_name: 'test_org', + name: 'Test Organization', + authority: { + active_roles: [ + 'CNA' + ] + }, + policies: { + id_quota: 100000 + } +} + module.exports = { headers, nonSecretariatUserHeaders, @@ -249,5 +263,6 @@ module.exports = { testCve, testCveEdited, testAdp, - testAdp2 + testAdp2, + testOrg } diff --git a/test/integration-tests/org/postOrgTest.js b/test/integration-tests/org/postOrgTest.js new file mode 100644 index 00000000..34697e28 --- /dev/null +++ b/test/integration-tests/org/postOrgTest.js @@ -0,0 +1,44 @@ +/* eslint-disable no-unused-expressions */ +const chai = require('chai') +chai.use(require('chai-http')) +const expect = chai.expect + +const constants = require('../constants.js') +const app = require('../../../src/index.js') + +describe('Testing Org post endpoint', () => { + context('Positive Tests', () => { + it('Allows creation of org', async () => { + await chai.request(app) + .post('/api/org') + .set({ ...constants.headers }) + .send(constants.testOrg) + .then((res, err) => { + expect(err).to.be.undefined + expect(res).to.have.status(200) + + expect(res.body).to.haveOwnProperty('message') + expect(res.body.message).to.equal(constants.testOrg.short_name + ' organization was successfully created.') + + expect(res.body).to.haveOwnProperty('created') + + expect(res.body.created).to.haveOwnProperty('name') + expect(res.body.created.name).to.equal(constants.testOrg.name) + + expect(res.body.created).to.haveOwnProperty('short_name') + expect(res.body.created.short_name).to.equal(constants.testOrg.short_name) + + expect(res.body.created).to.haveOwnProperty('UUID') + + expect(res.body.created).to.haveOwnProperty('policies') + expect(res.body.created.policies).to.deep.equal(constants.testOrg.policies) + + expect(res.body.created).to.haveOwnProperty('authority') + expect(res.body.created.authority).to.deep.equal(constants.testOrg.authority) + }) + }) + }) + context('Negitive Test', () => { + + }) +}) From 15fa2b62842862382ce5f69c41bf3355e301da35 Mon Sep 17 00:00:00 2001 From: "Daigneau, Jeremy T" Date: Tue, 21 Nov 2023 14:59:12 -0500 Subject: [PATCH 09/14] #962 added negative org creation test --- test/integration-tests/constants.js | 17 ++++++++++++++++- test/integration-tests/org/postOrgTest.js | 12 ++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/test/integration-tests/constants.js b/test/integration-tests/constants.js index 8eba9fe3..6aab5359 100644 --- a/test/integration-tests/constants.js +++ b/test/integration-tests/constants.js @@ -255,6 +255,20 @@ const testOrg = { } } +const existingOrg = { + + short_name: 'win_5', + name: 'Test Organization', + authority: { + active_roles: [ + 'CNA' + ] + }, + policies: { + id_quota: 100000 + } +} + module.exports = { headers, nonSecretariatUserHeaders, @@ -264,5 +278,6 @@ module.exports = { testCveEdited, testAdp, testAdp2, - testOrg + testOrg, + existingOrg } diff --git a/test/integration-tests/org/postOrgTest.js b/test/integration-tests/org/postOrgTest.js index 34697e28..08517dd3 100644 --- a/test/integration-tests/org/postOrgTest.js +++ b/test/integration-tests/org/postOrgTest.js @@ -39,6 +39,18 @@ describe('Testing Org post endpoint', () => { }) }) context('Negitive Test', () => { + it('Should fail to create an org that already exists ', async () => { + await chai.request(app) + .post('/api/org') + .set({ ...constants.headers }) + .send(constants.existingOrg) + .then((res, err) => { + expect(err).to.be.undefined + expect(res).to.have.status(400) + expect(res.body).to.haveOwnProperty('error') + expect(res.body.error).to.equal('ORG_EXISTS') + }) + }) }) }) From e5d0da2f56328a8f17c5d5fffea8030c0af2c296 Mon Sep 17 00:00:00 2001 From: david-rocca Date: Wed, 29 Nov 2023 09:27:02 -0500 Subject: [PATCH 10/14] #836 Updated the edit user code to check authentication before actions are taken place. Added a test --- .../org.controller/org.controller.js | 32 +++++++++++------- test/integration-tests/user/updateUserTest.js | 33 +++++++++++++++++++ 2 files changed, 53 insertions(+), 12 deletions(-) create mode 100644 test/integration-tests/user/updateUserTest.js diff --git a/src/controller/org.controller/org.controller.js b/src/controller/org.controller/org.controller.js index e60c76a5..73023be3 100644 --- a/src/controller/org.controller/org.controller.js +++ b/src/controller/org.controller/org.controller.js @@ -544,7 +544,6 @@ async function updateUser (req, res, next) { const shortName = req.ctx.params.shortname const newUser = new User() let newOrgShortName = null - let changesRequirePrivilegedRole = false // Set variable to true if protected fields are being modified const removeRoles = [] const addRoles = [] const userRepo = req.ctx.repositories.getUserRepository() @@ -584,15 +583,33 @@ async function updateUser (req, res, next) { newUser.name.middle = user.name.middle newUser.name.suffix = user.name.suffix + const queryParameterPermissions = { + new_username: true, + org_short_name: true, + 'name.first': false, + 'name.last': false, + 'name.middle': false, + 'name.suffix': false, + active: true, + 'active_roles.add': true, + 'active_roles.remove': true + + } + + // Check to ensure that the user has the right permissions to edit the fields tha they are requesting to edit, and fail fast if they do not. + if (Object.keys(req.ctx.query).length > 0 && Object.keys(req.ctx.query).some((key) => { return queryParameterPermissions[key] }) && !(isAdmin || isSecretariat)) { + logger.info({ uuid: req.ctx.uuid, message: 'The user could not be updated because ' + requesterUsername + ' user is not Org Admin or Secretariat to modify these fields.' }) + console.log('in failed admin') + return res.status(403).json(error.notOrgAdminOrSecretariatUpdate()) + } + for (const k in req.ctx.query) { const key = k.toLowerCase() if (key === 'new_username') { newUser.username = req.ctx.query.new_username - changesRequirePrivilegedRole = true } else if (key === 'org_short_name') { newOrgShortName = req.ctx.query.org_short_name - changesRequirePrivilegedRole = true if (!isSecretariat) { logger.info({ uuid: req.ctx.uuid, message: 'The user could not be updated because ' + requesterUsername + ' is an Org Admin and tried to reassign the organization.' }) return res.status(403).json(error.notAllowedToChangeOrganization()) @@ -607,13 +624,11 @@ async function updateUser (req, res, next) { newUser.name.suffix = req.ctx.query['name.suffix'] } else if (key === 'active') { newUser.active = booleanIsTrue(req.ctx.query.active) - changesRequirePrivilegedRole = true } else if (key === 'active_roles.add') { if (Array.isArray(req.ctx.query['active_roles.add'])) { req.ctx.query['active_roles.add'].forEach(r => { addRoles.push(r) }) - changesRequirePrivilegedRole = true } } else if (key === 'active_roles.remove') { if (Array.isArray(req.ctx.query['active_roles.remove'])) { @@ -624,17 +639,10 @@ async function updateUser (req, res, next) { } removeRoles.push(r) } - changesRequirePrivilegedRole = true } } } - // Check for correct privileges if the requested changes require them - if (changesRequirePrivilegedRole && !(isAdmin || isSecretariat)) { - logger.info({ uuid: req.ctx.uuid, message: 'The user could not be updated because ' + requesterUsername + ' user is not Org Admin or Secretariat to modify these fields.' }) - return res.status(403).json(error.notOrgAdminOrSecretariatUpdate()) - } - // check if the new org exist if (newOrgShortName) { newUser.org_UUID = await orgRepo.getOrgUUID(newOrgShortName) diff --git a/test/integration-tests/user/updateUserTest.js b/test/integration-tests/user/updateUserTest.js new file mode 100644 index 00000000..f25d55f7 --- /dev/null +++ b/test/integration-tests/user/updateUserTest.js @@ -0,0 +1,33 @@ +/* eslint-disable no-unused-expressions */ + +const chai = require('chai') +chai.use(require('chai-http')) + +const expect = chai.expect + +const constants = require('../constants.js') +const app = require('../../../src/index.js') + +describe('Testing Edit user endpoint', () => { + context('User edit tests', () => { + it('Should return 200 when only name changes are done', async () => { + await chai.request(app) + .put('/api/org/win_5/user/jasminesmith@win_5.com?name.first=NewName') + .set(constants.nonSecretariatUserHeaders) + .then((res, err) => { + expect(err).to.be.undefined + expect(res).to.have.status(200) + }) + }) + it('Should return an error when admin is required', async () => { + await chai.request(app) + .put('/api/org/win_5/user/jasminesmith@win_5.com?new_username=NewUsername') + .set(constants.nonSecretariatUserHeaders) + .then((res, err) => { + expect(err).to.be.undefined + expect(res).to.have.status(403) + expect(res.body.error).to.contain('NOT_ORG_ADMIN_OR_SECRETARIAT_UPDATE') + }) + }) + }) +}) From a3820832f40f529485c1e7bb0de6de37c013ee63 Mon Sep 17 00:00:00 2001 From: david-rocca Date: Wed, 29 Nov 2023 09:28:24 -0500 Subject: [PATCH 11/14] Added another test to create a non admin user. --- test/integration-tests/user/createUserTest.js | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/integration-tests/user/createUserTest.js b/test/integration-tests/user/createUserTest.js index 7de53d70..45c32f86 100644 --- a/test/integration-tests/user/createUserTest.js +++ b/test/integration-tests/user/createUserTest.js @@ -21,6 +21,20 @@ const body = { active_roles: ['Admin'] } } + +const nonAdminBody = { + username: 'nonAdminUser', + active: 'true', + name: { + first: 'TestCnaAdmin', + last: 'test', + middle: 'N', + suffix: 'I' + }, + authority: { + } +} + describe('Testing create user endpoint', () => { it('Should return 200 and new user', (done) => { chai.request(app) @@ -35,4 +49,17 @@ describe('Testing create user endpoint', () => { done() }) }) + it('Should return 200 and create a non admin user', (done) => { + chai.request(app) + .post('/api/org/range_4/user') + .set(constants.headers) + .send(nonAdminBody) + .end((err, res) => { + expect(err).to.be.null + expect(res.body).to.have.property('created') + expect(res.body.created.username).to.equal(nonAdminBody.username) + expect(res).to.have.status(200) + done() + }) + }) }) From 846af9f8a214c1598a6904af85def7c3fb83ee5f Mon Sep 17 00:00:00 2001 From: david-rocca Date: Wed, 29 Nov 2023 09:56:47 -0500 Subject: [PATCH 12/14] #836 Due to the new workflow, this test is no longer valid --- test-http/src/test/org_user_tests/regular_users.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/test-http/src/test/org_user_tests/regular_users.py b/test-http/src/test/org_user_tests/regular_users.py index 3d015c1e..c50c6270 100644 --- a/test-http/src/test/org_user_tests/regular_users.py +++ b/test-http/src/test/org_user_tests/regular_users.py @@ -75,19 +75,6 @@ def test_regular_user_cannot_update_duplicate_user_with_new_username(reg_user_he response_contains_json(res, 'error', 'NOT_ORG_ADMIN_OR_SECRETARIAT_UPDATE') -def test_regular_user_cannot_update_organization_with_new_shortname(reg_user_headers): - """ regular users cannot update organization """ - user = reg_user_headers['CVE-API-USER'] - org1 = reg_user_headers['CVE-API-ORG'] - org2 = str(uuid.uuid4())[:MAX_SHORTNAME_LENGTH] - res = requests.put( - f'{env.AWG_BASE_URL}{ORG_URL}/{org1}/user/{user}?org_short_name={org2}', - headers=reg_user_headers - ) - assert res.status_code == 403 - response_contains_json(res, 'error', 'NOT_ALLOWED_TO_CHANGE_ORGANIZATION') - - def test_regular_user_cannot_update_active_state(reg_user_headers): """ regular user cannot change its own active state """ org = reg_user_headers['CVE-API-ORG'] From 9a38c7d45c0196db0263ba62bf6aa80d0e584c92 Mon Sep 17 00:00:00 2001 From: david-rocca Date: Wed, 29 Nov 2023 10:07:48 -0500 Subject: [PATCH 13/14] Revert "#836 Due to the new workflow, this test is no longer valid" This reverts commit 846af9f8a214c1598a6904af85def7c3fb83ee5f. --- test-http/src/test/org_user_tests/regular_users.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test-http/src/test/org_user_tests/regular_users.py b/test-http/src/test/org_user_tests/regular_users.py index c50c6270..3d015c1e 100644 --- a/test-http/src/test/org_user_tests/regular_users.py +++ b/test-http/src/test/org_user_tests/regular_users.py @@ -75,6 +75,19 @@ def test_regular_user_cannot_update_duplicate_user_with_new_username(reg_user_he response_contains_json(res, 'error', 'NOT_ORG_ADMIN_OR_SECRETARIAT_UPDATE') +def test_regular_user_cannot_update_organization_with_new_shortname(reg_user_headers): + """ regular users cannot update organization """ + user = reg_user_headers['CVE-API-USER'] + org1 = reg_user_headers['CVE-API-ORG'] + org2 = str(uuid.uuid4())[:MAX_SHORTNAME_LENGTH] + res = requests.put( + f'{env.AWG_BASE_URL}{ORG_URL}/{org1}/user/{user}?org_short_name={org2}', + headers=reg_user_headers + ) + assert res.status_code == 403 + response_contains_json(res, 'error', 'NOT_ALLOWED_TO_CHANGE_ORGANIZATION') + + def test_regular_user_cannot_update_active_state(reg_user_headers): """ regular user cannot change its own active state """ org = reg_user_headers['CVE-API-ORG'] From 04a64e93d82c2ab43cf442fcbfb0e69e0501470b Mon Sep 17 00:00:00 2001 From: david-rocca Date: Wed, 29 Nov 2023 10:08:43 -0500 Subject: [PATCH 14/14] #836 Added code to throw specific error for org_short_name --- src/controller/org.controller/org.controller.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/controller/org.controller/org.controller.js b/src/controller/org.controller/org.controller.js index 73023be3..d1a094b5 100644 --- a/src/controller/org.controller/org.controller.js +++ b/src/controller/org.controller/org.controller.js @@ -596,6 +596,12 @@ async function updateUser (req, res, next) { } + // Specific check for org_short_name + if (Object.keys(req.ctx.query).length > 0 && Object.keys(req.ctx.query).includes('org_short_name') && !(isSecretariat)) { + logger.info({ uuid: req.ctx.uuid, message: 'The user could not be updated because ' + requesterUsername + ' is an Org Admin and tried to reassign the organization.' }) + return res.status(403).json(error.notAllowedToChangeOrganization()) + } + // Check to ensure that the user has the right permissions to edit the fields tha they are requesting to edit, and fail fast if they do not. if (Object.keys(req.ctx.query).length > 0 && Object.keys(req.ctx.query).some((key) => { return queryParameterPermissions[key] }) && !(isAdmin || isSecretariat)) { logger.info({ uuid: req.ctx.uuid, message: 'The user could not be updated because ' + requesterUsername + ' user is not Org Admin or Secretariat to modify these fields.' }) @@ -610,10 +616,6 @@ async function updateUser (req, res, next) { newUser.username = req.ctx.query.new_username } else if (key === 'org_short_name') { newOrgShortName = req.ctx.query.org_short_name - if (!isSecretariat) { - logger.info({ uuid: req.ctx.uuid, message: 'The user could not be updated because ' + requesterUsername + ' is an Org Admin and tried to reassign the organization.' }) - return res.status(403).json(error.notAllowedToChangeOrganization()) - } } else if (key === 'name.first') { newUser.name.first = req.ctx.query['name.first'] } else if (key === 'name.last') {