Merge branch 'dcouture-csp-snowplow' into 'master'

Automatically adapt the CSP when snowplow is enabled

See merge request gitlab-org/gitlab!78624

app/controllers/application_controller.rb

@@ -111,6 +111,15 @@ class ApplicationController < ActionController::Base
     render plain: e.message, status: :too_many_requests
   end
 
+  content_security_policy do |p|
+    next if p.directives.blank?
+    next unless Gitlab::CurrentSettings.snowplow_enabled? && !Gitlab::CurrentSettings.snowplow_collector_hostname.blank?
+
+    default_connect_src = p.directives['connect-src'] || p.directives['default-src']
+    connect_src_values = Array.wrap(default_connect_src) | [Gitlab::CurrentSettings.snowplow_collector_hostname]
+    p.connect_src(*connect_src_values)
+  end
+
   def redirect_back_or_default(default: root_path, options: {})
     redirect_back(fallback_location: default, **options)
   end

ee/spec/features/subscriptions_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe 'Subscriptions Content Security Policy' do
+  include ContentSecurityPolicyHelpers
+
   subject { response_headers['Content-Security-Policy'] }
 
   let_it_be(:default_csp_values) { "'self' https://some-cdn.test" }
@@ -13,9 +15,7 @@ RSpec.describe 'Subscriptions Content Security Policy' do
   before do
     stub_request(:get, /.*gitlab_plans.*/).to_return(status: 200, body: "{}")
 
-    expect_next_instance_of(SubscriptionsController) do |controller|
-      expect(controller).to receive(:current_content_security_policy).and_return(csp).twice
-    end
+    setup_existing_csp_for_controller(SubscriptionsController, csp, 3)
 
     sign_in(create(:user))
 

spec/features/jira_connect/subscriptions_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe 'Subscriptions Content Security Policy' do
+  include ContentSecurityPolicyHelpers
+
   let(:installation) { create(:jira_connect_installation) }
   let(:qsh) { Atlassian::Jwt.create_query_string_hash('https://gitlab.test/subscriptions', 'GET', 'https://gitlab.test') }
   let(:jwt) { Atlassian::Jwt.encode({ iss: installation.client_key, qsh: qsh }, installation.shared_secret) }
@@ -11,10 +13,7 @@ RSpec.describe 'Subscriptions Content Security Policy' do
 
   context 'when there is no global config' do
     before do
-      expect_next_instance_of(JiraConnect::SubscriptionsController) do |controller|
-        expect(controller).to receive(:current_content_security_policy)
-          .and_return(ActionDispatch::ContentSecurityPolicy.new)
-      end
+      setup_csp_for_controller(JiraConnect::SubscriptionsController)
     end
 
     it 'does not add CSP directives' do
@@ -31,9 +30,7 @@ RSpec.describe 'Subscriptions Content Security Policy' do
         p.style_src :self, 'https://some-cdn.test'
       end
 
-      expect_next_instance_of(JiraConnect::SubscriptionsController) do |controller|
-        expect(controller).to receive(:current_content_security_policy).and_return(csp)
-      end
+      setup_existing_csp_for_controller(JiraConnect::SubscriptionsController, csp)
     end
 
     it 'appends to CSP directives' do

spec/features/projects/tracings_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe 'Tracings Content Security Policy' do
+  include ContentSecurityPolicyHelpers
+
   let_it_be(:project) { create(:project) }
   let_it_be(:user) { create(:user) }
 
@@ -18,10 +20,7 @@ RSpec.describe 'Tracings Content Security Policy' do
 
   context 'when there is no global config' do
     before do
-      expect_next_instance_of(Projects::TracingsController) do |controller|
-        expect(controller).to receive(:current_content_security_policy)
-          .and_return(ActionDispatch::ContentSecurityPolicy.new)
-      end
+      setup_csp_for_controller(Projects::TracingsController)
     end
 
     it 'does not add CSP directives' do
@@ -37,9 +36,7 @@ RSpec.describe 'Tracings Content Security Policy' do
         p.frame_src 'https://global-policy.com'
       end
 
-      expect_next_instance_of(Projects::TracingsController) do |controller|
-        expect(controller).to receive(:current_content_security_policy).and_return(csp)
-      end
+      setup_existing_csp_for_controller(Projects::TracingsController, csp)
     end
 
     context 'when external_url is set' do

spec/features/static_site_editor_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe 'Static Site Editor' do
+  include ContentSecurityPolicyHelpers
+
   let_it_be(:user) { create(:user) }
   let_it_be(:project) { create(:project, :public, :repository) }
 
@@ -79,10 +81,7 @@ RSpec.describe 'Static Site Editor' do
 
     context 'when no global CSP config exists' do
       before do
-        expect_next_instance_of(Projects::StaticSiteEditorController) do |controller|
-          expect(controller).to receive(:current_content_security_policy)
-            .and_return(ActionDispatch::ContentSecurityPolicy.new)
-        end
+        setup_csp_for_controller(Projects::StaticSiteEditorController)
       end
 
       it 'does not add CSP directives' do
@@ -101,9 +100,7 @@ RSpec.describe 'Static Site Editor' do
           p.frame_src :self, cdn_url
         end
 
-        expect_next_instance_of(Projects::StaticSiteEditorController) do |controller|
-          expect(controller).to receive(:current_content_security_policy).and_return(csp)
-        end
+        setup_existing_csp_for_controller(Projects::StaticSiteEditorController, csp)
       end
 
       it 'appends youtube to the CSP frame-src policy' do

spec/requests/content_security_policy_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+# The AnonymousController doesn't support setting the CSP
+# This is why an arbitrary test request was chosen instead
+# of testing in application_controller_spec.
+RSpec.describe 'Content Security Policy' do
+  let(:snowplow_host) { 'snowplow.example.com' }
+
+  shared_examples 'snowplow is not in the CSP' do
+    it 'does not add the snowplow collector hostname to the CSP' do
+      get explore_root_url
+
+      expect(response).to have_gitlab_http_status(:ok)
+      expect(response.headers['Content-Security-Policy']).not_to include(snowplow_host)
+    end
+  end
+
+  describe 'GET #explore' do
+    context 'snowplow is enabled' do
+      before do
+        stub_application_setting(snowplow_enabled: true, snowplow_collector_hostname: snowplow_host)
+      end
+
+      it 'adds the snowplow collector hostname to the CSP' do
+        get explore_root_url
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response.headers['Content-Security-Policy']).to include(snowplow_host)
+      end
+    end
+
+    context 'snowplow is enabled but host is not configured' do
+      before do
+        stub_application_setting(snowplow_enabled: true)
+      end
+
+      it_behaves_like 'snowplow is not in the CSP'
+    end
+
+    context 'snowplow is disabled' do
+      before do
+        stub_application_setting(snowplow_enabled: false, snowplow_collector_hostname: snowplow_host)
+      end
+
+      it_behaves_like 'snowplow is not in the CSP'
+    end
+  end
+end

spec/support/helpers/content_security_policy_helpers.rb

+# frozen_string_literal: true
+
+module ContentSecurityPolicyHelpers
+  # Expecting 2 calls to current_content_security_policy by default, once for
+  # the call that's being tested and once for the call in ApplicationController
+  def setup_csp_for_controller(controller_class, times = 2)
+    expect_next_instance_of(controller_class) do |controller|
+      expect(controller).to receive(:current_content_security_policy)
+                              .and_return(ActionDispatch::ContentSecurityPolicy.new).exactly(times).times
+    end
+  end
+
+  # Expecting 2 calls to current_content_security_policy by default, once for
+  # the call that's being tested and once for the call in ApplicationController
+  def setup_existing_csp_for_controller(controller_class, csp, times = 2)
+    expect_next_instance_of(controller_class) do |controller|
+      expect(controller).to receive(:current_content_security_policy).and_return(csp).exactly(times).times
+    end
+  end
+end