thedevdojo / voyager

Voyager - The Missing Laravel Admin
https://voyager.devdojo.com
MIT License
11.77k stars 2.67k forks source link

[Feature Request] User Multi Roles #371

Closed akazorg closed 6 years ago

akazorg commented 7 years ago

In some circumstances multi roles per user are really missing.

handiwijoyo commented 7 years ago

@akaZorg may I know what are you trying to achieve?

akazorg commented 7 years ago

Hello @handiwijoyo sure! Thanks!! In fact I should have described this better.

In most case scenarios the existing permissions system will work great, but now imagine we have 2 different roles such has:

  1. Accounting (can access clients, orders, etc..)
  2. Editor (can access blog, pages, etc..)

For some reason you have a user that needs permission to both roles. We can create a new role and name it "Accounting + Editor" but that would look so ugly :O Am I missing something here? How would you do it? This situation can happen in many scenarios.

While at the front-end, we could use multi roles for enabling/disabling certain sections, more easily I guess.

Cheers

tonybyng commented 7 years ago

Hi

I recently installed voyager and I am thinking I may hit this problem too. A user may have access to specific features - gaining access to a reports module or to a more detailed interface that other users don't have access to. Its not so much a "role" facility - the role is still a user and they still have the basic user rights that a non-guest, non-admin would have, but Im thinking that maybe another table of access rights and have a lookup table containing an access right and a user id

From what Ive seen I think most of this could be created by us as part of the BREAD setup so if that is the case, its only a case of developing a simple rights access facility to check permissions before giving access to something. However if the right to do something determines whether it should be in the menu or not, that could cause some problems?

fletch3555 commented 7 years ago

@tonybyng, having multiple roles (or otherwise having multiple sets of granted and/or denied permissions) brings with it a series of issues that would need to be figured out. For example:

  1. What priority should permissions be assessed? If a role grants access to something, but a user is directly denied permission to the same thing, what should happen? What about the reverse (role denies, but user grants)? What if a user is in multiple roles, and one role grants but another denies?
  2. Should we allow "deny" permissions at all?

Every CMS I've ever touched handles these issues differently, so there's no "industry standard" to follow. This will take some thought/planning from the maintainers before we take any action with it.

akazorg commented 7 years ago

@fletch3555, if you seek in the web, one of the "industry standard" is RBAC, and that sort many of the issues you might have in the future.

I don't understand how you don't see the need of Multi Roles in a Organization.

fletch3555 commented 7 years ago

@akazorg, nowhere in my comment did I say that I don't see a need for supporting multiple roles.

RBAC doesn't explicitly define how permissions for multiple roles are to be handled. The document that defines RBAC (available from NIST here: http://csrc.nist.gov/groups/SNS/rbac/documents/ferraiolo-kuhn-92.pdf) outlines what RBAC is and what it isn't. What it DOES say is that users should have an "active" role, which determines it's permissions, meaning that users will only ever have one role in play at a time, which doesn't solve our problem.

So again, this will require some discussion by the maintainers on how we want to proceed.

akazorg commented 7 years ago

@fletch3555, I'm not into a discussion of what RBAC is, I'm into having a multi-role system. :)

RBAC serves as an example, if you google "Laravel RBAC", you'll find solid solutions, maybe you can get an eye and see how they made it. It's all about not reinventing the wheel I guess..

pimduijst commented 7 years ago

I also agree to have this featured in Voyager but I also agree with @fletch3555 that it is a complex feature with pros and cons and that this must be well thougth

akazorg commented 7 years ago

I'll reopen it since theres activity. @pimduijst this seams more complex indeed, but I guess theres some progress with this @fletch3555?

ux-engineer commented 7 years ago

Multiple roles would be useful. For example a blogger, and pages editor. It would be more practical to manage and use in the code with multi roles.

Talking about permissions assessment...wouldn't it be the simplest and easiest in such a way that if any logic grants an permission, user has the permission. Or actually I haven't yet seen any part of this system that would deny access - just that the particular permission has been granted. Or have I missed something?

akazorg commented 7 years ago

@envision You're right, there is no deny logic, just grant permission. There is work in progress, but we've been busy and this is still pending.

edmandiesamonte commented 7 years ago

+1 for supporting multi-roles. Maybe this similar package would help: https://github.com/larapacks/authorization. I don't know though if there would be conflicts if I install it alongside Voyager. If none, this might be a good tool to be integrated with Voyager, don't you think?

javierpacareu commented 6 years ago

@edmandiesamonte did you use the authorization package... Did it work with Voyager?

friendge commented 6 years ago

+1 for multi-roles.. My take is that you probably don't need deny options, but if it's decided you do, a deny ALWAYS takes precedence of any grant. i.e. I'm a part of this group that can do function x, but another group I'm in denies function x, therefore, I can't do function x..

nickbryan commented 6 years ago

Hi,

Is there a time scale on when this would be released? Is it currently in development?

We are considering Voyager for our latest project but having this multi role option is an essential part of moving forward with it. Everything else about Voyager seems a perfect fit.

If this is not in the pipeline, is there a way to customise this behaviour? I presume doing so would have a impact on upgrading to future versions so its something I am keen to stay away from if possible.

Thanks in advance.

fletch3555 commented 6 years ago

Voyager currently does not support this, and as far as I'm aware, it's not under development. So I can't give you a timeline right now

akazorg commented 6 years ago

@nickbryan I'm working on this... in progress..

nickbryan commented 6 years ago

@akazorg Thats great news! Thanks for the reply.

ptournet commented 6 years ago

@akazorg Is there any way to help you with that ?

akazorg commented 6 years ago

Hi @ptournet, I've sent PR #2290, check it out. Fell free to test it. Thanks for your help.

akazorg commented 6 years ago

Closing, this was merged.

github-actions[bot] commented 4 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. If you have further questions please ask in our Slack group.