Fix handling of inlined `instrument` in incoming `QuoteRequest` (#35714)

app/lib/activitypub/activity/quote_request.rb

@@ -19,13 +19,13 @@ class ActivityPub::Activity::QuoteRequest < ActivityPub::Activity
   private
 
   def accept_quote_request!(quoted_status)
-    status = status_from_uri(@json['instrument'])
-    # TODO: import inlined quote post if possible
-    status ||= ActivityPub::FetchRemoteStatusService.new.call(@json['instrument'], on_behalf_of: quoted_status.account, request_id: @options[:request_id])
+    status = status_from_uri(instrument_uri)
+    status ||= import_instrument(quoted_status)
+    status ||= ActivityPub::FetchRemoteStatusService.new.call(instrument_uri, on_behalf_of: quoted_status.account, request_id: @options[:request_id])
     # TODO: raise if status is nil
 
     # Sanity check
-    return unless status.quote.quoted_status == quoted_status
+    return unless status.quote.quoted_status == quoted_status && status.account == @account
 
     status.quote.ensure_quoted_access
     status.quote.update!(activity_uri: @json['id'])
@@ -35,15 +35,30 @@ class ActivityPub::Activity::QuoteRequest < ActivityPub::Activity
     ActivityPub::DeliveryWorker.perform_async(json, quoted_status.account_id, @account.inbox_url)
   end
 
+  def import_instrument(quoted_status)
+    return unless @json['instrument'].is_a?(Hash)
+
+    # NOTE: Replacing the object's context by that of the parent activity is
+    # not sound, but it's consistent with the rest of the codebase
+    instrument = @json['instrument'].merge({ '@context' => @json['@context'] })
+    return if non_matching_uri_hosts?(instrument['id'], @account.uri)
+
+    ActivityPub::FetchRemoteStatusService.new.call(instrument['id'], prefetched_body: instrument, on_behalf_of: quoted_status.account, request_id: @options[:request_id])
+  end
+
   def reject_quote_request!(quoted_status)
     quote = Quote.new(
       quoted_status: quoted_status,
       quoted_account: quoted_status.account,
-      status: Status.new(account: @account, uri: @json['instrument']),
+      status: Status.new(account: @account, uri: instrument_uri),
       account: @account,
       activity_uri: @json['id']
     )
     json = Oj.dump(serialize_payload(quote, ActivityPub::RejectQuoteRequestSerializer))
     ActivityPub::DeliveryWorker.perform_async(json, quoted_status.account_id, @account.inbox_url)
   end
+
+  def instrument_uri
+    value_or_id(@json['instrument'])
+  end
 end

spec/lib/activitypub/activity/quote_request_spec.rb

@@ -8,6 +8,7 @@ RSpec.describe ActivityPub::Activity::QuoteRequest, feature: :outgoing_quotes do
   let(:quoted_post) { Fabricate(:status, account: recipient) }
   let(:request_uri) { 'https://example.com/missing-ui' }
   let(:quoted_uri) { ActivityPub::TagManager.instance.uri_for(quoted_post) }
+  let(:instrument) { 'https://example.com/unknown-status' }
 
   let(:json) do
     {
@@ -21,8 +22,30 @@ RSpec.describe ActivityPub::Activity::QuoteRequest, feature: :outgoing_quotes do
       type: 'QuoteRequest',
       actor: ActivityPub::TagManager.instance.uri_for(sender),
       object: quoted_uri,
-      instrument: 'https://example.com/unknown-status',
-    }.with_indifferent_access
+      instrument: instrument,
+    }.deep_stringify_keys
+  end
+
+  let(:status_json) do
+    {
+      '@context': [
+        'https://www.w3.org/ns/activitystreams',
+        {
+          '@id': 'https://w3id.org/fep/044f#quote',
+          '@type': '@id',
+        },
+        {
+          '@id': 'https://w3id.org/fep/044f#quoteAuthorization',
+          '@type': '@id',
+        },
+      ],
+      id: 'https://example.com/unknown-status',
+      type: 'Note',
+      summary: 'Show more',
+      content: 'Hello universe',
+      quote: ActivityPub::TagManager.instance.uri_for(quoted_post),
+      attributedTo: ActivityPub::TagManager.instance.uri_for(sender),
+    }.deep_stringify_keys
   end
 
   describe '#perform' do
@@ -48,29 +71,20 @@ RSpec.describe ActivityPub::Activity::QuoteRequest, feature: :outgoing_quotes do
       end
     end
 
-    context 'when trying to quote a quotable local status' do
-      let(:status_json) do
-        {
-          '@context': [
-            'https://www.w3.org/ns/activitystreams',
-            {
-              '@id': 'https://w3id.org/fep/044f#quote',
-              '@type': '@id',
-            },
-            {
-              '@id': 'https://w3id.org/fep/044f#quoteAuthorization',
-              '@type': '@id',
-            },
-          ],
-          id: 'https://example.com/unknown-status',
-          type: 'Note',
-          summary: 'Show more',
-          content: 'Hello universe',
-          quote: ActivityPub::TagManager.instance.uri_for(quoted_post),
-          attributedTo: ActivityPub::TagManager.instance.uri_for(sender),
-        }.deep_stringify_keys
+    context 'when trying to quote an unquotable local status with an inlined instrument' do
+      let(:instrument) { status_json.without('@context') }
+
+      it 'sends a Reject activity' do
+        expect { subject.perform }
+          .to enqueue_sidekiq_job(ActivityPub::DeliveryWorker)
+          .with(satisfying do |body|
+            outgoing_json = Oj.load(body)
+            outgoing_json['type'] == 'Reject' && json['instrument']['id'] == outgoing_json['object']['instrument'] && %w(type id actor object).all? { |key| json[key] == outgoing_json['object'][key] }
+          end, recipient.id, sender.inbox_url)
       end
+    end
 
+    context 'when trying to quote a quotable local status' do
       before do
         stub_request(:get, 'https://example.com/unknown-status').to_return(status: 200, body: Oj.dump(status_json), headers: { 'Content-Type': 'application/activity+json' })
         quoted_post.update(quote_approval_policy: Status::QUOTE_APPROVAL_POLICY_FLAGS[:public] << 16)
@@ -86,5 +100,23 @@ RSpec.describe ActivityPub::Activity::QuoteRequest, feature: :outgoing_quotes do
           end, recipient.id, sender.inbox_url)
       end
     end
+
+    context 'when trying to quote a quotable local status with an inlined instrument' do
+      let(:instrument) { status_json.without('@context') }
+
+      before do
+        quoted_post.update(quote_approval_policy: Status::QUOTE_APPROVAL_POLICY_FLAGS[:public] << 16)
+      end
+
+      it 'accepts the quote and sends an Accept activity' do
+        expect { subject.perform }
+          .to change { quoted_post.reload.quotes.accepted.count }.by(1)
+          .and enqueue_sidekiq_job(ActivityPub::DeliveryWorker)
+          .with(satisfying do |body|
+            outgoing_json = Oj.load(body)
+            outgoing_json['type'] == 'Accept' && json['instrument']['id'] == outgoing_json['object']['instrument'] && %w(type id actor object).all? { |key| json[key] == outgoing_json['object'][key] }
+          end, recipient.id, sender.inbox_url)
+      end
+    end
   end
 end