From 7a1394923bc238ab7019b8a070d476c59b8880ee Mon Sep 17 00:00:00 2001 From: Patrick Da Silva Date: Tue, 29 Sep 2020 00:42:43 +0200 Subject: [PATCH 1/2] Re-factored ResetPasswordConfirm using the open-closed principle The .post method in ResetPasswordConfirm was very bulky and much of the functionality inside the post method made more sense as a collection of smaller methods with a more precise task, because they could be re-defined for another developer's use-case (this is my case). Functionality is left unchanged but the steps can now be adjusted if a developer subclasses this view. The post method is also now much more readable. - The token is only used to fetch the user from the DB, so we have the "get_user_from_token" method. - The purpose of the try-except block for the password validation is now clearer; isolated as a method, the method validates the password and converts the Django validation error into a DRF one - We used a contextmanager to trigger signals instead of just writing the signals. This gives a child class more control over what to do before and after the password change, improves readability, and puts all signal triggering in one place. - There's an actual "reset_password" method now. --- django_rest_passwordreset/views.py | 71 ++++++++++++++++++++---------- 1 file changed, 47 insertions(+), 24 deletions(-) diff --git a/django_rest_passwordreset/views.py b/django_rest_passwordreset/views.py index df7c8df..45afc45 100644 --- a/django_rest_passwordreset/views.py +++ b/django_rest_passwordreset/views.py @@ -14,6 +14,8 @@ get_password_reset_lookup_field from django_rest_passwordreset.signals import reset_password_token_created, pre_password_reset, post_password_reset +from contextlib import contextmanager + User = get_user_model() __all__ = [ @@ -57,34 +59,55 @@ def post(self, request, *args, **kwargs): password = serializer.validated_data['password'] token = serializer.validated_data['token'] - # find token - reset_password_token = ResetPasswordToken.objects.filter(key=token).first() + # find user with token + user = self.get_user_from_token(token) + + # change user's password (if we got to this code it means that the user is_active) + if user.eligible_for_reset(): + with self.password_change_signals(user): # Triggers signals before and after the context + self.validate_password(user, password) + self.reset_password(user, password) - # change users password (if we got to this code it means that the user is_active) - if reset_password_token.user.eligible_for_reset(): - pre_password_reset.send(sender=self.__class__, user=reset_password_token.user) - try: - # validate the password against existing validators - validate_password( - password, - user=reset_password_token.user, - password_validators=get_password_validators(settings.AUTH_PASSWORD_VALIDATORS) - ) - except ValidationError as e: - # raise a validation error for the serializer - raise exceptions.ValidationError({ - 'password': e.messages - }) - - reset_password_token.user.set_password(password) - reset_password_token.user.save() - post_password_reset.send(sender=self.__class__, user=reset_password_token.user) - - # Delete all password reset tokens for this user - ResetPasswordToken.objects.filter(user=reset_password_token.user).delete() + self.delete_previous_tokens(user=user) return Response({'status': 'OK'}) + @contextmanager + def password_change_signals(self,user): + """ triggers pre_password_reset signal before, triggers post_password_reset after """ + pre_password_reset.send(sender=self.__class__,user=user) + yield + post_password_reset.send(sender=self.__class__, user=user) + + def get_user_from_token(self, token): + """ uses raw reset_password_token to retrieve user from db """ + reset_password_token = ResetPasswordToken.objects.filter(key=token).first() + return reset_password_token.user + + def validate_password(self, user, password): + """ validate the password against the user and converts the Django ValidationError in a DRF ValidationError """ + try: + # validate the password against existing validators + validate_password( + password=password, + user=user, + password_validators=get_password_validators(settings.AUTH_PASSWORD_VALIDATORS) + ) + except ValidationError as e: + # raise a validation error for the serializer + raise exceptions.ValidationError({ + 'password': e.messages + }) + + def reset_password(self, user, password): + """ resets the user's password """ + user.set_password(password) + user.save() + + def delete_previous_tokens(self, user): + """ deletes tokens previously associated with this user """ + ResetPasswordToken.objects.filter(user=user).delete() + class ResetPasswordRequestToken(GenericAPIView): """ From 008ec76df941db67677e7c2c1844038d0301cbab Mon Sep 17 00:00:00 2001 From: Patrick Da Silva Date: Tue, 29 Sep 2020 01:04:18 +0200 Subject: [PATCH 2/2] Tab indent fix - Replaced tabs by spaces --- django_rest_passwordreset/views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/django_rest_passwordreset/views.py b/django_rest_passwordreset/views.py index 45afc45..509bcff 100644 --- a/django_rest_passwordreset/views.py +++ b/django_rest_passwordreset/views.py @@ -76,8 +76,8 @@ def post(self, request, *args, **kwargs): def password_change_signals(self,user): """ triggers pre_password_reset signal before, triggers post_password_reset after """ pre_password_reset.send(sender=self.__class__,user=user) - yield - post_password_reset.send(sender=self.__class__, user=user) + yield + post_password_reset.send(sender=self.__class__, user=user) def get_user_from_token(self, token): """ uses raw reset_password_token to retrieve user from db """