[Security][SecurityBundle] Change Exception thrown by IsCsrfTokenValid Attribute #57622
Conversation
InvalidCsrfTokenException
390fe9d to
5224b66
Compare
df7f6f2 to
58f68f4
Compare
|
seems test fail because it take old test with depedencies ?? |
|
The To me, the issue is that the |
|
Side note, the |
|
So I can keep in place InvalidCsrfTokenException and create another on in Security/Csrf extending BadRequestException and link it to the Exception throw in IsCsrfTokenValidAttributeListener ? |
|
change is OK, do I move IsCsrfTokenValidAttributeListener in Security/Csrf ? |
InvalidCsrfTokenException22c900f to
9f653c4
Compare
clear old getMessage function
bc01d90 to
9717ef8
Compare
There was a problem hiding this comment.
Thanks for working on this @eltharin!
Before going further with moving the listener/attribute to security-csrf, let's see what other @symfony/mergers think. Maybe we should just focus this PR on fixing the bug and the move in a follow-up PR (or not at all).
| @@ -11,6 +11,7 @@ CHANGELOG | |||
| * Add ability to fetch LDAP roles | |||
| * Add `OAuth2TokenHandlerFactory` for `AccessTokenFactory` | |||
| * Add discovery support to `OidcTokenHandler` and `OidcUserInfoTokenHandler` | |||
| * Change Exception thrown by IsCsrfTokenValid Attribute | |||
There was a problem hiding this comment.
This should be in security-csrf & security-http
| @@ -9,17 +9,17 @@ | |||
| * file that was distributed with this source code. | |||
| */ | |||
|
|
|||
| namespace Symfony\Component\Security\Http\EventListener; | |||
| namespace Symfony\Component\Security\Csrf\EventListener; | |||
There was a problem hiding this comment.
Unfortunately, it isn't this simple, we need to keep it in both places, deprecating the one in http. See #46094 for how we did this with the Security helper.
9717ef8 to
bfe93b9
Compare
|
Ok I revert the move and wait and see |
Change InvalidCsrfTokenException behavior :
Actually, when InvalidCsrfTokenException throw, a 403 Response is return or if there is a userAuthenticator, response is converted to 301-Redirect To Login,
For example, For a Basic Contact Form, if the CSRF is bad, I'm redirected to Login Page => Why ?
InvalidCsrfTokenException extends from AuthenticationException but a Csrf Error is not necessarily an authentication error.
And is 403 error code really associated to a CSRF error ?
repo to reproduce : https://github.com/eltharin/reproducer_symfony_57343/tree/main
So I propose to change parent from AuthenticationException to BadRequestHttpException for have a 400 Bad Request Error whitch is most adapted to CSRF error and to not have a redirection to login page;