From ea3d8a82c244cccb3c3a7673c206e3668fd3a8d3 Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 10 Jul 2026 16:16:46 +0200 Subject: [PATCH] Refactor `ActivityPub::ProcessAccountService` and `ActivityPub::FetchRemoteActorService` (#39783) --- .../concerns/signature_verification.rb | 2 +- app/lib/activitypub/activity/update.rb | 2 +- .../activitypub/fetch_remote_actor_service.rb | 43 +---- .../activitypub/process_account_service.rb | 113 ++++++++++--- .../fetch_remote_account_service_spec.rb | 6 +- .../fetch_remote_actor_service_spec.rb | 6 +- .../process_account_service_spec.rb | 151 ++++++++++++------ 7 files changed, 199 insertions(+), 124 deletions(-) diff --git a/app/controllers/concerns/signature_verification.rb b/app/controllers/concerns/signature_verification.rb index 1a876b0cade..54bc2c48adf 100644 --- a/app/controllers/concerns/signature_verification.rb +++ b/app/controllers/concerns/signature_verification.rb @@ -110,7 +110,7 @@ module SignatureVerification end rescue Mastodon::PrivateNetworkAddressError => e raise Mastodon::SignatureVerificationError, "Requests to private network addresses are disallowed (tried to query #{e.host})" - rescue Mastodon::HostValidationError, ActivityPub::FetchRemoteActorService::Error, ActivityPub::FetchRemoteKeyService::Error, Webfinger::Error => e + rescue Mastodon::HostValidationError, ActivityPub::ProcessAccountService::Error, ActivityPub::FetchRemoteActorService::Error, ActivityPub::FetchRemoteKeyService::Error, Webfinger::Error => e raise Mastodon::SignatureVerificationError, e.message end diff --git a/app/lib/activitypub/activity/update.rb b/app/lib/activitypub/activity/update.rb index 87d5a5fad56..e051cacc9d1 100644 --- a/app/lib/activitypub/activity/update.rb +++ b/app/lib/activitypub/activity/update.rb @@ -23,7 +23,7 @@ class ActivityPub::Activity::Update < ActivityPub::Activity def update_account return reject_payload! if @account.uri != object_uri - ActivityPub::ProcessAccountService.new.call(@account.username, @account.domain, @object, signed_with_known_key: true, request_id: @options[:request_id]) + ActivityPub::ProcessAccountService.new.call(@object, account: @account, signed_with_known_key: true, request_id: @options[:request_id]) end def update_status diff --git a/app/services/activitypub/fetch_remote_actor_service.rb b/app/services/activitypub/fetch_remote_actor_service.rb index f5f00ef120e..efb9b9b64b8 100644 --- a/app/services/activitypub/fetch_remote_actor_service.rb +++ b/app/services/activitypub/fetch_remote_actor_service.rb @@ -31,53 +31,14 @@ class ActivityPub::FetchRemoteActorService < BaseService @uri = @json['id'] - # FEP-2c59 defines a `webfinger` attribute that makes things more explicit and spares an extra request in some cases. - # It supersedes `preferredUsername`. - @username, @domain = split_acct(@json['webfinger']) if @json['webfinger'].present? && @json['webfinger'].is_a?(String) - - if @username.blank? || @domain.blank? - raise "Actor #{uri} has no `preferredUsername`, and either a bogus or missing `webfinger`, which is a requirement for Mastodon compatibility" if @json['preferredUsername'].blank? - - Rails.logger.debug { "Actor #{uri} has an invalid `webfinger` value, falling back to `preferredUsername`" } if @json['webfinger'].present? - @username = @json['preferredUsername'] - @domain = Addressable::URI.parse(@uri).normalized_host - end - - check_webfinger! unless only_key - - ActivityPub::ProcessAccountService.new.call(@username, @domain, @json, only_key: only_key, verified_webfinger: !only_key, request_id: request_id) - rescue Error => e + ActivityPub::ProcessAccountService.new.call(@json, only_key:, request_id:, suppress_errors:) + rescue Error, ActivityPub::ProcessAccountService::Error => e Rails.logger.debug { "Fetching actor #{uri} failed: #{e.message}" } raise unless suppress_errors end private - def check_webfinger! - webfinger = Webfinger.new("acct:#{@username}@#{@domain}").perform - confirmed_username, confirmed_domain = split_acct(webfinger.subject) - - if @username.casecmp(confirmed_username).zero? && @domain.casecmp(confirmed_domain).zero? - raise Error, "Webfinger response for #{@username}@#{@domain} does not loop back to #{@uri}" if webfinger.self_link_href != @uri - - return - end - - webfinger = Webfinger.new("acct:#{confirmed_username}@#{confirmed_domain}").perform - @username, @domain = split_acct(webfinger.subject) - - raise Webfinger::RedirectError, "Too many webfinger redirects for URI #{@uri} (stopped at #{@username}@#{@domain})" unless confirmed_username.casecmp(@username).zero? && confirmed_domain.casecmp(@domain).zero? - raise Error, "Webfinger response for #{@username}@#{@domain} does not loop back to #{@uri}" if webfinger.self_link_href != @uri - rescue Webfinger::RedirectError => e - raise Error, e.message - rescue Webfinger::Error => e - raise Error, "Webfinger error when resolving #{@username}@#{@domain}: #{e.message}" - end - - def split_acct(acct) - acct.delete_prefix('acct:').split('@') - end - def supported_context? super(@json) end diff --git a/app/services/activitypub/process_account_service.rb b/app/services/activitypub/process_account_service.rb index db18bdfc091..ec44d6391b3 100644 --- a/app/services/activitypub/process_account_service.rb +++ b/app/services/activitypub/process_account_service.rb @@ -16,29 +16,44 @@ class ActivityPub::ProcessAccountService < BaseService VALID_URI_SCHEMES = %w(http https).freeze - # Should be called with confirmed valid JSON - # and WebFinger-resolved username and domain - def call(username, domain, json, options = {}) - return if json['inbox'].blank? || unsupported_uri_scheme?(json['id']) || domain_not_allowed?(domain) + class Error < StandardError; end + + def call(json, request_id: nil, only_key: false, signed_with_known_key: false, account: nil, suppress_errors: true) + raise Error, "Actor #{json['id']} has unsupported URI scheme" if unsupported_uri_scheme?(json['id']) + raise Error, "Actor #{json['id']} has no inbox" if json['inbox'].blank? + raise Error, "Actor #{json['id']} does not correspond to provided Account (#{account.uri})" if account.present? && account.uri != json['id'] + return if domain_not_allowed?(json['id']) || account&.local? - @options = options @json = json @uri = @json['id'] - @username = username - @domain = TagManager.instance.normalize_domain(domain) + @account = account + @only_key = only_key + @webfinger_verified = false + + if @account.present? + # TODO: handle renaming + @username = account.username + @domain = account.domain + else + extract_username_and_domain! + end + + @domain = TagManager.instance.normalize_domain(@domain) + return if @account.nil? && domain_not_allowed?(@domain) + @collections = {} # The key does not need to be unguessable, it just needs to be somewhat unique - @options[:request_id] ||= "#{Time.now.utc.to_i}-#{username}@#{domain}" + @request_id = request_id || "#{Time.now.utc.to_i}-#{@username}@#{@domain}" with_redis_lock("process_account:#{@uri}") do - if @options[:only_key] + if @only_key # `only_key` is used to update an existing account known by its `uri`. # Lookup by handle and new account creation do not make sense in this case. - @account = Account.remote.find_by(uri: @uri) + @account ||= Account.remote.find_by(uri: @uri) return if @account.nil? else - @account = Account.find_remote(@username, @domain) + @account ||= Account.find_remote(@username, @domain) end @old_public_keys = @account.present? ? (@account.keypairs.pluck(:public_key) + [@account.public_key.presence].compact) : [] @@ -49,8 +64,8 @@ class ActivityPub::ProcessAccountService < BaseService with_redis do |redis| return nil if redis.pfcount("unique_subdomains_for:#{PublicSuffix.domain(@domain, ignore_private: true)}") >= SUBDOMAINS_RATELIMIT - discoveries = redis.incr("discovery_per_request:#{@options[:request_id]}") - redis.expire("discovery_per_request:#{@options[:request_id]}", 5.minutes.seconds) + discoveries = redis.incr("discovery_per_request:#{@request_id}") + redis.expire("discovery_per_request:#{@request_id}", 5.minutes.seconds) return nil if discoveries > DISCOVERIES_PER_REQUEST end @@ -60,16 +75,16 @@ class ActivityPub::ProcessAccountService < BaseService update_account process_tags - process_duplicate_accounts! if @options[:verified_webfinger] + process_duplicate_accounts! if @webfinger_verified end after_protocol_change! if protocol_changed? - after_key_change! if all_public_keys_changed? && !@options[:signed_with_known_key] + after_key_change! if all_public_keys_changed? && !signed_with_known_key # TODO: maybe tie tombstones to specific keys? i.e. we don't need to keep tombstones if all keys changed clear_tombstones! if all_public_keys_changed? after_suspension_change! if suspension_changed? - unless @options[:only_key] || @account.suspended? + unless @only_key || @account.suspended? check_featured_collection! if @json['featured'].present? check_featured_tags_collection! if @json['featuredTags'].present? check_featured_collections_collection! if @json['featuredCollections'].present? @@ -77,13 +92,62 @@ class ActivityPub::ProcessAccountService < BaseService end @account - rescue JSON::ParserError - nil + rescue JSON::ParserError => e + raise Error, "Error parsing JSON for actor #{json['id']}: #{e}" unless suppress_errors + rescue Error + raise unless suppress_errors end private + def extract_username_and_domain! + # FEP-2c59 defines a `webfinger` attribute that makes things more explicit and spares an extra request in some cases. + # It supersedes `preferredUsername`. + @username, @domain = split_acct(@json['webfinger']) if @json['webfinger'].present? && @json['webfinger'].is_a?(String) + + if @username.blank? || @domain.blank? + raise Error, "Actor #{@uri} has no `preferredUsername`, and either a bogus or missing `webfinger`, which is a requirement for Mastodon compatibility" if @json['preferredUsername'].blank? + + Rails.logger.debug { "Actor #{@uri} has an invalid `webfinger` value, falling back to `preferredUsername`" } if @json['webfinger'].present? + @username = @json['preferredUsername'] + @domain = Addressable::URI.parse(@uri).normalized_host + end + + check_webfinger! unless @only_key + end + + def check_webfinger! + webfinger = Webfinger.new("acct:#{@username}@#{@domain}").perform + confirmed_username, confirmed_domain = split_acct(webfinger.subject) + + if @username.casecmp(confirmed_username).zero? && @domain.casecmp(confirmed_domain).zero? + raise Error, "Webfinger response for #{@username}@#{@domain} does not loop back to #{@uri}" if webfinger.self_link_href != @uri + + @webfinger_verified = true + + return + end + + webfinger = Webfinger.new("acct:#{confirmed_username}@#{confirmed_domain}").perform + @username, @domain = split_acct(webfinger.subject) + + raise Webfinger::RedirectError, "Too many webfinger redirects for URI #{@uri} (stopped at #{@username}@#{@domain})" unless confirmed_username.casecmp(@username).zero? && confirmed_domain.casecmp(@domain).zero? + raise Error, "Webfinger response for #{@username}@#{@domain} does not loop back to #{@uri}" if webfinger.self_link_href != @uri + + @webfinger_verified = true + rescue Webfinger::RedirectError => e + raise Error, e.message + rescue Webfinger::Error => e + raise Error, "Webfinger error when resolving #{@username}@#{@domain}: #{e.message}" + end + + def split_acct(acct) + acct.delete_prefix('acct:').split('@') + end + def create_account + raise 'Attempting to create an account without having verified its webfinger handle' unless @webfinger_verified + @account = Account.new @account.protocol = :activitypub @account.username = @username @@ -99,14 +163,17 @@ class ActivityPub::ProcessAccountService < BaseService end def update_account - @account.last_webfingered_at = Time.now.utc unless @options[:only_key] + # NOTE: `last_webfingered_at` is a misnomer, it is meant to record when + # the profile has last been fully updated, which was historically tied to webfinger queries. + # Hence why we use `@only_key` and not `@webfinger_verified` + @account.last_webfingered_at = Time.now.utc unless @only_key @account.protocol = :activitypub set_suspension! set_immediate_protocol_attributes! set_fetchable_key! unless @account.suspended? && @account.suspension_origin_local? set_immediate_attributes! unless @account.suspended? - set_fetchable_attributes! unless @options[:only_key] || @account.suspended? + set_fetchable_attributes! unless @only_key || @account.suspended? @account.save_with_optional_media! end @@ -214,7 +281,7 @@ class ActivityPub::ProcessAccountService < BaseService end def check_featured_collection! - ActivityPub::SynchronizeFeaturedCollectionWorker.perform_async(@account.id, { 'hashtag' => @json['featuredTags'].blank?, 'collection' => @json['featured'], 'request_id' => @options[:request_id] }) + ActivityPub::SynchronizeFeaturedCollectionWorker.perform_async(@account.id, { 'hashtag' => @json['featuredTags'].blank?, 'collection' => @json['featured'], 'request_id' => @request_id }) end def check_featured_tags_collection! @@ -222,7 +289,7 @@ class ActivityPub::ProcessAccountService < BaseService end def check_featured_collections_collection! - ActivityPub::SynchronizeFeaturedCollectionsCollectionWorker.perform_async(@account.id, @options[:request_id]) + ActivityPub::SynchronizeFeaturedCollectionsCollectionWorker.perform_async(@account.id, @request_id) end def check_links! @@ -403,7 +470,7 @@ class ActivityPub::ProcessAccountService < BaseService def moved_account account = ActivityPub::TagManager.instance.uri_to_resource(@json['movedTo'], Account) - account ||= ActivityPub::FetchRemoteAccountService.new.call(@json['movedTo'], break_on_redirect: true, request_id: @options[:request_id]) + account ||= ActivityPub::FetchRemoteAccountService.new.call(@json['movedTo'], break_on_redirect: true, request_id: @request_id) account end diff --git a/spec/services/activitypub/fetch_remote_account_service_spec.rb b/spec/services/activitypub/fetch_remote_account_service_spec.rb index 6d3d8a252b5..fc7efdc8ce7 100644 --- a/spec/services/activitypub/fetch_remote_account_service_spec.rb +++ b/spec/services/activitypub/fetch_remote_account_service_spec.rb @@ -33,20 +33,16 @@ RSpec.describe ActivityPub::FetchRemoteAccountService do end context 'when the account does not have a inbox' do - let!(:webfinger) { { subject: 'acct:alice@example.com', links: [{ rel: 'self', href: 'https://example.com/alice', type: 'application/activity+json' }] } } - before do actor[:inbox] = nil stub_request(:get, 'https://example.com/alice').to_return(body: actor.to_json, headers: { 'Content-Type': 'application/activity+json' }) - stub_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:alice@example.com').to_return(body: webfinger.to_json, headers: { 'Content-Type': 'application/jrd+json' }) end - it 'fetches resource and looks up webfinger and returns nil' do + it 'fetches resource but skips webfinger lookup and returns nil' do expect(account).to be_nil expect(a_request(:get, 'https://example.com/alice')).to have_been_made.once - expect(a_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:alice@example.com')).to have_been_made.once end end diff --git a/spec/services/activitypub/fetch_remote_actor_service_spec.rb b/spec/services/activitypub/fetch_remote_actor_service_spec.rb index a014e234adb..f1dc327fab7 100644 --- a/spec/services/activitypub/fetch_remote_actor_service_spec.rb +++ b/spec/services/activitypub/fetch_remote_actor_service_spec.rb @@ -33,20 +33,16 @@ RSpec.describe ActivityPub::FetchRemoteActorService do end context 'when the account does not have a inbox' do - let!(:webfinger) { { subject: 'acct:alice@example.com', links: [{ rel: 'self', href: 'https://example.com/alice', type: 'application/activity+json' }] } } - before do actor[:inbox] = nil stub_request(:get, 'https://example.com/alice').to_return(body: actor.to_json, headers: { 'Content-Type': 'application/activity+json' }) - stub_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:alice@example.com').to_return(body: webfinger.to_json, headers: { 'Content-Type': 'application/jrd+json' }) end - it 'fetches resource and looks up webfinger and returns nil' do + it 'fetches resource but skips webfinger lookup and returns nil' do expect(account).to be_nil expect(a_request(:get, 'https://example.com/alice')).to have_been_made.once - expect(a_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:alice@example.com')).to have_been_made.once end end diff --git a/spec/services/activitypub/process_account_service_spec.rb b/spec/services/activitypub/process_account_service_spec.rb index 9536809e457..6232f03759b 100644 --- a/spec/services/activitypub/process_account_service_spec.rb +++ b/spec/services/activitypub/process_account_service_spec.rb @@ -5,12 +5,27 @@ require 'rails_helper' RSpec.describe ActivityPub::ProcessAccountService do subject { described_class.new } + def stub_webfinger! + webfinger = { + subject: "acct:#{payload['preferredUsername']}@#{Addressable::URI.parse(payload['id']).host}", + links: [ + { + rel: 'self', + href: payload['id'], + type: 'application/activity+json', + }, + ], + }.deep_stringify_keys + stub_request(:get, "#{Addressable::URI.parse(payload['id']).origin}/.well-known/webfinger?resource=#{webfinger['subject']}").to_return(body: webfinger.to_json, headers: { 'Content-Type': 'application/jrd+json' }) + end + context 'with property values, an avatar, and a profile header' do let(:payload) do { id: 'https://foo.test', type: 'Actor', inbox: 'https://foo.test/inbox', + preferredUsername: 'alice', attachment: [ { type: 'PropertyValue', name: 'Pronouns', value: 'They/them' }, { type: 'PropertyValue', name: 'Occupation', value: 'Unit test' }, @@ -30,16 +45,17 @@ RSpec.describe ActivityPub::ProcessAccountService do }, ], }, - }.with_indifferent_access + }.deep_stringify_keys end before do + stub_webfinger! stub_request(:get, 'https://foo.test/image.png').to_return(request_fixture('avatar.txt')) stub_request(:get, 'https://foo.test/icon.png').to_return(request_fixture('avatar.txt')) end it 'parses property values, avatar and profile header as expected' do - account = subject.call('alice', 'example.com', payload) + account = subject.call(payload) expect(account.fields) .to be_an(Array) @@ -68,6 +84,7 @@ RSpec.describe ActivityPub::ProcessAccountService do { 'id' => 'https://foo.test', 'type' => 'Actor', + 'preferredUsername' => 'alice', 'inbox' => 'https://foo.test/inbox', 'featured' => 'https://foo.test/featured', 'followers' => 'https://foo.test/followers', @@ -77,12 +94,13 @@ RSpec.describe ActivityPub::ProcessAccountService do end before do + stub_webfinger! stub_request(:get, %r{^https://foo\.test/follow}) .to_return(status: 200, body: '', headers: {}) end it 'parses and sets the URIs, queues jobs to synchronize' do - account = subject.call('alice', 'example.com', payload) + account = subject.call(payload) expect(account.featured_collection_url).to eq 'https://foo.test/featured' expect(account.followers_url).to eq 'https://foo.test/followers' @@ -108,11 +126,13 @@ RSpec.describe ActivityPub::ProcessAccountService do owner: 'https://foo.test/actor', publicKeyPem: public_key, }, - }.with_indifferent_access + }.deep_stringify_keys end + before { stub_webfinger! } + it 'stores the key' do - account = subject.call('alice', 'example.com', payload) + account = subject.call(payload) expect(account.public_key).to eq '' expect(account.keypairs).to contain_exactly( @@ -125,17 +145,17 @@ RSpec.describe ActivityPub::ProcessAccountService do end context 'when the account was known with a legacy key' do - let!(:alice) { Fabricate(:account, uri: 'https://foo.test/actor', domain: 'example.com', username: 'alice', legacy_keypair: true) } + let!(:alice) { Fabricate(:account, uri: 'https://foo.test/actor', domain: 'foo.test', username: 'alice', legacy_keypair: true) } it 'invalidates the legacy key and stores the new key' do - expect { subject.call('alice', 'example.com', payload) } + expect { subject.call(payload) } .to change { alice.reload.public_key }.to('') .and change { alice.reload.keypairs.to_a }.from([]).to(contain_exactly(have_attributes({ uri: 'https://foo.test/actor#key1', type: 'rsa', public_key: }))) end end context 'when the account was known with an old key' do - let!(:alice) { Fabricate(:account, uri: 'https://foo.test/actor', domain: 'example.com', username: 'alice') } + let!(:alice) { Fabricate(:account, uri: 'https://foo.test/actor', domain: 'foo.test', username: 'alice') } before do alice.keypairs.delete_all @@ -143,7 +163,7 @@ RSpec.describe ActivityPub::ProcessAccountService do end it 'invalidates the old key and stores the new key' do - expect { subject.call('alice', 'example.com', payload) } + expect { subject.call(payload) } .to change { alice.reload.keypairs.to_a }.from(contain_exactly(have_attributes({ uri: 'https://foo.test/actor#old-key' }))).to(contain_exactly(have_attributes({ uri: 'https://foo.test/actor#key1', type: 'rsa', public_key: }))) expect(alice.reload.public_key) @@ -175,11 +195,12 @@ RSpec.describe ActivityPub::ProcessAccountService do end before do + stub_webfinger! stub_request(:get, key_id).to_return(status: 200, body: key_document.to_json, headers: { 'Content-Type': 'application/activity+json' }) end it 'stores the key' do - account = subject.call('alice', 'example.com', payload) + account = subject.call(payload) expect(account.public_key).to eq '' expect(account.keypairs).to contain_exactly( @@ -209,7 +230,7 @@ RSpec.describe ActivityPub::ProcessAccountService do let(:key_document) { payload } it 'stores the key' do - account = subject.call('alice', 'example.com', payload) + account = subject.call(payload) expect(account.public_key).to eq '' expect(account.keypairs).to contain_exactly( @@ -223,17 +244,17 @@ RSpec.describe ActivityPub::ProcessAccountService do end context 'when the account was known with a legacy key' do - let!(:alice) { Fabricate(:account, uri: 'https://foo.test/actor', domain: 'example.com', username: 'alice', legacy_keypair: true) } + let!(:alice) { Fabricate(:account, uri: 'https://foo.test/actor', domain: 'foo.test', username: 'alice', legacy_keypair: true) } it 'invalidates the legacy key and stores the new key' do - expect { subject.call('alice', 'example.com', payload) } + expect { subject.call(payload) } .to change { alice.reload.public_key }.to('') .and change { alice.reload.keypairs.to_a }.from([]).to(contain_exactly(have_attributes({ uri: key_id, type: 'rsa', public_key: }))) end end context 'when the account was known with an old key' do - let!(:alice) { Fabricate(:account, uri: 'https://foo.test/actor', domain: 'example.com', username: 'alice') } + let!(:alice) { Fabricate(:account, uri: 'https://foo.test/actor', domain: 'foo.test', username: 'alice') } before do alice.keypairs.delete_all @@ -241,7 +262,7 @@ RSpec.describe ActivityPub::ProcessAccountService do end it 'invalidates the legacy key and stores the new key' do - expect { subject.call('alice', 'example.com', payload) } + expect { subject.call(payload) } .to change { alice.reload.keypairs.to_a }.from(contain_exactly(have_attributes({ uri: 'https://foo.test/actor#old-key' }))).to(contain_exactly(have_attributes({ uri: key_id, type: 'rsa', public_key: }))) expect(alice.reload.public_key) @@ -269,11 +290,13 @@ RSpec.describe ActivityPub::ProcessAccountService do publicKeyPem: 'bar', }, ], - }.with_indifferent_access + }.deep_stringify_keys end + before { stub_webfinger! } + it 'stores the keys' do - account = subject.call('alice', 'example.com', payload) + account = subject.call(payload) expect(account.public_key).to eq '' expect(account.keypairs).to contain_exactly( @@ -291,10 +314,10 @@ RSpec.describe ActivityPub::ProcessAccountService do end context 'when the account was known with a legacy key' do - let!(:alice) { Fabricate(:account, uri: 'https://foo.test/actor', domain: 'example.com', username: 'alice', legacy_keypair: true) } + let!(:alice) { Fabricate(:account, uri: 'https://foo.test/actor', domain: 'foo.test', username: 'alice', legacy_keypair: true) } it 'invalidates the legacy key and stores the new keys' do - expect { subject.call('alice', 'example.com', payload) } + expect { subject.call(payload) } .to change { alice.reload.public_key }.to('') .and change { alice.keypairs.to_a }.from([]).to( contain_exactly( @@ -327,11 +350,13 @@ RSpec.describe ActivityPub::ProcessAccountService do publicKeyMultibase: 'z2MGw4gk84USotaWf4AkJ83DcnrfgGaceF86KQXRYMfQ7xqnUG81FVWa2N5inzNigXsDkm2LxpuyYSajqZr1CwHqnJbVEw1rhN25tbJSFyej6TejRh3k67CK9nTVHdXFoVKgAFxLwgiqJwCyyYWesaQKXAQfwXYqCBxPyaDjFfWkya6xeLaNuKFYGLcVzZZQjL99dnzUpNiENFPkVmJokE1wKPpHttGpLgm9sizHNDFuwHaz2ZZRnnZ6CT95FzdrMmaDXofn1ikbKBTdumuiRWSVwwZXffcXRN6Ti1a8NfhxQDdqhT7CAmM9NjQhnrqs1vss6YdcrHP5GmQN2Mz8GenQZFnyhJZK2iPxETnxq7YJRqTduN8KC8SMfjLVB8LD7rBM5d6s8dopdgJCVBpy2p', # rubocop:disable Layout/LineLength }, ], - }.with_indifferent_access + }.deep_stringify_keys end + before { stub_webfinger! } + it 'stores the keys' do - account = subject.call('alice', 'example.com', payload) + account = subject.call(payload) expect(account.public_key).to eq '' expect(account.keypairs).to contain_exactly( @@ -347,10 +372,10 @@ RSpec.describe ActivityPub::ProcessAccountService do end context 'when the account was known with a legacy key' do - let!(:alice) { Fabricate(:account, uri: 'https://foo.test/actor', domain: 'example.com', username: 'alice', legacy_keypair: true) } + let!(:alice) { Fabricate(:account, uri: 'https://foo.test/actor', domain: 'foo.test', username: 'alice', legacy_keypair: true) } it 'invalidates the legacy key and stores the new keys' do - expect { subject.call('alice', 'example.com', payload) } + expect { subject.call(payload) } .to change { alice.reload.public_key }.to('') .and change { alice.keypairs.to_a }.from([]).to( contain_exactly( @@ -368,14 +393,17 @@ RSpec.describe ActivityPub::ProcessAccountService do id: 'https://foo.test', type: 'Actor', inbox: 'https://foo.test/inbox', + preferredUsername: 'alice', attributionDomains: [ 'example.com', ], - }.with_indifferent_access + }.deep_stringify_keys end + before { stub_webfinger! } + it 'parses attribution domains' do - account = subject.call('alice', 'example.com', payload) + account = subject.call(payload) expect(account.attribution_domains) .to match_array(%w(example.com)) @@ -388,14 +416,17 @@ RSpec.describe ActivityPub::ProcessAccountService do id: 'https://foo.test', type: 'Actor', inbox: 'https://foo.test/inbox', + preferredUsername: 'alice', showMedia: true, showRepliesInMedia: false, showFeatured: false, - }.with_indifferent_access + }.deep_stringify_keys end + before { stub_webfinger! } + it 'sets the profile settings as expected' do - account = subject.call('alice', 'example.com', payload) + account = subject.call(payload) expect(account) .to have_attributes( @@ -412,6 +443,7 @@ RSpec.describe ActivityPub::ProcessAccountService do id: 'https://foo.test', type: 'Actor', inbox: 'https://foo.test/inbox', + preferredUsername: 'alice', featured: { type: 'OrderedCollection', orderedItems: ['https://example.com/statuses/1'], @@ -419,8 +451,10 @@ RSpec.describe ActivityPub::ProcessAccountService do }.deep_stringify_keys end + before { stub_webfinger! } + it 'queues featured collection synchronization', :aggregate_failures do - account = subject.call('alice', 'example.com', payload) + account = subject.call(payload) expect(account.featured_collection_url).to eq '' expect(ActivityPub::SynchronizeFeaturedCollectionWorker).to have_enqueued_sidekiq_job(account.id, { 'hashtag' => true, 'request_id' => anything, 'collection' => payload['featured'] }) @@ -428,20 +462,23 @@ RSpec.describe ActivityPub::ProcessAccountService do end context 'when account is not suspended' do - subject { described_class.new.call(account.username, account.domain, payload) } - - let!(:account) { Fabricate(:account, username: 'alice', domain: 'example.com') } + subject { described_class.new.call(payload) } let(:payload) do { - id: 'https://foo.test', + id: 'https://example.com', + preferredUsername: 'alice', type: 'Actor', - inbox: 'https://foo.test/inbox', + inbox: 'https://example.com/inbox', suspended: true, - }.with_indifferent_access + }.deep_stringify_keys end before do + stub_webfinger! + + Fabricate(:account, username: 'alice', domain: 'example.com') + allow(Admin::SuspensionWorker).to receive(:perform_async) end @@ -457,21 +494,24 @@ RSpec.describe ActivityPub::ProcessAccountService do end context 'when account is suspended' do - subject { described_class.new.call('alice', 'example.com', payload) } + subject { described_class.new.call(payload) } let!(:account) { Fabricate(:account, username: 'alice', domain: 'example.com', display_name: '') } let(:payload) do { - id: 'https://foo.test', + id: 'https://example.com', + preferredUsername: 'alice', type: 'Actor', - inbox: 'https://foo.test/inbox', + inbox: 'https://example.com/inbox', suspended: false, name: 'Hoge', - }.with_indifferent_access + }.deep_stringify_keys end before do + stub_webfinger! + allow(Admin::UnsuspensionWorker).to receive(:perform_async) account.suspend!(origin: suspension_origin) @@ -513,15 +553,27 @@ RSpec.describe ActivityPub::ProcessAccountService do domain = "test#{i}.testdomain.com" json = { id: "https://#{domain}/users/1", + preferredUsername: 'alice', type: 'Actor', inbox: "https://#{domain}/inbox", - }.with_indifferent_access - described_class.new.call('alice', domain, json) + }.deep_stringify_keys + described_class.new.call(json) end end before do stub_const 'ActivityPub::ProcessAccountService::SUBDOMAINS_RATELIMIT', 5 + + 8.times do |i| + domain = "test#{i}.testdomain.com" + + webfinger = { + subject: "acct:alice@#{domain}", + links: [{ rel: 'self', href: "https://#{domain}/users/1", type: 'application/activity+json' }], + }.deep_stringify_keys + + stub_request(:get, "https://#{domain}/.well-known/webfinger?resource=acct:alice@#{domain}").to_return(body: webfinger.to_json, headers: { 'Content-Type': 'application/jrd+json' }) + end end it 'creates accounts without exceeding rate limit' do @@ -540,7 +592,7 @@ RSpec.describe ActivityPub::ProcessAccountService do inbox: 'https://foo.test/inbox', featured: 'https://foo.test/users/1/featured', preferredUsername: 'user1', - }.with_indifferent_access + }.deep_stringify_keys end before do @@ -554,7 +606,7 @@ RSpec.describe ActivityPub::ProcessAccountService do inbox: 'https://foo.test/inbox', featured: "https://foo.test/users/#{i}/featured", preferredUsername: "user#{i}", - }.with_indifferent_access + }.deep_stringify_keys status_json = { '@context': ['https://www.w3.org/ns/activitystreams'], id: "https://foo.test/users/#{i}/status", @@ -569,18 +621,18 @@ RSpec.describe ActivityPub::ProcessAccountService do }, ], to: ['as:Public', "https://foo.test/users/#{i + 1}"], - }.with_indifferent_access + }.deep_stringify_keys featured_json = { '@context': ['https://www.w3.org/ns/activitystreams'], id: "https://foo.test/users/#{i}/featured", type: 'OrderedCollection', totalItems: 1, orderedItems: [status_json], - }.with_indifferent_access + }.deep_stringify_keys webfinger = { subject: "acct:user#{i}@foo.test", links: [{ rel: 'self', href: "https://foo.test/users/#{i}", type: 'application/activity+json' }], - }.with_indifferent_access + }.deep_stringify_keys stub_request(:get, "https://foo.test/users/#{i}").to_return(status: 200, body: actor_json.to_json, headers: { 'Content-Type': 'application/activity+json' }) stub_request(:get, "https://foo.test/users/#{i}/featured").to_return(status: 200, body: featured_json.to_json, headers: { 'Content-Type': 'application/activity+json' }) stub_request(:get, "https://foo.test/users/#{i}/status").to_return(status: 200, body: status_json.to_json, headers: { 'Content-Type': 'application/activity+json' }) @@ -589,7 +641,7 @@ RSpec.describe ActivityPub::ProcessAccountService do end it 'creates accounts without exceeding rate limit', :inline_jobs do - expect { subject.call('user1', 'foo.test', payload) } + expect { subject.call(payload) } .to create_some_remote_accounts .and create_fewer_than_rate_limit_accounts end @@ -599,6 +651,7 @@ RSpec.describe ActivityPub::ProcessAccountService do let(:payload) do { id: 'https://foo.test', + preferredUsername: 'alice', type: 'Actor', inbox: 'https://foo.test/inbox', followers: 'https://foo.test/followers', @@ -612,17 +665,19 @@ RSpec.describe ActivityPub::ProcessAccountService do ], }, }, - }.with_indifferent_access + }.deep_stringify_keys end before do + stub_webfinger! + stub_request(:get, %r{^https://foo\.test/follow}) .to_return(status: 200, body: '', headers: {}) end context 'when collections feature is enabled' do it 'sets the interaction policy to the correct value' do - account = subject.call('user1', 'foo.test', payload) + account = subject.call(payload) expect(account.feature_approval_policy).to eq 0b100000000000000001100 end