diff --git a/app/finders/group_members_finder.rb b/app/finders/group_members_finder.rb
index 639db58b00d2718095d045e601a245da9294ab9a..0336135835ac5f94c35b3264cbf2e8c7be59b4ce 100644
--- a/app/finders/group_members_finder.rb
+++ b/app/finders/group_members_finder.rb
@@ -20,6 +20,8 @@ class GroupMembersFinder < UnionFinder
# search: string
# created_after: datetime
# created_before: datetime
+ # non_invite: boolean
+ # with_custom_role: boolean
attr_reader :params
def initialize(group, user = nil, params: {})
@@ -34,7 +36,10 @@ def execute(include_relations: DEFAULT_RELATIONS)
Group.shared_into_ancestors(group).public_or_visible_to_user(user)
end
- members = all_group_members(groups, shared_from_groups).distinct_on_user_with_max_access_level
+ members = all_group_members(groups, shared_from_groups)
+ if static_roles_only?
+ members = members.distinct_on_user_with_max_access_level
+ end
filter_members(members)
end
@@ -70,7 +75,10 @@ def filter_members(members)
members = filter_by_user_type(members)
members = apply_additional_filters(members)
- by_created_at(members)
+ members = by_created_at(members)
+ members = members.non_invite if params[:non_invite]
+
+ members
end
def can_manage_members
@@ -137,6 +145,10 @@ def apply_additional_filters(members)
# overridden in EE to include additional filtering conditions.
members
end
+
+ def static_roles_only?
+ true
+ end
end
GroupMembersFinder.prepend_mod_with('GroupMembersFinder')
diff --git a/config/feature_flags/development/custom_roles_in_members_page.yml b/config/feature_flags/development/custom_roles_in_members_page.yml
new file mode 100644
index 0000000000000000000000000000000000000000..cb6bea5ca421ba58a3e83e40b7d9a2bb0b37cddd
--- /dev/null
+++ b/config/feature_flags/development/custom_roles_in_members_page.yml
@@ -0,0 +1,8 @@
+---
+name: custom_roles_in_members_page
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/128491
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/422897
+milestone: '16.3'
+type: development
+group: group::authentication and authorization
+default_enabled: false
diff --git a/ee/app/controllers/ee/groups/group_members_controller.rb b/ee/app/controllers/ee/groups/group_members_controller.rb
index f4612e4ee729909391cbefe6659dcb41bd3d088d..6835c0153aac02781ca01351754fd5589eaf7990 100644
--- a/ee/app/controllers/ee/groups/group_members_controller.rb
+++ b/ee/app/controllers/ee/groups/group_members_controller.rb
@@ -33,7 +33,14 @@ def admin_not_required_endpoints
def index
super
- @banned = presented_banned_members # rubocop:disable Gitlab/ModuleWithInstanceVariables
+ # rubocop:disable Gitlab/ModuleWithInstanceVariables
+ @banned = presented_banned_members
+ @memberships_with_custom_role = if ::Feature.enabled?(:custom_roles_in_members_page, group)
+ present_group_members(group_memberships_with_custom_role)
+ else
+ []
+ end
+ # rubocop:enable Gitlab/ModuleWithInstanceVariables
end
# rubocop:disable Gitlab/ModuleWithInstanceVariables
@@ -115,6 +122,13 @@ def presented_banned_members
present_members(banned_members(params: filter_params))
end
+ def group_memberships_with_custom_role
+ @memberships_with_custom_role ||=
+ ::GroupMembersFinder
+ .new(group, current_user, params: filter_params.merge({ non_invite: true, with_custom_role: true }))
+ .execute(include_relations: requested_relations)
+ end
+
def authorize_update_group_member!
unless can?(current_user, :admin_group_member, group) || can?(current_user, :override_group_member, group)
render_403
diff --git a/ee/app/finders/ee/group_members_finder.rb b/ee/app/finders/ee/group_members_finder.rb
index d45b046266569fd5a7bd49606a2950d0156a31a5..238e6dc0aa2f43dcb02e161138dda87ccc0ee084 100644
--- a/ee/app/finders/ee/group_members_finder.rb
+++ b/ee/app/finders/ee/group_members_finder.rb
@@ -8,6 +8,14 @@ module EE::GroupMembersFinder
attr_reader :group
end
+ override :execute
+ def execute(include_relations: ::GroupMembersFinder::DEFAULT_RELATIONS)
+ members = super
+ members = members.with_custom_role if params[:with_custom_role]
+
+ members
+ end
+
# rubocop: disable CodeReuse/ActiveRecord
def not_managed
group.group_members.non_owners.joins(:user).merge(User.not_managed(group: group))
@@ -49,4 +57,9 @@ def filter_by_enterprise_users(members)
def can_filter_by_enterprise?
can_manage_members && group.root_ancestor.saml_enabled?
end
+
+ override :static_roles_only?
+ def static_roles_only?
+ !params[:with_custom_role]
+ end
end
diff --git a/ee/app/models/ee/member.rb b/ee/app/models/ee/member.rb
index fd5c47f2a377c9978a4d599308cc72db78b45aa5..03e1fd46723aaf9e43c4c71945760c465cf638e9 100644
--- a/ee/app/models/ee/member.rb
+++ b/ee/app/models/ee/member.rb
@@ -37,6 +37,8 @@ module Member
where.not(user_id: ::Namespaces::NamespaceBan.where(namespace: namespace).select(:user_id))
end
+ scope :with_custom_role, -> { where.not(member_role_id: nil) }
+
scope :elevated_guests, -> do
where(access_level: ::Gitlab::Access::GUEST).joins(:member_role).merge(MemberRole.elevating)
end
diff --git a/ee/spec/controllers/groups/group_members_controller_spec.rb b/ee/spec/controllers/groups/group_members_controller_spec.rb
index 2b0314acd0de56a786286db17195f2b4fb384781..cd5df43a62bb58f1e9ec8ce468b3cab57cdb8ea8 100644
--- a/ee/spec/controllers/groups/group_members_controller_spec.rb
+++ b/ee/spec/controllers/groups/group_members_controller_spec.rb
@@ -43,6 +43,85 @@
expect(count_queries).to be(false)
end
end
+
+ context 'when querying customizable roles' do
+ let_it_be(:root_group, reload: true) { create(:group) }
+ let_it_be(:sub_group, reload: true) { create(:group, parent: root_group) }
+ let_it_be(:sub_2_group, reload: true) { create(:group, parent: sub_group) }
+ let_it_be(:sub_3_group, reload: true) { create(:group, parent: sub_2_group) }
+
+ before do
+ sign_in(user)
+ end
+
+ context 'when there is no customizable role' do
+ it 'returns no membership' do
+ user = create(:user)
+
+ create(:group_member, user: user, group: sub_2_group, access_level: Gitlab::Access::OWNER)
+ create(:group_member, user: user, group: sub_group, access_level: Gitlab::Access::MAINTAINER)
+ create(:group_member, user: user, group: root_group, access_level: Gitlab::Access::DEVELOPER)
+
+ get :index, params: { group_id: sub_3_group }
+
+ expect(assigns(:memberships_with_custom_role)).to be_empty
+ end
+ end
+
+ context 'when there are customizable roles defined' do
+ let_it_be(:member_user) { create(:user) }
+ let_it_be(:sub_group_membership) do
+ maintainer_member_role = create(:member_role, { name: 'custom maintainer',
+ namespace: root_group,
+ base_access_level: ::Gitlab::Access::MAINTAINER })
+ create(:group_member, { user: member_user,
+ group: sub_group,
+ access_level: Gitlab::Access::MAINTAINER,
+ member_role: maintainer_member_role })
+ end
+
+ let_it_be(:custom_owner_role) do
+ create(:member_role, { name: 'custom owner',
+ namespace: root_group,
+ base_access_level: ::Gitlab::Access::OWNER })
+ end
+
+ let_it_be(:sub_2_group_membership) do
+ create(:group_member, { user: member_user,
+ group: sub_2_group,
+ access_level: Gitlab::Access::OWNER,
+ member_role: custom_owner_role })
+ end
+
+ let_it_be(:invited_membership) do
+ create(:group_member, :invited, { user: member_user,
+ group: sub_3_group,
+ access_level: Gitlab::Access::OWNER,
+ member_role: custom_owner_role })
+ end
+
+ it 'queries all customizable role of a user' do
+ create(:group_member, user: member_user, group: root_group, access_level: Gitlab::Access::DEVELOPER)
+
+ get :index, params: { group_id: sub_3_group }
+
+ expect(assigns(:memberships_with_custom_role).map(&:id))
+ .to(contain_exactly(sub_group_membership.id, sub_2_group_membership.id))
+ end
+
+ context 'when custom_roles_members_page is disabled' do
+ before do
+ stub_feature_flags(custom_roles_in_members_page: false)
+ end
+
+ it 'returns no membership' do
+ get :index, params: { group_id: sub_3_group }
+
+ expect(assigns(:memberships_with_custom_role)).to be_empty
+ end
+ end
+ end
+ end
end
describe 'DELETE #leave' do
diff --git a/ee/spec/finders/ee/group_members_finder_spec.rb b/ee/spec/finders/ee/group_members_finder_spec.rb
index d52ac0113c927fd648495b625064837137360086..a7ab1d903f1db6d6f4b178896669401179e04f03 100644
--- a/ee/spec/finders/ee/group_members_finder_spec.rb
+++ b/ee/spec/finders/ee/group_members_finder_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe GroupMembersFinder do
+RSpec.describe GroupMembersFinder, feature_category: :groups_and_projects do
subject(:finder) { described_class.new(group) }
let_it_be(:group) { create :group }
@@ -21,6 +21,145 @@
end
describe '#execute' do
+ context 'with custom roles' do
+ let_it_be(:group) { create(:group) }
+ let_it_be(:sub_group) { create(:group, parent: group) }
+ let_it_be(:sub_sub_group) { create(:group, parent: sub_group) }
+ let_it_be(:public_shared_group) { create(:group, :public) }
+ let_it_be(:private_shared_group) { create(:group, :private) }
+ let_it_be(:user1) { create(:user) }
+ let_it_be(:user2) { create(:user) }
+ let_it_be(:user3) { create(:user) }
+ let_it_be(:user4) { create(:user) }
+ let_it_be(:user5_2fa) { create(:user, :two_factor_via_otp) }
+
+ let_it_be(:link) do
+ create(:group_group_link, shared_group: group, shared_with_group: public_shared_group)
+ create(:group_group_link, shared_group: sub_group, shared_with_group: private_shared_group)
+ end
+
+ let(:groups) do
+ {
+ group: group,
+ sub_group: sub_group,
+ sub_sub_group: sub_sub_group,
+ public_shared_group: public_shared_group,
+ private_shared_group: private_shared_group
+ }
+ end
+
+ let_it_be(:members) do
+ group_custom_maintainer_role = create(:member_role, { name: 'custom maintainer',
+ namespace: group,
+ base_access_level: ::Gitlab::Access::MAINTAINER })
+ group_custom_developer_role = create(:member_role, { name: 'custom developer',
+ namespace: group,
+ base_access_level: ::Gitlab::Access::DEVELOPER })
+ group_custom_reporter_role = create(:member_role, { name: 'custom reporter',
+ namespace: group,
+ base_access_level: ::Gitlab::Access::REPORTER })
+ public_shared_group_custom_maintainer_role = create(:member_role, { name: 'custom maintainer',
+ namespace: public_shared_group,
+ base_access_level: ::Gitlab::Access::MAINTAINER })
+ public_shared_group_custom_developer_role = create(:member_role, { name: 'custom developer',
+ namespace: public_shared_group,
+ base_access_level: ::Gitlab::Access::DEVELOPER })
+ public_shared_group_custom_reporter_role = create(:member_role, { name: 'custom reporter',
+ namespace: public_shared_group,
+ base_access_level: ::Gitlab::Access::REPORTER })
+ private_shared_group_custom_maintainer_role = create(:member_role, { name: 'custom maintainer',
+ namespace: private_shared_group,
+ base_access_level: ::Gitlab::Access::MAINTAINER })
+ private_shared_group_custom_developer_role = create(:member_role, { name: 'custom developer',
+ namespace: private_shared_group,
+ base_access_level: ::Gitlab::Access::DEVELOPER })
+ private_shared_group_custom_reporter_role = create(:member_role, { name: 'custom reporter',
+ namespace: private_shared_group,
+ base_access_level: ::Gitlab::Access::REPORTER })
+ {
+ user1_sub_sub_group: create(:group_member, :maintainer, group: sub_sub_group, user: user1, member_role: group_custom_maintainer_role),
+ user1_sub_group: create(:group_member, :developer, group: sub_group, user: user1, member_role: group_custom_developer_role),
+ user1_group: create(:group_member, :reporter, group: group, user: user1, member_role: group_custom_reporter_role),
+ user1_public_shared_group: create(:group_member, :maintainer, group: public_shared_group, user: user1, member_role: public_shared_group_custom_maintainer_role),
+ user1_private_shared_group: create(:group_member, :maintainer, group: private_shared_group, user: user1, member_role: private_shared_group_custom_maintainer_role),
+ user2_sub_sub_group: create(:group_member, :reporter, group: sub_sub_group, user: user2, member_role: group_custom_reporter_role),
+ user2_sub_group: create(:group_member, :developer, group: sub_group, user: user2, member_role: group_custom_developer_role),
+ user2_group: create(:group_member, :maintainer, group: group, user: user2, member_role: group_custom_maintainer_role),
+ user2_public_shared_group: create(:group_member, :developer, group: public_shared_group, user: user2, member_role: public_shared_group_custom_developer_role),
+ user2_private_shared_group: create(:group_member, :developer, group: private_shared_group, user: user2, member_role: private_shared_group_custom_developer_role),
+ user3_sub_sub_group: create(:group_member, :developer, group: sub_sub_group, user: user3, expires_at: 1.day.from_now, member_role: group_custom_developer_role),
+ user3_sub_group: create(:group_member, :developer, group: sub_group, user: user3, expires_at: 2.days.from_now, member_role: group_custom_developer_role),
+ user3_group: create(:group_member, :reporter, group: group, user: user3, member_role: group_custom_reporter_role),
+ user3_public_shared_group: create(:group_member, :reporter, group: public_shared_group, user: user3, member_role: public_shared_group_custom_reporter_role),
+ user3_private_shared_group: create(:group_member, :reporter, group: private_shared_group, user: user3, member_role: private_shared_group_custom_reporter_role),
+ user4_sub_sub_group: create(:group_member, :reporter, group: sub_sub_group, user: user4, member_role: group_custom_reporter_role),
+ user4_sub_group: create(:group_member, :developer, group: sub_group, user: user4, expires_at: 1.day.from_now, member_role: group_custom_developer_role),
+ user4_group: create(:group_member, :developer, group: group, user: user4, expires_at: 2.days.from_now, member_role: group_custom_developer_role),
+ user4_public_shared_group: create(:group_member, :developer, group: public_shared_group, user: user4, member_role: public_shared_group_custom_developer_role),
+ user4_private_shared_group: create(:group_member, :developer, group: private_shared_group, user: user4, member_role: private_shared_group_custom_developer_role),
+ user5_private_shared_group: create(:group_member, :developer, group: private_shared_group, user: user5_2fa, member_role: private_shared_group_custom_developer_role)
+ }
+ end
+
+ using RSpec::Parameterized::TableSyntax
+
+ # rubocop: disable Layout/ArrayAlignment
+ where(:subject_relations, :subject_group, :expected_members) do
+ [] | :group | []
+ GroupMembersFinder::DEFAULT_RELATIONS | :group | [:user1_group, :user2_group, :user3_group, :user4_group]
+ [:direct] | :group | [:user1_group, :user2_group, :user3_group, :user4_group]
+ [:inherited] | :group | []
+ [:descendants] | :group | [:user1_sub_group, :user1_sub_sub_group,
+ :user2_sub_group, :user2_sub_sub_group,
+ :user3_sub_group, :user3_sub_sub_group,
+ :user4_sub_group, :user4_sub_sub_group]
+ [:shared_from_groups] | :group | [:user1_public_shared_group, :user2_public_shared_group, :user3_public_shared_group, :user4_public_shared_group]
+ [:direct, :inherited, :descendants, :shared_from_groups] | :group | [:user1_group, :user1_sub_group, :user1_sub_sub_group, :user1_public_shared_group,
+ :user2_group, :user2_sub_group, :user2_sub_sub_group, :user2_public_shared_group,
+ :user3_group, :user3_sub_group, :user3_sub_sub_group, :user3_public_shared_group,
+ :user4_group, :user4_sub_group, :user4_sub_sub_group, :user4_public_shared_group]
+ [] | :sub_group | []
+ GroupMembersFinder::DEFAULT_RELATIONS | :sub_group | [:user1_group, :user1_sub_group,
+ :user2_group, :user2_sub_group,
+ :user3_group, :user3_sub_group,
+ :user4_group, :user4_sub_group]
+ [: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 | [: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_group, :user1_sub_group, :user1_sub_sub_group, :user1_public_shared_group,
+ :user2_group, :user2_sub_group, :user2_sub_sub_group, :user2_public_shared_group,
+ :user3_group, :user3_sub_group, :user3_sub_sub_group, :user3_public_shared_group,
+ :user4_group, :user4_sub_group, :user4_sub_sub_group, :user4_public_shared_group]
+ [] | :sub_sub_group | []
+ GroupMembersFinder::DEFAULT_RELATIONS | :sub_sub_group | [:user1_group, :user1_sub_group, :user1_sub_sub_group,
+ :user2_group, :user2_sub_group, :user2_sub_sub_group,
+ :user3_group, :user3_sub_group, :user3_sub_sub_group,
+ :user4_group, :user4_sub_group, :user4_sub_sub_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_group, :user1_sub_group,
+ :user2_group, :user2_sub_group,
+ :user3_group, :user3_sub_group,
+ :user4_group, :user4_sub_group]
+ [:descendants] | :sub_sub_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_group, :user1_sub_group, :user1_sub_sub_group, :user1_public_shared_group,
+ :user2_group, :user2_sub_group, :user2_sub_sub_group, :user2_public_shared_group,
+ :user3_group, :user3_sub_group, :user3_sub_sub_group, :user3_public_shared_group,
+ :user4_group, :user4_sub_group, :user4_sub_sub_group, :user4_public_shared_group]
+ end
+ # rubocop: enable Layout/ArrayAlignment
+ with_them do
+ it 'returns correct members' do
+ result = described_class
+ .new(groups[subject_group], params: { with_custom_role: true })
+ .execute(include_relations: subject_relations)
+
+ expect(result.to_a).to match_array(expected_members.map { |name| members[name] })
+ end
+ end
+ end
+
context 'minimal access' do
let_it_be(:group_minimal_access_membership) do
create(:group_member, :minimal_access, source: group)
diff --git a/ee/spec/models/member_spec.rb b/ee/spec/models/member_spec.rb
index 0d0ae1bd1f7ee09bece31aa9d4ea3904926f6db9..193ac9dec633b6c36d7491a7f104a40986342ab3 100644
--- a/ee/spec/models/member_spec.rb
+++ b/ee/spec/models/member_spec.rb
@@ -168,6 +168,27 @@
end
end
+ describe 'scopes' do
+ describe '.with_custom_role' do
+ let_it_be(:membership_with_custom_role) do
+ member_role = create(:member_role, { name: 'custom maintainer',
+ namespace: group,
+ base_access_level: ::Gitlab::Access::MAINTAINER })
+
+ create(:group_member, group: sub_group, access_level: ::Gitlab::Access::MAINTAINER, member_role: member_role)
+ end
+
+ let_it_be(:membership_without_custom_role) do
+ create(:group_member, group: sub_group, access_level: ::Gitlab::Access::MAINTAINER)
+ end
+
+ subject { described_class.with_custom_role }
+
+ it { is_expected.to include(membership_with_custom_role) }
+ it { is_expected.not_to include(membership_without_custom_role) }
+ end
+ end
+
describe '#notification_service' do
it 'returns a NullNotificationService instance for LDAP users' do
member = described_class.new
diff --git a/spec/finders/group_members_finder_spec.rb b/spec/finders/group_members_finder_spec.rb
index 18473a5e70bb4872d52e79bf747132d2bb5f2732..b8a5be4424178e34d428832b17c230ac73f128cb 100644
--- a/spec/finders/group_members_finder_spec.rb
+++ b/spec/finders/group_members_finder_spec.rb
@@ -288,4 +288,39 @@
end
end
end
+
+ context 'filter by non-invite' do
+ let_it_be(:member) { group.add_maintainer(user1) }
+ let_it_be(:invited_member) do
+ create(:group_member, :invited, { user: user2, group: group })
+ end
+
+ context 'params is not passed in' do
+ subject { described_class.new(group, user1).execute }
+
+ it 'does not filter members by invite' do
+ expect(subject).to match_array([member, invited_member])
+ end
+ end
+
+ context 'params is passed in' do
+ subject { described_class.new(group, user1, params: { non_invite: non_invite_param }).execute }
+
+ context 'filtering is set to false' do
+ let(:non_invite_param) { false }
+
+ it 'does not filter members by invite' do
+ expect(subject).to match_array([member, invited_member])
+ end
+ end
+
+ context 'filtering is set to true' do
+ let(:non_invite_param) { true }
+
+ it 'filters members by invite' do
+ expect(subject).to match_array([member])
+ end
+ end
+ end
+ end
end