Commit 20f0bb08 authored by Paul Slaughter's avatar Paul Slaughter

Fix Content-Security-Policy with Sourcegraph

Also renamed SourcegraphGon to SourcegraphDecorator

**Note:**

- If CSP values are set, it uses the values from
default-src if connect-src is not available. This
way the intention of the CSP config remains intact.
parent 7eb94603
# frozen_string_literal: true # frozen_string_literal: true
module SourcegraphGon module SourcegraphDecorator
extend ActiveSupport::Concern extend ActiveSupport::Concern
included do included do
before_action :push_sourcegraph_gon, if: :html_request? before_action :push_sourcegraph_gon, if: :html_request?
content_security_policy do |p|
next if p.directives.blank?
next unless Gitlab::CurrentSettings.sourcegraph_enabled
default_connect_src = p.directives['connect-src'] || p.directives['default-src']
connect_src_values = Array.wrap(default_connect_src) | [Gitlab::CurrentSettings.sourcegraph_url]
p.connect_src(*connect_src_values)
end
end end
private private
......
...@@ -8,7 +8,7 @@ class Projects::BlobController < Projects::ApplicationController ...@@ -8,7 +8,7 @@ class Projects::BlobController < Projects::ApplicationController
include NotesHelper include NotesHelper
include ActionView::Helpers::SanitizeHelper include ActionView::Helpers::SanitizeHelper
include RedirectsForMissingPathOnTree include RedirectsForMissingPathOnTree
include SourcegraphGon include SourcegraphDecorator
prepend_before_action :authenticate_user!, only: [:edit] prepend_before_action :authenticate_user!, only: [:edit]
......
...@@ -8,7 +8,7 @@ class Projects::CommitController < Projects::ApplicationController ...@@ -8,7 +8,7 @@ class Projects::CommitController < Projects::ApplicationController
include CreatesCommit include CreatesCommit
include DiffForPath include DiffForPath
include DiffHelper include DiffHelper
include SourcegraphGon include SourcegraphDecorator
# Authorize # Authorize
before_action :require_non_empty_project before_action :require_non_empty_project
......
...@@ -9,7 +9,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -9,7 +9,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
include ToggleAwardEmoji include ToggleAwardEmoji
include IssuableCollections include IssuableCollections
include RecordUserLastActivity include RecordUserLastActivity
include SourcegraphGon include SourcegraphDecorator
skip_before_action :merge_request, only: [:index, :bulk_update] skip_before_action :merge_request, only: [:index, :bulk_update]
before_action :whitelist_query_limiting, only: [:assign_related_issues, :update] before_action :whitelist_query_limiting, only: [:assign_related_issues, :update]
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
require 'spec_helper' require 'spec_helper'
describe SourcegraphGon do describe SourcegraphDecorator do
let_it_be(:enabled_user) { create(:user, sourcegraph_enabled: true) } let_it_be(:enabled_user) { create(:user, sourcegraph_enabled: true) }
let_it_be(:disabled_user) { create(:user, sourcegraph_enabled: false) } let_it_be(:disabled_user) { create(:user, sourcegraph_enabled: false) }
let_it_be(:public_project) { create(:project, :public) } let_it_be(:public_project) { create(:project, :public) }
...@@ -17,7 +17,7 @@ describe SourcegraphGon do ...@@ -17,7 +17,7 @@ describe SourcegraphGon do
let(:project) { internal_project } let(:project) { internal_project }
controller(ApplicationController) do controller(ApplicationController) do
include SourcegraphGon include SourcegraphDecorator
def index def index
head :ok head :ok
......
# frozen_string_literal: true
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|
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
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 sourcegraph enabled' do
it 'appends to connect-src' do
is_expected.to eql("connect-src #{default_csp_values} #{sourcegraph_url}")
end
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 }
it 'does not add connect-src' do
is_expected.to eql("font-src #{default_csp_values}")
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment