From 3134691948aeacb16b7386ed77bbea4581beec40 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 8 Nov 2020 00:28:39 +0100 Subject: [PATCH] Add support for reversible suspensions through ActivityPub (#14989) --- app/controllers/accounts_controller.rb | 4 + .../activitypub/base_controller.rb | 4 + .../activitypub/inboxes_controller.rb | 4 + .../activitypub/replies_controller.rb | 2 +- .../concerns/account_owned_concern.rb | 20 ++- .../follower_accounts_controller.rb | 12 +- .../following_accounts_controller.rb | 12 +- .../settings/deletes_controller.rb | 2 +- .../well_known/webfinger_controller.rb | 2 +- app/javascript/styles/mastodon/widgets.scss | 4 + app/lib/activitypub/adapter.rb | 1 + app/lib/webfinger.rb | 4 + app/models/account.rb | 16 +- app/models/admin/account_action.rb | 2 +- app/policies/account_policy.rb | 4 +- .../activitypub/actor_serializer.rb | 33 ++-- .../activitypub/process_account_service.rb | 55 ++++-- .../activitypub/process_collection_service.rb | 10 +- app/services/block_domain_service.rb | 5 +- app/services/delete_account_service.rb | 40 +++-- app/services/resolve_account_service.rb | 27 ++- app/services/suspend_account_service.rb | 29 +++ app/services/unblock_domain_service.rb | 2 +- app/services/unsuspend_account_service.rb | 20 +++ app/workers/account_deletion_worker.rb | 4 +- ...33919_add_suspension_origin_to_accounts.rb | 5 + ...17234926_fill_account_suspension_origin.rb | 11 ++ db/schema.rb | 3 +- lib/mastodon/accounts_cli.rb | 2 +- .../account_follow_controller_spec.rb | 48 ++++- .../account_unfollow_controller_spec.rb | 48 ++++- spec/controllers/accounts_controller_spec.rb | 68 ++++++- .../collections_controller_spec.rb | 32 +++- ...lowers_synchronizations_controller_spec.rb | 31 +++- .../activitypub/inboxes_controller_spec.rb | 27 +++ .../activitypub/outboxes_controller_spec.rb | 58 +++++- .../activitypub/replies_controller_spec.rb | 39 +++- .../api/v1/admin/accounts_controller_spec.rb | 2 +- .../follower_accounts_controller_spec.rb | 63 +++++++ .../following_accounts_controller_spec.rb | 63 +++++++ .../remote_follow_controller_spec.rb | 33 +++- spec/controllers/statuses_controller_spec.rb | 38 +++- .../well_known/webfinger_controller_spec.rb | 169 +++++++++++------- spec/policies/account_policy_spec.rb | 35 +++- .../process_account_service_spec.rb | 80 +++++++++ .../process_collection_service_spec.rb | 43 ++++- spec/services/resolve_account_service_spec.rb | 29 ++- 47 files changed, 1045 insertions(+), 200 deletions(-) create mode 100644 db/migrate/20201017233919_add_suspension_origin_to_accounts.rb create mode 100644 db/post_migrate/20201017234926_fill_account_suspension_origin.rb diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index 6d711afd0..ccb5ef8e8 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -102,6 +102,10 @@ class AccountsController < ApplicationController params[:username] end + def skip_temporary_suspension_response? + request.format == :json + end + def rss_url if tag_requested? short_account_tag_url(@account, params[:tag], format: 'rss') diff --git a/app/controllers/activitypub/base_controller.rb b/app/controllers/activitypub/base_controller.rb index 0c2591e97..4cbc3ab8f 100644 --- a/app/controllers/activitypub/base_controller.rb +++ b/app/controllers/activitypub/base_controller.rb @@ -8,4 +8,8 @@ class ActivityPub::BaseController < Api::BaseController def set_cache_headers response.headers['Vary'] = 'Signature' if authorized_fetch_mode? end + + def skip_temporary_suspension_response? + false + end end diff --git a/app/controllers/activitypub/inboxes_controller.rb b/app/controllers/activitypub/inboxes_controller.rb index fdb60d590..d3044f180 100644 --- a/app/controllers/activitypub/inboxes_controller.rb +++ b/app/controllers/activitypub/inboxes_controller.rb @@ -33,6 +33,10 @@ class ActivityPub::InboxesController < ActivityPub::BaseController params[:account_username].present? end + def skip_temporary_suspension_response? + true + end + def body return @body if defined?(@body) diff --git a/app/controllers/activitypub/replies_controller.rb b/app/controllers/activitypub/replies_controller.rb index 43bf4e657..fde6c861f 100644 --- a/app/controllers/activitypub/replies_controller.rb +++ b/app/controllers/activitypub/replies_controller.rb @@ -31,7 +31,7 @@ class ActivityPub::RepliesController < ActivityPub::BaseController end def set_replies - @replies = only_other_accounts? ? Status.where.not(account_id: @account.id) : @account.statuses + @replies = only_other_accounts? ? Status.where.not(account_id: @account.id).joins(:account).merge(Account.without_suspended) : @account.statuses @replies = @replies.where(in_reply_to_id: @status.id, visibility: [:public, :unlisted]) @replies = @replies.paginate_by_min_id(DESCENDANTS_LIMIT, params[:min_id]) end diff --git a/app/controllers/concerns/account_owned_concern.rb b/app/controllers/concerns/account_owned_concern.rb index 460f71f65..62e379846 100644 --- a/app/controllers/concerns/account_owned_concern.rb +++ b/app/controllers/concerns/account_owned_concern.rb @@ -29,6 +29,24 @@ module AccountOwnedConcern end def check_account_suspension - expires_in(3.minutes, public: true) && gone if @account.suspended? + if @account.suspended_permanently? + permanent_suspension_response + elsif @account.suspended? && !skip_temporary_suspension_response? + temporary_suspension_response + end + end + + def skip_temporary_suspension_response? + false + end + + def permanent_suspension_response + expires_in(3.minutes, public: true) + gone + end + + def temporary_suspension_response + expires_in(3.minutes, public: true) + forbidden end end diff --git a/app/controllers/follower_accounts_controller.rb b/app/controllers/follower_accounts_controller.rb index ab0749963..ff4df2adf 100644 --- a/app/controllers/follower_accounts_controller.rb +++ b/app/controllers/follower_accounts_controller.rb @@ -52,6 +52,14 @@ class FollowerAccountsController < ApplicationController account_followers_url(@account, page: page) unless page.nil? end + def next_page_url + page_url(follows.next_page) if follows.respond_to?(:next_page) + end + + def prev_page_url + page_url(follows.prev_page) if follows.respond_to?(:prev_page) + end + def collection_presenter if page_requested? ActivityPub::CollectionPresenter.new( @@ -60,8 +68,8 @@ class FollowerAccountsController < ApplicationController size: @account.followers_count, items: follows.map { |f| ActivityPub::TagManager.instance.uri_for(f.account) }, part_of: account_followers_url(@account), - next: page_url(follows.next_page), - prev: page_url(follows.prev_page) + next: next_page_url, + prev: prev_page_url ) else ActivityPub::CollectionPresenter.new( diff --git a/app/controllers/following_accounts_controller.rb b/app/controllers/following_accounts_controller.rb index 918bdac0a..6bb95c454 100644 --- a/app/controllers/following_accounts_controller.rb +++ b/app/controllers/following_accounts_controller.rb @@ -52,6 +52,14 @@ class FollowingAccountsController < ApplicationController account_following_index_url(@account, page: page) unless page.nil? end + def next_page_url + page_url(follows.next_page) if follows.respond_to?(:next_page) + end + + def prev_page_url + page_url(follows.prev_page) if follows.respond_to?(:prev_page) + end + def collection_presenter if page_requested? ActivityPub::CollectionPresenter.new( @@ -60,8 +68,8 @@ class FollowingAccountsController < ApplicationController size: @account.following_count, items: follows.map { |f| ActivityPub::TagManager.instance.uri_for(f.target_account) }, part_of: account_following_index_url(@account), - next: page_url(follows.next_page), - prev: page_url(follows.prev_page) + next: next_page_url, + prev: prev_page_url ) else ActivityPub::CollectionPresenter.new( diff --git a/app/controllers/settings/deletes_controller.rb b/app/controllers/settings/deletes_controller.rb index f96c83b80..7b8f8d207 100644 --- a/app/controllers/settings/deletes_controller.rb +++ b/app/controllers/settings/deletes_controller.rb @@ -42,7 +42,7 @@ class Settings::DeletesController < Settings::BaseController end def destroy_account! - current_account.suspend! + current_account.suspend!(origin: :local) AccountDeletionWorker.perform_async(current_user.account_id) sign_out end diff --git a/app/controllers/well_known/webfinger_controller.rb b/app/controllers/well_known/webfinger_controller.rb index 9de9db6ba..0227f722a 100644 --- a/app/controllers/well_known/webfinger_controller.rb +++ b/app/controllers/well_known/webfinger_controller.rb @@ -35,7 +35,7 @@ module WellKnown end def check_account_suspension - expires_in(3.minutes, public: true) && gone if @account.suspended? + expires_in(3.minutes, public: true) && gone if @account.suspended_permanently? end def bad_request diff --git a/app/javascript/styles/mastodon/widgets.scss b/app/javascript/styles/mastodon/widgets.scss index 5b97d1ec4..47e02d41d 100644 --- a/app/javascript/styles/mastodon/widgets.scss +++ b/app/javascript/styles/mastodon/widgets.scss @@ -2,6 +2,10 @@ margin-bottom: 10px; box-shadow: 0 0 15px rgba($base-shadow-color, 0.2); + &:last-child { + margin-bottom: 0; + } + &__img { width: 100%; position: relative; diff --git a/app/lib/activitypub/adapter.rb b/app/lib/activitypub/adapter.rb index 9a786c9a4..2d6b87659 100644 --- a/app/lib/activitypub/adapter.rb +++ b/app/lib/activitypub/adapter.rb @@ -23,6 +23,7 @@ class ActivityPub::Adapter < ActiveModelSerializers::Adapter::Base discoverable: { 'toot' => 'http://joinmastodon.org/ns#', 'discoverable' => 'toot:discoverable' }, voters_count: { 'toot' => 'http://joinmastodon.org/ns#', 'votersCount' => 'toot:votersCount' }, olm: { 'toot' => 'http://joinmastodon.org/ns#', 'Device' => 'toot:Device', 'Ed25519Signature' => 'toot:Ed25519Signature', 'Ed25519Key' => 'toot:Ed25519Key', 'Curve25519Key' => 'toot:Curve25519Key', 'EncryptedMessage' => 'toot:EncryptedMessage', 'publicKeyBase64' => 'toot:publicKeyBase64', 'deviceId' => 'toot:deviceId', 'claim' => { '@type' => '@id', '@id' => 'toot:claim' }, 'fingerprintKey' => { '@type' => '@id', '@id' => 'toot:fingerprintKey' }, 'identityKey' => { '@type' => '@id', '@id' => 'toot:identityKey' }, 'devices' => { '@type' => '@id', '@id' => 'toot:devices' }, 'messageFranking' => 'toot:messageFranking', 'messageType' => 'toot:messageType', 'cipherText' => 'toot:cipherText' }, + suspended: { 'toot' => 'http://joinmastodon.org/ns#', 'suspended' => 'toot:suspended' }, }.freeze def self.default_key_transform diff --git a/app/lib/webfinger.rb b/app/lib/webfinger.rb index b2374c494..c7aa43bb3 100644 --- a/app/lib/webfinger.rb +++ b/app/lib/webfinger.rb @@ -2,6 +2,8 @@ class Webfinger class Error < StandardError; end + class GoneError < Error; end + class RedirectError < StandardError; end class Response def initialize(body) @@ -47,6 +49,8 @@ class Webfinger res.body_with_limit elsif res.code == 404 && use_fallback body_from_host_meta + elsif res.code == 410 + raise Webfinger::GoneError, "#{@uri} is gone from the server" else raise Webfinger::Error, "Request for #{@uri} returned HTTP #{res.code}" end diff --git a/app/models/account.rb b/app/models/account.rb index d2112a13d..bc9bcc72d 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -51,6 +51,7 @@ # header_storage_schema_version :integer # devices_url :string # sensitized_at :datetime +# suspension_origin :integer # class Account < ApplicationRecord @@ -73,6 +74,7 @@ class Account < ApplicationRecord }.freeze enum protocol: [:ostatus, :activitypub] + enum suspension_origin: [:local, :remote], _prefix: true validates :username, presence: true validates_with UniqueUsernameValidator, if: -> { will_save_change_to_username? } @@ -222,17 +224,25 @@ class Account < ApplicationRecord suspended_at.present? end - def suspend!(date = Time.now.utc) + def suspended_permanently? + suspended? && deletion_request.nil? + end + + def suspended_temporarily? + suspended? && deletion_request.present? + end + + def suspend!(date: Time.now.utc, origin: :local) transaction do create_deletion_request! - update!(suspended_at: date) + update!(suspended_at: date, suspension_origin: origin) end end def unsuspend! transaction do deletion_request&.destroy! - update!(suspended_at: nil) + update!(suspended_at: nil, suspension_origin: nil) end end diff --git a/app/models/admin/account_action.rb b/app/models/admin/account_action.rb index 11ce737f3..bf222391f 100644 --- a/app/models/admin/account_action.rb +++ b/app/models/admin/account_action.rb @@ -127,7 +127,7 @@ class Admin::AccountAction def handle_suspend! authorize(target_account, :suspend?) log_action(:suspend, target_account) - target_account.suspend! + target_account.suspend!(origin: :local) end def text_for_warning diff --git a/app/policies/account_policy.rb b/app/policies/account_policy.rb index 679119075..262ada42e 100644 --- a/app/policies/account_policy.rb +++ b/app/policies/account_policy.rb @@ -18,11 +18,11 @@ class AccountPolicy < ApplicationPolicy end def destroy? - record.suspended? && record.deletion_request.present? && admin? + record.suspended_temporarily? && admin? end def unsuspend? - staff? + staff? && record.suspension_origin_local? end def sensitive? diff --git a/app/serializers/activitypub/actor_serializer.rb b/app/serializers/activitypub/actor_serializer.rb index 5d2741b17..759ef30f9 100644 --- a/app/serializers/activitypub/actor_serializer.rb +++ b/app/serializers/activitypub/actor_serializer.rb @@ -7,7 +7,7 @@ class ActivityPub::ActorSerializer < ActivityPub::Serializer context_extensions :manually_approves_followers, :featured, :also_known_as, :moved_to, :property_value, :identity_proof, - :discoverable, :olm + :discoverable, :olm, :suspended attributes :id, :type, :following, :followers, :inbox, :outbox, :featured, :featured_tags, @@ -23,6 +23,7 @@ class ActivityPub::ActorSerializer < ActivityPub::Serializer attribute :devices, unless: :instance_actor? attribute :moved_to, if: :moved? attribute :also_known_as, if: :also_known_as? + attribute :suspended, if: :suspended? class EndpointsSerializer < ActivityPub::Serializer include RoutingHelper @@ -39,7 +40,7 @@ class ActivityPub::ActorSerializer < ActivityPub::Serializer has_one :icon, serializer: ActivityPub::ImageSerializer, if: :avatar_exists? has_one :image, serializer: ActivityPub::ImageSerializer, if: :header_exists? - delegate :moved?, :instance_actor?, to: :object + delegate :suspended?, :instance_actor?, to: :object def id object.instance_actor? ? instance_actor_url : account_url(object) @@ -93,12 +94,16 @@ class ActivityPub::ActorSerializer < ActivityPub::Serializer object.username end + def discoverable + object.suspended? ? false : (object.discoverable || false) + end + def name - object.display_name + object.suspended? ? '' : object.display_name end def summary - Formatter.instance.simplified_format(object) + object.suspended? ? '' : Formatter.instance.simplified_format(object) end def icon @@ -113,36 +118,44 @@ class ActivityPub::ActorSerializer < ActivityPub::Serializer object end + def suspended + object.suspended? + end + def url object.instance_actor? ? about_more_url(instance_actor: true) : short_account_url(object) end def avatar_exists? - object.avatar? + !object.suspended? && object.avatar? end def header_exists? - object.header? + !object.suspended? && object.header? end def manually_approves_followers - object.locked + object.suspended? ? false : object.locked end def virtual_tags - object.emojis + object.tags + object.suspended? ? [] : (object.emojis + object.tags) end def virtual_attachments - object.fields + object.identity_proofs.active + object.suspended? ? [] : (object.fields + object.identity_proofs.active) end def moved_to ActivityPub::TagManager.instance.uri_for(object.moved_to_account) end + def moved? + !object.suspended? && object.moved? + end + def also_known_as? - !object.also_known_as.empty? + !object.suspended? && !object.also_known_as.empty? end class CustomEmojiSerializer < ActivityPub::EmojiSerializer diff --git a/app/services/activitypub/process_account_service.rb b/app/services/activitypub/process_account_service.rb index 9f95f1950..4cb8e09db 100644 --- a/app/services/activitypub/process_account_service.rb +++ b/app/services/activitypub/process_account_service.rb @@ -18,10 +18,11 @@ class ActivityPub::ProcessAccountService < BaseService RedisLock.acquire(lock_options) do |lock| if lock.acquired? - @account = Account.remote.find_by(uri: @uri) if @options[:only_key] - @account ||= Account.find_remote(@username, @domain) - @old_public_key = @account&.public_key - @old_protocol = @account&.protocol + @account = Account.remote.find_by(uri: @uri) if @options[:only_key] + @account ||= Account.find_remote(@username, @domain) + @old_public_key = @account&.public_key + @old_protocol = @account&.protocol + @suspension_changed = false create_account if @account.nil? update_account @@ -37,8 +38,9 @@ class ActivityPub::ProcessAccountService < BaseService after_protocol_change! if protocol_changed? after_key_change! if key_changed? && !@options[:signed_with_known_key] clear_tombstones! if key_changed? + after_suspension_change! if suspension_changed? - unless @options[:only_key] + unless @options[:only_key] || @account.suspended? check_featured_collection! if @account.featured_collection_url.present? check_links! unless @account.fields.empty? end @@ -52,20 +54,23 @@ class ActivityPub::ProcessAccountService < BaseService def create_account @account = Account.new - @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? + @account.protocol = :activitypub + @account.username = @username + @account.domain = @domain + @account.private_key = nil + @account.suspended_at = domain_block.created_at if auto_suspend? + @account.suspension_origin = :local if auto_suspend? + @account.silenced_at = domain_block.created_at if auto_silence? + @account.save end def update_account @account.last_webfingered_at = Time.now.utc unless @options[:only_key] @account.protocol = :activitypub - set_immediate_attributes! - set_fetchable_attributes! unless @options[:only_keys] + set_suspension! + set_immediate_attributes! unless @account.suspended? + set_fetchable_attributes! unless @options[:only_keys] || @account.suspended? @account.save_with_optional_media! end @@ -99,6 +104,18 @@ class ActivityPub::ProcessAccountService < BaseService @account.moved_to_account = @json['movedTo'].present? ? moved_account : nil end + def set_suspension! + return if @account.suspended? && @account.suspension_origin_local? + + if @account.suspended? && !@json['suspended'] + @account.unsuspend! + @suspension_changed = true + elsif !@account.suspended? && @json['suspended'] + @account.suspend!(origin: :remote) + @suspension_changed = true + end + end + def after_protocol_change! ActivityPub::PostUpgradeWorker.perform_async(@account.domain) end @@ -107,6 +124,14 @@ class ActivityPub::ProcessAccountService < BaseService RefollowWorker.perform_async(@account.id) end + def after_suspension_change! + if @account.suspended? + Admin::SuspensionWorker.perform_async(@account.id) + else + Admin::UnsuspensionWorker.perform_async(@account.id) + end + end + def check_featured_collection! ActivityPub::SynchronizeFeaturedCollectionWorker.perform_async(@account.id) end @@ -227,6 +252,10 @@ class ActivityPub::ProcessAccountService < BaseService !@old_public_key.nil? && @old_public_key != @account.public_key end + def suspension_changed? + @suspension_changed + end + def clear_tombstones! Tombstone.where(account_id: @account.id).delete_all end diff --git a/app/services/activitypub/process_collection_service.rb b/app/services/activitypub/process_collection_service.rb index e6ccaccc9..f1d175dac 100644 --- a/app/services/activitypub/process_collection_service.rb +++ b/app/services/activitypub/process_collection_service.rb @@ -8,7 +8,7 @@ class ActivityPub::ProcessCollectionService < BaseService @json = Oj.load(body, mode: :strict) @options = options - return if !supported_context? || (different_actor? && verify_account!.nil?) || @account.suspended? || @account.local? + return if !supported_context? || (different_actor? && verify_account!.nil?) || suspended_actor? || @account.local? case @json['type'] when 'Collection', 'CollectionPage' @@ -28,6 +28,14 @@ class ActivityPub::ProcessCollectionService < BaseService @json['actor'].present? && value_or_id(@json['actor']) != @account.uri end + def suspended_actor? + @account.suspended? && !activity_allowed_while_suspended? + end + + def activity_allowed_while_suspended? + %w(Delete Reject Undo Update).include?(@json['type']) + end + def process_items(items) items.reverse_each.map { |item| process_item(item) }.compact end diff --git a/app/services/block_domain_service.rb b/app/services/block_domain_service.rb index 1cf3382b3..76cc36ff6 100644 --- a/app/services/block_domain_service.rb +++ b/app/services/block_domain_service.rb @@ -16,7 +16,7 @@ class BlockDomainService < BaseService scope = Account.by_domain_and_subdomains(domain_block.domain) scope.where(silenced_at: domain_block.created_at).in_batches.update_all(silenced_at: nil) unless domain_block.silence? - scope.where(suspended_at: domain_block.created_at).in_batches.update_all(suspended_at: nil) unless domain_block.suspend? + scope.where(suspended_at: domain_block.created_at).in_batches.update_all(suspended_at: nil, suspension_origin: nil) unless domain_block.suspend? end def process_domain_block! @@ -34,7 +34,8 @@ class BlockDomainService < BaseService end def suspend_accounts! - blocked_domain_accounts.without_suspended.in_batches.update_all(suspended_at: @domain_block.created_at) + blocked_domain_accounts.without_suspended.in_batches.update_all(suspended_at: @domain_block.created_at, suspension_origin: :local) + blocked_domain_accounts.where(suspended_at: @domain_block.created_at).reorder(nil).find_each do |account| DeleteAccountService.new.call(account, reserve_username: true, suspended_at: @domain_block.created_at) end diff --git a/app/services/delete_account_service.rb b/app/services/delete_account_service.rb index 15bdd13e3..de6488c78 100644 --- a/app/services/delete_account_service.rb +++ b/app/services/delete_account_service.rb @@ -64,8 +64,15 @@ class DeleteAccountService < BaseService def reject_follows! return if @account.local? || !@account.activitypub? + # When deleting a remote account, the account obviously doesn't + # actually become deleted on its origin server, i.e. unlike a + # locally deleted account it continues to have access to its home + # feed and other content. To prevent it from being able to continue + # to access toots it would receive because it follows local accounts, + # we have to force it to unfollow them. + ActivityPub::DeliveryWorker.push_bulk(Follow.where(account: @account)) do |follow| - [build_reject_json(follow), follow.target_account_id, follow.account.inbox_url] + [Oj.dump(serialize_payload(follow, ActivityPub::RejectFollowSerializer)), follow.target_account_id, @account.inbox_url] end end @@ -114,19 +121,20 @@ class DeleteAccountService < BaseService return unless @options[:reserve_username] - @account.silenced_at = nil - @account.suspended_at = @options[:suspended_at] || Time.now.utc - @account.locked = false - @account.memorial = false - @account.discoverable = false - @account.display_name = '' - @account.note = '' - @account.fields = [] - @account.statuses_count = 0 - @account.followers_count = 0 - @account.following_count = 0 - @account.moved_to_account = nil - @account.trust_level = :untrusted + @account.silenced_at = nil + @account.suspended_at = @options[:suspended_at] || Time.now.utc + @account.suspension_origin = :local + @account.locked = false + @account.memorial = false + @account.discoverable = false + @account.display_name = '' + @account.note = '' + @account.fields = [] + @account.statuses_count = 0 + @account.followers_count = 0 + @account.following_count = 0 + @account.moved_to_account = nil + @account.trust_level = :untrusted @account.avatar.destroy @account.header.destroy @account.save! @@ -154,10 +162,6 @@ class DeleteAccountService < BaseService @delete_actor_json ||= Oj.dump(serialize_payload(@account, ActivityPub::DeleteActorSerializer, signer: @account)) end - def build_reject_json(follow) - Oj.dump(serialize_payload(follow, ActivityPub::RejectFollowSerializer)) - end - def delivery_inboxes @delivery_inboxes ||= @account.followers.inboxes + Relay.enabled.pluck(:inbox_url) end diff --git a/app/services/resolve_account_service.rb b/app/services/resolve_account_service.rb index 3f7bb7cc5..4783e6d33 100644 --- a/app/services/resolve_account_service.rb +++ b/app/services/resolve_account_service.rb @@ -5,8 +5,6 @@ class ResolveAccountService < BaseService include DomainControlHelper include WebfingerHelper - class WebfingerRedirectError < StandardError; end - # Find or create an account record for a remote user. When creating, # look up the user's webfinger and fetch ActivityPub data # @param [String, Account] uri URI in the username@domain format or account record @@ -40,13 +38,18 @@ class ResolveAccountService < BaseService @account ||= Account.find_remote(@username, @domain) - return @account if @account&.local? || !webfinger_update_due? + if gone_from_origin? && not_yet_deleted? + queue_deletion! + return + end + + return @account if @account&.local? || gone_from_origin? || !webfinger_update_due? # Now it is certain, it is definitely a remote account, and it # either needs to be created, or updated from fresh data process_account! - rescue Webfinger::Error, WebfingerRedirectError, Oj::ParseError => e + rescue Webfinger::Error, Oj::ParseError => e Rails.logger.debug "Webfinger query for #{@uri} failed: #{e}" nil end @@ -86,10 +89,12 @@ class ResolveAccountService < BaseService elsif !redirected return process_webfinger!("#{confirmed_username}@#{confirmed_domain}", true) else - raise WebfingerRedirectError, "The URI #{uri} tries to hijack #{@username}@#{@domain}" + raise Webfinger::RedirectError, "The URI #{uri} tries to hijack #{@username}@#{@domain}" end @domain = nil if TagManager.instance.local_domain?(@domain) + rescue Webfinger::GoneError + @gone = true end def process_account! @@ -131,6 +136,18 @@ class ResolveAccountService < BaseService @actor_json = supported_context?(json) && equals_or_includes_any?(json['type'], ActivityPub::FetchRemoteAccountService::SUPPORTED_TYPES) ? json : nil end + def gone_from_origin? + @gone + end + + def not_yet_deleted? + @account.present? && !@account.local? + end + + def queue_deletion! + AccountDeletionWorker.perform_async(@account.id, reserve_username: false) + end + def lock_options { redis: Redis.current, key: "resolve:#{@username}@#{@domain}" } end diff --git a/app/services/suspend_account_service.rb b/app/services/suspend_account_service.rb index f08c41e17..d7f29963c 100644 --- a/app/services/suspend_account_service.rb +++ b/app/services/suspend_account_service.rb @@ -1,10 +1,14 @@ # frozen_string_literal: true class SuspendAccountService < BaseService + include Payloadable + def call(account) @account = account suspend! + reject_remote_follows! + distribute_update_actor! unmerge_from_home_timelines! unmerge_from_list_timelines! privatize_media_attachments! @@ -16,6 +20,31 @@ class SuspendAccountService < BaseService @account.suspend! unless @account.suspended? end + def reject_remote_follows! + return if @account.local? || !@account.activitypub? + + # When suspending a remote account, the account obviously doesn't + # actually become suspended on its origin server, i.e. unlike a + # locally suspended account it continues to have access to its home + # feed and other content. To prevent it from being able to continue + # to access toots it would receive because it follows local accounts, + # we have to force it to unfollow them. Unfortunately, there is no + # counterpart to this operation, i.e. you can't then force a remote + # account to re-follow you, so this part is not reversible. + + follows = Follow.where(account: @account).to_a + + ActivityPub::DeliveryWorker.push_bulk(follows) do |follow| + [Oj.dump(serialize_payload(follow, ActivityPub::RejectFollowSerializer)), follow.target_account_id, @account.inbox_url] + end + + follows.in_batches.destroy_all + end + + def distribute_update_actor! + ActivityPub::UpdateDistributionWorker.perform_async(@account.id) if @account.local? + end + def unmerge_from_home_timelines! @account.followers_for_local_distribution.find_each do |follower| FeedManager.instance.unmerge_from_home(@account, follower) diff --git a/app/services/unblock_domain_service.rb b/app/services/unblock_domain_service.rb index d502d9e49..e765fb7a8 100644 --- a/app/services/unblock_domain_service.rb +++ b/app/services/unblock_domain_service.rb @@ -13,6 +13,6 @@ class UnblockDomainService < BaseService scope = Account.by_domain_and_subdomains(domain_block.domain) scope.where(silenced_at: domain_block.created_at).in_batches.update_all(silenced_at: nil) unless domain_block.noop? - scope.where(suspended_at: domain_block.created_at).in_batches.update_all(suspended_at: nil) if domain_block.suspend? + scope.where(suspended_at: domain_block.created_at).in_batches.update_all(suspended_at: nil, suspension_origin: nil) if domain_block.suspend? end end diff --git a/app/services/unsuspend_account_service.rb b/app/services/unsuspend_account_service.rb index 91dbc9c18..a81d1ac4f 100644 --- a/app/services/unsuspend_account_service.rb +++ b/app/services/unsuspend_account_service.rb @@ -5,6 +5,10 @@ class UnsuspendAccountService < BaseService @account = account unsuspend! + refresh_remote_account! + + return if @account.nil? + merge_into_home_timelines! merge_into_list_timelines! publish_media_attachments! @@ -16,6 +20,22 @@ class UnsuspendAccountService < BaseService @account.unsuspend! if @account.suspended? end + def refresh_remote_account! + return if @account.local? + + # While we had the remote account suspended, it could be that + # it got suspended on its origin, too. So, we need to refresh + # it straight away so it gets marked as remotely suspended in + # that case. + + @account.update!(last_webfingered_at: nil) + @account = ResolveAccountService.new.call(@account) + + # Worth noting that it is possible that the remote has not only + # been suspended, but deleted permanently, in which case + # @account would now be nil. + end + def merge_into_home_timelines! @account.followers_for_local_distribution.find_each do |follower| FeedManager.instance.merge_into_home(@account, follower) diff --git a/app/workers/account_deletion_worker.rb b/app/workers/account_deletion_worker.rb index 0f6be71e1..b6016bf8c 100644 --- a/app/workers/account_deletion_worker.rb +++ b/app/workers/account_deletion_worker.rb @@ -5,8 +5,8 @@ class AccountDeletionWorker sidekiq_options queue: 'pull' - def perform(account_id) - DeleteAccountService.new.call(Account.find(account_id), reserve_username: true, reserve_email: false) + def perform(account_id, reserve_username: true) + DeleteAccountService.new.call(Account.find(account_id), reserve_username: reserve_username, reserve_email: false) rescue ActiveRecord::RecordNotFound true end diff --git a/db/migrate/20201017233919_add_suspension_origin_to_accounts.rb b/db/migrate/20201017233919_add_suspension_origin_to_accounts.rb new file mode 100644 index 000000000..f0db02143 --- /dev/null +++ b/db/migrate/20201017233919_add_suspension_origin_to_accounts.rb @@ -0,0 +1,5 @@ +class AddSuspensionOriginToAccounts < ActiveRecord::Migration[5.2] + def change + add_column :accounts, :suspension_origin, :integer + end +end diff --git a/db/post_migrate/20201017234926_fill_account_suspension_origin.rb b/db/post_migrate/20201017234926_fill_account_suspension_origin.rb new file mode 100644 index 000000000..ab7407d79 --- /dev/null +++ b/db/post_migrate/20201017234926_fill_account_suspension_origin.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class FillAccountSuspensionOrigin < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + def up + Account.suspended.where(suspension_origin: nil).in_batches.update_all(suspension_origin: :local) + end + + def down; end +end diff --git a/db/schema.rb b/db/schema.rb index 5ff6b6300..873c37f67 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_10_08_220312) do +ActiveRecord::Schema.define(version: 2020_10_17_234926) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -189,6 +189,7 @@ ActiveRecord::Schema.define(version: 2020_10_08_220312) do t.integer "avatar_storage_schema_version" t.integer "header_storage_schema_version" t.string "devices_url" + t.integer "suspension_origin" t.datetime "sensitized_at" t.index "(((setweight(to_tsvector('simple'::regconfig, (display_name)::text), 'A'::\"char\") || setweight(to_tsvector('simple'::regconfig, (username)::text), 'B'::\"char\")) || setweight(to_tsvector('simple'::regconfig, (COALESCE(domain, ''::character varying))::text), 'C'::\"char\")))", name: "search_index", using: :gin t.index "lower((username)::text), COALESCE(lower((domain)::text), ''::text)", name: "index_accounts_on_username_and_domain_lower", unique: true diff --git a/lib/mastodon/accounts_cli.rb b/lib/mastodon/accounts_cli.rb index 8f9279a3c..7565620cf 100644 --- a/lib/mastodon/accounts_cli.rb +++ b/lib/mastodon/accounts_cli.rb @@ -245,7 +245,7 @@ module Mastodon end if [404, 410].include?(code) - SuspendAccountService.new.call(account, reserve_username: false) unless options[:dry_run] + DeleteAccountService.new.call(account, reserve_username: false) unless options[:dry_run] 1 else # Touch account even during dry run to avoid getting the account into the window again diff --git a/spec/controllers/account_follow_controller_spec.rb b/spec/controllers/account_follow_controller_spec.rb index 9a93e1ebe..d33cd0499 100644 --- a/spec/controllers/account_follow_controller_spec.rb +++ b/spec/controllers/account_follow_controller_spec.rb @@ -16,17 +16,49 @@ describe AccountFollowController do allow(service).to receive(:call) end - it 'does not create for user who is not signed in' do - subject - expect(FollowService).not_to receive(:new) + context 'when account is permanently suspended' do + before do + alice.suspend! + alice.deletion_request.destroy + subject + end + + it 'returns http gone' do + expect(response).to have_http_status(410) + end + end + + context 'when account is temporarily suspended' do + before do + alice.suspend! + subject + end + + it 'returns http forbidden' do + expect(response).to have_http_status(403) + end + end + + context 'when signed out' do + before do + subject + end + + it 'does not follow' do + expect(FollowService).not_to receive(:new) + end end - it 'redirects to account path' do - sign_in(user) - subject + context 'when signed in' do + before do + sign_in(user) + subject + end - expect(service).to have_received(:call).with(user.account, alice, with_rate_limit: true) - expect(response).to redirect_to(account_path(alice)) + it 'redirects to account path' do + expect(service).to have_received(:call).with(user.account, alice, with_rate_limit: true) + expect(response).to redirect_to(account_path(alice)) + end end end end diff --git a/spec/controllers/account_unfollow_controller_spec.rb b/spec/controllers/account_unfollow_controller_spec.rb index bdebcfa94..a11f7aa68 100644 --- a/spec/controllers/account_unfollow_controller_spec.rb +++ b/spec/controllers/account_unfollow_controller_spec.rb @@ -16,17 +16,49 @@ describe AccountUnfollowController do allow(service).to receive(:call) end - it 'does not create for user who is not signed in' do - subject - expect(UnfollowService).not_to receive(:new) + context 'when account is permanently suspended' do + before do + alice.suspend! + alice.deletion_request.destroy + subject + end + + it 'returns http gone' do + expect(response).to have_http_status(410) + end + end + + context 'when account is temporarily suspended' do + before do + alice.suspend! + subject + end + + it 'returns http forbidden' do + expect(response).to have_http_status(403) + end + end + + context 'when signed out' do + before do + subject + end + + it 'does not unfollow' do + expect(UnfollowService).not_to receive(:new) + end end - it 'redirects to account path' do - sign_in(user) - subject + context 'when signed in' do + before do + sign_in(user) + subject + end - expect(service).to have_received(:call).with(user.account, alice) - expect(response).to redirect_to(account_path(alice)) + it 'redirects to account path' do + expect(service).to have_received(:call).with(user.account, alice) + expect(response).to redirect_to(account_path(alice)) + end end end end diff --git a/spec/controllers/accounts_controller_spec.rb b/spec/controllers/accounts_controller_spec.rb index b04f4650b..f7d0b1af5 100644 --- a/spec/controllers/accounts_controller_spec.rb +++ b/spec/controllers/accounts_controller_spec.rb @@ -48,10 +48,17 @@ RSpec.describe AccountsController, type: :controller do expect(response).to have_http_status(404) end end + end + + context 'as HTML' do + let(:format) { 'html' } - context 'when account is suspended' do + it_behaves_like 'preliminary checks' + + context 'when account is permanently suspended' do before do account.suspend! + account.deletion_request.destroy end it 'returns http gone' do @@ -59,12 +66,17 @@ RSpec.describe AccountsController, type: :controller do expect(response).to have_http_status(410) end end - end - context 'as HTML' do - let(:format) { 'html' } + context 'when account is temporarily suspended' do + before do + account.suspend! + end - it_behaves_like 'preliminary checks' + it 'returns http forbidden' do + get :show, params: { username: account.username, format: format } + expect(response).to have_http_status(403) + end + end shared_examples 'common response characteristics' do it 'returns http success' do @@ -325,6 +337,29 @@ RSpec.describe AccountsController, type: :controller do it_behaves_like 'preliminary checks' + context 'when account is suspended permanently' do + before do + account.suspend! + account.deletion_request.destroy + end + + it 'returns http gone' do + get :show, params: { username: account.username, format: format } + expect(response).to have_http_status(410) + end + end + + context 'when account is suspended temporarily' do + before do + account.suspend! + end + + it 'returns http success' do + get :show, params: { username: account.username, format: format } + expect(response).to have_http_status(200) + end + end + context do before do get :show, params: { username: account.username, format: format } @@ -435,6 +470,29 @@ RSpec.describe AccountsController, type: :controller do it_behaves_like 'preliminary checks' + context 'when account is permanently suspended' do + before do + account.suspend! + account.deletion_request.destroy + end + + it 'returns http gone' do + get :show, params: { username: account.username, format: format } + expect(response).to have_http_status(410) + end + end + + context 'when account is temporarily suspended' do + before do + account.suspend! + end + + it 'returns http forbidden' do + get :show, params: { username: account.username, format: format } + expect(response).to have_http_status(403) + end + end + shared_examples 'common response characteristics' do it 'returns http success' do expect(response).to have_http_status(200) diff --git a/spec/controllers/activitypub/collections_controller_spec.rb b/spec/controllers/activitypub/collections_controller_spec.rb index 89939d1d2..ac661e5e1 100644 --- a/spec/controllers/activitypub/collections_controller_spec.rb +++ b/spec/controllers/activitypub/collections_controller_spec.rb @@ -13,6 +13,7 @@ RSpec.describe ActivityPub::CollectionsController, type: :controller do end it 'does not set sessions' do + response expect(session).to be_empty end @@ -34,9 +35,8 @@ RSpec.describe ActivityPub::CollectionsController, type: :controller do context 'without signature' do let(:remote_account) { nil } - before do - get :show, params: { id: 'featured', account_username: account.username } - end + subject(:response) { get :show, params: { id: 'featured', account_username: account.username } } + subject(:body) { body_as_json } it 'returns http success' do expect(response).to have_http_status(200) @@ -49,9 +49,29 @@ RSpec.describe ActivityPub::CollectionsController, type: :controller do it_behaves_like 'cachable response' it 'returns orderedItems with pinned statuses' do - json = body_as_json - expect(json[:orderedItems]).to be_an Array - expect(json[:orderedItems].size).to eq 2 + expect(body[:orderedItems]).to be_an Array + expect(body[:orderedItems].size).to eq 2 + end + + context 'when account is permanently suspended' do + before do + account.suspend! + account.deletion_request.destroy + end + + it 'returns http gone' do + expect(response).to have_http_status(410) + end + end + + context 'when account is temporarily suspended' do + before do + account.suspend! + end + + it 'returns http forbidden' do + expect(response).to have_http_status(403) + end end end diff --git a/spec/controllers/activitypub/followers_synchronizations_controller_spec.rb b/spec/controllers/activitypub/followers_synchronizations_controller_spec.rb index a24d3f8e0..88f4554c2 100644 --- a/spec/controllers/activitypub/followers_synchronizations_controller_spec.rb +++ b/spec/controllers/activitypub/followers_synchronizations_controller_spec.rb @@ -32,9 +32,8 @@ RSpec.describe ActivityPub::FollowersSynchronizationsController, type: :controll context 'with signature from example.com' do let(:remote_account) { Fabricate(:account, domain: 'example.com', uri: 'https://example.com/instance') } - before do - get :show, params: { account_username: account.username } - end + subject(:response) { get :show, params: { account_username: account.username } } + subject(:body) { body_as_json } it 'returns http success' do expect(response).to have_http_status(200) @@ -45,14 +44,34 @@ RSpec.describe ActivityPub::FollowersSynchronizationsController, type: :controll end it 'returns orderedItems with followers from example.com' do - json = body_as_json - expect(json[:orderedItems]).to be_an Array - expect(json[:orderedItems].sort).to eq [follower_1.uri, follower_2.uri] + expect(body[:orderedItems]).to be_an Array + expect(body[:orderedItems].sort).to eq [follower_1.uri, follower_2.uri] end it 'returns private Cache-Control header' do expect(response.headers['Cache-Control']).to eq 'max-age=0, private' end + + context 'when account is permanently suspended' do + before do + account.suspend! + account.deletion_request.destroy + end + + it 'returns http gone' do + expect(response).to have_http_status(410) + end + end + + context 'when account is temporarily suspended' do + before do + account.suspend! + end + + it 'returns http forbidden' do + expect(response).to have_http_status(403) + end + end end end end diff --git a/spec/controllers/activitypub/inboxes_controller_spec.rb b/spec/controllers/activitypub/inboxes_controller_spec.rb index e5c004611..973ad83bb 100644 --- a/spec/controllers/activitypub/inboxes_controller_spec.rb +++ b/spec/controllers/activitypub/inboxes_controller_spec.rb @@ -20,6 +20,33 @@ RSpec.describe ActivityPub::InboxesController, type: :controller do it 'returns http accepted' do expect(response).to have_http_status(202) end + + context 'for a specific account' do + let(:account) { Fabricate(:account) } + + subject(:response) { post :create, params: { account_username: account.username }, body: '{}' } + + context 'when account is permanently suspended' do + before do + account.suspend! + account.deletion_request.destroy + end + + it 'returns http gone' do + expect(response).to have_http_status(410) + end + end + + context 'when account is temporarily suspended' do + before do + account.suspend! + end + + it 'returns http accepted' do + expect(response).to have_http_status(202) + end + end + end end context 'with Collection-Synchronization header' do diff --git a/spec/controllers/activitypub/outboxes_controller_spec.rb b/spec/controllers/activitypub/outboxes_controller_spec.rb index 1baf5a623..84e3a8956 100644 --- a/spec/controllers/activitypub/outboxes_controller_spec.rb +++ b/spec/controllers/activitypub/outboxes_controller_spec.rb @@ -10,6 +10,7 @@ RSpec.describe ActivityPub::OutboxesController, type: :controller do end it 'does not set sessions' do + response expect(session).to be_empty end @@ -34,9 +35,8 @@ RSpec.describe ActivityPub::OutboxesController, type: :controller do context 'without signature' do let(:remote_account) { nil } - before do - get :show, params: { account_username: account.username, page: page } - end + subject(:response) { get :show, params: { account_username: account.username, page: page } } + subject(:body) { body_as_json } context 'with page not requested' do let(:page) { nil } @@ -50,11 +50,31 @@ RSpec.describe ActivityPub::OutboxesController, type: :controller do end it 'returns totalItems' do - json = body_as_json - expect(json[:totalItems]).to eq 4 + expect(body[:totalItems]).to eq 4 end it_behaves_like 'cachable response' + + context 'when account is permanently suspended' do + before do + account.suspend! + account.deletion_request.destroy + end + + it 'returns http gone' do + expect(response).to have_http_status(410) + end + end + + context 'when account is temporarily suspended' do + before do + account.suspend! + end + + it 'returns http forbidden' do + expect(response).to have_http_status(403) + end + end end context 'with page requested' do @@ -69,13 +89,33 @@ RSpec.describe ActivityPub::OutboxesController, type: :controller do end it 'returns orderedItems with public or unlisted statuses' do - json = body_as_json - expect(json[:orderedItems]).to be_an Array - expect(json[:orderedItems].size).to eq 2 - expect(json[:orderedItems].all? { |item| item[:to].include?(ActivityPub::TagManager::COLLECTIONS[:public]) || item[:cc].include?(ActivityPub::TagManager::COLLECTIONS[:public]) }).to be true + expect(body[:orderedItems]).to be_an Array + expect(body[:orderedItems].size).to eq 2 + expect(body[:orderedItems].all? { |item| item[:to].include?(ActivityPub::TagManager::COLLECTIONS[:public]) || item[:cc].include?(ActivityPub::TagManager::COLLECTIONS[:public]) }).to be true end it_behaves_like 'cachable response' + + context 'when account is permanently suspended' do + before do + account.suspend! + account.deletion_request.destroy + end + + it 'returns http gone' do + expect(response).to have_http_status(410) + end + end + + context 'when account is temporarily suspended' do + before do + account.suspend! + end + + it 'returns http forbidden' do + expect(response).to have_http_status(403) + end + end end end diff --git a/spec/controllers/activitypub/replies_controller_spec.rb b/spec/controllers/activitypub/replies_controller_spec.rb index ed383864d..250259752 100644 --- a/spec/controllers/activitypub/replies_controller_spec.rb +++ b/spec/controllers/activitypub/replies_controller_spec.rb @@ -14,6 +14,7 @@ RSpec.describe ActivityPub::RepliesController, type: :controller do end it 'does not set sessions' do + response expect(session).to be_empty end @@ -36,8 +37,32 @@ RSpec.describe ActivityPub::RepliesController, type: :controller do describe 'GET #index' do context 'with no signature' do - before do - get :index, params: { account_username: status.account.username, status_id: status.id } + subject(:response) { get :index, params: { account_username: status.account.username, status_id: status.id } } + subject(:body) { body_as_json } + + context 'when account is permanently suspended' do + let(:parent_visibility) { :public } + + before do + status.account.suspend! + status.account.deletion_request.destroy + end + + it 'returns http gone' do + expect(response).to have_http_status(410) + end + end + + context 'when account is temporarily suspended' do + let(:parent_visibility) { :public } + + before do + status.account.suspend! + end + + it 'returns http forbidden' do + expect(response).to have_http_status(403) + end end context 'when status is public' do @@ -54,12 +79,10 @@ RSpec.describe ActivityPub::RepliesController, type: :controller do it_behaves_like 'cachable response' it 'returns items with account\'s own replies' do - json = body_as_json - - expect(json[:first]).to be_a Hash - expect(json[:first][:items]).to be_an Array - expect(json[:first][:items].size).to eq 1 - expect(json[:first][:items].all? { |item| item[:to].include?(ActivityPub::TagManager::COLLECTIONS[:public]) || item[:cc].include?(ActivityPub::TagManager::COLLECTIONS[:public]) }).to be true + expect(body[:first]).to be_a Hash + expect(body[:first][:items]).to be_an Array + expect(body[:first][:items].size).to eq 1 + expect(body[:first][:items].all? { |item| item[:to].include?(ActivityPub::TagManager::COLLECTIONS[:public]) || item[:cc].include?(ActivityPub::TagManager::COLLECTIONS[:public]) }).to be true end end diff --git a/spec/controllers/api/v1/admin/accounts_controller_spec.rb b/spec/controllers/api/v1/admin/accounts_controller_spec.rb index 89cadb222..f6be35f7f 100644 --- a/spec/controllers/api/v1/admin/accounts_controller_spec.rb +++ b/spec/controllers/api/v1/admin/accounts_controller_spec.rb @@ -111,7 +111,7 @@ RSpec.describe Api::V1::Admin::AccountsController, type: :controller do describe 'POST #unsuspend' do before do - account.touch(:suspended_at) + account.suspend! post :unsuspend, params: { id: account.id } end diff --git a/spec/controllers/follower_accounts_controller_spec.rb b/spec/controllers/follower_accounts_controller_spec.rb index 34a0cf3f4..f6d55f693 100644 --- a/spec/controllers/follower_accounts_controller_spec.rb +++ b/spec/controllers/follower_accounts_controller_spec.rb @@ -14,6 +14,27 @@ describe FollowerAccountsController do context 'when format is html' do subject(:response) { get :index, params: { account_username: alice.username, format: :html } } + context 'when account is permanently suspended' do + before do + alice.suspend! + alice.deletion_request.destroy + end + + it 'returns http gone' do + expect(response).to have_http_status(410) + end + end + + context 'when account is temporarily suspended' do + before do + alice.suspend! + end + + it 'returns http forbidden' do + expect(response).to have_http_status(403) + end + end + it 'assigns follows' do expect(response).to have_http_status(200) @@ -48,6 +69,27 @@ describe FollowerAccountsController do expect(body['totalItems']).to eq 2 expect(body['partOf']).to be_present end + + context 'when account is permanently suspended' do + before do + alice.suspend! + alice.deletion_request.destroy + end + + it 'returns http gone' do + expect(response).to have_http_status(410) + end + end + + context 'when account is temporarily suspended' do + before do + alice.suspend! + end + + it 'returns http forbidden' do + expect(response).to have_http_status(403) + end + end end context 'without page' do @@ -58,6 +100,27 @@ describe FollowerAccountsController do expect(body['totalItems']).to eq 2 expect(body['partOf']).to be_blank end + + context 'when account is permanently suspended' do + before do + alice.suspend! + alice.deletion_request.destroy + end + + it 'returns http gone' do + expect(response).to have_http_status(410) + end + end + + context 'when account is temporarily suspended' do + before do + alice.suspend! + end + + it 'returns http forbidden' do + expect(response).to have_http_status(403) + end + end end end end diff --git a/spec/controllers/following_accounts_controller_spec.rb b/spec/controllers/following_accounts_controller_spec.rb index e9a1f597d..0fc0967a6 100644 --- a/spec/controllers/following_accounts_controller_spec.rb +++ b/spec/controllers/following_accounts_controller_spec.rb @@ -14,6 +14,27 @@ describe FollowingAccountsController do context 'when format is html' do subject(:response) { get :index, params: { account_username: alice.username, format: :html } } + context 'when account is permanently suspended' do + before do + alice.suspend! + alice.deletion_request.destroy + end + + it 'returns http gone' do + expect(response).to have_http_status(410) + end + end + + context 'when account is temporarily suspended' do + before do + alice.suspend! + end + + it 'returns http forbidden' do + expect(response).to have_http_status(403) + end + end + it 'assigns follows' do expect(response).to have_http_status(200) @@ -48,6 +69,27 @@ describe FollowingAccountsController do expect(body['totalItems']).to eq 2 expect(body['partOf']).to be_present end + + context 'when account is permanently suspended' do + before do + alice.suspend! + alice.deletion_request.destroy + end + + it 'returns http gone' do + expect(response).to have_http_status(410) + end + end + + context 'when account is temporarily suspended' do + before do + alice.suspend! + end + + it 'returns http forbidden' do + expect(response).to have_http_status(403) + end + end end context 'without page' do @@ -58,6 +100,27 @@ describe FollowingAccountsController do expect(body['totalItems']).to eq 2 expect(body['partOf']).to be_blank end + + context 'when account is permanently suspended' do + before do + alice.suspend! + alice.deletion_request.destroy + end + + it 'returns http gone' do + expect(response).to have_http_status(410) + end + end + + context 'when account is temporarily suspended' do + before do + alice.suspend! + end + + it 'returns http forbidden' do + expect(response).to have_http_status(403) + end + end end end end diff --git a/spec/controllers/remote_follow_controller_spec.rb b/spec/controllers/remote_follow_controller_spec.rb index 7312dde58..01d43f48c 100644 --- a/spec/controllers/remote_follow_controller_spec.rb +++ b/spec/controllers/remote_follow_controller_spec.rb @@ -94,21 +94,42 @@ describe RemoteFollowController do end end - describe 'with a suspended account' do + context 'with a permanently suspended account' do before do - @account = Fabricate(:account, suspended: true) + @account = Fabricate(:account) + @account.suspend! + @account.deletion_request.destroy end - it 'returns 410 gone on GET to #new' do + it 'returns http gone on GET to #new' do get :new, params: { account_username: @account.to_param } - expect(response).to have_http_status(:gone) + expect(response).to have_http_status(410) end - it 'returns 410 gone on POST to #create' do + it 'returns http gone on POST to #create' do post :create, params: { account_username: @account.to_param } - expect(response).to have_http_status(:gone) + expect(response).to have_http_status(410) + end + end + + context 'with a temporarily suspended account' do + before do + @account = Fabricate(:account) + @account.suspend! + end + + it 'returns http forbidden on GET to #new' do + get :new, params: { account_username: @account.to_param } + + expect(response).to have_http_status(403) + end + + it 'returns http forbidden on POST to #create' do + post :create, params: { account_username: @account.to_param } + + expect(response).to have_http_status(403) end end end diff --git a/spec/controllers/statuses_controller_spec.rb b/spec/controllers/statuses_controller_spec.rb index cd6e1e607..9986efa51 100644 --- a/spec/controllers/statuses_controller_spec.rb +++ b/spec/controllers/statuses_controller_spec.rb @@ -24,10 +24,11 @@ describe StatusesController do let(:account) { Fabricate(:account) } let(:status) { Fabricate(:status, account: account) } - context 'when account is suspended' do - let(:account) { Fabricate(:account, suspended: true) } - + context 'when account is permanently suspended' do before do + account.suspend! + account.deletion_request.destroy + get :show, params: { account_username: account.username, id: status.id } end @@ -36,6 +37,18 @@ describe StatusesController do end end + context 'when account is temporarily suspended' do + before do + account.suspend! + + get :show, params: { account_username: account.username, id: status.id } + end + + it 'returns http forbidden' do + expect(response).to have_http_status(403) + end + end + context 'when status is a reblog' do let(:original_account) { Fabricate(:account, domain: 'example.com') } let(:original_status) { Fabricate(:status, account: original_account, url: 'https://example.com/123') } @@ -676,10 +689,11 @@ describe StatusesController do let(:account) { Fabricate(:account) } let(:status) { Fabricate(:status, account: account) } - context 'when account is suspended' do - let(:account) { Fabricate(:account, suspended: true) } - + context 'when account is permanently suspended' do before do + account.suspend! + account.deletion_request.destroy + get :activity, params: { account_username: account.username, id: status.id } end @@ -688,6 +702,18 @@ describe StatusesController do end end + context 'when account is temporarily suspended' do + before do + account.suspend! + + get :activity, params: { account_username: account.username, id: status.id } + end + + it 'returns http forbidden' do + expect(response).to have_http_status(403) + end + end + context 'when status is public' do pending end diff --git a/spec/controllers/well_known/webfinger_controller_spec.rb b/spec/controllers/well_known/webfinger_controller_spec.rb index 46f63185b..cf7005b0e 100644 --- a/spec/controllers/well_known/webfinger_controller_spec.rb +++ b/spec/controllers/well_known/webfinger_controller_spec.rb @@ -4,95 +4,134 @@ describe WellKnown::WebfingerController, type: :controller do render_views describe 'GET #show' do - let(:alice) do - Fabricate(:account, username: 'alice') + let(:alternate_domains) { [] } + let(:alice) { Fabricate(:account, username: 'alice') } + let(:resource) { nil } + + around(:each) do |example| + tmp = Rails.configuration.x.alternate_domains + Rails.configuration.x.alternate_domains = alternate_domains + example.run + Rails.configuration.x.alternate_domains = tmp end - before do - alice.private_key = <<-PEM ------BEGIN RSA PRIVATE KEY----- -MIICXQIBAAKBgQDHgPoPJlrfMZrVcuF39UbVssa8r4ObLP3dYl9Y17Mgp5K4mSYD -R/Y2ag58tSi6ar2zM3Ze3QYsNfTq0NqN1g89eAu0MbSjWqpOsgntRPJiFuj3hai2 -X2Im8TBrkiM/UyfTRgn8q8WvMoKbXk8Lu6nqv420eyqhhLxfUoCpxuem1QIDAQAB -AoGBAIKsOh2eM7spVI8mdgQKheEG/iEsnPkQ2R8ehfE9JzjmSbXbqghQJDaz9NU+ -G3Uu4R31QT0VbCudE9SSA/UPFl82GeQG4QLjrSE+PSjSkuslgSXelJHfAJ+ycGax -ajtPyiQD0e4c2loagHNHPjqK9OhHx9mFnZWmoagjlZ+mQGEpAkEA8GtqfS65IaRQ -uVhMzpp25rF1RWOwaaa+vBPkd7pGdJEQGFWkaR/a9UkU+2C4ZxGBkJDP9FApKVQI -RANEwN3/hwJBANRuw5+es6BgBv4PD387IJvuruW2oUtYP+Lb2Z5k77J13hZTr0db -Oo9j1UbbR0/4g+vAcsDl4JD9c/9LrGYEpcMCQBon9Yvs+2M3lziy7JhFoc3zXIjS -Ea1M4M9hcqe78lJYPeIH3z04o/+vlcLLgQRlmSz7NESmO/QtGkEcAezhuh0CQHji -pzO4LeO/gXslut3eGcpiYuiZquOjToecMBRwv+5AIKd367Che4uJdh6iPcyGURvh -IewfZFFdyZqnx20ui90CQQC1W2rK5Y30wAunOtSLVA30TLK/tKrTppMC3corjKlB -FTX8IvYBNTbpEttc1VCf/0ccnNpfb0CrFNSPWxRj7t7D ------END RSA PRIVATE KEY----- -PEM - - alice.public_key = <<-PEM ------BEGIN PUBLIC KEY----- -MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDHgPoPJlrfMZrVcuF39UbVssa8 -r4ObLP3dYl9Y17Mgp5K4mSYDR/Y2ag58tSi6ar2zM3Ze3QYsNfTq0NqN1g89eAu0 -MbSjWqpOsgntRPJiFuj3hai2X2Im8TBrkiM/UyfTRgn8q8WvMoKbXk8Lu6nqv420 -eyqhhLxfUoCpxuem1QIDAQAB ------END PUBLIC KEY----- -PEM - - alice.save! + subject do + get :show, params: { resource: resource }, format: :json end - around(:each) do |example| - before = Rails.configuration.x.alternate_domains - example.run - Rails.configuration.x.alternate_domains = before + shared_examples 'a successful response' do + it 'returns http success' do + expect(response).to have_http_status(200) + end + + it 'returns application/jrd+json' do + expect(response.content_type).to eq 'application/jrd+json' + end + + it 'returns links for the account' do + json = body_as_json + expect(json[:subject]).to eq 'acct:alice@cb6e6126.ngrok.io' + expect(json[:aliases]).to include('https://cb6e6126.ngrok.io/@alice', 'https://cb6e6126.ngrok.io/users/alice') + end end - it 'returns JSON when account can be found' do - get :show, params: { resource: alice.to_webfinger_s }, format: :json + context 'when an account exists' do + let(:resource) { alice.to_webfinger_s } - json = body_as_json + before do + subject + end - expect(response).to have_http_status(200) - expect(response.content_type).to eq 'application/jrd+json' - expect(json[:subject]).to eq 'acct:alice@cb6e6126.ngrok.io' - expect(json[:aliases]).to include('https://cb6e6126.ngrok.io/@alice', 'https://cb6e6126.ngrok.io/users/alice') + it_behaves_like 'a successful response' end - it 'returns http not found when account cannot be found' do - get :show, params: { resource: 'acct:not@existing.com' }, format: :json + context 'when an account is temporarily suspended' do + let(:resource) { alice.to_webfinger_s } - expect(response).to have_http_status(:not_found) + before do + alice.suspend! + subject + end + + it_behaves_like 'a successful response' end - it 'returns JSON when account can be found with alternate domains' do - Rails.configuration.x.alternate_domains = ['foo.org'] - username, = alice.to_webfinger_s.split('@') + context 'when an account is permanently suspended or deleted' do + let(:resource) { alice.to_webfinger_s } + + before do + alice.suspend! + alice.deletion_request.destroy + subject + end - get :show, params: { resource: "#{username}@foo.org" }, format: :json + it 'returns http gone' do + expect(response).to have_http_status(410) + end + end + + context 'when an account is not found' do + let(:resource) { 'acct:not@existing.com' } - json = body_as_json + before do + subject + end - expect(response).to have_http_status(200) - expect(response.content_type).to eq 'application/jrd+json' - expect(json[:subject]).to eq 'acct:alice@cb6e6126.ngrok.io' - expect(json[:aliases]).to include('https://cb6e6126.ngrok.io/@alice', 'https://cb6e6126.ngrok.io/users/alice') + it 'returns http not found' do + expect(response).to have_http_status(404) + end end - it 'returns http not found when account can not be found with alternate domains' do - Rails.configuration.x.alternate_domains = ['foo.org'] - username, = alice.to_webfinger_s.split('@') + context 'with an alternate domain' do + let(:alternate_domains) { ['foo.org'] } + + before do + subject + end + + context 'when an account exists' do + let(:resource) do + username, = alice.to_webfinger_s.split('@') + "#{username}@foo.org" + end + + it_behaves_like 'a successful response' + end - get :show, params: { resource: "#{username}@bar.org" }, format: :json + context 'when the domain is wrong' do + let(:resource) do + username, = alice.to_webfinger_s.split('@') + "#{username}@bar.org" + end - expect(response).to have_http_status(:not_found) + it 'returns http not found' do + expect(response).to have_http_status(404) + end + end end - it 'returns http bad request when not given a resource parameter' do - get :show, params: { }, format: :json - expect(response).to have_http_status(:bad_request) + context 'with no resource parameter' do + let(:resource) { nil } + + before do + subject + end + + it 'returns http bad request' do + expect(response).to have_http_status(400) + end end - it 'returns http bad request when given a nonsense parameter' do - get :show, params: { resource: 'df/:dfkj' } - expect(response).to have_http_status(:bad_request) + context 'with a nonsense parameter' do + let(:resource) { 'df/:dfkj' } + + before do + subject + end + + it 'returns http bad request' do + expect(response).to have_http_status(400) + end end end end diff --git a/spec/policies/account_policy_spec.rb b/spec/policies/account_policy_spec.rb index d27e9d5b0..1347ca4a0 100644 --- a/spec/policies/account_policy_spec.rb +++ b/spec/policies/account_policy_spec.rb @@ -7,8 +7,9 @@ RSpec.describe AccountPolicy do let(:subject) { described_class } let(:admin) { Fabricate(:user, admin: true).account } let(:john) { Fabricate(:user).account } + let(:alice) { Fabricate(:user).account } - permissions :index?, :show?, :unsuspend?, :unsensitive?, :unsilence?, :remove_avatar?, :remove_header? do + permissions :index? do context 'staff' do it 'permits' do expect(subject).to permit(admin) @@ -22,6 +23,38 @@ RSpec.describe AccountPolicy do end end + permissions :show?, :unsilence?, :unsensitive?, :remove_avatar?, :remove_header? do + context 'staff' do + it 'permits' do + expect(subject).to permit(admin, alice) + end + end + + context 'not staff' do + it 'denies' do + expect(subject).to_not permit(john, alice) + end + end + end + + permissions :unsuspend? do + before do + alice.suspend! + end + + context 'staff' do + it 'permits' do + expect(subject).to permit(admin, alice) + end + end + + context 'not staff' do + it 'denies' do + expect(subject).to_not permit(john, alice) + end + end + end + permissions :redownload?, :subscribe?, :unsubscribe? do context 'admin' do it 'permits' do diff --git a/spec/services/activitypub/process_account_service_spec.rb b/spec/services/activitypub/process_account_service_spec.rb index 5141e3f16..56e7f8321 100644 --- a/spec/services/activitypub/process_account_service_spec.rb +++ b/spec/services/activitypub/process_account_service_spec.rb @@ -73,4 +73,84 @@ RSpec.describe ActivityPub::ProcessAccountService, type: :service do expect(ProofProvider::Keybase::Worker).to have_received(:perform_async) end end + + context 'when account is not suspended' do + let!(:account) { Fabricate(:account, username: 'alice', domain: 'example.com') } + + let(:payload) do + { + id: 'https://foo.test', + type: 'Actor', + inbox: 'https://foo.test/inbox', + suspended: true, + }.with_indifferent_access + end + + before do + allow(Admin::SuspensionWorker).to receive(:perform_async) + end + + subject { described_class.new.call('alice', 'example.com', payload) } + + it 'suspends account remotely' do + expect(subject.suspended?).to be true + expect(subject.suspension_origin_remote?).to be true + end + + it 'queues suspension worker' do + subject + expect(Admin::SuspensionWorker).to have_received(:perform_async) + end + end + + context 'when account is suspended' do + let!(:account) { Fabricate(:account, username: 'alice', domain: 'example.com', display_name: '') } + + let(:payload) do + { + id: 'https://foo.test', + type: 'Actor', + inbox: 'https://foo.test/inbox', + suspended: false, + name: 'Hoge', + }.with_indifferent_access + end + + before do + allow(Admin::UnsuspensionWorker).to receive(:perform_async) + + account.suspend!(origin: suspension_origin) + end + + subject { described_class.new.call('alice', 'example.com', payload) } + + context 'locally' do + let(:suspension_origin) { :local } + + it 'does not unsuspend it' do + expect(subject.suspended?).to be true + end + + it 'does not update any attributes' do + expect(subject.display_name).to_not eq 'Hoge' + end + end + + context 'remotely' do + let(:suspension_origin) { :remote } + + it 'unsuspends it' do + expect(subject.suspended?).to be false + end + + it 'queues unsuspension worker' do + subject + expect(Admin::UnsuspensionWorker).to have_received(:perform_async) + end + + it 'updates attributes' do + expect(subject.display_name).to eq 'Hoge' + end + end + end end diff --git a/spec/services/activitypub/process_collection_service_spec.rb b/spec/services/activitypub/process_collection_service_spec.rb index b3baf6b6b..00d71a86a 100644 --- a/spec/services/activitypub/process_collection_service_spec.rb +++ b/spec/services/activitypub/process_collection_service_spec.rb @@ -22,7 +22,48 @@ RSpec.describe ActivityPub::ProcessCollectionService, type: :service do subject { described_class.new } describe '#call' do - context 'when actor is the sender' + context 'when actor is suspended' do + before do + actor.suspend!(origin: :remote) + end + + %w(Accept Add Announce Block Create Flag Follow Like Move Remove).each do |activity_type| + context "with #{activity_type} activity" do + let(:payload) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + id: 'foo', + type: activity_type, + actor: ActivityPub::TagManager.instance.uri_for(actor), + } + end + + it 'does not process payload' do + expect(ActivityPub::Activity).not_to receive(:factory) + subject.call(json, actor) + end + end + end + + %w(Delete Reject Undo Update).each do |activity_type| + context "with #{activity_type} activity" do + let(:payload) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + id: 'foo', + type: activity_type, + actor: ActivityPub::TagManager.instance.uri_for(actor), + } + end + + it 'processes the payload' do + expect(ActivityPub::Activity).to receive(:factory) + subject.call(json, actor) + end + end + end + end + context 'when actor differs from sender' do let(:forwarder) { Fabricate(:account, domain: 'example.com', uri: 'http://example.com/other_account') } diff --git a/spec/services/resolve_account_service_spec.rb b/spec/services/resolve_account_service_spec.rb index cea942e39..76cb9ed8d 100644 --- a/spec/services/resolve_account_service_spec.rb +++ b/spec/services/resolve_account_service_spec.rb @@ -13,16 +13,41 @@ RSpec.describe ResolveAccountService, type: :service do stub_request(:get, "https://ap.example.com/users/foo").to_return(request_fixture('activitypub-actor.txt')) stub_request(:get, "https://ap.example.com/users/foo.atom").to_return(request_fixture('activitypub-feed.txt')) stub_request(:get, %r{https://ap.example.com/users/foo/\w+}).to_return(status: 404) + stub_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:hoge@example.com').to_return(status: 410) end - it 'raises error if no such user can be resolved via webfinger' do + it 'returns nil if no such user can be resolved via webfinger' do expect(subject.call('catsrgr8@quitter.no')).to be_nil end - it 'raises error if the domain does not have webfinger' do + it 'returns nil if the domain does not have webfinger' do expect(subject.call('catsrgr8@example.com')).to be_nil end + context 'when webfinger returns http gone' do + context 'for a previously known account' do + before do + Fabricate(:account, username: 'hoge', domain: 'example.com', last_webfingered_at: nil) + allow(AccountDeletionWorker).to receive(:perform_async) + end + + it 'returns nil' do + expect(subject.call('hoge@example.com')).to be_nil + end + + it 'queues account deletion worker' do + subject.call('hoge@example.com') + expect(AccountDeletionWorker).to have_received(:perform_async) + end + end + + context 'for a previously unknown account' do + it 'returns nil' do + expect(subject.call('hoge@example.com')).to be_nil + end + end + end + context 'with an ActivityPub account' do it 'returns new remote account' do account = subject.call('foo@ap.example.com')