r43841 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r43840‎ | r43841 | r43842 >
Date:05:03, 22 November 2008
Author:mrzman
Status:resolved (Comments)
Tags:
Comment:
Move password resetting out of Special:Preferences, adapt Special:ResetPass to do the job, add Special:ChangePassword as an alias.
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/includes/SpecialPage.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialPreferences.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialResetpass.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialUserlogin.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesEn.php (modified) (history)
  • /trunk/phase3/maintenance/language/messages.inc (modified) (history)

Diff [purge]

Index: trunk/phase3/maintenance/language/messages.inc
——@@ -449,6 +449,10 @@
450450 'resetpass_bad_temporary',
451451 'resetpass_forbidden',
452452 'resetpass_missing',
 453+ 'resetpass-no-info',
 454+ 'resetpass-submit-loggedin',
 455+ 'resetpass-wrong-oldpass',
 456+ 'resetpass-temp-password',
453457 ),
454458 'toolbar' => array(
455459 'bold_sample',
——@@ -862,6 +866,7 @@
863867 'prefs-watchlist-edits',
864868 'prefs-watchlist-edits-max',
865869 'prefs-misc',
 870+ 'prefs-resetpass',
866871 'saveprefs',
867872 'resetprefs',
868873 'oldpassword',
Index: trunk/phase3/includes/specials/SpecialPreferences.php
——@@ -21,7 +21,7 @@
2222 * @ingroup SpecialPage
2323 */
2424 class PreferencesForm {
25 - var $mQuickbar, $mOldpass, $mNewpass, $mRetypePass, $mStubs;
 25+ var $mQuickbar, $mStubs;
2626 var $mRows, $mCols, $mSkin, $mMath, $mDate, $mUserEmail, $mEmailFlag, $mNick;
2727 var $mUserLanguage, $mUserVariant;
2828 var $mSearch, $mRecent, $mRecentDays, $mHourDiff, $mSearchLines, $mSearchChars, $mAction;
——@@ -36,9 +36,6 @@
3737 global $wgContLang, $wgUser, $wgAllowRealName;
3838
3939 $this->mQuickbar = $request->getVal( 'wpQuickbar' );
40 - $this->mOldpass = $request->getVal( 'wpOldpass' );
41 - $this->mNewpass = $request->getVal( 'wpNewpass' );
42 - $this->mRetypePass =$request->getVal( 'wpRetypePass' );
4340 $this->mStubs = $request->getVal( 'wpStubs' );
4441 $this->mRows = $request->getVal( 'wpRows' );
4542 $this->mCols = $request->getVal( 'wpCols' );
——@@ -212,30 +209,6 @@
213210 global $wgEmailAuthentication, $wgRCMaxAge;
214211 global $wgAuth, $wgEmailConfirmToEdit;
215212
216 -
217 - if ( ($this->mNewpass !== '' || $this->mOldpass !== '' ) && $wgAuth->allowPasswordChange() ) {
218 - if ( $this->mNewpass != $this->mRetypePass ) {
219 - wfRunHooks( 'PrefsPasswordAudit', array( $wgUser, $this->mNewpass, 'badretype' ) );
220 - $this->mainPrefsForm( 'error', wfMsg( 'badretype' ) );
221 - return;
222 - }
223 -
224 - if (!$wgUser->checkPassword( $this->mOldpass )) {
225 - wfRunHooks( 'PrefsPasswordAudit', array( $wgUser, $this->mNewpass, 'wrongpassword' ) );
226 - $this->mainPrefsForm( 'error', wfMsg( 'wrongpassword' ) );
227 - return;
228 - }
229 -
230 - try {
231 - $wgUser->setPassword( $this->mNewpass );
232 - wfRunHooks( 'PrefsPasswordAudit', array( $wgUser, $this->mNewpass, 'success' ) );
233 - $this->mNewpass = $this->mOldpass = $this->mRetypePass = '';
234 - } catch( PasswordError $e ) {
235 - wfRunHooks( 'PrefsPasswordAudit', array( $wgUser, $this->mNewpass, 'error' ) );
236 - $this->mainPrefsForm( 'error', $e->getMessage() );
237 - return;
238 - }
239 - }
240213 $wgUser->setRealName( $this->mRealName );
241214 $oldOptions = $wgUser->mOptions;
242215
——@@ -373,7 +346,6 @@
374347 function resetPrefs() {
375348 global $wgUser, $wgLang, $wgContLang, $wgContLanguageCode, $wgAllowRealName;
376349
377 - $this->mOldpass = $this->mNewpass = $this->mRetypePass = '';
378350 $this->mUserEmail = $wgUser->getEmail();
379351 $this->mUserEmailAuthenticationtimestamp = $wgUser->getEmailAuthenticationtimestamp();
380352 $this->mRealName = ($wgAllowRealName) ? $wgUser->getRealName() : '';
——@@ -755,28 +727,11 @@
756728
757729 # Password
758730 if( $wgAuth->allowPasswordChange() ) {
 731+ $link = $wgUser->getSkin()->link( SpecialPage::getTitleFor( 'ResetPass' ), wfMsgHtml( 'prefs-resetpass' ),
 732+ array() , array('returnto' => SpecialPage::getTitleFor( 'Preferences') ) );
759733 $wgOut->addHTML(
760734 $this->tableRow( Xml::element( 'h2', null, wfMsg( 'changepassword' ) ) ) .
761 - $this->tableRow(
762 - Xml::label( wfMsg( 'oldpassword' ), 'wpOldpass' ),
763 - Xml::password( 'wpOldpass', 25, $this->mOldpass, array( 'id' => 'wpOldpass', 'autocomplete' => 'off' ) )
764 - ) .
765 - $this->tableRow(
766 - Xml::label( wfMsg( 'newpassword' ), 'wpNewpass' ),
767 - Xml::password( 'wpNewpass', 25, $this->mNewpass, array( 'id' => 'wpNewpass', 'autocomplete' => 'off' ) )
768 - ) .
769 - $this->tableRow(
770 - Xml::label( wfMsg( 'retypenew' ), 'wpRetypePass' ),
771 - Xml::password( 'wpRetypePass', 25, $this->mRetypePass, array( 'id' => 'wpRetypePass', 'autocomplete' => 'off' ) )
772 - )
773 - );
774 - if( $wgCookieExpiration > 0 ){
775 - $wgOut->addHTML(
776 - $this->tableRow( $this->getToggle( "rememberpassword" ) )
777 - );
778 - } else {
779 - $this->mUsedToggles['rememberpassword'] = true;
780 - }
 735+ $this->tableRow( '<ul><li>' . $link . '</li></ul>' ) );
781736 }
782737
783738 # <FIXME>
Index: trunk/phase3/includes/specials/SpecialUserlogin.php
——@@ -582,7 +582,7 @@
583583
584584 function resetLoginForm( $error ) {
585585 global $wgOut;
586 - $wgOut->addWikiText( "<div class=\"errorbox\">$error</div>" );
 586+ $wgOut->addHTML( Xml::element('p', array( 'class' => 'error' ), $error ) );
587587 $reset = new PasswordResetForm( $this->mName, $this->mPassword );
588588 $reset->execute( null );
589589 }
Index: trunk/phase3/includes/specials/SpecialResetpass.php
——@@ -18,11 +18,11 @@
1919 function __construct( $name=null, $reset=null ) {
2020 if( $name !== null ) {
2121 $this->mName = $name;
22 - $this->mTemporaryPassword = $reset;
 22+ $this->mOldpass = $reset;
2323 } else {
2424 global $wgRequest;
2525 $this->mName = $wgRequest->getVal( 'wpName' );
26 - $this->mTemporaryPassword = $wgRequest->getVal( 'wpPassword' );
 26+ $this->mOldpass = $wgRequest->getVal( 'wpPassword' );
2727 }
2828 }
2929
——@@ -37,8 +37,8 @@
3838 return;
3939 }
4040
41 - if( $this->mName === null && !$wgRequest->wasPosted() ) {
42 - $this->error( wfMsg( 'resetpass_missing' ) );
 41+ if( !$wgRequest->wasPosted() && !$wgUser->isLoggedIn() ) {
 42+ $this->error( wfMsg( 'resetpass-no-info' ) );
4343 return;
4444 }
4545
——@@ -48,20 +48,24 @@
4949 try {
5050 $this->attemptReset( $newpass, $retype );
5151 $wgOut->addWikiMsg( 'resetpass_success' );
52 -
53 - $data = array(
54 - 'action' => 'submitlogin',
55 - 'wpName' => $this->mName,
56 - 'wpPassword' => $newpass,
57 - 'returnto' => $wgRequest->getVal( 'returnto' ),
58 - );
59 - if( $wgRequest->getCheck( 'wpRemember' ) ) {
60 - $data['wpRemember'] = 1;
 52+ if( !$wgUser->isLoggedIn() ) {
 53+ $data = array(
 54+ 'action' => 'submitlogin',
 55+ 'wpName' => $this->mName,
 56+ 'wpPassword' => $newpass,
 57+ 'returnto' => $wgRequest->getVal( 'returnto' ),
 58+ );
 59+ if( $wgRequest->getCheck( 'wpRemember' ) ) {
 60+ $data['wpRemember'] = 1;
 61+ }
 62+ $login = new LoginForm( new FauxRequest( $data, true ) );
 63+ $login->execute();
6164 }
62 - $login = new LoginForm( new FauxRequest( $data, true ) );
63 - $login->execute();
64 -
65 - return;
 65+ $titleObj = Title::newFromText( $wgRequest->getVal( 'returnto' ) );
 66+ if ( !$titleObj instanceof Title ) {
 67+ $titleObj = Title::newMainPage();
 68+ }
 69+ $wgOut->redirect( $titleObj->getFullURL() );
6670 } catch( PasswordError $e ) {
6771 $this->error( $e->getMessage() );
6872 }
——@@ -71,9 +75,7 @@
7276
7377 function error( $msg ) {
7478 global $wgOut;
75 - $wgOut->addHTML( '<div class="errorbox">' .
76 - htmlspecialchars( $msg ) .
77 - '</div>' );
 79+ $wgOut->addHTML( Xml::element('p', array( 'class' => 'error' ), $msg ) );
7880 }
7981
8082 function showForm() {
——@@ -82,44 +84,54 @@
8385 $wgOut->disallowUserJs();
8486
8587 $self = SpecialPage::getTitleFor( 'Resetpass' );
86 - $form =
87 - '<div id="userloginForm">' .
88 - wfOpenElement( 'form',
 88+ if ( !$this->mName ) {
 89+ $this->mName = $wgUser->getName();
 90+ }
 91+ $rememberMe = '';
 92+ if ( !$wgUser->isLoggedIn() ) {
 93+ $rememberMe = '<tr>' .
 94+ '<td></td>' .
 95+ '<td>' .
 96+ Xml::checkLabel( wfMsg( 'remembermypassword' ),
 97+ 'wpRemember', 'wpRemember',
 98+ $wgRequest->getCheck( 'wpRemember' ) ) .
 99+ '</td>' .
 100+ '</tr>';
 101+ $submitMsg = 'resetpass_submit';
 102+ $oldpassMsg = 'resetpass-temp-password';
 103+ } else {
 104+ $oldpassMsg = 'oldpassword';
 105+ $submitMsg = 'resetpass-submit-loggedin';
 106+ }
 107+ $wgOut->addHTML(
 108+ Xml::openElement( 'fieldset' ) .
 109+ Xml::element( 'legend', null, wfMsg( 'resetpass_header' ) ) .
 110+ Xml::openElement( 'form',
89111 array(
90112 'method' => 'post',
91 - 'action' => $self->getLocalUrl() ) ) .
92 - '<h2>' . wfMsgHtml( 'resetpass_header' ) . '</h2>' .
93 - '<div id="userloginprompt">' .
 113+ 'action' => $self->getLocalUrl(),
 114+ 'id' => 'mw-resetpass-form' ) ) .
 115+ Xml::hidden( 'token', $wgUser->editToken() ) .
 116+ Xml::hidden( 'wpName', $this->mName ) .
 117+ Xml::hidden( 'returnto', $wgRequest->getVal( 'returnto' ) ) .
94118 wfMsgExt( 'resetpass_text', array( 'parse' ) ) .
95 - '</div>' .
96119 '<table>' .
97 - wfHidden( 'token', $wgUser->editToken() ) .
98 - wfHidden( 'wpName', $this->mName ) .
99 - wfHidden( 'wpPassword', $this->mTemporaryPassword ) .
100 - wfHidden( 'returnto', $wgRequest->getVal( 'returnto' ) ) .
101120 $this->pretty( array(
102121 array( 'wpName', 'username', 'text', $this->mName ),
 122+ array( 'wpPassword', $oldpassMsg, 'password', $this->mOldpass ),
103123 array( 'wpNewPassword', 'newpassword', 'password', '' ),
104124 array( 'wpRetype', 'yourpasswordagain', 'password', '' ),
105125 ) ) .
 126+ $rememberMe .
106127 '<tr>' .
107128 '<td></td>' .
108129 '<td>' .
109 - Xml::checkLabel( wfMsg( 'remembermypassword' ),
110 - 'wpRemember', 'wpRemember',
111 - $wgRequest->getCheck( 'wpRemember' ) ) .
 130+ wfSubmitButton( wfMsgHtml( $submitMsg ) ) .
112131 '</td>' .
113132 '</tr>' .
114 - '<tr>' .
115 - '<td></td>' .
116 - '<td>' .
117 - wfSubmitButton( wfMsgHtml( 'resetpass_submit' ) ) .
118 - '</td>' .
119 - '</tr>' .
120133 '</table>' .
121 - wfCloseElement( 'form' ) .
122 - '</div>';
123 - $wgOut->addHTML( $form );
 134+ Xml::closeElement( 'form' ) .
 135+ Xml::closeElement( 'fieldset' ) );
124136 }
125137
126138 function pretty( $fields ) {
——@@ -127,16 +139,19 @@
128140 foreach( $fields as $list ) {
129141 list( $name, $label, $type, $value ) = $list;
130142 if( $type == 'text' ) {
131 - $field = '<tt>' . htmlspecialchars( $value ) . '</tt>';
 143+ $field = htmlspecialchars( $value );
132144 } else {
133145 $field = Xml::input( $name, 20, $value,
134146 array( 'id' => $name, 'type' => $type ) );
135147 }
136148 $out .= '<tr>';
137 - $out .= '<td align="right">';
138 - $out .= Xml::label( wfMsg( $label ), $name );
 149+ $out .= "<td class='mw-label'>";
 150+ if ( $type != 'text' )
 151+ $out .= Xml::label( wfMsg( $label ), $name );
 152+ else
 153+ $out .= wfMsg( $label );
139154 $out .= '</td>';
140 - $out .= '<td>';
 155+ $out .= "<td class='mw-input'>";
141156 $out .= $field;
142157 $out .= '</td>';
143158 $out .= '</tr>';
——@@ -153,8 +168,8 @@
154169 throw new PasswordError( 'no such user' );
155170 }
156171
157 - if( !$user->checkTemporaryPassword( $this->mTemporaryPassword ) ) {
158 - throw new PasswordError( wfMsg( 'resetpass_bad_temporary' ) );
 172+ if( !$user->checkTemporaryPassword( $this->mOldpass ) && !$user->checkPassword( $this->mOldpass ) ) {
 173+ throw new PasswordError( wfMsg( 'resetpass-wrong-oldpass' ) );
159174 }
160175
161176 if( $newpass !== $retype ) {
——@@ -162,6 +177,7 @@
163178 }
164179
165180 $user->setPassword( $newpass );
 181+ $user->setCookies();
166182 $user->saveSettings();
167183 }
168184 }
Index: trunk/phase3/includes/SpecialPage.php
——@@ -89,7 +89,9 @@
9090 'CreateAccount' => array( 'SpecialRedirectToSpecial', 'CreateAccount', 'Userlogin', 'signup', array( 'uselang' ) ),
9191 'Preferences' => array( 'SpecialPage', 'Preferences' ),
9292 'Watchlist' => array( 'SpecialPage', 'Watchlist' ),
 93+ 'Resetpass' => array( 'SpecialPage', 'Resetpass' ),
9394
 95+
9496 'Recentchanges' => 'SpecialRecentchanges',
9597 'Upload' => array( 'SpecialPage', 'Upload' ),
9698 'Imagelist' => array( 'SpecialPage', 'Imagelist' ),
——@@ -135,7 +137,6 @@
136138 'Recentchangeslinked' => 'SpecialRecentchangeslinked',
137139 'Movepage' => array( 'UnlistedSpecialPage', 'Movepage' ),
138140 'Blockme' => array( 'UnlistedSpecialPage', 'Blockme' ),
139 - 'Resetpass' => array( 'UnlistedSpecialPage', 'Resetpass' ),
140141 'Booksources' => 'SpecialBookSources',
141142 'Categories' => array( 'SpecialPage', 'Categories' ),
142143 'Export' => array( 'SpecialPage', 'Export' ),
Index: trunk/phase3/languages/messages/MessagesEn.php
——@@ -439,7 +439,7 @@
440440 'Listbots' => array( 'ListBots' ),
441441 'Popularpages' => array( 'PopularPages' ),
442442 'Search' => array( 'Search' ),
443 - 'Resetpass' => array( 'ResetPass', 'ResetPassword' ),
 443+ 'Resetpass' => array( 'ResetPass', 'ResetPassword', 'ChangePassword' ),
444444 'Withoutinterwiki' => array( 'WithoutInterwiki' ),
445445 'MergeHistory' => array( 'MergeHistory' ),
446446 'Filepath' => array( 'FilePath' ),
——@@ -1000,17 +1000,22 @@
10011001 * Nederlands|nl', # do not translate or duplicate this message to other languages
10021002
10031003 # Password reset dialog
1004 -'resetpass' => 'Reset account password',
1005 -'resetpass_announce' => 'You logged in with a temporary e-mailed code.
 1004+'resetpass' => 'Change or reset account password',
 1005+'resetpass_announce' => 'You logged in with a temporary e-mailed code.
10061006 To finish logging in, you must set a new password here:',
1007 -'resetpass_text' => '<!-- Add text here -->', # only translate this message to other languages if you have to change it
1008 -'resetpass_header' => 'Reset password',
1009 -'resetpass_submit' => 'Set password and log in',
1010 -'resetpass_success' => 'Your password has been changed successfully! Now logging you in...',
1011 -'resetpass_bad_temporary' => 'Invalid temporary password.
 1007+'resetpass_text' => '<!-- Add text here -->', # only translate this message to other languages if you have to change it
 1008+'resetpass_header' => 'Reset password',
 1009+'resetpass_submit' => 'Set password and log in',
 1010+'resetpass_success' => 'Your password has been changed successfully! Now logging you in...',
 1011+'resetpass_bad_temporary' => 'Invalid temporary password.
10121012 You may have already successfully changed your password or requested a new temporary password.',
1013 -'resetpass_forbidden' => 'Passwords cannot be changed',
1014 -'resetpass_missing' => 'No form data.',
 1013+'resetpass_forbidden' => 'Passwords cannot be changed',
 1014+'resetpass_missing' => 'No form data.',
 1015+'resetpass-no-info' => 'You must be logged in to access this page directly.',
 1016+'resetpass-submit-loggedin' => 'Change password',
 1017+'resetpass-wrong-oldpass' => 'Invalid temporary or current password.
 1018+You may have already successfully changed your password or requested a new temporary password.',
 1019+'resetpass-temp-password' => 'Temporary password:',
10151020
10161021 # Edit page toolbar
10171022 'bold_sample' => 'Bold text',
——@@ -1543,6 +1548,7 @@
15441549 'prefs-watchlist-edits' => 'Maximum number of changes to show in expanded watchlist:',
15451550 'prefs-watchlist-edits-max' => '(maximum number: 1000)',
15461551 'prefs-misc' => 'Misc',
 1552+'prefs-resetpass' => 'Change password',
15471553 'saveprefs' => 'Save',
15481554 'resetprefs' => 'Clear unsaved changes',
15491555 'oldpassword' => 'Old password:',
Index: trunk/phase3/RELEASE-NOTES
——@@ -212,6 +212,8 @@
213213 * Add id="mw-user-domain-section" to <tr> tag in Userlogin.php template so that
214214 admins with a single domain can hide the domain section using CSS
215215 * Dropped old Paser_OldPP class. Only new parser with preprocessor is used.
 216+* Moved password reset form from Special:Preferences to Special:ResetPass
 217+* Added Special:ChangePassword as a special page alias for Special:ResetPass
216218
217219 === Bug fixes in 1.14 ===
218220

Follow-up revisions

RevisionCommit summaryAuthorDate
r43971Cleanup for r43841:...aaron03:37, 26 November 2008

Comments

#Comment by IAlex (talk | contribs)   17:24, 22 November 2008

Seems that the PrefsPasswordAudit hook disappeared.

#Comment by 😂 (talk | contribs)   01:54, 26 November 2008

Also, the construction seems a bit odd. Why bother extending SpecialPage if you're going to put a wfSpecialResetPass() around it? Wouldn't it make more sense just to make it a class and let SpecialPage call it. Would just have to add a call to the parent constructor within this constructor, and you could drop the entire wfSpecialResetPass() stuff.

#Comment by Aaron Schulz (talk | contribs)   03:37, 26 November 2008

Cleaned up in r43971

Status & tagging log