Merge branch 'feature/ldap_sync_keeps_last_owner' into 'master'

Prevent LDAP group sync from removing a group's last owner

As discussed with Patricio in support request 3151

When setting up LDAP group sync, users can easily lock themselves out of their own group (cf. http://doc.gitlab.com/ee/integration/ldap.html#locking-yourself-out-of-your-own-group). The group may be left without any owner and requires intervention of the GitLab Administrators - a significant burden for a large deployment. In order to avoid this situation, this patch prevents the LDAP group sync from removing the last owner of a group, in the same way GitLab does not allow removing the last owner of a group via the Members page. This "last owner" is left in place until a new owner is nominated, the next sync after that will remove him/her.

See merge request !7

CHANGELOG-EE

@@ -2,6 +2,7 @@ v 7.12 (Unreleased)
   - Fix error when viewing merge request with a commit that includes "Closes #<issue id>".
   - Enhance LDAP group synchronization to check also for member attributes that only contain "uid=<username>"
   - Enhance LDAP group synchronization to check also for submember attributes
+  - Prevent LDAP group sync from removing a group's last owner 
 
 v 7.11.4
   - no changes specific to EE

doc/integration/ldap.md

@@ -161,9 +161,9 @@ If you have two LDAP group links, e.g. 'cn=Engineering' at level 'Developer' and
 
 ### Locking yourself out of your own group
 
-As an LDAP-enabled GitLab user, if you create a group and then set it to synchronize with an LDAP group you do not belong to, you will be removed from the grop as soon as the synchronization takes effect for you.
+As an LDAP-enabled GitLab user, if you create a group and then set it to synchronize with an LDAP group you do not belong to, you will be removed from the group as soon as the synchronization takes effect for you, unless you are the last owner of the group.
 
-If you accidentally lock yourself out of your own GitLab group, ask a GitLab administrator to change the LDAP synchronization settings for your group.
+If you accidentally lock yourself out of your own GitLab group, ask another owner of the group or a GitLab administrator to change the LDAP synchronization settings for your group.
 
 ### Non-LDAP GitLab users
 

lib/gitlab/ldap/access.rb

@@ -137,6 +137,8 @@ module Gitlab
 
           if active_group_links.any?
             group.add_users([user.id], fetch_group_access(group, user, active_group_links))
+          elsif group.last_owner?(user)
+            Rails.logger.warn "#{self.class.name}: LDAP group sync cannot remove #{user.name} (#{user.id}) from group #{group.name} (#{group.id}) as this is the group's last owner"
           else
             group.users.delete(user)
           end

spec/lib/gitlab/ldap/access_spec.rb

@@ -287,6 +287,35 @@ objectclass: posixGroup
           change{ gitlab_group_1.members.where(user_id: user).any? }.from(true).to(false)
       end
     end
+
+    context "existing access as owner for group-1 with no other owner, not allowed" do
+      before do
+        gitlab_group_1.group_members.owners.create(user_id: user.id)
+        gitlab_group_1.ldap_group_links.create({
+          cn: 'ldap-group1', group_access: Gitlab::Access::OWNER, provider: 'ldapmain'})
+        access.stub(cns_with_access: ['ldap-group2'])
+      end
+
+      it "does not remove the user from gitlab_group_1 since it's the last owner" do
+        expect { access.update_ldap_group_links }.not_to \
+          change{ gitlab_group_1.has_owner?(user) }
+      end
+    end
+
+    context "existing access as owner for group-1 while other owners present, not allowed" do
+      before do
+        owner2 = create(:user) # a 2nd owner
+        gitlab_group_1.group_members.owners.create([ {user_id: user.id}, {user_id: owner2.id} ] )
+        gitlab_group_1.ldap_group_links.create({
+          cn: 'ldap-group1', group_access: Gitlab::Access::OWNER, provider: 'ldapmain'})
+        access.stub(cns_with_access: ['ldap-group2'])
+      end
+
+      it "removes user from gitlab_group_1" do
+        expect { access.update_ldap_group_links }.to \
+          change{ gitlab_group_1.members.where(user_id: user).any? }.from(true).to(false)
+      end
+    end
   end
 
   describe 'ldap_groups' do