UserCreationForm.clean_username() crashes with a custom user model.

[Gary Jarrel]

Our project users a custom user model defined in settings via AUTH_USER_MODEL

There is a basic test case:

def test_username_validation_error_msg(self, user: User):
        # The user already exists,
        # hence cannot be created.
        form = UserAdminCreationForm(
            {
                "username": user.username,
                "password1": user.password,
                "password2": user.password,
            }
        )

        assert not form.is_valid()
        assert len(form.errors) == 1
        assert "username" in form.errors
        assert form.errors["username"][0] == _("This username has already been taken.")

This works in Django 4.1, however when I ran the test suite against 4.2rc1 and the current main branch, I get failures.

AttributeError: Manager isn't available; 'auth.User' has been swapped for 'users.User'

with the trace

../../contrib/django/django/forms/forms.py:197: in is_valid
    return self.is_bound and not self.errors
../../contrib/django/django/forms/forms.py:192: in errors
    self.full_clean()
../../contrib/django/django/forms/forms.py:327: in full_clean
    self._clean_fields()
../../contrib/django/django/forms/forms.py:342: in _clean_fields
    value = getattr(self, "clean_%s" % name)()
../../contrib/django/django/contrib/auth/forms.py:158: in clean_username
    if username and User.objects.filter(username__iexact=username).exists():

I believe it relates to this ​ commit

The line

if username and User.objects.filter(username__iexact=username).exists()

When I changed the line to:

if username and UserModel.objects.filter(username__iexact=username).exists()

The error was resolved.

This could potentially be related to: 28608, but it seems like a very old ticket, when compared to the ​ commit which was done in December 22

As a side note, the test then failed due to the change in the error message from:

"This username has already been taken."

to

A user with that username already exists.

But that's not a big issue.

[Mariusz Felisiak]

This works in Django 4.1, however when I ran the test suite against 4.2rc1 and the current main branch, I get failures.

It's ​documented that the UserChangeForm is tied to User and "need to be rewritten or extended to work with a custom user model" (you can find a ​full example in docs). However, I agree that could use self._meta.model instead of User to make it easier and not to break some existing configurations:

django/contrib/auth/forms.py

@@ -155 +155 @@
     def clean_username(self):
         """Reject usernames that differ only in case."""
         username = self.cleaned_data.get("username")
-        if username and User.objects.filter(username__iexact=username).exists():
+        if username and self._meta.model.objects.filter(username__iexact=username).exists():
             raise forms.ValidationError(self.error_messages["unique"], code="unique")
         else:
             return username

Does it work for you?

[Gary Jarrel]

Yes the change to:

self._meta.model.objects.filter

does indeed work and the form validations tests pass.

[Gary Jarrel]

Out of interest if I may ask, I just tried to write a test to reproduce the behaviour, then a patch, in a forked Django Repo, for example if I wanted to contribute a fix back to the code base via a pull request.

My first attempt was to simply add another test to the UserCreationFormTest, by copying the existing test_case_insensitive_username and adding {{@override_settings}} decorator as follows:

@override_settings(AUTH_USER_MODEL="auth_tests.CustomUser")
def test_case_insensitive_username_with_external_user_model(self):
    data = {
        "username": "TeStClIeNt",
        "password1": "test123",
        "password2": "test123",
    }
    form = UserCreationForm(data)
    self.assertFalse(form.is_valid())
    self.assertEqual(
        form["username"].errors,
        ["A user with that username already exists."],
    )

So I did that, and interestingly enough I started getting the same error as I did in our project:

ERROR: test_case_insensitive_username_with_external_user_model (auth_tests.test_forms.UserCreationFormTest.test_case_insensitive_username_with_external_user_model)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/garyj/.pyenv/versions/3.11.2/lib/python3.11/unittest/case.py", line 57, in testPartExecutor
    yield
  File "/home/garyj/.pyenv/versions/3.11.2/lib/python3.11/unittest/case.py", line 623, in run
    self._callTestMethod(testMethod)
    ^^^^^^^^^^^^^^^^^
  File "/home/garyj/.pyenv/versions/3.11.2/lib/python3.11/unittest/case.py", line 579, in _callTestMethod
    if method() is not None:
    ^^^^^^^^^^^^^^^^^
  File "/home/garyj/dev/contrib/django/django/test/utils.py", line 443, in inner
    return func(*args, **kwargs)
    ^^^^^^^^^^^^^^^^^
  File "/home/garyj/dev/contrib/django/tests/auth_tests/test_forms.py", line 383, in test_case_insensitive_username_with_external_user_model
    self.assertFalse(form.is_valid())
    ^^^^^^^^^^^^^^^^^
  File "/home/garyj/dev/contrib/django/django/forms/forms.py", line 197, in is_valid
    return self.is_bound and not self.errors
    ^^^^^^^^^^^^^^^^^
  File "/home/garyj/dev/contrib/django/django/forms/forms.py", line 192, in errors
    self.full_clean()
  File "/home/garyj/dev/contrib/django/django/forms/forms.py", line 327, in full_clean
    self._clean_fields()
    ^^^^^^^^^^^^^^^^^
  File "/home/garyj/dev/contrib/django/django/forms/forms.py", line 342, in _clean_fields
    value = getattr(self, "clean_%s" % name)()
    ^^^^^^^^^^^^^^^^^
  File "/home/garyj/dev/contrib/django/django/contrib/auth/forms.py", line 160, in clean_username
    and self._meta.model.objects.filter(username__iexact=username).exists()
    ^^^^^^^^^^^^^^^^^
  File "/home/garyj/dev/contrib/django/django/db/models/manager.py", line 196, in __get__
    raise AttributeError(
    ^^^^^^^^^^^^^^^^^
AttributeError: Manager isn't available; 'auth.User' has been swapped for 'auth_tests.CustomUser'

However on the same token the change self._meta.model.objects.filter(username__iexact=username).exists() is working in our project.

So I am just wondering what I am missing or doing wrong here?

Thank you

Gary

[Mariusz Felisiak]

Replying to Gary Jarrel:

Out of interest if I may ask, I just tried to write a test to reproduce the behaviour, then a patch, in a forked Django Repo, for example if I wanted to contribute a fix back to the code base via a pull request.

Please do!

However on the same token the change self._meta.model.objects.filter(username__iexact=username).exists() is working in our project.

So I am just wondering what I am missing or doing wrong here?

You must still subclass UserCreationForm to work with a custom user model, e.g.

tests/auth_tests/test_forms.py

@@ -372 +372 @@
             ["A user with that username already exists."],
         )
 
+    @override_settings(AUTH_USER_MODEL="auth_tests.ExtensionUser")
+    def test_case_insensitive_username_custom_user(self):
+        class CustomUserCreationForm(UserCreationForm):
+            class Meta(UserCreationForm.Meta):
+                model = ExtensionUser
+                fields = UserCreationForm.Meta.fields + ("date_of_birth",)
+
+        ExtensionUser.objects.create_user(
+            username="testclient",
+            password="password",
+            email="testclient@example.com",
+            date_of_birth=datetime.date(1986, 3, 4),
+        )
+        data = {
+            "username": "TeStClIeNt",
+            "password1": "test123",
+            "password2": "test123",
+            "date_of_birth": "1988-02-24",
+        }
+        form = CustomUserCreationForm(data)
+        self.assertIs(form.is_valid(), False)
+        self.assertEqual(
+            form["username"].errors,
+            ["A user with that username already exists."],
+        )
+
 
 # To verify that the login form rejects inactive users, use an authentication
 # backend that allows them.

[Gary Jarrel]

Of course that makes sense, as we use a custom form in the project as well.

I have made the code change (well copied yours) will push the changes up to my forked repo and post back here.

GitHub seems to be having some outages and it's refusing the pushes and new forks.

[Gary Jarrel]

I've also mentioned in my initial ticket a side note about the error message when a unique constraint is violated for a username.

I then noticed that our UserAdminCreationForm actually defines a custom error message:

class UserAdminCreationForm(admin_forms.UserCreationForm):
    class Meta(admin_forms.UserCreationForm.Meta):
        model = User
        error_messages = {"username": {"unique": _("This username has already been taken.")}}

I than ran tests again. With Django 4.1 the test line: assert form.errors["username"][0] == _("This username has already been taken.") passes.

However with 4.2rc1 and current main branch the test fails. Seems like the error messages are not taken into account.

So in my Django Fork, I have added another test in tests.auth_tests.test_forms.UserCreationFormTest as follows:

@override_settings(AUTH_USER_MODEL="auth_tests.ExtensionUser")
def test_case_insensitive_username_custom_user_override_error_message(self):
    class CustomUserCreationForm(UserCreationForm):
        class Meta(UserCreationForm.Meta):
            model = ExtensionUser
            fields = UserCreationForm.Meta.fields + ("date_of_birth",)
            
            # CUSTOM ERROR MESSAGE
            error_messages = {"username": {"unique": _("Username gone")}}

    ExtensionUser.objects.create_user(
        username="anotherclient",
        password="password",
        email="anotehrclient@example.com",
        date_of_birth=datetime.date(1980, 1, 1),
    )

    data = {
        "username": "AnotherClienT",
        "password1": "test123",
        "password2": "test123",
        "date_of_birth": "1980-01-01",
    }

    form = CustomUserCreationForm(data)
    self.assertIs(form.is_valid(), False)
    self.assertEqual(
        form["username"].errors,
        ["Username gone"],
    )

    assert form["username"].errors[0] == _(
        "Username gone"
    )
    assert form.errors["username"][0] == _(
        "Username gone"
    )

The test has also failed. Error Traceback:

FAIL: test_case_insensitive_username_custom_user_override_error_message (auth_tests.test_forms.UserCreationFormTest.test_case_insensitive_username_custom_user_override_error_message)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/garyj/.pyenv/versions/3.11.2/lib/python3.11/unittest/case.py", line 57, in testPartExecutor
    yield
  File "/home/garyj/.pyenv/versions/3.11.2/lib/python3.11/unittest/case.py", line 623, in run
    self._callTestMethod(testMethod)
    ^^^^^^^^^^^^^^^^^
  File "/home/garyj/.pyenv/versions/3.11.2/lib/python3.11/unittest/case.py", line 579, in _callTestMethod
    if method() is not None:
    ^^^^^^^^^^^^^^^^^
  File "/home/garyj/dev/contrib/django/django/test/utils.py", line 443, in inner
    return func(*args, **kwargs)
    ^^^^^^^^^^^^^^^^^
  File "/home/garyj/dev/contrib/django/tests/auth_tests/test_forms.py", line 428, in test_case_insensitive_username_custom_user_override_error_message
    self.assertEqual(
  File "/home/garyj/.pyenv/versions/3.11.2/lib/python3.11/unittest/case.py", line 873, in assertEqual
    assertion_func(first, second, msg=msg)
    ^^^^^^^^^^^^^^^^^
  File "/home/garyj/.pyenv/versions/3.11.2/lib/python3.11/unittest/case.py", line 866, in _baseAssertEqual
    raise self.failureException(msg)
    ^^^^^^^^^^^^^^^^^
AssertionError: ['A user with that username already exists.'] != ['Username gone']

Seems to me like this is an issue if it works in 4.1 but not 4.2rc1, nor current main branch, or am I doing something wrong?

[Gary Jarrel]

I have created a branch with the fixes and tests for the main issue that was raised with the custom user models in my repo here:

​https://github.com/oizik/django/tree/ticket_34438

[Sarah Boyce]

I think you might be defining error_messages in the wrong place, try this?

    class CustomUserCreationForm(UserCreationForm):
        error_messages = {
            **UserCreationForm.error_messages,
            "username": {"unique": _("Username gone")}
        }

        class Meta(UserCreationForm.Meta):
            model = ExtensionUser
            fields = UserCreationForm.Meta.fields + ("date_of_birth",)

Related to this, I found the docs slightly inconsistent.
According to the newly documented BaseUserCreationForm ​https://docs.djangoproject.com/en/4.2/topics/auth/default/#django.contrib.auth.forms.BaseUserCreationForm, that new form is the recommended base class if you need to customise the form.
But that is now slightly inconsistent to the code snippet here: ​https://docs.djangoproject.com/en/4.2/topics/auth/customizing/#custom-users-and-the-built-in-auth-forms which is still inheriting from UserCreationForm maybe it makes sense to update this also?

[Gary Jarrel]

Thank you for pointing out the changes in the docs, I did not see this: ​https://docs.djangoproject.com/en/4.2/topics/auth/default/#django.contrib.auth.forms.BaseUserCreationForm

I have just made the changes to the test as per the suggestion, however the test still fails.

Understandably it's a pretty obscure case, i.e. adding a custom admin form, and changing the error message. So I am not sure how to handle it, should it be documented somewhere about these error messages, or is it a bigger problem somewhere with the way error messages are handled?

Originally our project was started from the ​Django Cookiecutter, which had the error_messages set up as seen in ​forms.py. This makes me think that anyone who is using this cookie cutter will start getting errors in tests in Django 4.2 onward.

[Mariusz Felisiak]

Gary, Agreed, we should reuse the unique error message defined for fields and respect the overriding error messages in Meta (see ​comment).

[Gary Jarrel]

I've added the code as per the ​comment, and have update the pull request following the advice in ​Rebasing branches. I hope I am following the right procedures here for contributing (it is my first time with Django).

Also ran the tests in our project against the updated code base and everything is passing :)

[Mariusz Felisiak <felisiak.mariusz@…>]

In 99ba5b43:

[4.2.x] Fixed #34438 -- Reallowed extending UserCreationForm.

Regression in 298d02a77a69321af8c0023df3250663e9d1362d.

Backport of fcc7dc5781667932bf0bf8bec76df458836e5e95 from main

[Mariusz Felisiak <felisiak.mariusz@…>]

In fcc7dc5:

Fixed #34438 -- Reallowed extending UserCreationForm.

Regression in 298d02a77a69321af8c0023df3250663e9d1362d.