From f5ba9793175293a31231391a79ee544b350ae978 Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 23 May 2025 17:01:07 +0200 Subject: [PATCH] Fix handling of inlined `featured` collections in ActivityPub actor objects (#34789) --- .../fetch_featured_collection_service.rb | 6 +- .../activitypub/process_account_service.rb | 6 +- .../synchronize_featured_collection_worker.rb | 2 +- .../fetch_featured_collection_service_spec.rb | 55 ++++++++++++++++++- 4 files changed, 58 insertions(+), 11 deletions(-) diff --git a/app/services/activitypub/fetch_featured_collection_service.rb b/app/services/activitypub/fetch_featured_collection_service.rb index 89c3a1b6c0..267c0b4619 100644 --- a/app/services/activitypub/fetch_featured_collection_service.rb +++ b/app/services/activitypub/fetch_featured_collection_service.rb @@ -4,13 +4,11 @@ class ActivityPub::FetchFeaturedCollectionService < BaseService include JsonLdHelper def call(account, **options) - return if account.featured_collection_url.blank? || account.suspended? || account.local? + return if (account.featured_collection_url.blank? && options[:collection].blank?) || account.suspended? || account.local? @account = account @options = options - @json = fetch_resource(@account.featured_collection_url, true, local_follower) - - return unless supported_context?(@json) + @json = fetch_collection(options[:collection].presence || @account.featured_collection_url) process_items(collection_items(@json)) end diff --git a/app/services/activitypub/process_account_service.rb b/app/services/activitypub/process_account_service.rb index 8710424669..45f6bc3616 100644 --- a/app/services/activitypub/process_account_service.rb +++ b/app/services/activitypub/process_account_service.rb @@ -57,7 +57,7 @@ class ActivityPub::ProcessAccountService < BaseService after_suspension_change! if suspension_changed? unless @options[:only_key] || @account.suspended? - check_featured_collection! if @account.featured_collection_url.present? + check_featured_collection! if @json['featured'].present? check_featured_tags_collection! if @json['featuredTags'].present? check_links! if @account.fields.any?(&:requires_verification?) end @@ -121,7 +121,7 @@ class ActivityPub::ProcessAccountService < BaseService end def set_immediate_attributes! - @account.featured_collection_url = @json['featured'] || '' + @account.featured_collection_url = valid_collection_uri(@json['featured']) @account.devices_url = @json['devices'] || '' @account.display_name = @json['name'] || '' @account.note = @json['summary'] || '' @@ -186,7 +186,7 @@ class ActivityPub::ProcessAccountService < BaseService end def check_featured_collection! - ActivityPub::SynchronizeFeaturedCollectionWorker.perform_async(@account.id, { 'hashtag' => @json['featuredTags'].blank?, 'request_id' => @options[:request_id] }) + ActivityPub::SynchronizeFeaturedCollectionWorker.perform_async(@account.id, { 'hashtag' => @json['featuredTags'].blank?, 'collection' => @json['featured'], 'request_id' => @options[:request_id] }) end def check_featured_tags_collection! diff --git a/app/workers/activitypub/synchronize_featured_collection_worker.rb b/app/workers/activitypub/synchronize_featured_collection_worker.rb index 7a187d7f53..f2643d6960 100644 --- a/app/workers/activitypub/synchronize_featured_collection_worker.rb +++ b/app/workers/activitypub/synchronize_featured_collection_worker.rb @@ -6,7 +6,7 @@ class ActivityPub::SynchronizeFeaturedCollectionWorker sidekiq_options queue: 'pull', lock: :until_executed, lock_ttl: 1.day.to_i def perform(account_id, options = {}) - options = { note: true, hashtag: false }.deep_merge(options.deep_symbolize_keys) + options = { note: true, hashtag: false }.deep_merge(options.symbolize_keys) ActivityPub::FetchFeaturedCollectionService.new.call(Account.find(account_id), **options) rescue ActiveRecord::RecordNotFound diff --git a/spec/services/activitypub/fetch_featured_collection_service_spec.rb b/spec/services/activitypub/fetch_featured_collection_service_spec.rb index 237fc7123e..42d2aef400 100644 --- a/spec/services/activitypub/fetch_featured_collection_service_spec.rb +++ b/spec/services/activitypub/fetch_featured_collection_service_spec.rb @@ -67,7 +67,7 @@ RSpec.describe ActivityPub::FetchFeaturedCollectionService, type: :service do type: 'Collection', id: actor.featured_collection_url, items: items, - }.with_indifferent_access + }.deep_stringify_keys end shared_examples 'sets pinned posts' do @@ -91,6 +91,55 @@ RSpec.describe ActivityPub::FetchFeaturedCollectionService, type: :service do end describe '#call' do + subject { described_class.new.call(actor, note: true, hashtag: false) } + + shared_examples 'sets pinned posts' do + before do + stub_request(:get, 'https://example.com/account/pinned/known').to_return(status: 200, body: Oj.dump(status_json_pinned_known), headers: { 'Content-Type': 'application/activity+json' }) + stub_request(:get, 'https://example.com/account/pinned/unknown-inlined').to_return(status: 200, body: Oj.dump(status_json_pinned_unknown_inlined), headers: { 'Content-Type': 'application/activity+json' }) + stub_request(:get, 'https://example.com/account/pinned/unknown-unreachable').to_return(status: 404) + stub_request(:get, 'https://example.com/account/pinned/unknown-reachable').to_return(status: 200, body: Oj.dump(status_json_pinned_unknown_reachable), headers: { 'Content-Type': 'application/activity+json' }) + stub_request(:get, 'https://example.com/account/collections/featured').to_return(status: 200, body: Oj.dump(featured_with_null), headers: { 'Content-Type': 'application/activity+json' }) + + subject + end + + it 'sets expected posts as pinned posts' do + expect(actor.pinned_statuses.pluck(:uri)).to contain_exactly( + 'https://example.com/account/pinned/known', + 'https://example.com/account/pinned/unknown-inlined', + 'https://example.com/account/pinned/unknown-reachable' + ) + expect(actor.pinned_statuses).to_not include(known_status) + end + end + + context 'when passing the collection via an argument' do + subject { described_class.new.call(actor, note: true, hashtag: false, collection: collection_or_uri) } + + context 'when the collection is an URL' do + let(:collection_or_uri) { actor.featured_collection_url } + + before do + stub_request(:get, actor.featured_collection_url).to_return(status: 200, body: Oj.dump(payload), headers: { 'Content-Type': 'application/activity+json' }) + end + + it_behaves_like 'sets pinned posts' + end + + context 'when the collection is inlined' do + let(:collection_or_uri) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + type: 'Collection', + items: items, + }.deep_stringify_keys + end + + it_behaves_like 'sets pinned posts' + end + end + context 'when the endpoint is a Collection' do before do stub_request(:get, actor.featured_collection_url).to_return(status: 200, body: Oj.dump(payload), headers: { 'Content-Type': 'application/activity+json' }) @@ -120,7 +169,7 @@ RSpec.describe ActivityPub::FetchFeaturedCollectionService, type: :service do before do stub_request(:get, 'https://example.com/account/pinned/unknown-reachable').to_return(status: 200, body: Oj.dump(status_json_pinned_unknown_reachable), headers: { 'Content-Type': 'application/activity+json' }) - subject.call(actor, note: true, hashtag: false) + subject end it 'sets expected posts as pinned posts' do @@ -156,7 +205,7 @@ RSpec.describe ActivityPub::FetchFeaturedCollectionService, type: :service do before do stub_request(:get, 'https://example.com/account/pinned/unknown-reachable').to_return(status: 200, body: Oj.dump(status_json_pinned_unknown_reachable), headers: { 'Content-Type': 'application/activity+json' }) - subject.call(actor, note: true, hashtag: false) + subject end it 'sets expected posts as pinned posts' do