Refactor LdapResetService to reflect multiple LDAP groups

app/models/users_group.rb

@@ -27,6 +27,8 @@ class UsersGroup < ActiveRecord::Base
   scope :developers, -> { where(group_access: DEVELOPER) }
   scope :masters,  -> { where(group_access: MASTER) }
   scope :owners,  -> { where(group_access: OWNER) }
+  scope :with_ldap_dn, -> { references(:user).includes(:user).
+    where(users: { provider: 'ldap' }) }
 
   scope :with_group, ->(group) { where(group_id: group.id) }
   scope :with_user, ->(user) { where(user_id: user.id) }

app/services/ldap_group_reset_service.rb

 class LdapGroupResetService
   def execute(group, current_user)
     # Only for ldap connected users
-    # reset last_credential_check_at
-    # set Gitlab::Access::Guest
-    group.members.includes(:user).each do |member|
-      user = member.user
+    # reset last_credential_check_at to force LDAP::Access::update_permissions
+    # set Gitlab::Access::Guest to later on upgrade the access of a user
 
-      if user.ldap_user? && user != current_user
-        member.group_access = group.ldap_access
-        member.save
-      end
+    # trigger the lowest access possible for all LDAP connected users
+    a = group.members.with_ldap_dn.map do |member|
+      # don't unauthorize the current user
+      next if current_user == member.user
+      member.update_attribute :group_access, Gitlab::Access::GUEST
     end
+
+    group.users.ldap.update_all last_credential_check_at: nil
   end
 end

spec/services/ldap_group_reset_service_spec.rb

@@ -2,15 +2,16 @@ require 'spec_helper'
 
 describe LdapGroupResetService do
   # TODO: refactor to multi-ldap setup
-  let(:group) { create(:group, ldap_cn: 'developers', ldap_access: Gitlab::Access::DEVELOPER) }
+  let(:group) { create(:group) }
   let(:user) { create(:user) }
-  let(:ldap_user) { create(:user, extern_uid: 'john', provider: 'ldap') }
-  let(:ldap_user_2) { create(:user, extern_uid: 'mike', provider: 'ldap') }
+  let(:ldap_user) { create(:user, extern_uid: 'john', provider: 'ldap', last_credential_check_at: Time.now) }
+  let(:ldap_user_2) { create(:user, extern_uid: 'mike', provider: 'ldap', last_credential_check_at: Time.now) }
 
   before do
     group.add_owner(user)
     group.add_owner(ldap_user)
     group.add_user(ldap_user_2, Gitlab::Access::REPORTER)
+    group.ldap_group_links.create cn: 'developers', group_access: Gitlab::Access::DEVELOPER
   end
 
   describe '#execute' do
@@ -18,16 +19,20 @@ describe LdapGroupResetService do
       before { LdapGroupResetService.new.execute(group, ldap_user) }
 
       it { member_access(ldap_user).should == Gitlab::Access::OWNER }
-      it { member_access(ldap_user_2).should == Gitlab::Access::DEVELOPER }
+      it { member_access(ldap_user_2).should == Gitlab::Access::GUEST }
       it { member_access(user).should == Gitlab::Access::OWNER }
+      it { expect(ldap_user.reload.last_credential_check_at).to be_nil }
+      it { expect(ldap_user_2.reload.last_credential_check_at).to be_nil }
     end
 
     context 'initiated by regular user' do
       before { LdapGroupResetService.new.execute(group, user) }
 
-      it { member_access(ldap_user).should == Gitlab::Access::DEVELOPER }
-      it { member_access(ldap_user_2).should == Gitlab::Access::DEVELOPER }
+      it { member_access(ldap_user).should == Gitlab::Access::GUEST }
+      it { member_access(ldap_user_2).should == Gitlab::Access::GUEST }
       it { member_access(user).should == Gitlab::Access::OWNER }
+      it { expect(ldap_user.reload.last_credential_check_at).to be_nil }
+      it { expect(ldap_user_2.reload.last_credential_check_at).to be_nil }
     end
   end