Casl: Abilities don't behave the way I expected from the docs

[federico-terzi]

Hey @stalniy,
First of all, thank you for the great library. I've been going through the docs and, no matter what, I can't make the rules work as I would like to.

Describe the bug

My goal is to define a set of abilities so that we can only access a Person with id = 2. Therefore:

• I shouldn't be able to access a Person with an ID != 2
• I shouldn't be able to access a Person without specifying an ID

To Reproduce

Minimal example to reproduce:

import { Ability, InferSubjects, AbilityBuilder, subject } from "@casl/ability";

export type Actions = "create" | "read" | "update" | "delete";

interface Person {
  id: number;
}

export type SubjectTypes = Person | "Person";
export type Subjects = InferSubjects<SubjectTypes>;
export type MyAbility = Ability<[Actions, Subjects]>;

const getAbilities = (): MyAbility => {
  const { can, cannot, build } = new AbilityBuilder<MyAbility>();
  can("read", "Person", { id: { $in: [ 2 ] } })
  return build()
}

const abilities = getAbilities()
console.log(abilities.can("read", "Person"))
console.log(abilities.can("read", subject("Person", {id: 1})))
console.log(abilities.can("read", subject("Person", {id: 2})))

Expected behavior
I would expect to get:
-> console.log(abilities.can("read", "Person")) -> False (because I should only be able to access Person with ID = 2)
-> console.log(abilities.can("read", subject("Person", {id: 1}))) -> False
-> console.log(abilities.can("read", subject("Person", {id: 2}))) -> True

What I get instead is true to all the conditions.

I also tried using defineAbility instead:

const abilities = defineAbility((can) => {
  can("read", "Person", { id: { $in: [2] } });
});

and in that case, I get:
-> console.log(abilities.can("read", "Person")) -> True (shouldn't it be false?)
-> console.log(abilities.can("read", subject("Person", {id: 1}))) -> False Ok
-> console.log(abilities.can("read", subject("Person", {id: 2}))) -> True Ok

Interactive example

https://codesandbox.io/s/modern-breeze-q5ue1?file=/src/index.ts

CASL Version

@casl/ability - v 4.1.6

Environment:

Windows 10, Node JS: v12.16.1

Do you have any idea why I'm facing these problems?
Thank you for your time

[stalniy]

Hi,

It's all described in the guide:

• there are 2 concepts, subject type and subject instance
• the behavior is expected and explained here - https://casl.js.org/v4/en/guide/intro#checking-logic

[federico-terzi]

Thank you @stalniy, I missed that point! Sorry for the hassle

[J3m5]

This topic is coming again and again, and I've been thinking about that.
Maybe a "strict" flag or something could solve this issue ?
A flag that when passed would force the rule to return true only if a subject instance is passed ?

[stalniy]

If you use typescript, you should be able to restrict this through types

[J3m5]

I'm not using Typescript, not yet, but would it works with rules defined in JSON and stored in a DB ?
Could it be added on the AbilityBuilder for enforcing this behavior as a default and we will then need to opt out explicitly for allowing rules without subject instance ?
And what if an object instance return undefined, the rule would return true by mistake ?

[stalniy]

I’ll create a section or page in docs which shows how to do this. It works for dB rules as well because eventually what you want to do is to restrict what developers can pass in ability.can as the 2nd argument.

I’d been thinking about this when was working on types for v4. It should be enough.

[J3m5]

This could be done on the user side, sure.
but what about those that don't use Typescript ?

[stalniy]

This will NOT be implemented for JS users because it introduces a completely different logic to rules processing. Currently, if you do:

can('read', 'Post')
can('read', 'Post', { authorId: 1 })

will allow to read ANY post because the 1st statement says you can read all.

The requests from other users requires this to work differently. They want:

can('read', 'Post', { authorId: 1 })

To allow read posts that has authorId = 1 but not ability.can('read', 'Post') but this is half of the story. Later, they will have a question: "how can I check permissions without having an instance?" And a logical answer to this will be (in their minds):

can('read', 'Post')
can('read', 'Post', { authorId: 1 })

So, they will expect this code work like this:

ability.can('read', 'Post') // true
ability.can('read', a('Post', { authorId: 1 })) // true
ability.can('read', a('Post', { authorId: 2 })) // false

Eventually this will require to implement code with different rules processing strategies. I don't want to introduce complexity just because someone don't want to read docs carefully.

[stalniy]

However what can be done for js users is eslint-rule that will make sure they don't use subject types in ability.can. This is not a goal of CASL anyway, so if somebody really wants they can do it.

But better to use TypeScript (by the way, for a long time I was a TypeScript hater, now I like it, it's more mature than it was years ago and helps to write more efficient JS code)

[J3m5]

I understand all that, I've personally read he docs carefully and I understood the logic behind this rule.

But I understand users that don't want do be able to make a mistake or forget this rule or forget to pass the subject instance or like I said, having a subject instance that is undefined for whatever reason.

There are a lot of reason that can make it go wrong.

And about pure JS, I was no asking for an implementation in CASL but maybe a recommended way to add a safe guard on the user side.
Or it can be in CASL but without adding different rules processing strategies.
We could add a module that would implement it as a wrapper around the ability instance or that would extend it.

And IMO, I don't think adding a security feature only available with typescript is a good idea.
I'm not a Typescript hater, I've used it a bit and I'm planning to use it a some point but for now we have other priorities.

[J3m5]

I'm thinking about it.
The fact is, those two case could be expressed completely differently.
In the first case

ability.can("read", "Article", article)

The user can read this article, because it respect the conditions.

ability.might("read", "Article")

The user might read an article depending on the article.

It's all about semantic.

_Can_ only indicate a strict _permission_ or _ability_
_Might_ only indicate a _probability_ or _slight possibility_

If we push the logic and follow the semantic strictly, we should use _may_ instead of _can_.
Because _can_ express a capacity or ability where _may_ express a permission or logical possibility.
The difference is subtle and in some context the usage is interchangeable.
But this is another story.

And it's not about the way the rules are stored, processed or validated but more about how the methods are used and accessed and in what contexts.

IMO, it falls under the separation of concerns principal and apply 2 of the SOLID principles, Single Responsibility and Interface Segregation.

Each methods does one thing:

• give the permission to take an action on a specific item under certain conditions
• indicate the probability of an action to be taken on a type of item.

I hope you'll find those thoughts interesting and constructive.

[stalniy]

First of all, thanks for the thoughtful suggestion, I appreciate that.

From the first sight, it looks like a solution. Let me explain how I understood it: basically instead of having 1 method can, we add another one may (might in your proposal). can should be used when we want to check instances (i.e., by conditions) and may when we check by subject types (no instance available).

This philosophy works pretty good in case there are only conditions based rules but what about this case:

can("read", "Article")

Isn’t it strange that we explicitly said user can read any article and later do checks using something like might?

[stalniy]

Just to iterate, from another point of view why I think this feature is redundant.

The simple case:

1. I give you a red apple
2. And later, may ask you 2 questions:
   
   
   
   • do you have a red apple? You: yes
   
   
   • do you have any apple, at least one? You: yes
   
   

Do you see the analogy with giving permissions to somebody?

So, in my opinion, it’s a common sense, regular old fashion logic :) I don’t want to make something not logical because then you need to know additional “rules” how custom “logic” works in this case.

Another my opinion, is that the best things in the world was made by observing how “the world” actually works.

[stalniy]

There will be always “some guy” who don’t agree and it’s fine :)

[J3m5]

The way I see it is that you wouldn't be able to call can("read", "Article") without a subject, it would automatically return false and throw an error (or a warning).
And in the same way, you couldn't call may("read", "Article", { authorID } ) with the subject, it would automatically return false and throw an error (or a warning).

You are asking for two different things.
You use can to check with a subject and may ( or might ) without one.
They have two different purposes.

I see it more like that:

I have a big red apple.

Rule:

can("have", "apple")

• You: May I have an apple ? - Me : Yes

• You: Can I have this big red apple ? - Me: Yes.

  Rule:

 can("have", "apple", { color: "green" })

• You: May I have an apple ? - Me : Yes
• You: Can I have this big red apple ? - Me: No, not this one.

You see, depending on the rule, two purposes, two different questions, two meanings, two verbs, two usage, two responses.
Clearly separated and can't be used the same way.
There is no ambiguity.
Because currently, can doesn't do the same thing depending on whether you pass it 2 or more parameters.

Isn’t it strange that we explicitly said user can read any article and later do checks using something like might?

The difference is that for me and for others, it isn't any article, it is the type article.
When you define the rule:

can("read", "Article", { authorID } )

You are not saying:
"you can have access to any article but if you ask for a specific one, it must match with theses conditions".
You are saying:
"you have only access to this type of resource only if it match with these conditions."

And when we check for permissions, we are not necessarily aware of the rules, so no, it's not strange.
Might act as a "fail fast" function or "early return".
If you know that you don't have access to a type of resource, it's unnecessary to check with specific subjects.

Do you see the analogy with giving permissions to somebody?

Same answer, the fact is that the person is not aware of its permissions, it doesn't know what it can / might do.
And it might want to ask different questions.

[J3m5]

Maybe I can create an issue and propose it like an RFC and see what others think about it ?

[stalniy]

RFC is a good idea. Take into consideration:

1. rule definitions through AbilityBuilder (so we don’t need to add may to AbilityBuilder)
2. accessibleBy method in @casl/mongoose. Will it be clear what records will be returned if you define both can have Apple and can have green apple

So, we don’t confuse people more

[Todmy]

@J3m5 I have read your proposal and I see your pain. But I cannot understand what is the case when you need to implement such logic. I mean not from a programming perspective but from a user experience. You have mentioned that you have faced it a few times, but what was the case? When do you need to check that you can read exactly ALL Post not SOME of them?

[J3m5]

One of the case is in a pub/sub context.
A user is added in a channel in order to subscribe to events related to a type of resource.
I don't have the resource item to test ahead of time.
When an event is triggered with the actual item, I check if the user can read this item based on the rules conditions and the items.

The detail is that I don't want to check if I CAN read ALL the Post, but if I MIGHT read a resource of type POST, or SOME Post.
If I MIGHT read some Post, then I subscribe to the related events, and before one is sent, I check if I CAN read THIS POST.

And of course, some users have full access to theses resources, some on conditions and other not at all.

[J3m5]

With this in place, those who prefer to enforce strict permission checking with defined subjects, would always use can.
And when you only need to check permissions without subject nor conditions, to see if you have access to a type of resource, you use may or might.

If you already and always use can with a subject, it's fine, it doesn't change anything for you.
But you won't be able to use can without a subject anymore, you'll need to use may.

I'll try to submit the RFC in the coming week, I'll compile and organize what I said in this issue.

[stalniy]

I read about grammar difference between may and might. And the last one fits better. So let’s stick to “might” for now

[J3m5]

Yep, that was my conclusion too.