Skip to content

Enable updating the password of a user. #107

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

Closed
wants to merge 3 commits into from

Conversation

pspoerri
Copy link

@pspoerri pspoerri commented Feb 6, 2020

UserUpdate does not have a hashed_password.

@pspoerri pspoerri requested a review from tiangolo February 6, 2020 13:10
@@ -24,6 +24,11 @@ def create(self, db_session: Session, *, obj_in: UserCreate) -> User:
db_session.refresh(db_obj)
return db_obj

def update(self, db_session: Session, *, db_obj: ModelType, obj_in: UpdateSchemaType) -> ModelType:
if obj_in.password:
db_obj.hashed_password = get_password_hash(obj_in.password)
Copy link
Author

Choose a reason for hiding this comment

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

I am not sure if this is the correct place to update the password.

Copy link
Contributor

Choose a reason for hiding this comment

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

You probably want to use the reset_password method in login.py

Copy link
Author

Choose a reason for hiding this comment

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

You probably want to use the reset_password method in login.py

Shouldn't it be the other way around, e.g. the endpoint/login.py using CRUDUser?

@laurentS
Copy link

laurentS commented Feb 7, 2020

I was about to open almost the same PR to fix #109. I've put a small diff in there that you might want to add to your PR to fix the test as well :)

@pspoerri pspoerri removed the request for review from tiangolo February 10, 2020 07:30
@tiangolo
Copy link
Member

Thanks for your contribution! 🚀

I think this was solved/included in #106 , right?

@pspoerri
Copy link
Author

Yes, looks like it. 😅

@pspoerri pspoerri closed this Apr 22, 2020
@pspoerri pspoerri deleted the fix_user_update_password branch April 22, 2020 13:19
@tiangolo
Copy link
Member

Thanks for reporting back and closing it 👍

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.

4 participants