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

Prevent CSRF token from being leaked to cross-origin requests #2889

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Lucisu
Copy link
Contributor

@Lucisu Lucisu commented Aug 31, 2024

Currently, the code inside the html_load_controller.js makes the CSRF token being sent to every Axios and Turbo request:

window.axios.defaults.headers.common['X-CSRF-TOKEN'] = token.content;
window.axios.defaults.headers.common['X-Requested-With'] = 'XMLHttpRequest';
document.addEventListener("turbo:before-fetch-request", (event) => {
event.detail.fetchOptions.headers["X-CSRF-TOKEN"] = token.content;
});

This, however, leaks the token to cross-origin requests as well, defeating the whole purpose of CSRF protection.

This fix prevents the token from being leaked to cross-origin requests.

For Axios, it is certain that it needs to be implemented. For Turbo, I'm not sure the "turbo:before-fetch-request" is triggered during cross-origin requests, but I added the fix there as well, just in case.

@tabuna
Copy link
Member

tabuna commented Sep 1, 2024

Hi! Thanks for pointing that out.

To be honest, I'm not sure it's still necessary to manually set the CSRF token this way. Back when Hotwire was still Turbolinks, it was required, but now the framework should automatically fetch the value from:

<meta name="csrf-token" content="[your-value]">

Maybe we should try removing this and rely entirely on the framework?

tabuna added a commit that referenced this pull request Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants