Merge branch...

Merge branch '199076-elasticsearch-integration-menu-will-allow-entering-a-non-url-pattern-in-the-url-field-which' into 'master'

Resolve "Elasticsearch Integration Menu will allow entering a non-URL pattern in the URL field which will cause elusive errors during indexing"

Closes #199076

See merge request gitlab-org/gitlab!28209

ee/app/models/ee/application_setting.rb

@@ -60,6 +60,8 @@ module EE
                 presence: { message: "can't be blank when indexing is enabled" },
                 if: ->(setting) { setting.elasticsearch_indexing? }
 
+      validate :check_elasticsearch_url_scheme, if: :elasticsearch_url_changed?
+
       validates :elasticsearch_aws_region,
                 presence: { message: "can't be blank when using aws hosted elasticsearch" },
                 if: ->(setting) { setting.elasticsearch_indexing? && setting.elasticsearch_aws? }
@@ -304,5 +306,18 @@ module EE
     rescue ::Gitlab::CIDR::ValidationError => e
       errors.add(:geo_node_allowed_ips, e.message)
     end
+
+    def check_elasticsearch_url_scheme
+      # ElasticSearch only exposes a RESTful API, hence we need
+      # to use the HTTP protocol on all URLs.
+      elasticsearch_url.each do |str|
+        ::Gitlab::UrlBlocker.validate!(str,
+                                       schemes: %w[http https],
+                                       allow_localhost: true,
+                                       dns_rebind_protection: false)
+      end
+    rescue ::Gitlab::UrlBlocker::BlockedUrlError
+      errors.add(:elasticsearch_url, "only supports valid HTTP(S) URLs.")
+    end
   end
 end

ee/changelogs/unreleased/199076-elasticsearch-integration-menu-will-allow-entering-a-non-url-pa.yml

+---
+title: Validates ElasticSearch URLs are valid HTTP(S) URLs
+merge_request:
+author: mbergeron
+type: fixed

ee/spec/models/application_setting_spec.rb

@@ -102,6 +102,33 @@ describe ApplicationSetting do
         end
       end
     end
+
+    context 'when validating elasticsearch_url' do
+      where(:elasticsearch_url, :is_valid) do
+        "http://es.localdomain" | true
+        "https://es.localdomain" | true
+        "http://es.localdomain, https://es.localdomain " | true
+        "http://10.0.0.1" | true
+        "https://10.0.0.1" | true
+        "http://10.0.0.1, https://10.0.0.1" | true
+        "http://localhost" | true
+        "http://127.0.0.1" | true
+
+        "es.localdomain" | false
+        "10.0.0.1" | false
+        "http://es.localdomain, es.localdomain" | false
+        "http://es.localdomain, 10.0.0.1" | false
+        "this_isnt_a_url" | false
+      end
+
+      with_them do
+        it do
+          setting.elasticsearch_url = elasticsearch_url
+
+          expect(setting.valid?).to eq(is_valid)
+        end
+      end
+    end
   end
 
   describe '#should_check_namespace_plan?' do

lib/gitlab/url_blocker.rb

@@ -11,8 +11,8 @@ module Gitlab
       # Validates the given url according to the constraints specified by arguments.
       #
       # ports - Raises error if the given URL port does is not between given ports.
-      # allow_localhost - Raises error if URL resolves to a localhost IP address and argument is true.
-      # allow_local_network - Raises error if URL resolves to a link-local address and argument is true.
+      # allow_localhost - Raises error if URL resolves to a localhost IP address and argument is false.
+      # allow_local_network - Raises error if URL resolves to a link-local address and argument is false.
       # ascii_only - Raises error if URL has unicode characters and argument is true.
       # enforce_user - Raises error if URL user doesn't start with alphanumeric characters and argument is true.
       # enforce_sanitization - Raises error if URL includes any HTML/CSS/JS tags and argument is true.