Repository: CVEProject/cve-services
Pull request #320 (2021-02-19 01:41:06)

Secretariat can update user roles (aka. add/remove pre-defined user roles) 
Resolves issue #263 

* Updates `POST /org/:shortname/user/:username` endpoint
* Endpoint accessible only by the Secretariat user


Patch:
diff --git a/src/constants/index.js b/src/constants/index.js
index 355d33c1..5d69ee49 100644
--- a/src/constants/index.js
+++ b/src/constants/index.js
@@ -24,6 +24,9 @@ module.exports = {
   USER_ROLES: [
     'ADMIN'
   ],
+  USER_ROLE_ENUM: {
+    ADMIN: 'ADMIN'
+  },
   AUTH_HEADERS: {
     ORG: 'CVE-API-ORG',
     USER: 'CVE-API-USER',
diff --git a/src/controller/org.controller/index.js b/src/controller/org.controller/index.js
index 77486554..96b44f4f 100644
--- a/src/controller/org.controller/index.js
+++ b/src/controller/org.controller/index.js
@@ -3,7 +3,7 @@ const router = express.Router()
 const mw = require('../../middleware/middleware')
 const controller = require('./org.controller')
 const { body, param, query } = require('express-validator')
-const { parseGetParams, parsePostParams, parseError, isOrgRole } = require('./org.middleware')
+const { parseGetParams, parsePostParams, parseError, isOrgRole, isUserRole } = require('./org.middleware')
 const CONSTANTS = require('../../../src/constants')
 
 router.get('/org',
@@ -81,6 +81,10 @@ router.post('/org/:shortname/user/:username',
   query(['name.middle']).optional().isString().trim().escape(),
   query(['name.suffix']).optional().isString().trim().escape(),
   query(['name.surname']).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) }),
   parseError,
   parsePostParams,
   controller.USER_UPDATE_SINGLE)
diff --git a/src/controller/org.controller/org.controller.js b/src/controller/org.controller/org.controller.js
index 168eafd7..8e251cda 100644
--- a/src/controller/org.controller/org.controller.js
+++ b/src/controller/org.controller/org.controller.js
@@ -510,6 +510,8 @@ async function updateUser (req, res, next) {
       surname: null,
       suffix: null
     }
+    const removeRoles = []
+    const addRoles = []
     const userRepo = req.ctx.repositories.getUserRepository()
     const orgRepo = req.ctx.repositories.getOrgRepository()
 
@@ -562,6 +564,21 @@ async function updateUser (req, res, next) {
       nameChanged = true
     }
 
+    if ('active_roles.add' in req.ctx.query) {
+      if (Array.isArray(req.ctx.query['active_roles.add'])) {
+        req.ctx.query['active_roles.add'].forEach(r => {
+          addRoles.push(r)
+        })
+      }
+    }
+    if ('active_roles.remove' in req.ctx.query) {
+      if (Array.isArray(req.ctx.query['active_roles.remove'])) {
+        req.query['active_roles.remove'].forEach(r => {
+          removeRoles.push(r)
+        })
+      }
+    }
+
     // 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
@@ -611,7 +628,27 @@ async function updateUser (req, res, next) {
       }
     }
 
+    // updating the user's roles
     if (!returned) {
+      const roles = user.authority.active_roles
+
+      // adding roles
+      addRoles.forEach(role => {
+        if (!roles.includes(role)) {
+          roles.push(role)
+        }
+      })
+
+      // removing roles
+      removeRoles.forEach(role => {
+        const index = roles.indexOf(role)
+
+        if (index > -1) {
+          roles.splice(index, 1)
+        }
+      })
+
+      newUser.authority.active_roles = roles
       const result = await userRepo.updateByUserNameAndOrgUUID(username, orgUUID, newUser)
 
       if (result.n === 0) {
diff --git a/src/controller/org.controller/org.middleware.js b/src/controller/org.controller/org.middleware.js
index 41aa48d7..0f61883c 100644
--- a/src/controller/org.controller/org.middleware.js
+++ b/src/controller/org.controller/org.middleware.js
@@ -7,7 +7,17 @@ const utils = require('../../utils/utils')
 function isOrgRole (val) {
   val.forEach(role => {
     if (!CONSTANTS.ORG_ROLES.includes(role)) {
-      throw new Error('Organization role \'' + role + '\' does not exist.')
+      throw new Error('Organization role does not exist.')
+    }
+  })
+
+  return true
+}
+
+function isUserRole (val) {
+  val.forEach(role => {
+    if (!CONSTANTS.USER_ROLES.includes(role)) {
+      throw new Error('User role does not exist.')
     }
   })
 
@@ -45,5 +55,6 @@ module.exports = {
   parsePostParams,
   parseGetParams,
   parseError,
-  isOrgRole
+  isOrgRole,
+  isUserRole
 }


Test patch:
diff --git a/test-utils/index.js b/test-utils/index.js
index 03f5bfd8..7b66ef3c 100644
--- a/test-utils/index.js
+++ b/test-utils/index.js
@@ -382,6 +382,46 @@ app.route('/user-not-updated-user-doesnt-exist/:shortname/:username')
     next()
   }, orgParams.parsePostParams, orgController.USER_UPDATE_SINGLE)
 
+app.route('/user-updated-adding-role-1/:shortname/:username')
+  .post((req, res, next) => {
+    const factory = {
+      getOrgRepository: () => { return new repos.OrgUserUpdatedAddingRole() },
+      getUserRepository: () => { return new repos.UserUpdatedAddingRole() }
+    }
+    req.ctx.repositories = factory
+    next()
+  }, orgParams.parsePostParams, orgController.USER_UPDATE_SINGLE)
+
+app.route('/user-updated-adding-role-2/:shortname/:username')
+  .post((req, res, next) => {
+    const factory = {
+      getOrgRepository: () => { return new repos.OrgUserUpdatedAddingRole() },
+      getUserRepository: () => { return new repos.UserUpdatedAddingRoleAlreadyExists() }
+    }
+    req.ctx.repositories = factory
+    next()
+  }, orgParams.parsePostParams, orgController.USER_UPDATE_SINGLE)
+
+app.route('/user-updated-removing-role-1/:shortname/:username')
+  .post((req, res, next) => {
+    const factory = {
+      getOrgRepository: () => { return new repos.OrgUserUpdatedAddingRole() },
+      getUserRepository: () => { return new repos.UserUpdatedRemovingRole() }
+    }
+    req.ctx.repositories = factory
+    next()
+  }, orgParams.parsePostParams, orgController.USER_UPDATE_SINGLE)
+
+app.route('/user-updated-removing-role-2/:shortname/:username')
+  .post((req, res, next) => {
+    const factory = {
+      getOrgRepository: () => { return new repos.OrgUserUpdatedAddingRole() },
+      getUserRepository: () => { return new repos.UserUpdatedRemovingRoleAlreadyRemoved() }
+    }
+    req.ctx.repositories = factory
+    next()
+  }, orgParams.parsePostParams, orgController.USER_UPDATE_SINGLE)
+
 app.route('/user-not-updated-no-parameters/:shortname/:username')
   .post((req, res, next) => {
     const factory = {
diff --git a/test-utils/repositories.js b/test-utils/repositories.js
index fe5ac3e0..9bb90808 100644
--- a/test-utils/repositories.js
+++ b/test-utils/repositories.js
@@ -342,6 +342,12 @@ class OrgUserNotUpdatedOrgQueryDoesntExist {
   }
 }
 
+class OrgUserUpdatedAddingRole {
+  async getOrgUUID () {
+    return orgMockObj.owningOrg.UUID
+  }
+}
+
 class OrgUserCveIdUpdated {
   async getOrgUUID (shortname) {
     if (shortname === cveIdMockObj.owningOrg.short_name) {
@@ -2431,6 +2437,134 @@ class UserNotUpdatedOrgQueryDoesntExist {
   }
 }
 
+class UserUpdatedAddingRole {
+  constructor () {
+    this.user = {
+      org_UUID: orgMockObj.existentUserDummy.org_UUID,
+      username: orgMockObj.existentUserDummy.username,
+      UUID: orgMockObj.existentUserDummy.UUID,
+      active: orgMockObj.existentUserDummy.active,
+      name: orgMockObj.existentUserDummy.name,
+      authority: {
+        active_roles: []
+      },
+      secret: orgMockObj.existentUserDummy.secret
+    }
+  }
+
+  getUser () {
+    this.user.authority.active_roles.push(CONSTANTS.USER_ROLE_ENUM.ADMIN)
+    return this.user
+  }
+
+  async findOneByUserNameAndOrgUUID () {
+    return this.user
+  }
+
+  async updateByUserNameAndOrgUUID () {
+    return { n: 1, nModified: 1, ok: 1 }
+  }
+
+  async getUserUUID () {
+    return this.user.UUID
+  }
+}
+
+class UserUpdatedAddingRoleAlreadyExists {
+  constructor () {
+    this.user = {
+      org_UUID: orgMockObj.existentUserDummy.org_UUID,
+      username: orgMockObj.existentUserDummy.username,
+      UUID: orgMockObj.existentUserDummy.UUID,
+      active: orgMockObj.existentUserDummy.active,
+      name: orgMockObj.existentUserDummy.name,
+      authority: {
+        active_roles: [CONSTANTS.USER_ROLE_ENUM.ADMIN]
+      },
+      secret: orgMockObj.existentUserDummy.secret
+    }
+  }
+
+  getUser () {
+    return this.user
+  }
+
+  async findOneByUserNameAndOrgUUID () {
+    return this.user
+  }
+
+  async updateByUserNameAndOrgUUID () {
+    return { n: 1, nModified: 1, ok: 1 }
+  }
+
+  async getUserUUID () {
+    return this.user.UUID
+  }
+}
+
+class UserUpdatedRemovingRole {
+  constructor () {
+    this.user = {
+      org_UUID: orgMockObj.existentUserDummy.org_UUID,
+      username: orgMockObj.existentUserDummy.username,
+      UUID: orgMockObj.existentUserDummy.UUID,
+      active: orgMockObj.existentUserDummy.active,
+      name: orgMockObj.existentUserDummy.name,
+      authority: {
+        active_roles: [CONSTANTS.USER_ROLE_ENUM.ADMIN]
+      },
+      secret: orgMockObj.existentUserDummy.secret
+    }
+  }
+
+  getUser () {
+    this.user.authority.active_roles.splice(0, 1)
+    return this.user
+  }
+
+  async findOneByUserNameAndOrgUUID () {
+    return this.user
+  }
+
+  async updateByUserNameAndOrgUUID () {
+    return { n: 1, nModified: 1, ok: 1 }
+  }
+
+  async getUserUUID () {
+    return this.user.UUID
+  }
+}
+
+class UserUpdatedRemovingRoleAlreadyRemoved {
+  constructor () {
+    this.user = {
+      org_UUID: orgMockObj.existentUserDummy.org_UUID,
+      username: orgMockObj.existentUserDummy.username,
+      UUID: orgMockObj.existentUserDummy.UUID,
+      active: orgMockObj.existentUserDummy.active,
+      name: orgMockObj.existentUserDummy.name,
+      authority: orgMockObj.existentUserDummy.authority,
+      secret: orgMockObj.existentUserDummy.secret
+    }
+  }
+
+  getUser () {
+    return this.user
+  }
+
+  async findOneByUserNameAndOrgUUID () {
+    return this.user
+  }
+
+  async updateByUserNameAndOrgUUID () {
+    return { n: 1, nModified: 1, ok: 1 }
+  }
+
+  async getUserUUID () {
+    return this.user.UUID
+  }
+}
+
 class UserNotUpdatedNoQuery {
   async findOneByUserNameAndOrgUUID () {
     return orgMockObj.existentUser
@@ -2652,6 +2786,7 @@ module.exports = {
   OrgUserNotUpdatedOrgDoesntExist,
   OrgUserNotUpdatedUserDoesntExist,
   OrgUserNotUpdatedOrgQueryDoesntExist,
+  OrgUserUpdatedAddingRole,
   OrgUserCveIdUpdated,
   OrgGetUserCveIdUpdated,
   OrgUserSecretNotResetOrgDoesntExist,
@@ -2759,6 +2894,10 @@ module.exports = {
   UserNotCreatedAlreadyExists,
   UserNotUpdatedUserDoesntExist,
   UserNotUpdatedOrgQueryDoesntExist,
+  UserUpdatedAddingRole,
+  UserUpdatedAddingRoleAlreadyExists,
+  UserUpdatedRemovingRole,
+  UserUpdatedRemovingRoleAlreadyRemoved,
   UserNotUpdatedNoQuery,
   UserOrgCveIdUpdated,
   UserGetUserCveIdUpdated,
diff --git a/test/unit-tests/org/userTest.js b/test/unit-tests/org/userTest.js
index 9db6924c..5560ff5c 100644
--- a/test/unit-tests/org/userTest.js
+++ b/test/unit-tests/org/userTest.js
@@ -19,7 +19,9 @@ const existentOrgDummy = require('./mockObjects.org').existentOrgDummy
 const nonExistentOrg = require('./mockObjects.org').nonExistentOrg
 
 const errors = require('../../../src/controller/org.controller/error')
+const CONSTANTS = require('../../../src/constants')
 const error = new errors.OrgControllerError()
+const testRepo = require('../../../test-utils/repositories')
 
 describe('Test user functions in Org Controller', () => {
   context('Creating a user', () => {
@@ -263,8 +265,146 @@ describe('Test user functions in Org Controller', () => {
         })
     })
 
-    // check that the user is unchanged
-    it('No query parameters are provided', async () => {
+    it('User is updated: Adding a user role', (done) => {
+      const userTestRepo = new testRepo.UserUpdatedAddingRole()
+      const testUser = Object.assign({}, existentUserDummy)
+      testUser.authority = {
+        active_roles: [CONSTANTS.USER_ROLE_ENUM.ADMIN]
+      }
+
+      chai.request(server)
+        .post(`/user-updated-adding-role-1/${owningOrg.short_name}/${testUser.username}?active_roles.add=${CONSTANTS.USER_ROLE_ENUM.ADMIN}&active_roles.add=${CONSTANTS.USER_ROLE_ENUM.ADMIN}`)
+        .set(secretariatHeader)
+        .end((err, res) => {
+          if (err) {
+            done(err)
+          }
+
+          const user = userTestRepo.getUser()
+          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.authority.active_roles).to.have.lengthOf(1)
+          expect(res.body.updated.authority.active_roles[0]).to.equal(CONSTANTS.USER_ROLE_ENUM.ADMIN)
+          expect(user.authority.active_roles).to.have.lengthOf(1)
+          expect(user.authority.active_roles[0]).to.equal(testUser.authority.active_roles[0])
+          expect(user.org_UUID).to.equal(testUser.org_UUID)
+          expect(user.username).to.equal(testUser.username)
+          expect(user.UUID).to.equal(testUser.UUID)
+          expect(user.secret).to.equal(testUser.secret)
+          expect(user.active).to.equal(testUser.active)
+          expect(user.name.first).to.equal(testUser.name.first)
+          expect(user.name.last).to.equal(testUser.name.last)
+          expect(user.name.middle).to.equal(testUser.name.middle)
+          expect(user.name.suffix).to.equal(testUser.name.suffix)
+          expect(user.name.surname).to.equal(testUser.name.surname)
+          done()
+        })
+    })
+
+    it('User is unchanged: Adding a user role that the user already have', (done) => {
+      const userTestRepo = new testRepo.UserUpdatedAddingRoleAlreadyExists()
+      const testUser = Object.assign({}, existentUserDummy)
+      testUser.authority = {
+        active_roles: [CONSTANTS.USER_ROLE_ENUM.ADMIN]
+      }
+
+      chai.request(server)
+        .post(`/user-updated-adding-role-2/${owningOrg.short_name}/${testUser.username}?active_roles.add=${CONSTANTS.USER_ROLE_ENUM.ADMIN}&active_roles.add=${CONSTANTS.USER_ROLE_ENUM.ADMIN}`)
+        .set(secretariatHeader)
+        .end((err, res) => {
+          if (err) {
+            done(err)
+          }
+
+          const user = userTestRepo.getUser()
+          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.authority.active_roles).to.have.lengthOf(1)
+          expect(res.body.updated.authority.active_roles[0]).to.equal(CONSTANTS.USER_ROLE_ENUM.ADMIN)
+          expect(user.authority.active_roles).to.have.lengthOf(1)
+          expect(user.authority.active_roles[0]).to.equal(testUser.authority.active_roles[0])
+          expect(user.org_UUID).to.equal(testUser.org_UUID)
+          expect(user.username).to.equal(testUser.username)
+          expect(user.UUID).to.equal(testUser.UUID)
+          expect(user.secret).to.equal(testUser.secret)
+          expect(user.active).to.equal(testUser.active)
+          expect(user.name.first).to.equal(testUser.name.first)
+          expect(user.name.last).to.equal(testUser.name.last)
+          expect(user.name.middle).to.equal(testUser.name.middle)
+          expect(user.name.suffix).to.equal(testUser.name.suffix)
+          expect(user.name.surname).to.equal(testUser.name.surname)
+          done()
+        })
+    })
+
+    it('User is updated: Removing a user role', (done) => {
+      const userTestRepo = new testRepo.UserUpdatedRemovingRole()
+      const testUser = Object.assign({}, existentUserDummy)
+      testUser.authority = {
+        active_roles: [CONSTANTS.USER_ROLE_ENUM.ADMIN]
+      }
+
+      chai.request(server)
+        .post(`/user-updated-removing-role-1/${owningOrg.short_name}/${testUser.username}?active_roles.remove=${CONSTANTS.USER_ROLE_ENUM.ADMIN}&active_roles.remove=${CONSTANTS.USER_ROLE_ENUM.ADMIN}`)
+        .set(secretariatHeader)
+        .end((err, res) => {
+          if (err) {
+            done(err)
+          }
+
+          const user = userTestRepo.getUser()
+          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.authority.active_roles).to.have.lengthOf(0)
+          expect(user.authority.active_roles).to.have.lengthOf(0)
+          expect(user.org_UUID).to.equal(testUser.org_UUID)
+          expect(user.username).to.equal(testUser.username)
+          expect(user.UUID).to.equal(testUser.UUID)
+          expect(user.secret).to.equal(testUser.secret)
+          expect(user.active).to.equal(testUser.active)
+          expect(user.name.first).to.equal(testUser.name.first)
+          expect(user.name.last).to.equal(testUser.name.last)
+          expect(user.name.middle).to.equal(testUser.name.middle)
+          expect(user.name.suffix).to.equal(testUser.name.suffix)
+          expect(user.name.surname).to.equal(testUser.name.surname)
+          done()
+        })
+    })
+
+    it('User is unchanged: Removing a user role that the user does not have', (done) => {
+      const userTestRepo = new testRepo.UserUpdatedRemovingRoleAlreadyRemoved()
+      chai.request(server)
+        .post(`/user-updated-removing-role-2/${owningOrg.short_name}/${existentUserDummy.username}?active_roles.remove=${CONSTANTS.USER_ROLE_ENUM.ADMIN}&active_roles.remove=${CONSTANTS.USER_ROLE_ENUM.ADMIN}`)
+        .set(secretariatHeader)
+        .end((err, res) => {
+          if (err) {
+            done(err)
+          }
+
+          const user = userTestRepo.getUser()
+          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.authority.active_roles).to.have.lengthOf(0)
+          expect(user.authority.active_roles).to.have.lengthOf(0)
+          expect(user.org_UUID).to.equal(existentUserDummy.org_UUID)
+          expect(user.username).to.equal(existentUserDummy.username)
+          expect(user.UUID).to.equal(existentUserDummy.UUID)
+          expect(user.secret).to.equal(existentUserDummy.secret)
+          expect(user.active).to.equal(existentUserDummy.active)
+          expect(user.name.first).to.equal(existentUserDummy.name.first)
+          expect(user.name.last).to.equal(existentUserDummy.name.last)
+          expect(user.name.middle).to.equal(existentUserDummy.name.middle)
+          expect(user.name.suffix).to.equal(existentUserDummy.name.suffix)
+          expect(user.name.surname).to.equal(existentUserDummy.name.surname)
+          done()
+        })
+    })
+
+    it('User is unchanged: No query parameters are provided', async () => {
       const result = existentUser
 
       const res = await chai.request(server)
@@ -275,7 +415,7 @@ describe('Test user functions in Org Controller', () => {
       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.authority.active_roles[0]).to.equal(existentUser.authority.active_roles[0])
-      expect(result.cna_short_name).to.equal(existentUser.cna_short_name)
+      expect(result.org_UUID).to.equal(existentUser.org_UUID)
       expect(result.username).to.equal(existentUser.username)
       expect(result.UUID).to.equal(existentUser.UUID)
       expect(result.secret).to.equal(existentUser.secret)