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

app/finders/group_members_finder.rb

@@ -19,7 +19,7 @@ class GroupMembersFinder < UnionFinder
 
   # rubocop: disable CodeReuse/ActiveRecord
   def execute(include_relations: [:inherited, :direct])
-    group_members = group.members
+    group_members = group.members.non_unassigned
     relations = []
 
     return group_members if include_relations == [:direct]
@@ -27,7 +27,7 @@ class GroupMembersFinder < UnionFinder
     relations << group_members if include_relations.include?(:direct)
 
     if include_relations.include?(:inherited) && group.parent
-      parents_members = GroupMember.non_request
+      parents_members = GroupMember.non_request.non_unassigned
         .where(source_id: group.ancestors.select(:id))
         .where.not(user_id: group.users.select(:id))
 
@@ -35,7 +35,7 @@ class GroupMembersFinder < UnionFinder
     end
 
     if include_relations.include?(:descendants)
-      descendant_members = GroupMember.non_request
+      descendant_members = GroupMember.non_request.non_unassigned
         .where(source_id: group.descendants.select(:id))
         .where.not(user_id: group.users.select(:id))
 

app/finders/members_finder.rb

@@ -63,7 +63,7 @@ class MembersFinder
   def direct_group_members(include_descendants)
     requested_relations = [:inherited, :direct]
     requested_relations << :descendants if include_descendants
-    GroupMembersFinder.new(group).execute(include_relations: requested_relations).non_invite # rubocop: disable CodeReuse/Finder
+    GroupMembersFinder.new(group).execute(include_relations: requested_relations).non_invite.non_unassigned # rubocop: disable CodeReuse/Finder
   end
 
   def project_invited_groups_members
@@ -73,7 +73,7 @@ class MembersFinder
       .public_or_visible_to_user(current_user)
       .select(:id)
 
-    GroupMember.with_source_id(invited_groups_ids_including_ancestors)
+    GroupMember.with_source_id(invited_groups_ids_including_ancestors).non_unassigned
   end
 
   def distinct_union_of_members(union_members)

app/helpers/groups/group_members_helper.rb

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

app/models/group.rb

@@ -268,7 +268,7 @@ class Group < Namespace
     add_user(user, :owner, current_user: current_user)
   end
 
-  def member?(user, min_access_level = Gitlab::Access::GUEST)
+  def member?(user, min_access_level = Gitlab::Access::UNASSIGNED)
     return false unless user
 
     max_member_access_for_user(user) >= min_access_level
@@ -326,7 +326,7 @@ class Group < Namespace
   # rubocop: enable CodeReuse/ServiceClass
 
   def user_ids_for_project_authorizations
-    members_with_parents.pluck(:user_id)
+    members_with_parents.where.not(access_level: Gitlab::Access::UNASSIGNED).pluck(:user_id)
   end
 
   def self_and_ancestors_ids
@@ -352,7 +352,7 @@ class Group < Namespace
   end
 
   def members_from_self_and_ancestors_with_effective_access_level
-    members_with_parents.select([:user_id, 'MAX(access_level) AS access_level'])
+    members_with_parents.non_unassigned.select([:user_id, 'MAX(access_level) AS access_level'])
                         .group(:user_id)
   end
 

app/models/member.rb

@@ -25,7 +25,6 @@ class Member < ApplicationRecord
   validates :user_id, uniqueness: { scope: [:source_type, :source_id],
                                     message: "already exists in source",
                                     allow_nil: true }
-  validates :access_level, inclusion: { in: Gitlab::Access.all_values }, presence: true
   validate :higher_access_level_than_group, unless: :importing?
   validates :invite_email,
     presence: {
@@ -85,6 +84,7 @@ class Member < ApplicationRecord
   scope :developers, -> { active.where(access_level: DEVELOPER) }
   scope :maintainers, -> { active.where(access_level: MAINTAINER) }
   scope :non_guests, -> { where('members.access_level > ?', GUEST) }
+  scope :non_unassigned, -> { where('members.access_level > ?', UNASSIGNED) }
   scope :owners, -> { active.where(access_level: OWNER) }
   scope :owners_and_maintainers, -> { active.where(access_level: [OWNER, MAINTAINER]) }
   scope :with_user, -> (user) { where(user: user) }

app/models/members/group_member.rb

@@ -13,6 +13,8 @@ class GroupMember < Member
   # Make sure group member points only to group as it source
   default_value_for :source_type, SOURCE_TYPE
   validates :source_type, format: { with: /\ANamespace\z/ }
+  validates :access_level, inclusion: { in: Gitlab::Access.values_with_limited }, presence: true
+
   default_scope { where(source_type: SOURCE_TYPE) } # rubocop:disable Cop/DefaultScope
 
   scope :of_groups, ->(groups) { where(source_id: groups.select(:id)) }

app/models/user.rb

@@ -120,7 +120,7 @@ class User < ApplicationRecord
 
   # Groups
   has_many :members
-  has_many :group_members, -> { where(requested_at: nil) }, source: 'GroupMember'
+  has_many :group_members, -> { where(requested_at: nil).where("access_level >= ?", Gitlab::Access::GUEST) }, source: 'GroupMember'
   has_many :groups, through: :group_members
   has_many :owned_groups, -> { where(members: { access_level: Gitlab::Access::OWNER }) }, through: :group_members, source: :group
   has_many :maintainers_groups, -> { where(members: { access_level: Gitlab::Access::MAINTAINER }) }, through: :group_members, source: :group
@@ -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 :unassigned_group_members, -> { where(access_level: [Gitlab::Access::UNASSIGNED]) }, source: 'GroupMember', class_name: 'GroupMember'
+  has_many :unassigned_groups, through: :unassigned_group_members, source: :group
 
   # Projects
   has_many :groups_projects,          through: :groups, source: :projects
@@ -890,6 +892,7 @@ class User < ApplicationRecord
     Group.unscoped do
       Group.from_union([
         groups,
+        unassigned_groups,
         authorized_projects.joins(:namespace).select('namespaces.*')
       ])
     end

app/policies/group_policy.rb

@@ -12,6 +12,7 @@ class GroupPolicy < BasePolicy
 
   condition(:has_access) { access_level != GroupMember::NO_ACCESS }
 
+  condition(:unassigned) { access_level >= GroupMember::UNASSIGNED }
   condition(:guest) { access_level >= GroupMember::GUEST }
   condition(:developer) { access_level >= GroupMember::DEVELOPER }
   condition(:owner) { access_level >= GroupMember::OWNER }

ee/app/helpers/ee/groups/group_members_helper.rb

@@ -7,4 +7,12 @@ module EE::Groups::GroupMembersHelper
   def group_member_select_options
     super.merge(skip_ldap: @group.ldap_synced?)
   end
+
+  override :access_level_roles
+  def access_level_roles(group)
+    levels = GroupMember.access_level_roles
+    return levels if group.unassigned_role_allowed?
+
+    levels.except("Unassigned")
+  end
 end

ee/app/models/ee/group.rb

@@ -401,6 +401,10 @@ module EE
       root_ancestor.saml_provider&.prohibited_outer_forks?
     end
 
+    def unassigned_role_allowed?
+      feature_available?(:unassigned_role) && !has_parent?
+    end
+
     private
 
     def custom_project_templates_group_allowed

ee/app/models/ee/group_member.rb

@@ -26,9 +26,16 @@ module EE
     end
 
     class_methods do
+      extend ::Gitlab::Utils::Override
+
       def member_of_group?(group, user)
         exists?(group: group, user: user)
       end
+
+      override :access_level_roles
+      def access_level_roles
+        ::Gitlab::Access.options_with_unassigned
+      end
     end
 
     def group_has_domain_limitations?

ee/app/models/license.rb

@@ -108,6 +108,7 @@ class License < ApplicationRecord
     smartcard_auth
     group_timelogs
     type_of_work_analytics
+    unassigned_role
     unprotection_restrictions
     ci_project_subscriptions
   ]

ee/app/presenters/ee/member_presenter.rb

@@ -13,10 +13,21 @@ module EE
       can?(current_user, override_member_permission, member)
     end
 
+    override :valid_level_roles
+    def valid_level_roles
+      return super if member.source.is_a?(Project) || source_allows_unassigned_role?(member)
+
+      super.except("Unassigned")
+    end
+
     private
 
     def override_member_permission
       raise NotImplementedError
     end
+
+    def source_allows_unassigned_role?(member)
+      member.source.unassigned_role_allowed?
+    end
   end
 end

ee/lib/ee/gitlab/access.rb

@@ -15,6 +15,12 @@ module EE
         def vulnerability_access_levels
           @vulnerability_access_levels ||= options_with_owner.except('Guest')
         end
+
+        def options_with_unassigned
+          options_with_owner.merge(
+            "Unassigned" => ::Gitlab::Access::UNASSIGNED
+          )
+        end
       end
     end
   end

lib/gitlab/access.rb

@@ -9,12 +9,13 @@ module Gitlab
   module Access
     AccessDeniedError = Class.new(StandardError)
 
-    NO_ACCESS  = 0
-    GUEST      = 10
-    REPORTER   = 20
-    DEVELOPER  = 30
-    MAINTAINER = 40
-    OWNER      = 50
+    NO_ACCESS      = 0
+    UNASSIGNED     = 5
+    GUEST          = 10
+    REPORTER       = 20
+    DEVELOPER      = 30
+    MAINTAINER     = 40
+    OWNER          = 50
 
     # Branch protection settings
     PROTECTION_NONE          = 0
@@ -38,12 +39,16 @@ module Gitlab
         options_with_owner.values
       end
 
+      def values_with_limited
+        options_with_unassigned.values
+      end
+
       def options
         {
-          "Guest"      => GUEST,
-          "Reporter"   => REPORTER,
-          "Developer"  => DEVELOPER,
-          "Maintainer" => MAINTAINER
+          "Guest"          => GUEST,
+          "Reporter"       => REPORTER,
+          "Developer"      => DEVELOPER,
+          "Maintainer"     => MAINTAINER
         }
       end
 
@@ -86,7 +91,7 @@ module Gitlab
       end
 
       def human_access(access)
-        options_with_owner.key(access)
+        options_with_unassigned.key(access)
       end
 
       def human_access_with_none(access)

lib/gitlab/project_authorizations.rb

@@ -99,6 +99,7 @@ module Gitlab
         .and(members[:source_type].eq('Namespace'))
         .and(members[:requested_at].eq(nil))
         .and(members[:user_id].eq(user.id))
+        .and(members[:access_level].gt(Gitlab::Access::UNASSIGNED))
 
       Arel::Nodes::OuterJoin.new(members, Arel::Nodes::On.new(cond))
     end
@@ -119,6 +120,7 @@ module Gitlab
                     .and(members[:source_type].eq('Namespace'))
                     .and(members[:requested_at].eq(nil))
                     .and(members[:user_id].eq(user.id))
+                    .and(members[:access_level].gt(Gitlab::Access::UNASSIGNED))
       Arel::Nodes::InnerJoin.new(members, Arel::Nodes::On.new(cond))
     end
 

spec/finders/group_members_finder_spec.rb

@@ -16,6 +16,7 @@ RSpec.describe GroupMembersFinder, '#execute' do
     member1 = group.add_maintainer(user1)
     member2 = group.add_maintainer(user2)
     member3 = group.add_maintainer(user3)
+    create(:group_member, access_level: Gitlab::Access::UNASSIGNED, user: create(:user), group: group)
 
     result = described_class.new(group).execute
 

spec/finders/groups_finder_spec.rb

@@ -106,6 +106,14 @@ RSpec.describe GroupsFinder do
             parent_group.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PRIVATE)
           end
 
+          context 'being limited access member of parent group' do
+            it 'returns visible subgroups' do
+              create(:group_member, access_level: Gitlab::Access::UNASSIGNED, user: user, group: parent_group)
+
+              is_expected.to contain_exactly(parent_group, public_subgroup, internal_subgroup)
+            end
+          end
+
           context 'being member of parent group' do
             it 'returns all subgroups' do
               parent_group.add_guest(user)

spec/finders/members_finder_spec.rb

@@ -45,6 +45,18 @@ RSpec.describe MembersFinder, '#execute' do
     expect(result).to contain_exactly(member1)
   end
 
+  it 'does not return members of parent group with limited access' do
+    nested_group.request_access(user1)
+    member1 = group.add_maintainer(user2)
+    member2 = nested_group.add_maintainer(user3)
+    member3 = project.add_maintainer(user4)
+    create(:group_member, access_level: Gitlab::Access::UNASSIGNED, user: create(:user), group: group)
+
+    result = described_class.new(project, user2).execute
+
+    expect(result).to contain_exactly(member1, member2, member3)
+  end
+
   it 'includes only non-invite members if user do not have amdin permissions on project' do
     create(:project_member, :invited, project: project, invite_email: create(:user).email)
     member1 = project.add_maintainer(user1)

spec/lib/gitlab/project_authorizations_spec.rb

@@ -115,6 +115,66 @@ RSpec.describe Gitlab::ProjectAuthorizations do
     end
   end
 
+  context 'user with limited access to group' do
+    let_it_be(:group) { create(:group) }
+    let_it_be(:user) { create(:user) }
+
+    subject(:mapping) { map_access_levels(authorizations) }
+
+    context 'group membership' do
+      let!(:group_project) { create(:project, namespace: group) }
+
+      before do
+        create(:group_member, access_level: Gitlab::Access::UNASSIGNED, user: user, group: group)
+      end
+
+      it 'does not create authorization' do
+        expect(mapping[group_project.id]).to be_nil
+      end
+    end
+
+    context 'inherited group membership' do
+      let!(:sub_group) { create(:group, parent: group) }
+      let!(:sub_group_project) { create(:project, namespace: sub_group) }
+
+      before do
+        create(:group_member, access_level: Gitlab::Access::UNASSIGNED, user: user, group: group)
+      end
+
+      it 'does not create authorization' do
+        expect(mapping[sub_group_project.id]).to be_nil
+      end
+    end
+
+    context 'shared group' do
+      let!(:shared_group) { create(:group) }
+      let!(:shared_group_project) { create(:project, namespace: shared_group) }
+
+      before do
+        create(:group_group_link, shared_group: shared_group, shared_with_group: group)
+        create(:group_member, access_level: Gitlab::Access::UNASSIGNED, user: user, group: group)
+      end
+
+      it 'does not create authorization' do
+        expect(mapping[shared_group_project.id]).to be_nil
+      end
+    end
+
+    context 'shared project' do
+      let!(:another_group) { create(:group) }
+      let!(:shared_project) { create(:project, namespace: another_group) }
+
+      before do
+        create(:project_group_link, group: group, project: shared_project)
+        create(:group_member, access_level: Gitlab::Access::UNASSIGNED, user: user, group: group)
+      end
+
+      it 'does not create authorization' do
+        expect(mapping[shared_project.id]).to be_nil
+      end
+    end
+  end
+
   context 'with nested groups' do
     let(:group) { create(:group) }
     let!(:nested_group) { create(:group, parent: group) }

spec/models/group_spec.rb

@@ -692,6 +692,7 @@ RSpec.describe Group do
     before do
       create(:group_member, user: user, group: group_parent, access_level: parent_group_access_level)
       create(:group_member, user: user, group: group, access_level: group_access_level)
+      create(:group_member, user: create(:user), group: group, access_level: Gitlab::Access::UNASSIGNED)
       create(:group_member, user: user, group: group_child, access_level: child_group_access_level)
     end
 

spec/models/member_spec.rb

@@ -16,7 +16,6 @@ RSpec.describe Member do
 
     it { is_expected.to validate_presence_of(:user) }
     it { is_expected.to validate_presence_of(:source) }
-    it { is_expected.to validate_inclusion_of(:access_level).in_array(Gitlab::Access.all_values) }
 
     it_behaves_like 'an object with email-formated attributes', :invite_email do
       subject { build(:project_member) }
@@ -150,6 +149,7 @@ RSpec.describe Member do
 
       accepted_request_user = create(:user).tap { |u| project.request_access(u) }
       @accepted_request_member = project.requesters.find_by(user_id: accepted_request_user.id).tap { |m| m.accept_request }
+      @member_unassigned = create(:group_member, access_level: Gitlab::Access::UNASSIGNED, group: group)
     end
 
     describe '.access_for_user_ids' do
@@ -180,6 +180,15 @@ RSpec.describe Member do
       it { expect(described_class.non_invite).to include @accepted_request_member }
     end
 
+    describe '.non_unassigned' do
+      it { expect(described_class.non_unassigned).to include @maintainer }
+      it { expect(described_class.non_unassigned).to include @invited_member }
+      it { expect(described_class.non_unassigned).to include @accepted_invite_member }
+      it { expect(described_class.non_unassigned).to include @requested_member }
+      it { expect(described_class.non_unassigned).to include @accepted_request_member }
+      it { expect(described_class.non_unassigned).not_to include @member_unassigned }
+    end
+
     describe '.request' do
       it { expect(described_class.request).not_to include @maintainer }
       it { expect(described_class.request).not_to include @invited_member }

spec/services/authorized_project_update/project_create_service_spec.rb

@@ -81,6 +81,7 @@ RSpec.describe AuthorizedProjectUpdate::ProjectCreateService do
         before do
           create(:group_member, access_level: Gitlab::Access::REPORTER, group: group, user: group_user)
           create(:group_member, access_level: Gitlab::Access::MAINTAINER, group: shared_with_group, user: group_user)
+          create(:group_member, access_level: Gitlab::Access::UNASSIGNED, group: shared_with_group, user: create(:user))
 
           create(:group_group_link, shared_group: group, shared_with_group: shared_with_group, group_access: Gitlab::Access::DEVELOPER)
 
@@ -97,6 +98,11 @@ RSpec.describe AuthorizedProjectUpdate::ProjectCreateService do
             access_level: Gitlab::Access::DEVELOPER)
           expect(project_authorization).to exist
         end
+
+        it 'does not create project authorization for user with limited access' do
+          expect { service.execute }.to(
+            change { ProjectAuthorization.count }.from(0).to(1))
+        end
       end
     end
 
@@ -118,6 +124,17 @@ RSpec.describe AuthorizedProjectUpdate::ProjectCreateService do
       end
     end
 
+    context 'member with limited access' do
+      before do
+        create(:group_member, access_level: Gitlab::Access::UNASSIGNED, user: group_user, group: group)
+      end
+
+      it 'does not create project authorization' do
+        expect { service.execute }.not_to(
+          change { ProjectAuthorization.count }.from(0))
+      end
+    end
+
     context 'project has more user than BATCH_SIZE' do
       let(:batch_size) { 2 }
       let(:users) { create_list(:user, batch_size + 1 ) }

spec/services/authorized_project_update/project_group_link_create_service_spec.rb

@@ -112,6 +112,17 @@ RSpec.describe AuthorizedProjectUpdate::ProjectGroupLinkCreateService do
       end
     end
 
+    context 'unapproved access requests' do
+      before do
+        create(:group_member, access_level: Gitlab::Access::UNASSIGNED, user: group_user, group: group)
+      end
+
+      it 'does not create project authorization' do
+        expect { service.execute }.not_to(
+          change { ProjectAuthorization.count }.from(0))
+      end
+    end
+
     context 'project has more users than BATCH_SIZE' do
       let(:batch_size) { 2 }
       let(:users) { create_list(:user, batch_size + 1 ) }