Preventing Password Writing to Database

I noticed this problem too, and fixed it for 4.7. I meant to file a bug report about it a while ago, but didn't, sorry.

This patch avoids writing the password to the database by setting it to a random 20-character long password before saving. This prevents the user form logging in "directly". I am not using the "LDAP Authentication only" setting, but I don't think that matters.

The patch makes changes in two places, but I'm not sure why there are two sections of code that add users to the database.

This patch relies on the fact that password recovery is disabled when using LDAP.

Argh, I forgot to attach the patch (for 4.7)!

We solved this by adding

db_query("UPDATE {users} SET pass = '' WHERE name = '%s'", $name);

after the call to user_save();

A little lower-level but more correct since it's obvious at a glance in the database that no password is stored and there's no possible way anyone's password will ever hash to ''

Hope this isn't hijacking, but... a couple questions: 1) should this fix problems with users changing their LDAP passwords and then being able to login to Drupal with both the old and new passwords? 2) if I replace my ldap_authorization module with this one, will it work out of the box, or will I need to do something like UPDATE users SET pass='';

Please LMK if I should open new issue threads for these.

Thanks.

1) Yes, it will fix that.

2) You don't need to set all the existing passwords to '', though you could. If you don't, those users will still be able to log in with their old passwords, of course.

Hmmm.... it seems to me that this causes further issues, eg. when Gallery is embedded, since the Gallery username will then either be blank (so anyone could log in from the non-embedded interface) or it will be out of sync with the Drupal username. I think a more direct approach would be fitting, which is to compare the two passwords--which of course is impossible since, Drupal's is an MD5... unless you could do an MD5 of the LDAP password and compare that to Drupal's MD5?

Any thoughts on this? Is the SET pass='' solution Gallery-safe?

Or rather, I mean to say, just set Drupal's pass field to an MD5 of the LDAP password on every login.

I'm not sure what Gallery is, so I can't comment specifically on that. But, you shouldn't be able to log in with a blank password. The password field holds an MD5 hash of your password and there is no password that hashes to empty string. Thus the comparison will never succeed when the password field is empty in the database.

Setting the password to the MD5 hash of the LDAP password is what the code currently does, as I recall, but still allows the old password to continue working until you log in with the new password once.

After some putzing, I found that setting the Gallery users' passwords to an empty string as well, makes sure that the Drupal accounts are secure, Gallery is still accessible from within Drupal, but Gallery is no longer accessible from its standalone interface. I think this is acceptable for the time being.

Thanks for the help, jfitzell.

Here is a different patch for this same issue. This patch adds another configuration option so that a user who is using LDAP first then Drupal can select whether they want to store the password in the database or not. If never store is selected, a random password is actually written to the database just as if the user had selected LDAP authentication only.

As mentioned above, this patch is important in two ways:
1. If a user's LDAP password changes, but the password is stored in the Drupal database, the user will have to use the old password to login to a Drupal site instead of the new password. This is inconvenient for the user, but in addition if the user's LDAP account is closed, the user will still be able to login to a Drupal site if the password is stored. This might be undesirable in some circumstances.
2. In some situations where LDAP is used for authentication only (read only access), such as in a University or large organization setting, it may be prohibited to store the password (or any form of the password), and this patch will allow an administrator to prevent storing of any form of the password.

This patch is rolled against the most recent 4.7 build, unlike the first patch in this thread, which is from a previous version of 4.7. Unlike the SQL command mentioned earlier in this thread (which works just fine), this patch removes the password from the user object BEFORE user_save() is called. Thus, any other module that implements hook_user will get the random password instead of the real password. This makes it much easier to audit code for security problems (like other modules storing the password) because very few modules will see the real password. In addition, this helps with Gallery2 integration because the gallery2 password and the drupal password will now match (though both will be a random password, not the real password). I've tested this patch using gallery2 and everything seems to work fine.

AC

Here's the same patch for 5.x-1.0.

please check out HEAD. scafmac added the code required to write random passwords. It does not include the added functionality that this patch attempts to include.

Frankly, I also believe that the # of options of ldap/drupal integration is getting to be too many. scafmac's patch solves the issue and in efforts to keep the design simple, I'd rather keep it like that.