Commit d5774198 authored by Małgorzata Ksionek's avatar Małgorzata Ksionek Committed by manojmj

Add minimal access users in UI

Update admin views

Add specs for group

Fix little things

Fix

Fix

Rename to minimal access

From unassigned, in all related files

fix

Add cr remarks

Start with new concept

Update scope per maintainer recommendation

Rename to minimal access

From unassigned, in all related files

fix

Do not show unassigned members on admin group page

Update admin views

Add specs for group

Fix little things

Add new unassigned access role

Add comment to mark one line
as proof of concept

Change validation call

Filter limited access users for project members finder

In some finders, we want to limit users to
active ones.

Add specs for creating project authorizations

As we create authorizations in different places,
we need to add specs in different places to cover
all the cases.

Change scope definition

Fix the spec naming

Big rename to unassigned

Change all limited_access naming to unassigned

Fix rubocop error

Fix naming

Filter out unassigned from groupmembers finder

Modify spec to reflect changes made in group members finder

Add new option to member associations in UI

And stop displaying unassigned members on group page

Fix project presenter problem

Remove unassigned users from group members counter

Add validation that differ between tiers

Remove unused method

Update group members helpers

Add differentiating for top group in proper tier
 when it comes to unassigned members listing

Add specs for members finder

WIP

Add specs to members presenter

And other required changes

Add user specs

Fix specs

Fix rubocop issues

Update admin group view

Add specs for group

Fix rubocop issues

General cleaning

WIP

Add saml provider validation

Fix

Update user method

Add changelog entry

Rename unassigned to minimal access

In all relevant files

Add cr remarks

Update files after rebase

FIX
parent 2ec094d7
......@@ -17,9 +17,8 @@ class GroupMembersFinder < UnionFinder
@params = params
end
# rubocop: disable CodeReuse/ActiveRecord
def execute(include_relations: [:inherited, :direct])
group_members = group.members
group_members = group_members_list
relations = []
return group_members if include_relations == [:direct]
......@@ -27,17 +26,13 @@ class GroupMembersFinder < UnionFinder
relations << group_members if include_relations.include?(:direct)
if include_relations.include?(:inherited) && group.parent
parents_members = GroupMember.non_request.non_minimal_access
.where(source_id: group.ancestors.select(:id))
.where.not(user_id: group.users.select(:id))
parents_members = relation_group_members(group.ancestors)
relations << parents_members
end
if include_relations.include?(:descendants)
descendant_members = GroupMember.non_request.non_minimal_access
.where(source_id: group.descendants.select(:id))
.where.not(user_id: group.users.select(:id))
descendant_members = relation_group_members(group.descendants)
relations << descendant_members
end
......@@ -47,7 +42,6 @@ class GroupMembersFinder < UnionFinder
members = find_union(relations, GroupMember)
filter_members(members)
end
# rubocop: enable CodeReuse/ActiveRecord
private
......@@ -67,6 +61,18 @@ class GroupMembersFinder < UnionFinder
def can_manage_members
Ability.allowed?(user, :admin_group_member, group)
end
def group_members_list
group.members
end
# rubocop: disable CodeReuse/ActiveRecord
def relation_group_members(relation)
GroupMember.non_request.non_minimal_access
.where(source_id: relation.select(:id))
.where.not(user_id: group.users.select(:id))
end
# rubocop: enable CodeReuse/ActiveRecord
end
GroupMembersFinder.prepend_if_ee('EE::GroupMembersFinder')
......@@ -10,7 +10,11 @@ module Groups::GroupMembersHelper
end
def render_invite_member_for_group(group, default_access_level)
render 'shared/members/invite_member', submit_url: group_group_members_path(group), access_levels: GroupMember.access_level_roles, default_access_level: default_access_level
render 'shared/members/invite_member', submit_url: group_group_members_path(group), access_levels: access_level_roles(group), default_access_level: default_access_level
end
def access_level_roles(group)
GroupMember.access_level_roles
end
def linked_groups_data_json(group_links)
......
......@@ -348,6 +348,7 @@ class Group < Namespace
end
group_hierarchy_members = GroupMember.active_without_invites_and_requests
.non_minimal_access
.where(source_id: source_ids)
GroupMember.from_union([group_hierarchy_members,
......@@ -574,6 +575,14 @@ class Group < Namespace
owners.first || parent&.default_owner || owner
end
def access_level_roles_for_group
GroupMember.access_level_roles
end
def access_level_values_for_group
access_level_roles_for_group.values
end
private
def update_two_factor_requirement
......
......@@ -134,6 +134,8 @@ class User < ApplicationRecord
-> { where(members: { access_level: [Gitlab::Access::REPORTER, Gitlab::Access::DEVELOPER, Gitlab::Access::MAINTAINER, Gitlab::Access::OWNER] }) },
through: :group_members,
source: :group
has_many :minimal_access_group_members, -> { where(access_level: [Gitlab::Access::MINIMAL_ACCESS]) }, source: 'GroupMember', class_name: 'GroupMember'
has_many :minimal_access_groups, through: :minimal_access_group_members, source: :group
# Projects
has_many :groups_projects, through: :groups, source: :projects
......
......@@ -113,7 +113,7 @@
%div
= users_select_tag(:user_ids, multiple: true, email_user: true, skip_ldap: @group.ldap_synced?, scope: :all)
.gl-mt-3
= select_tag :access_level, options_for_select(GroupMember.access_level_roles), class: "project-access-select select2"
= select_tag :access_level, options_for_select(@group.access_level_roles_for_group), class: "project-access-select select2"
%hr
= button_tag _('Add users to group'), class: "btn btn-success"
= render 'shared/members/requests', membership_source: @group, requesters: @requesters, force_mobile_view: true
......
---
title: Add No Access Role for top group members
merge_request: 40942
author:
type: added
......@@ -4,6 +4,8 @@
module EE
module Admin
module GroupsController
extend ::Gitlab::Utils::Override
def reset_runners_minutes
group
......@@ -25,6 +27,13 @@ module EE
]
end
override :group_members
def group_members
return @group.all_group_members if @group.minimal_access_role_allowed?
@group.members
end
def groups
super.with_deletion_schedule
end
......
......@@ -2,6 +2,7 @@
module EE::GroupMembersFinder
extend ActiveSupport::Concern
extend ::Gitlab::Utils::Override
prepended do
attr_reader :group
......@@ -12,4 +13,24 @@ module EE::GroupMembersFinder
group.group_members.non_owners.joins(:user).merge(User.not_managed(group: group))
end
# rubocop: enable CodeReuse/ActiveRecord
override :group_members_list
def group_members_list
return group.all_group_members if group.minimal_access_role_allowed?
group.members
end
override :relation_group_members
# rubocop: disable CodeReuse/ActiveRecord
def relation_group_members(relation)
members = ::GroupMember.non_request
.where(source_id: relation.select(:id))
.where.not(user_id: group.users.select(:id))
return members if group.minimal_access_role_allowed?
members.non_minimal_access
end
# rubocop: enable CodeReuse/ActiveRecord
end
......@@ -8,6 +8,11 @@ module EE::Groups::GroupMembersHelper
super.merge(skip_ldap: @group.ldap_synced?)
end
override :access_level_roles
def access_level_roles(group)
group.access_level_roles_for_group
end
private
override :members_data
......
......@@ -408,6 +408,21 @@ module EE
minimal_access_role_allowed? ? ::Gitlab::Access::MINIMAL_ACCESS : ::Gitlab::Access::GUEST
end
override :access_level_roles_for_group
def access_level_roles_for_group
levels = ::GroupMember.access_level_roles
return levels unless minimal_access_role_allowed?
levels.merge(::Gitlab::Access::MINIMAL_ACCESS_HASH)
end
override :users_count
def users_count
return all_group_members.count unless minimal_access_role_allowed?
members.count
end
private
def custom_project_templates_group_allowed
......
......@@ -3,9 +3,9 @@
module EE
module GroupMember
extend ActiveSupport::Concern
extend ::Gitlab::Utils::Override
prepended do
extend ::Gitlab::Utils::Override
include UsageStatistics
validate :sso_enforcement, if: :group
......@@ -79,6 +79,14 @@ module EE
private
override :access_level_inclusion
def access_level_inclusion
levels = source.access_level_values_for_group
return if access_level.in?(levels)
errors.add(:access_level, "is not included in the list")
end
def email_does_not_match_any_allowed_domains(email)
_("email '%{email}' does not match the allowed domains of %{email_domains}" %
{ email: email, email_domains: ::Gitlab::Utils.to_exclusive_sentence(group_allowed_email_domains.map(&:domain)) })
......
......@@ -367,6 +367,18 @@ module EE
owns_paid_namespace?(plans: [::Plan::BRONZE, ::Plan::SILVER])
end
# Returns the groups a user has access to, either through a membership or a project authorization
override :authorized_groups
def authorized_groups
::Group.unscoped do
::Group.from_union([
groups,
available_minimal_access_groups,
authorized_projects.joins(:namespace).select('namespaces.*')
])
end
end
protected
override :password_required?
......@@ -397,5 +409,11 @@ module EE
highest_role > ::Gitlab::Access::GUEST
end
def available_minimal_access_groups
return minimal_access_groups if !::Gitlab::CurrentSettings.should_check_namespace_plan? && License.feature_available?(:minimal_access_role)
minimal_access_groups.with_feature_available_in_plan(:minimal_access_role)
end
end
end
......@@ -9,7 +9,8 @@ class SamlProvider < ApplicationRecord
validates :group, presence: true, top_level_group: true
validates :sso_url, presence: true, addressable_url: { schemes: %w(https), ascii_only: true }
validates :certificate_fingerprint, presence: true, certificate_fingerprint: true
validates :default_membership_role, presence: true, inclusion: { in: Gitlab::Access.values }
validates :default_membership_role, presence: true
validate :access_level_inclusion
after_initialize :set_defaults, if: :new_record?
......@@ -82,6 +83,15 @@ class SamlProvider < ApplicationRecord
private
def access_level_inclusion
return errors.add(:default_membership_role, "is dependent on a group") unless group
levels = group.access_level_values_for_group
return if default_membership_role.in?(levels)
errors.add(:default_membership_role, "is not included in the list")
end
def set_defaults
self.enabled = true
end
......
......@@ -2,6 +2,8 @@
module EE
module GroupMemberPresenter
extend ::Gitlab::Utils::Override
def group_sso?
member.user.group_sso?(source)
end
......@@ -10,6 +12,11 @@ module EE
member.user.group_managed_account?
end
override :access_level_roles
def access_level_roles
member.source.access_level_roles_for_group
end
private
def override_member_permission
......
......@@ -18,5 +18,9 @@ module EE
def override_member_permission
raise NotImplementedError
end
def source_allows_minimal_access_role?(member)
member.source.minimal_access_role_allowed?
end
end
end
......@@ -53,7 +53,7 @@
.well-segment.borderless.gl-mb-3.col-12.col-lg-9.gl-p-0
= f.label :default_membership_role, class: 'label-bold' do
= s_('GroupSAML|Default membership role')
= f.select :default_membership_role, options_for_select(::Gitlab::Access.options, saml_provider.default_membership_role), {}, class: 'form-control', data: { qa_selector: 'default_membership_role_dropdown' }
= f.select :default_membership_role, options_for_select(group.access_level_roles_for_group, saml_provider.default_membership_role), {}, class: 'form-control', data: { qa_selector: 'default_membership_role_dropdown' }
.form-text.text-muted
= s_('GroupSAML|This will be set as the access level of users added to the group.')
......
......@@ -10,16 +10,26 @@ module EE
module Access
extend ActiveSupport::Concern
ADMIN = 60
MINIMAL_ACCESS_HASH = { "Minimal Access" => ::Gitlab::Access::MINIMAL_ACCESS }.freeze
class_methods do
extend ::Gitlab::Utils::Override
def vulnerability_access_levels
@vulnerability_access_levels ||= options_with_owner.except('Guest')
end
def options_with_minimal_access
options_with_owner.merge(
"Minimal Access" => ::Gitlab::Access::MINIMAL_ACCESS
)
options_with_owner.merge(MINIMAL_ACCESS_HASH)
end
def values_with_minimal_access
options_with_minimal_access.values
end
override :human_access
def human_access(access)
options_with_minimal_access.key(access)
end
end
end
......
......@@ -19,4 +19,35 @@ RSpec.describe GroupMembersFinder do
expect(finder.not_managed).to eq([group_member_membership])
end
end
describe '#execute' do
let_it_be(:group_minimal_access_membership) do
create(:group_member, :minimal_access, source: group, user: create(:user))
end
context 'when group does not allow minimal access members' do
before do
stub_licensed_features(minimal_access_role: false)
end
it 'returns only members with full access' do
result = finder.execute(include_relations: [:direct, :descendants])
expect(result.to_a).to match_array([group_owner_membership, group_member_membership, dedicated_member_account_membership])
end
end
context 'when group allows minimal access members' do
before do
group.clear_memoization(:feature_available)
stub_licensed_features(minimal_access_role: true)
end
it 'returns only members with minimal access' do
result = finder.execute(include_relations: [:direct, :descendants])
expect(result.to_a).to match_array([group_owner_membership, group_member_membership, dedicated_member_account_membership, group_minimal_access_membership])
end
end
end
end
......@@ -107,6 +107,50 @@ RSpec.describe GroupMember do
end
end
end
describe 'access level inclusion' do
let(:group) { create(:group) }
context 'when minimal access user feature switched on' do
before do
stub_licensed_features(minimal_access_role: true)
end
it 'users can have access levels from minimal access to owner' do
expect(build(:group_member, group: group, user: create(:user), access_level: ::Gitlab::Access::NO_ACCESS)).to be_invalid
expect(build(:group_member, group: group, user: create(:user), access_level: ::Gitlab::Access::MINIMAL_ACCESS)).to be_valid
expect(build(:group_member, group: group, user: create(:user), access_level: ::Gitlab::Access::GUEST)).to be_valid
expect(build(:group_member, group: group, user: create(:user), access_level: ::Gitlab::Access::REPORTER)).to be_valid
expect(build(:group_member, group: group, user: create(:user), access_level: ::Gitlab::Access::DEVELOPER)).to be_valid
expect(build(:group_member, group: group, user: create(:user), access_level: ::Gitlab::Access::MAINTAINER)).to be_valid
expect(build(:group_member, group: group, user: create(:user), access_level: ::Gitlab::Access::OWNER)).to be_valid
end
context 'when group is a subgroup' do
let(:subgroup) { create(:group, parent: group) }
it 'users can have access levels from guest to owner' do
expect(build(:group_member, group: subgroup, user: create(:user), access_level: ::Gitlab::Access::MINIMAL_ACCESS)).to be_invalid
end
end
end
context 'when minimal access user feature switched off' do
before do
stub_licensed_features(minimal_access_role: false)
end
it 'users can have access levels from guest to owner' do
expect(build(:group_member, group: group, user: create(:user), access_level: ::Gitlab::Access::NO_ACCESS)).to be_invalid
expect(build(:group_member, group: group, user: create(:user), access_level: ::Gitlab::Access::MINIMAL_ACCESS)).to be_invalid
expect(build(:group_member, group: group, user: create(:user), access_level: ::Gitlab::Access::GUEST)).to be_valid
expect(build(:group_member, group: group, user: create(:user), access_level: ::Gitlab::Access::REPORTER)).to be_valid
expect(build(:group_member, group: group, user: create(:user), access_level: ::Gitlab::Access::DEVELOPER)).to be_valid
expect(build(:group_member, group: group, user: create(:user), access_level: ::Gitlab::Access::MAINTAINER)).to be_valid
expect(build(:group_member, group: group, user: create(:user), access_level: ::Gitlab::Access::OWNER)).to be_valid
end
end
end
end
describe 'scopes' do
......
......@@ -62,6 +62,42 @@ RSpec.describe SamlProvider do
expect(subject).to allow_value(group).for(:group)
expect(subject).not_to allow_value(nested_group).for(:group)
end
describe 'access level inclusion' do
let(:group) { create(:group) }
context 'when minimal access user feature is switched on' do
before do
stub_licensed_features(minimal_access_role: true)
end
it 'default membership role can have access levels from minimal access to owner' do
expect(build(:saml_provider, group: group, default_membership_role: ::Gitlab::Access::NO_ACCESS)).to be_invalid
expect(build(:saml_provider, group: group, default_membership_role: ::Gitlab::Access::MINIMAL_ACCESS)).to be_valid
expect(build(:saml_provider, group: group, default_membership_role: ::Gitlab::Access::GUEST)).to be_valid
expect(build(:saml_provider, group: group, default_membership_role: ::Gitlab::Access::REPORTER)).to be_valid
expect(build(:saml_provider, group: group, default_membership_role: ::Gitlab::Access::DEVELOPER)).to be_valid
expect(build(:saml_provider, group: group, default_membership_role: ::Gitlab::Access::MAINTAINER)).to be_valid
expect(build(:saml_provider, group: group, default_membership_role: ::Gitlab::Access::OWNER)).to be_valid
end
end
context 'when minimal access user feature switched off' do
before do
stub_licensed_features(minimal_access_role: false)
end
it 'default membership role can have access levels from guest to owner' do
expect(build(:saml_provider, group: group, default_membership_role: ::Gitlab::Access::NO_ACCESS)).to be_invalid
expect(build(:saml_provider, group: group, default_membership_role: ::Gitlab::Access::MINIMAL_ACCESS)).to be_invalid
expect(build(:saml_provider, group: group, default_membership_role: ::Gitlab::Access::GUEST)).to be_valid
expect(build(:saml_provider, group: group, default_membership_role: ::Gitlab::Access::REPORTER)).to be_valid
expect(build(:saml_provider, group: group, default_membership_role: ::Gitlab::Access::DEVELOPER)).to be_valid
expect(build(:saml_provider, group: group, default_membership_role: ::Gitlab::Access::MAINTAINER)).to be_valid
expect(build(:saml_provider, group: group, default_membership_role: ::Gitlab::Access::OWNER)).to be_valid
end
end
end
end
describe 'Default values' do
......
......@@ -1117,6 +1117,48 @@ RSpec.describe User do
end
end
describe '#authorized_groups' do
let_it_be(:user) { create(:user) }
let_it_be(:private_group) { create(:group) }
let_it_be(:child_group) { create(:group, parent: private_group) }
let_it_be(:minimal_access_group) { create(:group) }
let_it_be(:project_group) { create(:group) }
let_it_be(:project) { create(:project, group: project_group) }
before do
private_group.add_user(user, Gitlab::Access::MAINTAINER)
project.add_maintainer(user)
create(:group_member, :minimal_access, user: user, source: minimal_access_group)
end
subject { user.authorized_groups }
context 'with minimal access role feature switched off' do
it { is_expected.to contain_exactly private_group, project_group }
end
context 'with minimal access feature switched on' do
before do
stub_licensed_features(minimal_access_role: true)
allow(Gitlab::CurrentSettings)
.to receive(:should_check_namespace_plan?)
.and_return(false)
end
it { is_expected.to contain_exactly private_group, project_group, minimal_access_group }
end
context 'with minimal access feature switched on for one group only' do
before do
create(:gitlab_subscription, :gold, namespace: minimal_access_group)
create(:group_member, :minimal_access, user: user, source: create(:group))
end
it { is_expected.to contain_exactly private_group, project_group, minimal_access_group }
end
end
describe '#active_for_authentication?' do
subject { user.active_for_authentication? }
......
......@@ -60,4 +60,26 @@ RSpec.describe GroupMemberPresenter do
it { expect(presenter.can_update?).to eq(false) }
end
end
describe '#valid_level_roles?' do
context 'with minimal access role feature switched on' do
before do
allow(group_member).to receive(:highest_group_member)
allow(group_member).to receive_message_chain(:class, :access_level_roles).and_return(::Gitlab::Access.options_with_owner)
expect(group).to receive(:access_level_roles_for_group).and_return(::Gitlab::Access.options_with_minimal_access)
end
it { expect(presenter.valid_level_roles).to eq(::Gitlab::Access.options_with_minimal_access) }
end
context 'with minimal access role feature switched off' do
it_behaves_like '#valid_level_roles', :group do
let(:expected_roles) { { 'Developer' => 30, 'Maintainer' => 40, 'Owner' => 50, 'Reporter' => 20 } }
before do
entity.parent = group
end
end
end
end
end
......@@ -25,7 +25,7 @@ RSpec.shared_examples 'allowed user IDs are cached' do
expect(described_class.l1_cache_backend).to receive(:fetch).and_call_original
expect(described_class.l2_cache_backend).to receive(:fetch).and_call_original
expect(subject).to be_truthy
end.not_to exceed_query_limit(2)
end.not_to exceed_query_limit(3)
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