LDAP *: Why is ldap configuration keyed on user choosen langage ? [#1993092]

In the data the LDAP module attaches to the user ($account->ldap_user_prov_entries) the provisioning server is keyed on the langage of the user ?

Why is it so ?

The user language is purely a display thing and users can change it anytime, thus loosing the mapping in ldap_user_prov_entries[$language] ...

I don't understand the design decision but on top of that it induce bugs at user deletion from the LDAP (other issue opened for that) and probably in other places, since the configuration is always attached to the "und" langage (i.e. undefined I suppose) regardless of the default language on the site and thus when the user if finally created/updated with a language that is not "default"/"undefined" the module can't access its ldap_user_prov_entries anymore.
In the "delete user" sequence it means deletion fails ...

To reproduce

Add a langage to the site and allow it (even leaving english as default)
Create a user
checks its ldap_user_prov_entries the ldap_user_prov_entries entry wil be with language "und" while the account will have a language of "en".

=> user deletion in the LDAP fails when setup up to be done as such

Is there an awesome reason for this or can we get ride of the "language" keyed array for ldap_user_prov_entries ?

Comments

Comment #1

johnbarclay commented 15 May 2013 at 04:03

I agree. The code should be language independent of the user. A patch in this direction would be helpful.

Comment #2

Annakan commented 15 May 2013 at 18:09

It is not a big change but a sweeping change and I already have 3 patches in the pipeline and not a proper test environment.

For now I hard-coded the "und" key at retrieval time but that shall not last.

Could we solve "my" other issues and patches before getting to that ?

Comment #3

Annakan commented 20 May 2013 at 12:13

I am sending a test patch in bug #1994850: Group Sync fail with at least OpenLDAP and "memberOf" that include a temporary and ugly fix to that, i.e. forcing "und" key at retrieval time since only "und" is ever set.
Need more test to check if that behavior is dependent on the number of locale installed and then remove the whole "language keyed" thing but I need to stabilize a working copy before.

Plus removing the "language keyed" thing is probably easy but would touch lots of places and probably need somedb migration steps, something much easier for somebody more familiar with the code than I am and more familiar with "boilerplate update Drupal code" than I am.
And a decent git/test setup I lack at the moment.

Comment #4

kenorb commented 5 February 2014 at 12:56

Issue summary:	View changes
Related issues:		+#1994850: Group Sync fail with at least OpenLDAP and "memberOf"

Comment #5

johnbarclay commented 10 March 2014 at 03:54

Title:	Why is ldap configuration keyed on user choosen langage ?	» LDAP *: Why is ldap configuration keyed on user choosen langage ?
Priority:	Major	» Normal
Status:	Active	» Needs work

Can someone write this patch? I looked through #4 and didn't see the code related to this issue.

Comment #6

francescjr commented 3 March 2016 at 08:57

The part in #4 that solves this issue is just this: (LdapUserConf.class.php)

@@ -871,6 +871,7 @@
 
     $boolean_result = FALSE;
     $language = ($account->language) ? $account->language : 'und';
+    $language = 'und'; //HACK see issue #1993092: LDAP *: Why is ldap configuration keyed on user choosen langage ?
     if (isset($account->ldap_user_prov_entries[$language][0])) {
       foreach ($account->ldap_user_prov_entries[$language] as $i => $field_instance) {
         $parts = explode('|', $field_instance['value']);

I have this issue. I can't delete users on LDAP from Drupal because of this key. My site is multilingual and all accounts have a language, but the configuration is only set in 'und'.

This small patch helps for deleting user on LDAP. But I have no idea of the big picture. (still in my implementation everything seems to work fine, just this problem with deleting)

Comment #7

kenorb commented 4 March 2016 at 15:38

@francescjr Side note: 'und' can be replaced by LANGUAGE_NONE constant

Comment #8

francescjr commented 17 October 2016 at 18:39

The more I see the code, the less sense it makes to me that it is indexed by language. First of all, it is not indexed by language. It is indexed by the keyword "und" all the time. Independently of the language of the account...

This is where the information is set (function provisionLdapEntry from LdapUserconf.class.php)

  // need to store <sid>|<dn> in ldap_user_prov_entries field, which may contain more than one
        $ldap_user_prov_entry = $ldap_server->sid . '|' . $proposed_ldap_entry['dn'];
        if (!isset($user_entity->ldap_user_prov_entries['und'])) {
          $user_entity->ldap_user_prov_entries = array('und' => array());
        }
        $ldap_user_prov_entry_exists = FALSE;
        foreach ($user_entity->ldap_user_prov_entries['und'] as $i => $field_value_instance) {
          if ($field_value_instance == $ldap_user_prov_entry) {
            $ldap_user_prov_entry_exists = TRUE;
          }
        }
        if (!$ldap_user_prov_entry_exists) {
          $user_entity->ldap_user_prov_entries['und'][] = array(
            'value' =>  $ldap_user_prov_entry,
          );

The word "und" is just hard coded, the language is not checked at any time here. Also this structure "ldap_user_prov_entries" is not being modified anywhere else in none of the LDAP modules (checking for the word, it only returned here, on the other place is just checked, not changed).

I would just strip this key from all the code... But if for any reason this is needed for something, then the proper patch would be not on the deleting function, but here on creation-provisioning, just replace all 'und' for $account->language. The delete function doesn't need to be changed at all then.

I will write a proper patch when I have time.