ACLs: Users can escalate each other's privileges -- how to avoid?

[TwoMice]

On my system we have two types of users: some need to edit all contacts, and some only need to view all contacts.  To accomplish this I created two ACL groups, two Roles, and two ACLs, like so:

View all Contacts:
- Group: "Level 1 Group"
- group type: "Access control"
- visibility: "User and User Admin Only"

- ACL Role: "Level 1 Access"
- Assigned to: "Level 1 Group"

- ACL:
- - Type of Data: A group of contacts
- - Group: all groups
- - Operation: view
- - Role: Level 1 Access


Edit all Contacts:
- Group: "Level 2 Group"
- group type: "Access control"
- visibility: "User and User Admin Only"

- ACL Role: "Level 2 Access"
- Assigned to: "Level 2 Group"

- ACL:
- - Type of Data: A group of contacts
- - Group: all groups
- - Operation: edit
- - Role: Level 2 Access

This works fine, except for this problem:  Alice, who's a member of Level 2 Group, can open up the contact record for Bob, who's only in Level 1 Group, and add Bob to Level 2 Group, thus giving him edit access for all contacts.

Neither user has Drupal's "edit groups" or "edit memberships" permissions, but I'm only guessing as to the relevance of that.

BTW, this is CiviCRM 3.1.3, but this is also the behavior on the sandbox demo.

How can I configure these ACLs so that Alice can edit all records, and add people to groups, but can't add anyone to Level 2 Group (or to Level 1 Group either, just on general principles)?  Can I configure things so that just being able to edit a contact's groups is not enough to add that contact to an Access Control group, only to other groups?

Thanks,
Allen

[xavier]

Hi,

You will need to create a module. I'd check in the pre hook if it's trying to save a group-contact element, and if the group is one of the protected one, add additional checking that the current user is a super admin.

X+

[TwoMice]

Hi Xavier,

Thanks for your response.

Somehow I was thinking that Civi would be able to block "Access"-type groups from the "add groups" list, unless the current user has a specific "assign access groups" permission. This is similar to the way MySQL allows user to assign privileges to other users only if they have the GRANT OPTION privilege, and then only to those privileges that they themselves have.

Nothing like that exists?

- A.

[xavier]

Not to my knowledge, and seems to be fairly specific, as you might want that some of the acls are assignable by some group but not some other...

but the hook is fairly simple to write.

X+

[TwoMice]

Yeah, you're right on both counts.  Just thought I'd check.

Thanks for the help!

-A.

[TwoMice]

In the end, my solution was to block out all ACL groups using hook_civicrm_aclGroup.  I did this by using hardcoded references to the specific group IDs, which is not the most flexible method, but works just fine since I don't add and remove ACL groups very often.

Here's some code in case anyone's interested.  There are other examples for this kind of thing in the HRD module (http://svn.civicrm.org/hrd/trunk/drupal/hrd.module).

Code: [Select]


/**
 * Implementation of hook_civicrm_aclGroup()
 *
 * Hides access to the specified groups
 */
function fooModule_civicrm_aclGroup( $type, $contactID, $tableName, &$allGroups, &$currentGroups ) {

    if ($tableName != 'civicrm_saved_search') {
        // this hook (according to the docs) also runs for events and profiles, but we only need it for groups
        return;
    }

    /* Hide access to some groups
     */
    static $blockedGroupIds = array(7, 12, 15);

    foreach ($blockedGroupIds as $groupId) {
        if ( FALSE !== $index = array_search($groupId, $currentGroups) ) {
            unset($currentGroups[$index]);
        }
    }

}


[FrTommy]

I can't get this to work.

I need to restrict access to adding a user to an ACL group to only the Administrator. This code is not working for me. I can still log in under my test user and add a user to an ACL Group.

I used your code exactly except for changing the group ids to my group ids. That didn't work at all.

So I thought what is this custom search line? I thought maybe change that line to be based off of the Administrator Contact ID. Still nothing.

Code: [Select]

function mycivimodule_civicrm_aclGroup( $type, $contactID, $tableName, &$allGroups, &$currentGroups ) {

  if ( $contactID = 5 ) {
        // this hook (according to the docs) also runs for events and profiles, but we only need it for groups
        return;
    }

    /* Hide access to some groups
     */
    static $blockedGroupIds = array(1, 2, 4, 6, 8);

    foreach ($blockedGroupIds as $groupId) {
        if ( FALSE !== $index = array_search($groupId, $currentGroups) ) {
            unset($currentGroups[$index]);
        }
    }

}


[TwoMice]

Hi FrTommy,

Thanks for the feedback.  Here are some ideas; forgive me if I'm shooting to low; I usually start with the obvious:

My first (and probably too obvious) question is always: Is the hook being fired? Try adding this line at the top of the function: 

Code: [Select]

die('yes, hook is firing here.');
Another important point: This test will always evaluate to TRUE (so the function is always returning without doing anything):

Code: [Select]

if ( $contactID = 5 )You probably mean to use the comparison operator == instead of the assignment operator = .

But really, comparing the contactID won't help you much, as this needs to work properly for all users.  Comparing $tableName != 'civicrm_saved_search' ensures this only runs for "save_search" type objects, which includes Groups.  (I think that's an odd naming convention, but that's how it is.)

Also keep in mind this hook does not fire at all if the user has the "edit all users" permission.

Let me know what progress you make, and if you still need help with it.

- TM

[FrTommy]

TM,

Thanks, I finally got it to work. I'm not a programmer but know just enough to be dangerous.

I had turned off the edit all contacts permission but found some documentation somewhere that lead me to think that I needed to turn off View all contacts as well. I did and your original code worked as advertised. I was seriously about to throw the computer through the window, lol.

Sometimes CiviCRM is totally counter intuitive about things, yet if you can ever figure out its quirks it's a beautiful thing.

[Donald Lobo]

Would be good if you can edit the documentation page and make it a bit more clear, so other folks dont trip and make the same mistake

lobo