Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected behaviour when using multiple "checkPermissions" in one method #85

Open
noemission opened this issue Nov 8, 2020 · 0 comments

Comments

@noemission
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant