Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Security][Http][SwitchUserListener] Ignore all non existent username protection errors#36223
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
[Security][Http][SwitchUserListener] Ignore all non existent username protection errors#36223
Uh oh!
There was an error while loading.Please reload this page.
Conversation
nicolas-grekas commentedMar 26, 2020 • 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.
The driver/authenticator should be updated instead to me - it should throw an AuthenticationException |
ca680e1 to3843cdeCompareUh oh!
There was an error while loading.Please reload this page.
3843cde toa874f45Compare
nicolas-grekas left a comment• 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
(once fabbot's report is fixed) <= false positive
derrabus commentedMar 27, 2020
This does not feel right. |
fancyweb commentedMar 27, 2020 • 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.
Should we narrow to some specific error codes then? |
xabbuh commentedMar 27, 2020
That's indeed a good point. What about moving the |
fancyweb commentedMar 27, 2020
That's what I did in the first place, until Nicolas' comment. |
nicolas-grekas commentedMar 27, 2020
I confirm: the issue is in the Doctrine bridge, not in the |
wouterj commentedMar 27, 2020
Hmm, I would prefer to change this inner symfony/src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php Lines 156 to 167 ine0de6cc
Putting this in the The first |
nicolas-grekas commentedMar 31, 2020
OK, good argument! Let's revert to your initial proposal then@fancyweb, and sorry for the confusion :) |
a874f45 to42311d5Comparefancyweb commentedApr 1, 2020
I reverted to the initial change (on 4.4). Sorry for the massive ping 😅 |
nicolas-grekas commentedApr 1, 2020
Thank you@fancyweb. |
Uh oh!
There was an error while loading.Please reload this page.
Since we generate the non existent username blindly, it can lead to Doctrine exceptions or any other exception.
We can catch all exceptions here but I guess it reduces the protection since the SQL query was not executed?
Alternative: we can only catch Doctrine DriverException (in addition to the existing AuthenticationException) and only silent the reported error codes?