Include CSRF <meta> elements in frame layout
#697
Conversation
|
This is a more conservative, less disruptive alternative to #534. While this might resolve any CSRF-related inconsistencies in the While #534 might feel too ambitious of a proposal, it circumvents this entire class of issue, and removes any burden of responsibility to decide which |
|
Regarding the failing Rails 7.1 tests, I had the same issue in another branch and submitted #696 to address them. |
Closes [hotwired#669][] If a response to a request with the `Turbo-Frame:` header does not include the `<meta>` elements in the `<html>` document, it's likely that the browser will remove any `<meta>` element present after handling navigating the `<turbo-frame>` that originated the request. In support of testing this behavior, this commit enables CSRF protection in the test suite.
seanpdoyle
force-pushed
the
closes-669
branch
from
c91825c to
7f93935
Compare
|
@jorgemanrubia @brunoprietog are either of you experiencing this issue in your applications? |
|
I don't manage to reproduce this one. In the way turbo works, for turbo-frame requests Turbo won't do any head processing. Why would it remove CSRF tokens from the header? I'll ask for a reproduction case in the original issue. |
|
@jorgemanrubia in our experience of this issue, we're including turbo-tracked elements in our frame, i.e. We're also using |
|
thanks @sfnelson, that was a bit I was missing. @seanpdoyle could you verify your patch fixes this scenario, now we should be able to reproduce? |
|
@jorgemanrubia we've been using @seanpdoyle's patch in production since November when we identified the issue. This solution isn't perfect as @seanpdoyle pointed out – it brute force solves CSRF but removes all other meta tags. We don't want to send all the turbo tracked elements in our frame heads, but if we don't then performing a frame advance visit triggers #580 which leaves turbo in an inconsistent state where navigation pop no longer works. I think there's a wider discussion to be had about how turbo frame advance visits work, as the status quo is very challenging to navigate successfully. Our ideal outcome is being able to push a navigation from the server that changes the URL and participates in browser history, but doesn't change the background DOM, similar to a hotwire native modal visit. We'd be happy to contribute time and resources to if this is something that is on the hotwire/turbo team's radar. |
|
Thanks for the clarifications @sfnelson. I'll merge this one. Maybe a more general solution is what @seanpdoyle proposed in #534 (thanks @brunoprietog for pointing that out). But, for now, let's fix this one. |
Closes #669
If a response to a request with the
Turbo-Frame:header does not include the<meta>elements in the<html>document, it's likely that the browser will remove any<meta>element present after handling navigating the<turbo-frame>that originated the request.In support of testing this behavior, this commit enables CSRF protection in the test suite.