From d5774198f1be8460412a156d1bb069b84956a43e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Tue, 1 Sep 2020 17:46:18 +0200 Subject: [PATCH 1/9] 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 --- app/finders/group_members_finder.rb | 24 ++++++---- app/helpers/groups/group_members_helper.rb | 6 ++- app/models/group.rb | 9 ++++ app/models/user.rb | 2 + app/views/admin/groups/show.html.haml | 2 +- ...m-sso-create-no-access-role-fe-changes.yml | 5 +++ .../controllers/ee/admin/groups_controller.rb | 9 ++++ ee/app/finders/ee/group_members_finder.rb | 21 +++++++++ .../helpers/ee/groups/group_members_helper.rb | 5 +++ ee/app/models/ee/group.rb | 15 +++++++ ee/app/models/ee/group_member.rb | 10 ++++- ee/app/models/ee/user.rb | 18 ++++++++ ee/app/models/saml_provider.rb | 12 ++++- .../presenters/ee/group_member_presenter.rb | 7 +++ ee/app/presenters/ee/member_presenter.rb | 4 ++ .../groups/saml_providers/_form.html.haml | 2 +- ee/lib/ee/gitlab/access.rb | 16 +++++-- .../finders/ee/group_members_finder_spec.rb | 31 +++++++++++++ ee/spec/models/group_member_spec.rb | 44 +++++++++++++++++++ ee/spec/models/saml_provider_spec.rb | 36 +++++++++++++++ ee/spec/models/user_spec.rb | 42 ++++++++++++++++++ .../presenters/group_member_presenter_spec.rb | 22 ++++++++++ ...lowed_users_in_namespace_shared_context.rb | 2 +- 23 files changed, 326 insertions(+), 18 deletions(-) create mode 100644 changelogs/unreleased/220203-gitlab-com-sso-create-no-access-role-fe-changes.yml diff --git a/app/finders/group_members_finder.rb b/app/finders/group_members_finder.rb index ce0d52ad97a8..55f26d85d1fb 100644 --- a/app/finders/group_members_finder.rb +++ b/app/finders/group_members_finder.rb @@ -17,9 +17,8 @@ def initialize(group, user = nil, params: {}) @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 @@ def execute(include_relations: [:inherited, :direct]) 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 @@ def execute(include_relations: [:inherited, :direct]) members = find_union(relations, GroupMember) filter_members(members) end - # rubocop: enable CodeReuse/ActiveRecord private @@ -67,6 +61,18 @@ def filter_members(members) 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') diff --git a/app/helpers/groups/group_members_helper.rb b/app/helpers/groups/group_members_helper.rb index dcff2be34dac..c12a72bcc6d8 100644 --- a/app/helpers/groups/group_members_helper.rb +++ b/app/helpers/groups/group_members_helper.rb @@ -10,7 +10,11 @@ def group_member_select_options 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) diff --git a/app/models/group.rb b/app/models/group.rb index 01438ebb3a85..c17f4b6cbc14 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -348,6 +348,7 @@ def members_with_parents 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 @@ def default_owner 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 diff --git a/app/models/user.rb b/app/models/user.rb index 9b8fe4881adf..c90743983c2f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -134,6 +134,8 @@ def update_tracked_fields!(request) -> { 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 diff --git a/app/views/admin/groups/show.html.haml b/app/views/admin/groups/show.html.haml index 6c2c0b3a488f..4d9b7c149abc 100644 --- a/app/views/admin/groups/show.html.haml +++ b/app/views/admin/groups/show.html.haml @@ -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 diff --git a/changelogs/unreleased/220203-gitlab-com-sso-create-no-access-role-fe-changes.yml b/changelogs/unreleased/220203-gitlab-com-sso-create-no-access-role-fe-changes.yml new file mode 100644 index 000000000000..d8b42d0bebc6 --- /dev/null +++ b/changelogs/unreleased/220203-gitlab-com-sso-create-no-access-role-fe-changes.yml @@ -0,0 +1,5 @@ +--- +title: Add No Access Role for top group members +merge_request: 40942 +author: +type: added diff --git a/ee/app/controllers/ee/admin/groups_controller.rb b/ee/app/controllers/ee/admin/groups_controller.rb index f49254d80384..ae80521e8006 100644 --- a/ee/app/controllers/ee/admin/groups_controller.rb +++ b/ee/app/controllers/ee/admin/groups_controller.rb @@ -4,6 +4,8 @@ module EE module Admin module GroupsController + extend ::Gitlab::Utils::Override + def reset_runners_minutes group @@ -25,6 +27,13 @@ def allowed_group_params ] 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 diff --git a/ee/app/finders/ee/group_members_finder.rb b/ee/app/finders/ee/group_members_finder.rb index 951963a74540..16b9e8b841a6 100644 --- a/ee/app/finders/ee/group_members_finder.rb +++ b/ee/app/finders/ee/group_members_finder.rb @@ -2,6 +2,7 @@ module EE::GroupMembersFinder extend ActiveSupport::Concern + extend ::Gitlab::Utils::Override prepended do attr_reader :group @@ -12,4 +13,24 @@ def not_managed 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 diff --git a/ee/app/helpers/ee/groups/group_members_helper.rb b/ee/app/helpers/ee/groups/group_members_helper.rb index 81230b5c0579..095a84d816bc 100644 --- a/ee/app/helpers/ee/groups/group_members_helper.rb +++ b/ee/app/helpers/ee/groups/group_members_helper.rb @@ -8,6 +8,11 @@ def group_member_select_options 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 diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb index 8d705e5abc23..d2746c19d08a 100644 --- a/ee/app/models/ee/group.rb +++ b/ee/app/models/ee/group.rb @@ -408,6 +408,21 @@ def minimal_member_access_level 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 diff --git a/ee/app/models/ee/group_member.rb b/ee/app/models/ee/group_member.rb index b87296b9d1f2..bc50dc8632f4 100644 --- a/ee/app/models/ee/group_member.rb +++ b/ee/app/models/ee/group_member.rb @@ -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 @@ def group_saml_identity 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)) }) diff --git a/ee/app/models/ee/user.rb b/ee/app/models/ee/user.rb index b1a78a9bd7aa..27f265b78b87 100644 --- a/ee/app/models/ee/user.rb +++ b/ee/app/models/ee/user.rb @@ -367,6 +367,18 @@ def owns_upgradeable_namespace? 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 @@ def paid_in_current_license? 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 diff --git a/ee/app/models/saml_provider.rb b/ee/app/models/saml_provider.rb index ee995c43cc7d..93c3dce742f6 100644 --- a/ee/app/models/saml_provider.rb +++ b/ee/app/models/saml_provider.rb @@ -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 @@ def to_h 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 diff --git a/ee/app/presenters/ee/group_member_presenter.rb b/ee/app/presenters/ee/group_member_presenter.rb index 3c8524691e0b..451d22794b1f 100644 --- a/ee/app/presenters/ee/group_member_presenter.rb +++ b/ee/app/presenters/ee/group_member_presenter.rb @@ -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 @@ def group_managed_account? 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 diff --git a/ee/app/presenters/ee/member_presenter.rb b/ee/app/presenters/ee/member_presenter.rb index 62be1093e892..29540bc3df50 100644 --- a/ee/app/presenters/ee/member_presenter.rb +++ b/ee/app/presenters/ee/member_presenter.rb @@ -18,5 +18,9 @@ def can_override? def override_member_permission raise NotImplementedError end + + def source_allows_minimal_access_role?(member) + member.source.minimal_access_role_allowed? + end end end diff --git a/ee/app/views/groups/saml_providers/_form.html.haml b/ee/app/views/groups/saml_providers/_form.html.haml index 445007963186..766ac9596b97 100644 --- a/ee/app/views/groups/saml_providers/_form.html.haml +++ b/ee/app/views/groups/saml_providers/_form.html.haml @@ -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.') diff --git a/ee/lib/ee/gitlab/access.rb b/ee/lib/ee/gitlab/access.rb index d65f98921f83..2e48df1c5816 100644 --- a/ee/lib/ee/gitlab/access.rb +++ b/ee/lib/ee/gitlab/access.rb @@ -10,16 +10,26 @@ module Gitlab 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 diff --git a/ee/spec/finders/ee/group_members_finder_spec.rb b/ee/spec/finders/ee/group_members_finder_spec.rb index c388e4eafd4f..7060416949c1 100644 --- a/ee/spec/finders/ee/group_members_finder_spec.rb +++ b/ee/spec/finders/ee/group_members_finder_spec.rb @@ -19,4 +19,35 @@ 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 diff --git a/ee/spec/models/group_member_spec.rb b/ee/spec/models/group_member_spec.rb index 71413c82f216..366b7ef6cbe2 100644 --- a/ee/spec/models/group_member_spec.rb +++ b/ee/spec/models/group_member_spec.rb @@ -107,6 +107,50 @@ 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 diff --git a/ee/spec/models/saml_provider_spec.rb b/ee/spec/models/saml_provider_spec.rb index a54db964030f..a52adc30bfa9 100644 --- a/ee/spec/models/saml_provider_spec.rb +++ b/ee/spec/models/saml_provider_spec.rb @@ -62,6 +62,42 @@ 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 diff --git a/ee/spec/models/user_spec.rb b/ee/spec/models/user_spec.rb index 092d51541f03..2cfb613b3809 100644 --- a/ee/spec/models/user_spec.rb +++ b/ee/spec/models/user_spec.rb @@ -1117,6 +1117,48 @@ 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? } diff --git a/ee/spec/presenters/group_member_presenter_spec.rb b/ee/spec/presenters/group_member_presenter_spec.rb index 68b82be057e4..f27bcf1676b2 100644 --- a/ee/spec/presenters/group_member_presenter_spec.rb +++ b/ee/spec/presenters/group_member_presenter_spec.rb @@ -60,4 +60,26 @@ 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 diff --git a/spec/support/shared_contexts/cache_allowed_users_in_namespace_shared_context.rb b/spec/support/shared_contexts/cache_allowed_users_in_namespace_shared_context.rb index e3c1d0afa535..bfb719fd8405 100644 --- a/spec/support/shared_contexts/cache_allowed_users_in_namespace_shared_context.rb +++ b/spec/support/shared_contexts/cache_allowed_users_in_namespace_shared_context.rb @@ -25,7 +25,7 @@ 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 -- GitLab From b347c7fe506849842d2b062680e5225d72b85d42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Wed, 9 Sep 2020 16:03:59 +0200 Subject: [PATCH 2/9] Update user method --- ee/app/models/ee/user.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ee/app/models/ee/user.rb b/ee/app/models/ee/user.rb index 27f265b78b87..983a68f4c0a4 100644 --- a/ee/app/models/ee/user.rb +++ b/ee/app/models/ee/user.rb @@ -44,6 +44,9 @@ module User has_many :approvals, dependent: :destroy # rubocop: disable Cop/ActiveRecordDependent has_many :approvers, dependent: :destroy # rubocop: disable Cop/ActiveRecordDependent + 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 + has_many :users_ops_dashboard_projects has_many :ops_dashboard_projects, through: :users_ops_dashboard_projects, source: :project has_many :users_security_dashboard_projects -- GitLab From 59e1999bd913e42bf7fbe0898001bd0feb7237bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Wed, 9 Sep 2020 17:44:02 +0200 Subject: [PATCH 3/9] Add cr remarks Fix specs Fix specs --- app/helpers/groups/group_members_helper.rb | 6 +----- app/models/group.rb | 6 +++--- app/views/admin/groups/show.html.haml | 2 +- ee/app/helpers/ee/groups/group_members_helper.rb | 5 ----- ee/app/models/ee/group.rb | 4 ++-- ee/app/models/ee/group_member.rb | 2 +- ee/app/models/saml_provider.rb | 2 +- ee/app/presenters/ee/group_member_presenter.rb | 2 +- ee/app/presenters/ee/member_presenter.rb | 4 ---- ee/app/views/groups/saml_providers/_form.html.haml | 2 +- ee/spec/models/group_member_spec.rb | 2 +- ee/spec/models/group_spec.rb | 2 +- ee/spec/presenters/group_member_presenter_spec.rb | 2 +- 13 files changed, 14 insertions(+), 27 deletions(-) diff --git a/app/helpers/groups/group_members_helper.rb b/app/helpers/groups/group_members_helper.rb index c12a72bcc6d8..64a91ef7dd03 100644 --- a/app/helpers/groups/group_members_helper.rb +++ b/app/helpers/groups/group_members_helper.rb @@ -10,11 +10,7 @@ def group_member_select_options 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: access_level_roles(group), default_access_level: default_access_level - end - - def access_level_roles(group) - GroupMember.access_level_roles + render 'shared/members/invite_member', submit_url: group_group_members_path(group), access_levels: group.access_level_roles, default_access_level: default_access_level end def linked_groups_data_json(group_links) diff --git a/app/models/group.rb b/app/models/group.rb index c17f4b6cbc14..ce4afcfbd06e 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -575,12 +575,12 @@ def default_owner owners.first || parent&.default_owner || owner end - def access_level_roles_for_group + def access_level_roles GroupMember.access_level_roles end - def access_level_values_for_group - access_level_roles_for_group.values + def access_level_values + access_level_roles.values end private diff --git a/app/views/admin/groups/show.html.haml b/app/views/admin/groups/show.html.haml index 4d9b7c149abc..dc43b45195e9 100644 --- a/app/views/admin/groups/show.html.haml +++ b/app/views/admin/groups/show.html.haml @@ -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(@group.access_level_roles_for_group), class: "project-access-select select2" + = select_tag :access_level, options_for_select(@group.access_level_roles), 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 diff --git a/ee/app/helpers/ee/groups/group_members_helper.rb b/ee/app/helpers/ee/groups/group_members_helper.rb index 095a84d816bc..81230b5c0579 100644 --- a/ee/app/helpers/ee/groups/group_members_helper.rb +++ b/ee/app/helpers/ee/groups/group_members_helper.rb @@ -8,11 +8,6 @@ def group_member_select_options 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 diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb index d2746c19d08a..20ba5f214665 100644 --- a/ee/app/models/ee/group.rb +++ b/ee/app/models/ee/group.rb @@ -408,8 +408,8 @@ def minimal_member_access_level minimal_access_role_allowed? ? ::Gitlab::Access::MINIMAL_ACCESS : ::Gitlab::Access::GUEST end - override :access_level_roles_for_group - def access_level_roles_for_group + override :access_level_roles + def access_level_roles levels = ::GroupMember.access_level_roles return levels unless minimal_access_role_allowed? diff --git a/ee/app/models/ee/group_member.rb b/ee/app/models/ee/group_member.rb index bc50dc8632f4..956f14dcd6cc 100644 --- a/ee/app/models/ee/group_member.rb +++ b/ee/app/models/ee/group_member.rb @@ -81,7 +81,7 @@ def group_saml_identity override :access_level_inclusion def access_level_inclusion - levels = source.access_level_values_for_group + levels = source.access_level_values return if access_level.in?(levels) errors.add(:access_level, "is not included in the list") diff --git a/ee/app/models/saml_provider.rb b/ee/app/models/saml_provider.rb index 93c3dce742f6..8b312a66d778 100644 --- a/ee/app/models/saml_provider.rb +++ b/ee/app/models/saml_provider.rb @@ -86,7 +86,7 @@ def to_h def access_level_inclusion return errors.add(:default_membership_role, "is dependent on a group") unless group - levels = group.access_level_values_for_group + levels = group.access_level_values return if default_membership_role.in?(levels) errors.add(:default_membership_role, "is not included in the list") diff --git a/ee/app/presenters/ee/group_member_presenter.rb b/ee/app/presenters/ee/group_member_presenter.rb index 451d22794b1f..668251451bad 100644 --- a/ee/app/presenters/ee/group_member_presenter.rb +++ b/ee/app/presenters/ee/group_member_presenter.rb @@ -14,7 +14,7 @@ def group_managed_account? override :access_level_roles def access_level_roles - member.source.access_level_roles_for_group + member.source.access_level_roles end private diff --git a/ee/app/presenters/ee/member_presenter.rb b/ee/app/presenters/ee/member_presenter.rb index 29540bc3df50..62be1093e892 100644 --- a/ee/app/presenters/ee/member_presenter.rb +++ b/ee/app/presenters/ee/member_presenter.rb @@ -18,9 +18,5 @@ def can_override? def override_member_permission raise NotImplementedError end - - def source_allows_minimal_access_role?(member) - member.source.minimal_access_role_allowed? - end end end diff --git a/ee/app/views/groups/saml_providers/_form.html.haml b/ee/app/views/groups/saml_providers/_form.html.haml index 766ac9596b97..2bff553398df 100644 --- a/ee/app/views/groups/saml_providers/_form.html.haml +++ b/ee/app/views/groups/saml_providers/_form.html.haml @@ -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(group.access_level_roles_for_group, 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, 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.') diff --git a/ee/spec/models/group_member_spec.rb b/ee/spec/models/group_member_spec.rb index 366b7ef6cbe2..ce9a916f8ac0 100644 --- a/ee/spec/models/group_member_spec.rb +++ b/ee/spec/models/group_member_spec.rb @@ -129,7 +129,7 @@ context 'when group is a subgroup' do let(:subgroup) { create(:group, parent: group) } - it 'users can have access levels from guest to owner' do + it 'users cannot have minimal access level' do expect(build(:group_member, group: subgroup, user: create(:user), access_level: ::Gitlab::Access::MINIMAL_ACCESS)).to be_invalid end end diff --git a/ee/spec/models/group_spec.rb b/ee/spec/models/group_spec.rb index c3829d60947a..e727c481e964 100644 --- a/ee/spec/models/group_spec.rb +++ b/ee/spec/models/group_spec.rb @@ -705,7 +705,7 @@ context 'with `minimal_access_role` not licensed' do before do stub_licensed_features(minimal_access_role: false) - create(:group_member, :minimal_access, user: user, group: group) + create(:group_member, :minimal_access, user: user, source: group) end it { is_expected.to be_falsey } diff --git a/ee/spec/presenters/group_member_presenter_spec.rb b/ee/spec/presenters/group_member_presenter_spec.rb index f27bcf1676b2..891df9d44571 100644 --- a/ee/spec/presenters/group_member_presenter_spec.rb +++ b/ee/spec/presenters/group_member_presenter_spec.rb @@ -66,7 +66,7 @@ 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) + expect(group).to receive(:access_level_roles).and_return(::Gitlab::Access.options_with_minimal_access) end it { expect(presenter.valid_level_roles).to eq(::Gitlab::Access.options_with_minimal_access) } -- GitLab From 7ca0cc6480809d273aee854abb00450f5baf2fd1 Mon Sep 17 00:00:00 2001 From: manojmj Date: Mon, 14 Sep 2020 12:45:20 +0530 Subject: [PATCH 4/9] Address code review remarks --- ee/app/models/ee/user.rb | 3 ++- ee/spec/models/user_spec.rb | 32 ++++++++++++++++++++------------ 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/ee/app/models/ee/user.rb b/ee/app/models/ee/user.rb index 983a68f4c0a4..77cb690a21a5 100644 --- a/ee/app/models/ee/user.rb +++ b/ee/app/models/ee/user.rb @@ -414,7 +414,8 @@ def paid_in_current_license? end def available_minimal_access_groups - return minimal_access_groups if !::Gitlab::CurrentSettings.should_check_namespace_plan? && License.feature_available?(:minimal_access_role) + return ::Group.none unless License.feature_available?(:minimal_access_role) + return minimal_access_groups unless ::Gitlab::CurrentSettings.should_check_namespace_plan? minimal_access_groups.with_feature_available_in_plan(:minimal_access_role) end diff --git a/ee/spec/models/user_spec.rb b/ee/spec/models/user_spec.rb index 2cfb613b3809..1d84b09e6c4c 100644 --- a/ee/spec/models/user_spec.rb +++ b/ee/spec/models/user_spec.rb @@ -1134,28 +1134,36 @@ subject { user.authorized_groups } - context 'with minimal access role feature switched off' do + context 'with minimal access role feature unavailable' do it { is_expected.to contain_exactly private_group, project_group } end - context 'with minimal access feature switched on' do + context 'with minimal access feature available' 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 'feature turned on for all groups' do + before do + allow(Gitlab::CurrentSettings) + .to receive(:should_check_namespace_plan?) + .and_return(false) + 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)) + it { is_expected.to contain_exactly private_group, project_group, minimal_access_group } end - it { is_expected.to contain_exactly private_group, project_group, minimal_access_group } + context 'feature available for specific groups only' do + before do + allow(Gitlab::CurrentSettings) + .to receive(:should_check_namespace_plan?) + .and_return(true) + 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 end -- GitLab From 0533f83000351db3d72411ec86cbe50e86090b79 Mon Sep 17 00:00:00 2001 From: manojmj Date: Mon, 14 Sep 2020 14:38:30 +0530 Subject: [PATCH 5/9] Fix bug in group security dashboard --- ee/app/helpers/groups/security_features_helper.rb | 2 +- .../groups/security_features_helper_spec.rb | 7 ++++--- .../layouts/nav/sidebar/_group.html.haml_spec.rb | 15 +++++++++++---- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/ee/app/helpers/groups/security_features_helper.rb b/ee/app/helpers/groups/security_features_helper.rb index fdd928651835..a7cf1cd1aa92 100644 --- a/ee/app/helpers/groups/security_features_helper.rb +++ b/ee/app/helpers/groups/security_features_helper.rb @@ -2,7 +2,7 @@ module Groups::SecurityFeaturesHelper def group_level_security_dashboard_available?(group) - group.feature_available?(:security_dashboard) + can?(current_user, :read_group_security_dashboard, group) end def group_level_compliance_dashboard_available?(group) diff --git a/ee/spec/helpers/groups/security_features_helper_spec.rb b/ee/spec/helpers/groups/security_features_helper_spec.rb index f58a95656c7d..e67868ff4e98 100644 --- a/ee/spec/helpers/groups/security_features_helper_spec.rb +++ b/ee/spec/helpers/groups/security_features_helper_spec.rb @@ -9,18 +9,19 @@ let_it_be(:user, refind: true) { create(:user) } before do + allow(helper).to receive(:can?) { |*args| Ability.allowed?(*args) } allow(helper).to receive(:current_user).and_return(user) end describe '#group_level_security_dashboard_available?' do - where(:security_dashboard_feature_enabled, :result) do - true | true + where(:read_group_security_dashboard_permission, :result) do false | false + true | true end with_them do before do - stub_licensed_features(security_dashboard: security_dashboard_feature_enabled) + allow(helper).to receive(:can?).with(user, :read_group_security_dashboard, group).and_return(read_group_security_dashboard_permission) end it 'returns the expected result' do diff --git a/ee/spec/views/layouts/nav/sidebar/_group.html.haml_spec.rb b/ee/spec/views/layouts/nav/sidebar/_group.html.haml_spec.rb index 355ec66755d7..82a1f7637dee 100644 --- a/ee/spec/views/layouts/nav/sidebar/_group.html.haml_spec.rb +++ b/ee/spec/views/layouts/nav/sidebar/_group.html.haml_spec.rb @@ -109,11 +109,18 @@ stub_licensed_features(security_dashboard: true) end - it 'is visible' do - render + context 'when the user has access to Compliance dashboard' do + before do + group.add_developer(user) + allow(view).to receive(:current_user).and_return(user) + end - expect(rendered).to have_link 'Security & Compliance' - expect(rendered).to have_link 'Security' + it 'is visible' do + render + + expect(rendered).to have_link 'Security & Compliance' + expect(rendered).to have_link 'Security' + end end end -- GitLab From 52ea909721aadf4105fb341ccfe997f66eae7bcb Mon Sep 17 00:00:00 2001 From: manojmj Date: Tue, 15 Sep 2020 10:11:32 +0530 Subject: [PATCH 6/9] Refactor relation_group_members method This change refactors the `relation_group_members ` method --- app/finders/group_members_finder.rb | 8 ++++++-- ee/app/finders/ee/group_members_finder.rb | 12 +++--------- ee/spec/finders/ee/group_members_finder_spec.rb | 2 +- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/app/finders/group_members_finder.rb b/app/finders/group_members_finder.rb index 55f26d85d1fb..09283f061c08 100644 --- a/app/finders/group_members_finder.rb +++ b/app/finders/group_members_finder.rb @@ -66,9 +66,13 @@ def group_members_list group.members end - # rubocop: disable CodeReuse/ActiveRecord def relation_group_members(relation) - GroupMember.non_request.non_minimal_access + all_group_members(relation).non_minimal_access + end + + # rubocop: disable CodeReuse/ActiveRecord + def all_group_members(relation) + GroupMember.non_request .where(source_id: relation.select(:id)) .where.not(user_id: group.users.select(:id)) end diff --git a/ee/app/finders/ee/group_members_finder.rb b/ee/app/finders/ee/group_members_finder.rb index 16b9e8b841a6..43211cae9f15 100644 --- a/ee/app/finders/ee/group_members_finder.rb +++ b/ee/app/finders/ee/group_members_finder.rb @@ -18,19 +18,13 @@ def not_managed def group_members_list return group.all_group_members if group.minimal_access_role_allowed? - group.members + super 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? + return all_group_members(relation) if group.minimal_access_role_allowed? - members.non_minimal_access + super end - # rubocop: enable CodeReuse/ActiveRecord end diff --git a/ee/spec/finders/ee/group_members_finder_spec.rb b/ee/spec/finders/ee/group_members_finder_spec.rb index 7060416949c1..7c87953582d4 100644 --- a/ee/spec/finders/ee/group_members_finder_spec.rb +++ b/ee/spec/finders/ee/group_members_finder_spec.rb @@ -43,7 +43,7 @@ stub_licensed_features(minimal_access_role: true) end - it 'returns only members with minimal access' do + it 'also returns 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]) -- GitLab From 003a645afd6d2e5dac5a04604e2e84b8da3c2fe9 Mon Sep 17 00:00:00 2001 From: manojmj Date: Tue, 15 Sep 2020 10:17:50 +0530 Subject: [PATCH 7/9] Revert "Fix bug in group security dashboard" This reverts commit 627dfde1272ef142474e08e1a395d968d1ad8b1e. --- ee/app/helpers/groups/security_features_helper.rb | 2 +- .../groups/security_features_helper_spec.rb | 7 +++---- .../layouts/nav/sidebar/_group.html.haml_spec.rb | 15 ++++----------- 3 files changed, 8 insertions(+), 16 deletions(-) diff --git a/ee/app/helpers/groups/security_features_helper.rb b/ee/app/helpers/groups/security_features_helper.rb index a7cf1cd1aa92..fdd928651835 100644 --- a/ee/app/helpers/groups/security_features_helper.rb +++ b/ee/app/helpers/groups/security_features_helper.rb @@ -2,7 +2,7 @@ module Groups::SecurityFeaturesHelper def group_level_security_dashboard_available?(group) - can?(current_user, :read_group_security_dashboard, group) + group.feature_available?(:security_dashboard) end def group_level_compliance_dashboard_available?(group) diff --git a/ee/spec/helpers/groups/security_features_helper_spec.rb b/ee/spec/helpers/groups/security_features_helper_spec.rb index e67868ff4e98..f58a95656c7d 100644 --- a/ee/spec/helpers/groups/security_features_helper_spec.rb +++ b/ee/spec/helpers/groups/security_features_helper_spec.rb @@ -9,19 +9,18 @@ let_it_be(:user, refind: true) { create(:user) } before do - allow(helper).to receive(:can?) { |*args| Ability.allowed?(*args) } allow(helper).to receive(:current_user).and_return(user) end describe '#group_level_security_dashboard_available?' do - where(:read_group_security_dashboard_permission, :result) do - false | false + where(:security_dashboard_feature_enabled, :result) do true | true + false | false end with_them do before do - allow(helper).to receive(:can?).with(user, :read_group_security_dashboard, group).and_return(read_group_security_dashboard_permission) + stub_licensed_features(security_dashboard: security_dashboard_feature_enabled) end it 'returns the expected result' do diff --git a/ee/spec/views/layouts/nav/sidebar/_group.html.haml_spec.rb b/ee/spec/views/layouts/nav/sidebar/_group.html.haml_spec.rb index 82a1f7637dee..355ec66755d7 100644 --- a/ee/spec/views/layouts/nav/sidebar/_group.html.haml_spec.rb +++ b/ee/spec/views/layouts/nav/sidebar/_group.html.haml_spec.rb @@ -109,18 +109,11 @@ stub_licensed_features(security_dashboard: true) end - context 'when the user has access to Compliance dashboard' do - before do - group.add_developer(user) - allow(view).to receive(:current_user).and_return(user) - end - - it 'is visible' do - render + it 'is visible' do + render - expect(rendered).to have_link 'Security & Compliance' - expect(rendered).to have_link 'Security' - end + expect(rendered).to have_link 'Security & Compliance' + expect(rendered).to have_link 'Security' end end -- GitLab From b626e9e9c780dad9814065385bda41b581823a2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Wed, 30 Sep 2020 12:54:44 +0200 Subject: [PATCH 8/9] Prepare feature flag yaml file --- config/feature_flags/licensed/minimal_access_role.yml | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 config/feature_flags/licensed/minimal_access_role.yml diff --git a/config/feature_flags/licensed/minimal_access_role.yml b/config/feature_flags/licensed/minimal_access_role.yml new file mode 100644 index 000000000000..5d7930b2f065 --- /dev/null +++ b/config/feature_flags/licensed/minimal_access_role.yml @@ -0,0 +1,7 @@ +--- +name: minimal_access_role +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40942 +rollout_issue_url: +group: group::access +type: licensed +default_enabled: [false, true] -- GitLab From 6be83e9ab81cbe0c0e9d8d5d024546336b253a19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Wed, 30 Sep 2020 17:05:51 +0200 Subject: [PATCH 9/9] Update feature flag file --- config/feature_flags/licensed/minimal_access_role.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/feature_flags/licensed/minimal_access_role.yml b/config/feature_flags/licensed/minimal_access_role.yml index 5d7930b2f065..ca27b86d35ff 100644 --- a/config/feature_flags/licensed/minimal_access_role.yml +++ b/config/feature_flags/licensed/minimal_access_role.yml @@ -4,4 +4,4 @@ introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40942 rollout_issue_url: group: group::access type: licensed -default_enabled: [false, true] +default_enabled: true -- GitLab