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

Ignore undefined roles if one of them enough access #91

Open
isikhi opened this issue Sep 18, 2020 · 3 comments
Open

Ignore undefined roles if one of them enough access #91

isikhi opened this issue Sep 18, 2020 · 3 comments
Labels
feature request Considered to be implemented. partially considering Some aspects of this issue are being considered.

Comments

@isikhi
Copy link

isikhi commented Sep 18, 2020

If the user(or whatever subject) contains any of the roles required to access somewhere, they don't need to give an error if they can't find the role by looking for other roles. We can use as many roles as he it find here given accesses.

We can ignore invalid roles error if one or more of role is valid.

@onury
Copy link
Owner

onury commented Jan 16, 2021

I'm not sure I get what you mean but when a given role is invalid (does not exist), AccessControl should definitely throw because this is most probably due to invalid configuration. And since this is security related, AC takes it seriously and throws.

From the linked threads, I can only agree with the case where no roles are defined, AC can simply deny access instead of throwing for empty array of roles. (That can also be suppressed by assigning a "guest" role for non-privileged users.)

@onury onury added the partially considering Some aspects of this issue are being considered. label Jan 16, 2021
@isikhi
Copy link
Author

isikhi commented Jan 17, 2021

I ll quote an comment here directly;

Actually you could have the permission in the db or in memory with RB. The idea is that if you remove or suspend certain global permission or an action from your RB, you would not need to update all users affected, as the permission does not exist in memory, everything continues to work correctly.
Otherwise, as is happening right now, if any existing permission in the User that does not exist in the RB memory, either because it is misspelled or simply does not exist, the application throws an error. Ignore is better!

quoted from @ruslanguns
Rb=role builder
Edit: Thanks for your interest and this great library.

@onury onury added the feature request Considered to be implemented. label Jan 18, 2021
@onury
Copy link
Owner

onury commented Jan 18, 2021

@isikhi, thanks. Let me put it this way: the quoted comment suggests an opinionated system. They prefer to "ignore". Other systems are more strict when it comes to security aspects. They would not tolerate misspellings or enable no-role users.

So I think, it'd be best to make this configurable in AC constructor.
Sounds good?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Considered to be implemented. partially considering Some aspects of this issue are being considered.
Projects
None yet
Development

No branches or pull requests

2 participants