From 14f6ce2885f7999f2fcbbdda6241a035271076d4 Mon Sep 17 00:00:00 2001 From: ThibG Date: Tue, 14 May 2019 19:05:02 +0200 Subject: Record account suspend/silence time and keep track of domain blocks (#10660) * Record account suspend/silence time and keep track of domain blocks * Also unblock users who were suspended/silenced before dates were recorded * Add tests * Keep track of suspending date for users suspended through the CLI * Show accurate number of accounts that would be affected by unsuspending an instance * Change migration to set silenced_at and suspended_at * Revert "Also unblock users who were suspended/silenced before dates were recorded" This reverts commit a015c65d2d1e28c7b7cfab8b3f8cd5fb48b8b71c. * Switch from using suspended and silenced to suspended_at and silenced_at * Add post-deployment migration script to remove `suspended` and `silenced` columns * Use Account#silence! and Account#suspend! instead of updating the underlying property * Add silenced_at and suspended_at migration to post-migration * Change account fabricator to translate suspended and silenced attributes * Minor fixes * Make unblocking domains always retroactive --- app/controllers/admin/domain_blocks_controller.rb | 8 ++--- app/controllers/home_controller.rb | 2 +- app/models/account.rb | 40 ++++++++++++++-------- app/models/concerns/account_finder_concern.rb | 2 +- app/models/domain_block.rb | 7 ++-- app/models/status.rb | 4 +-- app/models/user.rb | 2 +- .../activitypub/process_account_service.rb | 12 +++---- app/services/block_domain_service.rb | 6 ++-- app/services/post_status_service.rb | 2 +- app/services/process_mentions_service.rb | 2 +- app/services/resolve_account_service.rb | 6 ++-- app/services/subscribe_service.rb | 2 +- app/services/suspend_account_service.rb | 4 +-- app/services/unblock_domain_service.rb | 15 +++++--- app/views/admin/domain_blocks/show.html.haml | 17 +++------ 16 files changed, 69 insertions(+), 62 deletions(-) (limited to 'app') diff --git a/app/controllers/admin/domain_blocks_controller.rb b/app/controllers/admin/domain_blocks_controller.rb index dd3f83389..71597763b 100644 --- a/app/controllers/admin/domain_blocks_controller.rb +++ b/app/controllers/admin/domain_blocks_controller.rb @@ -41,7 +41,7 @@ module Admin def destroy authorize @domain_block, :destroy? - UnblockDomainService.new.call(@domain_block, retroactive_unblock?) + UnblockDomainService.new.call(@domain_block) log_action :destroy, @domain_block redirect_to admin_instances_path(limited: '1'), notice: I18n.t('admin.domain_blocks.destroyed_msg') end @@ -53,11 +53,7 @@ module Admin end def resource_params - params.require(:domain_block).permit(:domain, :severity, :reject_media, :reject_reports, :retroactive) - end - - def retroactive_unblock? - ActiveRecord::Type.lookup(:boolean).cast(resource_params[:retroactive]) + params.require(:domain_block).permit(:domain, :severity, :reject_media, :reject_reports) end end end diff --git a/app/controllers/home_controller.rb b/app/controllers/home_controller.rb index d1bd0601e..85622a7b5 100644 --- a/app/controllers/home_controller.rb +++ b/app/controllers/home_controller.rb @@ -58,7 +58,7 @@ class HomeController < ApplicationController if request.path.start_with?('/web') new_user_session_path elsif single_user_mode? - short_account_path(Account.local.where(suspended: false).first) + short_account_path(Account.local.without_suspended.first) else about_path end diff --git a/app/models/account.rb b/app/models/account.rb index 51e01246e..a894d5be5 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -28,8 +28,6 @@ # header_updated_at :datetime # avatar_remote_url :string # subscription_expires_at :datetime -# silenced :boolean default(FALSE), not null -# suspended :boolean default(FALSE), not null # locked :boolean default(FALSE), not null # header_remote_url :string default(""), not null # last_webfingered_at :datetime @@ -45,6 +43,8 @@ # actor_type :string # discoverable :boolean # also_known_as :string is an Array +# silenced_at :datetime +# suspended_at :datetime # class Account < ApplicationRecord @@ -82,10 +82,10 @@ class Account < ApplicationRecord scope :local, -> { where(domain: nil) } scope :expiring, ->(time) { remote.where.not(subscription_expires_at: nil).where('subscription_expires_at < ?', time) } scope :partitioned, -> { order(Arel.sql('row_number() over (partition by domain)')) } - scope :silenced, -> { where(silenced: true) } - scope :suspended, -> { where(suspended: true) } - scope :without_suspended, -> { where(suspended: false) } - scope :without_silenced, -> { where(silenced: false) } + scope :silenced, -> { where.not(silenced_at: nil) } + scope :suspended, -> { where.not(suspended_at: nil) } + scope :without_suspended, -> { where(suspended_at: nil) } + scope :without_silenced, -> { where(silenced_at: nil) } scope :recent, -> { reorder(id: :desc) } scope :bots, -> { where(actor_type: %w(Application Service)) } scope :alphabetic, -> { order(domain: :asc, username: :asc) } @@ -165,25 +165,35 @@ class Account < ApplicationRecord ResolveAccountService.new.call(acct) end - def silence! - update!(silenced: true) + def silenced? + silenced_at.present? + end + + def silence!(date = nil) + date ||= Time.now.utc + update!(silenced_at: date) end def unsilence! - update!(silenced: false) + update!(silenced_at: nil) + end + + def suspended? + suspended_at.present? end - def suspend! + def suspend!(date = nil) + date ||= Time.now.utc transaction do user&.disable! if local? - update!(suspended: true) + update!(suspended_at: date) end end def unsuspend! transaction do user&.enable! if local? - update!(suspended: false) + update!(suspended_at: nil) end end @@ -399,7 +409,7 @@ class Account < ApplicationRecord ts_rank_cd(#{textsearch}, #{query}, 32) AS rank FROM accounts WHERE #{query} @@ #{textsearch} - AND accounts.suspended = false + AND accounts.suspended_at IS NULL AND accounts.moved_to_account_id IS NULL ORDER BY rank DESC LIMIT ? OFFSET ? @@ -427,7 +437,7 @@ class Account < ApplicationRecord LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = ?) OR (accounts.id = f.target_account_id AND f.account_id = ?) WHERE accounts.id IN (SELECT * FROM first_degree) AND #{query} @@ #{textsearch} - AND accounts.suspended = false + AND accounts.suspended_at IS NULL AND accounts.moved_to_account_id IS NULL GROUP BY accounts.id ORDER BY rank DESC @@ -443,7 +453,7 @@ class Account < ApplicationRecord FROM accounts LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = ?) OR (accounts.id = f.target_account_id AND f.account_id = ?) WHERE #{query} @@ #{textsearch} - AND accounts.suspended = false + AND accounts.suspended_at IS NULL AND accounts.moved_to_account_id IS NULL GROUP BY accounts.id ORDER BY rank DESC diff --git a/app/models/concerns/account_finder_concern.rb b/app/models/concerns/account_finder_concern.rb index 0ac49cc12..ccd7bfa12 100644 --- a/app/models/concerns/account_finder_concern.rb +++ b/app/models/concerns/account_finder_concern.rb @@ -13,7 +13,7 @@ module AccountFinderConcern end def representative - find_local(Setting.site_contact_username.strip.gsub(/\A@/, '')) || Account.local.find_by(suspended: false) + find_local(Setting.site_contact_username.strip.gsub(/\A@/, '')) || Account.local.without_suspended.first end def find_local(username) diff --git a/app/models/domain_block.rb b/app/models/domain_block.rb index 0b12617c6..84c08c158 100644 --- a/app/models/domain_block.rb +++ b/app/models/domain_block.rb @@ -17,8 +17,6 @@ class DomainBlock < ApplicationRecord enum severity: [:silence, :suspend, :noop] - attr_accessor :retroactive - validates :domain, presence: true, uniqueness: true has_many :accounts, foreign_key: :domain, primary_key: :domain @@ -36,4 +34,9 @@ class DomainBlock < ApplicationRecord return false if other_block.silence? && noop? (reject_media || !other_block.reject_media) && (reject_reports || !other_block.reject_reports) end + + def affected_accounts_count + scope = suspend? ? accounts.where(suspended_at: created_at) : accounts.where(silenced_at: created_at) + scope.count + end end diff --git a/app/models/status.rb b/app/models/status.rb index 8d31fd382..2f6101b90 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -84,8 +84,8 @@ class Status < ApplicationRecord scope :without_reblogs, -> { where('statuses.reblog_of_id IS NULL') } scope :with_public_visibility, -> { where(visibility: :public) } scope :tagged_with, ->(tag) { joins(:statuses_tags).where(statuses_tags: { tag_id: tag }) } - scope :excluding_silenced_accounts, -> { left_outer_joins(:account).where(accounts: { silenced: false }) } - scope :including_silenced_accounts, -> { left_outer_joins(:account).where(accounts: { silenced: true }) } + scope :excluding_silenced_accounts, -> { left_outer_joins(:account).where(accounts: { silenced_at: nil }) } + scope :including_silenced_accounts, -> { left_outer_joins(:account).where.not(accounts: { silenced_at: nil }) } scope :not_excluded_by_account, ->(account) { where.not(account_id: account.excluded_from_timeline_account_ids) } scope :not_domain_blocked_by_account, ->(account) { account.excluded_from_timeline_domains.blank? ? left_outer_joins(:account) : left_outer_joins(:account).where('accounts.domain IS NULL OR accounts.domain NOT IN (?)', account.excluded_from_timeline_domains) } scope :tagged_with_all, ->(tags) { diff --git a/app/models/user.rb b/app/models/user.rb index bce28aa5f..6941b040d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -88,7 +88,7 @@ class User < ApplicationRecord scope :confirmed, -> { where.not(confirmed_at: nil) } scope :enabled, -> { where(disabled: false) } scope :inactive, -> { where(arel_table[:current_sign_in_at].lt(ACTIVE_DURATION.ago)) } - scope :active, -> { confirmed.where(arel_table[:current_sign_in_at].gteq(ACTIVE_DURATION.ago)).joins(:account).where(accounts: { suspended: false }) } + scope :active, -> { confirmed.where(arel_table[:current_sign_in_at].gteq(ACTIVE_DURATION.ago)).joins(:account).where.not(accounts: { suspended_at: nil }) } scope :matches_email, ->(value) { where(arel_table[:email].matches("#{value}%")) } scope :emailable, -> { confirmed.enabled.joins(:account).merge(Account.searchable) } diff --git a/app/services/activitypub/process_account_service.rb b/app/services/activitypub/process_account_service.rb index 6d0609ca0..ad22d37fe 100644 --- a/app/services/activitypub/process_account_service.rb +++ b/app/services/activitypub/process_account_service.rb @@ -50,12 +50,12 @@ class ActivityPub::ProcessAccountService < BaseService def create_account @account = Account.new - @account.protocol = :activitypub - @account.username = @username - @account.domain = @domain - @account.suspended = true if auto_suspend? - @account.silenced = true if auto_silence? - @account.private_key = nil + @account.protocol = :activitypub + @account.username = @username + @account.domain = @domain + @account.private_key = nil + @account.suspended_at = domain_block.created_at if auto_suspend? + @account.silenced_at = domain_block.created_at if auto_silence? end def update_account diff --git a/app/services/block_domain_service.rb b/app/services/block_domain_service.rb index a1fe93665..497f0394b 100644 --- a/app/services/block_domain_service.rb +++ b/app/services/block_domain_service.rb @@ -29,7 +29,7 @@ class BlockDomainService < BaseService end def silence_accounts! - blocked_domain_accounts.in_batches.update_all(silenced: true) + blocked_domain_accounts.without_silenced.in_batches.update_all(silenced_at: @domain_block.created_at) end def clear_media! @@ -43,9 +43,9 @@ class BlockDomainService < BaseService end def suspend_accounts! - blocked_domain_accounts.where(suspended: false).reorder(nil).find_each do |account| + blocked_domain_accounts.without_suspended.reorder(nil).find_each do |account| UnsubscribeService.new.call(account) if account.subscribed? - SuspendAccountService.new.call(account) + SuspendAccountService.new.call(account, suspended_at: @domain_block.created_at) end end diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index e7366c7e8..b470a40e9 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -49,7 +49,7 @@ class PostStatusService < BaseService def preprocess_attributes! @text = @options.delete(:spoiler_text) if @text.blank? && @options[:spoiler_text].present? @visibility = @options[:visibility] || @account.user&.setting_default_privacy - @visibility = :unlisted if @visibility == :public && @account.silenced + @visibility = :unlisted if @visibility == :public && @account.silenced? @scheduled_at = @options[:scheduled_at]&.to_datetime @scheduled_at = nil if scheduled_in_the_past? rescue ArgumentError diff --git a/app/services/process_mentions_service.rb b/app/services/process_mentions_service.rb index 2595c5fd3..d27a7dab4 100644 --- a/app/services/process_mentions_service.rb +++ b/app/services/process_mentions_service.rb @@ -25,7 +25,7 @@ class ProcessMentionsService < BaseService end end - next match if mention_undeliverable?(mentioned_account) || mentioned_account&.suspended + next match if mention_undeliverable?(mentioned_account) || mentioned_account&.suspended? mentions << mentioned_account.mentions.where(status: status).first_or_create(status: status) diff --git a/app/services/resolve_account_service.rb b/app/services/resolve_account_service.rb index 4ff351c5f..11e33a83a 100644 --- a/app/services/resolve_account_service.rb +++ b/app/services/resolve_account_service.rb @@ -119,9 +119,9 @@ class ResolveAccountService < BaseService Rails.logger.debug "Creating new remote account for #{@username}@#{@domain}" @account = Account.new(username: @username, domain: @domain) - @account.suspended = true if auto_suspend? - @account.silenced = true if auto_silence? - @account.private_key = nil + @account.suspended_at = domain_block.created_at if auto_suspend? + @account.silenced_at = domain_block.created_at if auto_silence? + @account.private_key = nil end def update_account diff --git a/app/services/subscribe_service.rb b/app/services/subscribe_service.rb index 2893b5410..83fd64396 100644 --- a/app/services/subscribe_service.rb +++ b/app/services/subscribe_service.rb @@ -43,7 +43,7 @@ class SubscribeService < BaseService end def some_local_account - @some_local_account ||= Account.local.where(suspended: false).first + @some_local_account ||= Account.local.without_suspended.first end # Any response in the 3xx or 4xx range, except for 429 (rate limit) diff --git a/app/services/suspend_account_service.rb b/app/services/suspend_account_service.rb index 6c2ecad30..412873f84 100644 --- a/app/services/suspend_account_service.rb +++ b/app/services/suspend_account_service.rb @@ -88,8 +88,8 @@ class SuspendAccountService < BaseService return if @options[:destroy] - @account.silenced = false - @account.suspended = true + @account.silenced_at = nil + @account.suspended_at = @options[:suspended_at] || Time.now.utc @account.locked = false @account.display_name = '' @account.note = '' diff --git a/app/services/unblock_domain_service.rb b/app/services/unblock_domain_service.rb index 946b6d465..9b8526fbe 100644 --- a/app/services/unblock_domain_service.rb +++ b/app/services/unblock_domain_service.rb @@ -3,9 +3,9 @@ class UnblockDomainService < BaseService attr_accessor :domain_block - def call(domain_block, retroactive) + def call(domain_block) @domain_block = domain_block - process_retroactive_updates if retroactive + process_retroactive_updates domain_block.destroy end @@ -14,14 +14,19 @@ class UnblockDomainService < BaseService end def blocked_accounts - Account.where(domain: domain_block.domain) + scope = Account.where(domain: domain_block.domain) + if domain_block.silence? + scope.where(silenced_at: @domain_block.created_at) + else + scope.where(suspended_at: @domain_block.created_at) + end end def update_options - { domain_block_impact => false } + { domain_block_impact => nil } end def domain_block_impact - domain_block.silence? ? :silenced : :suspended + domain_block.silence? ? :silenced_at : :suspended_at end end diff --git a/app/views/admin/domain_blocks/show.html.haml b/app/views/admin/domain_blocks/show.html.haml index ea1929d44..dca4dbac7 100644 --- a/app/views/admin/domain_blocks/show.html.haml +++ b/app/views/admin/domain_blocks/show.html.haml @@ -3,18 +3,11 @@ = simple_form_for @domain_block, url: admin_domain_block_path(@domain_block), method: :delete do |f| - - if (@domain_block.noop?) - = f.input :retroactive, - as: :hidden, - input_html: { :value => "0" } - - else - = f.input :retroactive, - as: :boolean, - wrapper: :with_label, - label: t(".retroactive.#{@domain_block.severity}"), - hint: t(:affected_accounts, - scope: [:admin, :domain_blocks, :show], - count: @domain_block.accounts_count) + - unless (@domain_block.noop?) + %p= t(".retroactive.#{@domain_block.severity}") + %p.hint= t(:affected_accounts, + scope: [:admin, :domain_blocks, :show], + count: @domain_block.affected_accounts_count) .actions = f.button :button, t('.undo'), type: :submit -- cgit