From bdc11d08af81c386583d50fd030d6359e0060e2b Mon Sep 17 00:00:00 2001
From: Martin Urbanec <martin.urbanec@wikimedia.cz>
Date: Thu, 20 Aug 2020 21:51:51 +0200
Subject: [PATCH] SECURITY: ActorMigration: Load user from the correct database

In ActorMigration::getInsertValues, when creating a User object, calling
User::getActorId triggers a call to User::load, which ignores
the database passed to getInsertValues, meaning incorrect actor IDs
are returned.

To ensure that the correct (foreign) database is used, try
to get the actor ID from the correct database within ActorMigration
service, and if that fails, let User class handle the actor ID creation.

Todo notes are left in the patch to fix the issue properly,
by making User object wiki-aware.

Bug: T260485
Change-Id: Iaa886a1824e5a74f4501ca7e28917c780222aac0
---
 includes/ActorMigration.php | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/includes/ActorMigration.php b/includes/ActorMigration.php
index 161c7a923b..b0aeeaf8ae 100644
--- a/includes/ActorMigration.php
+++ b/includes/ActorMigration.php
@@ -181,6 +181,23 @@ class ActorMigration {
 		return $this->joinCache[$key];
 	}
 
+	/**
+	 * Get actor ID from UserIdentity, if it exists
+	 */
+	private function getExistingActorId( IDatabase $dbw, UserIdentity $user ) {
+		$row = $dbw->selectRow(
+			'actor',
+			[ 'actor_id' ],
+			[ 'actor_name' => $user->getName() ],
+			__METHOD__
+		);
+		if ( $row === false ) {
+			return false;
+		}
+
+		return $row->actor_id;
+	}
+
 	/**
 	 * Get UPDATE fields for the actor
 	 *
@@ -202,11 +219,18 @@ class ActorMigration {
 			$ret[$text] = $user->getName();
 		}
 		if ( $this->stage >= MIGRATION_WRITE_BOTH ) {
-			// 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 );
+			// 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.
+			$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] = $user->getActorId( $dbw );
 		}
 		return $ret;
 	}
-- 
2.25.1