Fix not handling Undo on some activity types when they aren't inlined (#14346)

* Fix not handling Undo on some activity types when they aren't inlined

When receiving an Undo for a non-inlined activity, try looking it up in
database using the URI. The queries are ad-hoc because we don't have a global
index of object URIs, and not all activity types are stored in database with
an index on their URI.

Announces are just statuses, and have an index on URIs, so this check can
be done efficiently.

Accepts cannot be handled at all because we don't record their URI at any
point.

Follows don't have an index on URI, but they have an index on the issuing
account, which should make such queries largely manageable.

Likes don't have an index on URI, they have an index on the issuing account,
but the number of favs per account may be very high, so I decided not to
handle that.

Blocks don't have an index on URI, but they have an index on the issuing
account, which should make such queries largely manageable.

In all cases, if an Undo could not be handled properly, we call `delete_later!`
because that does not require us to know more than the URI of the undone
property.

* Add tests

* Make newer blocks overwrite older ones

Allows re-synchronizing block info by re-blocking and un-blocking again
when the original Undo Block has been lost.
This commit is contained in:
ThibG 2020-07-22 11:45:35 +02:00 committed by GitHub
parent f55dd193f9
commit 5d9acc0ce4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 112 additions and 3 deletions

View File

@ -4,7 +4,12 @@ class ActivityPub::Activity::Block < ActivityPub::Activity
def perform
target_account = account_from_uri(object_uri)
return if target_account.nil? || !target_account.local? || @account.blocking?(target_account)
return if target_account.nil? || !target_account.local?
if @account.blocking?(target_account)
@account.block_relationships.find_by(target_account: target_account).update(uri: @json['id']) if @json['id'].present?
return
end
UnfollowService.new.call(target_account, @account) if target_account.following?(@account)

View File

@ -13,11 +13,62 @@ class ActivityPub::Activity::Undo < ActivityPub::Activity
undo_like
when 'Block'
undo_block
when nil
handle_reference
end
end
private
def handle_reference
# Some implementations do not inline the object, and as we don't have a
# global index, we have to guess what object it is.
return if object_uri.nil?
try_undo_announce || try_undo_accept || try_undo_follow || try_undo_like || try_undo_block || delete_later!(object_uri)
end
def try_undo_announce
status = Status.where.not(reblog_of_id: nil).find_by(uri: object_uri, account: @account)
if status.present?
RemoveStatusService.new.call(status)
true
else
false
end
end
def try_undo_accept
# We can't currently handle `Undo Accept` as we don't record `Accept`'s uri
false
end
def try_undo_follow
follow = @account.follow_requests.find_by(uri: object_uri) || @account.active_relationships.find_by(uri: object_uri)
if follow.present?
follow.destroy
true
else
false
end
end
def try_undo_like
# There is an index on accounts, but an account may have *many* favs, so this may be too costly
false
end
def try_undo_block
block = @account.block_relationships.find_by(uri: object_uri)
if block.present?
UnblockService.new.call(@account, block.target_account)
true
else
false
end
end
def undo_announce
return if object_uri.nil?

View File

@ -28,6 +28,28 @@ RSpec.describe ActivityPub::Activity::Block do
end
end
context 'when the recipient is already blocked' do
before do
sender.block!(recipient, uri: 'old')
end
describe '#perform' do
subject { described_class.new(json, sender) }
before do
subject.perform
end
it 'creates a block from sender to recipient' do
expect(sender.blocking?(recipient)).to be true
end
it 'sets the uri to that of last received block activity' do
expect(sender.block_relationships.find_by(target_account: recipient).uri).to eq 'foo'
end
end
end
context 'when the recipient follows the sender' do
before do
recipient.follow!(sender)

View File

@ -50,6 +50,19 @@ RSpec.describe ActivityPub::Activity::Undo do
expect(sender.reblogged?(status)).to be false
end
end
context 'with only object uri' do
let(:object_json) { 'bar' }
before do
Fabricate(:status, reblog: status, account: sender, uri: 'bar')
end
it 'deletes the reblog by uri' do
subject.perform
expect(sender.reblogged?(status)).to be false
end
end
end
context 'with Accept' do
@ -91,13 +104,22 @@ RSpec.describe ActivityPub::Activity::Undo do
end
before do
sender.block!(recipient)
sender.block!(recipient, uri: 'bar')
end
it 'deletes block from sender to recipient' do
subject.perform
expect(sender.blocking?(recipient)).to be false
end
context 'with only object uri' do
let(:object_json) { 'bar' }
it 'deletes block from sender to recipient' do
subject.perform
expect(sender.blocking?(recipient)).to be false
end
end
end
context 'with Follow' do
@ -113,13 +135,22 @@ RSpec.describe ActivityPub::Activity::Undo do
end
before do
sender.follow!(recipient)
sender.follow!(recipient, uri: 'bar')
end
it 'deletes follow from sender to recipient' do
subject.perform
expect(sender.following?(recipient)).to be false
end
context 'with only object uri' do
let(:object_json) { 'bar' }
it 'deletes follow from sender to recipient' do
subject.perform
expect(sender.following?(recipient)).to be false
end
end
end
context 'with Like' do