I had an issue where users couldnt log in. The message "Login attempt failed for : Sorry. Unrecognized username or password." would appear in the logs. I took a look at the user_login code and noticed that if a user didnt exist, it would create a new user fine, but if that user DID exist, the value that was returned for $user was null, which in turned caused the login failure message that I saw.

The code below has an added else statement below new user creation, so that if a user exists, it will return that user instead of null.

-Darrian

function _ldap_integration_code_added_user_login($login_string, $pass) {
  global $ldap;

  $user = null;

  // Strip name and server from ID:
  if ($server = strrchr($login_string, '@')) {
    $name = substr($login_string, 0, strlen($login_string) - strlen($server));
    $at = '@';
    $server = substr($server, 1);
  }
  else {
    $name = $login_string;
    $at = '';
    $server = '';
  }

  if (ldap_integration_auth($name, $pass, $server)) {
    $account = user_load(array('name' => "$name$at$server"));
    $tmp_user->name = "$name$at$server";
    if (!$account->uid) { // Register this new user.
      // Changes to this user_save():
      //   1. 'pass' => $pass . Obviously. What I wonder is how could
      //      it be otherwise. Really.
      //   2. 'mail' => value of LDAP_EMAIL_ATTRIBUTE in the LDAP directory
      //   3. 'init' => same. BTW: what's the use of this field?
      //   4. 'ldap_authentified' => TRUE . There is a need to mark
      //      people as externally authentified.
      $dn = _ldap_integration_login2dn("$name$at$server");
      $mail = $ldap->retrieveAttribute($dn, LDAP_EMAIL_ATTRIBUTE);
      $user = user_save('', array('name' => "$name$at$server", 'pass' => $pass, 'mail' => $mail, 'init' => $mail, 'status' => 1, "authname_$module" => "$name$at$server",'roles' => array(_user_authenticated_id()), 'ldap_authentified' => TRUE));
      watchdog('user', t('New external user: %user using module %module.', array('%user' => theme('placeholder', $name .'@'. $server), '%module' => theme('placeholder', $module))), WATCHDOG_NOTICE, l(t('edit'), 'user/'. $user->uid .'/edit'));
    }
    else { // need to return the user if it already exists so they can log in
      $user = $account;
    }
  }

  return $user;
}

Comments

pablobm’s picture

pablobm commented

The problem with the code you comment is that such a thing should be stopped much earlier in the code. When existing users log in, they are accepted in the _ldap_integration_fake_user_login(), around these lines:

    ...
    if (!$user->uid) {
      $user = user_authenticate($edit['name'], $edit['pass']);
      
      // === HACK STARTS ===
    ...

It is because the module saves the user as an standard one in its database the first time it logs in, as I understand that Drupal does. (There's still a confusing bit that I don't yet grasp, but that seems to be the big picture).

So the piece of code you add should only be reached if the same user exists in the LDAP directory and the database, but with different password, what raises the question of their equivalency and, therefore, should not be let in as a security measure.

Anyway, that code is almost copied from user.module, function user_authenticate()

But please, if you can, give it some further thought, because I can't still understand why the original Drupal code, at user_authenticate(), sets $user->pass to the result of a call to user_password(), instead to a $pass, as I do in my code. This discussion may be related to this mistery.

And thanks for your help, of course. Thanks a lot.

pablobm’s picture

pablobm commented
Assigned: Unassigned » pablobm
Status: Active » Closed (fixed)

I'm closing this, if nobody minds.