Merge branch 'add-static-object-external-storage-url-to-csp-rules' into 'master'

Inject CSP values when repository static objects external caching is enabled

Closes #198299

See merge request gitlab-org/gitlab!25711

app/controllers/concerns/static_object_external_storage_csp.rb

+# frozen_string_literal: true
+
+module StaticObjectExternalStorageCSP
+  extend ActiveSupport::Concern
+
+  included do
+    content_security_policy do |p|
+      next if p.directives.blank?
+      next unless Gitlab::CurrentSettings.static_objects_external_storage_enabled?
+
+      default_connect_src = p.directives['connect-src'] || p.directives['default-src']
+      connect_src_values = Array.wrap(default_connect_src) | [Gitlab::CurrentSettings.static_objects_external_storage_url]
+      p.connect_src(*connect_src_values)
+    end
+  end
+end

app/controllers/ide_controller.rb

@@ -3,6 +3,8 @@
 class IdeController < ApplicationController
   layout 'fullscreen'
 
+  include StaticObjectExternalStorageCSP
+
   def index
     Gitlab::UsageDataCounters::WebIdeCounter.increment_views_count
   end

changelogs/unreleased/add-static-object-external-storage-url-to-csp-rules.yml

+---
+title: Inject CSP values when repository static objects external caching is enabled
+merge_request: 25711
+author:
+type: fixed

config/initializers/0_inflections.rb

@@ -28,4 +28,5 @@ ActiveSupport::Inflector.inflections do |inflect|
     vulnerability_feedback
   )
   inflect.acronym 'EE'
+  inflect.acronym 'CSP'
 end

spec/features/ide/static_object_external_storage_csp_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'Static Object External Storage Content Security Policy' do
+  let_it_be(:user) { create(:user) }
+
+  shared_context 'disable feature' do
+    before do
+      allow_any_instance_of(ApplicationSetting).to receive(:static_objects_external_storage_url).and_return(nil)
+    end
+  end
+
+  it_behaves_like 'setting CSP connect-src' do
+    let_it_be(:whitelisted_url) { 'https://static-objects.test' }
+    let_it_be(:extended_controller_class) { IdeController }
+
+    subject do
+      visit ide_path
+
+      response_headers['Content-Security-Policy']
+    end
+
+    before do
+      allow_any_instance_of(ApplicationSetting).to receive(:static_objects_external_storage_url).and_return(whitelisted_url)
+      allow_any_instance_of(ApplicationSetting).to receive(:static_objects_external_storage_auth_token).and_return('letmein')
+
+      sign_in(user)
+    end
+  end
+end

spec/features/projects/sourcegraph_csp_spec.rb

@@ -5,94 +5,28 @@ require 'spec_helper'
 describe 'Sourcegraph Content Security Policy' do
   let_it_be(:user) { create(:user) }
   let_it_be(:project) { create(:project, :repository, namespace: user.namespace) }
-  let_it_be(:default_csp_values) { "'self' https://some-cdn.test" }
-  let_it_be(:sourcegraph_url) { 'https://sourcegraph.test' }
-  let(:sourcegraph_enabled) { true }
 
-  subject do
-    visit project_blob_path(project, File.join('master', 'README.md'))
-
-    response_headers['Content-Security-Policy']
-  end
-
-  before do
-    allow(Gitlab::CurrentSettings).to receive(:sourcegraph_url).and_return(sourcegraph_url)
-    allow(Gitlab::CurrentSettings).to receive(:sourcegraph_enabled).and_return(sourcegraph_enabled)
-
-    sign_in(user)
-  end
-
-  shared_context 'csp config' do |csp_rule|
+  shared_context 'disable feature' do
     before do
-      csp = ActionDispatch::ContentSecurityPolicy.new do |p|
-        p.send(csp_rule, default_csp_values) if csp_rule
-      end
-
-      expect_next_instance_of(Projects::BlobController) do |controller|
-        expect(controller).to receive(:current_content_security_policy).and_return(csp)
-      end
+      allow(Gitlab::CurrentSettings).to receive(:sourcegraph_enabled).and_return(false)
     end
   end
 
-  context 'when no CSP config' do
-    include_context 'csp config', nil
+  it_behaves_like 'setting CSP connect-src' do
+    let_it_be(:whitelisted_url) { 'https://sourcegraph.test' }
+    let_it_be(:extended_controller_class) { Projects::BlobController }
 
-    it 'does not add CSP directives' do
-      is_expected.to be_blank
-    end
-  end
-
-  describe 'when a CSP config exists for connect-src' do
-    include_context 'csp config', :connect_src
+    subject do
+      visit project_blob_path(project, File.join('master', 'README.md'))
 
-    context 'when sourcegraph enabled' do
-      it 'appends to connect-src' do
-        is_expected.to eql("connect-src #{default_csp_values} #{sourcegraph_url}")
-      end
+      response_headers['Content-Security-Policy']
     end
 
-    context 'when sourcegraph disabled' do
-      let(:sourcegraph_enabled) { false }
-
-      it 'keeps original connect-src' do
-        is_expected.to eql("connect-src #{default_csp_values}")
-      end
-    end
-  end
-
-  describe 'when a CSP config exists for default-src but not connect-src' do
-    include_context 'csp config', :default_src
-
-    context 'when sourcegraph enabled' do
-      it 'uses default-src values in connect-src' do
-        is_expected.to eql("default-src #{default_csp_values}; connect-src #{default_csp_values} #{sourcegraph_url}")
-      end
-    end
-
-    context 'when sourcegraph disabled' do
-      let(:sourcegraph_enabled) { false }
-
-      it 'does not add connect-src' do
-        is_expected.to eql("default-src #{default_csp_values}")
-      end
-    end
-  end
-
-  describe 'when a CSP config exists for font-src but not connect-src' do
-    include_context 'csp config', :font_src
-
-    context 'when sourcegraph enabled' do
-      it 'uses default-src values in connect-src' do
-        is_expected.to eql("font-src #{default_csp_values}; connect-src #{sourcegraph_url}")
-      end
-    end
-
-    context 'when sourcegraph disabled' do
-      let(:sourcegraph_enabled) { false }
+    before do
+      allow(Gitlab::CurrentSettings).to receive(:sourcegraph_url).and_return(whitelisted_url)
+      allow(Gitlab::CurrentSettings).to receive(:sourcegraph_enabled).and_return(true)
 
-      it 'does not add connect-src' do
-        is_expected.to eql("font-src #{default_csp_values}")
-      end
+      sign_in(user)
     end
   end
 end

spec/support/shared_examples/csp.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'setting CSP connect-src' do
+  let_it_be(:default_csp_values) { "'self' https://some-cdn.test" }
+
+  shared_context 'csp config' do |csp_rule|
+    before do
+      csp = ActionDispatch::ContentSecurityPolicy.new do |p|
+        p.send(csp_rule, default_csp_values) if csp_rule
+      end
+
+      expect_next_instance_of(extended_controller_class) do |controller|
+        expect(controller).to receive(:current_content_security_policy).and_return(csp)
+      end
+    end
+  end
+
+  context 'when no CSP config' do
+    include_context 'csp config', nil
+
+    it 'does not add CSP directives' do
+      is_expected.to be_blank
+    end
+  end
+
+  describe 'when a CSP config exists for connect-src' do
+    include_context 'csp config', :connect_src
+
+    context 'when feature is enabled' do
+      it 'appends to connect-src' do
+        is_expected.to eql("connect-src #{default_csp_values} #{whitelisted_url}")
+      end
+    end
+
+    context 'when feature is disabled' do
+      include_context 'disable feature'
+
+      it 'keeps original connect-src' do
+        is_expected.to eql("connect-src #{default_csp_values}")
+      end
+    end
+  end
+
+  describe 'when a CSP config exists for default-src but not connect-src' do
+    include_context 'csp config', :default_src
+
+    context 'when feature is enabled' do
+      it 'uses default-src values in connect-src' do
+        is_expected.to eql("default-src #{default_csp_values}; connect-src #{default_csp_values} #{whitelisted_url}")
+      end
+    end
+
+    context 'when feature is disabled' do
+      include_context 'disable feature'
+
+      it 'does not add connect-src' do
+        is_expected.to eql("default-src #{default_csp_values}")
+      end
+    end
+  end
+
+  describe 'when a CSP config exists for font-src but not connect-src' do
+    include_context 'csp config', :font_src
+
+    context 'when feature is enabled' do
+      it 'uses default-src values in connect-src' do
+        is_expected.to eql("font-src #{default_csp_values}; connect-src #{whitelisted_url}")
+      end
+    end
+
+    context 'when feature is disabled' do
+      include_context 'disable feature'
+
+      it 'does not add connect-src' do
+        is_expected.to eql("font-src #{default_csp_values}")
+      end
+    end
+  end
+end