Improve ApiLogin test coverage
Coverage is 100% except for one session-related bit that seems a bit
involved to test right now. It looks like it will be easier once
SessionManager becomes a service.
I removed the third parameter from the return value of
canonicalizeLoginData, since af37a4c7 made it always return true.
I also removed three lines of dead code from ApiLogin.php.
Change-Id: Ia0073eddd27c82827518e0031e3c313f83cfd7cc
This commit is contained in:
Aryeh Gregor
parent
8435e0907b
commit
1496fd4b4e
4 changed files with 172 additions and 25 deletions
|
|
@ -130,7 +130,7 @@ class ApiLogin extends ApiBase {
|
|||
$session = $status->getValue();
|
||||
$authRes = 'Success';
|
||||
$loginType = 'BotPassword';
|
||||
} elseif ( !$botLoginData[2] ||
|
||||
} elseif (
|
||||
$status->hasMessage( 'login-throttled' ) ||
|
||||
$status->hasMessage( 'botpasswords-needs-reset' ) ||
|
||||
$status->hasMessage( 'botpasswords-locked' )
|
||||
|
|
@ -141,6 +141,7 @@ class ApiLogin extends ApiBase {
|
|||
'BotPassword login failed: ' . $status->getWikiText( false, false, 'en' )
|
||||
);
|
||||
}
|
||||
// For other errors, let's see if it's a valid non-bot login
|
||||
}
|
||||
|
||||
if ( $authRes === false ) {
|
||||
|
|
@ -220,15 +221,15 @@ class ApiLogin extends ApiBase {
|
|||
);
|
||||
break;
|
||||
|
||||
// @codeCoverageIgnoreStart
|
||||
// Unreachable
|
||||
default:
|
||||
ApiBase::dieDebug( __METHOD__, "Unhandled case value: {$authRes}" );
|
||||
// @codeCoverageIgnoreEnd
|
||||
}
|
||||
|
||||
$this->getResult()->addValue( null, 'login', $result );
|
||||
|
||||
if ( $loginType === 'LoginForm' && isset( LoginForm::$statusCodes[$authRes] ) ) {
|
||||
$authRes = LoginForm::$statusCodes[$authRes];
|
||||
}
|
||||
LoggerFactory::getInstance( 'authevents' )->info( 'Login attempt', [
|
||||
'event' => 'login',
|
||||
'successful' => $authRes === 'Success',
|
||||
|
|
|
|||
|
|
@ -410,9 +410,7 @@ class BotPassword implements IDBAccessObject {
|
|||
/**
|
||||
* There are two ways to login with a bot password: "username@appId", "password" and
|
||||
* "username", "appId@password". Transform it so it is always in the first form.
|
||||
* Returns [bot username, bot password, could be normal password?] where the last one is a flag
|
||||
* meaning this could either be a bot password or a normal password, it cannot be decided for
|
||||
* certain (although in such cases it almost always will be a bot password).
|
||||
* Returns [bot username, bot password].
|
||||
* If this cannot be a bot password login just return false.
|
||||
* @param string $username
|
||||
* @param string $password
|
||||
|
|
@ -424,14 +422,14 @@ class BotPassword implements IDBAccessObject {
|
|||
if ( strlen( $password ) >= 32 && strpos( $username, $sep ) !== false ) {
|
||||
// the separator is not valid in new usernames but might appear in legacy ones
|
||||
if ( preg_match( '/^[0-9a-w]{32,}$/', $password ) ) {
|
||||
return [ $username, $password, true ];
|
||||
return [ $username, $password ];
|
||||
}
|
||||
} elseif ( strlen( $password ) > 32 && strpos( $password, $sep ) !== false ) {
|
||||
$segments = explode( $sep, $password );
|
||||
$password = array_pop( $segments );
|
||||
$appId = implode( $sep, $segments );
|
||||
if ( preg_match( '/^[0-9a-w]{32,}$/', $password ) ) {
|
||||
return [ $username . $sep . $appId, $password, true ];
|
||||
return [ $username . $sep . $appId, $password ];
|
||||
}
|
||||
}
|
||||
return false;
|
||||
|
|
|
|||
|
|
@ -13,6 +13,34 @@ use Wikimedia\TestingAccessWrapper;
|
|||
* @covers ApiLogin
|
||||
*/
|
||||
class ApiLoginTest extends ApiTestCase {
|
||||
public function setUp() {
|
||||
parent::setUp();
|
||||
|
||||
$this->tablesUsed[] = 'bot_passwords';
|
||||
}
|
||||
|
||||
public static function provideEnableBotPasswords() {
|
||||
return [
|
||||
'Bot passwords enabled' => [ true ],
|
||||
'Bot passwords disabled' => [ false ],
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider provideEnableBotPasswords
|
||||
*/
|
||||
public function testExtendedDescription( $enableBotPasswords ) {
|
||||
$this->setMwGlobals( 'wgEnableBotPasswords', $enableBotPasswords );
|
||||
$ret = $this->doApiRequest( [
|
||||
'action' => 'paraminfo',
|
||||
'modules' => 'login',
|
||||
'helpformat' => 'raw',
|
||||
] );
|
||||
$this->assertSame(
|
||||
'apihelp-login-extended-description' . ( $enableBotPasswords ? '' : '-nobotpasswords' ),
|
||||
$ret[0]['paraminfo']['modules'][0]['description'][1]['key']
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test result of attempted login with an empty username
|
||||
|
|
@ -30,7 +58,16 @@ class ApiLoginTest extends ApiTestCase {
|
|||
$this->assertSame( 'Failed', $ret[0]['login']['result'] );
|
||||
}
|
||||
|
||||
private function doUserLogin( $name, $password ) {
|
||||
/**
|
||||
* Attempts to log in with the given name and password, retrieves the returned token, and makes
|
||||
* a second API request to actually log in with the token.
|
||||
*
|
||||
* @param string $name
|
||||
* @param string $password
|
||||
* @param array $params To pass to second request
|
||||
* @return array Result of second doApiRequest
|
||||
*/
|
||||
private function doUserLogin( $name, $password, array $params = [] ) {
|
||||
$ret = $this->doApiRequest( [
|
||||
'action' => 'login',
|
||||
'lgname' => $name,
|
||||
|
|
@ -38,12 +75,25 @@ class ApiLoginTest extends ApiTestCase {
|
|||
|
||||
$this->assertSame( 'NeedToken', $ret[0]['login']['result'] );
|
||||
|
||||
return $this->doApiRequest( [
|
||||
'action' => 'login',
|
||||
'lgtoken' => $ret[0]['login']['token'],
|
||||
'lgname' => $name,
|
||||
'lgpassword' => $password,
|
||||
], $ret[2] );
|
||||
return $this->doApiRequest( array_merge(
|
||||
[
|
||||
'action' => 'login',
|
||||
'lgtoken' => $ret[0]['login']['token'],
|
||||
'lgname' => $name,
|
||||
'lgpassword' => $password,
|
||||
], $params
|
||||
), $ret[2] );
|
||||
}
|
||||
|
||||
public function testBadToken() {
|
||||
$user = self::$users['sysop'];
|
||||
$userName = $user->getUser()->getName();
|
||||
$password = $user->getPassword();
|
||||
$user->getUser()->logout();
|
||||
|
||||
$ret = $this->doUserLogin( $userName, $password, [ 'lgtoken' => 'invalid token' ] );
|
||||
|
||||
$this->assertSame( 'WrongToken', $ret[0]['login']['result'] );
|
||||
}
|
||||
|
||||
public function testBadPass() {
|
||||
|
|
@ -56,7 +106,12 @@ class ApiLoginTest extends ApiTestCase {
|
|||
$this->assertSame( 'Failed', $ret[0]['login']['result'] );
|
||||
}
|
||||
|
||||
public function testGoodPass() {
|
||||
/**
|
||||
* @dataProvider provideEnableBotPasswords
|
||||
*/
|
||||
public function testGoodPass( $enableBotPasswords ) {
|
||||
$this->setMwGlobals( 'wgEnableBotPasswords', $enableBotPasswords );
|
||||
|
||||
$user = self::$users['sysop'];
|
||||
$userName = $user->getUser()->getName();
|
||||
$password = $user->getPassword();
|
||||
|
|
@ -65,9 +120,56 @@ class ApiLoginTest extends ApiTestCase {
|
|||
$ret = $this->doUserLogin( $userName, $password );
|
||||
|
||||
$this->assertSame( 'Success', $ret[0]['login']['result'] );
|
||||
$this->assertSame(
|
||||
[ 'warnings' => ApiErrorFormatter::stripMarkup( wfMessage(
|
||||
'apiwarn-deprecation-login-' . ( $enableBotPasswords ? '' : 'no' ) . 'botpw' )->
|
||||
text() ) ],
|
||||
$ret[0]['warnings']['login']
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider provideEnableBotPasswords
|
||||
*/
|
||||
public function testUnsupportedAuthResponseType( $enableBotPasswords ) {
|
||||
$this->setMwGlobals( 'wgEnableBotPasswords', $enableBotPasswords );
|
||||
|
||||
$mockProvider = $this->createMock(
|
||||
MediaWiki\Auth\AbstractSecondaryAuthenticationProvider::class );
|
||||
$mockProvider->method( 'beginSecondaryAuthentication' )->willReturn(
|
||||
MediaWiki\Auth\AuthenticationResponse::newUI(
|
||||
[ new MediaWiki\Auth\UsernameAuthenticationRequest ],
|
||||
// Slightly silly message here
|
||||
wfMessage( 'mainpage' )
|
||||
)
|
||||
);
|
||||
$mockProvider->method( 'getAuthenticationRequests' )
|
||||
->willReturn( [] );
|
||||
|
||||
$this->mergeMwGlobalArrayValue( 'wgAuthManagerConfig', [
|
||||
'secondaryauth' => [ [
|
||||
'factory' => function () use ( $mockProvider ) {
|
||||
return $mockProvider;
|
||||
},
|
||||
] ],
|
||||
] );
|
||||
|
||||
$user = self::$users['sysop'];
|
||||
$userName = $user->getUser()->getName();
|
||||
$password = $user->getPassword();
|
||||
$user->getUser()->logout();
|
||||
|
||||
$ret = $this->doUserLogin( $userName, $password );
|
||||
|
||||
$this->assertSame( [ 'login' => [
|
||||
'result' => 'Aborted',
|
||||
'reason' => ApiErrorFormatter::stripMarkup( wfMessage(
|
||||
'api-login-fail-aborted' . ( $enableBotPasswords ? '' : '-nobotpw' ) )->text() ),
|
||||
] ], $ret[0] );
|
||||
}
|
||||
|
||||
/**
|
||||
* @todo Should this test just be deleted?
|
||||
* @group Broken
|
||||
*/
|
||||
public function testGotCookie() {
|
||||
|
|
@ -114,7 +216,10 @@ class ApiLoginTest extends ApiTestCase {
|
|||
);
|
||||
}
|
||||
|
||||
public function testBotPassword() {
|
||||
/**
|
||||
* @return [ $username, $password ] suitable for passing to an API request for successful login
|
||||
*/
|
||||
private function setUpForBotPassword() {
|
||||
global $wgSessionProviders;
|
||||
|
||||
$this->setMwGlobals( [
|
||||
|
|
@ -171,12 +276,51 @@ class ApiLoginTest extends ApiTestCase {
|
|||
|
||||
$lgName = $user->getUser()->getName() . BotPassword::getSeparator() . 'foo';
|
||||
|
||||
$ret = $this->doUserLogin( $lgName, $password );
|
||||
return [ $lgName, $password ];
|
||||
}
|
||||
|
||||
public function testBotPassword() {
|
||||
$ret = $this->doUserLogin( ...$this->setUpForBotPassword() );
|
||||
|
||||
$this->assertSame( 'Success', $ret[0]['login']['result'] );
|
||||
}
|
||||
|
||||
public function testLoginWithNoSameOriginSecurity() {
|
||||
public function testBotPasswordThrottled() {
|
||||
global $wgPasswordAttemptThrottle;
|
||||
|
||||
$this->setGroupPermissions( 'sysop', 'noratelimit', false );
|
||||
$this->setMwGlobals( 'wgMainCacheType', 'hash' );
|
||||
|
||||
list( $name, $password ) = $this->setUpForBotPassword();
|
||||
|
||||
for ( $i = 0; $i < $wgPasswordAttemptThrottle[0]['count']; $i++ ) {
|
||||
$this->doUserLogin( $name, 'incorrectpasswordincorrectpassword' );
|
||||
}
|
||||
|
||||
$ret = $this->doUserLogin( $name, $password );
|
||||
|
||||
$this->assertSame( [
|
||||
'result' => 'Failed',
|
||||
'reason' => ApiErrorFormatter::stripMarkup( wfMessage( 'login-throttled' )->
|
||||
durationParams( $wgPasswordAttemptThrottle[0]['seconds'] )->text() ),
|
||||
], $ret[0]['login'] );
|
||||
}
|
||||
|
||||
public function testBotPasswordLocked() {
|
||||
$this->setTemporaryHook( 'UserIsLocked', function ( User $unused, &$isLocked ) {
|
||||
$isLocked = true;
|
||||
return true;
|
||||
} );
|
||||
|
||||
$ret = $this->doUserLogin( ...$this->setUpForBotPassword() );
|
||||
|
||||
$this->assertSame( [
|
||||
'result' => 'Failed',
|
||||
'reason' => wfMessage( 'botpasswords-locked' )->text(),
|
||||
], $ret[0]['login'] );
|
||||
}
|
||||
|
||||
public function testNoSameOriginSecurity() {
|
||||
$this->setTemporaryHook( 'RequestHasSameOriginSecurity',
|
||||
function () {
|
||||
return false;
|
||||
|
|
@ -185,11 +329,15 @@ class ApiLoginTest extends ApiTestCase {
|
|||
|
||||
$ret = $this->doApiRequest( [
|
||||
'action' => 'login',
|
||||
'errorformat' => 'plaintext',
|
||||
] )[0]['login'];
|
||||
|
||||
$this->assertSame( [
|
||||
'result' => 'Aborted',
|
||||
'reason' => 'Cannot log in when the same-origin policy is not applied.',
|
||||
'reason' => [
|
||||
'code' => 'api-login-fail-sameorigin',
|
||||
'text' => 'Cannot log in when the same-origin policy is not applied.',
|
||||
],
|
||||
], $ret );
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -248,13 +248,13 @@ class BotPasswordTest extends MediaWikiTestCase {
|
|||
[ 'user', 'abc@def', false ],
|
||||
[ 'legacy@user', 'pass', false ],
|
||||
[ 'user@bot', '12345678901234567890123456789012',
|
||||
[ 'user@bot', '12345678901234567890123456789012', true ] ],
|
||||
[ 'user@bot', '12345678901234567890123456789012' ] ],
|
||||
[ 'user', 'bot@12345678901234567890123456789012',
|
||||
[ 'user@bot', '12345678901234567890123456789012', true ] ],
|
||||
[ 'user@bot', '12345678901234567890123456789012' ] ],
|
||||
[ 'user', 'bot@12345678901234567890123456789012345',
|
||||
[ 'user@bot', '12345678901234567890123456789012345', true ] ],
|
||||
[ 'user@bot', '12345678901234567890123456789012345' ] ],
|
||||
[ 'user', 'bot@x@12345678901234567890123456789012',
|
||||
[ 'user@bot@x', '12345678901234567890123456789012', true ] ],
|
||||
[ 'user@bot@x', '12345678901234567890123456789012' ] ],
|
||||
];
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue