Support simple string LDAP attribute specifications, and search for name rather than username attributesdm-ldap-adapter-attributes

4 files changed, 19 insertions, 8 deletions

diff --git a/changelogs/unreleased/dm-ldap-adapter-attributes.yml b/changelogs/unreleased/dm-ldap-adapter-attributes.yml
new file mode 100644
index 00000000000..edd68ef08e7
--- /dev/null
+++ b/changelogs/unreleased/dm-ldap-adapter-attributes.yml
@@ -0,0 +1,6 @@
+---
+title: Fix signing in using LDAP when attribute mapping uses simple strings instead
+  of arrays
+merge_request:
+author:
+type: fixed
diff --git a/lib/gitlab/ldap/adapter.rb b/lib/gitlab/ldap/adapter.rb
index 8867a91c244..cd7e4ca7b7e 100644
--- a/lib/gitlab/ldap/adapter.rb
+++ b/lib/gitlab/ldap/adapter.rb
@@ -73,7 +73,7 @@ module Gitlab
       private
 
       def user_options(field, value, limit)
-        options = { attributes: user_attributes }
+        options = { attributes: Gitlab::LDAP::Person.ldap_attributes(config).compact.uniq }
         options[:size] = limit if limit
 
         if field.to_sym == :dn
@@ -99,10 +99,6 @@ module Gitlab
           filter
         end
       end
-
-      def user_attributes
-        %W(#{config.uid} cn dn) + config.attributes['username'] + config.attributes['email']
-      end
     end
   end
 end
diff --git a/lib/gitlab/ldap/person.rb b/lib/gitlab/ldap/person.rb
index e138b466a34..4d6f8ac79de 100644
--- a/lib/gitlab/ldap/person.rb
+++ b/lib/gitlab/ldap/person.rb
@@ -21,6 +21,15 @@ module Gitlab
         adapter.dn_matches_filter?(dn, AD_USER_DISABLED)
       end
 
+      def self.ldap_attributes(config)
+        [
+          'dn', # Used in `dn`
+          config.uid, # Used in `uid`
+          *config.attributes['name'], # Used in `name`
+          *config.attributes['email'] # Used in `email`
+        ]
+      end
+
       def initialize(entry, provider)
         Rails.logger.debug { "Instantiating #{self.class.name} with LDIF:\n#{entry.to_ldif}" }
         @entry = entry
diff --git a/spec/lib/gitlab/ldap/adapter_spec.rb b/spec/lib/gitlab/ldap/adapter_spec.rb
index d17d440d833..d9ddb4326be 100644
--- a/spec/lib/gitlab/ldap/adapter_spec.rb
+++ b/spec/lib/gitlab/ldap/adapter_spec.rb
@@ -16,7 +16,7 @@ describe Gitlab::LDAP::Adapter do
       expect(adapter).to receive(:ldap_search) do |arg|
         expect(arg[:filter].to_s).to eq('(uid=johndoe)')
         expect(arg[:base]).to eq('dc=example,dc=com')
-        expect(arg[:attributes]).to match(%w{uid cn dn uid userid sAMAccountName mail email userPrincipalName})
+        expect(arg[:attributes]).to match(%w{dn uid cn mail email userPrincipalName})
       end.and_return({})
 
       adapter.users('uid', 'johndoe')
@@ -26,7 +26,7 @@ describe Gitlab::LDAP::Adapter do
       expect(adapter).to receive(:ldap_search).with(
         base: 'uid=johndoe,ou=users,dc=example,dc=com',
         scope: Net::LDAP::SearchScope_BaseObject,
-        attributes: %w{uid cn dn uid userid sAMAccountName mail email userPrincipalName},
+        attributes: %w{dn uid cn mail email userPrincipalName},
         filter: nil
       ).and_return({})
 
@@ -63,7 +63,7 @@ describe Gitlab::LDAP::Adapter do
     it 'uses the right uid attribute when non-default' do
       stub_ldap_config(uid: 'sAMAccountName')
       expect(adapter).to receive(:ldap_search).with(
-        hash_including(attributes: %w{sAMAccountName cn dn uid userid sAMAccountName mail email userPrincipalName})
+        hash_including(attributes: %w{dn sAMAccountName cn mail email userPrincipalName})
       ).and_return({})
 
       adapter.users('sAMAccountName', 'johndoe')