diff --git a/app/lib/activitypub/activity/accept.rb b/app/lib/activitypub/activity/accept.rb index dd4c8341ff..91610d68cf 100644 --- a/app/lib/activitypub/activity/accept.rb +++ b/app/lib/activitypub/activity/accept.rb @@ -58,7 +58,7 @@ class ActivityPub::Activity::Accept < ActivityPub::Activity def accept_quote!(quote) approval_uri = value_or_id(first_of_value(@json['result'])) - return if unsupported_uri_scheme?(approval_uri) || quote.quoted_account != @account || !quote.status.local? || !quote.pending? + return if unsupported_uri_scheme?(approval_uri) || non_matching_uri_hosts?(approval_uri, @account.uri) || quote.quoted_account != @account || !quote.status.local? || !quote.pending? # NOTE: we are not going through `ActivityPub::VerifyQuoteService` as the `Accept` is as authoritative # as the stamp, but this means we are not checking the stamp, which may lead to inconsistencies diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index 1b71c03b9e..aa0c7269d6 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -48,6 +48,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity @params = {} @quote = nil @quote_uri = nil + @quote_approval_uri = nil process_status_params process_tags @@ -229,9 +230,9 @@ class ActivityPub::Activity::Create < ActivityPub::Activity @quote_uri = @status_parser.quote_uri return unless @status_parser.quote? - approval_uri = @status_parser.quote_approval_uri - approval_uri = nil if unsupported_uri_scheme?(approval_uri) || TagManager.instance.local_url?(approval_uri) - @quote = Quote.new(account: @account, approval_uri: approval_uri, legacy: @status_parser.legacy_quote?, state: @status_parser.deleted_quote? ? :deleted : :pending) + @quote_approval_uri = @status_parser.quote_approval_uri + @quote_approval_uri = nil if unsupported_uri_scheme?(@quote_approval_uri) || TagManager.instance.local_url?(@quote_approval_uri) + @quote = Quote.new(account: @account, approval_uri: nil, legacy: @status_parser.legacy_quote?, state: @status_parser.deleted_quote? ? :deleted : :pending) end def process_hashtag(tag) @@ -391,9 +392,9 @@ class ActivityPub::Activity::Create < ActivityPub::Activity return if @quote.nil? embedded_quote = safe_prefetched_embed(@account, @status_parser.quoted_object, @json['context']) - ActivityPub::VerifyQuoteService.new.call(@quote, fetchable_quoted_uri: @quote_uri, prefetched_quoted_object: embedded_quote, request_id: @options[:request_id], depth: @options[:depth]) + ActivityPub::VerifyQuoteService.new.call(@quote, @quote_approval_uri, fetchable_quoted_uri: @quote_uri, prefetched_quoted_object: embedded_quote, request_id: @options[:request_id], depth: @options[:depth]) rescue Mastodon::RecursionLimitExceededError, Mastodon::UnexpectedResponseError, *Mastodon::HTTP_CONNECTION_ERRORS - ActivityPub::RefetchAndVerifyQuoteWorker.perform_in(rand(30..600).seconds, @quote.id, @quote_uri, { 'request_id' => @options[:request_id] }) + ActivityPub::RefetchAndVerifyQuoteWorker.perform_in(rand(30..600).seconds, @quote.id, @quote_uri, { 'request_id' => @options[:request_id], 'approval_uri' => @quote_approval_uri }) end def conversation_from_uri(uri) diff --git a/app/models/quote.rb b/app/models/quote.rb index e4f3b823fe..44c3599a7b 100644 --- a/app/models/quote.rb +++ b/app/models/quote.rb @@ -45,8 +45,12 @@ class Quote < ApplicationRecord after_destroy_commit :decrement_counter_caches! after_update_commit :update_counter_caches! - def accept! - update!(state: :accepted) + def accept!(approval_uri: nil) + if approval_uri.present? + update!(state: :accepted, approval_uri:) + else + update!(state: :accepted) + end reset_parent_cache! if attribute_previously_changed?(:state) end diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb index 2875686177..2279ff126c 100644 --- a/app/services/activitypub/process_status_update_service.rb +++ b/app/services/activitypub/process_status_update_service.rb @@ -52,7 +52,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService create_edits! end - fetch_and_verify_quote!(@quote, @status_parser.quote_uri) if @quote.present? + fetch_and_verify_quote!(@quote, @quote_approval_uri, @status_parser.quote_uri) if @quote.present? download_media_files! queue_poll_notifications! @@ -317,9 +317,9 @@ class ActivityPub::ProcessStatusUpdateService < BaseService approval_uri = @status_parser.quote_approval_uri approval_uri = nil if unsupported_uri_scheme?(approval_uri) || TagManager.instance.local_url?(approval_uri) - quote.update(approval_uri: approval_uri, state: :pending, legacy: @status_parser.legacy_quote?) if quote.approval_uri != @status_parser.quote_approval_uri + quote.update(approval_uri: nil, state: :pending, legacy: @status_parser.legacy_quote?) if quote.approval_uri.present? && quote.approval_uri != @status_parser.quote_approval_uri - fetch_and_verify_quote!(quote, quote_uri) + fetch_and_verify_quote!(quote, approval_uri, quote_uri) end def update_quote! @@ -335,18 +335,20 @@ class ActivityPub::ProcessStatusUpdateService < BaseService # Revoke the quote while we get a chance… maybe this should be a `before_destroy` hook? RevokeQuoteService.new.call(@status.quote) if @status.quote.quoted_account&.local? && @status.quote.accepted? @status.quote.destroy - quote = Quote.create(status: @status, approval_uri: approval_uri, legacy: @status_parser.legacy_quote?, state: @status_parser.deleted_quote? ? :deleted : :pending) + quote = Quote.create(status: @status, approval_uri: nil, legacy: @status_parser.legacy_quote?, state: @status_parser.deleted_quote? ? :deleted : :pending) @quote_changed = true else quote = @status.quote - quote.update(approval_uri: approval_uri, state: :pending, legacy: @status_parser.legacy_quote?) if quote.approval_uri != approval_uri + quote.update(approval_uri: nil, state: :pending, legacy: @status_parser.legacy_quote?) if quote.approval_uri.present? && quote.approval_uri != approval_uri end else - quote = Quote.create(status: @status, approval_uri: approval_uri, legacy: @status_parser.legacy_quote?) + quote = Quote.create(status: @status, approval_uri: nil, legacy: @status_parser.legacy_quote?) @quote_changed = true end @quote = quote + @quote_approval_uri = approval_uri + quote.save elsif @status.quote.present? @quote = nil @@ -355,11 +357,11 @@ class ActivityPub::ProcessStatusUpdateService < BaseService end end - def fetch_and_verify_quote!(quote, quote_uri) + def fetch_and_verify_quote!(quote, approval_uri, quote_uri) embedded_quote = safe_prefetched_embed(@account, @status_parser.quoted_object, @activity_json['context']) - ActivityPub::VerifyQuoteService.new.call(quote, fetchable_quoted_uri: quote_uri, prefetched_quoted_object: embedded_quote, request_id: @request_id) + ActivityPub::VerifyQuoteService.new.call(quote, approval_uri, fetchable_quoted_uri: quote_uri, prefetched_quoted_object: embedded_quote, request_id: @request_id) rescue Mastodon::UnexpectedResponseError, *Mastodon::HTTP_CONNECTION_ERRORS - ActivityPub::RefetchAndVerifyQuoteWorker.perform_in(rand(30..600).seconds, quote.id, quote_uri, { 'request_id' => @request_id }) + ActivityPub::RefetchAndVerifyQuoteWorker.perform_in(rand(30..600).seconds, quote.id, quote_uri, { 'request_id' => @request_id, 'approval_uri' => approval_uri }) end def update_counts! diff --git a/app/services/activitypub/verify_quote_service.rb b/app/services/activitypub/verify_quote_service.rb index 4dcf11cdfd..095e416c70 100644 --- a/app/services/activitypub/verify_quote_service.rb +++ b/app/services/activitypub/verify_quote_service.rb @@ -6,20 +6,21 @@ class ActivityPub::VerifyQuoteService < BaseService MAX_SYNCHRONOUS_DEPTH = 2 # Optionally fetch quoted post, and verify the quote is authorized - def call(quote, fetchable_quoted_uri: nil, prefetched_quoted_object: nil, prefetched_approval: nil, request_id: nil, depth: nil) + def call(quote, approval_uri, fetchable_quoted_uri: nil, prefetched_quoted_object: nil, prefetched_approval: nil, request_id: nil, depth: nil) @request_id = request_id @depth = depth || 0 @quote = quote + @approval_uri = approval_uri.presence || @quote.approval_uri @fetching_error = nil fetch_quoted_post_if_needed!(fetchable_quoted_uri, prefetched_body: prefetched_quoted_object) return if quote.quoted_account&.local? - return if fast_track_approval! || quote.approval_uri.blank? + return if fast_track_approval! || @approval_uri.blank? - @json = fetch_approval_object(quote.approval_uri, prefetched_body: prefetched_approval) + @json = fetch_approval_object(@approval_uri, prefetched_body: prefetched_approval) return quote.reject! if @json.nil? - return if non_matching_uri_hosts?(quote.approval_uri, value_or_id(@json['attributedTo'])) + return if non_matching_uri_hosts?(@approval_uri, value_or_id(@json['attributedTo'])) return unless matching_type? && matching_quote_uri? # Opportunistically import embedded posts if needed @@ -30,7 +31,7 @@ class ActivityPub::VerifyQuoteService < BaseService return unless matching_quoted_post? && matching_quoted_author? - quote.accept! + quote.accept!(approval_uri: @approval_uri) end private @@ -87,7 +88,7 @@ class ActivityPub::VerifyQuoteService < BaseService object = @json['interactionTarget'].merge({ '@context' => @json['@context'] }) # It's not safe to fetch if the inlined object is cross-origin or doesn't match expectations - return if object['id'] != uri || non_matching_uri_hosts?(@quote.approval_uri, object['id']) + return if object['id'] != uri || non_matching_uri_hosts?(@approval_uri, object['id']) status = ActivityPub::FetchRemoteStatusService.new.call(object['id'], prefetched_body: object, on_behalf_of: @quote.account.followers.local.first, request_id: @request_id, depth: @depth) diff --git a/app/workers/activitypub/quote_refresh_worker.rb b/app/workers/activitypub/quote_refresh_worker.rb index 7dabfddc80..5db84a1dac 100644 --- a/app/workers/activitypub/quote_refresh_worker.rb +++ b/app/workers/activitypub/quote_refresh_worker.rb @@ -10,6 +10,6 @@ class ActivityPub::QuoteRefreshWorker return if quote.nil? || quote.updated_at > Quote::BACKGROUND_REFRESH_INTERVAL.ago quote.touch - ActivityPub::VerifyQuoteService.new.call(quote) + ActivityPub::VerifyQuoteService.new.call(quote, quote.approval_uri) end end diff --git a/app/workers/activitypub/refetch_and_verify_quote_worker.rb b/app/workers/activitypub/refetch_and_verify_quote_worker.rb index 1a8dd40541..762d55e1be 100644 --- a/app/workers/activitypub/refetch_and_verify_quote_worker.rb +++ b/app/workers/activitypub/refetch_and_verify_quote_worker.rb @@ -9,7 +9,7 @@ class ActivityPub::RefetchAndVerifyQuoteWorker def perform(quote_id, quoted_uri, options = {}) quote = Quote.find(quote_id) - ActivityPub::VerifyQuoteService.new.call(quote, fetchable_quoted_uri: quoted_uri, request_id: options[:request_id]) + ActivityPub::VerifyQuoteService.new.call(quote, options['approval_uri'], fetchable_quoted_uri: quoted_uri, request_id: options['request_id']) ::DistributionWorker.perform_async(quote.status_id, { 'update' => true }) if quote.state_previously_changed? rescue ActiveRecord::RecordNotFound # Do nothing diff --git a/spec/services/activitypub/process_status_update_service_spec.rb b/spec/services/activitypub/process_status_update_service_spec.rb index dd24c3d8be..5df86eed54 100644 --- a/spec/services/activitypub/process_status_update_service_spec.rb +++ b/spec/services/activitypub/process_status_update_service_spec.rb @@ -940,10 +940,10 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService do stub_request(:get, approval_uri).to_return(headers: { 'Content-Type': 'application/activity+json' }, body: quote_authorization_json.to_json) end - it 'updates the approval URI but does not verify the quote' do + it 'does not update the approval URI and does not verify the quote' do expect { subject.call(status, json, json) } .to change(status, :quote).from(nil) - expect(status.quote.approval_uri).to eq approval_uri + expect(status.quote.approval_uri).to be_nil expect(status.quote.state).to_not eq 'accepted' expect(status.quote.quoted_status).to be_nil end diff --git a/spec/services/activitypub/verify_quote_service_spec.rb b/spec/services/activitypub/verify_quote_service_spec.rb index 9309eeb537..3ec00bad07 100644 --- a/spec/services/activitypub/verify_quote_service_spec.rb +++ b/spec/services/activitypub/verify_quote_service_spec.rb @@ -9,268 +9,284 @@ RSpec.describe ActivityPub::VerifyQuoteService do let(:quoted_account) { Fabricate(:account, domain: 'b.example.com') } let(:quoted_status) { Fabricate(:status, account: quoted_account) } let(:status) { Fabricate(:status, account: account) } - let(:quote) { Fabricate(:quote, status: status, quoted_status: quoted_status, approval_uri: approval_uri) } + let(:quote) { Fabricate(:quote, status: status, quoted_status: quoted_status, approval_uri: approval_uri_record) } - context 'with an unfetchable approval URI' do - let(:approval_uri) { 'https://b.example.com/approvals/1234' } + shared_examples 'common behavior' do + context 'with an unfetchable approval URI' do + let(:approval_uri) { 'https://b.example.com/approvals/1234' } - before do - stub_request(:get, approval_uri) - .to_return(status: 404) - end + before do + stub_request(:get, approval_uri) + .to_return(status: 404) + end - context 'with an already-fetched post' do - it 'does not update the status' do - expect { subject.call(quote) } - .to change(quote, :state).to('rejected') + context 'with an already-fetched post' do + it 'does not update the status' do + expect { subject.call(quote, approval_uri_arg) } + .to change(quote, :state).to('rejected') + end + end + + context 'with an already-verified quote' do + let(:quote) { Fabricate(:quote, status: status, quoted_status: quoted_status, approval_uri: approval_uri_record, state: :accepted) } + + it 'rejects the quote' do + expect { subject.call(quote, approval_uri_arg) } + .to change(quote, :state).to('revoked') + end end end - context 'with an already-verified quote' do - let(:quote) { Fabricate(:quote, status: status, quoted_status: quoted_status, approval_uri: approval_uri, state: :accepted) } + context 'with an approval URI' do + let(:approval_uri) { 'https://b.example.com/approvals/1234' } - it 'rejects the quote' do - expect { subject.call(quote) } - .to change(quote, :state).to('revoked') - end - end - end + let(:approval_type) { 'QuoteAuthorization' } + let(:approval_id) { approval_uri } + let(:approval_attributed_to) { ActivityPub::TagManager.instance.uri_for(quoted_account) } + let(:approval_interacting_object) { ActivityPub::TagManager.instance.uri_for(status) } + let(:approval_interaction_target) { ActivityPub::TagManager.instance.uri_for(quoted_status) } - context 'with an approval URI' do - let(:approval_uri) { 'https://b.example.com/approvals/1234' } - - let(:approval_type) { 'QuoteAuthorization' } - let(:approval_id) { approval_uri } - let(:approval_attributed_to) { ActivityPub::TagManager.instance.uri_for(quoted_account) } - let(:approval_interacting_object) { ActivityPub::TagManager.instance.uri_for(status) } - let(:approval_interaction_target) { ActivityPub::TagManager.instance.uri_for(quoted_status) } - - let(:json) do - { - '@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: approval_type, - id: approval_id, - attributedTo: approval_attributed_to, - interactingObject: approval_interacting_object, - interactionTarget: approval_interaction_target, - }.with_indifferent_access - end - - before do - stub_request(:get, approval_uri) - .to_return(status: 200, body: json.to_json, headers: { 'Content-Type': 'application/activity+json' }) - end - - context 'with a valid activity for already-fetched posts' do - it 'updates the status' do - expect { subject.call(quote) } - .to change(quote, :state).to('accepted') - - expect(a_request(:get, approval_uri)) - .to have_been_made.once - end - end - - context 'with a valid activity for a post that cannot be fetched but is passed as fetched_quoted_object' do - let(:quoted_status) { nil } - - let(:approval_interaction_target) { 'https://b.example.com/unknown-quoted' } - let(:prefetched_object) do + let(:json) do { - '@context': 'https://www.w3.org/ns/activitystreams', - type: 'Note', - id: 'https://b.example.com/unknown-quoted', - to: 'https://www.w3.org/ns/activitystreams#Public', - attributedTo: ActivityPub::TagManager.instance.uri_for(quoted_account), - content: 'previously unknown post', + '@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: approval_type, + id: approval_id, + attributedTo: approval_attributed_to, + interactingObject: approval_interacting_object, + interactionTarget: approval_interaction_target, }.with_indifferent_access end before do - stub_request(:get, 'https://b.example.com/unknown-quoted') - .to_return(status: 404) + stub_request(:get, approval_uri) + .to_return(status: 200, body: json.to_json, headers: { 'Content-Type': 'application/activity+json' }) end - it 'updates the status' do - expect { subject.call(quote, fetchable_quoted_uri: 'https://b.example.com/unknown-quoted', prefetched_quoted_object: prefetched_object) } - .to change(quote, :state).to('accepted') + context 'with a valid activity for already-fetched posts' do + it 'updates the status' do + expect { subject.call(quote, approval_uri_arg) } + .to change(quote, :state).to('accepted') - expect(a_request(:get, approval_uri)) - .to have_been_made.once + expect(a_request(:get, approval_uri)) + .to have_been_made.once + end + end - expect(quote.reload.quoted_status.content).to eq 'previously unknown post' + context 'with a valid activity for a post that cannot be fetched but is passed as fetched_quoted_object' do + let(:quoted_status) { nil } + + let(:approval_interaction_target) { 'https://b.example.com/unknown-quoted' } + let(:prefetched_object) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + type: 'Note', + id: 'https://b.example.com/unknown-quoted', + to: 'https://www.w3.org/ns/activitystreams#Public', + attributedTo: ActivityPub::TagManager.instance.uri_for(quoted_account), + content: 'previously unknown post', + }.with_indifferent_access + end + + before do + stub_request(:get, 'https://b.example.com/unknown-quoted') + .to_return(status: 404) + end + + it 'updates the status' do + expect { subject.call(quote, approval_uri_arg, fetchable_quoted_uri: 'https://b.example.com/unknown-quoted', prefetched_quoted_object: prefetched_object) } + .to change(quote, :state).to('accepted') + + expect(a_request(:get, approval_uri)) + .to have_been_made.once + + expect(quote.reload.quoted_status.content).to eq 'previously unknown post' + end + end + + context 'with a valid activity for a post that cannot be fetched but is inlined' do + let(:quoted_status) { nil } + + let(:approval_interaction_target) do + { + type: 'Note', + id: 'https://b.example.com/unknown-quoted', + to: 'https://www.w3.org/ns/activitystreams#Public', + attributedTo: ActivityPub::TagManager.instance.uri_for(quoted_account), + content: 'previously unknown post', + } + end + + before do + stub_request(:get, 'https://b.example.com/unknown-quoted') + .to_return(status: 404) + end + + it 'updates the status' do + expect { subject.call(quote, approval_uri_arg, fetchable_quoted_uri: 'https://b.example.com/unknown-quoted') } + .to change(quote, :state).to('accepted') + + expect(a_request(:get, approval_uri)) + .to have_been_made.once + + expect(quote.reload.quoted_status.content).to eq 'previously unknown post' + end + end + + context 'with a valid activity for a post that cannot be fetched and is inlined from an untrusted source' do + let(:quoted_status) { nil } + + let(:approval_interaction_target) do + { + type: 'Note', + id: 'https://example.com/unknown-quoted', + to: 'https://www.w3.org/ns/activitystreams#Public', + attributedTo: ActivityPub::TagManager.instance.uri_for(account), + content: 'previously unknown post', + } + end + + before do + stub_request(:get, 'https://example.com/unknown-quoted') + .to_return(status: 404) + end + + it 'does not update the status' do + expect { subject.call(quote, approval_uri_arg, fetchable_quoted_uri: 'https://example.com/unknown-quoted') } + .to not_change(quote, :state) + .and not_change(quote, :quoted_status) + + expect(a_request(:get, approval_uri)) + .to have_been_made.once + end + end + + context 'with a valid activity for already-fetched posts, with a pre-fetched approval' do + it 'updates the status without fetching the activity' do + expect { subject.call(quote, approval_uri_arg, prefetched_approval: JSON.generate(json)) } + .to change(quote, :state).to('accepted') + + expect(a_request(:get, approval_uri)) + .to_not have_been_made + end + end + + context 'with an unverifiable approval' do + let(:approval_uri) { 'https://evil.com/approvals/1234' } + + it 'does not update the status' do + expect { subject.call(quote, approval_uri_arg) } + .to_not change(quote, :state) + end + end + + context 'with an invalid approval document because of a mismatched ID' do + let(:approval_id) { 'https://evil.com/approvals/1234' } + + it 'does not accept the quote' do + # NOTE: maybe we want to skip that instead of rejecting it? + expect { subject.call(quote, approval_uri_arg) } + .to change(quote, :state).to('rejected') + end + end + + context 'with an approval from the wrong account' do + let(:approval_attributed_to) { ActivityPub::TagManager.instance.uri_for(Fabricate(:account, domain: 'b.example.com')) } + + it 'does not update the status' do + expect { subject.call(quote, approval_uri_arg) } + .to_not change(quote, :state) + end + end + + context 'with an approval for the wrong quoted post' do + let(:approval_interaction_target) { ActivityPub::TagManager.instance.uri_for(Fabricate(:status, account: quoted_account)) } + + it 'does not update the status' do + expect { subject.call(quote, approval_uri_arg) } + .to_not change(quote, :state) + end + end + + context 'with an approval for the wrong quote post' do + let(:approval_interacting_object) { ActivityPub::TagManager.instance.uri_for(Fabricate(:status, account: account)) } + + it 'does not update the status' do + expect { subject.call(quote, approval_uri_arg) } + .to_not change(quote, :state) + end + end + + context 'with an approval of the wrong type' do + let(:approval_type) { 'ReplyAuthorization' } + + it 'does not update the status' do + expect { subject.call(quote, approval_uri_arg) } + .to_not change(quote, :state) + end end end - context 'with a valid activity for a post that cannot be fetched but is inlined' do - let(:quoted_status) { nil } + context 'with fast-track authorizations' do + let(:approval_uri) { nil } - let(:approval_interaction_target) do - { - type: 'Note', - id: 'https://b.example.com/unknown-quoted', - to: 'https://www.w3.org/ns/activitystreams#Public', - attributedTo: ActivityPub::TagManager.instance.uri_for(quoted_account), - content: 'previously unknown post', - } + context 'without any fast-track condition' do + it 'does not update the status' do + expect { subject.call(quote, approval_uri_arg) } + .to_not change(quote, :state) + end end - before do - stub_request(:get, 'https://b.example.com/unknown-quoted') - .to_return(status: 404) + context 'when the account and the quoted account are the same' do + let(:quoted_account) { account } + + it 'updates the status' do + expect { subject.call(quote, approval_uri_arg) } + .to change(quote, :state).to('accepted') + end end - it 'updates the status' do - expect { subject.call(quote, fetchable_quoted_uri: 'https://b.example.com/unknown-quoted') } - .to change(quote, :state).to('accepted') + context 'when the account is mentioned by the quoted post' do + before do + quoted_status.mentions << Mention.new(account: account) + end - expect(a_request(:get, approval_uri)) - .to have_been_made.once - - expect(quote.reload.quoted_status.content).to eq 'previously unknown post' - end - end - - context 'with a valid activity for a post that cannot be fetched and is inlined from an untrusted source' do - let(:quoted_status) { nil } - - let(:approval_interaction_target) do - { - type: 'Note', - id: 'https://example.com/unknown-quoted', - to: 'https://www.w3.org/ns/activitystreams#Public', - attributedTo: ActivityPub::TagManager.instance.uri_for(account), - content: 'previously unknown post', - } - end - - before do - stub_request(:get, 'https://example.com/unknown-quoted') - .to_return(status: 404) - end - - it 'does not update the status' do - expect { subject.call(quote, fetchable_quoted_uri: 'https://example.com/unknown-quoted') } - .to not_change(quote, :state) - .and not_change(quote, :quoted_status) - - expect(a_request(:get, approval_uri)) - .to have_been_made.once - end - end - - context 'with a valid activity for already-fetched posts, with a pre-fetched approval' do - it 'updates the status without fetching the activity' do - expect { subject.call(quote, prefetched_approval: json.to_json) } - .to change(quote, :state).to('accepted') - - expect(a_request(:get, approval_uri)) - .to_not have_been_made - end - end - - context 'with an unverifiable approval' do - let(:approval_uri) { 'https://evil.com/approvals/1234' } - - it 'does not update the status' do - expect { subject.call(quote) } - .to_not change(quote, :state) - end - end - - context 'with an invalid approval document because of a mismatched ID' do - let(:approval_id) { 'https://evil.com/approvals/1234' } - - it 'does not accept the quote' do - # NOTE: maybe we want to skip that instead of rejecting it? - expect { subject.call(quote) } - .to change(quote, :state).to('rejected') - end - end - - context 'with an approval from the wrong account' do - let(:approval_attributed_to) { ActivityPub::TagManager.instance.uri_for(Fabricate(:account, domain: 'b.example.com')) } - - it 'does not update the status' do - expect { subject.call(quote) } - .to_not change(quote, :state) - end - end - - context 'with an approval for the wrong quoted post' do - let(:approval_interaction_target) { ActivityPub::TagManager.instance.uri_for(Fabricate(:status, account: quoted_account)) } - - it 'does not update the status' do - expect { subject.call(quote) } - .to_not change(quote, :state) - end - end - - context 'with an approval for the wrong quote post' do - let(:approval_interacting_object) { ActivityPub::TagManager.instance.uri_for(Fabricate(:status, account: account)) } - - it 'does not update the status' do - expect { subject.call(quote) } - .to_not change(quote, :state) - end - end - - context 'with an approval of the wrong type' do - let(:approval_type) { 'ReplyAuthorization' } - - it 'does not update the status' do - expect { subject.call(quote) } - .to_not change(quote, :state) + it 'does not update the status' do + expect { subject.call(quote, approval_uri_arg) } + .to_not change(quote, :state).from('pending') + end end end end - context 'with fast-track authorizations' do - let(:approval_uri) { nil } + context 'when approval URI is passed as argument' do + let(:approval_uri_arg) { approval_uri } + let(:approval_uri_record) { nil } - context 'without any fast-track condition' do - it 'does not update the status' do - expect { subject.call(quote) } - .to_not change(quote, :state) - end - end + it_behaves_like 'common behavior' + end - context 'when the account and the quoted account are the same' do - let(:quoted_account) { account } + context 'when approval URI is stored in the record (legacy)' do + let(:approval_uri_arg) { nil } + let(:approval_uri_record) { approval_uri } - it 'updates the status' do - expect { subject.call(quote) } - .to change(quote, :state).to('accepted') - end - end - - context 'when the account is mentioned by the quoted post' do - before do - quoted_status.mentions << Mention.new(account: account) - end - - it 'does not the status' do - expect { subject.call(quote) } - .to_not change(quote, :state).from('pending') - end - end + it_behaves_like 'common behavior' end end diff --git a/spec/workers/activitypub/quote_refresh_worker_spec.rb b/spec/workers/activitypub/quote_refresh_worker_spec.rb index bcdcc0b746..dbb72a09df 100644 --- a/spec/workers/activitypub/quote_refresh_worker_spec.rb +++ b/spec/workers/activitypub/quote_refresh_worker_spec.rb @@ -20,7 +20,7 @@ RSpec.describe ActivityPub::QuoteRefreshWorker do expect { worker.perform(quote.id) } .to(change { quote.reload.updated_at }) - expect(service).to have_received(:call).with(quote) + expect(service).to have_received(:call).with(quote, quote.approval_uri) end end @@ -31,7 +31,7 @@ RSpec.describe ActivityPub::QuoteRefreshWorker do expect { worker.perform(quote.id) } .to_not(change { quote.reload.updated_at }) - expect(service).to_not have_received(:call).with(quote) + expect(service).to_not have_received(:call).with(quote, quote.approval_uri) end end end diff --git a/spec/workers/activitypub/refetch_and_verify_quote_worker_spec.rb b/spec/workers/activitypub/refetch_and_verify_quote_worker_spec.rb index a925709885..3b3ed29489 100644 --- a/spec/workers/activitypub/refetch_and_verify_quote_worker_spec.rb +++ b/spec/workers/activitypub/refetch_and_verify_quote_worker_spec.rb @@ -13,11 +13,20 @@ RSpec.describe ActivityPub::RefetchAndVerifyQuoteWorker do let(:status) { Fabricate(:status, account: account) } let(:quote) { Fabricate(:quote, status: status, quoted_status: nil) } let(:url) { 'https://example.com/quoted-status' } + let(:approval_uri) { 'https://example.com/approval-uri' } it 'sends the status to the service' do - worker.perform(quote.id, url) + worker.perform(quote.id, url, { 'approval_uri' => approval_uri }) - expect(service).to have_received(:call).with(quote, fetchable_quoted_uri: url, request_id: anything) + expect(service).to have_received(:call).with(quote, approval_uri, fetchable_quoted_uri: url, request_id: anything) + end + + context 'with the old format' do + it 'sends the status to the service' do + worker.perform(quote.id, url) + + expect(service).to have_received(:call).with(quote, nil, fetchable_quoted_uri: url, request_id: anything) + end end it 'returns nil for non-existent record' do