From d63de55ef84eea883b72a121d680b8841af8e2c0 Mon Sep 17 00:00:00 2001 From: unarist Date: Wed, 23 Aug 2017 01:30:15 +0900 Subject: [PATCH] Fix bugs which OStatus accounts may detected as ActivityPub ready (#4662) * Fallback to OStatus in FetchAtomService * Skip activity+json link if that activity is Person without inbox * If unsupported activity was detected and all other URLs failed, retry with ActivityPub-less Accept header * Allow mention to OStatus account in ActivityPub * Don't update profile with inbox-less Person object --- app/lib/activitypub/activity/create.rb | 2 +- .../activitypub/process_account_service.rb | 2 + app/services/fetch_atom_service.rb | 60 ++++++++++++------- .../fetch_remote_account_service_spec.rb | 27 +++++++++ 4 files changed, 67 insertions(+), 24 deletions(-) diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index 1541257590..5c59c4b249 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -68,7 +68,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity def process_mention(tag, status) account = account_from_uri(tag['href']) - account = ActivityPub::FetchRemoteAccountService.new.call(tag['href']) if account.nil? + account = FetchRemoteAccountService.new.call(tag['href']) if account.nil? return if account.nil? account.mentions.create(status: status) end diff --git a/app/services/activitypub/process_account_service.rb b/app/services/activitypub/process_account_service.rb index 2f2dfd330e..99f9dbdc25 100644 --- a/app/services/activitypub/process_account_service.rb +++ b/app/services/activitypub/process_account_service.rb @@ -6,6 +6,8 @@ class ActivityPub::ProcessAccountService < BaseService # Should be called with confirmed valid JSON # and WebFinger-resolved username and domain def call(username, domain, json) + return unless json['inbox'].present? + @json = json @uri = @json['id'] @username = username diff --git a/app/services/fetch_atom_service.rb b/app/services/fetch_atom_service.rb index c6a4dc2e98..3cf39e006c 100644 --- a/app/services/fetch_atom_service.rb +++ b/app/services/fetch_atom_service.rb @@ -1,13 +1,17 @@ # frozen_string_literal: true class FetchAtomService < BaseService + include JsonLdHelper + def call(url) return if url.blank? - @url = url + result = process(url) - perform_request - process_response + # retry without ActivityPub + result ||= process(url) if @unsupported_activity + + result rescue OpenSSL::SSL::SSLError => e Rails.logger.debug "SSL error: #{e}" nil @@ -18,9 +22,18 @@ class FetchAtomService < BaseService private + def process(url, terminal = false) + @url = url + perform_request + process_response(terminal) + end + def perform_request + accept = 'text/html' + accept = 'application/activity+json, application/ld+json, application/atom+xml, ' + accept unless @unsupported_activity + @response = Request.new(:get, @url) - .add_headers('Accept' => 'application/activity+json, application/ld+json, application/atom+xml, text/html') + .add_headers('Accept' => accept) .perform end @@ -30,7 +43,12 @@ class FetchAtomService < BaseService if @response.mime_type == 'application/atom+xml' [@url, @response.to_s, :ostatus] elsif ['application/activity+json', 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"'].include?(@response.mime_type) - [@url, @response.to_s, :activitypub] + if supported_activity?(@response.to_s) + [@url, @response.to_s, :activitypub] + else + @unsupported_activity = true + nil + end elsif @response['Link'] && !terminal process_headers elsif @response.mime_type == 'text/html' && !terminal @@ -44,15 +62,10 @@ class FetchAtomService < BaseService json_link = page.xpath('//link[@rel="alternate"]').find { |link| ['application/activity+json', 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"'].include?(link['type']) } atom_link = page.xpath('//link[@rel="alternate"]').find { |link| link['type'] == 'application/atom+xml' } - if !json_link.nil? - @url = json_link['href'] - perform_request - process_response(true) - elsif !atom_link.nil? - @url = atom_link['href'] - perform_request - process_response(true) - end + result ||= process(json_link.href, terminal: true) unless json_link.nil? || @unsupported_activity + result ||= process(atom_link.href, terminal: true) unless atom_link.nil? + + result end def process_headers @@ -61,14 +74,15 @@ class FetchAtomService < BaseService json_link = link_header.find_link(%w(rel alternate), %w(type application/activity+json)) || link_header.find_link(%w(rel alternate), ['type', 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"']) atom_link = link_header.find_link(%w(rel alternate), %w(type application/atom+xml)) - if !json_link.nil? - @url = json_link.href - perform_request - process_response(true) - elsif !atom_link.nil? - @url = atom_link.href - perform_request - process_response(true) - end + result ||= process(json_link.href, terminal: true) unless json_link.nil? || @unsupported_activity + result ||= process(atom_link.href, terminal: true) unless atom_link.nil? + + result + end + + def supported_activity?(body) + json = body_to_json(body) + return false if json.nil? || !supported_context?(json) + json['type'] == 'Person' ? json['inbox'].present? : true end end diff --git a/spec/services/activitypub/fetch_remote_account_service_spec.rb b/spec/services/activitypub/fetch_remote_account_service_spec.rb index 786d7f7f2b..391d051c1a 100644 --- a/spec/services/activitypub/fetch_remote_account_service_spec.rb +++ b/spec/services/activitypub/fetch_remote_account_service_spec.rb @@ -11,6 +11,7 @@ RSpec.describe ActivityPub::FetchRemoteAccountService do preferredUsername: 'alice', name: 'Alice', summary: 'Foo bar', + inbox: 'http://example.com/alice/inbox', } end @@ -35,6 +36,32 @@ RSpec.describe ActivityPub::FetchRemoteAccountService do end end + context 'when the account does not have a inbox' do + let!(:webfinger) { { subject: 'acct:alice@example.com', links: [{ rel: 'self', href: 'https://example.com/alice' }] } } + + before do + actor[:inbox] = nil + + stub_request(:get, 'https://example.com/alice').to_return(body: Oj.dump(actor)) + stub_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:alice@example.com').to_return(body: Oj.dump(webfinger), headers: { 'Content-Type': 'application/jrd+json' }) + end + + it 'fetches resource' do + account + expect(a_request(:get, 'https://example.com/alice')).to have_been_made.once + end + + it 'looks up webfinger' do + account + expect(a_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:alice@example.com')).to have_been_made.once + end + + it 'returns nil' do + expect(account).to be_nil + end + + end + context 'when URI and WebFinger share the same host' do let!(:webfinger) { { subject: 'acct:alice@example.com', links: [{ rel: 'self', href: 'https://example.com/alice' }] } }