Change new local accounts to use the Keypair model instead of legacy attributes (#39684)

This commit is contained in:
Claire
2026-07-03 10:40:58 +02:00
committed by GitHub
parent eb167fdc3b
commit 2e486d29b5
12 changed files with 52 additions and 30 deletions

View File

@@ -490,7 +490,7 @@ class Account < ApplicationRecord
before_destroy :clean_feed_manager
def ensure_keys!
return unless local? && private_key.blank? && public_key.blank?
return unless local? && private_key.blank? && public_key.blank? && keypairs.empty?
generate_keys
save!
@@ -510,11 +510,10 @@ class Account < ApplicationRecord
end
def generate_keys
return unless local? && private_key.blank? && public_key.blank?
return unless local? && private_key.blank? && public_key.blank? && keypairs.empty?
keypair = OpenSSL::PKey::RSA.new(2048)
self.private_key = keypair.to_pem
self.public_key = keypair.public_key.to_pem
keypairs << keypairs.build(local_fragment: "#rsa-#{SecureRandom.hex(8)}", type: :rsa, public_key: keypair.public_key.to_pem, private_key: keypair.to_pem)
end
def normalize_domain

View File

@@ -1,17 +1,35 @@
# frozen_string_literal: true
Fabricator(:account) do
transient :suspended, :silenced
transient :suspended, :silenced, :legacy_keypair
username { sequence(:username) { |i| "#{Faker::Internet.user_name(separators: %w(_))}#{i}" } }
last_webfingered_at { Time.now.utc }
public_key { SigningKeysHelpers::PUBLIC_RSA_TEST_KEY }
private_key { |attrs| attrs[:domain].present? ? nil : SigningKeysHelpers::PRIVATE_RSA_TEST_KEY }
public_key { |attrs| attrs[:legacy_keypair] ? SigningKeysHelpers::PUBLIC_RSA_TEST_KEY : '' }
private_key { |attrs| attrs[:legacy_keypair] && attrs[:domain].nil? ? SigningKeysHelpers::PRIVATE_RSA_TEST_KEY : nil }
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]}" }
discoverable true
indexable true
# This is not strictly needed but this avoids generating multiple keys
# and, when `store_private_key` is passed, stores private keys for use in request specs
keypairs do |attrs|
if attrs[:legacy_keypair] || attrs[:public_key].present?
[]
else
[
Keypair.new(
type: :rsa,
public_key: SigningKeysHelpers::PUBLIC_RSA_TEST_KEY,
private_key: attrs[:domain].blank? ? SigningKeysHelpers::PRIVATE_RSA_TEST_KEY : nil,
uri: attrs[:domain].nil? ? nil : "https://#{attrs[:domain]}/users/#{attrs[:username]}#main-key",
local_fragment: attrs[:domain].nil? ? '#main-key' : nil
),
]
end
end
end
Fabricator(:remote_account, from: :account) do

View File

@@ -1,7 +1,7 @@
# frozen_string_literal: true
Fabricator(:keypair) do
account
account { Fabricate(:account, keypairs: []) }
type :rsa
public_key SigningKeysHelpers::PUBLIC_RSA_TEST_KEY
expires_at nil
@@ -12,7 +12,7 @@ Fabricator(:keypair) do
keypair.private_key ||= SigningKeysHelpers::PRIVATE_RSA_TEST_KEY
keypair.local_fragment ||= "##{Random.hex}"
else
keypair.uri ||= ActivityPub::TagManager.instance.key_uri_for(keypair.account)
keypair.uri ||= "#{ActivityPub::TagManager.instance.uri_for(keypair.account)}##{Random.hex}"
end
end
end

View File

@@ -7,8 +7,8 @@ RSpec.describe ActivityPub::LinkedDataSignature do
subject { described_class.new(json) }
let(:keyid) { 'http://example.com/alice#rsa-key' }
let!(:sender) { Fabricate(:account, uri: 'http://example.com/alice', domain: 'example.com') }
let(:keyid) { sender.keypair.uri }
let(:raw_json) do
{
@@ -55,8 +55,8 @@ RSpec.describe ActivityPub::LinkedDataSignature do
signature
# Unset key
old_key = sender.public_key
sender.update!(private_key: '', public_key: '')
old_key = sender.keypair.public_key
sender.keypairs.delete_all
allow(ActivityPub::FetchRemoteKeyService).to receive(:new).and_return(service_stub)
@@ -153,6 +153,9 @@ RSpec.describe ActivityPub::LinkedDataSignature do
expect(subject['signature']).to be_a Hash
expect(subject['signature']['signatureValue']).to be_present
expect(Array(subject['@context'])).to include('https://w3id.org/security/v1')
# Verification requires fetching the key, which we don't support for local keys
allow(Keypair).to receive(:from_keyid).with(sender.keypair(type: :rsa).full_uri).and_return(sender.keypair(type: :rsa))
expect(described_class.new(subject).verify_actor!).to eq sender
end
end

View File

@@ -655,7 +655,7 @@ RSpec.describe Mastodon::CLI::Accounts do
context 'when there are duplicate URI accounts' do
before do
Fabricate.times(2, :account, domain: 'host.example', uri: uri)
Fabricate.times(2, :account, domain: 'host.example', uri: uri, legacy_keypair: true)
allow(ActivityPub::FetchRemoteAccountService).to receive(:new).and_return(service_double)
end

View File

@@ -89,7 +89,7 @@ RSpec.describe Mastodon::CLI::Maintenance do
def prepare_duplicate_data
ActiveRecord::Base.connection.remove_index :accounts, name: :index_accounts_on_username_and_domain_lower
duplicate_record(:account, username: duplicate_account_username, domain: duplicate_account_domain)
duplicate_record(:account, username: duplicate_account_username, domain: duplicate_account_domain, legacy_keypair: true)
duplicate_record(:account, username: duplicate_account_username, domain: nil)
end

View File

@@ -736,9 +736,9 @@ 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.private_key).to be_nil
expect(account.public_key).to eq ''
expect(account.keypair.keypair)
.to be_private
.and be_public

View File

@@ -45,8 +45,8 @@ RSpec.describe Keypair do
end
end
context 'when no key with the expected key ID exists but there is an account with the same ID and a key' do
let(:account) { Fabricate(:account, domain: 'example.com') }
context 'when no key with the expected key ID exists but there is an account with the same ID and a legacy key' do
let(:account) { Fabricate(:account, domain: 'example.com', legacy_keypair: true) }
let(:keyid) { "#{ActivityPub::TagManager.instance.uri_for(account)}#main-rsa-key" }
it 'returns the expected Keypair' do
@@ -60,7 +60,7 @@ RSpec.describe Keypair do
end
context 'when no key with the expected key ID exists but there is an account with the same ID and no key' do
let(:account) { Fabricate(:account, domain: 'example.com', public_key: '', private_key: nil) }
let(:account) { Fabricate(:account, domain: 'example.com') }
let(:keyid) { "#{ActivityPub::TagManager.instance.uri_for(account)}#main-rsa-key" }
it 'returns nil' do

View File

@@ -23,7 +23,7 @@ RSpec.describe 'Instance actor endpoint' do
inbox: instance_actor_inbox_url,
outbox: instance_actor_outbox_url,
publicKey: include(
id: instance_actor_url(anchor: 'main-key')
id: start_with(instance_actor_url)
),
url: about_more_url(instance_actor: true)
)

View File

@@ -704,7 +704,7 @@ RSpec.describe 'signature verification concern' do
end
context 'with a known account' do
let!(:actor) { Fabricate(:account, domain: 'remote.domain', uri: 'https://remote.domain/users/bob', private_key: nil, public_key: '') }
let!(:actor) { Fabricate(:account, domain: 'remote.domain', uri: 'https://remote.domain/users/bob') }
before do
Fabricate(:keypair, account: actor, type: :ed25519, public_key: actor_keypair.public_to_pem, uri: 'https://remote.domain/users/bob#main-key')

View File

@@ -125,7 +125,7 @@ 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') }
let!(:alice) { Fabricate(:account, uri: 'https://foo.test/actor', domain: 'example.com', username: 'alice', legacy_keypair: true) }
it 'invalidates the legacy key and stores the new key' do
expect { subject.call('alice', 'example.com', payload) }
@@ -135,13 +135,14 @@ RSpec.describe ActivityPub::ProcessAccountService do
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', public_key: '') }
let!(:alice) { Fabricate(:account, uri: 'https://foo.test/actor', domain: 'example.com', username: 'alice') }
before do
alice.keypairs.delete_all
Fabricate(:keypair, account: alice, uri: 'https://foo.test/actor#old-key', type: :rsa)
end
it 'invalidates the legacy key and stores the new key' do
it 'invalidates the old key and stores the new key' do
expect { subject.call('alice', 'example.com', 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: })))
@@ -222,7 +223,7 @@ 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') }
let!(:alice) { Fabricate(:account, uri: 'https://foo.test/actor', domain: 'example.com', username: 'alice', legacy_keypair: true) }
it 'invalidates the legacy key and stores the new key' do
expect { subject.call('alice', 'example.com', payload) }
@@ -232,9 +233,10 @@ RSpec.describe ActivityPub::ProcessAccountService do
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', public_key: '') }
let!(:alice) { Fabricate(:account, uri: 'https://foo.test/actor', domain: 'example.com', username: 'alice') }
before do
alice.keypairs.delete_all
Fabricate(:keypair, account: alice, uri: 'https://foo.test/actor#old-key', type: :rsa)
end
@@ -289,7 +291,7 @@ 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') }
let!(:alice) { Fabricate(:account, uri: 'https://foo.test/actor', domain: 'example.com', username: 'alice', legacy_keypair: true) }
it 'invalidates the legacy key and stores the new keys' do
expect { subject.call('alice', 'example.com', payload) }
@@ -345,7 +347,7 @@ 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') }
let!(:alice) { Fabricate(:account, uri: 'https://foo.test/actor', domain: 'example.com', username: 'alice', legacy_keypair: true) }
it 'invalidates the legacy key and stores the new keys' do
expect { subject.call('alice', 'example.com', payload) }

View File

@@ -70,7 +70,7 @@ RSpec.describe FetchResourceService do
it 'signs request' do
subject
expect(a_request(:get, url).with(headers: { 'Signature' => /keyId="#{Regexp.escape(ActivityPub::TagManager.instance.key_uri_for(Account.representative))}"/ })).to have_been_made
expect(a_request(:get, url).with(headers: { 'Signature' => /keyId="#{Regexp.escape(Account.representative.keypair(type: :rsa).full_uri)}"/ })).to have_been_made
end
context 'when content type is application/atom+xml' do