Only accept follows for the named recipient

.github/changelog/fix-follow-target

@@ -0,0 +1,4 @@
+Significance: patch
+Type: fixed
+
+Follow requests are now only accepted when they are meant for the account that received them, and profile links on other sites no longer count as accounts on your site.

includes/class-application.php

@@ -298,18 +298,8 @@ public static function is_application_resource( $uri ) {
 
 		list( $identifier, $host ) = $identifier_and_host;
 
-		/*
-		 * The resource must point at this site, or at its pre-migration host. Both sides are folded:
-		 * the requested host is whatever the caller typed, and the stored ones are whatever an admin did.
-		 */
-		$host = normalize_host( fold_host( $host ) );
-
-		// A host of `.` or `[]` folds away to nothing, which would otherwise match an unset old host.
-		if ( '' === $host ) {
-			return false;
-		}
-
-		if ( normalize_host( fold_host( home_host() ) ) !== $host && normalize_host( fold_host( \get_option( 'activitypub_old_host' ) ) ) !== $host ) {
+		// The resource must point at this site, or at its pre-migration host.
+		if ( ! is_local_host( $host ) ) {
 			return false;
 		}
 

includes/collection/class-actors.php

@@ -13,9 +13,8 @@
 use Activitypub\Model\User;
 use Activitypub\Signature;
 
-use function Activitypub\fold_host;
+use function Activitypub\is_local_host;
 use function Activitypub\is_user_type_disabled;
-use function Activitypub\normalize_host;
 use function Activitypub\normalize_url;
 use function Activitypub\object_to_uri;
 use function Activitypub\url_to_authorid;
@@ -239,13 +238,18 @@ public static function get_id_by_resource( $uri ) {
 		// Extract the scheme, defaulting to `acct` for bare `user@host` handles.
 		$scheme = \wp_parse_url( $uri, PHP_URL_SCHEME ) ?: 'acct';
 
-		// @todo: handle old domain URIs here before we serve a new domain below when we shouldn't.
-		// Although maybe passing through to ::get_by_username() is enough?
-
 		switch ( $scheme ) {
 			// Check for http(s) URIs.
 			case 'http':
 			case 'https':
+				if ( ! is_local_host( \wp_parse_url( $uri, \PHP_URL_HOST ) ) ) {
+					return new \WP_Error(
+						'activitypub_no_user_found',
+						\__( 'Actor not found', 'activitypub' ),
+						array( 'status' => 404 )
+					);
+				}
+
 				// Check for http(s)://blog.example.com/@username.
 				$resource_path = \wp_parse_url( $uri, PHP_URL_PATH );
 
@@ -289,11 +293,8 @@ public static function get_id_by_resource( $uri ) {
 			case 'acct':
 				$uri        = \str_replace( 'acct:', '', $uri );
 				$identifier = \substr( $uri, 0, \strrpos( $uri, '@' ) );
-				$host       = normalize_host( fold_host( \substr( \strrchr( $uri, '@' ), 1 ) ) );
-				$blog_host  = normalize_host( fold_host( \wp_parse_url( \home_url( '/' ), \PHP_URL_HOST ) ) );
 
-				// A host of `.` or `[]` folds away to nothing, which would otherwise match an unset old host.
-				if ( '' === $host || ( $blog_host !== $host && normalize_host( fold_host( \get_option( 'activitypub_old_host' ) ) ) !== $host ) ) {
+				if ( ! is_local_host( \substr( (string) \strrchr( $uri, '@' ), 1 ) ) ) {
 					return new \WP_Error(
 						'activitypub_wrong_host',
 						\__( 'Resource host does not match blog host', 'activitypub' ),

includes/functions-url.php

@@ -235,6 +235,32 @@ function get_authority( $url ) {
 	return $parts['host'];
 }
 
+/**
+ * Check if a host is this site's host or the host it moved from.
+ *
+ * Both sides are folded before they are compared: the host is whatever a caller sent, and the stored
+ * ones are whatever an admin entered. A host of `.` or `[]` folds away to nothing and is dropped, so
+ * it never matches an unset old host.
+ *
+ * @since unreleased
+ *
+ * @param string|null $host The host.
+ *
+ * @return bool True if the host is local, false otherwise.
+ */
+function is_local_host( $host ) {
+	$local = \array_filter(
+		\array_map(
+			function ( $local_host ) {
+				return normalize_host( fold_host( $local_host ) );
+			},
+			array( home_host(), \get_option( 'activitypub_old_host' ) )
+		)
+	);
+
+	return \in_array( normalize_host( fold_host( $host ) ), $local, true );
+}
+
 /**
  * Check if a URL is from the same domain as the site.
  *

includes/handler/class-follow.php

@@ -43,8 +43,12 @@ public static function init() {
 	 * @return void
 	 */
 	public static function handle_follow( $activity, $user_ids ) {
-		// Extract the user ID (follow requests are always for a single user).
-		$user_id = \is_array( $user_ids ) ? \reset( $user_ids ) : $user_ids;
+		// Only a Follow of an actor that received it, and the follower is added to that actor.
+		$user_id = Actors::get_id_by_resource( $activity['object'] ?? '' );
+		if ( \is_wp_error( $user_id ) || ! \in_array( (int) $user_id, \array_map( 'intval', (array) $user_ids ), true ) ) {
+			return;
+		}
+		$user_id = (int) $user_id;
 
 		// Check if the actor already follows the user.
 		$already_following = false;

tests/phpunit/tests/includes/class-test-functions-url.php

@@ -250,4 +250,44 @@ public function authority_provider() {
 			'no host'              => array( '/inbox', '' ),
 		);
 	}
+
+	/**
+	 * A host is local when it folds to this site's host or to the host the site moved from.
+	 *
+	 * @covers \Activitypub\is_local_host
+	 * @dataProvider local_host_provider
+	 *
+	 * @param string|null $host     The host, with `%s` standing for this site's host.
+	 * @param string|null $old_host The stored old host, or null for none.
+	 * @param bool        $expected The expected result.
+	 */
+	public function test_is_local_host( $host, $old_host, $expected ) {
+		if ( null !== $old_host ) {
+			\update_option( 'activitypub_old_host', $old_host );
+		}
+
+		$result = \Activitypub\is_local_host( null === $host ? null : \sprintf( $host, \Activitypub\home_host() ) );
+
+		\delete_option( 'activitypub_old_host' );
+
+		$this->assertSame( $expected, $result );
+	}
+
+	/**
+	 * Hosts for test_is_local_host.
+	 *
+	 * @return array[]
+	 */
+	public function local_host_provider() {
+		return array(
+			'home host'             => array( '%s', null, true ),
+			'home host, other case' => array( 'WWW.%s.', null, true ),
+			'old host'              => array( 'old.example', 'old.example', true ),
+			'old host, unset'       => array( 'old.example', null, false ),
+			'foreign host'          => array( 'evil.example', 'old.example', false ),
+			'empty host'            => array( '', null, false ),
+			'host that folds away'  => array( '.', null, false ),
+			'no host'               => array( null, null, false ),
+		);
+	}
 }

tests/phpunit/tests/includes/collection/class-test-actors.php

@@ -93,11 +93,12 @@ public function test_get_by_various( $item, $expected ) {
 		$path = wp_parse_url( $item, PHP_URL_PATH ) ?? '';
 
 		if ( str_starts_with( $path, '/blog/' ) ) {
+			$host = wp_parse_url( $item, PHP_URL_HOST );
 			add_filter(
 				'home_url',
 				// phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable, Generic.CodeAnalysis.UnusedFunctionParameter.Found
-				function ( $url ) {
-					return 'http://example.org/blog/';
+				function ( $url ) use ( $host ) {
+					return 'http://' . $host . '/blog/';
 				}
 			);
 		}
@@ -228,6 +229,8 @@ public function the_resource_provider() {
 			array( \rtrim( $https_home_url, '/' ), 'Activitypub\Model\Blog' ),
 			array( $home_url . '/@blog/s', 'WP_Error' ),
 			array( $home_url . '/@blogs/', 'WP_Error' ),
+			array( 'https://remote.example/@admin', 'WP_Error' ),
+			array( 'https://remote.example/@blog', 'WP_Error' ),
 		);
 	}
 
@@ -268,7 +271,7 @@ public function test_get_type_by_id() {
 	public function test_disabled_blog_profile() {
 		\update_option( 'activitypub_actor_mode', ACTIVITYPUB_ACTOR_AND_BLOG_MODE );
 
-		$resource = 'http://example.org/@blog';
+		$resource = \home_url( '/@blog' );
 
 		$this->assertEquals( 'Activitypub\Model\Blog', get_class( Actors::get_by_resource( $resource ) ) );
 

tests/phpunit/tests/includes/handler/class-test-follow.php

@@ -547,4 +547,145 @@ public function test_new_hook_fires() {
 		\remove_action( 'activitypub_handled_follow', $new_hook_callback );
 		\remove_filter( 'pre_get_remote_metadata_by_actor', $mock_metadata_callback );
 	}
+
+	/**
+	 * A Follow of another actor is not added to the user whose inbox received it.
+	 *
+	 * @covers ::handle_follow
+	 */
+	public function test_handle_follow_ignores_other_object() {
+		$actor_url = 'https://example.com/other-object-actor';
+
+		$mock_metadata_callback = function () use ( $actor_url ) {
+			return array(
+				'id'                => $actor_url,
+				'type'              => 'Person',
+				'preferredUsername' => 'otherobject',
+				'inbox'             => 'https://example.com/inbox',
+			);
+		};
+		\add_filter( 'pre_get_remote_metadata_by_actor', $mock_metadata_callback );
+
+		$handled_before = \did_action( 'activitypub_handled_follow' );
+
+		Follow::handle_follow(
+			array(
+				'id'     => $actor_url . '/activity/1',
+				'type'   => 'Follow',
+				'actor'  => $actor_url,
+				'object' => 'https://remote.example/users/bob',
+			),
+			self::$user_id
+		);
+
+		\remove_filter( 'pre_get_remote_metadata_by_actor', $mock_metadata_callback );
+
+		$this->assertCount( 0, Followers::get_many( self::$user_id ), 'A Follow of another actor should not add a follower.' );
+		$this->assertSame( $handled_before, \did_action( 'activitypub_handled_follow' ), 'A Follow of another actor should not be handled.' );
+	}
+
+	/**
+	 * With several recipients, the follower is added to the one the Follow names, not the first.
+	 *
+	 * @covers ::handle_follow
+	 */
+	public function test_handle_follow_picks_the_named_recipient() {
+		$other_id  = self::factory()->user->create( array( 'role' => 'author' ) );
+		$actor_url = 'https://example.com/users/several-recipients';
+
+		$mock_metadata_callback = function () use ( $actor_url ) {
+			return array(
+				'id'                => $actor_url,
+				'type'              => 'Person',
+				'preferredUsername' => 'several',
+				'inbox'             => 'https://example.com/inbox',
+			);
+		};
+		\add_filter( 'pre_get_remote_metadata_by_actor', $mock_metadata_callback );
+
+		Follow::handle_follow(
+			array(
+				'id'     => $actor_url . '/activity/1',
+				'type'   => 'Follow',
+				'actor'  => $actor_url,
+				'object' => Actors::get_by_id( self::$user_id )->get_id(),
+			),
+			array( $other_id, self::$user_id )
+		);
+
+		\remove_filter( 'pre_get_remote_metadata_by_actor', $mock_metadata_callback );
+
+		$this->assertCount( 1, Followers::get_many( self::$user_id ), 'The named recipient should get the follower.' );
+		$this->assertCount( 0, Followers::get_many( $other_id ), 'The other recipient should not.' );
+	}
+
+	/**
+	 * A Follow names its object in any form the local actor can be addressed by.
+	 *
+	 * @dataProvider object_form_provider
+	 * @covers ::handle_follow
+	 *
+	 * @param string $form    The way the Follow names the local actor.
+	 * @param bool   $is_blog Whether the Follow is for the blog actor.
+	 */
+	public function test_handle_follow_accepts_object_forms( $form, $is_blog ) {
+		\update_option( 'activitypub_actor_mode', ACTIVITYPUB_ACTOR_AND_BLOG_MODE );
+
+		$user_id = $is_blog ? Actors::BLOG_USER_ID : self::$user_id;
+		$actor   = Actors::get_by_id( $user_id );
+		$objects = array(
+			'id'       => $actor->get_id(),
+			'inline'   => array(
+				'id'   => $actor->get_id(),
+				'type' => 'Person',
+			),
+			'acct'     => 'acct:' . $actor->get_webfinger(),
+			'at_name'  => \home_url( '/@' . $actor->get_preferred_username() ),
+			'home_url' => \home_url( '/' ),
+		);
+
+		$actor_url              = 'https://example.com/users/' . $form . ( $is_blog ? '-blog' : '' );
+		$mock_metadata_callback = function () use ( $actor_url ) {
+			return array(
+				'id'                => $actor_url,
+				'type'              => 'Person',
+				'preferredUsername' => 'follower',
+				'inbox'             => 'https://example.com/inbox',
+			);
+		};
+		\add_filter( 'pre_get_remote_metadata_by_actor', $mock_metadata_callback );
+
+		Follow::handle_follow(
+			array(
+				'id'     => $actor_url . '/activity/1',
+				'type'   => 'Follow',
+				'actor'  => $actor_url,
+				'object' => $objects[ $form ],
+			),
+			$user_id
+		);
+
+		\remove_filter( 'pre_get_remote_metadata_by_actor', $mock_metadata_callback );
+		\delete_option( 'activitypub_actor_mode' );
+
+		$this->assertCount( 1, Followers::get_many( $user_id ), 'A Follow of the local actor should add a follower.' );
+	}
+
+	/**
+	 * Data provider for test_handle_follow_accepts_object_forms.
+	 *
+	 * @return array The object forms.
+	 */
+	public function object_form_provider() {
+		return array(
+			'actor id'      => array( 'id', false ),
+			'inline object' => array( 'inline', false ),
+			'acct uri'      => array( 'acct', false ),
+			'at-name url'   => array( 'at_name', false ),
+			'blog actor id' => array( 'id', true ),
+			'blog acct uri' => array( 'acct', true ),
+			'blog at-name'  => array( 'at_name', true ),
+			'blog home url' => array( 'home_url', true ),
+		);
+	}
 }

tests/phpunit/tests/includes/rest/class-test-actors-inbox-controller.php

@@ -9,6 +9,7 @@
 
 use Activitypub\Application;
 use Activitypub\Collection\Actors;
+use Activitypub\Collection\Followers;
 use Activitypub\Collection\Inbox as Inbox_Collection;
 use Activitypub\Collection\Outbox;
 use Activitypub\Handler\Feature_Request;
@@ -233,6 +234,45 @@ public function test_follow_request() {
 		\remove_filter( 'activitypub_defer_signature_verification', '__return_true' );
 	}
 
+	/**
+	 * A Follow of another server's actor, delivered to a local inbox, adds no follower.
+	 *
+	 * @covers ::create_item
+	 */
+	public function test_follow_of_other_actor_adds_no_follower() {
+		\add_filter( 'activitypub_defer_signature_verification', '__return_true' );
+
+		$actor_url              = 'https://example.com/users/alice';
+		$mock_metadata_callback = function () use ( $actor_url ) {
+			return array(
+				'id'                => $actor_url,
+				'type'              => 'Person',
+				'preferredUsername' => 'alice',
+				'inbox'             => 'https://example.com/inbox',
+			);
+		};
+		\add_filter( 'pre_get_remote_metadata_by_actor', $mock_metadata_callback );
+
+		$json = array(
+			'id'     => 'https://example.com/activities/follow-bob',
+			'type'   => 'Follow',
+			'actor'  => $actor_url,
+			'object' => 'https://remote.example/actors/1',
+		);
+
+		$request = new \WP_REST_Request( 'POST', '/' . ACTIVITYPUB_REST_NAMESPACE . '/actors/' . self::$user_id . '/inbox' );
+		$request->set_header( 'Content-Type', 'application/activity+json' );
+		$request->set_body( \wp_json_encode( $json ) );
+
+		$response = \rest_do_request( $request );
+
+		\remove_filter( 'pre_get_remote_metadata_by_actor', $mock_metadata_callback );
+		\remove_filter( 'activitypub_defer_signature_verification', '__return_true' );
+
+		$this->assertEquals( 202, $response->get_status() );
+		$this->assertCount( 0, Followers::get_many( self::$user_id ), 'A Follow of another actor should not add a follower.' );
+	}
+
 	/**
 	 * Test that deliveries to the retired Application actor's inbox are handled by the shared inbox.
 	 *