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 "Forgot Password" Functionality #460

Closed
wants to merge 0 commits into from

Conversation

gaurav-jo1
Copy link
Contributor

What does this PR do?

This PR introduces the "Forgot Password" functionality to enhance user experience and security.

What issues does this PR fix or reference?

#411

If this PR changes the UI, include a screenshot below.

k_scale_labs.mp4

Additional Notes:

  • The /forgot-password endpoint response does not disclose whether the email exists in the system, preventing potential enumeration attacks.

  • Added the remove_existing_token_for_email function to ensure that if a user makes multiple reset password requests, any previous tokens are invalidated.

Please let me know if there are any changes you’d like to make.

Copy link
Collaborator

@Winston-Hsiao Winston-Hsiao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall good, but some adjustments needed

store/app/routers/users.py Outdated Show resolved Hide resolved
store/app/routers/users.py Outdated Show resolved Hide resolved
store/app/routers/users.py Outdated Show resolved Hide resolved
@gaurav-jo1
Copy link
Contributor Author

gaurav-jo1 commented Oct 11, 2024

I have added the PasswordResetToken model and implemented new CRUD operations.

Please review the changes and let me know if any modifications or improvements are needed.

@Winston-Hsiao Winston-Hsiao self-requested a review October 12, 2024 03:48
Copy link
Collaborator

@Winston-Hsiao Winston-Hsiao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Winston-Hsiao
Copy link
Collaborator

Hey @gaurav-jo1 sorry we've been busy shipping some larger features. There's some merge conflicts but we'd love to get your forgot password functionality you made merged in. Sorry for the delay!

@gaurav-jo1
Copy link
Contributor Author

Hey Winston, no worries at all! I'd love to help get that merged. Since the UI has changed quite a bit recently, I'll need to update the UI in my PR as well. Give me a little time, and I'll get that sorted!

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.

2 participants