From dfc9f35d711389cd870c6764639b9bad07e6e02b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?KMY=EF=BC=88=E9=9B=AA=E3=81=82=E3=81=99=E3=81=8B=EF=BC=89?= Date: Sun, 18 Feb 2024 10:48:48 +0900 Subject: [PATCH] =?UTF-8?q?Add:=20#586=20=E4=BF=9D=E7=95=99=E4=B8=AD?= =?UTF-8?q?=E3=81=AE=E3=83=AA=E3=83=A2=E3=83=BC=E3=83=88=E3=82=A2=E3=82=AB?= =?UTF-8?q?=E3=82=A6=E3=83=B3=E3=83=88=E3=81=8B=E3=82=89=E3=81=AE=E3=83=95?= =?UTF-8?q?=E3=82=A9=E3=83=AD=E3=83=BC=E3=81=8C=E9=A3=9B=E3=82=93=E3=81=A7?= =?UTF-8?q?=E3=81=8D=E3=81=9F=E5=A0=B4=E5=90=88=E3=81=AB=E8=A8=98=E9=8C=B2?= =?UTF-8?q?=E3=81=99=E3=82=8B=20(#590)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add: #586 保留中のリモートアカウントからのフォローが飛んできた場合に記録する * 本家に戻す処理を修正 * Fix test * Fix worker link * Fix test * リモートアカウント拒否時に既存のリクエストを削除 --- app/helpers/follow_helper.rb | 35 ++++++++++++ app/lib/activitypub/activity/follow.rb | 39 ++++--------- app/models/account.rb | 3 + app/models/concerns/account/interactions.rb | 1 + app/models/pending_follow_request.rb | 19 +++++++ .../activitypub/process_collection_service.rb | 6 +- app/services/delete_account_service.rb | 2 + .../enable_follow_requests_service.rb | 32 +++++++++++ app/workers/enable_follow_requests_worker.rb | 13 +++++ ...17230006_create_pending_follow_requests.rb | 17 ++++++ db/schema.rb | 15 ++++- lib/tasks/dangerous.rake | 2 + .../pending_follow_request_fabricator.rb | 7 +++ spec/lib/activitypub/activity/follow_spec.rb | 47 ++++++++++++++++ .../enable_follow_requests_service_spec.rb | 55 +++++++++++++++++++ 15 files changed, 262 insertions(+), 31 deletions(-) create mode 100644 app/helpers/follow_helper.rb create mode 100644 app/models/pending_follow_request.rb create mode 100644 app/services/enable_follow_requests_service.rb create mode 100644 app/workers/enable_follow_requests_worker.rb create mode 100644 db/migrate/20240217230006_create_pending_follow_requests.rb create mode 100644 spec/fabricators/pending_follow_request_fabricator.rb create mode 100644 spec/services/enable_follow_requests_service_spec.rb diff --git a/app/helpers/follow_helper.rb b/app/helpers/follow_helper.rb new file mode 100644 index 0000000000..d7d7ff35e9 --- /dev/null +++ b/app/helpers/follow_helper.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +module FollowHelper + def request_pending_follow?(source_account, target_account) + target_account.locked? || source_account.silenced? || block_straight_follow?(source_account) || + ((source_account.bot? || proxy_account?(source_account)) && target_account.user&.setting_lock_follow_from_bot) + end + + def block_straight_follow?(account) + return false if account.local? + + DomainBlock.reject_straight_follow?(account.domain) + end + + def proxy_account?(account) + (account.username.downcase.include?('_proxy') || + account.username.downcase.end_with?('proxy') || + account.username.downcase.include?('_bot_') || + account.username.downcase.end_with?('bot') || + account.display_name&.downcase&.include?('proxy') || + account.display_name&.include?('プロキシ') || + account.note&.include?('プロキシ')) && + (account.following_count.zero? || account.following_count > account.followers_count) && + proxyable_software?(account) + end + + def proxyable_software?(account) + return false if account.local? + + info = InstanceInfo.find_by(domain: account.domain) + return false if info.nil? + + %w(misskey calckey firefish meisskey cherrypick sharkey).include?(info.software) + end +end diff --git a/app/lib/activitypub/activity/follow.rb b/app/lib/activitypub/activity/follow.rb index c8cf16f822..d327e25475 100644 --- a/app/lib/activitypub/activity/follow.rb +++ b/app/lib/activitypub/activity/follow.rb @@ -2,6 +2,7 @@ class ActivityPub::Activity::Follow < ActivityPub::Activity include Payloadable + include FollowHelper def perform return request_follow_for_friend if friend_follow? @@ -11,7 +12,7 @@ class ActivityPub::Activity::Follow < ActivityPub::Activity return if target_account.nil? || !target_account.local? || delete_arrived_first?(@json['id']) # Update id of already-existing follow requests - existing_follow_request = ::FollowRequest.find_by(account: @account, target_account: target_account) + existing_follow_request = ::FollowRequest.find_by(account: @account, target_account: target_account) || PendingFollowRequest.find_by(account: @account, target_account: target_account) unless existing_follow_request.nil? existing_follow_request.update!(uri: @json['id']) return @@ -30,9 +31,16 @@ class ActivityPub::Activity::Follow < ActivityPub::Activity return end + if @account.suspended? && @account.remote_pending? + PendingFollowRequest.create!(account: @account, target_account: target_account, uri: @json['id']) + return + elsif @account.suspended? + return + end + follow_request = FollowRequest.create!(account: @account, target_account: target_account, uri: @json['id']) - if target_account.locked? || @account.silenced? || block_straight_follow? || ((@account.bot? || proxy_account?) && target_account.user&.setting_lock_follow_from_bot) + if request_pending_follow?(@account, target_account) LocalNotificationWorker.perform_async(target_account.id, follow_request.id, 'FollowRequest', 'follow_request') else AuthorizeFollowService.new.call(@account, target_account) @@ -79,37 +87,10 @@ class ActivityPub::Activity::Follow < ActivityPub::Activity @block_friend ||= DomainBlock.reject_friend?(@account.domain) || DomainBlock.blocked?(@account.domain) end - def block_straight_follow? - @block_straight_follow ||= DomainBlock.reject_straight_follow?(@account.domain) - end - def block_new_follow? @block_new_follow ||= DomainBlock.reject_new_follow?(@account.domain) end - def proxy_account? - (@account.username.downcase.include?('_proxy') || - @account.username.downcase.end_with?('proxy') || - @account.username.downcase.include?('_bot_') || - @account.username.downcase.end_with?('bot') || - @account.display_name&.downcase&.include?('proxy') || - @account.display_name&.include?('プロキシ') || - @account.note&.include?('プロキシ')) && - (@account.following_count.zero? || @account.following_count > @account.followers_count) && - proxyable_software? - end - - def proxyable_software? - info = instance_info - return false if info.nil? - - %w(misskey calckey firefish meisskey cherrypick sharkey).include?(info.software) - end - - def instance_info - @instance_info ||= InstanceInfo.find_by(domain: @account.domain) - end - def notify_staff_about_pending_friend_server! User.those_who_can(:manage_federation).includes(:account).find_each do |u| next unless u.allows_pending_friend_server_emails? diff --git a/app/models/account.rb b/app/models/account.rb index f65575bff5..ca4ad3c4a9 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -300,10 +300,13 @@ class Account < ApplicationRecord def approve_remote! update!(remote_pending: false) unsuspend! + EnableFollowRequestsWorker.perform_async(id) end def reject_remote! update!(remote_pending: false, suspension_origin: :local) + pending_follow_requests.destroy_all + suspend! end def sensitized? diff --git a/app/models/concerns/account/interactions.rb b/app/models/concerns/account/interactions.rb index d5c232336a..3d321c9ea8 100644 --- a/app/models/concerns/account/interactions.rb +++ b/app/models/concerns/account/interactions.rb @@ -74,6 +74,7 @@ module Account::Interactions included do # Follow relations has_many :follow_requests, dependent: :destroy + has_many :pending_follow_requests, dependent: :destroy with_options class_name: 'Follow', dependent: :destroy do has_many :active_relationships, foreign_key: 'account_id', inverse_of: :account diff --git a/app/models/pending_follow_request.rb b/app/models/pending_follow_request.rb new file mode 100644 index 0000000000..d9cb114031 --- /dev/null +++ b/app/models/pending_follow_request.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +# == Schema Information +# +# Table name: pending_follow_requests +# +# id :bigint(8) not null, primary key +# account_id :bigint(8) not null +# target_account_id :bigint(8) not null +# uri :string not null +# created_at :datetime not null +# updated_at :datetime not null +# +class PendingFollowRequest < ApplicationRecord + belongs_to :account + belongs_to :target_account, class_name: 'Account' + + validates :account_id, uniqueness: { scope: :target_account_id } +end diff --git a/app/services/activitypub/process_collection_service.rb b/app/services/activitypub/process_collection_service.rb index 4f049a5ae9..aaa9a9d668 100644 --- a/app/services/activitypub/process_collection_service.rb +++ b/app/services/activitypub/process_collection_service.rb @@ -48,13 +48,17 @@ class ActivityPub::ProcessCollectionService < BaseService end def suspended_actor? - @account.suspended? && !activity_allowed_while_suspended? + @account.suspended? && (@account.remote_pending ? !activity_allowed_while_remote_pending? : !activity_allowed_while_suspended?) end def activity_allowed_while_suspended? %w(Delete Reject Undo Update).include?(@json['type']) end + def activity_allowed_while_remote_pending? + %w(Follow).include?(@json['type']) || activity_allowed_while_suspended? + end + def process_items(items) items.reverse_each.filter_map { |item| process_item(item) } end diff --git a/app/services/delete_account_service.rb b/app/services/delete_account_service.rb index 8bb6fdc960..e790f547fb 100644 --- a/app/services/delete_account_service.rb +++ b/app/services/delete_account_service.rb @@ -28,6 +28,7 @@ class DeleteAccountService < BaseService notifications owned_lists passive_relationships + pending_follow_requests report_notes scheduled_statuses scheduled_expiration_statuses @@ -57,6 +58,7 @@ class DeleteAccountService < BaseService muted_by_relationships notifications owned_lists + pending_follow_requests scheduled_statuses scheduled_expiration_statuses status_pins diff --git a/app/services/enable_follow_requests_service.rb b/app/services/enable_follow_requests_service.rb new file mode 100644 index 0000000000..74e280619e --- /dev/null +++ b/app/services/enable_follow_requests_service.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +class EnableFollowRequestsService < BaseService + include Payloadable + include FollowHelper + + def call(account) + @account = account + + PendingFollowRequest.transaction do + PendingFollowRequest.where(account: account).find_each do |follow_request| + approve_follow!(follow_request) + end + end + end + + private + + def approve_follow!(pending) + follow_request = FollowRequest.create!(account: @account, target_account: pending.target_account, uri: pending.uri) + pending.destroy! + + target_account = follow_request.target_account + + if request_pending_follow?(@account, target_account) + LocalNotificationWorker.perform_async(target_account.id, follow_request.id, 'FollowRequest', 'follow_request') + else + AuthorizeFollowService.new.call(@account, target_account) + LocalNotificationWorker.perform_async(target_account.id, ::Follow.find_by(account: @account, target_account: target_account).id, 'Follow', 'follow') + end + end +end diff --git a/app/workers/enable_follow_requests_worker.rb b/app/workers/enable_follow_requests_worker.rb new file mode 100644 index 0000000000..b0993b2e22 --- /dev/null +++ b/app/workers/enable_follow_requests_worker.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class EnableFollowRequestsWorker + include Sidekiq::Worker + + def perform(account_id) + account = Account.find_by(id: account_id) + return true if account.nil? + return true if account.suspended? + + EnableFollowRequestsService.new.call(account) + end +end diff --git a/db/migrate/20240217230006_create_pending_follow_requests.rb b/db/migrate/20240217230006_create_pending_follow_requests.rb new file mode 100644 index 0000000000..9d5565ce21 --- /dev/null +++ b/db/migrate/20240217230006_create_pending_follow_requests.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class CreatePendingFollowRequests < ActiveRecord::Migration[7.1] + disable_ddl_transaction! + + def change + create_table :pending_follow_requests do |t| + t.references :account, null: false, foreign_key: { on_delete: :cascade }, index: false + t.references :target_account, null: false, foreign_key: { to_table: 'accounts', on_delete: :cascade } + t.string :uri, null: false, index: { unique: true } + + t.timestamps + end + + add_index :pending_follow_requests, [:account_id, :target_account_id], unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 0b1b5c863c..fc4b2843e2 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[7.1].define(version: 2024_02_17_215134) do +ActiveRecord::Schema[7.1].define(version: 2024_02_17_230006) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -954,6 +954,17 @@ ActiveRecord::Schema[7.1].define(version: 2024_02_17_215134) do t.index ["key_id"], name: "index_one_time_keys_on_key_id" end + create_table "pending_follow_requests", force: :cascade do |t| + t.bigint "account_id", null: false + t.bigint "target_account_id", null: false + t.string "uri", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["account_id", "target_account_id"], name: "idx_on_account_id_target_account_id_46f2a00f12", unique: true + t.index ["target_account_id"], name: "index_pending_follow_requests_on_target_account_id" + t.index ["uri"], name: "index_pending_follow_requests_on_uri", unique: true + end + create_table "pghero_space_stats", force: :cascade do |t| t.text "database" t.text "schema" @@ -1544,6 +1555,8 @@ ActiveRecord::Schema[7.1].define(version: 2024_02_17_215134) do add_foreign_key "oauth_access_tokens", "users", column: "resource_owner_id", name: "fk_e84df68546", on_delete: :cascade add_foreign_key "oauth_applications", "users", column: "owner_id", name: "fk_b0988c7c0a", on_delete: :cascade add_foreign_key "one_time_keys", "devices", on_delete: :cascade + add_foreign_key "pending_follow_requests", "accounts", column: "target_account_id", on_delete: :cascade + add_foreign_key "pending_follow_requests", "accounts", on_delete: :cascade add_foreign_key "poll_votes", "accounts", on_delete: :cascade add_foreign_key "poll_votes", "polls", on_delete: :cascade add_foreign_key "polls", "accounts", on_delete: :cascade diff --git a/lib/tasks/dangerous.rake b/lib/tasks/dangerous.rake index 9bbe4471db..30ba86fa23 100644 --- a/lib/tasks/dangerous.rake +++ b/lib/tasks/dangerous.rake @@ -87,6 +87,7 @@ namespace :dangerous do 20240216042730 20240217022038 20240217093511 + 20240217230006 ) # Removed: account_groups target_tables = %w( @@ -104,6 +105,7 @@ namespace :dangerous do instance_infos list_statuses ngword_histories + pending_follow_requests scheduled_expiration_statuses status_capability_tokens status_references diff --git a/spec/fabricators/pending_follow_request_fabricator.rb b/spec/fabricators/pending_follow_request_fabricator.rb new file mode 100644 index 0000000000..5ec74d725f --- /dev/null +++ b/spec/fabricators/pending_follow_request_fabricator.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +Fabricator(:pending_follow_request) do + account { Fabricate.build(:account) } + target_account { Fabricate.build(:account, locked: true) } + uri 'https://example.com/follow' +end diff --git a/spec/lib/activitypub/activity/follow_spec.rb b/spec/lib/activitypub/activity/follow_spec.rb index a2691c309f..ecb41ef25b 100644 --- a/spec/lib/activitypub/activity/follow_spec.rb +++ b/spec/lib/activitypub/activity/follow_spec.rb @@ -100,6 +100,10 @@ RSpec.describe ActivityPub::Activity::Follow do expect(sender.requested?(recipient)).to be true expect(sender.follow_requests.find_by(target_account: recipient).uri).to eq 'foo' end + + it 'does not create pending request' do + expect(sender.pending_follow_requests.find_by(target_account: recipient)).to be_nil + end end context 'when unlocked account but locked from bot' do @@ -169,6 +173,49 @@ RSpec.describe ActivityPub::Activity::Follow do end end + context 'when remote pending account follows unlocked account' do + before do + sender.update(suspended_at: Time.now.utc, suspension_origin: :local, remote_pending: true) + allow(LocalNotificationWorker).to receive(:perform_async).and_return(nil) + subject.perform + end + + it 'does not create a follow from sender to recipient' do + expect(sender.following?(recipient)).to be false + end + + it 'does not notify' do + expect(LocalNotificationWorker).to_not have_received(:perform_async) + end + + it 'creates a follow request' do + expect(sender.requested?(recipient)).to be false + + follow_request = sender.pending_follow_requests.find_by(target_account: recipient) + expect(follow_request).to_not be_nil + expect(follow_request.uri).to eq 'foo' + end + end + + context 'when remote pending account follows unlocked account but has already existing request' do + before do + sender.update(suspended_at: Time.now.utc, suspension_origin: :local, remote_pending: true) + Fabricate(:pending_follow_request, account: sender, target_account: recipient, uri: 'old') + subject.perform + end + + it 'does not create a follow from sender to recipient' do + expect(sender.following?(recipient)).to be false + expect(sender.requested?(recipient)).to be false + end + + it 'changes follow request uri' do + follow_request = sender.pending_follow_requests.find_by(target_account: recipient) + expect(follow_request).to_not be_nil + expect(follow_request.uri).to eq 'foo' + end + end + context 'when domain block reject_straight_follow' do before do Fabricate(:domain_block, domain: 'example.com', reject_straight_follow: true) diff --git a/spec/services/enable_follow_requests_service_spec.rb b/spec/services/enable_follow_requests_service_spec.rb new file mode 100644 index 0000000000..f4e93a32e1 --- /dev/null +++ b/spec/services/enable_follow_requests_service_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe EnableFollowRequestsService, type: :service do + subject { described_class.new.call(sender) } + + let(:sender) { Fabricate(:account, domain: 'example.com', uri: 'https://example.com/actor') } + let(:alice) { Fabricate(:account) } + let!(:follow_request) { Fabricate(:pending_follow_request, account: sender, target_account: alice) } + + before do + allow(LocalNotificationWorker).to receive(:perform_async).and_return(nil) + end + + context 'when has a silent follow request' do + before do + subject + end + + it 'follows immediately' do + follow = Follow.find_by(account: sender, target_account: alice) + expect(follow).to_not be_nil + expect(LocalNotificationWorker).to have_received(:perform_async).with(alice.id, follow.id, 'Follow', 'follow') + + new_follow_request = FollowRequest.find_by(account: sender, target_account: alice) + expect(new_follow_request).to be_nil + end + + it 'pending request is removed' do + expect { follow_request.reload }.to raise_error ActiveRecord::RecordNotFound + end + end + + context 'when target_account is locked' do + before do + alice.update!(locked: true) + subject + end + + it 'enable a follow request' do + new_follow_request = FollowRequest.find_by(account: sender, target_account: alice) + + expect(sender.following?(alice)).to be false + + expect(new_follow_request).to_not be_nil + expect(new_follow_request.uri).to eq follow_request.uri + expect(LocalNotificationWorker).to have_received(:perform_async).with(alice.id, new_follow_request.id, 'FollowRequest', 'follow_request') + end + + it 'pending request is removed' do + expect { follow_request.reload }.to raise_error ActiveRecord::RecordNotFound + end + end +end