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 c98a184..cab6e32 100644 (file)
--- 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 edee6fc..3548002 100644 (file)
--- 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() {