diff --git a/app/models/members/members/invited_private_group_accessibility_assigner.rb b/app/models/members/members/invited_private_group_accessibility_assigner.rb
index 91ff703f68a8e429923615905b4bfe1dab41cff6..8ac1600f6e53be2dcd12ab8deac968ef6d678cf7 100644
--- a/app/models/members/members/invited_private_group_accessibility_assigner.rb
+++ b/app/models/members/members/invited_private_group_accessibility_assigner.rb
@@ -23,13 +23,14 @@ def initialize(members, source:, current_user:)
end
def execute
+ return if Feature.disabled?(:webui_members_inherited_users, current_user)
+
# We don't need to calculate the access level of the current user in the invited groups if:
#
# 1. The current user can admin members then the user should be able to see the source of all memberships
# to enable management of group/project memberships.
# 2. There are no members invited from a private group.
return if can_admin_members? || private_invited_group_members.nil?
- return if Feature.disabled?(:webui_members_inherited_users, current_user)
private_invited_group_members.each do |member|
member.is_source_accessible_to_current_user = authorized_groups.include?(member.source)
@@ -56,11 +57,20 @@ def private_invited_group_members
member.is_a?(GroupMember) &&
member.source.visibility_level != Gitlab::VisibilityLevel::PUBLIC &&
member.source_id != source.id && # Exclude direct member
- member.source.traversal_ids.exclude?(source.id) # Exclude inherited member
+ source_traversal_ids.exclude?(member.source_id) # Exclude inherited member
end
end
strong_memoize_attr(:private_invited_group_members)
+ def source_traversal_ids
+ if source.is_a?(Project)
+ source.namespace.traversal_ids
+ else
+ source.traversal_ids
+ end
+ end
+ strong_memoize_attr(:source_traversal_ids)
+
def can_admin_members?
return can?(current_user, :admin_project_member, source) if source.is_a?(Project)
diff --git a/spec/features/groups/members/list_members_spec.rb b/spec/features/groups/members/list_members_spec.rb
index b6e0deb2e7335472d725fc8b5a213f2c0f4a4ab4..63d201be52ade9b141bc86437940fd1756aeb60a 100644
--- a/spec/features/groups/members/list_members_spec.rb
+++ b/spec/features/groups/members/list_members_spec.rb
@@ -34,6 +34,40 @@
expect(second_row).to be_blank
end
+ context 'for private groups' do
+ let(:group) { create(:group, :private, developers: user1) }
+ let(:nested_group) { create(:group, :private, parent: group, developers: user2) }
+
+ before do
+ sign_in(user2)
+ end
+
+ shared_examples 'direct or inherited member' do
+ it 'sees the sources of inherited members' do
+ visit group_group_members_path(nested_group)
+
+ expect(first_row.text).to include(user1.name, group.name)
+ expect(second_row.text).to include(user2.name)
+ end
+ end
+
+ context 'when signed in using parent group member' do
+ before do
+ sign_in(user1)
+ end
+
+ it_behaves_like 'direct or inherited member'
+ end
+
+ context 'when signed in using subgroup member' do
+ before do
+ sign_in(user2)
+ end
+
+ it_behaves_like 'direct or inherited member'
+ end
+ end
+
describe 'showing status of members' do
before do
group.add_developer(user2)
diff --git a/spec/models/members/members/invited_private_group_accessibility_assigner_spec.rb b/spec/models/members/members/invited_private_group_accessibility_assigner_spec.rb
index ac787d11ef690de734becaba997837ff3147343d..ef32c8fbb19f8517b8f579cf19e8106b7f093493 100644
--- a/spec/models/members/members/invited_private_group_accessibility_assigner_spec.rb
+++ b/spec/models/members/members/invited_private_group_accessibility_assigner_spec.rb
@@ -14,28 +14,51 @@
subject(:assigner) { described_class.new(members, source: source, current_user: current_user) }
+ shared_examples 'sets is_source_accessible_to_current_user to true for all members' do
+ specify do
+ assigner.execute
+
+ expect(members.first.is_source_accessible_to_current_user).to eq(true)
+ end
+ end
+
context 'for direct members' do
- let_it_be(:source) { create(source_type) } # rubocop:disable Rails/SaveBang -- Using factory, not ActiveRecord
- let_it_be(:direct_member) { source.add_developer(member_user) }
- let(:members) { [direct_member] }
+ where(source_visibility: %i[public private])
- it 'sets is_source_accessible_to_current_user to true for all members' do
- assigner.execute
+ with_them do
+ let(:source) { create(source_type, source_visibility) }
+ let(:direct_member) { source.add_developer(member_user) }
+ let(:members) { [direct_member] }
+ it_behaves_like 'sets is_source_accessible_to_current_user to true for all members'
- expect(members.map(&:is_source_accessible_to_current_user)).to all(be_truthy)
+ context 'with webui_members_inherited_users feature flag disabled' do
+ before do
+ stub_feature_flags(webui_members_inherited_users: false)
+ end
+
+ it_behaves_like 'sets is_source_accessible_to_current_user to true for all members'
+ end
end
end
context 'for inherited members' do
- let_it_be(:parent) { create(:group) }
- let_it_be(:source) { create(source_type, parent_key => parent) }
- let_it_be(:inherited_member) { parent.add_developer(member_user) }
- let(:members) { [inherited_member] }
+ where(source_visibility: %i[public private])
- it 'sets is_source_accessible_to_current_user to true for all members' do
- assigner.execute
+ with_them do
+ let(:parent) { create(:group, source_visibility) }
+ let(:source) { create(source_type, source_visibility, parent_key => parent) }
+ let(:inherited_member) { parent.add_developer(member_user) }
+ let(:members) { [inherited_member] }
- expect(members.first.is_source_accessible_to_current_user).to eq(true)
+ it_behaves_like 'sets is_source_accessible_to_current_user to true for all members'
+
+ context 'with webui_members_inherited_users feature flag disabled' do
+ before do
+ stub_feature_flags(webui_members_inherited_users: false)
+ end
+
+ it_behaves_like 'sets is_source_accessible_to_current_user to true for all members'
+ end
end
end
@@ -56,6 +79,14 @@
expect(members.first.is_source_accessible_to_current_user).to eq(can_see_invited_members_source?)
end
+ context 'with webui_members_inherited_users feature flag disabled' do
+ before do
+ stub_feature_flags(webui_members_inherited_users: false)
+ end
+
+ it_behaves_like 'sets is_source_accessible_to_current_user to true for all members'
+ end
+
context 'with multiple members belonging to the same source' do
it 'avoid N+1 queries' do
assigner # Initialize objects in let blocks
@@ -107,8 +138,7 @@
context 'when current user is a direct member of shared group and of invited group through sharing' do
before do
- group = create(:group, :private)
- group.add_developer(current_user)
+ group = create(:group, :private, developers: current_user)
create(:group_group_link, shared_group: invited_group, shared_with_group: group)
end