diff --git a/admin/forms.py b/admin/forms.py index 6d44b4c0..4ebf689c 100644 --- a/admin/forms.py +++ b/admin/forms.py @@ -1,6 +1,8 @@ from django import forms +from django.contrib.auth import password_validation from django.contrib.auth.forms import ReadOnlyPasswordHashField from django.contrib.auth.models import Group, User +from django.core.exceptions import ValidationError from django.urls import reverse_lazy from django.utils.text import format_lazy from django.utils.translation import gettext_lazy as _ @@ -102,6 +104,27 @@ class UserCreateForm(UserForm): content_type__app_label="accounts", codename="change_password" ) + def _post_clean(self): + # The validators in AUTH_PASSWORD_VALIDATORS, as on the password change + # forms; after the instance has the form's values (username, email). + super()._post_clean() + password = self.cleaned_data.get("password") + if password: + try: + password_validation.validate_password(password, self.instance) + except ValidationError as error: + self.add_error("password", error) + + def save(self, commit=True): + # hashed before the user is first written: the raw password never + # reaches the database + user = super().save(commit=False) + user.set_password(self.cleaned_data["password"]) + if commit: + user.save() + self.save_m2m() + return user + class Meta: model = User fields = [ diff --git a/admin/test_user_create.py b/admin/test_user_create.py new file mode 100644 index 00000000..cdb0b5c7 --- /dev/null +++ b/admin/test_user_create.py @@ -0,0 +1,65 @@ +"""Creating a user in the admin pages: the password reaches the database only +hashed, and the configured password validators apply as on the change forms.""" + +from django.contrib.auth.models import Group, Permission, User +from django.db import connection +from django.test import TestCase, override_settings +from django.urls import reverse + +RAW = "Raw-Passw0rd-in-the-form" +LIMITS = {"max_instances": 1, "max_cpus": 1, "max_memory": 1024, "max_disk_size": 4} + + +class UserCreateTests(TestCase): + def setUp(self): + self.client.force_login(User.objects.create_superuser("uc-admin", "uc@example.com", "pw")) + self.change_password = Permission.objects.get(content_type__app_label="accounts", codename="change_password") + + def create(self, **data): + return self.client.post(reverse("admin:user_create"), {"username": "newbie", "password": RAW, **LIMITS, **data}) + + def test_the_raw_password_is_never_written(self): + written = [] + + def record(execute, sql, params, many, context): + if RAW in repr(params): + written.append(sql) + return execute(sql, params, many, context) + + with connection.execute_wrapper(record): + response = self.create() + self.assertRedirects(response, reverse("admin:user_list")) + self.assertEqual(written, []) + self.assertTrue(User.objects.get(username="newbie").check_password(RAW)) + + def test_groups_and_permissions_are_saved(self): + group = Group.objects.create(name="uc-group") + self.create(groups=[group.pk], user_permissions=[self.change_password.pk]) + user = User.objects.get(username="newbie") + self.assertEqual(list(user.groups.all()), [group]) + self.assertEqual(list(user.user_permissions.all()), [self.change_password]) + + def test_an_unticked_permission_stays_off(self): + self.create() + self.assertFalse(User.objects.get(username="newbie").user_permissions.exists()) + + @override_settings( + AUTH_PASSWORD_VALIDATORS=[ + {"NAME": "django.contrib.auth.password_validation.MinimumLengthValidator", "OPTIONS": {"min_length": 30}} + ] + ) + def test_configured_password_validators_apply(self): + response = self.create() + self.assertEqual(response.status_code, 200) + self.assertContains(response, "at least 30 characters") + self.assertFalse(User.objects.filter(username="newbie").exists()) + + @override_settings( + AUTH_PASSWORD_VALIDATORS=[{"NAME": "django.contrib.auth.password_validation.UserAttributeSimilarityValidator"}] + ) + def test_the_password_is_checked_against_the_new_username(self): + response = self.client.post( + reverse("admin:user_create"), {"username": "rumpelstiltskin", "password": "rumpelstiltskin1", **LIMITS} + ) + self.assertEqual(response.status_code, 200) + self.assertContains(response, "too similar to the username") diff --git a/admin/views.py b/admin/views.py index 03d7433f..11bb27be 100644 --- a/admin/views.py +++ b/admin/views.py @@ -95,9 +95,6 @@ def user_create(request): attributes_form = forms.UserAttributesForm(request.POST or None) if user_form.is_valid() and attributes_form.is_valid(): user = user_form.save() - password = user_form.cleaned_data["password"] - user.set_password(password) - user.save() attributes = attributes_form.save(commit=False) attributes.user = user attributes.save()