Merge branch 'release-4.4.4'

Author: eduardoboucas

CHANGELOG.md

@@ -4,6 +4,18 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).
 
+## [4.4.4] (2018-12-20)
+
+### Changed
+
+- [#524](https://github.com/dadi/api/pull/524): upgrade `@dadi/status` package
+
+### Fixed
+
+- [#521](https://github.com/dadi/api/issues/521): PUT request should respect `fields` parameter in ACL
+- [#522](https://github.com/dadi/api/issues/522): resources API should only list resources which the client has access to
+- [#525](https://github.com/dadi/api/issues/525): required fields should not accept empty strings
+
 ## [4.4.3] (2018-11-29)
 
 ### Fixed

dadi/lib/controller/resources.js

@@ -14,11 +14,26 @@ Resources.prototype.get = function (req, res, next) {
     )
   }
 
-  let resources = acl.getResources()
+  let clientIsAdmin = acl.client.isAdmin(req.dadiApiClient)
 
-  return help.sendBackJSON(200, res, next)(null, {
-    results: Object.keys(resources).map(resource => {
-      return Object.assign({name: resource}, resources[resource])
+  return acl.access.get(req.dadiApiClient).then(access => {
+    return Object.keys(access).filter(resource => {
+      return Object.keys(access[resource]).some(accessType => {
+        return Boolean(access[resource][accessType])
+      })
+    })
+  }).then(allowedResources => {
+    let resources = acl.getResources()
+    let results = Object.keys(resources)
+      .filter(resource => {
+        return clientIsAdmin || allowedResources.includes(resource)
+      })
+      .map(resource => {
+        return Object.assign({name: resource}, resources[resource])
+      })
+
+    return help.sendBackJSON(200, res, next)(null, {
+      results
     })
   })
 }

dadi/lib/model/acl/access.js

@@ -152,7 +152,8 @@ Access.prototype.get = function ({clientId = null, accessType = null} = {}, reso
   }
 
   return this.model.get({
-    query
+    query,
+    rawOutput: true
   }).then(({results}) => {
     if (results.length === 0) {
       return {}

dadi/lib/model/acl/index.js

@@ -5,6 +5,9 @@ const Connection = require('./../connection')
 const Model = require('./../index')
 const role = require('./role')
 
+const ERROR_FORBIDDEN = 'FORBIDDEN'
+const ERROR_UNAUTHORISED = 'UNAUTHORISED'
+
 const ACL = function () {
   this.resources = {}
 
@@ -86,10 +89,10 @@ ACL.prototype.createError = function (client) {
   // authenticated, just not authorised. That is a 403. In any other case,
   // the request is unauthorised, so a 401 is returned.
   if (client && client.clientId && !client.error) {
-    return new Error('FORBIDDEN')
+    return new Error(ERROR_FORBIDDEN)
   }
 
-  return new Error('UNAUTHORISED')
+  return new Error(ERROR_UNAUTHORISED)
 }
 
 ACL.prototype.getResources = function () {
@@ -108,6 +111,8 @@ ACL.prototype.registerResource = function (name, description = null) {
 
 module.exports = new ACL()
 module.exports.ACL = ACL
+module.exports.ERROR_FORBIDDEN = ERROR_FORBIDDEN
+module.exports.ERROR_UNAUTHORISED = ERROR_UNAUTHORISED
 module.exports.access = access
 module.exports.client = client
 module.exports.role = role

dadi/lib/model/collections/create.js

@@ -58,17 +58,34 @@ function create ({
     return document
   })
 
+  let {hooks} = this.settings
+  let originalDocuments = documents
+
+  // If an ACL check is performed, this variable will contain the resulting
+  // access matrix.
+  let aclAccess
+
   return this.validateAccess({
     client,
+    documents,
     type: 'create'
-  }).then(({schema}) => {
+  }).then(({access, documents: newDocuments, fields, schema}) => {
     if (!validate) return
 
+    // Storing the access matrix in a variable that is global to the method.
+    aclAccess = access
+
+    // This is now the filtered documents object, containing only the fields
+    // which the client has access to.
+    documents = newDocuments
+
     return this.validator.validateDocuments({
       documents,
       schema
     }).catch(errors => {
-      let error = this._createValidationError('Validation Failed', errors)
+      let error = this._createValidationError('Validation Failed', errors, {
+        originalDocuments
+      })
 
       return Promise.reject(error)
     })
@@ -113,16 +130,16 @@ function create ({
     return transformQueue
   }).then(documents => {
     // Run any `beforeCreate` hooks.
-    if (this.settings.hooks && this.settings.hooks.beforeCreate) {
+    if (hooks && hooks.beforeCreate) {
       return new Promise((resolve, reject) => {
         let processedDocuments = 0
 
         documents.forEach((doc, docIndex) => {
-          async.reduce(this.settings.hooks.beforeCreate, doc, (current, hookConfig, callback) => {
+          async.reduce(hooks.beforeCreate, doc, (current, hookConfig, callback) => {
             let hook = new Hook(hookConfig, 'beforeCreate')
 
             Promise.resolve(hook.apply(current, this.schema, this.name, req))
-              .then((newDoc) => {
+              .then(newDoc => {
                 callback((newDoc === null) ? {} : null, newDoc)
               })
               .catch(err => {
@@ -151,34 +168,31 @@ function create ({
       schema: this.schema,
       settings: this.settings
     }).then(results => {
-      let returnData = {
-        results
-      }
-
       // Asynchronous search index.
-      this.searchHandler.index(returnData.results)
+      this.searchHandler.index(results)
 
       // Run any `afterCreate` hooks.
-      if (this.settings.hooks && (typeof this.settings.hooks.afterCreate === 'object')) {
-        returnData.results.forEach(document => {
-          this.settings.hooks.afterCreate.forEach((hookConfig, index) => {
-            let hook = new Hook(this.settings.hooks.afterCreate[index], 'afterCreate')
+      if (hooks && Array.isArray(hooks.afterCreate)) {
+        results.forEach(document => {
+          hooks.afterCreate.forEach((hookConfig, index) => {
+            let hook = new Hook(hooks.afterCreate[index], 'afterCreate')
 
             return hook.apply(document, this.schema, this.name)
           })
         })
       }
 
-      // Prepare result set for output.
-      if (!rawOutput) {
-        return this.formatForOutput(
-          returnData.results,
-          {
-            composeOverride: compose
-          }).then(results => ({results}))
+      // If `rawOutput` is truthy, we don't need to worry about formatting
+      // the result set for output. We return it as is.
+      if (rawOutput) {
+        return {results}
       }
 
-      return returnData
+      return this.formatForOutput(results, {
+        access: aclAccess && aclAccess.read,
+        client,
+        composeOverride: compose
+      }).then(results => ({results}))
     })
   })
 }

dadi/lib/model/collections/get.js

@@ -24,18 +24,20 @@ const logger = require('@dadi/logger')
  * Finds documents in the database, running any configured hooks
  * and formatting the result set for final output.
  *
- * @param  {Object} client - client to check permissions for
- * @param  {String} language - ISO code for the language to translate documents to
- * @param  {Object} query - query to match documents against
- * @param  {Object} options
- * @param  {Object} req - request object to pass to hooks
+ * @param  {Object}  client - client to check permissions for
+ * @param  {String}  language - ISO code for the language to translate documents to
+ * @param  {Object}  query - query to match documents against
+ * @param  {Object}  options
+ * @param  {Boolean} rawOutput - whether to bypass formatting routine
+ * @param  {Object}  req - request object to pass to hooks
  * @return {Promise<ResultSet>}
  */
 function get ({
   client,
   language,
   query = {},
   options = {},
+  rawOutput = false,
   req
 }) {
   // Is this a RESTful query by ID?
@@ -112,15 +114,19 @@ function get ({
 
     return response
   }).then(({metadata, results}) => {
-    return this.formatForOutput(
-      results,
-      {
-        client,
-        composeOverride: options.compose,
-        language,
-        urlFields: options.fields
-      }
-    ).then(results => {
+    let formatter = rawOutput
+      ? Promise.resolve(results)
+      : this.formatForOutput(
+          results,
+        {
+          client,
+          composeOverride: options.compose,
+          language,
+          urlFields: options.fields
+        }
+        )
+
+    return formatter.then(results => {
       return {results, metadata}
     })
   })

dadi/lib/model/collections/update.js

@@ -77,12 +77,26 @@ function update ({
   // Get a reference to the documents that will be updated.
   let updatedDocuments = []
 
+  // Removing internal API properties from the update object.
+  if (removeInternalProperties) {
+    update = this.removeInternalProperties(update)
+  }
+
+  let {hooks} = this.settings
+
+  // If an ACL check is performed, this variable will contain the resulting
+  // access matrix.
+  let aclAccess
+
   return this.validateAccess({
     client,
+    documents: update,
     query,
     type: 'update'
-  }).then(({query: aclQuery, schema}) => {
+  }).then(({access, documents: newUpdate, query: aclQuery, schema}) => {
+    aclAccess = access
     query = aclQuery
+    update = newUpdate
 
     // If merging the request query with ACL data resulted in
     // an impossible query, we can simply return an empty result
@@ -92,11 +106,6 @@ function update ({
       return Promise.reject(query)
     }
 
-    // Removing internal API properties from the update object.
-    if (removeInternalProperties) {
-      update = this.removeInternalProperties(update)
-    }
-
     if (!validate) return
 
     // Validating the query.
@@ -160,9 +169,9 @@ function update ({
       update = transformedUpdate
 
       // Run any `beforeUpdate` hooks.
-      if (this.settings.hooks && this.settings.hooks.beforeUpdate) {
+      if (hooks && hooks.beforeUpdate) {
         return new Promise((resolve, reject) => {
-          async.reduce(this.settings.hooks.beforeUpdate, update, (current, hookConfig, callback) => {
+          async.reduce(hooks.beforeUpdate, update, (current, hookConfig, callback) => {
             let hook = new Hook(hookConfig, 'beforeUpdate')
 
             Promise.resolve(hook.apply(current, updatedDocuments, this.schema, this.name, req))
@@ -220,20 +229,20 @@ function update ({
       }
 
       return this.find({
-        query: updatedDocumentsQuery,
         options: {
           compose: true
-        }
+        },
+        query: updatedDocumentsQuery
       })
     }).then(data => {
       if (data.results.length === 0) {
         return data
       }
 
       // Run any `afterUpdate` hooks.
-      if (this.settings.hooks && (typeof this.settings.hooks.afterUpdate === 'object')) {
-        this.settings.hooks.afterUpdate.forEach((hookConfig, index) => {
-          let hook = new Hook(this.settings.hooks.afterUpdate[index], 'afterUpdate')
+      if (hooks && Array.isArray(hooks.afterUpdate)) {
+        hooks.afterUpdate.forEach((hookConfig, index) => {
+          let hook = new Hook(hooks.afterUpdate[index], 'afterUpdate')
 
           return hook.apply(data.results, this.schema, this.name)
         })
@@ -245,6 +254,7 @@ function update ({
       // Format result set for output.
       if (!rawOutput) {
         return this.formatForOutput(data.results, {
+          access: aclAccess && aclAccess.read,
           client,
           composeOverride: compose
         }).then(results => {