Skip to content

Commit 7fcee22

Browse files
committed
Don't set Vary when forgery protection is skipped
When request forgery protection is skipped, we should not add "Sec-Fetch-Site" to the Vary header. See: f5a1915
1 parent 4e0337c commit 7fcee22

2 files changed

Lines changed: 47 additions & 1 deletion

File tree

actionpack/lib/action_controller/metal/request_forgery_protection.rb

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,10 +285,13 @@ def protect_from_forgery(options = {})
285285
# Turn off request forgery protection. This is a wrapper for:
286286
#
287287
# skip_before_action :verify_request_for_forgery_protection
288+
# skip_after_action :append_sec_fetch_site_to_vary_header
288289
#
289290
# See `skip_before_action` for allowed options.
290291
def skip_forgery_protection(options = {})
291-
skip_before_action :verify_request_for_forgery_protection, options.reverse_merge(raise: false)
292+
options = options.reverse_merge(raise: false)
293+
skip_before_action :verify_request_for_forgery_protection, options
294+
skip_after_action :append_sec_fetch_site_to_vary_header, options
292295
end
293296

294297
private

actionpack/test/controller/request_forgery_protection_test.rb

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,15 @@ class SkipProtectionWhenUnprotectedController < ActionController::Base
199199
skip_forgery_protection
200200
end
201201

202+
class ProtectedParentController < ActionController::Base
203+
protect_from_forgery with: :exception
204+
end
205+
206+
class SkipsInheritedProtectionController < ProtectedParentController
207+
include RequestForgeryProtectionActions
208+
skip_forgery_protection
209+
end
210+
202211
# Controller using the deprecated skip_before_action :verify_authenticity_token
203212
class DeprecatedSkipVerifyAuthenticityTokenController < ActionController::Base
204213
include RequestForgeryProtectionActions
@@ -1289,6 +1298,40 @@ def assert_not_blocked(&block)
12891298
assert_nothing_raised(&block)
12901299
assert_response :success
12911300
end
1301+
1302+
test "does not add Sec-Fetch-Site to Vary header when forgery protection is skipped" do
1303+
get :index
1304+
assert_response :success
1305+
assert_nil response.headers["Vary"]
1306+
end
1307+
1308+
test "response does not vary by Sec-Fetch-Site when forgery protection is skipped" do
1309+
@request.set_header "HTTP_SEC_FETCH_SITE", "same-origin"
1310+
post :index
1311+
assert_response :success
1312+
1313+
@request.set_header "HTTP_SEC_FETCH_SITE", "cross-site"
1314+
post :index
1315+
assert_response :success
1316+
end
1317+
end
1318+
1319+
class SkipsInheritedProtectionControllerTest < ActionController::TestCase
1320+
test "does not add Sec-Fetch-Site to Vary header when inherited forgery protection is skipped" do
1321+
get :index
1322+
assert_response :success
1323+
assert_nil response.headers["Vary"]
1324+
end
1325+
1326+
test "response does not vary by Sec-Fetch-Site when inherited forgery protection is skipped" do
1327+
@request.set_header "HTTP_SEC_FETCH_SITE", "same-origin"
1328+
post :index
1329+
assert_response :success
1330+
1331+
@request.set_header "HTTP_SEC_FETCH_SITE", "cross-site"
1332+
post :index
1333+
assert_response :success
1334+
end
12921335
end
12931336

12941337
class DeprecatedSkipVerifyAuthenticityTokenControllerTest < ActionController::TestCase

0 commit comments

Comments
 (0)