Fix Webfinger endpoint not handling new AP ID scheme (#38391)

app/controllers/well_known/webfinger_controller.rb

@@ -18,23 +18,7 @@ module WellKnown
     private
 
     def set_account
-      username = username_from_resource
-
-      @account = begin
-        if username == Rails.configuration.x.local_domain || username == Rails.configuration.x.web_domain
-          Account.representative
-        else
-          Account.find_local!(username)
-        end
-      end
-    end
-
-    def username_from_resource
-      resource_user    = resource_param
-      username, domain = resource_user.split('@')
-      resource_user    = "#{username}@#{Rails.configuration.x.local_domain}" if Rails.configuration.x.alternate_domains.include?(domain)
-
-      WebfingerResource.new(resource_user).username
+      @account = WebfingerResource.new(resource_param).account
     end
 
     def resource_param

app/lib/webfinger_resource.rb

@@ -9,14 +9,14 @@ class WebfingerResource
     @resource = resource
   end
 
-  def username
+  def account
     case resource
     when %r{\A(https?://)?#{instance_actor_regexp}/?\Z}
-      Rails.configuration.x.local_domain
+      Account.representative
     when /\Ahttps?/i
-      username_from_url
+      account_from_url
     when /@/
-      username_from_acct
+      account_from_acct
     else
       raise InvalidRequest
     end
@@ -31,11 +31,11 @@ class WebfingerResource
     Regexp.union(hosts)
   end
 
-  def username_from_url
+  def account_from_url
     if account_show_page?
-      path_params[:username]
+      path_params.key?(:username) ? Account.find_local!(path_params[:username]) : Account.local.find(path_params[:id])
     elsif instance_actor_page?
-      Rails.configuration.x.local_domain
+      Account.representative
     else
       raise ActiveRecord::RecordNotFound
     end
@@ -53,10 +53,13 @@ class WebfingerResource
     Rails.application.routes.recognize_path(resource)
   end
 
-  def username_from_acct
+  def account_from_acct
     raise ActiveRecord::RecordNotFound unless domain_matches_local?
 
-    local_username
+    username = local_username
+    return Account.representative if username == Rails.configuration.x.local_domain || username == Rails.configuration.x.web_domain
+
+    Account.find_local!(username)
   end
 
   def split_acct
@@ -76,6 +79,6 @@ class WebfingerResource
   end
 
   def domain_matches_local?
-    TagManager.instance.local_domain?(local_domain) || TagManager.instance.web_domain?(local_domain)
+    TagManager.instance.local_domain?(local_domain) || TagManager.instance.web_domain?(local_domain) || Rails.configuration.x.alternate_domains.include?(local_domain)
   end
 end

spec/lib/webfinger_resource_spec.rb

@@ -11,133 +11,127 @@ RSpec.describe WebfingerResource do
     Rails.configuration.x.web_domain = before_web
   end
 
-  describe '#username' do
+  describe '#account' do
+    subject { described_class.new(resource).account }
+
     describe 'with a URL value' do
-      it 'raises with a route whose controller is not AccountsController' do
-        resource = 'https://example.com/users/alice/other'
+      context 'with a route whose controller is not AccountsController' do
+        let(:resource) { 'https://example.com/users/alice/other' }
 
-        expect do
-          described_class.new(resource).username
-        end.to raise_error(ActiveRecord::RecordNotFound)
+        it 'raises an error' do
+          expect { subject }.to raise_error(ActiveRecord::RecordNotFound)
+        end
       end
 
-      it 'raises with a route whose action is not show' do
-        resource = 'https://example.com/users/alice'
+      context 'with a string that does not start with an URL' do
+        let(:resource) { 'website for http://example.com/users/alice.other' }
 
-        recognized = Rails.application.routes.recognize_path(resource)
-        allow(recognized).to receive(:[]).with(:controller).and_return('accounts')
-        allow(recognized).to receive(:[]).with(:username).and_return('alice')
-        allow(recognized).to receive(:[]).with(:action).and_return('create')
-
-        allow(Rails.application.routes).to receive(:recognize_path).with(resource).and_return(recognized)
-
-        expect do
-          described_class.new(resource).username
-        end.to raise_error(ActiveRecord::RecordNotFound)
-        expect(recognized).to have_received(:[]).exactly(3).times
-
-        expect(Rails.application.routes).to have_received(:recognize_path)
-          .with(resource)
-          .at_least(:once)
+        it 'raises an error' do
+          expect { subject }.to raise_error(described_class::InvalidRequest)
+        end
       end
 
-      it 'raises with a string that doesnt start with URL' do
-        resource = 'website for http://example.com/users/alice/other'
+      context 'with a valid HTTPS route to an existing user' do
+        let(:account) { Fabricate(:account) }
+        let(:resource) { "https://example.com/users/#{account.username}" }
 
-        expect do
-          described_class.new(resource).username
-        end.to raise_error(described_class::InvalidRequest)
+        it { is_expected.to eq(account) }
       end
 
-      it 'finds the username in a valid https route' do
-        resource = 'https://example.com/users/alice'
+      context 'with a valid HTTPS route to an existing user using the new API scheme' do
+        let(:account) { Fabricate(:account) }
+        let(:resource) { "https://example.com/ap/users/#{account.id}" }
 
-        result = described_class.new(resource).username
-        expect(result).to eq 'alice'
+        it { is_expected.to eq(account) }
       end
 
-      it 'finds the username in a mixed case http route' do
-        resource = 'HTTp://exAMPLe.com/users/alice'
+      context 'with a valid HTTPS route to a non-existing user' do
+        let(:account) { Fabricate(:account) }
+        let(:resource) { 'https://example.com/users/alice' }
 
-        result = described_class.new(resource).username
-        expect(result).to eq 'alice'
+        it 'raises an error' do
+          expect { subject }.to raise_error(ActiveRecord::RecordNotFound)
+        end
       end
 
-      it 'finds the username in a valid http route' do
-        resource = 'http://example.com/users/alice'
+      context 'with a mixed case HTTP but valid route to an existing user' do
+        let(:account) { Fabricate(:account) }
+        let(:resource) { "HTTp://example.com/users/#{account.username}" }
 
-        result = described_class.new(resource).username
-        expect(result).to eq 'alice'
+        it { is_expected.to eq(account) }
+      end
+
+      context 'with a valid HTTP route to an existing user' do
+        let(:account) { Fabricate(:account) }
+        let(:resource) { "http://example.com/users/#{account.username}" }
+
+        it { is_expected.to eq(account) }
       end
     end
 
     describe 'with a username and hostname value' do
-      it 'raises on a non-local domain' do
-        resource = 'user@remote-host.com'
+      context 'with a non-local domain' do
+        let(:account) { Fabricate(:account) }
+        let(:resource) { "#{account.username}@remote-host.com" }
 
-        expect do
-          described_class.new(resource).username
-        end.to raise_error(ActiveRecord::RecordNotFound)
+        it 'raises an error' do
+          expect { subject }.to raise_error(ActiveRecord::RecordNotFound)
+        end
       end
 
-      it 'finds username for a local domain' do
-        Rails.configuration.x.local_domain = 'example.com'
-        resource = 'alice@example.com'
+      context 'with a valid handle for a local user with local domain' do
+        let(:account) { Fabricate(:account) }
+        let(:resource) { "#{account.username}@example.com" }
 
-        result = described_class.new(resource).username
-        expect(result).to eq 'alice'
+        before { Rails.configuration.x.local_domain = 'example.com' }
+
+        it { is_expected.to eq(account) }
       end
 
-      it 'finds username for a web domain' do
-        Rails.configuration.x.web_domain = 'example.com'
-        resource = 'alice@example.com'
+      context 'with a valid handle for a local user with web domain' do
+        let(:account) { Fabricate(:account) }
+        let(:resource) { "#{account.username}@example.com" }
 
-        result = described_class.new(resource).username
-        expect(result).to eq 'alice'
+        before { Rails.configuration.x.web_domain = 'example.com' }
+
+        it { is_expected.to eq(account) }
       end
     end
 
     describe 'with an acct value' do
-      it 'raises on a non-local domain' do
-        resource = 'acct:user@remote-host.com'
+      context 'with a non-local domain' do
+        let(:account) { Fabricate(:account) }
+        let(:resource) { "acct:#{account.username}@remote-host.com" }
 
-        expect do
-          described_class.new(resource).username
-        end.to raise_error(ActiveRecord::RecordNotFound)
+        it 'raises an error' do
+          expect { subject }.to raise_error(ActiveRecord::RecordNotFound)
+        end
       end
 
-      it 'raises on a nonsense domain' do
-        resource = 'acct:user@remote-host@remote-hostess.remote.local@remote'
+      context 'with a valid handle for a local user with local domain' do
+        let(:account) { Fabricate(:account) }
+        let(:resource) { "acct:#{account.username}@example.com" }
 
-        expect do
-          described_class.new(resource).username
-        end.to raise_error(ActiveRecord::RecordNotFound)
+        before { Rails.configuration.x.local_domain = 'example.com' }
+
+        it { is_expected.to eq(account) }
       end
 
-      it 'finds the username for a local account if the domain is the local one' do
-        Rails.configuration.x.local_domain = 'example.com'
-        resource = 'acct:alice@example.com'
+      context 'with a valid handle for a local user with web domain' do
+        let(:account) { Fabricate(:account) }
+        let(:resource) { "acct:#{account.username}@example.com" }
 
-        result = described_class.new(resource).username
-        expect(result).to eq 'alice'
-      end
+        before { Rails.configuration.x.web_domain = 'example.com' }
 
-      it 'finds the username for a local account if the domain is the Web one' do
-        Rails.configuration.x.web_domain = 'example.com'
-        resource = 'acct:alice@example.com'
-
-        result = described_class.new(resource).username
-        expect(result).to eq 'alice'
+        it { is_expected.to eq(account) }
       end
     end
 
     describe 'with a nonsense resource' do
-      it 'raises InvalidRequest' do
-        resource = 'df/:dfkj'
+      let(:resource) { 'df/:dfkj' }
 
-        expect do
-          described_class.new(resource).username
-        end.to raise_error(described_class::InvalidRequest)
+      it 'raises an error' do
+        expect { subject }.to raise_error(described_class::InvalidRequest)
       end
     end
   end