Change ResolveAccountService's handling of skip_webfinger (#15750)

* Change ResolveAccountService's handling of skip_webfinger

Change it so it never makes any webfinger query, as the name would imply.

* Add tests

* Change FollowService to not take an URI for target_account

* Restore domain-block check in FollowService

* Fix tests

diff --git a/app/services/follow_service.rb b/app/services/follow_service.rb
index b98f7011d1887e55460ef042f2c99127293bed75..d3db07a743bf53b2f6d1620dcefef154659f203a 100644 (file)
--- a/app/services/follow_service.rb
+++ b/app/services/follow_service.rb
@@ -3,10 +3,11 @@
 class FollowService < BaseService
   include Redisable
   include Payloadable
 class FollowService < BaseService
   include Redisable
   include Payloadable

+  include DomainControlHelper

 
   # Follow a remote user, notify remote user about the follow
   # @param [Account] source_account From which to follow
 
   # Follow a remote user, notify remote user about the follow
   # @param [Account] source_account From which to follow

-  # @param [String, Account] uri User URI to follow in the form of username@domain (or account record)
+  # @param [Account] target_account Account to follow

   # @param [Hash] options
   # @option [Boolean] :reblogs Whether or not to show reblogs, defaults to true
   # @option [Boolean] :notify Whether to create notifications about new posts, defaults to false
   # @param [Hash] options
   # @option [Boolean] :reblogs Whether or not to show reblogs, defaults to true
   # @option [Boolean] :notify Whether to create notifications about new posts, defaults to false


@@ -15,7 +16,7 @@ class FollowService < BaseService
   # @option [Boolean] :with_rate_limit
   def call(source_account, target_account, options = {})
     @source_account = source_account
   # @option [Boolean] :with_rate_limit
   def call(source_account, target_account, options = {})
     @source_account = source_account

-    @target_account = ResolveAccountService.new.call(target_account, skip_webfinger: true)
+    @target_account = target_account

     @options        = { bypass_locked: false, bypass_limit: false, with_rate_limit: false }.merge(options)
 
     raise ActiveRecord::RecordNotFound if following_not_possible?
     @options        = { bypass_locked: false, bypass_limit: false, with_rate_limit: false }.merge(options)
 
     raise ActiveRecord::RecordNotFound if following_not_possible?


@@ -43,7 +44,7 @@ class FollowService < BaseService
   end
 
   def following_not_allowed?
   end
 
   def following_not_allowed?

-    @target_account.blocking?(@source_account) || @source_account.blocking?(@target_account) || @target_account.moved? || (!@target_account.local? && @target_account.ostatus?) || @source_account.domain_blocking?(@target_account.domain)
+    domain_not_allowed?(@target_account.domain) || @target_account.blocking?(@source_account) || @source_account.blocking?(@target_account) || @target_account.moved? || (!@target_account.local? && @target_account.ostatus?) || @source_account.domain_blocking?(@target_account.domain)

   end
 
   def change_follow_options!
   end
 
   def change_follow_options!

diff --git a/app/services/resolve_account_service.rb b/app/services/resolve_account_service.rb
index 3301aaf51b1311c2f19d5db6f7839aaffe7dd9e8..b8ddeb2adc033b814cbd5459a0fa60379032a291 100644 (file)
--- a/app/services/resolve_account_service.rb
+++ b/app/services/resolve_account_service.rb
@@ -10,7 +10,7 @@ class ResolveAccountService < BaseService
   # @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
   # @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
+  # @option options [Boolean] :skip_webfinger Do not attempt any webfinger query or refreshing account data

   # @return [Account]
   def call(uri, options = {})
     return if uri.blank?
   # @return [Account]
   def call(uri, options = {})
     return if uri.blank?


@@ -120,8 +120,9 @@ class ResolveAccountService < BaseService
 
   def webfinger_update_due?
     return false if @options[:check_delivery_availability] && !DeliveryFailureTracker.available?(@domain)
 
   def webfinger_update_due?
     return false if @options[:check_delivery_availability] && !DeliveryFailureTracker.available?(@domain)

+    return false if @options[:skip_webfinger]

 
 

-    @account.nil? || ((!@options[:skip_webfinger] || @account.ostatus?) && @account.possibly_stale?)
+    @account.nil? || (@account.ostatus? && @account.possibly_stale?)

   end
 
   def activitypub_ready?
   end
 
   def activitypub_ready?

diff --git a/spec/controllers/api/v1/follow_requests_controller_spec.rb b/spec/controllers/api/v1/follow_requests_controller_spec.rb
index ae92a9627a11346bd8b3d76c5dde929c2a7241e8..1034faa323375442b3326e7058d05bb9f3a94b63 100644 (file)
--- a/spec/controllers/api/v1/follow_requests_controller_spec.rb
+++ b/spec/controllers/api/v1/follow_requests_controller_spec.rb
@@ -8,7 +8,7 @@ RSpec.describe Api::V1::FollowRequestsController, type: :controller do
   let(:follower) { Fabricate(:account, username: 'bob') }
 
   before do
   let(:follower) { Fabricate(:account, username: 'bob') }
 
   before do

-    FollowService.new.call(follower, user.account.acct)
+    FollowService.new.call(follower, user.account)

     allow(controller).to receive(:doorkeeper_token) { token }
   end
 
     allow(controller).to receive(:doorkeeper_token) { token }
   end
 

diff --git a/spec/controllers/api/v1/notifications_controller_spec.rb b/spec/controllers/api/v1/notifications_controller_spec.rb
index db3f4b782be842445be286dde5b623cf12e1625b..5a0b24bbf8c7f23c6b3ec92091b0d1c8f96f4342 100644 (file)
--- a/spec/controllers/api/v1/notifications_controller_spec.rb
+++ b/spec/controllers/api/v1/notifications_controller_spec.rb
@@ -57,7 +57,7 @@ RSpec.describe Api::V1::NotificationsController, type: :controller do
       @mention_from_status = mentioning_status.mentions.first
       @favourite = FavouriteService.new.call(other.account, first_status)
       @second_favourite = FavouriteService.new.call(third.account, first_status)
       @mention_from_status = mentioning_status.mentions.first
       @favourite = FavouriteService.new.call(other.account, first_status)
       @second_favourite = FavouriteService.new.call(third.account, first_status)

-      @follow = FollowService.new.call(other.account, 'alice')
+      @follow = FollowService.new.call(other.account, user.account)

     end
 
     describe 'with no options' do
     end
 
     describe 'with no options' do

diff --git a/spec/controllers/authorize_interactions_controller_spec.rb b/spec/controllers/authorize_interactions_controller_spec.rb
index ce4257b68dcda85e45156d0e57c99f186976a58c..b4ce30cd7fe302375cb9081758615b30da6bf657 100644 (file)
--- a/spec/controllers/authorize_interactions_controller_spec.rb
+++ b/spec/controllers/authorize_interactions_controller_spec.rb
@@ -99,12 +99,10 @@ describe AuthorizeInteractionsController do
 
         allow(ResolveAccountService).to receive(:new).and_return(service)
         allow(service).to receive(:call).with('user@hostname').and_return(target_account)
 
         allow(ResolveAccountService).to receive(:new).and_return(service)
         allow(service).to receive(:call).with('user@hostname').and_return(target_account)

-        allow(service).to receive(:call).with(target_account, skip_webfinger: true).and_return(target_account)

 
 
         post :create, params: { acct: 'acct:user@hostname' }
 
 
 
         post :create, params: { acct: 'acct:user@hostname' }
 

-        expect(service).to have_received(:call).with(target_account, skip_webfinger: true)

         expect(account.following?(target_account)).to be true
         expect(response).to render_template(:success)
       end
         expect(account.following?(target_account)).to be true
         expect(response).to render_template(:success)
       end

diff --git a/spec/services/follow_service_spec.rb b/spec/services/follow_service_spec.rb
index ae863a9f0e98451ce3dc04e683fd6e9162ad67fb..63d6eb3bd8ef523617b795995ff45d11965b0fdb 100644 (file)
--- a/spec/services/follow_service_spec.rb
+++ b/spec/services/follow_service_spec.rb
@@ -10,7 +10,7 @@ RSpec.describe FollowService, type: :service do
       let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, locked: true, username: 'bob')).account }
 
       before do
       let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, locked: true, username: 'bob')).account }
 
       before do

-        subject.call(sender, bob.acct)
+        subject.call(sender, bob)

       end
 
       it 'creates a follow request with reblogs' do
       end
 
       it 'creates a follow request with reblogs' do


@@ -22,7 +22,7 @@ RSpec.describe FollowService, type: :service do
       let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, locked: true, username: 'bob')).account }
 
       before do
       let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, locked: true, username: 'bob')).account }
 
       before do

-        subject.call(sender, bob.acct, reblogs: false)
+        subject.call(sender, bob, reblogs: false)

       end
 
       it 'creates a follow request without reblogs' do
       end
 
       it 'creates a follow request without reblogs' do


@@ -35,7 +35,7 @@ RSpec.describe FollowService, type: :service do
 
       before do
         sender.touch(:silenced_at)
 
       before do
         sender.touch(:silenced_at)

-        subject.call(sender, bob.acct)
+        subject.call(sender, bob)

       end
 
       it 'creates a follow request with reblogs' do
       end
 
       it 'creates a follow request with reblogs' do


@@ -48,7 +48,7 @@ RSpec.describe FollowService, type: :service do
 
       before do
         bob.mute!(sender)
 
       before do
         bob.mute!(sender)

-        subject.call(sender, bob.acct)
+        subject.call(sender, bob)

       end
 
       it 'creates a following relation with reblogs' do
       end
 
       it 'creates a following relation with reblogs' do


@@ -61,7 +61,7 @@ RSpec.describe FollowService, type: :service do
       let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob')).account }
 
       before do
       let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob')).account }
 
       before do

-        subject.call(sender, bob.acct)
+        subject.call(sender, bob)

       end
 
       it 'creates a following relation with reblogs' do
       end
 
       it 'creates a following relation with reblogs' do


@@ -74,7 +74,7 @@ RSpec.describe FollowService, type: :service do
       let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob')).account }
 
       before do
       let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob')).account }
 
       before do

-        subject.call(sender, bob.acct, reblogs: false)
+        subject.call(sender, bob, reblogs: false)

       end
 
       it 'creates a following relation without reblogs' do
       end
 
       it 'creates a following relation without reblogs' do


@@ -88,7 +88,7 @@ RSpec.describe FollowService, type: :service do
 
       before do
         sender.follow!(bob)
 
       before do
         sender.follow!(bob)

-        subject.call(sender, bob.acct)
+        subject.call(sender, bob)

       end
 
       it 'keeps a following relation' do
       end
 
       it 'keeps a following relation' do


@@ -101,7 +101,7 @@ RSpec.describe FollowService, type: :service do
 
       before do
         sender.follow!(bob, reblogs: true)
 
       before do
         sender.follow!(bob, reblogs: true)

-        subject.call(sender, bob.acct, reblogs: false)
+        subject.call(sender, bob, reblogs: false)

       end
 
       it 'disables reblogs' do
       end
 
       it 'disables reblogs' do


@@ -114,7 +114,7 @@ RSpec.describe FollowService, type: :service do
 
       before do
         sender.follow!(bob, reblogs: false)
 
       before do
         sender.follow!(bob, reblogs: false)

-        subject.call(sender, bob.acct, reblogs: true)
+        subject.call(sender, bob, reblogs: true)

       end
 
       it 'disables reblogs' do
       end
 
       it 'disables reblogs' do


@@ -128,7 +128,7 @@ RSpec.describe FollowService, type: :service do
 
     before do
       stub_request(:post, "http://example.com/inbox").to_return(:status => 200, :body => "", :headers => {})
 
     before do
       stub_request(:post, "http://example.com/inbox").to_return(:status => 200, :body => "", :headers => {})

-      subject.call(sender, bob.acct)
+      subject.call(sender, bob)

     end
 
     it 'creates follow request' do
     end
 
     it 'creates follow request' do

diff --git a/spec/services/resolve_account_service_spec.rb b/spec/services/resolve_account_service_spec.rb
index a604e90b549794e33836b0ace0d091ca02c53c81..7b1e8885ccf4181abd58b0ef1d9123e5c2ed3fd3 100644 (file)
--- a/spec/services/resolve_account_service_spec.rb
+++ b/spec/services/resolve_account_service_spec.rb
@@ -13,6 +13,47 @@ RSpec.describe ResolveAccountService, type: :service do
     stub_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:hoge@example.com').to_return(status: 410)
   end
 
     stub_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:hoge@example.com').to_return(status: 410)
   end
 

+  context 'using skip_webfinger' do
+    context 'when account is known' do
+      let!(:remote_account) { Fabricate(:account, username: 'foo', domain: 'ap.example.com', protocol: 'activitypub') }
+
+      context 'when domain is banned' do
+        let!(:domain_block) { Fabricate(:domain_block, domain: 'ap.example.com', severity: :suspend) }
+
+        it 'does not return an account' do
+          expect(subject.call('foo@ap.example.com', skip_webfinger: true)).to be_nil
+        end
+
+        it 'does not make a webfinger query' do
+          subject.call('foo@ap.example.com', skip_webfinger: true)
+          expect(a_request(:get, 'https://ap.example.com/.well-known/webfinger?resource=acct:foo@ap.example.com')).to_not have_been_made
+        end
+      end
+
+      context 'when domain is not banned' do
+        it 'returns the expected account' do
+          expect(subject.call('foo@ap.example.com', skip_webfinger: true)).to eq remote_account
+        end
+
+        it 'does not make a webfinger query' do
+          subject.call('foo@ap.example.com', skip_webfinger: true)
+          expect(a_request(:get, 'https://ap.example.com/.well-known/webfinger?resource=acct:foo@ap.example.com')).to_not have_been_made
+        end
+      end
+    end
+
+    context 'when account is not known' do
+      it 'does not return an account' do
+        expect(subject.call('foo@ap.example.com', skip_webfinger: true)).to be_nil
+      end
+
+      it 'does not make a webfinger query' do
+        subject.call('foo@ap.example.com', skip_webfinger: true)
+        expect(a_request(:get, 'https://ap.example.com/.well-known/webfinger?resource=acct:foo@ap.example.com')).to_not have_been_made
+      end
+    end
+  end
+

   context 'when there is an LRDD endpoint but no resolvable account' do
     before do
       stub_request(:get, "https://quitter.no/.well-known/host-meta").to_return(request_fixture('.host-meta.txt'))
   context 'when there is an LRDD endpoint but no resolvable account' do
     before do
       stub_request(:get, "https://quitter.no/.well-known/host-meta").to_return(request_fixture('.host-meta.txt'))