I've been running the dev build of this module for some time because the last beta2 release is really out of date, and for the most part the dev builds have been exceptional. This last one, though, introduced a truly odd issue that I cannot track down.

Attempts to change the password for an LDAP account fail with no error message. Until the most recent dev build, this functionality worked fine. I'm running LDAP-only authentication, have properly configured the module for an SSL connection to Active Directory (and it does connect fine), and have correctly mapped the Drupal pass attribute to the AD unicodePwd attribute in the Drupal-LDAP fields mapping the LDAPData module. Field mapping is set to read/write.

I know the server is configured correctly because I have no problem altering the email address for users. Also, if I deliberately expose the unicodePwd attribute in the Attribute visibility & access control section of the same configuration page, making it both readable and editable by the user, password changes do work — it just requires going to the "ldap attributes" tab that then appears on /user/n/edit and setting it there. So I know the SSL configuration is set up properly. It just doesn't allow you to change your password the normal way via /user/n/edit.

In an effort to troubleshoot this, I added this after line 280 in LDAPInterface.inc:

print_r($attributes);
die();

The attributes array contained only the email address being changed and did not have any password in it. No wonder password changing is failing; the module is never asking AD to change it. But beyond that I'm somewhat a loss as to why that's actually happening.

Comments

macman824’s picture

macman824 commented
Status: Active » Needs review

Alright, I tracked this down. In ldapauth.module, there's this:

        if (isset($account->ldap_authentified) && (LDAPAUTH_LOGIN_PROCESS == LDAPAUTH_AUTH_EXCLUSIVED || !LDAPAUTH_SYNC_PASSWORDS)
          $edit['pass'] = NULL;
      }

Since I run an exclusive LDAP setup (and thereby do not sync passwords), this was causing the $edit['pass'] from the user account form to get obliterated before ldapdata could get its hands on it. Therefore, we need to make ldapauth aware that there are certain configurations of the ldapdata module that would make this behavior undesirable. To solve this, I've done the following:

Add to the top of ldapauth.module:

// If ldapdata is enabled, we need to be careful not to obliterate $edit['pass'] if attribute mapping is on.
if (module_exists("ldapdata")) {
	define('LDAPDATA_IS_ENABLED',				true);
	define('LDAPDATA_MAP_ATTRIBUTES',           6);
	if(_ldapdata_ldap_info($account, 'mapping_type') == LDAPDATA_MAP_ATTRIBUTES && count(_ldapdata_ldap_info($account, 'ldapdata_rwattrs')) > 0) {
		define ('LDAPAUTH_LEAVE_THE_PASSWORD_ALONE',	true);
	}
}

Then update ldapauth's conditionals for user edit form validation to take these into account:

        if (isset($account->ldap_authentified) && (LDAPAUTH_LOGIN_PROCESS == LDAPAUTH_AUTH_EXCLUSIVED || !LDAPAUTH_SYNC_PASSWORDS) && !(LDAPDATA_IS_ENABLED && LDAPAUTH_LEAVE_THE_PASSWORD_ALONE))
          $edit['pass'] = NULL;
      }

Now when this condition exists, we need to make ldapdata clean up what ldapauth can't. Lines 479-481 of ldapdata.module were:

    if (!($_ldapdata_ldap->writeAttributes($user->ldap_dn, $writeout))) {
      drupal_set_message(t('The data was not written to LDAP.'), 'error');
    }

Change them as follows:

    if (!($_ldapdata_ldap->writeAttributes($user->ldap_dn, $writeout))) {
      drupal_set_message(t('The data was not written to LDAP.'), 'error');
    }
	else {
		if (LDAPDATA_IS_ENABLED && LDAPAUTH_LEAVE_THE_PASSWORD_ALONE && !LDAPAUTH_SYNC_PASSWORDS) {
			$edit['pass'] = NULL;
		}
	}

That will correct the issue. ldapauth won't obliterate the password before ldapdata can use it, and ldapdata will in this configuration remember to clean up the password before Drupal can go and do something naughty by storing it in its database.

This has kept me at work way past closing time, so I'm not going to roll a patch tonight. I also think my solution is a bit ugly. I invite anyone to take this code, review it, make it cleaner, and roll their own patch. If that's not happened by the end of the week I'll do it as-is.

cgmonroe’s picture

cgmonroe commented

Whoops, it actually breaks it for changing all passwords.

This was an unexpected side effect of the fix for: #1525752: If "Remove email field from form" is selected, user email addresses can not be synced by ldapdata or changed at all by user_save

A simpler fix has been committed to the dev version (and will show up after it regenerated). The simple fix was to split the email and password handing in the ldapauth_user function into different hook_user ops.

The e-mail gets handled by the validate op so it doesn't interfere with ldapdata or ldapsync trying to update the user.

The password gets handled by the update op so it happens after ldapdata does it's update in the "submit" phase. And will prevent ldapdata/sync from setting it in the Drupal user entry. (as it was before).

E.g. New code looks like:

    case 'validate':
      if (LDAPAUTH_ALTER_EMAIL_FIELD == LDAPAUTH_EMAIL_FIELD_REMOVE || LDAPAUTH_ALTER_EMAIL_FIELD == LDAPAUTH_EMAIL_FIELD_DISABLE ) {
        unset($edit['mail']);
      }
      break;
    case 'update':  // Handle password mods after ldapdata does update in submit
      if ($category == 'account') {

        // If authentication is being done in "LDAP only" mode, passwords
        // should not be written to the database, or users would be able
        // to log in even after removing their LDAP entry.
        if (isset($account->ldap_authentified) && (LDAPAUTH_LOGIN_PROCESS == LDAPAUTH_AUTH_EXCLUSIVED || !LDAPAUTH_SYNC_PASSWORDS))
          $edit['pass'] = NULL;
      }
      break;
cgmonroe’s picture

cgmonroe commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.