Merge branch 'ce-jej/group-saml-sso-button-link-description' into 'master'

[CE] Backport SAML unlink changes: UrlBlocker#ascii_only

See merge request gitlab-org/gitlab-ce!23627

4 files changed, 59 insertions, 1 deletions

diff --git a/app/validators/url_validator.rb b/app/validators/url_validator.rb
index 216acf79cbd..5feb0b0f05b 100644
--- a/app/validators/url_validator.rb
+++ b/app/validators/url_validator.rb
@@ -69,6 +69,7 @@ class UrlValidator < ActiveModel::EachValidator
       ports: [],
       allow_localhost: true,
       allow_local_network: true,
+      ascii_only: false,
       enforce_user: false
     }
   end
diff --git a/lib/gitlab/url_blocker.rb b/lib/gitlab/url_blocker.rb
index b8040f73cee..44c71f8431d 100644
--- a/lib/gitlab/url_blocker.rb
+++ b/lib/gitlab/url_blocker.rb
@@ -8,7 +8,7 @@ module Gitlab
     BlockedUrlError = Class.new(StandardError)
 
     class << self
-      def validate!(url, allow_localhost: false, allow_local_network: true, enforce_user: false, ports: [], protocols: [])
+      def validate!(url, ports: [], protocols: [], allow_localhost: false, allow_local_network: true, ascii_only: false, enforce_user: false)
         return true if url.nil?
 
         # Param url can be a string, URI or Addressable::URI
@@ -22,6 +22,7 @@ module Gitlab
         validate_port!(port, ports) if ports.any?
         validate_user!(uri.user) if enforce_user
         validate_hostname!(uri.hostname)
+        validate_unicode_restriction!(uri) if ascii_only
 
         begin
           addrs_info = Addrinfo.getaddrinfo(uri.hostname, port, nil, :STREAM).map do |addr|
@@ -91,6 +92,12 @@ module Gitlab
         raise BlockedUrlError, "Hostname or IP address invalid"
       end
 
+      def validate_unicode_restriction!(uri)
+        return if uri.to_s.ascii_only?
+
+        raise BlockedUrlError, "URI must be ascii only #{uri.to_s.dump}"
+      end
+
       def validate_localhost!(addrs_info)
         local_ips = ["::", "0.0.0.0"]
         local_ips.concat(Socket.ip_address_list.map(&:ip_address))
diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb
index 39e0a17a307..62970bd8cb6 100644
--- a/spec/lib/gitlab/url_blocker_spec.rb
+++ b/spec/lib/gitlab/url_blocker_spec.rb
@@ -249,6 +249,27 @@ describe Gitlab::UrlBlocker do
         end
       end
     end
+
+    context 'when ascii_only is true' do
+      it 'returns true for unicode domain' do
+        expect(described_class.blocked_url?('https://𝕘itⅼαƄ.com/foo/foo.bar', ascii_only: true)).to be true
+      end
+
+      it 'returns true for unicode tld' do
+        expect(described_class.blocked_url?('https://gitlab.ᴄοｍ/foo/foo.bar', ascii_only: true)).to be true
+      end
+
+      it 'returns true for unicode path' do
+        expect(described_class.blocked_url?('https://gitlab.com/𝒇οο/𝒇οο.Ƅαꮁ', ascii_only: true)).to be true
+      end
+
+      it 'returns true for IDNA deviations' do
+        expect(described_class.blocked_url?('https://mißile.com/foo/foo.bar', ascii_only: true)).to be true
+        expect(described_class.blocked_url?('https://miςςile.com/foo/foo.bar', ascii_only: true)).to be true
+        expect(described_class.blocked_url?('https://git‍lab.com/foo/foo.bar', ascii_only: true)).to be true
+        expect(described_class.blocked_url?('https://git‌lab.com/foo/foo.bar', ascii_only: true)).to be true
+      end
+    end
   end
 
   describe '#validate_hostname!' do
diff --git a/spec/validators/url_validator_spec.rb b/spec/validators/url_validator_spec.rb
index 082d09d3f16..f3f3386382f 100644
--- a/spec/validators/url_validator_spec.rb
+++ b/spec/validators/url_validator_spec.rb
@@ -143,4 +143,33 @@ describe UrlValidator do
       end
     end
   end
+
+  context 'when ascii_only is' do
+    let(:url) { 'https://𝕘itⅼαƄ.com/foo/foo.bar'}
+    let(:validator) { described_class.new(attributes: [:link_url], ascii_only: ascii_only) }
+
+    context 'true' do
+      let(:ascii_only) { true }
+
+      it 'prevents unicode characters' do
+        badge.link_url = url
+
+        subject
+
+        expect(badge.errors.empty?).to be false
+      end
+    end
+
+    context 'false (default)' do
+      let(:ascii_only) { false }
+
+      it 'does not prevent unicode characters' do
+        badge.link_url = url
+
+        subject
+
+        expect(badge.errors.empty?).to be true
+      end
+    end
+  end
 end