FEATURE: Use group based setting for unsafe-none COOP (#27783)

Followup 3ff7ce78e782c7d28c8b5a1a3f40a1de897d89a1

Basing this setting on referrer was too brittle --
the referrer header can easily be ommitted or changed.
Instead, for the small amount of use cases that this
site setting serves, we can use a group-based setting
instead, changing it to `cross_origin_opener_unsafe_none_groups`
instead.
This commit is contained in:
Martin Brennan
2024-07-10 02:25:49 +10:00
committed by GitHub
parent a01be4150a
commit 7a7bdc9be5
4 changed files with 49 additions and 32 deletions

View File

@ -1023,16 +1023,11 @@ class ApplicationController < ActionController::Base
end end
def set_cross_origin_opener_policy_header def set_cross_origin_opener_policy_header
response.headers[ if current_user.present? && SiteSetting.cross_origin_opener_unsafe_none_groups_map.any? &&
"Cross-Origin-Opener-Policy" current_user.in_any_groups?(SiteSetting.cross_origin_opener_unsafe_none_groups_map)
] = if SiteSetting.cross_origin_opener_unsafe_none_referrers.present? && response.headers["Cross-Origin-Opener-Policy"] = "unsafe-none"
SiteSetting
.cross_origin_opener_unsafe_none_referrers
.split("|")
.include?(UrlHelper.relaxed_parse(request.referrer.to_s)&.host)
"unsafe-none"
else else
SiteSetting.cross_origin_opener_policy_header response.headers["Cross-Origin-Opener-Policy"] = SiteSetting.cross_origin_opener_policy_header
end end
end end

View File

@ -2075,9 +2075,12 @@ security:
- "same-origin" - "same-origin"
- "same-origin-allow-popups" - "same-origin-allow-popups"
hidden: true hidden: true
cross_origin_opener_unsafe_none_referrers: cross_origin_opener_unsafe_none_groups:
type: group_list
list_type: compact
default: "" default: ""
type: host_list allow_any: false
refresh: true
hidden: true hidden: true
onebox: onebox:

View File

@ -0,0 +1,14 @@
# frozen_string_literal: true
class RemoveCrossOriginUnsafeNoneReferrersSetting < ActiveRecord::Migration[7.1]
def up
execute <<~SQL
DELETE FROM site_settings
WHERE name = 'cross_origin_opener_unsafe_none_referrers'
SQL
end
def down
raise ActiveRecord::IrreversibleMigration
end
end

View File

@ -606,37 +606,42 @@ RSpec.describe ApplicationController do
end end
end end
describe "when `cross_origin_unsafe_none_referrers` site setting has been set" do describe "when `cross_origin_opener_unsafe_none_groups` site setting has been set" do
fab!(:group)
fab!(:current_user) { Fabricate(:user) }
before do before do
SiteSetting.cross_origin_opener_policy_header = "same-origin" SiteSetting.cross_origin_opener_policy_header = "same-origin"
SiteSetting.cross_origin_opener_unsafe_none_referrers = SiteSetting.cross_origin_opener_unsafe_none_groups = group.id
"meta.discourse.org|try.discourse.org"
end end
it "sets `Cross-Origin-Opener-Policy` to `unsafe-none` for a listed referrer" do context "for logged in user" do
get "/latest", headers: { "HTTP_REFERER" => "https://meta.discourse.org/" } before { sign_in(current_user) }
expect(response.status).to eq(200) it "sets `Cross-Origin-Opener-Policy` to `unsafe-none` for a listed group" do
expect(response.headers["Cross-Origin-Opener-Policy"]).to eq("unsafe-none") group.add(current_user)
get "/latest", headers: { "HTTP_REFERER" => "https://meta.discourse.org/hot" } get "/latest"
expect(response.status).to eq(200) expect(response.status).to eq(200)
expect(response.headers["Cross-Origin-Opener-Policy"]).to eq("unsafe-none") expect(response.headers["Cross-Origin-Opener-Policy"]).to eq("unsafe-none")
end
it "sets `Cross-Origin-Opener-Policy` to configured value when group is missing" do
get "/latest"
expect(response.status).to eq(200)
expect(response.headers["Cross-Origin-Opener-Policy"]).to eq("same-origin")
end
end end
it "sets `Cross-Origin-Opener-Policy` to configured value for a non-listed referrer" do context "for anon" do
get "/latest", headers: { "HTTP_REFERER" => "https://www.discourse.org/" } it "sets `Cross-Origin-Opener-Policy` to configured value" do
get "/latest"
expect(response.status).to eq(200) expect(response.status).to eq(200)
expect(response.headers["Cross-Origin-Opener-Policy"]).to eq("same-origin") expect(response.headers["Cross-Origin-Opener-Policy"]).to eq("same-origin")
end end
it "sets `Cross-Origin-Opener-Policy` to configured value when referrer is missing" do
get "/latest"
expect(response.status).to eq(200)
expect(response.headers["Cross-Origin-Opener-Policy"]).to eq("same-origin")
end end
end end
end end