Skip to content

Commit

Permalink
#609 Added validators and sanitizers for role creation and update
Browse files Browse the repository at this point in the history
  • Loading branch information
@brettp
brettp committed Feb 17, 2023
1 parent 2ccfe12 commit 4475197
Show file tree
Hide file tree
Showing 3 changed files with 210 additions and 12 deletions.
37 changes: 26 additions & 11 deletions src/controller/org.controller/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
52 changes: 51 additions & 1 deletion src/middleware/middleware.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -378,5 +426,7 @@ module.exports = {
validateCveJsonSchema,
errorHandler,
validateJsonSyntax,
rateLimiter: limiter
rateLimiter: limiter,
isFlatStringArray,
toUpperCaseArray
}
133 changes: 133 additions & 0 deletions test-http/src/test/org_user_tests/org.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 """
Expand Down Expand Up @@ -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 """
Expand All @@ -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(
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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 """
Expand Down

0 comments on commit 4475197

Please sign in to comment.