Commit 9347f47a authored by Aakriti Gupta's avatar Aakriti Gupta Committed by Małgorzata Ksionek

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
parent 1425a56c
......@@ -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
---
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
let_it_be(:issue) { create(:issue, project: project) }
let_it_be(: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(all.symbolize_keys).to include(username: 'all',
name: 'All Project and Group Members',
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('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(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
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