Page MenuHomePhabricator
Files F32358464

07-T260485-REL1_31.patch
Reedy (Sam Reed)
Actions

Authored By
Reedy
Sep 22 2020, 12:05 AM
Size
2 KB
Referenced Files
None
Subscribers
None

07-T260485-REL1_31.patch
View Options

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

File Metadata

Mime Type
text/x-diff
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
8573627
Default Alt Text
07-T260485-REL1_31.patch (2 KB)

Event Timeline

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL