Merge fdb7f8ae36 into de09e33c92

app/models/account.rb

@@ -271,7 +271,7 @@ class Account < ApplicationRecord
   end
 
   def keypair
-    @keypair ||= OpenSSL::PKey::RSA.new(private_key || public_key)
+    @keypair ||= OpenSSL::PKey::RSA.new(account_secret.private_key || public_key)
   end
 
   def tags_as_strings=(tag_names)
@@ -441,11 +441,11 @@ class Account < ApplicationRecord
   end
 
   before_validation :prepare_contents, if: :local?
-  before_create :generate_keys
+  after_create :generate_keys
   before_destroy :clean_feed_manager
 
   def ensure_keys!
-    return unless local? && private_key.blank? && public_key.blank?
+    return unless local? && (account_secret.nil? || account_secret.private_key.blank?) && public_key.blank?
 
     generate_keys
     save!
@@ -459,11 +459,17 @@ class Account < ApplicationRecord
   end
 
   def generate_keys
-    return unless local? && private_key.blank? && public_key.blank?
+    return unless local? && (account_secret.nil? || account_secret.private_key.blank?) && public_key.blank?
 
     keypair = OpenSSL::PKey::RSA.new(2048)
-    self.private_key = keypair.to_pem
-    self.public_key  = keypair.public_key.to_pem
+
+    if account_secret.nil?
+      create_account_secret!(private_key: keypair.to_pem)
+    else
+      account_secret.update!(private_key: keypair.to_pem)
+    end
+
+    update!(public_key: keypair.public_key.to_pem)
   end
 
   def normalize_domain
@@ -500,4 +506,13 @@ class Account < ApplicationRecord
   def trigger_update_webhooks
     TriggerWebhookWorker.perform_async('account.updated', 'Account', id) if local?
   end
+
+  # Temporary errors while private_key is still a field in the accounts table
+  def private_key=
+    raise 'private_key= is deprecated. Use account.account_secret.private_key= '
+  end
+
+  def private_key
+    raise 'private_key is deprecated. Use account.account_secret.private_key'
+  end
 end

app/models/account_secret.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+# == Schema Information
+#
+# Table name: account_secrets
+#
+#  id          :bigint(8)        not null, primary key
+#  private_key :text
+#  created_at  :datetime         not null
+#  updated_at  :datetime         not null
+#  account_id  :bigint(8)        not null
+#
+class AccountSecret < ApplicationRecord
+  belongs_to :account
+
+  encrypts :private_key
+end

app/models/concerns/account/associations.rb

@@ -38,6 +38,7 @@ module Account::Associations
         has_one :notification_policy
         has_one :statuses_cleanup_policy, class_name: 'AccountStatusesCleanupPolicy'
         has_one :user
+        has_one :account_secret
       end
 
       # Association where account is targeted by record

app/services/activitypub/process_account_service.rb

@@ -74,7 +74,6 @@ class ActivityPub::ProcessAccountService < BaseService
     @account.protocol          = :activitypub
     @account.username          = @username
     @account.domain            = @domain
-    @account.private_key       = nil
     @account.suspended_at      = domain_block.created_at if auto_suspend?
     @account.suspension_origin = :local if auto_suspend?
     @account.silenced_at       = domain_block.created_at if auto_silence?

db/migrate/20250828155921_create_account_secrets.rb

@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+class CreateAccountSecrets < ActiveRecord::Migration[8.0]
+  def change
+    create_table :account_secrets do |t|
+      t.references :account, null: false, foreign_key: { on_delete: :cascade }, index: { unique: true }
+      t.text :private_key
+
+      t.timestamps
+    end
+  end
+end

db/migrate/20250829133404_migrate_private_keys_to_account_secrets.rb

@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+class MigratePrivateKeysToAccountSecrets < ActiveRecord::Migration[8.0]
+  disable_ddl_transaction!
+
+  def up
+    Account.where.not(private_key: nil).in_batches do |batch|
+      batch.each do |account|
+        AccountSecret.create!(
+          account_id: account.id,
+          private_key: account.private_key
+        )
+      end
+    end
+  end
+
+  def down
+    AccountSecret.delete_all
+  end
+end

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_08_20_084312) do
+ActiveRecord::Schema[8.0].define(version: 2025_08_29_133404) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_catalog.plpgsql"
 
@@ -101,6 +101,14 @@ ActiveRecord::Schema[8.0].define(version: 2025_08_20_084312) do
     t.index ["relationship_severance_event_id"], name: "idx_on_relationship_severance_event_id_403f53e707"
   end
 
+  create_table "account_secrets", force: :cascade do |t|
+    t.bigint "account_id", null: false
+    t.text "private_key"
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+    t.index ["account_id"], name: "index_account_secrets_on_account_id", unique: true
+  end
+
   create_table "account_stats", force: :cascade do |t|
     t.bigint "account_id", null: false
     t.bigint "statuses_count", default: 0, null: false
@@ -1356,6 +1364,7 @@ ActiveRecord::Schema[8.0].define(version: 2025_08_20_084312) do
   add_foreign_key "account_pins", "accounts", on_delete: :cascade
   add_foreign_key "account_relationship_severance_events", "accounts", on_delete: :cascade
   add_foreign_key "account_relationship_severance_events", "relationship_severance_events", on_delete: :cascade
+  add_foreign_key "account_secrets", "accounts", on_delete: :cascade
   add_foreign_key "account_stats", "accounts", on_delete: :cascade
   add_foreign_key "account_statuses_cleanup_policies", "accounts", on_delete: :cascade
   add_foreign_key "account_warnings", "accounts", column: "target_account_id", on_delete: :cascade

lib/mastodon/cli/accounts.rb

@@ -614,9 +614,10 @@ module Mastodon::CLI
     def rotate_keys_for_account(account, delay = 0)
       fail_with_message 'No such account' if account.nil?
 
-      old_key = account.private_key
+      old_key = account.account_secret.private_key
       new_key = OpenSSL::PKey::RSA.new(2048)
-      account.update(private_key: new_key.to_pem, public_key: new_key.public_key.to_pem)
+      account.account_secret.update!(private_key: new_key.to_pem)
+      account.update!(public_key: new_key.public_key.to_pem)
       ActivityPub::UpdateDistributionWorker.perform_in(delay, account.id, { 'sign_with' => old_key })
     end
   end

lib/tasks/tests.rake

@@ -51,7 +51,7 @@ namespace :tests do
         exit(1)
       end
 
-      if Account.find(Account::INSTANCE_ACTOR_ID).private_key.blank?
+      if Account.find(Account::INSTANCE_ACTOR_ID).account_secret.private_key.blank?
         puts 'Instance actor does not have a private key'
         exit(1)
       end
@@ -236,10 +236,16 @@ namespace :tests do
           (4, 'User', 1, 'trends', E'--- false\n', now(), now());
 
         INSERT INTO "accounts"
-          (id, username, domain, private_key, public_key, created_at, updated_at)
+          (id, username, domain, public_key, created_at, updated_at)
         VALUES
-          (10, 'kmruser', NULL, #{user_private_key}, #{user_public_key}, now(), now()),
-          (11, 'qcuser', NULL, #{user_private_key}, #{user_public_key}, now(), now());
+          (10, 'kmruser', NULL, #{user_public_key}, now(), now()),
+          (11, 'qcuser', NULL, #{user_public_key}, now(), now());
+
+        INSERT INTO "account_secrets"
+          (account_id, private_key, created_at, updated_at)
+        VALUES
+          (10, #{user_private_key}, now(), now()),
+          (11, #{user_private_key}, now(), now());
 
         INSERT INTO "users"
           (id, account_id, email, created_at, updated_at, admin, locale, chosen_languages)
@@ -302,37 +308,49 @@ namespace :tests do
         -- accounts
 
         INSERT INTO "accounts"
-          (id, username, domain, private_key, public_key, created_at, updated_at)
+          (id, username, domain, public_key, created_at, updated_at)
         VALUES
-          (1, 'admin', NULL, #{admin_private_key}, #{admin_public_key}, now(), now()),
-          (2, 'user',  NULL, #{user_private_key},  #{user_public_key},  now(), now());
+          (1, 'admin', NULL, #{admin_public_key}, now(), now()),
+          (2, 'user',  NULL, #{user_public_key},  now(), now());
+
+        INSERT INTO "account_secrets"
+          (account_id, private_key, created_at, updated_at)
+        VALUES
+          (1, #{admin_private_key}, now(), now()),
+          (2, #{user_private_key}, now(), now());
 
         INSERT INTO "accounts"
-          (id, username, domain, private_key, public_key, created_at, updated_at, remote_url, salmon_url)
+          (id, username, domain, 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', #{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', #{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', #{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, 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', #{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"
-          (id, username, domain, private_key, public_key, created_at, updated_at)
+          (id, username, domain, public_key, created_at, updated_at)
         VALUES
-          (7, 'user', #{local_domain}, #{user_private_key}, #{user_public_key}, now(), now()),
-          (8, 'pt_user', NULL, #{user_private_key}, #{user_public_key}, now(), now());
+          (7, 'user', #{local_domain}, #{user_public_key}, now(), now()),
+          (8, 'pt_user', NULL, #{user_public_key}, now(), now());
+
+        INSERT INTO "account_secrets"
+          (account_id, private_key, created_at, updated_at)
+        VALUES
+          (7, #{user_private_key}, now(), now()),
+          (8, #{user_private_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, 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(),
+          (9, 'evil', 'activitypub.com', #{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);
 

spec/fabricators/account_fabricator.rb

@@ -9,11 +9,11 @@ Fabricator(:account) do
   username            { sequence(:username) { |i| "#{Faker::Internet.user_name(separators: %w(_))}#{i}" } }
   last_webfingered_at { Time.now.utc }
   public_key          { public_key }
-  private_key         { private_key }
   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]}" }
+  account_secret      { |attrs| attrs[:domain].nil? ? Fabricate.build(:account_secret, account: nil, private_key: private_key) : nil }
   discoverable        true
   indexable           true
 end

spec/fabricators/account_secret_fabricator.rb

@@ -0,0 +1,6 @@
+# frozen_string_literal: true
+
+Fabricator(:account_secret) do
+  account
+  private_key { OpenSSL::PKey::RSA.new(2048).to_pem }
+end

spec/lib/mastodon/cli/accounts_spec.rb

@@ -942,14 +942,14 @@ RSpec.describe Mastodon::CLI::Accounts do
       let(:arguments) { [account.username] }
 
       it 'correctly rotates keys for the specified account' do
-        old_private_key = account.private_key
+        old_private_key = account.account_secret.private_key
         old_public_key = account.public_key
 
         expect { subject }
           .to output_results('OK')
         account.reload
 
-        expect(account.private_key).to_not eq(old_private_key)
+        expect(account.account_secret.private_key).to_not eq(old_private_key)
         expect(account.public_key).to_not eq(old_public_key)
       end
 
@@ -977,14 +977,14 @@ RSpec.describe Mastodon::CLI::Accounts do
       let(:options) { { all: true } }
 
       it 'correctly rotates keys for all local accounts' do
-        old_private_keys = accounts.map(&:private_key)
+        old_private_keys = accounts.map { |account| account.account_secret.private_key }
         old_public_keys = accounts.map(&:public_key)
 
         expect { subject }
           .to output_results('rotated')
         accounts.each(&:reload)
 
-        expect(accounts.map(&:private_key)).to_not eq(old_private_keys)
+        expect(accounts.map { |account| account.account_secret.private_key }).to_not eq(old_private_keys)
         expect(accounts.map(&:public_key)).to_not eq(old_public_keys)
       end
 

spec/models/account_spec.rb

@@ -743,9 +743,8 @@ RSpec.describe Account do
     it 'generates keys' do
       account = described_class.create!(domain: nil, username: 'user_without_keys')
 
-      expect(account)
-        .to be_private_key
-        .and be_public_key
+      expect(account.account_secret.private_key).to be_present
+      expect(account.public_key).to be_present
       expect(account.keypair)
         .to be_private
         .and be_public