From: Brad Jorsch <bjorsch@wikimedia.org>
Date: Thu, 16 Jun 2016 21:44:44 +0000 (-0400)
Subject: Fix LegacyHookPreAuthenticationProvider::testUserForCreation
X-Git-Tag: 1.31.0-rc.0~6573^2
X-Git-Url: http://git.cyclocoop.org///%22%40url%40//%22?a=commitdiff_plain;h=78cb13c3691883cf6faa4e888dc21af3b1ebb686;p=lhc%2Fweb%2Fwiklou.git

Fix LegacyHookPreAuthenticationProvider::testUserForCreation

Simply testing shouldn't call AbortNewAccount, we only want to do that
when the account is actually being created.

Change-Id: Icb3d1ce63a2691aa232b4564ed88fee6d50d7ab7
---

diff --git a/includes/auth/LegacyHookPreAuthenticationProvider.php b/includes/auth/LegacyHookPreAuthenticationProvider.php
index c98a184612..cab6e32d5b 100644
--- a/includes/auth/LegacyHookPreAuthenticationProvider.php
+++ b/includes/auth/LegacyHookPreAuthenticationProvider.php
@@ -106,27 +106,6 @@ class LegacyHookPreAuthenticationProvider extends AbstractPreAuthenticationProvi
 					$user, $user, LoginForm::ABORTED, $abortError, 'AbortAutoAccount'
 				);
 			}
-		} else {
-			$abortError = '';
-			$abortStatus = null;
-			if ( !\Hooks::run( 'AbortNewAccount', [ $user, &$abortError, &$abortStatus ] ) ) {
-				// Hook point to add extra creation throttles and blocks
-				$this->logger->debug( __METHOD__ . ': a hook blocked creation' );
-				if ( $abortStatus === null ) {
-					// Report back the old string as a raw message status.
-					// This will report the error back as 'createaccount-hook-aborted'
-					// with the given string as the message.
-					// To return a different error code, return a StatusValue object.
-					$msg = wfMessage( 'createaccount-hook-aborted' )->rawParams( $abortError );
-					return StatusValue::newFatal( $msg );
-				} else {
-					// For MediaWiki 1.23+ and updated hooks, return the Status object
-					// returned from the hook.
-					$ret = StatusValue::newGood();
-					$ret->merge( $abortStatus );
-					return $ret;
-				}
-			}
 		}
 
 		return StatusValue::newGood();
diff --git a/tests/phpunit/includes/auth/LegacyHookPreAuthenticationProviderTest.php b/tests/phpunit/includes/auth/LegacyHookPreAuthenticationProviderTest.php
index edee6fc1e9..3548002682 100644
--- a/tests/phpunit/includes/auth/LegacyHookPreAuthenticationProviderTest.php
+++ b/tests/phpunit/includes/auth/LegacyHookPreAuthenticationProviderTest.php
@@ -334,22 +334,23 @@ class LegacyHookPreAuthenticationProviderTest extends \MediaWikiTestCase {
 	 * @param string|null $failMsg
 	 */
 	public function testTestUserForCreation( $error, $failMsg ) {
+		$testUser = self::getTestUser()->getUser();
+		$provider = $this->getProvider();
+		$options = [ 'flags' => \User::READ_LOCKING, 'creating' => true ];
+
 		$this->hook( 'AbortNewAccount', $this->never() );
 		$this->hook( 'AbortAutoAccount', $this->once() )
-			->will( $this->returnCallback( function ( $user, &$abortError ) use ( $error ) {
+			->will( $this->returnCallback( function ( $user, &$abortError ) use ( $testUser, $error ) {
 				$this->assertInstanceOf( 'User', $user );
-				$this->assertSame( 'UTSysop', $user->getName() );
+				$this->assertSame( $testUser->getName(), $user->getName() );
 				$abortError = $error;
 				return $error === null;
 			} ) );
-
-		$status = $this->getProvider()->testUserForCreation(
-			\User::newFromName( 'UTSysop' ), AuthManager::AUTOCREATE_SOURCE_SESSION
+		$status = $provider->testUserForCreation(
+			$testUser, AuthManager::AUTOCREATE_SOURCE_SESSION, $options
 		);
-
 		$this->unhook( 'AbortNewAccount' );
 		$this->unhook( 'AbortAutoAccount' );
-
 		if ( $failMsg === null ) {
 			$this->assertEquals( \StatusValue::newGood(), $status, 'should succeed' );
 		} else {
@@ -360,54 +361,11 @@ class LegacyHookPreAuthenticationProviderTest extends \MediaWikiTestCase {
 		}
 
 		$this->hook( 'AbortAutoAccount', $this->never() );
-		$this->hook( 'AbortNewAccount', $this->once() )
-			->will( $this->returnCallback(
-				function ( $user, &$abortError, &$abortStatus ) use ( $error ) {
-					$this->assertInstanceOf( 'User', $user );
-					$this->assertSame( 'UTSysop', $user->getName() );
-					$abortError = $error;
-					return $error === null;
-				}
-			) );
-		$status = $this->getProvider()->testUserForCreation( \User::newFromName( 'UTSysop' ), false );
+		$this->hook( 'AbortNewAccount', $this->never() );
+		$status = $provider->testUserForCreation( $testUser, false, $options );
 		$this->unhook( 'AbortNewAccount' );
 		$this->unhook( 'AbortAutoAccount' );
-		if ( $failMsg === null ) {
-			$this->assertEquals( \StatusValue::newGood(), $status, 'should succeed' );
-		} else {
-			$this->assertInstanceOf( 'StatusValue', $status, 'should fail (type)' );
-			$this->assertFalse( $status->isOk(), 'should fail (ok)' );
-			$errors = $status->getErrors();
-			$msg = $errors[0]['message'];
-			$this->assertInstanceOf( \Message::class, $msg );
-			$this->assertEquals(
-				'createaccount-hook-aborted', $msg->getKey(), 'should fail (message)'
-			);
-		}
-
-		if ( $error !== false ) {
-			$this->hook( 'AbortAutoAccount', $this->never() );
-			$this->hook( 'AbortNewAccount', $this->once() )
-				->will( $this->returnCallback(
-					function ( $user, &$abortError, &$abortStatus ) use ( $error ) {
-						$this->assertInstanceOf( 'User', $user );
-						$this->assertSame( 'UTSysop', $user->getName() );
-						$abortStatus = $error ? \Status::newFatal( $error ) : \Status::newGood();
-						return $error === null;
-					}
-			) );
-			$status = $this->getProvider()->testUserForCreation( \User::newFromName( 'UTSysop' ), false );
-			$this->unhook( 'AbortNewAccount' );
-			$this->unhook( 'AbortAutoAccount' );
-			if ( $failMsg === null ) {
-				$this->assertEquals( \StatusValue::newGood(), $status, 'should succeed' );
-			} else {
-				$this->assertInstanceOf( 'StatusValue', $status, 'should fail (type)' );
-				$this->assertFalse( $status->isOk(), 'should fail (ok)' );
-				$errors = $status->getErrors();
-				$this->assertEquals( $failMsg, $errors[0]['message'], 'should fail (message)' );
-			}
-		}
+		$this->assertEquals( \StatusValue::newGood(), $status, 'should succeed' );
 	}
 
 	public static function provideTestUserForCreation() {