From b01a19fe392e0dd16d6b3da3f0b56369f7837cc9 Mon Sep 17 00:00:00 2001 From: unarist Date: Thu, 24 Aug 2017 23:21:42 +0900 Subject: [PATCH] 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 --- app/controllers/statuses_controller.rb | 5 ++ app/lib/activitypub/tag_manager.rb | 8 +++ .../activitypub/activity_serializer.rb | 2 +- .../fetch_remote_status_service.rb | 30 +++++--- spec/controllers/statuses_controller_spec.rb | 12 ++++ .../fetch_remote_status_service_spec.rb | 72 ++++++++++++++++++- 6 files changed, 118 insertions(+), 11 deletions(-) diff --git a/app/controllers/statuses_controller.rb b/app/controllers/statuses_controller.rb index aa24f23c944..a9768d09298 100644 --- 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 3c16006cb16..de575d9e6f6 100644 --- 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 69e2160c5d4..d20ee99206a 100644 --- 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 993e5389c42..c114515cdd4 100644 --- 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 88d365624e3..95fb4d5945f 100644 --- 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 47a33b6cb6d..3b22257ed15 100644 --- 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