Commit a0de8659 authored by Jonas Wälter's avatar Jonas Wälter Committed by Thong Kuah

Prevent removal of last blocked group owner [RUN ALL RSPEC] [RUN AS-IF-FOSS]

parent a048fcf4
......@@ -340,6 +340,13 @@ class Group < Namespace
has_owner?(user) && members_with_parents.owners.size == 1
end
def last_blocked_owner?(user)
return false if members_with_parents.owners.any?
blocked_owners = members.blocked.where(access_level: Gitlab::Access::OWNER)
blocked_owners.size == 1 && blocked_owners.exists?(user_id: user)
end
def ldap_synced?
false
end
......
......@@ -75,7 +75,20 @@ class Member < ApplicationRecord
left_join_users
.where(user_ok)
.where(requested_at: nil)
.non_request
.non_minimal_access
.reorder(nil)
end
scope :blocked, -> do
is_external_invite = arel_table[:user_id].eq(nil).and(arel_table[:invite_token].not_eq(nil))
user_is_blocked = User.arel_table[:state].eq(:blocked)
user_ok = Arel::Nodes::Grouping.new(is_external_invite).or(user_is_blocked)
left_join_users
.where(user_ok)
.non_request
.non_minimal_access
.reorder(nil)
end
......
......@@ -4,7 +4,7 @@ class GroupMemberPolicy < BasePolicy
delegate :group
with_scope :subject
condition(:last_owner) { @subject.group.last_owner?(@subject.user) }
condition(:last_owner) { @subject.group.last_owner?(@subject.user) || @subject.group.last_blocked_owner?(@subject.user) }
desc "Membership is users' own"
with_score 0
......
---
title: Prevent removal of the last group owner if the last group owner is a blocked user
merge_request: 54587
author: Jonas Wälter @wwwjon
type: fixed
......@@ -565,6 +565,42 @@ RSpec.describe Group do
end
end
describe '#last_blocked_owner?' do
let(:blocked_user) { create(:user, :blocked) }
before do
group.add_user(blocked_user, GroupMember::OWNER)
end
it { expect(group.last_blocked_owner?(blocked_user)).to be_truthy }
context 'with another active owner' do
before do
group.add_user(create(:user), GroupMember::OWNER)
end
it { expect(group.last_blocked_owner?(blocked_user)).to be_falsy }
end
context 'with 2 blocked owners' do
before do
group.add_user(create(:user, :blocked), GroupMember::OWNER)
end
it { expect(group.last_blocked_owner?(blocked_user)).to be_falsy }
end
context 'with owners from a parent' do
before do
parent_group = create(:group)
create(:group_member, :owner, group: parent_group)
group.update(parent: parent_group)
end
it { expect(group.last_blocked_owner?(blocked_user)).to be_falsy }
end
end
describe '#lfs_enabled?' do
context 'LFS enabled globally' do
before do
......
......@@ -130,14 +130,18 @@ RSpec.describe Member do
@maintainer_user = create(:user).tap { |u| project.add_maintainer(u) }
@maintainer = project.members.find_by(user_id: @maintainer_user.id)
@blocked_user = create(:user).tap do |u|
@blocked_maintainer_user = create(:user).tap do |u|
project.add_maintainer(u)
u.block!
end
@blocked_developer_user = create(:user).tap do |u|
project.add_developer(u)
u.block!
end
@blocked_maintainer = project.members.find_by(user_id: @blocked_user.id, access_level: Gitlab::Access::MAINTAINER)
@blocked_developer = project.members.find_by(user_id: @blocked_user.id, access_level: Gitlab::Access::DEVELOPER)
@blocked_maintainer = project.members.find_by(user_id: @blocked_maintainer_user.id, access_level: Gitlab::Access::MAINTAINER)
@blocked_developer = project.members.find_by(user_id: @blocked_developer_user.id, access_level: Gitlab::Access::DEVELOPER)
@invited_member = create(:project_member, :developer,
project: project,
......@@ -161,7 +165,7 @@ RSpec.describe Member do
describe '.access_for_user_ids' do
it 'returns the right access levels' do
users = [@owner_user.id, @maintainer_user.id, @blocked_user.id]
users = [@owner_user.id, @maintainer_user.id, @blocked_maintainer_user.id]
expected = {
@owner_user.id => Gitlab::Access::OWNER,
@maintainer_user.id => Gitlab::Access::MAINTAINER
......@@ -382,6 +386,20 @@ RSpec.describe Member do
it { is_expected.not_to include @member_with_minimal_access }
end
describe '.blocked' do
subject { described_class.blocked.to_a }
it { is_expected.not_to include @owner }
it { is_expected.not_to include @maintainer }
it { is_expected.not_to include @invited_member }
it { is_expected.not_to include @accepted_invite_member }
it { is_expected.not_to include @requested_member }
it { is_expected.not_to include @accepted_request_member }
it { is_expected.to include @blocked_maintainer }
it { is_expected.to include @blocked_developer }
it { is_expected.not_to include @member_with_minimal_access }
end
describe '.active_without_invites_and_requests' do
subject { described_class.active_without_invites_and_requests.to_a }
......
......@@ -90,6 +90,14 @@ RSpec.describe GroupMemberPolicy do
specify { expect_allowed(:read_group) }
end
context 'with one blocked owner' do
let(:owner) { create(:user, :blocked) }
let(:current_user) { owner }
specify { expect_disallowed(*member_related_permissions) }
specify { expect_disallowed(:read_group) }
end
context 'with more than one owner' do
let(:current_user) { owner }
......
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