Fix support for quote verification in implicit status updates (#35384)

app/services/activitypub/process_status_update_service.rb

@@ -66,6 +66,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService
       update_interaction_policies!
       update_poll!(allow_significant_changes: false)
       queue_poll_notifications!
+      update_quote_approval!
       update_counts!
     end
   end
@@ -270,6 +271,23 @@ class ActivityPub::ProcessStatusUpdateService < BaseService
     end
   end
 
+  # This method is only concerned with approval and skips other meaningful changes,
+  # as it is used instead of `update_quote!` in implicit updates
+  def update_quote_approval!
+    quote_uri = @status_parser.quote_uri
+    return unless quote_uri.present? && @status.quote.present?
+
+    quote = @status.quote
+    return if quote.quoted_status.present? && ActivityPub::TagManager.instance.uri_for(quote.quoted_status) != quote_uri
+
+    approval_uri = @status_parser.quote_approval_uri
+    approval_uri = nil if unsupported_uri_scheme?(approval_uri)
+
+    quote.update(approval_uri: approval_uri, state: :pending, legacy: @status_parser.legacy_quote?) if quote.approval_uri != @status_parser.quote_approval_uri
+
+    fetch_and_verify_quote!(quote, quote_uri)
+  end
+
   def update_quote!
     quote_uri = @status_parser.quote_uri
 

spec/services/activitypub/process_status_update_service_spec.rb

@@ -435,7 +435,71 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService do
     end
   end
 
-  context 'when the status has an existing unverified quote and adds an approval link' do
+  context 'when the status has an existing unverified quote and adds an approval link through an implicit update' do
+    let(:quoted_account) { Fabricate(:account, domain: 'quoted.example.com') }
+    let(:quoted_status) { Fabricate(:status, account: quoted_account) }
+    let!(:quote) { Fabricate(:quote, status: status, quoted_status: quoted_status, approval_uri: nil) }
+    let(:approval_uri) { 'https://quoted.example.com/approvals/1' }
+
+    let(:payload) 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: 'foo',
+        type: 'Note',
+        summary: 'Show more',
+        content: 'Hello universe',
+        quote: ActivityPub::TagManager.instance.uri_for(quoted_status),
+        quoteAuthorization: approval_uri,
+      }
+    end
+
+    before do
+      stub_request(:get, approval_uri).to_return(headers: { 'Content-Type': 'application/activity+json' }, body: Oj.dump({
+        '@context': [
+          'https://www.w3.org/ns/activitystreams',
+          {
+            QuoteAuthorization: 'https://w3id.org/fep/044f#QuoteAuthorization',
+            gts: 'https://gotosocial.org/ns#',
+            interactionPolicy: {
+              '@id': 'gts:interactionPolicy',
+              '@type': '@id',
+            },
+            interactingObject: {
+              '@id': 'gts:interactingObject',
+              '@type': '@id',
+            },
+            interactionTarget: {
+              '@id': 'gts:interactionTarget',
+              '@type': '@id',
+            },
+          },
+        ],
+        type: 'QuoteAuthorization',
+        id: approval_uri,
+        attributedTo: ActivityPub::TagManager.instance.uri_for(quoted_status.account),
+        interactingObject: ActivityPub::TagManager.instance.uri_for(status),
+        interactionTarget: ActivityPub::TagManager.instance.uri_for(quoted_status),
+      }))
+    end
+
+    it 'updates the approval URI and verifies the quote' do
+      expect { subject.call(status, json, json) }
+        .to change(quote, :approval_uri).to(approval_uri)
+        .and change(quote, :state).to('accepted')
+    end
+  end
+
+  context 'when the status has an existing unverified quote and adds an approval link through an explicit update' do
     let(:quoted_account) { Fabricate(:account, domain: 'quoted.example.com') }
     let(:quoted_status) { Fabricate(:status, account: quoted_account) }
     let!(:quote) { Fabricate(:quote, status: status, quoted_status: quoted_status, approval_uri: nil) }
@@ -500,7 +564,7 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService do
     end
   end
 
-  context 'when the status has an existing verified quote and removes an approval link' do
+  context 'when the status has an existing verified quote and removes an approval link through an explicit update' do
     let(:quoted_account) { Fabricate(:account, domain: 'quoted.example.com') }
     let(:quoted_status) { Fabricate(:status, account: quoted_account) }
     let!(:quote) { Fabricate(:quote, status: status, quoted_status: quoted_status, approval_uri: approval_uri, state: :accepted) }
@@ -535,7 +599,7 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService do
     end
   end
 
-  context 'when the status adds a verifiable quote' do
+  context 'when the status adds a verifiable quote through an explicit update' do
     let(:quoted_account) { Fabricate(:account, domain: 'quoted.example.com') }
     let(:quoted_status) { Fabricate(:status, account: quoted_account) }
     let(:approval_uri) { 'https://quoted.example.com/approvals/1' }
@@ -600,7 +664,39 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService do
     end
   end
 
-  context 'when the status adds a unverifiable quote' do
+  context 'when the status adds a unverifiable quote through an implicit update' do
+    let(:quoted_account) { Fabricate(:account, domain: 'quoted.example.com') }
+    let(:quoted_status) { Fabricate(:status, account: quoted_account) }
+    let(:approval_uri) { 'https://quoted.example.com/approvals/1' }
+
+    let(:payload) 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: 'foo',
+        type: 'Note',
+        summary: 'Show more',
+        content: 'Hello universe',
+        quote: ActivityPub::TagManager.instance.uri_for(quoted_status),
+      }
+    end
+
+    it 'does not add the quote' do
+      expect { subject.call(status, json, json) }
+        .to not_change(status, :quote).from(nil)
+    end
+  end
+
+  context 'when the status adds a unverifiable quote through an explicit update' do
     let(:quoted_account) { Fabricate(:account, domain: 'quoted.example.com') }
     let(:quoted_status) { Fabricate(:status, account: quoted_account) }
     let(:approval_uri) { 'https://quoted.example.com/approvals/1' }
@@ -635,7 +731,29 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService do
     end
   end
 
-  context 'when the status removes a verified quote' do
+  context 'when the status removes a verified quote through an implicit update' do
+    let(:quoted_account) { Fabricate(:account, domain: 'quoted.example.com') }
+    let(:quoted_status) { Fabricate(:status, account: quoted_account) }
+    let!(:quote) { Fabricate(:quote, status: status, quoted_status: quoted_status, approval_uri: approval_uri, state: :accepted) }
+    let(:approval_uri) { 'https://quoted.example.com/approvals/1' }
+
+    let(:payload) do
+      {
+        '@context': 'https://www.w3.org/ns/activitystreams',
+        id: 'foo',
+        type: 'Note',
+        summary: 'Show more',
+        content: 'Hello universe',
+      }
+    end
+
+    it 'does not remove the quote' do
+      expect { subject.call(status, json, json) }
+        .to not_change { status.reload.quote }.from(quote)
+    end
+  end
+
+  context 'when the status removes a verified quote through an explicit update' do
     let(:quoted_account) { Fabricate(:account, domain: 'quoted.example.com') }
     let(:quoted_status) { Fabricate(:status, account: quoted_account) }
     let!(:quote) { Fabricate(:quote, status: status, quoted_status: quoted_status, approval_uri: approval_uri, state: :accepted) }
@@ -660,7 +778,7 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService do
     end
   end
 
-  context 'when the status removes an unverified quote' do
+  context 'when the status removes an unverified quote through an explicit update' do
     let(:quoted_account) { Fabricate(:account, domain: 'quoted.example.com') }
     let(:quoted_status) { Fabricate(:status, account: quoted_account) }
     let!(:quote) { Fabricate(:quote, status: status, quoted_status: quoted_status, approval_uri: nil, state: :pending) }
@@ -684,7 +802,44 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService do
     end
   end
 
-  context 'when the status swaps a verified quote with an unverifiable quote' do
+  context 'when the status swaps a verified quote with an unverifiable quote through an implicit update' do
+    let(:quoted_account) { Fabricate(:account, domain: 'quoted.example.com') }
+    let(:quoted_status) { Fabricate(:status, account: quoted_account) }
+    let(:second_quoted_status) { Fabricate(:status, account: quoted_account) }
+    let!(:quote) { Fabricate(:quote, status: status, quoted_status: quoted_status, approval_uri: approval_uri, state: :accepted) }
+    let(:approval_uri) { 'https://quoted.example.com/approvals/1' }
+
+    let(:payload) 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: 'foo',
+        type: 'Note',
+        summary: 'Show more',
+        content: 'Hello universe',
+        quote: ActivityPub::TagManager.instance.uri_for(second_quoted_status),
+        quoteAuthorization: approval_uri,
+      }
+    end
+
+    it 'does not update the URI or the quote verification status' do
+      expect { subject.call(status, json, json) }
+        .to not_change { status.reload.quote }.from(quote)
+        .and not_change { status.quote.quoted_status }.from(quoted_status)
+        .and not_change { status.quote.state }.from('accepted')
+    end
+  end
+
+  context 'when the status swaps a verified quote with an unverifiable quote through an explicit update' do
     let(:quoted_account) { Fabricate(:account, domain: 'quoted.example.com') }
     let(:quoted_status) { Fabricate(:status, account: quoted_account) }
     let(:second_quoted_status) { Fabricate(:status, account: quoted_account) }
@@ -752,7 +907,7 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService do
     end
   end
 
-  context 'when the status swaps a verified quote with another verifiable quote' do
+  context 'when the status swaps a verified quote with another verifiable quote through an explicit update' do
     let(:quoted_account) { Fabricate(:account, domain: 'quoted.example.com') }
     let(:second_quoted_account) { Fabricate(:account, domain: 'second-quoted.example.com') }
     let(:quoted_status) { Fabricate(:status, account: quoted_account) }