From 575dc620559a56a5586008066f85544739e365ca Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Tue, 24 Oct 2023 12:27:56 +0800 Subject: [PATCH 1/4] Support updates from other actors Some platforms, e.g. Discourse, allow Update activities on Objects from actors other than those the Object is attributedTo. --- app/lib/activitypub/activity.rb | 18 ++++--- app/lib/activitypub/activity/update.rb | 3 +- spec/lib/activitypub/activity/update_spec.rb | 53 ++++++++++++++++++++ 3 files changed, 66 insertions(+), 8 deletions(-) diff --git a/app/lib/activitypub/activity.rb b/app/lib/activitypub/activity.rb index 51384ef9846..3663e8b9e79 100644 --- a/app/lib/activitypub/activity.rb +++ b/app/lib/activitypub/activity.rb @@ -75,6 +75,14 @@ class ActivityPub::Activity @object_uri ||= uri_from_bearcap(value_or_id(@object)) end + def object_actor_uri + @object_actor_uri ||= value_or_id(first_of_value(@object['attributedTo'])) + end + + def object_actor + @object_actor ||= Account.find_by(uri: object_actor_uri) + end + def unsupported_object_type? @object.is_a?(String) || !(supported_object_type? || converted_object_type?) end @@ -102,13 +110,9 @@ class ActivityPub::Activity return status unless status.nil? # If the boosted toot is embedded and it is a self-boost, handle it like a Create - unless unsupported_object_type? - actor_id = value_or_id(first_of_value(@object['attributedTo'])) - - if actor_id == @account.uri - virtual_object = { 'type' => 'Create', 'actor' => actor_id, 'object' => @object } - return ActivityPub::Activity.factory(virtual_object, @account, request_id: @options[:request_id]).perform - end + if !unsupported_object_type? && (object_actor_uri == @account.uri) + virtual_object = { 'type' => 'Create', 'actor' => actor_id, 'object' => @object } + return ActivityPub::Activity.factory(virtual_object, @account, request_id: @options[:request_id]).perform end fetch_remote_original_status diff --git a/app/lib/activitypub/activity/update.rb b/app/lib/activitypub/activity/update.rb index 973706f5957..a469b948527 100644 --- a/app/lib/activitypub/activity/update.rb +++ b/app/lib/activitypub/activity/update.rb @@ -26,7 +26,8 @@ class ActivityPub::Activity::Update < ActivityPub::Activity def update_status return reject_payload! if non_matching_uri_hosts?(@account.uri, object_uri) - @status = Status.find_by(uri: object_uri, account_id: @account.id) + status_account_id = object_actor ? object_actor.id : @account.id + @status = Status.find_by(uri: object_uri, account_id: status_account_id) return if @status.nil? diff --git a/spec/lib/activitypub/activity/update_spec.rb b/spec/lib/activitypub/activity/update_spec.rb index 87e96d2d1b1..3b37381fe45 100644 --- a/spec/lib/activitypub/activity/update_spec.rb +++ b/spec/lib/activitypub/activity/update_spec.rb @@ -115,5 +115,58 @@ RSpec.describe ActivityPub::Activity::Update do expect(status.edited_at).to be_nil end end + + context 'with a Note object' do + let!(:status) { Fabricate(:status, uri: 'https://example.com/statuses', text: 'Foo', account: sender) } + let(:json) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + id: 'foo', + type: 'Update', + actor: sender.uri, + object: { + type: 'Note', + id: status.uri, + attributedTo: status.account.uri, + content: 'Bar', + published: '2022-01-22T15:00:00Z', + updated: '2022-01-22T16:00:00Z', + }, + }.with_indifferent_access + end + + before do + status.update!(uri: ActivityPub::TagManager.instance.uri_for(status)) + subject.perform + end + + it 'updates the status' do + expect(status.reload.text).to eq('Bar') + end + + context 'when the activity actor and object atttributedTo are different' do + let!(:another_actor) { Fabricate(:account) } + let(:json) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + id: 'foo', + type: 'Update', + actor: another_actor.uri, + object: { + type: 'Note', + id: status.uri, + attributedTo: status.account.uri, + content: 'Bar', + published: '2022-01-22T15:00:00Z', + updated: '2022-01-22T16:00:00Z', + }, + }.with_indifferent_access + end + + it 'updates the status' do + expect(status.reload.text).to eq('Bar') + end + end + end end end From dae6cd9d519b848365737222f5085d8d2dbb838e Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Tue, 24 Oct 2023 13:06:57 +0800 Subject: [PATCH 2/4] Add alternate editor support --- app/lib/activitypub/activity/update.rb | 6 +++--- .../process_status_update_service.rb | 6 ++++-- .../process_status_update_service_spec.rb | 18 ++++++++++++++++++ 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/app/lib/activitypub/activity/update.rb b/app/lib/activitypub/activity/update.rb index a469b948527..9309d366334 100644 --- a/app/lib/activitypub/activity/update.rb +++ b/app/lib/activitypub/activity/update.rb @@ -26,11 +26,11 @@ class ActivityPub::Activity::Update < ActivityPub::Activity def update_status return reject_payload! if non_matching_uri_hosts?(@account.uri, object_uri) - status_account_id = object_actor ? object_actor.id : @account.id + editor_id = @account.id + status_account_id = object_actor ? object_actor.id : editor_id @status = Status.find_by(uri: object_uri, account_id: status_account_id) - return if @status.nil? - ActivityPub::ProcessStatusUpdateService.new.call(@status, @json, @object, request_id: @options[:request_id]) + ActivityPub::ProcessStatusUpdateService.new.call(@status, @json, @object, request_id: @options[:request_id], editor_id: editor_id) end end diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb index 4ff92da01fb..d8b629fe872 100644 --- a/app/services/activitypub/process_status_update_service.rb +++ b/app/services/activitypub/process_status_update_service.rb @@ -5,7 +5,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService include Redisable include Lockable - def call(status, activity_json, object_json, request_id: nil) + def call(status, activity_json, object_json, request_id: nil, editor_id: nil) raise ArgumentError, 'Status has unsaved changes' if status.changed? @activity_json = activity_json @@ -17,6 +17,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService @media_attachments_changed = false @poll_changed = false @request_id = request_id + @editor_id = editor_id # Only native types can be updated at the moment return @status if !expected_type? || already_updated_more_recently? @@ -251,7 +252,8 @@ class ActivityPub::ProcessStatusUpdateService < BaseService return unless significant_changes? @previous_edit&.save! - @status.snapshot!(account_id: @account.id, rate_limit: false) + account_id = @editor_id || @account.id + @status.snapshot!(account_id: account_id, rate_limit: false) end def skip_download? diff --git a/spec/services/activitypub/process_status_update_service_spec.rb b/spec/services/activitypub/process_status_update_service_spec.rb index 9d91f31cc5c..0592b165732 100644 --- a/spec/services/activitypub/process_status_update_service_spec.rb +++ b/spec/services/activitypub/process_status_update_service_spec.rb @@ -462,5 +462,23 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService, type: :service do subject.call(status, json, json) expect(status.reload.edited_at.to_s).to eq '2021-09-08 22:39:25 UTC' end + + context 'with an editor_id' do + let(:editor) { Fabricate(:account) } + + it 'creates edits with the right editor' do + allow(DistributionWorker).to receive(:perform_async) + subject.call(status, json, json, editor_id: editor.id) + expect(status.edits.reload.last.account_id).to eq editor.id + end + end + + context 'without an editor_id' do + it 'creates edits with the right editor' do + allow(DistributionWorker).to receive(:perform_async) + subject.call(status, json, json) + expect(status.edits.reload.last.account_id).to eq status.account.id + end + end end end From f02b0061516bb46a36e73fea666f70a6d449318a Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Tue, 24 Oct 2023 15:43:41 +0800 Subject: [PATCH 3/4] Use consistent naming --- app/lib/activitypub/activity/update.rb | 8 +++++--- .../activitypub/process_status_update_service.rb | 11 +++++++---- .../activitypub/process_status_update_service_spec.rb | 10 +++++----- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/app/lib/activitypub/activity/update.rb b/app/lib/activitypub/activity/update.rb index 9309d366334..8789357eb3d 100644 --- a/app/lib/activitypub/activity/update.rb +++ b/app/lib/activitypub/activity/update.rb @@ -26,11 +26,13 @@ class ActivityPub::Activity::Update < ActivityPub::Activity def update_status return reject_payload! if non_matching_uri_hosts?(@account.uri, object_uri) - editor_id = @account.id - status_account_id = object_actor ? object_actor.id : editor_id @status = Status.find_by(uri: object_uri, account_id: status_account_id) return if @status.nil? - ActivityPub::ProcessStatusUpdateService.new.call(@status, @json, @object, request_id: @options[:request_id], editor_id: editor_id) + ActivityPub::ProcessStatusUpdateService.new.call(@status, @json, @object, request_id: @options[:request_id], activity_account_id: @account.id) + end + + def status_account_id + object_actor ? object_actor.id : @account.id end end diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb index d8b629fe872..8e41df301ef 100644 --- a/app/services/activitypub/process_status_update_service.rb +++ b/app/services/activitypub/process_status_update_service.rb @@ -5,7 +5,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService include Redisable include Lockable - def call(status, activity_json, object_json, request_id: nil, editor_id: nil) + def call(status, activity_json, object_json, request_id: nil, activity_account_id: nil) raise ArgumentError, 'Status has unsaved changes' if status.changed? @activity_json = activity_json @@ -17,7 +17,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService @media_attachments_changed = false @poll_changed = false @request_id = request_id - @editor_id = editor_id + @activity_account_id = activity_account_id # Only native types can be updated at the moment return @status if !expected_type? || already_updated_more_recently? @@ -252,8 +252,11 @@ class ActivityPub::ProcessStatusUpdateService < BaseService return unless significant_changes? @previous_edit&.save! - account_id = @editor_id || @account.id - @status.snapshot!(account_id: account_id, rate_limit: false) + @status.snapshot!(account_id: editor_account_id, rate_limit: false) + end + + def editor_account_id + @activity_account_id || @account.id end def skip_download? diff --git a/spec/services/activitypub/process_status_update_service_spec.rb b/spec/services/activitypub/process_status_update_service_spec.rb index 0592b165732..6d9d30f664a 100644 --- a/spec/services/activitypub/process_status_update_service_spec.rb +++ b/spec/services/activitypub/process_status_update_service_spec.rb @@ -463,17 +463,17 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService, type: :service do expect(status.reload.edited_at.to_s).to eq '2021-09-08 22:39:25 UTC' end - context 'with an editor_id' do - let(:editor) { Fabricate(:account) } + context 'with an activity_account_id' do + let(:activity_account) { Fabricate(:account) } it 'creates edits with the right editor' do allow(DistributionWorker).to receive(:perform_async) - subject.call(status, json, json, editor_id: editor.id) - expect(status.edits.reload.last.account_id).to eq editor.id + subject.call(status, json, json, activity_account_id: activity_account.id) + expect(status.edits.reload.last.account_id).to eq activity_account.id end end - context 'without an editor_id' do + context 'without an activity_account_id' do it 'creates edits with the right editor' do allow(DistributionWorker).to receive(:perform_async) subject.call(status, json, json) From f2103d12901ef2946279012fdf5ad00d7407f784 Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Tue, 24 Oct 2023 20:24:17 +0800 Subject: [PATCH 4/4] Update app/lib/activitypub/activity.rb Co-authored-by: Claire --- app/lib/activitypub/activity.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/lib/activitypub/activity.rb b/app/lib/activitypub/activity.rb index 3663e8b9e79..92b17b691d6 100644 --- a/app/lib/activitypub/activity.rb +++ b/app/lib/activitypub/activity.rb @@ -111,7 +111,7 @@ class ActivityPub::Activity # If the boosted toot is embedded and it is a self-boost, handle it like a Create if !unsupported_object_type? && (object_actor_uri == @account.uri) - virtual_object = { 'type' => 'Create', 'actor' => actor_id, 'object' => @object } + virtual_object = { 'type' => 'Create', 'actor' => object_actor_uri, 'object' => @object } return ActivityPub::Activity.factory(virtual_object, @account, request_id: @options[:request_id]).perform end