Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 7 additions & 19 deletions app/controllers/concerns/request_forgery_protection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,31 +2,19 @@ module RequestForgeryProtection
extend ActiveSupport::Concern

included do
after_action :append_sec_fetch_site_to_vary_header
protect_from_forgery using: :header_only, with: :exception
end

private
def append_sec_fetch_site_to_vary_header
vary_header = response.headers["Vary"].to_s.split(",").map(&:strip).reject(&:blank?)
response.headers["Vary"] = (vary_header + [ "Sec-Fetch-Site" ]).join(",")
def verified_via_header_only?
super || allowed_api_request? || allowed_insecure_context_request?
end

def verified_request?
request.get? || request.head? || !protect_against_forgery? ||
(valid_request_origin? && safe_fetch_site?)
def allowed_api_request?
sec_fetch_site_value.nil? && request.format.json?
end

SAFE_FETCH_SITES = %w[ same-origin same-site ]

def safe_fetch_site?
SAFE_FETCH_SITES.include?(sec_fetch_site_value) || (sec_fetch_site_value.nil? && api_request?)
end

def api_request?
request.format.json?
end

def sec_fetch_site_value
request.headers["Sec-Fetch-Site"].to_s.downcase.presence
def allowed_insecure_context_request?
sec_fetch_site_value.nil? && !request.ssl? && !Rails.configuration.force_ssl
end
end
86 changes: 17 additions & 69 deletions test/controllers/concerns/request_forgery_protection_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,108 +6,56 @@ class RequestForgeryProtectionTest < ActionDispatch::IntegrationTest

@original_allow_forgery_protection = ActionController::Base.allow_forgery_protection
ActionController::Base.allow_forgery_protection = true

@original_force_ssl = Rails.configuration.force_ssl
end

teardown do
ActionController::Base.allow_forgery_protection = @original_allow_forgery_protection
Rails.configuration.force_ssl = @original_force_ssl
end

test "fails if Sec-Fetch-Site is cross-site" do
assert_no_difference -> { Board.count } do
post boards_path,
params: { board: { name: "Test Board" } },
headers: { "Sec-Fetch-Site" => "cross-site" }
end

assert_response :unprocessable_entity
end

test "succeeds with same-origin Sec-Fetch-Site" do
test "JSON request succeeds with missing Sec-Fetch-Site header" do
assert_difference -> { Board.count }, +1 do
post boards_path,
params: { board: { name: "Test Board" } },
headers: { "Sec-Fetch-Site" => "same-origin" }
as: :json
end

assert_response :redirect
assert_response :created
end

test "succeeds with same-site Sec-Fetch-Site" do
test "HTTP request succeeds with missing Sec-Fetch-Site header when force_ssl is disabled" do
Rails.configuration.force_ssl = false

assert_difference -> { Board.count }, +1 do
post boards_path,
params: { board: { name: "Test Board" } },
headers: { "Sec-Fetch-Site" => "same-site" }
params: { board: { name: "Test Board" } }
end

assert_response :redirect
end

test "fails with none Sec-Fetch-Site" do
assert_no_difference -> { Board.count } do
post boards_path,
params: { board: { name: "Test Board" } },
headers: { "Sec-Fetch-Site" => "none" }
end
test "HTTP request fails with missing Sec-Fetch-Site header when force_ssl is enabled" do
Rails.configuration.force_ssl = true

assert_response :unprocessable_entity
end

test "fails when Sec-Fetch-Site header is missing" do
assert_no_difference -> { Board.count } do
post boards_path, params: { board: { name: "Test Board" } }
post boards_path,
params: { board: { name: "Test Board" } }
end

assert_response :unprocessable_entity
end

test "GET requests succeed regardless of Sec-Fetch-Site header" do
get board_path(boards(:writebook)), headers: { "Sec-Fetch-Site" => "cross-site" }
test "HTTPS request fails with missing Sec-Fetch-Site header" do
Rails.configuration.force_ssl = false

assert_response :success
end

test "appends Sec-Fetch-Site to Vary header on GET requests" do
get board_path(boards(:writebook))

assert_response :success
assert_includes response.headers["Vary"], "Sec-Fetch-Site"
end

test "appends Sec-Fetch-Site to Vary header on POST requests" do
post boards_path,
params: { board: { name: "Test Board" } },
headers: { "Sec-Fetch-Site" => "same-origin" }

assert_response :redirect
assert_includes response.headers["Vary"], "Sec-Fetch-Site"
end

test "JSON request succeeds with missing Sec-Fetch-Site" do
assert_difference -> { Board.count }, +1 do
post boards_path,
params: { board: { name: "Test Board" } },
as: :json
end

assert_response :created
end

test "JSON request fails with cross-site Sec-Fetch-Site" do
assert_no_difference -> { Board.count } do
post boards_path,
params: { board: { name: "Test Board" } },
headers: { "Sec-Fetch-Site" => "cross-site" },
as: :json
headers: { "X-Forwarded-Proto" => "https" }
end

assert_response :unprocessable_entity
end

private
def csrf_token
@csrf_token ||= begin
get new_board_path
response.body[/name="authenticity_token" value="([^"]+)"/, 1]
end
end
end