From 7f803c41e2ca54b7b787b1f111f91357136c0e68 Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 17 Dec 2021 23:01:21 +0100 Subject: [PATCH] Add ability to purge undeliverable domains from admin interface (#16686) * Add ability to purge undeliverable domains from admin interface * Add tests --- app/controllers/admin/instances_controller.rb | 9 +++++ app/helpers/admin/action_logs_helper.rb | 4 +++ app/models/admin/action_log_filter.rb | 1 + app/policies/instance_policy.rb | 4 +++ app/services/purge_domain_service.rb | 10 ++++++ app/views/admin/instances/show.html.haml | 2 ++ app/workers/admin/domain_purge_worker.rb | 9 +++++ config/locales/en.yml | 5 +++ config/routes.rb | 2 +- .../admin/instances_controller_spec.rb | 35 ++++++++++++++++--- spec/policies/instance_policy_spec.rb | 2 +- spec/services/purge_domain_service_spec.rb | 27 ++++++++++++++ .../workers/admin/domain_purge_worker_spec.rb | 18 ++++++++++ 13 files changed, 121 insertions(+), 7 deletions(-) create mode 100644 app/services/purge_domain_service.rb create mode 100644 app/workers/admin/domain_purge_worker.rb create mode 100644 spec/services/purge_domain_service_spec.rb create mode 100644 spec/workers/admin/domain_purge_worker_spec.rb diff --git a/app/controllers/admin/instances_controller.rb b/app/controllers/admin/instances_controller.rb index 748c5de5a..306ec1f53 100644 --- a/app/controllers/admin/instances_controller.rb +++ b/app/controllers/admin/instances_controller.rb @@ -14,6 +14,15 @@ module Admin authorize :instance, :show? end + def destroy + authorize :instance, :destroy? + + Admin::DomainPurgeWorker.perform_async(@instance.domain) + + log_action :destroy, @instance + redirect_to admin_instances_path, notice: I18n.t('admin.instances.destroyed_msg', domain: @instance.domain) + end + def clear_delivery_errors authorize :delivery, :clear_delivery_errors? diff --git a/app/helpers/admin/action_logs_helper.rb b/app/helpers/admin/action_logs_helper.rb index ae96f7a34..f3aa4be4f 100644 --- a/app/helpers/admin/action_logs_helper.rb +++ b/app/helpers/admin/action_logs_helper.rb @@ -31,6 +31,8 @@ module Admin::ActionLogsHelper link_to truncate(record.text), edit_admin_announcement_path(record.id) when 'IpBlock' "#{record.ip}/#{record.ip.prefix} (#{I18n.t("simple_form.labels.ip_block.severities.#{record.severity}")})" + when 'Instance' + record.domain end end @@ -54,6 +56,8 @@ module Admin::ActionLogsHelper truncate(attributes['text'].is_a?(Array) ? attributes['text'].last : attributes['text']) when 'IpBlock' "#{attributes['ip']}/#{attributes['ip'].prefix} (#{I18n.t("simple_form.labels.ip_block.severities.#{attributes['severity']}")})" + when 'Instance' + attributes['domain'] end end end diff --git a/app/models/admin/action_log_filter.rb b/app/models/admin/action_log_filter.rb index 2af9d7c9c..d1ad46526 100644 --- a/app/models/admin/action_log_filter.rb +++ b/app/models/admin/action_log_filter.rb @@ -26,6 +26,7 @@ class Admin::ActionLogFilter destroy_domain_allow: { target_type: 'DomainAllow', action: 'destroy' }.freeze, destroy_domain_block: { target_type: 'DomainBlock', action: 'destroy' }.freeze, destroy_email_domain_block: { target_type: 'EmailDomainBlock', action: 'destroy' }.freeze, + destroy_instance: { target_type: 'Instance', action: 'destroy' }.freeze, destroy_unavailable_domain: { target_type: 'UnavailableDomain', action: 'destroy' }.freeze, destroy_status: { target_type: 'Status', action: 'destroy' }.freeze, disable_2fa_user: { target_type: 'User', action: 'disable' }.freeze, diff --git a/app/policies/instance_policy.rb b/app/policies/instance_policy.rb index a73823556..801ca162e 100644 --- a/app/policies/instance_policy.rb +++ b/app/policies/instance_policy.rb @@ -8,4 +8,8 @@ class InstancePolicy < ApplicationPolicy def show? admin? end + + def destroy? + admin? + end end diff --git a/app/services/purge_domain_service.rb b/app/services/purge_domain_service.rb new file mode 100644 index 000000000..e10a8f0c8 --- /dev/null +++ b/app/services/purge_domain_service.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class PurgeDomainService < BaseService + def call(domain) + Account.remote.where(domain: domain).reorder(nil).find_each do |account| + DeleteAccountService.new.call(account, reserve_username: false, skip_side_effects: true) + end + Instance.refresh + end +end diff --git a/app/views/admin/instances/show.html.haml b/app/views/admin/instances/show.html.haml index d6542ac3e..e520bca0c 100644 --- a/app/views/admin/instances/show.html.haml +++ b/app/views/admin/instances/show.html.haml @@ -84,3 +84,5 @@ = link_to t('admin.instances.delivery.stop'), stop_delivery_admin_instance_path(@instance), data: { confirm: t('admin.accounts.are_you_sure'), method: :post }, class: 'button' - else = link_to t('admin.instances.delivery.restart'), restart_delivery_admin_instance_path(@instance), data: { confirm: t('admin.accounts.are_you_sure'), method: :post }, class: 'button' + - unless @instance.delivery_failure_tracker.available? && @instance.accounts_count > 0 + = link_to t('admin.instances.purge'), admin_instance_path(@instance), data: { confirm: t('admin.instances.confirm_purge'), method: :delete }, class: 'button' diff --git a/app/workers/admin/domain_purge_worker.rb b/app/workers/admin/domain_purge_worker.rb new file mode 100644 index 000000000..7cba2c89e --- /dev/null +++ b/app/workers/admin/domain_purge_worker.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class Admin::DomainPurgeWorker + include Sidekiq::Worker + + def perform(domain) + PurgeDomainService.new.call(domain) + end +end diff --git a/config/locales/en.yml b/config/locales/en.yml index e9a0aea54..080b20983 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -240,6 +240,7 @@ en: destroy_domain_allow: Delete Domain Allow destroy_domain_block: Delete Domain Block destroy_email_domain_block: Delete E-mail Domain Block + destroy_instance: Purge Domain destroy_ip_block: Delete IP rule destroy_status: Delete Post destroy_unavailable_domain: Delete Unavailable Domain @@ -287,6 +288,7 @@ en: destroy_domain_allow_html: "%{name} disallowed federation with domain %{target}" destroy_domain_block_html: "%{name} unblocked domain %{target}" destroy_email_domain_block_html: "%{name} unblocked e-mail domain %{target}" + destroy_instance_html: "%{name} purged domain %{target}" destroy_ip_block_html: "%{name} deleted rule for IP %{target}" destroy_status_html: "%{name} removed post by %{target}" destroy_unavailable_domain_html: "%{name} resumed delivery to domain %{target}" @@ -465,6 +467,7 @@ en: back_to_limited: Limited back_to_warning: Warning by_domain: Domain + confirm_purge: Are you sure you want to permanently delete data from this domain? delivery: all: All clear: Clear delivery errors @@ -480,6 +483,7 @@ en: delivery_available: Delivery is available delivery_error_days: Delivery error days delivery_error_hint: If delivery is not possible for %{count} days, it will be automatically marked as undeliverable. + destroyed_msg: Data from %{domain} is now queued for imminent deletion. empty: No domains found. known_accounts: one: "%{count} known account" @@ -490,6 +494,7 @@ en: title: Moderation private_comment: Private comment public_comment: Public comment + purge: Purge title: Federation total_blocked_by_us: Blocked by us total_followed_by_them: Followed by them diff --git a/config/routes.rb b/config/routes.rb index 31b398e2c..b3b80624d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -214,7 +214,7 @@ Rails.application.routes.draw do end end - resources :instances, only: [:index, :show], constraints: { id: /[^\/]+/ } do + resources :instances, only: [:index, :show, :destroy], constraints: { id: /[^\/]+/ } do member do post :clear_delivery_errors post :restart_delivery diff --git a/spec/controllers/admin/instances_controller_spec.rb b/spec/controllers/admin/instances_controller_spec.rb index 8c0b309f2..53427b874 100644 --- a/spec/controllers/admin/instances_controller_spec.rb +++ b/spec/controllers/admin/instances_controller_spec.rb @@ -3,8 +3,14 @@ require 'rails_helper' RSpec.describe Admin::InstancesController, type: :controller do render_views + let(:current_user) { Fabricate(:user, admin: true) } + + let!(:account) { Fabricate(:account, domain: 'popular') } + let!(:account2) { Fabricate(:account, domain: 'popular') } + let!(:account3) { Fabricate(:account, domain: 'less.popular') } + before do - sign_in Fabricate(:user, admin: true), scope: :user + sign_in current_user, scope: :user end describe 'GET #index' do @@ -16,10 +22,6 @@ RSpec.describe Admin::InstancesController, type: :controller do end it 'renders instances' do - Fabricate(:account, domain: 'popular') - Fabricate(:account, domain: 'popular') - Fabricate(:account, domain: 'less.popular') - get :index, params: { page: 2 } instances = assigns(:instances).to_a @@ -29,4 +31,27 @@ RSpec.describe Admin::InstancesController, type: :controller do expect(response).to have_http_status(200) end end + + describe 'DELETE #destroy' do + subject { delete :destroy, params: { id: Instance.first.id } } + + let(:current_user) { Fabricate(:user, admin: admin) } + let(:account) { Fabricate(:account) } + + context 'when user is admin' do + let(:admin) { true } + + it 'succeeds in purging instance' do + is_expected.to redirect_to admin_instances_path + end + end + + context 'when user is not admin' do + let(:admin) { false } + + it 'fails to purge instance' do + is_expected.to have_http_status :forbidden + end + end + end end diff --git a/spec/policies/instance_policy_spec.rb b/spec/policies/instance_policy_spec.rb index 77a3bde3f..72cf25f56 100644 --- a/spec/policies/instance_policy_spec.rb +++ b/spec/policies/instance_policy_spec.rb @@ -8,7 +8,7 @@ RSpec.describe InstancePolicy do let(:admin) { Fabricate(:user, admin: true).account } let(:john) { Fabricate(:user).account } - permissions :index? do + permissions :index?, :show?, :destroy? do context 'admin' do it 'permits' do expect(subject).to permit(admin, Instance) diff --git a/spec/services/purge_domain_service_spec.rb b/spec/services/purge_domain_service_spec.rb new file mode 100644 index 000000000..59285f126 --- /dev/null +++ b/spec/services/purge_domain_service_spec.rb @@ -0,0 +1,27 @@ +require 'rails_helper' + +RSpec.describe PurgeDomainService, type: :service do + let!(:old_account) { Fabricate(:account, domain: 'obsolete.org') } + let!(:old_status1) { Fabricate(:status, account: old_account) } + let!(:old_status2) { Fabricate(:status, account: old_account) } + let!(:old_attachment) { Fabricate(:media_attachment, account: old_account, status: old_status2, file: attachment_fixture('attachment.jpg')) } + + subject { PurgeDomainService.new } + + describe 'for a suspension' do + before do + subject.call('obsolete.org') + end + + it 'removes the remote accounts\'s statuses and media attachments' do + expect { old_account.reload }.to raise_exception ActiveRecord::RecordNotFound + expect { old_status1.reload }.to raise_exception ActiveRecord::RecordNotFound + expect { old_status2.reload }.to raise_exception ActiveRecord::RecordNotFound + expect { old_attachment.reload }.to raise_exception ActiveRecord::RecordNotFound + end + + it 'refreshes instances view' do + expect(Instance.where(domain: 'obsolete.org').exists?).to be false + end + end +end diff --git a/spec/workers/admin/domain_purge_worker_spec.rb b/spec/workers/admin/domain_purge_worker_spec.rb new file mode 100644 index 000000000..b67c58b23 --- /dev/null +++ b/spec/workers/admin/domain_purge_worker_spec.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Admin::DomainPurgeWorker do + subject { described_class.new } + + describe 'perform' do + it 'calls domain purge service for relevant domain block' do + service = double(call: nil) + allow(PurgeDomainService).to receive(:new).and_return(service) + result = subject.perform('example.com') + + expect(result).to be_nil + expect(service).to have_received(:call).with('example.com') + end + end +end