forked from mirrors/catstodon
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
This commit is contained in:
parent
a233a9bfb5
commit
1b4054256f
3 changed files with 115 additions and 9 deletions
|
@ -57,7 +57,16 @@ class ReportService < BaseService
|
||||||
end
|
end
|
||||||
|
|
||||||
def reported_status_ids
|
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
|
end
|
||||||
|
|
||||||
def payload
|
def payload
|
||||||
|
|
|
@ -1,7 +1,7 @@
|
||||||
require 'rails_helper'
|
require 'rails_helper'
|
||||||
|
|
||||||
RSpec.describe ActivityPub::Activity::Flag do
|
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(:flagged) { Fabricate(:account) }
|
||||||
let(:status) { Fabricate(:status, account: flagged, uri: 'foobar') }
|
let(:status) { Fabricate(:status, account: flagged, uri: 'foobar') }
|
||||||
let(:flag_id) { nil }
|
let(:flag_id) { nil }
|
||||||
|
@ -23,6 +23,7 @@ RSpec.describe ActivityPub::Activity::Flag do
|
||||||
describe '#perform' do
|
describe '#perform' do
|
||||||
subject { described_class.new(json, sender) }
|
subject { described_class.new(json, sender) }
|
||||||
|
|
||||||
|
context 'when the reported status is public' do
|
||||||
before do
|
before do
|
||||||
subject.perform
|
subject.perform
|
||||||
end
|
end
|
||||||
|
@ -36,6 +37,77 @@ RSpec.describe ActivityPub::Activity::Flag do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
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) }
|
||||||
|
|
||||||
|
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
|
||||||
|
|
||||||
describe '#perform with a defined uri' do
|
describe '#perform with a defined uri' do
|
||||||
subject { described_class.new(json, sender) }
|
subject { described_class.new(json, sender) }
|
||||||
let (:flag_id) { 'http://example.com/reports/1' }
|
let (:flag_id) { 'http://example.com/reports/1' }
|
||||||
|
|
|
@ -28,6 +28,31 @@ RSpec.describe ReportService, type: :service do
|
||||||
end
|
end
|
||||||
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
|
context 'when other reports already exist for the same target' do
|
||||||
let!(:target_account) { Fabricate(:account) }
|
let!(:target_account) { Fabricate(:account) }
|
||||||
let!(:other_report) { Fabricate(:report, target_account: target_account) }
|
let!(:other_report) { Fabricate(:report, target_account: target_account) }
|
||||||
|
|
Loading…
Reference in a new issue