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

add password reset token expiration #133

Merged
merged 3 commits into from
Feb 9, 2024
Merged

Conversation

alexskr
Copy link
Member

@alexskr alexskr commented Feb 6, 2024

Add password reset token expiration time to fix a security issue where reset token never expires and the same reset token can be used multiple times to reset password.

Changes:

  • Add resetTokenExpireTime attribute to user which is set to Time.now + 1.hour when reset password request is send.
  • resetTokenExpireTime is cleared out when user attempts to resets password. This means that users have only one attempt to reset password for each change password request.

Addresses #60

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7580da8) 72.17% compared to head (e78397b) 73.06%.
Report is 1 commits behind head on develop.

❗ Current head e78397b differs from pull request most recent head 0a2b100. Consider uploading reports for the commit 0a2b100 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #133      +/-   ##
===========================================
+ Coverage    72.17%   73.06%   +0.88%     
===========================================
  Files           52       52              
  Lines         2897     2903       +6     
===========================================
+ Hits          2091     2121      +30     
+ Misses         806      782      -24     
Flag Coverage Δ
unittests 73.06% <100.00%> (+0.88%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@alexskr alexskr merged commit 3c64382 into develop Feb 9, 2024
2 of 5 checks passed
@jvendetti
Copy link
Member

@alexskr - can you provide a description of how your modification changes this functionality? In other words, what happens now when an end user receives a reset password link via email? Can they only click the link once? We have a report on the support list from an end user that is getting an authorization error when they try to reset their password:

... while the credentials enable me to get a password reset email, when I click on the provided link to reset my password, I get the following error message: “Password reset not authorized with this token. Please reset your password again.” I have tried three times.

@alexskr
Copy link
Member Author

alexskr commented Mar 5, 2024

I updated the description of the PR.

It takes 50+ seconds for the password reset to go through when I try to do it in production which is a problem that should be looked into.
UI also doesn't provide any feedback to users that its working on the request which can cause people to leave the page before request completes

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

Successfully merging this pull request may close these issues.

3 participants