Modify

Opened 8 years ago

Closed 3 years ago

Last modified 3 years ago

#816 closed enhancement (fixed)

"forgot password" should not reset password directly

Reported by: moo Owned by: hasienda
Priority: high Component: AccountManagerPlugin
Severity: major Keywords: passwort reset forgot
Cc: mmitar@… Trac Release: 0.11

Description

it's easy to know which email someone use, trying to reset their password is funny, isn't it?

why not save a temp password in another table/file, so ppl can use it to reset password, and login later.

Attachments (0)

Change History (18)

comment:1 Changed 8 years ago by mgood

  • Keywords needinfo added

I'm not sure I see the point of this. Resetting the password mails a new random password to the user's email address. So, it doesn't give the other user access to that user's password. Saving the new password in a separate database doesn't change that someone could guess a user's password and reset their password, so I guess I don't see what effect this change would have.

I'm thinking this is a wontfix, but maybe I'm missing something, so if you can give a little more description about how this would actually affect users I'll consider it.

comment:2 follow-up: Changed 8 years ago by coderanger

I think the idea was to somehow prevent nuisance password changes by another person. As in I know your email so I just keep reseting your password to annoy you.

comment:3 in reply to: ↑ 2 ; follow-up: Changed 8 years ago by mgood

Replying to coderanger:

I think the idea was to somehow prevent nuisance password changes by another person. As in I know your email so I just keep reseting your password to annoy you.

Yes, but I don't see this actually preventing that. In general changing the password in the htpasswd file or whatever you're using won't affect the user if they're currently logged in, so they'd probably see the email that someone asked to reset their password before the next time they logged in and go fix it. Unfortunately I don't think there's a good way to verify that the person asking to reset the password is the actual user. The "security question" things are often silly things that also wouldn't be very hard for someone to guess.

comment:4 Changed 8 years ago by moo

sorry to make u miss my point. the topic is "don't reset password directly" instead of "don't send anonying email". the email should say "if it's not u requested it, ignore this email silently" instead of "hey, u must login with password !@#$!@#$ and set another pw immediately"

it's too bad if my password keep being reset by other ppl, making me have to use the new pw to login, and change it so my bad memory can remembered.

comment:5 in reply to: ↑ 3 Changed 8 years ago by tcp@…

I'm following what moo is saying here -- if someone other than the real user requests a password reset, it will reset the password, thereby inconveniencing the real user (imagine being somewhere you don't have easy access to your email, for example). Now, the verification is trivial, just as moo is getting at -- all you need to do is send the "reset email" and wait for the user to actually follow the link in the reset email before you actually reset the password.

Doing otherwise leaves the system open to a user specific denial of service if they're unable to access their email and discover that a reset email was sent and their password reset (by someone else). Moreover, simply resetting the password without that sort of verification leads to a potentially confusing situation of having to look at one's email just in case a password reset email is there, if one cannot access a website -- that or choose to reset the password (again) and then having to recognize which is the correct email (that is, the latter one) -- sure, it's workable, but it isn't intuitive and it may well confuse novice users.

comment:6 Changed 8 years ago by anonymous

Furthermore, one could prevent a user from logging in at all by constantly resetting their password -- using a script for instance.

comment:7 follow-up: Changed 7 years ago by ThurnerRupert

you mean you have to store the new password additionally to the old one while the user thinks if she should click the link, or login with the new password?

comment:8 in reply to: ↑ 7 ; follow-ups: Changed 7 years ago by anonymous

Replying to ThurnerRupert:

you mean you have to store the new password additionally to the old one while the user thinks if she should click the link, or login with the new password?

No. The password must remain unchanged until such time as the emailed link is followed -- i.e. you should store a record of reset request being sent, along with some sort of hashed value to authenticate the response to the reset email (this would all be in the url linked to in the reset email). This would ensure that only someone with access to the registered email address could in fact change the password. Anything short of that is an easy denial of service attack vector. I'm not sure how to further explain this, but it's a pretty common methodology and similar examples can be found on many many many websites.

comment:9 in reply to: ↑ 8 Changed 6 years ago by anonymous

Replying to anonymous: it's a pretty common methodology and similar examples can be found on many many many websites.

agree

comment:10 follow-up: Changed 5 years ago by anonymous

  • Trac Release changed from 0.10 to 0.11

Is there a way to disable the "Forgot your password?" functionality? I just tested filling out a username and a different email address than that is associated with the user, I received no mail on whichever address but the password was changed anyway?! This doesn't seem right, so I would like to disable the functionality.

comment:11 in reply to: ↑ 10 Changed 4 years ago by hasienda

  • Keywords passwort reset forgot added; needinfo removed
  • Owner changed from mgood to pacopablo
  • Priority changed from normal to high
  • Severity changed from normal to major

Replying to anonymous:

Is there a way to disable the "Forgot your password?" functionality? I just tested filling out a username and a different email address than that is associated with the user, I received no mail on whichever address but the password was changed anyway?! This doesn't seem right, so I would like to disable the functionality.

Of corse, this is known to be accomplished by disabling the AccountModule. Please RTFM and please, please with a cherry on top don't abuse/hijack related nor unrelated ticket for support questions, that belong to the mailing-list.

The issue itself should be really fixed anyway.

comment:12 in reply to: ↑ 8 Changed 4 years ago by hasienda

  • Owner changed from pacopablo to hasienda

Replying to anonymous:

Replying to ThurnerRupert:

you mean you have to store the new password additionally to the old one while the user thinks if she should click the link, or login with the new password?

No. The password must remain unchanged until such time as the emailed link is followed -- i.e. you should store a record of reset request being sent, along with some sort of hashed value to authenticate the response to the reset email (this would all be in the url linked to in the reset email). This would ensure that only someone with access to the registered email address could in fact change the password. Anything short of that is an easy denial of service attack vector. I'm not sure how to further explain this, but it's a pretty common methodology and similar examples can be found on many many many websites.

While I agree to non-forceful password changes in general, we should agree to an additional 'if triggered by user' condition.

As we plan to make the password reset available on the user account admin page as well (#7111), we'll certainly not want to add an administrative reset option, that could be bypassed somehow, right?

And I don't see much benefit in storing arbitrary reset request data. This could be a target to an external DoS attack as well. If we really do want to prevent unauthorized password resets, there seems nothing wrong to me with recording just the last temporary password sent to the user.

comment:13 Changed 3 years ago by hasienda

  • Status changed from new to assigned

Consider shared use of authentication stores, where pushing a temporary password to the store right away results in inconsistent behaviour:

The one environment initiating the password reset will trap the user until he/she does another password change, if the force_passwd_change has been set too, while all other environments just go and use the new (meant to be temporary) password without any restriction (see #4160).

comment:14 Changed 3 years ago by hasienda

(In [10263]) AccountManagerPlugin: Introduce new variable to class SessionStore, refs #816.

This allows for initialization of multiple concurrent instances of this IPasswordStore, that can exist side-by-side without interference.

comment:15 Changed 3 years ago by hasienda

(In [10264]) AccountManagerPlugin: Rework the 'lost password' procedure, refs #816.

Password reset is less intrusive now, not altering the current password before a successful login using it. The temporary password is stored in a separate SessionStore (sharing configuration with any other SessionStore) and merely checked as a fallback, if the regular authentication has failed. On authentication success with the old password any temporary password is deleted to prevent abuse of the 'lost password' procedure by others.

comment:16 Changed 3 years ago by mitar

  • Cc mmitar@… added; anonymous removed

comment:17 Changed 3 years ago by hasienda

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [10393]) AccountManagerPlugin: Releasing version 0.3, pushing development to 0.4.

This new feature release finally propagates a number of solutions into an official release, after some time of testing with trunk, so explicitely closes #442, #816, #2966, #3989, #4160, #6821, #7111, #8534, #8549, #8663, #8813, #8892, #8925, #8936 and #8939.

Should have made this months ago, but felt so many pending issues were too bad for a new release. But it has been a tremendous ticket burndown since last year, so it's really worth considering an upgrade now. See fresh changelog for details.

comment:18 Changed 3 years ago by hasienda

(In [10395]) AccountManagerPlugin: Releasing version 0.3, pushing development to 0.4.

This new feature release finally propagates a number of solutions into an official release, after some time of testing with trunk, so explicitely closes #442, #816, #2966, #3989, #4160, #6821, #7111, #8534, #8549, #8663, #8813, #8892, #8925, #8936 and #8939.

Should have made this months ago, but felt so many pending issues were too bad for a new release. But it has been a tremendous ticket burndown since last year, so it's really worth considering an upgrade now. See fresh changelog for details.

Add Comment

Modify Ticket

Action
as closed The owner will remain hasienda.
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.