Trim extraneous spaces from DNs

2 files changed, 44 insertions, 1 deletions

diff --git a/lib/gitlab/ldap/auth_hash.rb b/lib/gitlab/ldap/auth_hash.rb
index 4fbc5fa5262..3123da17fd9 100644
--- a/lib/gitlab/ldap/auth_hash.rb
+++ b/lib/gitlab/ldap/auth_hash.rb
@@ -3,6 +3,10 @@
 module Gitlab
   module LDAP
     class AuthHash < Gitlab::OAuth::AuthHash
+      def uid
+        Gitlab::LDAP::Person.normalize_dn(super)
+      end
+
       private
 
       def get_info(key)
diff --git a/lib/gitlab/ldap/person.rb b/lib/gitlab/ldap/person.rb
index 9a6f7827b16..4299d35fabc 100644
--- a/lib/gitlab/ldap/person.rb
+++ b/lib/gitlab/ldap/person.rb
@@ -36,6 +36,12 @@ module Gitlab
         ]
       end
 
+      def self.normalize_dn(dn)
+        dn.split(/([,+=])/).map do |part|
+          normalize_dn_part(part)
+        end.join('')
+      end
+
       def initialize(entry, provider)
         Rails.logger.debug { "Instantiating #{self.class.name} with LDIF:\n#{entry.to_ldif}" }
         @entry = entry
@@ -58,10 +64,43 @@ module Gitlab
         attribute_value(:email)
       end
 
-      delegate :dn, to: :entry
+      def dn
+        self.class.normalize_dn(entry.dn)
+      end
 
       private
 
+      def self.normalize_dn_part(part)
+        cleaned = part.strip
+
+        if cleaned.ends_with?('\\')
+          # If it ends with an escape character that is not followed by a
+          # character to be escaped, then this part may be malformed. But let's
+          # not worry too much about it, and just return it unmodified.
+          #
+          # Why? Because the reason we clean DNs is to make our simplistic
+          # string comparisons work better, even though there are all kinds of
+          # ways that equivalent DNs can vary as strings. If we run into a
+          # strange DN, we should just try to work with it.
+          #
+          # See https://www.ldap.com/ldap-dns-and-rdns for more.
+          return part unless part.ends_with?(' ')
+
+          # Ends with an escaped space (which is valid).
+          cleaned = cleaned + ' '
+        end
+
+        # Get rid of blanks. This can happen if a split character is followed by
+        # whitespace and then another split character.
+        #
+        # E.g. this DN: 'uid=john+telephoneNumber= +1 555-555-5555'
+        #
+        # Should be returned as: 'uid=john+telephoneNumber=+1 555-555-5555'
+        cleaned = '' if cleaned.blank?
+
+        cleaned
+      end
+
       def entry
         @entry
       end