Rules with empty field array

[qqilihq]

The following might be a far-fetched use case (I tried it as a workaround when implementing it into our current project), but I’d be really interested in some feedback.

I define rules which permit the admin to access a/some fields in the article subject. For the normal user role I defined an empty array, which should mean: “Do not allow to access any fields” (I realize, that it would be more appropriate to simply give no permissions at all, but it was needed as workaround in our environment).

function defineAbilities(role: 'user' | 'admin') {
  const { can, rules } = new AbilityBuilder<Ability>();
  can('get', 'article', []);
  if (role === 'admin') {
    can('get', 'article', ['secret']);
  }
  return new Ability(rules);
}

When I check the permitted fields for the user type, I get an empty array as expected:

const abilities = defineAbilities('user');
permittedFieldsOf(abilities, 'get', 'article'); // []

… but when I check my ability to get the secret field, this returns true:

abilities.can('get', 'article', 'secret'); // true

I’d appreciate any feedback!

Complete test case:

import { Ability, AbilityBuilder } from '@casl/ability';
import { permittedFieldsOf } from '@casl/ability/extra';

describe('abilities', () => {
  function defineAbilities(role: 'user' | 'admin') {
    const { can, rules } = new AbilityBuilder<Ability>();
    can('get', 'article', []);
    if (role === 'admin') {
      can('get', 'article', ['secret']);
    }
    return new Ability(rules);
  }

  describe('as admin', () => {
    const abilities = defineAbilities('admin');
    it('has permitted field secret', () => {
      expect(permittedFieldsOf(abilities, 'get', 'article')).toEqual(['secret']);
    });
    it('can get article secret', () => {
      expect(abilities.can('get', 'article', 'secret')).toBeTrue();
    });
  });

  describe('as user', () => {
    const abilities = defineAbilities('user');
    it('has not permitted fields', () => {
      expect(permittedFieldsOf(abilities, 'get', 'article')).toEqual([]);
    });
    it('cannot get article secret', () => {
      expect(abilities.can('get', 'article', 'secret')).toBeFalse(); // <-- fails
    });
  });
});

[stalniy]

Frankly speaking, the rule definition can('get', 'article', []); has no any sense in my mind, it says "user can get article but cannot get its fields", so basically this is the same as returning an empty object :)

CASL ignores empty arrays for fields here. So, passing an empty array and passing nothing is the same. CASL thinks that user have access to all fields. That's why you get true

permittedFieldsOf works a bit differently. It just returns unique fields from all matched rules. And in case there are no fields it provides a hook function, that allows to return all fields (this is how it's used in @casl/mongoose):

permittedFieldsOf(ability, action || 'read', subject, {
      fieldsFrom: rule => rule.fields || ALL_FIELDS
    });

[stalniy]

Hopefully, it makes sense. So, close as there is nothing to do from casl side.

Thanks for using casl. If you have any suggestions feel free to leave a comment later

[qqilihq]

Thanks so much for the clarification. I understand that passing an empty array is nonsensical -- however, considering the fact that people write security sensitive code, I would very much appreciate if the library would threw a validation error, to prevent accidental misusage.

Thanks for your work and the feedback!

[stalniy]

What is your intention to pass an empty array of fields?

[stalniy]

For now, casl will log a warning to console because this is potentially a breaking change

[stalniy]

will change to error in the next major version

[stalniy]

released in @casl/ability@4.1.3

[qqilihq]

Thanks for the update!

What is your intention to pass an empty array of fields?

I’m currently integrating CASL into a larger project with an existing structure which was using a custom auth solution so far. For now (during the transition phase) I did not define all entities’ attribute in the rules, but only put the “sensitive” ones there, which are not supposed to be accessed by everyone (I realize, that CASL is not designed to be used like that and that we should get this straight).

E.g. article has properties title, text, … (many more) which can be accessed by everyone anyways. Only secret can be accessed by the admin.

Taking a step back, what would have helped me here was the ability to negate or exclude attributes access, such as:

“admin can access all properties of article” “user can access all properties of article EXCEPT secret”

Not sure if this is possible or even makes sense :-)

[stalniy]

Use cannot to do this

[cyrus-za]

I know the issue is a few months old, but I believe it's worth pointing out that, the result here is helping me a lot. It might seem strange to pass in an empty array as it seems nonsensical, but with a data-driven solution, one requires an 'all' value and defaulting to an empty array to allow all fields is great.

[stalniy]

in v5, casl throws error when empty fields are found