Pick only those groups that the viewing user has access to,
in a project members' list. Add tests for possible scenarios Re-factor and remove N + 1 queries Remove author from changelog Don't use memoisation when not needed Include users part of parents of project's group Re-factor tests Create and add users according to roles Re-use group created earlier Add incomplete test for ancestoral groups Rename method to clarify category of groups Skip pending test, remove comments not needed Remove extra line Include ancestors from invited groups as well Add specs for participants service Add more specs Add more specs use instead of Use public group owner instead of project maintainer to test owner acess Remove tests that have now been moved into participants_service_spec Use :context instead of :all Create nested group instead of creating an ancestor separately Add comment explaining doubt on the failing spec Imrpove test setup Optimize sql queries Refactor specs file Add rubocop disablement Add special case for project owners Add small refactor Add explanation to the docs Fix wording Refactor group check Add small changes in specs Add cr remarks Add cr remarks Add specs Add small refactor Add code review remarks Refactor for better database usage Fix failing spec Remove rubocop offences Add cr remarks
This commit is contained in:
parent
8ac91ecfd1
commit
506bf42817
|
@ -8,6 +8,7 @@ class Member < ApplicationRecord
|
|||
include Gitlab::Access
|
||||
include Presentable
|
||||
include Gitlab::Utils::StrongMemoize
|
||||
include FromUnion
|
||||
|
||||
attr_accessor :raw_invite_token
|
||||
|
||||
|
|
|
@ -7,16 +7,69 @@ module Projects
|
|||
def execute(noteable)
|
||||
@noteable = noteable
|
||||
|
||||
participants = noteable_owner + participants_in_noteable + all_members + groups + project_members
|
||||
participants =
|
||||
noteable_owner +
|
||||
participants_in_noteable +
|
||||
all_members +
|
||||
groups +
|
||||
project_members
|
||||
|
||||
participants.uniq
|
||||
end
|
||||
|
||||
def project_members
|
||||
@project_members ||= sorted(project.team.members)
|
||||
@project_members ||= sorted(get_project_members)
|
||||
end
|
||||
|
||||
def get_project_members
|
||||
members = Member.from_union([project_members_through_ancestral_groups,
|
||||
project_members_through_invited_groups,
|
||||
individual_project_members])
|
||||
|
||||
User.id_in(members.select(:user_id))
|
||||
end
|
||||
|
||||
def all_members
|
||||
[{ username: "all", name: "All Project and Group Members", count: project_members.count }]
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def project_members_through_invited_groups
|
||||
groups_with_ancestors_ids = Gitlab::ObjectHierarchy
|
||||
.new(visible_groups)
|
||||
.base_and_ancestors
|
||||
.pluck_primary_key
|
||||
|
||||
GroupMember
|
||||
.active_without_invites_and_requests
|
||||
.with_source_id(groups_with_ancestors_ids)
|
||||
end
|
||||
|
||||
def visible_groups
|
||||
visible_groups = project.invited_groups
|
||||
|
||||
unless project_owner?
|
||||
visible_groups = visible_groups.public_or_visible_to_user(current_user)
|
||||
end
|
||||
|
||||
visible_groups
|
||||
end
|
||||
|
||||
def project_members_through_ancestral_groups
|
||||
project.group.present? ? project.group.members_with_parents : Member.none
|
||||
end
|
||||
|
||||
def individual_project_members
|
||||
project.project_members
|
||||
end
|
||||
|
||||
def project_owner?
|
||||
if project.group.present?
|
||||
project.group.owners.include?(current_user)
|
||||
else
|
||||
project.namespace.owner == current_user
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -0,0 +1,3 @@
|
|||
---
|
||||
title: "Don't leak private members in project member autocomplete suggestions"
|
||||
type: security
|
|
@ -44,6 +44,10 @@ Admins are able to share projects with any group in the system.
|
|||
|
||||
In the example above, the maximum access level of 'Developer' for members from 'Engineering' means that users with higher access levels in 'Engineering' ('Maintainer' or 'Owner') will only have 'Developer' access to 'Project Acme'.
|
||||
|
||||
## Sharing public project with private group
|
||||
|
||||
When sharing a public project with a private group, owners and maintainers of the project will see the name of the group in the `members` page. Owners will also have the possibility to see members of the private group they don't have access to when mentioning them in the issue or merge request.
|
||||
|
||||
## Share project with group lock
|
||||
|
||||
It is possible to prevent projects in a group from [sharing
|
||||
|
|
|
@ -8,6 +8,10 @@ describe Projects::AutocompleteSourcesController do
|
|||
set(:issue) { create(:issue, project: project) }
|
||||
set(:user) { create(:user) }
|
||||
|
||||
def members_by_username(username)
|
||||
json_response.find { |member| member['username'] == username }
|
||||
end
|
||||
|
||||
describe 'GET members' do
|
||||
before do
|
||||
group.add_owner(user)
|
||||
|
@ -17,22 +21,21 @@ describe Projects::AutocompleteSourcesController do
|
|||
it 'returns an array of member object' do
|
||||
get :members, format: :json, params: { namespace_id: group.path, project_id: project.path, type: issue.class.name, type_id: issue.id }
|
||||
|
||||
all = json_response.find {|member| member["username"] == 'all'}
|
||||
the_group = json_response.find {|member| member["username"] == group.full_path}
|
||||
the_user = json_response.find {|member| member["username"] == user.username}
|
||||
expect(members_by_username('all').symbolize_keys).to include(
|
||||
username: 'all',
|
||||
name: 'All Project and Group Members',
|
||||
count: 1)
|
||||
|
||||
expect(all.symbolize_keys).to include(username: 'all',
|
||||
name: 'All Project and Group Members',
|
||||
count: 1)
|
||||
expect(members_by_username(group.full_path).symbolize_keys).to include(
|
||||
type: group.class.name,
|
||||
name: group.full_name,
|
||||
avatar_url: group.avatar_url,
|
||||
count: 1)
|
||||
|
||||
expect(the_group.symbolize_keys).to include(type: group.class.name,
|
||||
name: group.full_name,
|
||||
avatar_url: group.avatar_url,
|
||||
count: 1)
|
||||
|
||||
expect(the_user.symbolize_keys).to include(type: user.class.name,
|
||||
name: user.name,
|
||||
avatar_url: user.avatar_url)
|
||||
expect(members_by_username(user.username).symbolize_keys).to include(
|
||||
type: user.class.name,
|
||||
name: user.name,
|
||||
avatar_url: user.avatar_url)
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -57,4 +57,108 @@ describe Projects::ParticipantsService do
|
|||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#project_members' do
|
||||
subject(:usernames) { service.project_members.map { |member| member[:username] } }
|
||||
|
||||
context 'when there is a project in group namespace' do
|
||||
set(:public_group) { create(:group, :public) }
|
||||
set(:public_project) { create(:project, :public, namespace: public_group)}
|
||||
|
||||
set(:public_group_owner) { create(:user) }
|
||||
|
||||
let(:service) { described_class.new(public_project, create(:user)) }
|
||||
|
||||
before do
|
||||
public_group.add_owner(public_group_owner)
|
||||
end
|
||||
|
||||
it 'returns members of a group' do
|
||||
expect(usernames).to include(public_group_owner.username)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when there is a private group and a public project' do
|
||||
set(:public_group) { create(:group, :public) }
|
||||
set(:private_group) { create(:group, :private, :nested) }
|
||||
set(:public_project) { create(:project, :public, namespace: public_group)}
|
||||
|
||||
set(:project_issue) { create(:issue, project: public_project)}
|
||||
|
||||
set(:public_group_owner) { create(:user) }
|
||||
set(:private_group_member) { create(:user) }
|
||||
set(:public_project_maintainer) { create(:user) }
|
||||
set(:private_group_owner) { create(:user) }
|
||||
|
||||
set(:group_ancestor_owner) { create(:user) }
|
||||
|
||||
before(:context) do
|
||||
public_group.add_owner public_group_owner
|
||||
private_group.add_developer private_group_member
|
||||
public_project.add_maintainer public_project_maintainer
|
||||
|
||||
private_group.add_owner private_group_owner
|
||||
private_group.parent.add_owner group_ancestor_owner
|
||||
end
|
||||
|
||||
context 'when the private group is invited to the public project' do
|
||||
before(:context) do
|
||||
create(:project_group_link, group: private_group, project: public_project)
|
||||
end
|
||||
|
||||
context 'when a user who is outside the public project and the private group is signed in' do
|
||||
let(:service) { described_class.new(public_project, create(:user)) }
|
||||
|
||||
it 'does not return the private group' do
|
||||
expect(usernames).not_to include(private_group.name)
|
||||
end
|
||||
|
||||
it 'does not return private group members' do
|
||||
expect(usernames).not_to include(private_group_member.username)
|
||||
end
|
||||
|
||||
it 'returns the project maintainer' do
|
||||
expect(usernames).to include(public_project_maintainer.username)
|
||||
end
|
||||
|
||||
it 'returns project members from an invited public group' do
|
||||
invited_public_group = create(:group, :public)
|
||||
invited_public_group.add_owner create(:user)
|
||||
|
||||
create(:project_group_link, group: invited_public_group, project: public_project)
|
||||
|
||||
expect(usernames).to include(invited_public_group.users.first.username)
|
||||
end
|
||||
|
||||
it 'does not return ancestors of the private group' do
|
||||
expect(usernames).not_to include(group_ancestor_owner.username)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when private group owner is signed in' do
|
||||
let(:service) { described_class.new(public_project, private_group_owner) }
|
||||
|
||||
it 'returns private group members' do
|
||||
expect(usernames).to include(private_group_member.username)
|
||||
end
|
||||
|
||||
it 'returns ancestors of the the private group' do
|
||||
expect(usernames).to include(group_ancestor_owner.username)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the namespace owner of the public project is signed in' do
|
||||
let(:service) { described_class.new(public_project, public_group_owner) }
|
||||
|
||||
it 'returns private group members' do
|
||||
expect(usernames).to include(private_group_member.username)
|
||||
end
|
||||
|
||||
it 'does not return members of the ancestral groups of the private group' do
|
||||
expect(usernames).to include(group_ancestor_owner.username)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue