Problem/Motivation

On busy websites with many users, it becomes essential to be able to delegate some tasks, for example the management of the user's roles. As it is, you can give the 'administer permissions' to the users who will be in charge of managing the user's roles, but this also allows them to access the Roles and Permissions admin pages!

The RoleAssign module creates this permission, but I believe this feature would be useful in the core, as opposed to having to use an extra module for such a basic feature. (see the desription of the module for more information).
Here is a statement of the access control page.

Permissions also allow trusted users to share the administrative burden of running a busy site.

No doubt this extra permission would extand the flexibility of sharing the administration of busy sites.

Proposed resolution

As suggested in #7 create two new permissions:

  • 'administer roles' to give access to admin/people/roles
  • 'assign roles' to show the role selector in user edit form (and show the role-related bulk operations on user admin view)

(and keep 'administer permissions' to give access to admin/people/permissions)

Remaining tasks

Contributor tasks needed
Task Novice task? Contributor instructions Complete?
Create a patch Instructions
Update the issue summary Instructions Done in #29
Update the issue summary noting if allowed during the beta Instructions
Add automated tests Instructions
Draft a change record for the API changes Instructions

User interface changes

Two new permissions

API changes

None.

Data model changes

None.

Comments

Dries’s picture

Looks interesting.

My only gripe is that when you ask a random person what the difference is between 'administer access control' and 'assign roles', he probably won't be able to answer that question correctly.

In other words, can we rename 'administer access control' to make it more explanatory? What access does 'administer access control' permission give you after this patch lands? Maybe we can come up with better wording for this permission and further streamline things?

scor’s picture

Assigned:Unassigned» scor

Good point Dries.

'administer access control' gives access to 3 pages:
- admin/user/access (Permissions page, determine access to features by selecting permissions for roles)
- admin/user/roles (List, edit, or add user roles)
- admin/user/rules (List and create rules to disallow usernames, e-mail addresses, and IP addresses)

I also found it in the system.module file, in the system_get_module_admin_tasks function, but it's not used, and there is a comment saying
TODO: this needs to be rewritten for the new menu system.

I can't think of anything better than 'administer access control'. Any other idea considering the 3 pages above?

Crell’s picture

+1 on this. For the rename, "administer access configuration" vs. "administer user roles"?

scor’s picture

"administer user roles" is too specific, its meaning doesn't include the third page:
- admin/user/rules (List and create rules to disallow usernames, e-mail addresses, and IP addresses)
This is typically access administration.

Or else an extra permission called 'administer access rules' could be created for this page.
At the end, the 'administer access control' permission would be split into 3 more explicit permissions: administer access rules, administer user roles, assign roles.
Dries, these 3 permissions are surely more explanatory than 'administer access control'.

Dries’s picture

If we can't come up with a single good name, it's best to further break up the permissions so that all of them are straight-forward and intuitive.

scor’s picture

Title:More granular permissions for the user module: assign roles» More granular and intuitive permissions for the user module
StatusFileSize
new3.87 KB

Let's break up the 'administer access control' into 3 more intuitive permissions:
- administer access rules (the access rules page!)
- administer roles (the access control page with all the permissions/roles, and the add and edit roles pages)
- assign roles (the role section on the user profile pages, and the mass role update on the admin user page)

Attached is a patch implementing this against the lastest HEAD. Do we want this in drupal 6?

Note: make sure to fire menu_rebuild() in order to rebuild the menu_router table and get rid of the former 'administer access control'. Or reinstall drupal after patching to make sure you have a clean set of permissions.

Dries’s picture

I'd like to rename "access control" to "permissions". Also the menu item ...

Given that, we could use the following:

1. administer permissions
2. administer access rules
3. create roles
4. assign roles

scor’s picture

StatusFileSize
new4.52 KB

straight forward patch changing the former 'administer access control' permission to one of these 4 new permissions:
1. administer permissions
2. administer access rules
3. create roles
4. assign roles

I don't think we can be more granular and explicit than that.
'create roles' is a bit restrictive since it also allows the edition and deletion of the roles, but if we call it 'administer roles', again it's confusing and people might think it deals with administer permissions or assign roles to users. So I believe 'create role' is good.

The ambiguous menu item 'Access control' has also been rename to 'Permissions'.

Successfully tested on a fresh install.

Crell’s picture

- +1 on fully splitting the permissions up to essentially permission per page.

- I'm getting warnings about DOS line endings, although the patch applies successfully anyway.

- It's not immediately apparent what "administer users" means now. I'm not sure if that's a task for this patch or not, but I will mention it.

- This may be a larger question for Drupal 6 in general, but all of these pages are beneath /admin/user, which has the permission "administer site configuration". In the new menu system that means you need that permission as well. I think that's acceptable if and only if that permission gives no access to any "real" pages, just the administrative index pages. I mention it here because I just noticed it while reviewing this patch.

- If "create roles" actually gives edit and delete permissions as well, shouldn't it be "manage roles"?

scor’s picture

StatusFileSize
new4.43 KB

'manage roles' is actually more accurate. +1
This patch implements it.

all of these pages are beneath /admin/user, which has the permission "administer site configuration".

This is correct, except for the 'assign roles' permission which is used to enable the role update feature on the edit profile page, and the user administration page.

fyi, 'administer users' permission allows to
- view, edit and delete all user accounts
- access the admin user page
- search through the users
- register new users

scor’s picture

Is there any chance to have these cool permissions in drupal 6? I think many people will like the flexibility offered by them, especially the 'assign roles' one.

alanburke’s picture

+1 great idea.
Certainly useful for larger sites where delegation of tasks has to be well planned out.

Alan

Crell’s picture

Status:Needs review» Needs work

One hunk fails. It's an easy fix.

However, the admin/user page is still set to "administer site configuration". That means you need that permission in order to get to any of the pages this patch affects, which gives you access to far too many extra pages. I'm not really certain what the best solution to that is. I'm not sure if it has to be handled in this patch directly, either.

scor’s picture

"administer site configuration": It's the same problem for many other modules (node, contact, statistics, system).

The "assign roles" permission does not require "administer site configuration". (This feature was the original idea of this thread)

The "administer site configuration" permission grants access to the user administration pages. Wouldn't it make more sense to seperate the users from the site configuration? How about having a different permission for the admin/user pages called "access user administration pages"?

forngren’s picture

Subscribe.

Will review if you reroll.

scor’s picture

Status:Needs work» Needs review
StatusFileSize
new4.54 KB

This patch separates the admin/user pages from the "administer site configuration" perm by using its own "access user administration pages" perm. The only perm required is "access administration pages" (which only grants access to the logs by default). Then you can choose the pages/permissions each role can access.
(patch against the latest HEAD 1.807)

Crell’s picture

OK, I *think* this works correctly. Unfortunately, I just realized I can't test it without doing a full reinstall, because the menu system is currently broken and never rebuilds menu_links. I'm afraid I don't have time to do that right now, as I need to get to bed.

Code looks OK visually, and lacking a better idea I'm OK with the "access user admin pages" permission. Can someone else test this with a fresh install after patching?

forngren’s picture

StatusFileSize
new4.41 KB

Patch works like a charm on a clean install. If someone can verify this, I think it's RTBC.

The attached patch is really them same patch with some cruft (i.e. Stripping trailing CRs from patch and eclipse workspace stuff) removed.

Wim Leers’s picture

Status:Needs review» Reviewed & tested by the community

- Patch applies cleanly.
- No real code additions, just some "minor" changes, hence no new comments which have to be reviewed.
- Everything works as advertised.

Seems like a RTBC to me!

Dries’s picture

Version:6.x-dev» 7.x-dev

Moving this to D7.

sun’s picture

Status:Reviewed & tested by the community» Postponed
Wim Leers’s picture

Status:Postponed» Needs review

Why mark this as postponed? Marking as cnr because it's probably outdated by now.

birdmanx35’s picture

Status:Needs review» Needs work

Just for the heck of it, this patch came up in the bingo. Tested against head, here's the failure message:
$ patch -p0 < 151311-user-perms.patch
patching file modules/user/user.module
Hunk #1 FAILED at 489.
Hunk #2 FAILED at 843.
Hunk #3 FAILED at 872.
Hunk #4 FAILED at 894.
Hunk #5 FAILED at 1493.
Hunk #6 FAILED at 1655.
Hunk #7 FAILED at 2345.
Hunk #8 FAILED at 2379.
8 out of 8 hunks FAILED -- saving rejects to file modules/user/user.module.rej

scor’s picture

well, not surpised, that's an old patch that will need to be rewritten for D7, and some things like renaming 'access control' to 'permissions' went in D6 already.

sun’s picture

Version:7.x-dev» 8.x-dev
mattyoung’s picture

sub

mr.baileys’s picture

Issue tags:+Security improvements

Tagging.

DuaelFr’s picture

Assigned:scor» Unassigned
Issue summary:View changes
Issue tags:+Needs issue summary update, +Needs beta evaluation

We absolutely need something like this!
Currently, there is only one permission to manage roles and permissions and to assign roles to users.

I think we could split that permissions as said above for 8.0.x then integrate something like Role Delegate in 8.1.x

Beware! I'm not sure this can still be included in 8.0. Do not start working on this before a proper beta evaluation is done.

DuaelFr’s picture

Issue summary:View changes
Issue tags:-Needs issue summary update
DuaelFr’s picture

Issue summary:View changes