_Taking naming suggestions, 'scope' might confuse with Model.scope_
Allow users to scope attributes for retrieval when using get() & toJSON().
Defining 'scopes' on attributes:
sequelize.define('User', {
email: {
type: DataTypes.STRING,
scopes: ['self'],
},
name: {
type: DataTypes.STRING,
scopes: ['self', 'public']
},
title: {
type: DataTypes.STRING // No scopes is default all scopes
},
parent_id: {
type: DataTypes.INTEGER,
scopes: false||[] // Don't EVER include
}
});
Defining attributes in scope:
// found some user
var values = user.get({scope: 'public'}); // {name: someValue, title: someValue}
var values = user.get({scope: 'self'}); // {email: someValue, email: someValue, title: someValue}
_WIP_
I'd like to see Scope Management facilities so we can create custom scopes.
Just out of memory, i use these scopes in my app:
You mean a generalized scope manager that could be used in multiple places?
Whats the usecase besides attribute retrieval?
How do we implement scopes for getter methods?
Perhaps this would be a case for a Sequelize.NONE/VIRTUAL datatype which has been suggested before.
var values = user.get({scopes: ['public', 'self']}) // ?
No, create a new scope combining the two
@thanpolas hmm, how would you do that? scopes that extend?
If you want some attributes to be in both public and self, but only some in self?
sequelize.define('User', {
email: {
type: DataTypes.STRING,
scopes: ['self'],
scope: 'self'
},
name: {
type: DataTypes.STRING,
scopes: ['self', 'public']
},
title: {
type: DataTypes.STRING // No scopes is default all scopes?
},
parent_id: {
type: DataTypes.INTEGER,
scope[s]: false // Don't EVER include?
}
}, {
scopes: ['self', 'public', 'custom']
});
Oh right, you were commenting on .get(), my bad.
In regards to the questions posed on schema design, here are my recommendations,
i'd prefer a single keyword scopes an Array, one scope, one array item scopes: ['self'] (no sugar)
To not show at all use false, has the same effect as an empty array.
If no scopes key is defined the attribute will be always visible.
scopes array as the only param is decided then!
+1
I was trying to this: https://gist.github.com/tanepiper/9549621
I don't want to expose the password from the user object, but I do need to check it so scopes would be perfect for this.
@tanepiper to solve your problem _now_ you could overwrite the toJSON function in the instancemethods section to return everything but the password :) of course... you could never use json.stringify/parse/yadda/yadda and need the password
+1. This would be extremely useful.
I vote for the term access instead of scope.
I think it would also be useful to differentiate between read and write. Something like this:
sequelize.define('User', {
email: {
type: DataTypes.STRING,
scopes: {
public: false,
self: ['read', 'write']
},
scope: 'self'
},
name: {
type: DataTypes.STRING,
// public can see name, but not edit it
scopes: {
public: ['read'],
self: ['read', 'write']
}
},
title: {
type: DataTypes.STRING // No scopes is default all scopes?
},
parent_id: {
type: DataTypes.INTEGER,
scope[s]: false // Don't EVER include?
}
}, {
scopes: ['self', 'public', 'custom']
});
And for that matter, create, read, and update. Create meaning you can define that property when building or createing an instance, update when setting attributes, and read when reading.
I like the idea of mass assignment protection via this, it does however make scopes a big feature (compared to relatively small before).
We could probably only do read/write - Right not create() and updateAttributes use .set() internally, i guess we could set a context parameter but that would require the user to set a context parameter whenever using set directly.
Thoughts on write scoping aswell (for mass assignment/api protection) @sequelize/owners?
It would certainly make Sequelize safer.
I can see a few wins with read/write protection but do understand that these are nothing else but validations.
Thus we are overloading the scope feature.
If you need to write/update protect an attribute a pre-save/pre-validation hook would do the job (i.e. the user table's password field that needs to be hashed).
To add a few more examples of use cases for write protection:
It would be useful for some of the system attributes like created_at, updated_at, deleted_at and id, which will pretty much always need to be write protected.
I just got done implementing a simplistic version of this in my own project across ~13 models most with 10+ attributes and I feel this functionality has made my code an order of magnitude more manageable.
My write-protected model attributes now look like this:
'password': {
type: DataTypes.STRING,
access: { read: false },
},
It assumes that they are publicly readable and writeable until you tell it otherwise. I am lucky that I don't need different access roles... yet. That will be around the corner though at which my point they will look like the model that I suggested above.
I also had it set these protections by default, which made things work pretty much "out of the box" for me:
var defaults = {
id: { read: true, write: false },
created_at: { read: true, write: false },
updated_at: { read: true, write: false },
deleted_at: { read: false, write: false },
};
@thanpolas can't provide a context to a validator hook currently though, but i see your point.
@cjroth system attributes are already write protected via .set()
just for the record, i dig the access feature, i'm just trying to say that it should be a different _thing_, not coupled with the scope feature
Yeah i agree with you there. It would be nice to keep scope read specific and do mass-assignment/access protection/acl as another thing.
+1, great feature.
I imagine something like this:
sequelize.define('User', {
isAdmin: {
type: Sequelize.BOOLEAN,
defaultValue: false
},
email: {
type: Sequelize.STRING,
scopes: ['create', 'private'],
},
name: {
type: Sequelize.STRING,
scopes: ['create', 'edit', 'public', 'private']
}
});
user.create({ email: '[email protected]', name: 'Foo', isAdmin: true }, { scope: 'create' });
> { email: '[email protected]', name: 'Foo', isAdmin: false }
user.get({ scope: 'public' });
> { name: 'Foo' }
user.updateAttributes({ email: '[email protected]', name: 'Updated Foo' }, { scope: 'edit' });
user.get({ scope: 'private' });
> { email: '[email protected]', name: 'Updated Foo' }
@fixe Good suggestion, it suppose it would be good to include scopes in .set() (which .updateAttributes() and .create() use internally besides using it with .get().
It would solve some of the mass assignment issues, obviously not all of them but it could be a good start.
Thoughts @cjroth, @thanpolas, @janmeier?
I don't understand the role of scopes in set/create/updateAttributes... can someone explain this?
Well it would just be implementing scopes in both set and get, i.e. whitelisting what attributes are set or retrieved.
I understand your concern about well .. mixing the concerns, but it would solve a few things without having to build a new feature concerning access.
oh right... well i still think we shouldn't mix apples with oranges, that happens when you add on the same set the create and public scopes... each represent a different thing, create is intended for protecting from writes, public in intended for filtering reads...
One more thing:
user.updateAttributes({ email: '[email protected]', name: 'Updated Foo' }, { scope: edit });
user.get({ scope: 'private' });
> { email: '[email protected]', name: 'Updated Foo' }
If we go forward with this, we need to have a way to dictate whether the updateAttributes should fail or ignore the email attribute that doesn't have the edit scope.
I agree that we shouldn't mix the two intents. Here's my proposed solution:
sequelize.define('User', {
isAdmin: {
type: Sequelize.BOOLEAN,
defaultValue: false
},
email: {
type: Sequelize.STRING,
scopes: {
write: ['create'],
read: ['private']
}
},
name: {
type: Sequelize.STRING,
scopes: {
write: ['create', 'edit'],
read: ['public', 'private']
}
}
});
What do you think @thanpolas?
"If we go forward with this, we need to have a way to dictate whether the updateAttributes should fail or ignore the email attribute that doesn't have the edit scope."
@fixe probably want to use set/get as the verbs rather than write/read for consistency. But the user has to supply the context to .set() themselves, or we define a default scope context for .create()/.updateAttributes()/.save()
If we decide to tack access control onto scopes, we might aswell don't do read/write at all though, and just let users prefix if they want that. They'll most likely have to provide the context when using set/get and related calls anyways.
my proposal would be something to the tune of:
name: {
type: Sequelize.STRING,
scopes: ['public', 'private'],
access: ['create', 'edit'],
}
That way we explicitly state that scopes is one thing and access is another, and everyone knows which is which. All get* operations have to do with scopes, all set* operations have to do with access
We implemented a very similar thing for MongoDB / mongoose (although the term "scopes" is much more fortunate than "contexts").
I think @thanpolas display of use cases is a great way to imagine this working.
As for default scope, unless you define a scope when invoking the call, it should not apply any scope? This would maintain compatibility by default. I like @fixe 's suggestion above, with a small change, replacing "write" with "set" and "read" with "get", like @mickhansen suggested.
However, I'd like to suggest making the scope names stand out
email: {
type: Sequelize.STRING,
scopes: {
create: { set: true },
private: { get: true, set: true }
public: { get: true, set: false }
api: { get: true, set: false }
}
},
The advantage here is that you could then expand this to create get/set transformation functions, to be applied only on a certain scope. For instance, a user name in public becoming first name and first letter of last name.
public: { get: function (value) { ... }, set: false }
I like both the @thanpolas proposal and the @jrpereira proposal. Seperation of concerns is always nice but yeah i'm kind of torn, need more opinions on the matter.
I suspect there might be a bit of confusion thinking one might be able to mix stuff like 'public' and 'create'/'edit'
@jrpereira If no scope or access level is defined, it will use original everything-is-alllowed funtionality.
@jrpereira If no scope or access level is defined, it will use original everything-is-alllowed funtionality.
I beg to differ, there should be an option to have a default scope (I personally would never enforce this, it's kind of a template standard in other ORMs).
I actually like @fixe's proposal
I know this will be up to each person, but the scope names should be contextual. I personally wouldn't use "create" as a scope, because that's an action, not the context in which the action is executed.
Let's consider acess to the Name property of a user, the scopes could be:
scopes: {
self: true, // shorthand for { get: true, set: true }
registered: { get: true } // default to false if you set a scope, so this implies { set: false }
anonymous: false // shorthand for { get: false, set: false }
So, when you're creating a new user, you could use the "self" scope (the user creates itself), which means you can get/set the user name.
At the end of the day, what matters here is to allow people the freedom to express the scopes that matter to them.
Sorry meant to say @jrpereira's idea which extended off of @fix's
@durango, that could be as simple as having a convention of using _default_ as a scope. Means that get/set methods use "default" as scope, if none is provided.
However, you shouldn't have to declare the "default" scope. If you don't define the "default" scope, it'd be defined for you as
scopes: {
default: true
}
But let's say you want to make it impossible to write to a field without specifying a scope - you'd do:
scopes: {
default: { get: true, set: false }
}
I know this will be up to each person, but the scope names should be contextual. I personally wouldn't use "create" as a scope, because that's an action, not the context in which the action is executed.
Completely agree.
I like @jrpereira's proposal as well - But i also respect @thanpolas's opinion quite a bit and am always wary of mixing concerns.
However i do not think that having access control under scopes together with read filtering is that concern mixing. I'd argue that its roughly the same domain and that @jrpereira's proposal creates a nice, clear (albeit a bit verbose) api
Not sure i'm a big fan on an implicit default scope. I'd rather keep non-scope attributes/functionality the same as it has always been, to ensure we don't break anything for now.
i like @jrpereira's proposal too, i'm happy we found our way.
Well ain't that sweet music to my ears :)
I'm jumping right onto the bandwagon and agreeing that @jrpereira's suggestion looks good to me :)
I'm also happy with the last proposal.
Yup, I think @jrpereira's idea looks good too.
what if:
user.as('self').get();
which is a nice pattern. i've also been using some extended functionality that i've built on top of sequelize in my project. for example:
.find(3)
.success(function(field) {
var fieldAsPublic = field
.as('public')
.get();
// do something with `fieldAsPublic`...
});
just a thought.
@cjroth we can add that as sugar later on, the base functionality will be an option to get.
The problem with stuff as .as() is we'd need to return a new model instance object, and that would impact performance - Or we'd need to use some option that's only availabe in that tick, but then user.as('self') could not be stored in a variable.
@mickhansen True. I suppose you'd have to clone the instance in order apply the as() to only that point in the chain and after. Not good for performance. Any ideas on a way to implement without cloning?
Well if you don't need to reuse the .as() return, i.e. it's a one-off where you have to chain the .get() immediatly i have a few ideas, but then you "might as well" use .get({scope: 'self'})
In my case I'm actually thinking it will be useful that it applies the as to the entire instance because I will generally never be using an instance as multiple roles, so I can do all of my writing and reading in one big chain:
.as('self')
.set(data)
.error(handler)
.success(function() {
console.log(user.get()); // user would be stringified as "self" here
})
Not sure how I feel about that though, it could make it too easy to accidentally access a DAO as the wrong role.
I'm doing some work with scopes (not on Sequelize) and noticed this detail:
// default to false if you set a scope
// so this implies { set: false } <--------
registered: { get: true }
I'd expect get/set scopes to default to true for any attribute... why would we want to default to false?
edit: expanded sample for clarity
@thanpolas i think the intention was that set would be false in that situation if you had only defined get as true.
@thanpolas, like @mickhansen pointed out, what I was thinking is that if you only define "get" as true, it means "set" would be false (otherwise you'd just use "registered: true").
@cjroth that's an interesting use case, but maybe "as" is a bit too inexpressive? In other words, maybe "as" doesn't reflect the fact that what would happen is that any fields outside the requested scope would be undefined.
I always think of this effect as "filtering", but user.filter({scope: 'xxx'}) doesn't seem quite right? What verbs are usually associated with this? (ala _.select and _.pick). Thoughts?
@jrpereira i understand what it means. I am asking why. It's counter intuitive.
@thanpolas please allow me to elaborate, maybe there's a flaw in my logic.
The default behavior is that if no scope is specified, you can get/set the field. This is like assuming that any field that doesn't define scopes in fact defines one like this:
scopes: {
default: { get: true, set: true }
}
We're using default as the name of a scope that gets applied if you don't specify one in get/set. It is the same as stating that get/set operations always have { scope: 'default' }. One could redefine the default scope, which is a great way to ensure no code paths get/set the field without explicitly using a scope that allows it.
As for the effect of defining a new scope, let's consider the options:
scopes: {
scope1: true,
scope2: false,
scope3: { get: _ , set: _ }
scope4: { get: true },
scope5: { get: false },
}
scope1 means we can get and set the field in this scope, and scope2 means we can neither get or set the field in this scope, and scope3 is obvious too, I think insofar it all makes sense.
Now, for scope4 and scope5, one of them is _useless_, depending on what we assume for the param that is omitted. The way I see it, we have 3 options:
Other than erroring, the reason I believe defaulting to _false_ makes more sense is precisely because the default scope for a field already get/set. This means that if we define a new scope, the _intent is to deny access_ (because allowing is the default) in either get/set, so the intent is to make at least one of them _false_. Otherwise, what would be the point of the scope to start with?
So, the reasoning is that faced with either scope4 or scope5 being pointless, I prefer to err on the side of restricting access, because that's the intent of scopes.
Disclaimer: I see _scopes_ as means to ensure data is get/set in a very controlled manner, which possibly taints my perspective.
Hope this makes sense and that I'm not missing anything.
@thanpolas, @mickhansen - I'd suggest we more closely align this with the typical REST / CRUD mechanics, and distinguish creation from getting/setting.
This makes sense in places where you need to ensure the object is created with the defaults for certain fields, but that you can manipulate them after creation.
_Example scenario:_
Consider an API that creates a new _blog post_. You cannot specify the starting number of _views_ for the post, it needs to start at 0. However, afterwards, you can increment this number.
_Suggestion_:
scopes: {
scope3: { get: _ , set: _, create: _ }
}
This also takes care of the fact that one of the main uses is precisely to distinguish what you can write at object creation, and what you can change afterwards. Therefore defining a context for "create", allows the usage of semantic scopes.
For instance, imagine the typical blogging webapp, which offers an API. Internally it'd use the get/set with "default" scope, but in order to ensure only very specific fields are used in the API, it'd create a scope called "api".
With this change, I won't need to have a scope named 'api_create' to use ONLY on the "create" method (which is what many use cases would entail).
@jrpereira lets keep scopes to set/get for now, it's simpler and should cover most cases. We can look at expanding it later on - I'll start working on this feature soon.
"default" scope will probably be left out to begin with to ensure better BC.
+1
+1
+1
+1
This is easily implemented as a plugin so we have decided not to implement it in core.
I personally need this so i'm developing an implementation here: https://github.com/mickhansen/ssacl-attribute-roles (as of writing, only .get() support - since that's where most of my usecases live, .set() to follow soon).
+1
@mickhansen This is a so needed feature in almost all systems, that I think it should be in the core, or at least a plugin with a strong support by the core sequelize developers.
Something as the mongoose .select() method would be awesome, since there are occasions when an attribute filtering needs to be made without having to specify it at the Model definition level, but for specific queries.
@diosney core sequelize developers = 2 people, one of them is me, so you could say it has strong support ;)
@mickhansen haha! good point :smile:
I will give it a try then.
@mickhansen Thanks, it worked like a charm :)
an ORM that does not allow you exclude fields at its core?
@pbrain19 It's supported by attributes.
Can you show me where this is documented? Can't seem to find it... do i just put the "-" if i want to exclude?
@pbrain19 Model.findAll({attributes: ['...']}) - it simply takes a list of attribute names, use lodash with Object.keys(Model.rawAttributes) to do an exclude.
It's not so magic but it can be accomplished in one line of code :)
Combine it with a beforeFind hook and it will work for all finds.
@mickhansen so Model.findAll({attributes: ['_DoNotInclude']}) will include all attributes but the DONotInclude?
@pbrain19 No. attributes defines exactly what attributes to load, like my previous comment should have suggested.
The only way to exclude is to include all the ones you want without the ones you don't? So in essence there is no exclude functionality?
I only ask because I have very large objects.
No native exclude at the moment, but it is something we'd like to improve on: https://github.com/sequelize/sequelize/issues/4074
However it can be implemented with three lines of code using the list of existing attributes Object.keys(Model.rawAttributes) and a filter (perhaps using lodash) + a find hook.
I see. thanks
So, any progress with this? I love Sequelize so far, but not being able to filter eager loads or exposing fields is really busting my behind.
@Neefay It's been decided that it's better off left as a plugin, you can give mine a try.
@mickhansen I'm assuming you refer to this? I read the example and it made my head spin for a bit, but I'll take your word for it and dive in deeper. Thank you for your excellent work with Sequelize.
@Neefay That's the one :)
For the people that were reading this thread looking for a full exemple (based on express-exemple) on how to hide attributes redefining the toJSON method:
var _ = require ('lodash');
module.exports = function (sequelize, DataTypes) {
var schema = {
name: {
type: DataTypes.STRING(255),
allowNull: false
},
email: {
type: DataTypes.STRING(255),
allowNull: false,
unique: true
},
password: DataTypes.STRING(20)
};
var User = sequelize.define('user', schema, {
instanceMethods: {
toJSON: function () {
var privateAttributes = [ 'password' ];
return _.omit(this.dataValues, privateAttributes);
}
}
});
return User;
}
I know it's a bit late but I have a proposal that after reading the Sequelize documentation for the first time it just made sense to me. Actually I went ahead and coded my model first just to find out it did not work that way :P.
This leaves the matter of access control for another day as suggested many times on this thread and focus only on the possibility to show/hide a column based on scopes. As documented here Sequelize already have scopes. So why not enrich that same facility to filter out columns as well as rows and avoid the naming confusion at the same time?
var User = sequelize.define('User', {
id: DataTypes.INTEGER,
name: DataTypes.STRING,
email: DataTypes.STRING,
active: DataTypes.BOOLEAN,
password: DataTypes.STRING
}, {
defaultScope: {
where: { active: true },
attributes: ['id', 'name']
},
limited: {
attributes: {exclude: ['password', 'email']}
},
complex: {
attributes: {exclude: ['password'], include: ['name', 'email']}
},
})
This new attributes property, if it's an array, it would behave the same as the attributes property we already use to filter columns in the query methods here. With the extra functionality of having include and exclude arrays when it's an object.
This way we can use model scopes not only to filter out rows but columns as well and don't have to repeat those filters on every query method.
@carragom We've discussed merging exclude/include into attributes with scopes but skipped in the first round since we're not exactly sure what should take precedence (what if a scope applied after another includes something that was excluded etc).
Well in that case one of the two should take precedence (i think exclude should) and the behavior should be documented.
But I don't consider this an issue since the same thing can already happen when mixing multiple scopes with conflicting filters, something like this:
active: {
where: {active: true}
},
inactive: {
where: {active: false}
}
In the end it looks like mixing multiple scopes is not a good idea and should be avoided on a general basis. Or maybe I'm missing something about scopes here ?.
@carragom you have to be careful about mixing scopes atleast :)
I may be missing something but this doesn't appear to cover the case where you have eager loaded an association for use with processing but don't want it to be returned to the user. However, @OlivierCuyp method does allow you to cover this case.
Though it should be the following so that getters are properly invoked.
toJSON: function () {
var privateAttributes = [ 'locales' ];
return _.omit(this.get({plain:true}), privateAttributes);
}
@smassin
There's still a potential loop hole with that implementation if you have nested models.
When you call toJSON() on the original object, the nested models will be returned in plain objects.
Those plain objects may have had their own toJSON() method when it was an instance but now it will be never called.
Instead of calling
_.omit(this.get({plain:true}), privateAttributes);
I called
_.omit(this.get(), privateAttributes);
and for every other models I have, I override the toJSON method with
toJSON: function () {
//Return a shallow clone so toJSON method of the nested models can be called recursively.
return Object.assign({}, this.get());
}
So, what is the conclusion?
I'm already using the v4 form of toJSON:
// ... v4 Model definition
User.prototype.toJSON = function() {
return _.omit(this.dataValues, 'password');
}
Will scopes be usable in attributes or just on model?
is still WIP? This would be really useful!
Guys, are you serious? 2 years of discussion on how to hide attributes in model and still no solution?
@nonamez there are plenty of solutions, it's a trivial problem to solve in userland, and there are plugins too.
Most people solve this in their API layer, and that makes a lot more sense too in my opinion.
Please consider your attitude and tone when posting on open source issue trackers.
https://github.com/mickhansen/ssacl-attribute-roles is an unmaintaned plugin that somewhat does what you need.
I never found a use for it, hence the status of being unmaintained, i believe it's a lot more appropriate to keep presentation logic in your presentational layer (be that a REST API or whatever).
@mickhansen my attitude and tone is ok, just little surprised as such a simple feature need so much work...
My situation is quite simple: I have user model in which I overwrite toJSON as suggested above and was surprised when I tried to get all messages from other model and included in it user and all my passwords are seen...
So basically I need to add double check in each model that is associated with user model to hide password attribute.
@nonamez I disagree.
Plenty of solutions have been proposed, take your pick.
If i were to make a personal suggestion i'd take a step back and consider why model instances are being piped directly to end users.
@mickhansen Well, as for me, one of the main tasks of ORM is in reducing development time by solving routine tasks.
For example one of those task is data validation/escape before saving - roughly speaking "data input".
On the "data output" in most cases we just need to return some data adding few where statements.
For example users private messages, or users comments, or users posts. And each time when we simply want to get comments for the post we need not forget to do some extra manipulations (which are the same for messages, comments, posts) to exclude sensitive user data.
Why we simple cant define property hidden: ['password', 'auth_token'] and exclude those items each time when model is called for serialization ? So that when we simple need to pipe post comments or user comments we do not need to do some extra stuff. Just call model and be calm.
Why we need to search for some sort of second-hand solutions ? Can we even trust those second-hand solutions?
@nonamez When asking why a feature doesn't exist you can usually assume the answer is "bloat" or "no one has implemented it".
I've given my personal reasons for why i think such a feature doesn't need to exist, i don't believe more can be added to the discussion.
I'm no longer an active maintainer, but PRs are always given appropriate attention i'm sure. Just remember the one true rule of OSS; "No is temporary, yes is forever".
Most helpful comment
@nonamez there are plenty of solutions, it's a trivial problem to solve in userland, and there are plugins too.
Most people solve this in their API layer, and that makes a lot more sense too in my opinion.
Please consider your attitude and tone when posting on open source issue trackers.