Merge pull request #1149 from CVEProject/dr-836

Resolves issue #836 - Update access management workflow in updateUser

src/controller/org.controller/org.controller.js

@@ -544,7 +544,6 @@ async function updateUser (req, res, next) {
     const shortName = req.ctx.params.shortname
     const newUser = new User()
     let newOrgShortName = null
-    let changesRequirePrivilegedRole = false // Set variable to true if protected fields are being modified
     const removeRoles = []
     const addRoles = []
     const userRepo = req.ctx.repositories.getUserRepository()
@@ -584,19 +583,39 @@ async function updateUser (req, res, next) {
     newUser.name.middle = user.name.middle
     newUser.name.suffix = user.name.suffix
 
+    const queryParameterPermissions = {
+      new_username: true,
+      org_short_name: true,
+      'name.first': false,
+      'name.last': false,
+      'name.middle': false,
+      'name.suffix': false,
+      active: true,
+      'active_roles.add': true,
+      'active_roles.remove': true
+
+    }
+
+    // Specific check for org_short_name
+    if (Object.keys(req.ctx.query).length > 0 && Object.keys(req.ctx.query).includes('org_short_name') && !(isSecretariat)) {
+      logger.info({ uuid: req.ctx.uuid, message: 'The user could not be updated because ' + requesterUsername + ' is an Org Admin and tried to reassign the organization.' })
+      return res.status(403).json(error.notAllowedToChangeOrganization())
+    }
+
+    // Check to ensure that the user has the right permissions to edit the fields tha they are requesting to edit, and fail fast if they do not.
+    if (Object.keys(req.ctx.query).length > 0 && Object.keys(req.ctx.query).some((key) => { return queryParameterPermissions[key] }) && !(isAdmin || isSecretariat)) {
+      logger.info({ uuid: req.ctx.uuid, message: 'The user could not be updated because ' + requesterUsername + ' user is not Org Admin or Secretariat to modify these fields.' })
+      console.log('in failed admin')
+      return res.status(403).json(error.notOrgAdminOrSecretariatUpdate())
+    }
+
     for (const k in req.ctx.query) {
       const key = k.toLowerCase()
 
       if (key === 'new_username') {
         newUser.username = req.ctx.query.new_username
-        changesRequirePrivilegedRole = true
       } else if (key === 'org_short_name') {
         newOrgShortName = req.ctx.query.org_short_name
-        changesRequirePrivilegedRole = true
-        if (!isSecretariat) {
-          logger.info({ uuid: req.ctx.uuid, message: 'The user could not be updated because ' + requesterUsername + ' is an Org Admin and tried to reassign the organization.' })
-          return res.status(403).json(error.notAllowedToChangeOrganization())
-        }
       } else if (key === 'name.first') {
         newUser.name.first = req.ctx.query['name.first']
       } else if (key === 'name.last') {
@@ -607,13 +626,11 @@ async function updateUser (req, res, next) {
         newUser.name.suffix = req.ctx.query['name.suffix']
       } else if (key === 'active') {
         newUser.active = booleanIsTrue(req.ctx.query.active)
-        changesRequirePrivilegedRole = true
       } else if (key === 'active_roles.add') {
         if (Array.isArray(req.ctx.query['active_roles.add'])) {
           req.ctx.query['active_roles.add'].forEach(r => {
             addRoles.push(r)
           })
-          changesRequirePrivilegedRole = true
         }
       } else if (key === 'active_roles.remove') {
         if (Array.isArray(req.ctx.query['active_roles.remove'])) {
@@ -624,17 +641,10 @@ async function updateUser (req, res, next) {
             }
             removeRoles.push(r)
           }
-          changesRequirePrivilegedRole = true
         }
       }
     }
 
-    // Check for correct privileges if the requested changes require them
-    if (changesRequirePrivilegedRole && !(isAdmin || isSecretariat)) {
-      logger.info({ uuid: req.ctx.uuid, message: 'The user could not be updated because ' + requesterUsername + ' user is not Org Admin or Secretariat to modify these fields.' })
-      return res.status(403).json(error.notOrgAdminOrSecretariatUpdate())
-    }
-
     // check if the new org exist
     if (newOrgShortName) {
       newUser.org_UUID = await orgRepo.getOrgUUID(newOrgShortName)

test/integration-tests/user/createUserTest.js

@@ -21,6 +21,20 @@ const body = {
     active_roles: ['Admin']
   }
 }
+
+const nonAdminBody = {
+  username: 'nonAdminUser',
+  active: 'true',
+  name: {
+    first: 'TestCnaAdmin',
+    last: 'test',
+    middle: 'N',
+    suffix: 'I'
+  },
+  authority: {
+  }
+}
+
 describe('Testing create user endpoint', () => {
   it('Should return 200 and new user', (done) => {
     chai.request(app)
@@ -35,4 +49,17 @@ describe('Testing create user endpoint', () => {
         done()
       })
   })
+  it('Should return 200 and create a non admin user', (done) => {
+    chai.request(app)
+      .post('/api/org/range_4/user')
+      .set(constants.headers)
+      .send(nonAdminBody)
+      .end((err, res) => {
+        expect(err).to.be.null
+        expect(res.body).to.have.property('created')
+        expect(res.body.created.username).to.equal(nonAdminBody.username)
+        expect(res).to.have.status(200)
+        done()
+      })
+  })
 })

test/integration-tests/user/updateUserTest.js

@@ -0,0 +1,33 @@
+/* eslint-disable no-unused-expressions */
+
+const chai = require('chai')
+chai.use(require('chai-http'))
+
+const expect = chai.expect
+
+const constants = require('../constants.js')
+const app = require('../../../src/index.js')
+
+describe('Testing Edit user endpoint', () => {
+  context('User edit tests', () => {
+    it('Should return 200 when only name changes are done', async () => {
+      await chai.request(app)
+        .put('/api/org/win_5/user/jasminesmith@win_5.com?name.first=NewName')
+        .set(constants.nonSecretariatUserHeaders)
+        .then((res, err) => {
+          expect(err).to.be.undefined
+          expect(res).to.have.status(200)
+        })
+    })
+    it('Should return an error when admin is required', async () => {
+      await chai.request(app)
+        .put('/api/org/win_5/user/jasminesmith@win_5.com?new_username=NewUsername')
+        .set(constants.nonSecretariatUserHeaders)
+        .then((res, err) => {
+          expect(err).to.be.undefined
+          expect(res).to.have.status(403)
+          expect(res.body.error).to.contain('NOT_ORG_ADMIN_OR_SECRETARIAT_UPDATE')
+        })
+    })
+  })
+})