Align CSRF cookie provisioning with Laravel's precedents to eliminate ActionController::InvalidAuthenticityToken edge cases #119
Conversation
…t to protect against forgery (not just for non-Inertia requests)
…ti-request logout flows
| it 'sets the XSRF-TOKEN cookie after the session is cleared during an inertia call' do | ||
| with_forgery_protection do | ||
| get initialize_session_path | ||
| expect(response).to have_http_status(:ok) | ||
| initial_xsrf_token_cookie = response.cookies['XSRF-TOKEN'] | ||
|
|
||
| post submit_form_to_test_csrf_path, headers: { 'X-Inertia' => true, 'X-XSRF-Token' => initial_xsrf_token_cookie } | ||
| expect(response).to have_http_status(:ok) | ||
|
|
||
| delete clear_session_path, headers: { 'X-Inertia' => true, 'X-XSRF-Token' => initial_xsrf_token_cookie } | ||
| expect(response).to have_http_status(:see_other) | ||
| expect(response.headers['Location']).to eq('http://www.example.com/initialize_session') | ||
|
|
||
| post_logout_xsrf_token_cookie = response.cookies['XSRF-TOKEN'] | ||
| expect(post_logout_xsrf_token_cookie).not_to be_nil | ||
| expect(post_logout_xsrf_token_cookie).not_to eq(initial_xsrf_token_cookie) | ||
|
|
||
| post submit_form_to_test_csrf_path, headers: { 'X-Inertia' => true, 'X-XSRF-Token' => post_logout_xsrf_token_cookie } | ||
| expect(response).to have_http_status(:ok) | ||
| end | ||
| end |
There was a problem hiding this comment.
I included this spec mostly just to demonstrate the kind of stateful, multi-request scenario that can lead to avoidable ActionController::InvalidAuthenticityToken errors, but it seems out of place here / overly-specific: I'm happy to remove it (and the supporting routes / dummy controllers). The core change is captured in the revised expectation in L116.
There was a problem hiding this comment.
Yea, I definitely want to keep this! While L116 test the behavior, this spells out the "why". Something Future Us may appreciate.
| cookies['XSRF-TOKEN'] = form_authenticity_token unless request.inertia? || !protect_against_forgery? | ||
| cookies['XSRF-TOKEN'] = form_authenticity_token unless !protect_against_forgery? |
There was a problem hiding this comment.
Past discussions:
|
Hey @jordanhiltunen ! Thanks for the super clear writeup. I'm going to reword it for my own benefit: InertiaRails has zero-config CSRF handling. It leverages Axios' default behavior to copy data from an However! Sometimes we may reset the session without fulling reloading the page. The (obvious in retrospect) example is logging out via Inertia requests. In that situation, the user is forced to manually hard refresh the page in order to send any further CSRF protected requests. The fix is to simply copy set the I checked out the branch and played around and this looks good to me! I'll target a release this week, alongside #111 (after I spend some more time with it) |
| it 'sets the XSRF-TOKEN cookie after the session is cleared during an inertia call' do | ||
| with_forgery_protection do | ||
| get initialize_session_path | ||
| expect(response).to have_http_status(:ok) | ||
| initial_xsrf_token_cookie = response.cookies['XSRF-TOKEN'] | ||
|
|
||
| post submit_form_to_test_csrf_path, headers: { 'X-Inertia' => true, 'X-XSRF-Token' => initial_xsrf_token_cookie } | ||
| expect(response).to have_http_status(:ok) | ||
|
|
||
| delete clear_session_path, headers: { 'X-Inertia' => true, 'X-XSRF-Token' => initial_xsrf_token_cookie } | ||
| expect(response).to have_http_status(:see_other) | ||
| expect(response.headers['Location']).to eq('http://www.example.com/initialize_session') | ||
|
|
||
| post_logout_xsrf_token_cookie = response.cookies['XSRF-TOKEN'] | ||
| expect(post_logout_xsrf_token_cookie).not_to be_nil | ||
| expect(post_logout_xsrf_token_cookie).not_to eq(initial_xsrf_token_cookie) | ||
|
|
||
| post submit_form_to_test_csrf_path, headers: { 'X-Inertia' => true, 'X-XSRF-Token' => post_logout_xsrf_token_cookie } | ||
| expect(response).to have_http_status(:ok) | ||
| end | ||
| end |
There was a problem hiding this comment.
Yea, I definitely want to keep this! While L116 test the behavior, this spells out the "why". Something Future Us may appreciate.
|
Quick update: I'll release this once we get #111 merged in as well. |
|
Released as part of |
In June 2023, #96 implemented the following strategy for
XSRF-TOKENcookies -- issue a new cookie if:skip_forgery_protection).Issue
This strategy assumes that there is no need to issue the
XSRF-TOKENcookie in response to subsequent Inertia requests made by the client; this is not necessarily the case, and can lead to client applications observing unexpectedActionController::InvalidAuthenticityTokenerrors as a result; a simplified example:/login, a full-HTML, non-Inertia request.inertia-railsissues anXSRF-TOKENcookie.X-Inertiaheader and theX-XSRF-TOKENset by Axios, all using theXSRF-TOKENissued byinertia-railsin response to the very first request.DELETE /logout.DELETE /logout, an Inertia request like all of the others that preceded it -- it does not issue a newXSRF-TOKENcookie.X-XSRF-TOKENthat was issued during the user's very first request.ActionController::InvalidAuthenticityToken. This will happen for any request eligible for CSRF protection until the user hard-reloads the page, triggering another initial non-Inertia request that will issue them a new cookie.Relevant Laravel CSRF Precedents
As it relates to CSRF protection, the interaction between
inertia-js&inertia-railsdiffers from the precedent established by Laravel &inertia-jsin one significant way: they issue a newXSRF-TOKENfor every request, preventing this kind of edge case:This PR proposes that
inertia-railsfollow Laravel's precedent, and issue anXSRF-TOKENcookie on every request, regardless of whether or not the request is an Inertia call.Thank you so much for your work on this fantastic gem!