From 5efb1ff337e7c1c090eb9c43dc3ba5b284302460 Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 11 Aug 2021 17:48:42 +0200 Subject: Fix followers synchronization mechanism not working when URI has empty path (#16510) * Fix followers synchronization mechanism not working when URI has empty path To my knowledge, there is no current implementation on the fediverse that can use bare domains (e.g., actor is at https://example.org instead of something like https://example.org/actor) that also plans to support the followers synchronization mechanism. However, Mastodon's current implementation would exclude such accounts from followers list. Also adds tests and rename them to reflect the proper method names. * Move url prefix regexp to its own constant --- spec/models/concerns/account_interactions_spec.rb | 61 +++++++++++++---------- spec/workers/activitypub/delivery_worker_spec.rb | 2 +- 2 files changed, 37 insertions(+), 26 deletions(-) (limited to 'spec') diff --git a/spec/models/concerns/account_interactions_spec.rb b/spec/models/concerns/account_interactions_spec.rb index 85fbf7e79..ca243ebc5 100644 --- a/spec/models/concerns/account_interactions_spec.rb +++ b/spec/models/concerns/account_interactions_spec.rb @@ -539,46 +539,57 @@ describe AccountInteractions do end end - describe '#followers_hash' do + describe '#remote_followers_hash' do let(:me) { Fabricate(:account, username: 'Me') } let(:remote_1) { Fabricate(:account, username: 'alice', domain: 'example.org', uri: 'https://example.org/users/alice') } let(:remote_2) { Fabricate(:account, username: 'bob', domain: 'example.org', uri: 'https://example.org/users/bob') } - let(:remote_3) { Fabricate(:account, username: 'eve', domain: 'foo.org', uri: 'https://foo.org/users/eve') } + let(:remote_3) { Fabricate(:account, username: 'instance-actor', domain: 'example.org', uri: 'https://example.org') } + let(:remote_4) { Fabricate(:account, username: 'eve', domain: 'foo.org', uri: 'https://foo.org/users/eve') } before do remote_1.follow!(me) remote_2.follow!(me) remote_3.follow!(me) + remote_4.follow!(me) me.follow!(remote_1) end - context 'on a local user' do - it 'returns correct hash for remote domains' do - expect(me.remote_followers_hash('https://example.org/')).to eq '707962e297b7bd94468a21bc8e506a1bcea607a9142cd64e27c9b106b2a5f6ec' - expect(me.remote_followers_hash('https://foo.org/')).to eq 'ccb9c18a67134cfff9d62c7f7e7eb88e6b803446c244b84265565f4eba29df0e' - end + it 'returns correct hash for remote domains' do + expect(me.remote_followers_hash('https://example.org/')).to eq '20aecbe774b3d61c25094370baf370012b9271c5b172ecedb05caff8d79ef0c7' + expect(me.remote_followers_hash('https://foo.org/')).to eq 'ccb9c18a67134cfff9d62c7f7e7eb88e6b803446c244b84265565f4eba29df0e' + expect(me.remote_followers_hash('https://foo.org.evil.com/')).to eq '0000000000000000000000000000000000000000000000000000000000000000' + expect(me.remote_followers_hash('https://foo')).to eq '0000000000000000000000000000000000000000000000000000000000000000' + end - it 'invalidates cache as needed when removing or adding followers' do - expect(me.remote_followers_hash('https://example.org/')).to eq '707962e297b7bd94468a21bc8e506a1bcea607a9142cd64e27c9b106b2a5f6ec' - remote_1.unfollow!(me) - expect(me.remote_followers_hash('https://example.org/')).to eq '241b00794ce9b46aa864f3220afadef128318da2659782985bac5ed5bd436bff' - remote_1.follow!(me) - expect(me.remote_followers_hash('https://example.org/')).to eq '707962e297b7bd94468a21bc8e506a1bcea607a9142cd64e27c9b106b2a5f6ec' - end + it 'invalidates cache as needed when removing or adding followers' do + expect(me.remote_followers_hash('https://example.org/')).to eq '20aecbe774b3d61c25094370baf370012b9271c5b172ecedb05caff8d79ef0c7' + remote_3.unfollow!(me) + expect(me.remote_followers_hash('https://example.org/')).to eq '707962e297b7bd94468a21bc8e506a1bcea607a9142cd64e27c9b106b2a5f6ec' + remote_1.unfollow!(me) + expect(me.remote_followers_hash('https://example.org/')).to eq '241b00794ce9b46aa864f3220afadef128318da2659782985bac5ed5bd436bff' + remote_1.follow!(me) + expect(me.remote_followers_hash('https://example.org/')).to eq '707962e297b7bd94468a21bc8e506a1bcea607a9142cd64e27c9b106b2a5f6ec' end + end - context 'on a remote user' do - it 'returns correct hash for remote domains' do - expect(remote_1.local_followers_hash).to eq Digest::SHA256.hexdigest(ActivityPub::TagManager.instance.uri_for(me)) - end + describe '#local_followers_hash' do + let(:me) { Fabricate(:account, username: 'Me') } + let(:remote_1) { Fabricate(:account, username: 'alice', domain: 'example.org', uri: 'https://example.org/users/alice') } - it 'invalidates cache as needed when removing or adding followers' do - expect(remote_1.local_followers_hash).to eq Digest::SHA256.hexdigest(ActivityPub::TagManager.instance.uri_for(me)) - me.unfollow!(remote_1) - expect(remote_1.local_followers_hash).to eq '0000000000000000000000000000000000000000000000000000000000000000' - me.follow!(remote_1) - expect(remote_1.local_followers_hash).to eq Digest::SHA256.hexdigest(ActivityPub::TagManager.instance.uri_for(me)) - end + before do + me.follow!(remote_1) + end + + it 'returns correct hash for local users' do + expect(remote_1.local_followers_hash).to eq Digest::SHA256.hexdigest(ActivityPub::TagManager.instance.uri_for(me)) + end + + it 'invalidates cache as needed when removing or adding followers' do + expect(remote_1.local_followers_hash).to eq Digest::SHA256.hexdigest(ActivityPub::TagManager.instance.uri_for(me)) + me.unfollow!(remote_1) + expect(remote_1.local_followers_hash).to eq '0000000000000000000000000000000000000000000000000000000000000000' + me.follow!(remote_1) + expect(remote_1.local_followers_hash).to eq Digest::SHA256.hexdigest(ActivityPub::TagManager.instance.uri_for(me)) end end diff --git a/spec/workers/activitypub/delivery_worker_spec.rb b/spec/workers/activitypub/delivery_worker_spec.rb index f4633731e..d39393d50 100644 --- a/spec/workers/activitypub/delivery_worker_spec.rb +++ b/spec/workers/activitypub/delivery_worker_spec.rb @@ -11,7 +11,7 @@ describe ActivityPub::DeliveryWorker do let(:payload) { 'test' } before do - allow_any_instance_of(Account).to receive(:remote_followers_hash).with('https://example.com/').and_return('somehash') + allow_any_instance_of(Account).to receive(:remote_followers_hash).with('https://example.com/api').and_return('somehash') end describe 'perform' do -- cgit From 67021484726e88040c5c1502dddd0eef45734c6b Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 20 Aug 2021 11:53:33 +0200 Subject: Add tests for SuspendAccountService and UnsuspendAccountService (#16627) * Add tests for SuspendAccountService * Add tests for UnsuspendAccountService --- spec/services/suspend_account_service_spec.rb | 85 +++++++++++++++ spec/services/unsuspend_account_service_spec.rb | 135 ++++++++++++++++++++++++ 2 files changed, 220 insertions(+) create mode 100644 spec/services/suspend_account_service_spec.rb create mode 100644 spec/services/unsuspend_account_service_spec.rb (limited to 'spec') diff --git a/spec/services/suspend_account_service_spec.rb b/spec/services/suspend_account_service_spec.rb new file mode 100644 index 000000000..cf7eb257a --- /dev/null +++ b/spec/services/suspend_account_service_spec.rb @@ -0,0 +1,85 @@ +require 'rails_helper' + +RSpec.describe SuspendAccountService, type: :service do + shared_examples 'common behavior' do + let!(:local_follower) { Fabricate(:user, current_sign_in_at: 1.hour.ago).account } + let!(:list) { Fabricate(:list, account: local_follower) } + + subject do + -> { described_class.new.call(account) } + end + + before do + allow(FeedManager.instance).to receive(:unmerge_from_home).and_return(nil) + allow(FeedManager.instance).to receive(:unmerge_from_list).and_return(nil) + + local_follower.follow!(account) + list.accounts << account + end + + it "unmerges from local followers' feeds" do + subject.call + expect(FeedManager.instance).to have_received(:unmerge_from_home).with(account, local_follower) + expect(FeedManager.instance).to have_received(:unmerge_from_list).with(account, list) + end + + it 'marks account as suspended' do + is_expected.to change { account.suspended? }.from(false).to(true) + end + end + + describe 'suspending a local account' do + def match_update_actor_request(req, account) + json = JSON.parse(req.body) + actor_id = ActivityPub::TagManager.instance.uri_for(account) + json['type'] == 'Update' && json['actor'] == actor_id && json['object']['id'] == actor_id && json['object']['suspended'] + end + + before do + stub_request(:post, 'https://alice.com/inbox').to_return(status: 201) + stub_request(:post, 'https://bob.com/inbox').to_return(status: 201) + end + + include_examples 'common behavior' do + let!(:account) { Fabricate(:account) } + let!(:remote_follower) { Fabricate(:account, uri: 'https://alice.com', inbox_url: 'https://alice.com/inbox', protocol: :activitypub) } + let!(:remote_reporter) { Fabricate(:account, uri: 'https://bob.com', inbox_url: 'https://bob.com/inbox', protocol: :activitypub) } + let!(:report) { Fabricate(:report, account: remote_reporter, target_account: account) } + + before do + remote_follower.follow!(account) + end + + it 'sends an update actor to followers and reporters' do + subject.call + expect(a_request(:post, remote_follower.inbox_url).with { |req| match_update_actor_request(req, account) }).to have_been_made.once + expect(a_request(:post, remote_reporter.inbox_url).with { |req| match_update_actor_request(req, account) }).to have_been_made.once + end + end + end + + describe 'suspending a remote account' do + def match_reject_follow_request(req, account, followee) + json = JSON.parse(req.body) + json['type'] == 'Reject' && json['actor'] == ActivityPub::TagManager.instance.uri_for(followee) && json['object']['actor'] == account.uri + end + + before do + stub_request(:post, 'https://bob.com/inbox').to_return(status: 201) + end + + include_examples 'common behavior' do + let!(:account) { Fabricate(:account, domain: 'bob.com', uri: 'https://bob.com', inbox_url: 'https://bob.com/inbox', protocol: :activitypub) } + let!(:local_followee) { Fabricate(:account) } + + before do + account.follow!(local_followee) + end + + it 'sends a reject follow' do + subject.call + expect(a_request(:post, account.inbox_url).with { |req| match_reject_follow_request(req, account, local_followee) }).to have_been_made.once + end + end + end +end diff --git a/spec/services/unsuspend_account_service_spec.rb b/spec/services/unsuspend_account_service_spec.rb new file mode 100644 index 000000000..d52cb6cc0 --- /dev/null +++ b/spec/services/unsuspend_account_service_spec.rb @@ -0,0 +1,135 @@ +require 'rails_helper' + +RSpec.describe UnsuspendAccountService, type: :service do + shared_examples 'common behavior' do + let!(:local_follower) { Fabricate(:user, current_sign_in_at: 1.hour.ago).account } + let!(:list) { Fabricate(:list, account: local_follower) } + + subject do + -> { described_class.new.call(account) } + end + + before do + allow(FeedManager.instance).to receive(:merge_into_home).and_return(nil) + allow(FeedManager.instance).to receive(:merge_into_list).and_return(nil) + + local_follower.follow!(account) + list.accounts << account + + account.suspend!(origin: :local) + end + end + + describe 'unsuspending a local account' do + def match_update_actor_request(req, account) + json = JSON.parse(req.body) + actor_id = ActivityPub::TagManager.instance.uri_for(account) + json['type'] == 'Update' && json['actor'] == actor_id && json['object']['id'] == actor_id && !json['object']['suspended'] + end + + before do + stub_request(:post, 'https://alice.com/inbox').to_return(status: 201) + stub_request(:post, 'https://bob.com/inbox').to_return(status: 201) + end + + it 'marks account as unsuspended' do + is_expected.to change { account.suspended? }.from(true).to(false) + end + + include_examples 'common behavior' do + let!(:account) { Fabricate(:account) } + let!(:remote_follower) { Fabricate(:account, uri: 'https://alice.com', inbox_url: 'https://alice.com/inbox', protocol: :activitypub) } + let!(:remote_reporter) { Fabricate(:account, uri: 'https://bob.com', inbox_url: 'https://bob.com/inbox', protocol: :activitypub) } + let!(:report) { Fabricate(:report, account: remote_reporter, target_account: account) } + + before do + remote_follower.follow!(account) + end + + it "merges back into local followers' feeds" do + subject.call + expect(FeedManager.instance).to have_received(:merge_into_home).with(account, local_follower) + expect(FeedManager.instance).to have_received(:merge_into_list).with(account, list) + end + + it 'sends an update actor to followers and reporters' do + subject.call + expect(a_request(:post, remote_follower.inbox_url).with { |req| match_update_actor_request(req, account) }).to have_been_made.once + expect(a_request(:post, remote_reporter.inbox_url).with { |req| match_update_actor_request(req, account) }).to have_been_made.once + end + end + end + + describe 'unsuspending a remote account' do + include_examples 'common behavior' do + let!(:account) { Fabricate(:account, domain: 'bob.com', uri: 'https://bob.com', inbox_url: 'https://bob.com/inbox', protocol: :activitypub) } + let!(:reslove_account_service) { double } + + before do + allow(ResolveAccountService).to receive(:new).and_return(reslove_account_service) + end + + context 'when the account is not remotely suspended' do + before do + allow(reslove_account_service).to receive(:call).with(account).and_return(account) + end + + it 're-fetches the account' do + subject.call + expect(reslove_account_service).to have_received(:call).with(account) + end + + it "merges back into local followers' feeds" do + subject.call + expect(FeedManager.instance).to have_received(:merge_into_home).with(account, local_follower) + expect(FeedManager.instance).to have_received(:merge_into_list).with(account, list) + end + + it 'marks account as unsuspended' do + is_expected.to change { account.suspended? }.from(true).to(false) + end + end + + context 'when the account is remotely suspended' do + before do + allow(reslove_account_service).to receive(:call).with(account) do |account| + account.suspend!(origin: :remote) + account + end + end + + it 're-fetches the account' do + subject.call + expect(reslove_account_service).to have_received(:call).with(account) + end + + it "does not merge back into local followers' feeds" do + subject.call + expect(FeedManager.instance).to_not have_received(:merge_into_home).with(account, local_follower) + expect(FeedManager.instance).to_not have_received(:merge_into_list).with(account, list) + end + + it 'does not mark the account as unsuspended' do + is_expected.not_to change { account.suspended? } + end + end + + context 'when the account is remotely deleted' do + before do + allow(reslove_account_service).to receive(:call).with(account).and_return(nil) + end + + it 're-fetches the account' do + subject.call + expect(reslove_account_service).to have_received(:call).with(account) + end + + it "does not merge back into local followers' feeds" do + subject.call + expect(FeedManager.instance).to_not have_received(:merge_into_home).with(account, local_follower) + expect(FeedManager.instance).to_not have_received(:merge_into_list).with(account, list) + end + end + end + end +end -- cgit