Added basic Django password validations and updated .gitignore (#136)

.gitignore

@@ -18,4 +18,6 @@ _static/
 .secret
 atlassian-ide-plugin.xml
 pretixeu/
+.project
+.pydevproject
 

src/pretix/base/forms/auth.py

@@ -1,5 +1,8 @@
 from django import forms
 from django.contrib.auth import authenticate
+from django.contrib.auth.password_validation import (
+    password_validators_help_texts, validate_password,
+)
 from django.utils.translation import ugettext_lazy as _
 
 from pretix.base.models import User
@@ -84,7 +87,7 @@ class RegistrationForm(forms.Form):
     )
 
     def clean(self):
-        password1 = self.cleaned_data.get('password')
+        password1 = self.cleaned_data.get('password', '')
         password2 = self.cleaned_data.get('password_repeat')
 
         if password1 and password1 != password2:
@@ -93,6 +96,12 @@ class RegistrationForm(forms.Form):
                 code='pw_mismatch'
             )
 
+        user = User(email=self.cleaned_data.get('email'))
+        if validate_password(password1, user=user) is not None:
+            raise forms.ValidationError(
+                _(password_validators_help_texts()),
+                code='pw_invalid'
+            )
         return self.cleaned_data
 
     def clean_email(self):
@@ -107,7 +116,7 @@ class RegistrationForm(forms.Form):
 
 class PasswordRecoverForm(forms.Form):
     error_messages = {
-        'pw_mismatch': _("Please enter the same password twice")
+        'pw_mismatch': _("Please enter the same password twice"),
     }
     password = forms.CharField(
         label=_('Password'),
@@ -119,11 +128,12 @@ class PasswordRecoverForm(forms.Form):
         widget=forms.PasswordInput
     )
 
-    def __init__(self, *args, **kwargs):
+    def __init__(self, user_id=None, *args, **kwargs):
+        self.user_id = user_id
         super().__init__(*args, **kwargs)
 
     def clean(self):
-        password1 = self.cleaned_data.get('password')
+        password1 = self.cleaned_data.get('password', '')
         password2 = self.cleaned_data.get('password_repeat')
 
         if password1 and password1 != password2:
@@ -132,6 +142,16 @@ class PasswordRecoverForm(forms.Form):
                 code='pw_mismatch'
             )
 
+        try:
+            user = User.objects.get(id=self.user_id)
+        except User.DoesNotExist:
+            user = None
+        if validate_password(password1, user=user) is not None:
+            raise forms.ValidationError(
+                _(password_validators_help_texts()),
+                code='pw_invalid'
+            )
+
         return self.cleaned_data
 
 

src/pretix/base/forms/user.py

@@ -1,5 +1,8 @@
 from django import forms
 from django.contrib.auth.hashers import check_password
+from django.contrib.auth.password_validation import (
+    password_validators_help_texts, validate_password,
+)
 from django.db.models import Q
 from django.utils.translation import ugettext_lazy as _
 
@@ -66,6 +69,15 @@ class UserSettingsForm(forms.ModelForm):
             )
         return email
 
+    def clean_new_pw(self):
+        password1 = self.cleaned_data.get('new_pw', '')
+        if password1 and validate_password(password1, user=self.user) is not None:
+            raise forms.ValidationError(
+                _(password_validators_help_texts()),
+                code='pw_invalid'
+            )
+        return password1
+
     def clean_new_pw_repeat(self):
         password1 = self.cleaned_data.get('new_pw')
         password2 = self.cleaned_data.get('new_pw_repeat')

src/pretix/control/views/auth.py

@@ -161,7 +161,8 @@ class Recover(TemplateView):
 
     @cached_property
     def form(self):
-        return PasswordRecoverForm(data=self.request.POST if self.request.method == 'POST' else None)
+        return PasswordRecoverForm(data=self.request.POST if self.request.method == 'POST' else None,
+                                   user_id=self.request.GET.get('id'))
 
     def get_context_data(self, **kwargs):
         context = super().get_context_data(**kwargs)

src/pretix/settings.py

@@ -388,3 +388,18 @@ CELERY_RESULT_SERIALIZER = 'pickle'
 BOOTSTRAP3 = {
     'success_css_class': ''
 }
+
+AUTH_PASSWORD_VALIDATORS = [
+    {
+        'NAME': 'django.contrib.auth.password_validation.UserAttributeSimilarityValidator',
+    },
+    {
+        'NAME': 'django.contrib.auth.password_validation.MinimumLengthValidator',
+    },
+    {
+        'NAME': 'django.contrib.auth.password_validation.CommonPasswordValidator',
+    },
+    {
+        'NAME': 'django.contrib.auth.password_validation.NumericPasswordValidator',
+    },
+]

src/tests/control/test_auth.py

@@ -114,20 +114,88 @@ class RegistrationFormTest(TestCase):
         })
         self.assertEqual(response.status_code, 200)
 
+    def test_user_attribute_similarity_passwords(self):
+        response = self.client.post('/control/register', {
+            'email': 'dummy@dummy.dummy',
+            'password': 'dummydummy',
+            'password_repeat': 'dummydummy'
+        })
+        self.assertEqual(response.status_code, 200)
+
+    def test_short_passwords(self):
+        response = self.client.post('/control/register', {
+            'email': 'dummy@dummy.dummy',
+            'password': 'foobar',
+            'password_repeat': 'foobar'
+        })
+        self.assertEqual(response.status_code, 200)
+
+    def test_common_passwords(self):
+        response = self.client.post('/control/register', {
+            'email': 'dummy@dummy.dummy',
+            'password': 'password',
+            'password_repeat': 'password'
+        })
+        self.assertEqual(response.status_code, 200)
+
+        response = self.client.post('/control/register', {
+            'email': 'dummy@dummy.dummy',
+            'password': 'football',
+            'password_repeat': 'football'
+        })
+        self.assertEqual(response.status_code, 200)
+
+        response = self.client.post('/control/register', {
+            'email': 'dummy@dummy.dummy',
+            'password': 'jennifer',
+            'password_repeat': 'jennifer'
+        })
+        self.assertEqual(response.status_code, 200)
+
+    def test_numeric_passwords(self):
+        response = self.client.post('/control/register', {
+            'email': 'dummy@dummy.dummy',
+            'password': '12345678',
+            'password_repeat': '12345678'
+        })
+        self.assertEqual(response.status_code, 200)
+
+        response = self.client.post('/control/register', {
+            'email': 'dummy@dummy.dummy',
+            'password': '23423523452345235',
+            'password_repeat': '23423523452345235'
+        })
+        self.assertEqual(response.status_code, 200)
+
+    def test_empty_passwords(self):
+        response = self.client.post('/control/register', {
+            'email': 'dummy@dummy.dummy',
+            'password': '',
+            'password_repeat': ''
+        })
+        self.assertEqual(response.status_code, 200)
+
+        response = self.client.post('/control/register', {
+            'email': 'dummy@dummy.dummy',
+            'password': 'foobarbar',
+            'password_repeat': ''
+        })
+        self.assertEqual(response.status_code, 200)
+
     def test_email_duplicate(self):
         self.user = User.objects.create_user('dummy@dummy.dummy', 'dummy')
         response = self.client.post('/control/register', {
             'email': 'dummy@dummy.dummy',
-            'password': 'foo',
+            'password': 'foobarbar',
-            'password_repeat': 'foo'
+            'password_repeat': 'foobarbar'
         })
         self.assertEqual(response.status_code, 200)
 
     def test_success(self):
         response = self.client.post('/control/register', {
             'email': 'dummy@dummy.dummy',
-            'password': 'foo',
+            'password': 'foobarbar',
-            'password_repeat': 'foo'
+            'password_repeat': 'foobarbar'
         })
         self.assertEqual(response.status_code, 302)
 
@@ -175,8 +243,8 @@ class PasswordRecoveryFormTest(TestCase):
         response = self.client.post(
             '/control/forgot/recover?id=%d&token=foo' % self.user.id,
             {
-                'password': 'foobar',
+                'password': 'foobarbar',
-                'password_repeat': 'foobar'
+                'password_repeat': 'foobarbar'
             }
         )
         self.assertEqual(response.status_code, 302)
@@ -197,8 +265,8 @@ class PasswordRecoveryFormTest(TestCase):
         response = self.client.post(
             '/control/forgot/recover?id=%d&token=%s' % (self.user.id, token),
             {
-                'password': 'foobar',
+                'password': 'foobarbar',
-                'password_repeat': 'foobar'
+                'password_repeat': 'foobarbar'
             }
         )
         self.assertEqual(response.status_code, 302)
@@ -212,15 +280,29 @@ class PasswordRecoveryFormTest(TestCase):
         response = self.client.post(
             '/control/forgot/recover?id=%d&token=%s' % (self.user.id, token),
             {
-                'password': 'foobar',
+                'password': 'foobarbar',
-                'password_repeat': 'foobar'
+                'password_repeat': 'foobarbar'
             }
         )
         self.assertEqual(response.status_code, 302)
         self.user = User.objects.get(id=self.user.id)
-        self.assertTrue(self.user.check_password('foobar'))
+        self.assertTrue(self.user.check_password('foobarbar'))
 
     def test_recovery_valid_token_empty_passwords(self):
+        token = default_token_generator.make_token(self.user)
+        response = self.client.get('/control/forgot/recover?id=%d&token=%s' % (self.user.id, token))
+        self.assertEqual(response.status_code, 200)
+        response = self.client.post(
+            '/control/forgot/recover?id=%d&token=%s' % (self.user.id, token),
+            {
+                'password': 'foobarbar',
+                'password_repeat': ''
+            }
+        )
+        self.assertEqual(response.status_code, 200)
+        self.user = User.objects.get(id=self.user.id)
+        self.assertTrue(self.user.check_password('demo'))
+
         token = default_token_generator.make_token(self.user)
         response = self.client.get('/control/forgot/recover?id=%d&token=%s' % (self.user.id, token))
         self.assertEqual(response.status_code, 200)
@@ -228,7 +310,7 @@ class PasswordRecoveryFormTest(TestCase):
             '/control/forgot/recover?id=%d&token=%s' % (self.user.id, token),
             {
                 'password': '',
-                'password_repeat': 'foobar'
+                'password_repeat': 'foobarbar'
             }
         )
         self.assertEqual(response.status_code, 200)
@@ -249,3 +331,63 @@ class PasswordRecoveryFormTest(TestCase):
         self.assertEqual(response.status_code, 200)
         self.user = User.objects.get(id=self.user.id)
         self.assertTrue(self.user.check_password('demo'))
+
+    def test_recovery_valid_token_user_attribute_similarity_passwords(self):
+        token = default_token_generator.make_token(self.user)
+        response = self.client.get('/control/forgot/recover?id=%d&token=%s' % (self.user.id, token))
+        self.assertEqual(response.status_code, 200)
+        response = self.client.post(
+            '/control/forgot/recover?id=%d&token=%s' % (self.user.id, token),
+            {
+                'password': 'dummydemo',
+                'password_repeat': 'dummydemo'
+            }
+        )
+        self.assertEqual(response.status_code, 200)
+        self.user = User.objects.get(id=self.user.id)
+        self.assertTrue(self.user.check_password('demo'))
+
+    def test_recovery_valid_token_short_passwords(self):
+        token = default_token_generator.make_token(self.user)
+        response = self.client.get('/control/forgot/recover?id=%d&token=%s' % (self.user.id, token))
+        self.assertEqual(response.status_code, 200)
+        response = self.client.post(
+            '/control/forgot/recover?id=%d&token=%s' % (self.user.id, token),
+            {
+                'password': 'foobar',
+                'password_repeat': 'foobar'
+            }
+        )
+        self.assertEqual(response.status_code, 200)
+        self.user = User.objects.get(id=self.user.id)
+        self.assertTrue(self.user.check_password('demo'))
+
+    def test_recovery_valid_token_common_passwords(self):
+        token = default_token_generator.make_token(self.user)
+        response = self.client.get('/control/forgot/recover?id=%d&token=%s' % (self.user.id, token))
+        self.assertEqual(response.status_code, 200)
+        response = self.client.post(
+            '/control/forgot/recover?id=%d&token=%s' % (self.user.id, token),
+            {
+                'password': 'football',
+                'password_repeat': 'football'
+            }
+        )
+        self.assertEqual(response.status_code, 200)
+        self.user = User.objects.get(id=self.user.id)
+        self.assertTrue(self.user.check_password('demo'))
+
+    def test_recovery_valid_token_numeric_passwords(self):
+        token = default_token_generator.make_token(self.user)
+        response = self.client.get('/control/forgot/recover?id=%d&token=%s' % (self.user.id, token))
+        self.assertEqual(response.status_code, 200)
+        response = self.client.post(
+            '/control/forgot/recover?id=%d&token=%s' % (self.user.id, token),
+            {
+                'password': '12345678',
+                'password_repeat': '12345678'
+            }
+        )
+        self.assertEqual(response.status_code, 200)
+        self.user = User.objects.get(id=self.user.id)
+        self.assertTrue(self.user.check_password('demo'))

src/tests/control/test_user.py

@@ -67,13 +67,33 @@ class UserSettingsTest(BrowserTest):
         assert self.user.password == pw
 
     def test_change_password_success(self):
-        self.driver.find_element_by_name("new_pw").send_keys("foo")
+        self.driver.find_element_by_name("new_pw").send_keys("foobarbar")
-        self.driver.find_element_by_name("new_pw_repeat").send_keys("foo")
+        self.driver.find_element_by_name("new_pw_repeat").send_keys("foobarbar")
         self.driver.find_element_by_name("old_pw").send_keys("dummy")
         self.scroll_and_click(self.driver.find_element_by_class_name('btn-save'))
         self.driver.find_element_by_class_name("alert-success")
         self.user = User.objects.get(pk=self.user.pk)
-        assert self.user.check_password("foo")
+        assert self.user.check_password("foobarbar")
+
+    def test_change_password_short(self):
+        self.driver.find_element_by_name("new_pw").send_keys("foobar")
+        self.driver.find_element_by_name("new_pw_repeat").send_keys("foobar")
+        self.driver.find_element_by_name("old_pw").send_keys("dummy")
+        self.scroll_and_click(self.driver.find_element_by_class_name('btn-save'))
+        self.driver.find_element_by_class_name("alert-danger")
+        pw = self.user.password
+        self.user = User.objects.get(pk=self.user.pk)
+        assert self.user.password == pw
+
+    def test_change_password_user_attribute_similarity(self):
+        self.driver.find_element_by_name("new_pw").send_keys("dummy123")
+        self.driver.find_element_by_name("new_pw_repeat").send_keys("dummy123")
+        self.driver.find_element_by_name("old_pw").send_keys("dummy")
+        self.scroll_and_click(self.driver.find_element_by_class_name('btn-save'))
+        self.driver.find_element_by_class_name("alert-danger")
+        pw = self.user.password
+        self.user = User.objects.get(pk=self.user.pk)
+        assert self.user.password == pw
 
     def test_change_password_require_repeat(self):
         self.driver.find_element_by_name("new_pw").send_keys("foo")