Fetch reblogs as Announce activity instead of Note object (#4672)

* Process Create / Announce activity in FetchRemoteStatusService

* Use activity URL in ActivityPub for reblogs

* Redirect to the original status on StatusesController#show

diff --git a/app/controllers/statuses_controller.rb b/app/controllers/statuses_controller.rb
index aa24f23c944ffde3e73408efcbe260c55e6c0a49..a9768d09298604d89abaacb51771c2342f98ae68 100644 (file)
--- a/app/controllers/statuses_controller.rb
+++ b/app/controllers/statuses_controller.rb
@@ -9,6 +9,7 @@ class StatusesController < ApplicationController
   before_action :set_status
   before_action :set_link_headers
   before_action :check_account_suspension
+  before_action :redirect_to_original, only: [:show]
 
   def show
     respond_to do |format|
@@ -58,4 +59,8 @@ class StatusesController < ApplicationController
   def check_account_suspension
     gone if @account.suspended?
   end
+
+  def redirect_to_original
+    redirect_to ::TagManager.instance.url_for(@status.reblog) if @status.reblog?
+  end
 end

diff --git a/app/lib/activitypub/tag_manager.rb b/app/lib/activitypub/tag_manager.rb
index 3c16006cb161fcadd914623b45f4554a1a47525f..de575d9e6f6ddbaa494614e21fde75f7711a977f 100644 (file)
--- a/app/lib/activitypub/tag_manager.rb
+++ b/app/lib/activitypub/tag_manager.rb
@@ -19,6 +19,7 @@ class ActivityPub::TagManager
     when :person
       short_account_url(target)
     when :note, :comment, :activity
+      return activity_account_status_url(target.account, target) if target.reblog?
       short_account_status_url(target.account, target)
     end
   end
@@ -30,10 +31,17 @@ class ActivityPub::TagManager
     when :person
       account_url(target)
     when :note, :comment, :activity
+      return activity_account_status_url(target.account, target) if target.reblog?
       account_status_url(target.account, target)
     end
   end
 
+  def activity_uri_for(target)
+    return nil unless %i(note comment activity).include?(target.object_type) && target.local?
+
+    activity_account_status_url(target.account, target)
+  end
+
   # Primary audience of a status
   # Public statuses go out to primarily the public collection
   # Unlisted and private statuses go out primarily to the followers collection

diff --git a/app/serializers/activitypub/activity_serializer.rb b/app/serializers/activitypub/activity_serializer.rb
index 69e2160c5d47e28975bf81bba55c7b1d35739c52..d20ee99206a832db72b2345878c8dbee64949093 100644 (file)
--- a/app/serializers/activitypub/activity_serializer.rb
+++ b/app/serializers/activitypub/activity_serializer.rb
@@ -6,7 +6,7 @@ class ActivityPub::ActivitySerializer < ActiveModel::Serializer
   has_one :proper, key: :object, serializer: ActivityPub::NoteSerializer
 
   def id
-    [ActivityPub::TagManager.instance.uri_for(object), '/activity'].join
+    [ActivityPub::TagManager.instance.activity_uri_for(object)].join
   end
 
   def type

diff --git a/app/services/activitypub/fetch_remote_status_service.rb b/app/services/activitypub/fetch_remote_status_service.rb
index 993e5389c42976d8ce7be0612d7cc2562306e66c..c114515cdd4e15c91299f0822c9fc61ec8e55436 100644 (file)
--- a/app/services/activitypub/fetch_remote_status_service.rb
+++ b/app/services/activitypub/fetch_remote_status_service.rb
@@ -7,21 +7,33 @@ class ActivityPub::FetchRemoteStatusService < BaseService
   def call(uri, prefetched_json = nil)
     @json = body_to_json(prefetched_json) || fetch_resource(uri)
 
-    return unless supported_context? && expected_type?
+    return unless supported_context?
 
-    attributed_to = first_of_value(@json['attributedTo'])
-    attributed_to = attributed_to['id'] if attributed_to.is_a?(Hash)
+    activity = activity_json
+    actor_id = value_or_id(activity['actor'])
 
-    return unless trustworthy_attribution?(uri, attributed_to)
+    return unless expected_type?(activity) && trustworthy_attribution?(uri, actor_id)
 
-    actor = ActivityPub::TagManager.instance.uri_to_resource(attributed_to, Account)
-    actor = ActivityPub::FetchRemoteAccountService.new.call(attributed_to) if actor.nil?
+    actor = ActivityPub::TagManager.instance.uri_to_resource(actor_id, Account)
+    actor = ActivityPub::FetchRemoteAccountService.new.call(actor_id) if actor.nil?
 
-    ActivityPub::Activity::Create.new({ 'object' => @json }, actor).perform
+    ActivityPub::Activity.factory(activity, actor).perform
   end
 
   private
 
+  def activity_json
+    if %w(Note Article).include? @json['type']
+      {
+        'type' => 'Create',
+        'actor' => first_of_value(@json['attributedTo']),
+        'object' => @json,
+      }
+    else
+      @json
+    end
+  end
+
   def trustworthy_attribution?(uri, attributed_to)
     Addressable::URI.parse(uri).normalized_host.casecmp(Addressable::URI.parse(attributed_to).normalized_host).zero?
   end
@@ -30,7 +42,7 @@ class ActivityPub::FetchRemoteStatusService < BaseService
     super(@json)
   end
 
-  def expected_type?
-    %w(Note Article).include? @json['type']
+  def expected_type?(json)
+    %w(Create Announce).include? json['type']
   end
 end

diff --git a/spec/controllers/statuses_controller_spec.rb b/spec/controllers/statuses_controller_spec.rb
index 88d365624e32bbd05d64862286bc89c662daaa22..95fb4d5945f2b2e7edb9406a9f18d5ad4813a195 100644 (file)
--- a/spec/controllers/statuses_controller_spec.rb
+++ b/spec/controllers/statuses_controller_spec.rb
@@ -30,6 +30,18 @@ describe StatusesController do
       end
     end
 
+    context 'status is a reblog' do
+      it 'redirects to the original status' do
+        original_account = Fabricate(:account, domain: 'example.com')
+        original_status = Fabricate(:status, account: original_account, uri: 'tag:example.com,2017:foo', url: 'https://example.com/123')
+        status = Fabricate(:status, reblog: original_status)
+
+        get :show, params: { account_username: status.account.username, id: status.id }
+
+        expect(response).to redirect_to(original_status.url)
+      end
+    end
+
     context 'account is not suspended and status is permitted' do
       it 'assigns @account' do
         status = Fabricate(:status)

diff --git a/spec/services/activitypub/fetch_remote_status_service_spec.rb b/spec/services/activitypub/fetch_remote_status_service_spec.rb
index 47a33b6cb6ddcba4860f74c3b5bd741ad6a115d1..3b22257ed159c98fc3397a0eebf21a3d2999447e 100644 (file)
--- a/spec/services/activitypub/fetch_remote_status_service_spec.rb
+++ b/spec/services/activitypub/fetch_remote_status_service_spec.rb
@@ -1,5 +1,75 @@
 require 'rails_helper'
 
 RSpec.describe ActivityPub::FetchRemoteStatusService do
-  pending
+  let(:sender) { Fabricate(:account) }
+  let(:recipient) { Fabricate(:account) }
+  let(:valid_domain) { Rails.configuration.x.local_domain }
+
+  let(:note) do
+    {
+      '@context': 'https://www.w3.org/ns/activitystreams',
+      id: "https://#{valid_domain}/@foo/1234",
+      type: 'Note',
+      content: 'Lorem ipsum',
+      attributedTo: ActivityPub::TagManager.instance.uri_for(sender),
+    }
+  end
+
+  let(:create) do
+    {
+      '@context': 'https://www.w3.org/ns/activitystreams',
+      id: "https://#{valid_domain}/@foo/1234/activity",
+      type: 'Create',
+      actor: ActivityPub::TagManager.instance.uri_for(sender),
+      object: note,
+    }
+  end
+
+  subject { described_class.new }
+
+  describe '#call' do
+    before do
+      subject.call(object[:id], Oj.dump(object))
+    end
+
+    context 'with Note object' do
+      let(:object) { note }
+
+      it 'creates status' do
+        status = sender.statuses.first
+        
+        expect(status).to_not be_nil
+        expect(status.text).to eq 'Lorem ipsum'
+      end
+    end
+
+    context 'with Create activity' do
+      let(:object) { create }
+
+      it 'creates status' do
+        status = sender.statuses.first
+        
+        expect(status).to_not be_nil
+        expect(status.text).to eq 'Lorem ipsum'
+      end
+    end
+
+    context 'with Announce activity' do
+      let(:status) { Fabricate(:status, account: recipient) }
+
+      let(:object) do
+        {
+          '@context': 'https://www.w3.org/ns/activitystreams',
+          id: "https://#{valid_domain}/@foo/1234/activity",
+          type: 'Announce',
+          actor: ActivityPub::TagManager.instance.uri_for(sender),
+          object: ActivityPub::TagManager.instance.uri_for(status),
+        }
+      end
+
+      it 'creates a reblog by sender of status' do
+        expect(sender.reblogged?(status)).to be true
+      end
+    end
+  end
 end