UserCreationForm does not support custom models.

[efrinut@…]

Hey,

It's my first ticket :)

I've found out that UserCreationForm() located in django\contrib\auth\forms.py does not support custom models.

In some places there should be used get_user_model() in my opinion.

    class Meta:
        model = User # Should be get_user_model()
        fields = ("username",)

    def clean_username(self):
        # Since User.username is unique, this check is redundant,
        # but it sets a nicer error message than the ORM. See #13147.
        username = self.cleaned_data["username"]
        try:
            User._default_manager.get(username=username) #Should be get_user_model()
        except User.DoesNotExist:
            return username
        raise forms.ValidationError(self.error_messages['duplicate_username'])

[Claude Paroz]

This is documented: ​https://docs.djangoproject.com/en/1.5/topics/auth/customizing/#custom-users-and-the-built-in-auth-forms

UserCreationForm

Depends on the User model. Must be re-written for any custom user model.

[benjaoming]

Hi all! During the eurocon sprints, I was working to implement compatibility with swapped user models in django-wiki and django-cms.

I somewhat disagree with the documentation here that UserCreationForm and UserChangeForm should not be intended for custom models. Rather, I'd say that the forms should depends on the custom user model inheriting from AbstractBaseUser as with all the other forms in django.contrib.auth. This would give the documentation a much-needed simplification, anyways.

In brief, this is my opinion: A custom user model that inherits from AbstractBaseUser and does not break the contract should be able to use ALL the forms in django.contrib.auth.forms.

The current problem with contrib.auth.forms is that stuff breaks if the user model has been swapped (as noted by OP).

I would like to reopen this after working with implementing support for AUTH_USER_MODEL in a reusable app that has a couple of utility forms for account handling. They inherit from django.contrib.auth.forms and they are as optional as the forms in django.contrib.auth.forms. The reason they inherit from django.contrib.auth.forms is because of an assumption that this is the right way to implement the django.contrib.auth API, however if this breaks because people swap the user forms (which they are going to do a lot, mind you!), then there is no point at all in extending anything from django.contrib.auth.forms. Henceforth, I'd have to either stop offering account handling in the app or copy most of django.contrib.auth.forms and rewrite it to respect custom user models.

The way I would like to see this fixed is by making ALL parts of the docs say Works with any subclass of AbstractBaseUser, and will adapt to use the field defined in USERNAME_FIELD.

One of the strengths of Django is to do stuff like authentication in a uniform/extendable way. First of all, custom user models is asking people to go their own ways, but at least if they extend from existing base classes, we should gain usability. We should not make matters worse by honering the implementations that don't break the AbstractUser/AbstractBaseUser interface.

My 2 cents, thanks for a great spring.. I hope the diffs below are understandable..

django/contrib/auth/forms.py
@@ -11,11 +11,11 @@
 from django.utils.translation import ugettext, ugettext_lazy as _
 
 from django.contrib.auth import authenticate, get_user_model
-from django.contrib.auth.models import User
 from django.contrib.auth.hashers import UNUSABLE_PASSWORD, identify_hasher
 from django.contrib.auth.tokens import default_token_generator
 from django.contrib.sites.models import get_current_site
 
+User = get_user_model()
 
 UNMASKED_DIGITS_TO_SHOW = 6
 
@@ -167,8 +167,7 @@
         super(AuthenticationForm, self).__init__(*args, **kwargs)
 
         # Set the label for the "username" field.
-        UserModel = get_user_model()
-        self.username_field = UserModel._meta.get_field(UserModel.USERNAME_FIELD)
+        self.username_field = User._meta.get_field(User.USERNAME_FIELD)
         if not self.fields['username'].label:
             self.fields['username'].label = capfirst(self.username_field.verbose_name)
 
@@ -215,9 +214,8 @@
         """
         Validates that an active user exists with the given email address.
         """
-        UserModel = get_user_model()
         email = self.cleaned_data["email"]
-        self.users_cache = UserModel._default_manager.filter(email__iexact=email)
+        self.users_cache = User._default_manager.filter(email__iexact=email)
         if not len(self.users_cache):
             raise forms.ValidationError(self.error_messages['unknown'])
         if not any(user.is_active for user in self.users_cache):

Also, to avoid circularity in imports, calling get_user_model cannot happen before models are loaded (right?), so django.contrib.auth.forms cannot be loaded before models are initialized - for instance in django.contrib.admin.sites, the following issue had to be corrected:

(This is - I think - an example of https://code.djangoproject.com/ticket/19753)

django/contrib/admin/sites.py
@@ -1,7 +1,6 @@
 from functools import update_wrapper
 from django.http import Http404, HttpResponseRedirect
 from django.contrib.admin import ModelAdmin, actions
-from django.contrib.admin.forms import AdminAuthenticationForm
 from django.contrib.auth import REDIRECT_FIELD_NAME
 from django.contrib.contenttypes import views as contenttype_views
 from django.views.decorators.csrf import csrf_protect
@@ -325,6 +324,7 @@
         }
         context.update(extra_context or {})
 
+        from django.contrib.admin.forms import AdminAuthenticationForm
         defaults = {
             'extra_context': context,
             'current_app': self.name,

[james@…]

what benjaoming said.

[Anssi Kääriäinen]

Hmmh, could the different forms be optional elements of the custom user model package? So, you could just reference get_user_model().user_creation_form and use that.

Now the creator of the custom user model could do:

class MyUser(AbstractBaseUser):
    ...

class MyUserAuthenticationForm(UserAuthenticationForm):
    class Meta:
        model = MyUser  # Assuming this way of modelform subclassing actually works...

MyUser.user_authentication_form = MyUserAuthenticationForm

This adds a bit of boilerplate code when using custom user models...

[Russell Keith-Magee]

I completely agree that this is desirable in theory. My problem is that I simply don't see how it is possible in practice.

The contract for AbstractAuthUser is that the User has a unique identifying field. We have no prior knowledge whether it is a char field, and email field, or even an integer field. If it's a char field, we don't know if it has any length or other validation constraints. And we don't know if there are any other fields on the user model that are needed for authentication purposes (for example, you may use a domain and a username as a pair for login purposes). And that's when you're only dealing with the *username* field. It doesn't deal with any of the other fields that UserCreationForm or UserChangeForm needs to deal with.

So, I'm closing this again. However, it's clearly an issue that people aren't happy with, so if you want to pursue this, please take it to Django-dev.

[denis_g]

Hello everyone!

I ran into this issue when subclassing UserCreationForm and I have an idea about partial solution. It doesn't solve the problem with custom field names, but since the UserCreationForm class has the Meta subclass, I think it would be appropriate to replace User with self.Meta.model.

django/contrib/auth/forms.py
@@ -95,8 +95,8 @@
         # but it sets a nicer error message than the ORM. See #13147.
         username = self.cleaned_data["username"]
         try:
-            User._default_manager.get(username=username)
-        except User.DoesNotExist:
+            self.Meta.model._default_manager.get(username=username)
+        except self.Meta.model.DoesNotExist:
             return username
         raise forms.ValidationError(
             self.error_messages['duplicate_username'],

[Baptiste Mispelon]

Hi,

There's a patch (by yours truly) for what you're suggesting in #19353.
It seems that ticket has gotten stuck somehow and I'm not too sure why.

I'll add it to my list of things to check out when I have some time (like in a week or so).

In the meantime, I'll revert this ticket back to invalid.

[anonymous]

Solved