about summary refs log tree commit diff
diff options
context:
space:
mode:
authorThibG <thib@sitedethib.com>2018-12-30 09:48:59 +0100
committerEugen Rochko <eugen@zeonfederated.com>2018-12-30 09:48:59 +0100
commit290932602b32e3bdc2a4c5cb278fb73755a2cd52 (patch)
tree45403aa491d707bd683197abf602b72dcaad2b9a
parentfb08039de58bee131e66c1a41db9d9f1f831d6e9 (diff)
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
-rw-r--r--app/lib/activitypub/activity/follow.rb2
-rw-r--r--app/services/activitypub/process_collection_service.rb2
-rw-r--r--app/services/after_block_domain_from_account_service.rb4
-rw-r--r--app/services/authorize_follow_service.rb4
-rw-r--r--app/services/block_service.rb6
-rw-r--r--app/services/follow_service.rb6
-rw-r--r--app/services/process_mentions_service.rb6
-rw-r--r--app/services/reject_follow_service.rb4
-rw-r--r--app/services/unblock_service.rb4
-rw-r--r--app/services/unfollow_service.rb4
-rw-r--r--app/workers/activitypub/distribution_worker.rb14
-rw-r--r--app/workers/activitypub/reply_distribution_worker.rb12
-rw-r--r--spec/services/activitypub/process_collection_service_spec.rb6
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