keystone-test-project
keystone-test-project copied to clipboard
keystonejs
List Permissions Proposal
This is a proposed implementation of List Permissions that may be supported by Keystone.
To explain what's going on:
Idea 1
You can dynamically change list and field options (specifically hidden, noedit and nodelete) based on the current user / item data. This lets you customise what's displayed in the Admin UI for a particular user (could be a permission group)
To do this, you can set the following methods on the List instance:
userOptions (user) => { /* list options to override for the user */ }
userFieldOptions (user) => { /* field paths containing options to override for each field */ }
itemOptions (item, user) => { /* list options to override on a per-item basis */ }
itemFieldOptions (item, user) => { /* field paths containing options to override on a per-item basis */ }
Idea 2
You can directly modify the queries used (after filters are applied) for Admin UI endpoints, including:
- get item(s)
- update item(s)
- delete item(s)
- download export
You could use this, for example, to limit which items a user can see or interact with in any way.
To do this, you could set the following methods on the List instance:
modifyListQuery (user, query) // modify the query by reference, based on the current user
modifyItemData (user, item) // update properties on the item before it is saved
Example use case
This PR implements these features to achieve two use cases:
- Protecting the demo user from being updated or deleted using generic functionality
- Adding a concept of Editor Users, and implementing restrictions on Posts, where non-editors can only see their own posts, can't change the state of a post, and can't edit certain fields after the post has been published
The API is a bit unnecessarily verbose, isn't it?
Why not something along the lines of
Post.permissions = {
userFieldOptions: () => {},
/* … */
};
@mxstbr happy to simplify it, that's why this is a proposal :smile:
Not sure how that would be different though, other than nesting the methods under a permission object? Could you propose the full API you'd like to implement the rules I've included in this PR?
Would dynamic permissions reflect across all adminUI Screens, including Home screen?
In idea 1, are you missing the field path argument for those functions which act in a field path?
I kind of like this. Would it work across inheritance in case I want to group permissions based on inheritance?
@webteckie in order:
How about nocreate ... can that be controlled as well?
Yes
Would dynamic permissions reflect across all adminUI Screens, including Home screen?
Yes
In idea 1, are you missing the field path argument for those functions which act in a field path?
No, this would be processed once and return an object containing field paths with options that override the general options for each field, for a specific user
I kind of like this. Would it work across inheritance in case I want to group permissions based on inheritance?
I'm not sure yet.
@mrprkr re: terminology, what's going on here is I'm proposing a way to change some list and field options on a per-item and/or per-user basis.
We're not otherwise introducing anything new, so there'd be no changes to terminology or otherwise changes to the features themselves (at least not with this proposal)
I do agree that the double negative is a bit weird, not sure that I prefer booleans that default to true though, which is why that was chosen in the first place. At least we should be consistent.
re: your example, as I'm reading it, we could use the functions I've proposed to achieve what you want, it's just the syntax would be different.
The challenge with what you're suggesting is that the variables are evaluated when the code is executed unless they're wrapped in functions, making it more verbose. For example:
territory: { type: Types.Relationship, ref: 'Territory', noedit: !this._user.isAdmin }
In this case this._user.isAdmin would be evaluated to something when the list is created. You'd need to do something like:
territory: { type: Types.Relationship, ref: 'Territory', noedit: (user) => !user.isAdmin }
... we could do that, but I think it would be more complex to implement as compared to:
ContentList .userFieldOptions = (user) => ({
territory: { noedit: !user.isAdmin }
});
Using the single method instead of specifying the functions inline will optimise it for larger models, and lets you structure your rules more powerfully, e.g
var protectedFields = ['territory'];
ContentList.userFieldOptions = (user) => {
var options = {};
if (!user.isAdmin) protectedFields.forEach(i => { options[i] = { noedit: true } });
return options;
};
Some thoughts:
We can consider/leverage https://www.npmjs.com/package/acl
Global Permissions - SingleTon Object - Only single Object is allowed for this List/Table NoCreate - Only created through code and not by users NoDelete - cannot be deleted any any user Global Permissions supersede other permissions
Permission at List/Table level : [ "create", "view", "update", "delete", "search" ] // every List/Table specifies which user can do what on the List // view and "download/export" can be considered same // List/Table Permissions supersede Object/Row level controls
Permission at Object/Row level : [ "all", "createdByUser", "ownedBYUser", user_supplied_filter_condition_or_Function ] // implemented using Mongoose Middleware pre and post hooks // all create/view/update/delete/search should be complient with the filter_condition // filters can be createdByUser, OwnedByUser, Where Department="DepartmentCode" etc - implemented through a function // Object/Row level permissions supercede Field/Column level permissions
Permission at Field/Column level : [ "visibleFields" : explicit_list ] // if not mentioned all fields are visible // maps to the hidden property // implemented by Keystone ?
I'm inclined to implement this sort of low-level support and then do an acl feature separately, probably as an optional package (or maybe by mapping to an existing project like acl)
An example of how this could work would be exporting Users and Roles list definitions from a keystone-roles package, as well as some functions to plug those permissions into your app's lists which would hook into the functions I've described above.
Ideally we keep the features in keystone core as simple and powerful as possible, allowing for developers to build or customise their own permissions solution on top of it.
In particular, I'm trying to keep keystone as unopinionated in its core implementation as possible, then build on that with ready to use (but optional) features that can be added "out of the box", or that serve as a reference implementation if somebody wants to do something more custom.
@VinayaSathyanarayana if you can see something we'd need to integrate acl as you've described that's not supported by the features in this proposal, please let me know!
@JedWatson experimented a little with your branch and...
Given the following I would have expected the Admin User to not be able to edit the state (because he's not an editor -- even though he created the post):
Post.userFieldOptions = (user) => ({
state: {
// normal users cannot change the state of a post
noedit: !user.isEditor,
},
author: {
// author is hidden from normal users
hidden: !user.isEditor,
},
});
Likewise, once the Admin User was able to change the state to published the following logic, again, should not let the same Admin User update the name field of the post but it did:
Post.itemFieldOptions = (item, user) => ({
name: {
// non-editors cannot change the name of a published post
noedit: item.state === 'published' && !user.isEditor,
},
});
Or are there checks in place to let the Admin User do whatever he/she wants? Not sure that should be the case, if so.
We're using 0.3.x in production and badly need some form of permission, and this seems to be a good compromise until we get full ACL. Looking at the code changes, other than the model-transform dependency (which looks optional?) is it safe to assume we could use this almost as-is on 0.3.x?
Can anyone point me in the right direction, as to where I can call Jed's field option methods from idea#1 ?
The code changes to the 2 models (Pull Request #21) alone do not implement the List Permissions functionality, is that correct?
I was trying to look for when the lists are queried and created for the initial admin view. Any help would be greatly appreciated!
I'm really looking for similar functionality, will this ever make it into keystone?
Has there been any update on this feature? I really need it for my project.
Any update on this? https://github.com/keystonejs/keystone/pull/2111 has interesting stuff that seems more secure.
I don't know what you are looking for, but for me there is only one thing missing in the current implementation.
Keystone already support permissions with this:
const User = new keystone.List('User')
User.add({
name: { type: Types.Name, required: true, index: true },
email: { type: Types.Email, initial: true, required: true, unique: true, index: true },
password: { type: Types.Password, initial: true, required: true },
}, 'Permissions', { // << That is it
isAdmin: { type: Boolean, label: 'Can access Keystone', index: true },
isRoot: { type: Boolean, label: 'Administrator', index: true, dependsOn: { isRoot: true} },
isManager: { type: Boolean, label: 'Channel manager', index: true, dependsOn: { isRoot: true} }
})
So you can consider them as roles: Admin, Root, Manager ...
For me the missing part is that I would like to be able to do this:
const Model = new keystone.List('Model', {
noedit: function() { ... },
nocreate: function() { ... },
nodelete: function() { ...},
hidden: function() { ...}
})
If it could be possible to use a function instead of a boolean to set noedit, nocreate ...
and if that function can be bound some how by Keystone to an object containing the logged in User instance, and the actual model instance (when applicable, for example in noedit and nodelete), so in that function I can do this.user.xxx and this.model.xxx, then I think we should be done with all this.
Is that possible ?
