FIX: Allow any protocol in wildcard url checker (#8651)

This is required for people using apps with custom protocols. We still verify the entire URL (including protocol) against the site setting value.

Refactored wildcard_url_checker so that it always returns a boolean, rather than sometimes returning a regex match.

app/services/wildcard_url_checker.rb

@@ -1,22 +1,20 @@
 # frozen_string_literal: true
 
 module WildcardUrlChecker
-  VALID_PROTOCOLS = %w(http https discourse).freeze
-
   def self.check_url(url, url_to_check)
-    return nil if !valid_url?(url_to_check)
+    return false if !valid_url?(url_to_check)
 
     escaped_url = Regexp.escape(url).sub("\\*", '\S*')
     url_regex = Regexp.new("\\A#{escaped_url}\\z", 'i')
 
-    url_to_check.match(url_regex)
+    url_to_check.match?(url_regex)
   end
 
   private
 
   def self.valid_url?(url)
     uri = URI.parse(url)
-    VALID_PROTOCOLS.include?(uri&.scheme) && uri&.host.present?
+    uri&.scheme.present? && uri&.host.present?
   rescue URI::InvalidURIError
     false
   end

spec/services/wildcard_url_checker_spec.rb

@@ -6,40 +6,46 @@ describe WildcardUrlChecker do
 
   describe 'check_url' do
     context 'valid url' do
-      it 'returns correct domain' do
+      it 'returns true' do
         result1 = described_class.check_url('https://*.discourse.org', 'https://anything.is.possible.discourse.org')
-        expect(result1[0]).to eq('https://anything.is.possible.discourse.org')
+        expect(result1).to eq(true)
 
         result2 = described_class.check_url('https://www.discourse.org', 'https://www.discourse.org')
-        expect(result2[0]).to eq('https://www.discourse.org')
+        expect(result2).to eq(true)
 
         result3 = described_class.check_url('*', 'https://hello.discourse.org')
-        expect(result3[0]).to eq('https://hello.discourse.org')
+        expect(result3).to eq(true)
 
         result4 = described_class.check_url('discourse://auth_redirect', 'discourse://auth_redirect')
-        expect(result4[0]).to eq('discourse://auth_redirect')
+        expect(result4).to eq(true)
+
+        result5 = described_class.check_url('customprotocol://www.discourse.org', "customprotocol://www.discourse.org")
+        expect(result5).to eq(true)
       end
     end
 
     context 'invalid domain' do
-      it "doesn't return the domain" do
+      it "returns false" do
         result1 = described_class.check_url('https://*.discourse.org', 'https://bad-domain.discourse.org.evil.com')
-        expect(result1).to eq(nil)
+        expect(result1).to eq(false)
 
         result2 = described_class.check_url('https://www.discourse.org', 'https://www.discourse.org.evil.com')
-        expect(result2).to eq(nil)
+        expect(result2).to eq(false)
 
         result3 = described_class.check_url('https://www.discourse.org', 'https://www.www.discourse.org')
-        expect(result3).to eq(nil)
+        expect(result3).to eq(false)
 
         result4 = described_class.check_url('https://www.discourse.org', "https://www.discourse.org\nwww.discourse.org.evil.com")
-        expect(result4).to eq(nil)
+        expect(result4).to eq(false)
 
-        result5 = described_class.check_url('ttps://www.discourse.org', "ttps://www.discourse.org")
-        expect(result5).to eq(nil)
+        result5 = described_class.check_url('https://', "https://")
+        expect(result5).to eq(false)
 
-        result6 = described_class.check_url('https://', "https://")
-        expect(result6).to eq(nil)
+        result6 = described_class.check_url('invalid$protocol://www.discourse.org', "invalid$protocol://www.discourse.org")
+        expect(result6).to eq(false)
+
+        result7 = described_class.check_url('noscheme', "noscheme")
+        expect(result7).to eq(false)
       end
     end
   end