Fix Undo Announce sometimes inlining the originally Announced status (#17516)

* Change tests to have more specific expectations on sent ActivityPub payloads

* Check that payload doesn't actually contain the contents of the boosted toot

* Fix Undo Announce sometimes inlining the originally Announced status
This commit is contained in:
Claire 2022-02-11 14:52:07 +01:00 committed by GitHub
parent 6f38765fcc
commit d4e6774a0c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 88 additions and 26 deletions

View file

@ -4,7 +4,7 @@ class ActivityPub::ActivityPresenter < ActiveModelSerializers::Model
attributes :id, :type, :actor, :published, :to, :cc, :virtual_object attributes :id, :type, :actor, :published, :to, :cc, :virtual_object
class << self class << self
def from_status(status) def from_status(status, allow_inlining: true)
new.tap do |presenter| new.tap do |presenter|
presenter.id = ActivityPub::TagManager.instance.activity_uri_for(status) presenter.id = ActivityPub::TagManager.instance.activity_uri_for(status)
presenter.type = status.reblog? ? 'Announce' : 'Create' presenter.type = status.reblog? ? 'Announce' : 'Create'
@ -15,7 +15,7 @@ class ActivityPub::ActivityPresenter < ActiveModelSerializers::Model
presenter.virtual_object = begin presenter.virtual_object = begin
if status.reblog? if status.reblog?
if status.account == status.proper.account && status.proper.private_visibility? && status.local? if allow_inlining && status.account == status.proper.account && status.proper.private_visibility? && status.local?
status.proper status.proper
else else
ActivityPub::TagManager.instance.uri_for(status.proper) ActivityPub::TagManager.instance.uri_for(status.proper)

View file

@ -22,6 +22,6 @@ class ActivityPub::UndoAnnounceSerializer < ActivityPub::Serializer
end end
def virtual_object def virtual_object
ActivityPub::ActivityPresenter.from_status(object) ActivityPub::ActivityPresenter.from_status(object, allow_inlining: false)
end end
end end

View file

@ -15,35 +15,97 @@ RSpec.describe RemoveStatusService, type: :service do
jeff.follow!(alice) jeff.follow!(alice)
hank.follow!(alice) hank.follow!(alice)
@status = PostStatusService.new.call(alice, text: 'Hello @bob@example.com')
FavouriteService.new.call(jeff, @status)
Fabricate(:status, account: bill, reblog: @status, uri: 'hoge')
end end
it 'removes status from author\'s home feed' do context 'when removed status is not a reblog' do
subject.call(@status) before do
expect(HomeFeed.new(alice).get(10)).to_not include(@status.id) @status = PostStatusService.new.call(alice, text: 'Hello @bob@example.com ThisIsASecret')
FavouriteService.new.call(jeff, @status)
Fabricate(:status, account: bill, reblog: @status, uri: 'hoge')
end
it 'removes status from author\'s home feed' do
subject.call(@status)
expect(HomeFeed.new(alice).get(10)).to_not include(@status.id)
end
it 'removes status from local follower\'s home feed' do
subject.call(@status)
expect(HomeFeed.new(jeff).get(10)).to_not include(@status.id)
end
it 'sends Delete activity to followers' do
subject.call(@status)
expect(a_request(:post, 'http://example.com/inbox').with(
body: hash_including({
'type' => 'Delete',
'object' => {
'type' => 'Tombstone',
'id' => ActivityPub::TagManager.instance.uri_for(@status),
'atomUri' => OStatus::TagManager.instance.uri_for(@status),
},
})
)).to have_been_made.once
end
it 'sends Delete activity to rebloggers' do
subject.call(@status)
expect(a_request(:post, 'http://example2.com/inbox').with(
body: hash_including({
'type' => 'Delete',
'object' => {
'type' => 'Tombstone',
'id' => ActivityPub::TagManager.instance.uri_for(@status),
'atomUri' => OStatus::TagManager.instance.uri_for(@status),
},
})
)).to have_been_made.once
end
it 'remove status from notifications' do
expect { subject.call(@status) }.to change {
Notification.where(activity_type: 'Favourite', from_account: jeff, account: alice).count
}.from(1).to(0)
end
end end
it 'removes status from local follower\'s home feed' do context 'when removed status is a private self-reblog' do
subject.call(@status) before do
expect(HomeFeed.new(jeff).get(10)).to_not include(@status.id) @original_status = Fabricate(:status, account: alice, text: 'Hello ThisIsASecret', visibility: :private)
@status = ReblogService.new.call(alice, @original_status)
end
it 'sends Undo activity to followers' do
subject.call(@status)
expect(a_request(:post, 'http://example.com/inbox').with(
body: hash_including({
'type' => 'Undo',
'object' => hash_including({
'type' => 'Announce',
'object' => ActivityPub::TagManager.instance.uri_for(@original_status),
}),
})
)).to have_been_made.once
end
end end
it 'sends delete activity to followers' do context 'when removed status is public self-reblog' do
subject.call(@status) before do
expect(a_request(:post, 'http://example.com/inbox')).to have_been_made.twice @original_status = Fabricate(:status, account: alice, text: 'Hello ThisIsASecret', visibility: :public)
end @status = ReblogService.new.call(alice, @original_status)
end
it 'sends delete activity to rebloggers' do it 'sends Undo activity to followers' do
subject.call(@status) subject.call(@status)
expect(a_request(:post, 'http://example2.com/inbox')).to have_been_made expect(a_request(:post, 'http://example.com/inbox').with(
end body: hash_including({
'type' => 'Undo',
it 'remove status from notifications' do 'object' => hash_including({
expect { subject.call(@status) }.to change { 'type' => 'Announce',
Notification.where(activity_type: 'Favourite', from_account: jeff, account: alice).count 'object' => ActivityPub::TagManager.instance.uri_for(@original_status),
}.from(1).to(0) }),
})
)).to have_been_made.once
end
end end
end end