Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[SecurityBundle] Passwords are not encoded when algorithm set to "true"#34738
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Merged
chalasr merged 1 commit intosymfony:3.4fromnieuwenhuisen:fix_algorithm_true_converts_to_plain_password_encoderDec 3, 2019
Merged
[SecurityBundle] Passwords are not encoded when algorithm set to "true"#34738
chalasr merged 1 commit intosymfony:3.4fromnieuwenhuisen:fix_algorithm_true_converts_to_plain_password_encoderDec 3, 2019
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
Member
derrabus commentedDec 1, 2019
Can we catch this earlier? imho, |
Author
nieuwenhuisen commentedDec 2, 2019
Sounds reasonable. I will take a look at the config validation. |
c2926d9 tocdb0b49CompareAuthor
nieuwenhuisen commentedDec 2, 2019
I have reset my previous updates and change the configuration validation. |
src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
derrabus approved these changesDec 3, 2019
src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
xabbuh approved these changesDec 3, 2019
chalasr approved these changesDec 3, 2019
851ffb9 tod00464fCompared00464f tocb429cdCompare5d593d5 to83a5517CompareMember
chalasr commentedDec 3, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Rebased on 3.4 since it applies there. Congratz for your first contrib! |
Member
chalasr commentedDec 3, 2019
Thank you@nieuwenhuisen. |
chalasr pushed a commit that referenced this pull requestDec 3, 2019
…set to "true" (nieuwenhuisen)This PR was merged into the 3.4 branch.Discussion----------[SecurityBundle] Passwords are not encoded when algorithm set to "true"| Q | A| ------------- | ---| Branch? | 3.4| Bug fix? | yes| New feature? | no| Deprecations? | no| Tickets |Fix#34725| License | MIT| Doc PR | -If the algorithm is set to `true`, password will be encode as plain password.```security: encoders: App\User\User: algorithm: true```The reason for this is the not strict comparison of php switches.```switch ($config['algorithm']) { case 'plaintext':}````true == 'plaintext'` is `true`, so the first case is hit. My first solution was to cast the algorithm to a string, to prevent this. After some feedback I have catch this problem earlier and does not allow true as valid value to the algorithm option.Ps. This is my first PR for Symfony, any feedback is welcome :-)!Commits-------83a5517 [SecurityBundle] Passwords are not encoded when algorithm set to \"true\"
Contributor
mhujer commentedDec 3, 2019
@nieuwenhuisen Thanks for fixing it! 👍 |
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading.Please reload this page.
If the algorithm is set to
true, password will be encode as plain password.The reason for this is the not strict comparison of php switches.
true == 'plaintext'istrue, so the first case is hit. My first solution was to cast the algorithm to a string, to prevent this. After some feedback I have catch this problem earlier and does not allow true as valid value to the algorithm option.Ps. This is my first PR for Symfony, any feedback is welcome :-)!