Great module - thanks. It really simplifies the process and will definitely clear user confusion in the reset process.

I have been testing the module and found that the reset password on the altered form is saved to the Drupal database un the user table in clear text rather than hashed. This definitely caused problems for me and the user password was insecure and does not function.

To address this issue, I updated the simple_pass_reset_pass_reset_submit function in the module to:

// This logic copied from user_pass_reset().
$account = $form_state['user'];
$GLOBALS['user'] = $form_state['user'];
$account->pass = user_hash_password($account->pass);
user_login_finalize();

Looking for confirmation before generating and posting a patch. Is anyone else seeing this?

CommentFileSizeAuthor
#8 2367529.simple_pass_reset_login.patch1.86 KBrszrama
#5 password_not_hashed-2367529-5.patch562 bytesheylookalive

Comments

sebastien m.’s picture

sebastien m. commented

Hi Mark,

Can you explain step by step how to reproduce this issue ?

Thanks,

choribaquero80’s picture

choribaquero80 commented

Your solution works great!

I had the same issue, I was having issues when any user reseted their password, after looking to the information with Devel I notice that the password was plain text (so a "123" password was stored as "123")

Thanks for the solution.

mxwitkowski’s picture

mxwitkowski commented

@Sebastien - I encountered this issue whenever a user reset their password using the 'forgot password' process. When the user follows the link emailed to them and resets their password, it is stored as plaintext in the drupal database.

vali hutchison’s picture

vali hutchison commented

Yes, great improvement to the overall User experience - should be in Drupal Core! For the Drupal sites I manage, the single most common support request we get form users is related to confusion they have with the password reset process and this just makes much more sense.

I also had the same issue with password's not getting hashed - but interestingly, only on the live site server (using Nginx and PHP 5.6.4-1) whereas on my local dev environment (using Apache on Mac with PHP 5.5.14) with all the same code and same DB, it works fine.

When I added the $account->pass = user_hash_password($account->pass); line from mxwitkowski's original post to the live site then it worked fine. Also worked fine on the dev site (ie it didn't disrupt it already working).

Anyone got a clue as to why it doesn't work in some cases?

heylookalive’s picture

heylookalive commented
StatusFileSize
new password_not_hashed-2367529-5.patch562 bytes

Just experienced this on an Acquia server, whereas was fine on my local vagrant box :/

Find attached a patch.

djschoone’s picture

djschoone commented
Priority: Normal » Critical

Just found this issue. I think it is Critical, because passwords should never go unhashed into the database.

Tested the patch: works like a charm.

heylookalive’s picture

heylookalive commented

I'd agree with that!

rszrama’s picture

rszrama commented
Priority: Critical » Major
Status: Active » Fixed
StatusFileSize
new 2367529.simple_pass_reset_login.patch1.86 KB

The issue isn't that the new password isn't being hashed. It's that some module or another on your site is writing to the database on hook_user_login() as far as I can tell. By the time the code this patch is concerned with is invoked, the core submit handlers for this form have been invoked and saved the new hashed password. But somehow the user profile data is being re-saved, and because this module's submit handler sets the currently logged in user using outdated data from the $form_state, the global user account object has the unhashed password in it.

All that to say, the solution is to imitate user_login_submit() and load the user object afresh. There may be other unknown changes in it anyways. See the attached patch if you're curious what it looks like. I think the PHP difference some people pointed out was irrelevant (I tested in both PHP 5.5 and 5.6 and had hashed passwords) and suspect there's something else different between your site's environments that caused the unhashed password to be saved after the hashed one was.

Committed.

(Since this module itself wasn't saving the unhashed password, I demoted it to Major.)

  • rszrama committed 237b78d on 7.x-1.x 
    Issue #2367529 by rszrama, heylookalive: load the user account afresh...
heylookalive’s picture

heylookalive commented

Will give latest dev a go, thanks for this. Do you have a sense when you'll next do a stable release?

rszrama’s picture

rszrama commented

Just rolled it! Waiting on d.o to package it up now. : )

rszrama’s picture

rszrama commented

It's up now. : )

vali hutchison’s picture

vali hutchison commented

Thanks @rszrama for the update - just updated to 7.x-1.5 and works fine now in both of the environments I'm using (Apache and Nginx) - passwords properly hashed.

rszrama’s picture

rszrama commented

W00t w00t, that's what I like to hear. : D

heylookalive’s picture

heylookalive commented

Excellent! Thanks again :)

Status: Fixed » Closed (fixed)

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

john_b’s picture

john_b commented

Thanks for the module, and for the report and the fix.

I know this is old. However, not everyone applies non-security updates, so I suggesting marking a fixed update as a security update.

The risk is that one leaves the update and ends up with many hundreds of user name + email + plain text password sets in the users table, which is a real security risk. I have seen that happen.