Merge branch 'mk-clean-ldap-user-dns' into 'master'

Normalize LDAP user DNs (downcase and remove excess spaces)

Closes #3151

See merge request gitlab-org/gitlab-ee!2942

changelogs/unreleased/mk-normalize-ldap-user-dns.yml

+---
+title: Search or compare LDAP DNs case-insensitively and ignore excess whitespace
+merge_request: 14697
+author:
+type: fixed

db/post_migrate/20170921101004_normalize_ldap_extern_uids.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class NormalizeLdapExternUids < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  MIGRATION = 'NormalizeLdapExternUidsRange'.freeze
+  DELAY_INTERVAL = 10.seconds
+
+  disable_ddl_transaction!
+
+  class Identity < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'identities'
+  end
+
+  def up
+    ldap_identities = Identity.where("provider like 'ldap%'")
+
+    if ldap_identities.any?
+      queue_background_migration_jobs_by_range_at_intervals(Identity, MIGRATION, DELAY_INTERVAL)
+    end
+  end
+
+  def down
+  end
+end

ee/lib/ee/gitlab/ldap/access_levels.rb

@@ -7,10 +7,10 @@ module EE
       class AccessLevels < Hash
         def set(dns, to:)
           dns.each do |dn|
-            current = self[dn.downcase]
+            current = self[dn]
 
             # Keep the higher of the access values.
-            self[dn.downcase] = to if current.nil? || to > current
+            self[dn] = to if current.nil? || to > current
           end
         end
       end

ee/lib/ee/gitlab/ldap/group.rb

@@ -36,7 +36,9 @@ module EE
         end
 
         def member_uids
-          entry.memberuid
+          @member_uids ||= entry.memberuid.map do |uid|
+            ::Gitlab::LDAP::Person.normalize_uid(uid)
+          end
         end
 
         delegate :dn, to: :entry
@@ -48,17 +50,7 @@ module EE
             dns.concat(active_directory_members(entry, nested_groups_to_skip))
           end
 
-          if (entry.respond_to? :member) && (entry.respond_to? :submember)
-            dns.concat(entry.member + entry.submember)
-          elsif entry.respond_to? :member
-            dns.concat(entry.member)
-          elsif entry.respond_to? :uniquemember
-            dns.concat(entry.uniquemember)
-          elsif entry.respond_to? :memberof
-            dns.concat(entry.memberof)
-          else
-            Rails.logger.warn("Could not find member DNs for LDAP group #{entry.inspect}")
-          end
+          dns.concat(entry_member_dns(entry))
 
           dns.uniq
         end
@@ -103,7 +95,9 @@ module EE
           members = []
 
           # Concatenate the members in the current range
-          members.concat(entry[member_range_attribute(entry)])
+          dns = entry[member_range_attribute(entry)]
+          dns = normalize_dns(dns)
+          members.concat(dns)
 
           # Recursively concatenate members until end of ranges
           if has_more_member_ranges?(entry)
@@ -161,20 +155,38 @@ module EE
         # the user DN match, profit!
         def members_within_base(members)
           begin
-            base = Net::LDAP::DN.new(adapter.config.base.downcase).to_a
-          rescue RuntimeError
-            Rails.logger.error "Configured LDAP `base` is invalid: '#{adapter.config.base}'"
+            base = ::Gitlab::LDAP::DN.new(adapter.config.base).to_a
+          rescue ::Gitlab::LDAP::DN::FormatError => e
+            Rails.logger.error "Configured LDAP `base` is invalid: '#{adapter.config.base}'. Error: \"#{e.message}\""
             return []
           end
 
           members.select do |dn|
             begin
-              Net::LDAP::DN.new(dn.downcase).to_a.last(base.length) == base
-            rescue RuntimeError
-              Rails.logger.warn "Received invalid member DN from LDAP group '#{cn}': '#{dn}'. Skipping"
+              ::Gitlab::LDAP::DN.new(dn).to_a.last(base.length) == base
+            rescue ::Gitlab::LDAP::DN::FormatError => e
+              Rails.logger.warn "Received invalid member DN from LDAP group '#{cn}': '#{dn}'. Error: \"#{e.message}\". Skipping"
             end
           end
         end
+
+        def normalize_dns(dns)
+          dns.map do |dn|
+            ::Gitlab::LDAP::Person.normalize_dn(dn)
+          end
+        end
+
+        def entry_member_dns(entry)
+          dns = entry.try(:member) || entry.try(:uniquemember) || entry.try(:memberof)
+          dns&.concat(entry.try(:submember) || [])
+
+          if dns
+            normalize_dns(dns)
+          else
+            Rails.logger.warn("Could not find member DNs for LDAP group #{entry.inspect}")
+            []
+          end
+        end
       end
     end
   end

ee/lib/ee/gitlab/ldap/person.rb

@@ -34,7 +34,7 @@ module EE
           # LDAP DN and constructs a domain name from them
           def domain_from_dn(dn)
             dn_components = []
-            Net::LDAP::DN.new(dn).each_pair { |name, value| dn_components << { name: name, value: value } }
+            ::Gitlab::LDAP::DN.new(dn).each_pair { |name, value| dn_components << { name: name, value: value } }
             dn_components
               .reverse
               .take_while { |rdn| rdn[:name].casecmp('DC').zero? } # Domain Component

ee/lib/ee/gitlab/ldap/sync/proxy.rb

@@ -69,10 +69,9 @@ module EE
           def ensure_full_dns!(dns)
             dns.map! do |dn|
               begin
-                parsed_dn = Net::LDAP::DN.new(dn).to_a
-              rescue RuntimeError => e
-                # Net::LDAP raises a generic RuntimeError. Bad library! Bad!
-                logger.error { "Found malformed DN: '#{dn}'. Skipping. #{e.message}" }
+                parsed_dn = ::Gitlab::LDAP::DN.new(dn).to_a
+              rescue ::Gitlab::LDAP::DN::FormatError => e
+                logger.error { "Found malformed DN: '#{dn}'. Skipping. Error: \"#{e.message}\"" }
                 next
               end
 
@@ -81,6 +80,9 @@ module EE
                 # or at least the probability is higher.
                 if parsed_dn.count > 2
                   dn
+                elsif parsed_dn.count == 0
+                  logger.warn { "Found null DN. Skipping." }
+                  nil
                 elsif parsed_dn[0] == 'uid'
                   dn_for_uid(parsed_dn[1])
                 else

lib/gitlab/background_migration/normalize_ldap_extern_uids_range.rb

+module Gitlab
+  module BackgroundMigration
+    class NormalizeLdapExternUidsRange
+      class Identity < ActiveRecord::Base
+        self.table_name = 'identities'
+      end
+
+      # Copied this class to make this migration resilient to future code changes.
+      # And if the normalize behavior is changed in the future, it must be
+      # accompanied by another migration.
+      module Gitlab
+        module LDAP
+          class DN
+            FormatError = Class.new(StandardError)
+            MalformedError = Class.new(FormatError)
+            UnsupportedError = Class.new(FormatError)
+
+            def self.normalize_value(given_value)
+              dummy_dn = "placeholder=#{given_value}"
+              normalized_dn = new(*dummy_dn).to_normalized_s
+              normalized_dn.sub(/\Aplaceholder=/, '')
+            end
+
+            ##
+            # Initialize a DN, escaping as required. Pass in attributes in name/value
+            # pairs. If there is a left over argument, it will be appended to the dn
+            # without escaping (useful for a base string).
+            #
+            # Most uses of this class will be to escape a DN, rather than to parse it,
+            # so storing the dn as an escaped String and parsing parts as required
+            # with a state machine seems sensible.
+            def initialize(*args)
+              if args.length > 1
+                initialize_array(args)
+              else
+                initialize_string(args[0])
+              end
+            end
+
+            ##
+            # Parse a DN into key value pairs using ASN from
+            # http://tools.ietf.org/html/rfc2253 section 3.
+            # rubocop:disable Metrics/AbcSize
+            # rubocop:disable Metrics/CyclomaticComplexity
+            # rubocop:disable Metrics/PerceivedComplexity
+            def each_pair
+              state = :key
+              key = StringIO.new
+              value = StringIO.new
+              hex_buffer = ""
+
+              @dn.each_char.with_index do |char, dn_index|
+                case state
+                when :key then
+                  case char
+                  when 'a'..'z', 'A'..'Z' then
+                    state = :key_normal
+                    key << char
+                  when '0'..'9' then
+                    state = :key_oid
+                    key << char
+                  when ' ' then state = :key
+                  else raise(MalformedError, "Unrecognized first character of an RDN attribute type name \"#{char}\"")
+                  end
+                when :key_normal then
+                  case char
+                  when '=' then state = :value
+                  when 'a'..'z', 'A'..'Z', '0'..'9', '-', ' ' then key << char
+                  else raise(MalformedError, "Unrecognized RDN attribute type name character \"#{char}\"")
+                  end
+                when :key_oid then
+                  case char
+                  when '=' then state = :value
+                  when '0'..'9', '.', ' ' then key << char
+                  else raise(MalformedError, "Unrecognized RDN OID attribute type name character \"#{char}\"")
+                  end
+                when :value then
+                  case char
+                  when '\\' then state = :value_normal_escape
+                  when '"' then state = :value_quoted
+                  when ' ' then state = :value
+                  when '#' then
+                    state = :value_hexstring
+                    value << char
+                  when ',' then
+                    state = :key
+                    yield key.string.strip, rstrip_except_escaped(value.string, dn_index)
+                    key = StringIO.new
+                    value = StringIO.new
+                  else
+                    state = :value_normal
+                    value << char
+                  end
+                when :value_normal then
+                  case char
+                  when '\\' then state = :value_normal_escape
+                  when ',' then
+                    state = :key
+                    yield key.string.strip, rstrip_except_escaped(value.string, dn_index)
+                    key = StringIO.new
+                    value = StringIO.new
+                  when '+' then raise(UnsupportedError, "Multivalued RDNs are not supported")
+                  else value << char
+                  end
+                when :value_normal_escape then
+                  case char
+                  when '0'..'9', 'a'..'f', 'A'..'F' then
+                    state = :value_normal_escape_hex
+                    hex_buffer = char
+                  else
+                    state = :value_normal
+                    value << char
+                  end
+                when :value_normal_escape_hex then
+                  case char
+                  when '0'..'9', 'a'..'f', 'A'..'F' then
+                    state = :value_normal
+                    value << "#{hex_buffer}#{char}".to_i(16).chr
+                  else raise(MalformedError, "Invalid escaped hex code \"\\#{hex_buffer}#{char}\"")
+                  end
+                when :value_quoted then
+                  case char
+                  when '\\' then state = :value_quoted_escape
+                  when '"' then state = :value_end
+                  else value << char
+                  end
+                when :value_quoted_escape then
+                  case char
+                  when '0'..'9', 'a'..'f', 'A'..'F' then
+                    state = :value_quoted_escape_hex
+                    hex_buffer = char
+                  else
+                    state = :value_quoted
+                    value << char
+                  end
+                when :value_quoted_escape_hex then
+                  case char
+                  when '0'..'9', 'a'..'f', 'A'..'F' then
+                    state = :value_quoted
+                    value << "#{hex_buffer}#{char}".to_i(16).chr
+                  else raise(MalformedError, "Expected the second character of a hex pair inside a double quoted value, but got \"#{char}\"")
+                  end
+                when :value_hexstring then
+                  case char
+                  when '0'..'9', 'a'..'f', 'A'..'F' then
+                    state = :value_hexstring_hex
+                    value << char
+                  when ' ' then state = :value_end
+                  when ',' then
+                    state = :key
+                    yield key.string.strip, rstrip_except_escaped(value.string, dn_index)
+                    key = StringIO.new
+                    value = StringIO.new
+                  else raise(MalformedError, "Expected the first character of a hex pair, but got \"#{char}\"")
+                  end
+                when :value_hexstring_hex then
+                  case char
+                  when '0'..'9', 'a'..'f', 'A'..'F' then
+                    state = :value_hexstring
+                    value << char
+                  else raise(MalformedError, "Expected the second character of a hex pair, but got \"#{char}\"")
+                  end
+                when :value_end then
+                  case char
+                  when ' ' then state = :value_end
+                  when ',' then
+                    state = :key
+                    yield key.string.strip, rstrip_except_escaped(value.string, dn_index)
+                    key = StringIO.new
+                    value = StringIO.new
+                  else raise(MalformedError, "Expected the end of an attribute value, but got \"#{char}\"")
+                  end
+                else raise "Fell out of state machine"
+                end
+              end
+
+              # Last pair
+              raise(MalformedError, 'DN string ended unexpectedly') unless
+                [:value, :value_normal, :value_hexstring, :value_end].include? state
+
+              yield key.string.strip, rstrip_except_escaped(value.string, @dn.length)
+            end
+
+            def rstrip_except_escaped(str, dn_index)
+              str_ends_with_whitespace = str.match(/\s\z/)
+
+              if str_ends_with_whitespace
+                dn_part_ends_with_escaped_whitespace = @dn[0, dn_index].match(/\\(\s+)\z/)
+
+                if dn_part_ends_with_escaped_whitespace
+                  dn_part_rwhitespace = dn_part_ends_with_escaped_whitespace[1]
+                  num_chars_to_remove = dn_part_rwhitespace.length - 1
+                  str = str[0, str.length - num_chars_to_remove]
+                else
+                  str.rstrip!
+                end
+              end
+
+              str
+            end
+
+            ##
+            # Returns the DN as an array in the form expected by the constructor.
+            def to_a
+              a = []
+              self.each_pair { |key, value| a << key << value } unless @dn.empty?
+              a
+            end
+
+            ##
+            # Return the DN as an escaped string.
+            def to_s
+              @dn
+            end
+
+            ##
+            # Return the DN as an escaped and normalized string.
+            def to_normalized_s
+              self.class.new(*to_a).to_s.downcase
+            end
+
+            # https://tools.ietf.org/html/rfc4514 section 2.4 lists these exceptions
+            # for DN values. All of the following must be escaped in any normal string
+            # using a single backslash ('\') as escape. The space character is left
+            # out here because in a "normalized" string, spaces should only be escaped
+            # if necessary (i.e. leading or trailing space).
+            NORMAL_ESCAPES = [',', '+', '"', '\\', '<', '>', ';', '='].freeze
+
+            # The following must be represented as escaped hex
+            HEX_ESCAPES = {
+              "\n" => '\0a',
+              "\r" => '\0d'
+            }.freeze
+
+            # Compiled character class regexp using the keys from the above hash, and
+            # checking for a space or # at the start, or space at the end, of the
+            # string.
+            ESCAPE_RE = Regexp.new("(^ |^#| $|[" +
+                                   NORMAL_ESCAPES.map { |e| Regexp.escape(e) }.join +
+                                   "])")
+
+            HEX_ESCAPE_RE = Regexp.new("([" +
+                                   HEX_ESCAPES.keys.map { |e| Regexp.escape(e) }.join +
+                                   "])")
+
+            ##
+            # Escape a string for use in a DN value
+            def self.escape(string)
+              escaped = string.gsub(ESCAPE_RE) { |char| "\\" + char }
+              escaped.gsub(HEX_ESCAPE_RE) { |char| HEX_ESCAPES[char] }
+            end
+
+            private
+
+            def initialize_array(args)
+              buffer = StringIO.new
+
+              args.each_with_index do |arg, index|
+                if index.even? # key
+                  buffer << "," if index > 0
+                  buffer << arg
+                else # value
+                  buffer << "="
+                  buffer << self.class.escape(arg)
+                end
+              end
+
+              @dn = buffer.string
+            end
+
+            def initialize_string(arg)
+              @dn = arg.to_s
+            end
+
+            ##
+            # Proxy all other requests to the string object, because a DN is mainly
+            # used within the library as a string
+            # rubocop:disable GitlabSecurity/PublicSend
+            def method_missing(method, *args, &block)
+              @dn.send(method, *args, &block)
+            end
+
+            ##
+            # Redefined to be consistent with redefined `method_missing` behavior
+            def respond_to?(sym, include_private = false)
+              @dn.respond_to?(sym, include_private)
+            end
+          end
+        end
+      end
+
+      def perform(start_id, end_id)
+        return unless migrate?
+
+        ldap_identities = Identity.where("provider like 'ldap%'").where(id: start_id..end_id)
+        ldap_identities.each do |identity|
+          begin
+            identity.extern_uid = Gitlab::LDAP::DN.new(identity.extern_uid).to_normalized_s
+            unless identity.save
+              Rails.logger.info "Unable to normalize \"#{identity.extern_uid}\". Skipping."
+            end
+          rescue Gitlab::LDAP::DN::FormatError => e
+            Rails.logger.info "Unable to normalize \"#{identity.extern_uid}\" due to \"#{e.message}\". Skipping."
+          end
+        end
+      end
+
+      def migrate?
+        Identity.table_exists?
+      end
+    end
+  end
+end

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)

lib/gitlab/ldap/dn.rb

+# -*- ruby encoding: utf-8 -*-
+
+# Based on the `ruby-net-ldap` gem's `Net::LDAP::DN`
+#
+# For our purposes, this class is used to normalize DNs in order to allow proper
+# comparison.
+#
+# E.g. DNs should be compared case-insensitively (in basically all LDAP
+# implementations or setups), therefore we downcase every DN.
+
+##
+# Objects of this class represent an LDAP DN ("Distinguished Name"). A DN
+# ("Distinguished Name") is a unique identifier for an entry within an LDAP
+# directory. It is made up of a number of other attributes strung together,
+# to identify the entry in the tree.
+#
+# Each attribute that makes up a DN needs to have its value escaped so that
+# the DN is valid. This class helps take care of that.
+#
+# A fully escaped DN needs to be unescaped when analysing its contents. This
+# class also helps take care of that.
+module Gitlab
+  module LDAP
+    class DN
+      FormatError = Class.new(StandardError)
+      MalformedError = Class.new(FormatError)
+      UnsupportedError = Class.new(FormatError)
+
+      def self.normalize_value(given_value)
+        dummy_dn = "placeholder=#{given_value}"
+        normalized_dn = new(*dummy_dn).to_normalized_s
+        normalized_dn.sub(/\Aplaceholder=/, '')
+      end
+
+      ##
+      # Initialize a DN, escaping as required. Pass in attributes in name/value
+      # pairs. If there is a left over argument, it will be appended to the dn
+      # without escaping (useful for a base string).
+      #
+      # Most uses of this class will be to escape a DN, rather than to parse it,
+      # so storing the dn as an escaped String and parsing parts as required
+      # with a state machine seems sensible.
+      def initialize(*args)
+        if args.length > 1
+          initialize_array(args)
+        else
+          initialize_string(args[0])
+        end
+      end
+
+      ##
+      # Parse a DN into key value pairs using ASN from
+      # http://tools.ietf.org/html/rfc2253 section 3.
+      # rubocop:disable Metrics/AbcSize
+      # rubocop:disable Metrics/CyclomaticComplexity
+      # rubocop:disable Metrics/PerceivedComplexity
+      def each_pair
+        state = :key
+        key = StringIO.new
+        value = StringIO.new
+        hex_buffer = ""
+
+        @dn.each_char.with_index do |char, dn_index|
+          case state
+          when :key then
+            case char
+            when 'a'..'z', 'A'..'Z' then
+              state = :key_normal
+              key << char
+            when '0'..'9' then
+              state = :key_oid
+              key << char
+            when ' ' then state = :key
+            else raise(MalformedError, "Unrecognized first character of an RDN attribute type name \"#{char}\"")
+            end
+          when :key_normal then
+            case char
+            when '=' then state = :value
+            when 'a'..'z', 'A'..'Z', '0'..'9', '-', ' ' then key << char
+            else raise(MalformedError, "Unrecognized RDN attribute type name character \"#{char}\"")
+            end
+          when :key_oid then
+            case char
+            when '=' then state = :value
+            when '0'..'9', '.', ' ' then key << char
+            else raise(MalformedError, "Unrecognized RDN OID attribute type name character \"#{char}\"")
+            end
+          when :value then
+            case char
+            when '\\' then state = :value_normal_escape
+            when '"' then state = :value_quoted
+            when ' ' then state = :value
+            when '#' then
+              state = :value_hexstring
+              value << char
+            when ',' then
+              state = :key
+              yield key.string.strip, rstrip_except_escaped(value.string, dn_index)
+              key = StringIO.new
+              value = StringIO.new
+            else
+              state = :value_normal
+              value << char
+            end
+          when :value_normal then
+            case char
+            when '\\' then state = :value_normal_escape
+            when ',' then
+              state = :key
+              yield key.string.strip, rstrip_except_escaped(value.string, dn_index)
+              key = StringIO.new
+              value = StringIO.new
+            when '+' then raise(UnsupportedError, "Multivalued RDNs are not supported")
+            else value << char
+            end
+          when :value_normal_escape then
+            case char
+            when '0'..'9', 'a'..'f', 'A'..'F' then
+              state = :value_normal_escape_hex
+              hex_buffer = char
+            else
+              state = :value_normal
+              value << char
+            end
+          when :value_normal_escape_hex then
+            case char
+            when '0'..'9', 'a'..'f', 'A'..'F' then
+              state = :value_normal
+              value << "#{hex_buffer}#{char}".to_i(16).chr
+            else raise(MalformedError, "Invalid escaped hex code \"\\#{hex_buffer}#{char}\"")
+            end
+          when :value_quoted then
+            case char
+            when '\\' then state = :value_quoted_escape
+            when '"' then state = :value_end
+            else value << char
+            end
+          when :value_quoted_escape then
+            case char
+            when '0'..'9', 'a'..'f', 'A'..'F' then
+              state = :value_quoted_escape_hex
+              hex_buffer = char
+            else
+              state = :value_quoted
+              value << char
+            end
+          when :value_quoted_escape_hex then
+            case char
+            when '0'..'9', 'a'..'f', 'A'..'F' then
+              state = :value_quoted
+              value << "#{hex_buffer}#{char}".to_i(16).chr
+            else raise(MalformedError, "Expected the second character of a hex pair inside a double quoted value, but got \"#{char}\"")
+            end
+          when :value_hexstring then
+            case char
+            when '0'..'9', 'a'..'f', 'A'..'F' then
+              state = :value_hexstring_hex
+              value << char
+            when ' ' then state = :value_end
+            when ',' then
+              state = :key
+              yield key.string.strip, rstrip_except_escaped(value.string, dn_index)
+              key = StringIO.new
+              value = StringIO.new
+            else raise(MalformedError, "Expected the first character of a hex pair, but got \"#{char}\"")
+            end
+          when :value_hexstring_hex then
+            case char
+            when '0'..'9', 'a'..'f', 'A'..'F' then
+              state = :value_hexstring
+              value << char
+            else raise(MalformedError, "Expected the second character of a hex pair, but got \"#{char}\"")
+            end
+          when :value_end then
+            case char
+            when ' ' then state = :value_end
+            when ',' then
+              state = :key
+              yield key.string.strip, rstrip_except_escaped(value.string, dn_index)
+              key = StringIO.new
+              value = StringIO.new
+            else raise(MalformedError, "Expected the end of an attribute value, but got \"#{char}\"")
+            end
+          else raise "Fell out of state machine"
+          end
+        end
+
+        # Last pair
+        raise(MalformedError, 'DN string ended unexpectedly') unless
+          [:value, :value_normal, :value_hexstring, :value_end].include? state
+
+        yield key.string.strip, rstrip_except_escaped(value.string, @dn.length)
+      end
+
+      def rstrip_except_escaped(str, dn_index)
+        str_ends_with_whitespace = str.match(/\s\z/)
+
+        if str_ends_with_whitespace
+          dn_part_ends_with_escaped_whitespace = @dn[0, dn_index].match(/\\(\s+)\z/)
+
+          if dn_part_ends_with_escaped_whitespace
+            dn_part_rwhitespace = dn_part_ends_with_escaped_whitespace[1]
+            num_chars_to_remove = dn_part_rwhitespace.length - 1
+            str = str[0, str.length - num_chars_to_remove]
+          else
+            str.rstrip!
+          end
+        end
+
+        str
+      end
+
+      ##
+      # Returns the DN as an array in the form expected by the constructor.
+      def to_a
+        a = []
+        self.each_pair { |key, value| a << key << value } unless @dn.empty?
+        a
+      end
+
+      ##
+      # Return the DN as an escaped string.
+      def to_s
+        @dn
+      end
+
+      ##
+      # Return the DN as an escaped and normalized string.
+      def to_normalized_s
+        self.class.new(*to_a).to_s.downcase
+      end
+
+      # https://tools.ietf.org/html/rfc4514 section 2.4 lists these exceptions
+      # for DN values. All of the following must be escaped in any normal string
+      # using a single backslash ('\') as escape. The space character is left
+      # out here because in a "normalized" string, spaces should only be escaped
+      # if necessary (i.e. leading or trailing space).
+      NORMAL_ESCAPES = [',', '+', '"', '\\', '<', '>', ';', '='].freeze
+
+      # The following must be represented as escaped hex
+      HEX_ESCAPES = {
+        "\n" => '\0a',
+        "\r" => '\0d'
+      }.freeze
+
+      # Compiled character class regexp using the keys from the above hash, and
+      # checking for a space or # at the start, or space at the end, of the
+      # string.
+      ESCAPE_RE = Regexp.new("(^ |^#| $|[" +
+                             NORMAL_ESCAPES.map { |e| Regexp.escape(e) }.join +
+                             "])")
+
+      HEX_ESCAPE_RE = Regexp.new("([" +
+                             HEX_ESCAPES.keys.map { |e| Regexp.escape(e) }.join +
+                             "])")
+
+      ##
+      # Escape a string for use in a DN value
+      def self.escape(string)
+        escaped = string.gsub(ESCAPE_RE) { |char| "\\" + char }
+        escaped.gsub(HEX_ESCAPE_RE) { |char| HEX_ESCAPES[char] }
+      end
+
+      private
+
+      def initialize_array(args)
+        buffer = StringIO.new
+
+        args.each_with_index do |arg, index|
+          if index.even? # key
+            buffer << "," if index > 0
+            buffer << arg
+          else # value
+            buffer << "="
+            buffer << self.class.escape(arg)
+          end
+        end
+
+        @dn = buffer.string
+      end
+
+      def initialize_string(arg)
+        @dn = arg.to_s
+      end
+
+      ##
+      # Proxy all other requests to the string object, because a DN is mainly
+      # used within the library as a string
+      # rubocop:disable GitlabSecurity/PublicSend
+      def method_missing(method, *args, &block)
+        @dn.send(method, *args, &block)
+      end
+
+      ##
+      # Redefined to be consistent with redefined `method_missing` behavior
+      def respond_to?(sym, include_private = false)
+        @dn.respond_to?(sym, include_private)
+      end
+    end
+  end
+end

lib/gitlab/ldap/person.rb

@@ -40,6 +40,26 @@ module Gitlab
         ]
       end
 
+      def self.normalize_dn(dn)
+        ::Gitlab::LDAP::DN.new(dn).to_normalized_s
+      rescue ::Gitlab::LDAP::DN::FormatError => e
+        Rails.logger.info("Returning original DN \"#{dn}\" due to error during normalization attempt: #{e.message}")
+
+        dn
+      end
+
+      # Returns the UID in a normalized form.
+      #
+      # 1. Excess spaces are stripped
+      # 2. The string is downcased (for case-insensitivity)
+      def self.normalize_uid(uid)
+        ::Gitlab::LDAP::DN.normalize_value(uid)
+      rescue ::Gitlab::LDAP::DN::FormatError => e
+        Rails.logger.info("Returning original UID \"#{uid}\" due to error during normalization attempt: #{e.message}")
+
+        uid
+      end
+
       def initialize(entry, provider)
         Rails.logger.debug { "Instantiating #{self.class.name} with LDIF:\n#{entry.to_ldif}" }
         @entry = entry
@@ -62,7 +82,9 @@ module Gitlab
         attribute_value(:email)
       end
 
-      delegate :dn, to: :entry
+      def dn
+        self.class.normalize_dn(entry.dn)
+      end
 
       private
 

spec/ee/spec/lib/ee/gitlab/ldap/adapter_spec.rb

@@ -29,14 +29,14 @@ describe Gitlab::LDAP::Adapter do
     end
 
     it 'returns a group object if search returns a result' do
-      entry = ldap_group_entry(%w(john mary), cn: 'group1')
+      entry = ldap_group_entry(%w(uid=john uid=mary), cn: 'group1')
       allow(adapter).to receive(:ldap_search).and_return([entry])
 
       results = adapter.groups('group1')
 
       expect(results.first).to be_a(EE::Gitlab::LDAP::Group)
       expect(results.first.cn).to eq('group1')
-      expect(results.first.member_dns).to match_array(%w(john mary))
+      expect(results.first.member_dns).to match_array(%w(uid=john uid=mary))
     end
   end
 end

spec/ee/spec/lib/ee/gitlab/ldap/group_spec.rb

@@ -120,7 +120,7 @@ describe EE::Gitlab::LDAP::Group do
 
         expect(group.member_dns).not_to include('cn=ldap_group1,ou=groups,dc=example,dc=com')
         expect(group.member_dns).not_to include('uid=foo,ou=users,dc=other,dc=com')
-        expect(group.member_dns).to include('uid=bar,ou=users,dc=example , dc=com')
+        expect(group.member_dns).to include('uid=bar,ou=users,dc=example,dc=com')
       end
 
       it 'logs an error when the LDAP base is invalid' do
@@ -133,7 +133,7 @@ describe EE::Gitlab::LDAP::Group do
         stub_ldap_adapter_nested_groups(group2_entry.dn, [], adapter)
 
         expect(Rails.logger)
-          .to receive(:error).with("Configured LDAP `base` is invalid: 'invalid,dc=example,dc=com'")
+          .to receive(:error).with(/Configured LDAP `base` is invalid: 'invalid,dc=example,dc=com'/)
         # Users in the top-level group always get added - they're not filtered
         # through the nested groups shenanigans.
         expect(group.member_dns).to match_array(
@@ -157,10 +157,35 @@ describe EE::Gitlab::LDAP::Group do
         stub_ldap_adapter_nested_groups(group2_entry.dn, [], adapter)
         stub_ldap_adapter_nested_groups(group3_entry.dn, [], adapter)
 
+        expect(Rails.logger)
+          .to receive(:info).with(/Returning original DN/)
         expect(Rails.logger)
           .to receive(:warn).with(/Received invalid member/)
         expect(group.member_dns).not_to include('invalid,ou=user,ou=groups,dc=example,dc=com')
       end
     end
+
+    it 'removes extraneous spaces from DNs' do
+      group_entry_page1 = ldap_group_entry_with_member_range(
+        [' uid =  user1   ,  ou = users,dc=example,dc=com'],
+        range_start: '0',
+        range_end: '0'
+      )
+      group_entry_page2 = ldap_group_entry_with_member_range(
+        [' uid =user2,  ou = users, dc  = example, dc=com  '],
+        range_start: '1',
+        range_end: '*'
+      )
+      group = described_class.new(group_entry_page1, adapter)
+      stub_ldap_adapter_group_members_in_range(group_entry_page2, adapter, range_start: '1')
+      stub_ldap_adapter_nested_groups(group.dn, [], adapter)
+
+      expect(group.member_dns).to match_array(
+        %w(
+          uid=user1,ou=users,dc=example,dc=com
+          uid=user2,ou=users,dc=example,dc=com
+        )
+      )
+    end
   end
 end

spec/ee/spec/lib/ee/gitlab/ldap/sync/proxy_spec.rb

@@ -33,7 +33,7 @@ describe EE::Gitlab::LDAP::Sync::Proxy do
         ldap_group = ldap_group_entry(dns)
         stub_ldap_group_find_by_cn('ldap_group1', ldap_group, adapter)
 
-        expect(sync_proxy.dns_for_group_cn('ldap_group1').first).to include("uid=Méräy")
+        expect(sync_proxy.dns_for_group_cn('ldap_group1').first).to include("uid=méräy")
       end
     end
 

spec/lib/gitlab/background_migration/normalize_ldap_extern_uids_range_spec.rb

+require 'spec_helper'
+
+describe Gitlab::BackgroundMigration::NormalizeLdapExternUidsRange, :migration, schema: 20170921101004 do
+  let!(:identities) { table(:identities) }
+
+  before do
+    # LDAP identities
+    (1..4).each do |i|
+      identities.create!(id: i, provider: 'ldapmain', extern_uid: " uid = foo #{i}, ou = People, dc = example, dc = com ", user_id: i)
+    end
+
+    # Non-LDAP identity
+    identities.create!(id: 5, provider: 'foo', extern_uid: " uid = foo 5, ou = People, dc = example, dc = com ", user_id: 5)
+
+    # Another LDAP identity
+    identities.create!(id: 6, provider: 'ldapmain', extern_uid: " uid = foo 6, ou = People, dc = example, dc = com ", user_id: 6)
+  end
+
+  it 'normalizes the LDAP identities in the range' do
+    described_class.new.perform(1, 3)
+    expect(identities.find(1).extern_uid).to eq("uid=foo 1,ou=people,dc=example,dc=com")
+    expect(identities.find(2).extern_uid).to eq("uid=foo 2,ou=people,dc=example,dc=com")
+    expect(identities.find(3).extern_uid).to eq("uid=foo 3,ou=people,dc=example,dc=com")
+    expect(identities.find(4).extern_uid).to eq(" uid = foo 4, ou = People, dc = example, dc = com ")
+    expect(identities.find(5).extern_uid).to eq(" uid = foo 5, ou = People, dc = example, dc = com ")
+    expect(identities.find(6).extern_uid).to eq(" uid = foo 6, ou = People, dc = example, dc = com ")
+
+    described_class.new.perform(4, 6)
+    expect(identities.find(1).extern_uid).to eq("uid=foo 1,ou=people,dc=example,dc=com")
+    expect(identities.find(2).extern_uid).to eq("uid=foo 2,ou=people,dc=example,dc=com")
+    expect(identities.find(3).extern_uid).to eq("uid=foo 3,ou=people,dc=example,dc=com")
+    expect(identities.find(4).extern_uid).to eq("uid=foo 4,ou=people,dc=example,dc=com")
+    expect(identities.find(5).extern_uid).to eq(" uid = foo 5, ou = People, dc = example, dc = com ")
+    expect(identities.find(6).extern_uid).to eq("uid=foo 6,ou=people,dc=example,dc=com")
+  end
+end

spec/lib/gitlab/ldap/auth_hash_spec.rb

@@ -4,7 +4,7 @@ describe Gitlab::LDAP::AuthHash do
   let(:auth_hash) do
     described_class.new(
       OmniAuth::AuthHash.new(
-        uid: '123456',
+        uid: given_uid,
         provider: 'ldapmain',
         info: info,
         extra: {
@@ -32,6 +32,8 @@ describe Gitlab::LDAP::AuthHash do
   end
 
   context "without overridden attributes" do
+    let(:given_uid) { 'uid=John Smith,ou=People,dc=example,dc=com' }
+
     it "has the correct username" do
       expect(auth_hash.username).to eq("123456")
     end
@@ -42,6 +44,8 @@ describe Gitlab::LDAP::AuthHash do
   end
 
   context "with overridden attributes" do
+    let(:given_uid) { 'uid=John Smith,ou=People,dc=example,dc=com' }
+
     let(:attributes) do
       {
         'username'  => %w(mail email),
@@ -61,4 +65,22 @@ describe Gitlab::LDAP::AuthHash do
       expect(auth_hash.name).to eq("John Smith")
     end
   end
+
+  describe '#uid' do
+    context 'when there is extraneous (but valid) whitespace' do
+      let(:given_uid) { 'uid     =john smith ,  ou = people, dc=  example,dc =com' }
+
+      it 'removes the extraneous whitespace' do
+        expect(auth_hash.uid).to eq('uid=john smith,ou=people,dc=example,dc=com')
+      end
+    end
+
+    context 'when there are upper case characters' do
+      let(:given_uid) { 'UID=John Smith,ou=People,dc=example,dc=com' }
+
+      it 'downcases' do
+        expect(auth_hash.uid).to eq('uid=john smith,ou=people,dc=example,dc=com')
+      end
+    end
+  end
 end

spec/lib/gitlab/ldap/dn_spec.rb

+require 'spec_helper'
+
+describe Gitlab::LDAP::DN do
+  using RSpec::Parameterized::TableSyntax
+
+  describe '#normalize_value' do
+    subject { described_class.normalize_value(given) }
+
+    it_behaves_like 'normalizes a DN attribute value'
+
+    context 'when the given DN is malformed' do
+      context 'when ending with a comma' do
+        let(:given) { 'John Smith,' }
+
+        it 'raises MalformedError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::DN::MalformedError, 'DN string ended unexpectedly')
+        end
+      end
+
+      context 'when given a BER encoded attribute value with a space in it' do
+        let(:given) { '#aa aa' }
+
+        it 'raises MalformedError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::DN::MalformedError, "Expected the end of an attribute value, but got \"a\"")
+        end
+      end
+
+      context 'when given a BER encoded attribute value with a non-hex character in it' do
+        let(:given) { '#aaXaaa' }
+
+        it 'raises MalformedError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::DN::MalformedError, "Expected the first character of a hex pair, but got \"X\"")
+        end
+      end
+
+      context 'when given a BER encoded attribute value with a non-hex character in it' do
+        let(:given) { '#aaaYaa' }
+
+        it 'raises MalformedError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::DN::MalformedError, "Expected the second character of a hex pair, but got \"Y\"")
+        end
+      end
+
+      context 'when given a hex pair with a non-hex character in it, inside double quotes' do
+        let(:given) { '"Sebasti\\cX\\a1n"' }
+
+        it 'raises MalformedError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::DN::MalformedError, "Expected the second character of a hex pair inside a double quoted value, but got \"X\"")
+        end
+      end
+
+      context 'with an open (as opposed to closed) double quote' do
+        let(:given) { '"James' }
+
+        it 'raises MalformedError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::DN::MalformedError, 'DN string ended unexpectedly')
+        end
+      end
+
+      context 'with an invalid escaped hex code' do
+        let(:given) { 'J\ames' }
+
+        it 'raises MalformedError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::DN::MalformedError, 'Invalid escaped hex code "\am"')
+        end
+      end
+
+      context 'with a value ending with the escape character' do
+        let(:given) { 'foo\\' }
+
+        it 'raises MalformedError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::DN::MalformedError, 'DN string ended unexpectedly')
+        end
+      end
+    end
+  end
+
+  describe '#to_normalized_s' do
+    subject { described_class.new(given).to_normalized_s }
+
+    it_behaves_like 'normalizes a DN'
+
+    context 'when we do not support the given DN format' do
+      context 'multivalued RDNs' do
+        context 'without extraneous whitespace' do
+          let(:given) { 'uid=john smith+telephonenumber=+1 555-555-5555,ou=people,dc=example,dc=com' }
+
+          it 'raises UnsupportedError' do
+            expect { subject }.to raise_error(Gitlab::LDAP::DN::UnsupportedError)
+          end
+        end
+
+        context 'with extraneous whitespace' do
+          context 'around the phone number plus sign' do
+            let(:given) { 'uid = John Smith  + telephoneNumber  = + 1 555-555-5555 , ou = People,dc=example,dc=com' }
+
+            it 'raises UnsupportedError' do
+              expect { subject }.to raise_error(Gitlab::LDAP::DN::UnsupportedError)
+            end
+          end
+
+          context 'not around the phone number plus sign' do
+            let(:given) { 'uid = John Smith  + telephoneNumber  = +1 555-555-5555 , ou = People,dc=example,dc=com' }
+
+            it 'raises UnsupportedError' do
+              expect { subject }.to raise_error(Gitlab::LDAP::DN::UnsupportedError)
+            end
+          end
+        end
+      end
+    end
+
+    context 'when the given DN is malformed' do
+      context 'when ending with a comma' do
+        let(:given) { 'uid=John Smith,' }
+
+        it 'raises MalformedError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::DN::MalformedError, 'DN string ended unexpectedly')
+        end
+      end
+
+      context 'when given a BER encoded attribute value with a space in it' do
+        let(:given) { '0.9.2342.19200300.100.1.25=#aa aa' }
+
+        it 'raises MalformedError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::DN::MalformedError, "Expected the end of an attribute value, but got \"a\"")
+        end
+      end
+
+      context 'when given a BER encoded attribute value with a non-hex character in it' do
+        let(:given) { '0.9.2342.19200300.100.1.25=#aaXaaa' }
+
+        it 'raises MalformedError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::DN::MalformedError, "Expected the first character of a hex pair, but got \"X\"")
+        end
+      end
+
+      context 'when given a BER encoded attribute value with a non-hex character in it' do
+        let(:given) { '0.9.2342.19200300.100.1.25=#aaaYaa' }
+
+        it 'raises MalformedError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::DN::MalformedError, "Expected the second character of a hex pair, but got \"Y\"")
+        end
+      end
+
+      context 'when given a hex pair with a non-hex character in it, inside double quotes' do
+        let(:given) { 'uid="Sebasti\\cX\\a1n"' }
+
+        it 'raises MalformedError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::DN::MalformedError, "Expected the second character of a hex pair inside a double quoted value, but got \"X\"")
+        end
+      end
+
+      context 'without a name value pair' do
+        let(:given) { 'John' }
+
+        it 'raises MalformedError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::DN::MalformedError, 'DN string ended unexpectedly')
+        end
+      end
+
+      context 'with an open (as opposed to closed) double quote' do
+        let(:given) { 'cn="James' }
+
+        it 'raises MalformedError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::DN::MalformedError, 'DN string ended unexpectedly')
+        end
+      end
+
+      context 'with an invalid escaped hex code' do
+        let(:given) { 'cn=J\ames' }
+
+        it 'raises MalformedError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::DN::MalformedError, 'Invalid escaped hex code "\am"')
+        end
+      end
+
+      context 'with a value ending with the escape character' do
+        let(:given) { 'cn=\\' }
+
+        it 'raises MalformedError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::DN::MalformedError, 'DN string ended unexpectedly')
+        end
+      end
+
+      context 'with an invalid OID attribute type name' do
+        let(:given) { '1.2.d=Value' }
+
+        it 'raises MalformedError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::DN::MalformedError, 'Unrecognized RDN OID attribute type name character "d"')
+        end
+      end
+
+      context 'with a period in a non-OID attribute type name' do
+        let(:given) { 'd1.2=Value' }
+
+        it 'raises MalformedError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::DN::MalformedError, 'Unrecognized RDN attribute type name character "."')
+        end
+      end
+
+      context 'when starting with non-space, non-alphanumeric character' do
+        let(:given) { ' -uid=John Smith' }
+
+        it 'raises MalformedError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::DN::MalformedError, 'Unrecognized first character of an RDN attribute type name "-"')
+        end
+      end
+
+      context 'when given a UID with an escaped equal sign' do
+        let(:given) { 'uid\\=john' }
+
+        it 'raises MalformedError' do
+          expect { subject }.to raise_error(Gitlab::LDAP::DN::MalformedError, 'Unrecognized RDN attribute type name character "\\"')
+        end
+      end
+    end
+  end
+
+  def assert_generic_test(test_description, got, expected)
+    test_failure_message = "Failed test description: '#{test_description}'\n\n    expected: \"#{expected}\"\n         got: \"#{got}\""
+    expect(got).to eq(expected), test_failure_message
+  end
+end

spec/lib/gitlab/ldap/person_spec.rb

@@ -16,6 +16,34 @@ describe Gitlab::LDAP::Person do
     )
   end
 
+  describe '.normalize_dn' do
+    subject { described_class.normalize_dn(given) }
+
+    it_behaves_like 'normalizes a DN'
+
+    context 'with an exception during normalization' do
+      let(:given) { 'John "Smith,' } # just something that will cause an exception
+
+      it 'returns the given DN unmodified' do
+        expect(subject).to eq(given)
+      end
+    end
+  end
+
+  describe '.normalize_uid' do
+    subject { described_class.normalize_uid(given) }
+
+    it_behaves_like 'normalizes a DN attribute value'
+
+    context 'with an exception during normalization' do
+      let(:given) { 'John "Smith,' } # just something that will cause an exception
+
+      it 'returns the given UID unmodified' do
+        expect(subject).to eq(given)
+      end
+    end
+  end
+
   describe '#name' do
     it 'uses the configured name attribute and handles values as an array' do
       name = 'John Doe'
@@ -43,4 +71,9 @@ describe Gitlab::LDAP::Person do
       expect(person.email).to eq([user_principal_name])
     end
   end
+
+  def assert_generic_test(test_description, got, expected)
+    test_failure_message = "Failed test description: '#{test_description}'\n\n    expected: #{expected}\n         got: #{got}"
+    expect(got).to eq(expected), test_failure_message
+  end
 end

spec/lib/gitlab/ldap/user_spec.rb

@@ -13,7 +13,7 @@ describe Gitlab::LDAP::User do
     }
   end
   let(:auth_hash) do
-    OmniAuth::AuthHash.new(uid: 'my-uid', provider: 'ldapmain', info: info)
+    OmniAuth::AuthHash.new(uid: 'uid=John Smith,ou=People,dc=example,dc=com', provider: 'ldapmain', info: info)
   end
   let(:ldap_user_upper_case) { described_class.new(auth_hash_upper_case) }
   let(:info_upper_case) do
@@ -24,13 +24,13 @@ describe Gitlab::LDAP::User do
     }
   end
   let(:auth_hash_upper_case) do
-    OmniAuth::AuthHash.new(uid: 'my-uid', provider: 'ldapmain', info: info_upper_case)
+    OmniAuth::AuthHash.new(uid: 'uid=John Smith,ou=People,dc=example,dc=com', provider: 'ldapmain', info: info_upper_case)
   end
   let!(:fake_proxy) { fake_ldap_sync_proxy('ldapmain') }
 
   describe '#changed?' do
     it "marks existing ldap user as changed" do
-      create(:omniauth_user, extern_uid: 'my-uid', provider: 'ldapmain')
+      create(:omniauth_user, extern_uid: 'uid=John Smith,ou=People,dc=example,dc=com', provider: 'ldapmain')
       expect(ldap_user.changed?).to be_truthy
     end
 
@@ -40,7 +40,7 @@ describe Gitlab::LDAP::User do
     end
 
     it "does not mark existing ldap user as changed" do
-      create(:omniauth_user, email: 'john@example.com', extern_uid: 'my-uid', provider: 'ldapmain')
+      create(:omniauth_user, email: 'john@example.com', extern_uid: 'uid=john smith,ou=people,dc=example,dc=com', provider: 'ldapmain')
       ldap_user.gl_user.user_synced_attributes_metadata(provider: 'ldapmain', email: true)
       expect(ldap_user.changed?).to be_falsey
     end
@@ -63,7 +63,7 @@ describe Gitlab::LDAP::User do
 
   describe 'find or create' do
     it "finds the user if already existing" do
-      create(:omniauth_user, extern_uid: 'my-uid', provider: 'ldapmain')
+      create(:omniauth_user, extern_uid: 'uid=John Smith,ou=People,dc=example,dc=com', provider: 'ldapmain')
 
       expect { ldap_user.save }.not_to change { User.count }
     end
@@ -73,7 +73,7 @@ describe Gitlab::LDAP::User do
       expect { ldap_user.save }.not_to change { User.count }
 
       existing_user.reload
-      expect(existing_user.ldap_identity.extern_uid).to eql 'my-uid'
+      expect(existing_user.ldap_identity.extern_uid).to eql 'uid=john smith,ou=people,dc=example,dc=com'
       expect(existing_user.ldap_identity.provider).to eql 'ldapmain'
     end
 
@@ -82,7 +82,7 @@ describe Gitlab::LDAP::User do
       expect { ldap_user.save }.not_to change { User.count }
 
       existing_user.reload
-      expect(existing_user.ldap_identity.extern_uid).to eql 'my-uid'
+      expect(existing_user.ldap_identity.extern_uid).to eql 'uid=john smith,ou=people,dc=example,dc=com'
       expect(existing_user.ldap_identity.provider).to eql 'ldapmain'
       expect(existing_user.id).to eql ldap_user.gl_user.id
     end
@@ -92,7 +92,7 @@ describe Gitlab::LDAP::User do
       expect { ldap_user_upper_case.save }.not_to change { User.count }
 
       existing_user.reload
-      expect(existing_user.ldap_identity.extern_uid).to eql 'my-uid'
+      expect(existing_user.ldap_identity.extern_uid).to eql 'uid=john smith,ou=people,dc=example,dc=com'
       expect(existing_user.ldap_identity.provider).to eql 'ldapmain'
       expect(existing_user.id).to eql ldap_user.gl_user.id
     end

spec/migrations/normalize_ldap_extern_uids_spec.rb

+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20170921101004_normalize_ldap_extern_uids')
+
+describe NormalizeLdapExternUids, :migration, :sidekiq do
+  let!(:identities) { table(:identities) }
+
+  around do |example|
+    Timecop.freeze { example.run }
+  end
+
+  before do
+    stub_const("Gitlab::Database::MigrationHelpers::BACKGROUND_MIGRATION_BATCH_SIZE", 2)
+    stub_const("Gitlab::Database::MigrationHelpers::BACKGROUND_MIGRATION_JOB_BUFFER_SIZE", 2)
+
+    # LDAP identities
+    (1..4).each do |i|
+      identities.create!(id: i, provider: 'ldapmain', extern_uid: " uid = foo #{i}, ou = People, dc = example, dc = com ", user_id: i)
+    end
+
+    # Non-LDAP identity
+    identities.create!(id: 5, provider: 'foo', extern_uid: " uid = foo 5, ou = People, dc = example, dc = com ", user_id: 5)
+  end
+
+  it 'correctly schedules background migrations' do
+    Sidekiq::Testing.fake! do
+      Timecop.freeze do
+        migrate!
+
+        expect(BackgroundMigrationWorker.jobs[0]['args']).to eq([described_class::MIGRATION, [1, 2]])
+        expect(BackgroundMigrationWorker.jobs[0]['at']).to eq(10.seconds.from_now.to_f)
+        expect(BackgroundMigrationWorker.jobs[1]['args']).to eq([described_class::MIGRATION, [3, 4]])
+        expect(BackgroundMigrationWorker.jobs[1]['at']).to eq(20.seconds.from_now.to_f)
+        expect(BackgroundMigrationWorker.jobs[2]['args']).to eq([described_class::MIGRATION, [5, 5]])
+        expect(BackgroundMigrationWorker.jobs[2]['at']).to eq(30.seconds.from_now.to_f)
+        expect(BackgroundMigrationWorker.jobs.size).to eq 3
+      end
+    end
+  end
+
+  it 'migrates the LDAP identities' do
+    Sidekiq::Testing.inline! do
+      migrate!
+      identities.where(id: 1..4).each do |identity|
+        expect(identity.extern_uid).to eq("uid=foo #{identity.id},ou=people,dc=example,dc=com")
+      end
+    end
+  end
+
+  it 'does not modify non-LDAP identities' do
+    Sidekiq::Testing.inline! do
+      migrate!
+      identity = identities.last
+      expect(identity.extern_uid).to eq(" uid = foo 5, ou = People, dc = example, dc = com ")
+    end
+  end
+end

spec/support/ldap_shared_examples.rb

+shared_examples_for 'normalizes a DN' do
+  using RSpec::Parameterized::TableSyntax
+
+  where(:test_description, :given, :expected) do
+    'strips extraneous whitespace'                                                                 | 'uid     =John Smith ,  ou = People, dc=  example,dc =com'                                            | 'uid=john smith,ou=people,dc=example,dc=com'
+    'strips extraneous whitespace for a DN with a single RDN'                                      | 'uid  =  John Smith'                                                                                  | 'uid=john smith'
+    'unescapes non-reserved, non-special Unicode characters'                                       | 'uid   =  Sebasti\\c3\\a1n\\ C.\\20Smith,   ou=People (aka. \\22humans\\")  ,dc=example, dc=com'      | 'uid=sebastián c. smith,ou=people (aka. \\"humans\\"),dc=example,dc=com'
+    'downcases the whole string'                                                                   | 'UID=John Smith,ou=People,dc=example,dc=com'                                                          | 'uid=john smith,ou=people,dc=example,dc=com'
+    'for a null DN (empty string), returns empty string and does not error'                        | ''                                                                                                    | ''
+    'does not strip an escaped leading space in an attribute value'                                | 'uid=\\ John Smith,ou=People,dc=example,dc=com'                                                       | 'uid=\\ john smith,ou=people,dc=example,dc=com'
+    'does not strip an escaped leading space in the last attribute value'                          | 'uid=\\ John Smith'                                                                                   | 'uid=\\ john smith'
+    'does not strip an escaped trailing space in an attribute value'                               | 'uid=John Smith\\ ,ou=People,dc=example,dc=com'                                                       | 'uid=john smith\\ ,ou=people,dc=example,dc=com'
+    'strips extraneous spaces after an escaped trailing space'                                     | 'uid=John Smith\\   ,ou=People,dc=example,dc=com'                                                     | 'uid=john smith\\ ,ou=people,dc=example,dc=com'
+    'strips extraneous spaces after an escaped trailing space at the end of the DN'                | 'uid=John Smith,ou=People,dc=example,dc=com\\   '                                                     | 'uid=john smith,ou=people,dc=example,dc=com\\ '
+    'properly preserves escaped trailing space after unescaped trailing spaces'                    | 'uid=John Smith  \\  ,ou=People,dc=example,dc=com'                                                    | 'uid=john smith  \\ ,ou=people,dc=example,dc=com'
+    'preserves multiple inner spaces in an attribute value'                                        | 'uid=John   Smith,ou=People,dc=example,dc=com'                                                        | 'uid=john   smith,ou=people,dc=example,dc=com'
+    'preserves inner spaces after an escaped space'                                                | 'uid=John\\   Smith,ou=People,dc=example,dc=com'                                                      | 'uid=john   smith,ou=people,dc=example,dc=com'
+    'hex-escapes an escaped leading newline in an attribute value'                                 | "uid=\\\nJohn Smith,ou=People,dc=example,dc=com"                                                      | "uid=\\0ajohn smith,ou=people,dc=example,dc=com"
+    'hex-escapes and does not strip an escaped trailing newline in an attribute value'             | "uid=John Smith\\\n,ou=People,dc=example,dc=com"                                                      | "uid=john smith\\0a,ou=people,dc=example,dc=com"
+    'hex-escapes an unescaped leading newline (actually an invalid DN?)'                           | "uid=\nJohn Smith,ou=People,dc=example,dc=com"                                                        | "uid=\\0ajohn smith,ou=people,dc=example,dc=com"
+    'strips an unescaped trailing newline (actually an invalid DN?)'                               | "uid=John Smith\n,ou=People,dc=example,dc=com"                                                        | "uid=john smith,ou=people,dc=example,dc=com"
+    'does not strip if no extraneous whitespace'                                                   | 'uid=John Smith,ou=People,dc=example,dc=com'                                                          | 'uid=john smith,ou=people,dc=example,dc=com'
+    'does not modify an escaped equal sign in an attribute value'                                  | 'uid= foo  \\=  bar'                                                                                  | 'uid=foo  \\=  bar'
+    'converts an escaped hex equal sign to an escaped equal sign in an attribute value'            | 'uid= foo  \\3D  bar'                                                                                 | 'uid=foo  \\=  bar'
+    'does not modify an escaped comma in an attribute value'                                       | 'uid= John C. Smith, ou=San Francisco\\, CA'                                                          | 'uid=john c. smith,ou=san francisco\\, ca'
+    'converts an escaped hex comma to an escaped comma in an attribute value'                      | 'uid= John C. Smith, ou=San Francisco\\2C CA'                                                         | 'uid=john c. smith,ou=san francisco\\, ca'
+    'does not modify an escaped hex carriage return character in an attribute value'               | 'uid= John C. Smith, ou=San Francisco\\,\\0DCA'                                                       | 'uid=john c. smith,ou=san francisco\\,\\0dca'
+    'does not modify an escaped hex line feed character in an attribute value'                     | 'uid= John C. Smith, ou=San Francisco\\,\\0ACA'                                                       | 'uid=john c. smith,ou=san francisco\\,\\0aca'
+    'does not modify an escaped hex CRLF in an attribute value'                                    | 'uid= John C. Smith, ou=San Francisco\\,\\0D\\0ACA'                                                   | 'uid=john c. smith,ou=san francisco\\,\\0d\\0aca'
+    'allows attribute type name OIDs'                                                              | '0.9.2342.19200300.100.1.25=Example,0.9.2342.19200300.100.1.25=Com'                                   | '0.9.2342.19200300.100.1.25=example,0.9.2342.19200300.100.1.25=com'
+    'strips extraneous whitespace from attribute type name OIDs'                                   | '0.9.2342.19200300.100.1.25 = Example, 0.9.2342.19200300.100.1.25 = Com'                              | '0.9.2342.19200300.100.1.25=example,0.9.2342.19200300.100.1.25=com'
+  end
+
+  with_them do
+    it 'normalizes the DN' do
+      assert_generic_test(test_description, subject, expected)
+    end
+  end
+end
+
+shared_examples_for 'normalizes a DN attribute value' do
+  using RSpec::Parameterized::TableSyntax
+
+  where(:test_description, :given, :expected) do
+    'strips extraneous whitespace'                                                      | '   John Smith   '                 | 'john smith'
+    'unescapes non-reserved, non-special Unicode characters'                            | 'Sebasti\\c3\\a1n\\ C.\\20Smith'   | 'sebastián c. smith'
+    'downcases the whole string'                                                        | 'JoHn C. Smith'                    | 'john c. smith'
+    'does not strip an escaped leading space in an attribute value'                     | '\\ John Smith'                    | '\\ john smith'
+    'does not strip an escaped trailing space in an attribute value'                    | 'John Smith\\ '                    | 'john smith\\ '
+    'hex-escapes an escaped leading newline in an attribute value'                      | "\\\nJohn Smith"                   | "\\0ajohn smith"
+    'hex-escapes and does not strip an escaped trailing newline in an attribute value'  | "John Smith\\\n"                   | "john smith\\0a"
+    'hex-escapes an unescaped leading newline (actually an invalid DN value?)'          | "\nJohn Smith"                     | "\\0ajohn smith"
+    'strips an unescaped trailing newline (actually an invalid DN value?)'              | "John Smith\n"                     | "john smith"
+    'does not strip if no extraneous whitespace'                                        | 'John Smith'                       | 'john smith'
+    'does not modify an escaped equal sign in an attribute value'                       | ' foo  \\=  bar'                   | 'foo  \\=  bar'
+    'converts an escaped hex equal sign to an escaped equal sign in an attribute value' | ' foo  \\3D  bar'                  | 'foo  \\=  bar'
+    'does not modify an escaped comma in an attribute value'                            | 'San Francisco\\, CA'              | 'san francisco\\, ca'
+    'converts an escaped hex comma to an escaped comma in an attribute value'           | 'San Francisco\\2C CA'             | 'san francisco\\, ca'
+    'does not modify an escaped hex carriage return character in an attribute value'    | 'San Francisco\\,\\0DCA'           | 'san francisco\\,\\0dca'
+    'does not modify an escaped hex line feed character in an attribute value'          | 'San Francisco\\,\\0ACA'           | 'san francisco\\,\\0aca'
+    'does not modify an escaped hex CRLF in an attribute value'                         | 'San Francisco\\,\\0D\\0ACA'       | 'san francisco\\,\\0d\\0aca'
+  end
+
+  with_them do
+    it 'normalizes the DN attribute value' do
+      assert_generic_test(test_description, subject, expected)
+    end
+  end
+end