Only admins and group owners can set LDAP overrides

It was always the intention to restrict LDAP overrides to the
owner. During implementation it was allowed for masters mistakenly.
Now we are restricting this action to owners, to match the behavior
that only owners can manage LDAP group links, too.

app/policies/ee/group_policy.rb

+module EE
+  module GroupPolicy
+    def rules
+      raise NotImplementedError unless defined?(super)
+
+      super
+
+      return unless @user
+
+      if @subject.ldap_synced?
+        cannot! :admin_group_member
+        can! :override_group_member if @user.admin? || @subject.has_owner?(@user)
+      end
+    end
+  end
+end

app/policies/group_policy.rb

 class GroupPolicy < BasePolicy
+  prepend EE::GroupPolicy
+
   def rules
     can! :read_group if @subject.public?
     return unless @user
@@ -34,8 +36,6 @@ class GroupPolicy < BasePolicy
     if globally_viewable && @subject.request_access_enabled && !member
       can! :request_access
     end
-
-    additional_rules!(master)
   end
 
   def can_read_group?
@@ -47,11 +47,4 @@ class GroupPolicy < BasePolicy
 
     GroupProjectsFinder.new(@subject).execute(@user).any?
   end
-
-  def additional_rules!(master)
-    if @subject.ldap_synced?
-      cannot! :admin_group_member
-      can! :override_group_member if master
-    end
-  end
 end

changelogs/unreleased-ee/ldap_overrides_by_owner_only.yml

+---
+title: Only admins or group owners can set LDAP overrides
+merge_request:
+author:

spec/policies/ee/group_policy_spec.rb

+require 'spec_helper'
+
+describe GroupPolicy, models: true do
+  let(:guest) { create(:user) }
+  let(:reporter) { create(:user) }
+  let(:developer) { create(:user) }
+  let(:master) { create(:user) }
+  let(:owner) { create(:user) }
+  let(:auditor) { create(:user, :auditor) }
+  let(:admin) { create(:admin) }
+  let(:group) { create(:group) }
+
+  before do
+    group.add_guest(guest)
+    group.add_reporter(reporter)
+    group.add_developer(developer)
+    group.add_master(master)
+    group.add_owner(owner)
+  end
+
+  subject { described_class.abilities(current_user, group).to_set }
+
+  context 'when LDAP sync is not enabled' do
+    context 'owner' do
+      let(:current_user) { owner }
+
+      it { is_expected.not_to include(:override_group_member) }
+    end
+
+    context 'admin' do
+      let(:current_user) { admin }
+
+      it { is_expected.not_to include(:override_group_member) }
+    end
+  end
+
+  context 'when LDAP sync is enabled' do
+    before do
+      allow(group).to receive(:ldap_synced?).and_return(true)
+    end
+
+    context 'with no user' do
+      let(:current_user) { nil }
+
+      it { is_expected.not_to include(:override_group_member) }
+    end
+
+    context 'guests' do
+      let(:current_user) { guest }
+
+      it { is_expected.not_to include(:override_group_member) }
+    end
+
+    context 'reporter' do
+      let(:current_user) { reporter }
+
+      it { is_expected.not_to include(:override_group_member) }
+    end
+
+    context 'developer' do
+      let(:current_user) { developer }
+
+      it { is_expected.not_to include(:override_group_member) }
+    end
+
+    context 'master' do
+      let(:current_user) { master }
+
+      it { is_expected.not_to include(:override_group_member) }
+    end
+
+    context 'owner' do
+      let(:current_user) { owner }
+
+      it { is_expected.to include(:override_group_member) }
+    end
+
+    context 'admin' do
+      let(:current_user) { admin }
+
+      it { is_expected.to include(:override_group_member) }
+    end
+  end
+end