Merge branch '7352-sync-on-first-sign-in-only-redux' into 'master'

Perform LDAP group sync on sign in only for new users

Closes #7352

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

doc/administration/auth/ldap-ee.md

@@ -63,10 +63,10 @@ be created on first sign in.
 
 ## Group Sync
 
-If your LDAP supports the `memberof` property, GitLab will add the user to any
-new groups they might be added to when the user logs in. That way they don't need
-to wait for the hourly sync to be granted access to the groups that they are in
-in LDAP.
+If your LDAP supports the `memberof` property, when the user signs in for the
+first time GitLab will trigger a sync for groups the user should be a member of.
+That way they don't need to wait for the hourly sync to be granted
+access to their groups and projects.
 
 In GitLab Premium, we can also add a GitLab group to sync with one or multiple LDAP groups or we can
 also add a filter. The filter must comply with the syntax defined in [RFC 2254](https://tools.ietf.org/search/rfc2254).

ee/changelogs/unreleased/7352-sync-on-first-sign-in-only-redux.yml

+---
+title: Perform LDAP group sync on sign in only for new users
+merge_request:
+author:
+type: changed

ee/lib/ee/gitlab/auth/ldap/access.rb

@@ -125,6 +125,10 @@ module EE
           def update_memberships
             return if ldap_user.nil? || ldap_user.group_cns.empty?
 
+            # Only update membership for new users. After this, regular
+            # LDAP group sync will take care of things.
+            return unless user.created_at > 5.minutes.ago && !user.last_credential_check_at
+
             group_ids = ::LdapGroupLink.where(cn: ldap_user.group_cns, provider: provider)
                           .distinct(:group_id)
                           .pluck(:group_id)

ee/spec/lib/gitlab/auth/ldap/access_spec.rb

@@ -125,7 +125,7 @@ describe Gitlab::Auth::LDAP::Access do
           stub_ldap_person_find_by_dn(entry, provider)
         end
 
-        it 'triggers a sync for all groups found in `memberof`' do
+        it 'triggers a sync for all groups found in `memberof` for new users' do
           group_link_1 = create(:ldap_group_link, cn: 'Group1', provider: provider)
           group_link_2 = create(:ldap_group_link, cn: 'Group2', provider: provider)
           group_ids = [group_link_1, group_link_2].map(&:group_id)
@@ -155,6 +155,17 @@ describe Gitlab::Auth::LDAP::Access do
 
           access.update_user
         end
+
+        it 'does not performs the membership update for existing users' do
+          user.created_at = Time.now - 10.minutes
+          user.last_credential_check_at = Time.now
+          user.save
+
+          expect(LdapGroupLink).not_to receive(:where)
+          expect(LdapGroupSyncWorker).not_to receive(:perform_async)
+
+          access.update_user
+        end
       end
 
       it "doesn't continue when there is no `memberOf` param" do