diff --git a/app/controllers/invitations_controller.rb b/app/controllers/invitations_controller.rb index 5dbca1e43..88bd6043a 100644 --- a/app/controllers/invitations_controller.rb +++ b/app/controllers/invitations_controller.rb @@ -1,4 +1,10 @@ class InvitationsController < ApplicationController + # The invitation token in the URL is the authenticator for these actions; + # CSRF is redundant and fails when browsers withhold the session cookie + # (e.g. Safari/WebKit ITP on cross-site navigation). Same rationale as + # FeedbackController#submit (PR #2641, Rollbar #535). + skip_forgery_protection only: %i[attend reject] + before_action :require_login, only: [:index] before_action :set_invitation, only: %i[show attend reject] diff --git a/app/controllers/waiting_lists_controller.rb b/app/controllers/waiting_lists_controller.rb index b922e3bde..37df15a16 100644 --- a/app/controllers/waiting_lists_controller.rb +++ b/app/controllers/waiting_lists_controller.rb @@ -1,6 +1,12 @@ class WaitingListsController < ApplicationController include WorkshopInvitationConcerns + # The invitation token in the URL is the authenticator for these actions; + # CSRF is redundant and fails when browsers withhold the session cookie + # (e.g. Safari/WebKit ITP on cross-site navigation). Same rationale as + # FeedbackController#submit (PR #2641, Rollbar #535). + skip_forgery_protection only: %i[create destroy] + def create @invitation.assign_attributes(invitation_params) diff --git a/app/controllers/workshop_invitation_controller.rb b/app/controllers/workshop_invitation_controller.rb index 91a3fba8c..e9166437c 100644 --- a/app/controllers/workshop_invitation_controller.rb +++ b/app/controllers/workshop_invitation_controller.rb @@ -5,6 +5,12 @@ class WorkshopInvitationController < ApplicationController # It provides accept/reject RSVP actions for workshop attendees via token-based links. # Routes: /invitation/:token (legacy) and /workshop_invitation/:token + # The invitation token in the URL is the authenticator for these actions; + # CSRF is redundant and fails when browsers withhold the session cookie + # (e.g. Safari/WebKit ITP on cross-site navigation). Same rationale as + # FeedbackController#submit (PR #2641, Rollbar #535). + skip_forgery_protection only: %i[update accept] + def show @announcements = @invitation.member.announcements.active @tutorial_titles = Tutorial.all_titles diff --git a/spec/controllers/feedback_controller_spec.rb b/spec/controllers/feedback_controller_spec.rb index cdac0bfd2..046c32098 100644 --- a/spec/controllers/feedback_controller_spec.rb +++ b/spec/controllers/feedback_controller_spec.rb @@ -63,13 +63,13 @@ end context 'without a CSRF token (browser did not send session cookie)' do - it 'still accepts the feedback submission' do - # Simulate the real-world scenario where the browser withholds the - # session cookie (e.g. Safari ITP, cross-site navigation, or cookie - # blocking). With protect_from_forgery enabled, this would normally - # raise ActionController::InvalidAuthenticityToken. - ActionController::Base.allow_forgery_protection = true + # Simulate the real-world scenario where the browser withholds the + # session cookie (e.g. Safari/WebKit ITP, cross-site navigation, or cookie + # blocking). With protect_from_forgery enabled, this would normally + # raise ActionController::InvalidAuthenticityToken. + include_context 'with forgery protection enforced' + it 'still accepts the feedback submission' do patch :submit, params: { id: feedback_request.token, feedback: { @@ -84,8 +84,6 @@ expect(response).to redirect_to(root_path) expect(flash[:notice]).to eq(I18n.t('messages.feedback_saved')) expect(feedback_request.reload.submited).to be true - ensure - ActionController::Base.allow_forgery_protection = false end end end diff --git a/spec/controllers/invitations_controller_spec.rb b/spec/controllers/invitations_controller_spec.rb index 830cce365..ef90d4d1e 100644 --- a/spec/controllers/invitations_controller_spec.rb +++ b/spec/controllers/invitations_controller_spec.rb @@ -17,6 +17,20 @@ expect(response).to have_http_status(:not_found) end end + + context 'without a CSRF token (browser did not send session cookie)' do + # Simulate the real-world scenario where the browser withholds the + # session cookie (e.g. Safari/WebKit ITP on cross-site navigation). + # The invitation token in the URL is the authenticator. + include_context 'with forgery protection enforced' + let(:invitation) { Fabricate(:coach_invitation, event:) } + + it 'still accepts the RSVP' do + post :attend, params: { event_id: event.id, token: invitation.token } + + expect(invitation.reload.attending).to be true + end + end end describe 'POST #reject' do @@ -26,5 +40,16 @@ expect(response).to have_http_status(:not_found) end end + + context 'without a CSRF token (browser did not send session cookie)' do + include_context 'with forgery protection enforced' + let(:invitation) { Fabricate(:attending_event_invitation, event:) } + + it 'still cancels the RSVP' do + post :reject, params: { event_id: event.id, token: invitation.token } + + expect(invitation.reload.attending).to be false + end + end end end diff --git a/spec/controllers/waiting_lists_controller_spec.rb b/spec/controllers/waiting_lists_controller_spec.rb index 6df4c2bc5..47ca54650 100644 --- a/spec/controllers/waiting_lists_controller_spec.rb +++ b/spec/controllers/waiting_lists_controller_spec.rb @@ -30,5 +30,33 @@ expect(WaitingList.where(invitation:).count).to eq(1) end + + context 'without a CSRF token (browser did not send session cookie)' do + # Simulate the real-world scenario where the browser withholds the + # session cookie (e.g. Safari/WebKit ITP on cross-site navigation). + # The invitation token in the URL is the authenticator. + include_context 'with forgery protection enforced' + + it 'still adds the member to the waiting list' do + expect do + post :create, params: { invitation_id: invitation.token } + end.to change(WaitingList, :count).by(1) + end + end + end + + describe 'DELETE #destroy' do + context 'without a CSRF token (browser did not send session cookie)' do + include_context 'with forgery protection enforced' + + it 'still removes the member from the waiting list' do + waiting_list = Fabricate(:waiting_list) + invitation = waiting_list.invitation + + expect do + delete :destroy, params: { invitation_id: invitation.token } + end.to change(WaitingList, :count).by(-1) + end + end end end diff --git a/spec/controllers/workshop_invitation_controller_spec.rb b/spec/controllers/workshop_invitation_controller_spec.rb index a7b38f929..d8f5665ed 100644 --- a/spec/controllers/workshop_invitation_controller_spec.rb +++ b/spec/controllers/workshop_invitation_controller_spec.rb @@ -93,6 +93,19 @@ expect(flash[:notice].join).to include('Tutorial') end end + + context 'without a CSRF token (browser did not send session cookie)' do + # Simulate the real-world scenario where the browser withholds the + # session cookie (e.g. Safari/WebKit ITP on cross-site navigation). + # The invitation token in the URL is the authenticator. + include_context 'with forgery protection enforced' + + it 'still accepts the RSVP' do + post :accept, params: { id: invitation.token } + + expect(invitation.reload.attending).to be true + end + end end describe 'POST #reject' do @@ -184,5 +197,17 @@ expect(flash[:notice].join).to include('Tutorial') end end + + context 'without a CSRF token (browser did not send session cookie)' do + include_context 'with forgery protection enforced' + + it 'still updates the invitation details' do + new_tutorial = Fabricate(:tutorial) + + patch :update, params: { id: invitation.token, workshop_invitation: { tutorial: new_tutorial.title } } + + expect(invitation.reload.tutorial).to eq(new_tutorial.title) + end + end end end diff --git a/spec/support/shared_contexts/forgery_protection.rb b/spec/support/shared_contexts/forgery_protection.rb new file mode 100644 index 000000000..9e4dd6b22 --- /dev/null +++ b/spec/support/shared_contexts/forgery_protection.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +RSpec.shared_context 'with forgery protection enforced' do + before { ActionController::Base.allow_forgery_protection = true } + after { ActionController::Base.allow_forgery_protection = false } +end