Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.2k
[Cookbook][Security] don't output message from AuthenticationException#4761
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
ping@weaverryan@wouterj I think this is something we should merge as soon as possible to avoid giving insecure advices. |
Thanks@xabbuh. I'm actually not familiar with this - I knew this approach could leak info, but had the approach you suggested always worked, or is it new? Where did the idea come from? Forgive my lack of knowledge here :) |
@stof made the suggestion in#4723 (comment). These methods are present in Symfony 2.3 too:http://api.symfony.com/2.3/Symfony/Component/Security/Core/Exception/AuthenticationException.html |
@weaverryan this is a feature added in Symfony 2.2 |
@xabbuh indeed, I made a mistake when giving the method name (I haven't checked the code or the api doc when commenting) |
@stof No problem, was not hard to guess the right method name. ;) |
| // ADD THIS use STATEMENT above your class | ||
| use Symfony\Component\Security\Core\SecurityContextInterface; | ||
| public function loginAction(Request $request) | ||
| { | ||
| $session = $request->getSession(); | ||
| $error = null; |
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.
I think having this in the else is more clear
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.
Good suggestion, this might not be the way it would usually be done in the Symfony core, but it's actually more readable. Changed it.
Displaying the message of an `AuthenticationException` might exposesensitive data to the user.
Thanks guys! I can't believe I had missed this. I used it recently in Silex in a KnpU tutorial, without even realizing how it fits into Symfony. I'm very happy to have this improved! |
…cationException (xabbuh)This PR was merged into the 2.3 branch.Discussion----------[Cookbook][Security] don't output message from AuthenticationException| Q | A| ------------- | ---| Doc fix? | yes| New docs? | no| Applies to | all| Fixed tickets |Displaying the message of an `AuthenticationException` might exposesensitive data to the user.Commits-------44277c7 don't output message from AuthenticationException
Displaying the message of an
AuthenticationExceptionmight exposesensitive data to the user.