From acc180611f7112d4bc11530668dd05749c26592c Mon Sep 17 00:00:00 2001
From: Osama Sayegh <asooomaasoooma90@gmail.com>
Date: Mon, 9 Dec 2024 14:25:31 +0300
Subject: [PATCH] FEATURE: Add an option to block IPs and emails to bulk user
 delete (#29993)

This commit adds an option for blocking the IP and email addresses when bulk-deleting users.

Internal topic: t/140321/11.
---
 .../bulk-user-delete-confirmation.gjs         | 21 ++++-
 app/controllers/admin/users_controller.rb     |  6 +-
 app/services/user/bulk_destroy.rb             |  6 +-
 config/locales/client.en.yml                  |  1 +
 spec/requests/admin/users_controller_spec.rb  | 82 +++++++++++++++++--
 spec/system/admin_users_list_spec.rb          | 29 +++++++
 .../modals/bulk_user_delete_confirmation.rb   |  4 +
 7 files changed, 140 insertions(+), 9 deletions(-)

diff --git a/app/assets/javascripts/admin/addon/components/bulk-user-delete-confirmation.gjs b/app/assets/javascripts/admin/addon/components/bulk-user-delete-confirmation.gjs
index 36c246a0d75..267d5d5f9f4 100644
--- a/app/assets/javascripts/admin/addon/components/bulk-user-delete-confirmation.gjs
+++ b/app/assets/javascripts/admin/addon/components/bulk-user-delete-confirmation.gjs
@@ -22,6 +22,7 @@ export default class BulkUserDeleteConfirmation extends Component {
   failedUsernames = [];
 
   callAfterBulkDelete = false;
+  blockIpAndEmail = false;
 
   constructor() {
     super(...arguments);
@@ -117,7 +118,10 @@ export default class BulkUserDeleteConfirmation extends Component {
     try {
       await ajax("/admin/users/destroy-bulk.json", {
         type: "DELETE",
-        data: { user_ids: this.args.model.userIds },
+        data: {
+          user_ids: this.args.model.userIds,
+          block_ip_and_email: this.blockIpAndEmail,
+        },
       });
       this.callAfterBulkDelete = true;
     } catch (err) {
@@ -134,6 +138,11 @@ export default class BulkUserDeleteConfirmation extends Component {
     }
   }
 
+  @action
+  toggleBlockIpAndEmail(event) {
+    this.blockIpAndEmail = event.target.checked;
+  }
+
   <template>
     <DModal
       class="bulk-user-delete-confirmation"
@@ -169,6 +178,16 @@ export default class BulkUserDeleteConfirmation extends Component {
             placeholder={{this.confirmDeletePhrase}}
             {{on "input" this.onPromptInput}}
           />
+          <label class="checkbox-label">
+            <input
+              type="checkbox"
+              class="block-ip-and-email"
+              {{on "change" this.toggleBlockIpAndEmail}}
+            />
+            {{i18n
+              "admin.users.bulk_actions.delete.confirmation_modal.block_ip_and_email"
+            }}
+          </label>
         {{/if}}
       </:body>
       <:footer>
diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index 073b373451b..eef332fbe70 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -403,8 +403,12 @@ class Admin::UsersController < Admin::StaffController
   end
 
   def destroy_bulk
+    # capture service_params outside the hijack block to avoid thread safety
+    # issues
+    service_arg = service_params
+
     hijack do
-      User::BulkDestroy.call(service_params) do
+      User::BulkDestroy.call(service_arg) do
         on_success { render json: { deleted: true } }
 
         on_failed_contract do |contract|
diff --git a/app/services/user/bulk_destroy.rb b/app/services/user/bulk_destroy.rb
index cf1f119a546..c6d36215137 100644
--- a/app/services/user/bulk_destroy.rb
+++ b/app/services/user/bulk_destroy.rb
@@ -5,6 +5,7 @@ class User::BulkDestroy
 
   params do
     attribute :user_ids, :array
+    attribute :block_ip_and_email, :boolean, default: false
 
     validates :user_ids, length: { maximum: 100 }
   end
@@ -27,7 +28,8 @@ class User::BulkDestroy
     users.all? { |u| guardian.can_delete_user?(u) }
   end
 
-  def delete(users:, guardian:)
+  def delete(users:, guardian:, params:)
+    actor_ip = guardian.user.ip_address
     users
       .find_each
       .with_index(1) do |user, position|
@@ -37,6 +39,8 @@ class User::BulkDestroy
             delete_posts: true,
             prepare_for_destroy: true,
             context: I18n.t("staff_action_logs.bulk_user_delete"),
+            block_ip: params.block_ip_and_email && actor_ip != user.ip_address,
+            block_email: params.block_ip_and_email,
           )
 
         if success
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 4d79d314e20..73d86f7bf35 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -6678,6 +6678,7 @@ en:
                 one: "Delete %{count} user"
                 other: "Delete %{count} users"
               bulk_delete_starting: "Starting bulk delete…"
+              block_ip_and_email: "Block IP addresses and emails of all selected users"
               user_delete_succeeded: "[%{position}/%{total}] Successfully deleted @%{username}"
               user_delete_failed: "[%{position}/%{total}] Failed to delete @%{username} - %{error}"
               bulk_delete_finished: "Bulk delete operation completed."
diff --git a/spec/requests/admin/users_controller_spec.rb b/spec/requests/admin/users_controller_spec.rb
index 6803524db48..0f533ac58f5 100644
--- a/spec/requests/admin/users_controller_spec.rb
+++ b/spec/requests/admin/users_controller_spec.rb
@@ -1439,6 +1439,8 @@ RSpec.describe Admin::UsersController do
     fab!(:deleted_users) { Fabricate.times(3, :user) }
 
     shared_examples "bulk user deletion possible" do
+      before { sign_in(current_user) }
+
       it "can delete multiple users" do
         delete "/admin/users/destroy-bulk.json", params: { user_ids: deleted_users.map(&:id) }
         expect(response.status).to eq(200)
@@ -1477,18 +1479,86 @@ RSpec.describe Admin::UsersController do
         expect(response.status).to eq(200)
         expect(User.where(id: deleted_users.map(&:id)).count).to eq(0)
       end
+
+      it "blocks emails and IPs of deleted users if block_ip_and_email is true" do
+        current_user.update!(ip_address: IPAddr.new("127.189.34.11"))
+        deleted_users[0].update!(ip_address: IPAddr.new("127.189.34.11"))
+        deleted_users[1].update!(ip_address: IPAddr.new("249.21.44.3"))
+        deleted_users[2].update!(ip_address: IPAddr.new("3.1.22.88"))
+
+        expect do
+          delete "/admin/users/destroy-bulk.json",
+                 params: {
+                   user_ids: deleted_users.map(&:id),
+                   block_ip_and_email: true,
+                 }
+        end.to change {
+          ScreenedIpAddress.where(action_type: ScreenedIpAddress.actions[:block]).count
+        }.by(2).and change {
+                ScreenedEmail.where(action_type: ScreenedEmail.actions[:block]).count
+              }.by(3)
+
+        expect(
+          ScreenedIpAddress.exists?(
+            ip_address: "249.21.44.3",
+            action_type: ScreenedIpAddress.actions[:block],
+          ),
+        ).to be_truthy
+        expect(
+          ScreenedIpAddress.exists?(
+            ip_address: "3.1.22.88",
+            action_type: ScreenedIpAddress.actions[:block],
+          ),
+        ).to be_truthy
+        expect(ScreenedIpAddress.exists?(ip_address: current_user.ip_address)).to be_falsey
+
+        expect(
+          ScreenedEmail.exists?(
+            email: deleted_users[0].email,
+            action_type: ScreenedEmail.actions[:block],
+          ),
+        ).to be_truthy
+        expect(
+          ScreenedEmail.exists?(
+            email: deleted_users[1].email,
+            action_type: ScreenedEmail.actions[:block],
+          ),
+        ).to be_truthy
+        expect(
+          ScreenedEmail.exists?(
+            email: deleted_users[2].email,
+            action_type: ScreenedEmail.actions[:block],
+          ),
+        ).to be_truthy
+        expect(response.status).to eq(200)
+        expect(User.where(id: deleted_users.map(&:id)).count).to eq(0)
+      end
+
+      it "doesn't block emails and IPs of deleted users if block_ip_and_email is false" do
+        expect do
+          delete "/admin/users/destroy-bulk.json",
+                 params: {
+                   user_ids: deleted_users.map(&:id),
+                   block_ip_and_email: false,
+                 }
+        end.to not_change {
+          ScreenedIpAddress.where(action_type: ScreenedIpAddress.actions[:block]).count
+        }.and not_change { ScreenedEmail.where(action_type: ScreenedEmail.actions[:block]).count }
+        expect(response.status).to eq(200)
+        expect(User.where(id: deleted_users.map(&:id)).count).to eq(0)
+      end
     end
 
     context "when logged in as an admin" do
-      before { sign_in(admin) }
-
-      include_examples "bulk user deletion possible"
+      include_examples "bulk user deletion possible" do
+        let(:current_user) { admin }
+      end
     end
 
     context "when logged in as a moderator" do
-      before { sign_in(moderator) }
-
-      include_examples "bulk user deletion possible"
+      include_examples "bulk user deletion possible" do
+        let(:current_user) { moderator }
+      end
     end
 
     context "when logged in as a non-staff user" do
diff --git a/spec/system/admin_users_list_spec.rb b/spec/system/admin_users_list_spec.rb
index a1a4fe1fcf1..b6e9061fc89 100644
--- a/spec/system/admin_users_list_spec.rb
+++ b/spec/system/admin_users_list_spec.rb
@@ -143,6 +143,35 @@ describe "Admin Users Page", type: :system do
       confirmation_modal.close
       expect(admin_users_page).to have_users([user_1.id])
     end
+
+    it "has an option to block IPs and emails" do
+      user_1.update!(ip_address: IPAddr.new("44.22.11.33"))
+
+      admin_users_page.visit
+      admin_users_page.bulk_select_button.click
+
+      admin_users_page.user_row(user_1.id).bulk_select_checkbox.click
+      admin_users_page.bulk_actions_dropdown.expand
+      admin_users_page.bulk_actions_dropdown.option(".bulk-delete").click
+      confirmation_modal.fill_in_confirmation_phase(user_count: 1)
+      confirmation_modal.block_ip_and_email_checkbox.click
+      confirmation_modal.confirm_button.click
+
+      expect(confirmation_modal).to have_successful_log_entry_for_user(
+        user: user_1,
+        position: 1,
+        total: 1,
+      )
+      expect(
+        ScreenedIpAddress.exists?(
+          ip_address: user_1.ip_address,
+          action_type: ScreenedIpAddress.actions[:block],
+        ),
+      ).to be_truthy
+      expect(
+        ScreenedEmail.exists?(email: user_1.email, action_type: ScreenedEmail.actions[:block]),
+      ).to be_truthy
+    end
   end
 
   context "when visiting an admin's page" do
diff --git a/spec/system/page_objects/modals/bulk_user_delete_confirmation.rb b/spec/system/page_objects/modals/bulk_user_delete_confirmation.rb
index f70b93b1a3f..c42664ea2e7 100644
--- a/spec/system/page_objects/modals/bulk_user_delete_confirmation.rb
+++ b/spec/system/page_objects/modals/bulk_user_delete_confirmation.rb
@@ -9,6 +9,10 @@ module PageObjects
         within(modal) { find(".btn.confirm-delete") }
       end
 
+      def block_ip_and_email_checkbox
+        within(modal) { find("input.block-ip-and-email") }
+      end
+
       def has_confirm_button_disabled?
         within(modal) { has_css?(".btn.confirm-delete[disabled]") }
       end