Improve collection item verification (#39096)

app/services/activitypub/process_featured_item_service.rb

@@ -11,7 +11,10 @@ class ActivityPub::ProcessFeaturedItemService
     @collection = collection
     @request_id = request_id
     @item_json = uri_or_object.is_a?(String) ? fetch_resource(uri_or_object, true) : uri_or_object
+    @actor_uri = value_or_id(@item_json['featuredObject'])
+    @approval_uri = value_or_id(@item_json['featureAuthorization'])
     return if non_matching_uri_hosts?(@collection.uri, @item_json['id'])
+    return if non_matching_actor_and_approval_uris?
 
     with_redis_lock("collection_item:#{@item_json['id']}") do
       @collection_item = existing_item || pre_approved_item || new_item
@@ -22,8 +25,6 @@ class ActivityPub::ProcessFeaturedItemService
         object_uri: value_or_id(@item_json['featuredObject'])
       )
 
-      @approval_uri = @item_json['featureAuthorization']
-
       verify_authorization! unless @collection_item&.account&.local?
 
       @collection_item
@@ -48,6 +49,12 @@ class ActivityPub::ProcessFeaturedItemService
     )
   end
 
+  def non_matching_actor_and_approval_uris?
+    return false if ActivityPub::TagManager.instance.local_uri?(@actor_uri)
+
+    non_matching_uri_hosts?(@actor_uri, @approval_uri)
+  end
+
   def verify_authorization!
     ActivityPub::VerifyFeaturedItemService.new.call(@collection_item, @approval_uri, request_id: @request_id)
   rescue Mastodon::RecursionLimitExceededError, Mastodon::UnexpectedResponseError, *Mastodon::HTTP_CONNECTION_ERRORS

app/services/activitypub/verify_featured_item_service.rb

@@ -12,8 +12,11 @@ class ActivityPub::VerifyFeaturedItemService
       return
     end
 
-    return if non_matching_uri_hosts?(approval_uri, @authorization['interactionTarget'])
+    @collection_uri = value_or_id(@authorization['interactingObject'])
-    return unless matching_type? && matching_collection_uri?
+    @actor_uri = value_or_id(@authorization['interactionTarget'])
+
+    return if non_matching_uri_hosts?(approval_uri, @actor_uri)
+    return unless matching_type? && matching_collection_uri? && matching_actors?
 
     account = Account.where(uri: @collection_item.object_uri).first
     account ||= ActivityPub::FetchRemoteAccountService.new.call(@collection_item.object_uri, request_id:)
@@ -29,6 +32,10 @@ class ActivityPub::VerifyFeaturedItemService
   end
 
   def matching_collection_uri?
-    @collection_item.collection.uri == @authorization['interactingObject']
+    @collection_item.collection.uri == @collection_uri
+  end
+
+  def matching_actors?
+    @collection_item.object_uri == @actor_uri
   end
 end

spec/services/activitypub/process_featured_item_service_spec.rb

@@ -40,6 +40,17 @@ RSpec.describe ActivityPub::ProcessFeaturedItemService do
         end.to_not change(CollectionItem, :count)
       end
     end
+
+    context 'when the actor URI does not match the approval URI' do
+      let(:featured_object_uri) { 'https://example.com/actor/1' }
+      let(:feature_authorization_uri) { 'https://other.example.com/auth/1' }
+
+      it 'does not create a collection item and returns `nil`' do
+        expect do
+          expect(subject.call(collection, object, position:)).to be_nil
+        end.to_not change(CollectionItem, :count)
+      end
+    end
   end
 
   context 'when the collection item is inlined' do

spec/services/activitypub/verify_featured_item_service_spec.rb

@@ -85,4 +85,25 @@ RSpec.describe ActivityPub::VerifyFeaturedItemService do
       expect(collection_item).to be_rejected
     end
   end
+
+  context 'when the authorization references a different account' do
+    let(:verification_json) do
+      {
+        '@context' => 'https://www.w3.org/ns/activitystreams',
+        'type' => 'FeatureAuthorization',
+        'id' => approval_uri,
+        'interactionTarget' => 'https://example.com/actor/2',
+        'interactingObject' => collection.uri,
+      }
+    end
+
+    before { featured_account }
+
+    it 'does not verify the item' do
+      subject.call(collection_item, approval_uri)
+
+      expect(collection_item.account_id).to be_nil
+      expect(collection_item).to be_pending
+    end
+  end
 end