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
This commit is contained in:
Claire 2021-02-24 06:32:13 +01:00 committed by GitHub
parent eb83ab45e2
commit 5f4c0b79c2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 60 additions and 19 deletions

View file

@ -3,10 +3,11 @@
class FollowService < BaseService class FollowService < BaseService
include Redisable include Redisable
include Payloadable include Payloadable
include DomainControlHelper
# Follow a remote user, notify remote user about the follow # Follow a remote user, notify remote user about the follow
# @param [Account] source_account From which to 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 # @param [Hash] options
# @option [Boolean] :reblogs Whether or not to show reblogs, defaults to true # @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 # @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 # @option [Boolean] :with_rate_limit
def call(source_account, target_account, options = {}) def call(source_account, target_account, options = {})
@source_account = source_account @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) @options = { bypass_locked: false, bypass_limit: false, with_rate_limit: false }.merge(options)
raise ActiveRecord::RecordNotFound if following_not_possible? raise ActiveRecord::RecordNotFound if following_not_possible?
@ -43,7 +44,7 @@ class FollowService < BaseService
end end
def following_not_allowed? 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 end
def change_follow_options! def change_follow_options!

View file

@ -10,7 +10,7 @@ class ResolveAccountService < BaseService
# @param [String, Account] uri URI in the username@domain format or account record # @param [String, Account] uri URI in the username@domain format or account record
# @param [Hash] options # @param [Hash] options
# @option options [Boolean] :redirected Do not follow further Webfinger redirects # @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] # @return [Account]
def call(uri, options = {}) def call(uri, options = {})
return if uri.blank? return if uri.blank?
@ -120,8 +120,9 @@ class ResolveAccountService < BaseService
def webfinger_update_due? def webfinger_update_due?
return false if @options[:check_delivery_availability] && !DeliveryFailureTracker.available?(@domain) 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 end
def activitypub_ready? def activitypub_ready?

View file

@ -8,7 +8,7 @@ RSpec.describe Api::V1::FollowRequestsController, type: :controller do
let(:follower) { Fabricate(:account, username: 'bob') } let(:follower) { Fabricate(:account, username: 'bob') }
before do before do
FollowService.new.call(follower, user.account.acct) FollowService.new.call(follower, user.account)
allow(controller).to receive(:doorkeeper_token) { token } allow(controller).to receive(:doorkeeper_token) { token }
end end

View file

@ -57,7 +57,7 @@ RSpec.describe Api::V1::NotificationsController, type: :controller do
@mention_from_status = mentioning_status.mentions.first @mention_from_status = mentioning_status.mentions.first
@favourite = FavouriteService.new.call(other.account, first_status) @favourite = FavouriteService.new.call(other.account, first_status)
@second_favourite = FavouriteService.new.call(third.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 end
describe 'with no options' do describe 'with no options' do

View file

@ -99,12 +99,10 @@ describe AuthorizeInteractionsController do
allow(ResolveAccountService).to receive(:new).and_return(service) 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('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(account.following?(target_account)).to be true
expect(response).to render_template(:success) expect(response).to render_template(:success)
end end

View file

@ -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 } let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, locked: true, username: 'bob')).account }
before do before do
subject.call(sender, bob.acct) subject.call(sender, bob)
end end
it 'creates a follow request with reblogs' do 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 } let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, locked: true, username: 'bob')).account }
before do before do
subject.call(sender, bob.acct, reblogs: false) subject.call(sender, bob, reblogs: false)
end end
it 'creates a follow request without reblogs' do it 'creates a follow request without reblogs' do
@ -35,7 +35,7 @@ RSpec.describe FollowService, type: :service do
before do before do
sender.touch(:silenced_at) sender.touch(:silenced_at)
subject.call(sender, bob.acct) subject.call(sender, bob)
end end
it 'creates a follow request with reblogs' do it 'creates a follow request with reblogs' do
@ -48,7 +48,7 @@ RSpec.describe FollowService, type: :service do
before do before do
bob.mute!(sender) bob.mute!(sender)
subject.call(sender, bob.acct) subject.call(sender, bob)
end end
it 'creates a following relation with reblogs' do 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 } let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob')).account }
before do before do
subject.call(sender, bob.acct) subject.call(sender, bob)
end end
it 'creates a following relation with reblogs' do 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 } let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob')).account }
before do before do
subject.call(sender, bob.acct, reblogs: false) subject.call(sender, bob, reblogs: false)
end end
it 'creates a following relation without reblogs' do it 'creates a following relation without reblogs' do
@ -88,7 +88,7 @@ RSpec.describe FollowService, type: :service do
before do before do
sender.follow!(bob) sender.follow!(bob)
subject.call(sender, bob.acct) subject.call(sender, bob)
end end
it 'keeps a following relation' do it 'keeps a following relation' do
@ -101,7 +101,7 @@ RSpec.describe FollowService, type: :service do
before do before do
sender.follow!(bob, reblogs: true) sender.follow!(bob, reblogs: true)
subject.call(sender, bob.acct, reblogs: false) subject.call(sender, bob, reblogs: false)
end end
it 'disables reblogs' do it 'disables reblogs' do
@ -114,7 +114,7 @@ RSpec.describe FollowService, type: :service do
before do before do
sender.follow!(bob, reblogs: false) sender.follow!(bob, reblogs: false)
subject.call(sender, bob.acct, reblogs: true) subject.call(sender, bob, reblogs: true)
end end
it 'disables reblogs' do it 'disables reblogs' do
@ -128,7 +128,7 @@ RSpec.describe FollowService, type: :service do
before do before do
stub_request(:post, "http://example.com/inbox").to_return(:status => 200, :body => "", :headers => {}) stub_request(:post, "http://example.com/inbox").to_return(:status => 200, :body => "", :headers => {})
subject.call(sender, bob.acct) subject.call(sender, bob)
end end
it 'creates follow request' do it 'creates follow request' do

View file

@ -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) stub_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:hoge@example.com').to_return(status: 410)
end 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 context 'when there is an LRDD endpoint but no resolvable account' do
before do before do
stub_request(:get, "https://quitter.no/.well-known/host-meta").to_return(request_fixture('.host-meta.txt')) stub_request(:get, "https://quitter.no/.well-known/host-meta").to_return(request_fixture('.host-meta.txt'))