From: Brad Jorsch Date: Fri, 4 Sep 2015 16:17:42 +0000 (-0400) Subject: User: Mostly remove password handling X-Git-Tag: 1.31.0-rc.0~9408 X-Git-Url: https://git.cyclocoop.org//%22?a=commitdiff_plain;h=3d0b4fea3dfb94610be0f0e9d8ff1cb24f106707;p=lhc%2Fweb%2Fwiklou.git User: Mostly remove password handling AuthManager is coming, which will make it easier to add alternative methods of authentication. But in order to do that, we need to finally get around to ripping the password-related bits out of the User class. The password expiration handling isn't used anywhere in core or extensions in Gerrit beyond testing for expired passwords on login and resetting the expiry date on password change. Those bits have been inlined and the functions removed; AuthManager will allow each "authentication provider" to handle its own password expiration. The methods for fetching passwords, including the fact that mPassword and other fields are public, has also been removed. This is already broken in combination with basically any extension that messes with authentication, and the major use outside of that was in creating system users like MassMessage's "MediaWiki message delivery" user. Password setting methods are silently deprecated, since most of the replacements won't be available until AuthManager. But uses in unit testing can be replaced with TestUser::setPasswordForUser() immediately. User::randomPassword() and User::getPasswordFactory() don't really belong in User either. For the former a new PasswordFactory method has been created, while the latter should just be replaced by the two lines to create a PasswordFactory via its constructor. Bug: T47716 Change-Id: I2c736ad72d946fa9b859e6cd335fa58aececc0d5 --- diff --git a/RELEASE-NOTES-1.27 b/RELEASE-NOTES-1.27 index 697210c5a9..85151ee970 100644 --- a/RELEASE-NOTES-1.27 +++ b/RELEASE-NOTES-1.27 @@ -25,6 +25,25 @@ production. * $wgScriptExtension (and support for '.php5' entry points) was removed. See the deprecation notice in the release notes for version 1.25 for advice on how to preserve support for '.php5' entry points via URL rewriting. +* Password handling via the User object has been deprecated and partially + removed, pending the future introduction of AuthManager. In particular: +** expirePassword(), getPasswordExpireDate(), resetPasswordExpiration(), and + getPasswordExpired() have been removed. They were unused outside of core. +** The mPassword, mNewpassword, mNewpassTime, and mPasswordExpires fields are + now private and will be removed in the future. +** The getPassword() and getTemporaryPassword() methods now throw + BadMethodCallException and will be removed in the future. +** The ability to pass 'password' and 'newpassword' to createNew() has been + removed. The only users of it seem to have been using it to set invalid + passwords, and so shouldn't be greatly affected. +** setPassword(), setInternalPassword(), and setNewpassword() have been + deprecated, pending the introduction of AuthManager. +** User::randomPassword() is deprecated in favor of a new method + PasswordFactory::generateRandomPasswordString() +** User::getPasswordFactory() is deprecated, callers should just create a + PasswordFactory themselves. +** A new constructor, User::newSystemUser(), has been added to simplify the + creation of passwordless "system" users for logged actions. === New features in 1.27 === * $wgDataCenterId and $wgDataCenterRoles where added, which will serve as @@ -47,6 +66,8 @@ production. * Added OOUI dialogs and layout for file upload interfaces. See documentation for mw.Upload.Dialog, mw.Upload.BookletLayout and its subclasses for more information. +* User::newSystemUser() may be used to simplify the creation of passwordless + "system" users for logged actions from scripts and extensions. ==== External libraries ==== diff --git a/includes/User.php b/includes/User.php index 75649a7881..5bf6f44d70 100644 --- a/includes/User.php +++ b/includes/User.php @@ -28,7 +28,7 @@ define( 'EDIT_TOKEN_SUFFIX', '+\\' ); /** * The User object encapsulates all of the user-specific settings (user_id, - * name, rights, password, email address, options, last login time). Client + * name, rights, email address, options, last login time). Client * classes use the getXXX() functions to access these fields. These functions * do all the work of determining whether the user is logged in, * whether the requested option can be satisfied from cookies or @@ -64,11 +64,6 @@ class User implements IDBAccessObject { */ const GETOPTIONS_EXCLUDE_DEFAULTS = 1; - /** - * @var PasswordFactory Lazily loaded factory object for passwords - */ - private static $mPasswordFactory = null; - /** * Array of Strings List of member variables which are saved to the * shared cache (memcached). Any operation which changes the @@ -190,20 +185,22 @@ class User implements IDBAccessObject { public $mName; /** @var string */ public $mRealName; + /** - * @todo Make this actually private - * @private - * @var Password - */ - public $mPassword; - /** - * @todo Make this actually private - * @private - * @var Password + * These fields were marked "@private", but were defined as public to + * maintain compatibility with PHP4 code since PHP4 didn't support access + * restrictions. AuthManager makes password handling pluggable, meaning + * these fields don't make sense anymore. If this broke something, see + * T89459 for the context of the change. + * @deprecated These are mostly unused, but kept for now to raise errors on attempted access. */ - public $mNewpassword; - /** @var string */ - public $mNewpassTime; + // @{ + private $mPassword = null; + private $mNewpassword; + private $mNewpassTime; + private $mPasswordExpires; + // @} + /** @var string */ public $mEmail; /** @var string TS_MW timestamp from the DB */ @@ -226,8 +223,6 @@ class User implements IDBAccessObject { public $mGroups; /** @var array */ protected $mOptionOverrides; - /** @var string */ - protected $mPasswordExpires; // @} /** @@ -559,7 +554,7 @@ class User implements IDBAccessObject { * The row should have the following fields from the user table in it: * - either user_name or user_id to load further data if needed (or both) * - user_real_name - * - all other fields (email, password, etc.) + * - all other fields (email, etc.) * It is useless to provide the remaining fields if either user_id, * user_name and user_real_name are not provided because the whole row * will be loaded once more from the database when accessing them. @@ -574,6 +569,97 @@ class User implements IDBAccessObject { return $user; } + /** + * Static factory method for creation of a "system" user from username. + * + * A "system" user is an account that's used to attribute logged actions + * taken by MediaWiki itself, as opposed to a bot or human user. Examples + * might include the 'Maintenance script' or 'Conversion script' accounts + * used by various scripts in the maintenance/ directory or accounts such + * as 'MediaWiki message delivery' used by the MassMessage extension. + * + * This can optionally create the user if it doesn't exist, and "steal" the + * account if it does exist. + * + * @param string $name Username + * @param array $options Options are: + * - validate: As for User::getCanonicalName(), default 'valid' + * - create: Whether to create the user if it doesn't already exist, default true + * - steal: Whether to reset the account's password and email if it + * already exists, default false + * @return User|null + */ + public static function newSystemUser( $name, $options = array() ) { + $options += array( + 'validate' => 'valid', + 'create' => true, + 'steal' => false, + ); + + $name = self::getCanonicalName( $name, $options['validate'] ); + if ( $name === false ) { + return null; + } + + $dbw = wfGetDB( DB_MASTER ); + $row = $dbw->selectRow( + 'user', + array_merge( + self::selectFields(), + array( 'user_password', 'user_newpassword' ) + ), + array( 'user_name' => $name ), + __METHOD__ + ); + if ( !$row ) { + // No user. Create it? + return $options['create'] ? self::createNew( $name ) : null; + } + $user = self::newFromRow( $row ); + + // A user is considered to exist as a non-system user if it has a + // password set, or a temporary password set, or an email set. + $passwordFactory = new PasswordFactory(); + $passwordFactory->init( RequestContext::getMain()->getConfig() ); + try { + $password = $passwordFactory->newFromCiphertext( $row->user_password ); + } catch ( PasswordError $e ) { + wfDebug( 'Invalid password hash found in database.' ); + $password = PasswordFactory::newInvalidPassword(); + } + try { + $newpassword = $passwordFactory->newFromCiphertext( $row->user_newpassword ); + } catch ( PasswordError $e ) { + wfDebug( 'Invalid password hash found in database.' ); + $newpassword = PasswordFactory::newInvalidPassword(); + } + if ( !$password instanceof InvalidPassword || !$newpassword instanceof InvalidPassword + || $user->mEmail + ) { + // User exists. Steal it? + if ( !$options['steal'] ) { + return null; + } + + $nopass = PasswordFactory::newInvalidPassword()->toString(); + + $dbw->update( + 'user', + array( + 'user_password' => $nopass, + 'user_newpassword' => $nopass, + 'user_newpass_time' => null, + ), + array( 'user_id' => $user->getId() ), + __METHOD__ + ); + $user->invalidateEmail(); + $user->saveSettings(); + } + + return $user; + } + // @} /** @@ -875,73 +961,6 @@ class User implements IDBAccessObject { } } - /** - * Expire a user's password - * @since 1.23 - * @param int $ts Optional timestamp to convert, default 0 for the current time - */ - public function expirePassword( $ts = 0 ) { - $this->loadPasswords(); - $timestamp = wfTimestamp( TS_MW, $ts ); - $this->mPasswordExpires = $timestamp; - $this->saveSettings(); - } - - /** - * Clear the password expiration for a user - * @since 1.23 - * @param bool $load Ensure user object is loaded first - */ - public function resetPasswordExpiration( $load = true ) { - global $wgPasswordExpirationDays; - if ( $load ) { - $this->load(); - } - $newExpire = null; - if ( $wgPasswordExpirationDays ) { - $newExpire = wfTimestamp( - TS_MW, - time() + ( $wgPasswordExpirationDays * 24 * 3600 ) - ); - } - // Give extensions a chance to force an expiration - Hooks::run( 'ResetPasswordExpiration', array( $this, &$newExpire ) ); - $this->mPasswordExpires = $newExpire; - } - - /** - * Check if the user's password is expired. - * TODO: Put this and password length into a PasswordPolicy object - * @since 1.23 - * @return string|bool The expiration type, or false if not expired - * hard: A password change is required to login - * soft: Allow login, but encourage password change - * false: Password is not expired - */ - public function getPasswordExpired() { - global $wgPasswordExpireGrace; - $expired = false; - $now = wfTimestamp(); - $expiration = $this->getPasswordExpireDate(); - $expUnix = wfTimestamp( TS_UNIX, $expiration ); - if ( $expiration !== null && $expUnix < $now ) { - $expired = ( $expUnix + $wgPasswordExpireGrace < $now ) ? 'hard' : 'soft'; - } - return $expired; - } - - /** - * Get this user's password expiration date. Since this may be using - * the cached User object, we assume that whatever mechanism is setting - * the expiration date is also expiring the User cache. - * @since 1.23 - * @return string|null The datestamp of the expiration, or null if not set - */ - public function getPasswordExpireDate() { - $this->load(); - return $this->mPasswordExpires; - } - /** * Given unvalidated user input, return a canonical username, or false if * the username is invalid. @@ -1022,19 +1041,12 @@ class User implements IDBAccessObject { /** * Return a random password. * + * @deprecated since 1.27, use PasswordFactory::generateRandomPasswordString() * @return string New random password */ public static function randomPassword() { global $wgMinimalPasswordLength; - // Decide the final password length based on our min password length, - // stopping at a minimum of 10 chars. - $length = max( 10, $wgMinimalPasswordLength ); - // Multiply by 1.25 to get the number of hex characters we need - $length = $length * 1.25; - // Generate random hex chars - $hex = MWCryptRand::generateHex( $length ); - // Convert from base 16 to base 32 to get a proper password like string - return wfBaseConvert( $hex, 16, 32 ); + return PasswordFactory::generateRandomPasswordString( $wgMinimalPasswordLength ); } /** @@ -1046,15 +1058,9 @@ class User implements IDBAccessObject { * @param string|bool $name */ public function loadDefaults( $name = false ) { - - $passwordFactory = self::getPasswordFactory(); - $this->mId = 0; $this->mName = $name; $this->mRealName = ''; - $this->mPassword = $passwordFactory->newFromCiphertext( null ); - $this->mNewpassword = $passwordFactory->newFromCiphertext( null ); - $this->mNewpassTime = null; $this->mEmail = ''; $this->mOptionOverrides = null; $this->mOptionsLoaded = false; @@ -1070,8 +1076,6 @@ class User implements IDBAccessObject { $this->mEmailAuthenticated = null; $this->mEmailToken = ''; $this->mEmailTokenExpires = null; - $this->mPasswordExpires = null; - $this->resetPasswordExpiration( false ); $this->mRegistration = wfTimestamp( TS_MW ); $this->mGroups = array(); @@ -1243,7 +1247,6 @@ class User implements IDBAccessObject { */ protected function loadFromRow( $row, $data = null ) { $all = true; - $passwordFactory = self::getPasswordFactory(); $this->mGroups = null; // deferred @@ -1280,31 +1283,6 @@ class User implements IDBAccessObject { $all = false; } - if ( isset( $row->user_password ) ) { - // Check for *really* old password hashes that don't even have a type - // The old hash format was just an md5 hex hash, with no type information - if ( preg_match( '/^[0-9a-f]{32}$/', $row->user_password ) ) { - $row->user_password = ":A:{$this->mId}:{$row->user_password}"; - } - - try { - $this->mPassword = $passwordFactory->newFromCiphertext( $row->user_password ); - } catch ( PasswordError $e ) { - wfDebug( 'Invalid password hash found in database.' ); - $this->mPassword = $passwordFactory->newFromCiphertext( null ); - } - - try { - $this->mNewpassword = $passwordFactory->newFromCiphertext( $row->user_newpassword ); - } catch ( PasswordError $e ) { - wfDebug( 'Invalid password hash found in database.' ); - $this->mNewpassword = $passwordFactory->newFromCiphertext( null ); - } - - $this->mNewpassTime = wfTimestampOrNull( TS_MW, $row->user_newpass_time ); - $this->mPasswordExpires = wfTimestampOrNull( TS_MW, $row->user_password_expires ); - } - if ( isset( $row->user_email ) ) { $this->mEmail = $row->user_email; $this->mTouched = wfTimestamp( TS_MW, $row->user_touched ); @@ -1367,33 +1345,6 @@ class User implements IDBAccessObject { } } - /** - * Load the user's password hashes from the database - * - * This is usually called in a scenario where the actual User object was - * loaded from the cache, and then password comparison needs to be performed. - * Password hashes are not stored in memcached. - * - * @since 1.24 - */ - private function loadPasswords() { - if ( $this->getId() !== 0 && - ( $this->mPassword === null || $this->mNewpassword === null ) - ) { - $db = ( $this->queryFlagsUsed & self::READ_LATEST ) - ? wfGetDB( DB_MASTER ) - : wfGetDB( DB_SLAVE ); - - $this->loadFromRow( $db->selectRow( - 'user', - array( 'user_password', 'user_newpassword', - 'user_newpass_time', 'user_password_expires' ), - array( 'user_id' => $this->getId() ), - __METHOD__ - ) ); - } - } - /** * Add the user to the group if he/she meets given criteria. * @@ -2383,23 +2334,21 @@ class User implements IDBAccessObject { } /** + * @deprecated Removed in 1.27. * @return Password * @since 1.24 */ public function getPassword() { - $this->loadPasswords(); - - return $this->mPassword; + throw new BadMethodCallException( __METHOD__ . ' has been removed in 1.27' ); } /** + * @deprecated Removed in 1.27. * @return Password * @since 1.24 */ public function getTemporaryPassword() { - $this->loadPasswords(); - - return $this->mNewpassword; + throw new BadMethodCallException( __METHOD__ . ' has been removed in 1.27' ); } /** @@ -2413,16 +2362,14 @@ class User implements IDBAccessObject { * wipes it, so the account cannot be logged in until * a new password is set, for instance via e-mail. * + * @deprecated since 1.27. AuthManager is coming. * @param string $str New password to set * @throws PasswordError On failure - * * @return bool */ public function setPassword( $str ) { global $wgAuth; - $this->loadPasswords(); - if ( $str !== null ) { if ( !$wgAuth->allowPasswordChange() ) { throw new PasswordError( wfMessage( 'password-change-forbidden' )->text() ); @@ -2438,7 +2385,9 @@ class User implements IDBAccessObject { throw new PasswordError( wfMessage( 'externaldberror' )->text() ); } - $this->setInternalPassword( $str ); + $this->setToken(); + $this->setOption( 'watchlisttoken', false ); + $this->setPasswordInternal( $str ); return true; } @@ -2446,19 +2395,49 @@ class User implements IDBAccessObject { /** * Set the password and reset the random token unconditionally. * + * @deprecated since 1.27. AuthManager is coming. * @param string|null $str New password to set or null to set an invalid * password hash meaning that the user will not be able to log in * through the web interface. */ public function setInternalPassword( $str ) { - $this->setToken(); - $this->setOption( 'watchlisttoken', false ); + global $wgAuth; - $passwordFactory = self::getPasswordFactory(); - $this->mPassword = $passwordFactory->newFromPlaintext( $str ); + if ( $wgAuth->allowSetLocalPassword() ) { + $this->setToken(); + $this->setOption( 'watchlisttoken', false ); + $this->setPasswordInternal( $str ); + } + } - $this->mNewpassword = $passwordFactory->newFromCiphertext( null ); - $this->mNewpassTime = null; + /** + * Actually set the password and such + * @param string|null $str New password to set or null to set an invalid + * password hash meaning that the user will not be able to log in + * through the web interface. + */ + private function setPasswordInternal( $str ) { + $id = self::idFromName( $this->getName() ); + if ( $id ) { + $passwordFactory = new PasswordFactory(); + $passwordFactory->init( RequestContext::getMain()->getConfig() ); + $dbw = wfGetDB( DB_MASTER ); + $dbw->update( + 'user', + array( + 'user_password' => $passwordFactory->newFromPlaintext( $str )->toString(), + 'user_newpassword' => PasswordFactory::newInvalidPassword()->toString(), + 'user_newpass_time' => $dbw->timestampOrNull( null ), + ), + array( + 'user_id' => $id, + ), + __METHOD__ + ); + $this->mPassword = null; + } else { + $this->mPassword = $str; + } } /** @@ -2493,19 +2472,27 @@ class User implements IDBAccessObject { /** * Set the password for a password reminder or new account email * + * @deprecated since 1.27, AuthManager is coming * @param string $str New password to set or null to set an invalid * password hash meaning that the user will not be able to use it * @param bool $throttle If true, reset the throttle timestamp to the present */ public function setNewpassword( $str, $throttle = true ) { - $this->loadPasswords(); + $dbw = wfGetDB( DB_MASTER ); + + $passwordFactory = new PasswordFactory(); + $passwordFactory->init( RequestContext::getMain()->getConfig() ); + $update = array( + 'user_newpassword' => $passwordFactory->newFromPlaintext( $str )->toString(), + ); - $this->mNewpassword = self::getPasswordFactory()->newFromPlaintext( $str ); if ( $str === null ) { - $this->mNewpassTime = null; + $update['user_newpass_time'] = null; } elseif ( $throttle ) { - $this->mNewpassTime = wfTimestampNow(); + $update['user_newpass_time'] = $dbw->timestamp(); } + + $dbw->update( 'user', $update, array( 'user_id' => $id ), __METHOD__ ); } /** @@ -2515,11 +2502,27 @@ class User implements IDBAccessObject { */ public function isPasswordReminderThrottled() { global $wgPasswordReminderResendTime; + + if ( !$wgPasswordReminderResendTime ) { + return false; + } + $this->load(); - if ( !$this->mNewpassTime || !$wgPasswordReminderResendTime ) { + + $db = ( $this->queryFlagsUsed & self::READ_LATEST ) + ? wfGetDB( DB_MASTER ) + : wfGetDB( DB_SLAVE ); + $newpassTime = $db->selectField( + 'user', + 'user_newpass_time', + array( 'user_id' => $this->getId() ), + __METHOD__ + ); + + if ( $newpassTime === null ) { return false; } - $expiry = wfTimestamp( TS_UNIX, $this->mNewpassTime ) + $wgPasswordReminderResendTime * 3600; + $expiry = wfTimestamp( TS_UNIX, $newpassTime ) + $wgPasswordReminderResendTime * 3600; return time() < $expiry; } @@ -3657,8 +3660,6 @@ class User implements IDBAccessObject { * @todo Only rarely do all these fields need to be set! */ public function saveSettings() { - global $wgAuth; - if ( wfReadOnly() ) { // @TODO: caller should deal with this instead! // This should really just be an exception. @@ -3670,7 +3671,6 @@ class User implements IDBAccessObject { } $this->load(); - $this->loadPasswords(); if ( 0 == $this->mId ) { return; // anon } @@ -3681,17 +3681,10 @@ class User implements IDBAccessObject { $oldTouched = $this->mTouched; $newTouched = $this->newTouchedTimestamp(); - if ( !$wgAuth->allowSetLocalPassword() ) { - $this->mPassword = self::getPasswordFactory()->newFromCiphertext( null ); - } - $dbw = wfGetDB( DB_MASTER ); $dbw->update( 'user', array( /* SET */ 'user_name' => $this->mName, - 'user_password' => $this->mPassword->toString(), - 'user_newpassword' => $this->mNewpassword->toString(), - 'user_newpass_time' => $dbw->timestampOrNull( $this->mNewpassTime ), 'user_real_name' => $this->mRealName, 'user_email' => $this->mEmail, 'user_email_authenticated' => $dbw->timestampOrNull( $this->mEmailAuthenticated ), @@ -3699,7 +3692,6 @@ class User implements IDBAccessObject { 'user_token' => strval( $this->mToken ), 'user_email_token' => $this->mEmailToken, 'user_email_token_expires' => $dbw->timestampOrNull( $this->mEmailTokenExpires ), - 'user_password_expires' => $dbw->timestampOrNull( $this->mPasswordExpires ), ), array( /* WHERE */ 'user_id' => $this->mId, 'user_touched' => $dbw->timestamp( $oldTouched ) // CAS check @@ -3757,10 +3749,6 @@ class User implements IDBAccessObject { * @param string $name Username to add * @param array $params Array of Strings Non-default parameters to save to * the database as user_* fields: - * - password: The user's password hash. Password logins will be disabled - * if this is omitted. - * - newpassword: Hash for a temporary password that has been mailed to - * the user. * - email: The user's email address. * - email_authenticated: The email authentication timestamp. * - real_name: The user's real name. @@ -3771,9 +3759,15 @@ class User implements IDBAccessObject { * @return User|null User object, or null if the username already exists. */ public static function createNew( $name, $params = array() ) { + foreach ( array( 'password', 'newpassword', 'newpass_time', 'password_expires' ) as $field ) { + if ( isset( $params[$field] ) ) { + wfDeprecated( __METHOD__ . " with param '$field'", '1.27' ); + unset( $params[$field] ); + } + } + $user = new User; $user->load(); - $user->loadPasswords(); $user->setToken(); // init token if ( isset( $params['options'] ) ) { $user->mOptions = $params['options'] + (array)$user->mOptions; @@ -3782,12 +3776,13 @@ class User implements IDBAccessObject { $dbw = wfGetDB( DB_MASTER ); $seqVal = $dbw->nextSequenceValue( 'user_user_id_seq' ); + $noPass = PasswordFactory::newInvalidPassword()->toString(); + $fields = array( 'user_id' => $seqVal, 'user_name' => $name, - 'user_password' => $user->mPassword->toString(), - 'user_newpassword' => $user->mNewpassword->toString(), - 'user_newpass_time' => $dbw->timestampOrNull( $user->mNewpassTime ), + 'user_password' => $noPass, + 'user_newpassword' => $noPass, 'user_email' => $user->mEmail, 'user_email_authenticated' => $dbw->timestampOrNull( $user->mEmailAuthenticated ), 'user_real_name' => $user->mRealName, @@ -3836,13 +3831,14 @@ class User implements IDBAccessObject { */ public function addToDatabase() { $this->load(); - $this->loadPasswords(); if ( !$this->mToken ) { $this->setToken(); // init token } $this->mTouched = $this->newTouchedTimestamp(); + $noPass = PasswordFactory::newInvalidPassword()->toString(); + $dbw = wfGetDB( DB_MASTER ); $inWrite = $dbw->writesOrCallbacksPending(); $seqVal = $dbw->nextSequenceValue( 'user_user_id_seq' ); @@ -3850,9 +3846,8 @@ class User implements IDBAccessObject { array( 'user_id' => $seqVal, 'user_name' => $this->mName, - 'user_password' => $this->mPassword->toString(), - 'user_newpassword' => $this->mNewpassword->toString(), - 'user_newpass_time' => $dbw->timestampOrNull( $this->mNewpassTime ), + 'user_password' => $noPass, + 'user_newpassword' => $noPass, 'user_email' => $this->mEmail, 'user_email_authenticated' => $dbw->timestampOrNull( $this->mEmailAuthenticated ), 'user_real_name' => $this->mRealName, @@ -3894,6 +3889,11 @@ class User implements IDBAccessObject { } $this->mId = $dbw->insertId(); + // Set the password now that it's in the DB, if applicable + if ( $this->mPassword !== null ) { + $this->setPasswordInternal( $this->mPassword ); + } + // Clear instance cache other than user table data, which is already accurate $this->clearInstanceCache(); @@ -4002,13 +4002,14 @@ class User implements IDBAccessObject { /** * Check to see if the given clear-text password is one of the accepted passwords + * @deprecated since 1.27. AuthManager is coming. * @param string $password User password * @return bool True if the given password is correct, otherwise False */ public function checkPassword( $password ) { global $wgAuth, $wgLegacyEncoding; - $this->loadPasswords(); + $this->load(); // Some passwords will give a fatal Status, which means there is // some sort of technical or security reason for this password to @@ -4030,12 +4031,27 @@ class User implements IDBAccessObject { return false; } - if ( !$this->mPassword->equals( $password ) ) { + $passwordFactory = new PasswordFactory(); + $passwordFactory->init( RequestContext::getMain()->getConfig() ); + $db = ( $this->queryFlagsUsed & self::READ_LATEST ) + ? wfGetDB( DB_MASTER ) + : wfGetDB( DB_SLAVE ); + + try { + $mPassword = $passwordFactory->newFromCiphertext( $db->selectField( + 'user', 'user_password', array( 'user_id' => $this->getId() ), __METHOD__ + ) ); + } catch ( PasswordError $e ) { + wfDebug( 'Invalid password hash found in database.' ); + $mPassword = PasswordFactory::newInvalidPassword(); + } + + if ( !$mPassword->equals( $password ) ) { if ( $wgLegacyEncoding ) { // Some wikis were converted from ISO 8859-1 to UTF-8, the passwords can't be converted // Check for this with iconv $cp1252Password = iconv( 'UTF-8', 'WINDOWS-1252//TRANSLIT', $password ); - if ( $cp1252Password === $password || !$this->mPassword->equals( $cp1252Password ) ) { + if ( $cp1252Password === $password || !$mPassword->equals( $cp1252Password ) ) { return false; } } else { @@ -4043,10 +4059,8 @@ class User implements IDBAccessObject { } } - $passwordFactory = self::getPasswordFactory(); - if ( $passwordFactory->needsUpdate( $this->mPassword ) && !wfReadOnly() ) { - $this->mPassword = $passwordFactory->newFromPlaintext( $password ); - $this->saveSettings(); + if ( $passwordFactory->needsUpdate( $mPassword ) && !wfReadOnly() ) { + $this->setPasswordInternal( $password ); } return true; @@ -4056,20 +4070,39 @@ class User implements IDBAccessObject { * Check if the given clear-text password matches the temporary password * sent by e-mail for password reset operations. * + * @deprecated since 1.27. AuthManager is coming. * @param string $plaintext - * * @return bool True if matches, false otherwise */ public function checkTemporaryPassword( $plaintext ) { global $wgNewPasswordExpiry; $this->load(); - $this->loadPasswords(); - if ( $this->mNewpassword->equals( $plaintext ) ) { - if ( is_null( $this->mNewpassTime ) ) { + + $passwordFactory = new PasswordFactory(); + $passwordFactory->init( RequestContext::getMain()->getConfig() ); + $db = ( $this->queryFlagsUsed & self::READ_LATEST ) + ? wfGetDB( DB_MASTER ) + : wfGetDB( DB_SLAVE ); + + $row = $db->selectRow( + 'user', + array( 'user_newpassword', 'user_newpass_time' ), + array( 'user_id' => $this->getId() ), + __METHOD__ + ); + try { + $mNewpassword = $passwordFactory->newFromCiphertext( $row->user_newpassword ); + } catch ( PasswordError $e ) { + wfDebug( 'Invalid password hash found in database.' ); + $mNewpassword = PasswordFactory::newInvalidPassword(); + } + + if ( $mNewpassword->equals( $plaintext ) ) { + if ( is_null( $row->user_newpass_time ) ) { return true; } - $expiry = wfTimestamp( TS_UNIX, $this->mNewpassTime ) + $wgNewPasswordExpiry; + $expiry = wfTimestamp( TS_UNIX, $row->user_newpass_time ) + $wgNewPasswordExpiry; return ( time() < $expiry ); } else { return false; @@ -4927,7 +4960,9 @@ class User implements IDBAccessObject { */ public static function crypt( $password, $salt = false ) { wfDeprecated( __METHOD__, '1.24' ); - $hash = self::getPasswordFactory()->newFromPlaintext( $password ); + $passwordFactory = new PasswordFactory(); + $passwordFactory->init( RequestContext::getMain()->getConfig() ); + $hash = $passwordFactory->newFromPlaintext( $password ); return $hash->toString(); } @@ -4956,7 +4991,9 @@ class User implements IDBAccessObject { } } - $hash = self::getPasswordFactory()->newFromCiphertext( $hash ); + $passwordFactory = new PasswordFactory(); + $passwordFactory->init( RequestContext::getMain()->getConfig() ); + $hash = $passwordFactory->newFromCiphertext( $hash ); return $hash->equals( $password ); } @@ -5166,15 +5203,14 @@ class User implements IDBAccessObject { /** * Lazily instantiate and return a factory object for making passwords * + * @deprecated since 1.27, create a PasswordFactory directly instead * @return PasswordFactory */ public static function getPasswordFactory() { - if ( self::$mPasswordFactory === null ) { - self::$mPasswordFactory = new PasswordFactory(); - self::$mPasswordFactory->init( RequestContext::getMain()->getConfig() ); - } - - return self::$mPasswordFactory; + wfDeprecated( __METHOD__, '1.27' ); + $ret = new PasswordFactory(); + $ret->init( RequestContext::getMain()->getConfig() ); + return $ret; } /** @@ -5196,6 +5232,7 @@ class User implements IDBAccessObject { * * @todo FIXME: This does not belong here; put it in Html or Linker or somewhere * + * @deprecated since 1.27 * @return array Array of HTML attributes suitable for feeding to * Html::element(), directly or indirectly. (Don't feed to Xml::*()! * That will get confused by the boolean attribute syntax used.) diff --git a/includes/password/PasswordFactory.php b/includes/password/PasswordFactory.php index 86a3fefd58..e1f272b4c3 100644 --- a/includes/password/PasswordFactory.php +++ b/includes/password/PasswordFactory.php @@ -188,4 +188,38 @@ final class PasswordFactory { return $password->needsUpdate(); } } + + /** + * Generate a random string suitable for a password + * + * @param int $minLength Minimum length of password to generate + * @return string + */ + public static function generateRandomPasswordString( $minLength = 10 ) { + // Decide the final password length based on our min password length, + // stopping at a minimum of 10 chars. + $length = max( 10, $minLength ); + // Multiply by 1.25 to get the number of hex characters we need + $length = $length * 1.25; + // Generate random hex chars + $hex = MWCryptRand::generateHex( $length ); + // Convert from base 16 to base 32 to get a proper password like string + return wfBaseConvert( $hex, 16, 32 ); + } + + /** + * Create an InvalidPassword + * + * @return InvalidPassword + */ + public static function newInvalidPassword() { + static $password = null; + + if ( $password === null ) { + $factory = new self(); + $password = new InvalidPassword( $factory, array( 'type' => '' ), null ); + } + + return $password; + } } diff --git a/includes/specials/SpecialChangePassword.php b/includes/specials/SpecialChangePassword.php index 6a4347df97..df68b12c3c 100644 --- a/includes/specials/SpecialChangePassword.php +++ b/includes/specials/SpecialChangePassword.php @@ -301,8 +301,8 @@ class SpecialChangePassword extends FormSpecialPage { $remember = $this->getRequest()->getCookie( 'Token' ) !== null; $user->setCookies( null, null, $remember ); } - $user->resetPasswordExpiration(); $user->saveSettings(); + $this->resetPasswordExpiration( $user ); } public function requiresUnblock() { @@ -312,4 +312,28 @@ class SpecialChangePassword extends FormSpecialPage { protected function getGroupName() { return 'users'; } + + /** + * For resetting user password expiration, until AuthManager comes along + * @param User $user + */ + private function resetPasswordExpiration( User $user ) { + global $wgPasswordExpirationDays; + $newExpire = null; + if ( $wgPasswordExpirationDays ) { + $newExpire = wfTimestamp( + TS_MW, + time() + ( $wgPasswordExpirationDays * 24 * 3600 ) + ); + } + // Give extensions a chance to force an expiration + Hooks::run( 'ResetPasswordExpiration', array( $this, &$newExpire ) ); + $dbw = wfGetDB( DB_MASTER ); + $dbw->update( + 'user', + array( 'user_password_expires' => $dbw->timestampOrNull( $newExpire ) ), + array( 'user_id' => $user->getID() ), + __METHOD__ + ); + } } diff --git a/includes/specials/SpecialPasswordReset.php b/includes/specials/SpecialPasswordReset.php index f1eb8c2325..53e1d1b9bc 100644 --- a/includes/specials/SpecialPasswordReset.php +++ b/includes/specials/SpecialPasswordReset.php @@ -139,7 +139,7 @@ class SpecialPasswordReset extends FormSpecialPage { * @return bool|array */ public function onSubmit( array $data ) { - global $wgAuth; + global $wgAuth, $wgMinimalPasswordLength; if ( isset( $data['Domain'] ) ) { if ( $wgAuth->validDomain( $data['Domain'] ) ) { @@ -254,7 +254,7 @@ class SpecialPasswordReset extends FormSpecialPage { $passwords = array(); foreach ( $users as $user ) { - $password = $user->randomPassword(); + $password = PasswordFactory::generateRandomPasswordString( $wgMinimalPasswordLength ); $user->setNewpassword( $password ); $user->saveSettings(); $passwords[] = $this->msg( 'passwordreset-emailelement', $user->getName(), $password ) diff --git a/includes/specials/SpecialUserlogin.php b/includes/specials/SpecialUserlogin.php index 085cfee4f0..0da598bfa0 100644 --- a/includes/specials/SpecialUserlogin.php +++ b/includes/specials/SpecialUserlogin.php @@ -826,7 +826,7 @@ class LoginForm extends SpecialPage { } elseif ( $wgBlockDisablesLogin && $u->isBlocked() ) { // If we've enabled it, make it so that a blocked user cannot login $retval = self::USER_BLOCKED; - } elseif ( $u->getPasswordExpired() == 'hard' ) { + } elseif ( $this->checkUserPasswordExpired( $u ) == 'hard' ) { // Force reset now, without logging in $retval = self::RESET_PASS; $this->mAbortLoginErrorMsg = 'resetpass-expired'; @@ -994,7 +994,7 @@ class LoginForm extends SpecialPage { $this->getContext()->setLanguage( $userLang ); // Reset SessionID on Successful login (bug 40995) $this->renewSessionId(); - if ( $this->getUser()->getPasswordExpired() == 'soft' ) { + if ( $this->checkUserPasswordExpired( $this->getUser() ) == 'soft' ) { $this->resetLoginForm( $this->msg( 'resetpass-expired-soft' ) ); } elseif ( $wgInvalidPasswordReset && !$user->isValidPassword( $this->mPassword ) @@ -1121,7 +1121,7 @@ class LoginForm extends SpecialPage { function mailPasswordInternal( $u, $throttle = true, $emailTitle = 'passwordremindertitle', $emailText = 'passwordremindertext' ) { - global $wgNewPasswordExpiry; + global $wgNewPasswordExpiry, $wgMinimalPasswordLength; if ( $u->getEmail() == '' ) { return Status::newFatal( 'noemail', $u->getName() ); @@ -1134,7 +1134,7 @@ class LoginForm extends SpecialPage { $currentUser = $this->getUser(); Hooks::run( 'User::mailPasswordInternal', array( &$currentUser, &$ip, &$u ) ); - $np = $u->randomPassword(); + $np = PasswordFactory::generateRandomPasswordString( $wgMinimalPasswordLength ); $u->setNewpassword( $np, $throttle ); $u->saveSettings(); $userLanguage = $u->getOption( 'language' ); @@ -1717,4 +1717,25 @@ class LoginForm extends SpecialPage { protected function getGroupName() { return 'login'; } + + /** + * Private function to check password expiration, until AuthManager comes + * along to handle that. + * @param User $user + * @return string|bool + */ + private function checkUserPasswordExpired( User $user ) { + global $wgPasswordExpireGrace; + $dbr = wfGetDB( DB_SLAVE ); + $ts = $dbr->selectField( 'user', 'user_password_expires', array( 'user_id' => $user->getId() ) ); + + $expired = false; + $now = wfTimestamp(); + $expUnix = wfTimestamp( TS_UNIX, $ts ); + if ( $ts !== null && $expUnix < $now ) { + $expired = ( $expUnix + $wgPasswordExpireGrace < $now ) ? 'hard' : 'soft'; + } + return $expired; + } + } diff --git a/maintenance/cleanupCaps.php b/maintenance/cleanupCaps.php index 6234db48ca..e0a0f494fb 100644 --- a/maintenance/cleanupCaps.php +++ b/maintenance/cleanupCaps.php @@ -53,7 +53,7 @@ class CapsCleanup extends TableCleanup { $this->error( "\$wgCapitalLinks is on -- no need for caps links cleanup.", true ); } - $this->user = User::newFromName( 'Conversion script' ); + $this->user = User::newSystemUser( 'Conversion script', array( 'steal' => true ) ); $this->namespace = intval( $this->getOption( 'namespace', 0 ) ); $this->dryrun = $this->hasOption( 'dry-run' ); diff --git a/maintenance/cleanupSpam.php b/maintenance/cleanupSpam.php index f4a5147a56..44d5810748 100644 --- a/maintenance/cleanupSpam.php +++ b/maintenance/cleanupSpam.php @@ -45,7 +45,7 @@ class CleanupSpam extends Maintenance { global $IP, $wgLocalDatabases, $wgUser; $username = wfMessage( 'spambot_username' )->text(); - $wgUser = User::newFromName( $username ); + $wgUser = User::newSystemUser( $username ); if ( !$wgUser ) { $this->error( "Invalid username specified in 'spambot_username' message: $username", true ); } diff --git a/maintenance/cleanupTable.inc b/maintenance/cleanupTable.inc index f6259e95c8..84e3aee0fc 100644 --- a/maintenance/cleanupTable.inc +++ b/maintenance/cleanupTable.inc @@ -49,11 +49,12 @@ class TableCleanup extends Maintenance { public function execute() { global $wgUser; - $wgUser = User::newFromName( 'Conversion script' ); $this->dryrun = $this->hasOption( 'dry-run' ); if ( $this->dryrun ) { + $wgUser = User::newFromName( 'Conversion script' ); $this->output( "Checking for bad titles...\n" ); } else { + $wgUser = User::newSystemUser( 'Conversion script', array( 'steal' => true ) ); $this->output( "Checking and fixing bad titles...\n" ); } $this->runTable( $this->defaultParams ); diff --git a/maintenance/deleteBatch.php b/maintenance/deleteBatch.php index e6321e1ffb..a3b1561a69 100644 --- a/maintenance/deleteBatch.php +++ b/maintenance/deleteBatch.php @@ -55,11 +55,15 @@ class DeleteBatch extends Maintenance { chdir( $oldCwd ); # Options processing - $username = $this->getOption( 'u', 'Delete page script' ); + $username = $this->getOption( 'u', false ); $reason = $this->getOption( 'r', '' ); $interval = $this->getOption( 'i', 0 ); - $user = User::newFromName( $username ); + if ( $username === false ) { + $user = User::newSystemUser( 'Delete page script', array( 'steal' => true ) ); + } else { + $user = User::newFromName( $username ); + } if ( !$user ) { $this->error( "Invalid username", true ); } diff --git a/maintenance/deleteEqualMessages.php b/maintenance/deleteEqualMessages.php index 478e0d70c0..de5c5e23a4 100644 --- a/maintenance/deleteEqualMessages.php +++ b/maintenance/deleteEqualMessages.php @@ -162,7 +162,7 @@ class DeleteEqualMessages extends Maintenance { return; } - $user = User::newFromName( 'MediaWiki default' ); + $user = User::newSystemUser( 'MediaWiki default', array( 'steal' => true ) ); if ( !$user ) { $this->error( "Invalid username", true ); } diff --git a/maintenance/edit.php b/maintenance/edit.php index 75ec12bfdd..b67a957162 100644 --- a/maintenance/edit.php +++ b/maintenance/edit.php @@ -46,14 +46,18 @@ class EditCLI extends Maintenance { public function execute() { global $wgUser; - $userName = $this->getOption( 'user', 'Maintenance script' ); + $userName = $this->getOption( 'user', false ); $summary = $this->getOption( 'summary', '' ); $minor = $this->hasOption( 'minor' ); $bot = $this->hasOption( 'bot' ); $autoSummary = $this->hasOption( 'autosummary' ); $noRC = $this->hasOption( 'no-rc' ); - $wgUser = User::newFromName( $userName ); + if ( $userName === false ) { + $wgUser = User::newSystemUser( 'Maintenance script', array( 'steal' => true ) ); + } else { + $wgUser = User::newFromName( $userName ); + } if ( !$wgUser ) { $this->error( "Invalid username", true ); } diff --git a/maintenance/importImages.php b/maintenance/importImages.php index ad385e57bc..a0402483ac 100644 --- a/maintenance/importImages.php +++ b/maintenance/importImages.php @@ -70,9 +70,9 @@ $files = findFiles( $dir, $extensions, isset( $options['search-recursively'] ) ) # Initialise the user for this operation $user = isset( $options['user'] ) ? User::newFromName( $options['user'] ) - : User::newFromName( 'Maintenance script' ); + : User::newSystemUser( 'Maintenance script', array( 'steal' => true ) ); if ( !$user instanceof User ) { - $user = User::newFromName( 'Maintenance script' ); + $user = User::newSystemUser( 'Maintenance script', array( 'steal' => true ) ); } $wgUser = $user; diff --git a/maintenance/importSiteScripts.php b/maintenance/importSiteScripts.php index 6566a60d97..5dfd2a8795 100644 --- a/maintenance/importSiteScripts.php +++ b/maintenance/importSiteScripts.php @@ -41,7 +41,12 @@ class ImportSiteScripts extends Maintenance { public function execute() { global $wgUser; - $user = User::newFromName( $this->getOption( 'username', 'ScriptImporter' ) ); + $username = $this->getOption( 'username', false ); + if ( $username === false ) { + $user = User::newSystemUser( 'ScriptImporter', array( 'steal' => true ) ); + } else { + $user = User::newFromName( $username ); + } $wgUser = $user; $baseUrl = $this->getArg( 1 ); diff --git a/maintenance/moveBatch.php b/maintenance/moveBatch.php index a27a77262c..58499082b8 100644 --- a/maintenance/moveBatch.php +++ b/maintenance/moveBatch.php @@ -61,7 +61,7 @@ class MoveBatch extends Maintenance { chdir( $oldCwd ); # Options processing - $user = $this->getOption( 'u', 'Move page script' ); + $user = $this->getOption( 'u', false ); $reason = $this->getOption( 'r', '' ); $interval = $this->getOption( 'i', 0 ); $noredirects = $this->getOption( 'noredirects', false ); @@ -75,7 +75,11 @@ class MoveBatch extends Maintenance { if ( !$file ) { $this->error( "Unable to read file, exiting", true ); } - $wgUser = User::newFromName( $user ); + if ( $user === false ) { + $wgUser = User::newSystemUser( 'Move page script', array( 'steal' => true ) ); + } else { + $wgUser = User::newFromName( $user ); + } if ( !$wgUser ) { $this->error( "Invalid username", true ); } diff --git a/maintenance/protect.php b/maintenance/protect.php index ec03f9349d..449a7ad6f0 100644 --- a/maintenance/protect.php +++ b/maintenance/protect.php @@ -41,7 +41,7 @@ class Protect extends Maintenance { } public function execute() { - $userName = $this->getOption( 'u', 'Maintenance script' ); + $userName = $this->getOption( 'u', false ); $reason = $this->getOption( 'r', '' ); $cascade = $this->hasOption( 'cascade' ); @@ -53,7 +53,11 @@ class Protect extends Maintenance { $protection = ""; } - $user = User::newFromName( $userName ); + if ( $userName === false ) { + $user = User::newSystemUser( 'Maintenance script', array( 'steal' => true ) ); + } else { + $user = User::newFromName( $userName ); + } if ( !$user ) { $this->error( "Invalid username", true ); } diff --git a/maintenance/rollbackEdits.php b/maintenance/rollbackEdits.php index 967dda85ce..7be5a1f990 100644 --- a/maintenance/rollbackEdits.php +++ b/maintenance/rollbackEdits.php @@ -76,7 +76,7 @@ class RollbackEdits extends Maintenance { return; } - $doer = User::newFromName( 'Maintenance script' ); + $doer = User::newSystemUser( 'Maintenance script', array( 'steal' => true ) ); foreach ( $titles as $t ) { $page = WikiPage::factory( $t ); diff --git a/maintenance/tables.sql b/maintenance/tables.sql index 4b58b60fbf..638b99d6ff 100644 --- a/maintenance/tables.sql +++ b/maintenance/tables.sql @@ -130,8 +130,7 @@ CREATE TABLE /*_*/user ( -- user_editcount int, - -- Expiration date for user password. Use $user->expirePassword() - -- to force a password reset. + -- Expiration date for user password. user_password_expires varbinary(14) DEFAULT NULL ) /*$wgDBTableOptions*/; diff --git a/maintenance/undelete.php b/maintenance/undelete.php index adebd277aa..71b4de1e40 100644 --- a/maintenance/undelete.php +++ b/maintenance/undelete.php @@ -35,7 +35,7 @@ class Undelete extends Maintenance { public function execute() { global $wgUser; - $user = $this->getOption( 'user', 'Command line script' ); + $user = $this->getOption( 'user', false ); $reason = $this->getOption( 'reason', '' ); $pageName = $this->getArg(); @@ -43,7 +43,11 @@ class Undelete extends Maintenance { if ( !$title ) { $this->error( "Invalid title", true ); } - $wgUser = User::newFromName( $user ); + if ( $user === false ) { + $wgUser = User::newSystemUser( 'Command line script', array( 'steal' => true ) ); + } else { + $wgUser = User::newFromName( $user ); + } if ( !$wgUser ) { $this->error( "Invalid username", true ); } diff --git a/tests/phpunit/MediaWikiTestCase.php b/tests/phpunit/MediaWikiTestCase.php index 9bbbf9f985..90d40a5ddd 100644 --- a/tests/phpunit/MediaWikiTestCase.php +++ b/tests/phpunit/MediaWikiTestCase.php @@ -493,13 +493,13 @@ abstract class MediaWikiTestCase extends PHPUnit_Framework_TestCase { if ( $user->idForName() == 0 ) { $user->addToDatabase(); - $user->setPassword( 'UTSysopPassword' ); - - $user->addGroup( 'sysop' ); - $user->addGroup( 'bureaucrat' ); - $user->saveSettings(); + TestUser::setPasswordForUser( $user, 'UTSysopPassword' ); } + // Always set groups, because $this->resetDB() wipes them out + $user->addGroup( 'sysop' ); + $user->addGroup( 'bureaucrat' ); + // Make 1 page with 1 revision $page = WikiPage::factory( Title::newFromText( 'UTPage' ) ); if ( $page->getId() == 0 ) { diff --git a/tests/phpunit/includes/BlockTest.php b/tests/phpunit/includes/BlockTest.php index 7b0de8667b..e69fa200c6 100644 --- a/tests/phpunit/includes/BlockTest.php +++ b/tests/phpunit/includes/BlockTest.php @@ -26,7 +26,7 @@ class BlockTest extends MediaWikiLangTestCase { $user = User::newFromName( 'UTBlockee' ); if ( $user->getID() == 0 ) { $user->addToDatabase(); - $user->setPassword( 'UTBlockeePassword' ); + TestUser::setPasswordForUser( $user, 'UTBlockeePassword' ); $user->saveSettings(); } @@ -136,9 +136,9 @@ class BlockTest extends MediaWikiLangTestCase { public function testBlockedUserCanNotCreateAccount() { $username = 'BlockedUserToCreateAccountWith'; $u = User::newFromName( $username ); - $u->setPassword( 'NotRandomPass' ); $u->setId( 14146 ); $u->addToDatabase(); + TestUser::setPasswordForUser( $u, 'NotRandomPass' ); unset( $u ); // Sanity check @@ -374,8 +374,8 @@ class BlockTest extends MediaWikiLangTestCase { # Set up the target $u = User::newFromName( $username ); if ( $u->getID() == 0 ) { - $u->setPassword( 'TotallyObvious' ); $u->addToDatabase(); + TestUser::setPasswordForUser( $u, 'TotallyObvious' ); } unset( $u ); diff --git a/tests/phpunit/includes/TestUser.php b/tests/phpunit/includes/TestUser.php index 96b2251d3c..6887ed175f 100644 --- a/tests/phpunit/includes/TestUser.php +++ b/tests/phpunit/includes/TestUser.php @@ -65,8 +65,8 @@ class TestUser { } // Update the user to use the password and other details - $change = $this->setPassword( $this->password ) || - $this->setEmail( $email ) || + $this->setPassword( $this->password ); + $change = $this->setEmail( $email ) || $this->setRealName( $realname ); // Adjust groups by adding any missing ones and removing any extras @@ -110,26 +110,36 @@ class TestUser { /** * @param string $password - * @return bool */ private function setPassword( $password ) { - $passwordFactory = $this->user->getPasswordFactory(); - $oldDefaultType = $passwordFactory->getDefaultType(); - - // A is unsalted MD5 (thus fast) ... we don't care about security here, this is test only - $passwordFactory->setDefaultType( 'A' ); - $newPassword = $passwordFactory->newFromPlaintext( $password, $this->user->getPassword() ); + self::setPasswordForUser( $this->user, $password ); + } - $change = false; - if ( !$this->user->getPassword()->equals( $newPassword ) ) { - // Password changed - $this->user->setPassword( $password ); - $change = true; + /** + * Set the password on a testing user + * + * This assumes we're still using the generic AuthManager config from + * PHPUnitMaintClass::finalSetup(), and just sets the password in the + * database directly. + * @param User $user + * @param string $password + */ + public static function setPasswordForUser( User $user, $password ) { + if ( !$user->getId() ) { + throw new MWException( "Passed User has not been added to the database yet!" ); } - $passwordFactory->setDefaultType( $oldDefaultType ); - - return $change; + $passwordFactory = new PasswordFactory(); + $passwordFactory->init( RequestContext::getMain()->getConfig() ); + // A is unsalted MD5 (thus fast) ... we don't care about security here, this is test only + $passwordFactory->setDefaultType( 'A' ); + $pwhash = $passwordFactory->newFromPlaintext( $password ); + wfGetDB( DB_MASTER )->update( + 'user', + array( 'user_password' => $pwhash->toString() ), + array( 'user_id' => $user->getId() ), + __METHOD__ + ); } /** diff --git a/tests/phpunit/includes/UserTest.php b/tests/phpunit/includes/UserTest.php index 17c1b8e05e..4c6f083bb7 100644 --- a/tests/phpunit/includes/UserTest.php +++ b/tests/phpunit/includes/UserTest.php @@ -276,26 +276,6 @@ class UserTest extends MediaWikiTestCase { $this->assertEquals( 'test', $this->user->getOption( 'userjs-someoption' ) ); } - /** - * Test password expiration. - * @covers User::getPasswordExpired() - */ - public function testPasswordExpire() { - $this->setMwGlobals( 'wgPasswordExpireGrace', 3600 * 24 * 7 ); // 7 days - - $user = User::newFromName( 'UnitTestUser' ); - $user->loadDefaults( 'UnitTestUser' ); - $this->assertEquals( false, $user->getPasswordExpired() ); - - $ts = time() - ( 3600 * 24 * 1 ); // 1 day ago - $user->expirePassword( $ts ); - $this->assertEquals( 'soft', $user->getPasswordExpired() ); - - $ts = time() - ( 3600 * 24 * 10 ); // 10 days ago - $user->expirePassword( $ts ); - $this->assertEquals( 'hard', $user->getPasswordExpired() ); - } - /** * Test password validity checks. There are 3 checks in core, * - ensure the password meets the minimal length diff --git a/tests/phpunit/includes/api/ApiBlockTest.php b/tests/phpunit/includes/api/ApiBlockTest.php index e0488b78b1..ffd2f5ed75 100644 --- a/tests/phpunit/includes/api/ApiBlockTest.php +++ b/tests/phpunit/includes/api/ApiBlockTest.php @@ -22,7 +22,7 @@ class ApiBlockTest extends ApiTestCase { if ( $user->getId() == 0 ) { $user->addToDatabase(); - $user->setPassword( 'UTApiBlockeePassword' ); + TestUser::setPasswordForUser( $user, 'UTApiBlockeePassword' ); $user->saveSettings(); } diff --git a/tests/phpunit/includes/api/ApiCreateAccountTest.php b/tests/phpunit/includes/api/ApiCreateAccountTest.php index 8d134f7635..3945102bb7 100644 --- a/tests/phpunit/includes/api/ApiCreateAccountTest.php +++ b/tests/phpunit/includes/api/ApiCreateAccountTest.php @@ -29,7 +29,7 @@ class ApiCreateAccountTest extends ApiTestCase { $this->markTestIncomplete( 'This test needs $wgServer to be set in LocalSettings.php' ); } - $password = User::randomPassword(); + $password = PasswordFactory::generateRandomPasswordString(); $ret = $this->doApiRequest( array( 'action' => 'createaccount', diff --git a/tests/phpunit/includes/api/UserWrapper.php b/tests/phpunit/includes/api/UserWrapper.php index f8da0ff4e3..d187feda63 100644 --- a/tests/phpunit/includes/api/UserWrapper.php +++ b/tests/phpunit/includes/api/UserWrapper.php @@ -15,7 +15,7 @@ class UserWrapper { "email" => "test@example.com", "real_name" => "Test User" ) ); } - $this->user->setPassword( $this->password ); + TestUser::setPasswordForUser( $this->user, $this->password ); if ( $group !== '' ) { $this->user->addGroup( $group ); diff --git a/tests/phpunit/includes/cache/GenderCacheTest.php b/tests/phpunit/includes/cache/GenderCacheTest.php index 6b22000116..d931b3901d 100644 --- a/tests/phpunit/includes/cache/GenderCacheTest.php +++ b/tests/phpunit/includes/cache/GenderCacheTest.php @@ -13,7 +13,7 @@ class GenderCacheTest extends MediaWikiLangTestCase { $user = User::newFromName( 'UTMale' ); if ( $user->getID() == 0 ) { $user->addToDatabase(); - $user->setPassword( 'UTMalePassword' ); + TestUser::setPasswordForUser( $user, 'UTMalePassword' ); } // ensure the right gender $user->setOption( 'gender', 'male' ); @@ -22,7 +22,7 @@ class GenderCacheTest extends MediaWikiLangTestCase { $user = User::newFromName( 'UTFemale' ); if ( $user->getID() == 0 ) { $user->addToDatabase(); - $user->setPassword( 'UTFemalePassword' ); + TestUser::setPasswordForUser( $user, 'UTFemalePassword' ); } // ensure the right gender $user->setOption( 'gender', 'female' ); @@ -31,7 +31,7 @@ class GenderCacheTest extends MediaWikiLangTestCase { $user = User::newFromName( 'UTDefaultGender' ); if ( $user->getID() == 0 ) { $user->addToDatabase(); - $user->setPassword( 'UTDefaultGenderPassword' ); + TestUser::setPasswordForUser( $user, 'UTDefaultGenderPassword' ); } // ensure the default gender $user->setOption( 'gender', null ); diff --git a/tests/phpunit/includes/password/PasswordTest.php b/tests/phpunit/includes/password/PasswordTest.php index 5ad8aca676..e8a4d5c686 100644 --- a/tests/phpunit/includes/password/PasswordTest.php +++ b/tests/phpunit/includes/password/PasswordTest.php @@ -25,14 +25,16 @@ class PasswordTest extends MediaWikiTestCase { * @covers InvalidPassword::equals */ public function testInvalidUnequalInvalid() { - $invalid1 = User::getPasswordFactory()->newFromCiphertext( null ); - $invalid2 = User::getPasswordFactory()->newFromCiphertext( null ); + $passwordFactory = new PasswordFactory(); + $invalid1 = $passwordFactory->newFromCiphertext( null ); + $invalid2 = $passwordFactory->newFromCiphertext( null ); $this->assertFalse( $invalid1->equals( $invalid2 ) ); } public function testInvalidPlaintext() { - $invalid = User::getPasswordFactory()->newFromPlaintext( null ); + $passwordFactory = new PasswordFactory(); + $invalid = $passwordFactory->newFromPlaintext( null ); $this->assertInstanceOf( 'InvalidPassword', $invalid ); }