Hello

I noticed that if "Recreate Group Mappings Upon Login" is enabled, users are not removed from groups before repopulating the users_roles table. For the record, my module is configured with "Groups exist as LDAP entries where a multivalued attribute contains the members' CNs".

It seems like _ldap_integration_take_role_from_user($user, $role) is called with $role as a full DN. So, the function queries for whole DNs in the role table instead of using short names.

To fix the problem, I used the same code found before _ldap_integration_give_role_to_user($user, $role) to parse the ldap_registered_groups value. Is this the correct way for me to tackle the problem or should the value for ldap_group_entries_attribute_full_dn be evaluated first?

Old:

    642   $registered_groups = variable_get('ldap_registered_groups', array());
    643   if (variable_get('ldap_map_group_signoffs', false)) {
    644     // First, we take every mapped role from the user, later below
    645     // we'll grant back those deserved.
    646     foreach ($registered_groups as $role) {
    647       _ldap_integration_take_role_from_user($user, $role);
    648     }
    649   }

New:

    642   $registered_groups = variable_get('ldap_registered_groups', array());
    643   if (variable_get('ldap_map_group_signoffs', false)) {
    644     // First, we take every mapped role from the user, later below
    645     // we'll grant back those deserved.
    646     foreach ($registered_groups as $group) {
    647       if ($role = $ldap_group_role_mappings[$group]) {
    648         // Just that
    649       } else if (preg_match('/^[^=]*=([^,]*),.*$/', $group, $matches)) {
    650         $role = $matches[1];
    651       }
    652       _ldap_integration_take_role_from_user($user, $role);
    653     }
    654   }

Thank you

CommentFileSizeAuthor
ldap_integration.patch.txt824 bytesoliveaddict

Comments

oliveaddict’s picture

Status: Active » Needs review
pablobm’s picture

Assigned: Unassigned » pablobm
Status: Needs review » Reviewed & tested by the community

Thanks again oliveaddict. I am working on a big code refactoring now, and I noticed those name transformations were not taken into account when denying roles at startup. Next version will address this.

kreaper’s picture

Status: Reviewed & tested by the community » Closed (fixed)

Please download the new code and test