Last modified: 2010-07-04 17:19:34 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T4242, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 2242 - Proposal: introduce an expiry time for the temporary password
Proposal: introduce an expiry time for the temporary password
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
User login and signup (Other open bugs)
1.5.x
All All
: Normal enhancement with 3 votes (vote)
: ---
Assigned To: Nobody - You can work on this!
: patch
Depends on:
Blocks: 9816
  Show dependency treegraph
 
Reported: 2005-05-24 22:01 UTC by T. Gries
Modified: 2010-07-04 17:19 UTC (History)
5 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
Adds 48 hour default expiry time for password reminder emails. (4.24 KB, patch)
2008-06-28 15:03 UTC, Tyler Romeo
Details
Updated diff - Blind check of temporary password (3.10 KB, patch)
2008-12-30 17:58 UTC, Tyler Romeo
Details
Update to last patch (3.16 KB, patch)
2009-01-02 20:53 UTC, Chad H.
Details

Description T. Gries 2005-05-24 22:01:18 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
Comment 1 Brion Vibber 2006-12-12 21:47:10 UTC
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.
Comment 2 T. Gries 2006-12-12 22:11:06 UTC
(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
Comment 3 Titoxd 2007-05-14 19:56:54 UTC
So is this a WORKSFORME?
Comment 4 Aryeh Gregor (not reading bugmail, please e-mail directly) 2007-05-15 01:20:19 UTC
No, because there's still no expiry.  I don't know if we need one, though.
Comment 5 Rob Church 2007-06-08 02:16:56 UTC
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.
Comment 6 Tyler Romeo 2008-06-28 15:03:57 UTC
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.
Comment 7 Brion Vibber 2008-12-30 03:45:02 UTC
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. :)
Comment 8 Tyler Romeo 2008-12-30 17:58:05 UTC
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?
Comment 9 Brion Vibber 2008-12-30 18:06:27 UTC
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.
Comment 10 Chad H. 2009-01-02 20:53:42 UTC
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.
Comment 11 Chad H. 2009-01-06 16:22:19 UTC
Went ahead and applied patch in r45450.
Comment 12 Brion Vibber 2009-01-07 03:55:13 UTC
<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
Comment 13 Chad H. 2009-01-07 14:35:43 UTC
Re-fixed in r45503

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links