dav_auth: small refactorings and improvements under the hood

dav_auth/apps.py

@@ -3,6 +3,11 @@ from dav_base.config.apps import AppConfig as _AppConfig, DefaultSetting
 DEFAULT_SETTINGS = (
     DefaultSetting('login_redirect_url', 'root'),
     DefaultSetting('logout_redirect_url', 'root'),
+    DefaultSetting('auto_password_length', 32),
+    DefaultSetting('auto_password_characters', 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'
+        'abcdefghijklmnopqrstuvwxyz'
+        '0123456789'
+        '#$%&@^~.,:;/_-*+!?'),
 )
 
 

dav_auth/django_project_config/settings-dav_auth.py

@@ -1,2 +1,4 @@
 # LOGIN_REDIRECT_URL = 'root'
 # LOGOUT_REDIRECT_URL = 'root'
+# AUTO_PASSWORD_LENGTH = 32
+# AUTO_PASSWORD_CHARACTERS = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789#$%&@^~.,:;/_-*+!?'

dav_auth/templates/dav_auth/includes/weak_password_warning.html

@@ -0,0 +1,11 @@
+{% load bootstrap3 %}
+{% load i18n %}
+<br />
+<p>
+    Dein Passwort entspricht nicht mehr den aktuellen Passwortrichtlinien.<br />
+    Bitte hilf uns die Daten deiner Teilnehmer zu schützen und ändere dein Passwort.<br />
+</p>
+<p>
+    <a href="{% url 'dav_auth:set_password' %}">{% trans 'Passwort ändern' %}</a>
+</p>
+<br />

dav_auth/tests/test_apps.py

@@ -9,4 +9,9 @@ class TestCase(AppsTestCase):
     settings = (
         AppSetting('login_redirect_url', 'root', str),
         AppSetting('logout_redirect_url', 'root', str),
+        AppSetting('auto_password_length', 32, int),
+        AppSetting('auto_password_characters', 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'
+                                               'abcdefghijklmnopqrstuvwxyz'
+                                               '0123456789'
+                                               '#$%&@^~.,:;/_-*+!?', str),
     )

dav_auth/tests/test_screenshots.py

@@ -9,7 +9,8 @@ from selenium.webdriver.common.keys import Keys
 from dav_base.tests.generic import ScreenshotTestCase
 
 TEST_USERNAME = 'root@localhost'
-TEST_PASSWORD = 'me||ön 21ABll'
+TEST_STRONG_PASSWORD = 'me||ön 21ABll'
+TEST_WEAK_PASSWORD = 'mellon'
 TEST_EMAIL = TEST_USERNAME
 
 
@@ -20,10 +21,11 @@ class TestCase(ScreenshotTestCase):
     def setUp(self):
         super().setUp()
         # Need a test user
+        # (we start with a weak password, because we want to see the weak password warning)
         self.test_username = TEST_USERNAME
-        self.test_password = TEST_PASSWORD
+        self.test_password = TEST_WEAK_PASSWORD
         model = get_user_model()
-        self.user = model.objects.create_user(username=TEST_USERNAME, password=TEST_PASSWORD, email=TEST_EMAIL)
+        self.user = model.objects.create_user(username=TEST_USERNAME, password=TEST_WEAK_PASSWORD, email=TEST_EMAIL)
 
     def test_screenshots(self):
         sequence_name = 'walkthrough'
@@ -86,7 +88,7 @@ class TestCase(ScreenshotTestCase):
         self.user.is_active = True
         self.user.save()
 
-        # Login -> save success message
+        # Login -> save success message and weak password warning
         username_field = c.find_element(By.ID, 'id_username')
         username_field.clear()
         username_field.send_keys(self.test_username)
@@ -95,7 +97,7 @@ class TestCase(ScreenshotTestCase):
         password_field.send_keys(self.test_password)
         password_field.send_keys(Keys.RETURN)
         alert_button = self.wait_on_presence(c, (By.CSS_SELECTOR, '#messages .alert-success button.close'))
-        self.save_screenshot('login_succeed', sequence=sequence_name)
+        self.save_screenshot('login_weak_password_succeed', sequence=sequence_name)
         alert_button.click()
 
         # Open user dropdown menu -> save menu
@@ -122,9 +124,9 @@ class TestCase(ScreenshotTestCase):
 
         # New passwords mismatch -> save error message
         password_field.clear()
-        password_field.send_keys(self.test_password)
+        password_field.send_keys(TEST_STRONG_PASSWORD)
         password2_field.clear()
-        password2_field.send_keys(self.test_password[::-1])
+        password2_field.send_keys(TEST_WEAK_PASSWORD)
         password2_field.send_keys(Keys.RETURN)
         self.wait_until_stale(c, password2_field)
         self.save_screenshot('error_mismatch', sequence=sequence_name)
@@ -166,7 +168,7 @@ class TestCase(ScreenshotTestCase):
         self.save_screenshot('error_too_similar', sequence=sequence_name)
 
         # Change password -> save success message
-        password = self.test_password[::-1]
+        password = TEST_STRONG_PASSWORD
         password_field = c.find_element(By.ID, 'id_new_password')
         password_field.clear()
         password_field.send_keys(password)
@@ -176,6 +178,7 @@ class TestCase(ScreenshotTestCase):
         password2_field.send_keys(Keys.RETURN)
         self.wait_until_stale(c, password2_field)
         self.save_screenshot('set_password_succeed', sequence=sequence_name)
+        self.test_password = password
 
         # Get password recreate page -> since we are logged in, it should
         # redirect to set password page again -> save
@@ -196,6 +199,31 @@ class TestCase(ScreenshotTestCase):
         self.wait_until_stale(c, user_menu)
         self.save_screenshot('logout_succeed', sequence=sequence_name)
 
+        # Login again, this time with a new strong password
+        link = c.find_element(By.CSS_SELECTOR, '#login-widget a')
+        link.click()
+        self.wait_on_presence(c, (By.ID, 'id_username'))
+        username_field = c.find_element(By.ID, 'id_username')
+        username_field.clear()
+        username_field.send_keys(self.test_username)
+        password_field = c.find_element(By.ID, 'id_password')
+        password_field.clear()
+        password_field.send_keys(self.test_password)
+        password_field.send_keys(Keys.RETURN)
+        alert_button = self.wait_on_presence(c, (By.CSS_SELECTOR, '#messages .alert-success button.close'))
+        self.save_screenshot('login_strong_password_succeed', sequence=sequence_name)
+        alert_button.click()
+
+        # Logout again
+        dropdown_button = self.wait_on_presence(c, (By.ID, 'user_dropdown_button'))
+        dropdown_button.click()
+        user_menu = c.find_element(By.CSS_SELECTOR, '#login-widget ul')
+        #link = user_menu.find_element(By.PARTIAL_LINK_TEXT, gettext('Logout'))
+        #link.click()
+        button = c.find_element(By.ID, 'id_logout_button')
+        button.click()
+        self.wait_until_stale(c, user_menu)
+
         # Click on 'login' to access password recreate link
         link = c.find_element(By.CSS_SELECTOR, '#login-widget a')
         link.click()
@@ -207,7 +235,7 @@ class TestCase(ScreenshotTestCase):
         username_field = self.wait_on_presence(c, (By.ID, 'id_username'))
         self.save_screenshot('empty_recreate_password_form', sequence=sequence_name)
 
-        # Enter invalid username -> save result (login form, no message)
+        # Enter invalid username -> save result (login form, success message - do not reveal user does not exist)
         username_field.send_keys(self.test_username[::-1])
         username_field.send_keys(Keys.RETURN)
         self.wait_until_stale(c, username_field)

dav_auth/tests/test_views.py

@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
 from django.apps import apps
 from django.contrib.auth import get_user_model
+from django.contrib.auth.password_validation import validate_password
 from django.contrib.messages import get_messages
 from django.core import mail as django_mail
 from django.shortcuts import resolve_url
@@ -115,7 +116,7 @@ class ViewsTestCase(TestCase):
 
         with self.assertLogs('dav_auth.views', level='WARNING') as cm:
             response = self.client.post(self.login_url, {'username': username, 'password': password})
-        self.assertStartsWith(cm.output[0], 'WARNING:dav_auth.views:Weak password')
+        self.assertStartsWith(cm.output[0], 'WARNING:dav_auth.views:Detected weak password for user id')
         self.assertEqual(response.status_code, 302)
         self.assertEqual(response.url, self.login_redirect_url)
 
@@ -248,6 +249,7 @@ class ViewsTestCase(TestCase):
         location = self.recreate_password_url
 
         response = self.client.post(location, {'username': self.user.username})
+        new_password = response.context['password']
         messages = list(get_messages(response.wsgi_request))
         self.assertEqual(len(messages), 1)
         self.assertEqual(messages[0].message, self.new_password_sent_message)
@@ -259,6 +261,7 @@ class ViewsTestCase(TestCase):
         recipients = mail.recipients()
         self.assertIn(recipient, recipients)
         self.assertEqual(len(recipients), 1)
+        self.assertIn(new_password, mail.body)
 
         response = self.client.get(location)
         self.assertFalse(response.context['user'].is_authenticated, 'User is logged in')
@@ -276,3 +279,18 @@ class ViewsTestCase(TestCase):
         self.assertRedirects(response, self.login_url)
 
         self.assertEqual(len(django_mail.outbox), 0)
+
+    def test_new_password_length(self):
+        location = self.recreate_password_url
+
+        default_password_length = 32
+
+        response = self.client.post(location, {'username': self.user.username})
+        new_password = response.context['password']
+        self.assertEqual(len(new_password), default_password_length)
+
+    def test_new_password_is_valid(self):
+        location = self.recreate_password_url
+        response = self.client.post(location, {'username': self.user.username})
+        new_password = response.context['password']
+        validate_password(new_password)

dav_auth/views.py

@@ -8,6 +8,7 @@ from django.contrib.auth import views as auth_views, get_user_model
 from django.contrib.auth.password_validation import validate_password
 from django.http import HttpResponseRedirect
 from django.shortcuts import resolve_url
+from django.template.loader import render_to_string
 from django.urls import reverse_lazy, reverse
 from django.utils.safestring import mark_safe
 from django.utils.translation import gettext as _
@@ -23,6 +24,7 @@ logger = logging.getLogger(__name__)
 class LoginView(auth_views.LoginView):
     form_class = forms.LoginForm
     template_name = 'dav_auth/forms/login.html'
+    weak_password_warning_template_name = 'dav_auth/includes/weak_password_warning.html'
 
     def get_redirect_url(self):
         url = super().get_redirect_url()
@@ -36,14 +38,8 @@ class LoginView(auth_views.LoginView):
         try:
             validate_password(form.cleaned_data['password'])
         except ValidationError as e:
-            logger.warning('Weak password (%d): %s', self.request.user.pk, e)
+            logger.warning('Detected weak password for user id %d: %s', self.request.user.pk, e)
-            message = '<br />\n<p>\n'
+            message = render_to_string(self.weak_password_warning_template_name)
-            message += 'Dein Passwort entspricht nicht mehr den aktuellen Passwortrichtlinien.<br />\n'
-            message += 'Bitte hilf uns die Daten deiner Teilnehmer zu schützen und ändere dein Passwort.<br />\n'
-            message += '</p>\n'
-            message += '<p>\n'
-            message += '<a href="%(href)s">Passwort ändern</a>\n' % {'href': reverse('dav_auth:set_password')}
-            message += '</p>\n<br />\n'
             messages.warning(self.request, mark_safe(message))
         return r
 
@@ -72,7 +68,7 @@ class SetPasswordView(auth_views.PasswordChangeView):
     def form_valid(self, form):
         r = super().form_valid(form)
         messages.success(self.request, _('Passwort gespeichert.'))
-        logger.info('Changed Password for user \'%s\'', self.request.user)
+        logger.info('Changed password for user \'%s\'', self.request.user)
         if form.cleaned_data.get('send_password_mail', False):
             email = emails.PasswordSetEmail(self.request.user, form.cleaned_data['new_password'])
             email.send()
@@ -83,24 +79,34 @@ class CreateAndSendPasswordView(generic.FormView):
     form_class = forms.CreateAndSendPasswordForm
     template_name = 'dav_auth/forms/recreate_password.html'
     success_url = reverse_lazy('dav_auth:login')
-    password_chars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789#$%&@^~.,:;/_-*+!?'
+    password_length = app_config.settings.auto_password_length
-    password_length = 32
+    password_chars = app_config.settings.auto_password_characters
+
+    def _create_new_password(self, length=None, characters=None):
+        if length is None:
+            length = self.password_length
+        if characters is None:
+            characters = self.password_chars
+        return ''.join(secrets.choice(characters) for i in range(length))
 
     def form_valid(self, form):
         username = form.cleaned_data.get('username')
         user_model = get_user_model()
+
+        # Generate a new password (even if the user does not exist, to avoid revealing that fact).
+        random_password = self._create_new_password()
+
         try:
             user = user_model.objects.get(username=username)
-            random_password = ''.join(secrets.choice(self.password_chars) for i in range(self.password_length))
             user.set_password(random_password)
             user.save()
             email = emails.PasswordSetEmail(user, random_password)
             email.send()
-            messages.success(self.request, _('Neues Passwort versendet.'))
+            logger.info('Recreated password for user \'%s\'', username)
-            logger.info('Password recreated for user \'%s\'', username)
         except user_model.DoesNotExist:
-            logger.warning('Password recreated for unknown user \'%s\'', username)
+            logger.warning('Recreated password for unknown user \'%s\'', username)
-            # Pretend we sent an email, so we do not reveal that the user doesn't exist.
+
+        # Show message, that we sent an email, even we did not, so we do not reveal that the user doesn't exist.
         messages.success(self.request, _('Neues Passwort versendet.'))
 
         return super().form_valid(form)