Add custom UrlValidator

10 files changed, 79 insertions, 37 deletions

diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 5ddcf3d9a0b..1880ad9f33c 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -43,12 +43,12 @@ class ApplicationSetting < ActiveRecord::Base
 
   validates :home_page_url,
     allow_blank: true,
-    format: { with: /\A#{URI.regexp(%w(http https))}\z/, message: "should be a valid url" },
+    url: true,
     if: :home_page_url_column_exist
 
   validates :after_sign_out_path,
     allow_blank: true,
-    format: { with: /\A#{URI.regexp(%w(http https))}\z/, message: "should be a valid url" }
+    url: true
 
   validates :admin_notification_email,
     allow_blank: true,
diff --git a/app/models/ci/web_hook.rb b/app/models/ci/web_hook.rb
index 7ca16a1bde8..0dc15eb6683 100644
--- a/app/models/ci/web_hook.rb
+++ b/app/models/ci/web_hook.rb
@@ -20,8 +20,7 @@ module Ci
     # HTTParty timeout
     default_timeout 10
 
-    validates :url, presence: true,
-                    format: { with: URI::regexp(%w(http https)), message: "should be a valid url" }
+    validates :url, presence: true, url: true
 
     def execute(data)
       parsed_url = URI.parse(url)
diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb
index 2caf26cc8c9..715ec5908b7 100644
--- a/app/models/hooks/web_hook.rb
+++ b/app/models/hooks/web_hook.rb
@@ -31,8 +31,7 @@ class WebHook < ActiveRecord::Base
   # HTTParty timeout
   default_timeout Gitlab.config.gitlab.webhook_timeout
 
-  validates :url, presence: true,
-                  format: { with: /\A#{URI.regexp(%w(http https))}\z/, message: "should be a valid url" }
+  validates :url, presence: true, url: true
 
   def execute(data, hook_name)
     parsed_url = URI.parse(url)
diff --git a/app/models/project.rb b/app/models/project.rb
index 6010770a5f2..af034a6692b 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -152,7 +152,7 @@ class Project < ActiveRecord::Base
   validates_uniqueness_of :name, scope: :namespace_id
   validates_uniqueness_of :path, scope: :namespace_id
   validates :import_url,
-    format: { with: /\A#{URI.regexp(%w(ssh git http https))}\z/, message: 'should be a valid url' },
+    url: { protocols: %w(ssh git http https) },
     if: :external_import?
   validates :star_count, numericality: { greater_than_or_equal_to: 0 }
   validate :check_limit, on: :create
diff --git a/app/models/project_services/bamboo_service.rb b/app/models/project_services/bamboo_service.rb
index d31b12f539e..0a61ad96a0e 100644
--- a/app/models/project_services/bamboo_service.rb
+++ b/app/models/project_services/bamboo_service.rb
@@ -23,10 +23,7 @@ class BambooService < CiService
 
   prop_accessor :bamboo_url, :build_key, :username, :password
 
-  validates :bamboo_url,
-    presence: true,
-    format: { with: /\A#{URI.regexp}\z/ },
-    if: :activated?
+  validates :bamboo_url, presence: true, url: true, if: :activated?
   validates :build_key, presence: true, if: :activated?
   validates :username,
     presence: true,
@@ -84,7 +81,7 @@ class BambooService < CiService
   def supported_events
     %w(push)
   end
-  
+
   def build_info(sha)
     url = URI.parse("#{bamboo_url}/rest/api/latest/result?label=#{sha}")
 
diff --git a/app/models/project_services/drone_ci_service.rb b/app/models/project_services/drone_ci_service.rb
index 06c3922593c..08e5ccb3855 100644
--- a/app/models/project_services/drone_ci_service.rb
+++ b/app/models/project_services/drone_ci_service.rb
@@ -19,14 +19,11 @@
 #
 
 class DroneCiService < CiService
-  
+
   prop_accessor :drone_url, :token, :enable_ssl_verification
-  validates :drone_url,
-    presence: true,
-    format: { with: /\A#{URI.regexp(%w(http https))}\z/, message: "should be a valid url" }, if: :activated?
-  validates :token,
-    presence: true,
-    if: :activated?
+
+  validates :drone_url, presence: true, url: true, if: :activated?
+  validates :token, presence: true, if: :activated?
 
   after_save :compose_service_hook, if: :activated?
 
@@ -58,16 +55,16 @@ class DroneCiService < CiService
   end
 
   def merge_request_status_path(iid, sha = nil, ref = nil)
-    url = [drone_url, 
-           "gitlab/#{project.namespace.path}/#{project.path}/pulls/#{iid}", 
+    url = [drone_url,
+           "gitlab/#{project.namespace.path}/#{project.path}/pulls/#{iid}",
            "?access_token=#{token}"]
 
     URI.join(*url).to_s
   end
 
   def commit_status_path(sha, ref)
-    url = [drone_url, 
-           "gitlab/#{project.namespace.path}/#{project.path}/commits/#{sha}", 
+    url = [drone_url,
+           "gitlab/#{project.namespace.path}/#{project.path}/commits/#{sha}",
            "?branch=#{URI::encode(ref.to_s)}&access_token=#{token}"]
 
     URI.join(*url).to_s
@@ -114,15 +111,15 @@ class DroneCiService < CiService
   end
 
   def merge_request_page(iid, sha, ref)
-    url = [drone_url, 
+    url = [drone_url,
            "gitlab/#{project.namespace.path}/#{project.path}/redirect/pulls/#{iid}"]
 
     URI.join(*url).to_s
   end
 
   def commit_page(sha, ref)
-    url = [drone_url, 
-           "gitlab/#{project.namespace.path}/#{project.path}/redirect/commits/#{sha}", 
+    url = [drone_url,
+           "gitlab/#{project.namespace.path}/#{project.path}/redirect/commits/#{sha}",
            "?branch=#{URI::encode(ref.to_s)}"]
 
     URI.join(*url).to_s
@@ -163,10 +160,10 @@ class DroneCiService < CiService
   end
 
   def push_valid?(data)
-    opened_merge_requests = project.merge_requests.opened.where(source_project_id: project.id, 
+    opened_merge_requests = project.merge_requests.opened.where(source_project_id: project.id,
                                                                 source_branch: Gitlab::Git.ref_name(data[:ref]))
 
-    opened_merge_requests.empty? && data[:total_commits_count] > 0 && 
+    opened_merge_requests.empty? && data[:total_commits_count] > 0 &&
       !Gitlab::Git.blank_ref?(data[:after])
   end
 
diff --git a/app/models/project_services/external_wiki_service.rb b/app/models/project_services/external_wiki_service.rb
index 9c46af7e721..74c57949b4d 100644
--- a/app/models/project_services/external_wiki_service.rb
+++ b/app/models/project_services/external_wiki_service.rb
@@ -22,10 +22,8 @@ class ExternalWikiService < Service
   include HTTParty
 
   prop_accessor :external_wiki_url
-  validates :external_wiki_url,
-            presence: true,
-            format: { with: /\A#{URI.regexp}\z/ },
-            if: :activated?
+
+  validates :external_wiki_url, presence: true, url: true, if: :activated?
 
   def title
     'External Wiki'
diff --git a/app/models/project_services/teamcity_service.rb b/app/models/project_services/teamcity_service.rb
index 0b022461250..29d4236745a 100644
--- a/app/models/project_services/teamcity_service.rb
+++ b/app/models/project_services/teamcity_service.rb
@@ -23,16 +23,16 @@ class TeamcityService < CiService
 
   prop_accessor :teamcity_url, :build_type, :username, :password
 
-  validates :teamcity_url,
-    presence: true,
-    format: { with: /\A#{URI.regexp}\z/ }, if: :activated?
+  validates :teamcity_url, presence: true, url: true, if: :activated?
   validates :build_type, presence: true, if: :activated?
   validates :username,
     presence: true,
-    if: ->(service) { service.password? }, if: :activated?
+    if: ->(service) { service.password? },
+    if: :activated?
   validates :password,
     presence: true,
-    if: ->(service) { service.username? }, if: :activated?
+    if: ->(service) { service.username? },
+    if: :activated?
 
   attr_accessor :response
 
diff --git a/app/validators/url_validator.rb b/app/validators/url_validator.rb
new file mode 100644
index 00000000000..2848b9cd33d
--- /dev/null
+++ b/app/validators/url_validator.rb
@@ -0,0 +1,36 @@
+# UrlValidator
+#
+# Custom validator for URLs.
+#
+# By default, only URLs for the HTTP(S) protocols will be considered valid.
+# Provide a `:protocols` option to configure accepted protocols.
+#
+# Example:
+#
+#   class User < ActiveRecord::Base
+#     validates :personal_url, url: true
+#
+#     validates :ftp_url, url: { protocols: %w(ftp) }
+#
+#     validates :git_url, url: { protocols: %w(http https ssh git) }
+#   end
+#
+class UrlValidator < ActiveModel::EachValidator
+  def validate_each(record, attribute, value)
+    unless valid_url?(value)
+      record.errors.add(attribute, "must be a valid URL")
+    end
+  end
+
+  private
+
+  def default_options
+    @default_options ||= { protocols: %w(http https) }
+  end
+
+  def valid_url?(value)
+    options = default_options.merge(self.options)
+
+    value =~ /\A#{URI.regexp(options[:protocols])}\z/
+  end
+end
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index dfbac7b4004..b67b84959d9 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -36,6 +36,22 @@ describe ApplicationSetting, models: true do
 
   it { expect(setting).to be_valid }
 
+  describe 'validations' do
+    let(:http)  { 'http://example.com' }
+    let(:https) { 'https://example.com' }
+    let(:ftp)   { 'ftp://example.com' }
+
+    it { is_expected.to allow_value(nil).for(:home_page_url) }
+    it { is_expected.to allow_value(http).for(:home_page_url) }
+    it { is_expected.to allow_value(https).for(:home_page_url) }
+    it { is_expected.not_to allow_value(ftp).for(:home_page_url) }
+
+    it { is_expected.to allow_value(nil).for(:after_sign_out_path) }
+    it { is_expected.to allow_value(http).for(:after_sign_out_path) }
+    it { is_expected.to allow_value(https).for(:after_sign_out_path) }
+    it { is_expected.not_to allow_value(ftp).for(:after_sign_out_path) }
+  end
+
   context 'restricted signup domains' do
     it 'set single domain' do
       setting.restricted_signup_domains_raw = 'example.com'