Refactor domain block checks (#11268)

11 files changed, 108 insertions, 51 deletions

diff --git a/app/controllers/concerns/signature_verification.rb b/app/controllers/concerns/signature_verification.rb
index 90a57197c..0ccdf5ec9 100644
--- a/app/controllers/concerns/signature_verification.rb
+++ b/app/controllers/concerns/signature_verification.rb
@@ -5,6 +5,8 @@
 module SignatureVerification
   extend ActiveSupport::Concern
 
+  include DomainControlHelper
+
   def signed_request?
     request.headers['Signature'].present?
   end
@@ -126,6 +128,8 @@ module SignatureVerification
     if key_id.start_with?('acct:')
       stoplight_wrap_request { ResolveAccountService.new.call(key_id.gsub(/\Aacct:/, '')) }
     elsif !ActivityPub::TagManager.instance.local_uri?(key_id)
+      return if domain_not_allowed?(key_id)
+
       account   = ActivityPub::TagManager.instance.uri_to_resource(key_id, Account)
       account ||= stoplight_wrap_request { ActivityPub::FetchRemoteKeyService.new.call(key_id, id: false) }
       account
diff --git a/app/helpers/domain_control_helper.rb b/app/helpers/domain_control_helper.rb
new file mode 100644
index 000000000..efd328f81
--- /dev/null
+++ b/app/helpers/domain_control_helper.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+module DomainControlHelper
+  def domain_not_allowed?(uri_or_domain)
+    return if uri_or_domain.blank?
+
+    domain = begin
+      if uri_or_domain.include?('://')
+        Addressable::URI.parse(uri_or_domain).domain
+      else
+        uri_or_domain
+      end
+    end
+
+    DomainBlock.blocked?(domain)
+  end
+end
diff --git a/app/lib/tag_manager.rb b/app/lib/tag_manager.rb
index daf4f556b..c88cf4994 100644
--- a/app/lib/tag_manager.rb
+++ b/app/lib/tag_manager.rb
@@ -24,13 +24,16 @@ class TagManager
 
   def same_acct?(canonical, needle)
     return true if canonical.casecmp(needle).zero?
+
     username, domain = needle.split('@')
+
     local_domain?(domain) && canonical.casecmp(username).zero?
   end
 
   def local_url?(url)
     uri    = Addressable::URI.parse(url).normalize
     domain = uri.host + (uri.port ? ":#{uri.port}" : '')
+
     TagManager.instance.web_domain?(domain)
   end
 end
diff --git a/app/services/activitypub/fetch_featured_collection_service.rb b/app/services/activitypub/fetch_featured_collection_service.rb
index 6a137b520..2c2770466 100644
--- a/app/services/activitypub/fetch_featured_collection_service.rb
+++ b/app/services/activitypub/fetch_featured_collection_service.rb
@@ -4,13 +4,12 @@ class ActivityPub::FetchFeaturedCollectionService < BaseService
   include JsonLdHelper
 
   def call(account)
-    return if account.featured_collection_url.blank?
+    return if account.featured_collection_url.blank? || account.suspended? || account.local?
 
     @account = account
     @json    = fetch_resource(@account.featured_collection_url, true)
 
     return unless supported_context?
-    return if @account.suspended? || @account.local?
 
     case @json['type']
     when 'Collection', 'CollectionPage'
diff --git a/app/services/activitypub/fetch_remote_account_service.rb b/app/services/activitypub/fetch_remote_account_service.rb
index 3c2044941..d65c8f951 100644
--- a/app/services/activitypub/fetch_remote_account_service.rb
+++ b/app/services/activitypub/fetch_remote_account_service.rb
@@ -2,18 +2,22 @@
 
 class ActivityPub::FetchRemoteAccountService < BaseService
   include JsonLdHelper
+  include DomainControlHelper
 
   SUPPORTED_TYPES = %w(Application Group Organization Person Service).freeze
 
   # Does a WebFinger roundtrip on each call, unless `only_key` is true
   def call(uri, id: true, prefetched_body: nil, break_on_redirect: false, only_key: false)
+    return if domain_not_allowed?(uri)
     return ActivityPub::TagManager.instance.uri_to_resource(uri, Account) if ActivityPub::TagManager.instance.local_uri?(uri)
 
-    @json = if prefetched_body.nil?
-              fetch_resource(uri, id)
-            else
-              body_to_json(prefetched_body, compare_id: id ? uri : nil)
-            end
+    @json = begin
+      if prefetched_body.nil?
+        fetch_resource(uri, id)
+      else
+        body_to_json(prefetched_body, compare_id: id ? uri : nil)
+      end
+    end
 
     return if !supported_context? || !expected_type? || (break_on_redirect && @json['movedTo'].present?)
 
diff --git a/app/services/activitypub/fetch_remote_poll_service.rb b/app/services/activitypub/fetch_remote_poll_service.rb
index 854a32d05..1c79ecf11 100644
--- a/app/services/activitypub/fetch_remote_poll_service.rb
+++ b/app/services/activitypub/fetch_remote_poll_service.rb
@@ -5,7 +5,9 @@ class ActivityPub::FetchRemotePollService < BaseService
 
   def call(poll, on_behalf_of = nil)
     json = fetch_resource(poll.status.uri, true, on_behalf_of)
+
     return unless supported_context?(json)
+
     ActivityPub::ProcessPollService.new.call(poll, json)
   end
 end
diff --git a/app/services/activitypub/process_account_service.rb b/app/services/activitypub/process_account_service.rb
index 3857e7c16..603e27ed9 100644
--- a/app/services/activitypub/process_account_service.rb
+++ b/app/services/activitypub/process_account_service.rb
@@ -2,11 +2,12 @@
 
 class ActivityPub::ProcessAccountService < BaseService
   include JsonLdHelper
+  include DomainControlHelper
 
   # Should be called with confirmed valid JSON
   # and WebFinger-resolved username and domain
   def call(username, domain, json, options = {})
-    return if json['inbox'].blank? || unsupported_uri_scheme?(json['id'])
+    return if json['inbox'].blank? || unsupported_uri_scheme?(json['id']) || domain_not_allowed?(domain)
 
     @options     = options
     @json        = json
@@ -15,8 +16,6 @@ class ActivityPub::ProcessAccountService < BaseService
     @domain      = domain
     @collections = {}
 
-    return if auto_suspend?
-
     RedisLock.acquire(lock_options) do |lock|
       if lock.acquired?
         @account        = Account.find_remote(@username, @domain)
diff --git a/app/services/activitypub/process_collection_service.rb b/app/services/activitypub/process_collection_service.rb
index 881df478b..a2a2e7071 100644
--- a/app/services/activitypub/process_collection_service.rb
+++ b/app/services/activitypub/process_collection_service.rb
@@ -8,9 +8,7 @@ class ActivityPub::ProcessCollectionService < BaseService
     @json    = Oj.load(body, mode: :strict)
     @options = options
 
-    return unless supported_context?
-    return if different_actor? && verify_account!.nil?
-    return if @account.suspended? || @account.local?
+    return if !supported_context? || (different_actor? && verify_account!.nil?) || @account.suspended? || @account.local?
 
     case @json['type']
     when 'Collection', 'CollectionPage'
diff --git a/app/services/activitypub/process_poll_service.rb b/app/services/activitypub/process_poll_service.rb
index 61357abd3..2fbce65b9 100644
--- a/app/services/activitypub/process_poll_service.rb
+++ b/app/services/activitypub/process_poll_service.rb
@@ -5,6 +5,7 @@ class ActivityPub::ProcessPollService < BaseService
 
   def call(poll, json)
     @json = json
+
     return unless expected_type?
 
     previous_expires_at = poll.expires_at
diff --git a/app/services/resolve_account_service.rb b/app/services/resolve_account_service.rb
index 0ea31a0d8..41a2eb158 100644
--- a/app/services/resolve_account_service.rb
+++ b/app/services/resolve_account_service.rb
@@ -1,75 +1,108 @@
 # frozen_string_literal: true
 
-require_relative '../models/account'
-
 class ResolveAccountService < BaseService
   include JsonLdHelper
+  include DomainControlHelper
+
+  class WebfingerRedirectError < StandardError; end
 
-  # Find or create a local account for a remote user.
-  # When creating, look up the user's webfinger and fetch all
-  # important information from their feed
-  # @param [String, Account] uri User URI in the form of username@domain
+  # Find or create an account record for a remote user. When creating,
+  # look up the user's webfinger and fetch ActivityPub data
+  # @param [String, Account] uri URI in the username@domain format or account record
   # @param [Hash] options
+  # @option options [Boolean] :redirected Do not follow further Webfinger redirects
+  # @option options [Boolean] :skip_webfinger Do not attempt to refresh account data
   # @return [Account]
   def call(uri, options = {})
+    return if uri.blank?
+
+    process_options!(uri, options)
+
+    # First of all we want to check if we've got the account
+    # record with the URI already, and if so, we can exit early
+
+    return if domain_not_allowed?(@domain)
+
+    @account ||= Account.find_remote(@username, @domain)
+
+    return @account if @account&.local? || !webfinger_update_due?
+
+    # At this point we are in need of a Webfinger query, which may
+    # yield us a different username/domain through a redirect
+
+    process_webfinger!
+
+    # Because the username/domain pair may be different than what
+    # we already checked, we need to check if we've already got
+    # the record with that URI, again
+
+    return if domain_not_allowed?(@domain)
+
+    @account ||= Account.find_remote(@username, @domain)
+
+    return @account if @account&.local? || !webfinger_update_due?
+
+    # Now it is certain, it is definitely a remote account, and it
+    # either needs to be created, or updated from fresh data
+
+    process_account!
+  rescue Goldfinger::Error, WebfingerRedirectError, Oj::ParseError => e
+    Rails.logger.debug "Webfinger query for #{@uri} failed: #{e}"
+    nil
+  end
+
+  private
+
+  def process_options!(uri, options)
     @options = options
 
     if uri.is_a?(Account)
       @account  = uri
       @username = @account.username
       @domain   = @account.domain
-      uri       = "#{@username}@#{@domain}"
-
-      return @account if @account.local? || !webfinger_update_due?
+      @uri      = [@username, @domain].compact.join('@')
     else
+      @uri               = uri
       @username, @domain = uri.split('@')
-
-      return Account.find_local(@username) if TagManager.instance.local_domain?(@domain)
-
-      @account = Account.find_remote(@username, @domain)
-
-      return @account unless webfinger_update_due?
     end
 
-    Rails.logger.debug "Looking up webfinger for #{uri}"
-
-    @webfinger = Goldfinger.finger("acct:#{uri}")
+    @domain = nil if TagManager.instance.local_domain?(@domain)
+  end
 
+  def process_webfinger!
+    @webfinger                           = Goldfinger.finger("acct:#{@uri}")
     confirmed_username, confirmed_domain = @webfinger.subject.gsub(/\Aacct:/, '').split('@')
 
     if confirmed_username.casecmp(@username).zero? && confirmed_domain.casecmp(@domain).zero?
       @username = confirmed_username
       @domain   = confirmed_domain
-    elsif options[:redirected].nil?
-      return call("#{confirmed_username}@#{confirmed_domain}", options.merge(redirected: true))
+    elsif @options[:redirected].nil?
+      @account = ResolveAccountService.new.call("#{confirmed_username}@#{confirmed_domain}", @options.merge(redirected: true))
     else
-      Rails.logger.debug 'Requested and returned acct URIs do not match'
-      return
+      raise WebfingerRedirectError, "The URI #{uri} tries to hijack #{@username}@#{@domain}"
     end
 
-    return Account.find_local(@username) if TagManager.instance.local_domain?(@domain)
+    @domain = nil if TagManager.instance.local_domain?(@domain)
+  end
+
+  def process_account!
     return unless activitypub_ready?
 
     RedisLock.acquire(lock_options) do |lock|
       if lock.acquired?
         @account = Account.find_remote(@username, @domain)
 
-        next unless @account.nil? || @account.activitypub?
+        next if (@account.present? && !@account.activitypub?) || actor_json.nil?
 
-        handle_activitypub
+        @account = ActivityPub::ProcessAccountService.new.call(@username, @domain, actor_json)
       else
         raise Mastodon::RaceConditionError
       end
     end
 
     @account
-  rescue Goldfinger::Error => e
-    Rails.logger.debug "Webfinger query for #{uri} unsuccessful: #{e}"
-    nil
   end
 
-  private
-
   def webfinger_update_due?
     @account.nil? || ((!@options[:skip_webfinger] || @account.ostatus?) && @account.possibly_stale?)
   end
@@ -78,14 +111,6 @@ class ResolveAccountService < BaseService
     !@webfinger.link('self').nil? && ['application/activity+json', 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"'].include?(@webfinger.link('self').type)
   end
 
-  def handle_activitypub
-    return if actor_json.nil?
-
-    @account = ActivityPub::ProcessAccountService.new.call(@username, @domain, actor_json)
-  rescue Oj::ParseError
-    nil
-  end
-
   def actor_url
     @actor_url ||= @webfinger.link('self').href
   end
diff --git a/spec/services/resolve_account_service_spec.rb b/spec/services/resolve_account_service_spec.rb
index 7a64f4161..cea942e39 100644
--- a/spec/services/resolve_account_service_spec.rb
+++ b/spec/services/resolve_account_service_spec.rb
@@ -53,6 +53,11 @@ RSpec.describe ResolveAccountService, type: :service do
     fail_occurred  = false
     return_values  = Concurrent::Array.new
 
+    # Preload classes that throw circular dependency errors in threads
+    Account
+    TagManager
+    DomainBlock
+
     threads = Array.new(5) do
       Thread.new do
         true while wait_for_start