SECURITY: ensure actor ID from correct wiki is used.

This builds on top of Urbanecm's patch, now also covering the case
where the actor ID does not exist in the target DB, but does exist in
the local DB.

Bug: T260485
Change-Id: I2336954c665366a99f9995df9b08071d4de6db79
(cherry picked from commit ca4094db9e7f6f5e330d89db6bf70a8af48e1561)

1 files changed, 63 insertions, 10 deletions

diff --git a/includes/ActorMigration.php b/includes/ActorMigration.php
index 056be2548f32..67aefb41aa75 100644
--- a/includes/ActorMigration.php
+++ b/includes/ActorMigration.php
@@ -244,12 +244,12 @@ class ActorMigration {
 	 *
 	 * @since 1.35.0
 	 *
-	 * @param IDatabase $dbw
+	 * @param IDatabase $db
 	 * @param UserIdentity $user
 	 * @return int|false
 	 */
-	private function getExistingActorId( IDatabase $dbw, UserIdentity $user ) {
-		$row = $dbw->selectRow(
+	public function getExistingActorId( IDatabase $db, UserIdentity $user ) {
+		$row = $db->selectRow(
 			'actor',
 			[ 'actor_id' ],
 			[ 'actor_name' => $user->getName() ],
@@ -263,6 +263,63 @@ class ActorMigration {
 	}
 
 	/**
+	 * Attempt to assign an actor ID to the given user.
+	 * If it is already assigned, return the existing ID.
+	 *
+	 * @since 1.35.0
+	 *
+	 * @param IDatabase $dbw
+	 * @param UserIdentity $user
+	 *
+	 * @return int The new actor ID
+	 */
+	public function getNewActorId( IDatabase $dbw, UserIdentity $user ) {
+		// TODO: inject
+		$userNameUtils = MediaWikiServices::getInstance()->getUserNameUtils();
+
+		$q = [
+			'actor_user' => $user->getId() ?: null,
+			'actor_name' => (string)$user->getName(),
+		];
+		if ( $q['actor_user'] === null && $userNameUtils->isUsable( $q['actor_name'] ) ) {
+			throw new CannotCreateActorException(
+				'Cannot create an actor for a usable name that is not an existing user: ' .
+				"user_id={$user->getId()} user_name=\"{$user->getName()}\""
+			);
+		}
+		if ( $q['actor_name'] === '' ) {
+			throw new CannotCreateActorException(
+				'Cannot create an actor for a user with no name: ' .
+				"user_id={$user->getId()} user_name=\"{$user->getName()}\""
+			);
+		}
+
+		$dbw->insert( 'actor', $q, __METHOD__, [ 'IGNORE' ] );
+
+		if ( $dbw->affectedRows() ) {
+			$actorId = (int)$dbw->insertId();
+		} else {
+			// Outdated cache?
+			// Use LOCK IN SHARE MODE to bypass any MySQL REPEATABLE-READ snapshot.
+			$actorId = (int)$dbw->selectField(
+				'actor',
+				'actor_id',
+				$q,
+				__METHOD__,
+				[ 'LOCK IN SHARE MODE' ]
+			);
+			if ( !$actorId ) {
+				throw new CannotCreateActorException(
+					"Failed to create actor ID for " .
+					"user_id={$user->getId()} user_name=\"{$user->getName()}\""
+				);
+			}
+		}
+
+		return $actorId;
+	}
+
+	/**
 	 * Get UPDATE fields for the actor
 	 *
 	 * @param IDatabase $dbw Database to use for creating an actor ID, if necessary
@@ -285,17 +342,13 @@ class ActorMigration {
 			$ret[$text] = $user->getName();
 		}
 		if ( $this->stage & SCHEMA_COMPAT_WRITE_NEW ) {
-			// UBN fix for T260485 - do not let User object to handle existing actor IDs
-			// TODO: Make User object wiki-aware and let it handle all cases.
+			// NOTE: Don't use $user->getActorId(), since that may be for the wrong wiki (T260485)
+			// TODO: Make User object wiki-aware and let it handle all cases (T260933)
 			$existingActorId = $this->getExistingActorId( $dbw, $user );
 			if ( $existingActorId !== false ) {
 				$ret[$actor] = $existingActorId;
 			} else {
-				// We need to be able to assign an actor ID if none exists
-				if ( !$user instanceof User && !$user->getActorId() ) {
-					$user = User::newFromAnyId( $user->getId(), $user->getName(), null );
-				}
-				$ret[$actor] = $user->getActorId( $dbw );
+				$ret[$actor] = $this->getNewActorId( $dbw, $user );
 			}
 		}
 		return $ret;