diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 372b8923c7d..0b2813f461c 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1023,16 +1023,11 @@ class ApplicationController < ActionController::Base end def set_cross_origin_opener_policy_header - response.headers[ - "Cross-Origin-Opener-Policy" - ] = if SiteSetting.cross_origin_opener_unsafe_none_referrers.present? && - SiteSetting - .cross_origin_opener_unsafe_none_referrers - .split("|") - .include?(UrlHelper.relaxed_parse(request.referrer.to_s)&.host) - "unsafe-none" + if current_user.present? && SiteSetting.cross_origin_opener_unsafe_none_groups_map.any? && + current_user.in_any_groups?(SiteSetting.cross_origin_opener_unsafe_none_groups_map) + response.headers["Cross-Origin-Opener-Policy"] = "unsafe-none" else - SiteSetting.cross_origin_opener_policy_header + response.headers["Cross-Origin-Opener-Policy"] = SiteSetting.cross_origin_opener_policy_header end end diff --git a/config/site_settings.yml b/config/site_settings.yml index e77bae476f4..9285629f24c 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -2075,9 +2075,12 @@ security: - "same-origin" - "same-origin-allow-popups" hidden: true - cross_origin_opener_unsafe_none_referrers: + cross_origin_opener_unsafe_none_groups: + type: group_list + list_type: compact default: "" - type: host_list + allow_any: false + refresh: true hidden: true onebox: diff --git a/db/migrate/20240709010639_remove_cross_origin_unsafe_none_referrers_setting.rb b/db/migrate/20240709010639_remove_cross_origin_unsafe_none_referrers_setting.rb new file mode 100644 index 00000000000..e4f0df137dc --- /dev/null +++ b/db/migrate/20240709010639_remove_cross_origin_unsafe_none_referrers_setting.rb @@ -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 diff --git a/spec/requests/application_controller_spec.rb b/spec/requests/application_controller_spec.rb index ef52c015f3e..65e86683057 100644 --- a/spec/requests/application_controller_spec.rb +++ b/spec/requests/application_controller_spec.rb @@ -606,37 +606,42 @@ RSpec.describe ApplicationController do 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 SiteSetting.cross_origin_opener_policy_header = "same-origin" - SiteSetting.cross_origin_opener_unsafe_none_referrers = - "meta.discourse.org|try.discourse.org" + SiteSetting.cross_origin_opener_unsafe_none_groups = group.id end - it "sets `Cross-Origin-Opener-Policy` to `unsafe-none` for a listed referrer" do - get "/latest", headers: { "HTTP_REFERER" => "https://meta.discourse.org/" } + context "for logged in user" do + before { sign_in(current_user) } - expect(response.status).to eq(200) - expect(response.headers["Cross-Origin-Opener-Policy"]).to eq("unsafe-none") + it "sets `Cross-Origin-Opener-Policy` to `unsafe-none` for a listed group" do + group.add(current_user) - get "/latest", headers: { "HTTP_REFERER" => "https://meta.discourse.org/hot" } + get "/latest" - expect(response.status).to eq(200) - expect(response.headers["Cross-Origin-Opener-Policy"]).to eq("unsafe-none") + expect(response.status).to eq(200) + 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 - it "sets `Cross-Origin-Opener-Policy` to configured value for a non-listed referrer" do - get "/latest", headers: { "HTTP_REFERER" => "https://www.discourse.org/" } + context "for anon" do + it "sets `Cross-Origin-Opener-Policy` to configured value" do + get "/latest" - expect(response.status).to eq(200) - expect(response.headers["Cross-Origin-Opener-Policy"]).to eq("same-origin") - 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") + expect(response.status).to eq(200) + expect(response.headers["Cross-Origin-Opener-Policy"]).to eq("same-origin") + end end end end