From 8b8004a9626442ae31e4dffd79e874e9cde050c6 Mon Sep 17 00:00:00 2001
From: ThibG <thib@sitedethib.com>
Date: Thu, 19 Nov 2020 19:52:06 +0100
Subject: Fix webfinger redirect handling in ResolveAccountService (#15187)

* Fix webfinger redirect handling in ResolveAccountService

ResolveAccountService#process_webfinger! handled a one-step webfinger
redirection, but only accepting the result if it matched the exact URI passed
as input, defeating the point of a redirection check.

Instead, use the same logic as in `ActivityPub::FetchRemoteAccountService`,
updating the resulting `acct:` URI with the result of the first webfinger
query.

* Add tests
---
 app/services/resolve_account_service.rb | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

(limited to 'app/services')

diff --git a/app/services/resolve_account_service.rb b/app/services/resolve_account_service.rb
index c5291fa67..74b0b82d0 100644
--- a/app/services/resolve_account_service.rb
+++ b/app/services/resolve_account_service.rb
@@ -29,6 +29,7 @@ class ResolveAccountService < BaseService
     # At this point we are in need of a Webfinger query, which may
     # yield us a different username/domain through a redirect
     process_webfinger!(@uri)
+    @domain = nil if TagManager.instance.local_domain?(@domain)
 
     # Because the username/domain pair may be different than what
     # we already checked, we need to check if we've already got
@@ -78,25 +79,31 @@ class ResolveAccountService < BaseService
     @uri = [@username, @domain].compact.join('@')
   end
 
-  def process_webfinger!(uri, redirected = false)
+  def process_webfinger!(uri)
     @webfinger                           = webfinger!("acct:#{uri}")
-    confirmed_username, confirmed_domain = @webfinger.subject.gsub(/\Aacct:/, '').split('@')
+    confirmed_username, confirmed_domain = split_acct(@webfinger.subject)
 
     if confirmed_username.casecmp(@username).zero? && confirmed_domain.casecmp(@domain).zero?
       @username = confirmed_username
       @domain   = confirmed_domain
-      @uri      = uri
-    elsif !redirected
-      return process_webfinger!("#{confirmed_username}@#{confirmed_domain}", true)
-    else
-      raise Webfinger::RedirectError, "The URI #{uri} tries to hijack #{@username}@#{@domain}"
+      return
     end
 
-    @domain = nil if TagManager.instance.local_domain?(@domain)
+    # Account doesn't match, so it may have been redirected
+    @webfinger         = webfinger!("acct:#{confirmed_username}@#{confirmed_domain}")
+    @username, @domain = split_acct(@webfinger.subject)
+
+    unless confirmed_username.casecmp(@username).zero? && confirmed_domain.casecmp(@domain).zero?
+      raise Webfinger::RedirectError, "The URI #{uri} tries to hijack #{@username}@#{@domain}"
+    end
   rescue Webfinger::GoneError
     @gone = true
   end
 
+  def split_acct(acct)
+    acct.gsub(/\Aacct:/, '').split('@')
+  end
+
   def process_account!
     return unless activitypub_ready?
 
-- 
cgit