From c83453077e029b8e5142af265aa13b0d3c440288 Mon Sep 17 00:00:00 2001 From: Oliver Falk Date: Fri, 19 Jun 2020 12:36:08 +0200 Subject: [PATCH] Enhance the password reset functionality and add appropriate unit tests --- ivatar/ivataraccount/test_views.py | 77 ++++++++++++++++++++++++++++++ ivatar/ivataraccount/views.py | 45 +++++++++++++---- 2 files changed, 114 insertions(+), 8 deletions(-) diff --git a/ivatar/ivataraccount/test_views.py b/ivatar/ivataraccount/test_views.py index 4a68d54..0b9ca67 100644 --- a/ivatar/ivataraccount/test_views.py +++ b/ivatar/ivataraccount/test_views.py @@ -10,6 +10,7 @@ import django from django.test import TestCase from django.test import Client from django.urls import reverse +from django.core import mail from django.contrib.auth.models import User from django.contrib.auth import authenticate import hashlib @@ -1513,3 +1514,79 @@ class Tester(TestCase): # pylint: disable=too-many-public-methods 200, 'First and last name not correctly listed in profile page', ) + + def test_password_reset_page(self): + ''' + Just test if the password reset page come up correctly + ''' + response = self.client.get(reverse('password_reset')) + self.assertEqual(response.status_code, 200, 'no 200 ok?') + + def test_password_reset_wo_mail(self): + ''' + Test if the password reset doesn't error out + if the mail address doesn't exist + ''' + # Avoid sending out mails + settings.EMAIL_BACKEND = 'django.core.mail.backends.locmem.EmailBackend' + + # Empty database / eliminate existing users + User.objects.all().delete() + url = reverse('password_reset') + response = self.client.post( + url, { + 'email': 'asdf@asdf.local', + }, + follow=True, + ) + self.assertEqual(response.status_code, 200, 'password reset page not working?') + self.assertEqual(len(mail.outbox), 0, 'user does not exist, there should be no mail in the outbox!') + + def test_password_reset_w_mail(self): + ''' + Test if the password reset works correctly with email in + User object + ''' + # Avoid sending out mails + settings.EMAIL_BACKEND = 'django.core.mail.backends.locmem.EmailBackend' + + url = reverse('password_reset') + # Our test user doesn't have an email address by default - but we need one set + self.user.email = 'asdf@asdf.local' + self.user.save() + response = self.client.post( + url, { + 'email': self.user.email, + }, + follow=True, + ) + self.assertEqual(response.status_code, 200, 'password reset page not working?') + self.assertEqual(len(mail.outbox), 1, 'User exists, there should be a mail in the outbox!') + self.assertEqual(mail.outbox[0].to[0], self.user.email, 'Sending mails to the wrong \ + mail address?') + + def test_password_reset_w_confirmed_mail(self): + ''' + Test if the password reset works correctly with confirmed + mail + ''' + # Avoid sending out mails + settings.EMAIL_BACKEND = 'django.core.mail.backends.locmem.EmailBackend' + + url = reverse('password_reset') + # Our test user doesn't have a confirmed mail identity - add one + self.user.confirmedemail_set.create(email='asdf@asdf.local') + self.user.save() + + response = self.client.post( + url, { + 'email': self.user.confirmedemail_set.first().email, + }, + follow=True, + ) + # Since the object is touched in another process, we need to refresh it + self.user.refresh_from_db() + self.assertEqual(response.status_code, 200, 'password reset page not working?') + self.assertEqual(self.user.email, self.user.confirmedemail_set.first().email, 'The password reset view, should have corrected this!') + self.assertEqual(len(mail.outbox), 1, 'user exists, there should be a mail in the outbox!') + self.assertEqual(mail.outbox[0].to[0], self.user.email, 'why are we sending mails to the wrong mail address?') diff --git a/ivatar/ivataraccount/views.py b/ivatar/ivataraccount/views.py index bcdc7b4..ab78a25 100644 --- a/ivatar/ivataraccount/views.py +++ b/ivatar/ivataraccount/views.py @@ -963,18 +963,47 @@ class PasswordResetView(PasswordResetViewOriginal): ''' Since we have the mail addresses in ConfirmedEmail model, we need to set the email on the user object in order for the - PasswordResetView class to pick up the correct user + PasswordResetView class to pick up the correct user. + In case we have the mail address in the User objecct, we still + need to assign a random password in order for PasswordResetView + class to pick up the user - else it will silently do nothing. ''' if 'email' in request.POST: + user = None + + # Try to find the user via the normal user class try: - confirmed_email = ConfirmedEmail.objects.get(email=request.POST['email']) - confirmed_email.user.email = confirmed_email.email - if not confirmed_email.user.password or confirmed_email.user.password == '!': - random_pass = User.objects.make_random_password() - confirmed_email.user.set_pasword(random_pass) - confirmed_email.user.save() - except Exception as exc: + user = User.objects.get(email=request.POST['email']) + except ObjectDoesNotExist as exc: # pylint: disable=unused-variable + # keep this for debugging only + # print('Exception: %s' % exc) pass + + # If we didn't find the user in the previous step, + # try the ConfirmedEmail class instead. + # If we find the user there, we need to set the mail + # attribute on the user object accordingly + if not user: + try: + confirmed_email = ConfirmedEmail.objects.get(email=request.POST['email']) + user = confirmed_email.user + user.email = confirmed_email.email + user.save() + except ObjectDoesNotExist as exc: # pylint: disable=unused-variable + # keep this for debugging only + # print('Exception: %s' % exc) + pass + + # If we found the user, set a random password. Else, the + # ResetPasswordView class will silently ignore the password + # reset request + if user: + if not user.password or user.password == '!': + random_pass = User.objects.make_random_password() + user.set_password(random_pass) + user.save() + + # Whatever happens above, let the original function handle the rest return super().post(self, request, args, kwargs)