Add support for multiple keypairs for remote accounts (#38279)

spec/fabricators/keypair_fabricator.rb

@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+keypair     = OpenSSL::PKey::RSA.new(2048)
+public_key  = keypair.public_key.to_pem
+private_key = keypair.to_pem
+
+Fabricator(:keypair) do
+  account
+  type        :rsa
+  public_key  public_key
+  expires_at  nil
+  revoked     false
+
+  after_build do |keypair|
+    keypair.uri ||= ActivityPub::TagManager.instance.key_uri_for(keypair.account)
+    keypair.private_key ||= private_key if keypair.account.local?
+  end
+end

spec/lib/activitypub/linked_data_signature_spec.rb

@@ -7,6 +7,7 @@ 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(:raw_json) do
@@ -25,7 +26,7 @@ RSpec.describe ActivityPub::LinkedDataSignature do
     context 'when signature matches' do
       let(:raw_signature) do
         {
-          'creator' => 'http://example.com/alice',
+          'creator' => keyid,
           'created' => '2017-09-23T20:21:34Z',
         }
       end
@@ -40,7 +41,7 @@ RSpec.describe ActivityPub::LinkedDataSignature do
     context 'when local account record is missing a public key' do
       let(:raw_signature) do
         {
-          'creator' => 'http://example.com/alice',
+          'creator' => keyid,
           'created' => '2017-09-23T20:21:34Z',
         }
       end
@@ -59,15 +60,14 @@ RSpec.describe ActivityPub::LinkedDataSignature do
 
         allow(ActivityPub::FetchRemoteKeyService).to receive(:new).and_return(service_stub)
 
-        allow(service_stub).to receive(:call).with('http://example.com/alice') do
-          sender.update!(public_key: old_key)
-          sender
+        allow(service_stub).to receive(:call).with(keyid) do
+          Keypair.new(account: sender, type: :rsa, public_key: old_key, uri: keyid)
         end
       end
 
       it 'fetches key and returns creator' do
         expect(subject.verify_actor!).to eq sender
-        expect(service_stub).to have_received(:call).with('http://example.com/alice').once
+        expect(service_stub).to have_received(:call).with(keyid).once
       end
     end
 
@@ -82,7 +82,7 @@ RSpec.describe ActivityPub::LinkedDataSignature do
     context 'when signature is tampered' do
       let(:raw_signature) do
         {
-          'creator' => 'http://example.com/alice',
+          'creator' => keyid,
           'created' => '2017-09-23T20:21:34Z',
         }
       end
@@ -100,7 +100,7 @@ RSpec.describe ActivityPub::LinkedDataSignature do
 
       let(:raw_signature) do
         {
-          'creator' => 'http://example.com/alice',
+          'creator' => keyid,
           'created' => '2017-09-23T20:21:34Z',
         }
       end
@@ -116,7 +116,7 @@ RSpec.describe ActivityPub::LinkedDataSignature do
 
       let(:raw_signature) do
         {
-          'creator' => 'http://example.com/alice',
+          'creator' => keyid,
           'created' => '2017-09-23T20:21:34Z',
         }
       end
@@ -132,7 +132,7 @@ RSpec.describe ActivityPub::LinkedDataSignature do
 
       let(:raw_signature) do
         {
-          'creator' => 'http://example.com/alice',
+          'creator' => keyid,
           'created' => '2017-09-23T20:21:34Z',
         }
       end

spec/models/keypair_spec.rb

@@ -0,0 +1,58 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Keypair do
+  describe '#keypair' do
+    let(:keypair) { Fabricate(:keypair) }
+
+    it 'returns an RSA key pair' do
+      expect(keypair.keypair).to be_instance_of OpenSSL::PKey::RSA
+    end
+  end
+
+  describe 'from_keyid' do
+    context 'when a key with the given key ID exists' do
+      let(:account) { Fabricate(:account, domain: 'example.com') }
+      let(:keypair) { Fabricate(:keypair, account: account) }
+
+      it 'returns the expected Keypair' do
+        expect(described_class.from_keyid(keypair.uri))
+          .to eq keypair
+      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') }
+      let(:keyid) { "#{ActivityPub::TagManager.instance.uri_for(account)}#main-rsa-key" }
+
+      it 'returns the expected Keypair' do
+        expect(described_class.from_keyid(keyid))
+          .to have_attributes(
+            account: account,
+            type: 'rsa',
+            uri: keyid
+          )
+      end
+    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(:keyid) { "#{ActivityPub::TagManager.instance.uri_for(account)}#main-rsa-key" }
+
+      it 'returns nil' do
+        expect(described_class.from_keyid(keyid))
+          .to be_nil
+      end
+    end
+
+    context 'when no key with the expected key ID exists and no matching account exists' do
+      let(:keyid) { 'https://example.com/alice#main-key' }
+
+      it 'returns nil' do
+        expect(described_class.from_keyid(keyid))
+          .to be_nil
+      end
+    end
+  end
+end

spec/services/activitypub/fetch_remote_key_service_spec.rb

@@ -55,7 +55,7 @@ RSpec.describe ActivityPub::FetchRemoteKeyService do
   end
 
   describe '#call' do
-    let(:account) { subject.call(public_key_id) }
+    let(:keypair) { subject.call(public_key_id) }
 
     context 'when the key is a sub-object from the actor' do
       before do
@@ -63,7 +63,7 @@ RSpec.describe ActivityPub::FetchRemoteKeyService do
       end
 
       it 'returns the expected account' do
-        expect(account.uri).to eq 'https://example.com/alice'
+        expect(keypair.account.uri).to eq 'https://example.com/alice'
       end
     end
 
@@ -75,7 +75,7 @@ RSpec.describe ActivityPub::FetchRemoteKeyService do
       end
 
       it 'returns the expected account' do
-        expect(account.uri).to eq 'https://example.com/alice'
+        expect(keypair.account.uri).to eq 'https://example.com/alice'
       end
     end
 
@@ -88,7 +88,7 @@ RSpec.describe ActivityPub::FetchRemoteKeyService do
       end
 
       it 'returns the nil' do
-        expect(account).to be_nil
+        expect(keypair).to be_nil
       end
     end
   end

spec/services/activitypub/process_account_service_spec.rb

@@ -94,6 +94,109 @@ RSpec.describe ActivityPub::ProcessAccountService do
     end
   end
 
+  context 'with a single keypair' do
+    let(:payload) do
+      {
+        id: 'https://foo.test/actor',
+        type: 'Actor',
+        inbox: 'https://foo.test/inbox',
+        preferredUsername: 'alice',
+        publicKey: {
+          id: 'https://foo.test/actor#key1',
+          owner: 'https://foo.test/actor',
+          publicKeyPem: 'foo',
+        },
+      }.with_indifferent_access
+    end
+
+    it 'stores the key' do
+      account = subject.call('alice', 'example.com', payload)
+
+      expect(account.public_key).to eq ''
+      expect(account.keypairs).to contain_exactly(
+        have_attributes(
+          uri: 'https://foo.test/actor#key1',
+          type: 'rsa'
+        )
+      )
+    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') }
+
+      it 'invalidates the legacy key and stores the new key' do
+        expect { subject.call('alice', 'example.com', payload) }
+          .to change { alice.reload.public_key }.to('')
+          .and change { alice.reload.keypairs.to_a }.from([]).to(contain_exactly(have_attributes({ uri: 'https://foo.test/actor#key1', type: 'rsa' })))
+      end
+    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: '') }
+
+      before do
+        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
+        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' })))
+
+        expect(alice.reload.public_key)
+          .to eq ''
+      end
+    end
+  end
+
+  context 'with multiple keypairs' do
+    let(:payload) do
+      {
+        id: 'https://foo.test/actor',
+        type: 'Actor',
+        inbox: 'https://foo.test/inbox',
+        preferredUsername: 'alice',
+        publicKey: [
+          {
+            id: 'https://foo.test/actor#key1',
+            owner: 'https://foo.test/actor',
+            publicKeyPem: 'foo',
+          },
+          {
+            id: 'https://foo.test/actor#key2',
+            owner: 'https://foo.test/actor',
+            publicKeyPem: 'bar',
+          },
+        ],
+      }.with_indifferent_access
+    end
+
+    it 'stores the keys' do
+      account = subject.call('alice', 'example.com', payload)
+
+      expect(account.public_key).to eq ''
+      expect(account.keypairs).to contain_exactly(
+        have_attributes(
+          uri: 'https://foo.test/actor#key1',
+          type: 'rsa'
+        ),
+        have_attributes(
+          uri: 'https://foo.test/actor#key2',
+          type: 'rsa'
+        )
+      )
+    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') }
+
+      it 'invalidates the legacy key and stores the new keys' do
+        expect { subject.call('alice', 'example.com', payload) }
+          .to change { alice.reload.public_key }.to('')
+          .and change { alice.keypairs.to_a }.from([]).to(contain_exactly(have_attributes({ uri: 'https://foo.test/actor#key1', type: 'rsa' }), have_attributes({ uri: 'https://foo.test/actor#key2', type: 'rsa' })))
+      end
+    end
+  end
+
   context 'with attribution domains' do
     let(:payload) do
       {