Fix FetchRemoteKeyService possibly returning incorrect keys (#38511)

app/services/activitypub/fetch_remote_key_service.rb

@@ -9,12 +9,14 @@ class ActivityPub::FetchRemoteKeyService < BaseService
   def call(uri, suppress_errors: true)
     raise Error, 'No key URI given' if uri.blank?
 
+    @suppress_errors = suppress_errors
+    @uri = uri
     @json = fetch_resource(uri, false)
 
     raise Error, "Unable to fetch key JSON at #{uri}" if @json.nil?
     raise Error, "Unsupported JSON-LD context for document #{uri}" unless supported_context?(@json) || (supported_security_context?(@json) && @json['owner'].present? && !actor_type?)
     raise Error, "Unexpected object type for key #{uri}" unless expected_type?
-    return Keypair.from_legacy_account(find_actor(@json['id'], @json, suppress_errors), uri: uri) if actor_type?
+    return keypair_from_actor_json(@json['id'], @json) if actor_type?
 
     @owner = fetch_resource(owner_uri, true)
 
@@ -23,8 +25,7 @@ class ActivityPub::FetchRemoteKeyService < BaseService
     raise Error, "Unexpected object type for actor #{owner_uri} (expected any of: #{SUPPORTED_TYPES})" unless expected_owner_type?
     raise Error, "publicKey id for #{owner_uri} does not correspond to #{@json['id']}" unless confirmed_owner?
 
-    # TODO: change to fetch and persist key
-    Keypair.from_legacy_account(find_actor(owner_uri, @owner, suppress_errors), uri: uri)
+    keypair_from_actor_json(owner_uri, @owner)
   rescue Error => e
     Rails.logger.debug { "Fetching key #{uri} failed: #{e.message}" }
     raise unless suppress_errors
@@ -32,9 +33,19 @@ class ActivityPub::FetchRemoteKeyService < BaseService
 
   private
 
-  def find_actor(uri, prefetched_body, suppress_errors)
+  def keypair_from_actor_json(actor_uri, actor_json)
+    actor = find_actor(actor_uri, actor_json)
+    return if actor.nil?
+
+    keypair = actor.keypairs.find_by(uri: @uri)
+    return keypair if keypair.present?
+
+    Keypair.from_legacy_account(actor, uri: @uri) if actor.public_key.present?
+  end
+
+  def find_actor(uri, prefetched_body)
     actor   = ActivityPub::TagManager.instance.uri_to_actor(uri)
-    actor ||= ActivityPub::FetchRemoteActorService.new.call(uri, prefetched_body: prefetched_body, suppress_errors: suppress_errors)
+    actor ||= ActivityPub::FetchRemoteActorService.new.call(uri, prefetched_body: prefetched_body, suppress_errors: @suppress_errors)
     actor
   end
 

spec/services/activitypub/fetch_remote_key_service_spec.rb

@@ -64,6 +64,8 @@ RSpec.describe ActivityPub::FetchRemoteKeyService do
 
       it 'returns the expected account' do
         expect(keypair.account.uri).to eq 'https://example.com/alice'
+        expect(keypair.uri).to eq public_key_id
+        expect(keypair.public_key).to eq public_key_pem
       end
     end
 
@@ -76,6 +78,8 @@ RSpec.describe ActivityPub::FetchRemoteKeyService do
 
       it 'returns the expected account' do
         expect(keypair.account.uri).to eq 'https://example.com/alice'
+        expect(keypair.uri).to eq public_key_id
+        expect(keypair.public_key).to eq public_key_pem
       end
     end