prefs_users_overview UI is misleading in its display and manipulation of user Roles

[esrever_otua]

prefs_users_overview page shows Roles on a User that are only actually assigned to a Group they're a member of. This means that an admin may attempt to 'untick' a Role that is showing for a User, and apply the change (which appears to complete successfully), but if one of the Groups they are a member of has the Role, then the screen continues to show a ticked checkbox for that Role for the User, even after 'Applying' to remove the Role.

This seems to be a very bad design/UI decision both in the prefs_users_overview screen, but also in the underlying API, e.g. if Bob is a member of a Group that has the Role 'Reviewer' (but has not been granted it directly himself):
portal_role_manager.getRolesForPrincipal('Bob') *will* show 'Reviewer' in the list (even though Reviewer has not been granted directly to *that* Principal), but,
portal_role_manager.listAssignedPrincipals('Reviewer') will *not* include 'Bob'.

This definitely seems buggy and/or unexpected. Especially so because there is no "portal_role_manager.getDirectlyAssignedRolesForPrincipal('Bob')" to show the real truth.

[esteele]

I can confirm this behavior. Here's where this really becomes an issue...

In a site with at least 2 users...

1. Go to /prefs_users_overview, show all. User 1 has role "member",
2. Set the "authenticated users" group (or any group User 1 belongs to) to 'editor', save.
3. Go to /prefs_users_overview, show all. User 1 now shows 'Editor' as an assigned role.
4. Add the "Reader" role to User 2.
5. Remove "Editor" role from group.
6. Go to /prefs_users_overview, show all. User 1 will still have the 'Editor' role, which was unknowingly assigned specifically to the user in step 4.

[glenfant]

Hi,

I've investigated this issue in depth.

The @@usergroup-userprefs view should *not* check by default additional roles that are granted by the groups each user belongs.

When submitting the form, this grants the role to the user. That is undesirable.

The factory that makes users through the @@pas_search multi adapter (using whatever PlonePAS API) grants roles to user objects with all activated plugins that implement IRolesPlugin.

Among those activated plugins, we got portal_role_manager. This is a Products.PlonePAS.plugins.role.GroupAwareRoleManager object.

Its "getRolesForPrincipal" method will find all groups the principal (user) belongs, merges the direct roles of that user and the roles the groups he belongs have.

To have a clean resolution for that issue, we would need to add an "ignore_groups_roles=True" that could be passed through the request, from the search view to the plugin.

I made a couple of patches that work. It's perhaps ugly and may seem somehow cryptic but it works.

Now I dunno if it may raise a security issue since hackings through the URL may only remove roles to the user.

Anyway, the unit tests for plone.app.controlpanel (branches/2.0) smile.

glenfant@Bidule:~/zope/instances/plip9292
$ diff -u src/Products.PlonePAS/Products/PlonePAS/plugins/role.py.new src/Products.PlonePAS/Products/PlonePAS/plugins/role.py
--- src/Products.PlonePAS/Products/PlonePAS/plugins/role.py.new	2009-08-25 20:13:23.000000000 +0200
+++ src/Products.PlonePAS/Products/PlonePAS/plugins/role.py	2009-08-25 20:16:07.000000000 +0200
@@ -107,7 +107,7 @@
         principal_ids = [principal.getId()]
         # not all user objects are propertied users with groups support.
         # theres no interface for now - so use an ugly hasattr
-        if not self.REQUEST.get('__ignore_group_roles__', False) and hasattr(principal, 'getGroups'):
+        if hasattr(principal, 'getGroups'):
             principal_ids.extend( principal.getGroups() )
         for pid in principal_ids:
             roles.extend( self._principal_roles.get( pid, () ) )
glenfant@Bidule:~/zope/instances/plip9292
$ diff -u src/plone.app.controlpanel/plone/app/controlpanel/usergroups.py.new src/plone.app.controlpanel/plone/app/controlpanel/usergroups.py
--- src/plone.app.controlpanel/plone/app/controlpanel/usergroups.py.new	2009-08-25 20:10:39.000000000 +0200
+++ src/plone.app.controlpanel/plone/app/controlpanel/usergroups.py	2009-08-25 20:19:48.000000000 +0200
@@ -91,7 +91,6 @@
         return self.index()
 
     def doSearch(self, searchString):
-        self.request.set('__ignore_group_roles__', True)
         searchView = getMultiAdapter((aq_inner(self.context), self.request), name='pas_search')
         return searchView.merge(chain(*[searchView.searchUsers(**{field: searchString}) for field in ['login', 'fullname', 'email']]), 'userid')
 
@@ -106,7 +105,7 @@
glenfant@Bidule:~/zope/instances/plip9292
$ 

[glenfant]

This is fixed for me on Plone 4 dev branches.

Changes in plone.app.controlpanel r29369 and r29370

Changes in Products.PlonePAS

http://dev.plone.org/collective/changeset/95748/Products.PlonePAS/trunk/Products and http://dev.plone.org/collective/changeset/95751/Products.PlonePAS/trunk/Products

Test case

http://dev.plone.org/collective/changeset/95756/Products.PlonePAS/trunk/Products and http://dev.plone.org/collective/changeset/95779/Products.PlonePAS/trunk/Products

This could be backportable to Plone 3.3 and (perhaps) Plone 3.2 maintenance if any.