Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update read-only.js #10

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Update read-only.js #10

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
09e269b
Update read-only.js
devonatdomandtom Aug 19, 2016
10a3b3e
Update read-only.js
devonatdomandtom Aug 19, 2016
e145967
Update read-only.js
devonatdomandtom Aug 19, 2016
a3dd249
added allowed roles property
devonatdomandtom Aug 31, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 21 additions & 10 deletions lib/read-only.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
var debug = require('debug')('loopback-ds-readonly-mixin');
var _ = require("lodash");

module.exports = function(Model, options) {
'use strict';
Expand All @@ -7,23 +8,33 @@ module.exports = function(Model, options) {

// Make sure emailVerified is not set by creation
Model.stripReadOnlyProperties = function(ctx, modelInstance, next) {

var body = ctx.req.body;
if (!body) {
return next();
}

var currentUser = ctx.req.currentUser;
if (currentUser && options.allowedRoles) {
var roleNames = _.map(currentUser.toObject().roles, 'name');
var allowed = _.intersection(options.allowedRoles, roleNames);
if (allowed.length) return next();
}

var err = new Error('Unable to update: ' + Model.modelName + ' property is read only');
err.statusCode = 403;

var properties = (Object.keys(options).length) ? options : null;
if (properties) {
debug('Creating %s : Read only properties are %j', Model.modelName, properties);
Object.keys(properties).forEach(function(key) {
if (!properties) return next(err);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the response a 403 given a model using this mixin where the mixin options = an empty object?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error seems inappropriate in general if the goal is to address an empty body and subsequent SQL errors when using SQL connectors.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we create a new error and response code you think?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, just noticed the SQL errors will happen for empty bodies against the remote to update attributes when using the SQL connectors... maybe the solution should be somewhere else at a lower level?

debug('Creating %s : Read only properties are %j', Model.modelName, properties);
Object.keys(properties).forEach(function(key) {
if (key !== "allowedRoles") {
debug('The \'%s\' property is read only, removing incoming data', key);
delete body[key];
});
next();
} else {
var err = new Error('Unable to update: ' + Model.modelName + ' is read only.');
err.statusCode = 403;
next(err);
}
}
});
if (!Object.keys(body).length) return next(err);
next();
};

// Make sure emailVerified is not set by creation
Expand Down