Problem/Motivation

There is 'Administer permissions' permission. It does not mention that it grants permission to manage user roles.

Proposed resolution

Change the title of 'Administer permissions' to 'Administer roles and permissions'.

administer roles and permissions

Original report by @dpi

administer permissions is arguably the most important permission on the site. Roles who have this permission have keys to the whole kingdom; they can assign themselves any permission. This permission does not currently have a description. It deserves one. A description which communicates that roles with this permission effectively have full control of the site.

A description for admin permissions will help to contrast against the administer users permission. administer users grants almost all control over users, except role assignment and permission grants.

For reference, description for admin permissions:

Manage all user accounts. This includes editing all user information, changes of email addresses and passwords, issuing emails to users and blocking and deleting user accounts.

CommentFileSizeAuthor
#20 drupal-permissions.JPG19 KBKrzysztof Domański
#16 interdiff.2847808.14-16.txt1.19 KBlongwave
#16 2847808-16.drupal.Add-permission-description-to-administer-permissions.patch1.61 KBlongwave
#14 user-administer-permissions-description-2847808-14.patch433 bytesaburrows
#2 user-administer-permissions-description-2847808.patch623 bytesdpi
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dpi created an issue. See original summary.

dpi’s picture

dpi
Location Perth, Australia
CreditAttribution: dpi as a volunteer commented
Status: Active » Needs review
FileSize
user-administer-permissions-description-2847808.patch623 bytes

Proposed description for administer permissions:

Add, edit, and delete all roles. Including granting any permissions to any role, adding roles to users, and removing roles from users. Warning: Users with this permission can grant themselves any permission.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran as a volunteer and at PreviousNext commented
Status: Needs review » Reviewed & tested by the community

Yeah, new description makes sense.

yoroy’s picture

yoroy CreditAttribution: yoroy at Roy Scholten commented
Status: Reviewed & tested by the community » Needs review

Not sure we need two warnings with this description. It already gets "Warning: Give to trusted roles only; this permission has security implications." added to it. All the other permissions that have this warning do not elaborate on what those implications might be, we probably don't have to do that here either.

Would something like "Manage roles, set their permissions and assign them to user accounts." be enough?

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented

The extra description for this permission was removed on purpose in #620446: Rewrite permission titles and descriptions.

Also, this patch will conflict with the changes proposed in #2846365: [regression] User roles field access is inconsistent, users with 'administer users' permission can gain full access. I would almost close it as a duplicate of that issue, which is the one addressing the actual problem with this permission. Better to fix than to explain what's broken. :) Edit: sorry, disregard that paragraph; it was based on a misunderstanding.

dpi’s picture

dpi
Location Perth, Australia
CreditAttribution: dpi as a volunteer commented

Also, this patch will conflict with the changes proposed in

I don't see how this is the case. That issue had a problem because someone used the wrong permission, because they did not know which permission to use.

which is the one addressing the actual problem with this permission.

Theres no problem or 'broken'-ness, just role assignment is not defined well. On the surface, it would seem correct that either administer users or administer permissions would be relevant permissions.

dpi’s picture

dpi
Location Perth, Australia
CreditAttribution: dpi as a volunteer commented

The extra description for this permission was removed on purpose

#620446: Rewrite permission titles and descriptions removed the description because "Currently a lot of permission descriptions say the same as their titles", and it was true at the time: "Manage the permissions assigned to user roles.".

But this permission does more than manage permissions.

I think its important that the description at least mention role assignment.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented

I don't see how this is the case. That issue had a problem because someone used the wrong permission, because they did not know which permission to use.

Yeah sorry, this is my mistake. I misread the patch and issue. Edited my past comment to correct this.

Can we simply change the user-facing title of the permission to "Administer roles and permissions", instead of adding a lengthy description? It already has the "Warning" flag.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

spitzialist’s picture

spitzialist CreditAttribution: spitzialist at Unic commented
Assigned: dpi » spitzialist
Status: Needs review » Needs work
aburrows’s picture

aburrows CreditAttribution: aburrows at DigiDrop commented
Issue tags: +DistributedSprintUK18

Working on this at DistributedSprintUK18.

aburrows’s picture

aburrows CreditAttribution: aburrows at DigiDrop commented
Assigned: spitzialist » aburrows
Status: Needs work » Needs review
FileSize
user-administer-permissions-description-2847808-14.patch433 bytes

I have attached a patch for this.

Status: Needs review » Needs work

The last submitted patch, 14: user-administer-permissions-description-2847808-14.patch, failed testing. View results

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave as a volunteer commented
Status: Needs work » Needs review
FileSize
2847808-16.drupal.Add-permission-description-to-administer-permissions.patch1.61 KB
interdiff.2847808.14-16.txt1.19 KB

Updated test to match new permission name.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Krzysztof Domański’s picture

Krzysztof Domański
Location Poland
CreditAttribution: Krzysztof Domański at abventor commented
Title: Add permission description to 'administer permissions' » Change "Administer permissions" to "Administer roles and permissions"
Assigned: aburrows » Unassigned
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
FileSize
drupal-permissions.JPG19 KB

Looks good. Roles and permissions are the basic concept of Drupal. Changing the title to 'Administer roles and permissions' seems sufficient. This means that a user with this permission can also manage user roles. An additional description is not necessary.
Only local images are allowed.

  • catch committed 191df98 on 9.1.x 
    Issue #2847808 by longwave, dpi, aburrows, Krzysztof Domański, xjm,...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed 191df98 and pushed to 9.1.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

headstartcms’s picture

headstartcms CreditAttribution: headstartcms commented

Need advise. I found issue on the the user permission access. All user permissions won't work when i try to customized the permissions. The check list multiple icons won't able to functions. The save button won't make changess on the user permissions.