Edit user password should use LDAP to authenticate

We recently had the same issue, it can be done without making a patch.
Ofcourse a patch can be made if must be in the core.

Place the following in a form alter, it replace the validator that checks the password.

if ($form_id == 'user_profile_form') {
    foreach ($form['#validate'] as $key => $validate) {
      if ($validate == 'user_validate_current_pass' && $user->uid != 1) {
        unset($form['#validate'][$key]);
        $form['#validate'] = array_merge($form['#validate'], array(mymodule_current_pass'));
      }
    }
}

Then copy paste the original validate and replace to LDAP where needed (note this is "as is" copied from own module)

function mymodule_current_pass(&$form, &$form_state) {
  $account = $form['#user'];
  foreach ($form_state['values']['current_pass_required_values'] as $key => $name) {
    // This validation only works for required textfields (like mail) or
    // form values like password_confirm that have their own validation
    // that prevent them from being empty if they are changed.
    if ((strlen(trim($form_state['values'][$key])) > 0) && ($form_state['values'][$key] != $account->$key)) {
      $server = new LdapServer(LDAP_SERVER);
      $server->connect();
      $server->bind();
      if (isset($account->data['ldap_user']['init']['dn'])) {
        $dn = $account->data['ldap_user']['init']['dn'];
      }
      elseif (isset($account->ldap_user_current_dn['und'][0]['value'])){
        $dn = $account->ldap_user_current_dn['und'][0]['value'];
      }   
      $parts = explode(',', $dn);
      foreach ($parts as $part) {
        $piece = explode('=', $part);
        if ($piece[0] == 'uid') {
          $uid = $piece[1];
          break;
        }        
      }
      $result = $server->search('', '(uid='. $uid .')', array('mail', 'dn', 'userPassword'));
      $oldpass =  '{md5}'. base64_encode(pack('H*', md5($form_state['values']['current_pass'])));
      if ($oldpass != $result[0]['userpassword'][0]) {
        form_set_error('current_pass', t("Your current password is missing or incorrect; it's required to change the %name.", array('%name' => $name)));
        form_set_error($key);
      }
      // We only need to check the password once.
      break;
    }
  }
}

Hope this helps you

This only works for MD5 or SHA passwords. It doesn't work at all for SSHA, crypt or other schemes.

It also requires all users in an LDAP database to use the same password hashing scheme, which may not be the case when a long running LDAP server changes its default password scheme or if a block of users are imported into an LDAP system using a different password hash scheme.

I needed this functionality as well, i used the custom-module from #1, but changed the password check method, which was also mentioned in #4

function custom_ldap_extensions_form_alter(&$form, &$form_state, $form_id) {
  //Change user profile form, except for user 1.
  if ($form_id == 'user_profile_form'  && $form['#user']->uid != 1) {
    foreach ($form['#validate'] as $key => $validate) {
      //Remove current_pass validation, and replace with custom current pass validation.
      if ($validate == 'user_validate_current_pass') {
        unset($form['#validate'][$key]);
        $form['#validate'] = array_merge($form['#validate'], array('custom_ldap_extensions_current_pass'));
      }
    }
  }
}

function custom_ldap_extensions_current_pass(&$form, &$form_state) {
  $account = $form['#user'];
  //Validate each field which requires password. 
  foreach ($form_state['values']['current_pass_required_values'] as $key => $name) {
    //Validate only when field is not empty, and changed. 
    if ((strlen(trim($form_state['values'][$key])) > 0) && ($form_state['values'][$key] != $form['#user']->$key)) {
      $current_password = $form_state['values']['current_pass'];
      $dn = $form['#user']->data['ldap_user']['init']['dn'];
      $sid = $form['#user']->data['ldap_user']['init']['sid'];
      $credentials_test = custom_ldap_test_credentials($sid, $dn, $current_password);
      if(!$credentials_test){
        form_set_error('current_pass', t("Your current password is missing or incorrect; it's required to change the %name.", array('%name' => $name)));
        form_set_error($key);
      }
    }
  }
}

function custom_ldap_test_credentials($sid, $dn, $password){
  $result = FALSE;
  $ldap_server = ldap_servers_get_servers($sid)[$sid];
  $ldap_server->connect();
  if($dn != NULL && $password != NULL){
    $ldapbind = @ldap_bind($ldap_server->connection, $dn, $password);
    if ($ldapbind) {
      $result = TRUE;
    }
  }
  $ldap_server->disconnect();
  return $result;
}

It doesn't get the hashed password field to compare the passwords, but tries to bind with the given password.

Rerolled patch in #2 in a drush make friendly format.

I also stumped upon this problem and commented in #1884922: LDAP User: Password field disabled Makes use case of Provisioning Passwords from Drupal to LDAP unusable. I too think that the password should be verified against LDAP instead of Drupal.

But instead of altering or adding a validator, I think we should use the core's mechanism to override the password_inc path. I don't know if this has been introduced in a recent Drupal core version, but now there's a password_inc variable to allow other password checking & hashing mechanisms.

So here's my patch with a custom ldap_authentication.password.inc file.
However, by overriding the core's file and more specific by removing or not copying over parts of the code some tests (user.test & password.test) wouldn't pass anymore. So I have patched these tests as well in #2238603: The user test should not assume there's a _password_get_count_log2 function & #2238599: The password test should not assume there's a _password_get_count_log2 function.

By the way, for my patch to work you should disable and re-enable the ldap_authentication module, since the password_inc variable is being set in the hook_enable hook.

I noticed my patch didn't take the mixed authentication mode into account nor the user 1 authentication exception, so I uploaded a new patch which incorporates this.

Do we need to apply both patches at #6 and #10 to test that?

kenorb, my patch in #10 is another approach to the problem than was used in #6. So you don't need to apply both. I think both work but #10 is solving the problem by using Drupal's core possibility to provide an alternative password.inc file with an alternative user_check_password function.

I've tested patch #10 and seems to work fine.
Created user with 1st password, log-in and changed the password, re-login and 2nd password works fine.

I was able to change the LDAP password and create a new password within Drupal. However, both passwords still work, even with the newly created patch applied correctly. The only other thing I can think of, is that my provisioning is off. Do any of you know the correct provision mappings from Drupal to LDAP? And which encryption method to use? I've been having a hard time with this. I've checked other posts and nothing has helped. Appreciate it.

@pn255005

This issue is only about checking your current password through LDAP instead of comparing it to the the Drupal password.

There are some caveat's when you want to change your LDAP password from Drupal. I set this up using only the LDAP password for all authentication.
I use a custom module to bypass this problem with the code from comment #5
When changing your password also in AD (or another store) you need to disable the use of a current password, and enable provisioning for the password field. Under provisioning from Drupal to LDAP I added 'Pwd: User Only' to [unicodePwd] provisioning. You have to disable LDAP to Drupal provisioning for the password.

When using AD there are some more requirements specific for the password.

• you need to use an SSL connection (ldaps://)
• the password needs to be enclosed in quotes
• the (quoted) password needs to be encoded in 16-bit unicode (UTF-16LE)

A patch in #1954744: LDAP User: Can't provision from Drupal to Active Directory because password not sent as unicode solves the last two points.

Further does your bind-user (of other bind-method to LDAP) has to have permissions to change the LDAP-attributes. My sugggestion would be to experiment first to provision another ldap-field (username or mail) and then test further for the password field.

Also watch out for the 'password concerns'

• Provisioning passwords to LDAP means passwords must meet the LDAP's password requirements. Password Policy module can be used to add requirements.
• Some LDAPs require a user to reset their password if it has been changed by someone other that user. Consider this when provisioning LDAP passwords.

The first problem can cause some silent fails, that Drupal says the password has changed, but that it was rejected by LDAP without telling something.

@mallezie

Thank you for getting back to me. I really appreciate it.

I applied everything that you had mentioned. I received two errors.

First-

Parse error: syntax error, unexpected '[' in C:\inetpub\wwwroot\sites\all\modules\customldap\customldap.module on line 36

I commented out [$sid] and it gave me this:

Fatal error: Call to a member function connect() on a non-object in C:\inetpub\wwwroot\sites\all\modules\customldap\customldap.module on line 37

What do you think these could be stemming from? Or ways I can get around it. Should the custom module be tied to LDAP Server Module? Thanks again!

hmmm, i can't really reproduce this.

This points actually to a syntax error, could you post your lines 35-37?

It could be the way i get the server id ($sid) in

$sid = $form['#user']->data['ldap_user']['init']['sid'];

isn't very generic.
I use ' Use server which performed the authentication. Useful for multi-domain environments.' this fills in the $sid object on a user, using an other option, probably doesn't store the $sid on the user.

If you're only using one server. You could probably user ldap_servers_get_servers() (without function arguments). If you check the result of that function, you should be able to get the server-object.

$result = FALSE;
$ldap_server = ldap_servers_get_servers($sid)/*[$sid]*/;
$ldap_server->connect();

These are the lines for 35-37. I am only using one server. I changed the lines to read:
$result = FALSE;
$ldap_server = ldap_servers_get_servers();
$ldap_server->connect();

It is still giving me the Fatal Error (line 37) as it did before. Here is the entire code, maybe I am throwing a Syntax error that I'm not seeing.

function customldap_form_alter(&$form, &$form_state, $form_id) {
  //Change user profile form, except for user 1.
  if ($form_id == 'user_profile_form'  && $form['#user']->uid != 1) {
    foreach ($form['#validate'] as $key => $validate) {
      //Remove current_pass validation, and replace with custom current pass validation.
      if ($validate == 'user_validate_current_pass') {
        unset($form['#validate'][$key]);
        $form['#validate'] = array_merge($form['#validate'], array('customldap_current_pass'));
      }
    }
  }
}

function customldap_current_pass(&$form, &$form_state) {
  $account = $form['#user'];
  //Validate each field which requires password.
  foreach ($form_state['values']['current_pass_required_values'] as $key => $name) {
    //Validate only when field is not empty, and changed.
    if ((strlen(trim($form_state['values'][$key])) > 0) && ($form_state['values'][$key] != $form['#user']->$key)) {
      $current_password = $form_state['values']['current_pass'];
      $dn = $form['#user']->data['ldap_user']['init']['dn'];
      $sid = $form['#user']->data['ldap_user']['init']['sid'];
      $credentials_test = customldap_test_credentials($sid, $dn, $current_password);
      if(!$credentials_test){
        form_set_error('current_pass', t("Your current password is missing or incorrect; it's required to change the %name.", array('%name' => $name)));
        form_set_error($key);
      }
    }
  }
}

function customldap_test_credentials($sid, $dn, $password){
  $result = FALSE;
  $ldap_server = ldap_servers_get_servers();
  $ldap_server->connect();
  if($dn != NULL && $password != NULL){
    $ldapbind = @ldap_bind($ldap_server->connection, $dn, $password);
    if ($ldapbind) {
      $result = TRUE;
    }
  }
  $ldap_server->disconnect();
  return $result;
}

Try $ldap_server = reset(ldap_servers_get_servers());

OK I applied that code. It said that the password was changed but still did the same thing - able to login with both AD password and newly formed Drupal password. AD password still the same.

You mentioned "You have to disable LDAP to Drupal provisioning for the password". How do I go about doing that? Or is that already taken care of in the patches?

Thanks!

The provisioning was something wrong from my side, never mind that remark.

You should check if your drupal users table, contains a hashes password, that shouldn't be the case. Only your remote AD should contain the password. You could clear that field for some test-users.
It also seems your password change is rejected by the AD. Could you chek you could login to the AD with the new password (not from within drupal).

Sorry, don't want to be rude. But I don't think this ticket is supposed to turn into a personal support ticket, is it?

Ok, back to topic.

@Brian Altenhofel: You said my patch in #10 stores the password in clear text in LDAP. It has been a while, but if I remember right, the only important thing in my patch is the function user_check_password which checks the password against LDAP, as the title of this issue suggests. The other functions in my custom password.inc file are copied from Drupal's core password.inc so is the problem you point out related to those functions?

You're right Sneakyvv, sorry for derailing this issue.

Doesn't the approach in #5 works around the issues of storing and comparing hashed passwords?
This keeps the LDAP store responsible for the password storage. It's like logging in?
I could roll that approach in a patch, if it seems a more feasable approach? I think it's also much simpler?

I've read your code in #5 again, and it certainly looks like it will resolve the problem, but to me it seems like it's not "fixing" the underlying issue. It's a hack/altering of the existing core functionality. Meanwhile Drupal provides a mechanism to override the core password.inc file, so I'm led to think that it would be a cleaner approach to "throw" the core mechanism out and use our own password.inc file which does all password verification against LDAP. Of course altering forms is also something the core provides, so I'm not saying my solution is without a doubt the best approach.

Now i see, i do agree to override core password approach is cleaner than the form altering. I need to look some deeper inside the patch, but i'm sort of worried there is a lot of code duplication. Isn't there a way to only override the user_check_password, which is the only function we need to change here. The appraoch taken there seems great. (Test credentials is probably even better than binding).

I agree there's code duplication, because if mixed authentication mode is chosen I still need to do create/check the password just like the core does. Sadly there is no way to include the core's password.inc in our own password.inc since Drupal is scripted, not OO. So including the core password.inc would produce a fatal error (can not redeclare function). This leaves a dependency on the core's password.inc, which is obviously not desired. But I can't see any other way we can define our own password.inc and still use functionality from the core's password.inc without duplicating it. If you come up with something, I'm pleased to find out.

I'm not comfortable swapping out password.inc from core, because we have to sync with any core SAs

Those who want to use the patch are on their own to keep their password.inc in sync with core.

Reroll of #10, making it usable as a patch in a build system.

@jcnventura: Do you mind commenting on which version this is meant for? I assume for the Drupal 7 version? or is it for D8?

Thank you for doing this!

Edit: To follow-up, I applied this patch cleanly in the D7 version. I unloaded and reloaded the authentication module. it works beautifully!

tarek : )

Hello all,

I implemented the patch as above, but it creates two separate passwords that both work. One for Drupal and one for LDAP. As such, the LDAP password is never updated. I'm not sure how best to debug this. Any suggestions?

tarek : )