diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-02-28 22:58:33 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-02-28 22:58:33 +0300 |
commit | e316c4740c1b604de112bbad52c2531d2261a8f8 (patch) | |
tree | 60114a22eb5fd1772fe2f9455a756e1a27f6f237 /spec | |
parent | 90768b3af0385ae687c3d7d45d0424f572cd6cfd (diff) |
Add latest changes from gitlab-org/security/gitlab@12-8-stable-ee
Diffstat (limited to 'spec')
-rw-r--r-- | spec/lib/gitlab/asset_proxy_spec.rb | 50 | ||||
-rw-r--r-- | spec/lib/gitlab/utils_spec.rb | 14 | ||||
-rw-r--r-- | spec/migrations/clean_grafana_url_spec.rb | 37 | ||||
-rw-r--r-- | spec/models/application_setting_spec.rb | 48 | ||||
-rw-r--r-- | spec/models/badge_spec.rb | 16 | ||||
-rw-r--r-- | spec/validators/addressable_url_validator_spec.rb | 16 |
6 files changed, 181 insertions, 0 deletions
diff --git a/spec/lib/gitlab/asset_proxy_spec.rb b/spec/lib/gitlab/asset_proxy_spec.rb new file mode 100644 index 00000000000..f5aa1819982 --- /dev/null +++ b/spec/lib/gitlab/asset_proxy_spec.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::AssetProxy do + context 'when asset proxy is disabled' do + before do + stub_asset_proxy_setting(enabled: false) + end + + it 'returns the original URL' do + url = 'http://example.com/test.png' + + expect(described_class.proxy_url(url)).to eq(url) + end + end + + context 'when asset proxy is enabled' do + before do + stub_asset_proxy_setting(whitelist: %w(gitlab.com *.mydomain.com)) + stub_asset_proxy_setting( + enabled: true, + url: 'https://assets.example.com', + secret_key: 'shared-secret', + domain_regexp: Banzai::Filter::AssetProxyFilter.compile_whitelist(Gitlab.config.asset_proxy.whitelist) + ) + end + + it 'returns a proxied URL' do + url = 'http://example.com/test.png' + proxied_url = 'https://assets.example.com/08df250eeeef1a8cf2c761475ac74c5065105612/687474703a2f2f6578616d706c652e636f6d2f746573742e706e67' + + expect(described_class.proxy_url(url)).to eq(proxied_url) + end + + context 'whitelisted domain' do + it 'returns original URL for single domain whitelist' do + url = 'http://gitlab.com/test.png' + + expect(described_class.proxy_url(url)).to eq(url) + end + + it 'returns original URL for wildcard subdomain whitelist' do + url = 'http://test.mydomain.com/test.png' + + expect(described_class.proxy_url(url)).to eq(url) + end + end + end +end diff --git a/spec/lib/gitlab/utils_spec.rb b/spec/lib/gitlab/utils_spec.rb index 85a536ee6ad..6841e7719dc 100644 --- a/spec/lib/gitlab/utils_spec.rb +++ b/spec/lib/gitlab/utils_spec.rb @@ -283,4 +283,18 @@ describe Gitlab::Utils do expect(described_class.string_to_ip_object('1:0:0:0:0:0:0:0/124')).to eq(IPAddr.new('1:0:0:0:0:0:0:0/124')) end end + + describe '.parse_url' do + it 'returns Addressable::URI object' do + expect(described_class.parse_url('http://gitlab.com')).to be_instance_of(Addressable::URI) + end + + it 'returns nil when URI cannot be parsed' do + expect(described_class.parse_url('://gitlab.com')).to be nil + end + + it 'returns nil with invalid parameter' do + expect(described_class.parse_url(1)).to be nil + end + end end diff --git a/spec/migrations/clean_grafana_url_spec.rb b/spec/migrations/clean_grafana_url_spec.rb new file mode 100644 index 00000000000..9f060fbaf7d --- /dev/null +++ b/spec/migrations/clean_grafana_url_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join('db', 'migrate', '20200214085940_clean_grafana_url.rb') + +describe CleanGrafanaUrl, :migration do + let(:application_settings_table) { table(:application_settings) } + + [ + 'javascript:alert(window.opener.document.location)', + ' javascript:alert(window.opener.document.location)' + ].each do |grafana_url| + it "sets grafana_url back to its default value when grafana_url is '#{grafana_url}'" do + application_settings = application_settings_table.create!(grafana_url: grafana_url) + + migrate! + + expect(application_settings.reload.grafana_url).to eq('/-/grafana') + end + end + + ['/-/grafana', '/some/relative/url', 'http://localhost:9000'].each do |grafana_url| + it "does not modify grafana_url when grafana_url is '#{grafana_url}'" do + application_settings = application_settings_table.create!(grafana_url: grafana_url) + + migrate! + + expect(application_settings.reload.grafana_url).to eq(grafana_url) + end + end + + context 'when application_settings table has no rows' do + it 'does not fail' do + migrate! + end + end +end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index bbd50f1c0ef..abbaa22ff3e 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -19,6 +19,7 @@ describe ApplicationSetting do let(:http) { 'http://example.com' } let(:https) { 'https://example.com' } let(:ftp) { 'ftp://example.com' } + let(:javascript) { 'javascript:alert(window.opener.document.location)' } it { is_expected.to allow_value(nil).for(:home_page_url) } it { is_expected.to allow_value(http).for(:home_page_url) } @@ -81,6 +82,53 @@ describe ApplicationSetting do it { is_expected.not_to allow_value('abc').for(:minimum_password_length) } it { is_expected.to allow_value(10).for(:minimum_password_length) } + context 'grafana_url validations' do + before do + subject.instance_variable_set(:@parsed_grafana_url, nil) + end + + it { is_expected.to allow_value(http).for(:grafana_url) } + it { is_expected.to allow_value(https).for(:grafana_url) } + it { is_expected.not_to allow_value(ftp).for(:grafana_url) } + it { is_expected.not_to allow_value(javascript).for(:grafana_url) } + it { is_expected.to allow_value('/-/grafana').for(:grafana_url) } + it { is_expected.to allow_value('http://localhost:9000').for(:grafana_url) } + + context 'when local URLs are not allowed in system hooks' do + before do + stub_application_setting(allow_local_requests_from_system_hooks: false) + end + + it { is_expected.not_to allow_value('http://localhost:9000').for(:grafana_url) } + end + + context 'with invalid grafana URL' do + it 'adds an error' do + subject.grafana_url = ' ' + http + expect(subject.save).to be false + + expect(subject.errors[:grafana_url]).to eq([ + 'must be a valid relative or absolute URL. ' \ + 'Please check your Grafana URL setting in ' \ + 'Admin Area > Settings > Metrics and profiling > Metrics - Grafana' + ]) + end + end + + context 'with blocked grafana URL' do + it 'adds an error' do + subject.grafana_url = javascript + expect(subject.save).to be false + + expect(subject.errors[:grafana_url]).to eq([ + 'is blocked: Only allowed schemes are http, https. Please check your ' \ + 'Grafana URL setting in ' \ + 'Admin Area > Settings > Metrics and profiling > Metrics - Grafana' + ]) + end + end + end + context 'when snowplow is enabled' do before do setting.snowplow_enabled = true diff --git a/spec/models/badge_spec.rb b/spec/models/badge_spec.rb index 60ae579eb03..fba8f40e99b 100644 --- a/spec/models/badge_spec.rb +++ b/spec/models/badge_spec.rb @@ -91,6 +91,22 @@ describe Badge do let(:method) { :image_url } it_behaves_like 'rendered_links' + + context 'when asset proxy is enabled' do + let(:placeholder_url) { 'http://www.example.com/image' } + + before do + stub_asset_proxy_setting( + enabled: true, + url: 'https://assets.example.com', + secret_key: 'shared-secret' + ) + end + + it 'returns a proxied URL' do + expect(badge.rendered_image_url).to start_with('https://assets.example.com') + end + end end end end diff --git a/spec/validators/addressable_url_validator_spec.rb b/spec/validators/addressable_url_validator_spec.rb index e8a44f7a12a..46b1bebb074 100644 --- a/spec/validators/addressable_url_validator_spec.rb +++ b/spec/validators/addressable_url_validator_spec.rb @@ -5,6 +5,9 @@ require 'spec_helper' describe AddressableUrlValidator do let!(:badge) { build(:badge, link_url: 'http://www.example.com') } + let(:validator) { described_class.new(validator_options.reverse_merge(attributes: [:link_url])) } + let(:validator_options) { {} } + subject { validator.validate(badge) } include_examples 'url validator examples', described_class::DEFAULT_OPTIONS[:schemes] @@ -114,6 +117,19 @@ describe AddressableUrlValidator do end end + context 'when blocked_message is set' do + let(:message) { 'is not allowed due to: %{exception_message}' } + let(:validator_options) { { blocked_message: message } } + + it 'blocks url with provided error message' do + badge.link_url = 'javascript:alert(window.opener.document.location)' + + subject + + expect(badge.errors.first[1]).to eq 'is not allowed due to: Only allowed schemes are http, https' + end + end + context 'when allow_nil is set to true' do let(:validator) { described_class.new(attributes: [:link_url], allow_nil: true) } |