Fix URI of repeat follow requests not being recorded (#15662)

* Fix URI of repeat follow requests not being recorded

In case we receive a “repeat” or “duplicate” follow request, we automatically
fast-forward the accept with the latest received Activity `id`, but we don't
record it.

In general, a “repeat” or “duplicate” follow request may happen if for some
reason (e.g. inconsistent handling of Block or Undo Accept activities, an
instance being brought back up from the dead, etc.) the local instance thought
the remote actor were following them while the remote actor thought otherwise.

In those cases, the remote instance does not know about the older Follow
activity `id`, so keeping that record serves no purpose, but knowing the most
recent one is useful if the remote implementation at some point refers to it
by `id` without inlining it.

* Add tests
This commit is contained in:
Claire 2021-02-11 01:53:44 +01:00 committed by GitHub
parent f5fefdc11a
commit be3b9f8151
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 154 additions and 36 deletions

View file

@ -6,7 +6,14 @@ class ActivityPub::Activity::Follow < ActivityPub::Activity
def perform def perform
target_account = account_from_uri(object_uri) target_account = account_from_uri(object_uri)
return if target_account.nil? || !target_account.local? || delete_arrived_first?(@json['id']) || @account.requested?(target_account) return if target_account.nil? || !target_account.local? || delete_arrived_first?(@json['id'])
# Update id of already-existing follow requests
existing_follow_request = ::FollowRequest.find_by(account: @account, target_account: target_account)
unless existing_follow_request.nil?
existing_follow_request.update!(uri: @json['id'])
return
end
if target_account.blocking?(@account) || target_account.domain_blocking?(@account.domain) || target_account.moved? || target_account.instance_actor? if target_account.blocking?(@account) || target_account.domain_blocking?(@account.domain) || target_account.moved? || target_account.instance_actor?
reject_follow_request!(target_account) reject_follow_request!(target_account)
@ -14,7 +21,9 @@ class ActivityPub::Activity::Follow < ActivityPub::Activity
end end
# Fast-forward repeat follow requests # Fast-forward repeat follow requests
if @account.following?(target_account) existing_follow = ::Follow.find_by(account: @account, target_account: target_account)
unless existing_follow.nil?
existing_follow.update!(uri: @json['id'])
AuthorizeFollowService.new.call(@account, target_account, skip_follow_request: true, follow_request_uri: @json['id']) AuthorizeFollowService.new.call(@account, target_account, skip_follow_request: true, follow_request_uri: @json['id'])
return return
end end

View file

@ -17,62 +17,171 @@ RSpec.describe ActivityPub::Activity::Follow do
describe '#perform' do describe '#perform' do
subject { described_class.new(json, sender) } subject { described_class.new(json, sender) }
context 'unlocked account' do context 'with no prior follow' do
before do context 'unlocked account' do
subject.perform before do
subject.perform
end
it 'creates a follow from sender to recipient' do
expect(sender.following?(recipient)).to be true
expect(sender.active_relationships.find_by(target_account: recipient).uri).to eq 'foo'
end
it 'does not create a follow request' do
expect(sender.requested?(recipient)).to be false
end
end end
it 'creates a follow from sender to recipient' do context 'silenced account following an unlocked account' do
expect(sender.following?(recipient)).to be true before do
sender.touch(:silenced_at)
subject.perform
end
it 'does not create a follow from sender to recipient' do
expect(sender.following?(recipient)).to be false
end
it 'creates a follow request' do
expect(sender.requested?(recipient)).to be true
expect(sender.follow_requests.find_by(target_account: recipient).uri).to eq 'foo'
end
end end
it 'does not create a follow request' do context 'unlocked account muting the sender' do
expect(sender.requested?(recipient)).to be false before do
recipient.mute!(sender)
subject.perform
end
it 'creates a follow from sender to recipient' do
expect(sender.following?(recipient)).to be true
expect(sender.active_relationships.find_by(target_account: recipient).uri).to eq 'foo'
end
it 'does not create a follow request' do
expect(sender.requested?(recipient)).to be false
end
end
context 'locked account' do
before do
recipient.update(locked: true)
subject.perform
end
it 'does not create a follow from sender to recipient' do
expect(sender.following?(recipient)).to be false
end
it 'creates a follow request' do
expect(sender.requested?(recipient)).to be true
expect(sender.follow_requests.find_by(target_account: recipient).uri).to eq 'foo'
end
end end
end end
context 'silenced account following an unlocked account' do context 'when a follow relationship already exists' do
before do before do
sender.touch(:silenced_at) sender.active_relationships.create!(target_account: recipient, uri: 'bar')
subject.perform
end end
it 'does not create a follow from sender to recipient' do context 'unlocked account' do
expect(sender.following?(recipient)).to be false before do
subject.perform
end
it 'correctly sets the new URI' do
expect(sender.active_relationships.find_by(target_account: recipient).uri).to eq 'foo'
end
it 'does not create a follow request' do
expect(sender.requested?(recipient)).to be false
end
end end
it 'creates a follow request' do context 'silenced account following an unlocked account' do
expect(sender.requested?(recipient)).to be true before do
sender.touch(:silenced_at)
subject.perform
end
it 'correctly sets the new URI' do
expect(sender.active_relationships.find_by(target_account: recipient).uri).to eq 'foo'
end
it 'does not create a follow request' do
expect(sender.requested?(recipient)).to be false
end
end
context 'unlocked account muting the sender' do
before do
recipient.mute!(sender)
subject.perform
end
it 'correctly sets the new URI' do
expect(sender.active_relationships.find_by(target_account: recipient).uri).to eq 'foo'
end
it 'does not create a follow request' do
expect(sender.requested?(recipient)).to be false
end
end
context 'locked account' do
before do
recipient.update(locked: true)
subject.perform
end
it 'correctly sets the new URI' do
expect(sender.active_relationships.find_by(target_account: recipient).uri).to eq 'foo'
end
it 'does not create a follow request' do
expect(sender.requested?(recipient)).to be false
end
end end
end end
context 'unlocked account muting the sender' do context 'when a follow request already exists' do
before do before do
recipient.mute!(sender) sender.follow_requests.create!(target_account: recipient, uri: 'bar')
subject.perform
end end
it 'creates a follow from sender to recipient' do context 'silenced account following an unlocked account' do
expect(sender.following?(recipient)).to be true before do
sender.touch(:silenced_at)
subject.perform
end
it 'does not create a follow from sender to recipient' do
expect(sender.following?(recipient)).to be false
end
it 'correctly sets the new URI' do
expect(sender.requested?(recipient)).to be true
expect(sender.follow_requests.find_by(target_account: recipient).uri).to eq 'foo'
end
end end
it 'does not create a follow request' do context 'locked account' do
expect(sender.requested?(recipient)).to be false before do
end recipient.update(locked: true)
end subject.perform
end
context 'locked account' do it 'does not create a follow from sender to recipient' do
before do expect(sender.following?(recipient)).to be false
recipient.update(locked: true) end
subject.perform
end
it 'does not create a follow from sender to recipient' do it 'correctly sets the new URI' do
expect(sender.following?(recipient)).to be false expect(sender.requested?(recipient)).to be true
end expect(sender.follow_requests.find_by(target_account: recipient).uri).to eq 'foo'
end
it 'creates a follow request' do
expect(sender.requested?(recipient)).to be true
end end
end end
end end