diff --git a/app/controllers/concerns/membership_actions.rb b/app/controllers/concerns/membership_actions.rb index fb11bece79c582939d638e29db8b8cfc25cbbb53..8a67b62f28b71e8d751d5567e624697b6f853f8c 100644 --- a/app/controllers/concerns/membership_actions.rb +++ b/app/controllers/concerns/membership_actions.rb @@ -150,7 +150,11 @@ def requested_relations(inherited_permissions = :with_inherited_permissions) when 'only' [:inherited] else - [:inherited, :direct] + if Feature.enabled?(:webui_members_inherited_users, current_user) + [:inherited, :direct, :shared_from_groups] + else + [:inherited, :direct] + end end end end diff --git a/app/finders/group_members_finder.rb b/app/finders/group_members_finder.rb index 048e25046da25cfbd72d144e4164233f9086ed89..4688d5618976a6df322abc795cdee4a0b2ff9755 100644 --- a/app/finders/group_members_finder.rb +++ b/app/finders/group_members_finder.rb @@ -47,7 +47,7 @@ def groups_by_relations(include_relations) related_groups << Group.by_id(group.id) if include_relations&.include?(:direct) related_groups << group.ancestors if include_relations&.include?(:inherited) related_groups << group.descendants if include_relations&.include?(:descendants) - related_groups << group.shared_with_groups.public_or_visible_to_user(user) if include_relations&.include?(:shared_from_groups) + related_groups << Group.shared_into_ancestors(group).public_or_visible_to_user(user) if include_relations&.include?(:shared_from_groups) find_union(related_groups, Group) end diff --git a/app/models/group.rb b/app/models/group.rb index a1eece54f0cc789376ddfcd0d36eb51ca39e8994..ba45e80b7f3a2ee44f67eeb9203fdea733013635 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -186,6 +186,11 @@ def of_ancestors_and_self where(project_creation_level: permitted_levels) end + scope :shared_into_ancestors, -> (group) do + joins(:shared_group_links) + .where(group_group_links: { shared_group_id: group.self_and_ancestors }) + end + class << self def sort_by_attribute(method) if method == 'storage_size_desc' diff --git a/config/feature_flags/development/webui_members_inherited_users.yml b/config/feature_flags/development/webui_members_inherited_users.yml new file mode 100644 index 0000000000000000000000000000000000000000..14704fd8341d7bcda562e15c0f5c9b2533327f1e --- /dev/null +++ b/config/feature_flags/development/webui_members_inherited_users.yml @@ -0,0 +1,8 @@ +--- +name: webui_members_inherited_users +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/83214 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/364078 +milestone: '15.4' +type: development +group: group::workspace +default_enabled: false diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index c6fd184ede06acbafaf2bc3bcd2f96c545fefe91..a3659ae91630c0e9dce99dbf9540004f3997aeef 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -97,6 +97,25 @@ expect(assigns(:members).map(&:user_id)).to contain_exactly(user.id) end end + + context 'when webui_members_inherited_users is disabled' do + let_it_be(:shared_group) { create(:group) } + let_it_be(:shared_group_user) { create(:user) } + let_it_be(:group_link) { create(:group_group_link, shared_group: shared_group, shared_with_group: group) } + + before do + group.add_owner(user) + shared_group.add_owner(shared_group_user) + stub_feature_flags(webui_members_inherited_users: false) + sign_in(user) + end + + it 'lists inherited group members only' do + get :index, params: { group_id: shared_group } + + expect(assigns(:members).map(&:user_id)).to contain_exactly(shared_group_user.id) + end + end end describe 'PUT update' do diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index 46eb340cbbaf0c3cf2a9a7bb9f0989c354c5c55b..fb27fe58cd9ec7058a24c975649b28f898d86ac5 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -5,6 +5,7 @@ RSpec.describe Projects::ProjectMembersController do let_it_be(:user) { create(:user) } let_it_be(:group) { create(:group, :public) } + let_it_be(:sub_group) { create(:group, parent: group) } let_it_be(:project, reload: true) { create(:project, :public) } before do @@ -52,7 +53,36 @@ end end - context 'when invited members are present' do + context 'when project belongs to a sub-group' do + let_it_be(:user_in_group) { create(:user) } + let_it_be(:project_in_group) { create(:project, :public, group: sub_group) } + + before do + group.add_owner(user_in_group) + project_in_group.add_maintainer(user) + sign_in(user) + end + + it 'lists inherited project members by default' do + get :index, params: { namespace_id: project_in_group.namespace, project_id: project_in_group } + + expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user.id, user_in_group.id) + end + + it 'lists direct project members only' do + get :index, params: { namespace_id: project_in_group.namespace, project_id: project_in_group, with_inherited_permissions: 'exclude' } + + expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user.id) + end + + it 'lists inherited project members only' do + get :index, params: { namespace_id: project_in_group.namespace, project_id: project_in_group, with_inherited_permissions: 'only' } + + expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user_in_group.id) + end + end + + context 'when invited project members are present' do let!(:invited_member) { create(:project_member, :invited, project: project) } before do diff --git a/spec/finders/group_members_finder_spec.rb b/spec/finders/group_members_finder_spec.rb index 00aa14209a219c1f9afda691d12f4d3029fec06d..6a97dfae8fba21d2249f76d423c697b2c9324a1c 100644 --- a/spec/finders/group_members_finder_spec.rb +++ b/spec/finders/group_members_finder_spec.rb @@ -51,7 +51,8 @@ user4_sub_group: create(:group_member, :developer, group: sub_group, user: user4, expires_at: 1.day.from_now), user4_group: create(:group_member, :developer, group: group, user: user4, expires_at: 2.days.from_now), user4_public_shared_group: create(:group_member, :developer, group: public_shared_group, user: user4), - user4_private_shared_group: create(:group_member, :developer, group: private_shared_group, user: user4) + user4_private_shared_group: create(:group_member, :developer, group: private_shared_group, user: user4), + user5_private_shared_group: create(:group_member, :developer, group: private_shared_group, user: user5) } end @@ -76,15 +77,15 @@ [:direct] | :sub_group | [:user1_sub_group, :user2_sub_group, :user3_sub_group, :user4_sub_group] [:inherited] | :sub_group | [:user1_group, :user2_group, :user3_group, :user4_group] [:descendants] | :sub_group | [:user1_sub_sub_group, :user2_sub_sub_group, :user3_sub_sub_group, :user4_sub_sub_group] - [:shared_from_groups] | :sub_group | [] - [:direct, :inherited, :descendants, :shared_from_groups] | :sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group] + [:shared_from_groups] | :sub_group | [:user1_public_shared_group, :user2_public_shared_group, :user3_public_shared_group, :user4_public_shared_group] + [:direct, :inherited, :descendants, :shared_from_groups] | :sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_public_shared_group] [] | :sub_sub_group | [] GroupMembersFinder::DEFAULT_RELATIONS | :sub_sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group] [:direct] | :sub_sub_group | [:user1_sub_sub_group, :user2_sub_sub_group, :user3_sub_sub_group, :user4_sub_sub_group] [:inherited] | :sub_sub_group | [:user1_sub_group, :user2_group, :user3_sub_group, :user4_group] [:descendants] | :sub_sub_group | [] - [:shared_from_groups] | :sub_sub_group | [] - [:direct, :inherited, :descendants, :shared_from_groups] | :sub_sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group] + [:shared_from_groups] | :sub_sub_group | [:user1_public_shared_group, :user2_public_shared_group, :user3_public_shared_group, :user4_public_shared_group] + [:direct, :inherited, :descendants, :shared_from_groups] | :sub_sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_public_shared_group] end with_them do diff --git a/spec/requests/api/graphql/group/group_members_spec.rb b/spec/requests/api/graphql/group/group_members_spec.rb index bab8d5b770c3f3929c658267f946b1b271583bdd..5f8becc0726496400457b24f5d03495d52aca7c7 100644 --- a/spec/requests/api/graphql/group/group_members_spec.rb +++ b/spec/requests/api/graphql/group/group_members_spec.rb @@ -156,13 +156,20 @@ expect_array_response(child_user) end - it 'returns invited members plus inherited members' do + it 'returns invited members and inherited members of a shared group' do fetch_members(group: child_group, args: { relations: [:DIRECT, :INHERITED, :SHARED_FROM_GROUPS] }) expect(graphql_errors).to be_nil expect_array_response(invited_user, user_1, user_2, child_user) end + it 'returns invited members and inherited members of an ancestor of a shared group' do + fetch_members(group: grandchild_group, args: { relations: [:DIRECT, :INHERITED, :SHARED_FROM_GROUPS] }) + + expect(graphql_errors).to be_nil + expect_array_response(grandchild_user, invited_user, user_1, user_2, child_user) + end + it 'returns direct and inherited members' do fetch_members(group: child_group, args: { relations: [:DIRECT, :INHERITED] })