I found that users can login using a space character as the password as long as the user name is valid.

Here's a quick fix.

// $Id: LDAPInterface.php,v 1.2.2.3 2006/07/23 09:29:50 pablobm Exp $

function connectAndBind($dn = '', $pass = '') {
//heres a quick fix
if ($dn && !$pass) return NULL;
//
$this->initConnection();

$con = $this->connection;

if (!$this->bind($dn, $pass)) {
watchdog('user', t('LDAP Bind failure for user %user. Error %errno: %error', array('%user' => $dn,'%errno' => ldap_errno($con), '%error' => ldap_error($con))));
return NULL;
}

return $con;
}

Comments

pablobm’s picture

Ugly one, but I can't reproduce it on my machine.

Do you work in "Only LDAP" mode or "Drupal, if fails, use LDAP" mode?.

Also, could you please try to log in that way as a user who has not been authenticated before?. I would like to know if that happens when the user has not yet been copied to Drupal's DB.

Thanks,

cmsanche’s picture

I work in "Only LDAP" mode. If the user has not been authenticated before and there is a valid ldap account, the user is created in the drupal database.

aclight’s picture

I'm able to reproduce the bug. I'm in "Drupal, if fails, use LDAP" mode. If the account has already been created via a LDAP login, using a single space as the password results in a successful login. However, if an account does not already exist for the username, the login fails.

pablobm’s picture

OK, then. I still cannot reproduce this problem, but I have commited a fix. Hope it works for you.

aclight’s picture

The changes you made didn't work on my system, and in fact prevent logging in. The error message returned is "Sorry. Unrecognized username or password. Have you forgotten your password?"

Commenting out the following lines you added (around line 54) works fine, but of course doesn't fix the problem.

if ( ! trim($pass)) {
     return $ret;
}

Using the fix provided by cmsanche at the top of this thread results in the same error message.

I've successfully solved the problem by adding the following line at line 72 in ldapauth.module:

if (!$pass) return FALSE;

I'm not sure if it's a bad idea to fix the problem this way, though.

I wonder if you aren't able to replicate the problem because your LDAP server doesn't allow anonymous binding? That seems to me to be the reason for this problem (accepting a space as a password), but I'm not positive that's the case.

pablobm’s picture

Assigned: Unassigned » pablobm

Ooops... Addressed again. Still, I could not reproduce it, tried many different configurations and none showed this error.

aclight’s picture

I'm sorry to say, but this still doesn't fix the problem for me. It does allow me to login normally, but a space will still allow login.

I know you said you couldn't reproduce the problem, so I have tried to do a little more debugging. The reason your fix doesn't work is that the password is already blank (not a space) by the time your ($pass != trim($pass)) line is called, so that evaluates to true.

I tried adding the debug line:
watchdog('user', "1pass:-->".$pass."<--");
just after the global $ldap line in the function ldapauth_auth($name, $pass, $server) of the file ldapauth.module. The message printed in the error log of drupal was:
1pass:--><--

I used a single space as the password, so clearly it's not getting through as a single space to this line. Is this the first function called in the ldap_integration module, or is there another function called somewhere else? If so, let me know and I will try to help you figure out where the problem is.

Thanks for your quick responses so far.

aclight’s picture

I now see why the password is arriving at the ldap_integration module already trimmed--in user.module, around line 940 (in function user_login_validate()), is the following:

   else if ($form_values['pass']) {
      $user = user_authenticate($form_values['name'], trim($form_values['pass']));

so it looks like drupal is trimming the password that is entered by the user, thus your fix to compare the trimmed versus untrimmed password won't work since the password will already be trimmed.

Is there a problem with putting a check for a blank password in the ldap_auth.module, as I mentioned a few posts ago?

Thanks

pablobm’s picture

Hi again guys. I finally put that check for empty passwords on the place your are doing it. "This should work now".

I started adding the check on LDAPInterface.php because it was a more generic place for that to be done, and it is common for both 4.6.0 and 4.7.0 versions. I couldn't just check for passwords being empty, though, as there are cases when that function is used for anonymous binding.

On the other hand, after examining the code, I think this issue has more to do with the LDAP directory configuration than with the module itself. But adding this check causes no harm, so there it is.

So... everybody happy now?

aclight’s picture

That fixes the problem for me. Thanks for making the change.

pablobm’s picture

Status: Active » Fixed

At last...

Anonymous’s picture

Status: Fixed » Closed (fixed)