From 9bb2875e2eb7e5deeff7b6647616eb279906b27b Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Thu, 26 May 2016 13:09:14 -0400 Subject: [PATCH] =?utf8?q?AuthManager=20fixups=20around=20the=20login?= =?utf8?q?=E2=86=92RESTART=E2=86=92create=20flow?= MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit * ApiQueryAuthManagerInfo will differentiate between preserved linking data and a preserved createRequest. * ApiQueryAuthManagerInfo will indicate the preserved username, if any, because the client will have to pass that back to action=createaccount. * ApiClientLogin won't tell about the confusing CreateFromLoginAuthenticationRequest returned on RESTART responses. * Explain how 'preservestate' works in ApiAMCreateAccount's auto-doc. * ConfirmLinkSecondaryAuthenticationProvider will filter out requests that can no longer be used (i.e. if it was for linking the account that got used for creation). * All the complicated code in AuthManager::beginAccountCreation() was trying to deal with allowing the client to pass only the CreateFromLoginAuthenticationRequest. That was dumb, removed it. * Added methods to CreateFromLoginAuthenticationRequest to indicate its status with respect to different kinds of preserved state. * Increase accuracy of the AuthenticationResponse::$createRequest doc. Change-Id: I726d79de18e739d6e60c1eea51453433c21ba207 --- includes/api/ApiAMCreateAccount.php | 5 +- includes/api/ApiClientLogin.php | 8 ++ includes/api/ApiQueryAuthManagerInfo.php | 22 +++++- includes/api/i18n/en.json | 1 + includes/api/i18n/qqq.json | 1 + includes/auth/AuthManager.php | 72 +++++++----------- includes/auth/AuthenticationResponse.php | 13 ++-- ...irmLinkSecondaryAuthenticationProvider.php | 5 +- .../CreateFromLoginAuthenticationRequest.php | 36 ++++++++- .../phpunit/includes/auth/AuthManagerTest.php | 74 +++++-------------- ...inkSecondaryAuthenticationProviderTest.php | 19 ++++- ...eateFromLoginAuthenticationRequestTest.php | 31 ++++++++ 12 files changed, 171 insertions(+), 116 deletions(-) diff --git a/includes/api/ApiAMCreateAccount.php b/includes/api/ApiAMCreateAccount.php index 806b8d2344..0a4b6dc214 100644 --- a/includes/api/ApiAMCreateAccount.php +++ b/includes/api/ApiAMCreateAccount.php @@ -109,9 +109,12 @@ class ApiAMCreateAccount extends ApiBase { } public function getAllowedParams() { - return ApiAuthManagerHelper::getStandardParams( AuthManager::ACTION_CREATE, + $ret = ApiAuthManagerHelper::getStandardParams( AuthManager::ACTION_CREATE, 'requests', 'messageformat', 'mergerequestfields', 'preservestate', 'returnurl', 'continue' ); + $ret['preservestate'][ApiBase::PARAM_HELP_MSG_APPEND][] = + 'apihelp-createaccount-param-preservestate'; + return $ret; } public function dynamicParameterDocumentation() { diff --git a/includes/api/ApiClientLogin.php b/includes/api/ApiClientLogin.php index 711234a65b..cffccb15b5 100644 --- a/includes/api/ApiClientLogin.php +++ b/includes/api/ApiClientLogin.php @@ -23,6 +23,7 @@ use MediaWiki\Auth\AuthManager; use MediaWiki\Auth\AuthenticationRequest; use MediaWiki\Auth\AuthenticationResponse; +use MediaWiki\Auth\CreateFromLoginAuthenticationRequest; /** * Log in to the wiki with AuthManager @@ -90,6 +91,13 @@ class ApiClientLogin extends ApiBase { $res = $manager->beginAuthentication( $reqs, $params['returnurl'] ); } + // Remove CreateFromLoginAuthenticationRequest from $res->neededRequests. + // It's there so a RESTART treated as UI will work right, but showing + // it to the API client is just confusing. + $res->neededRequests = ApiAuthManagerHelper::blacklistAuthenticationRequests( + $res->neededRequests, [ CreateFromLoginAuthenticationRequest::class ] + ); + $this->getResult()->addValue( null, 'clientlogin', $helper->formatAuthenticationResponse( $res ) ); } diff --git a/includes/api/ApiQueryAuthManagerInfo.php b/includes/api/ApiQueryAuthManagerInfo.php index b591f9c00a..9068e11ae1 100644 --- a/includes/api/ApiQueryAuthManagerInfo.php +++ b/includes/api/ApiQueryAuthManagerInfo.php @@ -43,7 +43,6 @@ class ApiQueryAuthManagerInfo extends ApiQueryBase { 'canauthenticatenow' => $manager->canAuthenticateNow(), 'cancreateaccounts' => $manager->canCreateAccounts(), 'canlinkaccounts' => $manager->canLinkAccounts(), - 'haspreservedstate' => $helper->getPreservedRequest() !== null, ]; if ( $params['securitysensitiveoperation'] !== null ) { @@ -53,10 +52,27 @@ class ApiQueryAuthManagerInfo extends ApiQueryBase { } if ( $params['requestsfor'] ) { - $reqs = $manager->getAuthenticationRequests( $params['requestsfor'], $this->getUser() ); + $action = $params['requestsfor']; + + $preservedReq = $helper->getPreservedRequest(); + if ( $preservedReq ) { + $ret += [ + 'haspreservedstate' => $preservedReq->hasStateForAction( $action ), + 'hasprimarypreservedstate' => $preservedReq->hasPrimaryStateForAction( $action ), + 'preservedusername' => (string)$preservedReq->username, + ]; + } else { + $ret += [ + 'haspreservedstate' => false, + 'hasprimarypreservedstate' => false, + 'preservedusername' => '', + ]; + } + + $reqs = $manager->getAuthenticationRequests( $action, $this->getUser() ); // Filter out blacklisted requests, depending on the action - switch ( $params['requestsfor'] ) { + switch ( $action ) { case AuthManager::ACTION_CHANGE: $reqs = ApiAuthManagerHelper::blacklistAuthenticationRequests( $reqs, $this->getConfig()->get( 'ChangeCredentialsBlacklist' ) diff --git a/includes/api/i18n/en.json b/includes/api/i18n/en.json index 4e9309e699..0f7548bf47 100644 --- a/includes/api/i18n/en.json +++ b/includes/api/i18n/en.json @@ -60,6 +60,7 @@ "apihelp-compare-example-1": "Create a diff between revision 1 and 2.", "apihelp-createaccount-description": "Create a new user account.", + "apihelp-createaccount-param-preservestate": "If [[Special:ApiHelp/query+authmanagerinfo|action=query&meta=authmanagerinfo]] returned true for hasprimarypreservedstate, requests marked as primary-required should be omitted. If it returned a non-empty value for preservedusername, that username must be used for the username parameter.", "apihelp-createaccount-example-create": "Start the process of creating user Example with password ExamplePassword.", "apihelp-createaccount-param-name": "Username.", "apihelp-createaccount-param-password": "Password (ignored if $1mailpassword is set).", diff --git a/includes/api/i18n/qqq.json b/includes/api/i18n/qqq.json index 6137457c1b..991326d8b9 100644 --- a/includes/api/i18n/qqq.json +++ b/includes/api/i18n/qqq.json @@ -62,6 +62,7 @@ "apihelp-compare-param-torev": "{{doc-apihelp-param|compare|torev}}", "apihelp-compare-example-1": "{{doc-apihelp-example|compare}}", "apihelp-createaccount-description": "{{doc-apihelp-description|createaccount}}", + "apihelp-createaccount-param-preservestate": "{{doc-apihelp-param|createaccount|preservestate|info=This message is displayed in addition to {{msg-mw|api-help-authmanagerhelper-preservestate}}.}}", "apihelp-createaccount-example-create": "{{doc-apihelp-example|createaccount}}", "apihelp-createaccount-param-name": "{{doc-apihelp-param|createaccount|name}}\n{{Identical|Username}}", "apihelp-createaccount-param-password": "{{doc-apihelp-param|createaccount|password}}", diff --git a/includes/auth/AuthManager.php b/includes/auth/AuthManager.php index efee53c6dc..136ce262a7 100644 --- a/includes/auth/AuthManager.php +++ b/includes/auth/AuthManager.php @@ -231,6 +231,17 @@ class AuthManager implements LoggerAwareInterface { /** * Start an authentication flow + * + * In addition to the AuthenticationRequests returned by + * $this->getAuthenticationRequests(), a client might include a + * CreateFromLoginAuthenticationRequest from a previous login attempt to + * preserve state. + * + * Instead of the AuthenticationRequests returned by + * $this->getAuthenticationRequests(), a client might pass a + * CreatedAccountAuthenticationRequest from an account creation that just + * succeeded to log in to the just-created account. + * * @param AuthenticationRequest[] $reqs * @param string $returnToUrl Url that REDIRECT responses should eventually * return to. @@ -344,8 +355,7 @@ class AuthManager implements LoggerAwareInterface { * Return values are interpreted as follows: * - status FAIL: Authentication failed. If $response->createRequest is * set, that may be passed to self::beginAuthentication() or to - * self::beginAccountCreation() (after adding a username, if necessary) - * to preserve state. + * self::beginAccountCreation() to preserve state. * - status REDIRECT: The client should be redirected to the contained URL, * new AuthenticationRequests should be made (if any), then * AuthManager::continueAuthentication() should be called. @@ -950,6 +960,17 @@ class AuthManager implements LoggerAwareInterface { /** * Start an account creation flow + * + * In addition to the AuthenticationRequests returned by + * $this->getAuthenticationRequests(), a client might include a + * CreateFromLoginAuthenticationRequest from a previous login attempt. If + * + * $createFromLoginAuthenticationRequest->hasPrimaryStateForAction( AuthManager::ACTION_CREATE ) + * + * returns true, any AuthenticationRequest::PRIMARY_REQUIRED requests + * should be omitted. If the CreateFromLoginAuthenticationRequest has a + * username set, that username must be used for all other requests. + * * @param User $creator User doing the account creation * @param AuthenticationRequest[] $reqs * @param string $returnToUrl Url that REDIRECT responses should eventually @@ -1038,44 +1059,10 @@ class AuthManager implements LoggerAwareInterface { if ( $req ) { $state['maybeLink'] = $req->maybeLink; - // If we get here, the user didn't submit a form with any of the - // usual AuthenticationRequests that are needed for an account - // creation. So we need to determine if there are any and return a - // UI response if so. if ( $req->createRequest ) { - // We have a createRequest from a - // PrimaryAuthenticationProvider, so don't ask. - $providers = $this->getPreAuthenticationProviders() + - $this->getSecondaryAuthenticationProviders(); - } else { - // We're only preserving maybeLink, so ask for primary fields - // too. - $providers = $this->getPreAuthenticationProviders() + - $this->getPrimaryAuthenticationProviders() + - $this->getSecondaryAuthenticationProviders(); - } - $reqs = $this->getAuthenticationRequestsInternal( - self::ACTION_CREATE, - [], - $providers - ); - // See if we need any requests to begin - foreach ( (array)$reqs as $r ) { - if ( !$r instanceof UsernameAuthenticationRequest && - !$r instanceof UserDataAuthenticationRequest && - !$r instanceof CreationReasonAuthenticationRequest - ) { - // Needs some reqs, so request them - $reqs[] = new CreateFromLoginAuthenticationRequest( $req->createRequest, [] ); - $state['continueRequests'] = $reqs; - $session->setSecret( 'AuthManager::accountCreationState', $state ); - $session->persist(); - return AuthenticationResponse::newUI( $reqs, wfMessage( 'authmanager-create-from-login' ) ); - } + $reqs[] = $req->createRequest; + $state['reqs'][] = $req->createRequest; } - // No reqs needed, so we can just continue. - $req->createRequest->returnToUrl = $returnToUrl; - $reqs = [ $req->createRequest ]; } $session->setSecret( 'AuthManager::accountCreationState', $state ); @@ -1213,15 +1200,6 @@ class AuthManager implements LoggerAwareInterface { $req->username = $state['username']; } - // If we're coming in from a create-from-login UI response, we need - // to extract the createRequest (if any). - $req = AuthenticationRequest::getRequestByClass( - $reqs, CreateFromLoginAuthenticationRequest::class - ); - if ( $req && $req->createRequest ) { - $reqs[] = $req->createRequest; - } - // Run pre-creation tests, if we haven't already if ( !$state['ranPreTests'] ) { $providers = $this->getPreAuthenticationProviders() + diff --git a/includes/auth/AuthenticationResponse.php b/includes/auth/AuthenticationResponse.php index db0182552d..5048cf84dd 100644 --- a/includes/auth/AuthenticationResponse.php +++ b/includes/auth/AuthenticationResponse.php @@ -83,13 +83,14 @@ class AuthenticationResponse { /** * @var AuthenticationRequest|null * - * Returned with a PrimaryAuthenticationProvider login FAIL, this holds a - * request that should result in a PASS when passed to that provider's - * PrimaryAuthenticationProvider::beginPrimaryAccountCreation(). + * Returned with a PrimaryAuthenticationProvider login FAIL or a PASS with + * no username, this holds a request that should result in a PASS when + * passed to that provider's PrimaryAuthenticationProvider::beginPrimaryAccountCreation(). * - * Returned with an AuthManager login FAIL or RESTART, this holds a request - * that may be passed to AuthManager::beginCreateAccount() after setting - * its ->returnToUrl property. It may also be passed to + * Returned with an AuthManager login FAIL or RESTART, this holds a + * CreateFromLoginAuthenticationRequest that may be passed to + * AuthManager::beginCreateAccount(), possibly in place of any + * "primary-required" requests. It may also be passed to * AuthManager::beginAuthentication() to preserve state. */ public $createRequest = null; diff --git a/includes/auth/ConfirmLinkSecondaryAuthenticationProvider.php b/includes/auth/ConfirmLinkSecondaryAuthenticationProvider.php index 180aaae34e..700b91ec93 100644 --- a/includes/auth/ConfirmLinkSecondaryAuthenticationProvider.php +++ b/includes/auth/ConfirmLinkSecondaryAuthenticationProvider.php @@ -50,7 +50,10 @@ class ConfirmLinkSecondaryAuthenticationProvider extends AbstractSecondaryAuthen if ( !is_array( $state ) ) { return AuthenticationResponse::newAbstain(); } - $maybeLink = $state['maybeLink']; + + $maybeLink = array_filter( $state['maybeLink'], function ( $req ) { + return $this->manager->allowsAuthenticationDataChange( $req )->isGood(); + } ); if ( !$maybeLink ) { return AuthenticationResponse::newAbstain(); } diff --git a/includes/auth/CreateFromLoginAuthenticationRequest.php b/includes/auth/CreateFromLoginAuthenticationRequest.php index 949302d8bc..ddeb13d9d6 100644 --- a/includes/auth/CreateFromLoginAuthenticationRequest.php +++ b/includes/auth/CreateFromLoginAuthenticationRequest.php @@ -25,7 +25,8 @@ namespace MediaWiki\Auth; * This transfers state between the login and account creation flows. * * AuthManager::getAuthenticationRequests() won't return this type, but it - * may be passed to AuthManager::beginAccountCreation() anyway. + * may be passed to AuthManager::beginAuthentication() or + * AuthManager::beginAccountCreation() anyway. * * @ingroup Auth * @since 1.27 @@ -50,6 +51,7 @@ class CreateFromLoginAuthenticationRequest extends AuthenticationRequest { ) { $this->createRequest = $createRequest; $this->maybeLink = $maybeLink; + $this->username = $createRequest ? $createRequest->username : null; } public function getFieldInfo() { @@ -59,4 +61,36 @@ class CreateFromLoginAuthenticationRequest extends AuthenticationRequest { public function loadFromSubmission( array $data ) { return true; } + + /** + * Indicate whether this request contains any state for the specified + * action. + * @param string $action One of the AuthManager::ACTION_* constants + * @return boolean + */ + public function hasStateForAction( $action ) { + switch ( $action ) { + case AuthManager::ACTION_LOGIN: + return (bool)$this->maybeLink; + case AuthManager::ACTION_CREATE: + return $this->maybeLink || $this->createRequest; + default: + return false; + } + } + + /** + * Indicate whether this request contains state for the specified + * action sufficient to replace other primary-required requests. + * @param string $action One of the AuthManager::ACTION_* constants + * @return boolean + */ + public function hasPrimaryStateForAction( $action ) { + switch ( $action ) { + case AuthManager::ACTION_CREATE: + return (bool)$this->createRequest; + default: + return false; + } + } } diff --git a/tests/phpunit/includes/auth/AuthManagerTest.php b/tests/phpunit/includes/auth/AuthManagerTest.php index 377abe2b55..e681be0957 100644 --- a/tests/phpunit/includes/auth/AuthManagerTest.php +++ b/tests/phpunit/includes/auth/AuthManagerTest.php @@ -781,6 +781,12 @@ class AuthManagerTest extends \MediaWikiTestCase { $userReq = new UsernameAuthenticationRequest; $userReq->username = 'UTDummy'; + $req1->returnToUrl = 'http://localhost/'; + $req2->returnToUrl = 'http://localhost/'; + $req3->returnToUrl = 'http://localhost/'; + $req3->username = 'UTDummy'; + $userReq->returnToUrl = 'http://localhost/'; + // Passing one into beginAuthentication(), and an immediate FAIL $primary = $this->getMockForAbstractClass( AbstractPrimaryAuthenticationProvider::class ); $this->primaryauthMocks = [ $primary ]; @@ -824,71 +830,29 @@ class AuthManagerTest extends \MediaWikiTestCase { $this->assertSame( $req2, $ret->createRequest->createRequest ); $this->assertEquals( [], $ret->createRequest->maybeLink ); - // Pass into beginAccountCreation(), no createRequest, primary needs reqs - $primary = $this->getMockBuilder( AbstractPrimaryAuthenticationProvider::class ) - ->setMethods( [ 'testForAccountCreation' ] ) - ->getMockForAbstractClass(); + // Pass into beginAccountCreation(), see that maybeLink and createRequest get copied + $primary = $this->getMockForAbstractClass( AbstractPrimaryAuthenticationProvider::class ); $this->primaryauthMocks = [ $primary ]; $this->initializeManager( true ); + $createReq = new CreateFromLoginAuthenticationRequest( $req3, [ $req2 ] ); + $createReq->returnToUrl = 'http://localhost/'; + $createReq->username = 'UTDummy'; + $res = AuthenticationResponse::newUI( [ $req1 ], wfMessage( 'foo' ) ); + $primary->expects( $this->any() )->method( 'beginPrimaryAccountCreation' ) + ->with( $this->anything(), $this->anything(), [ $userReq, $createReq, $req3 ] ) + ->will( $this->returnValue( $res ) ); $primary->expects( $this->any() )->method( 'accountCreationType' ) ->will( $this->returnValue( PrimaryAuthenticationProvider::TYPE_CREATE ) ); - $primary->expects( $this->any() )->method( 'getAuthenticationRequests' ) - ->will( $this->returnValue( [ $req1 ] ) ); - $primary->expects( $this->any() )->method( 'testForAccountCreation' ) - ->will( $this->returnValue( StatusValue::newFatal( 'fail' ) ) ); - $createReq = new CreateFromLoginAuthenticationRequest( - null, [ $req2->getUniqueId() => $req2 ] - ); $this->logger->setCollect( true ); $ret = $this->manager->beginAccountCreation( $user, [ $userReq, $createReq ], 'http://localhost/' ); $this->logger->setCollect( false ); $this->assertSame( AuthenticationResponse::UI, $ret->status ); - $this->assertCount( 4, $ret->neededRequests ); - $this->assertSame( $req1, $ret->neededRequests[0] ); - $this->assertInstanceOf( UsernameAuthenticationRequest::class, $ret->neededRequests[1] ); - $this->assertInstanceOf( UserDataAuthenticationRequest::class, $ret->neededRequests[2] ); - $this->assertInstanceOf( CreateFromLoginAuthenticationRequest::class, $ret->neededRequests[3] ); - $this->assertSame( null, $ret->neededRequests[3]->createRequest ); - $this->assertEquals( [], $ret->neededRequests[3]->maybeLink ); - - // Pass into beginAccountCreation(), with createRequest, primary needs reqs - $createReq = new CreateFromLoginAuthenticationRequest( $req2, [] ); - $this->logger->setCollect( true ); - $ret = $this->manager->beginAccountCreation( - $user, [ $userReq, $createReq ], 'http://localhost/' - ); - $this->logger->setCollect( false ); - $this->assertSame( AuthenticationResponse::FAIL, $ret->status ); - $this->assertSame( 'fail', $ret->message->getKey() ); - - // Again, with a secondary needing reqs too - $secondary = $this->getMockBuilder( AbstractSecondaryAuthenticationProvider::class ) - ->getMockForAbstractClass(); - $this->secondaryauthMocks = [ $secondary ]; - $this->initializeManager( true ); - $secondary->expects( $this->any() )->method( 'getAuthenticationRequests' ) - ->will( $this->returnValue( [ $req3 ] ) ); - $createReq = new CreateFromLoginAuthenticationRequest( $req2, [] ); - $this->logger->setCollect( true ); - $ret = $this->manager->beginAccountCreation( - $user, [ $userReq, $createReq ], 'http://localhost/' - ); - $this->logger->setCollect( false ); - $this->assertSame( AuthenticationResponse::UI, $ret->status ); - $this->assertCount( 4, $ret->neededRequests ); - $this->assertSame( $req3, $ret->neededRequests[0] ); - $this->assertInstanceOf( UsernameAuthenticationRequest::class, $ret->neededRequests[1] ); - $this->assertInstanceOf( UserDataAuthenticationRequest::class, $ret->neededRequests[2] ); - $this->assertInstanceOf( CreateFromLoginAuthenticationRequest::class, $ret->neededRequests[3] ); - $this->assertSame( $req2, $ret->neededRequests[3]->createRequest ); - $this->assertEquals( [], $ret->neededRequests[3]->maybeLink ); - $this->logger->setCollect( true ); - $ret = $this->manager->continueAccountCreation( $ret->neededRequests ); - $this->logger->setCollect( false ); - $this->assertSame( AuthenticationResponse::FAIL, $ret->status ); - $this->assertSame( 'fail', $ret->message->getKey() ); + $state = $this->request->getSession()->getSecret( 'AuthManager::accountCreationState' ); + $this->assertNotNull( $state ); + $this->assertEquals( [ $userReq, $createReq, $req3 ], $state['reqs'] ); + $this->assertEquals( [ $req2 ], $state['maybeLink'] ); } /** diff --git a/tests/phpunit/includes/auth/ConfirmLinkSecondaryAuthenticationProviderTest.php b/tests/phpunit/includes/auth/ConfirmLinkSecondaryAuthenticationProviderTest.php index 09d046c8b9..d254e81277 100644 --- a/tests/phpunit/includes/auth/ConfirmLinkSecondaryAuthenticationProviderTest.php +++ b/tests/phpunit/includes/auth/ConfirmLinkSecondaryAuthenticationProviderTest.php @@ -128,12 +128,27 @@ class ConfirmLinkSecondaryAuthenticationProviderTest extends \MediaWikiTestCase } public function testBeginLinkAttempt() { + $badReq = $this->getMockBuilder( AuthenticationRequest::class ) + ->setMethods( [ 'getUniqueId' ] ) + ->getMockForAbstractClass(); + $badReq->expects( $this->any() )->method( 'getUniqueId' ) + ->will( $this->returnValue( "BadReq" ) ); + $user = \User::newFromName( 'UTSysop' ); $provider = \TestingAccessWrapper::newFromObject( new ConfirmLinkSecondaryAuthenticationProvider ); $request = new \FauxRequest(); - $manager = new AuthManager( $request, \RequestContext::getMain()->getConfig() ); + $manager = $this->getMockBuilder( AuthManager::class ) + ->setMethods( [ 'allowsAuthenticationDataChange' ] ) + ->setConstructorArgs( [ $request, \RequestContext::getMain()->getConfig() ] ) + ->getMock(); + $manager->expects( $this->any() )->method( 'allowsAuthenticationDataChange' ) + ->will( $this->returnCallback( function ( $req ) { + return $req->getUniqueId() !== 'BadReq' + ? \StatusValue::newGood() + : \StatusValue::newFatal( 'no' ); + } ) ); $provider->setManager( $manager ); $this->assertEquals( @@ -151,7 +166,7 @@ class ConfirmLinkSecondaryAuthenticationProviderTest extends \MediaWikiTestCase $reqs = $this->getLinkRequests(); $request->getSession()->setSecret( 'state', [ - 'maybeLink' => $reqs + 'maybeLink' => $reqs + [ 'BadReq' => $badReq ] ] ); $res = $provider->beginLinkAttempt( $user, 'state' ); $this->assertInstanceOf( AuthenticationResponse::class, $res ); diff --git a/tests/phpunit/includes/auth/CreateFromLoginAuthenticationRequestTest.php b/tests/phpunit/includes/auth/CreateFromLoginAuthenticationRequestTest.php index fb0613d185..d166caa64e 100644 --- a/tests/phpunit/includes/auth/CreateFromLoginAuthenticationRequestTest.php +++ b/tests/phpunit/includes/auth/CreateFromLoginAuthenticationRequestTest.php @@ -23,4 +23,35 @@ class CreateFromLoginAuthenticationRequestTest extends AuthenticationRequestTest ], ]; } + + /** + * @dataProvider provideState + */ + public function testState( + $createReq, $maybeLink, $username, $loginState, $createState, $createPrimaryState + ) { + $req = new CreateFromLoginAuthenticationRequest( $createReq, $maybeLink ); + $this->assertSame( $username, $req->username ); + $this->assertSame( $loginState, $req->hasStateForAction( AuthManager::ACTION_LOGIN ) ); + $this->assertSame( $createState, $req->hasStateForAction( AuthManager::ACTION_CREATE ) ); + $this->assertFalse( $req->hasStateForAction( AuthManager::ACTION_LINK ) ); + $this->assertFalse( $req->hasPrimaryStateForAction( AuthManager::ACTION_LOGIN ) ); + $this->assertSame( $createPrimaryState, + $req->hasPrimaryStateForAction( AuthManager::ACTION_CREATE ) ); + } + + public static function provideState() { + $req1 = new UsernameAuthenticationRequest; + $req2 = new UsernameAuthenticationRequest; + $req2->username = 'Bob'; + + return [ + 'Nothing' => [ null, [], null, false, false, false ], + 'Link, no create' => [ null, [ $req2 ], null, true, true, false ], + 'No link, create but no name' => [ $req1, [], null, false, true, true ], + 'Link and create but no name' => [ $req1, [ $req2 ], null, true, true, true ], + 'No link, create with name' => [ $req2, [], 'Bob', false, true, true ], + 'Link and create with name' => [ $req2, [ $req2 ], 'Bob', true, true, true ], + ]; + } } -- 2.20.1