Last modified: 2010-07-04 17:19:34 UTC
Hello all, after a discussion with Anders Wegge because of the bugzilla 2126 [1] (now solved), please let me propose: - to add for security reasons a reasonable expiry time (minutes/hours/days) for the temporary password, which the wiki sends on a user's request. Remark 1: Currently, the temporary password remain valid until a new is requested (~forever) Remark 2: The regular password is never touched by this mechanism; it remains valid until is is changed by the user in via Special:Preferences. The expiry time can be handled in a similar way as it was recently introduced by Brion for the e-mail address confirmation token [2]). [1] http://bugzilla.wikimedia.org/show_bug.cgi?id=2126 Unable to set new password after using emailed password (= temporary password can only be used once) [2] http://bugzilla.wikipedia.org/show_bug.cgi?id=866 EConfirm (EC): e-mail address confirmation by sending a link comprising a token to the unconfirmed mailaddress
The temporary password now is revoked after the first login with it (which forces a password change), which should remove some of this pressure. There's not currently an expiry though.
(In reply to comment #1) > The temporary password now is revoked after the first login with it (which > forces a password change), which should remove some of this pressure. There's > not currently an expiry though. Brion: sounds reasonable. Greetings
So is this a WORKSFORME?
No, because there's still no expiry. I don't know if we need one, though.
The timestamp a new password was last requested is stored in user.user_newpass_time; Tim added it in r17217 to facilitate throttling, but there's no reason it couldn't be used to force expiration, if desired.
Created attachment 5032 [details] Adds 48 hour default expiry time for password reminder emails. Patch against HEAD: Uses $wgPasswordReminderResetTime to check for expired passwords. Uses user.user_newpass_time.
Couple notes: 1) Setting the expiration in hours seems sub-ideal to me, since nearly all time-based config options are in seconds. I'd recommend renaming $wgPasswordReminderResetTime to $wgNewPasswordExpiry and setting it in seconds. 2) The password reset form also uses checkTemporaryPassword() and it looks like it'll take the 'EXPIRED' return as 'true', indicating that it's ok to do the reset, thus bypassing the expiry. As a general security principle against exposing information leaks, as well as to avoid any other potential call funkiness, it might be best to simply return false here, considering the expired password to just not match. This would be the same as if the temporary password had been wiped out, say by another new password request or a successful reset completion -- these cases would not tell you that it used to be correct, they'd just consider it invalid. 2) The new password email text should include the expiry time. 3) I'd recommend 7 days rather than 2 as the default; I know I don't get around to some websites within 48 hours if I get busy doing something else (say, over the weekend). 4) Patch appears to be adding UTF-8 BOM characters, need to be removed. :)
Created attachment 5631 [details] Updated diff - Blind check of temporary password OK, this patch has checkTemporaryPassword() return false if the password is expired, meaning it will only appear as a wrong password. The only thing I am not sure of is what happens after their password expires. Is there account simply locked out?
Looking good! A couple quick bits... + if( time() < $expiry ) { + return false; + } else { + return true; + } I think you want to reverse these... +Your temporary password will expire in $5 days. This should probably use {{PLURAL}} to properly handle the case where it's set to 1 day. Might want to round, also, in case a non-even number is used (7.33327 days :) > The only thing I am not sure of is what happens after their password expires. > Is there account simply locked out? Once the temporary password is expired, it just won't work anymore. To log in with a temporary password, a new temporary password will have to be requested. The original password continues to work the whole time, unless the temporary password is actually used successfully.
Created attachment 5642 [details] Update to last patch Updated against the last patch. (In reply to comment #9) > Looking good! A couple quick bits... > > + if( time() < $expiry ) { > + return false; > + } else { > + return true; > + } > > I think you want to reverse these... > Done. > +Your temporary password will expire in $5 days. > > This should probably use {{PLURAL}} to properly handle the case where it's set > to 1 day. Might want to round, also, in case a non-even number is used (7.33327 > days :) > Did the PLURAL support, as well as rounded.
Went ahead and applied patch in r45450.
<b>Notice</b>: Undefined variable: wgNewPasswordExpiry in <b>/Library/WebServer/Documents/trunk/includes/User.php</b> on line <b>2710</b><br /> Reverted in r45485
Re-fixed in r45503
