[Django] #20086: UserCreationForm does not support custom models.
Django
------------------------------+--------------------
Reporter: efrinut@… | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+--------------------
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'])
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20086>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: efrinut@… | Owner: nobody
Type: Bug | Status: closed
Component: contrib.auth | Version: 1.5
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* status: new => closed
* needs_better_patch: => 0
* resolution: => invalid
* needs_tests: => 0
* needs_docs: => 0
Comment:
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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20086#comment:1>
Django
Reporter: efrinut@… | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by benjaoming):
* status: closed => new
* resolution: invalid =>
Comment:
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,
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20086#comment:2>
Django
Reporter: efrinut@… | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by james@…):
what benjaoming said.
--
Ticket URL: <https://code.djangoproject.com/ticket/20086#comment:3>
Django
Reporter: efrinut@… | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by akaariai):
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...
--
Ticket URL: <https://code.djangoproject.com/ticket/20086#comment:4>
Django
Reporter: efrinut@… | Owner: nobody
Type: Bug | Status: closed
Component: contrib.auth | Version: 1.5
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by russellm):
* status: new => closed
* resolution: => invalid
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/20086#comment:5>
Django
Reporter: efrinut@… | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.5
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by denis_g):
* status: closed => new
* resolution: invalid =>
Comment:
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'],
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20086#comment:6>
Django
Reporter: efrinut@… | Owner: nobody
Type: Bug | Status: closed
Component: contrib.auth | Version: 1.5
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by bmispelon):
* status: new => closed
* resolution: => invalid
Comment:
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`.
--
Ticket URL: <https://code.djangoproject.com/ticket/20086#comment:7>
Django
Reporter: efrinut@… | Owner: nobody
Type: Bug | Status: closed
Component: contrib.auth | Version: 1.5
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by anonymous):
* has_patch: 0 => 1
* resolution: invalid => fixed
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/20086#comment:8>
Django
Reporter: efrinut@… | Owner: nobody
Type: Bug | Status: closed
Component: contrib.auth | Version: 1.5
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by anonymous):
Solved
--
Ticket URL: <https://code.djangoproject.com/ticket/20086#comment:9>