Include group parents into read access for project and group
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
This commit is contained in:
parent
645412b57f
commit
7b4b3d5f26
|
@ -155,15 +155,17 @@ class Group < Namespace
|
||||||
end
|
end
|
||||||
|
|
||||||
def has_owner?(user)
|
def has_owner?(user)
|
||||||
owners.include?(user)
|
members_with_parents.owners.where(user_id: user).any?
|
||||||
end
|
end
|
||||||
|
|
||||||
def has_master?(user)
|
def has_master?(user)
|
||||||
members.masters.where(user_id: user).any?
|
members_with_parents.masters.where(user_id: user).any?
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# Check if user is a last owner of the group.
|
||||||
|
# Parent owners are ignored for nested groups.
|
||||||
def last_owner?(user)
|
def last_owner?(user)
|
||||||
has_owner?(user) && owners.size == 1
|
owners.include?(user) && owners.size == 1
|
||||||
end
|
end
|
||||||
|
|
||||||
def avatar_type
|
def avatar_type
|
||||||
|
@ -189,6 +191,14 @@ class Group < Namespace
|
||||||
end
|
end
|
||||||
|
|
||||||
def refresh_members_authorized_projects
|
def refresh_members_authorized_projects
|
||||||
UserProjectAccessChangedService.new(users.pluck(:id)).execute
|
UserProjectAccessChangedService.new(users_with_parents.pluck(:id)).execute
|
||||||
|
end
|
||||||
|
|
||||||
|
def members_with_parents
|
||||||
|
GroupMember.where(requested_at: nil, source_id: parents.map(&:id).push(id))
|
||||||
|
end
|
||||||
|
|
||||||
|
def users_with_parents
|
||||||
|
User.where(id: members_with_parents.pluck(:user_id))
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -4,7 +4,7 @@ class GroupPolicy < BasePolicy
|
||||||
return unless @user
|
return unless @user
|
||||||
|
|
||||||
globally_viewable = @subject.public? || (@subject.internal? && !@user.external?)
|
globally_viewable = @subject.public? || (@subject.internal? && !@user.external?)
|
||||||
member = @subject.users.include?(@user)
|
member = @subject.users_with_parents.include?(@user)
|
||||||
owner = @user.admin? || @subject.has_owner?(@user)
|
owner = @user.admin? || @subject.has_owner?(@user)
|
||||||
master = owner || @subject.has_master?(@user)
|
master = owner || @subject.has_master?(@user)
|
||||||
|
|
||||||
|
|
|
@ -245,7 +245,7 @@ class ProjectPolicy < BasePolicy
|
||||||
def project_group_member?(user)
|
def project_group_member?(user)
|
||||||
project.group &&
|
project.group &&
|
||||||
(
|
(
|
||||||
project.group.members.exists?(user_id: user.id) ||
|
project.group.members_with_parents.exists?(user_id: user.id) ||
|
||||||
project.group.requesters.exists?(user_id: user.id)
|
project.group.requesters.exists?(user_id: user.id)
|
||||||
)
|
)
|
||||||
end
|
end
|
||||||
|
|
|
@ -277,4 +277,15 @@ describe Group, models: true do
|
||||||
it { is_expected.to be_valid }
|
it { is_expected.to be_valid }
|
||||||
it { expect(subject.parent).to be_kind_of(Group) }
|
it { expect(subject.parent).to be_kind_of(Group) }
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe '#members_with_parents' do
|
||||||
|
let!(:group) { create(:group, :nested) }
|
||||||
|
let!(:master) { group.parent.add_user(create(:user), GroupMember::MASTER) }
|
||||||
|
let!(:developer) { group.add_user(create(:user), GroupMember::DEVELOPER) }
|
||||||
|
|
||||||
|
it 'returns parents members' do
|
||||||
|
expect(group.members_with_parents).to include(developer)
|
||||||
|
expect(group.members_with_parents).to include(master)
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -105,4 +105,70 @@ describe GroupPolicy, models: true do
|
||||||
is_expected.to include(*owner_permissions)
|
is_expected.to include(*owner_permissions)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe 'private nested group inherit permissions' do
|
||||||
|
let(:nested_group) { create(:group, :private, parent: group) }
|
||||||
|
|
||||||
|
subject { described_class.abilities(current_user, nested_group).to_set }
|
||||||
|
|
||||||
|
context 'with no user' do
|
||||||
|
let(:current_user) { nil }
|
||||||
|
|
||||||
|
it do
|
||||||
|
is_expected.not_to include(:read_group)
|
||||||
|
is_expected.not_to include(*master_permissions)
|
||||||
|
is_expected.not_to include(*owner_permissions)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'guests' do
|
||||||
|
let(:current_user) { guest }
|
||||||
|
|
||||||
|
it do
|
||||||
|
is_expected.to include(:read_group)
|
||||||
|
is_expected.not_to include(*master_permissions)
|
||||||
|
is_expected.not_to include(*owner_permissions)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'reporter' do
|
||||||
|
let(:current_user) { reporter }
|
||||||
|
|
||||||
|
it do
|
||||||
|
is_expected.to include(:read_group)
|
||||||
|
is_expected.not_to include(*master_permissions)
|
||||||
|
is_expected.not_to include(*owner_permissions)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'developer' do
|
||||||
|
let(:current_user) { developer }
|
||||||
|
|
||||||
|
it do
|
||||||
|
is_expected.to include(:read_group)
|
||||||
|
is_expected.not_to include(*master_permissions)
|
||||||
|
is_expected.not_to include(*owner_permissions)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'master' do
|
||||||
|
let(:current_user) { master }
|
||||||
|
|
||||||
|
it do
|
||||||
|
is_expected.to include(:read_group)
|
||||||
|
is_expected.to include(*master_permissions)
|
||||||
|
is_expected.not_to include(*owner_permissions)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'owner' do
|
||||||
|
let(:current_user) { owner }
|
||||||
|
|
||||||
|
it do
|
||||||
|
is_expected.to include(:read_group)
|
||||||
|
is_expected.to include(*master_permissions)
|
||||||
|
is_expected.to include(*owner_permissions)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in New Issue