- 🇩🇪Germany Anybody Porta Westfalica
Just ran into this in a large project and was wondering why "Administer user roles" is still combined with administering permission.
So +1 on finishing this, where Drupal is today! Some comments and suggestions
-
The latest patch for this issue has some changes in AccountForm.php around
+ if (!$user->hasPermission('administer permissions')) { + $form['account']['roles']['administrator'] = [
but the administrator role is not necessarily 'administrator', it is configurable per site (see /admin/people/role-settings )
Therefore, whilst
!$user->hasPermission('administer permissions')
checks if the user doesn't have the site's administrator role, the next line doesn't necessarily call on this same role.The best I've come up with to fix this is
+ if (!$user->hasPermission('administer permissions')) { + $roles = \Drupal\user\Entity\Role::loadMultiple(); + foreach ($roles as $role) { + if($role->isAdmin()){ + $current_admin_role = $role->id(); + } + }; + $form['account']['roles'][$current_admin_role] = [
but I've resisted making and submitting a patch with this change, as there must be a more succinct (one-liner) to determine the site's administrators role id. Suggestions?
- In reference to the concern raised in #68 point 2 by @catch, I support an upgrade path too (is that an update hook?) that assigns this new permission (‘administer user roles’) to anyone who currently has the ‘administer permissions’ permission.
-
Do we want to reference #2846365 in the patch when we don't know what (or even when) a finished solution could come from that separate issue? @catch in #68 point 3 notes how 🐛 [regression] User roles field access is inconsistent, users with 'administer users' permission can gain full access Needs work doesn’t even touch the AccountForm.php.
Wouldn't we want to keep references for the other issue out of this issue's patch?
-
The latest patch for this issue has some changes in AccountForm.php around