Support local installations where the app is loaded over HTTP
In this case requests won't be performed from a secure context [1] and the browser won't send the Sec-Fetch-Site header. This means non-GET requests will be rejected because CSRF protection will fail. With this change, we allow these requests with missing Sec-Fetch-Site headers if: - They happen over HTTP - The app is not configured to force SSL The Origin check happens in any case. [1] https://developer.mozilla.org/en-US/docs/Web/Security/Defenses/Secure_Contexts#potentially_trustworthy_origins
This commit is contained in:
committed by
Rosa Gutierrez
parent
ffb9847a4e
commit
5f390f72df
@@ -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
|
||||
|
||||
@@ -6,83 +6,16 @@ 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
|
||||
assert_difference -> { Board.count }, +1 do
|
||||
post boards_path,
|
||||
params: { board: { name: "Test Board" } },
|
||||
headers: { "Sec-Fetch-Site" => "same-origin" }
|
||||
end
|
||||
|
||||
assert_response :redirect
|
||||
end
|
||||
|
||||
test "succeeds with same-site Sec-Fetch-Site" do
|
||||
assert_difference -> { Board.count }, +1 do
|
||||
post boards_path,
|
||||
params: { board: { name: "Test Board" } },
|
||||
headers: { "Sec-Fetch-Site" => "same-site" }
|
||||
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
|
||||
|
||||
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" } }
|
||||
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" }
|
||||
|
||||
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
|
||||
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" } },
|
||||
@@ -92,22 +25,37 @@ class RequestForgeryProtectionTest < ActionDispatch::IntegrationTest
|
||||
assert_response :created
|
||||
end
|
||||
|
||||
test "JSON request fails with cross-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" } }
|
||||
end
|
||||
|
||||
assert_response :redirect
|
||||
end
|
||||
|
||||
test "HTTP request fails with missing Sec-Fetch-Site header when force_ssl is enabled" do
|
||||
Rails.configuration.force_ssl = true
|
||||
|
||||
assert_no_difference -> { Board.count } do
|
||||
post boards_path,
|
||||
params: { board: { name: "Test Board" } },
|
||||
headers: { "Sec-Fetch-Site" => "cross-site" },
|
||||
as: :json
|
||||
params: { board: { name: "Test Board" } }
|
||||
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
|
||||
test "HTTPS request fails with missing Sec-Fetch-Site header" do
|
||||
Rails.configuration.force_ssl = false
|
||||
|
||||
assert_no_difference -> { Board.count } do
|
||||
post boards_path,
|
||||
params: { board: { name: "Test Board" } },
|
||||
headers: { "X-Forwarded-Proto" => "https" }
|
||||
end
|
||||
|
||||
assert_response :unprocessable_entity
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user