anexia-it / django-rest-passwordreset

An extension of django rest framework, providing a configurable password reset strategy
BSD 3-Clause "New" or "Revised" License
419 stars 148 forks source link

Implement Variant of ResetPasswordConfirm that validates a token #59

Closed Hall-Erik closed 5 years ago

Hall-Erik commented 5 years ago

This implements #45

If the user sets DJANGO_REST_PASSWORDRESET_ALLOW_TOKEN_VALIDATION = True in settings, the password field becomes optional in ResetPasswordConfirm. It will return a 200 if a valid token is provided without a password.

If DJANGO_REST_PASSWORDRESET_ALLOW_TOKEN_VALIDATION is not set, or is set to False, ResetPasswordConfirm behaves as it did before - returning a 400 if a password isn't given.

I also added a couple of tests for this and updated the README.

anx-ckreuzberger commented 5 years ago

This looks good.

That's what a good PR looks like :)

Anyway, personally I would have gone for a separate endpoint, e.g., /validate_token/, but sending the token without a password at the same existing endpoint is fine for me too.

I'm going to leave this open for a week or so, as I would like to see some opinions on whether the implemented endpoint logic within this PR is considered as okay or not.

Hall-Erik commented 5 years ago

I agree with you about using separate endpoints. That way when the user gets a 200 from /confirm/, they know it means the password was updated and not just that the token is valid.

anx-ckreuzberger commented 5 years ago

closed in favour of #60