Add check to force users to change password (#2284)

src/pretix/base/forms/user.py

@@ -55,6 +55,7 @@ class UserSettingsForm(forms.ModelForm):
         'pw_current_wrong': _("The current password you entered was not correct."),
         'pw_mismatch': _("Please enter the same password twice"),
         'rate_limit': _("For security reasons, please wait 5 minutes before you try again."),
+        'pw_equal': _("Please choose a password different to your current one.")
     }
 
     old_pw = forms.CharField(max_length=255,
@@ -158,6 +159,12 @@ class UserSettingsForm(forms.ModelForm):
                 code='pw_current'
             )
 
+        if password1 and password1 == old_pw:
+            raise forms.ValidationError(
+                self.error_messages['pw_equal'],
+                code='pw_equal'
+            )
+
         if password1:
             self.instance.set_password(password1)
 

src/pretix/base/migrations/0202_user_needs_password_change.py

@@ -0,0 +1,18 @@
+# Generated by Django 3.2.9 on 2021-11-04 13:05
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('pretixbase', '0201_invoiceline_event_location'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='user',
+            name='needs_password_change',
+            field=models.BooleanField(default=False),
+        ),
+    ]

src/pretix/base/models/auth.py

@@ -113,6 +113,8 @@ class User(AbstractBaseUser, PermissionsMixin, LoggingMixin):
     :type date_joined: datetime
     :param locale: The user's preferred locale code.
     :type locale: str
+    :param needs_password_change: Whether this user's password needs to be changed.
+    :type needs_password_change: bool
     :param timezone: The user's preferred timezone.
     :type timezone: str
     """
@@ -130,6 +132,8 @@ class User(AbstractBaseUser, PermissionsMixin, LoggingMixin):
                                    verbose_name=_('Is site admin'))
     date_joined = models.DateTimeField(auto_now_add=True,
                                        verbose_name=_('Date joined'))
+    needs_password_change = models.BooleanField(default=False,
+                                                verbose_name=_('Force user to select a new password'))
     locale = models.CharField(max_length=50,
                               choices=settings.LANGUAGES,
                               default=settings.LANGUAGE_CODE,

src/pretix/control/forms/users.py

@@ -70,6 +70,7 @@ class UserEditForm(forms.ModelForm):
             'require_2fa',
             'is_active',
             'is_staff',
+            'needs_password_change',
             'last_login'
         ]
 

src/pretix/control/middleware.py

@@ -69,6 +69,11 @@ class PermissionMiddleware:
         "user.settings.notifications.off",
     )
 
+    EXCEPTIONS_FORCED_PW_CHANGE = (
+        "user.settings",
+        "auth.logout"
+    )
+
     EXCEPTIONS_2FA = (
         "user.settings.2fa",
         "user.settings.2fa.add",
@@ -130,6 +135,9 @@ class PermissionMiddleware:
             if url_name not in ('user.reauth', 'auth.logout'):
                 return redirect(reverse('control:user.reauth') + '?next=' + quote(request.get_full_path()))
 
+        if request.user.needs_password_change and url_name not in self.EXCEPTIONS_FORCED_PW_CHANGE:
+            return redirect(reverse('control:user.settings') + '?next=' + quote(request.get_full_path()))
+
         if not request.user.require_2fa and settings.PRETIX_OBLIGATORY_2FA \
                 and url_name not in self.EXCEPTIONS_2FA:
             return redirect(reverse('control:user.settings.2fa'))

src/pretix/control/templates/pretixcontrol/base.html

@@ -429,6 +429,15 @@
                         </div>
                     {% endif %}
 
+                    {% if request.user.needs_password_change %}
+                        <div class="alert alert-warning">
+                            {% blocktrans trimmed %}
+                                For security reasons, please change your password before you continue. Afterwards you
+                                will be redirected to your original destination.
+                            {% endblocktrans %}
+                        </div>
+                    {% endif %}
+
                     {% block content %}
                     {% endblock %}
                     <footer>

src/pretix/control/templates/pretixcontrol/users/create.html

@@ -19,6 +19,7 @@
             {% bootstrap_field form.email layout='control' %}
             {% bootstrap_field form.new_pw layout='control' %}
             {% bootstrap_field form.new_pw_repeat layout='control' %}
+            {% bootstrap_field form.needs_password_change layout='control' %}
         </fieldset>
         <div class="form-group submit-group">
             <button type="submit" class="btn btn-primary btn-save">

src/pretix/control/templates/pretixcontrol/users/form.html

@@ -45,6 +45,7 @@
                     {% endif %}
                     {% bootstrap_field form.last_login layout='control' %}
                     {% bootstrap_field form.require_2fa layout='control' %}
+                    {% bootstrap_field form.needs_password_change layout='control' %}
                 </fieldset>
                 <fieldset>
                     <legend>{% trans "Team memberships" %}</legend>

src/pretix/control/views/user.py

@@ -226,6 +226,7 @@ class UserSettings(UpdateView):
         msgs = []
 
         if 'new_pw' in form.changed_data:
+            self.request.user.needs_password_change = False
             msgs.append(_('Your password has been changed.'))
 
         if 'email' in form.changed_data:
@@ -243,6 +244,8 @@ class UserSettings(UpdateView):
         return sup
 
     def get_success_url(self):
+        if "next" in self.request.GET and url_has_allowed_host_and_scheme(self.request.GET.get("next"), allowed_hosts=None):
+            return self.request.GET.get("next")
         return reverse('control:user.settings')
 
 

src/tests/control/test_auth.py

@@ -787,7 +787,9 @@ class SessionTimeOutTest(TestCase):
         assert self.client.session['pretix_auth_last_used'] > t1
 
     def test_pinned_user_agent(self):
-        self.client.defaults['HTTP_USER_AGENT'] = 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/64.0.3282.140 Safari/537.36'
+        self.client.defaults['HTTP_USER_AGENT'] = 'Mozilla/5.0 (X11; Linux x86_64) ' \
+                                                  'AppleWebKit/537.36 (KHTML, like Gecko) ' \
+                                                  'Chrome/64.0.3282.140 Safari/537.36'
         response = self.client.get('/control/')
         self.assertEqual(response.status_code, 200)
 
@@ -927,3 +929,45 @@ class Obligatory2FATest(TestCase):
 
         response = self.client.get('/control/events/')
         assert response.status_code == 200
+
+
+class PasswordChangeRequiredTest(TestCase):
+    def setUp(self):
+        super().setUp()
+        self.user = User.objects.create_user('dummy@dummy.dummy', 'dummy')
+
+    def test_redirect_to_settings(self):
+        self.user.needs_password_change = True
+        self.user.save()
+        self.client.login(email='dummy@dummy.dummy', password='dummy')
+
+        response = self.client.get('/control/events/')
+
+        self.assertEqual(response.status_code, 302)
+        assert self.user.needs_password_change is True
+        self.assertIn('/control/settings?next=/control/events/', response['Location'])
+
+    def test_redirect_to_2fa_to_settings(self):
+        self.user.require_2fa = True
+        self.user.needs_password_change = True
+        self.user.save()
+
+        response = self.client.post('/control/login?next=/control/events/', {
+            'email': 'dummy@dummy.dummy',
+            'password': 'dummy',
+        })
+
+        self.assertEqual(response.status_code, 302)
+        self.assertIn('/control/login/2fa?next=/control/events/', response['Location'])
+
+        d = TOTPDevice.objects.create(user=self.user, name='test')
+        totp = TOTP(d.bin_key, d.step, d.t0, d.digits, d.drift)
+        totp.time = time.time()
+
+        self.client.post('/control/login/2fa?next=/control/events/'.format(d.pk), {
+            'token': str(totp.token())
+        })
+        response = self.client.get('/control/events/')
+
+        self.assertEqual(response.status_code, 302)
+        self.assertIn('/control/settings?next=/control/events/', response['Location'])

src/tests/control/test_user.py

@@ -51,8 +51,8 @@ from pretix.testutils.mock import mocker_context
 class UserSettingsTest(SoupTest):
     def setUp(self):
         super().setUp()
-        self.user = User.objects.create_user('dummy@dummy.dummy', 'dummy')
-        self.client.login(email='dummy@dummy.dummy', password='dummy')
+        self.user = User.objects.create_user('dummy@dummy.dummy', 'barfoofoo')
+        self.client.login(email='dummy@dummy.dummy', password='barfoofoo')
         doc = self.get_doc('/control/settings')
         self.form_data = extract_form_fields(doc.select('.container-fluid form')[0])
 
@@ -80,7 +80,7 @@ class UserSettingsTest(SoupTest):
     def test_change_email_success(self):
         doc = self.save({
             'email': 'foo@example.com',
-            'old_pw': 'dummy'
+            'old_pw': 'barfoofoo'
         })
         assert doc.select(".alert-success")
         self.user = User.objects.get(pk=self.user.pk)
@@ -90,7 +90,7 @@ class UserSettingsTest(SoupTest):
         User.objects.create_user('foo@example.com', 'foo')
         doc = self.save({
             'email': 'foo@example.com',
-            'old_pw': 'dummy'
+            'old_pw': 'barfoofoo'
         })
         assert doc.select(".alert-danger")
         self.user = User.objects.get(pk=self.user.pk)
@@ -112,7 +112,7 @@ class UserSettingsTest(SoupTest):
         self.save({
             'new_pw': 'foobarbar',
             'new_pw_repeat': 'foobarbar',
-            'old_pw': 'dummy',
+            'old_pw': 'barfoofoo',
         })
         pw = self.user.password
         self.user = User.objects.get(pk=self.user.pk)
@@ -122,7 +122,7 @@ class UserSettingsTest(SoupTest):
         doc = self.save({
             'new_pw': 'foobarbar',
             'new_pw_repeat': 'foobarbar',
-            'old_pw': 'dummy',
+            'old_pw': 'barfoofoo',
         })
         assert doc.select(".alert-success")
         self.user = User.objects.get(pk=self.user.pk)
@@ -132,7 +132,7 @@ class UserSettingsTest(SoupTest):
         doc = self.save({
             'new_pw': 'foo',
             'new_pw_repeat': 'foo',
-            'old_pw': 'dummy',
+            'old_pw': 'barfoofoo',
         })
         assert doc.select(".alert-danger")
         pw = self.user.password
@@ -143,7 +143,7 @@ class UserSettingsTest(SoupTest):
         doc = self.save({
             'new_pw': 'dummy123',
             'new_pw_repeat': 'dummy123',
-            'old_pw': 'dummy',
+            'old_pw': 'barfoofoo',
         })
         assert doc.select(".alert-danger")
         pw = self.user.password
@@ -154,13 +154,44 @@ class UserSettingsTest(SoupTest):
         doc = self.save({
             'new_pw': 'foooooooooooooo',
             'new_pw_repeat': 'baaaaaaaaaaaar',
-            'old_pw': 'dummy',
+            'old_pw': 'barfoofoo',
         })
         assert doc.select(".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_new(self):
+        doc = self.save({
+            'new_pw': 'barfoofoo',
+            'new_pw_repeat': 'barfoofoo',
+            'old_pw': 'barfoofoo',
+        })
+        assert doc.select(".alert-danger")
+
+    def test_needs_password_change(self):
+        self.user.needs_password_change = True
+        self.user.save()
+        doc = self.save({
+            'email': 'foo@example.com',
+            'old_pw': 'barfoofoo'
+        })
+        assert doc.select(".alert-success")
+        assert doc.select(".alert-warning")
+        self.user.refresh_from_db()
+        assert self.user.needs_password_change is True
+
+    def test_needs_password_change_changed(self):
+        self.user.needs_password_change = True
+        self.user.save()
+        self.save({
+            'new_pw': 'foobarbar',
+            'new_pw_repeat': 'foobarbar',
+            'old_pw': 'barfoofoo'
+        })
+        self.user.refresh_from_db()
+        assert self.user.needs_password_change is False
+
 
 @pytest.fixture
 def class_monkeypatch(request, monkeypatch):