Downgrade group member role if LDAP dictates

lib/gitlab/ldap/access.rb

@@ -164,7 +164,7 @@ module Gitlab
         end
       end
 
-      # Loop throug all ldap conneted groups, and update the users link with it
+      # Loop through all ldap connected groups, and update the users link with it
       #
       # We documented what sort of queries an LDAP server can expect from
       # GitLab EE in doc/integration/ldap.md. Please remember to update that
@@ -174,7 +174,7 @@ module Gitlab
           active_group_links = group.ldap_group_links.where(cn: cns_with_access)
 
           if active_group_links.any?
-            group.add_users([user.id], fetch_group_access(group, user, active_group_links), skip_notification: true)
+            group.add_users([user.id], active_group_links.maximum(:group_access), skip_notification: true)
           elsif group.last_owner?(user)
             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
@@ -221,16 +221,6 @@ module Gitlab
           where(ldap_group_links: { provider: provider })
       end
 
-      # Get the group_access for a give user.
-      # Always respect the current level, never downgrade it.
-      def fetch_group_access(group, user, active_group_links)
-        current_access_level = group.group_members.where(user_id: user).maximum(:access_level)
-        max_group_access_level = active_group_links.maximum(:group_access)
-
-        # TODO: Test if nil value of current_access_level in handled properly
-        [current_access_level, max_group_access_level].compact.max
-      end
-
       def logger
         Rails.logger
       end

spec/lib/gitlab/ldap/access_spec.rb

@@ -328,19 +328,19 @@ objectclass: posixGroup
       end
     end
 
-    context "existing access as MASTER for group-1, allowed via ldap-group1 as DEVELOPER" do
+    context 'existing access as MASTER for group-1, allowed via ldap-group1 as DEVELOPER' do
       before do
         gitlab_group_1.group_members.masters.create(user_id: user.id)
         gitlab_group_1.ldap_group_links.create({
           cn: 'ldap-group1', group_access: Gitlab::Access::DEVELOPER, provider: 'ldapmain' })
       end
 
-      it "keeps the users master access for group 1" do
-        expect { access.update_ldap_group_links }.not_to \
-          change{ gitlab_group_1.has_master?(user) }
+      it 'downgrades the users access' do
+        expect { access.update_ldap_group_links }.to \
+          change{ gitlab_group_1.has_master?(user) }.from(true).to(false)
       end
 
-      it "doesn't send a notification email" do
+      it 'does not send a notification email' do
         expect { access.update_ldap_group_links }.not_to \
           change { ActionMailer::Base.deliveries }
       end