-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
Fix generating logout link with stateless csrf #62037
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Hey! Thanks for your PR. You are targeting branch "7.3" but it seems your PR description refers to branch "7.3 for bug fixes". Cheers! Carsonbot |
src/Symfony/Component/Security/Http/Logout/LogoutUrlGenerator.php
Outdated
Show resolved
Hide resolved
1328ea3
to
9e5e32b
Compare
Thank you @pierredup. |
|
||
if (null !== $this->csrfTokenManager) { | ||
$csrfToken = ParameterBagUtils::getRequestParameterValue($request, $this->options['csrf_parameter']); | ||
$csrfToken = ParameterBagUtils::getRequestParameterValue($request, $this->options['csrf_parameter'], $request->request->all()); |
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.
$request->request
should be used only for POST requests, not for GET requests
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.
IMO we need to revert the whole PR as is (see #62086) and work on fixing the root issue instead (a URL with an invalid CSRF token being generated).
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.
$request->request->all()
is going to be empty so that we'll properly fallback to the query-string on GET requests.
When using the
logout_path
(orlogout_url
) twig function with stateless csrf, the generator creates a link with an invalid CSRF token parameter (/logout?_csrf_token=csrf-token
), which then causes an error during logout (Invalid CSRF token
), since the LogoutListener reads the token from the query parameter first.Reproducer: