Merge branch '223851-calculate-effective-member-access-level-for-projects' into 'master'

Add a method to obtain effective access_level of members in a project

See merge request gitlab-org/gitlab!60693

app/finders/projects/members/effective_access_level_finder.rb

+# frozen_string_literal: true
+
+module Projects
+  module Members
+    class EffectiveAccessLevelFinder
+      include Gitlab::Utils::StrongMemoize
+
+      USER_ID_AND_ACCESS_LEVEL = [:user_id, :access_level].freeze
+      BATCH_SIZE = 5
+
+      def initialize(project)
+        @project = project
+      end
+
+      def execute
+        return Member.none if no_members?
+
+        # rubocop: disable CodeReuse/ActiveRecord
+        Member.from(generate_from_statement(user_ids_and_access_levels_from_all_memberships))
+          .select([:user_id, 'MAX(access_level) AS access_level'])
+          .group(:user_id)
+        # rubocop: enable CodeReuse/ActiveRecord
+      end
+
+      private
+
+      attr_reader :project
+
+      def generate_from_statement(user_ids_and_access_levels)
+        "(VALUES #{generate_values_expression(user_ids_and_access_levels)}) members (user_id, access_level)"
+      end
+
+      def generate_values_expression(user_ids_and_access_levels)
+        user_ids_and_access_levels.map do |user_id, access_level|
+          "(#{user_id}, #{access_level})"
+        end.join(",")
+      end
+
+      def no_members?
+        user_ids_and_access_levels_from_all_memberships.blank?
+      end
+
+      def all_possible_avenues_of_membership
+        avenues = [authorizable_project_members]
+
+        avenues << if project.personal?
+                     project_owner_acting_as_maintainer
+                   else
+                     authorizable_group_members
+                   end
+
+        if include_membership_from_project_group_shares?
+          avenues << members_from_project_group_shares
+        end
+
+        avenues
+      end
+
+      # @return [Array<[user_id, access_level]>]
+      def user_ids_and_access_levels_from_all_memberships
+        strong_memoize(:user_ids_and_access_levels_from_all_memberships) do
+          all_possible_avenues_of_membership.flat_map do |relation|
+            relation.pluck(*USER_ID_AND_ACCESS_LEVEL) # rubocop: disable CodeReuse/ActiveRecord
+          end
+        end
+      end
+
+      def authorizable_project_members
+        project.members.authorizable
+      end
+
+      def authorizable_group_members
+        project.group.authorizable_members_with_parents
+      end
+
+      def members_from_project_group_shares
+        members = []
+
+        project.project_group_links.each_batch(of: BATCH_SIZE) do |relation|
+          members_per_batch = []
+
+          relation.includes(:group).each do |link| # rubocop: disable CodeReuse/ActiveRecord
+            members_per_batch << link.group.authorizable_members_with_parents.select(*user_id_and_access_level_for_project_group_shares(link))
+          end
+
+          members << Member.from_union(members_per_batch)
+        end
+
+        members.flatten
+      end
+
+      def project_owner_acting_as_maintainer
+        user_id = project.namespace.owner.id
+        access_level = Gitlab::Access::MAINTAINER
+
+        Member
+          .from(generate_from_statement([[user_id, access_level]])) # rubocop: disable CodeReuse/ActiveRecord
+          .limit(1)
+      end
+
+      def include_membership_from_project_group_shares?
+        project.allowed_to_share_with_group? && project.project_group_links.any?
+      end
+
+      # methods for `select` options
+
+      def user_id_and_access_level_for_project_group_shares(link)
+        least_access_level_among_group_membership_and_project_share =
+          smallest_value_arel([link.group_access, GroupMember.arel_table[:access_level]], 'access_level')
+
+        [
+          :user_id,
+          least_access_level_among_group_membership_and_project_share
+        ]
+      end
+
+      def smallest_value_arel(args, column_alias)
+        Arel::Nodes::As.new(
+          Arel::Nodes::NamedFunction.new('LEAST', args),
+          Arel.sql(column_alias)
+        )
+      end
+    end
+  end
+end

app/models/group.rb

@@ -450,6 +450,20 @@ class Group < Namespace
                .where(source_id: id)
   end
 
+  def authorizable_members_with_parents
+    source_ids =
+      if has_parent?
+        self_and_ancestors.reorder(nil).select(:id)
+      else
+        id
+      end
+
+    group_hierarchy_members = GroupMember.where(source_id: source_ids)
+
+    GroupMember.from_union([group_hierarchy_members,
+                            members_from_self_and_ancestor_group_shares]).authorizable
+  end
+
   def members_with_parents
     # Avoids an unnecessary SELECT when the group has no parents
     source_ids =

app/models/member.rb

@@ -92,6 +92,15 @@ class Member < ApplicationRecord
       .reorder(nil)
   end
 
+  # This scope is exclusively used to get the members
+  # that can possibly have project_authorization records
+  # to projects/groups.
+  scope :authorizable, -> do
+    where.not(user_id: nil)
+      .non_request
+      .non_minimal_access
+  end
+
   # Like active, but without invites. For when a User is required.
   scope :active_without_invites_and_requests, -> do
     left_join_users

app/models/project_group_link.rb

@@ -2,6 +2,7 @@
 
 class ProjectGroupLink < ApplicationRecord
   include Expirable
+  include EachBatch
 
   belongs_to :project
   belongs_to :group

spec/finders/projects/members/effective_access_level_finder_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+RSpec.describe Projects::Members::EffectiveAccessLevelFinder, '#execute' do
+  let_it_be(:group) { create(:group) }
+  let_it_be(:project) { create(:project, group: group) }
+
+  # The result set is being converted to json just for the ease of testing.
+  subject { described_class.new(project).execute.as_json }
+
+  context 'for a personal project' do
+    let_it_be(:project) { create(:project) }
+
+    shared_examples_for 'includes access level of the owner of the project as Maintainer' do
+      it 'includes access level of the owner of the project as Maintainer' do
+        expect(subject).to(
+          contain_exactly(
+            hash_including(
+              'user_id' => project.namespace.owner.id,
+              'access_level' => Gitlab::Access::MAINTAINER
+            )
+          )
+        )
+      end
+    end
+
+    context 'when the project owner is a member of the project' do
+      it_behaves_like 'includes access level of the owner of the project as Maintainer'
+    end
+
+    context 'when the project owner is not explicitly a member of the project' do
+      before do
+        project.members.find_by(user_id: project.namespace.owner.id).destroy!
+      end
+
+      it_behaves_like 'includes access level of the owner of the project as Maintainer'
+    end
+  end
+
+  context 'direct members of the project' do
+    it 'includes access levels of the direct members of the project' do
+      developer = create(:project_member, :developer, source: project)
+      maintainer = create(:project_member, :maintainer, source: project)
+
+      expect(subject).to(
+        include(
+          hash_including(
+            'user_id' => developer.user.id,
+            'access_level' => Gitlab::Access::DEVELOPER
+          ),
+          hash_including(
+            'user_id' => maintainer.user.id,
+            'access_level' => Gitlab::Access::MAINTAINER
+          )
+        )
+      )
+    end
+
+    it 'does not include access levels of users who have requested access to the project' do
+      member_with_access_request = create(:project_member, :access_request, :developer, source: project)
+
+      expect(subject).not_to(
+        include(
+          hash_including(
+            'user_id' => member_with_access_request.user.id
+          )
+        )
+      )
+    end
+
+    it 'includes access levels of users who are in non-active state' do
+      blocked_member = create(:project_member, :blocked, :developer, source: project)
+
+      expect(subject).to(
+        include(
+          hash_including(
+            'user_id' => blocked_member.user.id,
+            'access_level' => Gitlab::Access::DEVELOPER
+          )
+        )
+      )
+    end
+  end
+
+  context 'for a project within a group' do
+    context 'project in a root group' do
+      it 'includes access levels of users who are direct members of the parent group' do
+        group_member = create(:group_member, :developer, source: group)
+
+        expect(subject).to(
+          include(
+            hash_including(
+              'user_id' => group_member.user.id,
+              'access_level' => Gitlab::Access::DEVELOPER
+            )
+          )
+        )
+      end
+    end
+
+    context 'project in a subgroup' do
+      let_it_be(:project) { create(:project, group: create(:group, :nested)) }
+
+      it 'includes access levels of users who are members of the ancestors of the parent group' do
+        group_member = create(:group_member, :maintainer, source: project.group.parent)
+
+        expect(subject).to(
+          include(
+            hash_including(
+              'user_id' => group_member.user.id,
+              'access_level' => Gitlab::Access::MAINTAINER
+            )
+          )
+        )
+      end
+    end
+
+    context 'user is both a member of the project and a member of the parent group' do
+      let_it_be(:user) { create(:user) }
+
+      before do
+        group.add_developer(user)
+        project.add_maintainer(user)
+      end
+
+      it 'includes the maximum access level among project and group membership' do
+        expect(subject).to(
+          include(
+            hash_including(
+              'user_id' => user.id,
+              'access_level' => Gitlab::Access::MAINTAINER
+            )
+          )
+        )
+      end
+    end
+
+    context 'members from group share' do
+      let_it_be(:shared_with_group) { create(:group) }
+      let_it_be(:user_from_shared_with_group) { create(:user) }
+
+      before do
+        shared_with_group.add_guest(user_from_shared_with_group)
+        create(:group_group_link, :developer, shared_group: project.group, shared_with_group: shared_with_group)
+      end
+
+      it 'includes the user from the group share with the right access level' do
+        expect(subject).to(
+          include(
+            hash_including(
+              'user_id' => user_from_shared_with_group.id,
+              'access_level' => Gitlab::Access::GUEST
+            )
+          )
+        )
+      end
+
+      context 'when the project also has the same user as a member, but with a different access level' do
+        before do
+          project.add_maintainer(user_from_shared_with_group)
+        end
+
+        it 'includes the maximum access level among project and group membership' do
+          expect(subject).to(
+            include(
+              hash_including(
+                'user_id' => user_from_shared_with_group.id,
+                'access_level' => Gitlab::Access::MAINTAINER
+              )
+            )
+          )
+        end
+      end
+
+      context "when the project's ancestor also has the same user as a member, but with a different access level" do
+        before do
+          project.group.add_maintainer(user_from_shared_with_group)
+        end
+
+        it 'includes the maximum access level among project and group membership' do
+          expect(subject).to(
+            include(
+              hash_including(
+                'user_id' => user_from_shared_with_group.id,
+                'access_level' => Gitlab::Access::MAINTAINER
+              )
+            )
+          )
+        end
+      end
+    end
+  end
+
+  context 'for a project that is shared with other group(s)' do
+    let_it_be(:shared_with_group) { create(:group) }
+    let_it_be(:user_from_shared_with_group) { create(:user) }
+
+    before do
+      create(:project_group_link, :developer, project: project, group: shared_with_group)
+      shared_with_group.add_maintainer(user_from_shared_with_group)
+    end
+
+    it 'includes the least among the specified access levels' do
+      expect(subject).to(
+        include(
+          hash_including(
+            'user_id' => user_from_shared_with_group.id,
+            'access_level' => Gitlab::Access::DEVELOPER
+          )
+        )
+      )
+    end
+
+    context 'when the group containing the project has forbidden group shares for any of its projects' do
+      let_it_be(:project) { create(:project, group: create(:group)) }
+
+      before do
+        project.namespace.update!(share_with_group_lock: true)
+      end
+
+      it 'does not include the users from any group shares' do
+        expect(subject).not_to(
+          include(
+            hash_including(
+              'user_id' => user_from_shared_with_group.id
+            )
+          )
+        )
+      end
+    end
+  end
+
+  context 'a combination of all possible avenues of membership' do
+    let_it_be(:user) { create(:user) }
+    let_it_be(:shared_with_group) { create(:group) }
+
+    before do
+      create(:project_group_link, :maintainer, project: project, group: shared_with_group)
+      create(:group_group_link, :reporter, shared_group: project.group, shared_with_group: shared_with_group)
+
+      shared_with_group.add_maintainer(user)
+      group.add_guest(user)
+      project.add_developer(user)
+    end
+
+    it 'includes the highest access level from all avenues of memberships' do
+      expect(subject).to(
+        include(
+          hash_including(
+            'user_id' => user.id,
+            'access_level' => Gitlab::Access::MAINTAINER # From project_group_link
+          )
+        )
+      )
+    end
+  end
+end

spec/models/group_spec.rb

@@ -1285,7 +1285,7 @@ RSpec.describe Group do
     end
   end
 
-  describe '#members_with_parents' do
+  shared_examples_for 'members_with_parents' do
     let!(:group) { create(:group, :nested) }
     let!(:maintainer) { group.parent.add_user(create(:user), GroupMember::MAINTAINER) }
     let!(:developer) { group.add_user(create(:user), GroupMember::DEVELOPER) }
@@ -1309,6 +1309,50 @@ RSpec.describe Group do
     end
   end
 
+  describe '#members_with_parents' do
+    it_behaves_like 'members_with_parents'
+  end
+
+  describe '#authorizable_members_with_parents' do
+    let(:group) { create(:group) }
+
+    it_behaves_like 'members_with_parents'
+
+    context 'members with associated user but also having invite_token' do
+      let!(:member) { create(:group_member, :developer, :invited, user: create(:user), group: group) }
+
+      it 'includes such members in the result' do
+        expect(group.authorizable_members_with_parents).to include(member)
+      end
+    end
+
+    context 'invited members' do
+      let!(:member) { create(:group_member, :developer, :invited, group: group) }
+
+      it 'does not include such members in the result' do
+        expect(group.authorizable_members_with_parents).not_to include(member)
+      end
+    end
+
+    context 'members from group shares' do
+      let(:shared_group) { group }
+      let(:shared_with_group) { create(:group) }
+
+      before do
+        create(:group_group_link, shared_group: shared_group, shared_with_group: shared_with_group)
+      end
+
+      context 'an invited member that is part of the shared_with_group' do
+        let!(:member) { create(:group_member, :developer, :invited, group: shared_with_group) }
+
+        it 'does not include such members in the result' do
+          expect(shared_group.authorizable_members_with_parents).not_to(
+            include(member))
+        end
+      end
+    end
+  end
+
   describe '#members_from_self_and_ancestors_with_effective_access_level' do
     let!(:group_parent) { create(:group, :private) }
     let!(:group) { create(:group, :private, parent: group_parent) }

spec/models/member_spec.rb

@@ -408,6 +408,30 @@ RSpec.describe Member do
       it { is_expected.not_to include @member_with_minimal_access }
     end
 
+    describe '.authorizable' do
+      subject { described_class.authorizable.to_a }
+
+      it 'includes the member who has an associated user record,'\
+       'but also having an invite_token' do
+        member = create(:project_member,
+                        :developer,
+                        :invited,
+                        user: create(:user))
+
+        expect(subject).to include(member)
+      end
+
+      it { is_expected.to include @owner }
+      it { is_expected.to include @maintainer }
+      it { is_expected.to include @accepted_invite_member }
+      it { is_expected.to include @accepted_request_member }
+      it { is_expected.to include @blocked_maintainer }
+      it { is_expected.to include @blocked_developer }
+      it { is_expected.not_to include @invited_member }
+      it { is_expected.not_to include @requested_member }
+      it { is_expected.not_to include @member_with_minimal_access }
+    end
+
     describe '.distinct_on_user_with_max_access_level' do
       let_it_be(:other_group) { create(:group) }
       let_it_be(:member_with_lower_access_level) { create(:group_member, :developer, group: other_group, user: @owner_user) }