fix: properly stringify+parse ACL objects

dadi · Jun 27, 2018 · 3db3dbf · 3db3dbf
1 parent be5de8b
commit 3db3dbf
Show file tree

Hide file tree

Showing 14 changed files with 354 additions and 316 deletions.
diff --git a/dadi/lib/auth/index.js b/dadi/lib/auth/index.js
@@ -100,7 +100,7 @@ AuthMiddleware.prototype.generateToken = function (req, res, next) {
       return this.handleInvalidCredentials(req, res, next)
     }
 
-    let client = clientModel.sanitise(results[0])
+    let client = clientModel.formatForOutput(results[0])
     let payload = {
       clientId,
       accessType: client.accessType

diff --git a/dadi/lib/controller/clients.js b/dadi/lib/controller/clients.js
@@ -148,7 +148,7 @@ Clients.prototype.get = function (req, res, next) {
     }
 
     let sanitisedClients = clients.results.map(client => {
-      return model.sanitise(client)
+      return model.formatForOutput(client)
     })
 
     help.sendBackJSON(200, res, next)(null, {
@@ -236,7 +236,7 @@ Clients.prototype.post = function (req, res, next) {
   }).then(({results}) => {
     help.sendBackJSON(201, res, next)(null, {
       results: [
-        model.sanitise(results[0])
+        model.formatForOutput(results[0])
       ]
     })
   }).catch(this.handleError(res, next))
@@ -270,23 +270,27 @@ Clients.prototype.postResource = function (req, res, next) {
       )
     }
 
-    return acl.access.get(req.dadiApiClient, req.body.name).then(access => {
-      let forbiddenType = Object.keys(req.body.access).find(type => {
-        return access[type] !== true
-      })
-
-      if (forbiddenType) {
-        return Promise.reject(
-          acl.createError(req.dadiApiClient)
-        )
-      }
+    // If the client does not have admin access, we need to ensure that
+    // they have each of the access types they are trying to assign.
+    if (!model.isAdmin(req.dadiApiClient)) {
+      return acl.access.get(req.dadiApiClient, req.body.name).then(access => {
+        let forbiddenType = Object.keys(req.body.access).find(type => {
+          return access[type] !== true
+        })
 
-      return model.resourceAdd(
-        req.params.clientId,
-        req.body.name,
-        req.body.access
-      )
-    })
+        if (forbiddenType) {
+          return Promise.reject(
+            acl.createError(req.dadiApiClient)
+          )
+        }
+      })
+    }
+  }).then(() => {
+    return model.resourceAdd(
+      req.params.clientId,
+      req.body.name,
+      req.body.access
+    )
   }).then(({results}) => {
     help.sendBackJSON(201, res, next)(null, {results})
   }).catch(this.handleError(res, next))
@@ -361,23 +365,27 @@ Clients.prototype.putResource = function (req, res, next) {
       )
     }
 
-    return acl.access.get(req.dadiApiClient, req.params.resource).then(access => {
-      let forbiddenType = Object.keys(req.body).find(type => {
-        return access[type] !== true
-      })
-
-      if (forbiddenType) {
-        return Promise.reject(
-          acl.createError(req.dadiApiClient)
-        )
-      }
+    // If the client does not have admin access, we need to ensure that
+    // they have each of the access types they are trying to assign.
+    if (!model.isAdmin(req.dadiApiClient)) {
+      return acl.access.get(req.dadiApiClient, req.params.resource).then(access => {
+        let forbiddenType = Object.keys(req.body).find(type => {
+          return access[type] !== true
+        })
 
-      return model.resourceUpdate(
-        req.params.clientId,
-        req.params.resource,
-        req.body
-      )
-    })
+        if (forbiddenType) {
+          return Promise.reject(
+            acl.createError(req.dadiApiClient)
+          )
+        }
+      })
+    }
+  }).then(() => {
+    return model.resourceUpdate(
+      req.params.clientId,
+      req.params.resource,
+      req.body
+    )
   }).then(({results}) => {
     help.sendBackJSON(200, res, next)(null, {results})
   }).catch(this.handleError(res, next))

diff --git a/dadi/lib/controller/roles.js b/dadi/lib/controller/roles.js
@@ -132,7 +132,7 @@ Roles.prototype.get = function (req, res, next) {
 
     help.sendBackJSON(200, res, next)(null, {
       results: roles.results.map(client => {
-        return acl.role.sanitise(client)
+        return acl.role.formatForOutput(client)
       }).sort((a, b) => {
         return a.name < b.name
           ? -1
@@ -214,7 +214,7 @@ Roles.prototype.post = function (req, res, next) {
   }).then(({results}) => {
     help.sendBackJSON(201, res, next)(null, {
       results: [
-        acl.role.sanitise(results[0])
+        acl.role.formatForOutput(results[0])
       ]
     })
   }).catch(this.handleError(res, next))
@@ -329,7 +329,7 @@ Roles.prototype.put = function (req, res, next) {
   }).then(({results}) => {
     help.sendBackJSON(200, res, next)(null, {
       results: [
-        acl.role.sanitise(results[0])
+        acl.role.formatForOutput(results[0])
       ]
     })
   }).catch(this.handleError(res, next))

diff --git a/dadi/lib/model/acl/access.js b/dadi/lib/model/acl/access.js
@@ -1,7 +1,9 @@
-const ACCESS_TYPES = require('./matrix').ACCESS_TYPES
+const ACLMatrix = require('./matrix')
 const clientModel = require('./client')
 const roleModel = require('./role')
 
+const ACCESS_TYPES = ACLMatrix.ACCESS_TYPES
+
 const Access = function () {
   clientModel.setWriteCallback(
     this.write.bind(this)
@@ -152,30 +154,25 @@ Access.prototype.get = function ({clientId = null, accessType = null} = {}, reso
   return this.model.get({
     query
   }).then(({results}) => {
-    if (!resource) {
-      let accessByResource = results.reduce((output, result) => {
-        output[result.resource] = resolveOwnTypes
-          ? this.resolveOwnTypes(result.access, clientId)
-          : result.access
+    if (results.length === 0) {
+      return {}
+    }
 
-        return output
-      }, {})
+    let accessMap = new ACLMatrix()
 
-      return accessByResource
-    }
+    results.forEach(result => {
+      accessMap.set(result.resource, result.access)
+    })
 
-    if (
-      results.length > 0 &&
-      typeof results[0].access === 'object'
-    ) {
-      if (resolveOwnTypes) {
-        return this.resolveOwnTypes(results[0].access, clientId)
-      }
+    if (resource) {
+      let matrix = accessMap.get(resource)
 
-      return results[0].access
+      return resolveOwnTypes
+        ? this.resolveOwnTypes(matrix, clientId)
+        : matrix
     }
 
-    return {}
+    return accessMap.getAll()
   })
 }
 
@@ -389,6 +386,7 @@ Access.prototype.write = function () {
         return this.getRoleChain(client.roles, roleCache).then(chain => {
           // Start with the resources assigned to the client directly.
           let clientResources = client.resources || {}
+          let clientResourcesMap = new ACLMatrix(clientResources)
 
           // Take the resources associated with each role and extend
           // the corresponding entry in `clientResources`.
@@ -397,19 +395,31 @@ Access.prototype.write = function () {
 
             if (!role) return
 
+            let roleMap = new ACLMatrix(role)
+
             Object.keys(role).forEach(resource => {
-              clientResources[resource] = this.combineAccessMatrices(
-                clientResources[resource],
-                role[resource]
+              let combinedMatrix = this.combineAccessMatrices(
+                clientResourcesMap.get(resource, {
+                  parseObjects: true
+                }),
+                roleMap.get(resource, {
+                  parseObjects: true
+                })
               )
+
+              clientResourcesMap.set(resource, combinedMatrix)
             })
           })
 
-          Object.keys(clientResources).forEach(resource => {
+          let combinedResources = clientResourcesMap.getAll({
+            stringifyObjects: true
+          })
+
+          Object.keys(combinedResources).forEach(resource => {
             entries.push({
               client: client.clientId,
               resource,
-              access: clientResources[resource]
+              access: combinedResources[resource]
             })
           })
         })

diff --git a/dadi/lib/model/acl/client.js b/dadi/lib/model/acl/client.js
@@ -98,6 +98,36 @@ Client.prototype.delete = function (clientId) {
   })
 }
 
+/**
+ * Sanitises a client, preparing it for output. It removes all
+ * internal properties as well as sensitive information, such as
+ * the client secret.
+ *
+ * @param  {Object} client
+ * @return {Object}
+ */
+Client.prototype.formatForOutput = function (client) {
+  let sanitisedClient = Object.keys(this.schema).reduce((output, key) => {
+    if (!this.schema[key].hidden) {
+      let value = client[key] || this.schema[key].default
+
+      if (key === 'resources') {
+        let resources = new ACLMatrix(value)
+
+        value = resources.getAll({
+          addFalsyTypes: true
+        })
+      }
+
+      output[key] = value
+    }
+
+    return output
+  }, {})
+
+  return sanitisedClient
+}
+
 /**
  * Retrieves clients by ID.
  *
@@ -179,7 +209,7 @@ Client.prototype.resourceAdd = function (clientId, resource, access) {
       rawOutput: true,
       update: {
         resources: resources.getAll({
-          formatForInput: true
+          stringifyObjects: true
         })
       },
       validate: false
@@ -188,7 +218,7 @@ Client.prototype.resourceAdd = function (clientId, resource, access) {
     return this.broadcastWrite(result)
   }).then(({results}) => {
     return {
-      results: results.map(client => this.sanitise(client))
+      results: results.map(client => this.formatForOutput(client))
     }
   })
 }
@@ -243,7 +273,7 @@ Client.prototype.resourceRemove = function (clientId, resource) {
     return this.broadcastWrite(result)
   }).then(({results}) => {
     return {
-      results: results.map(client => this.sanitise(client))
+      results: results.map(client => this.formatForOutput(client))
     }
   })
 }
@@ -293,7 +323,7 @@ Client.prototype.resourceUpdate = function (clientId, resource, access) {
       rawOutput: true,
       update: {
         resources: resources.getAll({
-          formatForInput: true
+          stringifyObjects: true
         })
       },
       validate: false
@@ -302,7 +332,7 @@ Client.prototype.resourceUpdate = function (clientId, resource, access) {
     return this.broadcastWrite(result)
   }).then(({results}) => {
     return {
-      results: results.map(client => this.sanitise(client))
+      results: results.map(client => this.formatForOutput(client))
     }
   })
 }
@@ -366,7 +396,7 @@ Client.prototype.roleAdd = function (clientId, roles) {
     return this.broadcastWrite(result)
   }).then(({results}) => {
     return {
-      results: results.map(client => this.sanitise(client))
+      results: results.map(client => this.formatForOutput(client))
     }
   })
 }
@@ -426,41 +456,11 @@ Client.prototype.roleRemove = function (clientId, roles) {
   }).then(({results}) => {
     return {
       removed: rolesRemoved,
-      results: results.map(client => this.sanitise(client))
+      results: results.map(client => this.formatForOutput(client))
     }
   })
 }
 
-/**
- * Sanitises a client, preparing it for output. It removes all
- * internal properties as well as sensitive information, such as
- * the client secret.
- *
- * @param  {Object} client
- * @return {Object}
- */
-Client.prototype.sanitise = function (client) {
-  let sanitisedClient = Object.keys(this.schema).reduce((output, key) => {
-    if (!this.schema[key].hidden) {
-      let value = client[key] || this.schema[key].default
-
-      if (key === 'resources') {
-        let resources = new ACLMatrix(value)
-
-        value = resources.getAll({
-          formatForOutput: true
-        })
-      }
-
-      output[key] = value
-    }
-
-    return output
-  }, {})
-
-  return sanitisedClient
-}
-
 Client.prototype.setModel = function (model) {
   this.model = model
 }