Skip to content

Commit

Permalink
Improve hidden/protected property checks
Browse files Browse the repository at this point in the history 
- Fixes #1645
  - do not apply hidden property check for data
- Fixes #1648
  - allow prohibitHiddenPropertiesInQuery to be set in model/ds
  • Loading branch information
@raymondfeng
raymondfeng committed Oct 28, 2018
1 parent 8ebb734 commit 8fa7c94
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 49 deletions.
113 changes: 65 additions & 48 deletions lib/dao.js
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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;
};
Expand Down Expand Up @@ -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];
}
Expand All @@ -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);

This comment has been minimized.

Sign in to view

Copy link
@bajtos

bajtos Nov 9, 2018

Member

@raymondfeng is this console log intentional? Isn't it going to print false or undefined instead of a meaningful warning?

This comment has been minimized.

Sign in to view

Copy link
@raymondfeng

raymondfeng Nov 9, 2018

Author Contributor

No, it was fixed in later PRs.

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

/*
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/include.js
View file
Expand Up @@ -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);
Expand Down
18 changes: 18 additions & 0 deletions test/model-definition.test.js
View file
Expand Up @@ -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() {
Expand Down

0 comments on commit 8fa7c94

Please sign in to comment.