Merge pull request #1649 from strongloop/fix-1645

Improve hidden/protected property checks
This commit is contained in:
Raymond Feng 2018-10-29 13:09:33 -07:00 committed by GitHub
commit 7609585dc3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 160 additions and 52 deletions

View File

@ -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
var normalizeUndefinedInQuery = Model._getSetting('normalizeUndefinedInQuery');
// alter configuration of how sanitizeQuery handles undefined values
ctx.where = Model._sanitize(ctx.where, {normalizeUndefinedInQuery: normalizeUndefinedInQuery});
ctx.where = Model._sanitizeQuery(ctx.where);
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]) {
return m.settings[key];
if (m && m.settings) {
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');
// alter configuration of how sanitizeQuery handles undefined values
filter = this._sanitize(filter, {normalizeUndefinedInQuery: normalizeUndefinedInQuery});
filter = this._sanitizeQuery(filter);
this._coerce(filter.where, options);
return filter;
};
@ -1644,9 +1643,7 @@ function coerceArray(val) {
return arrayVal;
}
DataAccessObject._getHiddenProperties = function() {
var settings = this.definition.settings || {};
var result = settings.hiddenProperties || settings.hidden || [];
function _normalizeAsArray(result) {
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') {
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);
}
return _normalizeAsArray(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 (options.excludeProtectedProperties) {
prohibitedKeys = prohibitedKeys.concat(this._getProtectedProperties());
if (prohibitHiddenPropertiesInQuery) {
prohibitedKeys = this._getHiddenProperties();
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

View File

@ -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);

View File

@ -302,6 +302,19 @@ function selectFields(fields) {
};
}
function isProhibited(key, prohibitedKeys) {
if (!prohibitedKeys || !prohibitedKeys.length) return false;
if (typeof key !== 'string') {
return false;
}
for (var k of prohibitedKeys) {
if (k === key) return true;
// x.secret, secret.y, or x.secret.y
if (key.split('.').indexOf(k) !== -1) return true;
}
return false;
}
/**
* Sanitize the query object
* @param query {object} The query object
@ -341,7 +354,7 @@ function sanitizeQuery(query, options) {
* Make sure prohibited keys are removed from the query to prevent
* sensitive values from being guessed
*/
if (prohibitedKeys && prohibitedKeys.indexOf(this.key) !== -1) {
if (isProhibited(this.key, prohibitedKeys)) {
offendingKeys.push(this.key);
this.remove();
return;

View File

@ -394,7 +394,6 @@ describe('DataSource define model', function() {
User.create({name: 'Jeff'}, function(err, data) {
if (err) {
console.log(err);
return;
}
var post = data.posts.build({title: 'My Post'});

View File

@ -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() {
@ -377,7 +395,10 @@ describe('ModelDefinition class', function() {
*/
function givenChildren(hiddenProps) {
hiddenProps = hiddenProps || {hidden: ['secret']};
Child = memory.createModel('child', {}, hiddenProps);
Child = memory.createModel('child', {
name: String,
secret: String,
}, hiddenProps);
return Child.create([{
name: 'childA',
secret: 'secret',
@ -394,6 +415,64 @@ describe('ModelDefinition class', function() {
}
});
describe('hidden nested properties', function() {
var Child;
beforeEach(givenChildren);
it('should be removed if used in where as a composite key - x.secret', function() {
return Child.find({
where: {'x.secret': 'guess'},
}).then(assertHiddenPropertyIsIgnored);
});
it('should be removed if used in where as a composite key - secret.y', function() {
return Child.find({
where: {'secret.y': 'guess'},
}).then(assertHiddenPropertyIsIgnored);
});
it('should be removed if used in where as a composite key - a.secret.b', function() {
return Child.find({
where: {'a.secret.b': 'guess'},
}).then(assertHiddenPropertyIsIgnored);
});
function givenChildren() {
var hiddenProps = {hidden: ['secret']};
Child = memory.createModel('child', {
name: String,
x: {
secret: String,
},
secret: {
y: String,
},
a: {
secret: {
b: String,
},
},
}, hiddenProps);
return Child.create([{
name: 'childA',
x: {secret: 'secret'},
secret: {y: 'secret'},
a: {secret: {b: 'secret'}},
}, {
name: 'childB',
x: {secret: 'guess'},
secret: {y: 'guess'},
a: {secret: {b: '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);
}