Add latest changes from gitlab-org/security/gitlab@12-8-stable-ee

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) }