Multiple user roles

I am currently working on a project that requires a user to possibly have multiple roles. As this is not posisble with the current toolkit and panel, I’ll have to code that myself, which is fine.

The only question is, whether I quickly hack it together or try to implement it properly for everyone. My only concern is that there will be a problem with backwards compatibility: The current User class defines a method role() - what should that return in case that multiple roles are defined? Deleting the method is probably not an option as it would break old kirby code.

@distantnative, @bastianallgeier: Do you think multiple roles per user should be in Kirby? If so, do you have an idea on how to fix the above issue?

Coming from Drupal, which has extensive roles and permissions features, I also feel this is missing in Kirby.

Would be great to have multiple user roles in Kirby, does anyone know if the new permission system they are developing will include this feature?

As you mentioned, thr critical question would be what role() would return. Most other things like hasRole() would be rather simple to adapt for multiple roles. That’s for the Kirby core. The panel would need some more changes.

That’s me personally looking at that idea, can’t say anything about official plans or priorities.

Also ou of interest: what would you need multiple roles per user for? What would this achieve that you can’t do already? What would you say would be the advantages?

I am redoing the webpage of a climbing club. Trainers are managing their group, and some trainers have multiple groups or additional special permissions. Every trainer should be able to edit the data of their own group(s), but not the others.

The idea was to create one role per group and to assign it to the respective trainers. The only alternative I can see is an exponential amount of roles to cover all possible combinations.

I was thinking about Kirby for a school site with different sections, and teachers are only allowed to edit their own section. Pretty much the same use case as @FabianSperrle.
Add to that a photo album with separate permissions, because only a couple teachers are in charge of the photo album, and then this [quote=“FabianSperrle, post:6, topic:1145”]
The only alternative I can see is an exponential amount of roles to cover all possible combinations.
[/quote] is a good enough reason.

My use case would not necessarily be for panel user roles, but multiple roles per user for access restriction. So that one user can access different parts of a site without the need to create multiple users and passwords per user which would be a nightmare for usability.

Thanks for the answers. As this personally interests me as well, I tried a possible implementation. As I cannot grasp all usage scenarios and possible walls to run into, I’d be happy if the ones of you, who are also interested in a solution, could test this code, so we could refine it:

https://gist.github.com/distantnative/8700d5836c29134ef6e4

The workaround for $user->role() atm is that if multiple roles are assigned, only the first role (in the order present in the account file) is returned EXCEPT if the admin role is assigned (no matter in which order, then admin is returned).

It implies also that you really shouldnt something like $user->role() == 'teacher', but always rather use hasRole(), which also works with multiple roles and returns true if the user has at least one of these roles.

Would appreciate some feedback and potential bug reports :smiley:

Thanks, @distantnative, will give this a try asap …

I took a gander at it and tried to add roles to a test user like this:

role:
 - client
 - editor

Is that correct?
Then when I test the role with hasRole() it always seems to return true if the user has Panel access.

Edit: line 129 looks suspect:

return count(array_intersect($roles, $roles)) > 0;

Shouldn’t it be ($role, $roles)? I made the change and it seems to work.

Oh yea, of course - dumb mistake. Fixed it in the gist - thanks!
And yea specifying it that way is exactly what I thought of.

FYI: roles that are specified in an account/user file but don’t exist under this ID in the config file (or the default admin & editor) will be replaced by the default role.

Ha good to know, I had already tested with an “invalid” role but didn’t realize there was a fallback.

Probably a stupid idea but here it is anyway: to help with a possible transition to multiple roles, what about deprecating role: and role() (but keeping the code in for backward compatibility) and adding roles: and roles()?

Not sure - what if both are included in the user file? Which one would hasRole() use?

A blueprint shouldn’t contain both. It should be clear in the docs that the former field is deprecated, then I’m not even sure we should test if both are included.
Or maybe add the check only in role() and not roles(), so it is tested only when people use the old function?
hasRole() would always use the new field, because it would have most probably been added in the blueprint later with up-to-date roles.

Hmm, personally I don’t see the advantages compared to keeping the field called role and trying to work with it depending on what it contains. The current approach should still work fine with old files that only specify one single role.

Or where would you see the advantages for splitting it up and having both ways around?

Oh I wondered if it could avoid lots of tests and ifs in the code, to keep it clean. But you’re right, from a developer’s perspective it’s easier to keep the same field and function. And the old function would still have to be maintained anyway.

I don’t actually see a reason not to use the current role field instead of introducing a new roles field. After all, the template option in page blueprints also accepts one or more templates despite its name …

2 Likes

Yea, I think IF multiple roles should be a thing, I prefer the current workaround for role(): If you have multiple roles and you still use only role() you will get either the first one or admin.

And it seems to work for that - at least in my tested scenarios, which are not at all comprehensive :laughing: I just updated the gist again, replacing the foreach-loop from roles() with an array_map (which seems to be nicer to me atm).

A whole other thing would be the question how to implement this in the panel. Ideas for a good design element for multiple roles?

@distantnative: let’s be more hopeful: not IF, but when finally … :smile: And then panel permissions! Yeah!

1 Like