From 9c71f76e2b49c070c35cb209fe3729e01a7ce92c Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Sun, 23 Feb 2020 00:09:14 +0000 Subject: Add latest changes from gitlab-org/gitlab@master --- .../concerns/static_object_external_storage_csp.rb | 16 ++++ app/controllers/ide_controller.rb | 2 + ...ic-object-external-storage-url-to-csp-rules.yml | 5 ++ config/initializers/0_inflections.rb | 1 + .../ide/static_object_external_storage_csp_spec.rb | 31 ++++++++ spec/features/projects/sourcegraph_csp_spec.rb | 90 +++------------------- spec/support/shared_examples/csp.rb | 79 +++++++++++++++++++ 7 files changed, 146 insertions(+), 78 deletions(-) create mode 100644 app/controllers/concerns/static_object_external_storage_csp.rb create mode 100644 changelogs/unreleased/add-static-object-external-storage-url-to-csp-rules.yml create mode 100644 spec/features/ide/static_object_external_storage_csp_spec.rb create mode 100644 spec/support/shared_examples/csp.rb diff --git a/app/controllers/concerns/static_object_external_storage_csp.rb b/app/controllers/concerns/static_object_external_storage_csp.rb new file mode 100644 index 00000000000..0be83e31d8b --- /dev/null +++ b/app/controllers/concerns/static_object_external_storage_csp.rb @@ -0,0 +1,16 @@ +# 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 diff --git a/app/controllers/ide_controller.rb b/app/controllers/ide_controller.rb index 4c9aac9a327..d94f18beb5d 100644 --- a/app/controllers/ide_controller.rb +++ b/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 diff --git a/changelogs/unreleased/add-static-object-external-storage-url-to-csp-rules.yml b/changelogs/unreleased/add-static-object-external-storage-url-to-csp-rules.yml new file mode 100644 index 00000000000..a5b60d127ba --- /dev/null +++ b/changelogs/unreleased/add-static-object-external-storage-url-to-csp-rules.yml @@ -0,0 +1,5 @@ +--- +title: Inject CSP values when repository static objects external caching is enabled +merge_request: 25711 +author: +type: fixed diff --git a/config/initializers/0_inflections.rb b/config/initializers/0_inflections.rb index 5c38859a667..1fabce9a57e 100644 --- a/config/initializers/0_inflections.rb +++ b/config/initializers/0_inflections.rb @@ -28,4 +28,5 @@ ActiveSupport::Inflector.inflections do |inflect| vulnerability_feedback ) inflect.acronym 'EE' + inflect.acronym 'CSP' end diff --git a/spec/features/ide/static_object_external_storage_csp_spec.rb b/spec/features/ide/static_object_external_storage_csp_spec.rb new file mode 100644 index 00000000000..93c22b35786 --- /dev/null +++ b/spec/features/ide/static_object_external_storage_csp_spec.rb @@ -0,0 +1,31 @@ +# 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 diff --git a/spec/features/projects/sourcegraph_csp_spec.rb b/spec/features/projects/sourcegraph_csp_spec.rb index 57d1e8e3034..385a797368c 100644 --- a/spec/features/projects/sourcegraph_csp_spec.rb +++ b/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 diff --git a/spec/support/shared_examples/csp.rb b/spec/support/shared_examples/csp.rb new file mode 100644 index 00000000000..10c4158522f --- /dev/null +++ b/spec/support/shared_examples/csp.rb @@ -0,0 +1,79 @@ +# 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 -- cgit v1.2.1