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] Add methods param in IsCsrfTokenValid attribute#60007
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] Add methods param in IsCsrfTokenValid attribute#60007
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Spomky commentedMar 20, 2025
Hi@Oviglo, I believe the best approach would be to separate the logic into two distinct methods: one for |
Oviglo commentedMar 20, 2025
Yes, but it will make my controller heavier. Another solution is to create an empty form for using native csrf validation. Is it an official recommendation to use only one method per action ? |
Spomky commentedMar 20, 2025 • 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.
I’m not sure what you mean by "heavier" in this context. There is no official recommendation to use only one method per action, it is more about the single-responsibility principle. Here’s an example that should work as expected: #[Route('/delete/{id}', name:'get', methods: ['GET'], requirements: ['id' => Requirement::UUID])]publicfunctionget(User$user):Response{return$this->render('/admin/user/delete.html.twig', ['entity' =>$user]);}#[Route('/delete/{id}', name:'delete', methods: ['DELETE'], requirements: ['id' => Requirement::UUID])]#[IsCsrfTokenValid(newExpression('"delete" ~ args["user"].getId()'))]publicfunctiondelete(User$user,UserManager$userManager):Response{$userManager->remove($user);return$this->redirectToRoute('admin_user_index');} |
nicolas-grekas commentedMar 21, 2025
So, better close? |
Oviglo commentedMar 21, 2025
If you don’t find any utility, yes 😥 Merci à vous. |
nicolas-grekas left a comment
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.
Work for me despite the proposed alternative.
src/Symfony/Component/Security/Http/Attribute/IsCsrfTokenValid.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Security/Http/Attribute/IsCsrfTokenValid.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Security/Http/EventListener/IsCsrfTokenValidAttributeListener.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Spomky commentedMar 22, 2025
Instead of a list of methods, what about checking if it is a non-safe method? |
Oviglo commentedMar 22, 2025
Yes you are right, a csrf token is only used for POST PUT DELETE and PATCH is’nt ? |
nicolas-grekas commentedMar 22, 2025
CSRF tokens can also be used for GET requests, eg for logout links (that's a common practice, even if not recommended). |
adea370 to28a9102Compare
Spomky left a comment
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.
Looks good. Thank you.
0b4edd1 to640e7a4Comparenicolas-grekas commentedMar 24, 2025
Thank you@Oviglo. |
1492e46 intosymfony:7.3Uh oh!
There was an error while loading.Please reload this page.
…tribute (Oviglo)This PR was squashed before being merged into the 7.3 branch.Discussion----------[Security] Add methods param doc for isCsrfTokenValid attributeAdd new params for isCsrfTokenValid attributePR:symfony/symfony#60007Issue:#20810Commits-------6d7c87f [Security] Add methods param doc for isCsrfTokenValid attribute
Uh oh!
There was an error while loading.Please reload this page.
I use a controller action to show a confirmation message for delete entity, i think it could be usefull to add 'methods' param in
#[IsCsrfTokenValid]attribute.The
isCsrfTokenValidfunction is ignored if request method is not settings in param.What do you think about this ?