Fix crash when a remote Flag activity mentions a private post (#18760)

* Add tests

* Fix crash when a remote Flag activity mentions a private post

diff --git a/app/services/report_service.rb b/app/services/report_service.rb
index 70212a6a75e3507d92b57e77e084f80cb28613b4..bd67ff8d3ecb335b902caa9a42abb5d9ddfaad15 100644 (file)
--- a/app/services/report_service.rb
+++ b/app/services/report_service.rb
@@ -57,7 +57,16 @@ class ReportService < BaseService
   end
 
   def reported_status_ids
-    AccountStatusesFilter.new(@target_account, @source_account).results.with_discarded.find(Array(@status_ids)).pluck(:id)
+    return AccountStatusesFilter.new(@target_account, @source_account).results.with_discarded.find(Array(@status_ids)).pluck(:id) if @source_account.local?
+
+    # If the account making reports is remote, it is likely anonymized so we have to relax the requirements for attaching statuses.
+    domain = @source_account.domain.to_s.downcase
+    has_followers = @target_account.followers.where(Account.arel_table[:domain].lower.eq(domain)).exists?
+    visibility = has_followers ? %i(public unlisted private) : %i(public unlisted)
+    scope = @target_account.statuses.with_discarded
+    scope.merge!(scope.where(visibility: visibility).or(scope.where('EXISTS (SELECT 1 FROM mentions m JOIN accounts a ON m.account_id = a.id WHERE lower(a.domain) = ?)', domain)))
+    # Allow missing posts to not drop reports that include e.g. a deleted post
+    scope.where(id: Array(@status_ids)).pluck(:id)
   end
 
   def payload

diff --git a/spec/lib/activitypub/activity/flag_spec.rb b/spec/lib/activitypub/activity/flag_spec.rb
index ec7359f2fe00bdeeded19ce10403d4959aa5b4d2..2f2d13876760dfa7570aca1a6ff31851b90e1e54 100644 (file)
--- a/spec/lib/activitypub/activity/flag_spec.rb
+++ b/spec/lib/activitypub/activity/flag_spec.rb
@@ -1,7 +1,7 @@
 require 'rails_helper'
 
 RSpec.describe ActivityPub::Activity::Flag do
-  let(:sender)  { Fabricate(:account, domain: 'example.com', uri: 'http://example.com/account') }
+  let(:sender)  { Fabricate(:account, username: 'example.com', domain: 'example.com', uri: 'http://example.com/actor') }
   let(:flagged) { Fabricate(:account) }
   let(:status)  { Fabricate(:status, account: flagged, uri: 'foobar') }
   let(:flag_id) { nil }
@@ -23,16 +23,88 @@ RSpec.describe ActivityPub::Activity::Flag do
   describe '#perform' do
     subject { described_class.new(json, sender) }
 
-    before do
-      subject.perform
+    context 'when the reported status is public' do
+      before do
+        subject.perform
+      end
+
+      it 'creates a report' do
+        report = Report.find_by(account: sender, target_account: flagged)
+
+        expect(report).to_not be_nil
+        expect(report.comment).to eq 'Boo!!'
+        expect(report.status_ids).to eq [status.id]
+      end
     end
 
-    it 'creates a report' do
-      report = Report.find_by(account: sender, target_account: flagged)
+    context 'when the reported status is private and should not be visible to the remote server' do
+      let(:status) { Fabricate(:status, account: flagged, uri: 'foobar', visibility: :private) }
 
-      expect(report).to_not be_nil
-      expect(report.comment).to eq 'Boo!!'
-      expect(report.status_ids).to eq [status.id]
+      before do
+        subject.perform
+      end
+
+      it 'creates a report with no attached status' do
+        report = Report.find_by(account: sender, target_account: flagged)
+
+        expect(report).to_not be_nil
+        expect(report.comment).to eq 'Boo!!'
+        expect(report.status_ids).to eq []
+      end
+    end
+
+    context 'when the reported status is private and the author has a follower on the remote instance' do
+      let(:status) { Fabricate(:status, account: flagged, uri: 'foobar', visibility: :private) }
+      let(:follower) { Fabricate(:account, domain: 'example.com', uri: 'http://example.com/users/account') }
+
+      before do
+        follower.follow!(flagged)
+        subject.perform
+      end
+
+      it 'creates a report with the attached status' do
+        report = Report.find_by(account: sender, target_account: flagged)
+
+        expect(report).to_not be_nil
+        expect(report.comment).to eq 'Boo!!'
+        expect(report.status_ids).to eq [status.id]
+      end
+    end
+
+    context 'when the reported status is private and the author mentions someone else on the remote instance' do
+      let(:status) { Fabricate(:status, account: flagged, uri: 'foobar', visibility: :private) }
+      let(:mentioned) { Fabricate(:account, domain: 'example.com', uri: 'http://example.com/users/account') }
+
+      before do
+        status.mentions.create(account: mentioned)
+        subject.perform
+      end
+
+      it 'creates a report with the attached status' do
+        report = Report.find_by(account: sender, target_account: flagged)
+
+        expect(report).to_not be_nil
+        expect(report.comment).to eq 'Boo!!'
+        expect(report.status_ids).to eq [status.id]
+      end
+    end
+
+    context 'when the reported status is private and the author mentions someone else on the local instance' do
+      let(:status) { Fabricate(:status, account: flagged, uri: 'foobar', visibility: :private) }
+      let(:mentioned) { Fabricate(:account) }
+
+      before do
+        status.mentions.create(account: mentioned)
+        subject.perform
+      end
+
+      it 'creates a report with no attached status' do
+        report = Report.find_by(account: sender, target_account: flagged)
+
+        expect(report).to_not be_nil
+        expect(report.comment).to eq 'Boo!!'
+        expect(report.status_ids).to eq []
+      end
     end
   end
 

diff --git a/spec/services/report_service_spec.rb b/spec/services/report_service_spec.rb
index 7e6a113e023466fe59a7d7e24008f3594b295802..ea68b3344dc6d3753c3dc81db33f2b91e9d50523 100644 (file)
--- a/spec/services/report_service_spec.rb
+++ b/spec/services/report_service_spec.rb
@@ -28,6 +28,31 @@ RSpec.describe ReportService, type: :service do
     end
   end
 
+  context 'when the reported status is a DM' do
+    let(:target_account) { Fabricate(:account) }
+    let(:status) { Fabricate(:status, account: target_account, visibility: :direct) }
+
+    subject do
+      -> { described_class.new.call(source_account, target_account, status_ids: [status.id]) }
+    end
+
+    context 'when it is addressed to the reporter' do
+      before do
+        status.mentions.create(account: source_account)
+      end
+
+      it 'creates a report' do
+        is_expected.to change { target_account.targeted_reports.count }.from(0).to(1)
+      end
+    end
+
+    context 'when it is not addressed to the reporter' do
+      it 'errors out' do
+        is_expected.to raise_error
+      end
+    end
+  end
+
   context 'when other reports already exist for the same target' do
     let!(:target_account) { Fabricate(:account) }
     let!(:other_report)   { Fabricate(:report, target_account: target_account) }