Patch #17717
closed
Password/Email address change should invalidate security tokens
90%
Description
To improve user account security, we believe it is a good practice to:
- invalidate the password reset token (sent via email) once an account's email address is changed.
(This prevents hackers who may be able to change a user's address (or trick him into doing it) to use an "old" password reset link previously sent via email once the email address has been changed back by the user.) - invalidate the password reset token and autologin token once an account's password is changed.
(This prevents hackers from being still able to login after a user has potentially discovered a breach into his/her account and changed their password.)
The attached patch against current Redmine trunk implements this; tests included.
Files
Updated by Jan from Planio www.plan.io over 10 years ago
- % Done changed from 0 to 90
Updated by Jean-Baptiste Barth over 10 years ago
- Assignee set to Jean-Baptiste Barth
Looks good, and I confirm tests pass! There's just a little typo in the comment but nothing serious.
I'd like to have details by Jean-Philippe about how we deal with that kind of security improvements: I was about to commit it but it may not be a good idea weeks or months before a new major version is released. Jean-Philippe ?
Updated by Jan from Planio www.plan.io over 10 years ago
Jean-Baptiste Barth wrote:
Looks good, and I confirm tests pass! There's just a little typo in the comment but nothing serious.
Thanks.
I'd like to have details by Jean-Philippe about how we deal with that kind of security improvements: I was about to commit it but it may not be a good idea weeks or months before a new major version is released. Jean-Philippe ?
Personally, I don't think it would be a problem since it's not a fix for a security issue per se. Both this and #17796 can only be exploited in conjunction with social engineering or other security problems and not by itself. But I agree, let's wait for Jean-Philippe's opinion on this!
Updated by Jean-Philippe Lang over 10 years ago
Jan from Planio www.plan.io wrote:
Personally, I don't think it would be a problem since it's not a fix for a security issue per se. Both this and #17796 can only be exploited in conjunction with social engineering or other security problems and not by itself. But I agree, let's wait for Jean-Philippe's opinion on this!
Agreed, this change and #17796 can be committed now for 2.6. Thanks.
Updated by Etienne Massip over 10 years ago
Why are both issues in Security? Do you have any objection if we move them to public?
More generally, now that issues can be set as private, do we still need Security project?
Updated by Jan from Planio www.plan.io over 10 years ago
Etienne Massip wrote:
Why are both issues in Security? Do you have any objection if we move them to public?
It was just a precautionary measure. In theory, the issues can be exploited (together with some other vector such as social engineering), so I wanted to be sure to not tell more people about this as necessary before a fix is available for Redmine admins.
It may make sense to mov security issues to the public Redmine project, once they're fixed and released, to increase transparency. (Or we use the private flag as you suggest and remove it once released.)
Updated by Etienne Massip over 10 years ago
- Project changed from 2 to Redmine
- Category set to Security
- Target version set to 2.6.0
Updated by Jean-Baptiste Barth over 10 years ago
- Status changed from Needs feedback to Closed
Committed it in r13396.