Secure Password Hashes (phpass) integration for 6.x-1.x

patch should be run from your sites/*/modules directory, not the password_policy directory

I have done some testing and cannot get this to work with the phpass... going to revert back to the original storage of md5.
If anybody has better knowledge of how to implement this, please share. I am wondering how the 5.x patch works since this calls the same phpass functions.

re-rolling the patch, it is working correctly now. I needed to fix the validation check in the history constraint function: password_policy_constraint_history_validate. Original patch was hashing when it should have been checking.

changing status back to needs review

It would be nice somehow resolve the history constraint javascript. We have a requirement that all new password validations should happen without a page refresh.

i agree. I looked into it when making the patch and you would need to convert phpass's PasswordHash.php into a js version... which was a little bit more involved than I had time for.

I'm not sure porting PasswordHash.php is necessary, maybe some AJAX could help, don't like the async: false part of attached patch, but this way it fits nicely without a need to change password_policy.js.

I always forget about the AJAX approach.
Thanks for the patch update. I tested it and it appears to work correctly on my system. Have you experience any side effects with the synchronous request in your testing?

The problem with synchronous request is that it blocks UI for up to a second, which isn't noticable on a fast server but could be really annoying on slower ones.

Truth be told I considered lack of js support in this patch to be a feature, as the current approach of sending history of hashes embedded in html is IMHO a really bad idea, they are enough of a problem in the database, wouldn't want them laying aroud in browser's cache or being collected by some other user with 'administer users' privilege.

Attached patch makes the request asynchronous and appends to passwordDescription when it finishes, providing much better experience. Had to add additional param to evaluatePasswordStrength(), and change how passwordCheck() was invoked from passwordDelayedCheck() as context was lost, maybe it should be changed to outerItemElement altogether, not sure what the intention was, but currently there is separate state kept for each input (password & confirm). Also got rid of the old embedding hashes in HTML code because of security implications mentioned in previous post, so now both phpass and plain hashes are checked via AJAX.

I believe the loss of context you had describe is happening due to the lax usage of this throughout the original code... using this across function scopes makes my head hurt.

I agree this is a better implementation and handling of the historic passwords. I have been using the patch with no issues. Hopefully the module maintainer will implement the new patch in the next release.

Setting to reviewed for non-JS part as i didn't change anything there and it works as described, assuming recrit reviewed JS part. Just added some empty lines.

...and removed some trailing spaces.

I ran into one problem when using with javascript_aggregator. When aggregated and minified, a javascript error is produced due to the newline characters("\n") appended to the end of the lines in function password_policy_constraint_history_js. Since the original had this as well, I suspect it could also generate such an issue. I removed the newlines and everything worked as expected.

JS aggregation doesn't affect inline scripts, so I don't see how it could be the cause of described problem.

Added missing var keyword and throbber.

you're right the aggregation and minification should not affect he inline scripts...
I just tested a different drupal distribution with the "\n" still in the code and it works fine. There must be something crazy going on with the other distribution that I had errors on. Just confusing as to why removing the "\n" fixed the issue.