Repository: CVEProject/cve-services
Pull request #325 (2021-02-25 18:36:36)

Allow Users to update their own certain fields
Users may update their own fields except for: active, roles, org, which Secretariats and their Org Admin can update.
#250 


Patch:
diff --git a/src/controller/org.controller/error.js b/src/controller/org.controller/error.js
index d0ae2cf8..b5ed943c 100644
--- a/src/controller/org.controller/error.js
+++ b/src/controller/org.controller/error.js
@@ -22,10 +22,10 @@ class OrgControllerError extends idrErr.IDRError {
     return err
   }
 
-  notSameUserOrSecretariat () { // org
+  notOrgAdminOrSecretariat () { // org
     const err = {}
-    err.error = 'NOT_SAME_USER_OR_SECRETARIAT'
-    err.message = 'This information can only be viewed or modified by the Secretariat, an Org Admin or if the requester is the user.'
+    err.error = 'NOT_ORG_ADMIN_OR_SECRETARIAT'
+    err.message = 'This information can only be viewed or modified by the Secretariat or Org Admin.'
     return err
   }
 
diff --git a/src/controller/org.controller/index.js b/src/controller/org.controller/index.js
index 3a92162a..6f9d7a12 100644
--- a/src/controller/org.controller/index.js
+++ b/src/controller/org.controller/index.js
@@ -27,7 +27,6 @@ router.get('/org/:shortname',
   parseGetParams,
   controller.ORG_SINGLE)
 router.post('/org/:shortname',
-  mw.onlySecretariat,
   mw.validateUser,
   param(['shortname']).isString().trim().escape(),
   query(['shortname']).optional().isString().trim().escape().notEmpty(),
@@ -70,7 +69,6 @@ router.get('/org/:shortname/users',
   parseGetParams,
   controller.USER_ALL)
 router.post('/org/:shortname/user/:username',
-  mw.onlySecretariat,
   mw.validateUser,
   param(['shortname']).isString().trim().escape().notEmpty(),
   param(['username']).isString().trim().escape().notEmpty(),
diff --git a/src/controller/org.controller/org.controller.js b/src/controller/org.controller/org.controller.js
index bd117b80..fc858f0b 100644
--- a/src/controller/org.controller/org.controller.js
+++ b/src/controller/org.controller/org.controller.js
@@ -334,7 +334,7 @@ async function updateOrg (req, res, next) {
     }
     if ('active_roles.remove' in req.ctx.query) {
       if (Array.isArray(req.ctx.query['active_roles.remove'])) {
-        req.query['active_roles.remove'].forEach(r => {
+        req.ctx.query['active_roles.remove'].forEach(r => {
           removeRoles.push(r)
         })
       }
@@ -506,6 +506,8 @@ async function createUser (req, res, next) {
 // Updates a user only if the user exist for the specified username. If no user exists, it does not create the user.
 async function updateUser (req, res, next) {
   try {
+    const requesterShortName = req.ctx.org
+    const requesterUsername = req.ctx.user
     const username = req.ctx.params.username
     const shortName = req.ctx.params.shortname
     const newUser = new User()
@@ -516,7 +518,7 @@ async function updateUser (req, res, next) {
     let active
     const name = {
       first: null,
-      ast: null,
+      last: null,
       middle: null,
       surname: null,
       suffix: null
@@ -525,8 +527,11 @@ async function updateUser (req, res, next) {
     const addRoles = []
     const userRepo = req.ctx.repositories.getUserRepository()
     const orgRepo = req.ctx.repositories.getOrgRepository()
-
     const orgUUID = await orgRepo.getOrgUUID(shortName)
+    const isSecretariat = await orgRepo.isSecretariat(requesterShortName)
+    // Check if requester is Admin of the designated user's org
+    const isAdmin = await userRepo.isAdmin(requesterUsername, requesterShortName)
+
     if (!orgUUID) {
       returned = true
       logger.info({ uuid: req.ctx.uuid, message: 'The user could not be updated because ' + shortName + ' organization does not exist.' })
@@ -540,6 +545,15 @@ async function updateUser (req, res, next) {
       return res.status(404).json(error.userDne(username))
     }
 
+    // check if the user is not the requester or if the requester is not a secretariat
+    if ((shortName !== requesterShortName || username !== requesterUsername) && !isSecretariat) {
+      // check if the requester is not and admin; if admin, the requester must be from the same org as the user
+      if (!isAdmin || (isAdmin && shortName !== requesterShortName)) {
+        logger.info({ uuid: req.ctx.uuid, message: 'The user can only be updated by the Secretariat, an Org admin or if the requester is the user.' })
+        return res.status(403).json(error.notSameUserOrSecretariat())
+      }
+    }
+
     if ('new_username' in req.ctx.query) {
       newUsername = req.ctx.query.new_username
     }
@@ -584,12 +598,21 @@ async function updateUser (req, res, next) {
     }
     if ('active_roles.remove' in req.ctx.query) {
       if (Array.isArray(req.ctx.query['active_roles.remove'])) {
-        req.query['active_roles.remove'].forEach(r => {
+        req.ctx.query['active_roles.remove'].forEach(r => {
           removeRoles.push(r)
         })
       }
     }
 
+    // updating user's roles and org_uuid is only allowed for secretariats and org admins
+    if ((active || removeRoles.length || addRoles.length || newOrgShortName) && !isSecretariat) {
+      if (!isAdmin || (isAdmin && shortName !== requesterShortName)) {
+        returned = true
+        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.notOrgAdminOrSecretariat())
+      }
+    }
+
     // updating the user's username (the username field is never an empty string due to the stripping of the quotes and double quotes)
     if (newUsername && !returned) {
       newUser.username = newUsername
diff --git a/src/utils/error.js b/src/utils/error.js
index 9362324e..28c26a8f 100644
--- a/src/utils/error.js
+++ b/src/utils/error.js
@@ -72,6 +72,13 @@ class IDRError {
     return err
   }
 
+  notSameUserOrSecretariat () { // super
+    const err = {}
+    err.error = 'NOT_SAME_USER_OR_SECRETARIAT'
+    err.message = 'This information can only be viewed or modified by the Secretariat, an Org Admin or if the requester is the user.'
+    return err
+  }
+
   notOrgAdminOrSecretariat () { // super
     const err = {}
     err.error = 'NOT_ORG_ADMIN_OR_SECRETARIAT'


Test patch:
diff --git a/test-utils/repositories.js b/test-utils/repositories.js
index 5320aef2..d1e309d5 100644
--- a/test-utils/repositories.js
+++ b/test-utils/repositories.js
@@ -380,12 +380,20 @@ class OrgUserNotUpdatedOrgDoesntExist {
   async getOrgUUID () {
     return null
   }
+
+  async isSecretariat () {
+    return true
+  }
 }
 
 class OrgUserNotUpdatedUserDoesntExist {
   async getOrgUUID () {
     return orgMockObj.existentOrg
   }
+
+  async isSecretariat () {
+    return true
+  }
 }
 
 class OrgUserNotUpdatedOrgQueryDoesntExist {
@@ -395,12 +403,20 @@ class OrgUserNotUpdatedOrgQueryDoesntExist {
     }
     return null
   }
+
+  async isSecretariat () {
+    return true
+  }
 }
 
 class OrgUserUpdatedAddingRole {
   async getOrgUUID () {
     return orgMockObj.owningOrg.UUID
   }
+
+  async isSecretariat () {
+    return true
+  }
 }
 
 class OrgUserCveIdUpdated {
@@ -2548,12 +2564,20 @@ class UserNotUpdatedUserDoesntExist {
   async findOneByUserNameAndOrgUUID () {
     return null
   }
+
+  async isAdmin () {
+    return null
+  }
 }
 
 class UserNotUpdatedOrgQueryDoesntExist {
   async findOneByUserNameAndOrgUUID () {
     return orgMockObj.existentUser
   }
+
+  async isAdmin () {
+    return null
+  }
 }
 
 class UserUpdatedAddingRole {
@@ -2587,6 +2611,10 @@ class UserUpdatedAddingRole {
   async getUserUUID () {
     return this.user.UUID
   }
+
+  async isAdmin () {
+    return true
+  }
 }
 
 class UserUpdatedAddingRoleAlreadyExists {
@@ -2619,6 +2647,10 @@ class UserUpdatedAddingRoleAlreadyExists {
   async getUserUUID () {
     return this.user.UUID
   }
+
+  async isAdmin () {
+    return false
+  }
 }
 
 class UserUpdatedRemovingRole {
@@ -2652,6 +2684,10 @@ class UserUpdatedRemovingRole {
   async getUserUUID () {
     return this.user.UUID
   }
+
+  async isAdmin () {
+    return false
+  }
 }
 
 class UserUpdatedRemovingRoleAlreadyRemoved {
@@ -2682,6 +2718,10 @@ class UserUpdatedRemovingRoleAlreadyRemoved {
   async getUserUUID () {
     return this.user.UUID
   }
+
+  async isAdmin () {
+    return false
+  }
 }
 
 class UserNotUpdatedNoQuery {
@@ -2696,6 +2736,10 @@ class UserNotUpdatedNoQuery {
   async getUserUUID () {
     return orgMockObj.existentUser.UUID
   }
+
+  async isAdmin () {
+    return false
+  }
 }
 
 class UserOrgCveIdUpdated {
diff --git a/test/unit-tests/org/mockObjects.org.js b/test/unit-tests/org/mockObjects.org.js
index 053a2f66..c04a97f9 100644
--- a/test/unit-tests/org/mockObjects.org.js
+++ b/test/unit-tests/org/mockObjects.org.js
@@ -32,6 +32,7 @@ const userAHeader = {
   'PAGINATOR-PAGE': CONSTANTS.PAGINATOR_PAGE
 }
 
+// admin user header
 const userDHeader = {
   'content-type': 'application/json',
   'CVE-API-ORG': 'google',
diff --git a/test/unit-tests/org/userTest.js b/test/unit-tests/org/userTest.js
index 105db554..a076af5e 100644
--- a/test/unit-tests/org/userTest.js
+++ b/test/unit-tests/org/userTest.js
@@ -19,10 +19,13 @@ const existentUserDummy = require('./mockObjects.org').existentUserDummy
 const existentOrgDummy = require('./mockObjects.org').existentOrgDummy
 const nonExistentOrg = require('./mockObjects.org').nonExistentOrg
 
+const orgController = require('../../../src/controller/org.controller/org.controller')
+const orgParams = require('../../../src/controller/org.controller/org.middleware')
 const errors = require('../../../src/controller/org.controller/error')
 const CONSTANTS = require('../../../src/constants')
 const error = new errors.OrgControllerError()
 const testRepo = require('../../../test-utils/repositories')
+const { userD } = require('./mockObjects.org')
 
 describe('Test user functions in Org Controller', () => {
   context('Creating a user', () => {
@@ -373,6 +376,154 @@ describe('Test user functions in Org Controller', () => {
         })
     })
 
+    it('User is not updated because requestor is not Org Admin, Secretariat, or user', (done) => {
+      class Org {
+        async getOrgUUID () {
+          return existentOrg.UUID
+        }
+
+        async isSecretariat () {
+          return false
+        }
+      }
+
+      class User {
+        async findOneByUserNameAndOrgUUID () {
+          return existentUser
+        }
+
+        async isAdmin () {
+          return false
+        }
+      }
+
+      server.route('/user-not-updated-requestor-not-admin-secretariat-user/:shortname/:username')
+        .post((req, res, next) => {
+          const factory = {
+            getOrgRepository: () => { return new Org() },
+            getUserRepository: () => { return new User() }
+          }
+          req.ctx.repositories = factory
+          next()
+        }, orgParams.parsePostParams, orgController.USER_UPDATE_SINGLE)
+
+      chai.request(server)
+        .post(`/user-not-updated-requestor-not-admin-secretariat-user/${existentOrg.short_name}/${existentUser.username}?org_shortname=${existentOrg.short_name}`)
+        .set(owningOrgHeader)
+        .end((err, res) => {
+          if (err) {
+            done(err)
+          }
+
+          expect(res).to.have.status(403)
+          expect(res).to.have.property('body').and.to.be.a('object')
+          const errObj = error.notSameUserOrSecretariat()
+          expect(res.body.error).to.equal(errObj.error)
+          expect(res.body.message).to.equal(errObj.message)
+          done()
+        })
+    })
+
+    it('User is not updated because requestor is Org Admin of different organization', (done) => {
+      class Org {
+        async getOrgUUID () {
+          return existentOrg.UUID
+        }
+
+        async isSecretariat () {
+          return false
+        }
+      }
+
+      class User {
+        async findOneByUserNameAndOrgUUID () {
+          return existentUser
+        }
+
+        async isAdmin (username, shortname) {
+          expect(username).to.equal(userDHeader['CVE-API-USER'])
+          expect(shortname).to.equal(existentOrgDummy.short_name)
+          return true
+        }
+      }
+
+      server.route('/user-not-updated-requestor-different-admin/:shortname/:username')
+        .post((req, res, next) => {
+          const factory = {
+            getOrgRepository: () => { return new Org() },
+            getUserRepository: () => { return new User() }
+          }
+          req.ctx.repositories = factory
+          next()
+        }, orgParams.parsePostParams, orgController.USER_UPDATE_SINGLE)
+
+      chai.request(server)
+        .post(`/user-not-updated-requestor-different-admin/${existentOrg.short_name}/${existentUser.username}?org_shortname=${existentOrg.short_name}`)
+        .set(userDHeader)
+        .end((err, res) => {
+          if (err) {
+            done(err)
+          }
+
+          expect(res).to.have.status(403)
+          expect(res).to.have.property('body').and.to.be.a('object')
+          const errObj = error.notSameUserOrSecretariat()
+          expect(res.body.error).to.equal(errObj.error)
+          expect(res.body.message).to.equal(errObj.message)
+          done()
+        })
+    })
+
+    it('User is not updated because user can\'t update their own active field', (done) => {
+      class Org {
+        async getOrgUUID () {
+          return existentOrgDummy.UUID
+        }
+
+        async isSecretariat () {
+          return false
+        }
+      }
+
+      class User {
+        async findOneByUserNameAndOrgUUID () {
+          return userA
+        }
+
+        async isAdmin (username, shortname) {
+          expect(username).to.equal(userAHeader['CVE-API-USER'])
+          expect(shortname).to.equal(existentOrgDummy.short_name)
+          return false
+        }
+      }
+
+      server.route('/user-not-updated-cant-update-active-field/:shortname/:username')
+        .post((req, res, next) => {
+          const factory = {
+            getOrgRepository: () => { return new Org() },
+            getUserRepository: () => { return new User() }
+          }
+          req.ctx.repositories = factory
+          next()
+        }, orgParams.parsePostParams, orgController.USER_UPDATE_SINGLE)
+
+      chai.request(server)
+        .post(`/user-not-updated-cant-update-active-field/${existentOrgDummy.short_name}/${userA.username}?active=true`)
+        .set(userAHeader)
+        .end((err, res) => {
+          if (err) {
+            done(err)
+          }
+
+          expect(res).to.have.status(403)
+          expect(res).to.have.property('body').and.to.be.a('object')
+          const errObj = error.notOrgAdminOrSecretariat()
+          expect(res.body.error).to.equal(errObj.error)
+          expect(res.body.message).to.equal(errObj.message)
+          done()
+        })
+    })
+
     it('User is updated: Adding a user role', (done) => {
       const userTestRepo = new testRepo.UserUpdatedAddingRole()
       const testUser = Object.assign({}, existentUserDummy)
@@ -512,6 +663,123 @@ describe('Test user functions in Org Controller', () => {
         })
     })
 
+    it('User is updated: Deactivating User as Admin', (done) => {
+      class Org {
+        async getOrgUUID () {
+          return existentOrgDummy.UUID
+        }
+
+        async isSecretariat () {
+          return false
+        }
+      }
+
+      class User {
+        async findOneByUserNameAndOrgUUID () {
+          return userA
+        }
+
+        async isAdmin (username, shortname) {
+          expect(username).to.equal(userDHeader['CVE-API-USER'])
+          expect(shortname).to.equal(existentOrgDummy.short_name)
+          return true
+        }
+
+        async updateByUserNameAndOrgUUID () {
+          return { n: 1 }
+        }
+
+        async getUserUUID () {
+          return userD.UUID
+        }
+      }
+
+      server.route('/user-updated-requestor-admin-deactivate-user/:shortname/:username')
+        .post((req, res, next) => {
+          const factory = {
+            getOrgRepository: () => { return new Org() },
+            getUserRepository: () => { return new User() }
+          }
+          req.ctx.repositories = factory
+          next()
+        }, orgParams.parsePostParams, orgController.USER_UPDATE_SINGLE)
+
+      chai.request(server)
+        .post(`/user-updated-requestor-admin-deactivate-user/${existentOrgDummy.short_name}/${userA.username}?active=false`)
+        .set(userDHeader)
+        .end((err, res) => {
+          if (err) {
+            done(err)
+          }
+          expect(res).to.have.status(200)
+          expect(res).to.have.property('body').and.to.be.a('object')
+          expect(res.body).to.have.property('updated').and.to.be.a('object')
+          expect(res.body.updated.active).to.equal(false)
+          done()
+        })
+    })
+
+    it('User is updated: Username changed as user', (done) => {
+      class Org {
+        async getOrgUUID () {
+          return existentOrgDummy.UUID
+        }
+
+        async isSecretariat () {
+          return false
+        }
+      }
+
+      class User {
+        async findOneByUserNameAndOrgUUID () {
+          return userA
+        }
+
+        async isAdmin (username, shortname) {
+          expect(username).to.equal(userAHeader['CVE-API-USER'])
+          expect(shortname).to.equal(existentOrgDummy.short_name)
+          return false
+        }
+
+        async updateByUserNameAndOrgUUID () {
+          return { n: 1 }
+        }
+
+        async getUserUUID () {
+          return userA.UUID
+        }
+
+        async find () {
+          return []
+        }
+      }
+
+      server.route('/user-updated-requestor-user-username-changed/:shortname/:username')
+        .post((req, res, next) => {
+          const factory = {
+            getOrgRepository: () => { return new Org() },
+            getUserRepository: () => { return new User() }
+          }
+          req.ctx.repositories = factory
+          next()
+        }, orgParams.parsePostParams, orgController.USER_UPDATE_SINGLE)
+
+      chai.request(server)
+        .post(`/user-updated-requestor-user-username-changed/${existentOrgDummy.short_name}/${userA.username}?new_username=TESTER`)
+        .set(userAHeader)
+        .end((err, res) => {
+          if (err) {
+            done(err)
+          }
+
+          expect(res).to.have.status(200)
+          expect(res).to.have.property('body').and.to.be.a('object')
+          expect(res.body).to.have.property('updated').and.to.be.a('object')
+          expect(res.body.updated.username).to.equal('TESTER')
+          done()
+        })
+    })
+
     it('User is unchanged: No query parameters are provided', async () => {
       const result = existentUser