From 71f1259e72f47455061077174784b5161af36463 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Fri, 26 Oct 2018 09:18:25 -0700 Subject: [PATCH] Prevent hidden/protected props from being searched - report errors if where contains hidden properties - report errors if incldue scope contains hidden or protected properties --- lib/dao.js | 27 +++++++ lib/include.js | 6 ++ lib/utils.js | 37 +++++++++- test/model-definition.test.js | 135 +++++++++++++++++++++++++++++++++- 4 files changed, 199 insertions(+), 6 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index 8a232c80..1faff391 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -1643,6 +1643,26 @@ function coerceArray(val) { return arrayVal; } +DataAccessObject._getHiddenProperties = function() { + var settings = this.definition.settings || {}; + var result = settings.hiddenProperties || settings.hidden; + if (typeof result === 'string') { + result = [result]; + } + result = result || []; + return result; +}; + +DataAccessObject._getProtectedProperties = function() { + var settings = this.definition.settings || {}; + var result = settings.protectedProperties || settings.protected; + if (typeof result === 'string') { + result = [result]; + } + result = result || []; + return result; +}; + /* * Coerce values based the property types * @param {Object} where The where clause @@ -1659,6 +1679,13 @@ DataAccessObject._coerce = function(where, options) { options = options || {}; + // Check violation of keys + var prohibitedKeys = this._getHiddenProperties(); + if (options.excludeProtectedProperties) { + prohibitedKeys = prohibitedKeys.concat(this._getProtectedProperties()); + } + utils.validateKeys(where, prohibitedKeys); + var err; if (typeof where !== 'object' || Array.isArray(where)) { err = new Error(g.f('The where clause %j is not an {{object}}', where)); diff --git a/lib/include.js b/lib/include.js index 195c5e3a..18e94f37 100644 --- a/lib/include.js +++ b/lib/include.js @@ -204,6 +204,12 @@ Inclusion.include = function(objects, include, options, cb) { * @param cb */ function findWithForeignKeysByPage(model, filter, fkName, pageSize, options, cb) { + try { + model._coerce(filter.where, {excludeProtectedProperties: true}); + } catch (e) { + return cb(e); + } + var foreignKeys = []; if (filter.where[fkName]) { foreignKeys = filter.where[fkName].inq; diff --git a/lib/utils.js b/lib/utils.js index dcd9760b..bd1a75f1 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -26,6 +26,7 @@ exports.findIndexOf = findIndexOf; exports.collectTargetIds = collectTargetIds; exports.idName = idName; exports.rankArrayElements = rankArrayElements; +exports.validateKeys = validateKeys; var g = require('strong-globalize')(); var traverse = require('traverse'); @@ -302,7 +303,7 @@ function selectFields(fields) { } /** - * Remove undefined values from the queury object + * Remove undefined values from the query object * @param query * @param handleUndefined {String} either "nullify", "throw" or "ignore" (default: "ignore") * @returns {exports.map|*} @@ -339,6 +340,38 @@ function removeUndefined(query, handleUndefined) { }); } +/** + * Check the where clause to report prohibited keys + * @param {object} where Where object + * @param {string[]} [prohibitedKeys] An array of prohibited keys + */ +function validateKeys(where, prohibitedKeys) { + if (!prohibitedKeys || prohibitedKeys.length === 0) return where; + if (typeof where !== 'object' || where === null) { + return where; + } + prohibitedKeys = prohibitedKeys || []; + const offendingKeys = []; + // WARNING: [rfeng] Use traverse.map() will cause mongodb to produce invalid BSON + // as traverse doesn't transform the ObjectId correctly. Instead we have to use + // traverse(where).forEach(...) + const result = traverse(where).forEach(function(x) { + if (prohibitedKeys.indexOf(this.key) !== -1) { + offendingKeys.push(this.key); + } + return x; + }); + if (offendingKeys.length) { + const msg = 'Properties "' + offendingKeys.join(', ') + '" are not allowed in query'; + const err = new Error(msg); + err.code = 'PROPERTY_NOT_ALLOWED_IN_QUERY'; + err.statusCode = 400; + err.details = {properties: offendingKeys, where: where}; + throw err; + } + return result; +} + var url = require('url'); var qs = require('qs'); @@ -505,7 +538,7 @@ function defineCachedRelations(obj) { /** * Check if the argument is plain object - * @param {*) obj The obj value + * @param {*} obj The obj value * @returns {boolean} */ function isPlainObject(obj) { diff --git a/test/model-definition.test.js b/test/model-definition.test.js index bf7a0e97..b0e025de 100644 --- a/test/model-definition.test.js +++ b/test/model-definition.test.js @@ -258,7 +258,6 @@ describe('ModelDefinition class', function() { }); it('should serialize protected properties into JSON', function() { - var modelBuilder = memory.modelBuilder; var ProtectedModel = memory.createModel('protected', {}, { protected: ['protectedProperty'], }); @@ -272,7 +271,6 @@ describe('ModelDefinition class', function() { }); it('should not serialize protected properties of nested models into JSON', function(done) { - var modelBuilder = memory.modelBuilder; var Parent = memory.createModel('parent'); var Child = memory.createModel('child', {}, {protected: ['protectedProperty']}); Parent.hasMany(Child); @@ -295,7 +293,6 @@ describe('ModelDefinition class', function() { }); it('should not serialize hidden properties into JSON', function() { - var modelBuilder = memory.modelBuilder; var HiddenModel = memory.createModel('hidden', {}, { hidden: ['secret'], }); @@ -312,7 +309,6 @@ describe('ModelDefinition class', function() { }); it('should not serialize hidden properties of nested models into JSON', function(done) { - var modelBuilder = memory.modelBuilder; var Parent = memory.createModel('parent'); var Child = memory.createModel('child', {}, {hidden: ['secret']}); Parent.hasMany(Child); @@ -334,6 +330,137 @@ describe('ModelDefinition class', function() { }); }); + function assertPropertyNotAllowed(err) { + should.exist(err); + err.message.should.match(/Properties "secret" are not allowed in query/); + err.code.should.equal('PROPERTY_NOT_ALLOWED_IN_QUERY'); + err.statusCode.should.equal(400); + err.details.should.have.property('properties'); + err.details.should.have.property('where'); + } + + it('should report errors if a hidden property is used in where', function(done) { + var Child = memory.createModel('child', {}, {hidden: ['secret']}); + Child.create({ + name: 'child', + secret: 'secret', + }, function(err) { + if (err) return done(err); + Child.find({ + where: {secret: 'secret'}, + }, function(err) { + assertPropertyNotAllowed(err); + done(); + }); + }); + }); + + it('should report errors if a hidden property is used in where.and', function(done) { + var Child = memory.createModel('child', {}, {hidden: ['secret']}); + Child.create({ + name: 'child', + secret: 'secret', + }, function(err) { + if (err) return done(err); + Child.find({ + where: {and: [{secret: 'secret'}]}, + }, function(err) { + assertPropertyNotAllowed(err); + done(); + }); + }); + }); + + it('should report errors if a protected property is used in include scope', function(done) { + var Parent = memory.createModel('parent'); + var Child = memory.createModel('child', {}, {protected: ['secret']}); + Parent.hasMany(Child); + Parent.create({ + name: 'parent', + }, function(err, parent) { + if (err) return done(err); + parent.children.create({ + name: 'child', + secret: 'secret', + }, function(err) { + if (err) return done(err); + Parent.find({ + include: { + relation: 'children', + scope: { + where: { + secret: 'x', + }, + }, + }, + }, function(err) { + assertPropertyNotAllowed(err); + done(); + }); + }); + }); + }); + + it('should report errors if a protected property is used in include scope.where.and', function(done) { + var Parent = memory.createModel('parent'); + var Child = memory.createModel('child', {}, {protected: ['secret']}); + Parent.hasMany(Child); + Parent.create({ + name: 'parent', + }, function(err, parent) { + if (err) return done(err); + parent.children.create({ + name: 'child', + secret: 'secret', + }, function(err) { + if (err) return done(err); + Parent.find({ + include: { + relation: 'children', + scope: { + where: { + and: [{secret: 'x'}], + }, + }, + }, + }, function(err) { + assertPropertyNotAllowed(err); + done(); + }); + }); + }); + }); + + it('should report errors if a hidden property is used in include scope', function(done) { + var Parent = memory.createModel('parent'); + var Child = memory.createModel('child', {}, {hidden: ['secret']}); + Parent.hasMany(Child); + Parent.create({ + name: 'parent', + }, function(err, parent) { + if (err) return done(err); + parent.children.create({ + name: 'child', + secret: 'secret', + }, function(err) { + if (err) return done(err); + Parent.find({ + include: { + relation: 'children', + scope: { + where: { + secret: 'x', + }, + }, + }, + }, function(err) { + assertPropertyNotAllowed(err); + done(); + }); + }); + }); + }); + it('should throw error for property names containing dot', function() { (function() { memory.createModel('Dotted', {'dot.name': String}); }) .should