user: Assert wikiId in UserGroupManager

For interwiki userright changes a UserRightsProxy instance is passed to
UserGroupManager. UserRightsProxy does not implement a check for wikiId
in getId().
Without the check for the wikiId and the extra argument to getId() it is
not possible to pass non-local instances of UserIdentityValue to
UserGroupManager.

Bug: T255309
Change-Id: Ice67ff4337165c0f022267186c88332e0e574869

includes/user/UserGroupManager.php

@@ -284,6 +284,7 @@ class UserGroupManager implements IDBAccessObject {
 		int $queryFlags = self::READ_NORMAL,
 		bool $recache = false
 	): array {
+		$user->assertWiki( $this->dbDomain );
 		$userKey = $this->getCacheKey( $user );
 		if ( $recache ||
 			!isset( $this->userGroupCache[$userKey][self::CACHE_IMPLICIT] ) ||
@@ -328,6 +329,7 @@ class UserGroupManager implements IDBAccessObject {
 		int $queryFlags = self::READ_NORMAL,
 		bool $recache = false
 	): array {
+		$user->assertWiki( $this->dbDomain );
 		$userKey = $this->getCacheKey( $user );
 		// Ignore cache if the $recache flag is set, cached values can not be used
 		// or the cache value is missing
@@ -370,6 +372,7 @@ class UserGroupManager implements IDBAccessObject {
 		UserIdentity $user,
 		int $queryFlags = self::READ_NORMAL
 	): array {
+		$user->assertWiki( $this->dbDomain );
 		$userKey = $this->getCacheKey( $user );
 
 		if ( $this->canUseCachedValues( $user, self::CACHE_FORMER, $queryFlags ) &&
@@ -386,7 +389,7 @@ class UserGroupManager implements IDBAccessObject {
 		$res = $this->getDBConnectionRefForQueryFlags( $queryFlags )->newSelectQueryBuilder()
 			->select( 'ufg_group' )
 			->from( 'user_former_groups' )
-			->where( [ 'ufg_user' => $user->getId() ] )
+			->where( [ 'ufg_user' => $user->getId( $this->dbDomain ) ] )
 			->caller( __METHOD__ )
 			->fetchResultSet();
 		$formerGroups = [];
@@ -407,6 +410,7 @@ class UserGroupManager implements IDBAccessObject {
 	 * @see $wgAutopromote
 	 */
 	public function getUserAutopromoteGroups( UserIdentity $user ): array {
+		$user->assertWiki( $this->dbDomain );
 		$promote = [];
 		// TODO: remove the need for the full user object
 		$userObj = User::newFromIdentity( $user );
@@ -436,6 +440,7 @@ class UserGroupManager implements IDBAccessObject {
 		UserIdentity $user,
 		string $event
 	): array {
+		$user->assertWiki( $this->dbDomain );
 		$autopromoteOnce = $this->options->get( MainConfigNames::AutopromoteOnce );
 		$promote = [];
 
@@ -624,6 +629,7 @@ class UserGroupManager implements IDBAccessObject {
 			!$this->dbDomain || WikiMap::isCurrentWikiDbDomain( $this->dbDomain ),
 			__METHOD__ . " is not supported for foreign domains: {$this->dbDomain} used"
 		);
+		$user->assertWiki( $this->dbDomain );
 
 		if ( $this->readOnlyMode->isReadOnly() || !$user->isRegistered() ) {
 			return [];
@@ -698,6 +704,7 @@ class UserGroupManager implements IDBAccessObject {
 		UserIdentity $user,
 		int $queryFlags = self::READ_NORMAL
 	): array {
+		$user->assertWiki( $this->dbDomain );
 		$userKey = $this->getCacheKey( $user );
 
 		if ( $this->canUseCachedValues( $user, self::CACHE_MEMBERSHIP, $queryFlags ) &&
@@ -714,7 +721,7 @@ class UserGroupManager implements IDBAccessObject {
 
 		$queryBuilder = $this->newQueryBuilder( $this->getDBConnectionRefForQueryFlags( $queryFlags ) );
 		$res = $queryBuilder
-			->where( [ 'ug_user' => $user->getId() ] )
+			->where( [ 'ug_user' => $user->getId( $this->dbDomain ) ] )
 			->caller( __METHOD__ )
 			->fetchResultSet();
 
@@ -753,6 +760,7 @@ class UserGroupManager implements IDBAccessObject {
 		string $expiry = null,
 		bool $allowUpdate = false
 	): bool {
+		$user->assertWiki( $this->dbDomain );
 		if ( $this->readOnlyMode->isReadOnly() ) {
 			return false;
 		}
@@ -785,7 +793,7 @@ class UserGroupManager implements IDBAccessObject {
 		$dbw->insert(
 			'user_groups',
 			[
-				'ug_user' => $user->getId(),
+				'ug_user' => $user->getId( $this->dbDomain ),
 				'ug_group' => $group,
 				'ug_expiry' => $expiry ? $dbw->timestamp( $expiry ) : null,
 			],
@@ -798,7 +806,7 @@ class UserGroupManager implements IDBAccessObject {
 			// Conflicting row already exists; it should be overridden if it is either expired
 			// or if $allowUpdate is true and the current row is different than the loaded row.
 			$conds = [
-				'ug_user' => $user->getId(),
+				'ug_user' => $user->getId( $this->dbDomain ),
 				'ug_group' => $group
 			];
 			if ( $allowUpdate ) {
@@ -834,7 +842,7 @@ class UserGroupManager implements IDBAccessObject {
 		} );
 
 		if ( $affected > 0 ) {
-			$oldUgms[$group] = new UserGroupMembership( $user->getId(), $group, $expiry );
+			$oldUgms[$group] = new UserGroupMembership( $user->getId( $this->dbDomain ), $group, $expiry );
 			if ( !$oldUgms[$group]->isExpired() ) {
 				$this->setCache(
 					$this->getCacheKey( $user ),
@@ -885,6 +893,7 @@ class UserGroupManager implements IDBAccessObject {
 	 * @return bool
 	 */
 	public function removeUserFromGroup( UserIdentity $user, string $group ): bool {
+		$user->assertWiki( $this->dbDomain );
 		// TODO: Deprecate passing out user object in the hook by introducing
 		// an alternative hook
 		if ( $this->hookContainer->isRegistered( 'UserRemoveGroup' ) ) {
@@ -911,7 +920,7 @@ class UserGroupManager implements IDBAccessObject {
 		$dbw = $this->loadBalancer->getConnectionRef( DB_PRIMARY, [], $this->dbDomain );
 		$dbw->newDeleteQueryBuilder()
 			->delete( 'user_groups' )
-			->where( [ 'ug_user' => $user->getId(), 'ug_group' => $group ] )
+			->where( [ 'ug_user' => $user->getId( $this->dbDomain ), 'ug_group' => $group ] )
 			->caller( __METHOD__ )->execute();
 
 		if ( !$dbw->affectedRows() ) {
@@ -920,7 +929,7 @@ class UserGroupManager implements IDBAccessObject {
 		// Remember that the user was in this group
 		$dbw->insert(
 			'user_former_groups',
-			[ 'ufg_user' => $user->getId(), 'ufg_group' => $group ],
+			[ 'ufg_user' => $user->getId( $this->dbDomain ), 'ufg_group' => $group ],
 			__METHOD__,
 			[ 'IGNORE' ]
 		);
@@ -1118,6 +1127,7 @@ class UserGroupManager implements IDBAccessObject {
 	 * @param UserIdentity $user
 	 */
 	public function clearCache( UserIdentity $user ) {
+		$user->assertWiki( $this->dbDomain );
 		$userKey = $this->getCacheKey( $user );
 		unset( $this->userGroupCache[$userKey] );
 		unset( $this->queryFlagsUsedForCaching[$userKey] );
@@ -1168,7 +1178,7 @@ class UserGroupManager implements IDBAccessObject {
 	 * @return string
 	 */
 	private function getCacheKey( UserIdentity $user ): string {
-		return $user->isRegistered() ? "u:{$user->getId()}" : "anon:{$user->getName()}";
+		return $user->isRegistered() ? "u:{$user->getId( $this->dbDomain )}" : "anon:{$user->getName()}";
 	}
 
 	/**

tests/phpunit/includes/user/UserGroupManagerTest.php

@@ -568,7 +568,8 @@ class UserGroupManagerTest extends MediaWikiIntegrationTestCase {
 	}
 
 	public function provideGetUserAutopromoteEmailConfirmed() {
-		$successUserMock = $this->createNoOpMock( User::class, [ 'getEmail', 'getEmailAuthenticationTimestamp' ] );
+		$successUserMock = $this->createNoOpMock( User::class, [ 'getEmail', 'getEmailAuthenticationTimestamp', 'assertWiki' ] );
+		$successUserMock->method( 'assertWiki' )->willReturn( true );
 		$successUserMock->expects( $this->once() )
 			->method( 'getEmail' )
 			->willReturn( 'test@test.com' );
@@ -578,14 +579,16 @@ class UserGroupManagerTest extends MediaWikiIntegrationTestCase {
 		yield 'Successfull autopromote' => [
 			true, $successUserMock, [ 'test_autoconfirmed' ]
 		];
-		$emailAuthMock = $this->createNoOpMock( User::class, [ 'getEmail' ] );
+		$emailAuthMock = $this->createNoOpMock( User::class, [ 'getEmail', 'assertWiki' ] );
+		$emailAuthMock->method( 'assertWiki' )->willReturn( true );
 		$emailAuthMock->expects( $this->once() )
 			->method( 'getEmail' )
 			->willReturn( 'test@test.com' );
 		yield 'wgEmailAuthentication is false' => [
 			false, $emailAuthMock, [ 'test_autoconfirmed' ]
 		];
-		$invalidEmailMock = $this->createNoOpMock( User::class, [ 'getEmail' ] );
+		$invalidEmailMock = $this->createNoOpMock( User::class, [ 'getEmail', 'assertWiki' ] );
+		$invalidEmailMock->method( 'assertWiki' )->willReturn( true );
 		$invalidEmailMock
 			->expects( $this->once() )
 			->method( 'getEmail' )
@@ -593,7 +596,8 @@ class UserGroupManagerTest extends MediaWikiIntegrationTestCase {
 		yield 'Invalid email' => [
 			true, $invalidEmailMock, []
 		];
-		$nullTimestampMock = $this->createNoOpMock( User::class, [ 'getEmail', 'getEmailAuthenticationTimestamp' ] );
+		$nullTimestampMock = $this->createNoOpMock( User::class, [ 'getEmail', 'getEmailAuthenticationTimestamp', 'assertWiki' ] );
+		$nullTimestampMock->method( 'assertWiki' )->willReturn( true );
 		$nullTimestampMock->expects( $this->once() )
 			->method( 'getEmail' )
 			->willReturn( 'test@test.com' );
@@ -715,7 +719,8 @@ class UserGroupManagerTest extends MediaWikiIntegrationTestCase {
 			'AutoConfirmAge' => 10000000,
 			'Autopromote' => [ 'test_autoconfirmed' => $requiredCondition ]
 		] );
-		$user = $this->createNoOpMock( User::class, [ 'getRegistration' ] );
+		$user = $this->createNoOpMock( User::class, [ 'getRegistration', 'assertWiki' ] );
+		$user->method( 'assertWiki' )->willReturn( true );
 		$user->method( 'getRegistration' )
 			->willReturn( $registrationTs );
 		$this->assertArrayEquals( $expected, $manager->getUserAutopromoteGroups( $user ) );
@@ -818,7 +823,8 @@ class UserGroupManagerTest extends MediaWikiIntegrationTestCase {
 		$requestMock->expects( $this->once() )
 			->method( 'getIP' )
 			->willReturn( $userIp );
-		$user = $this->createNoOpMock( User::class, [ 'getRequest' ] );
+		$user = $this->createNoOpMock( User::class, [ 'getRequest', 'assertWiki' ] );
+		$user->method( 'assertWiki' )->willReturn( true );
 		$user->expects( $this->once() )
 			->method( 'getRequest' )
 			->willReturn( $requestMock );