[PasswordHasher] Use sodium as "best" hasher if with algorithm=auto #41646
Conversation
|
Thank you for the PR.
I'm sorry but this is intended, we switched back to bcrypt as default algorithm in 5.3 (see rationale in #40176) so I'm going to close this PR. The bug seems to be in the docs, which is weird as it was changed in symfony/symfony-docs#14992. That's probably a merge that went wrong. Fixing the docs again would be great if you can. |
|
Oh, the docs are wrong. Okay, I didn't realize that, thanks for the explanation. #40176 sent me down an interesting rabbit hole ;-) I'll try to create a PR for the docs in the next few days. I'll basically just have to re-apply symfony/symfony-docs#14992, seems easy enough. |
|
Thanks! In case you struggle with something, just ping me on the symfony-devs slack :) |
…(pableu) This PR was merged into the 5.3 branch. Discussion ---------- [Security] Update description of password hasher config The description of the password hashers in the reference isn't up to date for Symfony 5.3. "Auto" now always uses bcrypt (see #14980 and #14992), but it wasn't reflected here. I initially thought this was a bug in the password hasher component itself and created a symfony/symfony#41646, but I've since learned that the switch to bcrypt was intentional :-) I updated all the hasher descriptions a bit and removed the part about sodium before PHP 7.2 because Symfony 5.3 requires PHP >= 7.2. I also added an extra paragraph for the bcrypt hasher because it was a bit mixed into the description of the "auto" hasher. Commits ------- d404d19 [Security] update description of password hasher config
According to the docs, setting
password_hashers.xxx.algorithm=autoshould prefer Sodium to native (bcrypt) if available:But this is not what's actually happening. New passwords are hashed with bcrypt (
$2y$) instead of Argon2ID ($argon2id$). TheMigratingPasswordHashergets an instance ofNativePasswordHasherinstead ofSodiumPasswordHasheras first parameter ($bestHasher).I think this is a simple mixup and the fix seems easy enough.
But I haven't found a good way to write a test for it:
MigratingPasswordHasherto run an assertion on itPasswordHasherFactory::getHasherConfigFromAlgorithmis private, otherwise I could simply test its outputI'm open for ideas. Or can we merge the fix without a test?