diff --git a/src/controller/org.controller/index.js b/src/controller/org.controller/index.js index 7951cd2ad..794654242 100644 --- a/src/controller/org.controller/index.js +++ b/src/controller/org.controller/index.js @@ -5,6 +5,7 @@ const errorMsgs = require('../../middleware/errorMessages') const controller = require('./org.controller') const { body, param, query } = require('express-validator') const { parseGetParams, parsePostParams, parseError, isOrgRole, isUserRole, isValidUsername } = require('./org.middleware') +const { toUpperCaseArray, isFlatStringArray } = require('../../middleware/middleware') const getConstants = require('../../../src/constants').getConstants const CONSTANTS = getConstants() @@ -157,7 +158,10 @@ router.post('/org', 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(['authority.active_roles']).optional().customSanitizer(val => { return val.map(x => { return x.toUpperCase() }) }).custom(val => { return isOrgRole(val) }), + body(['authority.active_roles']).optional() + .custom(isFlatStringArray) + .customSanitizer(toUpperCaseArray) + .custom(isOrgRole), body(['policies.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), parseError, parsePostParams, @@ -310,10 +314,14 @@ router.put('/org/:shortname', 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), query(['name']).optional().isString().trim().escape().notEmpty(), - query(['active_roles.add']).optional().toArray(), - query(['active_roles.add']).optional().customSanitizer(val => { return val.map(x => { return x.toUpperCase() }) }).custom(val => { return isOrgRole(val) }), - query(['active_roles.remove']).optional().toArray(), - query(['active_roles.remove']).optional().customSanitizer(val => { return val.map(x => { return x.toUpperCase() }) }).custom(val => { return isOrgRole(val) }), + query(['active_roles.add']).optional().toArray() + .custom(isFlatStringArray) + .customSanitizer(toUpperCaseArray) + .custom(isOrgRole), + query(['active_roles.remove']).optional().toArray() + .custom(isFlatStringArray) + .customSanitizer(toUpperCaseArray) + .custom(isOrgRole), parseError, parsePostParams, controller.ORG_UPDATE_SINGLE) @@ -539,14 +547,17 @@ router.post('/org/:shortname/user', mw.onlySecretariatOrAdmin, mw.onlyOrgWithRole, param(['shortname']).isString().trim().escape().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }), - body(['username']).isString().trim().escape().notEmpty().custom(val => { return isValidUsername(val) }), + 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(), body(['name.last']).optional().isString().trim().escape(), body(['name.middle']).optional().isString().trim().escape(), body(['name.suffix']).optional().isString().trim().escape(), - body(['authority.active_roles']).optional().customSanitizer(val => { return val.map(x => { return x.toUpperCase() }) }).custom(val => { return isUserRole(val) }), + body(['authority.active_roles']).optional() + .custom(mw.isFlatStringArray) + .customSanitizer(toUpperCaseArray) + .custom(isUserRole), parseError, parsePostParams, controller.USER_CREATE_SINGLE) @@ -715,10 +726,14 @@ router.put('/org/:shortname/user/:username', query(['name.last']).optional().isString().trim().escape(), query(['name.middle']).optional().isString().trim().escape(), query(['name.suffix']).optional().isString().trim().escape(), - query(['active_roles.add']).optional().toArray(), - query(['active_roles.add']).optional().customSanitizer(val => { return val.map(x => { return x.toUpperCase() }) }).custom(val => { return isUserRole(val) }), - query(['active_roles.remove']).optional().toArray(), - query(['active_roles.remove']).optional().customSanitizer(val => { return val.map(x => { return x.toUpperCase() }) }).custom(val => { return isUserRole(val) }), + query(['active_roles.add']).optional().toArray() + .custom(isFlatStringArray) + .customSanitizer(toUpperCaseArray) + .custom(isUserRole), + query(['active_roles.remove']).optional().toArray() + .custom(isFlatStringArray) + .customSanitizer(toUpperCaseArray) + .custom(isUserRole), parseError, parsePostParams, controller.USER_UPDATE_SINGLE) diff --git a/src/middleware/middleware.js b/src/middleware/middleware.js index 5ddb0727c..10256b159 100644 --- a/src/middleware/middleware.js +++ b/src/middleware/middleware.js @@ -363,6 +363,54 @@ const limiter = rateLimit({ message: error.tooManyRequests() }) +/** + * Ensures value is a flat (one-dimensional) array of any length with values + * that are safe to cast to a string + * + * @param {Any} val + * @returns + */ +function isFlatStringArray (val) { + const errorMsg = 'Parameter must be a one-dimensional array of strings' + + if (!Array.isArray(val)) { + throw new Error(errorMsg) + } + + const validTypes = [ + 'boolean', 'number', 'bigint', 'string' + ] + + for (const k of val) { + if (!validTypes.includes(typeof k)) { + throw new Error(errorMsg) + } + } + + return true +} + +/** + * Recursively casts to strings and upper-cases all items in array + * + * @param {Array} val + */ +function toUpperCaseArray (val) { + if (!Array.isArray(val)) { + return val.toString().toUpperCase() + } + + const newArr = val.map(k => { + if (Array.isArray(k)) { + return toUpperCaseArray(k) + } else { + return k.toString().toUpperCase() + } + }) + + return newArr +} + module.exports = { setCacheControl, optionallyValidateUser, @@ -378,5 +426,7 @@ module.exports = { validateCveJsonSchema, errorHandler, validateJsonSyntax, - rateLimiter: limiter + rateLimiter: limiter, + isFlatStringArray, + toUpperCaseArray } diff --git a/test-http/src/test/org_user_tests/org.py b/test-http/src/test/org_user_tests/org.py index 59d5fa4c9..a8692211a 100644 --- a/test-http/src/test/org_user_tests/org.py +++ b/test-http/src/test/org_user_tests/org.py @@ -7,6 +7,7 @@ import random import requests import uuid +from urllib.parse import urlencode from src import env, utils from src.utils import (assert_contains, ok_response_contains, ok_response_contains_json, response_contains, @@ -16,6 +17,43 @@ MAX_SHORTNAME_LENGTH = 32 #### GET /org #### +ROLES_BAD_VALUES = [ + # sending an object / assoc array + # username=bob&authority.active_roles[a]=b + { 'a': 'ADMIN' }, + + # sending multi-dimensional arrays + # username=bob&authority.active_roles[][a]=b + [{ 'a': 'ADMIN' }], + [[ 'ADMIN' ]] +] + +ROLES_BAD_QUERY_VALUES = [ + # valid: single value + # "active_roles.add=CNA", + + # valid: ['CNA'] + # "active_roles.add[]=CNA", + + # valid: ['CNA'] + # "active_roles.add[][]=CNA", + + # valid: ['CNA'] + # "active_roles.add[8]=CNA" + + # valid: ['CNA', 'CNA'] + # "active_roles.add[]=CNA&active_roles.add=CNA" + + # bad assoc arrays are caught by valid param checks + # "active_roles.add[a]=CNA", + + + # invalid: [{'a': 'CNA'}] + "active_roles.add[][a]=CNA" + + # invalid: [{'CNA': null}] + "active_roles.add[][CNA]" +] def test_get_all_orgs(): """ secretariat users can request a list of all organizations """ @@ -226,6 +264,33 @@ def test_post_new_org_uuid_parameter(): assert res.status_code == 404 +def test_post_new_org_malformed_roles(): + """ service should return useful error message for malformed roles """ + + for data in ROLES_BAD_VALUES: + uid = str(uuid.uuid4())[:MAX_SHORTNAME_LENGTH] + + res = requests.post( + f'{env.AWG_BASE_URL}{ORG_URL}', + headers=utils.BASE_HEADERS, + json={ + 'short_name': uid, + 'name': uid, + 'uuid': str(uuid.uuid4()), + 'authority': { + 'active_roles': data + } + } + ) + + assert res.status_code == 400 + response_contains_json(res, 'message', 'Parameters were invalid') + + assert res.json()['details'][0]['msg'] == 'Parameter must be a one-dimensional array of strings' + assert res.json()['details'][0]['param'] == 'authority.active_roles' + assert res.json()['details'][0]['location'] == 'body' + + #### POST /org/:shortname/user #### def test_post_new_org_user(): """ secretariat user can create a new user without active roles """ @@ -250,6 +315,30 @@ def test_post_new_org_bad_role_blarg(): response_contains_json(res, 'message', f'{uid} was successfully created.') +def test_post_new_org_user_malformed_roles(): + """ service should return useful error message for malformed roles """ + uid = str(uuid.uuid4())[:MAX_SHORTNAME_LENGTH] + + for data in ROLES_BAD_VALUES: + res = requests.post( + f'{env.AWG_BASE_URL}{ORG_URL}/mitre/user', + headers=utils.BASE_HEADERS, + json={ + 'username': uid, + 'authority': { + 'active_roles': data + } + } + ) + + assert res.status_code == 400 + response_contains_json(res, 'message', 'Parameters were invalid') + + assert res.json()['details'][0]['msg'] == 'Parameter must be a one-dimensional array of strings' + assert res.json()['details'][0]['param'] == 'authority.active_roles' + assert res.json()['details'][0]['location'] == 'body' + + def test_post_new_org_user_empty_body(): """ an empty new org user body is invalid for username """ res = requests.post( @@ -302,6 +391,23 @@ def test_put_update_org_that_does_not_exist(): assert_contains(res, 'by the shortname path parameter does not exist') +def test_put_update_mitre_org_malformed_roles(): + """ service should return useful error message for malformed roles """ + + for data in ROLES_BAD_QUERY_VALUES: + res = requests.put( + f'{env.AWG_BASE_URL}{ORG_URL}/mitre', + headers=utils.BASE_HEADERS, + params=data + ) + + assert res.status_code == 400, str(data) + response_contains_json(res, 'message', 'Parameters were invalid') + + assert res.json()['details'][0]['msg'] == 'Parameter must be a one-dimensional array of strings' + assert res.json()['details'][0]['param'] == 'active_roles.add' + assert res.json()['details'][0]['location'] == 'query' + def test_put_update_mitre_org_nonexistent_user(): """ cve services api will fail requests from users that don't exist """ uid = str(uuid.uuid4())[:MAX_SHORTNAME_LENGTH] @@ -469,6 +575,33 @@ def test_put_update_user_remove_admin_role(): response_contains(res, 'User role does not exist.') +def test_put_update_user_malformed_roles(): + """ service should return useful error message for malformed roles """ + + org, user = create_new_user_with_new_org_by_uuid() + + res = requests.put( + f'{env.AWG_BASE_URL}{ORG_URL}/{org}/user/{user}', + headers=utils.BASE_HEADERS, + params={'active_roles.add': 'ADMIN'} + ) + assert res.status_code == 200 + + for data in ROLES_BAD_QUERY_VALUES: + res = requests.put( + f'{env.AWG_BASE_URL}{ORG_URL}/{org}/user/{user}', + headers=utils.BASE_HEADERS, + params=data + ) + + assert res.status_code == 400, str(data) + response_contains_json(res, 'message', 'Parameters were invalid') + + assert res.json()['details'][0]['msg'] == 'Parameter must be a one-dimensional array of strings' + assert res.json()['details'][0]['param'] == 'active_roles.add' + assert res.json()['details'][0]['location'] == 'query' + + #### PUT /org/:shortname/user/:username/reset_secret #### def test_put_update_user_reset_secret(): """ services api allows the secretariat to reset user secrets """