Rework admin_group_member policy to allow owners

The change will check for unlock_membership_to_ldap and the instance
settings.
parent 972e5e68
...@@ -30,6 +30,10 @@ module EE ...@@ -30,6 +30,10 @@ module EE
::Gitlab::CurrentSettings.lock_memberships_to_ldap? ::Gitlab::CurrentSettings.lock_memberships_to_ldap?
end end
condition(:owners_bypass_ldap_lock) do
ldap_lock_bypassable?
end
condition(:security_dashboard_enabled) do condition(:security_dashboard_enabled) do
@subject.feature_available?(:security_dashboard) @subject.feature_available?(:security_dashboard)
end end
...@@ -132,13 +136,14 @@ module EE ...@@ -132,13 +136,14 @@ module EE
rule { admin | (can_owners_manage_ldap & owner) }.enable :admin_ldap_group_links rule { admin | (can_owners_manage_ldap & owner) }.enable :admin_ldap_group_links
rule { ldap_synced }.prevent :admin_group_member
rule { ldap_synced & ~owners_bypass_ldap_lock }.prevent :admin_group_member
rule { ldap_synced & (admin | owner) }.enable :update_group_member rule { ldap_synced & (admin | owner) }.enable :update_group_member
rule { ldap_synced & (admin | (can_owners_manage_ldap & owner)) }.enable :override_group_member rule { ldap_synced & (admin | (can_owners_manage_ldap & owner)) }.enable :override_group_member
rule { memberships_locked_to_ldap & ~admin }.policy do rule { memberships_locked_to_ldap & ~admin & ~owners_bypass_ldap_lock }.policy do
prevent :admin_group_member prevent :admin_group_member
prevent :update_group_member prevent :update_group_member
prevent :override_group_member prevent :override_group_member
...@@ -179,6 +184,12 @@ module EE ...@@ -179,6 +184,12 @@ module EE
super super
end end
def ldap_lock_bypassable?
return false unless ::Gitlab::CurrentSettings.allow_group_owners_to_manage_ldap?
!!subject.unlock_membership_to_ldap? && subject.owned_by?(user)
end
def sso_enforcement_prevents_access? def sso_enforcement_prevents_access?
return false unless subject.persisted? return false unless subject.persisted?
return false if user&.admin? return false if user&.admin?
......
...@@ -375,6 +375,42 @@ describe GroupPolicy do ...@@ -375,6 +375,42 @@ describe GroupPolicy do
it { is_expected.not_to be_allowed(:override_group_member) } it { is_expected.not_to be_allowed(:override_group_member) }
it { is_expected.not_to be_allowed(:update_group_member) } it { is_expected.not_to be_allowed(:update_group_member) }
end end
context 'when LDAP sync is enabled' do
let(:current_user) { owner }
before do
allow(group).to receive(:ldap_synced?).and_return(true)
end
context 'Group Owner disable membership lock' do
before do
group.update!(unlock_membership_to_ldap: true)
end
it { is_expected.to be_allowed(:admin_group_member) }
it { is_expected.to be_allowed(:override_group_member) }
it { is_expected.to be_allowed(:update_group_member) }
end
context 'Group Owner keeps the membership lock' do
before do
group.update!(unlock_membership_to_ldap: false)
end
it { is_expected.not_to be_allowed(:admin_group_member) }
it { is_expected.not_to be_allowed(:override_group_member) }
it { is_expected.not_to be_allowed(:update_group_member) }
end
end
context 'when LDAP sync is disable' do
let(:current_user) { owner }
it { is_expected.not_to be_allowed(:admin_group_member) }
it { is_expected.not_to be_allowed(:override_group_member) }
it { is_expected.not_to be_allowed(:update_group_member) }
end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment