. ' is not allowed to reset passwords' );
}
+ // Check against the rate limiter. If the $wgRateLimit is reached, we want to pretend
+ // that the request was good to avoid displaying an error message.
+ if ( $performingUser->pingLimiter( 'mailpassword' ) ) {
+ return StatusValue::newGood();
+ }
+
$username = $username ?? '';
$email = $email ?? '';
$users = [ $this->lookupUser( $username ) ];
} elseif ( $resetRoutes['email'] && $email ) {
if ( !Sanitizer::validateEmail( $email ) ) {
- return StatusValue::newFatal( 'passwordreset-invalidemail' );
+ // Only email was supplied but not valid: pretend everything's fine.
+ return StatusValue::newGood();
}
+ // Only email was provided
$method = 'email';
$users = $this->getUsersByEmail( $email );
$username = null;
+ // Remove users whose preference 'requireemail' is on since username was not submitted
+ if ( $this->config->get( 'AllowRequiringEmailForResets' ) ) {
+ foreach ( $users as $index => $user ) {
+ if ( $user->getBoolOption( 'requireemail' ) ) {
+ unset( $users[$index] );
+ }
+ }
+ }
} else {
// The user didn't supply any data
return StatusValue::newFatal( 'passwordreset-nodata' );
// Email gets set to null for backward compatibility
'Email' => $method === 'email' ? $email : null,
];
+
+ // Recreate the $users array with its values so that we reset the numeric keys since
+ // the key '0' might have been unset from $users array. 'SpecialPasswordResetOnSubmit'
+ // hook assumes that index '0' is defined if $users is not empty.
+ $users = array_values( $users );
+
if ( !Hooks::run( 'SpecialPasswordResetOnSubmit', [ &$users, $data, &$error ] ) ) {
return StatusValue::newFatal( Message::newFromSpecifier( $error ) );
}
- $firstUser = $users[0] ?? null;
+ // Get the first element in $users by using `reset` function just in case $users is changed
+ // in 'SpecialPasswordResetOnSubmit' hook.
+ $firstUser = reset( $users ) ?? null;
+
$requireEmail = $this->config->get( 'AllowRequiringEmailForResets' )
&& $method === 'username'
&& $firstUser
&& $firstUser->getBoolOption( 'requireemail' );
- if ( $requireEmail ) {
- if ( $email === '' ) {
- return StatusValue::newFatal( 'passwordreset-username-email-required' );
- }
-
- if ( !Sanitizer::validateEmail( $email ) ) {
- return StatusValue::newFatal( 'passwordreset-invalidemail' );
- }
- }
-
- // Check against the rate limiter
- if ( $performingUser->pingLimiter( 'mailpassword' ) ) {
- return StatusValue::newFatal( 'actionthrottledtext' );
+ if ( $requireEmail && ( $email === '' || !Sanitizer::validateEmail( $email ) ) ) {
+ // Email is required, and not supplied or not valid: pretend everything's fine.
+ return StatusValue::newGood();
}
if ( !$users ) {
if ( $method === 'email' ) {
// Don't reveal whether or not an email address is in use
- return StatusValue::newGood( [] );
+ return StatusValue::newGood();
} else {
return StatusValue::newFatal( 'noname' );
}
}
+ // If the username is not valid, tell the user.
+ if ( $username && !User::getCanonicalName( $username ) ) {
+ return StatusValue::newFatal( 'noname' );
+ }
+
+ // If the username doesn't exist, don't tell the user.
+ // This is not to avoid disclosure, as this information is available elsewhere,
+ // but it simplifies the password reset UX. T238961.
if ( !$firstUser instanceof User || !$firstUser->getId() ) {
- // Don't parse username as wikitext (T67501)
- return StatusValue::newFatal( wfMessage( 'nosuchuser', wfEscapeWikiText( $username ) ) );
+ return StatusValue::newGood();
}
- // All the users will have the same email address
+ // The user doesn't have an email address, but pretend everything's fine to avoid
+ // disclosing this fact. Note that all the users will have the same email address (or none),
+ // so there's no need to check more than the first.
if ( !$firstUser->getEmail() ) {
- // This won't be reachable from the email route, so safe to expose the username
- return StatusValue::newFatal( wfMessage( 'noemail',
- wfEscapeWikiText( $firstUser->getName() ) ) );
+ return StatusValue::newGood();
}
+ // Email is required but the email doesn't match: pretend everything's fine.
if ( $requireEmail && $firstUser->getEmail() !== $email ) {
- // Pretend everything's fine to avoid disclosure
return StatusValue::newGood();
}
$req->username = $user->getName();
$req->mailpassword = true;
$req->caller = $performingUser->getName();
+
$status = $this->authManager->allowsAuthenticationDataChange( $req, true );
+ // If status is good and the value is 'throttled-mailpassword', we want to pretend
+ // that the request was good to avoid displaying an error message and disclose
+ // if a reset password was previously sent.
+ if ( $status->isGood() && $status->getValue() === 'throttled-mailpassword' ) {
+ return StatusValue::newGood();
+ }
+
if ( $status->isGood() && $status->getValue() !== 'ignored' ) {
$reqs[] = $req;
} elseif ( $result->isGood() ) {
use MediaWiki\Auth\AuthManager;
use MediaWiki\Auth\TemporaryPasswordAuthenticationRequest;
-use MediaWiki\Block\DatabaseBlock;
use MediaWiki\Block\CompositeBlock;
+use MediaWiki\Block\DatabaseBlock;
use MediaWiki\Block\SystemBlock;
use MediaWiki\Config\ServiceOptions;
use MediaWiki\Permissions\PermissionManager;
->with( $user, 'editmyprivateinfo' )
->willReturn( $canEditPrivate );
- $loadBalancer = $this->getMockBuilder( ILoadBalancer::class )->getMock();
+ $loadBalancer = $this->createMock( ILoadBalancer::class );
$passwordReset = new PasswordReset(
$config,
];
}
- /**
- * @expectedException \LogicException
- */
public function testExecute_notAllowed() {
- $user = $this->getMock( User::class );
+ $user = $this->createMock( User::class );
/** @var User $user */
$passwordReset = $this->getMockBuilder( PasswordReset::class )
->willReturn( Status::newFatal( 'somestatuscode' ) );
/** @var PasswordReset $passwordReset */
+ $this->expectException( \LogicException::class );
$passwordReset->execute( $user );
}
'SpecialPasswordResetOnSubmit' => [],
] );
- $loadBalancer = $this->getMockBuilder( ILoadBalancer::class )
- ->getMock();
+ $loadBalancer = $this->createMock( ILoadBalancer::class );
$users = $this->makeUsers();
$permissionManager = $this->makePermissionManager( $performingUser, true );
return [
- 'Invalid email' => [
- 'expectedError' => 'passwordreset-invalidemail',
+ 'Throttled, pretend everything is ok' => [
+ 'expectedError' => false,
'config' => $defaultConfig,
'performingUser' => $throttledUser,
'permissionManager' => $permissionManager,
'authManager' => $this->makeAuthManager(),
+ 'username' => 'User1',
+ 'email' => '',
+ 'usersWithEmail' => [],
+ ],
+ 'Throttled, email required for resets, is invalid, pretend everything is ok' => [
+ 'expectedError' => false,
+ 'config' => $emailRequiredConfig,
+ 'performingUser' => $throttledUser,
+ 'permissionManager' => $permissionManager,
+ 'authManager' => $this->makeAuthManager(),
+ 'username' => 'User1',
+ 'email' => '[invalid email]',
+ 'usersWithEmail' => [],
+ ],
+ 'Invalid email, pretend everything is OK' => [
+ 'expectedError' => false,
+ 'config' => $defaultConfig,
+ 'performingUser' => $performingUser,
+ 'permissionManager' => $permissionManager,
+ 'authManager' => $this->makeAuthManager(),
'username' => '',
'email' => '[invalid email]',
'usersWithEmail' => [],
'No username, no email' => [
'expectedError' => 'passwordreset-nodata',
'config' => $defaultConfig,
- 'performingUser' => $throttledUser,
+ 'performingUser' => $performingUser,
'permissionManager' => $permissionManager,
'authManager' => $this->makeAuthManager(),
'username' => '',
'Email route not enabled' => [
'expectedError' => 'passwordreset-nodata',
'config' => $this->makeConfig( true, [ 'username' => true ], false ),
- 'performingUser' => $throttledUser,
+ 'performingUser' => $performingUser,
'permissionManager' => $permissionManager,
'authManager' => $this->makeAuthManager(),
'username' => '',
'Username route not enabled' => [
'expectedError' => 'passwordreset-nodata',
'config' => $this->makeConfig( true, [ 'email' => true ], false ),
- 'performingUser' => $throttledUser,
+ 'performingUser' => $performingUser,
'permissionManager' => $permissionManager,
'authManager' => $this->makeAuthManager(),
'username' => 'User1',
'No routes enabled' => [
'expectedError' => 'passwordreset-nodata',
'config' => $this->makeConfig( true, [], false ),
- 'performingUser' => $throttledUser,
+ 'performingUser' => $performingUser,
'permissionManager' => $permissionManager,
'authManager' => $this->makeAuthManager(),
'username' => 'User1',
'email' => self::VALID_EMAIL,
'usersWithEmail' => [],
],
- 'Email reqiured for resets, but is empty' => [
- 'expectedError' => 'passwordreset-username-email-required',
+ 'Email required for resets but is empty, pretend everything is OK' => [
+ 'expectedError' => false,
'config' => $emailRequiredConfig,
- 'performingUser' => $throttledUser,
+ 'performingUser' => $performingUser,
'permissionManager' => $permissionManager,
'authManager' => $this->makeAuthManager(),
'username' => 'User1',
'email' => '',
'usersWithEmail' => [],
],
- 'Email reqiured for resets, is invalid' => [
- 'expectedError' => 'passwordreset-invalidemail',
+ 'Email required for resets but is invalid, pretend everything is OK' => [
+ 'expectedError' => false,
'config' => $emailRequiredConfig,
- 'performingUser' => $throttledUser,
+ 'performingUser' => $performingUser,
'permissionManager' => $permissionManager,
'authManager' => $this->makeAuthManager(),
'username' => 'User1',
'email' => '[invalid email]',
'usersWithEmail' => [],
],
- 'Throttled' => [
- 'expectedError' => 'actionthrottledtext',
+ 'Password email already sent within 24 hours, pretend everything is ok' => [
+ 'expectedError' => false,
'config' => $defaultConfig,
- 'performingUser' => $throttledUser,
+ 'performingUser' => $performingUser,
'permissionManager' => $permissionManager,
- 'authManager' => $this->makeAuthManager(),
+ 'authManager' => $this->makeAuthManager( [ 'User1' ], 0, [], [ 'User1' ] ),
'username' => 'User1',
'email' => '',
- 'usersWithEmail' => [],
+ 'usersWithEmail' => [ 'User1' ],
],
- 'No user by this username' => [
- 'expectedError' => 'nosuchuser',
+ 'No user by this username, pretend everything is OK' => [
+ 'expectedError' => false,
'config' => $defaultConfig,
'performingUser' => $performingUser,
'permissionManager' => $permissionManager,
'email' => '',
'usersWithEmail' => [],
],
+ 'Username is not valid' => [
+ 'expectedError' => 'noname',
+ 'config' => $defaultConfig,
+ 'performingUser' => $performingUser,
+ 'permissionManager' => $permissionManager,
+ 'authManager' => $this->makeAuthManager(),
+ 'username' => 'Invalid|username',
+ 'email' => '',
+ 'usersWithEmail' => [],
+ ],
'If no users with this email found, pretend everything is OK' => [
'expectedError' => false,
'config' => $defaultConfig,
'email' => 'some@not.found.email',
'usersWithEmail' => [],
],
- 'No email for the user' => [
- 'expectedError' => 'noemail',
+ 'No email for the user, pretend everything is OK' => [
+ 'expectedError' => false,
'config' => $defaultConfig,
'performingUser' => $performingUser,
'permissionManager' => $permissionManager,
'email' => '',
'usersWithEmail' => [],
],
- 'Email reqiured for resets, no match' => [
+ 'Email required for resets, no match' => [
'expectedError' => false,
'config' => $emailRequiredConfig,
'performingUser' => $performingUser,
'email' => self::VALID_EMAIL,
'usersWithEmail' => [ 'User2' ],
],
+ 'Reset three users via email that did not opt in, multiple users with same email' => [
+ 'expectedError' => false,
+ 'config' => $emailRequiredConfig,
+ 'performingUser' => $performingUser,
+ 'permissionManager' => $permissionManager,
+ 'authManager' => $this->makeAuthManager( [ 'User2', 'User3', 'User4' ], 3, [ 'User1' ] ),
+ 'username' => '',
+ 'email' => self::VALID_EMAIL,
+ 'usersWithEmail' => [ 'User1', 'User2', 'User3', 'User4' ],
+ ],
];
}
}
/**
- * @param string[] $allowed
- * @param int $numUsersToAuth
- * @param string[] $ignored
+ * @param string[] $allowed Usernames that are allowed to send password reset email
+ * by AuthManager's allowsAuthenticationDataChange method.
+ * @param int $numUsersToAuth Number of users that will receive email
+ * @param string[] $ignored Usernames that are allowed but ignored by AuthManager's
+ * allowsAuthenticationDataChange method and will not receive password reset email.
+ * @param string[] $mailThrottledLimited Usernames that have already
+ * received the password reset email within a given time, and AuthManager
+ * changeAuthenticationData method will mark them as 'throttled-mailpassword.'
* @return AuthManager
*/
private function makeAuthManager(
array $allowed = [],
$numUsersToAuth = 0,
- array $ignored = []
+ array $ignored = [],
+ array $mailThrottledLimited = []
) : AuthManager {
$authManager = $this->getMockBuilder( AuthManager::class )
->disableOriginalConstructor()
->getMock();
$authManager->method( 'allowsAuthenticationDataChange' )
->willReturnCallback(
- function ( TemporaryPasswordAuthenticationRequest $req ) use ( $allowed, $ignored ) {
+ function ( TemporaryPasswordAuthenticationRequest $req )
+ use ( $allowed, $ignored, $mailThrottledLimited ) {
+ if ( in_array( $req->username, $mailThrottledLimited, true ) ) {
+ return Status::newGood( 'throttled-mailpassword' );
+ }
+
$value = in_array( $req->username, $ignored, true )
? 'ignored'
: 'okie dokie';
+
return in_array( $req->username, $allowed, true )
? Status::newGood( $value )
: Status::newFatal( 'rejected by test mock' );
private function makeUsers() {
$user1 = $this->getMockBuilder( User::class )->getMock();
$user2 = $this->getMockBuilder( User::class )->getMock();
+ $user3 = $this->getMockBuilder( User::class )->getMock();
+ $user4 = $this->getMockBuilder( User::class )->getMock();
$user1->method( 'getName' )->willReturn( 'User1' );
$user2->method( 'getName' )->willReturn( 'User2' );
+ $user3->method( 'getName' )->willReturn( 'User3' );
+ $user4->method( 'getName' )->willReturn( 'User4' );
$user1->method( 'getId' )->willReturn( 1 );
$user2->method( 'getId' )->willReturn( 2 );
+ $user3->method( 'getId' )->willReturn( 3 );
+ $user4->method( 'getId' )->willReturn( 4 );
$user1->method( 'getEmail' )->willReturn( self::VALID_EMAIL );
$user2->method( 'getEmail' )->willReturn( self::VALID_EMAIL );
+ $user3->method( 'getEmail' )->willReturn( self::VALID_EMAIL );
+ $user4->method( 'getEmail' )->willReturn( self::VALID_EMAIL );
$user1->method( 'getBoolOption' )
->with( 'requireemail' )
$badUser = $this->getMockBuilder( User::class )->getMock();
$badUser->method( 'getName' )->willReturn( 'BadUser' );
- $badUser->method( 'getId' )->willReturn( 3 );
+ $badUser->method( 'getId' )->willReturn( 5 );
$badUser->method( 'getEmail' )->willReturn( null );
return [
'User1' => $user1,
'User2' => $user2,
+ 'User3' => $user3,
+ 'User4' => $user4,
'BadUser' => $badUser,
];
}
dépôts / lhc / web / wiklou.git / commitdiff