Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

User: Allow newSystemUser() to create over anonymous actors

Browse source 
Various maintenance scripts assume reserved usernames like
"MediaWiki default" exist, but since they're reserved
User::isUsableName() returns false and therefore the actor migration
created them as anonymous actors. Which would then prevent those
maintenance scripts from using User::newSystemUser() to ensure they
actually exist.

This adjusts User::newSystemUser() to be able to create users for
those anonymous actors.

This also adjusts uses of "MediaWiki default" in core to create it as a
system user.

Bug: T236444
Change-Id: I59a646df36ff9343cc43c05aa20b2b69b2ee124a
This commit is contained in:
Brad Jorsch 2019-10-28 11:51:05 -04:00
parent 68d87dba9f
commit 685b505628
4 changed files with 148 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
includes/installer/Installer.php
View file

@ -1753,7 +1753,7 @@ abstract class Installer {
'',
EDIT_NEW,
false,
User::newFromName( 'MediaWiki default' )
User::newSystemUser( 'MediaWiki default' )
);
} catch ( Exception $e ) {
// using raw, because $wgShowExceptionDetails can not be set yet

57
includes/user/User.php
View file

@ -110,6 +110,9 @@ class User implements IDBAccessObject, UserIdentity {
'mActorId',
];
/** @var string[]|false Cache for self::isUsableName() */
private static $reservedUsernames = false;
/** Cache variables */
// @{
/** @var int */
@ -789,9 +792,48 @@ class User implements IDBAccessObject, UserIdentity {
if ( !$row ) {
// No user. Create it?
return $options['create']
? self::createNew( $name, [ 'token' => self::INVALID_TOKEN ] )
: null;
if ( !$options['create'] ) {
// No.
return null;
}
// If it's a reserved user that had an anonymous actor created for it at
// some point, we need special handling.
if ( !self::isValidUserName( $name ) || self::isUsableName( $name ) ) {
// Not reserved, so just create it.
return self::createNew( $name, [ 'token' => self::INVALID_TOKEN ] );
}
// It is reserved. Check for an anonymous actor row.
$dbw = wfGetDB( DB_MASTER );
return $dbw->doAtomicSection( __METHOD__, function ( IDatabase $dbw, $fname ) use ( $name ) {
$row = $dbw->selectRow(
'actor',
[ 'actor_id' ],
[ 'actor_name' => $name, 'actor_user' => null ],
$fname,
[ 'FOR UPDATE' ]
);
if ( !$row ) {
// No anonymous actor.
return self::createNew( $name, [ 'token' => self::INVALID_TOKEN ] );
}
// There is an anonymous actor. Delete the actor row so we can create the user,
// then restore the old actor_id so as to not break existing references.
// @todo If MediaWiki ever starts using foreign keys for `actor`, this will break things.
$dbw->delete( 'actor', [ 'actor_id' => $row->actor_id ], $fname );
$user = self::createNew( $name, [ 'token' => self::INVALID_TOKEN ] );
$dbw->update(
'actor',
[ 'actor_id' => $row->actor_id ],
[ 'actor_id' => $user->getActorId() ],
$fname
);
$user->clearInstanceCache( 'id' );
$user->invalidateCache();
return $user;
} );
}
$user = self::newFromRow( $row );
@ -988,14 +1030,13 @@ class User implements IDBAccessObject, UserIdentity {
return false;
}
static $reservedUsernames = false;
if ( !$reservedUsernames ) {
$reservedUsernames = $wgReservedUsernames;
Hooks::run( 'UserGetReservedNames', [ &$reservedUsernames ] );
if ( !self::$reservedUsernames ) {
self::$reservedUsernames = $wgReservedUsernames;
Hooks::run( 'UserGetReservedNames', [ &self::$reservedUsernames ] );
}
// Certain names may be reserved for batch processes.
foreach ( $reservedUsernames as $reserved ) {
foreach ( self::$reservedUsernames as $reserved ) {
if ( substr( $reserved, 0, 4 ) == 'msg:' ) {
$reserved = wfMessage( substr( $reserved, 4 ) )->inContentLanguage()->plain();
}

2
maintenance/deleteDefaultMessages.php
View file

@ -76,7 +76,7 @@ class DeleteDefaultMessages extends Maintenance {
// Deletions will be made by $user temporarly added to the bot group
// in order to hide it in RecentChanges.
$user = User::newFromName( 'MediaWiki default' );
$user = User::newSystemUser( 'MediaWiki default', [ 'steal' => true ] );
if ( !$user ) {
$this->fatalError( "Invalid username" );
}

97
tests/phpunit/includes/user/UserTest.php
View file

@ -36,6 +36,13 @@ class UserTest extends MediaWikiTestCase {
$this->setUpPermissionGlobals();
$this->user = $this->getTestUser( [ 'unittesters' ] )->getUser();
TestingAccessWrapper::newFromClass( User::class )->reservedUsernames = false;
}
protected function tearDown() : void {
parent::tearDown();
TestingAccessWrapper::newFromClass( User::class )->reservedUsernames = false;
}
private function setUpPermissionGlobals() {
@ -1569,4 +1576,94 @@ class UserTest extends MediaWikiTestCase {
);
$this->assertSame( null, User::idFromName( 'NotExisitngUser' ) );
}
/**
* @covers User::newSystemUser
* @dataProvider provideNewSystemUser
* @param string $exists How/whether to create the user before calling User::newSystemUser
* - 'missing': Do not create the user
* - 'actor': Create an anonymous actor
* - 'user': Create a non-system user
* - 'system': Create a system user
* @param string $options Options to User::newSystemUser
* @param array $testOpts Test options
* @param string $expect 'user', 'exception', or 'null'
*/
public function testNewSystemUser( $exists, $options, $testOpts, $expect ) {
$origUser = null;
$actorId = null;
switch ( $exists ) {
case 'missing':
$name = 'TestNewSystemUser ' . TestUserRegistry::getNextId();
break;
case 'actor':
$name = 'TestNewSystemUser ' . TestUserRegistry::getNextId();
$this->db->insert( 'actor', [ 'actor_name' => $name ] );
$actorId = (int)$this->db->insertId();
break;
case 'user':
$origUser = $this->getMutableTestUser()->getUser();
$name = $origUser->getName();
$actorId = $origUser->getActorId();
break;
case 'system':
$name = 'TestNewSystemUser ' . TestUserRegistry::getNextId();
$user = User::newSystemUser( $name ); // Heh.
$actorId = $user->getActorId();
// Use this hook as a proxy for detecting when a "steal" happens.
$this->setTemporaryHook( 'InvalidateEmailComplete', function () {
$this->fail( 'InvalidateEmailComplete hook should not have been called' );
} );
break;
}
$globals = $testOpts['globals'] ?? [];
if ( !empty( $testOpts['reserved'] ) ) {
$globals['wgReservedUsernames'] = [ $name ];
}
$this->setMwGlobals( $globals );
$this->assertTrue( User::isValidUserName( $name ) );
$this->assertSame( empty( $testOpts['reserved'] ), User::isUsableName( $name ) );
if ( $expect === 'exception' ) {
$this->expectException( Exception::class );
}
$user = User::newSystemUser( $name, $options );
if ( $expect === 'null' ) {
$this->assertNull( $user );
if ( $origUser ) {
$this->assertNotSame(
User::INVALID_TOKEN, TestingAccessWrapper::newFromObject( $origUser )->mToken
);
$this->assertNotSame( '', $origUser->getEmail() );
}
} else {
$this->assertInstanceOf( User::class, $user );
$this->assertSame( $name, $user->getName() );
if ( $actorId !== null ) {
$this->assertSame( $actorId, $user->getActorId() );
}
$this->assertSame( User::INVALID_TOKEN, TestingAccessWrapper::newFromObject( $user )->mToken );
$this->assertSame( '', $user->getEmail() );
}
}
public static function provideNewSystemUser() {
return [
'Basic creation' => [ 'missing', [], [], 'user' ],
'No creation' => [ 'missing', [ 'create' => false ], [], 'null' ],
'Validation fail' => [ 'missing', [ 'validate' => 'usable' ], [ 'reserved' => true ], 'null' ],
'No stealing' => [ 'user', [], [], 'null' ],
'Stealing allowed' => [ 'user', [ 'steal' => true ], [], 'user' ],
'Stealing an already-system user' => [ 'system', [ 'steal' => true ], [], 'user' ],
'Anonymous actor (T236444)' => [ 'actor', [], [ 'reserved' => true ], 'user' ],
'Reserved but no anonymous actor' => [ 'missing', [], [ 'reserved' => true ], 'user' ],
'Anonymous actor but no creation' => [ 'actor', [ 'create' => false ], [], 'null' ],
'Anonymous actor but not reserved' => [ 'actor', [], [], 'exception' ],
];
}
}