Skip to content
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

Allow CSRF token to be retrieved via the X-XSRF-TOKEN header #242

Open
SherinBloemendaal opened this issue Aug 11, 2024 · 5 comments
Open

Comments

@SherinBloemendaal
Copy link

Description

I am integrating a 2FA flow within a Single Page Application (SPA) and have implemented an endpoint (GET /set_csrf_cookie) similar to Laravel Sanctum. This endpoint sets a CSRF-TOKEN cookie (httpOnly: false + SameSite: LAX) for the current session. Consequently, libraries like Axios and other JS XHR tools automatically include the CSRF token in the X-XSRF-TOKEN header of every request.

While it might initially seem unsafe to store a CSRF token in a cookie, this approach is secure due to the SameSite policy combined with CORS. Since a custom header is used and the content type is application/json, the request is not classified as "simple," mitigating potential risks. Although it is also possible to send the CSRF token within the JSON payload, I think we all prefer a single source of truth, rather than varying the method based on endpoints.

It would be very nice if some extension point or configuration option was being added to specify that the CSRF token should be extracted from the request header. Adding such an extension point or configuration option would significantly simplify this process. If this feature request is approved I can create a PR :)

@scheb
Copy link
Owner

scheb commented Aug 16, 2024

That's outside of the bundle's influence. It is using Symfony's CSRF token manager, specifically the security.csrf.token_manager service. If you want to change CSRF token behaviour in your application, you have to configure it on the framework-level. Please have a look at the official Symfony documentation on CSRF token.

Alternatively, you could inject your own implementation of Symfony\Component\Security\Csrf\CsrfTokenManagerInterface as scheb_two_factor.csrf_token_manager.

@SherinBloemendaal
Copy link
Author

Thanks for the reply :) I've realised my initial explanation wasn't clear enough. Let me clarify:
In TwoFactorFirewallConfig.php::114, the CSRF token is extracted from the request payload, either from JSON or FormData.
However, I'm unable to find a way to decorate or modify this behavior without fully cloning TwoFactorAuthenticator.php. Is there an alternative approach to achieve this without duplicating the entire authenticator class?

@scheb
Copy link
Owner

scheb commented Aug 27, 2024

Okay, I see what the problem is. To be honest, I totally forgot this was part of the implementation.

Stupid question, how did you solve this for the initial login? Symfony's login mechanism should have the exact same challenge and I couldn't find anything how to make it read the csrf token from a header.

@SherinBloemendaal
Copy link
Author

SherinBloemendaal commented Aug 28, 2024

For the initial login, I resolved the issue by creating a custom authenticator that extracts the token from the header and passes it to a CsrfTokenBadge.

I wanted to implement the same approach for the TwoFactorAuthenticator. However, doing so requires removing the entire two_factor configuration from the firewall, which in turn disables prepare_on_login, prepare_on_access_denied and other features.

An alternative solution could be to force the JSON content-type instead, which would eliminate the need for CSRF protection if I am not mistaken, similar to how Symfony's JsonLoginAuthenticator operates.

@scheb
Copy link
Owner

scheb commented Oct 5, 2024

Did you find a way to solve this? Given the current implementation, the only way I can think of would be to replace either TwoFactorFirewallConfig or RequestDataReader with a custom implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants