Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolves issue #887. Do not create orgs if UUID is passed. Correct error messages. #1007

Merged
merged 1 commit into from
Feb 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/controller/org.controller/error.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ class OrgControllerError extends idrErr.IDRError {
return err
}

uuidProvided () { // org
uuidProvided (creationType) {
const err = {}
err.error = 'UUID_PROVIDED'
err.message = 'Providing UUIDs for user creation or update is not allowed.'
err.message = `Providing UUIDs for ${creationType} creation or update is not allowed.`
return err
}

Expand Down
2 changes: 0 additions & 2 deletions src/controller/org.controller/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,6 @@ 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(),
// TODO remove uuid - providing it throws an error in the controller
body(['uuid']).optional().isString().trim().escape(),
body(['authority.active_roles']).optional().customSanitizer(val => { return val.map(x => { return x.toUpperCase() }) }).custom(val => { return isOrgRole(val) }),
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,
Expand Down
45 changes: 27 additions & 18 deletions src/controller/org.controller/org.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,25 +241,34 @@ async function createOrg (req, res, next) {
const newOrg = new Org()
const orgRepo = req.ctx.repositories.getOrgRepository()

Object.keys(req.ctx.body).forEach(k => {
for (const k in req.ctx.body) {
const key = k.toLowerCase()

if (key === 'short_name') {
newOrg.short_name = decodeEntities(req.ctx.body.short_name)
} else if (key === 'name') {
newOrg.name = decodeEntities(req.ctx.body.name)
} else if (key === 'authority') {
if ('active_roles' in req.ctx.body.authority === true) {
newOrg.authority.active_roles = req.ctx.body.authority.active_roles
}
} else if (key === 'policies') {
if ('id_quota' in req.ctx.body.policies === true) {
newOrg.policies.id_quota = req.ctx.body.policies.id_quota
}
} else if (key === 'uuid') {
return res.status(400).json(error.uuidProvided())
switch (key) {
case 'short_name':
newOrg.short_name = decodeEntities(req.ctx.body.short_name)
break

case 'name':
newOrg.name = decodeEntities(req.ctx.body.name)
break

case 'authority':
if ('active_roles' in req.ctx.body.authority) {
newOrg.authority.active_roles = req.ctx.body.authority.active_roles
}
break

case 'policies':
if ('id_quota' in req.ctx.body.policies) {
newOrg.policies.id_quota = req.ctx.body.policies.id_quota
}
break

case 'uuid':
return res.status(400).json(error.uuidProvided('org'))
}
})
}

let result = await orgRepo.findOneByShortName(newOrg.short_name) // Find org in MongoDB
if (result) {
Expand Down Expand Up @@ -455,9 +464,9 @@ async function createUser (req, res, next) {
newUser.name.suffix = decodeEntities(req.ctx.body.name.suffix)
}
} else if (key === 'org_uuid') {
return res.status(400).json(error.uuidProvided())
return res.status(400).json(error.uuidProvided('org'))
} else if (key === 'uuid') {
return res.status(400).json(error.uuidProvided())
return res.status(400).json(error.uuidProvided('user'))
}
})

Expand Down
30 changes: 30 additions & 0 deletions test-http/src/test/org_user_tests/org.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,36 @@ def test_post_new_org_duplicate_parameter():
response_contains_json(res, 'message', 'Parameters were invalid')


def test_post_new_org_uuid_parameter():
""" should reject creating new orgs with uuid param """
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())
}
)

json_content = json.loads(res.content.decode())
assert res.status_code == 400
assert 'error' in json_content
assert json_content['error'] == 'UUID_PROVIDED'
assert 'message' in json_content
response_contains_json(res, 'message', 'Providing UUIDs for org creation or update is not allowed.')

# check that it didn't actually create the org:
# https://github.com/CVEProject/cve-services/issues/887
res = requests.get(
f'{env.AWG_BASE_URL}{ORG_URL}/{uid}',
headers=utils.BASE_HEADERS
)

assert res.status_code == 404


#### POST /org/:shortname/user ####
def test_post_new_org_user():
""" secretariat user can create a new user without active roles """
Expand Down
8 changes: 7 additions & 1 deletion test/unit-tests/org/orgCreateTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,17 @@ describe('Testing the POST /org endpoint in Org Controller', () => {

expect(res).to.have.status(400)
expect(res).to.have.property('body').and.to.be.a('object')
const errObj = error.uuidProvided()
const errObj = error.uuidProvided('org')
expect(res.body.error).to.equal(errObj.error)
expect(res.body.message).to.equal(errObj.message)
done()
})

// check that it really didn't create the org
// https://github.com/CVEProject/cve-services/issues/887

chai.request(app)
.get('/')
})

it('Org is not created because it already exists', (done) => {
Expand Down
4 changes: 2 additions & 2 deletions test/unit-tests/user/userCreateTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ describe('Testing the POST /org/:shortname/user endpoint in Org Controller', ()

expect(res).to.have.status(400)
expect(res).to.have.property('body').and.to.be.a('object')
const errObj = error.uuidProvided()
const errObj = error.uuidProvided('user')
expect(res.body.error).to.equal(errObj.error)
expect(res.body.message).to.equal(errObj.message)
done()
Expand Down Expand Up @@ -201,7 +201,7 @@ describe('Testing the POST /org/:shortname/user endpoint in Org Controller', ()

expect(res).to.have.status(400)
expect(res).to.have.property('body').and.to.be.a('object')
const errObj = error.uuidProvided()
const errObj = error.uuidProvided('user')
expect(res.body.error).to.equal(errObj.error)
expect(res.body.message).to.equal(errObj.message)
done()
Expand Down