From fcd13a6474cae9993fccfb6383f2020139d538be 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: Wed, 24 Jan 2024 09:03:24 +0900 Subject: [PATCH] =?UTF-8?q?Change:=20#375=20=E6=8A=95=E7=A8=BF=E3=82=92?= =?UTF-8?q?=E7=B7=A8=E9=9B=86=E3=81=97=E3=81=A6=E6=8B=A1=E5=BC=B5=E3=83=89?= =?UTF-8?q?=E3=83=A1=E3=82=A4=E3=83=B3=E3=83=96=E3=83=AD=E3=83=83=E3=82=AF?= =?UTF-8?q?=E3=81=AE=E6=9D=A1=E4=BB=B6=E3=81=AB=E3=81=B2=E3=81=A3=E3=81=8B?= =?UTF-8?q?=E3=81=8B=E3=82=8B=E7=8A=B6=E6=85=8B=E3=81=AB=E3=81=AA=E3=81=A3?= =?UTF-8?q?=E3=81=9F=E5=A0=B4=E5=90=88=E3=80=81=E5=AF=BE=E8=B1=A1=E3=82=B5?= =?UTF-8?q?=E3=83=BC=E3=83=90=E3=83=BC=E3=81=AB=E3=81=AF=E6=8A=95=E7=A8=BF?= =?UTF-8?q?=E5=89=8A=E9=99=A4=E3=81=AEActivity=E3=82=92=E9=80=81=E4=BF=A1?= =?UTF-8?q?=E3=81=99=E3=82=8B=20(#495)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Change: #375 投稿を編集して拡張ドメインブロックの条件にひっかかる状態になった場合、対象サーバーには投稿削除のActivityを送信する * Fix test * Add test --- app/lib/status_reach_finder.rb | 26 +++++++++- .../concerns/status/domain_block_concern.rb | 2 +- app/services/post_status_service.rb | 6 +-- app/services/update_status_service.rb | 7 ++- .../status_update_distribution_worker.rb | 35 +++++++++++++ spec/lib/activitypub/activity/update_spec.rb | 27 ++++++++++ spec/lib/status_reach_finder_spec.rb | 6 +-- spec/services/update_status_service_spec.rb | 49 +++++++++++++++++++ 8 files changed, 146 insertions(+), 12 deletions(-) diff --git a/app/lib/status_reach_finder.rb b/app/lib/status_reach_finder.rb index aad6bb735f..98e19b5e37 100644 --- a/app/lib/status_reach_finder.rb +++ b/app/lib/status_reach_finder.rb @@ -31,6 +31,10 @@ class StatusReachFinder ) end + def inboxes_diff_for_sending_domain_block + (reached_account_inboxes_for_sending_domain_block + followers_inboxes_for_sending_domain_block).uniq + end + def all_inboxes (inboxes + inboxes_for_misskey + inboxes_for_friend).uniq end @@ -58,6 +62,14 @@ class StatusReachFinder end end + def reached_account_inboxes_for_sending_domain_block + if @status.reblog? || @status.limited_visibility? + [] + else + Account.where(id: reached_account_ids, domain: banned_domains_of_status(@status)).inboxes + end + end + def reached_account_ids # When the status is a reblog, there are no interactions with it # directly, we assume all interactions are with the original one @@ -144,6 +156,10 @@ class StatusReachFinder end end + def followers_inboxes_for_sending_domain_block + @status.account.followers.where(domain: banned_domains_of_status(@status)).inboxes + end + def relay_inboxes if @status.public_visibility? Relay.enabled.pluck(:inbox_url) @@ -192,9 +208,15 @@ class StatusReachFinder end def banned_domains_of_status(status) + return [] unless status.sending_sensitive? + return @banned_domains_of_status if defined?(@banned_domains_of_status) && status.id == @status.id + blocks = DomainBlock.where(domain: nil) - blocks = blocks.or(DomainBlock.where(reject_send_sensitive: true)) if (status.with_media? && status.sensitive) || status.spoiler_text? - blocks.pluck(:domain).uniq + blocks = blocks.or(DomainBlock.where(reject_send_sensitive: true)) if status.sending_sensitive? + domains = blocks.pluck(:domain).uniq + + @banned_domains_of_status = domains if status.id == @status.id + domains end def banned_domains_for_misskey diff --git a/app/models/concerns/status/domain_block_concern.rb b/app/models/concerns/status/domain_block_concern.rb index 68313d8380..aa31d5a08a 100644 --- a/app/models/concerns/status/domain_block_concern.rb +++ b/app/models/concerns/status/domain_block_concern.rb @@ -6,7 +6,7 @@ module Status::DomainBlockConcern def sending_sensitive? return false unless local? - (with_media? && sensitive) || spoiler_text? + sensitive end def sending_maybe_compromised_privacy? diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index 5a99b79295..91520557d4 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -68,11 +68,7 @@ class PostStatusService < BaseService private def preprocess_attributes! - @sensitive = (if @options[:sensitive].nil? - @media.any? ? @account.user&.setting_default_sensitive : false - else - @options[:sensitive] - end) || @options[:spoiler_text].present? + @sensitive = (@options[:sensitive].nil? ? @account.user&.setting_default_sensitive : @options[:sensitive]) || @options[:spoiler_text].present? @text = @options.delete(:spoiler_text) if @text.blank? && @options[:spoiler_text].present? @visibility = @options[:visibility]&.to_sym || @account.user&.setting_default_privacy&.to_sym @visibility = :limited if %w(mutual circle reply).include?(@options[:visibility]) diff --git a/app/services/update_status_service.rb b/app/services/update_status_service.rb index 57dd7ccc1a..3bd234a1ef 100644 --- a/app/services/update_status_service.rb +++ b/app/services/update_status_service.rb @@ -24,6 +24,7 @@ class UpdateStatusService < BaseService @account_id = account_id @media_attachments_changed = false @poll_changed = false + @old_sensitive = sensitive? clear_histories! if @options[:no_history] @@ -189,7 +190,7 @@ class UpdateStatusService < BaseService def broadcast_updates! DistributionWorker.perform_async(@status.id, { 'update' => true }) - ActivityPub::StatusUpdateDistributionWorker.perform_async(@status.id) + ActivityPub::StatusUpdateDistributionWorker.perform_async(@status.id, { 'sensitive' => sensitive?, 'sensitive_changed' => @old_sensitive != sensitive? && sensitive? }) end def queue_poll_notifications! @@ -227,4 +228,8 @@ class UpdateStatusService < BaseService @status.edited_at = nil @status.save! end + + def sensitive? + @status.sensitive + end end diff --git a/app/workers/activitypub/status_update_distribution_worker.rb b/app/workers/activitypub/status_update_distribution_worker.rb index 2c7922e419..df415ab23a 100644 --- a/app/workers/activitypub/status_update_distribution_worker.rb +++ b/app/workers/activitypub/status_update_distribution_worker.rb @@ -12,6 +12,7 @@ class ActivityPub::StatusUpdateDistributionWorker < ActivityPub::DistributionWor distribute_limited! else distribute! + distribute_delete_activity! end rescue ActiveRecord::RecordNotFound true @@ -19,6 +20,17 @@ class ActivityPub::StatusUpdateDistributionWorker < ActivityPub::DistributionWor protected + def inboxes + return super if @status.limited_visibility? + return super unless sensitive? + + super - inboxes_diff_for_sending_domain_block + end + + def inboxes_diff_for_sending_domain_block + status_reach_finder.inboxes_diff_for_sending_domain_block + end + def inboxes_for_limited @inboxes_for_limited ||= @status.mentioned_accounts.inboxes end @@ -47,7 +59,30 @@ class ActivityPub::StatusUpdateDistributionWorker < ActivityPub::DistributionWor build_activity(for_friend: true) end + def delete_activity + @delete_activity ||= Oj.dump(serialize_payload(@status, ActivityPub::DeleteSerializer, signer: @account)) + end + + def distribute_delete_activity! + return unless sensitive_changed? + + target_inboxes = inboxes_diff_for_sending_domain_block + return if target_inboxes.empty? + + ActivityPub::DeliveryWorker.push_bulk(target_inboxes, limit: 1_000) do |inbox_url| + [delete_activity, @account.id, inbox_url, {}] + end + end + def always_sign @status.limited_visibility? end + + def sensitive? + @options[:sensitive] + end + + def sensitive_changed? + @options[:sensitive_changed] + end end diff --git a/spec/lib/activitypub/activity/update_spec.rb b/spec/lib/activitypub/activity/update_spec.rb index 4b27c7548c..7f312e044b 100644 --- a/spec/lib/activitypub/activity/update_spec.rb +++ b/spec/lib/activitypub/activity/update_spec.rb @@ -117,6 +117,33 @@ RSpec.describe ActivityPub::Activity::Update do end end + context 'when the status is not existing' do + let(:json) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + id: 'foo', + type: 'Update', + actor: sender.uri, + signature: 'foo', + object: { + type: 'Note', + id: 'https://example.com/note', + content: 'Ohagi is tsubuan', + }, + }.with_indifferent_access + end + + before do + stub_request(:post, 'https://example.com/inbox').to_return(status: 200) + subject.perform + end + + it 'does not create a new status', :sidekiq_inline do + status = Status.find_by(uri: 'https://example.com/note') + expect(status).to be_nil + end + end + context 'when the status is limited post and has conversation' do let(:status) { Fabricate(:status, visibility: :limited, account: sender, uri: 'https://example.com/note', text: 'Ohagi is koshian') } let(:conversation) { Fabricate(:conversation, ancestor_status: status) } diff --git a/spec/lib/status_reach_finder_spec.rb b/spec/lib/status_reach_finder_spec.rb index 9bbd1c09a6..5efe30738d 100644 --- a/spec/lib/status_reach_finder_spec.rb +++ b/spec/lib/status_reach_finder_spec.rb @@ -379,15 +379,15 @@ describe StatusReachFinder do let(:visibility) { :public } let(:searchability) { :public } let(:dissubscribable) { false } - let(:spoiler_text) { '' } - let(:status) { Fabricate(:status, account: alice, visibility: visibility, searchability: searchability, spoiler_text: spoiler_text) } + let(:sensitive) { false } + let(:status) { Fabricate(:status, account: alice, visibility: visibility, searchability: searchability, sensitive: sensitive) } let(:alice) { Fabricate(:account, username: 'alice', master_settings: { subscription_policy: dissubscribable ? 'block' : 'allow' }) } let(:bob) { Fabricate(:account, username: 'bob', domain: 'example.com', protocol: :activitypub, uri: 'https://example.com/', inbox_url: 'https://example.com/inbox') } let(:tom) { Fabricate(:account, username: 'tom', domain: 'tom.com', protocol: :activitypub, uri: 'https://tom.com/', inbox_url: 'https://tom.com/inbox') } context 'when reject_send_sensitive' do let(:properties) { { reject_send_sensitive: true } } - let(:spoiler_text) { 'CW' } + let(:sensitive) { true } it 'does not include the inbox of blocked domain' do expect(subject.inboxes).to_not include 'https://example.com/inbox' diff --git a/spec/services/update_status_service_spec.rb b/spec/services/update_status_service_spec.rb index 370f7b6d48..f422d2a206 100644 --- a/spec/services/update_status_service_spec.rb +++ b/spec/services/update_status_service_spec.rb @@ -62,6 +62,55 @@ RSpec.describe UpdateStatusService, type: :service do end end + context 'when content warning changes and has remote user', :sidekiq_inline do + let(:remote_follower) { Fabricate(:account, domain: 'example.com', uri: 'https://example.com/actor', protocol: :activitypub, inbox_url: 'https://example.com/inbox') } + let(:status) { Fabricate(:status, text: 'Foo', spoiler_text: '', account: Fabricate(:user).account) } + + before do + remote_follower.follow!(status.account) + stub_request(:post, 'https://example.com/inbox').to_return(status: 200) + end + + def match_update_request(req, type) + json = JSON.parse(req.body) + actor_id = ActivityPub::TagManager.instance.uri_for(status.account) + status_id = ActivityPub::TagManager.instance.uri_for(status) + json['type'] == type && json['actor'] == actor_id && json['object']['id'] == status_id + end + + it 'edit activity is sent' do + subject.call(status, status.account_id, text: 'Foo', spoiler_text: 'Bar') + + expect(a_request(:post, 'https://example.com/inbox').with { |req| match_update_request(req, 'Update') }).to have_been_made.once + expect(a_request(:post, 'https://example.com/inbox').with { |req| match_update_request(req, 'Delete') }).to_not have_been_made + end + + it 'edit activity is sent for target user' do + Fabricate(:domain_block, domain: 'example.com', severity: :noop, reject_send_sensitive: true) + subject.call(status, status.account_id, text: 'Ohagi') + + expect(a_request(:post, 'https://example.com/inbox').with { |req| match_update_request(req, 'Update') }).to have_been_made.once + expect(a_request(:post, 'https://example.com/inbox').with { |req| match_update_request(req, 'Delete') }).to_not have_been_made + end + + it 'delete activity is sent when follower is target user' do + Fabricate(:domain_block, domain: 'example.com', severity: :noop, reject_send_sensitive: true) + subject.call(status, status.account_id, text: 'Foo', spoiler_text: 'Bar') + + expect(a_request(:post, 'https://example.com/inbox').with { |req| match_update_request(req, 'Delete') }).to have_been_made.once + expect(a_request(:post, 'https://example.com/inbox').with { |req| match_update_request(req, 'Update') }).to_not have_been_made + end + + it 'delete activity is sent and update activity is not sent when follower is target user' do + Fabricate(:domain_block, domain: 'example.com', severity: :noop, reject_send_sensitive: true) + subject.call(status, status.account_id, text: 'Foo', spoiler_text: 'Bar') + subject.call(status, status.account_id, text: 'Ohagi', spoiler_text: 'Bar') + + expect(a_request(:post, 'https://example.com/inbox').with { |req| match_update_request(req, 'Delete') }).to have_been_made.once + expect(a_request(:post, 'https://example.com/inbox').with { |req| match_update_request(req, 'Update') }).to_not have_been_made + end + end + context 'when media attachments change' do let!(:status) { Fabricate(:status, text: 'Foo') } let!(:detached_media_attachment) { Fabricate(:media_attachment, account: status.account) }