From 264d068d8dc8107a492cc1ecebc4a141696466c1 Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 14 Oct 2025 18:36:55 +0200 Subject: [PATCH] Change new accounts to use new ActivityPub numeric ID scheme (#36365) --- app/models/account.rb | 9 +-- ...5_change_defaults_for_account_id_scheme.rb | 7 ++ db/schema.rb | 4 +- .../account_controller_concern_spec.rb | 2 +- .../follower_accounts_controller_spec.rb | 4 +- .../following_accounts_controller_spec.rb | 4 +- spec/lib/activitypub/tag_manager_spec.rb | 67 ++++++++++--------- spec/models/remote_follow_spec.rb | 2 +- spec/requests/activitypub/outboxes_spec.rb | 2 +- spec/requests/link_headers_spec.rb | 2 +- spec/requests/oauth/userinfo_spec.rb | 2 +- spec/requests/well_known/webfinger_spec.rb | 2 +- 12 files changed, 54 insertions(+), 53 deletions(-) create mode 100644 db/migrate/20251007142305_change_defaults_for_account_id_scheme.rb diff --git a/app/models/account.rb b/app/models/account.rb index a837cc6a6..5f4caf9ea 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -52,7 +52,7 @@ # requested_review_at :datetime # indexable :boolean default(FALSE), not null # attribution_domains :string default([]), is an Array -# id_scheme :integer default("username_ap_id") +# id_scheme :integer default("numeric_ap_id") # class Account < ApplicationRecord @@ -446,7 +446,6 @@ class Account < ApplicationRecord before_validation :prepare_contents, if: :local? before_create :generate_keys - before_create :set_id_scheme before_destroy :clean_feed_manager def ensure_keys! @@ -471,12 +470,6 @@ class Account < ApplicationRecord self.public_key = keypair.public_key.to_pem end - def set_id_scheme - return unless local? && Mastodon::Feature.numeric_ap_ids_enabled? - - self.id_scheme = :numeric_ap_id - end - def normalize_domain return if local? diff --git a/db/migrate/20251007142305_change_defaults_for_account_id_scheme.rb b/db/migrate/20251007142305_change_defaults_for_account_id_scheme.rb new file mode 100644 index 000000000..6c0723696 --- /dev/null +++ b/db/migrate/20251007142305_change_defaults_for_account_id_scheme.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class ChangeDefaultsForAccountIdScheme < ActiveRecord::Migration[8.0] + def change + change_column_default :accounts, :id_scheme, from: 0, to: 1 + end +end diff --git a/db/schema.rb b/db/schema.rb index 47937f865..8910bac36 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2025_10_07_100813) do +ActiveRecord::Schema[8.0].define(version: 2025_10_07_142305) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" @@ -199,7 +199,7 @@ ActiveRecord::Schema[8.0].define(version: 2025_10_07_100813) do t.boolean "indexable", default: false, null: false t.string "attribution_domains", default: [], array: true t.string "following_url", default: "", null: false - t.integer "id_scheme", default: 0 + t.integer "id_scheme", default: 1 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" diff --git a/spec/controllers/concerns/account_controller_concern_spec.rb b/spec/controllers/concerns/account_controller_concern_spec.rb index df6278a6e..b5c8c1664 100644 --- a/spec/controllers/concerns/account_controller_concern_spec.rb +++ b/spec/controllers/concerns/account_controller_concern_spec.rb @@ -58,7 +58,7 @@ RSpec.describe AccountControllerConcern do expect(response) .to have_http_status(200) .and have_http_link_header(webfinger_url(resource: account.to_webfinger_s)).for(rel: 'lrdd', type: 'application/jrd+json') - .and have_http_link_header(account_url(account, protocol: :https)).for(rel: 'alternate', type: 'application/activity+json') + .and have_http_link_header(ActivityPub::TagManager.instance.uri_for(account)).for(rel: 'alternate', type: 'application/activity+json') expect(response.body) .to include(account.username) end diff --git a/spec/controllers/follower_accounts_controller_spec.rb b/spec/controllers/follower_accounts_controller_spec.rb index e14ed00e6..dab6aadba 100644 --- a/spec/controllers/follower_accounts_controller_spec.rb +++ b/spec/controllers/follower_accounts_controller_spec.rb @@ -49,8 +49,8 @@ RSpec.describe FollowerAccountsController do expect(response.parsed_body) .to include( orderedItems: contain_exactly( - include(follow_from_bob.account.username), - include(follow_from_chris.account.username) + ActivityPub::TagManager.instance.uri_for(follow_from_bob.account), + ActivityPub::TagManager.instance.uri_for(follow_from_chris.account) ), totalItems: eq(2), partOf: be_present diff --git a/spec/controllers/following_accounts_controller_spec.rb b/spec/controllers/following_accounts_controller_spec.rb index fea4d4845..666c655d7 100644 --- a/spec/controllers/following_accounts_controller_spec.rb +++ b/spec/controllers/following_accounts_controller_spec.rb @@ -49,8 +49,8 @@ RSpec.describe FollowingAccountsController do expect(response.parsed_body) .to include( orderedItems: contain_exactly( - include(follow_of_bob.target_account.username), - include(follow_of_chris.target_account.username) + ActivityPub::TagManager.instance.uri_for(follow_of_bob.target_account), + ActivityPub::TagManager.instance.uri_for(follow_of_chris.target_account) ), totalItems: eq(2), partOf: be_present diff --git a/spec/lib/activitypub/tag_manager_spec.rb b/spec/lib/activitypub/tag_manager_spec.rb index 70e084a9c..cad46ad90 100644 --- a/spec/lib/activitypub/tag_manager_spec.rb +++ b/spec/lib/activitypub/tag_manager_spec.rb @@ -23,7 +23,7 @@ RSpec.describe ActivityPub::TagManager do describe '#url_for' do context 'with a local account' do - let(:account) { Fabricate(:account) } + let(:account) { Fabricate(:account, id_scheme: :username_ap_id) } it 'returns a string starting with web domain and with the expected path' do expect(subject.url_for(account)) @@ -49,7 +49,8 @@ RSpec.describe ActivityPub::TagManager do end context 'with a local status' do - let(:status) { Fabricate(:status) } + let(:account) { Fabricate(:account, id_scheme: :username_ap_id) } + let(:status) { Fabricate(:status, account: account) } it 'returns a string starting with web domain and with the expected path' do expect(subject.url_for(status)) @@ -58,7 +59,6 @@ RSpec.describe ActivityPub::TagManager do context 'when using a numeric ID based scheme' do let(:account) { Fabricate(:account, id_scheme: :numeric_ap_id) } - let(:status) { Fabricate(:status, account: account) } it 'returns a string starting with web domain and with the expected path' do expect(subject.url_for(status)) @@ -86,7 +86,7 @@ RSpec.describe ActivityPub::TagManager do end context 'with a local account' do - let(:account) { Fabricate(:account) } + let(:account) { Fabricate(:account, id_scheme: :username_ap_id) } it 'returns a string starting with web domain and with the expected path' do expect(subject.uri_for(account)) @@ -112,7 +112,8 @@ RSpec.describe ActivityPub::TagManager do end context 'with a local status' do - let(:status) { Fabricate(:status) } + let(:account) { Fabricate(:account, id_scheme: :username_ap_id) } + let(:status) { Fabricate(:status, account: account) } it 'returns a string starting with web domain and with the expected path' do expect(subject.uri_for(status)) @@ -121,7 +122,6 @@ RSpec.describe ActivityPub::TagManager do context 'when using a numeric ID based scheme' do let(:account) { Fabricate(:account, id_scheme: :numeric_ap_id) } - let(:status) { Fabricate(:status, account: account) } it 'returns a string starting with web domain and with the expected path' do expect(subject.uri_for(status)) @@ -140,7 +140,8 @@ RSpec.describe ActivityPub::TagManager do end context 'with a local conversation' do - let(:status) { Fabricate(:status) } + let(:account) { Fabricate(:account, id_scheme: :username_ap_id) } + let(:status) { Fabricate(:status, account: account) } it 'returns a string starting with web domain and with the expected path' do expect(subject.uri_for(status.conversation)) @@ -149,7 +150,6 @@ RSpec.describe ActivityPub::TagManager do context 'when using a numeric ID based scheme' do let(:account) { Fabricate(:account, id_scheme: :numeric_ap_id) } - let(:status) { Fabricate(:status, account: account) } it 'returns a string starting with web domain and with the expected path' do expect(subject.uri_for(status.conversation)) @@ -181,7 +181,7 @@ RSpec.describe ActivityPub::TagManager do end context 'with a local account' do - let(:account) { Fabricate(:account) } + let(:account) { Fabricate(:account, id_scheme: :username_ap_id) } it 'returns a string starting with web domain and with the expected path' do expect(subject.key_uri_for(account)) @@ -218,7 +218,9 @@ RSpec.describe ActivityPub::TagManager do describe '#approval_uri_for' do context 'with a valid local approval' do - let(:quote) { Fabricate(:quote, state: :accepted) } + let(:quoted_account) { Fabricate(:account, id_scheme: :username_ap_id) } + let(:quoted_status) { Fabricate(:status, account: quoted_account) } + let(:quote) { Fabricate(:quote, state: :accepted, quoted_status: quoted_status) } it 'returns a string with the web domain and expected path' do expect(subject.approval_uri_for(quote)) @@ -227,8 +229,6 @@ RSpec.describe ActivityPub::TagManager do context 'when using a numeric ID based scheme' do let(:quoted_account) { Fabricate(:account, id_scheme: :numeric_ap_id) } - let(:quoted_status) { Fabricate(:status, account: quoted_account) } - let(:quote) { Fabricate(:quote, state: :accepted, quoted_status: quoted_status) } it 'returns a string with the web domain and expected path' do expect(subject.approval_uri_for(quote)) @@ -238,7 +238,9 @@ RSpec.describe ActivityPub::TagManager do end context 'with an unapproved local quote' do - let(:quote) { Fabricate(:quote, state: :rejected) } + let(:quoted_account) { Fabricate(:account, id_scheme: :username_ap_id) } + let(:quoted_status) { Fabricate(:status, account: quoted_account) } + let(:quote) { Fabricate(:quote, state: :rejected, quoted_status: quoted_status) } it 'returns nil' do expect(subject.approval_uri_for(quote)) @@ -247,8 +249,6 @@ RSpec.describe ActivityPub::TagManager do context 'when using a numeric ID based scheme' do let(:quoted_account) { Fabricate(:account, id_scheme: :numeric_ap_id) } - let(:quoted_status) { Fabricate(:status, account: quoted_account) } - let(:quote) { Fabricate(:quote, state: :rejected, quoted_status: quoted_status) } it 'returns nil' do expect(subject.approval_uri_for(quote)) @@ -260,7 +260,7 @@ RSpec.describe ActivityPub::TagManager do context 'with a valid remote approval' do let(:quoted_account) { Fabricate(:account, domain: 'example.com') } let(:quoted_status) { Fabricate(:status, account: quoted_account) } - let(:quote) { Fabricate(:quote, state: :accepted, quoted_status: quoted_status, approval_uri: 'https://example.com/approvals/1') } + let(:quote) { Fabricate(:quote, status: Fabricate(:status), state: :accepted, quoted_status: quoted_status, approval_uri: 'https://example.com/approvals/1') } it 'returns the expected URI' do expect(subject.approval_uri_for(quote)).to eq quote.approval_uri @@ -268,7 +268,9 @@ RSpec.describe ActivityPub::TagManager do end context 'with an unapproved local quote but check_approval override' do - let(:quote) { Fabricate(:quote, state: :rejected) } + let(:quoted_account) { Fabricate(:account, id_scheme: :username_ap_id) } + let(:quoted_status) { Fabricate(:status, account: quoted_account) } + let(:quote) { Fabricate(:quote, state: :rejected, quoted_status: quoted_status) } it 'returns a string with the web domain and expected path' do expect(subject.approval_uri_for(quote, check_approval: false)) @@ -277,8 +279,6 @@ RSpec.describe ActivityPub::TagManager do context 'when using a numeric ID based scheme' do let(:quoted_account) { Fabricate(:account, id_scheme: :numeric_ap_id) } - let(:quoted_status) { Fabricate(:status, account: quoted_account) } - let(:quote) { Fabricate(:quote, state: :rejected, quoted_status: quoted_status) } it 'returns a string with the web domain and expected path' do expect(subject.approval_uri_for(quote, check_approval: false)) @@ -290,7 +290,8 @@ RSpec.describe ActivityPub::TagManager do describe '#replies_uri_for' do context 'with a local status' do - let(:status) { Fabricate(:status) } + let(:account) { Fabricate(:account, id_scheme: :username_ap_id) } + let(:status) { Fabricate(:status, account: account) } it 'returns a string starting with web domain and with the expected path' do expect(subject.replies_uri_for(status)) @@ -299,7 +300,6 @@ RSpec.describe ActivityPub::TagManager do context 'when using a numeric ID based scheme' do let(:account) { Fabricate(:account, id_scheme: :numeric_ap_id) } - let(:status) { Fabricate(:status, account: account) } it 'returns a string starting with web domain and with the expected path' do expect(subject.replies_uri_for(status)) @@ -311,7 +311,8 @@ RSpec.describe ActivityPub::TagManager do describe '#likes_uri_for' do context 'with a local status' do - let(:status) { Fabricate(:status) } + let(:account) { Fabricate(:account, id_scheme: :username_ap_id) } + let(:status) { Fabricate(:status, account: account) } it 'returns a string starting with web domain and with the expected path' do expect(subject.likes_uri_for(status)) @@ -320,7 +321,6 @@ RSpec.describe ActivityPub::TagManager do context 'when using a numeric ID based scheme' do let(:account) { Fabricate(:account, id_scheme: :numeric_ap_id) } - let(:status) { Fabricate(:status, account: account) } it 'returns a string starting with web domain and with the expected path' do expect(subject.likes_uri_for(status)) @@ -332,7 +332,8 @@ RSpec.describe ActivityPub::TagManager do describe '#shares_uri_for' do context 'with a local status' do - let(:status) { Fabricate(:status) } + let(:account) { Fabricate(:account, id_scheme: :username_ap_id) } + let(:status) { Fabricate(:status, account: account) } it 'returns a string starting with web domain and with the expected path' do expect(subject.shares_uri_for(status)) @@ -341,7 +342,6 @@ RSpec.describe ActivityPub::TagManager do context 'when using a numeric ID based scheme' do let(:account) { Fabricate(:account, id_scheme: :numeric_ap_id) } - let(:status) { Fabricate(:status, account: account) } it 'returns a string starting with web domain and with the expected path' do expect(subject.shares_uri_for(status)) @@ -353,7 +353,7 @@ RSpec.describe ActivityPub::TagManager do describe '#following_uri_for' do context 'with a local account' do - let(:account) { Fabricate(:account) } + let(:account) { Fabricate(:account, id_scheme: :username_ap_id) } it 'returns a string starting with web domain and with the expected path' do expect(subject.following_uri_for(account)) @@ -373,7 +373,7 @@ RSpec.describe ActivityPub::TagManager do describe '#followers_uri_for' do context 'with a local account' do - let(:account) { Fabricate(:account) } + let(:account) { Fabricate(:account, id_scheme: :username_ap_id) } it 'returns a string starting with web domain and with the expected path' do expect(subject.followers_uri_for(account)) @@ -400,7 +400,7 @@ RSpec.describe ActivityPub::TagManager do end context 'with a local account' do - let(:account) { Fabricate(:account) } + let(:account) { Fabricate(:account, id_scheme: :username_ap_id) } it 'returns a string starting with web domain and with the expected path' do expect(subject.inbox_uri_for(account)) @@ -427,7 +427,7 @@ RSpec.describe ActivityPub::TagManager do end context 'with a local account' do - let(:account) { Fabricate(:account) } + let(:account) { Fabricate(:account, id_scheme: :username_ap_id) } it 'returns a string starting with web domain and with the expected path' do expect(subject.outbox_uri_for(account)) @@ -452,7 +452,7 @@ RSpec.describe ActivityPub::TagManager do end it 'returns followers collection for unlisted status' do - status = Fabricate(:status, visibility: :unlisted) + status = Fabricate(:status, visibility: :unlisted, account: Fabricate(:account, id_scheme: :username_ap_id)) expect(subject.to(status)).to eq [account_followers_url(status.account)] end @@ -462,7 +462,7 @@ RSpec.describe ActivityPub::TagManager do end it 'returns followers collection for private status' do - status = Fabricate(:status, visibility: :private) + status = Fabricate(:status, visibility: :private, account: Fabricate(:account, id_scheme: :username_ap_id)) expect(subject.to(status)).to eq [account_followers_url(status.account)] end @@ -514,7 +514,7 @@ RSpec.describe ActivityPub::TagManager do describe '#cc' do it 'returns followers collection for public status' do - status = Fabricate(:status, visibility: :public) + status = Fabricate(:status, visibility: :public, account: Fabricate(:account, id_scheme: :username_ap_id)) expect(subject.cc(status)).to eq [account_followers_url(status.account)] end @@ -591,8 +591,9 @@ RSpec.describe ActivityPub::TagManager do end describe '#uri_to_local_id' do + let(:account) { Fabricate(:account, id_scheme: :username_ap_id) } + it 'returns the local ID' do - account = Fabricate(:account) expect(subject.uri_to_local_id(subject.uri_for(account), :username)).to eq account.username end end diff --git a/spec/models/remote_follow_spec.rb b/spec/models/remote_follow_spec.rb index b3ec7b737..aad328256 100644 --- a/spec/models/remote_follow_spec.rb +++ b/spec/models/remote_follow_spec.rb @@ -68,7 +68,7 @@ RSpec.describe RemoteFollow do Addressable::URI.new( host: 'quitter.no', path: '/main/ostatussub', - query_values: { profile: "https://#{Rails.configuration.x.local_domain}/users/alice" }, + query_values: { profile: ActivityPub::TagManager.instance.uri_for(account) }, scheme: 'https' ).to_s end diff --git a/spec/requests/activitypub/outboxes_spec.rb b/spec/requests/activitypub/outboxes_spec.rb index 22b2f97c0..c10921463 100644 --- a/spec/requests/activitypub/outboxes_spec.rb +++ b/spec/requests/activitypub/outboxes_spec.rb @@ -215,7 +215,7 @@ RSpec.describe 'ActivityPub Outboxes' do def targets_followers_collection?(item, account) item[:to].include?( - account_followers_url(account, ActionMailer::Base.default_url_options) + ActivityPub::TagManager.instance.followers_uri_for(account) ) end end diff --git a/spec/requests/link_headers_spec.rb b/spec/requests/link_headers_spec.rb index 5010f7f22..c782744f0 100644 --- a/spec/requests/link_headers_spec.rb +++ b/spec/requests/link_headers_spec.rb @@ -11,7 +11,7 @@ RSpec.describe 'Link headers' do expect(response) .to have_http_link_header(webfinger_url(resource: account.to_webfinger_s)).for(rel: 'lrdd', type: 'application/jrd+json') - .and have_http_link_header(account_url(account)).for(rel: 'alternate', type: 'application/activity+json') + .and have_http_link_header(ActivityPub::TagManager.instance.uri_for(account)).for(rel: 'alternate', type: 'application/activity+json') end end end diff --git a/spec/requests/oauth/userinfo_spec.rb b/spec/requests/oauth/userinfo_spec.rb index 0aa5a2112..280a3a354 100644 --- a/spec/requests/oauth/userinfo_spec.rb +++ b/spec/requests/oauth/userinfo_spec.rb @@ -19,7 +19,7 @@ RSpec.describe 'OAuth Userinfo Endpoint' do expect(response.content_type).to start_with('application/json') expect(response.parsed_body).to include({ iss: root_url, - sub: account_url(account), + sub: ActivityPub::TagManager.instance.uri_for(account), name: account.display_name, preferred_username: account.username, profile: short_account_url(account), diff --git a/spec/requests/well_known/webfinger_spec.rb b/spec/requests/well_known/webfinger_spec.rb index 0c4a3c034..593cecbb8 100644 --- a/spec/requests/well_known/webfinger_spec.rb +++ b/spec/requests/well_known/webfinger_spec.rb @@ -27,7 +27,7 @@ RSpec.describe 'The /.well-known/webfinger endpoint' do expect(response.parsed_body) .to include( subject: eq(alice.to_webfinger_s), - aliases: include("https://#{Rails.configuration.x.local_domain}/@alice", "https://#{Rails.configuration.x.local_domain}/users/alice") + aliases: include("https://#{Rails.configuration.x.local_domain}/@alice", ActivityPub::TagManager.instance.uri_for(alice)) ) end end