From: Brion Vibber Date: Tue, 12 Dec 2006 04:15:00 +0000 (+0000) Subject: * Change behavior of logins using the temporary e-mailed password (as stored X-Git-Tag: 1.31.0-rc.0~54928 X-Git-Url: http://git.cyclocoop.org/%22%20.%20generer_url_aide%28?a=commitdiff_plain;h=1c4daa97242e6068b00411c73ebbde7aff3ae39d;p=lhc%2Fweb%2Fwiklou.git * Change behavior of logins using the temporary e-mailed password (as stored in user_newpassword hash field). Instead of just logging in silently and leaving the previous user_password field in place indefinitely, the user is now prompted to set a new password. The password-changing form is at Special:Resetpass; currently it's only usable for changing from the temporary password during login, but it could perhaps be generalized, replacing the subform in preferences. Once the new password is set successfully, the temporary password is wiped so it cannot be used to login a second time, and the login process is completed. * Suppress 'mail new password' button on login form if $wgAuth forbids changing user passwords; it wouldn't work very well... * Consolidate password length checks and $wgAuth manipulation into User::setPassword() to avoid duplicate code in different places that set passwords. * User::setPassword() now throws PasswordError exceptions if the password is illegal or cannot be set via $wgAuth. These can be caught and a human- readable error message displayed by UI code. --- diff --git a/RELEASE-NOTES b/RELEASE-NOTES index fb167e0f85..e187f42daa 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -272,6 +272,27 @@ it from source control: http://www.mediawiki.org/wiki/Download_from_SVN * (bug 8121) wfRandom() was not between 0 and 1 * Add static method Parser::createAssocArgs($args), so parser functions can use the same code to parse arguments as the templates do. +* Change behavior of logins using the temporary e-mailed password (as stored + in user_newpassword hash field). Instead of just logging in silently and + leaving the previous user_password field in place indefinitely, the user + is now prompted to set a new password. + + The password-changing form is at Special:Resetpass; currently it's only + usable for changing from the temporary password during login, but it + could perhaps be generalized, replacing the subform in preferences. + + Once the new password is set successfully, the temporary password is wiped + so it cannot be used to login a second time, and the login process + is completed. +* Suppress 'mail new password' button on login form if $wgAuth forbids + changing user passwords; it wouldn't work very well... +* Consolidate password length checks and $wgAuth manipulation into + User::setPassword() to avoid duplicate code in different places + that set passwords. +* User::setPassword() now throws PasswordError exceptions if the password + is illegal or cannot be set via $wgAuth. These can be caught and a human- + readable error message displayed by UI code. + == Languages updated == diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php index 8b0fb31b7c..041df096c4 100644 --- a/includes/AutoLoader.php +++ b/includes/AutoLoader.php @@ -241,6 +241,7 @@ function __autoload($className) { 'UsercreateTemplate' => 'includes/templates/Userlogin.php', 'UserloginTemplate' => 'includes/templates/Userlogin.php', 'Language' => 'languages/Language.php', + 'PasswordResetForm' => 'includes/SpecialResetpass.php', // API classes 'ApiBase' => 'includes/api/ApiBase.php', diff --git a/includes/SpecialPage.php b/includes/SpecialPage.php index e8db18b742..5b892fda7a 100644 --- a/includes/SpecialPage.php +++ b/includes/SpecialPage.php @@ -119,6 +119,7 @@ class SpecialPage 'Recentchangeslinked' => array( 'UnlistedSpecialPage', 'Recentchangeslinked' ), 'Movepage' => array( 'UnlistedSpecialPage', 'Movepage' ), 'Blockme' => array( 'UnlistedSpecialPage', 'Blockme' ), + 'Resetpass' => array( 'UnlistedSpecialPage', 'Resetpass' ), 'Booksources' => array( 'SpecialPage', 'Booksources' ), 'Categories' => array( 'SpecialPage', 'Categories' ), 'Export' => array( 'SpecialPage', 'Export' ), diff --git a/includes/SpecialPreferences.php b/includes/SpecialPreferences.php index 29614a8089..a622cb890e 100644 --- a/includes/SpecialPreferences.php +++ b/includes/SpecialPreferences.php @@ -216,22 +216,18 @@ class PreferencesForm { return; } - if ( strlen( $this->mNewpass ) < $wgMinimalPasswordLength ) { - $this->mainPrefsForm( 'error', wfMsg( 'passwordtooshort', $wgMinimalPasswordLength ) ); - return; - } - if (!$wgUser->checkPassword( $this->mOldpass )) { $this->mainPrefsForm( 'error', wfMsg( 'wrongpassword' ) ); return; } - if (!$wgAuth->setPassword( $wgUser, $this->mNewpass )) { - $this->mainPrefsForm( 'error', wfMsg( 'externaldberror' ) ); + + try { + $wgUser->setPassword( $this->mNewpass ); + $this->mNewpass = $this->mOldpass = $this->mRetypePass = ''; + } catch( PasswordError $e ) { + $this->mainPrefsForm( 'error', $e->getMessage() ); return; } - $wgUser->setPassword( $this->mNewpass ); - $this->mNewpass = $this->mOldpass = $this->mRetypePass = ''; - } $wgUser->setRealName( $this->mRealName ); diff --git a/includes/SpecialResetpass.php b/includes/SpecialResetpass.php new file mode 100644 index 0000000000..cde582b1fe --- /dev/null +++ b/includes/SpecialResetpass.php @@ -0,0 +1,158 @@ +execute( $par ); +} + +class PasswordResetForm extends SpecialPage { + function __construct( $name=null, $reset=null ) { + if( $name !== null ) { + $this->mName = $name; + $this->mTemporaryPassword = $reset; + } else { + global $wgRequest; + $this->mName = $wgRequest->getVal( 'wpName' ); + $this->mTemporaryPassword = $wgRequest->getVal( 'wpPassword' ); + } + } + + /** + * Main execution point + */ + function execute( $par='' ) { + global $wgUser, $wgAuth, $wgOut, $wgRequest; + + if( !$wgAuth->allowPasswordChange() ) { + $this->error( wfMsg( 'resetpass_forbidden' ) ); + return; + } + + if( $this->mName === null && !$wgRequest->wasPosted() ) { + $this->error( wfMsg( 'resetpass_missing' ) ); + return; + } + + if( $wgRequest->wasPosted() && $wgUser->matchEditToken( $wgRequest->getVal( 'token' ) ) ) { + $newpass = $wgRequest->getVal( 'wpNewPassword' ); + $retype = $wgRequest->getVal( 'wpRetype' ); + try { + $this->attemptReset( $newpass, $retype ); + $wgOut->addWikiText( wfMsg( 'resetpass_success' ) ); + + $data = array( + 'action' => 'submitlogin', + 'wpName' => $this->mName, + 'wpPassword' => $newpass, + 'returnto' => $wgRequest->getVal( 'returnto' ), + ); + if( $wgRequest->getCheck( 'wpRemember' ) ) { + $data['wpRemember'] = 1; + } + $login = new LoginForm( new FauxRequest( $data, true ) ); + $login->execute(); + + return; + } catch( PasswordError $e ) { + $this->error( $e->getMessage() ); + } + } + $this->showForm(); + } + + function error( $msg ) { + global $wgOut; + $wgOut->addHtml( '
' . + htmlspecialchars( $msg ) . + '
' ); + } + + function showForm() { + global $wgOut, $wgUser, $wgLang, $wgRequest; + + $self = SpecialPage::getTitleFor( 'Resetpass' ); + $form = + '
' . + wfOpenElement( 'form', + array( + 'method' => 'post', + 'action' => $self->getLocalUrl() ) ) . + '

' . wfMsgHtml( 'resetpass_header' ) . '

' . + '
' . + wfMsgExt( 'resetpass_text', array( 'parse' ) ) . + '
' . + '' . + wfHidden( 'token', $wgUser->editToken() ) . + wfHidden( 'wpName', $this->mName ) . + wfHidden( 'wpPassword', $this->mTemporaryPassword ) . + wfHidden( 'returnto', $wgRequest->getVal( 'returnto' ) ) . + $this->pretty( array( + array( 'wpName', 'username', 'text', $this->mName ), + array( 'wpNewPassword', 'newpassword', 'password', '' ), + array( 'wpRetype', 'yourpasswordagain', 'password', '' ), + ) ) . + '' . + '' . + '' . + '' . + '' . + '' . + '' . + '' . + '
' . + Xml::checkLabel( wfMsg( 'remembermypassword' ), + 'wpRemember', 'wpRemember', + $wgRequest->getCheck( 'wpRemember' ) ) . + '
' . + wfSubmitButton( wfMsgHtml( 'resetpass_submit' ) ) . + '
' . + wfCloseElement( 'form' ) . + '
'; + $wgOut->addHtml( $form ); + } + + function pretty( $fields ) { + $out = ''; + foreach( $fields as $list ) { + list( $name, $label, $type, $value ) = $list; + if( $type == 'text' ) { + $field = '' . htmlspecialchars( $value ) . ''; + } else { + $field = Xml::input( $name, 20, $value, + array( 'id' => $name, 'type' => $type ) ); + } + $out .= ''; + $out .= ''; + $out .= Xml::label( wfMsg( $label ), $name ); + $out .= ''; + $out .= ''; + $out .= $field; + $out .= ''; + $out .= ''; + } + return $out; + } + + /** + * @throws PasswordError + */ + function attemptReset( $newpass, $retype ) { + $user = User::newFromName( $this->mName ); + if( $user->isAnon() ) { + throw new PasswordError( 'no such user' ); + } + + if( !$user->checkTemporaryPassword( $this->mTemporaryPassword ) ) { + throw new PasswordError( wfMsg( 'resetpass_bad_temporary' ) ); + } + + if( $newpass !== $retype ) { + throw new PasswordError( wfMsg( 'badretype' ) ); + } + + $user->setPassword( $newpass ); + $user->saveSettings(); + } +} + +?> diff --git a/includes/SpecialUserlogin.php b/includes/SpecialUserlogin.php index d43d249c25..3288d80faf 100644 --- a/includes/SpecialUserlogin.php +++ b/includes/SpecialUserlogin.php @@ -34,6 +34,7 @@ class LoginForm { const NOT_EXISTS = 4; const WRONG_PASS = 5; const EMPTY_PASS = 6; + const RESET_PASS = 7; var $mName, $mPassword, $mRetype, $mReturnTo, $mCookieCheck, $mPosted; var $mAction, $mCreateaccount, $mCreateaccountMail, $mMailmypassword; @@ -351,7 +352,29 @@ class LoginForm { } if (!$u->checkPassword( $this->mPassword )) { - return '' == $this->mPassword ? self::EMPTY_PASS : self::WRONG_PASS; + if( $u->checkTemporaryPassword( $this->mPassword ) ) { + // The e-mailed temporary password should not be used + // for actual logins; that's a very sloppy habit, + // and insecure if an attacker has a few seconds to + // click "search" on someone's open mail reader. + // + // Allow it to be used only to reset the password + // a single time to a new value, which won't be in + // the user's e-mail archives. + // + // For backwards compatibility, we'll still recognize + // it at the login form to minimize surprises for + // people who have been logging in with a temporary + // password for some time. + // + // At this point we just return an appropriate code + // indicating that the UI should show a password + // reset form; bot interfaces etc will probably just + // fail cleanly here. + return self::RESET_PASS; + } else { + return '' == $this->mPassword ? self::EMPTY_PASS : self::WRONG_PASS; + } } else { $wgAuth->updateUser( $u ); $wgUser = $u; @@ -398,16 +421,31 @@ class LoginForm { case self::EMPTY_PASS: $this->mainLoginForm( wfMsg( 'wrongpasswordempty' ) ); break; + case self::RESET_PASS: + $this->resetLoginForm( wfMsg( 'resetpass_announce' ) ); + break; default: wfDebugDieBacktrace( "Unhandled case value" ); } } + + function resetLoginForm( $error ) { + global $wgOut; + $wgOut->addWikiText( "
$error
" ); + $reset = new PasswordResetForm( $this->mName, $this->mPassword ); + $reset->execute(); + } /** * @private */ function mailPassword() { - global $wgUser, $wgOut; + global $wgUser, $wgOut, $wgAuth; + + if( !$wgAuth->allowPasswordChange() ) { + $this->mainLoginForm( wfMsg( 'resetpass_forbidden' ) ); + return; + } # Check against blocked IPs # fixme -- should we not? @@ -547,6 +585,7 @@ class LoginForm { function mainLoginForm( $msg, $msgtype = 'error' ) { global $wgUser, $wgOut, $wgAllowRealName, $wgEnableEmail; global $wgCookiePrefix, $wgAuth, $wgLoginLanguageSelector; + global $wgAuth; if ( $this->mType == 'signup' ) { if ( !$wgUser->isAllowed( 'createaccount' ) ) { @@ -614,6 +653,7 @@ class LoginForm { $template->set( 'createemail', $wgEnableEmail && $wgUser->isLoggedIn() ); $template->set( 'userealname', $wgAllowRealName ); $template->set( 'useemail', $wgEnableEmail ); + $template->set( 'canreset', $wgAuth->allowPasswordChange() ); $template->set( 'remember', $wgUser->getOption( 'rememberpassword' ) or $this->mRemember ); # Prepare language selection links as needed diff --git a/includes/User.php b/includes/User.php index 92381e767e..53e651b02c 100644 --- a/includes/User.php +++ b/includes/User.php @@ -16,6 +16,13 @@ define( 'MW_USER_VERSION', 4 ); # places, so we can't safely include ' or " even though we really should. define( 'EDIT_TOKEN_SUFFIX', '\\' ); +/** + * Thrown by User::setPassword() on error + */ +class PasswordError extends MWException { + // NOP +} + /** * * @package MediaWiki @@ -1290,14 +1297,39 @@ class User { } /** - * Set the password and reset the random token + * Set the password and reset the random token + * Calls through to authentication plugin if necessary; + * will have no effect if the auth plugin refuses to + * pass the change through or if the legal password + * checks fail. + * + * @param string $str + * @throws PasswordError on failure */ function setPassword( $str ) { + global $wgAuth, $wgMinimalPasswordLength; + + if( !$wgAuth->allowPasswordChange() ) { + throw new PasswordError( wfMsg( 'password-change-forbidden' ) ); + } + + if( $wgMinimalPasswordLength && + strlen( $str ) < $wgMinimalPasswordLength ) { + throw new PasswordError( wfMsg( 'passwordtooshort', + $wgMinimalPasswordLength ) ); + } + + if( !$wgAuth->setPassword( $this, $str ) ) { + throw new PasswordError( wfMsg( 'externaldberror' ) ); + } + $this->load(); $this->setToken(); $this->mPassword = $this->encryptPassword( $str ); $this->mNewpassword = ''; $this->mNewpassTime = NULL; + + return true; } /** @@ -2064,8 +2096,6 @@ class User { $ep = $this->encryptPassword( $password ); if ( 0 == strcmp( $ep, $this->mPassword ) ) { return true; - } elseif ( ($this->mNewpassword != '') && (0 == strcmp( $ep, $this->mNewpassword )) ) { - return true; } elseif ( function_exists( 'iconv' ) ) { # Some wikis were converted from ISO 8859-1 to UTF-8, the passwords can't be converted # Check for this with iconv @@ -2076,6 +2106,16 @@ class User { } return false; } + + /** + * Check if the given clear-text password matches the temporary password + * sent by e-mail for password reset operations. + * @return bool + */ + function checkTemporaryPassword( $plaintext ) { + $hash = $this->encryptPassword( $plaintext ); + return $hash === $this->mNewpassword; + } /** * Initialize (if necessary) and return a session token value diff --git a/includes/templates/Userlogin.php b/includes/templates/Userlogin.php index 83ef4920f6..953fbd4713 100644 --- a/includes/templates/Userlogin.php +++ b/includes/templates/Userlogin.php @@ -78,7 +78,7 @@ class UserloginTemplate extends QuickTemplate { -  data['useemail'] ) { ?> data['useemail'] && $this->data['canreset']) { ?> diff --git a/languages/messages/MessagesEn.php b/languages/messages/MessagesEn.php index eac35cd45a..e5fb2a1b8b 100644 --- a/languages/messages/MessagesEn.php +++ b/languages/messages/MessagesEn.php @@ -866,6 +866,18 @@ format. Please enter a well-formatted address or empty that field.', 'accountcreated' => 'Account created', 'accountcreatedtext' => 'The user account for $1 has been created.', +# Password reset dialog +'resetpass' => 'Reset account password', +'resetpass_announce' => 'Login with temporary e-mailed code. To finish logging in, you must set a new password.', +'resetpass_text' => "", +'resetpass_header' => 'Reset password', +'resetpass_submit' => 'Set password and log in', +'resetpass_success' => 'Your password has been changed successfully! Now logging you in...', +'resetpass_bad_temporary' => 'Invalid temporary password. You may have already successfully changed your password or requested a new temporary password.', +'resetpass_forbidden' => 'Passwords cannot be changed on this wiki', +'resetpass_missing' => 'No form data.', + + # Edit page toolbar 'bold_sample'=>'Bold text', 'bold_tip'=>'Bold text',