diff --git a/lib/dao.js b/lib/dao.js index 8a232c80..96d0692a 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -26,7 +26,7 @@ var geo = require('./geo'); var Memory = require('./connectors/memory').Memory; var utils = require('./utils'); var fieldsToArray = utils.fieldsToArray; -var removeUndefined = utils.removeUndefined; +var sanitizeQueryOrData = utils.sanitizeQuery; var setScopeValuesFromWhere = utils.setScopeValuesFromWhere; var idEquals = utils.idEquals; var mergeQuery = utils.mergeQuery; @@ -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 = removeUndefined(obj.toObject(true)); + var val = Model._sanitize(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 = removeUndefined(update); + update = Model._sanitize(update); context = { Model: Model, where: ctx.where, @@ -804,11 +804,11 @@ DataAccessObject.upsertWithWhere = function(where, data, options, cb) { function callConnector() { try { // Support an optional where object - var handleUndefined = Model._getSetting('normalizeUndefinedInQuery'); - // alter configuration of how removeUndefined handles undefined values - ctx.where = removeUndefined(ctx.where, handleUndefined); + 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 = removeUndefined(update); + update = Model._sanitize(update); update = Model._coerce(update, options); } catch (err) { return process.nextTick(function() { @@ -989,7 +989,7 @@ DataAccessObject.replaceOrCreate = function replaceOrCreate(data, options, cb) { }, update, options); function callConnector() { - update = removeUndefined(update); + update = Model._sanitize(update); context = { Model: Model, where: where, @@ -1170,7 +1170,7 @@ DataAccessObject.findOrCreate = function findOrCreate(query, data, options, cb) }); } - data = removeUndefined(data); + data = Model._sanitize(data); var context = { Model: Model, where: query.where, @@ -1477,6 +1477,7 @@ DataAccessObject._getSetting = function(key) { }; var operators = { + eq: '=', gt: '>', gte: '>=', lt: '<', @@ -1578,9 +1579,9 @@ DataAccessObject._normalize = function(filter, options) { Object.keys(this.definition.properties), this.settings.strict); } - var handleUndefined = this._getSetting('normalizeUndefinedInQuery'); - // alter configuration of how removeUndefined handles undefined values - filter = removeUndefined(filter, handleUndefined); + var normalizeUndefinedInQuery = this._getSetting('normalizeUndefinedInQuery'); + // alter configuration of how sanitizeQuery handles undefined values + filter = this._sanitize(filter, {normalizeUndefinedInQuery: normalizeUndefinedInQuery}); this._coerce(filter.where, options); return filter; }; @@ -1643,6 +1644,38 @@ 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; +}; + +DataAccessObject._sanitize = function(query, options) { + options = options || {}; + + // Check violation of keys + var prohibitedKeys = this._getHiddenProperties(); + if (options.excludeProtectedProperties) { + prohibitedKeys = prohibitedKeys.concat(this._getProtectedProperties()); + } + return sanitizeQueryOrData(query, + Object.assign({prohibitedKeys: prohibitedKeys}, options)); +}; + /* * Coerce values based the property types * @param {Object} where The where clause @@ -1653,10 +1686,9 @@ function coerceArray(val) { */ DataAccessObject._coerce = function(where, options) { var self = this; - if (!where) { + if (where == null) { return where; } - options = options || {}; var err; @@ -1791,6 +1823,7 @@ DataAccessObject._coerce = function(where, options) { // NOOP when not coercable into an array. } + var allowExtendedOperators = self._allowExtendedOperators(options); // Coerce the array items if (Array.isArray(val)) { for (var i = 0; i < val.length; i++) { @@ -1802,7 +1835,6 @@ DataAccessObject._coerce = function(where, options) { } } else { if (val != null) { - var allowExtendedOperators = self._allowExtendedOperators(options); if (operator === null && val instanceof RegExp) { // Normalize {name: /A/} to {name: {regexp: /A/}} operator = 'regexp'; @@ -1814,12 +1846,28 @@ DataAccessObject._coerce = function(where, options) { } else if (allowExtendedOperators && typeof val === 'object') { // Do not coerce object values when extended operators are allowed } else { + if (!allowExtendedOperators) { + var extendedOperators = Object.keys(val).filter(function(k) { + return k[0] === '$'; + }); + if (extendedOperators.length) { + const msg = g.f('Operators "' + extendedOperators.join(', ') + '" are not allowed in query'); + const err = new Error(msg); + err.code = 'OPERATOR_NOT_ALLOWED_IN_QUERY'; + err.statusCode = 400; + err.details = { + operators: extendedOperators, + where: where, + }; + throw err; + } + } val = DataType(val); } } } // Rebuild {property: {operator: value}} - if (operator) { + if (operator && operator !== 'eq') { var value = {}; value[operator] = val; if (exp.options) { @@ -2277,9 +2325,9 @@ DataAccessObject.destroyAll = function destroyAll(where, options, cb) { } else { try { // Support an optional where object - var handleUndefined = Model._getSetting('normalizeUndefinedInQuery'); - // alter configuration of how removeUndefined handles undefined values - where = removeUndefined(where, handleUndefined); + var normalizeUndefinedInQuery = Model._getSetting('normalizeUndefinedInQuery'); + // alter configuration of how sanitizeQuery handles undefined values + where = Model._sanitize(where, {normalizeUndefinedInQuery: normalizeUndefinedInQuery}); where = Model._coerce(where, options); } catch (err) { return process.nextTick(function() { @@ -2432,9 +2480,9 @@ DataAccessObject.count = function(where, options, cb) { where = query.where; try { - var handleUndefined = Model._getSetting('normalizeUndefinedInQuery'); - // alter configuration of how removeUndefined handles undefined values - where = removeUndefined(where, handleUndefined); + var normalizeUndefinedInQuery = Model._getSetting('normalizeUndefinedInQuery'); + // alter configuration of how sanitizeQuery handles undefined values + where = Model._sanitize(where, {normalizeUndefinedInQuery: normalizeUndefinedInQuery}); where = this._coerce(where, options); } catch (err) { process.nextTick(function() { @@ -2540,7 +2588,7 @@ DataAccessObject.prototype.save = function(options, cb) { function save() { inst.trigger('save', function(saveDone) { inst.trigger('update', function(updateDone) { - data = removeUndefined(data); + data = Model._sanitize(data); function saveCallback(err, unusedData, result) { if (err) { return cb(err, inst); @@ -2727,11 +2775,11 @@ DataAccessObject.updateAll = function(where, data, options, cb) { function doUpdate(where, data) { try { // Support an optional where object - var handleUndefined = Model._getSetting('normalizeUndefinedInQuery'); - // alter configuration of how removeUndefined handles undefined values - where = removeUndefined(where, handleUndefined); + var normalizeUndefinedInQuery = Model._getSetting('normalizeUndefinedInQuery'); + // alter configuration of how sanitizeQuery handles undefined values + where = Model._sanitize(where, {normalizeUndefinedInQuery: normalizeUndefinedInQuery}); where = Model._coerce(where, options); - data = removeUndefined(data); + data = Model._sanitize(data); data = Model._coerce(data, options); } catch (err) { return process.nextTick(function() { @@ -3064,7 +3112,7 @@ DataAccessObject.replaceById = function(id, data, options, cb) { function validateAndCallConnector(err, data) { if (err) return cb(err); - data = removeUndefined(data); + data = Model._sanitize(data); // update instance's properties inst.setAttributes(data); @@ -3216,7 +3264,7 @@ function(data, options, cb) { if (data instanceof Model) { data = data.toObject(false); } - data = removeUndefined(data); + data = Model._sanitize(data); // Make sure id(s) cannot be changed var idNames = Model.definition.idNames(); @@ -3289,7 +3337,7 @@ function(data, options, cb) { inst.trigger('update', function(done) { copyData(data, inst); var typedData = convertSubsetOfPropertiesByType(inst, data); - context.data = removeUndefined(typedData); + context.data = Model._sanitize(typedData); // Depending on the connector, the database response can // contain information about the updated record(s), but also diff --git a/lib/include.js b/lib/include.js index 195c5e3a..04530236 100644 --- a/lib/include.js +++ b/lib/include.js @@ -204,6 +204,13 @@ Inclusion.include = function(objects, include, options, cb) { * @param cb */ function findWithForeignKeysByPage(model, filter, fkName, pageSize, options, cb) { + try { + model._sanitize(filter.where, {excludeProtectedProperties: true}); + model._coerce(filter.where); + } catch (e) { + return cb(e); + } + var foreignKeys = []; if (filter.where[fkName]) { foreignKeys = filter.where[fkName].inq; diff --git a/lib/include_utils.js b/lib/include_utils.js index 70d70597..e08d722f 100644 --- a/lib/include_utils.js +++ b/lib/include_utils.js @@ -5,6 +5,8 @@ 'use strict'; +var g = require('strong-globalize')(); + module.exports.buildOneToOneIdentityMapWithOrigKeys = buildOneToOneIdentityMapWithOrigKeys; module.exports.buildOneToManyIdentityMapWithOrigKeys = buildOneToManyIdentityMapWithOrigKeys; module.exports.join = join; @@ -15,7 +17,7 @@ const util = require('util'); function getId(obj, idName) { var id = obj && obj[idName]; if (id == null) { - const msg = util.format('ID property "%s" is missing for included item: %j. ' + + const msg = g.f('ID property "%s" is missing for included item: %j. ' + 'Please make sure `fields` include "%s" if it\'s present in the `filter`', idName, obj, idName); const err = new Error(msg); diff --git a/lib/utils.js b/lib/utils.js index fe8e0e42..7620dba8 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -7,7 +7,7 @@ exports.safeRequire = safeRequire; exports.fieldsToArray = fieldsToArray; exports.selectFields = selectFields; -exports.removeUndefined = removeUndefined; +exports.sanitizeQuery = sanitizeQuery; exports.parseSettings = parseSettings; exports.mergeSettings = exports.deepMerge = deepMerge; exports.deepMergeProperty = deepMergeProperty; @@ -31,6 +31,7 @@ exports.idsHaveDuplicates = idsHaveDuplicates; var g = require('strong-globalize')(); var traverse = require('traverse'); var assert = require('assert'); +var debug = require('debug')('loopback:juggler:utils'); function safeRequire(module) { try { @@ -302,26 +303,60 @@ function selectFields(fields) { } /** - * Remove undefined values from the queury object - * @param query - * @param handleUndefined {String} either "nullify", "throw" or "ignore" (default: "ignore") - * @returns {exports.map|*} + * Sanitize the query object + * @param query {object} The query object + * @param options + * @property normalizeUndefinedInQuery {String} either "nullify", "throw" or "ignore" (default: "ignore") + * @property prohibitedKeys {String[]} An array of prohibited keys to be removed + * @returns {*} */ -function removeUndefined(query, handleUndefined) { +function sanitizeQuery(query, options) { + debug('Sanitizing query object: %j', query); if (typeof query !== 'object' || query === null) { return query; } + options = options || {}; + if (typeof options === 'string') { + // Keep it backward compatible + options = {normalizeUndefinedInQuery: options}; + } + const prohibitedKeys = options.prohibitedKeys; + const offendingKeys = []; + const normalizeUndefinedInQuery = options.normalizeUndefinedInQuery; + const maxDepth = options.maxDepth || 10; // WARNING: [rfeng] Use map() will cause mongodb to produce invalid BSON // as traverse doesn't transform the ObjectId correctly - return traverse(query).forEach(function(x) { + const result = traverse(query).forEach(function(x) { + /** + * Security risk if the client passes in a very deep where object + */ + if (this.level > maxDepth || this.circular) { + const msg = g.f('The query object is too deep or circular'); + const err = new Error(msg); + err.statusCode = 400; + err.code = 'WHERE_OBJECT_TOO_DEEP'; + throw err; + } + /** + * Make sure prohibited keys are removed from the query to prevent + * sensitive values from being guessed + */ + if (prohibitedKeys && prohibitedKeys.indexOf(this.key) !== -1) { + offendingKeys.push(this.key); + this.remove(); + return; + } + + /** + * Handle undefined values + */ if (x === undefined) { - switch (handleUndefined) { + switch (normalizeUndefinedInQuery) { case 'nullify': this.update(null); break; case 'throw': throw new Error(g.f('Unexpected `undefined` in query')); - break; case 'ignore': default: this.remove(); @@ -337,10 +372,20 @@ function removeUndefined(query, handleUndefined) { return x; }); + + if (offendingKeys.length) { + console.error( + g.f( + 'Potential security alert: hidden/protected properties %j are used in query.', + offendingKeys + ) + ); + } + return result; } -var url = require('url'); -var qs = require('qs'); +const url = require('url'); +const qs = require('qs'); /** * Parse a URL into a settings object @@ -505,7 +550,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/allow-extended-operators.test.js b/test/allow-extended-operators.test.js index 359b73f3..19331dd3 100644 --- a/test/allow-extended-operators.test.js +++ b/test/allow-extended-operators.test.js @@ -67,50 +67,59 @@ describe('allowExtendedOperators', () => { } } + function assertOperatorNotAllowed(err) { + should.exist(err); + err.message.should.match(/Operators "\$exists" are not allowed in query/); + err.code.should.equal('OPERATOR_NOT_ALLOWED_IN_QUERY'); + err.statusCode.should.equal(400); + err.details.should.have.property('operators'); + err.details.should.have.property('where'); + } + describe('dataSource.settings.allowExtendedOperators', () => { context('DAO.find()', () => { - it('converts extended operators to string value by default', () => { + it('reports invalid operator by default', () => { const TestModel = createTestModel(); - return TestModel.find(extendedQuery()).then((results) => { - should(results[0].value).eql('[object Object]'); + return TestModel.find(extendedQuery()).catch(err => { + assertOperatorNotAllowed(err); }); }); it('preserves extended operators with allowExtendedOperators set', () => { const TestModel = createTestModel({allowExtendedOperators: true}); - return TestModel.find(extendedQuery()).then((results) => { + return TestModel.find(extendedQuery()).then(results => { should(results[0].value).eql({$exists: true}); }); }); it('`Model.settings.allowExtendedOperators` override data source settings - ' + - 'converts extended operators', () => { + 'reports invalid operator', () => { const TestModel = createTestModel({allowExtendedOperators: true}, {allowExtendedOperators: false}); - return TestModel.find(extendedQuery()).then((results) => { - should(results[0].value).eql('[object Object]'); + return TestModel.find(extendedQuery()).catch(err => { + assertOperatorNotAllowed(err); }); }); it('`Model.settings.allowExtendedOperators` override data source settings - ' + 'preserves extended operators', () => { const TestModel = createTestModel({allowExtendedOperators: false}, {allowExtendedOperators: true}); - return TestModel.find(extendedQuery()).then((results) => { + return TestModel.find(extendedQuery()).then(results => { should(results[0].value).eql({$exists: true}); }); }); it('`options.allowExtendedOperators` override data source settings - ' + - 'converts extended operators', () => { + 'reports invalid operator', () => { const TestModel = createTestModel({allowExtendedOperators: true}); - return TestModel.find(extendedQuery(), {allowExtendedOperators: false}).then((results) => { - should(results[0].value).eql('[object Object]'); + return TestModel.find(extendedQuery(), {allowExtendedOperators: false}).catch(err => { + assertOperatorNotAllowed(err); }); }); it('`options.allowExtendedOperators` override data source settings - ' + 'preserves extended operators', () => { const TestModel = createTestModel({allowExtendedOperators: false}); - return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then((results) => { + return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then(results => { should(results[0].value).eql({$exists: true}); }); }); @@ -168,37 +177,37 @@ describe('allowExtendedOperators', () => { context('DAO.find()', () => { it('preserves extended operators with allowExtendedOperators set', () => { const TestModel = createTestModel({}, {allowExtendedOperators: true}); - return TestModel.find(extendedQuery()).then((results) => { + return TestModel.find(extendedQuery()).then(results => { should(results[0].value).eql({$exists: true}); }); }); it('`dataSource.settings.allowExtendedOperators` honor Model settings - ' + - 'converts extended operators', () => { + 'reports invalid operator', () => { const TestModel = createTestModel({allowExtendedOperators: true}, {allowExtendedOperators: false}); - return TestModel.find(extendedQuery()).then((results) => { - should(results[0].value).eql('[object Object]'); + return TestModel.find(extendedQuery()).catch(err => { + assertOperatorNotAllowed(err); }); }); it('`dataSource.settings.allowExtendedOperators` honor Model settings - ' + 'preserves extended operators', () => { const TestModel = createTestModel({allowExtendedOperators: false}, {allowExtendedOperators: true}); - return TestModel.find(extendedQuery()).then((results) => { + return TestModel.find(extendedQuery()).then(results => { should(results[0].value).eql({$exists: true}); }); }); it('`options.allowExtendedOperators` override Model settings - converts extended operators', () => { const TestModel = createTestModel({allowExtendedOperators: true}); - return TestModel.find(extendedQuery(), {allowExtendedOperators: false}).then((results) => { - should(results[0].value).eql('[object Object]'); + return TestModel.find(extendedQuery(), {allowExtendedOperators: false}).catch(err => { + assertOperatorNotAllowed(err); }); }); it('`options.allowExtendedOperators` Model settings - preserves extended operators', () => { const TestModel = createTestModel({allowExtendedOperators: false}); - return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then((results) => { + return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then(results => { should(results[0].value).eql({$exists: true}); }); }); @@ -255,7 +264,7 @@ describe('allowExtendedOperators', () => { context('DAO.find()', () => { it('preserves extended operators with allowExtendedOperators set', () => { const TestModel = createTestModel(); - return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then((results) => { + return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then(results => { should(results[0].value).eql({$exists: true}); }); }); @@ -263,15 +272,15 @@ describe('allowExtendedOperators', () => { it('`dataSource.settings.allowExtendedOperators` honor options settings - ' + 'converts extended operators', () => { const TestModel = createTestModel({allowExtendedOperators: true}); - return TestModel.find(extendedQuery(), {allowExtendedOperators: false}).then((results) => { - should(results[0].value).eql('[object Object]'); + return TestModel.find(extendedQuery(), {allowExtendedOperators: false}).catch(err => { + assertOperatorNotAllowed(err); }); }); it('`dataSource.settings.allowExtendedOperators` honor options settings - ' + 'preserves extended operators', () => { const TestModel = createTestModel({allowExtendedOperators: false}); - return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then((results) => { + return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then(results => { should(results[0].value).eql({$exists: true}); }); }); @@ -279,15 +288,15 @@ describe('allowExtendedOperators', () => { it('`Model.settings.allowExtendedOperators` honor options settings - ' + 'converts extended operators', () => { const TestModel = createTestModel({}, {allowExtendedOperators: true}); - return TestModel.find(extendedQuery(), {allowExtendedOperators: false}).then((results) => { - should(results[0].value).eql('[object Object]'); + return TestModel.find(extendedQuery(), {allowExtendedOperators: false}).catch(err => { + assertOperatorNotAllowed(err); }); }); it('`Model.settings.allowExtendedOperators` honor options settings - ' + 'preserves extended operators', () => { const TestModel = createTestModel({}, {allowExtendedOperators: false}); - return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then((results) => { + return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then(results => { should(results[0].value).eql({$exists: true}); }); }); diff --git a/test/model-definition.test.js b/test/model-definition.test.js index bf7a0e97..e8082962 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,18 +271,20 @@ 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); Parent.create({ name: 'parent', }, function(err, parent) { + if (err) return done(err); parent.children.create({ name: 'child', protectedProperty: 'protectedValue', }, function(err, child) { + if (err) return done(err); Parent.find({include: 'children'}, function(err, parents) { + if (err) return done(err); var serialized = parents[0].toJSON(); var child = serialized.children[0]; assert.equal(child.name, 'child'); @@ -295,7 +296,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,18 +312,20 @@ 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); Parent.create({ name: 'parent', }, function(err, parent) { + if (err) return done(err); parent.children.create({ name: 'child', secret: 'secret', }, function(err, child) { + if (err) return done(err); Parent.find({include: 'children'}, function(err, parents) { + if (err) return done(err); var serialized = parents[0].toJSON(); var child = serialized.children[0]; assert.equal(child.name, 'child'); @@ -334,6 +336,140 @@ describe('ModelDefinition class', function() { }); }); + describe('hidden properties', function() { + var Child; + beforeEach(givenChildren); + + it('should be removed if used in where', function() { + return Child.find({ + where: {secret: 'guess'}, + }).then(assertHiddenPropertyIsIgnored); + }); + + it('should be removed if used in where.and', function() { + return Child.find({ + where: {and: [{secret: 'guess'}]}, + }).then(assertHiddenPropertyIsIgnored); + }); + + /** + * Create two children with a hidden property, one with a matching + * value, the other with a non-matching value + */ + function givenChildren() { + Child = memory.createModel('child', {}, {hidden: ['secret']}); + return Child.create([{ + name: 'childA', + secret: 'secret', + }, { + name: 'childB', + secret: 'guess', + }]); + } + + function assertHiddenPropertyIsIgnored(children) { + // All children are found whether the `secret` condition matches or not + // as the condition is removed because it's hidden + children.length.should.equal(2); + } + }); + + function assertParentIncludeChildren(parents) { + parents[0].toJSON().children.length.should.equal(1); + } + + describe('protected properties', function() { + var Parent; + var Child; + beforeEach(givenParentAndChild); + + it('should be removed if used in include scope', function() { + Parent.find({ + include: { + relation: 'children', + scope: { + where: { + secret: 'x', + }, + }, + }, + }).then(assertParentIncludeChildren); + }); + + it('should be rejected if used in include scope.where.and', function() { + return Parent.find({ + include: { + relation: 'children', + scope: { + where: { + and: [{secret: 'x'}], + }, + }, + }, + }).then(assertParentIncludeChildren); + }); + + it('should be removed if a hidden property is used in include scope', function() { + return Parent.find({ + include: { + relation: 'children', + scope: { + where: { + secret: 'x', + }, + }, + }, + }).then(assertParentIncludeChildren); + }); + + function givenParentAndChild() { + Parent = memory.createModel('parent'); + Child = memory.createModel('child', {}, {protected: ['secret']}); + Parent.hasMany(Child); + return Parent.create({ + name: 'parent', + }).then(parent => { + return parent.children.create({ + name: 'child', + secret: 'secret', + }); + }); + } + }); + + describe('hidden properties in include', function() { + var Parent; + var Child; + beforeEach(givenParentAndChildWithHiddenProperty); + + it('should be rejected if used in scope', function() { + return Parent.find({ + include: { + relation: 'children', + scope: { + where: { + secret: 'x', + }, + }, + }, + }).then(assertParentIncludeChildren); + }); + + function givenParentAndChildWithHiddenProperty() { + Parent = memory.createModel('parent'); + Child = memory.createModel('child', {}, {hidden: ['secret']}); + Parent.hasMany(Child); + return Parent.create({ + name: 'parent', + }).then(parent => { + return parent.children.create({ + name: 'child', + secret: 'secret', + }); + }); + } + }); + it('should throw error for property names containing dot', function() { (function() { memory.createModel('Dotted', {'dot.name': String}); }) .should diff --git a/test/util.test.js b/test/util.test.js index 132bdf64..3874faeb 100644 --- a/test/util.test.js +++ b/test/util.test.js @@ -8,7 +8,7 @@ var should = require('./init.js'); var utils = require('../lib/utils'); var ObjectID = require('bson').ObjectID; var fieldsToArray = utils.fieldsToArray; -var removeUndefined = utils.removeUndefined; +var sanitizeQuery = utils.sanitizeQuery; var deepMerge = utils.deepMerge; var rankArrayElements = utils.rankArrayElements; var mergeIncludes = utils.mergeIncludes; @@ -52,33 +52,65 @@ describe('util.fieldsToArray', function() { }); }); -describe('util.removeUndefined', function() { +describe('util.sanitizeQuery', function() { it('Remove undefined values from the query object', function() { var q1 = {where: {x: 1, y: undefined}}; - should.deepEqual(removeUndefined(q1), {where: {x: 1}}); + should.deepEqual(sanitizeQuery(q1), {where: {x: 1}}); var q2 = {where: {x: 1, y: 2}}; - should.deepEqual(removeUndefined(q2), {where: {x: 1, y: 2}}); + should.deepEqual(sanitizeQuery(q2), {where: {x: 1, y: 2}}); var q3 = {where: {x: 1, y: {in: [2, undefined]}}}; - should.deepEqual(removeUndefined(q3), {where: {x: 1, y: {in: [2]}}}); + should.deepEqual(sanitizeQuery(q3), {where: {x: 1, y: {in: [2]}}}); - should.equal(removeUndefined(null), null); + should.equal(sanitizeQuery(null), null); - should.equal(removeUndefined(undefined), undefined); + should.equal(sanitizeQuery(undefined), undefined); - should.equal(removeUndefined('x'), 'x'); + should.equal(sanitizeQuery('x'), 'x'); var date = new Date(); var q4 = {where: {x: 1, y: date}}; - should.deepEqual(removeUndefined(q4), {where: {x: 1, y: date}}); + should.deepEqual(sanitizeQuery(q4), {where: {x: 1, y: date}}); // test handling of undefined var q5 = {where: {x: 1, y: undefined}}; - should.deepEqual(removeUndefined(q5, 'nullify'), {where: {x: 1, y: null}}); + should.deepEqual(sanitizeQuery(q5, 'nullify'), {where: {x: 1, y: null}}); + + q5 = {where: {x: 1, y: undefined}}; + should.deepEqual(sanitizeQuery(q5, {normalizeUndefinedInQuery: 'nullify'}), {where: {x: 1, y: null}}); var q6 = {where: {x: 1, y: undefined}}; - (function() { removeUndefined(q6, 'throw'); }).should.throw(/`undefined` in query/); + (function() { sanitizeQuery(q6, 'throw'); }).should.throw(/`undefined` in query/); + }); + + it('Report errors for circular or deep query objects', function() { + var q7 = {where: {x: 1}}; + q7.where.y = q7; + (function() { sanitizeQuery(q7); }).should.throw( + /The query object is too deep or circular/ + ); + + var q8 = {where: {and: [{and: [{and: [{and: [{and: [{and: + [{and: [{and: [{and: [{x: 1}]}]}]}]}]}]}]}]}]}}; + (function() { sanitizeQuery(q8); }).should.throw( + /The query object is too deep or circular/ + ); + + var q9 = {where: {and: [{and: [{and: [{and: [{x: 1}]}]}]}]}}; + (function() { sanitizeQuery(q8, {maxDepth: 4}); }).should.throw( + /The query object is too deep or circular/ + ); + }); + + it('Removed prohibited properties in query objects', function() { + var q1 = {where: {secret: 'guess'}}; + sanitizeQuery(q1, {prohibitedKeys: ['secret']}); + q1.where.should.eql({}); + + var q2 = {and: [{secret: 'guess'}, {x: 1}]}; + sanitizeQuery(q2, {prohibitedKeys: ['secret']}); + q2.should.eql({and: [{}, {x: 1}]}); }); });