I have users that need the "administer permissions" permission and to assign roles but they must not be able to assign the admin role to themselves or others. I have tried just checking the role I want them to assign and not the admin role but they can still do it.
I am using user protect which prevents them from editing the admin role or deleting it but it doesn't prevent them from assigning themselves the role. I am also using locked permissions so they cannot adjust their own permissions but only change the permissions for other users roles. I thought role delegation might do this. Can someone please tell me if I am missing something here? Can we not restrict users from assigning themselves certain roles if they have the "administer permissions" permission?
Jason
Comment | File | Size | Author |
---|---|---|---|
#19 | role_delegation-add-compatibility-with-permission_lock-582524-19.patch | 799 bytes | Shevchuk |
| |||
#14 | role_delegation.patch | 5.39 KB | dgersting |
Comments
Comment #1
jasont28 CreditAttribution: jasont28 commentedIs this module dead? It has not had a commit for 20 weeks and requests are not being responded to here? Would it be a good idea to merge this module with the auto assign role module?
Jason
Comment #2
Martin.Schwenke CreditAttribution: Martin.Schwenke commentedHi Jason,
It sounds like your original question doesn't relate to the Role Delegation module. The idea of this module seems to be that you don't assign the "administer permissions" permission. Instead you use the Role Delegation module to say that they can assign certain roles... and the roles that you allow people to assign probably don't include admin.
It sounds like the Role Delegation module is what you need... but you're not actually using it. Or have I missed something in your question?
peace & happiness,
martin
Comment #3
jasont28 CreditAttribution: jasont28 commentedHi Martin,
Thanks for the reply.
"without them needing the administer permissions permission" and requiring them not to have the administer permissions are very different. This module is rendered useless if "administer permissions" is enabled. Instead, my opinion is that "administer permissions" should be optional instead of disallowed.
Here is where the problem lies. If, as a system administrator, you would like an admin to create roles for users or assign roles to users, it makes sense to provide them with "administer permissions".
Instead of system administrators being contacted each time an admin wishes to change a permission for a role they can delegate, they can do it themselves. If system admins wish to prevent the admins from adjusting other permissions, they can use another module like permission lock. I just find the efficiency behind blending role delegation with administering permissions rather floored.
Jason
Comment #4
Andrew Schulman CreditAttribution: Andrew Schulman commentedJason, this is not possible as you've stated it. "administer permissions" allows a user to create roles, assign permissions to roles, and assign roles to users. So once you grant someone "administer permissions", they can do anything on the site, either right away for the operations I just mentioned, or by first granting themselves the necessary permission.
What I think you're asking for is a new permission, say "administer permissions on delegated roles", that would allow a user to set the permissions for any role that he's allowed to delegate. To make this useful you would not grant "administer permissions", since that allows everything. Also, there would have to be a way to ensure that the admin couldn't grant any permission at all to the delegated roles, but only a subset determined by the sysadmin. For example, he might only be able to grant permissions that he himself has: see #491732: Enhance "administer permissions" control mechanism.
I agree that this would be a nice enhancement. It might not even be that hard-- newbuntu seems to have worked out some details at #491732.
BTW, re whether role_delegation is dead: it's not, but see #661622: Is role_delegation still being maintained?.
Comment #5
jasont28 CreditAttribution: jasont28 commentedThanks for the reply Andrew. With the help of the Permissions Lock module, one can limit the permissions a user can access, even when they have the 'administer permissions' permission. That module can do what you have suggested above, however the Role Delegation module loses all of its control when 'administer permissions' is granted.
In my opinion, it has been a bit of an oversight when coding this module, users who can assign roles will often need access to change the role capabilities. At least in an environment where developers are passing on websites to administrators. The only change that is needed is to prevent Role Delegation from being overridden by 'administer permissions'.
That is a great offer by you to get in on the co-maintaining of this module. It seems as though David Lesieur has been active in the forums maintaining other modules but this one seems to need some love.
Jason
Comment #6
Andrew Schulman CreditAttribution: Andrew Schulman commentedWell I'm not sure if I'd really call it an oversight. "administer permissions" is known as the sledgehammer permission for the reasons I've mentioned. Role delegation just obeys its traditional meaning: if a user has "administer permissions", then role delegation lets him assign any role.
It would be simple to remove that condition, but it would change the behavior of role delegation in some probably unexpected ways. Maybe a new permission, "administer roles", could be introduced to allow assigning all roles, and no longer allow "administer permissions" to do it.
I'll take a look at Permissions lock, and think about this some more. I see the problem you're describing, and I agree that there should be a solution, but I'm not sure yet what the best solution is. Proposals are welcome.
Andrew.
Comment #7
Andrew Schulman CreditAttribution: Andrew Schulman commentedSorry, there is of course already an "assign all roles" permission. Maybe really all that's needed is to remove the assumption that "administer permissions" also implies "assign all roles". Again I need to think about that.
Comment #8
David Lesieur CreditAttribution: David Lesieur commented@Andrew: You have explained the point every well (thanks!). Role delegation just follows the logic that if a user can administer permissions (thus create and edit roles), then it would be counter-intuitive to prevent that user from assigning some roles.
@jasont28: Your request seems to be a valid one, but I don't feel that those use cases should be handled by Role Delegation, especially if the Permissions Lock module already does the job. I'm very much tempted to set this issue as "won't fix", unless perhaps if Role Delegation happens to not play nice with Permissions Lock.
Comment #9
jasont28 CreditAttribution: jasont28 commentedThat would be a good solution Andrew. It would require the least amount of adjustments according to how the module code looks.
That is the problem David, Permissions Lock and Role Delegation to do not play nice together. The point of Permission Lock is to allow 'admins' the ability to change some permissions without giving them the entire website's permissions. The problem is, if you do that, Role Delegation becomes useless because it assumes that having the 'administer permissions' permission, lets you add and delete every role available.
Comment #10
David Lesieur CreditAttribution: David Lesieur commentedThe Permissions Lock module can limit what roles are available to users with the "administer permissions" permission. It would be nice if Role Delegation honored Permissions Lock's limits.
Comment #11
jasont28 CreditAttribution: jasont28 commentedThanks for that David.
Comment #12
Andrew Schulman CreditAttribution: Andrew Schulman commentedI can see two possible ways of handling this.
The advantage of this approach is that it makes Role delegation automatically work with either Permissions Lock or any other module that restricts the power of 'administer permissions'. The disadvantage is that at sites that aren't using Permissions Lock, 'administer permissions' will no longer be a complete administrative right-- the admin will have to first assign 'assign all roles' to a user, before that user can assign all roles.
Personally I prefer #1. The change isn't really that big for non-Permissions Lock sites, and can be easily documented.
Either way, I think it would be a good idea to provide this feature. There's been a lot of discussion that 'administer permissions' is too broad, and it would be useful to support sites that want to limit it. In fact, I'd like to see Role delegation expand at some point into Role and Permissions Delegation, to provide a complete solution-- but that's for another day. At least this would be a step in that direction.
Comment #13
David Lesieur CreditAttribution: David Lesieur commentedThanks for the excellent analysis.
#1 seems a reasonable solution in comparison to a module-specific solution like #2.
I have checked the patches only very quickly (for now). I'm not sure what happens after removing the
user_access('administer permissions')
check inrole_delegation_user_operations()
. I'm afraid that this change will cause duplicate add/remove role operations being added (those of core and those of Role Delegation) when the user has the 'administer permissions' permission. Perhaps Permissions Lock does take care of this (I have not tested), but for sites without it, it looks like we'll have duplicates for sure.Comment #14
dgersting CreditAttribution: dgersting commentedThis issue hasn't been updated in over a year, therefore I've developed a working patch for this issue since it looks like active work on this module has stalled.
This patch effectively applies the 1st patch mentioned by Andrew (in post #12), but also solves many integration issues with the back-end as raised by David (in post #13). This patch treats "assign all roles" as the 'god' permission when it comes to permissions and roles, whereas Drupal's "administer permissions" is only used to allow access to the permissions page within the admin area.
This patch, when applied, will restrict the user's permissions page to only show the roles the user may delegate, as well as only show the "assign ROLE role" permissions for those roles. The "Permissions Lock" module may then be used to further restrict which permissions the user sees, but is not required for this to work.
Comment #15
Andrew Schulman CreditAttribution: Andrew Schulman commenteddgersting, thanks for posting the patch. Work on this module hasn't stalled, but providing a patch is a good way to get work moving on this particular issue.
The issue is a bit complex to reproduce and test. But I'll look at your patch within the next week and see if it seems to work right.
Comment #16
Juan C CreditAttribution: Juan C commentedSubscribe
Comment #17
marinex CreditAttribution: marinex commented@dgersting Hi, I tried your patch and it gave me:
warning: in_array() [function.in-array]: Wrong datatype for second argument in /var/www/website.ltd/sites/all/modules/role_delegation/role_delegation.module on line 356
This warnig is when user Tester (is not user 1) with 'administer permissions' permission go to 'admin/user/permissions'. The user has not 'assign all roles' permission. The $removedRoles is NULL
I tried the 1.4 and 6.x-1.x-dev-2011-Jun-17
Edit: If I declare $removedRoles = array(); then the warning is away
thanks M.
Comment #18
Shevchuk CreditAttribution: Shevchuk commentedQuick and dirty D7 patch (against 7.x.-1.1 version).
Comment #19
Shevchuk CreditAttribution: Shevchuk commentedComment #20
candelas CreditAttribution: candelas commentedI have tried dev and I don't have this problem...
Comment #21
wranvaud CreditAttribution: wranvaud commentedThe problem is still present in dev, I've checked the permission "Administer permissions" and on the user account I can assign the role "Administrator" to anyone = not secure!
Patch at #19 did work for me against 7.x-1.1. THKS
Comment #22
JeroenTIt’s been a while since the last comments in this issue. If this is still a problem feel free to reopen and provide patches.
Comment #23
NWOM CreditAttribution: NWOM commented#19 has not been commited yet. Please review.