feathers-permissions icon indicating copy to clipboard operation
feathers-permissions copied to clipboard

Unexpected behaviour when using multiple "checkPermissions" in one method

Open noemission opened this issue 4 years ago • 0 comments

Steps to reproduce

export default {
  before: {
    all: [authenticate('jwt')],
    get: [
      // Allow only users with the "products:get" permission
      checkPermissions({ roles: ['products:get'] }),

      // The user will only get his product unless he is an admin
      checkPermissions({ roles: ['admin'], error: false }),
      (context: any) => console.log(context.params.permitted, context.params.user),
      iff(
          (context) => !context.params.permitted,
          setField({
               from: 'params.user._id',
               as: 'params.query.user',
         })
      ),
    ],
  }
}

Expected behavior

I was expecting this code first to allow users with the products:get permission and second to check if the user is an admin or not. In case he is an admin don't limit the results, otherwise, let the user get only the products that belong to him.

Actual behavior

The context.params.permitted is always true no matter whether the user is an admin or not. I believe this is due to the order here. The second call which is making the permitted: false is overwritten by the first one since you are destructing ...params after setting the value of permitted. This makes the first call to checkPermissions to have the max priority when it would make sense to have the last call of checkPermissions being able to determine whether a user has access to perform the requested action. Also since this is a permissions and roles library it should be strict by default, meaning even if one permission check has failed it should make the permitted value false.

noemission avatar Nov 08 '20 13:11 noemission