Merge branch '1338-fix_dn_mismatch_bug' into 'master'

Fix LDAP DN case-mismatch bug in LDAP group sync

Closes #1338

See merge request !1337

changelogs/unreleased-ee/1338-fix_dn_mismatch_bug.yml

+---
+title: Fix LDAP DN case-mismatch bug in LDAP group sync
+merge_request: 1337
+author:

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]
+            current = self[dn.downcase]
 
             # Keep the higher of the access values.
-            self[dn] = to if current.nil? || to > current
+            self[dn.downcase] = to if current.nil? || to > current
           end
         end
       end

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

@@ -103,7 +103,7 @@ module EE
               user = member.user
               identity = user.identities.select(:id, :extern_uid)
                            .with_provider(provider).first
-              member_dn = identity.extern_uid
+              member_dn = identity.extern_uid.downcase
 
               # Skip if this is not an LDAP user with a valid `extern_uid`.
               next unless member_dn.present?

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

@@ -126,7 +126,8 @@ describe EE::Gitlab::LDAP::Sync::Group, lib: true do
     end
 
     let(:group) do
-      create(:group_with_ldap_group_link, :access_requestable,
+      create(:group_with_ldap_group_link,
+             :access_requestable,
              cn: 'ldap_group1',
              group_access: ::Gitlab::Access::DEVELOPER)
     end
@@ -253,6 +254,42 @@ describe EE::Gitlab::LDAP::Sync::Group, lib: true do
             .to eq([::Gitlab::Access::DEVELOPER, ::Gitlab::Access::OWNER])
         end
       end
+
+      context 'when the extern_uid and group member DNs have different case' do
+        let(:user1) { create(:user) }
+        let(:user2) { create(:user) }
+
+        # Change the case once on the LDAP group, and once on the GitLab Identity
+        # to test that both sides can handle the differing case.
+        let(:ldap_group1) do
+          ldap_group_entry(%W(
+            #{user_dn(user1.username).upcase}
+            #{user_dn(user2.username)}
+          ))
+        end
+
+        it 'does not revert the overrides' do
+          create(:identity, user: user1, extern_uid: user_dn(user1.username))
+          create(:identity, user: user2, extern_uid: user_dn(user2.username).upcase)
+          group.members.create(
+            user: user1,
+            access_level: ::Gitlab::Access::MASTER,
+            ldap: true,
+            override: true
+          )
+          group.members.create(
+            user: user2,
+            access_level: ::Gitlab::Access::OWNER,
+            ldap: true,
+            override: true
+          )
+
+          sync_group.update_permissions
+
+          expect(group.members.pluck(:access_level))
+            .to match_array([::Gitlab::Access::MASTER, ::Gitlab::Access::OWNER])
+        end
+      end
     end
 
     # Test that membership can be resolved for all different type of LDAP groups