From 94e98864e39c010635e839fea984f2b4893bef1a Mon Sep 17 00:00:00 2001 From: Levi Bard Date: Mon, 16 May 2022 09:29:01 +0200 Subject: [PATCH] Allow import/export of instance-level domain blocks/allows (#1754) * Allow import/export of instance-level domain blocks/allows. Fixes #15095 * Pacify circleci * Address simple code review feedback * Add headers to exported CSV * Extract common import/export functionality to AdminExportControllerConcern * Add additional fields to instance-blocked domain export * Address review feedback * Split instance domain block/allow import/export into separate pages/controllers * Address code review feedback * Pacify DeepSource * Work around Paperclip::HasAttachmentFile for Rails 6 * Fix deprecated API warning in export tests * Remove after_commit workaround --- .../admin/export_domain_allows_controller.rb | 60 ++++++++++++++++ .../admin/export_domain_blocks_controller.rb | 68 +++++++++++++++++++ .../admin_export_controller_concern.rb | 39 +++++++++++ app/models/admin/import.rb | 29 ++++++++ app/models/domain_allow.rb | 4 ++ app/validators/admin_import_validator.rb | 19 ++++++ .../admin/export_domain_allows/new.html.haml | 10 +++ .../admin/export_domain_blocks/new.html.haml | 10 +++ app/views/admin/instances/index.html.haml | 4 ++ config/locales/en.yml | 12 ++++ config/routes.rb | 16 ++++- .../admin/domain_allows_controller_spec.rb | 48 +++++++++++++ .../export_domain_allows_controller_spec.rb | 42 ++++++++++++ .../export_domain_blocks_controller_spec.rb | 47 +++++++++++++ spec/fixtures/files/domain_allows.csv | 3 + spec/fixtures/files/domain_blocks.csv | 4 ++ 16 files changed, 414 insertions(+), 1 deletion(-) create mode 100644 app/controllers/admin/export_domain_allows_controller.rb create mode 100644 app/controllers/admin/export_domain_blocks_controller.rb create mode 100644 app/controllers/concerns/admin_export_controller_concern.rb create mode 100644 app/models/admin/import.rb create mode 100644 app/validators/admin_import_validator.rb create mode 100644 app/views/admin/export_domain_allows/new.html.haml create mode 100644 app/views/admin/export_domain_blocks/new.html.haml create mode 100644 spec/controllers/admin/domain_allows_controller_spec.rb create mode 100644 spec/controllers/admin/export_domain_allows_controller_spec.rb create mode 100644 spec/controllers/admin/export_domain_blocks_controller_spec.rb create mode 100644 spec/fixtures/files/domain_allows.csv create mode 100644 spec/fixtures/files/domain_blocks.csv diff --git a/app/controllers/admin/export_domain_allows_controller.rb b/app/controllers/admin/export_domain_allows_controller.rb new file mode 100644 index 0000000000..eb2955ac38 --- /dev/null +++ b/app/controllers/admin/export_domain_allows_controller.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +require 'csv' + +module Admin + class ExportDomainAllowsController < BaseController + include AdminExportControllerConcern + + before_action :set_dummy_import!, only: [:new] + + ROWS_PROCESSING_LIMIT = 20_000 + + def new + authorize :domain_allow, :create? + end + + def export + authorize :instance, :index? + send_export_file + end + + def import + authorize :domain_allow, :create? + begin + @import = Admin::Import.new(import_params) + parse_import_data!(export_headers) + + @data.take(ROWS_PROCESSING_LIMIT).each do |row| + domain = row['#domain'].strip + next if DomainAllow.allowed?(domain) + + domain_allow = DomainAllow.new(domain: domain) + log_action :create, domain_allow if domain_allow.save + end + flash[:notice] = I18n.t('admin.domain_allows.created_msg') + rescue ActionController::ParameterMissing + flash[:error] = I18n.t('admin.export_domain_allows.no_file') + end + redirect_to admin_instances_path + end + + private + + def export_filename + 'domain_allows.csv' + end + + def export_headers + %w(#domain) + end + + def export_data + CSV.generate(headers: export_headers, write_headers: true) do |content| + DomainAllow.allowed_domains.each do |instance| + content << [instance.domain] + end + end + end + end +end diff --git a/app/controllers/admin/export_domain_blocks_controller.rb b/app/controllers/admin/export_domain_blocks_controller.rb new file mode 100644 index 0000000000..0ad5b92b50 --- /dev/null +++ b/app/controllers/admin/export_domain_blocks_controller.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require 'csv' + +module Admin + class ExportDomainBlocksController < BaseController + include AdminExportControllerConcern + + before_action :set_dummy_import!, only: [:new] + + ROWS_PROCESSING_LIMIT = 20_000 + + def new + authorize :domain_block, :create? + end + + def export + authorize :instance, :index? + send_export_file + end + + def import + authorize :domain_block, :create? + begin + @import = Admin::Import.new(import_params) + parse_import_data!(export_headers) + + @data.take(ROWS_PROCESSING_LIMIT).each do |row| + domain = row['#domain'].strip + next if DomainBlock.rule_for(domain).present? + + domain_block = DomainBlock.new(domain: domain, + severity: row['#severity'].strip, + reject_media: row['#reject_media'].strip, + reject_reports: row['#reject_reports'].strip, + public_comment: row['#public_comment'].strip, + obfuscate: row['#obfuscate'].strip) + if domain_block.save + DomainBlockWorker.perform_async(domain_block.id) + log_action :create, domain_block + end + end + flash[:notice] = I18n.t('admin.domain_blocks.created_msg') + rescue ActionController::ParameterMissing + flash[:error] = I18n.t('admin.export_domain_blocks.no_file') + end + redirect_to admin_instances_path(limited: '1') + end + + private + + def export_filename + 'domain_blocks.csv' + end + + def export_headers + %w(#domain #severity #reject_media #reject_reports #public_comment #obfuscate) + end + + def export_data + CSV.generate(headers: export_headers, write_headers: true) do |content| + DomainBlock.with_user_facing_limitations.each do |instance| + content << [instance.domain, instance.severity, instance.reject_media, instance.reject_reports, instance.public_comment, instance.obfuscate] + end + end + end + end +end diff --git a/app/controllers/concerns/admin_export_controller_concern.rb b/app/controllers/concerns/admin_export_controller_concern.rb new file mode 100644 index 0000000000..013915d025 --- /dev/null +++ b/app/controllers/concerns/admin_export_controller_concern.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module AdminExportControllerConcern + extend ActiveSupport::Concern + + private + + def send_export_file + respond_to do |format| + format.csv { send_data export_data, filename: export_filename } + end + end + + def export_data + raise 'Override in controller' + end + + def export_filename + raise 'Override in controller' + end + + def set_dummy_import! + @import = Admin::Import.new + end + + def import_params + params.require(:admin_import).permit(:data) + end + + def import_data + Paperclip.io_adapters.for(@import.data).read + end + + def parse_import_data!(default_headers) + data = CSV.parse(import_data, headers: true) + data = CSV.parse(import_data, headers: default_headers) unless data.headers&.first&.strip&.include?(default_headers[0]) + @data = data.reject(&:blank?) + end +end diff --git a/app/models/admin/import.rb b/app/models/admin/import.rb new file mode 100644 index 0000000000..c305be237a --- /dev/null +++ b/app/models/admin/import.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +# A non-activerecord helper class for csv upload +class Admin::Import + extend ActiveModel::Callbacks + include ActiveModel::Model + include Paperclip::Glue + + FILE_TYPES = %w(text/plain text/csv application/csv).freeze + + # Paperclip required callbacks + define_model_callbacks :save, only: [:after] + define_model_callbacks :destroy, only: [:before, :after] + + attr_accessor :data_file_name, :data_content_type + + has_attached_file :data + validates_attachment_content_type :data, content_type: FILE_TYPES + validates_attachment_presence :data + validates_with AdminImportValidator, on: :create + + def save + run_callbacks :save + end + + def destroy + run_callbacks :destroy + end +end diff --git a/app/models/domain_allow.rb b/app/models/domain_allow.rb index 4b0a89c184..2e14fce25e 100644 --- a/app/models/domain_allow.rb +++ b/app/models/domain_allow.rb @@ -23,6 +23,10 @@ class DomainAllow < ApplicationRecord !rule_for(domain).nil? end + def allowed_domains + select(:domain) + end + def rule_for(domain) return if domain.blank? diff --git a/app/validators/admin_import_validator.rb b/app/validators/admin_import_validator.rb new file mode 100644 index 0000000000..338ceb3a78 --- /dev/null +++ b/app/validators/admin_import_validator.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class AdminImportValidator < ActiveModel::Validator + FIRST_HEADER = '#domain' + + def validate(import) + return if import.type.blank? || import.data.blank? + + # We parse because newlines could be part of individual rows. This + # runs on create so we should be reading the local file here before + # it is uploaded to object storage or moved anywhere... + csv_data = CSV.parse(import.data.queued_for_write[:original].read) + + row_count = csv_data.size + row_count -= 1 if csv_data.first&.first == FIRST_HEADER + + import.errors.add(:data, I18n.t('imports.errors.over_rows_processing_limit', count: Admin::DomainBlocksController::ROWS_PROCESSING_LIMIT)) if row_count > Admin::DomainBlocksController::ROWS_PROCESSING_LIMIT + end +end diff --git a/app/views/admin/export_domain_allows/new.html.haml b/app/views/admin/export_domain_allows/new.html.haml new file mode 100644 index 0000000000..dc0cf8c52a --- /dev/null +++ b/app/views/admin/export_domain_allows/new.html.haml @@ -0,0 +1,10 @@ +- content_for :page_title do + = t('.title') + += simple_form_for @import, url: import_admin_export_domain_allows_path, html: { multipart: true } do |f| + .fields-row + .fields-group.fields-row__column.fields-row__column-6 + = f.input :data, wrapper: :with_block_label, hint: t('simple_form.hints.imports.data'), as: :file + + .actions + = f.button :button, t('imports.upload'), type: :submit diff --git a/app/views/admin/export_domain_blocks/new.html.haml b/app/views/admin/export_domain_blocks/new.html.haml new file mode 100644 index 0000000000..0291aeed77 --- /dev/null +++ b/app/views/admin/export_domain_blocks/new.html.haml @@ -0,0 +1,10 @@ +- content_for :page_title do + = t('.title') + += simple_form_for @import, url: import_admin_export_domain_blocks_path, html: { multipart: true } do |f| + .fields-row + .fields-group.fields-row__column.fields-row__column-6 + = f.input :data, wrapper: :with_block_label, hint: t('simple_form.hints.imports.data'), as: :file + + .actions + = f.button :button, t('imports.upload'), type: :submit diff --git a/app/views/admin/instances/index.html.haml b/app/views/admin/instances/index.html.haml index ee1b3d0ce7..abb2d8c0e5 100644 --- a/app/views/admin/instances/index.html.haml +++ b/app/views/admin/instances/index.html.haml @@ -4,8 +4,12 @@ - content_for :heading_actions do - if whitelist_mode? = link_to t('admin.domain_allows.add_new'), new_admin_domain_allow_path, class: 'button', id: 'add-instance-button' + = link_to t('admin.domain_allows.export'), export_admin_export_domain_allows_path(format: :csv), class: 'button' + = link_to t('admin.domain_allows.import'), new_admin_export_domain_allow_path, class: 'button' - else = link_to t('admin.domain_blocks.add_new'), new_admin_domain_block_path, class: 'button', id: 'add-instance-button' + = link_to t('admin.domain_blocks.export'), export_admin_export_domain_blocks_path(format: :csv), class: 'button' + = link_to t('admin.domain_blocks.import'), new_admin_export_domain_block_path, class: 'button' .filters .filter-subset diff --git a/config/locales/en.yml b/config/locales/en.yml index 50e762db72..583683bff5 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -421,6 +421,8 @@ en: add_new: Allow federation with domain created_msg: Domain has been successfully allowed for federation destroyed_msg: Domain has been disallowed from federation + export: Export + import: Import undo: Disallow federation with domain domain_blocks: add_new: Add new domain block @@ -429,6 +431,8 @@ en: domain: Domain edit: Edit domain block existing_domain_block_html: You have already imposed stricter limits on %{name}, you need to unblock it first. + export: Export + import: Import new: create: Create block hint: The domain block will not prevent creation of account entries in the database, but will retroactively and automatically apply specific moderation methods on those accounts. @@ -469,6 +473,14 @@ en: resolved_dns_records_hint_html: The domain name resolves to the following MX domains, which are ultimately responsible for accepting e-mail. Blocking an MX domain will block sign-ups from any e-mail address which uses the same MX domain, even if the visible domain name is different. Be careful not to block major e-mail providers. resolved_through_html: Resolved through %{domain} title: Blocked e-mail domains + export_domain_allows: + new: + title: Import domain allows + no_file: No file selected + export_domain_blocks: + new: + title: Import domain blocks + no_file: No file selected follow_recommendations: description_html: "Follow recommendations help new users quickly find interesting content. When a user has not interacted with others enough to form personalized follow recommendations, these accounts are recommended instead. They are re-calculated on a daily basis from a mix of accounts with the highest recent engagements and highest local follower counts for a given language." language: For language diff --git a/config/routes.rb b/config/routes.rb index 5747157057..7876651924 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -194,7 +194,21 @@ Rails.application.routes.draw do get '/dashboard', to: 'dashboard#index' resources :domain_allows, only: [:new, :create, :show, :destroy] - resources :domain_blocks, only: [:new, :create, :destroy, :update, :edit] + resources :domain_blocks, only: [:new, :create, :show, :destroy, :update, :edit] + + resources :export_domain_allows, only: [:new] do + collection do + get :export, constraints: { format: :csv } + post :import + end + end + + resources :export_domain_blocks, only: [:new] do + collection do + get :export, constraints: { format: :csv } + post :import + end + end resources :email_domain_blocks, only: [:index, :new, :create] do collection do diff --git a/spec/controllers/admin/domain_allows_controller_spec.rb b/spec/controllers/admin/domain_allows_controller_spec.rb new file mode 100644 index 0000000000..8bacdd3e40 --- /dev/null +++ b/spec/controllers/admin/domain_allows_controller_spec.rb @@ -0,0 +1,48 @@ +require 'rails_helper' + +RSpec.describe Admin::DomainAllowsController, type: :controller do + render_views + + before do + sign_in Fabricate(:user, admin: true), scope: :user + end + + describe 'GET #new' do + it 'assigns a new domain allow' do + get :new + + expect(assigns(:domain_allow)).to be_instance_of(DomainAllow) + expect(response).to have_http_status(200) + end + end + + describe 'POST #create' do + it 'blocks the domain when succeeded to save' do + post :create, params: { domain_allow: { domain: 'example.com' } } + + expect(flash[:notice]).to eq I18n.t('admin.domain_allows.created_msg') + expect(response).to redirect_to(admin_instances_path) + end + + it 'renders new when failed to save' do + Fabricate(:domain_allow, domain: 'example.com') + + post :create, params: { domain_allow: { domain: 'example.com' } } + + expect(response).to render_template :new + end + end + + describe 'DELETE #destroy' do + it 'disallows the domain' do + service = double(call: true) + allow(UnallowDomainService).to receive(:new).and_return(service) + domain_allow = Fabricate(:domain_allow) + delete :destroy, params: { id: domain_allow.id } + + expect(service).to have_received(:call).with(domain_allow) + expect(flash[:notice]).to eq I18n.t('admin.domain_allows.destroyed_msg') + expect(response).to redirect_to(admin_instances_path) + end + end +end diff --git a/spec/controllers/admin/export_domain_allows_controller_spec.rb b/spec/controllers/admin/export_domain_allows_controller_spec.rb new file mode 100644 index 0000000000..f6275c2d65 --- /dev/null +++ b/spec/controllers/admin/export_domain_allows_controller_spec.rb @@ -0,0 +1,42 @@ +require 'rails_helper' + +RSpec.describe Admin::ExportDomainAllowsController, type: :controller do + render_views + + before do + sign_in Fabricate(:user, admin: true), scope: :user + end + + describe 'GET #export' do + it 'renders instances' do + Fabricate(:domain_allow, domain: 'good.domain') + Fabricate(:domain_allow, domain: 'better.domain') + + get :export, params: { format: :csv } + expect(response).to have_http_status(200) + expect(response.body).to eq(IO.read(File.join(file_fixture_path, 'domain_allows.csv'))) + end + end + + describe 'POST #import' do + it 'allows imported domains' do + post :import, params: { admin_import: { data: fixture_file_upload('domain_allows.csv') } } + + expect(response).to redirect_to(admin_instances_path) + + # Header should not be imported + expect(DomainAllow.where(domain: '#domain').present?).to eq(false) + + # Domains should now be added + get :export, params: { format: :csv } + expect(response).to have_http_status(200) + expect(response.body).to eq(IO.read(File.join(file_fixture_path, 'domain_allows.csv'))) + end + + it 'displays error on no file selected' do + post :import, params: { admin_import: {} } + expect(response).to redirect_to(admin_instances_path) + expect(flash[:error]).to eq(I18n.t('admin.export_domain_allows.no_file')) + end + end +end diff --git a/spec/controllers/admin/export_domain_blocks_controller_spec.rb b/spec/controllers/admin/export_domain_blocks_controller_spec.rb new file mode 100644 index 0000000000..0cb2219723 --- /dev/null +++ b/spec/controllers/admin/export_domain_blocks_controller_spec.rb @@ -0,0 +1,47 @@ +require 'rails_helper' + +RSpec.describe Admin::ExportDomainBlocksController, type: :controller do + render_views + + before do + sign_in Fabricate(:user, admin: true), scope: :user + end + + describe 'GET #export' do + it 'renders instances' do + Fabricate(:domain_block, domain: 'bad.domain', severity: 'silence', public_comment: 'bad') + Fabricate(:domain_block, domain: 'worse.domain', severity: 'suspend', reject_media: true, reject_reports: true, public_comment: 'worse', obfuscate: true) + Fabricate(:domain_block, domain: 'reject.media', severity: 'noop', reject_media: true, public_comment: 'reject media') + Fabricate(:domain_block, domain: 'no.op', severity: 'noop', public_comment: 'noop') + + get :export, params: { format: :csv } + expect(response).to have_http_status(200) + expect(response.body).to eq(IO.read(File.join(file_fixture_path, 'domain_blocks.csv'))) + end + end + + describe 'POST #import' do + it 'blocks imported domains' do + allow(DomainBlockWorker).to receive(:perform_async).and_return(true) + + post :import, params: { admin_import: { data: fixture_file_upload('domain_blocks.csv') } } + + expect(response).to redirect_to(admin_instances_path(limited: '1')) + expect(DomainBlockWorker).to have_received(:perform_async).exactly(3).times + + # Header should not be imported + expect(DomainBlock.where(domain: '#domain').present?).to eq(false) + + # Domains should now be added + get :export, params: { format: :csv } + expect(response).to have_http_status(200) + expect(response.body).to eq(IO.read(File.join(file_fixture_path, 'domain_blocks.csv'))) + end + end + + it 'displays error on no file selected' do + post :import, params: { admin_import: {} } + expect(response).to redirect_to(admin_instances_path(limited: '1')) + expect(flash[:error]).to eq(I18n.t('admin.export_domain_blocks.no_file')) + end +end diff --git a/spec/fixtures/files/domain_allows.csv b/spec/fixtures/files/domain_allows.csv new file mode 100644 index 0000000000..4200ac3f52 --- /dev/null +++ b/spec/fixtures/files/domain_allows.csv @@ -0,0 +1,3 @@ +#domain +good.domain +better.domain diff --git a/spec/fixtures/files/domain_blocks.csv b/spec/fixtures/files/domain_blocks.csv new file mode 100644 index 0000000000..28ffb91751 --- /dev/null +++ b/spec/fixtures/files/domain_blocks.csv @@ -0,0 +1,4 @@ +#domain,#severity,#reject_media,#reject_reports,#public_comment,#obfuscate +bad.domain,silence,false,false,bad,false +worse.domain,suspend,true,true,worse,true +reject.media,noop,true,false,reject media,false