Gitlab::LDAP::Person uses LDAP attributes configuration

We allow users to configure LDAP attribute preferences. For example,
email can be configured to use `mail`, `email` and `userPrincipalName`,
falling through to the next until a value is found. Prior to this
change, Gitlab::LDAP::Person did not honor this configuration. Now,
the class will honor `name` and `mail` configuration. It does not
handle `username`, or fallback to `first_name` + `last_name` in the
absence of `name`.

3 files changed, 65 insertions, 2 deletions

diff --git a/changelogs/unreleased/ldap_person_attributes.yml b/changelogs/unreleased/ldap_person_attributes.yml
new file mode 100644
index 00000000000..d04b5dbe7e0
--- /dev/null
+++ b/changelogs/unreleased/ldap_person_attributes.yml
@@ -0,0 +1,4 @@
+---
+title: Gitlab::LDAP::Person uses LDAP attributes configuration
+merge_request: 8418
+author:
diff --git a/lib/gitlab/ldap/person.rb b/lib/gitlab/ldap/person.rb
index b81f3e8e8f5..333f170a484 100644
--- a/lib/gitlab/ldap/person.rb
+++ b/lib/gitlab/ldap/person.rb
@@ -28,7 +28,7 @@ module Gitlab
       end
 
       def name
-        entry.cn.first
+        attribute_value(:name)
       end
 
       def uid
@@ -40,7 +40,7 @@ module Gitlab
       end
 
       def email
-        entry.try(:mail)
+        attribute_value(:email)
       end
 
       def dn
@@ -56,6 +56,21 @@ module Gitlab
       def config
         @config ||= Gitlab::LDAP::Config.new(provider)
       end
+
+      # Using the LDAP attributes configuration, find and return the first
+      # attribute with a value. For example, by default, when given 'email',
+      # this method looks for 'mail', 'email' and 'userPrincipalName' and
+      # returns the first with a value.
+      def attribute_value(attribute)
+        attributes = Array(config.attributes[attribute.to_sym])
+        selected_attr = attributes.find { |attr| entry.respond_to?(attr) }
+
+        return nil unless selected_attr
+
+        # Some LDAP attributes return an array,
+        # even if it is a single value (like 'cn')
+        Array(entry.public_send(selected_attr)).first
+      end
     end
   end
 end
diff --git a/spec/lib/gitlab/ldap/person_spec.rb b/spec/lib/gitlab/ldap/person_spec.rb
new file mode 100644
index 00000000000..60afe046788
--- /dev/null
+++ b/spec/lib/gitlab/ldap/person_spec.rb
@@ -0,0 +1,44 @@
+require 'spec_helper'
+
+describe Gitlab::LDAP::Person do
+  include LdapHelpers
+
+  let(:entry) { ldap_user_entry('john.doe') }
+
+  before do
+    stub_ldap_config(
+      attributes: {
+        name: 'cn',
+        email: %w(mail email userPrincipalName)
+      }
+    )
+  end
+
+  describe '#name' do
+    it 'uses the configured name attribute and handles values as an array' do
+      name = 'John Doe'
+      entry['cn'] = [name]
+      person = Gitlab::LDAP::Person.new(entry, 'ldapmain')
+
+      expect(person.name).to eq(name)
+    end
+  end
+
+  describe '#email' do
+    it 'returns the value of mail, if present' do
+      mail = 'john@example.com'
+      entry['mail'] = mail
+      person = Gitlab::LDAP::Person.new(entry, 'ldapmain')
+
+      expect(person.email).to eq(mail)
+    end
+
+    it 'returns the value of userPrincipalName, if mail and email are not present' do
+      user_principal_name = 'john.doe@example.com'
+      entry['userPrincipalName'] = user_principal_name
+      person = Gitlab::LDAP::Person.new(entry, 'ldapmain')
+
+      expect(person.email).to eq(user_principal_name)
+    end
+  end
+end