Refresh ProjectAuthorization during Group deletion

Since ProjectAuthorizations are cached, update need to be triggered in
certain cases. Eg. when the group was shared with another group, access
for the members of the shared with group need to be revoked from the
projects of the shared group, as the GroupGroupLink no longer exists.

app/services/groups/destroy_service.rb

@@ -29,7 +29,15 @@ module Groups
 
       group.chat_team&.remove_mattermost_team(current_user)
 
+      user_ids_for_project_authorizations_refresh = group.user_ids_for_project_authorizations
+
       group.destroy
+
+      UserProjectAccessChangedService
+        .new(user_ids_for_project_authorizations_refresh)
+        .execute(blocking: true)
+
+      group
     end
     # rubocop: enable CodeReuse/ActiveRecord
   end

changelogs/unreleased/213665-update_project_authorizations_on_group_delete.yml

+---
+title: Refresh ProjectAuthorization during Group deletion
+merge_request:
+author:
+type: security

db/post_migrate/20200204113225_schedule_recalculate_project_authorizations_third_run.rb

+# frozen_string_literal: true
+
+class ScheduleRecalculateProjectAuthorizationsThirdRun < ActiveRecord::Migration[5.1]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  MIGRATION = 'RecalculateProjectAuthorizationsWithMinMaxUserId'
+  BATCH_SIZE = 2_500
+  DELAY_INTERVAL = 2.minutes.to_i
+
+  disable_ddl_transaction!
+
+  class User < ActiveRecord::Base
+    include ::EachBatch
+
+    self.table_name = 'users'
+  end
+
+  def up
+    say "Scheduling #{MIGRATION} jobs"
+
+    queue_background_migration_jobs_by_range_at_intervals(User, MIGRATION, DELAY_INTERVAL, batch_size: BATCH_SIZE)
+  end
+
+  def down
+  end
+end

db/structure.sql

@@ -12762,6 +12762,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200204070729
 20200204113223
 20200204113224
+20200204113225
 20200204131054
 20200204131831
 20200205143231

spec/migrations/schedule_recalculate_project_authorizations_third_run_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20200204113225_schedule_recalculate_project_authorizations_third_run.rb')
+
+describe ScheduleRecalculateProjectAuthorizationsThirdRun do
+  let(:users_table) { table(:users) }
+
+  before do
+    stub_const("#{described_class}::BATCH_SIZE", 2)
+
+    1.upto(4) do |i|
+      users_table.create!(id: i, name: "user#{i}", email: "user#{i}@example.com", projects_limit: 1)
+    end
+  end
+
+  it 'schedules background migration' do
+    Sidekiq::Testing.fake! do
+      Timecop.freeze do
+        migrate!
+
+        expect(BackgroundMigrationWorker.jobs.size).to eq(2)
+        expect(described_class::MIGRATION).to be_scheduled_migration(1, 2)
+        expect(described_class::MIGRATION).to be_scheduled_migration(3, 4)
+      end
+    end
+  end
+end

spec/services/groups/destroy_service_spec.rb

@@ -133,4 +133,55 @@ describe Groups::DestroyService do
       end
     end
   end
+
+  describe 'authorization updates', :sidekiq_inline do
+    context 'shared groups' do
+      let!(:shared_group) { create(:group, :private) }
+      let!(:shared_group_child) { create(:group, :private, parent: shared_group) }
+
+      let!(:project) { create(:project, group: shared_group) }
+      let!(:project_child) { create(:project, group: shared_group_child) }
+
+      before do
+        create(:group_group_link, shared_group: shared_group, shared_with_group: group)
+        group.refresh_members_authorized_projects
+      end
+
+      it 'updates project authorization' do
+        expect(user.can?(:read_project, project)).to eq(true)
+        expect(user.can?(:read_project, project_child)).to eq(true)
+
+        destroy_group(group, user, false)
+
+        expect(user.can?(:read_project, project)).to eq(false)
+        expect(user.can?(:read_project, project_child)).to eq(false)
+      end
+    end
+
+    context 'shared groups in the same group hierarchy' do
+      let!(:subgroup) { create(:group, :private, parent: group) }
+      let!(:subgroup_user) { create(:user) }
+
+      before do
+        subgroup.add_user(subgroup_user, Gitlab::Access::MAINTAINER)
+
+        create(:group_group_link, shared_group: group, shared_with_group: subgroup)
+        subgroup.refresh_members_authorized_projects
+      end
+
+      context 'group is deleted' do
+        it 'updates project authorization' do
+          expect { destroy_group(group, user, false) }.to(
+            change { subgroup_user.can?(:read_project, project) }.from(true).to(false))
+        end
+      end
+
+      context 'subgroup is deleted' do
+        it 'updates project authorization' do
+          expect { destroy_group(subgroup, user, false) }.to(
+            change { subgroup_user.can?(:read_project, project) }.from(true).to(false))
+        end
+      end
+    end
+  end
 end