From 290932602b32e3bdc2a4c5cb278fb73755a2cd52 Mon Sep 17 00:00:00 2001 From: ThibG Date: Sun, 30 Dec 2018 09:48:59 +0100 Subject: [PATCH] Reduce usage of LD signatures (#9659) * Do not LDS-sign Follow, Accept, Reject, Undo, Block * Do not use LDS for Create activities of private toots * Minor cleanup * Ignore unsigned activities instead of misattributing them * Use status.distributable? instead of querying visibility directly --- app/lib/activitypub/activity/follow.rb | 2 +- .../activitypub/process_collection_service.rb | 2 +- .../after_block_domain_from_account_service.rb | 4 ++-- app/services/authorize_follow_service.rb | 4 ++-- app/services/block_service.rb | 6 ++---- app/services/follow_service.rb | 6 ++---- app/services/process_mentions_service.rb | 6 ++++-- app/services/reject_follow_service.rb | 4 ++-- app/services/unblock_service.rb | 4 ++-- app/services/unfollow_service.rb | 4 ++-- app/workers/activitypub/distribution_worker.rb | 14 +++++++++----- .../activitypub/reply_distribution_worker.rb | 12 ++++++++---- .../activitypub/process_collection_service_spec.rb | 6 +++--- 13 files changed, 40 insertions(+), 34 deletions(-) diff --git a/app/lib/activitypub/activity/follow.rb b/app/lib/activitypub/activity/follow.rb index 7ca650fdf..1e805c0d1 100644 --- a/app/lib/activitypub/activity/follow.rb +++ b/app/lib/activitypub/activity/follow.rb @@ -28,7 +28,7 @@ class ActivityPub::Activity::Follow < ActivityPub::Activity end def reject_follow_request!(target_account) - json = Oj.dump(ActivityPub::LinkedDataSignature.new(ActiveModelSerializers::SerializableResource.new(FollowRequest.new(account: @account, target_account: target_account, uri: @json['id']), serializer: ActivityPub::RejectFollowSerializer, adapter: ActivityPub::Adapter).as_json).sign!(target_account)) + json = ActiveModelSerializers::SerializableResource.new(FollowRequest.new(account: @account, target_account: target_account, uri: @json['id']), serializer: ActivityPub::RejectFollowSerializer, adapter: ActivityPub::Adapter).to_json ActivityPub::DeliveryWorker.perform_async(json, target_account.id, @account.inbox_url) end end diff --git a/app/services/activitypub/process_collection_service.rb b/app/services/activitypub/process_collection_service.rb index 79cdca297..5c54aad89 100644 --- a/app/services/activitypub/process_collection_service.rb +++ b/app/services/activitypub/process_collection_service.rb @@ -27,7 +27,7 @@ class ActivityPub::ProcessCollectionService < BaseService private def different_actor? - @json['actor'].present? && value_or_id(@json['actor']) != @account.uri && @json['signature'].present? + @json['actor'].present? && value_or_id(@json['actor']) != @account.uri end def process_items(items) diff --git a/app/services/after_block_domain_from_account_service.rb b/app/services/after_block_domain_from_account_service.rb index 56cc819fb..180f13403 100644 --- a/app/services/after_block_domain_from_account_service.rb +++ b/app/services/after_block_domain_from_account_service.rb @@ -31,11 +31,11 @@ class AfterBlockDomainFromAccountService < BaseService return unless follow.account.activitypub? - json = Oj.dump(ActivityPub::LinkedDataSignature.new(ActiveModelSerializers::SerializableResource.new( + json = ActiveModelSerializers::SerializableResource.new( follow, serializer: ActivityPub::RejectFollowSerializer, adapter: ActivityPub::Adapter - ).as_json).sign!(@account)) + ).to_json ActivityPub::DeliveryWorker.perform_async(json, @account.id, follow.account.inbox_url) end diff --git a/app/services/authorize_follow_service.rb b/app/services/authorize_follow_service.rb index 1674239df..f2e3ebe7d 100644 --- a/app/services/authorize_follow_service.rb +++ b/app/services/authorize_follow_service.rb @@ -24,11 +24,11 @@ class AuthorizeFollowService < BaseService end def build_json(follow_request) - Oj.dump(ActivityPub::LinkedDataSignature.new(ActiveModelSerializers::SerializableResource.new( + ActiveModelSerializers::SerializableResource.new( follow_request, serializer: ActivityPub::AcceptFollowSerializer, adapter: ActivityPub::Adapter - ).as_json).sign!(follow_request.target_account)) + ).to_json end def build_xml(follow_request) diff --git a/app/services/block_service.rb b/app/services/block_service.rb index b39c3eef2..140b238df 100644 --- a/app/services/block_service.rb +++ b/app/services/block_service.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true class BlockService < BaseService - include StreamEntryRenderer - def call(account, target_account) return if account.id == target_account.id @@ -27,11 +25,11 @@ class BlockService < BaseService end def build_json(block) - Oj.dump(ActivityPub::LinkedDataSignature.new(ActiveModelSerializers::SerializableResource.new( + ActiveModelSerializers::SerializableResource.new( block, serializer: ActivityPub::BlockSerializer, adapter: ActivityPub::Adapter - ).as_json).sign!(block.account)) + ).to_json end def build_xml(block) diff --git a/app/services/follow_service.rb b/app/services/follow_service.rb index 862926260..9d36a1449 100644 --- a/app/services/follow_service.rb +++ b/app/services/follow_service.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true class FollowService < BaseService - include StreamEntryRenderer - # Follow a remote user, notify remote user about the follow # @param [Account] source_account From which to follow # @param [String, Account] uri User URI to follow in the form of username@domain (or account record) @@ -82,10 +80,10 @@ class FollowService < BaseService end def build_json(follow_request) - Oj.dump(ActivityPub::LinkedDataSignature.new(ActiveModelSerializers::SerializableResource.new( + ActiveModelSerializers::SerializableResource.new( follow_request, serializer: ActivityPub::FollowSerializer, adapter: ActivityPub::Adapter - ).as_json).sign!(follow_request.account)) + ).to_json end end diff --git a/app/services/process_mentions_service.rb b/app/services/process_mentions_service.rb index ec7d33b1d..2595c5fd3 100644 --- a/app/services/process_mentions_service.rb +++ b/app/services/process_mentions_service.rb @@ -60,11 +60,13 @@ class ProcessMentionsService < BaseService end def activitypub_json - @activitypub_json ||= Oj.dump(ActivityPub::LinkedDataSignature.new(ActiveModelSerializers::SerializableResource.new( + return @activitypub_json if defined?(@activitypub_json) + payload = ActiveModelSerializers::SerializableResource.new( @status, serializer: ActivityPub::ActivitySerializer, adapter: ActivityPub::Adapter - ).as_json).sign!(@status.account)) + ).as_json + @activitypub_json = Oj.dump(@status.distributable? ? ActivityPub::LinkedDataSignature.new(payload).sign!(@status.account) : payload) end def resolve_account_service diff --git a/app/services/reject_follow_service.rb b/app/services/reject_follow_service.rb index c1f7bcb60..a91266aa4 100644 --- a/app/services/reject_follow_service.rb +++ b/app/services/reject_follow_service.rb @@ -19,11 +19,11 @@ class RejectFollowService < BaseService end def build_json(follow_request) - Oj.dump(ActivityPub::LinkedDataSignature.new(ActiveModelSerializers::SerializableResource.new( + ActiveModelSerializers::SerializableResource.new( follow_request, serializer: ActivityPub::RejectFollowSerializer, adapter: ActivityPub::Adapter - ).as_json).sign!(follow_request.target_account)) + ).to_json end def build_xml(follow_request) diff --git a/app/services/unblock_service.rb b/app/services/unblock_service.rb index 869f62d1c..72fc5ab15 100644 --- a/app/services/unblock_service.rb +++ b/app/services/unblock_service.rb @@ -20,11 +20,11 @@ class UnblockService < BaseService end def build_json(unblock) - Oj.dump(ActivityPub::LinkedDataSignature.new(ActiveModelSerializers::SerializableResource.new( + ActiveModelSerializers::SerializableResource.new( unblock, serializer: ActivityPub::UndoBlockSerializer, adapter: ActivityPub::Adapter - ).as_json).sign!(unblock.account)) + ).to_json end def build_xml(block) diff --git a/app/services/unfollow_service.rb b/app/services/unfollow_service.rb index 73a64929f..03e45912d 100644 --- a/app/services/unfollow_service.rb +++ b/app/services/unfollow_service.rb @@ -43,11 +43,11 @@ class UnfollowService < BaseService end def build_json(follow) - Oj.dump(ActivityPub::LinkedDataSignature.new(ActiveModelSerializers::SerializableResource.new( + ActiveModelSerializers::SerializableResource.new( follow, serializer: ActivityPub::UndoFollowSerializer, adapter: ActivityPub::Adapter - ).as_json).sign!(follow.account)) + ).to_json end def build_xml(follow) diff --git a/app/workers/activitypub/distribution_worker.rb b/app/workers/activitypub/distribution_worker.rb index 17c1ef7ff..59aacabfd 100644 --- a/app/workers/activitypub/distribution_worker.rb +++ b/app/workers/activitypub/distribution_worker.rb @@ -12,7 +12,7 @@ class ActivityPub::DistributionWorker return if skip_distribution? ActivityPub::DeliveryWorker.push_bulk(inboxes) do |inbox_url| - [signed_payload, @account.id, inbox_url] + [payload, @account.id, inbox_url] end relay! if relayable? @@ -35,20 +35,24 @@ class ActivityPub::DistributionWorker end def signed_payload - @signed_payload ||= Oj.dump(ActivityPub::LinkedDataSignature.new(payload).sign!(@account)) + Oj.dump(ActivityPub::LinkedDataSignature.new(unsigned_payload).sign!(@account)) end - def payload - @payload ||= ActiveModelSerializers::SerializableResource.new( + def unsigned_payload + ActiveModelSerializers::SerializableResource.new( @status, serializer: ActivityPub::ActivitySerializer, adapter: ActivityPub::Adapter ).as_json end + def payload + @payload ||= @status.distributable? ? signed_payload : Oj.dump(unsigned_payload) + end + def relay! ActivityPub::DeliveryWorker.push_bulk(Relay.enabled.pluck(:inbox_url)) do |inbox_url| - [signed_payload, @account.id, inbox_url] + [payload, @account.id, inbox_url] end end end diff --git a/app/workers/activitypub/reply_distribution_worker.rb b/app/workers/activitypub/reply_distribution_worker.rb index c0ed3a1f4..892cc1474 100644 --- a/app/workers/activitypub/reply_distribution_worker.rb +++ b/app/workers/activitypub/reply_distribution_worker.rb @@ -12,7 +12,7 @@ class ActivityPub::ReplyDistributionWorker return unless @account.present? && @status.distributable? ActivityPub::DeliveryWorker.push_bulk(inboxes) do |inbox_url| - [signed_payload, @status.account_id, inbox_url] + [payload, @status.account_id, inbox_url] end rescue ActiveRecord::RecordNotFound true @@ -25,14 +25,18 @@ class ActivityPub::ReplyDistributionWorker end def signed_payload - @signed_payload ||= Oj.dump(ActivityPub::LinkedDataSignature.new(payload).sign!(@status.account)) + Oj.dump(ActivityPub::LinkedDataSignature.new(unsigned_payload).sign!(@status.account)) end - def payload - @payload ||= ActiveModelSerializers::SerializableResource.new( + def unsigned_payload + ActiveModelSerializers::SerializableResource.new( @status, serializer: ActivityPub::ActivitySerializer, adapter: ActivityPub::Adapter ).as_json end + + def payload + @payload ||= @status.distributable? ? signed_payload : Oj.dump(unsigned_payload) + end end diff --git a/spec/services/activitypub/process_collection_service_spec.rb b/spec/services/activitypub/process_collection_service_spec.rb index bbe97d211..b3baf6b6b 100644 --- a/spec/services/activitypub/process_collection_service_spec.rb +++ b/spec/services/activitypub/process_collection_service_spec.rb @@ -26,9 +26,9 @@ RSpec.describe ActivityPub::ProcessCollectionService, type: :service do context 'when actor differs from sender' do let(:forwarder) { Fabricate(:account, domain: 'example.com', uri: 'http://example.com/other_account') } - it 'processes payload with sender if no signature exists' do - expect_any_instance_of(ActivityPub::LinkedDataSignature).not_to receive(:verify_account!) - expect(ActivityPub::Activity).to receive(:factory).with(instance_of(Hash), forwarder, instance_of(Hash)) + it 'does not process payload if no signature exists' do + expect_any_instance_of(ActivityPub::LinkedDataSignature).to receive(:verify_account!).and_return(nil) + expect(ActivityPub::Activity).not_to receive(:factory) subject.call(json, forwarder) end