From e95b7202e16f77c02edaaa58354d769badf4db67 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 c2ad1cdb8c..192e0409c6 100644
--- a/includes/ActorMigration.php
+++ b/includes/ActorMigration.php
@@ -239,6 +239,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
 	 *
@@ -262,11 +279,18 @@ class ActorMigration {
 			$ret[$text] = $user->getName();
 		}
 		if ( $this->stage & SCHEMA_COMPAT_WRITE_NEW ) {
-			// 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.20.1