Improve hidden/protected property checks

- Fixes #1645
  - do not apply hidden property check for data
- Fixes #1648
  - allow prohibitHiddenPropertiesInQuery to be set in model/ds

lib/dao.js

@@ -406,7 +406,7 @@ DataAccessObject.create = function(data, options, cb) {
     obj.trigger('create', function(createDone) {
       obj.trigger('save', function(saveDone) {
         var _idName = idName(Model);
-        var val = Model._sanitize(obj.toObject(true));
+        var val = Model._sanitizeData(obj.toObject(true));
         function createCallback(err, id, rev) {
           if (id) {
             obj.__data[_idName] = id;
@@ -635,7 +635,7 @@ DataAccessObject.upsert = function(data, options, cb) {
         }
 
         function callConnector() {
-          update = Model._sanitize(update);
+          update = Model._sanitizeData(update);
           context = {
             Model: Model,
             where: ctx.where,
@@ -803,12 +803,9 @@ DataAccessObject.upsertWithWhere = function(where, data, options, cb) {
 
         function callConnector() {
           try {
-            // Support an optional where object
+            ctx.where = Model._sanitizeQuery(ctx.where);
-            var normalizeUndefinedInQuery = Model._getSetting('normalizeUndefinedInQuery');
-            // alter configuration of how sanitizeQuery handles undefined values
-            ctx.where = Model._sanitize(ctx.where, {normalizeUndefinedInQuery: normalizeUndefinedInQuery});
             ctx.where = Model._coerce(ctx.where, options);
-            update = Model._sanitize(update);
+            update = Model._sanitizeData(update);
             update = Model._coerce(update, options);
           } catch (err) {
             return process.nextTick(function() {
@@ -989,7 +986,7 @@ DataAccessObject.replaceOrCreate = function replaceOrCreate(data, options, cb) {
           }, update, options);
 
           function callConnector() {
-            update = Model._sanitize(update);
+            update = Model._sanitizeData(update);
             context = {
               Model: Model,
               where: where,
@@ -1170,7 +1167,7 @@ DataAccessObject.findOrCreate = function findOrCreate(query, data, options, cb)
       });
     }
 
-    data = Model._sanitize(data);
+    data = Model._sanitizeData(data);
     var context = {
       Model: Model,
       where: query.where,
@@ -1456,24 +1453,28 @@ DataAccessObject.all = function() {
 };
 
 /**
- * Get settings via hiarchical determiniation
+ * Get settings via hierarchical determination
  *
  * @param {String} key The setting key
  */
 DataAccessObject._getSetting = function(key) {
   // Check for settings in model
   var m = this.definition;
-  if (m && m.settings && m.settings[key]) {
+  if (m && m.settings) {
-    return m.settings[key];
+    var val = m.settings[key];
+    if (val !== undefined) {
+      return m.settings[key];
+    }
+    // Fall back to datasource level
   }
 
   // Check for settings in connector
   var ds = this.getDataSource();
-  if (ds && ds.settings && ds.settings[key]) {
+  if (ds && ds.settings) {
     return ds.settings[key];
   }
 
-  return;
+  return undefined;
 };
 
 var operators = {
@@ -1579,9 +1580,7 @@ DataAccessObject._normalize = function(filter, options) {
       Object.keys(this.definition.properties), this.settings.strict);
   }
 
-  var normalizeUndefinedInQuery = this._getSetting('normalizeUndefinedInQuery');
+  filter = this._sanitizeQuery(filter);
-  // alter configuration of how sanitizeQuery handles undefined values
-  filter = this._sanitize(filter, {normalizeUndefinedInQuery: normalizeUndefinedInQuery});
   this._coerce(filter.where, options);
   return filter;
 };
@@ -1644,9 +1643,7 @@ function coerceArray(val) {
   return arrayVal;
 }
 
-DataAccessObject._getHiddenProperties = function() {
+function _normalizeAsArray(result) {
-  var settings = this.definition.settings || {};
-  var result = settings.hiddenProperties || settings.hidden || [];
   if (typeof result === 'string') {
     result = [result];
   }
@@ -1655,35 +1652,59 @@ DataAccessObject._getHiddenProperties = function() {
   } else {
     // See https://github.com/strongloop/loopback-datasource-juggler/issues/1646
     // `ModelBaseClass` normalize the properties to an object such as `{secret: true}`
-    return Object.keys(result);
+    var keys = [];
+    for (var k in result) {
+      if (result[k]) keys.push(k);
+    }
+    return keys;
   }
+}
+
+DataAccessObject._getHiddenProperties = function() {
+  var settings = this.definition.settings || {};
+  var result = settings.hiddenProperties || settings.hidden || [];
+  return _normalizeAsArray(result);
 };
 
 DataAccessObject._getProtectedProperties = function() {
   var settings = this.definition.settings || {};
   var result = settings.protectedProperties || settings.protected || [];
-  if (typeof result === 'string') {
+  return _normalizeAsArray(result);
-    result = [result];
-  }
-  if (Array.isArray(result)) {
-    return result;
-  } else {
-    // See https://github.com/strongloop/loopback-datasource-juggler/issues/1646
-    // `ModelBaseClass` normalize the properties to an object such as `{secret: true}`
-    return Object.keys(result);
-  }
 };
 
-DataAccessObject._sanitize = function(query, options) {
+DataAccessObject._sanitizeQuery = function(query, options) {
   options = options || {};
 
+  // Get settings to normalize `undefined` values
+  var normalizeUndefinedInQuery = this._getSetting('normalizeUndefinedInQuery');
+  // Get setting to prohibit hidden/protected properties in query
+  var prohibitHiddenPropertiesInQuery = this._getSetting('prohibitHiddenPropertiesInQuery');
+  if (prohibitHiddenPropertiesInQuery == null) {
+    // By default, hidden properties are prohibited in query
+    prohibitHiddenPropertiesInQuery = true;
+  }
+
+  if (!prohibitHiddenPropertiesInQuery)
+    console.log(prohibitHiddenPropertiesInQuery);
+
+  var prohibitedKeys = [];
   // Check violation of keys
-  var prohibitedKeys = this._getHiddenProperties();
+  if (prohibitHiddenPropertiesInQuery) {
-  if (options.excludeProtectedProperties) {
+    prohibitedKeys = this._getHiddenProperties();
-    prohibitedKeys = prohibitedKeys.concat(this._getProtectedProperties());
+    if (options.prohibitProtectedPropertiesInQuery) {
+      prohibitedKeys = prohibitedKeys.concat(this._getProtectedProperties());
+    }
   }
   return sanitizeQueryOrData(query,
-    Object.assign({prohibitedKeys: prohibitedKeys}, options));
+    Object.assign({
+      prohibitedKeys: prohibitedKeys,
+      normalizeUndefinedInQuery: normalizeUndefinedInQuery,
+    }, options));
+};
+
+DataAccessObject._sanitizeData = function(data, options) {
+  options = options || {};
+  return sanitizeQueryOrData(data, options);
 };
 
 /*
@@ -2335,9 +2356,8 @@ DataAccessObject.destroyAll = function destroyAll(where, options, cb) {
     } else {
       try {
         // Support an optional where object
-        var normalizeUndefinedInQuery = Model._getSetting('normalizeUndefinedInQuery');
         // alter configuration of how sanitizeQuery handles undefined values
-        where = Model._sanitize(where, {normalizeUndefinedInQuery: normalizeUndefinedInQuery});
+        where = Model._sanitizeQuery(where);
         where = Model._coerce(where, options);
       } catch (err) {
         return process.nextTick(function() {
@@ -2490,9 +2510,8 @@ DataAccessObject.count = function(where, options, cb) {
   where = query.where;
 
   try {
-    var normalizeUndefinedInQuery = Model._getSetting('normalizeUndefinedInQuery');
     // alter configuration of how sanitizeQuery handles undefined values
-    where = Model._sanitize(where, {normalizeUndefinedInQuery: normalizeUndefinedInQuery});
+    where = Model._sanitizeQuery(where);
     where = this._coerce(where, options);
   } catch (err) {
     process.nextTick(function() {
@@ -2598,7 +2617,7 @@ DataAccessObject.prototype.save = function(options, cb) {
     function save() {
       inst.trigger('save', function(saveDone) {
         inst.trigger('update', function(updateDone) {
-          data = Model._sanitize(data);
+          data = Model._sanitizeData(data);
           function saveCallback(err, unusedData, result) {
             if (err) {
               return cb(err, inst);
@@ -2784,12 +2803,10 @@ DataAccessObject.updateAll = function(where, data, options, cb) {
 
   function doUpdate(where, data) {
     try {
-      // Support an optional where object
-      var normalizeUndefinedInQuery = Model._getSetting('normalizeUndefinedInQuery');
       // alter configuration of how sanitizeQuery handles undefined values
-      where = Model._sanitize(where, {normalizeUndefinedInQuery: normalizeUndefinedInQuery});
+      where = Model._sanitizeQuery(where);
       where = Model._coerce(where, options);
-      data = Model._sanitize(data);
+      data = Model._sanitizeData(data);
       data = Model._coerce(data, options);
     } catch (err) {
       return process.nextTick(function() {
@@ -3122,7 +3139,7 @@ DataAccessObject.replaceById = function(id, data, options, cb) {
 
     function validateAndCallConnector(err, data) {
       if (err) return cb(err);
-      data = Model._sanitize(data);
+      data = Model._sanitizeData(data);
       // update instance's properties
       inst.setAttributes(data);
 
@@ -3274,7 +3291,7 @@ function(data, options, cb) {
   if (data instanceof Model) {
     data = data.toObject(false);
   }
-  data = Model._sanitize(data);
+  data = Model._sanitizeData(data);
 
   // Make sure id(s) cannot be changed
   var idNames = Model.definition.idNames();
@@ -3347,7 +3364,7 @@ function(data, options, cb) {
           inst.trigger('update', function(done) {
             copyData(data, inst);
             var typedData = convertSubsetOfPropertiesByType(inst, data);
-            context.data = Model._sanitize(typedData);
+            context.data = Model._sanitizeData(typedData);
 
             // Depending on the connector, the database response can
             // contain information about the updated record(s), but also

lib/include.js

@@ -205,7 +205,7 @@ Inclusion.include = function(objects, include, options, cb) {
    */
   function findWithForeignKeysByPage(model, filter, fkName, pageSize, options, cb) {
     try {
-      model._sanitize(filter.where, {excludeProtectedProperties: true});
+      model._sanitizeQuery(filter.where, {prohibitProtectedPropertiesInQuery: true});
       model._coerce(filter.where);
     } catch (e) {
       return cb(e);

test/model-definition.test.js

@@ -353,6 +353,24 @@ describe('ModelDefinition class', function() {
           where: {and: [{secret: 'guess'}]},
         }).then(assertHiddenPropertyIsIgnored);
       });
+
+      it('should be allowed for update', function() {
+        return Child.update({name: 'childA'}, {secret: 'new-secret'}).then(
+          function(result) {
+            result.count.should.equal(1);
+          }
+        );
+      });
+
+      it('should be allowed if prohibitHiddenPropertiesInQuery is `false`', function() {
+        Child.definition.settings.prohibitHiddenPropertiesInQuery = false;
+        return Child.find({
+          where: {secret: 'guess'},
+        }).then(function(children) {
+          children.length.should.equal(1);
+          children[0].secret.should.equal('guess');
+        });
+      });
     });
 
     describe('with hidden object', function() {