Merge branch '296817-group-permissions-do-not-allow-project-moves-between-namespaces' into 'master'

Resolve "Show groups arising from group shares in the list of permissible locations a project can be transferred to"

See merge request !90127

app/finders/groups/accepting_project_transfers_finder.rb

+# frozen_string_literal: true
+
+module Groups
+  class AcceptingProjectTransfersFinder
+    def initialize(current_user)
+      @current_user = current_user
+    end
+
+    def execute
+      if Feature.disabled?(:include_groups_from_group_shares_in_project_transfer_locations)
+        return current_user.manageable_groups
+      end
+
+      groups_accepting_project_transfers =
+        [
+          current_user.manageable_groups,
+          managable_groups_originating_from_group_shares
+        ]
+
+      groups = ::Group.from_union(groups_accepting_project_transfers)
+
+      groups.project_creation_allowed
+    end
+
+    private
+
+    attr_reader :current_user
+
+    def managable_groups_originating_from_group_shares
+      GroupGroupLink
+        .with_owner_or_maintainer_access
+        .groups_accessible_via(
+          groups_that_user_has_owner_or_maintainer_access_via_direct_membership
+          .select(:id)
+        )
+    end
+
+    def groups_that_user_has_owner_or_maintainer_access_via_direct_membership
+      # Only maintainers or above in a group has access to transfer projects to that group
+      current_user.owned_or_maintainers_groups
+    end
+  end
+end

app/finders/groups/user_groups_finder.rb

@@ -48,7 +48,7 @@ def by_permission_scope
       if permission_scope_create_projects?
         target_user.manageable_groups(include_groups_with_developer_maintainer_access: true)
       elsif permission_scope_transfer_projects?
-        target_user.manageable_groups(include_groups_with_developer_maintainer_access: false)
+        Groups::AcceptingProjectTransfersFinder.new(target_user).execute # rubocop: disable CodeReuse/Finder
       else
         target_user.groups
       end

app/models/group.rb

@@ -176,6 +176,16 @@ def of_ancestors_and_self
       .where(project_authorizations: { user_id: user_ids })
   end
 
+  scope :project_creation_allowed, -> do
+    permitted_levels = [
+      ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS,
+      ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS,
+      nil
+    ]
+
+    where(project_creation_level: permitted_levels)
+  end
+
   class << self
     def sort_by_attribute(method)
       if method == 'storage_size_desc'

app/models/group_group_link.rb

@@ -14,6 +14,23 @@ class GroupGroupLink < ApplicationRecord
                            presence: true
 
   scope :non_guests, -> { where('group_access > ?', Gitlab::Access::GUEST) }
+
+  scope :with_owner_or_maintainer_access, -> do
+    where(group_access: [Gitlab::Access::OWNER, Gitlab::Access::MAINTAINER])
+  end
+
+  scope :groups_accessible_via, -> (shared_with_group_ids) do
+    links = where(shared_with_group_id: shared_with_group_ids)
+    # a group share also gives you access to the descendants of the group being shared,
+    # so we must include the descendants as well in the result.
+    Group.id_in(links.select(:shared_group_id)).self_and_descendants
+  end
+
+  scope :groups_having_access_to, -> (shared_group_ids) do
+    links = where(shared_group_id: shared_group_ids)
+    Group.id_in(links.select(:shared_with_group_id))
+  end
+
   scope :preload_shared_with_groups, -> { preload(:shared_with_group) }
 
   scope :distinct_on_shared_with_group_id_with_group_access, -> do

config/feature_flags/development/include_groups_from_group_shares_in_project_transfer_locations.yml

+---
+name: include_groups_from_group_shares_in_project_transfer_locations
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/90127
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/366305
+milestone: '15.2'
+type: development
+group: group::workspace
+default_enabled: false

spec/finders/groups/accepting_project_transfers_finder_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Groups::AcceptingProjectTransfersFinder do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:group_where_direct_owner) { create(:group) }
+  let_it_be(:subgroup_of_group_where_direct_owner) { create(:group, parent: group_where_direct_owner) }
+  let_it_be(:group_where_direct_maintainer) { create(:group) }
+  let_it_be(:group_where_direct_maintainer_but_cant_create_projects) do
+    create(:group, project_creation_level: Gitlab::Access::NO_ONE_PROJECT_ACCESS)
+  end
+
+  let_it_be(:group_where_direct_developer) { create(:group) }
+  let_it_be(:shared_with_group_where_direct_owner_as_owner) { create(:group) }
+  let_it_be(:shared_with_group_where_direct_owner_as_guest) { create(:group) }
+  let_it_be(:shared_with_group_where_direct_owner_as_maintainer) { create(:group) }
+  let_it_be(:shared_with_group_where_direct_developer_as_owner) { create(:group) }
+  let_it_be(:subgroup_of_shared_with_group_where_direct_owner_as_maintainer) do
+    create(:group, parent: shared_with_group_where_direct_owner_as_maintainer)
+  end
+
+  before do
+    group_where_direct_owner.add_owner(user)
+    group_where_direct_maintainer.add_maintainer(user)
+    group_where_direct_developer.add_developer(user)
+
+    create(:group_group_link, :owner,
+      shared_with_group: group_where_direct_owner,
+      shared_group: shared_with_group_where_direct_owner_as_owner
+    )
+
+    create(:group_group_link, :guest,
+      shared_with_group: group_where_direct_owner,
+      shared_group: shared_with_group_where_direct_owner_as_guest
+    )
+
+    create(:group_group_link, :maintainer,
+      shared_with_group: group_where_direct_owner,
+      shared_group: shared_with_group_where_direct_owner_as_maintainer
+    )
+
+    create(:group_group_link, :owner,
+      shared_with_group: group_where_direct_developer,
+      shared_group: shared_with_group_where_direct_developer_as_owner
+    )
+  end
+
+  describe '#execute' do
+    subject(:result) { described_class.new(user).execute }
+
+    it 'only returns groups where the user has access to transfer projects to' do
+      expect(result).to match_array([
+        group_where_direct_owner,
+        subgroup_of_group_where_direct_owner,
+        group_where_direct_maintainer,
+        shared_with_group_where_direct_owner_as_owner,
+        shared_with_group_where_direct_owner_as_maintainer,
+        subgroup_of_shared_with_group_where_direct_owner_as_maintainer
+      ])
+    end
+  end
+end

spec/models/group_group_link_spec.rb

@@ -30,6 +30,54 @@
       end
     end
 
+    describe '.with_owner_or_maintainer_access' do
+      let_it_be(:group_group_link_maintainer) { create :group_group_link, :maintainer }
+      let_it_be(:group_group_link_owner) { create :group_group_link, :owner }
+      let_it_be(:group_group_link_reporter) { create :group_group_link, :reporter }
+      let_it_be(:group_group_link_guest) { create :group_group_link, :guest }
+
+      it 'returns all records which have OWNER or MAINTAINER access' do
+        expect(described_class.with_owner_or_maintainer_access).to match_array([
+                                                                                  group_group_link_maintainer,
+                                                                                  group_group_link_owner
+                                                                              ])
+      end
+    end
+
+    context 'access via group shares' do
+      let_it_be(:shared_with_group_1) { create(:group) }
+      let_it_be(:shared_with_group_2) { create(:group) }
+      let_it_be(:shared_with_group_3) { create(:group) }
+      let_it_be(:shared_group_1) { create(:group) }
+      let_it_be(:shared_group_2) { create(:group) }
+      let_it_be(:shared_group_3) { create(:group) }
+      let_it_be(:shared_group_1_subgroup) { create(:group, parent: shared_group_1) }
+
+      before do
+        create :group_group_link, shared_with_group: shared_with_group_1, shared_group: shared_group_1
+        create :group_group_link, shared_with_group: shared_with_group_2, shared_group: shared_group_2
+        create :group_group_link, shared_with_group: shared_with_group_3, shared_group: shared_group_3
+      end
+
+      describe '.groups_accessible_via' do
+        it 'returns other groups that you can get access to, via the group shares of the specified groups' do
+          group_ids = [shared_with_group_1.id, shared_with_group_2.id]
+          expected_result = Group.id_in([shared_group_1.id, shared_group_1_subgroup.id, shared_group_2.id])
+
+          expect(described_class.groups_accessible_via(group_ids)).to match_array(expected_result)
+        end
+      end
+
+      describe '.groups_having_access_to' do
+        it 'returns all other groups that are having access to these specified groups, via group share' do
+          group_ids = [shared_group_1.id, shared_group_2.id]
+          expected_result = Group.id_in([shared_with_group_1.id, shared_with_group_2.id])
+
+          expect(described_class.groups_having_access_to(group_ids)).to match_array(expected_result)
+        end
+      end
+    end
+
     describe '.distinct_on_shared_with_group_id_with_group_access' do
       let_it_be(:sub_shared_group) { create(:group, parent: shared_group) }
       let_it_be(:other_group) { create(:group) }

spec/models/group_spec.rb

@@ -806,6 +806,20 @@
       end
     end
 
+    describe '.project_creation_allowed' do
+      let_it_be(:group_1) { create(:group, project_creation_level: Gitlab::Access::NO_ONE_PROJECT_ACCESS) }
+      let_it_be(:group_2) { create(:group, project_creation_level: Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS) }
+      let_it_be(:group_3) { create(:group, project_creation_level: Gitlab::Access::MAINTAINER_PROJECT_ACCESS) }
+      let_it_be(:group_4) { create(:group, project_creation_level: nil) }
+
+      it 'only includes groups where project creation is allowed' do
+        result = described_class.project_creation_allowed
+
+        expect(result).to include(group_2, group_3, group_4)
+        expect(result).not_to include(group_1)
+      end
+    end
+
     describe 'by_ids_or_paths' do
       let(:group_path) { 'group_path' }
       let!(:group) { create(:group, path: group_path) }