diff --git a/src/pretix/base/auth.py b/src/pretix/base/auth.py index fbdb1415a..297905a39 100644 --- a/src/pretix/base/auth.py +++ b/src/pretix/base/auth.py @@ -32,13 +32,16 @@ # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations under the License. +import string from collections import OrderedDict from importlib import import_module from django import forms from django.conf import settings from django.contrib.auth import authenticate -from django.utils.translation import gettext_lazy as _ +from django.contrib.auth.hashers import check_password, make_password +from django.core.exceptions import ValidationError +from django.utils.translation import gettext_lazy as _, ngettext def get_auth_backends(): @@ -160,3 +163,62 @@ class NativeAuthBackend(BaseAuthBackend): u = authenticate(request=request, email=form_data['email'].lower(), password=form_data['password']) if u and u.auth_backend == self.identifier: return u + + +class NumericAndAlphabeticPasswordValidator: + + def validate(self, password, user=None): + has_numeric = any(c in string.digits for c in password) + has_alpha = any(c in string.ascii_letters for c in password) + if not has_numeric or not has_alpha: + raise ValidationError( + _( + "Your password must contain both numeric and alphabetic characters.", + ), + code="password_numeric_and_alphabetic", + ) + + def get_help_text(self): + return _( + "Your password must contain both numeric and alphabetic characters.", + ) + + +class HistoryPasswordValidator: + + def __init__(self, history_length=4): + self.history_length = history_length + + def validate(self, password, user=None): + from pretix.base.models import User + + if not user or not user.pk or not isinstance(user, User): + return + + for hp in user.historic_passwords.order_by("-created")[:self.history_length]: + if check_password(password, hp.password): + raise ValidationError( + ngettext( + "Your password may not be the same as your previous password.", + "Your password may not be the same as one of your %(history_length)s previous passwords.", + self.history_length, + ), + code="password_history", + params={"history_length": self.history_length}, + ) + + def get_help_text(self): + return ngettext( + "Your password may not be the same as your previous password.", + "Your password may not be the same as one of your %(history_length)s previous passwords.", + self.history_length, + ) % {"history_length": self.history_length} + + def password_changed(self, password, user=None): + if not user: + pass + + user.historic_passwords.create(password=make_password(password)) + user.historic_passwords.filter( + pk__in=user.historic_passwords.order_by("-created")[self.history_length:].values_list("pk", flat=True), + ).delete() diff --git a/src/pretix/base/migrations/0270_historicpassword.py b/src/pretix/base/migrations/0270_historicpassword.py new file mode 100644 index 000000000..6a42e15c7 --- /dev/null +++ b/src/pretix/base/migrations/0270_historicpassword.py @@ -0,0 +1,36 @@ +# Generated by Django 4.2.15 on 2024-09-16 15:10 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("pretixbase", "0269_order_api_meta"), + ] + + operations = [ + migrations.CreateModel( + name="HistoricPassword", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, primary_key=True, serialize=False + ), + ), + ("created", models.DateTimeField(auto_now_add=True)), + ("password", models.CharField(max_length=128)), + ( + "user", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="historic_passwords", + to=settings.AUTH_USER_MODEL, + ), + ), + ], + ), + ] diff --git a/src/pretix/base/models/auth.py b/src/pretix/base/models/auth.py index 3903db3b0..d5f7a51cd 100644 --- a/src/pretix/base/models/auth.py +++ b/src/pretix/base/models/auth.py @@ -654,3 +654,9 @@ class WebAuthnDevice(Device): @property def webauthnpubkey(self): return websafe_decode(self.pub_key) + + +class HistoricPassword(models.Model): + user = models.ForeignKey(User, on_delete=models.CASCADE, related_name="historic_passwords") + created = models.DateTimeField(auto_now_add=True) + password = models.CharField(verbose_name=_("Password"), max_length=128) diff --git a/src/pretix/presale/forms/customer.py b/src/pretix/presale/forms/customer.py index f3db3e8d8..e26b32925 100644 --- a/src/pretix/presale/forms/customer.py +++ b/src/pretix/presale/forms/customer.py @@ -19,6 +19,7 @@ # You should have received a copy of the GNU Affero General Public License along with this program. If not, see # . # +import functools import hashlib import ipaddress import random @@ -27,7 +28,7 @@ from django import forms from django.conf import settings from django.contrib.auth.hashers import check_password from django.contrib.auth.password_validation import ( - password_validators_help_texts, validate_password, + get_password_validators, password_validators_help_texts, validate_password, ) from django.contrib.auth.tokens import PasswordResetTokenGenerator from django.core import signing @@ -271,6 +272,11 @@ class RegistrationForm(forms.Form): return customer +@functools.lru_cache(maxsize=None) +def get_customer_password_validators(): + return get_password_validators(settings.CUSTOMER_AUTH_PASSWORD_VALIDATORS) + + class SetPasswordForm(forms.Form): required_css_class = 'required' error_messages = { @@ -311,7 +317,7 @@ class SetPasswordForm(forms.Form): def clean_password(self): password1 = self.cleaned_data.get('password', '') - if validate_password(password1, user=self.customer) is not None: + if validate_password(password1, user=self.customer, password_validators=get_customer_password_validators()) is not None: raise forms.ValidationError(_(password_validators_help_texts()), code='pw_invalid') return password1 @@ -405,7 +411,7 @@ class ChangePasswordForm(forms.Form): def clean_password(self): password1 = self.cleaned_data.get('password', '') - if validate_password(password1, user=self.customer) is not None: + if validate_password(password1, user=self.customer, password_validators=get_customer_password_validators()) is not None: raise forms.ValidationError(_(password_validators_help_texts()), code='pw_invalid') return password1 diff --git a/src/pretix/settings.py b/src/pretix/settings.py index 64ccd28cc..5b8b0d3ba 100644 --- a/src/pretix/settings.py +++ b/src/pretix/settings.py @@ -714,6 +714,10 @@ BOOTSTRAP3 = { } PASSWORD_HASHERS = [ + # Note that when updating this, all user passwords will be re-hashed on next login, however, + # the HistoricPassword model will not be changed automatically. In case a serious issue with a hasher + # comes to light, dropping the contents of the HistoricPassword table might be the more risk-adequate + # decision. "django.contrib.auth.hashers.Argon2PasswordHasher", "django.contrib.auth.hashers.PBKDF2PasswordHasher", "django.contrib.auth.hashers.PBKDF2SHA1PasswordHasher", @@ -725,7 +729,44 @@ AUTH_PASSWORD_VALIDATORS = [ 'NAME': 'django.contrib.auth.password_validation.UserAttributeSimilarityValidator', }, { - 'NAME': 'django.contrib.auth.password_validation.MinimumLengthValidator', + "NAME": "django.contrib.auth.password_validation.MinimumLengthValidator", + "OPTIONS": { + # To fulfill per PCI DSS requirement 8.3.6 + "min_length": 12, + }, + }, + { + # To fulfill per PCI DSS requirement 8.3.6 + 'NAME': 'pretix.base.auth.NumericAndAlphabeticPasswordValidator', + }, + { + "NAME": "pretix.base.auth.HistoryPasswordValidator", + "OPTIONS": { + # To fulfill per PCI DSS requirement 8.3.7 + "history_length": 4, + }, + }, + { + 'NAME': 'django.contrib.auth.password_validation.CommonPasswordValidator', + }, + { + 'NAME': 'django.contrib.auth.password_validation.NumericPasswordValidator', + }, +] +CUSTOMER_AUTH_PASSWORD_VALIDATORS = [ + # For customer accounts, we apply a little less strict requirements to provide a risk-adequate + # user experience. + { + 'NAME': 'django.contrib.auth.password_validation.UserAttributeSimilarityValidator', + }, + { + "NAME": "django.contrib.auth.password_validation.MinimumLengthValidator", + "OPTIONS": { + "min_length": 8, + }, + }, + { + 'NAME': 'pretix.base.auth.NumericAndAlphabeticPasswordValidator', }, { 'NAME': 'django.contrib.auth.password_validation.CommonPasswordValidator', diff --git a/src/tests/control/test_auth.py b/src/tests/control/test_auth.py index ea749d878..6622ed82d 100644 --- a/src/tests/control/test_auth.py +++ b/src/tests/control/test_auth.py @@ -337,7 +337,7 @@ class RegistrationFormTest(TestCase): response = self.client.post('/control/register', { 'email': 'dummy@dummy.dummy', - 'password': 'foobarbar', + 'password': 'f00barbarbar', 'password_repeat': '' }) self.assertEqual(response.status_code, 200) @@ -347,8 +347,8 @@ class RegistrationFormTest(TestCase): self.user = User.objects.create_user('dummy@dummy.dummy', 'dummy') response = self.client.post('/control/register', { 'email': 'dummy@dummy.dummy', - 'password': 'foobarbar', - 'password_repeat': 'foobarbar' + 'password': 'f00barbarbar', + 'password_repeat': 'f00barbarbar' }) self.assertEqual(response.status_code, 200) @@ -356,8 +356,8 @@ class RegistrationFormTest(TestCase): def test_success(self): response = self.client.post('/control/register', { 'email': 'dummy@dummy.dummy', - 'password': 'foobarbar', - 'password_repeat': 'foobarbar' + 'password': 'f00barbarbar', + 'password_repeat': 'f00barbarbar' }) self.assertEqual(response.status_code, 302) assert time.time() - self.client.session['pretix_auth_login_time'] < 60 @@ -367,8 +367,8 @@ class RegistrationFormTest(TestCase): def test_disabled(self): response = self.client.post('/control/register', { 'email': 'dummy@dummy.dummy', - 'password': 'foobarbar', - 'password_repeat': 'foobarbar' + 'password': 'f00barbarbar', + 'password_repeat': 'f00barbarbar' }) self.assertEqual(response.status_code, 403) @@ -376,8 +376,8 @@ class RegistrationFormTest(TestCase): def test_no_native_auth(self): response = self.client.post('/control/register', { 'email': 'dummy@dummy.dummy', - 'password': 'foobarbar', - 'password_repeat': 'foobarbar' + 'password': 'f00barbarbar', + 'password_repeat': 'f00barbarbar' }) self.assertEqual(response.status_code, 403) @@ -593,8 +593,8 @@ class PasswordRecoveryFormTest(TestCase): response = self.client.post( '/control/forgot/recover?id=%d&token=foo' % self.user.id, { - 'password': 'foobarbar', - 'password_repeat': 'foobarbar' + 'password': 'f00barbarbar', + 'password_repeat': 'f00barbarbar' } ) self.assertEqual(response.status_code, 302) @@ -615,8 +615,8 @@ class PasswordRecoveryFormTest(TestCase): response = self.client.post( '/control/forgot/recover?id=%d&token=%s' % (self.user.id, token), { - 'password': 'foobarbar', - 'password_repeat': 'foobarbar' + 'password': 'f00barbarbar', + 'password_repeat': 'f00barbarbar' } ) self.assertEqual(response.status_code, 302) @@ -630,13 +630,13 @@ class PasswordRecoveryFormTest(TestCase): response = self.client.post( '/control/forgot/recover?id=%d&token=%s' % (self.user.id, token), { - 'password': 'foobarbar', - 'password_repeat': 'foobarbar' + 'password': 'f00barbarbar', + 'password_repeat': 'f00barbarbar' } ) self.assertEqual(response.status_code, 302) self.user = User.objects.get(id=self.user.id) - self.assertTrue(self.user.check_password('foobarbar')) + self.assertTrue(self.user.check_password('f00barbarbar')) def test_recovery_valid_token_empty_passwords(self): token = default_token_generator.make_token(self.user) @@ -645,7 +645,7 @@ class PasswordRecoveryFormTest(TestCase): response = self.client.post( '/control/forgot/recover?id=%d&token=%s' % (self.user.id, token), { - 'password': 'foobarbar', + 'password': 'f00barbarbar', 'password_repeat': '' } ) @@ -660,7 +660,7 @@ class PasswordRecoveryFormTest(TestCase): '/control/forgot/recover?id=%d&token=%s' % (self.user.id, token), { 'password': '', - 'password_repeat': 'foobarbar' + 'password_repeat': 'f00barbarbar' } ) self.assertEqual(response.status_code, 200) @@ -697,6 +697,48 @@ class PasswordRecoveryFormTest(TestCase): self.user = User.objects.get(id=self.user.id) self.assertTrue(self.user.check_password('demo')) + def test_recovery_valid_token_password_reuse(self): + self.user.set_password("GsvdU4gGZDb4J9WgIhLNcZT9PO7CZ3") + self.user.save() + self.user.set_password("hLPqPpuZIjouGBk9xTLu1aXYqjpRYS") + self.user.save() + self.user.set_password("Jn2nQSa25ZJAc5GUI1HblrneWCXotD") + self.user.save() + self.user.set_password("cboaBj3yIfgnQeKClDgvKNvWC69cV1") + self.user.save() + self.user.set_password("Kkj8f3kGXbXmbgcwHBgf3WKmzkUOhM") + self.user.save() + + assert self.user.historic_passwords.count() == 4 + + 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': 'cboaBj3yIfgnQeKClDgvKNvWC69cV1', + 'password_repeat': 'cboaBj3yIfgnQeKClDgvKNvWC69cV1' + } + ) + self.assertEqual(response.status_code, 200) + self.user = User.objects.get(id=self.user.id) + self.assertTrue(self.user.check_password('Kkj8f3kGXbXmbgcwHBgf3WKmzkUOhM')) + + 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': 'GsvdU4gGZDb4J9WgIhLNcZT9PO7CZ3', + 'password_repeat': 'GsvdU4gGZDb4J9WgIhLNcZT9PO7CZ3' + } + ) + self.assertEqual(response.status_code, 302) + self.user = User.objects.get(id=self.user.id) + self.assertTrue(self.user.check_password('GsvdU4gGZDb4J9WgIhLNcZT9PO7CZ3')) + 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)) @@ -704,8 +746,8 @@ class PasswordRecoveryFormTest(TestCase): response = self.client.post( '/control/forgot/recover?id=%d&token=%s' % (self.user.id, token), { - 'password': 'foobar', - 'password_repeat': 'foobar' + 'password': 'foobarfooba', + 'password_repeat': 'foobarfooba' } ) self.assertEqual(response.status_code, 200) diff --git a/src/tests/control/test_teams.py b/src/tests/control/test_teams.py index 743767a4f..6391414a4 100644 --- a/src/tests/control/test_teams.py +++ b/src/tests/control/test_teams.py @@ -349,8 +349,8 @@ def test_invite_new_user(event, admin_team, client): assert b'