From 73a2e3326fe25d52d7067d946be509aeb0eea5c2 Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 16 May 2022 18:26:49 +0200 Subject: [PATCH] Add confirmation page when importing blocked domains (#1773) * Move glitch-soc-specific strings to glitch-soc-specific locale files * Add confirmation page when importing blocked domains --- .../admin/domain_blocks_controller.rb | 21 ++++++++ .../admin/export_domain_blocks_controller.rb | 49 ++++++++++--------- app/javascript/core/admin.js | 6 +++ app/models/form/domain_block_batch.rb | 35 +++++++++++++ .../_domain_block.html.haml | 27 ++++++++++ .../export_domain_blocks/import.html.haml | 21 ++++++++ config/locales-glitch/en.yml | 20 ++++++++ config/locales/en.yml | 12 ----- config/routes.rb | 7 ++- .../admin/domain_blocks_controller_spec.rb | 21 ++++++++ .../export_domain_blocks_controller_spec.rb | 16 +----- 11 files changed, 185 insertions(+), 50 deletions(-) create mode 100644 app/models/form/domain_block_batch.rb create mode 100644 app/views/admin/export_domain_blocks/_domain_block.html.haml create mode 100644 app/views/admin/export_domain_blocks/import.html.haml diff --git a/app/controllers/admin/domain_blocks_controller.rb b/app/controllers/admin/domain_blocks_controller.rb index 16defc1ea8..48e9781d60 100644 --- a/app/controllers/admin/domain_blocks_controller.rb +++ b/app/controllers/admin/domain_blocks_controller.rb @@ -4,6 +4,17 @@ module Admin class DomainBlocksController < BaseController before_action :set_domain_block, only: [:show, :destroy, :edit, :update] + def batch + @form = Form::DomainBlockBatch.new(form_domain_block_batch_params.merge(current_account: current_account, action: action_from_button)) + @form.save + rescue ActionController::ParameterMissing + flash[:alert] = I18n.t('admin.email_domain_blocks.no_domain_block_selected') + rescue Mastodon::NotPermittedError + flash[:alert] = I18n.t('admin.domain_blocks.created_msg') + else + redirect_to admin_instances_path(limited: '1'), notice: I18n.t('admin.domain_blocks.created_msg') + end + def new authorize :domain_block, :create? @domain_block = DomainBlock.new(domain: params[:_domain]) @@ -76,5 +87,15 @@ module Admin def resource_params params.require(:domain_block).permit(:domain, :severity, :reject_media, :reject_reports, :private_comment, :public_comment, :obfuscate) end + + def form_domain_block_batch_params + params.require(:form_domain_block_batch).permit(domain_blocks_attributes: [:enabled, :domain, :severity, :reject_media, :reject_reports, :private_comment, :public_comment, :obfuscate]) + end + + def action_from_button + if params[:save] + 'save' + end + end end end diff --git a/app/controllers/admin/export_domain_blocks_controller.rb b/app/controllers/admin/export_domain_blocks_controller.rb index 0ad5b92b50..db8863551c 100644 --- a/app/controllers/admin/export_domain_blocks_controller.rb +++ b/app/controllers/admin/export_domain_blocks_controller.rb @@ -21,30 +21,33 @@ module Admin 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') + + @import = Admin::Import.new(import_params) + parse_import_data!(export_headers) + + @global_private_comment = I18n.t('admin.export_domain_blocks.import.private_comment_template', source: @import.data_file_name, date: I18n.l(Time.now.utc)) + + @form = Form::DomainBlockBatch.new + @domain_blocks = @data.take(ROWS_PROCESSING_LIMIT).filter_map 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, + private_comment: @global_private_comment, + public_comment: row['#public_comment']&.strip, + obfuscate: row['#obfuscate'].strip) + + domain_block if domain_block.valid? end - redirect_to admin_instances_path(limited: '1') + + @warning_domains = Instance.where(domain: @domain_blocks.map(&:domain)).where('EXISTS (SELECT 1 FROM follows JOIN accounts ON follows.account_id = accounts.id OR follows.target_account_id = accounts.id WHERE accounts.domain = instances.domain)').pluck(:domain) + rescue ActionController::ParameterMissing + flash.now[:alert] = I18n.t('admin.export_domain_blocks.no_file') + set_dummy_import! + render :new end private diff --git a/app/javascript/core/admin.js b/app/javascript/core/admin.js index ef0a8f267b..c1b9f07a4c 100644 --- a/app/javascript/core/admin.js +++ b/app/javascript/core/admin.js @@ -102,6 +102,12 @@ ready(() => { const registrationMode = document.getElementById('form_admin_settings_registrations_mode'); if (registrationMode) onChangeRegistrationMode(registrationMode); + const checkAllElement = document.querySelector('#batch_checkbox_all'); + if (checkAllElement) { + checkAllElement.checked = [].every.call(document.querySelectorAll(batchCheckboxClassName), (content) => content.checked); + checkAllElement.indeterminate = !checkAllElement.checked && [].some.call(document.querySelectorAll(batchCheckboxClassName), (content) => content.checked); + } + document.querySelector('a#add-instance-button')?.addEventListener('click', (e) => { const domain = document.getElementById('by_domain')?.value; diff --git a/app/models/form/domain_block_batch.rb b/app/models/form/domain_block_batch.rb new file mode 100644 index 0000000000..39012df517 --- /dev/null +++ b/app/models/form/domain_block_batch.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +class Form::DomainBlockBatch + include ActiveModel::Model + include Authorization + include AccountableConcern + + attr_accessor :domain_blocks_attributes, :action, :current_account + + def save + case action + when 'save' + save! + end + end + + private + + def domain_blocks + @domain_blocks ||= domain_blocks_attributes.values.filter_map do |attributes| + DomainBlock.new(attributes.without('enabled')) if ActiveModel::Type::Boolean.new.cast(attributes['enabled']) + end + end + + def save! + domain_blocks.each do |domain_block| + authorize(domain_block, :create?) + next if DomainBlock.rule_for(domain_block.domain).present? + + domain_block.save! + DomainBlockWorker.perform_async(domain_block.id) + log_action :create, domain_block + end + end +end diff --git a/app/views/admin/export_domain_blocks/_domain_block.html.haml b/app/views/admin/export_domain_blocks/_domain_block.html.haml new file mode 100644 index 0000000000..5d4b6c4d0d --- /dev/null +++ b/app/views/admin/export_domain_blocks/_domain_block.html.haml @@ -0,0 +1,27 @@ +- existing_relationships ||= false + +.batch-table__row{ class: [existing_relationships && 'batch-table__row--attention'] } + %label.batch-table__row__select.batch-table__row__select--aligned.batch-checkbox + = f.check_box :enabled, checked: !existing_relationships + .batch-table__row__content.pending-account + .pending-account__header + %strong + = f.object.domain + = f.hidden_field :domain + = f.hidden_field :severity + = f.hidden_field :reject_media + = f.hidden_field :reject_reports + = f.hidden_field :obfuscate + = f.hidden_field :private_comment + = f.hidden_field :public_comment + + %br/ + + = f.object.policies.map { |policy| t(policy, scope: 'admin.instances.content_policies.policies') }.join(' • ') + - if f.object.public_comment.present? + • + = f.object.public_comment + - if existing_relationships + • + = fa_icon 'warning fw' + = t('admin.export_domain_blocks.import.existing_relationships_warning') diff --git a/app/views/admin/export_domain_blocks/import.html.haml b/app/views/admin/export_domain_blocks/import.html.haml new file mode 100644 index 0000000000..01add232d1 --- /dev/null +++ b/app/views/admin/export_domain_blocks/import.html.haml @@ -0,0 +1,21 @@ +- content_for :page_title do + = t('admin.export_domain_blocks.import.title') + +%p= t('admin.export_domain_blocks.import.description_html') + +- if defined?(@global_private_comment) && @global_private_comment.present? + %p= t('admin.export_domain_blocks.import.private_comment_description_html', comment: @global_private_comment) + += form_for(@form, url: batch_admin_domain_blocks_path) do |f| + .batch-table + .batch-table__toolbar + %label.batch-table__toolbar__select.batch-checkbox-all + = check_box_tag :batch_checkbox_all, nil, false + .batch-table__toolbar__actions + = f.button safe_join([fa_icon('copy'), t('admin.domain_blocks.import')]), name: :save, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') } + .batch-table__body + - if @domain_blocks.empty? + = nothing_here 'nothing-here--under-tabs' + - else + = f.simple_fields_for :domain_blocks, @domain_blocks do |ff| + = render 'domain_block', f: ff, existing_relationships: @warning_domains.include?(ff.object.domain) diff --git a/config/locales-glitch/en.yml b/config/locales-glitch/en.yml index 3b554f4a2b..78933f54e0 100644 --- a/config/locales-glitch/en.yml +++ b/config/locales-glitch/en.yml @@ -4,6 +4,26 @@ en: custom_emojis: batch_copy_error: 'An error occurred when copying some of the selected emoji: %{message}' batch_error: 'An error occurred: %{message}' + domain_allows: + export: Export + import: Import + domain_blocks: + export: Export + import: Import + export_domain_allows: + new: + title: Import domain allows + no_file: No file selected + export_domain_blocks: + import: + description_html: You are about to import a list of domain blocks. Please review this list very carefully, especially if you have not authored this list yourself. + existing_relationships_warning: Existing follow relationships + private_comment_description_html: 'To help you track where imported blocks come from, imported blocks will be created with the following private comment: %{comment}' + private_comment_template: Imported from %{source} on %{date} + title: Import domain blocks + new: + title: Import domain blocks + no_file: No file selected settings: captcha_enabled: desc_html: This relies on external scripts from hCaptcha, which may be a security and privacy concern. In addition, this can make the registration process significantly less accessible to some (especially disabled) people. For these reasons, please consider alternative measures such as approval-based or invite-based registration.
Users that have been invited through a limited-use invite will not need to solve a CAPTCHA diff --git a/config/locales/en.yml b/config/locales/en.yml index 4b39d549bb..b90402cdd4 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -421,8 +421,6 @@ 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 @@ -431,8 +429,6 @@ 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. @@ -473,14 +469,6 @@ 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 7876651924..5ab3ec1e13 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -194,7 +194,11 @@ Rails.application.routes.draw do get '/dashboard', to: 'dashboard#index' resources :domain_allows, only: [:new, :create, :show, :destroy] - resources :domain_blocks, only: [:new, :create, :show, :destroy, :update, :edit] + resources :domain_blocks, only: [:new, :create, :show, :destroy, :update, :edit] do + collection do + post :batch + end + end resources :export_domain_allows, only: [:new] do collection do @@ -485,6 +489,7 @@ Rails.application.routes.draw do end resource :domain_blocks, only: [:show, :create, :destroy] + resource :directory, only: [:show] resources :follow_requests, only: [:index] do diff --git a/spec/controllers/admin/domain_blocks_controller_spec.rb b/spec/controllers/admin/domain_blocks_controller_spec.rb index ecc79292b4..a35b2fb3b9 100644 --- a/spec/controllers/admin/domain_blocks_controller_spec.rb +++ b/spec/controllers/admin/domain_blocks_controller_spec.rb @@ -16,6 +16,27 @@ RSpec.describe Admin::DomainBlocksController, type: :controller do end end + describe 'POST #batch' do + it 'blocks the domains when succeeded to save' do + allow(DomainBlockWorker).to receive(:perform_async).and_return(true) + + post :batch, params: { + save: '', + form_domain_block_batch: { + domain_blocks_attributes: { + '0' => { enabled: '1', domain: 'example.com', severity: 'silence' }, + '1' => { enabled: '0', domain: 'mastodon.social', severity: 'suspend' }, + '2' => { enabled: '1', domain: 'mastodon.online', severity: 'suspend' } + } + } + } + + expect(DomainBlockWorker).to have_received(:perform_async).exactly(2).times + expect(flash[:notice]).to eq I18n.t('admin.domain_blocks.created_msg') + expect(response).to redirect_to(admin_instances_path(limited: '1')) + end + end + describe 'POST #create' do it 'blocks the domain when succeeded to save' do allow(DomainBlockWorker).to receive(:perform_async).and_return(true) diff --git a/spec/controllers/admin/export_domain_blocks_controller_spec.rb b/spec/controllers/admin/export_domain_blocks_controller_spec.rb index 0cb2219723..0493df8591 100644 --- a/spec/controllers/admin/export_domain_blocks_controller_spec.rb +++ b/spec/controllers/admin/export_domain_blocks_controller_spec.rb @@ -22,26 +22,14 @@ RSpec.describe Admin::ExportDomainBlocksController, type: :controller do 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'))) + expect(assigns(:domain_blocks).map(&:domain)).to match_array ['bad.domain', 'worse.domain', 'reject.media'] 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')) + expect(flash[:alert]).to eq(I18n.t('admin.export_domain_blocks.no_file')) end end