Merge branch 'fix_ldap_access_request_temp' into 'master'

Temporary fix for #825 - LDAP sync converts access requests to members

Interim fix for #825. It's a critical enough issue that we need the workaround for now and when the true fix comes we can remove the conditional. The spec will still be relevant, thankfully. 


See merge request !655

CHANGELOG-EE

@@ -4,6 +4,7 @@ v 8.11.0 (unreleased)
   - Allow projects to be moved between repository storages
   - Add rake task to remove old repository copies from repositories moved to another storage
   - Performance improvement of push rules
+  - Temporary fix for #825 - LDAP sync converts access requests to members. !655
   - Change LdapGroupSyncWorker to use new LDAP group sync classes
   - Removed unused GitLab GEO database index
   - Enable monitoring for ES classes

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

@@ -124,9 +124,18 @@ module EE
             if access < ::Gitlab::Access::OWNER && group.last_owner?(user)
               warn_cannot_remove_last_owner(user, group)
             else
-              # If you pass the user object, instead of just user ID,
-              # it saves an extra user database query.
-              group.add_users([user], access, skip_notification: true)
+              # Temporarily handle access requests until
+              # gitlab-org/gitlab-ee#825 is properly resolved.
+              member = group.requesters.find_by(user_id: user.id)
+              if member.present?
+                member.access_level = access
+                member.requested_at = nil
+                member.save
+              else
+                # If you pass the user object, instead of just user ID,
+                # it saves an extra user database query.
+                group.add_users([user], access, skip_notification: true)
+              end
             end
           end
 

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

@@ -49,7 +49,7 @@ describe EE::Gitlab::LDAP::Sync::Group, lib: true do
 
             sync_group.update_permissions
 
-            expect(group.members).not_to include(user)
+            expect(group.members.pluck(:user_id)).not_to include(user.id)
           end
         end
 
@@ -59,6 +59,22 @@ describe EE::Gitlab::LDAP::Sync::Group, lib: true do
           expect(group.members.pluck(:user_id)).to include(user.id)
         end
 
+        it 'converts an existing membership access request to a real member' do
+          group.members.create(
+            user: user,
+            access_level: ::Gitlab::Access::MASTER,
+            requested_at: DateTime.now
+          )
+          # Validate that the user is properly created as a requester first.
+          expect(group.requesters.pluck(:user_id)).to include(user.id)
+
+          sync_group.update_permissions
+
+          expect(group.members.pluck(:user_id)).to include(user.id)
+          expect(group.members.find_by(user_id: user.id).access_level)
+            .to eq(::Gitlab::Access::DEVELOPER)
+        end
+
         it 'downgrades existing member access' do
           # Create user with higher access
           group.add_users([user],