Improved permittedFieldsOf behavior with allow('manage', 'all')

[jobarjo]

Hello,

I have been trying to combine the permittedFieldsOf utility function with the concept of admin e.g. allow('manage', 'all'). However, I found in case that the utility brought by permittedFieldsOf and allow('manage', 'all') was lost as I needed to explicitly define permissions for an admin.

Here is my context:

const defineAbilityFor = (user) => {
  const { can: allow, build } = new AbilityBuilder(Ability);

  if (user.isAdmin) {
    allow('manage', 'all'); 
  }

  allow(
    'patch',
    'Foo',
    ['bar'],
    { userId: user.id },
  );

  return build();
};

Let's say I have the following input data:

const admin_user = {
  isAdmin: true,
  id: 1
};
const normal_user = {
  isAdmin: false,
  id: 2
};
const other_normal_user = {
  isAdmin: false,
  id: 3
};

const foo = new Foo({
  bar: 'abc',
  userId: 2
});

const ability_for_admin = defineAbilityFor(admin_user);
const ability_for_normal_user = defineAbilityFor(normal_user);
const ability_for_other_normal_user = defineAbilityFor(other_normal_user);

As detailed in CASL documentation, I'm then testing on incoming patch requests the permission with

function isAllowed(ability, action, target) {
  const permittedFields = permittedFieldsOf(ability, action, target);
  isAllowed = Object.keys(req.body).every((key) => R.includes(key, permittedFields));
}

This gives us the following results:

isAllowed(ability_for_admin, 'patch', foo) // [] (1)
isAllowed(ability_for_normal_user, 'patch', foo) // ['bar'] (2)
isAllowed(ability_for_other_normal_user, 'patch', foo) // [''] (3)

All this to say that I find unfortunate that results (1) and (3) are similar when 1 is an admin with complete rights and 3 is a user not allowed to make any modification on that specific incoming request.

I feel there is a missing piece with permittedFields allowing to understand that all fields are accessible for a situation where allow('manage', 'all') is used.

As per proposed solutions, I would make permittedFields return all fields defined in the ability under other scopes; so for the above, it would return bar. This solution does not cover other fields that are not defined in the file and to which an admin could have right to modify, but I couldn't find something for that case.

[stalniy]

Thanks for the issue. But this case is already covered. There is no way for casl to know what means allFields. You need to pass the 4th argument to override fieldsFrom callback. Check the api docs and reference implementation in @casl/mongoose

In casl v5, that parameter is mandatory. So, this confusion will disappear very soon

[jobarjo]

Thanks @stalniy; not seeing documentation on the option parameter I admit I passed through it (maybe a few words in the doc about it would be nice?).

Thanks to your input, I ended up doing something like this which works well:

const setOptions = (user) => {
  const fieldsFrom = user.isAdmin
    ? () => (['all', 'the', 'fields', 'here'])
    : undefined;

  return {fieldsFrom};
}

[stalniy]

V5 will be available very soon I’m currently in the process of updating examples, so in the next docs there will be other examples