mastofe

commit: f7937d903c681769801e4f3edcdac7e3c71ad9cf
parent: 6b2be5dbfb73e2daf78cf050e5f24c2a8b31ece5
Author: unarist <m.unarist@gmail.com>
Date:   Fri,  1 Sep 2017 00:18:49 +0900

Don't process ActivityPub payload if signature is invalid (#4752)

* Don't process ActivityPub payload if signature is invalid

* Fix style issue

Diffstat:
M
app/services/activitypub/process_collection_service.rb
5
++---
M
spec/services/activitypub/process_collection_service_spec.rb
47
++++++++++++++++++++++++++++++++++++++++++++++-

2 files changed, 48 insertions(+), 4 deletions(-)
diff --git a/app/services/activitypub/process_collection_service.rb b/app/services/activitypub/process_collection_service.rb
@@ -9,7 +9,7 @@ class ActivityPub::ProcessCollectionService < BaseService
 
     return if @account.suspended? || !supported_context?
 
-    verify_account! if different_actor?
+    return if different_actor? && verify_account!.nil?
 
     case @json['type']
     when 'Collection', 'CollectionPage'
@@ -43,7 +43,6 @@ class ActivityPub::ProcessCollectionService < BaseService
   end
 
   def verify_account!
-    account  = ActivityPub::LinkedDataSignature.new(@json).verify_account!
-    @account = account unless account.nil?
+    @account = ActivityPub::LinkedDataSignature.new(@json).verify_account!
   end
 end
diff --git a/spec/services/activitypub/process_collection_service_spec.rb b/spec/services/activitypub/process_collection_service_spec.rb
@@ -1,10 +1,55 @@
 require 'rails_helper'
 
 RSpec.describe ActivityPub::ProcessCollectionService do
+  let(:actor) { Fabricate(:account) }
+
+  let(:payload) do
+    {
+      '@context': 'https://www.w3.org/ns/activitystreams',
+      id: 'foo',
+      type: 'Create',
+      actor: ActivityPub::TagManager.instance.uri_for(actor),
+      object: {
+        id: 'bar',
+        type: 'Note',
+        content: 'Lorem ipsum',
+      },
+    }
+  end
+
+  let(:json) { Oj.dump(payload) }
+
   subject { described_class.new }
 
   describe '#call' do
     context 'when actor is the sender'
-    context 'when actor differs from sender'
+    context 'when actor differs from sender' do
+      let(:forwarder) { Fabricate(: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)
+
+        subject.call(json, forwarder)
+      end
+
+      it 'processes payload with actor if valid signature exists' do
+        payload['signature'] = {'type' => 'RsaSignature2017'}
+
+        expect_any_instance_of(ActivityPub::LinkedDataSignature).to receive(:verify_account!).and_return(actor)
+        expect(ActivityPub::Activity).to receive(:factory).with(instance_of(Hash), actor)
+
+        subject.call(json, forwarder)
+      end
+
+      it 'does not process payload if invalid signature exists' do
+        payload['signature'] = {'type' => 'RsaSignature2017'}
+
+        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
+    end
   end
 end