Check and report on Sec-Fetch-Site header for forgery protection

This is a great, solid alternative to CSRF tokens for CSRF protection
when we aren't worried about older browsers or other kind of actors
doing modifying requests in our app, and could be a good test for future
upstreaming to Rails (although there we'd need to continue using CSRF
tokens or at least letting people opt out manually).

Let's start checking the header and reporting on it when CSRF fails or
when it doesn't match the other checks Rails does, and then promote this
to be the only way to defend from CSRF.

app/controllers/application_controller.rb

@@ -2,6 +2,7 @@ class ApplicationController < ActionController::Base
   include Authentication
   include Authorization
   include CurrentRequest, CurrentTimezone, SetPlatform
+  include RequestForgeryProtection
   include TurboFlash, ViewTransitions
   include Saas
   include RoutingHeaders

app/controllers/concerns/request_forgery_protection.rb

@@ -0,0 +1,54 @@
+module RequestForgeryProtection
+  extend ActiveSupport::Concern
+
+  included do
+    after_action :append_set_fetch_site_to_vary_header
+  end
+
+  private
+    def append_set_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(",")
+    end
+
+    def verified_request?
+      return true if request.get? || request.head? || !protect_against_forgery?
+
+      origin = valid_request_origin?
+      token = any_authenticity_token_valid?
+      sec_fetch_site = safe_fetch_site?
+      report_on_forgery_protection_results(origin:, token:, sec_fetch_site:)
+
+      origin && token
+    end
+
+    SAFE_FETCH_SITES = %w[ same-origin same-site ]
+
+    def safe_fetch_site?
+      SAFE_FETCH_SITES.include?(safe_fetch_site_value)
+    end
+
+    def safe_fetch_site_value
+      request.headers["Sec-Fetch-Site"].to_s.downcase
+    end
+
+    def report_on_forgery_protection_results(origin:, token:, sec_fetch_site:)
+      unless [ origin, token, sec_fetch_site ].all?
+        info = {
+          origin: "#{pass_or_fail_value(origin)} (#{request.origin})",
+          token: "#{pass_or_fail_value(token)}",
+          sec_fetch_site: "#{pass_or_fail_value(sec_fetch_site)} (#{safe_fetch_site_value})"
+        }
+
+        Rails.logger.info "CSRF protection check: " + info.map { it.join(" ") }.join(", ")
+
+        if (origin && token) != sec_fetch_site
+          Sentry.capture_message "CSRF protection mismatch", level: :info, extra: { info: info }
+        end
+      end
+    end
+
+    def pass_or_fail_value(result)
+      result ? "pass" : "fail"
+    end
+end

test/controllers/concerns/request_forgery_protection_test.rb

@@ -0,0 +1,148 @@
+require "test_helper"
+
+class RequestForgeryProtectionTest < ActionDispatch::IntegrationTest
+  setup do
+    sign_in_as :kevin
+
+    # Forgery protection is disabled in test environment so we need to
+    # enable it here
+    @original_allow_forgery_protection = ActionController::Base.allow_forgery_protection
+    ActionController::Base.allow_forgery_protection = true
+  end
+
+  teardown do
+    ActionController::Base.allow_forgery_protection = @original_allow_forgery_protection
+  end
+
+  test "don't report when Sec-Fetch-Site is same-origin and CSRF token matches" do
+    assert_log(excludes: "CSRF protection check") do
+      post boards_path,
+        params: { board: { name: "Test Board" }, authenticity_token: csrf_token },
+        headers: { "Sec-Fetch-Site" => "same-origin" }
+    end
+  end
+
+  test "don't report when Sec-Fetch-Site is same-site and CSRF token matches" do
+    assert_log(excludes: "CSRF protection check") do
+      post boards_path,
+        params: { board: { name: "Test Board" }, authenticity_token: csrf_token },
+        headers: { "Sec-Fetch-Site" => "same-site" }
+    end
+  end
+
+  test "fail and report when token doesn't match, regardless of Sec-Fetch-Site" do
+    assert_report
+
+    assert_log(includes: [ "CSRF protection check", "sec_fetch_site pass (same-origin)", "token fail" ]) do
+      assert_no_difference -> { Board.count } do
+        post boards_path,
+          params: { board: { name: "Test Board" }, authenticity_token: "invalid-token" },
+          headers: { "Sec-Fetch-Site" => "same-origin" }
+      end
+    end
+  end
+
+  test "fail and report when Origin doesn't match, regardless of Sec-Fetch-Site" do
+    assert_report
+
+    assert_log(includes: [ "CSRF protection check", "sec_fetch_site pass (same-origin)",
+      "token pass", "origin fail (evil-site.com)" ]) do
+      assert_no_difference -> { Board.count } do
+        post boards_path,
+          params: { board: { name: "Test Board" }, authenticity_token: csrf_token },
+          headers: { "Sec-Fetch-Site" => "same-origin", Origin: "evil-site.com" }
+      end
+    end
+  end
+
+  test "succeed and report when Sec-Fetch-Site is cross-site and CSRF token matches" do
+    assert_report
+
+    assert_log(includes: [ "CSRF protection check", "sec_fetch_site fail (cross-site)", "token pass" ]) do
+      assert_difference -> { Board.count }, +1 do
+        post boards_path,
+          params: { board: { name: "Test Board" }, authenticity_token: csrf_token },
+          headers: { "Sec-Fetch-Site" => "cross-site" }
+      end
+    end
+  end
+
+  test "succeed and report when Sec-Fetch-Site is none" do
+    assert_report
+
+    assert_log(includes: [ "CSRF protection check", "sec_fetch_site fail (none)", "token pass" ]) do
+      assert_difference -> { Board.count }, +1 do
+        post boards_path,
+          params: { board: { name: "Test Board" }, authenticity_token: csrf_token },
+          headers: { "Sec-Fetch-Site" => "none" }
+      end
+    end
+  end
+
+  test "succeed and report when Sec-Fetch-Site is missing" do
+    assert_report
+
+    assert_log(includes: [ "CSRF protection check", "sec_fetch_site fail ()", "token pass" ]) do
+      assert_difference -> { Board.count }, +1 do
+        post boards_path, params: { board: { name: "Test Board" }, authenticity_token: csrf_token }
+      end
+    end
+  end
+
+  test "don't report and succeed for GET requests" do
+    assert_log(excludes: "CSRF protection check") do
+      get board_url(boards(:writebook)), headers: { "Sec-Fetch-Site" => "cross-site" }
+      assert_response :success
+    end
+  end
+
+  test "GET requests succeed regardless of Sec-Fetch-Site header" do
+    get board_url(boards(:writebook)), headers: { "Sec-Fetch-Site" => "cross-site" }
+
+    assert_response :success
+  end
+
+  test "appends Sec-Fetch-Site to existing Vary header" do
+    get board_url(boards(:writebook)), headers: { "Accept" => "text/html" }
+
+    assert_response :success
+    vary_values = response.headers["Vary"].split(",").map(&:strip)
+    assert_includes vary_values, "Sec-Fetch-Site"
+  end
+
+  test "appends Sec-Fetch-Site to Vary header on POST requests" do
+    post boards_path,
+      params: { board: { name: "Test Board" }, authenticity_token: csrf_token },
+      headers: { "Sec-Fetch-Site" => "same-origin" }
+
+    assert_response :redirect
+    assert_includes response.headers["Vary"], "Sec-Fetch-Site"
+  end
+
+  private
+    def assert_log(includes: [], excludes: [], &block)
+      original_logger = Rails.logger
+      log_output = StringIO.new
+      Rails.logger = ActiveSupport::TaggedLogging.new(ActiveSupport::Logger.new(log_output))
+
+      yield
+
+      Array(includes).each { assert_includes(log_output.string, it) }
+      Array(excludes).each { assert_not_includes(log_output.string, it) }
+    ensure
+      Rails.logger = original_logger
+    end
+
+    def assert_report
+      Sentry.expects(:capture_message).with do |message, **kwargs|
+        message == "CSRF protection mismatch" && kwargs[:level] == :info
+      end
+    end
+
+    def csrf_token
+      @csrf_token ||= begin
+        get new_board_url
+        response.body[/name="authenticity_token" value="([^"]+)"/, 1]
+      end
+    end
+end