I found that when using Auth Strategy, it is still possible to create an admin user using graphql anonymously. To prevent this I had to set default access on Keystone instance or individual list.
I propose to have Access Control Policies
I am also thinking the authentication scenario should be a short hand for imperative/static access
type StaticAccess = boolean | 'auth' | 'own';
question is how do we find owner user, it would be easy for user list but for other, it may not be much difficult as we can have createdBy meta field. another way to make list configuration for own item assignment. for example
keystone.createList('Post', {
fields: {
author: {
type: Relationship,
ref: 'User', // we can infer here but it is highly likely that ref field may not be where you want to add restriction.
access: ({ authentication: { item, listKey }, existingItem }) => {
return true;
},
},
},
ownerResolver: item => item.author, // here we can use the resolver for owner
});
We can also have delegated access, like createdBy field can be meta, and user can change this field in future, but there can be assigned field. like I post the blog for another user, I become createBy but I can add that user as author, then that user should also be able to change the blog. then I should change the config like this
ownerResolver: item => [item.createdBy, item.author],
Default Access Control policy should kick in when you use any authentication strategy
It is already possible to set a default at the keystone level:
const keystone = new Keystone({
defaultAccess: {
list: true,
field: true,
}
});
And/or on a per-list level (which overwrites the keystone level):
keystone.createList('User', {
access: true,
});
Default policy should be Opt out
I agree - there was discussion about this in https://github.com/keystonejs/keystone-5/pull/118#issuecomment-396455389
It should prevent update to relational fields for user (like post.author -> User) in case a list is excluded from policy
There are two separate access controls at play here: 1) for the post.author field, it needs to have update: true, and also for the User list, if you're creating a new item, it also has to have create: true. This usecase is already covered by existing access control logic.
I am also thinking the authentication scenario should be a short hand for imperative/static access
I'd love to see this as another module! I could imagine an API something like the following:
const keystone = new Keystone(/* ... */);
const authStrategy = keystone.createAuthStrategy({ list: 'User' });
const accessOnlyByOwner = createOwnerAccess('User');
keystone.createList('Post', accessOnlyByOwner({
fields: { /* .. */ }
}));
Where createOwnerAccess is something like:
const createOwnerAccess = list => config => ({
...config,
access: () => { /* ... */ }
});
We can also have delegated access, like createdBy field can be meta, and user can change this field in future, but there can be assigned field. like I post the blog for another user, I become createBy but I can add that user as author, then that user should also be able to change the blog. then I should change the config like this
Personally, I'd prefer this to be explicit:
keystone.createList('User', { /* ... */ });
keystone.createList('Post', {
fields: {
title: { type: Text }
authors: { type: Relationship, many: true, ref: 'User' },
},
access: ({ authentication: { item }, existingItem }) => existingItem.authors.contains(item.id),
});
Do you think it's possible to implement the suggested features as higher order functions used when configuring lists? Or do you see it as a more core-feature built into Keystone?
The current access control stuff is very low level and was designed to be a base for us to build off, but I'm not sure where that should live - in core, or in userland.
It is already possible to set a default at the keystone level:
You can only say true or false, or have to return Boolean from function. I suggested shorthand for inbuilt function like auth | authenticated should automatically run (authentication) => !!authentication.item
There are two separate access controls at play here: 1) for the post.author field, it needs to have update: true, and also for the User list, if you're creating a new item, it also has to have create: true. This usecase is already covered by existing access control logic.
this is in play only after you setup access logic for each list/field. My suggestion is to have this baked in with zero configuration. moment I use auth strategy, the policy should prevent modification of such fields anonymously, meaning post.author should be protected even though there is no access config defined for list or field. Opt Out means, you now have to configure allowing anonymous changes versus existing opt in (config the access control)
Do you think it's possible to implement the suggested features as higher order functions used when configuring lists? Or do you see it as a more core-feature built into Keystone?
I like higher order functions, it could be outside of core package if api is enabled. I would incline towards core-features with option for others to build upon.
some of the ways these higher order function or say access modifiers be available:-
access modifiers as part of config option and run them immediately or part of prepare/connect callaccess modifiers as part of additional arguments [array] and run them immediately or part of prepare/connect callAnyone can create their own access modifiers or possibly other use case. I see this as one time immediately invoked hook.
const { Keystone, enableAuthenticatedAccess, enableOwnerOnlyAccess }
const keystone = new Keystone(/* ... */);
const authStrategy = keystone.createAuthStrategy({ list: 'User' });
keystone.createList('Post', enableAuthenticatedAccess({
fields: { /* .. */ }
}));
// ----- modifiers as part of argument
keystone.createList('Post', {
fields: { /* .. */ },
}, [enableAuthenticatedAccess, enableOwnerOnlyAccess]);
//---------- defaultModifiers as part of config
const keystone = new Keystone('My App', {
// Initial values shown here:
defaultAccess: {
list: true,
field: true,
},
modifiers: [enableAuthenticatedAccess, enableOwnerOnlyAccess]
});
Personally, I'd prefer this to be explicit:
having explicit makes keystone unique from other node based cms, less convention means more developer friendly. (like strapi.io is more like ui based content type/list creation, which I would not prefer over Keystone). At the same time some of the things can be added for ease of use, you would still have an option to go explicit. similar to recent changes in how you run Keystone using cli and --entrypoint in addition to being explicit in server.js/index.js
The current access control stuff is very low level and was designed to be a base for us to build off, but I'm not sure where that should live - in core, or in userland.
userland is awesome for creativity, I feel basic security related stuffs should be in core. that will also serve as examples for userland
These are all really great suggestions!
For potentially complex implementations like this, we've used a README-based RFC process in the past to think through the problem: We'll write out most of the documentation we'd expect for the proposed feature(s) into a README.md somewhere, and open a PR containing only those docs, eg; https://github.com/keystonejs/keystone-5/pull/118
Would you like to write up some/all of these features as a README-based RFC so we can get a more complete feel of what the APIs would be?
It looks like there hasn't been any activity here in over 6 months. Sorry about that! We've flagged this issue for special attention. It wil be manually reviewed by maintainers, not automatically closed. If you have any additional information please leave us a comment. It really helps! Thank you for you contribution. :)
Most helpful comment
These are all really great suggestions!
For potentially complex implementations like this, we've used a README-based RFC process in the past to think through the problem: We'll write out most of the documentation we'd expect for the proposed feature(s) into a
README.mdsomewhere, and open a PR containing only those docs, eg; https://github.com/keystonejs/keystone-5/pull/118Would you like to write up some/all of these features as a README-based RFC so we can get a more complete feel of what the APIs would be?