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

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

Open
lucfranken opened this issue Oct 25, 2023 · 1 comment

Comments

@lucfranken
Copy link

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
Copy link

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

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

No branches or pull requests

1 participant