Merge branch 'fix_ldap_config_lookup' into 'master'

Fix LDAP config lookup for provider 'ldap'

See merge request !1213

3 files changed, 32 insertions, 12 deletions

diff --git a/CHANGELOG b/CHANGELOG
index e7708bd0c1d..69419b0adfe 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -2,6 +2,7 @@ v 7.5.0
   - API: Add support for Hipchat (Kevin Houdebert)
   - Add time zone configuration on gitlab.yml (Sullivan Senechal)
   - Fix LDAP authentication for Git HTTP access
+  - Fix LDAP config lookup for provider 'ldap'
 
 v 7.4.0
   - Refactored membership logic
diff --git a/lib/gitlab/ldap/config.rb b/lib/gitlab/ldap/config.rb
index d41bfba9b0f..0cb24d0ccc1 100644
--- a/lib/gitlab/ldap/config.rb
+++ b/lib/gitlab/ldap/config.rb
@@ -16,10 +16,23 @@ module Gitlab
         servers.map {|server| server['provider_name'] }
       end
 
+      def self.valid_provider?(provider)
+        providers.include?(provider)
+      end
+
+      def self.invalid_provider(provider)
+        raise "Unknown provider (#{provider}). Available providers: #{providers}"
+      end
+
       def initialize(provider)
-        @provider = provider
-        invalid_provider unless valid_provider?
-        @options = config_for(provider)
+        if self.class.valid_provider?(provider)
+          @provider = provider
+        elsif provider == 'ldap'
+          @provider = self.class.providers.first
+        else
+          self.class.invalid_provider(provider)
+        end
+        @options = config_for(@provider) # Use @provider, not provider
       end
 
       def enabled?
@@ -89,14 +102,6 @@ module Gitlab
         end
       end
 
-      def valid_provider?
-        self.class.providers.include?(provider)
-      end
-
-      def invalid_provider
-        raise "Unknown provider (#{provider}). Available providers: #{self.class.providers}"
-      end
-
       def auth_options
         {
           auth: {
diff --git a/spec/lib/gitlab/ldap/config_spec.rb b/spec/lib/gitlab/ldap/config_spec.rb
index 76cc7f95c47..3ebb8aae243 100644
--- a/spec/lib/gitlab/ldap/config_spec.rb
+++ b/spec/lib/gitlab/ldap/config_spec.rb
@@ -16,5 +16,19 @@ describe Gitlab::LDAP::Config do
     it "raises an error if a unknow provider is used" do
       expect{ Gitlab::LDAP::Config.new 'unknown' }.to raise_error
     end
+
+    context "if 'ldap' is the provider name" do
+      let(:provider) { 'ldap' }
+
+      context "and 'ldap' is not in defined as a provider" do
+        before { Gitlab::LDAP::Config.stub(providers: %w{ldapmain}) }
+
+        it "uses the first provider" do
+          # Fetch the provider_name attribute from 'options' so that we know
+          # that the 'options' Hash is not empty/nil.
+          expect(config.options['provider_name']).to eq('ldapmain')
+        end
+      end
+    end
   end
-end
\ No newline at end of file
+end