Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[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

Conversation

nieuwenhuisen
Copy link

@nieuwenhuisennieuwenhuisen commentedDec 1, 2019
edited by chalasr
Loading

QA
Branch?3.4
Bug fix?yes
New feature?no
Deprecations?no
TicketsFix#34725
LicenseMIT
Doc PR-

If the algorithm is set totrue, 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' 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 :-)!

dmaicher reacted with thumbs up emojiderrabus reacted with hooray emoji
@derrabus
Copy link
Member

Can we catch this earlier? imho,algorithm: true shouldn't pass the config validation.

jvasseur, dmaicher, nieuwenhuisen, xabbuh, keichinger, and apfelbox reacted with thumbs up emoji

@nieuwenhuisen
Copy link
Author

Sounds reasonable. I will take a look at the config validation.

@nicolas-grekasnicolas-grekas added this to the4.3 milestoneDec 2, 2019
@nicolas-grekasnicolas-grekas changed the base branch frommaster to4.3December 2, 2019 10:07
@nieuwenhuisennieuwenhuisenforce-pushed thefix_algorithm_true_converts_to_plain_password_encoder branch fromc2926d9 tocdb0b49CompareDecember 2, 2019 20:59
@nieuwenhuisen
Copy link
Author

I have reset my previous updates and change the configuration validation.
Now is true not allowed as algorithm config value.

@nieuwenhuisennieuwenhuisen changed the title[Security] Passwords are not encoded when algorithm set to "true"[SecurityBundle] Passwords are not encoded when algorithm set to "true"Dec 2, 2019
@chalasrchalasrforce-pushed thefix_algorithm_true_converts_to_plain_password_encoder branch from851ffb9 tod00464fCompareDecember 3, 2019 20:55
@chalasrchalasr modified the milestones:4.3,3.4Dec 3, 2019
@chalasrchalasrforce-pushed thefix_algorithm_true_converts_to_plain_password_encoder branch fromd00464f tocb429cdCompareDecember 3, 2019 21:00
@chalasrchalasr changed the base branch from4.3 to3.4December 3, 2019 21:04
@chalasrchalasrforce-pushed thefix_algorithm_true_converts_to_plain_password_encoder branch from5d593d5 to83a5517CompareDecember 3, 2019 21:05
@chalasr
Copy link
Member

chalasr commentedDec 3, 2019
edited
Loading

Rebased on 3.4 since it applies there. Congratz for your first contrib!

@chalasr
Copy link
Member

Thank you@nieuwenhuisen.

nieuwenhuisen reacted with thumbs up emoji

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\"
@chalasrchalasr merged commit83a5517 intosymfony:3.4Dec 3, 2019
@mhujer
Copy link
Contributor

@nieuwenhuisen Thanks for fixing it! 👍

nieuwenhuisen reacted with thumbs up emoji

@nieuwenhuisennieuwenhuisen deleted the fix_algorithm_true_converts_to_plain_password_encoder branchDecember 3, 2019 21:11
This was referencedDec 19, 2019
This was referencedJan 21, 2020
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@derrabusderrabusderrabus approved these changes

@xabbuhxabbuhxabbuh approved these changes

@chalasrchalasrchalasr approved these changes

@nicolas-grekasnicolas-grekasAwaiting requested review from nicolas-grekas

Assignees
No one assigned
Projects
None yet
Milestone
3.4
Development

Successfully merging this pull request may close these issues.

7 participants
@nieuwenhuisen@derrabus@chalasr@mhujer@nicolas-grekas@xabbuh@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp