Meteor-Community-Packages / meteor-roles

Authorization package for Meteor, compatible with built-in accounts packages
http://meteor-community-packages.github.io/meteor-roles/
MIT License
920 stars 167 forks source link

addRolesToParent -> Reset inheritance as unwanted roles may still be connected #385

Open lucfranken opened 11 months ago

lucfranken commented 11 months ago

Is your feature request related to a problem? Please describe.

Unclear to me is how the project maintainers look into the tree of roles, running into some issues and looking for the vision:

I have a hardcoded tree of roles:

Roles.addRolesToParent(ROLE_USER, ROLE_ADMIN);
Roles.addRolesToParent(ROLE_DO_SOMETHING, ROLE_USER);
Roles.addRolesToParent(ROLE_DO_BANK_TRANSACTION, ROLE_USER);

Now I remove a role in my code, so this becomes the new code:

Roles.addRolesToParent(ROLE_USER, ROLE_ADMIN);
Roles.addRolesToParent(ROLE_DO_SOMETHING, ROLE_USER);

The user now will be able to do the bank transaction. Even when the role is not connected anymore to the parent.

The why is clear to me: The roles are stored in the database as children.

But it is not intuitive.

I also see a security risk here. A role can be attached while not in the code connected anymore.

Also the example does not make this clear:

https://github.com/Meteor-Community-Packages/meteor-roles/blob/master/README.md?plain=1#L93

If a user starts with this example and later removes a role they are insecure.

Describe the solution you'd like

When you have a hard-coded inheritance tree it should somehow be respected. OR: It should be clear that your configuration (that's how it feels) gets "cached" in the database.

But I also see the functionality of the package as a tool to really dynamically add/edit roles to parents.

So maybe there should be some call to "reset inheritance" on the build step.

Describe alternatives you've considered

Clearing the all roles when starting up the server (not an option as we run multiple hosts).

Additional context

We got it visible because we made an interface for the roles, otherwise it might have slipped if it was code only:

image

github-actions[bot] commented 11 months ago

Thank you for submitting this issue!

We, the Members of Meteor Community Packages take every issue seriously. Our goal is to provide long-term lifecycles for packages and keep up with the newest changes in Meteor and the overall NodeJs/JavaScript ecosystem.

However, we contribute to these packages mostly in our free time. Therefore, we can't guarantee your issues to be solved within certain time.

If you think this issue is trivial to solve, don't hesitate to submit a pull request, too! We will accompany you in the process with reviews and hints on how to get development set up.

Please also consider sponsoring the maintainers of the package. If you don't know who is currently maintaining this package, just leave a comment and we'll let you know