Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate a new reset_password_token only when it isn't valid anymore or nil #4653

Closed
fernandomm opened this issue Sep 26, 2017 · 3 comments
Closed

Comments

@fernandomm
Copy link

fernandomm commented Sep 26, 2017

When you call set_reset_password_token, it will always generate a new reset_password_token.

This can cause some issues:

  1. An user may click twice at the submit button and will receive two emails. One of emails will have an invalid token ( the older one ). This can be "fixed" by disabling the submit button after click with Javascript.
  2. There might be a delay in the email delivery and user will request another reset password email. Again one of the emails will have an old and invalid reset password token. I don't see a viable option to fix this with the current code.

Because of this we get a lot of support requests in our application regarding users that can't reset their password.

Would you be willing to accept a PR that changes set_reset_password_token method to only change reset_password_token value if it's not valid anymore?

If yes, should this be the default and only behavior or something that can be changed using a Devise option?

@rietta
Copy link

rietta commented Oct 5, 2017

I disagree because that may change the threat posture in unexpected ways. Generating a new token on request is the correct behavior. Possibly debouncing the request/submit button would be the better solution to the double tap.

@cross-p6
Copy link

I agree, all the token-based flows in fact should regenerate on a subsequent request. I think confirmation currently doesn't do that. Maybe a PR is needed

@tegon
Copy link
Member

tegon commented Dec 21, 2017

Hello @fernandomm, thanks for your report.
I agree with @rietta - generating a new token on request is the correct behavior.
I know it's not ideal, but if you need this behavior you can override it in your application's code.

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants