diff options
author | Brett Walker <bwalker@gitlab.com> | 2019-02-21 02:51:55 +0300 |
---|---|---|
committer | Brett Walker <bwalker@gitlab.com> | 2019-08-24 07:44:53 +0300 |
commit | ad05e488636ebe05b4985dbf3c7d912fd8d56f49 (patch) | |
tree | 427b631b34fa8ed7511f3ed789185cd82a1a6da9 /spec | |
parent | 892e4c0da818006159cc26bc79f1fa48b76c9b3f (diff) |
Add support for using a Camo proxy server
User images and videos will get proxied through
the Camo server in order to keep malicious
sites from collecting the IP address of users.
Diffstat (limited to 'spec')
-rw-r--r-- | spec/initializers/asset_proxy_setting_spec.rb | 13 | ||||
-rw-r--r-- | spec/lib/banzai/filter/asset_proxy_filter_spec.rb | 95 | ||||
-rw-r--r-- | spec/lib/banzai/filter/external_link_filter_spec.rb | 12 | ||||
-rw-r--r-- | spec/lib/banzai/filter/image_link_filter_spec.rb | 7 | ||||
-rw-r--r-- | spec/lib/banzai/filter/video_link_filter_spec.rb | 22 | ||||
-rw-r--r-- | spec/lib/banzai/pipeline/gfm_pipeline_spec.rb | 44 | ||||
-rw-r--r-- | spec/models/application_setting_spec.rb | 65 | ||||
-rw-r--r-- | spec/requests/api/settings_spec.rb | 28 | ||||
-rw-r--r-- | spec/services/application_settings/update_service_spec.rb | 33 | ||||
-rw-r--r-- | spec/support/helpers/stub_configuration.rb | 4 |
10 files changed, 323 insertions, 0 deletions
diff --git a/spec/initializers/asset_proxy_setting_spec.rb b/spec/initializers/asset_proxy_setting_spec.rb new file mode 100644 index 00000000000..42e4d4aa594 --- /dev/null +++ b/spec/initializers/asset_proxy_setting_spec.rb @@ -0,0 +1,13 @@ +require 'spec_helper' + +describe 'Asset proxy settings initialization' do + describe '#asset_proxy' do + it 'defaults to disabled' do + expect(Banzai::Filter::AssetProxyFilter).to receive(:initialize_settings) + + require_relative '../../config/initializers/asset_proxy_settings' + + expect(Gitlab.config.asset_proxy.enabled).to be_falsey + end + end +end diff --git a/spec/lib/banzai/filter/asset_proxy_filter_spec.rb b/spec/lib/banzai/filter/asset_proxy_filter_spec.rb new file mode 100644 index 00000000000..b7f45421b2a --- /dev/null +++ b/spec/lib/banzai/filter/asset_proxy_filter_spec.rb @@ -0,0 +1,95 @@ +require 'spec_helper' + +describe Banzai::Filter::AssetProxyFilter do + include FilterSpecHelper + + def image(path) + %(<img src="#{path}" />) + end + + it 'does not replace if disabled' do + stub_asset_proxy_setting(enabled: false) + + context = described_class.transform_context({}) + src = 'http://example.com/test.png' + doc = filter(image(src), context) + + expect(doc.at_css('img')['src']).to eq src + end + + context 'during initialization' do + after do + Gitlab.config.asset_proxy['enabled'] = false + end + + it '#initialize_settings' do + stub_application_setting(asset_proxy_enabled: true) + stub_application_setting(asset_proxy_secret_key: 'shared-secret') + stub_application_setting(asset_proxy_url: 'https://assets.example.com') + stub_application_setting(asset_proxy_whitelist: %w(gitlab.com *.mydomain.com)) + + described_class.initialize_settings + + expect(Gitlab.config.asset_proxy.enabled).to be_truthy + expect(Gitlab.config.asset_proxy.secret_key).to eq 'shared-secret' + expect(Gitlab.config.asset_proxy.url).to eq 'https://assets.example.com' + expect(Gitlab.config.asset_proxy.whitelist).to eq %w(gitlab.com *.mydomain.com) + expect(Gitlab.config.asset_proxy.domain_regexp).to eq /^(gitlab\.com|.*?\.mydomain\.com)$/i + end + end + + context 'when properly configured' do + before do + stub_asset_proxy_setting(enabled: true) + stub_asset_proxy_setting(secret_key: 'shared-secret') + stub_asset_proxy_setting(url: 'https://assets.example.com') + stub_asset_proxy_setting(whitelist: %W(gitlab.com *.mydomain.com #{Gitlab.config.gitlab.host})) + stub_asset_proxy_setting(domain_regexp: described_class.compile_whitelist(Gitlab.config.asset_proxy.whitelist)) + @context = described_class.transform_context({}) + end + + it 'replaces img src' do + src = 'http://example.com/test.png' + new_src = 'https://assets.example.com/08df250eeeef1a8cf2c761475ac74c5065105612/687474703a2f2f6578616d706c652e636f6d2f746573742e706e67' + doc = filter(image(src), @context) + + expect(doc.at_css('img')['src']).to eq new_src + expect(doc.at_css('img')['data-canonical-src']).to eq src + end + + it 'skips internal images' do + src = "#{Gitlab.config.gitlab.url}/test.png" + doc = filter(image(src), @context) + + expect(doc.at_css('img')['src']).to eq src + end + + it 'skip relative urls' do + src = "/test.png" + doc = filter(image(src), @context) + + expect(doc.at_css('img')['src']).to eq src + end + + it 'skips single domain' do + src = "http://gitlab.com/test.png" + doc = filter(image(src), @context) + + expect(doc.at_css('img')['src']).to eq src + end + + it 'skips single domain and ignores url in query string' do + src = "http://gitlab.com/test.png?url=http://example.com/test.png" + doc = filter(image(src), @context) + + expect(doc.at_css('img')['src']).to eq src + end + + it 'skips wildcarded domain' do + src = "http://images.mydomain.com/test.png" + doc = filter(image(src), @context) + + expect(doc.at_css('img')['src']).to eq src + end + end +end diff --git a/spec/lib/banzai/filter/external_link_filter_spec.rb b/spec/lib/banzai/filter/external_link_filter_spec.rb index 59fea5766ee..4b2500b31f7 100644 --- a/spec/lib/banzai/filter/external_link_filter_spec.rb +++ b/spec/lib/banzai/filter/external_link_filter_spec.rb @@ -156,6 +156,18 @@ describe Banzai::Filter::ExternalLinkFilter do expect(doc_email.to_html).to include('http://xn--example-6p25f.com/</a>') end end + + context 'autolinked image' do + let(:html) { %q(<a href="https://assets.example.com/6d8b/634c" data-canonical-src="http://exa%F0%9F%98%84mple.com/test.png"><img src="http://exa%F0%9F%98%84mple.com/test.png" data-canonical-src="http://exa%F0%9F%98%84mple.com/test.png"></a>) } + let(:doc) { filter(html) } + + it_behaves_like 'an external link with rel attribute' + + it 'adds a toolip with punycode' do + expect(doc.to_html).to include('class="has-tooltip"') + expect(doc.to_html).to include('title="http://xn--example-6p25f.com/test.png"') + end + end end context 'for links that look malicious' do diff --git a/spec/lib/banzai/filter/image_link_filter_spec.rb b/spec/lib/banzai/filter/image_link_filter_spec.rb index 7b0cb675551..011e3a1e2da 100644 --- a/spec/lib/banzai/filter/image_link_filter_spec.rb +++ b/spec/lib/banzai/filter/image_link_filter_spec.rb @@ -28,4 +28,11 @@ describe Banzai::Filter::ImageLinkFilter do doc = filter(%Q(<p>test #{image('/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg')} inline</p>)) expect(doc.to_html).to match %r{^<p>test <a[^>]*><img[^>]*></a> inline</p>$} end + + it 'keep the data-canonical-src' do + doc = filter(%q(<img src="http://assets.example.com/6cd/4d7" data-canonical-src="http://example.com/test.png" />)) + + expect(doc.at_css('img')['src']).to eq doc.at_css('a')['href'] + expect(doc.at_css('img')['data-canonical-src']).to eq doc.at_css('a')['data-canonical-src'] + end end diff --git a/spec/lib/banzai/filter/video_link_filter_spec.rb b/spec/lib/banzai/filter/video_link_filter_spec.rb index 483e806624c..cd932f502f3 100644 --- a/spec/lib/banzai/filter/video_link_filter_spec.rb +++ b/spec/lib/banzai/filter/video_link_filter_spec.rb @@ -49,4 +49,26 @@ describe Banzai::Filter::VideoLinkFilter do expect(element['src']).to eq '/path/my_image.jpg' end end + + context 'when asset proxy is enabled' do + it 'uses the correct src' do + stub_asset_proxy_setting(enabled: true) + + proxy_src = 'https://assets.example.com/6d8b63' + canonical_src = 'http://example.com/test.mp4' + image = %(<img src="#{proxy_src}" data-canonical-src="#{canonical_src}" />) + container = filter(image, asset_proxy_enabled: true).children.first + + expect(container['class']).to eq 'video-container' + + video, paragraph = container.children + + expect(video['src']).to eq proxy_src + expect(video['data-canonical-src']).to eq canonical_src + + link = paragraph.children.first + + expect(link['href']).to eq proxy_src + end + end end diff --git a/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb b/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb index 0a3e0962452..3a9ecd2fb81 100644 --- a/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb +++ b/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb @@ -142,4 +142,48 @@ describe Banzai::Pipeline::GfmPipeline do expect(output).to include(Gitlab::Routing.url_helpers.milestone_path(milestone)) end end + + describe 'asset proxy' do + let(:project) { create(:project, :public) } + let(:image) { '![proxy](http://example.com/test.png)' } + let(:proxy) { 'https://assets.example.com/08df250eeeef1a8cf2c761475ac74c5065105612/687474703a2f2f6578616d706c652e636f6d2f746573742e706e67' } + let(:version) { Gitlab::CurrentSettings.current_application_settings.local_markdown_version } + + before do + stub_asset_proxy_setting(enabled: true) + stub_asset_proxy_setting(secret_key: 'shared-secret') + stub_asset_proxy_setting(url: 'https://assets.example.com') + stub_asset_proxy_setting(whitelist: %W(gitlab.com *.mydomain.com #{Gitlab.config.gitlab.host})) + stub_asset_proxy_setting(domain_regexp: Banzai::Filter::AssetProxyFilter.compile_whitelist(Gitlab.config.asset_proxy.whitelist)) + end + + it 'replaces a lazy loaded img src' do + output = described_class.to_html(image, project: project) + doc = Nokogiri::HTML.fragment(output) + result = doc.css('img').first + + expect(result['data-src']).to eq(proxy) + end + + it 'autolinks images to the proxy' do + output = described_class.to_html(image, project: project) + doc = Nokogiri::HTML.fragment(output) + result = doc.css('a').first + + expect(result['href']).to eq(proxy) + expect(result['data-canonical-src']).to eq('http://example.com/test.png') + end + + it 'properly adds tooltips to link for IDN images' do + image = '![proxy](http://exa😄mple.com/test.png)' + proxy = 'https://assets.example.com/6d8b634c412a23c6bfe1b2963f174febf5635ddd/687474703a2f2f6578612546302539462539382538346d706c652e636f6d2f746573742e706e67' + output = described_class.to_html(image, project: project) + doc = Nokogiri::HTML.fragment(output) + result = doc.css('a').first + + expect(result['href']).to eq(proxy) + expect(result['data-canonical-src']).to eq('http://exa%F0%9F%98%84mple.com/test.png') + expect(result['title']).to eq 'http://xn--example-6p25f.com/test.png' + end + end end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index db80b85360f..4f7a6d102b8 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -355,6 +355,71 @@ describe ApplicationSetting do end end end + + context 'asset proxy settings' do + before do + subject.asset_proxy_enabled = true + end + + describe '#asset_proxy_url' do + it { is_expected.not_to allow_value('').for(:asset_proxy_url) } + it { is_expected.to allow_value(http).for(:asset_proxy_url) } + it { is_expected.to allow_value(https).for(:asset_proxy_url) } + it { is_expected.not_to allow_value(ftp).for(:asset_proxy_url) } + + it 'is not required when asset proxy is disabled' do + subject.asset_proxy_enabled = false + subject.asset_proxy_url = '' + + expect(subject).to be_valid + end + end + + describe '#asset_proxy_secret_key' do + it { is_expected.not_to allow_value('').for(:asset_proxy_secret_key) } + it { is_expected.to allow_value('anything').for(:asset_proxy_secret_key) } + + it 'is not required when asset proxy is disabled' do + subject.asset_proxy_enabled = false + subject.asset_proxy_secret_key = '' + + expect(subject).to be_valid + end + + it 'is encrypted' do + subject.asset_proxy_secret_key = 'shared secret' + + expect(subject.encrypted_asset_proxy_secret_key).to be_present + expect(subject.encrypted_asset_proxy_secret_key).not_to eq(subject.asset_proxy_secret_key) + end + end + + describe '#asset_proxy_whitelist' do + context 'when given an Array' do + it 'sets the domains and adds current running host' do + setting.asset_proxy_whitelist = ['example.com', 'assets.example.com'] + expect(setting.asset_proxy_whitelist).to eq(['example.com', 'assets.example.com', 'localhost']) + end + end + + context 'when given a String' do + it 'sets multiple domains with spaces' do + setting.asset_proxy_whitelist = 'example.com *.example.com' + expect(setting.asset_proxy_whitelist).to eq(['example.com', '*.example.com', 'localhost']) + end + + it 'sets multiple domains with newlines and a space' do + setting.asset_proxy_whitelist = "example.com\n *.example.com" + expect(setting.asset_proxy_whitelist).to eq(['example.com', '*.example.com', 'localhost']) + end + + it 'sets multiple domains with commas' do + setting.asset_proxy_whitelist = "example.com, *.example.com" + expect(setting.asset_proxy_whitelist).to eq(['example.com', '*.example.com', 'localhost']) + end + end + end + end end context 'restrict creating duplicates' do diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index 590107d5161..048d04cdefd 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -224,5 +224,33 @@ describe API::Settings, 'Settings' do expect(json_response['error']).to eq('plantuml_url is missing') end end + + context 'asset_proxy settings' do + it 'updates application settings' do + put api('/application/settings', admin), + params: { + asset_proxy_enabled: true, + asset_proxy_url: 'http://assets.example.com', + asset_proxy_secret_key: 'shared secret', + asset_proxy_whitelist: ['example.com', '*.example.com'] + } + + expect(response).to have_gitlab_http_status(200) + expect(json_response['asset_proxy_enabled']).to be(true) + expect(json_response['asset_proxy_url']).to eq('http://assets.example.com') + expect(json_response['asset_proxy_secret_key']).to be_nil + expect(json_response['asset_proxy_whitelist']).to eq(['example.com', '*.example.com', 'localhost']) + end + + it 'allows a string for asset_proxy_whitelist' do + put api('/application/settings', admin), + params: { + asset_proxy_whitelist: 'example.com, *.example.com' + } + + expect(response).to have_gitlab_http_status(200) + expect(json_response['asset_proxy_whitelist']).to eq(['example.com', '*.example.com', 'localhost']) + end + end end end diff --git a/spec/services/application_settings/update_service_spec.rb b/spec/services/application_settings/update_service_spec.rb index adb5219d691..74f94f8ed0f 100644 --- a/spec/services/application_settings/update_service_spec.rb +++ b/spec/services/application_settings/update_service_spec.rb @@ -110,6 +110,39 @@ describe ApplicationSettings::UpdateService do end end + describe 'markdown cache invalidators' do + shared_examples 'invalidates markdown cache' do |attribute| + let(:params) { attribute } + + it 'increments cache' do + expect { subject.execute }.to change(application_settings, :local_markdown_version).by(1) + end + end + + it_behaves_like 'invalidates markdown cache', { asset_proxy_enabled: true } + it_behaves_like 'invalidates markdown cache', { asset_proxy_url: 'http://test.com' } + it_behaves_like 'invalidates markdown cache', { asset_proxy_secret_key: 'another secret' } + it_behaves_like 'invalidates markdown cache', { asset_proxy_whitelist: ['domain.com'] } + + context 'when also setting the local_markdown_version' do + let(:params) { { asset_proxy_enabled: true, local_markdown_version: 12 } } + + it 'does not increment' do + expect { subject.execute }.to change(application_settings, :local_markdown_version).to(12) + end + end + + context 'do not invalidate if value does not change' do + let(:params) { { asset_proxy_enabled: true, asset_proxy_secret_key: 'secret', asset_proxy_url: 'http://test.com' } } + + it 'does not increment' do + described_class.new(application_settings, admin, params).execute + + expect { described_class.new(application_settings, admin, params).execute }.not_to change(application_settings, :local_markdown_version) + end + end + end + describe 'performance bar settings' do using RSpec::Parameterized::TableSyntax diff --git a/spec/support/helpers/stub_configuration.rb b/spec/support/helpers/stub_configuration.rb index dec7898d8d2..f364e4fd158 100644 --- a/spec/support/helpers/stub_configuration.rb +++ b/spec/support/helpers/stub_configuration.rb @@ -105,6 +105,10 @@ module StubConfiguration allow(Gitlab.config.gitlab_shell).to receive_messages(to_settings(messages)) end + def stub_asset_proxy_setting(messages) + allow(Gitlab.config.asset_proxy).to receive_messages(to_settings(messages)) + end + def stub_rack_attack_setting(messages) allow(Gitlab.config.rack_attack).to receive(:git_basic_auth).and_return(messages) allow(Gitlab.config.rack_attack.git_basic_auth).to receive_messages(to_settings(messages)) |