Skip to content
This repository has been archived by the owner on Dec 21, 2023. It is now read-only.

Commit

Permalink
Fix URI of repeat follow requests not being recorded (mastodon#15662)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@ClearlyClaire
ClearlyClaire authored Feb 11, 2021
1 parent f5fefdc commit be3b9f8
Show file tree
Hide file tree
Showing 2 changed files with 151 additions and 33 deletions.
13 changes: 11 additions & 2 deletions app/lib/activitypub/activity/follow.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,24 @@ class ActivityPub::Activity::Follow < ActivityPub::Activity
def perform
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?
reject_follow_request!(target_account)
return
end

# 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'])
return
end
Expand Down
171 changes: 140 additions & 31 deletions spec/lib/activitypub/activity/follow_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,62 +17,171 @@
describe '#perform' do
subject { described_class.new(json, sender) }

context 'unlocked account' do
before do
subject.perform
context 'with no prior follow' do
context 'unlocked account' do
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

it 'creates a follow from sender to recipient' do
expect(sender.following?(recipient)).to be true
context 'silenced account following an unlocked account' do
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

it 'does not create a follow request' do
expect(sender.requested?(recipient)).to be false
context 'unlocked account muting the sender' do
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

context 'silenced account following an unlocked account' do
context 'when a follow relationship already exists' do
before do
sender.touch(:silenced_at)
subject.perform
sender.active_relationships.create!(target_account: recipient, uri: 'bar')
end

it 'does not create a follow from sender to recipient' do
expect(sender.following?(recipient)).to be false
end
context 'unlocked account' do
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 'creates a follow request' do
expect(sender.requested?(recipient)).to be true
it 'does not create a follow request' do
expect(sender.requested?(recipient)).to be false
end
end
end

context 'unlocked account muting the sender' do
before do
recipient.mute!(sender)
subject.perform
context 'silenced account following an unlocked account' do
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

it 'creates a follow from sender to recipient' do
expect(sender.following?(recipient)).to be true
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

it 'does not create a follow request' do
expect(sender.requested?(recipient)).to be false
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

context 'locked account' do
context 'when a follow request already exists' do
before do
recipient.update(locked: true)
subject.perform
sender.follow_requests.create!(target_account: recipient, uri: 'bar')
end

it 'does not create a follow from sender to recipient' do
expect(sender.following?(recipient)).to be false
context 'silenced account following an unlocked account' do
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

it 'creates a follow request' do
expect(sender.requested?(recipient)).to be true
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 '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
end
Expand Down

0 comments on commit be3b9f8

Please sign in to comment.