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.

diff --git a/app/lib/activitypub/activity/block.rb b/app/lib/activitypub/activity/block.rb
index a17a2d50a6af0a738b2c36f41bae3afa5dcf309a..90477bf3369dca5416f54bdf813fe3e40400bce3 100644 (file)
--- a/app/lib/activitypub/activity/block.rb
+++ b/app/lib/activitypub/activity/block.rb
@@ -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)
 

diff --git a/app/lib/activitypub/activity/undo.rb b/app/lib/activitypub/activity/undo.rb
index 599823c6e25db8ffac3ed5c1a513e92ef40bf7a8..9eff1b71c9db366289b3399d024ee256a33d750d 100644 (file)
--- a/app/lib/activitypub/activity/undo.rb
+++ b/app/lib/activitypub/activity/undo.rb
@@ -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?
 

diff --git a/spec/lib/activitypub/activity/block_spec.rb b/spec/lib/activitypub/activity/block_spec.rb
index 94d37356dd14c23a98436a65f00161689d8d6554..42bdfdc810ef6ff23b774d246a3eed75122191d3 100644 (file)
--- a/spec/lib/activitypub/activity/block_spec.rb
+++ b/spec/lib/activitypub/activity/block_spec.rb
@@ -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)

diff --git a/spec/lib/activitypub/activity/undo_spec.rb b/spec/lib/activitypub/activity/undo_spec.rb
index 9545e1f46af6b98ba924ad53b5e84f2e2cc7ebdb..c0309e49dafcee0f6f523ec14957f4815d7b1f67 100644 (file)
--- a/spec/lib/activitypub/activity/undo_spec.rb
+++ b/spec/lib/activitypub/activity/undo_spec.rb
@@ -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