Merge branch 'master' of gitlab.com:gitlab-org/gitlab-ee

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

@@ -168,9 +168,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