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

4 files changed, 67 insertions, 24 deletions

diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb
index 154125759..5c59c4b24 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 2f2dfd330..99f9dbdc2 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 c6a4dc2e9..3cf39e006 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 786d7f7f2..391d051c1 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' }] } }