Keystone: Access Control Policies

Created on 6 Mar 2019  路  4Comments  路  Source: keystonejs/keystone

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

  1. Default Access Control policy should kick in when you use any authentication strategy
  2. Default policy should be Opt out, by default all list will be tagged with this policy and you have to exclude lists. User list should be locked in or You have to specifically say you want to remove user list from this policy.
  3. It should prevent write access in above case by default
  4. It should prevent update to relational fields for user (like post.author -> User) in case a list is excluded from policy

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],

access-control needs-review question / idea

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.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?

All 4 comments

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:-

  • use higher order function to wrap the field/list declaration (not very much readable)
  • add these access modifiers as part of config option and run them immediately or part of prepare/connect call
  • add access modifiers as part of additional arguments [array] and run them immediately or part of prepare/connect call

Anyone 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. :)

Was this page helpful?
0 / 5 - 0 ratings

Related issues

cowjen01 picture cowjen01  路  13Comments

bothwellw picture bothwellw  路  18Comments

ricardonogues picture ricardonogues  路  10Comments

amorote picture amorote  路  21Comments

bholloway picture bholloway  路  18Comments