CGM_Public/pretix_cgo
1
0
Fork 0
forked from CGM_Public/pretix_original
Code Pull Requests Actions Activity

[SECURITY] Fix old password not validated on password change

Browse Source
This commit is contained in:
Raphael Michel
2025-11-26 19:39:32 +01:00
parent 2fc7c23960
commit 8f69cb166d
2 changed files with 52 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
src/pretix/base/forms/user.py
View File

@@ -89,8 +89,6 @@ class User2FADeviceAddForm(forms.Form):
class UserPasswordChangeForm(forms.Form): class UserPasswordChangeForm(forms.Form):
error_messages = { error_messages = {
'pw_current': _("Please enter your current password if you want to change your email address "
"or password."),
'pw_current_wrong': _("The current password you entered was not correct."), 'pw_current_wrong': _("The current password you entered was not correct."),
'pw_mismatch': _("Please enter the same password twice"), 'pw_mismatch': _("Please enter the same password twice"),
'rate_limit': _("For security reasons, please wait 5 minutes before you try again."), 'rate_limit': _("For security reasons, please wait 5 minutes before you try again."),
@@ -103,19 +101,19 @@ class UserPasswordChangeForm(forms.Form):
attrs={'autocomplete': 'username'}, attrs={'autocomplete': 'username'},
)) ))
old_pw = forms.CharField(max_length=255, old_pw = forms.CharField(max_length=255,
required=False, required=True,
label=_("Your current password"), label=_("Your current password"),
widget=forms.PasswordInput( widget=forms.PasswordInput(
attrs={'autocomplete': 'current-password'}, attrs={'autocomplete': 'current-password'},
)) ))
new_pw = forms.CharField(max_length=255, new_pw = forms.CharField(max_length=255,
required=False, required=True,
label=_("New password"), label=_("New password"),
widget=forms.PasswordInput( widget=forms.PasswordInput(
attrs={'autocomplete': 'new-password'}, attrs={'autocomplete': 'new-password'},
)) ))
new_pw_repeat = forms.CharField(max_length=255, new_pw_repeat = forms.CharField(max_length=255,
required=False, required=True,
label=_("Repeat new password"), label=_("Repeat new password"),
widget=forms.PasswordInput( widget=forms.PasswordInput(
attrs={'autocomplete': 'new-password'}, attrs={'autocomplete': 'new-password'},
@@ -130,7 +128,7 @@ class UserPasswordChangeForm(forms.Form):
def clean_old_pw(self): def clean_old_pw(self):
old_pw = self.cleaned_data.get('old_pw') old_pw = self.cleaned_data.get('old_pw')
if old_pw and settings.HAS_REDIS: if settings.HAS_REDIS:
from django_redis import get_redis_connection from django_redis import get_redis_connection
rc = get_redis_connection("redis") rc = get_redis_connection("redis")
cnt = rc.incr('pretix_pwchange_%s' % self.user.pk) cnt = rc.incr('pretix_pwchange_%s' % self.user.pk)
@@ -141,7 +139,7 @@ class UserPasswordChangeForm(forms.Form):
code='rate_limit', code='rate_limit',
) )
if old_pw and not check_password(old_pw, self.user.password): if not check_password(old_pw, self.user.password):
raise forms.ValidationError( raise forms.ValidationError(
self.error_messages['pw_current_wrong'], self.error_messages['pw_current_wrong'],
code='pw_current_wrong', code='pw_current_wrong',
@@ -151,17 +149,22 @@ class UserPasswordChangeForm(forms.Form):
def clean_new_pw(self): def clean_new_pw(self):
password1 = self.cleaned_data.get('new_pw', '') password1 = self.cleaned_data.get('new_pw', '')
if password1 and validate_password(password1, user=self.user) is not None: if validate_password(password1, user=self.user) is not None:
raise forms.ValidationError( raise forms.ValidationError(
_(password_validators_help_texts()), _(password_validators_help_texts()),
code='pw_invalid' code='pw_invalid'
) )
if self.user.check_password(password1):
raise forms.ValidationError(
self.error_messages['pw_equal'],
code='pw_equal',
)
return password1 return password1
def clean_new_pw_repeat(self): def clean_new_pw_repeat(self):
password1 = self.cleaned_data.get('new_pw') password1 = self.cleaned_data.get('new_pw')
password2 = self.cleaned_data.get('new_pw_repeat') password2 = self.cleaned_data.get('new_pw_repeat')
if password1 and password1 != password2: if password1 != password2:
raise forms.ValidationError( raise forms.ValidationError(
self.error_messages['pw_mismatch'], self.error_messages['pw_mismatch'],
code='pw_mismatch' code='pw_mismatch'

62
src/tests/control/test_user.py
View File

@@ -42,8 +42,8 @@ from pretix.testutils.mock import mocker_context
class UserSettingsTest(SoupTest): class UserSettingsTest(SoupTest):
def setUp(self): def setUp(self):
super().setUp() super().setUp()
self.user = User.objects.create_user('dummy@dummy.dummy', 'barfoofoo') self.user = User.objects.create_user('dummy@dummy.dummy', 'old_qvuSpukdKWUV7m7PoRrWwpCd2Taij9')
self.client.login(email='dummy@dummy.dummy', password='barfoofoo') self.client.login(email='dummy@dummy.dummy', password='old_qvuSpukdKWUV7m7PoRrWwpCd2Taij9')
doc = self.get_doc('/control/settings') doc = self.get_doc('/control/settings')
self.form_data = extract_form_fields(doc.select('form[data-testid="usersettingsform"]')[0]) self.form_data = extract_form_fields(doc.select('form[data-testid="usersettingsform"]')[0])
@@ -74,8 +74,8 @@ class UserSettingsTest(SoupTest):
class UserEmailChangeTest(SoupTest): class UserEmailChangeTest(SoupTest):
def setUp(self): def setUp(self):
super().setUp() super().setUp()
self.user = User.objects.create_user('dummy@dummy.dummy', 'barfoofoo') self.user = User.objects.create_user('dummy@dummy.dummy', 'old_qvuSpukdKWUV7m7PoRrWwpCd2Taij9')
self.client.login(email='dummy@dummy.dummy', password='barfoofoo') self.client.login(email='dummy@dummy.dummy', password='old_qvuSpukdKWUV7m7PoRrWwpCd2Taij9')
session = self.client.session session = self.client.session
session['pretix_auth_login_time'] = int(time.time()) session['pretix_auth_login_time'] = int(time.time())
session.save() session.save()
@@ -92,7 +92,7 @@ class UserEmailChangeTest(SoupTest):
self.assertEqual(response.status_code, 302) self.assertEqual(response.status_code, 302)
response = self.client.post('/control/reauth/?next=/control/settings/email/change', { response = self.client.post('/control/reauth/?next=/control/settings/email/change', {
'password': 'barfoofoo' 'password': 'old_qvuSpukdKWUV7m7PoRrWwpCd2Taij9'
}) })
self.assertIn('/control/settings/email/change', response['Location']) self.assertIn('/control/settings/email/change', response['Location'])
self.assertEqual(response.status_code, 302) self.assertEqual(response.status_code, 302)
@@ -151,8 +151,8 @@ class UserEmailChangeTest(SoupTest):
class UserPasswordChangeTest(SoupTest): class UserPasswordChangeTest(SoupTest):
def setUp(self): def setUp(self):
super().setUp() super().setUp()
self.user = User.objects.create_user('dummy@dummy.dummy', 'barfoofoo') self.user = User.objects.create_user('dummy@dummy.dummy', 'old_qvuSpukdKWUV7m7PoRrWwpCd2Taij9')
self.client.login(email='dummy@dummy.dummy', password='barfoofoo') self.client.login(email='dummy@dummy.dummy', password='old_qvuSpukdKWUV7m7PoRrWwpCd2Taij9')
doc = self.get_doc('/control/settings/password/change') doc = self.get_doc('/control/settings/password/change')
self.form_data = extract_form_fields(doc.select('.container-fluid form')[0]) self.form_data = extract_form_fields(doc.select('.container-fluid form')[0])
@@ -163,10 +163,23 @@ class UserPasswordChangeTest(SoupTest):
def test_change_password_require_password(self): def test_change_password_require_password(self):
doc = self.save({ doc = self.save({
'new_pw': 'foo', 'new_pw': 'f00barbarbar',
'new_pw_repeat': 'foo', 'new_pw_repeat': 'f00barbarbar',
}) })
assert doc.select(".alert-danger") assert doc.select(".alert-danger")
assert "This field is required." in doc.select(".has-error")[0].text
pw = self.user.password
self.user = User.objects.get(pk=self.user.pk)
assert self.user.password == pw
def test_change_password_old_password_wrong(self):
doc = self.save({
'new_pw': 'f00barbarbar',
'new_pw_repeat': 'f00barbarbar',
'old_pw': 'lolwrong',
})
assert doc.select(".alert-danger")
assert "The current password you entered was not correct." in doc.select(".has-error")[0].text
pw = self.user.password pw = self.user.password
self.user = User.objects.get(pk=self.user.pk) self.user = User.objects.get(pk=self.user.pk)
assert self.user.password == pw assert self.user.password == pw
@@ -177,7 +190,7 @@ class UserPasswordChangeTest(SoupTest):
self.save({ self.save({
'new_pw': 'f00barbarbar', 'new_pw': 'f00barbarbar',
'new_pw_repeat': 'f00barbarbar', 'new_pw_repeat': 'f00barbarbar',
'old_pw': 'barfoofoo', 'old_pw': 'old_qvuSpukdKWUV7m7PoRrWwpCd2Taij9',
}) })
pw = self.user.password pw = self.user.password
self.user = User.objects.get(pk=self.user.pk) self.user = User.objects.get(pk=self.user.pk)
@@ -187,7 +200,7 @@ class UserPasswordChangeTest(SoupTest):
doc = self.save({ doc = self.save({
'new_pw': 'f00barbarbar', 'new_pw': 'f00barbarbar',
'new_pw_repeat': 'f00barbarbar', 'new_pw_repeat': 'f00barbarbar',
'old_pw': 'barfoofoo', 'old_pw': 'old_qvuSpukdKWUV7m7PoRrWwpCd2Taij9',
}) })
assert doc.select(".alert-success") assert doc.select(".alert-success")
self.user = User.objects.get(pk=self.user.pk) self.user = User.objects.get(pk=self.user.pk)
@@ -197,9 +210,10 @@ class UserPasswordChangeTest(SoupTest):
doc = self.save({ doc = self.save({
'new_pw': 'foo', 'new_pw': 'foo',
'new_pw_repeat': 'foo', 'new_pw_repeat': 'foo',
'old_pw': 'barfoofoo', 'old_pw': 'old_qvuSpukdKWUV7m7PoRrWwpCd2Taij9',
}) })
assert doc.select(".alert-danger") assert doc.select(".alert-danger")
assert "This password is too short." in doc.select(".has-error")[0].text
pw = self.user.password pw = self.user.password
self.user = User.objects.get(pk=self.user.pk) self.user = User.objects.get(pk=self.user.pk)
assert self.user.password == pw assert self.user.password == pw
@@ -208,37 +222,40 @@ class UserPasswordChangeTest(SoupTest):
doc = self.save({ doc = self.save({
'new_pw': 'dummy123', 'new_pw': 'dummy123',
'new_pw_repeat': 'dummy123', 'new_pw_repeat': 'dummy123',
'old_pw': 'barfoofoo', 'old_pw': 'old_qvuSpukdKWUV7m7PoRrWwpCd2Taij9',
}) })
assert doc.select(".alert-danger") assert doc.select(".alert-danger")
assert "The password is too similar to the Email." in doc.select(".has-error")[0].text
pw = self.user.password pw = self.user.password
self.user = User.objects.get(pk=self.user.pk) self.user = User.objects.get(pk=self.user.pk)
assert self.user.password == pw assert self.user.password == pw
def test_change_password_require_repeat(self): def test_change_password_require_repeat(self):
doc = self.save({ doc = self.save({
'new_pw': 'foooooooooooooo', 'new_pw': 'foooooooooooooo1234',
'new_pw_repeat': 'baaaaaaaaaaaar', 'new_pw_repeat': 'baaaaaaaaaaaar1234',
'old_pw': 'barfoofoo', 'old_pw': 'old_qvuSpukdKWUV7m7PoRrWwpCd2Taij9',
}) })
assert doc.select(".alert-danger") assert doc.select(".alert-danger")
assert "Please enter the same password twice" in doc.select(".has-error")[0].text
pw = self.user.password pw = self.user.password
self.user = User.objects.get(pk=self.user.pk) self.user = User.objects.get(pk=self.user.pk)
assert self.user.password == pw assert self.user.password == pw
def test_change_password_require_new(self): def test_change_password_require_new(self):
doc = self.save({ doc = self.save({
'new_pw': 'barfoofoo', 'new_pw': 'old_qvuSpukdKWUV7m7PoRrWwpCd2Taij9',
'new_pw_repeat': 'barfoofoo', 'new_pw_repeat': 'old_qvuSpukdKWUV7m7PoRrWwpCd2Taij9',
'old_pw': 'barfoofoo', 'old_pw': 'old_qvuSpukdKWUV7m7PoRrWwpCd2Taij9',
}) })
assert doc.select(".alert-danger") assert doc.select(".has-error")
assert "Your password may not be the same as" in doc.select(".has-error")[0].text
def test_change_password_history(self): def test_change_password_history(self):
doc = self.save({ doc = self.save({
'new_pw': 'qvuSpukdKWUV7m7PoRrWwpCd2Taij9', 'new_pw': 'qvuSpukdKWUV7m7PoRrWwpCd2Taij9',
'new_pw_repeat': 'qvuSpukdKWUV7m7PoRrWwpCd2Taij9', 'new_pw_repeat': 'qvuSpukdKWUV7m7PoRrWwpCd2Taij9',
'old_pw': 'barfoofoo', 'old_pw': 'old_qvuSpukdKWUV7m7PoRrWwpCd2Taij9',
}) })
assert doc.select(".alert-success") assert doc.select(".alert-success")
@@ -255,6 +272,7 @@ class UserPasswordChangeTest(SoupTest):
'old_pw': '9UQl4lSwHLMVUIMgw0L1X8XEFmyvdn', 'old_pw': '9UQl4lSwHLMVUIMgw0L1X8XEFmyvdn',
}) })
assert doc.select(".alert-danger") assert doc.select(".alert-danger")
assert "Your password may not be the same as one of your 4 previous passwords." in doc.select(".has-error")[0].text
def test_needs_password_change_changed(self): def test_needs_password_change_changed(self):
self.user.needs_password_change = True self.user.needs_password_change = True
@@ -262,7 +280,7 @@ class UserPasswordChangeTest(SoupTest):
self.save({ self.save({
'new_pw': 'f00barbarbar', 'new_pw': 'f00barbarbar',
'new_pw_repeat': 'f00barbarbar', 'new_pw_repeat': 'f00barbarbar',
'old_pw': 'barfoofoo' 'old_pw': 'old_qvuSpukdKWUV7m7PoRrWwpCd2Taij9'
}) })
self.user.refresh_from_db() self.user.refresh_from_db()
assert self.user.needs_password_change is False assert self.user.needs_password_change is False