Fix LDAP group sync regression for groups with member value `uid=<username>`

CHANGELOG-EE

@@ -5,6 +5,9 @@ v 8.7.0 (unreleased)
   - Refactor group sync to pull access level logic to its own class. !306
   - [Elastic] Stabilize database indexer if database is inconsistent
 
+v 8.6.6
+  - Fix LDAP group sync regression for groups with member value `uid=<username>` !335
+
 v 8.6.5
   - No EE-specific changes
 

lib/gitlab/ldap/group_sync.rb

@@ -165,12 +165,30 @@ module Gitlab
           return [] unless ldap_group.memberuid?
 
           members = ldap_group.member_uids
-          member_dns = members.map { |uid| dn_for_uid(uid) }.compact
+          member_dns = members.map { |uid| dn_for_uid(uid) }
         end
+        ensure_full_dns!(member_dns)
 
         logger.debug { "Members in '#{ldap_group.name}' LDAP group: #{member_dns}" }
 
-        member_dns
+        # Various lookups in this method could return `nil` values.
+        # Compact the array to remove those entries
+        member_dns.compact
+      end
+
+      # At least one customer reported that their LDAP `member` values contain
+      # only `uid=username` and not the full DN. This method allows us to
+      # account for that. See gitlab-ee#442
+      def ensure_full_dns!(dns)
+        dns.map! do |dn|
+          # If there is more than one equal sign we must have a full DN
+          # Or at least the probability is higher.
+          return dn if dn.count('=') > 1
+
+          # If there is only one equal sign, we may only have a `uid`.
+          # In this case, strip the first part and look up full DN by UID
+          dn_for_uid(dn.split('=')[1])
+        end
       end
 
       def member_uid_to_dn(uid)

spec/lib/gitlab/ldap/group_sync_spec.rb

@@ -434,6 +434,61 @@ describe Gitlab::LDAP::GroupSync, lib: true do
             .not_to change { group1.members.count }
         end
       end
+
+      # See gitlab-ee#442 and comment in GroupSync#ensure_full_dns!
+      context 'with uid=username member format' do
+        let(:ldap_group) do
+          Gitlab::LDAP::Group.new(
+            Net::LDAP::Entry.from_single_ldif_string(
+              <<-EOS.strip_heredoc
+                dn: cn=ldap_group1,ou=groups,dc=example,dc=com
+                cn: ldap_group1
+                member: uid=#{user1.username}
+                description: LDAP Group 1
+                objectclass: top
+                objectclass: groupOfUniqueNames
+              EOS
+            )
+          )
+        end
+        let(:ldap_user) do
+          Gitlab::LDAP::Person.new(
+            Net::LDAP::Entry.from_single_ldif_string(
+              "dn: uid=#{user1.username},ou=users,dc=example,dc=com"
+            ),
+            'ldapmain'
+          )
+        end
+
+        before do
+          allow(Gitlab::LDAP::Person)
+            .to receive(:find_by_uid)
+              .with(user1.username, any_args)
+              .and_return(ldap_user)
+        end
+
+        it 'adds the user to the group' do
+          expect { group_sync.sync_groups }
+            .to change { group1.members.where(user_id: user1.id).any? }
+              .from(false).to(true)
+        end
+
+        it 'expects Gitlab::LDAP::Person to be called' do
+          expect(Gitlab::LDAP::Person).to receive(:find_by_uid)
+
+          group_sync.sync_groups
+        end
+
+        it do
+          expect { group_sync.sync_groups }
+            .to change {
+              user1.identities.find_by(
+                provider: group_sync.provider,
+                extern_uid: ldap_user.dn
+              ).secondary_extern_uid
+            }.from(nil).to(user1.username)
+        end
+      end
     end
   end