Merge pull request #1027 from CVEProject/jd-809

Resolves #809 Prevents org admins from removing their admin role
CVEProject · Feb 17, 2023 · 2ccfe12 · 2ccfe12
2 parents 75c6bac + cf7f073
commit 2ccfe12
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 22 deletions.
diff --git a/src/controller/org.controller/error.js b/src/controller/org.controller/error.js
@@ -77,6 +77,13 @@ class OrgControllerError extends idrErr.IDRError {
     err.message = `'${param}' is not a valid parameter.`
     return err
   }
+
+  notAllowedToSelfDemote () {
+    const err = {}
+    err.error = 'NOT_ALLOWED_TO_SELF_DEMOTE'
+    err.message = 'Please have another admin user from your organization change your role.'
+    return err
+  }
 }
 
 module.exports = {

diff --git a/src/controller/org.controller/org.controller.js b/src/controller/org.controller/org.controller.js
@@ -575,7 +575,7 @@ async function updateUser (req, res, next) {
     newUser.name.middle = user.name.middle
     newUser.name.suffix = user.name.suffix
 
-    Object.keys(req.ctx.query).forEach(k => {
+    for (const k in req.ctx.query) {
       const key = k.toLowerCase()
 
       if (key === 'new_username') {
@@ -608,13 +608,17 @@ async function updateUser (req, res, next) {
         }
       } else if (key === 'active_roles.remove') {
         if (Array.isArray(req.ctx.query['active_roles.remove'])) {
-          req.ctx.query['active_roles.remove'].forEach(r => {
+          for (const r of req.ctx.query['active_roles.remove']) {
+            if (r.toLowerCase() === 'admin' && requesterUsername === username && (isAdmin || isSecretariat)) {
+              logger.info({ uuid: req.ctx.uuid, message: 'The user could not be updated because ' + requesterUsername + ' is an Org Admin changing their own role.' })
+              return res.status(403).json(error.notAllowedToSelfDemote())
+            }
             removeRoles.push(r)
-          })
+          }
           changesRequirePrivilegedRole = true
         }
       }
-    })
+    }
 
     // Check for correct privileges if the requested changes require them
     if (changesRequirePrivilegedRole && !(isAdmin || isSecretariat)) {

diff --git a/test-http/src/test/org_user_tests/org_as_org_admin.py b/test-http/src/test/org_user_tests/org_as_org_admin.py
@@ -455,25 +455,9 @@ def test_org_admin_update_own_user_roles_name(org_admin_headers):
         f'{env.AWG_BASE_URL}{ORG_URL}/{org}/user/{user}?active_roles.remove=admin',
         headers=org_admin_headers
     )
-    assert res.status_code == 200
-    assert json.loads(res.content.decode())[
-        'updated']['authority']['active_roles'] == []
-    res = requests.put(
-        # adding role
-        f'{env.AWG_BASE_URL}{ORG_URL}/{org}/user/{user}?active_roles.add=admin',
-        headers=org_admin_headers
-    )
     assert res.status_code == 403
-    # cannot add role because org admin doesn't have "ADMIN" role anymore
-    response_contains_json(res, 'error', 'NOT_ORG_ADMIN_OR_SECRETARIAT_UPDATE')
-    res = requests.put(
-        # adding "ADMIN" role back to org admin user
-        f'{env.AWG_BASE_URL}{ORG_URL}/{org}/user/{user}?active_roles.add=admin',
-        headers=utils.BASE_HEADERS
-    )
-    assert res.status_code == 200
-    assert json.loads(res.content.decode())[
-        'updated']['authority']['active_roles'] == ["ADMIN"]
+    response_contains_json(res, 'error', 'NOT_ALLOWED_TO_SELF_DEMOTE')
+
     res = requests.put(
         # updating name
         f'{env.AWG_BASE_URL}{ORG_URL}/{org}/user/{user}?name.first=t&name.last=e&name.middle=s&name.suffix=t',