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.

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

Problem with allowed fields and patch action #229

Closed
bodo22 opened this issue Oct 10, 2019 · 3 comments
Closed

Problem with allowed fields and patch action #229

bodo22 opened this issue Oct 10, 2019 · 3 comments
Labels

Comments

@bodo22
Copy link

bodo22 commented Oct 10, 2019

Hi, thanks for this great package, its really a big help!

I am having problems wrapping my head around how casl handles ability checking when fields and limiting is involved. I am using mongoose.

When implementing a rule like so

{
  actions: ['patch'],
  subject: ['Post'],
  fields: ['isPublic'],
  conditions: { title: { $in: ['Super awesome title', 'Really fun title']}},
}

then I expect casl to evaluate ability.can('patch', {isPublic: true}); to true, because a patch is just a patch, i'm not trying to replace the dataset, just updating the specified fields.

Am i thinking about this the wrong way, should there be different handling between 'patch' & 'update' ?

One workaround is to fetch the current dataset and check the ability with that instead of the new one, and afterwards always use lodash.pick to constrain the new dataset.

Another workaround would be to add undefined to the $in array.

But those feel a little wrong. Do you have ideas on how to tackle this problem? Maybe implement different handling between update & patch operations?

Thanks!

@stalniy
Copy link
Owner

stalniy commented Oct 11, 2019

Hi,

Checking by fields is a separate thing. So, your permissions says:
Can patch Post field isPublic if post’s title is one of ...

So, then you have several ways to check permissions:

  1. Can I patch at least one field of the post:
    ability.can(“patch”, post). Basically this just asks whether user can patch this post
  2. Can I patch isPublic field of the post: ability.can(“patch”, post, “isPublic”)
  3. Which fields of the post can I patch?
    permittedFieldsOf(ability, “patch”, post) -> returns an array of fields which can be patched: [“isPublic”]. Later you can use lodash.pick and check whether such fields exist in req.body

@bodo22
Copy link
Author

bodo22 commented Oct 14, 2019

Ah thanks a lot for the explanations. Would it make sense to, instead of only passing a single field key to the ability.can() function, also being able to pass an array? Then one could pass the return value of the permittedFieldsOf() function.

@stalniy
Copy link
Owner

stalniy commented Oct 15, 2019

There is no sense to pass the result of permittedFieldsOf into ability.can as you for sure will get true. Casl doesn’t check fields on the passed subject (2nd argument in `ability.can), it just checks whether that field is specified in permissions/roles for a particular subject and action.

I considered passing an array of fields as a 3rd argument to ability.can but there is an issue:

  • some people will expect ability.can(“read”, post, [“title”, “summary”]) to return true when user can read both title and summary fields on post (AND logic)
  • others will expect to get true when on of the fields can be read

I haven’t solved this issue yet, that’s why the 3rd argument is a single string

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants