From 161c72d66d25bb8f5ff492e36a8521c701ab8afc Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Thu, 4 Jan 2018 02:08:57 +0900 Subject: [PATCH] Allow to dereference Follow object for ActivityPub (#5772) * Allow to dereference Follow object for ActivityPub * Accept IRI as object representation for Accept activity --- .../activitypub/follows_controller.rb | 18 +++++++ app/lib/activitypub/activity/accept.rb | 17 +++--- app/lib/activitypub/tag_manager.rb | 8 +++ app/models/follow_request.rb | 4 ++ .../activitypub/follow_serializer.rb | 9 +++- app/views/accounts/_header.html.haml | 2 +- config/routes.rb | 3 +- .../activitypub/follows_controller_spec.rb | 43 +++++++++++++++ spec/lib/activitypub/activity/accept_spec.rb | 53 ++++++++++++------- spec/models/follow_request_spec.rb | 8 +++ 10 files changed, 134 insertions(+), 31 deletions(-) create mode 100644 app/controllers/activitypub/follows_controller.rb create mode 100644 spec/controllers/activitypub/follows_controller_spec.rb diff --git a/app/controllers/activitypub/follows_controller.rb b/app/controllers/activitypub/follows_controller.rb new file mode 100644 index 000000000..8b1cddeb4 --- /dev/null +++ b/app/controllers/activitypub/follows_controller.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class ActivityPub::FollowsController < Api::BaseController + include SignatureVerification + + def show + render( + json: FollowRequest.includes(:account).references(:account).find_by!( + id: params.require(:id), + accounts: { domain: nil, username: params.require(:account_username) }, + target_account: signed_request_account + ), + serializer: ActivityPub::FollowSerializer, + adapter: ActivityPub::Adapter, + content_type: 'application/activity+json' + ) + end +end diff --git a/app/lib/activitypub/activity/accept.rb b/app/lib/activitypub/activity/accept.rb index bd90c9019..d0082483c 100644 --- a/app/lib/activitypub/activity/accept.rb +++ b/app/lib/activitypub/activity/accept.rb @@ -2,16 +2,18 @@ class ActivityPub::Activity::Accept < ActivityPub::Activity def perform - case @object['type'] - when 'Follow' - accept_follow + if @object.respond_to?(:[]) && + @object['type'] == 'Follow' && @object['actor'].present? + accept_follow_from @object['actor'] + else + accept_follow_object @object end end private - def accept_follow - target_account = account_from_uri(target_uri) + def accept_follow_from(actor) + target_account = account_from_uri(value_or_id(actor)) return if target_account.nil? || !target_account.local? @@ -19,7 +21,8 @@ class ActivityPub::Activity::Accept < ActivityPub::Activity follow_request&.authorize! end - def target_uri - @target_uri ||= value_or_id(@object['actor']) + def accept_follow_object(object) + follow_request = ActivityPub::TagManager.instance.uri_to_resource(value_or_id(object), FollowRequest) + follow_request&.authorize! end end diff --git a/app/lib/activitypub/tag_manager.rb b/app/lib/activitypub/tag_manager.rb index 0708713e6..1c35e1672 100644 --- a/app/lib/activitypub/tag_manager.rb +++ b/app/lib/activitypub/tag_manager.rb @@ -28,6 +28,8 @@ class ActivityPub::TagManager return target.uri if target.respond_to?(:local?) && !target.local? case target.object_type + when :follow + account_follow_url(target.account.username, target) when :person account_url(target) when :note, :comment, :activity @@ -97,6 +99,12 @@ class ActivityPub::TagManager case klass.name when 'Account' klass.find_local(uri_to_local_id(uri, :username)) + when 'FollowRequest' + params = Rails.application.routes.recognize_path(uri) + klass.joins(:account).find_by!( + accounts: { domain: nil, username: params[:account_username] }, + id: params[:id] + ) else StatusFinder.new(uri).status end diff --git a/app/models/follow_request.rb b/app/models/follow_request.rb index ebf6959ce..33e5fec12 100644 --- a/app/models/follow_request.rb +++ b/app/models/follow_request.rb @@ -21,6 +21,10 @@ class FollowRequest < ApplicationRecord validates :account_id, uniqueness: { scope: :target_account_id } + def object_type + :follow + end + def authorize! account.follow!(target_account, reblogs: show_reblogs) MergeWorker.perform_async(target_account.id, account.id) diff --git a/app/serializers/activitypub/follow_serializer.rb b/app/serializers/activitypub/follow_serializer.rb index 86c9992fe..eecd64701 100644 --- a/app/serializers/activitypub/follow_serializer.rb +++ b/app/serializers/activitypub/follow_serializer.rb @@ -1,11 +1,12 @@ # frozen_string_literal: true class ActivityPub::FollowSerializer < ActiveModel::Serializer - attributes :id, :type, :actor + attributes :type, :actor + attribute :id, if: :dereferencable? attribute :virtual_object, key: :object def id - [ActivityPub::TagManager.instance.uri_for(object.account), '#follows/', object.id].join + ActivityPub::TagManager.instance.uri_for(object) end def type @@ -19,4 +20,8 @@ class ActivityPub::FollowSerializer < ActiveModel::Serializer def virtual_object ActivityPub::TagManager.instance.uri_for(object.target_account) end + + def dereferencable? + object.respond_to?(:object_type) + end end diff --git a/app/views/accounts/_header.html.haml b/app/views/accounts/_header.html.haml index d4081af64..3800bd837 100644 --- a/app/views/accounts/_header.html.haml +++ b/app/views/accounts/_header.html.haml @@ -8,7 +8,7 @@ = fa_icon 'user-times' = t('accounts.unfollow') - else - = link_to account_follow_path(account), data: { method: :post }, class: 'icon-button' do + = link_to account_follows_path(account), data: { method: :post }, class: 'icon-button' do = fa_icon 'user-plus' = t('accounts.follow') - elsif !user_signed_in? diff --git a/config/routes.rb b/config/routes.rb index 80a2c6d13..21ad0a93f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -54,7 +54,8 @@ Rails.application.routes.draw do resources :followers, only: [:index], controller: :follower_accounts resources :following, only: [:index], controller: :following_accounts - resource :follow, only: [:create], controller: :account_follow + resources :follows, only: [:show], module: :activitypub + resource :follow, only: [:create], controller: :account_follow, as: :follows resource :unfollow, only: [:create], controller: :account_unfollow resource :outbox, only: [:show], module: :activitypub resource :inbox, only: [:create], module: :activitypub diff --git a/spec/controllers/activitypub/follows_controller_spec.rb b/spec/controllers/activitypub/follows_controller_spec.rb new file mode 100644 index 000000000..6026cd353 --- /dev/null +++ b/spec/controllers/activitypub/follows_controller_spec.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe ActivityPub::FollowsController, type: :controller do + let(:follow_request) { Fabricate(:follow_request, account: account) } + + render_views + + context 'with local account' do + let(:account) { Fabricate(:account, domain: nil) } + + it 'returns follow request' do + signed_request = Request.new(:get, account_follow_url(account, follow_request)) + signed_request.on_behalf_of(follow_request.target_account) + request.headers.merge! signed_request.headers + + get :show, params: { id: follow_request, account_username: account.username } + + expect(body_as_json[:id]).to eq ActivityPub::TagManager.instance.uri_for(follow_request) + expect(response).to have_http_status :success + end + + it 'returns http 404 without signature' do + get :show, params: { id: follow_request, account_username: account.username } + expect(response).to have_http_status 404 + end + end + + context 'with remote account' do + let(:account) { Fabricate(:account, domain: Faker::Internet.domain_name) } + + it 'returns http 404' do + signed_request = Request.new(:get, account_follow_url(account, follow_request)) + signed_request.on_behalf_of(follow_request.target_account) + request.headers.merge! signed_request.headers + + get :show, params: { id: follow_request, account_username: account.username } + + expect(response).to have_http_status 404 + end + end +end diff --git a/spec/lib/activitypub/activity/accept_spec.rb b/spec/lib/activitypub/activity/accept_spec.rb index 6503c83e3..9f43be35d 100644 --- a/spec/lib/activitypub/activity/accept_spec.rb +++ b/spec/lib/activitypub/activity/accept_spec.rb @@ -3,36 +3,49 @@ require 'rails_helper' RSpec.describe ActivityPub::Activity::Accept do let(:sender) { Fabricate(:account) } let(:recipient) { Fabricate(:account) } - - let(:json) do - { - '@context': 'https://www.w3.org/ns/activitystreams', - id: 'foo', - type: 'Accept', - actor: ActivityPub::TagManager.instance.uri_for(sender), - object: { - id: 'bar', - type: 'Follow', - actor: ActivityPub::TagManager.instance.uri_for(recipient), - object: ActivityPub::TagManager.instance.uri_for(sender), - }, - }.with_indifferent_access - end + let!(:follow_request) { Fabricate(:follow_request, account: recipient, target_account: sender) } describe '#perform' do subject { described_class.new(json, sender) } before do - Fabricate(:follow_request, account: recipient, target_account: sender) subject.perform end - it 'creates a follow relationship' do - expect(recipient.following?(sender)).to be true + context 'with concerete object representation' do + let(:json) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + id: 'foo', + type: 'Accept', + actor: ActivityPub::TagManager.instance.uri_for(sender), + object: { + type: 'Follow', + actor: ActivityPub::TagManager.instance.uri_for(recipient), + object: ActivityPub::TagManager.instance.uri_for(sender), + }, + }.with_indifferent_access + end + + it 'creates a follow relationship' do + expect(recipient.following?(sender)).to be true + end end - it 'removes the follow request' do - expect(recipient.requested?(sender)).to be false + context 'with object represented by id' do + let(:json) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + id: 'foo', + type: 'Accept', + actor: ActivityPub::TagManager.instance.uri_for(sender), + object: ActivityPub::TagManager.instance.uri_for(follow_request), + }.with_indifferent_access + end + + it 'creates a follow relationship' do + expect(recipient.following?(sender)).to be true + end end end end diff --git a/spec/models/follow_request_spec.rb b/spec/models/follow_request_spec.rb index 59893a14f..7e10631a3 100644 --- a/spec/models/follow_request_spec.rb +++ b/spec/models/follow_request_spec.rb @@ -27,4 +27,12 @@ RSpec.describe FollowRequest, type: :model do expect(follow_request.account.muting_reblogs?(target)).to be true end end + + describe '#object_type' do + let(:follow_request) { Fabricate(:follow_request) } + + it 'equals to :follow' do + expect(follow_request.object_type).to eq :follow + end + end end