From 74b15fc8c457889d4b86bb4c06ddf8dae90db433 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Sat, 23 Jul 2011 00:48:39 +0000 Subject: [PATCH] * Moved email changing from sp:Preferences to new sp:ChangeEmail, which requires confirming the user password. This reduces the impact of session hijacking, which was increased slightly with r86482. Changing a password already required confirming the old one. This change closes the loophole of changing the email address and then doing a reset. * Parse 'mailerror' message correctly --- includes/AutoLoader.php | 1 + includes/Preferences.php | 87 ++++++---- includes/SpecialPageFactory.php | 7 +- includes/specials/SpecialChangeEmail.php | 206 +++++++++++++++++++++++ includes/specials/SpecialPreferences.php | 5 - languages/messages/MessagesEn.php | 14 ++ maintenance/language/messages.inc | 13 ++ 7 files changed, 293 insertions(+), 40 deletions(-) create mode 100644 includes/specials/SpecialChangeEmail.php diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php index af97abf693..9ebd53afe9 100644 --- a/includes/AutoLoader.php +++ b/includes/AutoLoader.php @@ -743,6 +743,7 @@ $wgAutoloadLocalClasses = array( 'SpecialBlockme' => 'includes/specials/SpecialBlockme.php', 'SpecialBookSources' => 'includes/specials/SpecialBooksources.php', 'SpecialCategories' => 'includes/specials/SpecialCategories.php', + 'SpecialChangeEmail' => 'includes/specials/SpecialChangeEmail.php', 'SpecialChangePassword' => 'includes/specials/SpecialChangePassword.php', 'SpecialComparePages' => 'includes/specials/SpecialComparePages.php', 'SpecialContributions' => 'includes/specials/SpecialContributions.php', diff --git a/includes/Preferences.php b/includes/Preferences.php index e36117a3ee..62b99a4e63 100644 --- a/includes/Preferences.php +++ b/includes/Preferences.php @@ -353,13 +353,20 @@ class Preferences { $helpMessages[] = 'prefs-help-email-others'; } + $link = $wgUser->getSkin()->link( + SpecialPage::getTitleFor( 'ChangeEmail' ), + wfMsgHtml( $user->getEmail() ? 'prefs-changeemail' : 'prefs-setemail' ), + array(), + array( 'returnto' => SpecialPage::getTitleFor( 'Preferences' ) ) ); + $defaultPreferences['emailaddress'] = array( - 'type' => $wgAuth->allowPropChange( 'emailaddress' ) ? 'email' : 'info', - 'default' => $user->getEmail(), - 'section' => 'personal/email', + 'type' => 'info', + 'raw' => true, + 'default' => $user->getEmail() + ? htmlspecialchars( $user->getEmail() ) . " ($link)" + : $link, 'label-message' => 'youremail', - 'help-messages' => $helpMessages, - 'validation-callback' => array( 'Preferences', 'validateEmail' ), + 'section' => 'personal/email', ); global $wgEmailAuthentication; @@ -1342,7 +1349,7 @@ class Preferences { * @return bool|Status|string */ static function tryFormSubmit( $formData, $entryPoint = 'internal' ) { - global $wgUser, $wgEmailAuthentication, $wgEnableEmail; + global $wgUser; $result = true; @@ -1360,34 +1367,6 @@ class Preferences { 'emailaddress', ); - if ( $wgEnableEmail ) { - $newaddr = $formData['emailaddress']; - $oldaddr = $wgUser->getEmail(); - if ( ( $newaddr != '' ) && ( $newaddr != $oldaddr ) ) { - # the user has supplied a new email address on the login page - # new behaviour: set this new emailaddr from login-page into user database record - $wgUser->setEmail( $newaddr ); - # but flag as "dirty" = unauthenticated - $wgUser->invalidateEmail(); - if ( $wgEmailAuthentication ) { - # Mail a temporary password to the dirty address. - # User can come back through the confirmation URL to re-enable email. - $type = $oldaddr != '' ? 'changed' : 'set'; - $result = $wgUser->sendConfirmationMail( $type ); - if ( !$result->isGood() ) { - return htmlspecialchars( $result->getWikiText( 'mailerror' ) ); - } elseif ( $entryPoint == 'ui' ) { - $result = 'eauth'; - } - } - } else { - $wgUser->setEmail( $newaddr ); - } - if ( $oldaddr != $newaddr ) { - wfRunHooks( 'PrefsEmailAudit', array( $wgUser, $oldaddr, $newaddr ) ); - } - } - // Fortunately, the realname field is MUCH simpler global $wgHiddenPrefs; if ( !in_array( 'realname', $wgHiddenPrefs ) ) { @@ -1447,6 +1426,46 @@ class Preferences { return Status::newGood(); } + /* + * Try to set a user's email address. + * This does *not* try to validate the address. + * @param $user User + * @param $newaddr string New email address + * @return Array (true on success or Status on failure, info string) + */ + public static function trySetUserEmail( User $user, $newaddr ) { + global $wgEnableEmail, $wgEmailAuthentication; + $info = ''; // none + + if ( $wgEnableEmail ) { + $oldaddr = $user->getEmail(); + if ( ( $newaddr != '' ) && ( $newaddr != $oldaddr ) ) { + # The user has supplied a new email address on the login page + # new behaviour: set this new emailaddr from login-page into user database record + $user->setEmail( $newaddr ); + # But flag as "dirty" = unauthenticated + $user->invalidateEmail(); + if ( $wgEmailAuthentication ) { + # Mail a temporary password to the dirty address. + # User can come back through the confirmation URL to re-enable email. + $type = $oldaddr != '' ? 'changed' : 'set'; + $result = $user->sendConfirmationMail( $type ); + if ( !$result->isGood() ) { + return array( $result, 'mailerror' ); + } + $info = 'eauth'; + } + } else { + $user->setEmail( $newaddr ); + } + if ( $oldaddr != $newaddr ) { + wfRunHooks( 'PrefsEmailAudit', array( $user, $oldaddr, $newaddr ) ); + } + } + + return array( true, $info ); + } + /** * @param $user User * @return array diff --git a/includes/SpecialPageFactory.php b/includes/SpecialPageFactory.php index ade2558c3f..94d40e1e91 100644 --- a/includes/SpecialPageFactory.php +++ b/includes/SpecialPageFactory.php @@ -160,6 +160,7 @@ class SpecialPageFactory { static function getList() { global $wgSpecialPages; global $wgDisableCounters, $wgDisableInternalSearch, $wgEmailAuthentication; + global $wgEnableEmail; if ( !is_object( self::$mList ) ) { wfProfileIn( __METHOD__ ); @@ -177,6 +178,10 @@ class SpecialPageFactory { self::$mList['Invalidateemail'] = 'EmailInvalidation'; } + if ( $wgEnableEmail ) { + self::$mList['ChangeEmail'] = 'SpecialChangeEmail'; + } + // Add extension special pages self::$mList = array_merge( self::$mList, $wgSpecialPages ); @@ -512,7 +517,7 @@ class SpecialPageFactory { static function getLocalNameFor( $name, $subpage = false ) { global $wgContLang; $aliases = $wgContLang->getSpecialPageAliases(); - + if ( isset( $aliases[$name][0] ) ) { $name = $aliases[$name][0]; } else { diff --git a/includes/specials/SpecialChangeEmail.php b/includes/specials/SpecialChangeEmail.php new file mode 100644 index 0000000000..13f490bde3 --- /dev/null +++ b/includes/specials/SpecialChangeEmail.php @@ -0,0 +1,206 @@ +getOutput(); + if ( wfReadOnly() ) { + $out->readOnlyPage(); + return; + } + + $user = $this->getUser(); + + $this->mPassword = $wgRequest->getVal( 'wpPassword' ); + $this->mNewEmail = $wgRequest->getVal( 'wpNewEmail' ); + + $this->setHeaders(); + $this->outputHeader(); + $out->disallowUserJs(); + + if ( !$wgRequest->wasPosted() && !$user->isLoggedIn() ) { + $this->error( wfMsg( 'changeemail-no-info' ) ); + return; + } + + if ( $wgRequest->wasPosted() && $wgRequest->getBool( 'wpCancel' ) ) { + $this->doReturnTo(); + return; + } + + if ( $wgRequest->wasPosted() + && $user->matchEditToken( $wgRequest->getVal( 'token' ) ) ) + { + $info = $this->attemptChange( $user, $this->mPassword, $this->mNewEmail ); + if ( $info === true ) { + $this->doReturnTo(); + } elseif ( $info === 'eauth' ) { + # Notify user that a confirmation email has been sent... + $out->wrapWikiMsg( "
\n$1\n
", + 'eauthentsent', $user->getName() ); + $this->doReturnTo( 'soft' ); // just show the link to go back + return; // skip form + } + } + + $this->showForm(); + } + + protected function doReturnTo( $type = 'hard' ) { + global $wgRequest; + $titleObj = Title::newFromText( $wgRequest->getVal( 'returnto' ) ); + if ( !$titleObj instanceof Title ) { + $titleObj = Title::newMainPage(); + } + if ( $type == 'hard' ) { + $this->getOutput()->redirect( $titleObj->getFullURL() ); + } else { + $this->getOutput()->addReturnTo( $titleObj ); + } + } + + protected function error( $msg ) { + $this->getOutput()->addHTML( Xml::element('p', array( 'class' => 'error' ), $msg ) ); + } + + protected function showForm() { + global $wgRequest; + + $user = $this->getUser(); + + $oldEmailText = $user->getEmail() + ? $user->getEmail() + : wfMsg( 'changeemail-none' ); + + $this->getOutput()->addHTML( + Xml::fieldset( wfMsg( 'changeemail-header' ) ) . + Xml::openElement( 'form', + array( + 'method' => 'post', + 'action' => $this->getTitle()->getLocalUrl(), + 'id' => 'mw-changeemail-form' ) ) . "\n" . + Html::hidden( 'token', $user->editToken() ) . "\n" . + Html::hidden( 'returnto', $wgRequest->getVal( 'returnto' ) ) . "\n" . + wfMsgExt( 'changeemail-text', array( 'parse' ) ) . "\n" . + Xml::openElement( 'table', array( 'id' => 'mw-changeemail-table' ) ) . "\n" . + $this->pretty( array( + array( 'wpName', 'username', 'text', $user->getName() ), + array( 'wpOldEmail', 'changeemail-oldemail', 'text', $oldEmailText ), + array( 'wpNewEmail', 'changeemail-newemail', 'input', $this->mNewEmail ), + array( 'wpPassword', 'yourpassword', 'password', $this->mPassword ), + ) ) . "\n" . + "\n" . + "\n" . + '' . + Xml::submitButton( wfMsg( 'changeemail-submit' ) ) . + Xml::submitButton( wfMsg( 'changeemail-cancel' ), array( 'name' => 'wpCancel' ) ) . + "\n" . + "\n" . + Xml::closeElement( 'table' ) . + Xml::closeElement( 'form' ) . + Xml::closeElement( 'fieldset' ) . "\n" + ); + } + + protected function pretty( $fields ) { + $out = ''; + foreach ( $fields as $list ) { + list( $name, $label, $type, $value ) = $list; + if( $type == 'text' ) { + $field = htmlspecialchars( $value ); + } else { + $attribs = array( 'id' => $name ); + if ( $name == 'wpPassword' ) { + $attribs[] = 'autofocus'; + } + $field = Html::input( $name, $value, $type, $attribs ); + } + $out .= "\n"; + $out .= "\t"; + if ( $type != 'text' ) { + $out .= Xml::label( wfMsg( $label ), $name ); + } else { + $out .= wfMsgHtml( $label ); + } + $out .= "\n"; + $out .= "\t"; + $out .= $field; + $out .= "\n"; + $out .= ""; + } + return $out; + } + + /** + * @return bool|string true or string on success, false on failure + */ + protected function attemptChange( User $user, $pass, $newaddr ) { + if ( $newaddr != '' && !Sanitizer::validateEmail( $newaddr ) ) { + $this->error( wfMsgExt( 'invalidemailaddress', 'parseinline' ) ); + return false; + } + + $throttleCount = LoginForm::incLoginThrottle( $user->getName() ); + if ( $throttleCount === true ) { + $this->error( wfMsgHtml( 'login-throttled' ) ); + return false; + } + + if ( !$user->checkPassword( $pass ) ) { + $this->error( wfMsgHtml( 'wrongpassword' ) ); + return false; + } + + if ( $throttleCount ) { + LoginForm::clearLoginThrottle( $user->getName() ); + } + + list( $status, $info ) = Preferences::trySetUserEmail( $user, $newaddr ); + if ( $status !== true ) { + if ( $status instanceof Status ) { + $this->getOutput()->addHTML( + '

' . + $this->getOutput()->parseInline( $status->getWikiText( $info ) ) . + '

' ); + } + return false; + } + + $user->saveSettings(); + return $info ? $info : true; + } +} diff --git a/includes/specials/SpecialPreferences.php b/includes/specials/SpecialPreferences.php index edc26bc1a6..eeb7fd2e96 100644 --- a/includes/specials/SpecialPreferences.php +++ b/includes/specials/SpecialPreferences.php @@ -61,11 +61,6 @@ class SpecialPreferences extends SpecialPage { ); } - if ( $wgRequest->getCheck( 'eauth' ) ) { - $wgOut->wrapWikiMsg( "
\n$1\n
", - 'eauthentsent', $wgUser->getName() ); - } - $htmlForm = Preferences::getFormObject( $wgUser ); $htmlForm->setSubmitCallback( array( 'Preferences', 'tryUISubmit' ) ); diff --git a/languages/messages/MessagesEn.php b/languages/messages/MessagesEn.php index d7871f4be5..b4e4817597 100644 --- a/languages/messages/MessagesEn.php +++ b/languages/messages/MessagesEn.php @@ -369,6 +369,7 @@ $specialPageAliases = array( 'Booksources' => array( 'BookSources' ), 'BrokenRedirects' => array( 'BrokenRedirects' ), 'Categories' => array( 'Categories' ), + 'ChangeEmail' => array( 'ChangeEmail' ), 'ChangePassword' => array( 'ChangePassword', 'ResetPass', 'ResetPassword' ), 'ComparePages' => array( 'ComparePages' ), 'Confirmemail' => array( 'ConfirmEmail' ), @@ -1200,6 +1201,17 @@ password.', Temporary password: $2', 'passwordreset-emailsent' => 'A reminder e-mail has been sent.', +# Special:ChangeEmail +'changeemail' => 'Change E-mail address', +'changeemail-header' => 'Change account e-mail address', +'changeemail-text' => 'Complete this form to change your e-mail address. You will need to enter your password to confirm this change.', +'changeemail-no-info' => 'You must be logged in to access this page directly.', +'changeemail-oldemail' => 'Current E-mail address:', +'changeemail-newemail' => 'New E-mail address:', +'changeemail-none' => '(none)', +'changeemail-submit' => 'Change E-mail', +'changeemail-cancel' => 'Cancel', + # Edit page toolbar 'bold_sample' => 'Bold text', 'bold_tip' => 'Bold text', @@ -1757,6 +1769,8 @@ Note that their indexes of {{SITENAME}} content may be out of date.', 'prefs-watchlist-token' => 'Watchlist token:', 'prefs-misc' => 'Misc', 'prefs-resetpass' => 'Change password', +'prefs-changeemail' => 'Change E-mail', +'prefs-setemail' => 'Set an E-mail address', 'prefs-email' => 'E-mail options', 'prefs-rendering' => 'Appearance', 'saveprefs' => 'Save', diff --git a/maintenance/language/messages.inc b/maintenance/language/messages.inc index 90b325290b..bb3a6148c7 100644 --- a/maintenance/language/messages.inc +++ b/maintenance/language/messages.inc @@ -525,6 +525,16 @@ $wgMessageStructure = array( 'passwordreset-emailelement', 'passwordreset-emailsent', ), + 'changeemail' => array( + 'changeemail', + 'changeemail-header', + 'changeemail-text', + 'changeemail-no-info', + 'changeemail-oldemail', + 'changeemail-newemail', + 'changeemail-submit', + 'changeemail-cancel', + ), 'toolbar' => array( 'bold_sample', 'bold_tip', @@ -935,6 +945,8 @@ $wgMessageStructure = array( 'prefs-watchlist-token', 'prefs-misc', // continue checking if used from here on (r49916) 'prefs-resetpass', + 'prefs-changeemail', + 'prefs-setemail', 'prefs-email', 'prefs-rendering', 'saveprefs', @@ -3516,6 +3528,7 @@ XHTML id names.", 'mail' => 'E-mail sending', 'passwordstrength' => 'JavaScript password checks', 'resetpass' => 'Change password dialog', + 'changeemail' => 'Change E-mail dialog', 'passwordreset' => 'Special:PasswordReset', 'toolbar' => 'Edit page toolbar', 'edit' => 'Edit pages', -- 2.20.1