From 16912ad425d3babe850da2062d6932558c6195d7 Mon Sep 17 00:00:00 2001 From: Kyle Jackson Date: Thu, 10 Mar 2022 20:47:46 +1000 Subject: [PATCH 01/16] Fixed invalid org server error --- src/controller/cve-id.controller/index.js | 2 +- src/controller/cve.controller/index.js | 8 ++--- src/middleware/error.js | 7 ++++ src/middleware/middleware.js | 5 ++- .../middleware/onlyCnasMiddlewareTest.js | 36 +++++++++++++++++++ 5 files changed, 52 insertions(+), 6 deletions(-) diff --git a/src/controller/cve-id.controller/index.js b/src/controller/cve-id.controller/index.js index 1cd9fc4f9..ff7cc5ed1 100644 --- a/src/controller/cve-id.controller/index.js +++ b/src/controller/cve-id.controller/index.js @@ -37,8 +37,8 @@ router.get('/cve-id/:id', parseGetParams, controller.CVEID_GET_SINGLE) router.put('/cve-id/:id', - mw.onlyCnas, mw.validateUser, + mw.onlyCnas, param(['id']).isString().matches(/^CVE-[0-9]{4}-[0-9]{4,}$/, 'i'), query(['state']).optional().isString().trim().escape().customSanitizer(val => { return val.toUpperCase() }).isIn(CHOICES), query(['org']).optional().isString().trim().escape(), diff --git a/src/controller/cve.controller/index.js b/src/controller/cve.controller/index.js index cc0779660..52dbae0f9 100644 --- a/src/controller/cve.controller/index.js +++ b/src/controller/cve.controller/index.js @@ -56,8 +56,8 @@ router.put('/cve/:id', controller.CVE_UPDATE_SINGLE) router.post('/cve/:id/cna', - mw.onlyCnas, mw.validateUser, + mw.onlyCnas, validateCveCnaContainerJsonSchema, param(['id']).isString().matches(/^CVE-[0-9]{4}-[0-9]{4,}$/i), parseError, @@ -66,8 +66,8 @@ router.post('/cve/:id/cna', controller.CVE_SUBMIT_CNA) router.put('/cve/:id/cna', - mw.onlyCnas, mw.validateUser, + mw.onlyCnas, validateCveCnaContainerJsonSchema, param(['id']).isString().matches(/^CVE-[0-9]{4}-[0-9]{4,}$/i), parseError, @@ -76,8 +76,8 @@ router.put('/cve/:id/cna', controller.CVE_UPDATE_CNA) router.post('/cve/:id/reject', - mw.onlyCnas, mw.validateUser, + mw.onlyCnas, validateRejectBody, param(['id']).isString().matches(/^CVE-[0-9]{4}-[0-9]{4,}$/i), body(['cnaContainer.rejectedReasons']).isArray().custom((arr) => { @@ -93,8 +93,8 @@ router.post('/cve/:id/reject', controller.CVE_REJECT_RECORD) router.put('/cve/:id/reject', - mw.onlyCnas, mw.validateUser, + mw.onlyCnas, validateRejectBody, param(['id']).isString().matches(/^CVE-[0-9]{4}-[0-9]{4,}$/i), body(['cnaContainer.rejectedReasons']).isArray().custom((arr) => { diff --git a/src/middleware/error.js b/src/middleware/error.js index fd357cf5d..115a3b2a9 100644 --- a/src/middleware/error.js +++ b/src/middleware/error.js @@ -28,6 +28,13 @@ class MiddlewareError extends idrErr.IDRError { return err } + cnaDoesNotExist () { //mw + const err = {} + err.error = 'CNA_DOES_NOT_EXIST' + err.message = 'The organization designated by the shortname parameter does not exist.' + return err + } + orgDoesNotOwnId (org, id) { const err = { error: 'ORG_DOES_NOT_OWN_ID', diff --git a/src/middleware/middleware.js b/src/middleware/middleware.js index 9a0be869f..caafe584a 100644 --- a/src/middleware/middleware.js +++ b/src/middleware/middleware.js @@ -129,7 +129,10 @@ async function onlyCnas (req, res, next) { try { const org = await orgRepo.findOneByShortName(shortName) // org exists - if (org.authority.active_roles.includes(CONSTANTS.AUTH_ROLE_ENUM.SECRETARIAT)) { + if (org === null) { + logger.info({ uuid: req.ctx.uuid, message: 'Shortname parameter set is not a valid CNA' }) + return res.status(404).json(error.cnaDoesNotExist()) + } else if (org.authority.active_roles.includes(CONSTANTS.AUTH_ROLE_ENUM.SECRETARIAT)) { logger.info({ uuid: req.ctx.uuid, message: org.short_name + ' is a ' + CONSTANTS.AUTH_ROLE_ENUM.SECRETARIAT + ' so until Root organizations are implemented this role is allowed.' }) next() } else if (org.authority.active_roles.includes(CONSTANTS.AUTH_ROLE_ENUM.CNA)) { // the org is a CNA diff --git a/test/unit-tests/middleware/onlyCnasMiddlewareTest.js b/test/unit-tests/middleware/onlyCnasMiddlewareTest.js index 1a15273c1..0d732a86c 100644 --- a/test/unit-tests/middleware/onlyCnasMiddlewareTest.js +++ b/test/unit-tests/middleware/onlyCnasMiddlewareTest.js @@ -160,5 +160,41 @@ describe('Test only CNA middleware', () => { done() }) }) + + it('Requester organization shortname is not valid', function (done) { + class OrgOnlyCnasOrgNull { + async findOneByShortName () { + return null + } + } + + app.route('/only-cnas-org-equals-null') + .post((req, res, next) => { + const factory = { + getOrgRepository: () => { return new OrgOnlyCnasOrgNull() } + } + req.ctx.repositories = factory + next() + }, middleware.onlyCnas, (req, res) => { + return res.status(200).json({ message: 'Success! You have reached the target endpoint.' }) + }) + + chai.request(app) + .post('/only-cnas-org-equals-null') + .set(mwCnaFixtures.notCnaHeaders) + .send() + .end((err, res) => { + if (err) { + done(err) + } + + expect(res).to.have.status(404) + expect(res).to.have.property('body').and.to.be.a('object') + const errObj = error.cnaDoesNotExist() + expect(res.body.error).to.equal(errObj.error) + expect(res.body.message).to.equal(errObj.message) + done() + }) + }) }) }) From 90e5cc03e788c532e78663f04b19827d58833664 Mon Sep 17 00:00:00 2001 From: Kyle Jackson Date: Thu, 10 Mar 2022 21:00:23 +1000 Subject: [PATCH 02/16] Updated error message --- src/middleware/error.js | 4 ++-- src/middleware/middleware.js | 4 ++-- test/unit-tests/middleware/onlyCnasMiddlewareTest.js | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/middleware/error.js b/src/middleware/error.js index 115a3b2a9..ecd50a816 100644 --- a/src/middleware/error.js +++ b/src/middleware/error.js @@ -28,10 +28,10 @@ class MiddlewareError extends idrErr.IDRError { return err } - cnaDoesNotExist () { //mw + cnaDoesNotExist (shortname) { //mw const err = {} err.error = 'CNA_DOES_NOT_EXIST' - err.message = 'The organization designated by the shortname parameter does not exist.' + err.message = `The '${shortname}' organization designated by the shortname parameter does not exist.` return err } diff --git a/src/middleware/middleware.js b/src/middleware/middleware.js index caafe584a..9664978bf 100644 --- a/src/middleware/middleware.js +++ b/src/middleware/middleware.js @@ -130,8 +130,8 @@ async function onlyCnas (req, res, next) { try { const org = await orgRepo.findOneByShortName(shortName) // org exists if (org === null) { - logger.info({ uuid: req.ctx.uuid, message: 'Shortname parameter set is not a valid CNA' }) - return res.status(404).json(error.cnaDoesNotExist()) + logger.info({ uuid: req.ctx.uuid, message: shortName + ' is NOT a ' + CONSTANTS.AUTH_ROLE_ENUM.CNA }) + return res.status(404).json(error.cnaDoesNotExist(shortName)) } else if (org.authority.active_roles.includes(CONSTANTS.AUTH_ROLE_ENUM.SECRETARIAT)) { logger.info({ uuid: req.ctx.uuid, message: org.short_name + ' is a ' + CONSTANTS.AUTH_ROLE_ENUM.SECRETARIAT + ' so until Root organizations are implemented this role is allowed.' }) next() diff --git a/test/unit-tests/middleware/onlyCnasMiddlewareTest.js b/test/unit-tests/middleware/onlyCnasMiddlewareTest.js index 0d732a86c..8a4542f0e 100644 --- a/test/unit-tests/middleware/onlyCnasMiddlewareTest.js +++ b/test/unit-tests/middleware/onlyCnasMiddlewareTest.js @@ -190,7 +190,7 @@ describe('Test only CNA middleware', () => { expect(res).to.have.status(404) expect(res).to.have.property('body').and.to.be.a('object') - const errObj = error.cnaDoesNotExist() + const errObj = error.cnaDoesNotExist(mwCnaFixtures.notCnaHeaders['CVE-API-ORG']) expect(res.body.error).to.equal(errObj.error) expect(res.body.message).to.equal(errObj.message) done() From baf98ac5bd27b49190de5283bf33e9a4d94b9dea Mon Sep 17 00:00:00 2001 From: Kyle Jackson Date: Thu, 10 Mar 2022 21:05:25 +1000 Subject: [PATCH 03/16] Fixed linting --- src/middleware/error.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/middleware/error.js b/src/middleware/error.js index ecd50a816..bc001593a 100644 --- a/src/middleware/error.js +++ b/src/middleware/error.js @@ -28,7 +28,7 @@ class MiddlewareError extends idrErr.IDRError { return err } - cnaDoesNotExist (shortname) { //mw + cnaDoesNotExist (shortname) { // mw const err = {} err.error = 'CNA_DOES_NOT_EXIST' err.message = `The '${shortname}' organization designated by the shortname parameter does not exist.` From 10f6ca1ab8204d4554cef93aa2398a667a573473 Mon Sep 17 00:00:00 2001 From: Kyle Jackson Date: Fri, 11 Mar 2022 17:26:07 +1000 Subject: [PATCH 04/16] Testing CodeQL failures --- src/controller/cve.controller/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/controller/cve.controller/index.js b/src/controller/cve.controller/index.js index 52dbae0f9..5baf36856 100644 --- a/src/controller/cve.controller/index.js +++ b/src/controller/cve.controller/index.js @@ -93,8 +93,8 @@ router.post('/cve/:id/reject', controller.CVE_REJECT_RECORD) router.put('/cve/:id/reject', - mw.validateUser, mw.onlyCnas, + mw.validateUser, validateRejectBody, param(['id']).isString().matches(/^CVE-[0-9]{4}-[0-9]{4,}$/i), body(['cnaContainer.rejectedReasons']).isArray().custom((arr) => { From 91da416bca70fed1c347c30ea82bbeb43e53b071 Mon Sep 17 00:00:00 2001 From: Kyle Jackson Date: Fri, 11 Mar 2022 19:33:52 +1000 Subject: [PATCH 05/16] Switched order of onlyCnas --- src/controller/cve.controller/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/controller/cve.controller/index.js b/src/controller/cve.controller/index.js index 5baf36856..52dbae0f9 100644 --- a/src/controller/cve.controller/index.js +++ b/src/controller/cve.controller/index.js @@ -93,8 +93,8 @@ router.post('/cve/:id/reject', controller.CVE_REJECT_RECORD) router.put('/cve/:id/reject', - mw.onlyCnas, mw.validateUser, + mw.onlyCnas, validateRejectBody, param(['id']).isString().matches(/^CVE-[0-9]{4}-[0-9]{4,}$/i), body(['cnaContainer.rejectedReasons']).isArray().custom((arr) => { From 31f3c8a0f452829a891043e46b80bf415c011ecd Mon Sep 17 00:00:00 2001 From: Colby Prior Date: Wed, 23 Mar 2022 08:10:05 +1000 Subject: [PATCH 06/16] add support for rfc3339 date-time --- src/controller/cve-id.controller/cve-id.middleware.js | 5 +++-- src/controller/cve.controller/cve.middleware.js | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/controller/cve-id.controller/cve-id.middleware.js b/src/controller/cve-id.controller/cve-id.middleware.js index 7c157f416..c040d4722 100644 --- a/src/controller/cve-id.controller/cve-id.middleware.js +++ b/src/controller/cve-id.controller/cve-id.middleware.js @@ -17,10 +17,11 @@ function parsePostParams (req, res, next) { // Sanitizer for dates function toDate (val) { - let value = val.match(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}$/) + let value = val.match(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.\d+)(Z|((-|\s)\d{2}:\d{2}))$/) let result if (value) { - result = new Date(`${value[0]}.000+00:00`) + value[0] = value[0].replace(' ', '+') // Re-add literal '+' which was stripped + result = new Date(value[0]) } else { value = val.match(/^\d{4}-\d{2}-\d{2}$/) if (value) { diff --git a/src/controller/cve.controller/cve.middleware.js b/src/controller/cve.controller/cve.middleware.js index dacacde55..46d38d451 100644 --- a/src/controller/cve.controller/cve.middleware.js +++ b/src/controller/cve.controller/cve.middleware.js @@ -27,10 +27,11 @@ function parseGetParams (req, res, next) { // Sanitizer for dates function toDate (val) { - let value = val.match(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}$/) + let value = val.match(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.\d+)(Z|((-|\s)\d{2}:\d{2}))$/) let result if (value) { - result = new Date(`${value[0]}.000+00:00`) + value[0] = value[0].replace(' ', '+') // Re-add literal '+' which was stripped + result = new Date(value[0]) } else { value = val.match(/^\d{4}-\d{2}-\d{2}$/) if (value) { From 9bb14eb3f1d8aecea60f6708c86e5f8152fce152 Mon Sep 17 00:00:00 2001 From: Colby Prior Date: Wed, 23 Mar 2022 08:25:53 +1000 Subject: [PATCH 07/16] fix up optional millisecond --- src/controller/cve-id.controller/cve-id.middleware.js | 2 +- src/controller/cve.controller/cve.middleware.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/controller/cve-id.controller/cve-id.middleware.js b/src/controller/cve-id.controller/cve-id.middleware.js index c040d4722..4d5679034 100644 --- a/src/controller/cve-id.controller/cve-id.middleware.js +++ b/src/controller/cve-id.controller/cve-id.middleware.js @@ -17,7 +17,7 @@ function parsePostParams (req, res, next) { // Sanitizer for dates function toDate (val) { - let value = val.match(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.\d+)(Z|((-|\s)\d{2}:\d{2}))$/) + let value = val.match(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(\.\d+)?(Z|((-|\s)\d{2}:\d{2}))$/) let result if (value) { value[0] = value[0].replace(' ', '+') // Re-add literal '+' which was stripped diff --git a/src/controller/cve.controller/cve.middleware.js b/src/controller/cve.controller/cve.middleware.js index 46d38d451..26adfe857 100644 --- a/src/controller/cve.controller/cve.middleware.js +++ b/src/controller/cve.controller/cve.middleware.js @@ -27,7 +27,7 @@ function parseGetParams (req, res, next) { // Sanitizer for dates function toDate (val) { - let value = val.match(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.\d+)(Z|((-|\s)\d{2}:\d{2}))$/) + let value = val.match(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(\.\d+)?(Z|((-|\s)\d{2}:\d{2}))$/) let result if (value) { value[0] = value[0].replace(' ', '+') // Re-add literal '+' which was stripped From 020e4ceb01db4bf4319fc74fac09e1212ff825b6 Mon Sep 17 00:00:00 2001 From: Kyle Jackson Date: Thu, 24 Mar 2022 16:41:51 +1000 Subject: [PATCH 08/16] #605 Updating validateJsonSyntax logic --- src/middleware/middleware.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/middleware/middleware.js b/src/middleware/middleware.js index 5070a981a..9b115b21e 100644 --- a/src/middleware/middleware.js +++ b/src/middleware/middleware.js @@ -207,14 +207,14 @@ function validateCveJsonSchema (req, res, next) { function validateJsonSyntax (err, req, res, next) { if (err.status && err.message) { - if (err.message.includes('Unexpected token')) { - console.warn('Request failed validation because JSON syntax is incorrect') - console.info((JSON.stringify(err))) - return res.status(400).json(error.invalidJsonSyntax(err.message)) - } else if (err.message.includes('request entity too large')) { + if (err.message.includes('request entity too large')) { console.warn('Request failed validation because entity too large') console.info((JSON.stringify(err))) return res.status(413).json(error.recordTooLarge(errors)) + } else if (err.status === 400) { + console.warn('Request failed validation because JSON syntax is incorrect') + console.info((JSON.stringify(err))) + return res.status(400).json(error.invalidJsonSyntax(err.message)) } } else { next(err) From e78c3921c68c9fb20c0cd83a8fc9971084ef151c Mon Sep 17 00:00:00 2001 From: Colby Prior Date: Tue, 29 Mar 2022 08:20:44 +1000 Subject: [PATCH 09/16] add test for timezone format and allow backwards compatibility --- .../cve-id.controller/cve-id.middleware.js | 2 +- src/controller/cve.controller/cve.middleware.js | 2 +- .../src/test/cve_id_tests/cve_id_as_org_admin.py | 15 ++++++++++++++- test-http/src/test/cve_tests/cve.py | 13 +++++++++++++ 4 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/controller/cve-id.controller/cve-id.middleware.js b/src/controller/cve-id.controller/cve-id.middleware.js index 4d5679034..dead9984c 100644 --- a/src/controller/cve-id.controller/cve-id.middleware.js +++ b/src/controller/cve-id.controller/cve-id.middleware.js @@ -17,7 +17,7 @@ function parsePostParams (req, res, next) { // Sanitizer for dates function toDate (val) { - let value = val.match(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(\.\d+)?(Z|((-|\s)\d{2}:\d{2}))$/) + let value = val.match(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(\.\d+)?(|Z|((-|\+|\s)\d{2}:\d{2}))$/) let result if (value) { value[0] = value[0].replace(' ', '+') // Re-add literal '+' which was stripped diff --git a/src/controller/cve.controller/cve.middleware.js b/src/controller/cve.controller/cve.middleware.js index 26adfe857..f3bce775c 100644 --- a/src/controller/cve.controller/cve.middleware.js +++ b/src/controller/cve.controller/cve.middleware.js @@ -27,7 +27,7 @@ function parseGetParams (req, res, next) { // Sanitizer for dates function toDate (val) { - let value = val.match(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(\.\d+)?(Z|((-|\s)\d{2}:\d{2}))$/) + let value = val.match(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(\.\d+)?(|Z|((-|\+|\s)\d{2}:\d{2}))$/) let result if (value) { value[0] = value[0].replace(' ', '+') // Re-add literal '+' which was stripped diff --git a/test-http/src/test/cve_id_tests/cve_id_as_org_admin.py b/test-http/src/test/cve_id_tests/cve_id_as_org_admin.py index dc5a359e2..06d091b62 100644 --- a/test-http/src/test/cve_id_tests/cve_id_as_org_admin.py +++ b/test-http/src/test/cve_id_tests/cve_id_as_org_admin.py @@ -317,10 +317,12 @@ def test_get_cve_id_by_time_modified(org_admin_headers): n_ids = 10 time.sleep(1) t_before = dt.datetime.now().strftime('%Y-%m-%dT%H:%M:%S') + t_before_alt = dt.datetime.now().strftime('%Y-%m-%dT%H:%M:%S+00:00') time.sleep(1) res_ids = get_reserve_cve_ids(n_ids, utils.CURRENT_YEAR, org_admin_headers['CVE-API-ORG']) time.sleep(1) t_after = dt.datetime.now().strftime('%Y-%m-%dT%H:%M:%S') + t_after_alt = dt.datetime.now().strftime('%Y-%m-%dT%H:%M:%S+00:00') res_get_ids = requests.get( f'{env.AWG_BASE_URL}{CVE_ID_URL}', @@ -330,8 +332,19 @@ def test_get_cve_id_by_time_modified(org_admin_headers): 'time_modified.gt': t_before } ) + # Test alternate timezone format + res_get_ids_alt = requests.get( + f'{env.AWG_BASE_URL}{CVE_ID_URL}', + headers=utils.BASE_HEADERS, + params={ + 'time_modified.lt': t_after_alt, + 'time_modified.gt': t_before_alt + } + ) ok_response_contains(res_get_ids, f'CVE-{utils.CURRENT_YEAR}-') assert len(json.loads(res_get_ids.content.decode())['cve_ids']) == n_ids + ok_response_contains(res_get_ids_alt, f'CVE-{utils.CURRENT_YEAR}-') + assert len(json.loads(res_get_ids_alt.content.decode())['cve_ids']) == n_ids def test_get_cve_id_with_params(org_admin_headers): @@ -470,4 +483,4 @@ def get_reserve_cve_ids( 'cve_year': f'{year}', 'short_name': cna_short_name } - ) \ No newline at end of file + ) diff --git a/test-http/src/test/cve_tests/cve.py b/test-http/src/test/cve_tests/cve.py index 766e65b54..cfc5dc710 100644 --- a/test-http/src/test/cve_tests/cve.py +++ b/test-http/src/test/cve_tests/cve.py @@ -36,12 +36,14 @@ def test_get_cve_by_time_modified(): time.sleep(1) post_cve('CVE-2021-0005_published', 'CVE-2021-0005') t_before = dt.datetime.now().strftime('%Y-%m-%dT%H:%M:%S') + t_before_alt = dt.datetime.now().strftime('%Y-%m-%dT%H:%M:%S+00:00') time.sleep(1) update_cve('CVE-2021-0004_published', 'CVE-2021-0004') time.sleep(1) update_cve('CVE-2021-0005_published', 'CVE-2021-0005') time.sleep(1) t_after = dt.datetime.now().strftime('%Y-%m-%dT%H:%M:%S') + t_after_alt = dt.datetime.now().strftime('%Y-%m-%dT%H:%M:%S+00:00') res = requests.get( f'{env.AWG_BASE_URL}{CVE_URL}/', @@ -51,8 +53,19 @@ def test_get_cve_by_time_modified(): 'time_modified.gt': t_before } ) + # Alternat timezone format + res_alt = requests.get( + f'{env.AWG_BASE_URL}{CVE_URL}/', + headers=utils.BASE_HEADERS, + params={ + 'time_modified.lt': t_after_alt, + 'time_modified.gt': t_before_alt + } + ) assert res.status_code == utils.HTTP_OK assert len(json.loads(res.content.decode())['cveRecords']) >= 2 + assert res_alt.status_code == utils.HTTP_OK + assert len(json.loads(res_alt.content.decode())['cveRecords']) >= 2 def test_get_cve_by_count_only_true(): From 9c73b578492ac4a2b9e7605675e5bffccbe86044 Mon Sep 17 00:00:00 2001 From: Kyle Jackson Date: Wed, 30 Mar 2022 20:59:33 +1000 Subject: [PATCH 10/16] #607 Updated validateJsonSyntax to include catch all --- src/middleware/error.js | 9 ++++++++- src/middleware/middleware.js | 4 ++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/middleware/error.js b/src/middleware/error.js index 4e7ea779c..fd712380e 100644 --- a/src/middleware/error.js +++ b/src/middleware/error.js @@ -54,7 +54,14 @@ class MiddlewareError extends idrErr.IDRError { return err } - recordTooLarge () { + genericBadRequest (errors) { // mw + const err = {} + err.error = 'BAD_REQUEST' + err.message = errors + return err + } + + recordTooLarge () { // mw const err = {} err.error = 'RECORD_TOO_LARGE' err.message = 'Records must be less than 16MB.' diff --git a/src/middleware/middleware.js b/src/middleware/middleware.js index 5070a981a..4df74c619 100644 --- a/src/middleware/middleware.js +++ b/src/middleware/middleware.js @@ -215,6 +215,10 @@ function validateJsonSyntax (err, req, res, next) { console.warn('Request failed validation because entity too large') console.info((JSON.stringify(err))) return res.status(413).json(error.recordTooLarge(errors)) + } else { + console.warn('Request failed') + console.info((JSON.stringify(err))) + return res.status(400).json(error.genericBadRequest(err.message)) } } else { next(err) From 303024b18c94c93b9b08d7e4bb02a75cd3889909 Mon Sep 17 00:00:00 2001 From: Suhani Pant Date: Mon, 4 Apr 2022 11:49:02 -0400 Subject: [PATCH 11/16] modified function to find duplicate lang values --- .../cve.controller/cve.middleware.js | 24 +++++++++---------- src/controller/cve.controller/index.js | 6 ++--- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/controller/cve.controller/cve.middleware.js b/src/controller/cve.controller/cve.middleware.js index f3bce775c..049f0c4ac 100644 --- a/src/controller/cve.controller/cve.middleware.js +++ b/src/controller/cve.controller/cve.middleware.js @@ -53,19 +53,19 @@ function parseError (req, res, next) { next() } -function onlyOneEnglishDescription (arr) { - const arrayLength = arr.length - let numEnglishFound = 0 // for checking how many times an english field shows up - for (var i = 0; i < arrayLength; i++) { - if (arr[i].lang === 'en') { - numEnglishFound += 1 - } +function uniqueEnglishDescription (arr) { + // make all array values lowercase for comparison + var lowercaseArr = [] + for (var i = 0; i < arr.length; i++) { + lowercaseArr.push(arr[i].lang.toLowerCase()); +} + const toFindDuplicates = lowercaseArr => lowercaseArr.filter((lang, index) => lowercaseArr.indexOf(lang) !== index) // check whether a value appears twice + const duplicateElements = toFindDuplicates(lowercaseArr); // create an array of repeating lang values - if (numEnglishFound > 1) { // return error if more than 1 english description is found - return null - } + if (duplicateElements.length === 0) { // check that there are 0 repeating values + return true } - return numEnglishFound + return false } function validateRejectBody (req, res, next) { @@ -109,6 +109,6 @@ module.exports = { parseError, toDate, validateCveCnaContainerJsonSchema, - onlyOneEnglishDescription, + uniqueEnglishDescription, validateRejectBody } diff --git a/src/controller/cve.controller/index.js b/src/controller/cve.controller/index.js index cc0779660..f22bcc0c0 100644 --- a/src/controller/cve.controller/index.js +++ b/src/controller/cve.controller/index.js @@ -3,7 +3,7 @@ const router = express.Router() const mw = require('../../middleware/middleware') const controller = require('./cve.controller') const { body, param, query } = require('express-validator') -const { parseGetParams, parsePostParams, parseError, toDate, validateCveCnaContainerJsonSchema, validateRejectBody, onlyOneEnglishDescription } = require('./cve.middleware') +const { parseGetParams, parsePostParams, parseError, toDate, validateCveCnaContainerJsonSchema, validateRejectBody, uniqueEnglishDescription } = require('./cve.middleware') const CONSTANTS = require('../../constants') const CHOICES = [CONSTANTS.CVE_STATES.REJECTED, CONSTANTS.CVE_STATES.PUBLISHED] @@ -81,7 +81,7 @@ router.post('/cve/:id/reject', validateRejectBody, param(['id']).isString().matches(/^CVE-[0-9]{4}-[0-9]{4,}$/i), body(['cnaContainer.rejectedReasons']).isArray().custom((arr) => { - if (onlyOneEnglishDescription(arr) !== 1) { + if (!uniqueEnglishDescription(arr)) { throw new Error(400, 'Bad request, more than one English description found') } return true @@ -98,7 +98,7 @@ router.put('/cve/:id/reject', validateRejectBody, param(['id']).isString().matches(/^CVE-[0-9]{4}-[0-9]{4,}$/i), body(['cnaContainer.rejectedReasons']).isArray().custom((arr) => { - if (onlyOneEnglishDescription(arr) !== 1) { + if (!uniqueEnglishDescription(arr)) { throw new Error(400, 'Bad request, more than one English description found') } return true From 57f03cc51193239d8cd3536668e51eb0880598c2 Mon Sep 17 00:00:00 2001 From: Suhani Pant Date: Mon, 4 Apr 2022 11:53:59 -0400 Subject: [PATCH 12/16] #573 fixed linting errors --- src/controller/cve.controller/cve.middleware.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/controller/cve.controller/cve.middleware.js b/src/controller/cve.controller/cve.middleware.js index 049f0c4ac..4d1a4c5bd 100644 --- a/src/controller/cve.controller/cve.middleware.js +++ b/src/controller/cve.controller/cve.middleware.js @@ -57,11 +57,11 @@ function uniqueEnglishDescription (arr) { // make all array values lowercase for comparison var lowercaseArr = [] for (var i = 0; i < arr.length; i++) { - lowercaseArr.push(arr[i].lang.toLowerCase()); -} + lowercaseArr.push(arr[i].lang.toLowerCase()) + } + // find whether duplicate values exist const toFindDuplicates = lowercaseArr => lowercaseArr.filter((lang, index) => lowercaseArr.indexOf(lang) !== index) // check whether a value appears twice - const duplicateElements = toFindDuplicates(lowercaseArr); // create an array of repeating lang values - + const duplicateElements = toFindDuplicates(lowercaseArr) // create an array of repeating lang values if (duplicateElements.length === 0) { // check that there are 0 repeating values return true } From d25318bdbbf56aa661319d8a6748036e66d90b6c Mon Sep 17 00:00:00 2001 From: Suhani Pant Date: Thu, 7 Apr 2022 16:29:40 -0400 Subject: [PATCH 13/16] #573 simplified uniqueEnglishDescription function and added tests for 3 scenarios --- .../cve.controller/cve.middleware.js | 24 ++++----- test-http/src/test/cve_tests/cve.py | 52 +++++++++++++++++-- ...n => rejectBodyMultipleDiffEngValues.json} | 13 +---- .../rejectBodyMultipleNonEngValues.json | 32 ++++++++++++ .../rejectBodyMultipleSameEngValues.json | 32 ++++++++++++ 5 files changed, 125 insertions(+), 28 deletions(-) rename test-http/src/test/cve_tests/cve_record_fixtures/{rejectBodyMultipleEngDescriptions.json => rejectBodyMultipleDiffEngValues.json} (69%) create mode 100644 test-http/src/test/cve_tests/cve_record_fixtures/rejectBodyMultipleNonEngValues.json create mode 100644 test-http/src/test/cve_tests/cve_record_fixtures/rejectBodyMultipleSameEngValues.json diff --git a/src/controller/cve.controller/cve.middleware.js b/src/controller/cve.controller/cve.middleware.js index 4d1a4c5bd..1d9378b60 100644 --- a/src/controller/cve.controller/cve.middleware.js +++ b/src/controller/cve.controller/cve.middleware.js @@ -53,19 +53,19 @@ function parseError (req, res, next) { next() } -function uniqueEnglishDescription (arr) { - // make all array values lowercase for comparison - var lowercaseArr = [] - for (var i = 0; i < arr.length; i++) { - lowercaseArr.push(arr[i].lang.toLowerCase()) - } - // find whether duplicate values exist - const toFindDuplicates = lowercaseArr => lowercaseArr.filter((lang, index) => lowercaseArr.indexOf(lang) !== index) // check whether a value appears twice - const duplicateElements = toFindDuplicates(lowercaseArr) // create an array of repeating lang values - if (duplicateElements.length === 0) { // check that there are 0 repeating values - return true +function uniqueEnglishDescription (rejectedReasonsArr) { + const langArray = rejectedReasonsArr.map(function (reason) {return reason.lang.toLowerCase()})// create arr of lowercase lang values + const foundValues = new Set() // set to hold languages found + // loop through the lang array and find duplicates + for (var i = 0; i < langArray.length; i++) { + if (langArray[i].startsWith("en")) { // check case only if lang starts with "en" + if (foundValues.has(langArray[i])) { + return false; // duplicate found so return false + } + foundValues.add(langArray[i]) // add each unique value to set + } } - return false + return true // if no duplicate found, then all lang values were unique } function validateRejectBody (req, res, next) { diff --git a/test-http/src/test/cve_tests/cve.py b/test-http/src/test/cve_tests/cve.py index cfc5dc710..c0f02d5b1 100644 --- a/test-http/src/test/cve_tests/cve.py +++ b/test-http/src/test/cve_tests/cve.py @@ -363,8 +363,8 @@ def test_submit_record_rejection_id_dne(): assert res.status_code == 403 -def test_submit_record_rejection_multiple_english_descriptions(): - """ submit a reject request with descriptions array that has more than one english description """ +def test_submit_record_rejection_multiple_different_english_values(): + """ submit a reject request with descriptions array that has multiple different English values (ex: "en" and "en-Ca") """ res = requests.post( f'{env.AWG_BASE_URL}/api/cve-id/', headers=utils.BASE_HEADERS, @@ -375,14 +375,58 @@ def test_submit_record_rejection_multiple_english_descriptions(): } ) id_num = json.loads(res.content.decode())['cve_ids'][0]['cve_id'] # obtain id number - with open('./src/test/cve_tests/cve_record_fixtures/rejectBodyMultipleEngDescriptions.json') as json_file: + with open('./src/test/cve_tests/cve_record_fixtures/rejectBodyMultipleDiffEngValues.json') as json_file: data = json.load(json_file) res = requests.post( f'{env.AWG_BASE_URL}{CVE_URL}/{id_num}/reject', headers=utils.BASE_HEADERS, json=data ) - assert res.status_code == 400 + assert res.status_code == 200 # lang values are unique + + +def test_submit_record_rejection_multiple_non_English_values(): + """ submit a reject request with descriptions array that has multiple non English values (ex: "fr" and "fr") """ + res = requests.post( + f'{env.AWG_BASE_URL}/api/cve-id/', + headers=utils.BASE_HEADERS, + params={ + 'amount': 1, + 'cve_year': 2000, + 'short_name': 'mitre' + } + ) + id_num = json.loads(res.content.decode())['cve_ids'][0]['cve_id'] # obtain id number + with open('./src/test/cve_tests/cve_record_fixtures/rejectBodyMultipleNonEngValues.json') as json_file: + data = json.load(json_file) + res = requests.post( + f'{env.AWG_BASE_URL}{CVE_URL}/{id_num}/reject', + headers=utils.BASE_HEADERS, + json=data + ) + assert res.status_code == 200 # lang values are unique + + +def test_submit_record_rejection_multiple_same_English_values(): + """ submit a reject request with descriptions array that has multiple same English values (ex: "en-Gb" and "en-Gb") """ + res = requests.post( + f'{env.AWG_BASE_URL}/api/cve-id/', + headers=utils.BASE_HEADERS, + params={ + 'amount': 1, + 'cve_year': 2000, + 'short_name': 'mitre' + } + ) + id_num = json.loads(res.content.decode())['cve_ids'][0]['cve_id'] # obtain id number + with open('./src/test/cve_tests/cve_record_fixtures/rejectBodyMultipleSameEngValues.json') as json_file: + data = json.load(json_file) + res = requests.post( + f'{env.AWG_BASE_URL}{CVE_URL}/{id_num}/reject', + headers=utils.BASE_HEADERS, + json=data + ) + assert res.status_code == 400 # lang values are not unique #### PUT /cve/:id #### diff --git a/test-http/src/test/cve_tests/cve_record_fixtures/rejectBodyMultipleEngDescriptions.json b/test-http/src/test/cve_tests/cve_record_fixtures/rejectBodyMultipleDiffEngValues.json similarity index 69% rename from test-http/src/test/cve_tests/cve_record_fixtures/rejectBodyMultipleEngDescriptions.json rename to test-http/src/test/cve_tests/cve_record_fixtures/rejectBodyMultipleDiffEngValues.json index 73e7bdb18..1700eff8c 100644 --- a/test-http/src/test/cve_tests/cve_record_fixtures/rejectBodyMultipleEngDescriptions.json +++ b/test-http/src/test/cve_tests/cve_record_fixtures/rejectBodyMultipleDiffEngValues.json @@ -16,18 +16,7 @@ ] }, { - "lang": "en", - "value": "I professional site herself recently behavior. Situation institution meeting recognize successful.", - "supportingMedia": [ - { - "type": "test/markdown", - "base64": false, - "value": "*this* _is_ supporting media in ~markdown~" - } - ] - }, - { - "lang": "en", + "lang": "en-Ca", "value": "I professional site herself recently behavior. Situation institution meeting recognize successful.", "supportingMedia": [ { diff --git a/test-http/src/test/cve_tests/cve_record_fixtures/rejectBodyMultipleNonEngValues.json b/test-http/src/test/cve_tests/cve_record_fixtures/rejectBodyMultipleNonEngValues.json new file mode 100644 index 000000000..10a90e1f9 --- /dev/null +++ b/test-http/src/test/cve_tests/cve_record_fixtures/rejectBodyMultipleNonEngValues.json @@ -0,0 +1,32 @@ +{ + "cnaContainer": { + "providerMetadata": { + "orgId" : "f972b356-145d-4b2e-9a5c-b114d0982a3b" + }, + "rejectedReasons": [ + { + "lang": "fr", + "value": "I professional site herself recently behavior. Situation institution meeting recognize successful.", + "supportingMedia": [ + { + "type": "test/markdown", + "base64": false, + "value": "*this* _is_ supporting media in ~markdown~" + } + ] + }, + { + "lang": "fr", + "value": "I professional site herself recently behavior. Situation institution meeting recognize successful.", + "supportingMedia": [ + { + "type": "test/markdown", + "base64": false, + "value": "*this* _is_ supporting media in ~markdown~" + } + ] + } + ], + "replacedBy": ["CVE-1999-0006"] + } +} \ No newline at end of file diff --git a/test-http/src/test/cve_tests/cve_record_fixtures/rejectBodyMultipleSameEngValues.json b/test-http/src/test/cve_tests/cve_record_fixtures/rejectBodyMultipleSameEngValues.json new file mode 100644 index 000000000..e1c1974a6 --- /dev/null +++ b/test-http/src/test/cve_tests/cve_record_fixtures/rejectBodyMultipleSameEngValues.json @@ -0,0 +1,32 @@ +{ + "cnaContainer": { + "providerMetadata": { + "orgId" : "f972b356-145d-4b2e-9a5c-b114d0982a3b" + }, + "rejectedReasons": [ + { + "lang": "en-Gb", + "value": "I professional site herself recently behavior. Situation institution meeting recognize successful.", + "supportingMedia": [ + { + "type": "test/markdown", + "base64": false, + "value": "*this* _is_ supporting media in ~markdown~" + } + ] + }, + { + "lang": "en-Gb", + "value": "I professional site herself recently behavior. Situation institution meeting recognize successful.", + "supportingMedia": [ + { + "type": "test/markdown", + "base64": false, + "value": "*this* _is_ supporting media in ~markdown~" + } + ] + } + ], + "replacedBy": ["CVE-1999-0006"] + } +} \ No newline at end of file From 358c7ba48003768a553ee2a0cec488b059b141ec Mon Sep 17 00:00:00 2001 From: Suhani Pant Date: Thu, 7 Apr 2022 16:32:01 -0400 Subject: [PATCH 14/16] 573 fixed linting issues --- src/controller/cve.controller/cve.middleware.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/controller/cve.controller/cve.middleware.js b/src/controller/cve.controller/cve.middleware.js index 1d9378b60..6159705d0 100644 --- a/src/controller/cve.controller/cve.middleware.js +++ b/src/controller/cve.controller/cve.middleware.js @@ -54,13 +54,13 @@ function parseError (req, res, next) { } function uniqueEnglishDescription (rejectedReasonsArr) { - const langArray = rejectedReasonsArr.map(function (reason) {return reason.lang.toLowerCase()})// create arr of lowercase lang values + const langArray = rejectedReasonsArr.map(function (reason) { return reason.lang.toLowerCase() } )// create arr of lowercase lang values const foundValues = new Set() // set to hold languages found // loop through the lang array and find duplicates for (var i = 0; i < langArray.length; i++) { - if (langArray[i].startsWith("en")) { // check case only if lang starts with "en" + if (langArray[i].startsWith('en')) { // check case only if lang starts with "en" if (foundValues.has(langArray[i])) { - return false; // duplicate found so return false + return false // duplicate found so return false } foundValues.add(langArray[i]) // add each unique value to set } From abe8c94943d433c4773a84eb89a2c1164d6eaa00 Mon Sep 17 00:00:00 2001 From: Suhani Pant Date: Thu, 7 Apr 2022 16:33:52 -0400 Subject: [PATCH 15/16] 573 fixed unit test --- test/unit-tests/cve/cveRecordRejectionTest.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit-tests/cve/cveRecordRejectionTest.js b/test/unit-tests/cve/cveRecordRejectionTest.js index 07c5ee1a7..29068c77c 100644 --- a/test/unit-tests/cve/cveRecordRejectionTest.js +++ b/test/unit-tests/cve/cveRecordRejectionTest.js @@ -17,7 +17,7 @@ const cveController = require('../../../src/controller/cve.controller/cve.contro const cveParams = require('../../../src/controller/cve.controller/cve.middleware') const rejectedBody = require('../../../test-http/src/test/cve_tests/cve_record_fixtures/rejectBody.json') -const multipleEngDescriptions = require('../../../test-http/src/test/cve_tests/cve_record_fixtures/rejectBodyMultipleEngDescriptions.json') +const multipleEngDescriptions = require('../../../test-http/src/test/cve_tests/cve_record_fixtures/rejectBodyMultipleSameEngValues.json') const nonExistentId = 'CVE-1800-0001' const cveIdReserved = 'CVE-2019-1421' From 767694770e5ee5e72c0ad2dbaef63c1c31c0a214 Mon Sep 17 00:00:00 2001 From: Suhani Pant Date: Thu, 7 Apr 2022 16:35:52 -0400 Subject: [PATCH 16/16] 573 fix linting --- src/controller/cve.controller/cve.middleware.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/controller/cve.controller/cve.middleware.js b/src/controller/cve.controller/cve.middleware.js index 6159705d0..03ead98a2 100644 --- a/src/controller/cve.controller/cve.middleware.js +++ b/src/controller/cve.controller/cve.middleware.js @@ -54,7 +54,7 @@ function parseError (req, res, next) { } function uniqueEnglishDescription (rejectedReasonsArr) { - const langArray = rejectedReasonsArr.map(function (reason) { return reason.lang.toLowerCase() } )// create arr of lowercase lang values + const langArray = rejectedReasonsArr.map(function (reason) { return reason.lang.toLowerCase() })// create arr of lowercase lang values const foundValues = new Set() // set to hold languages found // loop through the lang array and find duplicates for (var i = 0; i < langArray.length; i++) {