Casl: Question about operating with fields that have access restrictions

Created on 10 May 2020  Â·  6Comments  Â·  Source: stalniy/casl

After some playing with it and cross-referencing with the docs for my current requirements, I'm still not quite sure if I'm doing it right. Would like to ask if there are other ways of doing things as I'm quite uncertain at this point.

Scenario:

  1. A user can have many roles.
  2. There are 2 roles concerned about an update operation, HR and the default role for employees. The default role applies to everyone so HR also inherits the default role along with its associated permissions.
  3. HR can edit more fields (e.g. Job Title, Employment Status) for an employee (can only edit basic profile).
  4. I use TS w/ ObjectionJS + GraphQL and there's only one mutation operation for editing the employee entity/model.
  5. On the frontend side, there's only one component for both create/edit forms so there'd be fields that are hidden/shown based on your role and permissions for certain fields.
  6. I'd like to pick fields based on the user's permissions when doing an operation that involves writing to the database.

So when editing information about an employee, depending on your role, there'd be certain fields that shouldn't go through the Objection model.

Say we have the Employee model as:

class Employee {
  id: string;
  firstname: string;
  lastname: string;
  workEmail: string;
  jobTitleId: string;
  employmentStatus: EmploymentStatus;  // where EmploymentStatus is an enum
}

If you have the default role, you

can('update', 'Employee', ['firstname', 'lastname']);
cannot('update', 'Employee', ['jobTitleId', 'employmentStatus', 'workEmail']);

If you have the HR role, you

can('update', 'Employee', ['jobTitleId', 'employmentStatus', 'workEmail']);
cannot('update', 'Employee', ['firstname', 'lastname']);

My questions/concerns are:

  1. I experimented with the permittedFieldsOf helper to pick fields from the graphql mutation input but I noticed that the order matters as the inverted/cannot rules can unset a field. Now since HR inherits the default, this is fine when the rules are defined in that order (apply rules for default role first then rules for HR role). But when you start adding more roles to a user that also have different field access restrictions, the order returned by the database can be unpredictable which then affects the order and can then affect the results of permittedFieldsOf. One way to fix this is to ensure the order of the roles by sorting it based on the "role hierarchy" of the app. But one potential issue of that is if there's a role higher than HR that can update firstname and lastname. That would replace the inverted rules of HR. Then some time in the future, another role is added higher than that which cannot update firstname and lastname and now there's an issue of which one to follow. Since there's no and/or operators, we'll have to rely on the order of the rules now instead of the roles.

But if our implementation of the rolePermissions is the same as the one below, then there's no way to reorder the rules.
image

So to get around this, we instead return an array of RawRule then sort it accordingly but that can get messy/hard to maintain.

Ultimately, what one would want here is to get the fields from the can rules for picking from an object. But since an inverted rule could unset a field if it comes afterward can rules, now we lose the ability to pick that field since it won't be returned by the permittedFieldsOf as that relies on the order of the rules. There seems to be no built-in way of achieve this currently.

What's the best way then to go about getting all fields from non-inverted rules for that action and subject? Accessing the rules from the ability object and extracting non-inverted rules then use permittedFieldsOf is one way I could think of to achieve that but is there a better way currently? I'm not sure if this is common scenario but if it is, then can we possibly consider adding an ignoreInvertedRules option (which accepts a boolean value, false by default) in the PermittedFieldOptions (4th parameter of permittedFieldsOf helper) that does that?

  1. Once you start having field access restrictions for certain subjects and actions, is it then expected for other rules with the same subject and action to follow suit since one would most likely be picking the fields during create/update/some-custom-action? Wildcards don't work on lodash's pick so this got me wondering if there's a better way to manage the picking of permitted fields when there are field access restrictions.

  2. On which layer do you think should this checking/picking occur?

  3. As a middleware, before/after validation
  4. On the interface layer (controller for REST / resolver for GraphQL)
  5. On the business logic layer (services/handlers, functions would then would somehow need to accept the ability and actor/user objects)
  6. On the data access layer (saw the objection-authorize plugin and I guess that works too but the interface layer shouldn't be directly performing any operations from the data access layer so the ability object gets passed first to the business logic layer then to the data access layer.)
question

All 6 comments

The question is quite too long :)

permittedFieldsOf returns fields based on defined permissions only. It doesn’t care on the order of fields in object.

According to your domain, I guess your permissions are wrong!

If I a regular user and an HR at the same time then I should have these permissions (in my opinion):

  • I can update details about myself (this is default permission)
  • I can update details about other employees because I’m an HR

If we put this together:

can(“update”, “Employee”,  [“firstName”, “lastName”], { id: user.id })
can(“update”, “Employee”,  [“jobTitle”, “etc”])

Then use permitted fields extractor by passing respective model inside:

const employee = {} // model instance here obviously
const me = {}

permittedFieldsOf(ability, “update”, employee) // [jobTitle, etc]

permittedFieldsOf(ability, “update”, me) // [firstName, lastName  ]

Don’t overuse cannot. “Give” permissions not “take” out them. Inverted logic are always harder to thing about! And I guess I’ll remove rule shadowing for cannot In the next major release (not sure yet) because as you have already seen in complex cases it confuses

I’d do this on controller/middleware level. Permissions are not BL concern

Could you please move this question to stackoverflow? So others can find it.

I close this, let’s continue discussion on stackoverflow if you have other questions

permittedFieldsOf returns fields based on defined permissions only. It doesn’t care on the order of fields in object.

According to your domain, I guess your permissions are wrong!

I have the conditions set in my code so no worries there 🙂 The permittedFieldsOf helper ignore what's defined in the conditions and focuses more on the fields so I thought there was no need to include that. The concern was more on how the permittedFieldsOf helper extracts fields from the rules when the ability has cannot/inverted rules, it causes undesirable behavior if one do not pay full attention about the _order of the rules_ and so if one has to really use cannot, one has to start thinking about the order and it can get messy as described above.

Don’t overuse cannot. “Give” permissions not “take” out them.

But then again, the inverted rules could also be written in its can/non-inverted equivalent so by always going with giving permissions consistently, that makes it easier to reason about. You might want to put that in the docs as a tip. I think it'd be good to set one's mental model around that as it can be tempting to use cannot when it could've been written in its can equivalent🙂

Thank you for the input and taking the time to answer!

Nice to know it solves the issue :)

I wrote about the complexity of inverted rules in the intro guide I guess. But yes, probably it will be good to add a tip as a statement “give permissions, don’t take them away”

added "Best practice" section to "Define rules" page - https://stalniy.github.io/casl/v4/en/guide/define-rules#best-practice

Was this page helpful?
0 / 5 - 0 ratings