Posted by pvasili on February 17, 2009 at 10:22pm
| Project: | Admin role |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | dww |
| Status: | closed (fixed) |
Issue Summary
See at: http://www.drupal.ru/node/24003
English:
The module defined as (for example D5.x): user_access('administer site configuration')
as needed user_access('administer permissions')
Now (The user has the right to configure the module to access to administer the site, even if he does not have rights to manage rights.)
Comments
#1
6.x is affected too
#2
Yes, definetely a glaring security hole here.
A simple check could be sufficient to close this hole:
- require that a user has the "administer permissions" privilege to be able to change adminrole setting(s).
Example:
If you want to assign some external administrators to do admin work on your drupal site, and then prepare a role and users for them so that they can do most of the admin work, including be members of "administer site configuration", you still do NOT want them to access user accounts, and of course not be able to assign themselves more privileges than they have been given. Therefore they should not be able to administer adminrole so that they can give one of their own roles full permissions... (they can as of now)
This is actually a critical issue with a seemingly easy fix.
Strange it has been untouched here in the issue queue for such a long time...
Very handy module, "cannot live without it", but this bug renders this module unusable on sites that need to give some admins "medium administrator privileges".
#3
"This is actually a critical issue with a seemingly easy fix." Maybe if you submit a patch fixing this, it will get rolled into the module.
#4
Having heard positive things about this module, I decided to take a look today to see if it was suitable for a site I've got to setup. Yeah, this issue is really bad -- basically makes any kind of admin priv separation impossible. It's a little discouraging that it was reported over 1/2 year ago with no reply from either maintainer, much less a fix. The patches are indeed trivial. Attached here for the end of the DRUPAL-6--1 and DRUPAL-5 branches.
#5
We should just use the 'administer users' permission like we do with this functionality in D7. This would be fixed with #615336: Admin Role - Permissions and Sync module with D7-implementation for upgrading ease..
#6
Fixed as a part of #615336: Admin Role - Permissions and Sync module with D7-implementation for upgrading ease.. The fieldset is now on the Admin > Users > User settings page like it is in Drupal 7. That page is also controlled by the 'administer users' permission, so I'm confident this is solved now.
#7
Automatically closed -- issue fixed for 2 weeks with no activity.
#8
Wouldn't it make more sense to use the
administer permissionspermission instead of theadminister userspermission? After all, the adminrole module basically does administer permissions for the administrator role: It turns them all on.I have a situation right now where I want to give my client the
administer userspermission so that he can administer users, and customise the emails that are sent out to them, etc. It is *not* my intention to let him administer permissions, and I don't want him to be able to give himself access to everything under the hood on his new drupal site.Please pardon my ignorance if the solution that is already in place makes a lot of sense in a D7 / upgradability issue, because I haven't looked much into either of those things. But just for a D6 site like the one I am working on, it seems to me like adminrole would be better off hooking itself onto the
administer permissionspermission.#9
The 'administer users' permission is the role that can assign roles to users, so it makes sense that this module uses that permission. Plus you'd run into this same problem with D7 core.
#10
Oh, right-- I should have thought of that.
So I guess the issue is more that it would be nice is there were separate administer user permissions: One for setting permissions, and one for configuring the emails that go out....
And now that I'm thinking about it, I'm realising that there is already a solution to this: the Role Delegation module. That way, you can set it up so the site managers can assign any role to anyone... *except* for the administrator role, which is only assigned to the drupal experts.
#11
I'm just looking more into this. I have a bunch of roles, including "manager" and "administrator". My client is a manager right now, and I'd prefer that he not become an administrator.
I am logged in as him, and went to edit his user page, to see what he would see, expecting to see the checkbox there that he could have used all along to make himself into an administrator... but it isn't there.
I wan't sure if this was because of some custom code that I might have put in place months ago to hide this from him, but it is not. It is coming from
user.module, in theuser_edit_form()function. This snippet shows where the checkboxes are inserted that let you assign roles to users.if (user_access('administer permissions')) {
$roles = user_roles(TRUE);
...
$form['account']['roles'] = array(
'#type' => 'checkboxes',
'#title' => t('Roles'),
'#default_value' => $default,
'#options' => $roles,
DRUPAL_AUTHENTICATED_RID => $checkbox_authenticated,
);
...
}
Notice the
administer permissionspermission that is required to do this.So, I say again what I said in my comment #8 above: That you should need the "administer permission" permission to get to choose which role is the adminrole.