diff options
author | ThibG <thib@sitedethib.com> | 2020-12-18 23:26:26 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-12-18 23:26:26 +0100 |
commit | a60d9335d8e7c4aa070f081719ee2a438b0e0202 (patch) | |
tree | 1ec5e4628240a3eb7bf3d16e310adb48775de05d /app | |
parent | 052249588b77fe3d8e29658076eb385f64511d6b (diff) |
Fix resolving accounts sometimes creating duplicate records for a given AP id (#15364)
* Fix ResolveAccountService accepting mismatching acct: URI * Set attributes that should be updated regardless of suspension * Fix key fetching * Automatically merge remote accounts with duplicate `uri` * Add tests * Add "tootctl accounts fix-duplicates" Finds duplicate accounts sharing a same ActivityPub `id`, re-fetch them and merge them under the canonical `acct:` URI. Co-authored-by: Claire <claire.github-309c@sitedethib.com>
Diffstat (limited to 'app')
-rw-r--r-- | app/services/activitypub/fetch_remote_account_service.rb | 2 | ||||
-rw-r--r-- | app/services/activitypub/process_account_service.rb | 28 | ||||
-rw-r--r-- | app/services/resolve_account_service.rb | 17 | ||||
-rw-r--r-- | app/workers/account_merging_worker.rb | 18 |
4 files changed, 44 insertions, 21 deletions
diff --git a/app/services/activitypub/fetch_remote_account_service.rb b/app/services/activitypub/fetch_remote_account_service.rb index e5bd0c47c..9d01f5386 100644 --- a/app/services/activitypub/fetch_remote_account_service.rb +++ b/app/services/activitypub/fetch_remote_account_service.rb @@ -28,7 +28,7 @@ class ActivityPub::FetchRemoteAccountService < BaseService return unless only_key || verified_webfinger? - ActivityPub::ProcessAccountService.new.call(@username, @domain, @json, only_key: only_key) + ActivityPub::ProcessAccountService.new.call(@username, @domain, @json, only_key: only_key, verified_webfinger: !only_key) rescue Oj::ParseError nil end diff --git a/app/services/activitypub/process_account_service.rb b/app/services/activitypub/process_account_service.rb index 4cb8e09db..6afeb92d6 100644 --- a/app/services/activitypub/process_account_service.rb +++ b/app/services/activitypub/process_account_service.rb @@ -28,6 +28,8 @@ class ActivityPub::ProcessAccountService < BaseService update_account process_tags process_attachments + + process_duplicate_accounts! if @options[:verified_webfinger] else raise Mastodon::RaceConditionError end @@ -69,34 +71,42 @@ class ActivityPub::ProcessAccountService < BaseService @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_keys] || @account.suspended? + set_fetchable_attributes! unless @options[:only_key] || @account.suspended? @account.save_with_optional_media! end - def set_immediate_attributes! + def set_immediate_protocol_attributes! @account.inbox_url = @json['inbox'] || '' @account.outbox_url = @json['outbox'] || '' @account.shared_inbox_url = (@json['endpoints'].is_a?(Hash) ? @json['endpoints']['sharedInbox'] : @json['sharedInbox']) || '' @account.followers_url = @json['followers'] || '' - @account.featured_collection_url = @json['featured'] || '' - @account.devices_url = @json['devices'] || '' @account.url = url || @uri @account.uri = @uri + @account.actor_type = actor_type + end + + def set_immediate_attributes! + @account.featured_collection_url = @json['featured'] || '' + @account.devices_url = @json['devices'] || '' @account.display_name = @json['name'] || '' @account.note = @json['summary'] || '' @account.locked = @json['manuallyApprovesFollowers'] || false @account.fields = property_values || {} @account.also_known_as = as_array(@json['alsoKnownAs'] || []).map { |item| value_or_id(item) } - @account.actor_type = actor_type @account.discoverable = @json['discoverable'] || false end + def set_fetchable_key! + @account.public_key = public_key || '' + end + def set_fetchable_attributes! @account.avatar_remote_url = image_url('icon') || '' unless skip_download? @account.header_remote_url = image_url('image') || '' unless skip_download? - @account.public_key = public_key || '' @account.statuses_count = outbox_total_items if outbox_total_items.present? @account.following_count = following_total_items if following_total_items.present? @account.followers_count = followers_total_items if followers_total_items.present? @@ -140,6 +150,12 @@ class ActivityPub::ProcessAccountService < BaseService VerifyAccountLinksWorker.perform_async(@account.id) end + def process_duplicate_accounts! + return unless Account.where(uri: @account.uri).where.not(id: @account.id).exists? + + AccountMergingWorker.perform_async(@account.id) + end + def actor_type if @json['type'].is_a?(Array) @json['type'].find { |type| ActivityPub::FetchRemoteAccountService::SUPPORTED_TYPES.include?(type) } diff --git a/app/services/resolve_account_service.rb b/app/services/resolve_account_service.rb index 74b0b82d0..3301aaf51 100644 --- a/app/services/resolve_account_service.rb +++ b/app/services/resolve_account_service.rb @@ -49,7 +49,7 @@ class ResolveAccountService < BaseService # Now it is certain, it is definitely a remote account, and it # either needs to be created, or updated from fresh data - process_account! + fetch_account! rescue Webfinger::Error, Oj::ParseError => e Rails.logger.debug "Webfinger query for #{@uri} failed: #{e}" nil @@ -104,16 +104,12 @@ class ResolveAccountService < BaseService acct.gsub(/\Aacct:/, '').split('@') end - def process_account! + def fetch_account! return unless activitypub_ready? RedisLock.acquire(lock_options) do |lock| if lock.acquired? - @account = Account.find_remote(@username, @domain) - - next if actor_json.nil? - - @account = ActivityPub::ProcessAccountService.new.call(@username, @domain, actor_json) + @account = ActivityPub::FetchRemoteAccountService.new.call(actor_url) else raise Mastodon::RaceConditionError end @@ -136,13 +132,6 @@ class ResolveAccountService < BaseService @actor_url ||= @webfinger.link('self', 'href') end - def actor_json - return @actor_json if defined?(@actor_json) - - json = fetch_resource(actor_url, false) - @actor_json = supported_context?(json) && equals_or_includes_any?(json['type'], ActivityPub::FetchRemoteAccountService::SUPPORTED_TYPES) ? json : nil - end - def gone_from_origin? @gone end diff --git a/app/workers/account_merging_worker.rb b/app/workers/account_merging_worker.rb new file mode 100644 index 000000000..8c234e7ac --- /dev/null +++ b/app/workers/account_merging_worker.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class AccountMergingWorker + include Sidekiq::Worker + + sidekiq_options queue: 'pull' + + def perform(account_id) + account = Account.find(account_id) + + return true if account.nil? || account.local? + + Account.where(uri: account.uri).where.not(id: account.id).find_each do |duplicate| + account.merge_with!(duplicate) + duplicate.destroy + end + end +end |