U4-8632 - User management - Usergroup functionality

Created by Jeffrey Schoemaker 22 Jun 2016, 12:59:59 Updated by Sebastiaan Janssen 17 Sep 2017, 09:22:40

Tags: PR

Is duplicated by: U4-12

Depends on: U4-9155

Parent for: U4-2474

Parent for: U4-6411

Parent for: U4-7907

Parent for: U4-8643

Parent for: U4-9831

Parent for: UAASSCRUM-913

Parent for: UAASSCRUM-914

Parent for: UAASSCRUM-915

Parent for: U4-9888

Parent for: U4-9889

Parent for: U4-9900

Parent for: U4-9913

Parent for: U4-9914

Parent for: U4-9915

Parent for: U4-9944

Parent for: U4-9946

Parent for: U4-9963

Parent for: U4-9964

Parent for: U4-9965

Parent for: U4-10023

Parent for: U4-10053

Parent for: U4-10059

Parent for: U4-10075

Parent for: U4-10083

Parent for: U4-10095

Parent for: U4-10096

Parent for: U4-10196

Parent for: U4-10104

Parent for: U4-10106

Parent for: U4-10107

Parent for: U4-10109

Parent for: U4-10111

Parent for: U4-10126

Parent for: U4-10133

Parent for: U4-10134

Parent for: U4-10138

Parent for: U4-10142

Parent for: U4-10146

Parent for: U4-10155

Parent for: U4-10173

Parent for: U4-10174

Parent for: U4-10175

Parent for: U4-10202

Parent for: U4-10210

Parent for: U4-10212

Parent for: U4-10227

Parent for: U4-10228

Parent for: U4-10230

Parent for: U4-10231

Parent for: U4-10233

Parent for: U4-10235

Parent for: U4-10236

Parent for: U4-10237

Parent for: U4-10243

Parent for: U4-10244

Parent for: U4-10256

Parent for: U4-10257

Parent for: U4-10265

Parent for: U4-10278

Parent for: U4-10279

Parent for: U4-10280

Subtask of: UAASSCRUM-1073

A feature we all wanted for a long time is Usergroups. During CG16 I've discussed the functionality with a lot of people and this is a sort of summary of the requested functionality.

Must haves:

  • One user could belong to one or more usergroups, with a minimal of 1.
  • Usertype-functionality should be migrated to usergroups so for a user group you specify whether he/she should be able to publish / rollback / etc. If you belong to more than one usergroup these permissions are added. Example: The group 'Editors' exist with 'Update'-rights and another group 'Writers' exist with the permission 'Create'. If you belong to both groups you should do both.
  • On a usergroup you determine to which Umbraco sections you have permission to browse (Content, Media, Settings, etc)
  • In the contenttree => Permissions are set on grouplevel

Nice-to-have:

  • Extend permissions (http://umbraco.usermanagement.perplex.eu/usertype.html for some inspiration): => Recylcle bin permissions => Mediapermissions => Media recyclebin permissions
  • CRUD rights on doctypes and mediatypes. The most common usecase is that you have a user that should be able to create blogs and news. By adding CRUD-rights to doctypes can be done.

Worth a good discussion:

  • Where to set the startnode of the contenttree and mediatree. On a usergroup (but what about multiple usergroups?), or on a user? If you set this on a user, then you should also be able to copy an user to make it as easy as possible

Some more inspiration can be found here: http://umbraco.usermanagement.perplex.eu/specificusergroup.html (see different tabs)

Comments

Andy Butland 23 Jun 2016, 10:36:56

Thanks for creating this issue Jeffrey - I was also at the CG16 session and have been giving this feature a bit of thought too since then. Had a couple of points for discussion on your specification above.

User permissions

On the point ''"In the contenttree => Permissions are set on grouplevel"'' - whilst this sounds like it might be simpler were the system being designed from scratch, I think permissions being set on the user level would have to remain as well. Mainly due to site upgrades - if user permissions were removed and instead set on groups, then given a situation where a site has a number of users with permissions set, really the only way I can see to migrate this automatically would be to create one group per user - which isn't likely what is wanted.

It's also perhaps of value to allow setting of permissions on the user or the groups. Could be a simple setup doesn't need groups. Or could be some particular user exceptions were it's handy to set most permissions on groups, but some on users as well.

Given that though as you say the "net" permissions would be the most permissive set for all the groups and the specific user permissions your account has. So if a user is in a group that can do A, another group that can do B and their direct user permissions allow C - then they can do A, B and C.

User types and groups

Seems there's two approaches here - what's outlined above (and discussed at CG16), which is to effectively migrate user types to groups. So instead of having a many to one relationship between users and user types, we'd have a many to many relationship instead. Again, if starting from scratch, that would likely be the simplest set up. Alternatively though we could keep types and have groups as well. Not sure what's best here but wanted to high-level outline some considerations for each.

Move from types to groups:

-Rework database structure to remove many to one and introduce many to many relationship between users and user types. -Rework such that a user can be created without a type (currently a type is required in the constructor). -Refactor names so "types" become "groups". To bring things in line with how groups are referred to in terms of members. Might be would live with the database table and field names being "wrong", but ongoing it would likely be clearer to refer to "groups" in the code and UI. -Set up an upgrade migration process for the schema changes -Set up an upgrade migration process for user data - so each type would become a group with the same default permissions, and all users of each type would be assigned to the group -Implement UI and logic for creating groups, setting their default permissions and adding users to them. -Implement UI and logic for setting section and node specific permision on groups. -Implement permissions for a user's access sections and nodes based on the net most permissive set of permissions based on all their groups and their direct user permissions.

Keep types and introduce groups:

-Likely simpler (certainly less intrusive) from a coding perspective. Types would remain and groups would be an added feature. -Migration would be simpler. We'd just introduce, new, empty tables for groups, group/users, group/sections and group/node permissions. Users, types and existing permissions would stay as they are. No need to transform data. -Maybe in future having types as well as groups would be useful (say if properties were added like with members)? -If someone just wanted to carry on with the current setup they could, and ignore groups. -Downside though perhaps is for new users - being able to set permissions on types, groups and users may be more confusing than it needs to be. -Implement UI and logic for creating groups, setting their default permissions and adding users to them. -Implement UI and logic for setting section and node specific permission on groups. -Implement permissions for a user's access sections and nodes based on the net most permissive set of permissions based on their type, all their groups and their direct user permissions.


Jeffrey Schoemaker 08 Jul 2016, 13:06:31

Hi Andy,

thanks for your comment! I've had some post-CG-stress, so hadn't time to comment back on your comment, but finally found some time.

=== User permissions === Assigning permission on a group- and a userlevel would be fantastic, but I have no idea on the impact of complexity when building it. Also we have to make sure that it's pretty easy to trackback which user has which net permission set.

=== User types and groups === I agree that is probably more complex to remove the types and introduce groups, but seen from a end user or new user-perspective it should make more logic to only have groups, I guess. I am wondering how other CMS's handle this?

I know we have to be keen on upgrading existing sites, but I think we should design the end-solution first and then see if there's a upgrade path available. If not, I don't have a problem that it's only released into Umbraco 8 and loose some backwards compatibility. I'd rather have a good end solution than a solution that has an upgrade path.

I'm wondering what the opinion of HQ is about this. I'm willing to start making some sort of wireframe/design, but some guidance would be useful. I'll be gone for a long summer holiday, but will try to come up with something after that?


Andy Butland 17 Jul 2016, 16:42:19

I've had a bit of time to start looking at this, the results of which are in this PR: https://github.com/umbraco/Umbraco-CMS/pull/1384. I've implemented here something that follows what I described above in "Keep types but add groups". I'm a bit torn as to whether that's best but I suspect the ease of upgrading existing solutions may make it the better option (and if not, it may still be a step on the way to the larger change that migrates a single user type to multiple user groups).


Shannon Deminick 22 Jul 2016, 11:15:27

Hi Guys,

Thanks for putting this all together, I replied to the PR here: https://github.com/umbraco/Umbraco-CMS/pull/1384#issuecomment-234485519 and was just explaining that having both user and group permissions might add too much complexity. I'm mostly worried about performance because currently the way user permissions is structured is less than ideal and depending on how many are assigned can cause the cache size to become enormous. If we have 2x this same cache structure bad things could start to happen.

I haven't reviewed the code just yet but will let you know when I do, perhaps it is actually ok :)

Last thing to note, because we've been talking about this for many many years and I believe that this same document has been written over and over again (and then lost), there was always an idea of having an additional permission of "Deny". Currently we don't have any "Deny" permissions but this becomes very powerful and flexible. Deny would always trump an allow and with overlapping user and user group permissions, it may become an important thing to have. That said, it also adds more complexity.


Cameron W 08 Sep 2016, 02:26:54

Just wanted to bump up this feature request and add my two cents.

This (missing user roles) is definitely one area where Umbraco falls short of its commercial competitors when evaluating for a large / multi-site deployment. Something I am currently evaluating CMSs for...

Shannon to your point about 'Deny' permissions, I think it adds flexibility, but at the cost of complexity. In the majority of cases(?) you can usually achieve the same permission set with a default of 'deny' all and add explicit 'allow' permissions - which are by inherited by child nodes in the content tree unless the child node has its own set of permissions (user's might need some action to clear permissions and 'use the parent permissions').

Also, role-based inherited permissions should also be able to be applied to the Umbraco Forms section in backoffice, which would need the concept of a 'folder' added within forms + worflows + datasources. That way permissions can be applied to a folder and be inherited by all children.

Given this feature could be a breaking change, it seems like the ideal candidate for a major release, say Umbraco 8? Any chance of this?


Shannon Deminick 08 Sep 2016, 06:00:40

Hi, thanks for the feedback. With regards to Deny, we'd still need the notion of 'Deny' to be able to have any default deny settings, we currently don't have this notion at all which is why I'm saying we could/should add it.

This doesn't have to be a breaking change, there's no reason to wait till v8 to achieve this if we don't have to. When we find some time we'll be reviewing/tweaking Andy's PR. We'd like to try to keep this as simple as possible, permissions currently are not stored in a simple manner and consume a ton of cache memory if there are a lot applied, so having permissions set per member and per role still doesn't sit well with me and then if we add Deny as well, things will become very complex (which will result in performance penalties).

I totally agree that we should be able to make the permissions APIs utilized by other parts of the system, so Forms could plugin to it's pipeline as well instead of storing it's own permissions structure - but we'll see how complex that becomes since that might require large db changes and in which case might have to wait until v8.

We will certainly be reviewing all of this soon, there's just a few things on the HQ plate currently that we need to get completed first.


Shannon Deminick 24 Oct 2016, 07:56:56

Hi @abutland

We would like to proceed with getting this into the core for a 7.x release, however as I previously mentioned, the decision has been made to simplify this implementation. We will only have user groups assigned for permissions.

The reason for this decision is to maintain simplicity, having permissions assignable for both user groups and individual users adds a lot of complexity and it will add a ton of memory and lookup implications. Currently the memory utilized for storing the permissions is huge (I'd like to fix this in the future ... but we can cross that bridge later). If people truly want single user permissions, it's easily solved by creating a group for that particular user.

So moving forward, we'll need to update this PR to just have permissions assignable at the user group level and that is it. This should hopefully simplify much of this code and probably allow us to remove quite a bit too.

The upgrade plan will be pretty simple:

  • Create a user group per user
  • Assign the permissions from the user to this user group

Once that is done then the admin after upgrading can move users to different user groups, etc.. but this will maintain the same permissions as they had previously. As for deny permissions, this is also something we can look at in the future, the first plan will just be to get user groups working for permissions. At some stage we'll also have to update the user membership provider and the user identity implementation to return these groups as roles.


Andy Butland 24 Oct 2016, 15:45:34

OK, noted. I'll try to take a look back at my PR and see if can be updated (or used as a resource for a new one) to match this simplified requirement. Currently quite busy and will need to get my head back into it, so not sure quite when - so if anyone else wants to pick at what I've done please feel free.


Andy Butland 27 Oct 2016, 13:58:18

Started to have a look at reworking my PR to match this @Shandem. You've discussed node permissions above, but how do you want to handle user/app (sections like "content", "media") permissions? It would seem to follow to move them to groups as well. But then would mean if a user is created and not added to a group, they would have no app permissions - and hence in practice you'd have to create and add them to at least one group. That sound OK to you?


Shannon Deminick 27 Oct 2016, 14:05:26

Hi @abutland great news :) Very interesting point about the app permissions but yes I'd concur that they should be done at the user group level too since we'll do all permissions by group. The only thing I can think of is that we have one (or more) built in groups that cannot be removed via the API and are installed probably with custom system IDs like we have for other system entities. We sort of have this already now with 'editor' 'admin' 'translator', so maybe we keep those but make them user groups that cannot be removed, then by default we can add a user to the 'editor' group. Something like that work?


Andy Butland 27 Oct 2016, 14:22:17

It would I imagine yes. It sounds then like we need to also migrate the current user types to become user groups. I discussed pros and cons of this a little at the top of this issue. In short I think it's how you would design it if starting from scratch but it'll add complexity to the code changes and the data migrations for upgrades.


Shannon Deminick 27 Oct 2016, 14:41:17

Yup we'll definitely have to do that. User Types currently don't really do anything, all they do is provide a way to have 'default' permissions for a given user. But if they are groups, then they are the permissions.


Shannon Deminick 27 Oct 2016, 14:41:45

We can deal with the upgrade data later on, we'll worry about getting it functional now first :)


Andy Butland 30 Oct 2016, 11:15:07

Have created another PR for review now here: https://github.com/umbraco/Umbraco-CMS/pull/1559


Priority: Normal

Type: Feature (planned)

State: Fixed

Assignee:

Difficulty: Difficult

Category:

Backwards Compatible: True

Fix Submitted: Pull request

Affected versions:

Due in version: 7.7.0

Sprint: Sprint 58

Story Points:

Cycle: