Leahnaya/mastodon
1
0
Fork 0
mirror of https://github.com/mastodon/mastodon.git synced 2026-09-30 11:00:21 -05:00
Code Issues Actions 3 Packages Projects Releases Wiki Activity

Add uniqueness constraint on Account uri (#39882)

Browse Source
This commit is contained in:
Claire
2026-07-28 10:09:21 +02:00
committed by GitHub
parent 20c105c194
commit e0efc9683f
14 changed files with 236 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
app/models/account.rb
View File

@@ -54,7 +54,7 @@
# suspended_at :datetime
# suspension_origin :integer
# trendable :boolean
# uri :string default(""), not null
# uri :string
# url :string
# username :string default(""), not null
# created_at :datetime not null
@@ -122,7 +122,7 @@ class Account < ApplicationRecord
validates :username, format: { with: USERNAME_ONLY_RE }, length: { maximum: USERNAME_LENGTH_HARD_LIMIT }, if: -> { (remote? || actor_type_application?) && will_save_change_to_username? }
# Remote user validations
validates :uri, presence: true, unless: :local?, on: :create
validates :uri, presence: true, exclusion: { in: [''] }, uniqueness: true, unless: :local?, on: :create
# Local user validations
validates :username, format: { with: /\A[a-z0-9_]+\z/i }, length: { maximum: USERNAME_LENGTH_LIMIT }, if: -> { local? && will_save_change_to_username? && !actor_type_application? }
@@ -136,7 +136,7 @@ class Account < ApplicationRecord
validates :following_url, absence: true
validates :inbox_url, absence: true
validates :shared_inbox_url, absence: true
validates :uri, absence: true
validates :uri, absence: true, exclusion: { in: [''] }
end
validates :domain, exclusion: { in: [''] }

19
db/migrate/20260720090737_change_account_uri_nullable.rb Normal file
View File

@@ -0,0 +1,19 @@
# frozen_string_literal: true
class ChangeAccountUriNullable < ActiveRecord::Migration[8.1]
disable_ddl_transaction!
class Account < ApplicationRecord; end
def up
change_column_null :accounts, :uri, true
end
def down
change_column_null :accounts, :uri, false
rescue ActiveRecord::NotNullViolation
Account.where(uri: nil).update_all(uri: '')
retry
end
end

7
db/migrate/20260720092724_change_account_uri_default.rb Normal file
View File

@@ -0,0 +1,7 @@
# frozen_string_literal: true
class ChangeAccountUriDefault < ActiveRecord::Migration[8.1]
def change
change_column_default :accounts, :uri, from: '', to: nil
end
end

14
db/post_migrate/20260720100326_fix_blank_account_uri.rb Normal file
View File

@@ -0,0 +1,14 @@
# frozen_string_literal: true
class FixBlankAccountUri < ActiveRecord::Migration[8.1]
# Dummy classes to make migration possible across version changes
class Account < ApplicationRecord; end
def up
Account.where(uri: '').in_batches.update_all(uri: nil)
end
def down
Account.where(uri: nil).in_batches.update_all(uri: '')
end
end

7
db/post_migrate/20260720103819_rename_index_accounts_on_uri_to_old.rb Normal file
View File

@@ -0,0 +1,7 @@
# frozen_string_literal: true
class RenameIndexAccountsOnUriToOld < ActiveRecord::Migration[8.1]
def change
rename_index :accounts, :index_accounts_on_uri, :old_index_accounts_on_uri
end
end

115
db/post_migrate/20260720104058_add_unique_index_on_accounts_uri.rb Normal file
View File

@@ -0,0 +1,115 @@
# frozen_string_literal: true
class AddUniqueIndexOnAccountsUri < ActiveRecord::Migration[8.1]
disable_ddl_transaction!
# Dummy classes to make migration possible across version changes
class Account < ApplicationRecord; end
class AccountDeletionRequest < ApplicationRecord; end
class AccountModerationNote < ApplicationRecord; end
class AccountNote < ApplicationRecord; end
class AccountPin < ApplicationRecord; end
class AccountStat < ApplicationRecord; end
class Appeal < ApplicationRecord; end
class Block < ApplicationRecord; end
class CanonicalEmailBlock < ApplicationRecord; end
class Collection < ApplicationRecord; end
class CollectionItem < ApplicationRecord; end
class Favourite < ApplicationRecord; end
class Follow < ApplicationRecord; end
class FollowRecommendationSuppression < ApplicationRecord; end
class FollowRequest < ApplicationRecord; end
class ListAccount < ApplicationRecord; end
class MediaAttachment < ApplicationRecord
self.inheritance_column = nil
end
class Mention < ApplicationRecord; end
class Mute < ApplicationRecord; end
class Notification < ApplicationRecord
self.inheritance_column = nil
end
class NotificationPermission < ApplicationRecord; end
class NotificationRequest < ApplicationRecord; end
class Poll < ApplicationRecord; end
class PollVote < ApplicationRecord; end
class Quote < ApplicationRecord; end
class Report < ApplicationRecord; end
class SeveredRelationship < ApplicationRecord; end
class Status < ApplicationRecord; end
class StatusPin < ApplicationRecord; end
class TagFollow < ApplicationRecord; end
class Tombstone < ApplicationRecord; end
def up
add_index :accounts, :uri, algorithm: :concurrently, unique: true
rescue ActiveRecord::RecordNotUnique
deduplicate_and_reindex!
rescue
remove_index :accounts, name: :index_accounts_on_uri
raise
end
def down
remove_index :accounts, name: :index_accounts_on_uri
end
private
def deduplicate_and_reindex!
deduplicate_accounts!
safety_assured { execute 'REINDEX INDEX CONCURRENTLY index_accounts_on_uri' }
rescue ActiveRecord::RecordNotUnique
retry
end
def deduplicate_accounts!
duplicate_uris = select_all('SELECT uri FROM accounts WHERE uri IS NOT NULL GROUP BY uri HAVING count(*) > 1').rows
duplicate_uris.each do |uri|
# Fetch the conflicting accounts and keep the most recently-discovered one as reference
duplicate_records = Account.where(uri: uri).reorder(Arel.sql("COALESCE(last_webfingered_at, 'epoch'::date)"), :created_at, :id).to_a
reference_account = duplicate_records.pop
duplicate_records.each do |other_account|
merge_accounts!(reference_account, other_account)
other_account.destroy
end
end
end
def merge_accounts!(account, other_account)
{
account_id: [
Status, StatusPin, MediaAttachment, Poll, Report, Tombstone, Favourite,
Follow, FollowRequest, Block, Mute,
AccountModerationNote, AccountPin, AccountStat, ListAccount,
PollVote, Mention, AccountDeletionRequest, AccountNote, FollowRecommendationSuppression,
Appeal, TagFollow, Quote, Collection, CollectionItem
],
from_account_id: [
Notification, NotificationPermission, NotificationRequest
],
target_account_id: [
Follow, FollowRequest, Block, Mute, AccountModerationNote, AccountPin, AccountNote
],
reference_account_id: [CanonicalEmailBlock],
account_warning_id: [Appeal],
local_account_id: [SeveredRelationship],
remote_account_id: [SeveredRelationship],
quoted_account_id: [Quote],
}.each do |attribute, classes|
classes.each do |klass|
klass.where({ attribute => other_account.id }).reorder(nil).find_each do |record|
record.update_attribute(attribute, account.id)
rescue ActiveRecord::RecordNotUnique
next
end
end
end
end
end

9
db/post_migrate/20260720113713_remove_old_index_on_accounts_uri.rb Normal file
View File

@@ -0,0 +1,9 @@
# frozen_string_literal: true
class RemoveOldIndexOnAccountsUri < ActiveRecord::Migration[8.1]
disable_ddl_transaction!
def change
remove_index :accounts, :uri, name: :old_index_accounts_on_uri, algorithm: :concurrently
end
end

15
db/post_migrate/20260720124731_clean_up_invalid_accounts.rb Normal file
View File

@@ -0,0 +1,15 @@
# frozen_string_literal: true
class CleanUpInvalidAccounts < ActiveRecord::Migration[8.1]
# Dummy classes to make migration possible across version changes
class Account < ApplicationRecord; end
def up
# A very old bug could cause incompletely-processed remote accounts to be added
# to the database; those would not have a URI, which could be an issue in the
# future. Delete them.
Account.where.not(domain: nil).where(uri: nil).in_batches.delete_all
end
def down; end
end

6
db/schema.rb
View File

@@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema[8.1].define(version: 2026_07_06_143100) do
ActiveRecord::Schema[8.1].define(version: 2026_07_20_124731) do
# These are extensions that must be enabled in order to support this database
enable_extension "pg_catalog.plpgsql"
@@ -204,14 +204,14 @@ ActiveRecord::Schema[8.1].define(version: 2026_07_06_143100) do
t.integer "suspension_origin"
t.boolean "trendable"
t.datetime "updated_at", precision: nil, null: false
t.string "uri", default: "", null: false
t.string "uri"
t.string "url"
t.string "username", default: "", null: false
t.index "(((setweight(to_tsvector('simple'::regconfig, (display_name)::text), 'A'::\"char\") || setweight(to_tsvector('simple'::regconfig, (username)::text), 'B'::\"char\")) || setweight(to_tsvector('simple'::regconfig, (COALESCE(domain, ''::character varying))::text), 'C'::\"char\")))", name: "search_index", using: :gin
t.index "lower((username)::text), COALESCE(lower((domain)::text), ''::text)", name: "index_accounts_on_username_and_domain_lower", unique: true
t.index ["domain", "id"], name: "index_accounts_on_domain_and_id"
t.index ["moved_to_account_id"], name: "index_accounts_on_moved_to_account_id", where: "(moved_to_account_id IS NOT NULL)"
t.index ["uri"], name: "index_accounts_on_uri"
t.index ["uri"], name: "index_accounts_on_uri", unique: true
t.index ["url"], name: "index_accounts_on_url", opclass: :text_pattern_ops, where: "(url IS NOT NULL)"
end

15
lib/mastodon/cli/accounts.rb
View File

@@ -253,20 +253,11 @@ module Mastodon::CLI
desc 'fix-duplicates', 'Find duplicate remote accounts and merge them'
option :dry_run, type: :boolean
long_desc <<-LONG_DESC
Merge known remote accounts sharing an ActivityPub actor identifier.
Such duplicates can occur when a remote server admin misconfigures their
domain configuration.
This command is deprecated as of Mastodon v4.7.0.
LONG_DESC
def fix_duplicates
Account.remote.duplicate_uris.pluck(:uri).each do |uri|
say("Duplicates found for #{uri}")
begin
ActivityPub::FetchRemoteAccountService.new.call(uri) unless dry_run?
rescue => e
say("Error processing #{uri}: #{e}", :red)
end
end
# TODO: remove this after 4.7.0
say('This command is deprecated as Mastodon v4.7.0 migrations enforce ActivityPub actor identifier uniqueness', :yellow)
end
desc 'backup USERNAME', 'Request a backup for a user'

44
lib/tasks/tests.rake
View File

@@ -164,11 +164,23 @@ namespace :tests do
exit(1)
end
unless Account.find_by(uri: 'https://social.example.com/alice').domain == 'example.com'
puts 'Duplicate accounts with same URI incorrectly deduplicated'
exit(1)
end
if Account.exists?(domain: 'example.com', username: %w(bogus1 bogus2 bogus3))
puts 'Bogus remote accounts not correctly purged'
exit(1)
end
puts 'No errors found. Database state is consistent with a successful migration process.'
end
desc 'Populate the database with test data for 3.3.0'
task populate_v3_3_0: :environment do # rubocop:disable Naming/VariableNumber
user_public_key = ActiveRecord::Base.connection.quote(OpenSSL::PKey::RSA.new(2048).public_key.to_pem)
ActiveRecord::Base.connection.execute(<<~SQL.squish)
INSERT INTO "webauthn_credentials"
(user_id, nickname, external_id, public_key, created_at, updated_at)
@@ -206,6 +218,19 @@ namespace :tests do
(token, application_id, scopes, resource_owner_id, created_at)
VALUES
('secret', 2, 'write:accounts read:me', 4, now());
/* Duplicate remote accounts
Before Mastodon v4.7.0, we had no uniqueness constraint on account `uri`, which lead to issues.
Furthermore, older tests caused remote accounts without protocol identifiers.
*/
INSERT INTO "accounts"
(id, username, domain, uri, private_key, public_key, created_at, updated_at, last_webfingered_at)
VALUES
(12, 'alice', 'social.example.com', 'https://social.example.com/alice', NULL, #{user_public_key}, '2021-01-01'::date, now(), '2021-01-09'::date),
(13, 'alice', 'example.com', 'https://social.example.com/alice', NULL, #{user_public_key}, '2021-01-10'::date, now(), '2021-01-11'::date),
(14, 'bogus1', 'example.com', '', NULL, '', now(), now(), now()),
(15, 'bogus2', 'example.com', '', NULL, '', now(), now(), now()),
(16, 'bogus3', 'example.com', '', NULL, '', now(), now(), now());
SQL
end
@@ -336,19 +361,19 @@ namespace :tests do
(2, 'user', NULL, #{user_private_key}, #{user_public_key}, now(), now());
INSERT INTO "accounts"
(id, username, domain, private_key, public_key, created_at, updated_at, remote_url, salmon_url)
(id, username, domain, uri, private_key, public_key, created_at, updated_at, remote_url, salmon_url)
VALUES
(3, 'remote', 'remote.com', NULL, #{remote_public_key}, now(), now(),
(3, 'remote', 'remote.com', 'https://remote.com/remote', NULL, #{remote_public_key}, now(), now(),
'https://remote.com/@remote', 'https://remote.com/salmon/1'),
(4, 'Remote', 'remote.com', NULL, #{remote_public_key}, now(), now(),
(4, 'Remote', 'remote.com', 'https://remote.com/Remote', NULL, #{remote_public_key}, now(), now(),
'https://remote.com/@Remote', 'https://remote.com/salmon/1'),
(5, 'REMOTE', 'Remote.com', NULL, #{remote_public_key2}, now() - interval '1 year', now() - interval '1 year',
(5, 'REMOTE', 'Remote.com', 'https://remote.com/stale/remote', NULL, #{remote_public_key2}, now() - interval '1 year', now() - interval '1 year',
'https://remote.com/stale/@REMOTE', 'https://remote.com/stale/salmon/1');
INSERT INTO "accounts"
(id, username, domain, private_key, public_key, created_at, updated_at, protocol, inbox_url, outbox_url, followers_url)
(id, username, domain, uri, private_key, public_key, created_at, updated_at, protocol, inbox_url, outbox_url, followers_url)
VALUES
(6, 'bob', 'ActivityPub.com', NULL, #{remote_public_key_ap}, now(), now(),
(6, 'bob', 'ActivityPub.com', 'https://activitypub.com/users/bob', NULL, #{remote_public_key_ap}, now(), now(),
1, 'https://activitypub.com/users/bob/inbox', 'https://activitypub.com/users/bob/outbox', 'https://activitypub.com/users/bob/followers');
INSERT INTO "accounts"
@@ -358,10 +383,11 @@ namespace :tests do
(8, 'pt_user', NULL, #{user_private_key}, #{user_public_key}, now(), now());
INSERT INTO "accounts"
(id, username, domain, private_key, public_key, created_at, updated_at, protocol, inbox_url, outbox_url, followers_url, suspended)
(id, username, domain, uri, private_key, public_key, created_at, updated_at, protocol, inbox_url, outbox_url, followers_url, suspended)
VALUES
(9, 'evil', 'activitypub.com', NULL, #{remote_public_key_ap}, now(), now(),
1, 'https://activitypub.com/users/evil/inbox', 'https://activitypub.com/users/evil/outbox',
(9, 'evil', 'activitypub.com', 'https://activitypub.com/users/evil', NULL,
#{remote_public_key_ap}, now(), now(), 1,
'https://activitypub.com/users/evil/inbox', 'https://activitypub.com/users/evil/outbox',
'https://activitypub.com/users/evil/followers', true);
-- users

2
spec/fabricators/account_fabricator.rb
View File

@@ -11,7 +11,7 @@ Fabricator(:account) do
suspended_at { |attrs| attrs[:suspended] ? Time.now.utc : nil }
silenced_at { |attrs| attrs[:silenced] ? Time.now.utc : nil }
user { |attrs| attrs[:domain].nil? ? Fabricate.build(:user, account: nil) : nil }
uri { |attrs| attrs[:domain].nil? ? '' : "https://#{attrs[:domain]}/users/#{attrs[:username]}" }
uri { |attrs| attrs[:domain].nil? ? nil : "https://#{attrs[:domain]}/users/#{attrs[:username]}" }
discoverable true
indexable true

19
spec/lib/mastodon/cli/accounts_spec.rb
View File

@@ -648,25 +648,6 @@ RSpec.describe Mastodon::CLI::Accounts do
end
end
describe '#fix_duplicates' do
let(:action) { :fix_duplicates }
let(:service_double) { instance_double(ActivityPub::FetchRemoteAccountService, call: nil) }
let(:uri) { 'https://host.example/same/value' }
context 'when there are duplicate URI accounts' do
before do
Fabricate.times(2, :account, domain: 'host.example', uri: uri, legacy_keypair: true)
allow(ActivityPub::FetchRemoteAccountService).to receive(:new).and_return(service_double)
end
it 'finds the duplicates and calls fetch remote account service' do
expect { subject }
.to output_results('Duplicates found')
expect(service_double).to have_received(:call).with(uri)
end
end
end
describe '#backup' do
let(:action) { :backup }

5
spec/lib/mastodon/cli/maintenance_spec.rb
View File

@@ -89,6 +89,11 @@ RSpec.describe Mastodon::CLI::Maintenance do
def prepare_duplicate_data
ActiveRecord::Base.connection.remove_index :accounts, name: :index_accounts_on_username_and_domain_lower
# Re-create without uniqueness constraint for test purposes
ActiveRecord::Base.connection.remove_index :accounts, :uri
ActiveRecord::Base.connection.add_index :accounts, :uri
duplicate_record(:account, username: duplicate_account_username, domain: duplicate_account_domain, legacy_keypair: true)
duplicate_record(:account, username: duplicate_account_username, domain: nil)
end