Merge branch '220203-gitlab-com-sso-create-no-access-role-fe-changes' into 'master'

Resolve "Create "No Access" Role" - user facing changes

Closes #220203

See merge request gitlab-org/gitlab!40942

app/finders/group_members_finder.rb

@@ -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,22 @@ class GroupMembersFinder < UnionFinder
   def can_manage_members
     Ability.allowed?(user, :admin_group_member, group)
   end
+
+  def group_members_list
+    group.members
+  end
+
+  def relation_group_members(relation)
+    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
+  # rubocop: enable CodeReuse/ActiveRecord
 end
 
 GroupMembersFinder.prepend_if_ee('EE::GroupMembersFinder')

app/helpers/groups/group_members_helper.rb

@@ -10,7 +10,7 @@ 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: group.access_level_roles, default_access_level: default_access_level
   end
 
   def linked_groups_data_json(group_links)

app/models/group.rb

@@ -356,6 +356,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,
@@ -550,6 +551,14 @@ class Group < Namespace
     owners.first || parent&.default_owner || owner
   end
 
+  def access_level_roles
+    GroupMember.access_level_roles
+  end
+
+  def access_level_values
+    access_level_roles.values
+  end
+
   private
 
   def update_two_factor_requirement

app/models/user.rb

@@ -132,6 +132,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

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), 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

changelogs/unreleased/220203-gitlab-com-sso-create-no-access-role-fe-changes.yml

+---
+title: Add No Access Role for top group members
+merge_request: 40942
+author:
+type: added

config/feature_flags/licensed/minimal_access_role.yml

+---
+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: true

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 @@ 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

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,18 @@ 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?
+
+    super
+  end
+
+  override :relation_group_members
+  def relation_group_members(relation)
+    return all_group_members(relation) if group.minimal_access_role_allowed?
+
+    super
+  end
 end

ee/app/models/ee/group.rb

@@ -408,6 +408,21 @@ module EE
       minimal_access_role_allowed? ? ::Gitlab::Access::MINIMAL_ACCESS : ::Gitlab::Access::GUEST
     end
 
+    override :access_level_roles
+    def access_level_roles
+      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

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
@@ -80,6 +80,14 @@ module EE
 
     private
 
+    override :access_level_inclusion
+    def access_level_inclusion
+      levels = source.access_level_values
+      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)) })

ee/app/models/ee/user.rb

@@ -44,6 +44,9 @@ module EE
       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
@@ -364,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?
@@ -394,5 +409,12 @@ module EE
 
       highest_role > ::Gitlab::Access::GUEST
     end
+
+    def available_minimal_access_groups
+      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
   end
 end

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 @@ 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
+    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

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 @@ module EE
       member.user.group_managed_account?
     end
 
+    override :access_level_roles
+    def access_level_roles
+      member.source.access_level_roles
+    end
+
     private
 
     def override_member_permission

ee/app/views/groups/saml_providers/_form.html.haml

@@ -50,7 +50,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, 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.')
 

ee/lib/ee/gitlab/access.rb

@@ -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

ee/spec/finders/ee/group_members_finder_spec.rb

@@ -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 '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])
+      end
+    end
+  end
 end

ee/spec/models/group_member_spec.rb

@@ -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 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
+      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

ee/spec/models/group_spec.rb

@@ -705,7 +705,7 @@ RSpec.describe Group do
     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 }

ee/spec/models/saml_provider_spec.rb

@@ -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

ee/spec/models/user_spec.rb

@@ -1117,6 +1117,56 @@ 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 unavailable' do
+      it { is_expected.to contain_exactly private_group, project_group }
+    end
+
+    context 'with minimal access feature available' do
+      before do
+        stub_licensed_features(minimal_access_role: true)
+      end
+
+      context 'feature turned on for all groups' do
+        before do
+          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 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
+
   describe '#active_for_authentication?' do
     subject { user.active_for_authentication? }
 

ee/spec/presenters/group_member_presenter_spec.rb

@@ -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).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

spec/support/shared_contexts/cache_allowed_users_in_namespace_shared_context.rb

@@ -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