Merge branch 'security-37766-transfer-group-reindex' into 'master'

Trigger Elasticsearch indexing when public group moved to private

See merge request gitlab/gitlab-ee!1510

app/services/groups/transfer_service.rb

@@ -39,9 +39,15 @@ module Groups
         ensure_ownership
       end
 
+      post_update_hooks(@updated_project_ids)
+
       true
     end
 
+    # Overridden in EE
+    def post_update_hooks(updated_project_ids)
+    end
+
     def ensure_allowed_transfer
       raise_transfer_error(:group_is_already_root) if group_is_already_root?
       raise_transfer_error(:same_parent_as_current) if same_parent?
@@ -96,9 +102,16 @@ module Groups
         .where(id: descendants.select(:id))
         .update_all(visibility_level: @new_parent_group.visibility_level)
 
-      @group
+      projects_to_update = @group
         .all_projects
         .where("visibility_level > ?", @new_parent_group.visibility_level)
+
+      # Used in post_update_hooks in EE. Must use pluck (and not select)
+      # here as after we perform the update below we won't be able to find
+      # these records again.
+      @updated_project_ids = projects_to_update.pluck(:id)
+
+      projects_to_update
         .update_all(visibility_level: @new_parent_group.visibility_level)
     end
     # rubocop: enable CodeReuse/ActiveRecord

ee/app/services/ee/groups/transfer_service.rb

@@ -22,6 +22,24 @@ module EE
         end
       end
 
+      override :post_update_hooks
+      # rubocop: disable CodeReuse/ActiveRecord
+      def post_update_hooks(updated_project_ids)
+        ::Project.where(id: updated_project_ids).find_each do |project|
+          # TODO: Refactor out this duplication per https://gitlab.com/gitlab-org/gitlab/issues/38232
+          if ::Gitlab::CurrentSettings.elasticsearch_indexing? && project.searchable?
+            ElasticIndexerWorker.perform_async(
+              :update,
+              project.class.to_s,
+              project.id,
+              project.es_id,
+              changed_fields: ['visibility_level']
+            )
+          end
+        end
+      end
+      # rubocop: enable CodeReuse/ActiveRecord
+
       def raise_ee_transfer_error(message)
         raise ::Groups::TransferService::TransferError, EE_ERROR_MESSAGES[message]
       end

ee/changelogs/unreleased/security-37766-transfer-group-reindex.yml

+---
+title: Fix stale Elasticsearch permissions when moving group from public group to private parent group
+merge_request:
+author:
+type: security

ee/spec/services/groups/transfer_service_spec.rb

@@ -52,4 +52,30 @@ describe Groups::TransferService, '#execute' do
       end
     end
   end
+
+  context 'when visibility changes' do
+    let(:new_group) { create(:group, :private) }
+
+    before do
+      stub_ee_application_setting(elasticsearch_indexing: true)
+    end
+
+    it 'reindexes projects' do
+      project1 = create(:project, :repository, :public, namespace: group)
+      project2 = create(:project, :repository, :public, namespace: group)
+      project3 = create(:project, :repository, :private, namespace: group)
+
+      expect(ElasticIndexerWorker).to receive(:perform_async)
+        .with(:update, "Project", project1.id, project1.es_id, changed_fields: array_including('visibility_level'))
+      expect(ElasticIndexerWorker).to receive(:perform_async)
+        .with(:update, "Project", project2.id, project2.es_id, changed_fields: array_including('visibility_level'))
+      expect(ElasticIndexerWorker).not_to receive(:perform_async)
+        .with(:update, "Project", project3.id, project3.es_id, changed_fields: array_including('visibility_level'))
+
+      transfer_service.execute(new_group)
+
+      expect(transfer_service.error).not_to be
+      expect(group.parent).to eq(new_group)
+    end
+  end
 end