From 5d7d607b5f240afdb13c26c384dcbf20ef5d5393 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Thu, 31 Aug 2023 20:50:06 +0800 Subject: [PATCH] DEV: Add hidden `cross_origin_opener_policy_header` site setting (#23346) Why this change? As part of our ongoing efforts to security harden the Discourse application, we are adding the `cross_origin_opener_policy_header` site setting which allows the `Cross-Origin-Opener-Policy` response header to be set on requests that preloads the Discourse application. In more technical terms, only GET requests that are not json or xhr will have the response header set. The `cross_origin_opener_policy_header` site setting is hidden for now for testing purposes and will either be released as a public site setting or be remove if we decide to be opinionated and ship a default for the `Cross-Origin-Opener-Policy` response header. --- app/controllers/application_controller.rb | 7 +++++ config/site_settings.yml | 9 +++++- spec/requests/application_controller_spec.rb | 31 ++++++++++++++++++++ 3 files changed, 46 insertions(+), 1 deletion(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 6aeed83ee5d..4b8b060dc72 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -51,6 +51,7 @@ class ApplicationController < ActionController::Base after_action :add_noindex_header, if: -> { is_feed_request? || !SiteSetting.allow_index_in_robots_txt } after_action :add_noindex_header_to_non_canonical, if: :spa_boot_request? + after_action :set_cross_origin_opener_policy_header, if: :spa_boot_request? around_action :link_preload, if: -> { spa_boot_request? && GlobalSetting.preload_link_header } HONEYPOT_KEY ||= "HONEYPOT_KEY" @@ -985,6 +986,12 @@ class ApplicationController < ActionController::Base end end + def set_cross_origin_opener_policy_header + if SiteSetting.cross_origin_opener_policy_header != "unsafe-none" + response.headers["Cross-Origin-Opener-Policy"] = SiteSetting.cross_origin_opener_policy_header + end + end + protected def honeypot_value diff --git a/config/site_settings.yml b/config/site_settings.yml index 034901389b5..b5a100f04c3 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1842,6 +1842,14 @@ security: default: false display_personal_messages_tag_counts: default: false + cross_origin_opener_policy_header: + default: "unsafe-none" + type: enum + choices: + - "unsafe-none" + - "same-origin" + - "same-origin-allow-popups" + hidden: true onebox: post_onebox_maxlength: @@ -2680,7 +2688,6 @@ user_preferences: default: false default_hide_profile_and_presence: default: false - default_other_new_topic_duration_minutes: enum: "NewTopicDurationSiteSetting" default: 2880 diff --git a/spec/requests/application_controller_spec.rb b/spec/requests/application_controller_spec.rb index a2d86582990..976e267277c 100644 --- a/spec/requests/application_controller_spec.rb +++ b/spec/requests/application_controller_spec.rb @@ -517,6 +517,37 @@ RSpec.describe ApplicationController do end end + describe "setting `Cross-Origin-Opener-Policy` header" do + describe "when `cross_origin_opener_policy_header` site setting is set to `same-origin`" do + before { SiteSetting.cross_origin_opener_policy_header = "same-origin" } + + it "sets `Cross-Origin-Opener-Policy` header to `same-origin`" do + get "/latest" + + expect(response.status).to eq(200) + expect(response.headers["Cross-Origin-Opener-Policy"]).to eq("same-origin") + end + + it "does not set the `Cross-Origin-Opener-Policy` header for a JSON request" do + get "/latest.json" + + expect(response.status).to eq(200) + expect(response.headers["Cross-Origin-Opener-Policy"]).to eq(nil) + end + end + + describe "when `cross_origin_opener_policy_header` site setting is set to `unsafe-none`" do + it "does not set the `Cross-Origin-Opener-Policy` header" do + SiteSetting.cross_origin_opener_policy_header = "unsafe-none" + + get "/latest" + + expect(response.status).to eq(200) + expect(response.headers["Cross-Origin-Opener-Policy"]).to eq(nil) + end + end + end + describe "splash_screen" do let(:admin) { Fabricate(:admin) }