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

implement account logic with jwt token #4

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Mxbely
Copy link

@Mxbely Mxbely commented Jan 21, 2025

No description provided.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Great job on implementing the core functionalities for user registration, activation, password reset, login, and token refresh! 🎉 While there are some minor issues like unused imports and a potential security concern with password handling, these do not significantly impact the overall functionality. Make sure to address these in future iterations to keep your code clean and secure. Keep up the good work! 🚀

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

def login(
data: UserLogin,
db: Session = Depends(get_db),
jwt_manager: JWTAuthManagerInterface = Depends(get_jwt_auth_manager),

Choose a reason for hiding this comment

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

There is a potential issue here: user._hashed_password should be user.password if the password is stored in the password field. Ensure that the correct attribute is being accessed for password verification.

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