UserAdmin.add_view should handle USERNAME_FIELD on custom User models

[gwahl@…]

I have a custom user model whose USERNAME_FIELD is email. The UserAdmin class provided by Django should work, once the the fieldsets and form properties are overridden. However, UserAdmin.add_view assumes there is a field called username on the user model, which is not true any more with custom users. It should use whatever field is defined in USERNAME_FIELD.

patch to come.

[gwahl@…]

A field called username is also assumed in user_change_password. str(user) should be used instead.

[gwahl@…]

Patch: ​https://github.com/django/django/pull/427

[Preston Holmes]

​https://docs.djangoproject.com/en/dev/topics/auth/#custom-users-and-django-contrib-admin

does not list 'username' as a requirement - but what you have found is not the only place in admin where it is assumed:

contrib/admin/templates/admin/base.html
29:            <strong>{% filter force_escape %}{% firstof user.get_short_name user.username %}{% endfilter %}</strong>.

contrib/admin/templates/admin/object_history.html
32:            <td>{{ action.user.username }}{% if action.user.get_full_name %} ({{ action.user.get_full_name }}){% endif %}</td>

contrib/admin/templates/registration/password_reset_email.html
8:{% trans "Your username, in case you've forgotten:" %} {{ user.username }}

contrib/auth/admin.py
41:        (None, {'fields': ('username', 'password')}),
56:    list_display = ('username', 'email', 'first_name', 'last_name', 'is_staff')
58:    search_fields = ('username', 'first_name', 'last_name', 'email')
59:    ordering = ('username',)

Other places a username field is assumed (Caveat I've not yet analyzed these occurrences - they may be optional uses):

contrib/auth/middleware.py
58:            if request.user.username == self.clean_username(username, request):

contrib/comments/admin.py
27:    search_fields = ('comment', 'user__username', 'user_name', 'user_email', 'user_url', 'ip_address')

contrib/comments/models.py
114:                    userinfo["name"] = u.username
195:            (self.flag, self.comment_id, self.user.username)

contrib/comments/views/comments.py
43:            data["name"] = request.user.get_full_name() or request.user.username

I'm left wondering, given that we can't de-reference USERNAME_FIELD in all cases (eg admin list_view args etc), whether requiring a field that represents a username, and requiring a class USERNAME_FIELD attr, just so we can call the field something else may not be worth it in the end. We could just stipulate that the unique field be called username out of historical convention, but that it could be nearly any field type - and need not be a simple varchar

alternately/additionally we may want to say something about what more is in the contract for contrib.admin when it comes to username

[gwahl@…]

How about a User.get_username() method? I use email addresses as usernames a lot, and would be dissapointed to have to call my field username. get_username() could be implimented in AbstractBaseUser as simply as

def get_username(self):
    return getattr(self, getattr(self, 'USERNAME_FIELD', 'username'))
    # or, I'm not sure what the preferred way to do this is
    username_field = self._meta.get_field(getattr(self, 'USERNAME_FIELD', 'username'))
    return username_field.value_from_object(self)

[Preston Holmes]

Yes, get_username could also be a reasonable compromise, as it can be used in templates etc - Just to clarify - I was musing, not prescribing in my previous comment.

It is also why I marked this as DDN + release blocker as Russ and I have agreed we need to nail down these contracts as best we can in this release.

[Russell Keith-Magee]

One immediate improvement I can see is to actually make USERNAME_FIELD required, like it says in the docs, rather than a "defaults to username" attribute. That simplifies a lot of internal logic. Beyond that: I can certainly see the benefit in having a get_username()-esque field.

As for the admin registration issues: UserAdmin will really only ever work for auth.User. If you subclass AbstractBaseUser, you'll have a completely different set of fields to display in admin; If you're subclassing AbstractUser, you're doing so do add fields, so you'll have at least *some* different admin requirements. Writing your own UserAdmin is inevitable; all we can do is make it easier.

The good news is that if you subclass UserAdmin, you can just override the fieldset and form definitions, register your User class, and everything will work as you expect. This warrants documentation, but I don't think massive changes to UserAdmin (the original issue raised in this ticket notwithstanding) are called for.

Admin registrations for Comments (and any other model that references User) are an other issue entirely... however I think we can get some traction with lazy evaluation of attributes.

[Preston Holmes]

I've clarified this in the docs:

​https://github.com/django/django/pull/430

[gwahl@…]

The documentation change alone doesn't fix this -- they were changed to say that UserAdmin respects USERNAME_FIELD, but without the code to support USERNAME_FIELD, there is not much functionality left in UserAdmin to subclass. I understand that the fieldsets and forms must be changed, but I think the actual view code in UserAdmin should work with any User that follows the basic contract. My normal user model just changes username to email, and first/last name for preferred/full name. Without UserAdmin respecting USERNAME_FIELD, it is almost entirely useless for even the slightest modification to users because several of the view functions have to be completely overridden.

I agree that UserAdmin should not undergo massive changes to support custom users, but it should at least support the USERNAME_FIELD part of the contract, so it is possible to usefully subclass it for your own user.

[Preston Holmes]

I think the conclusion is that UserAdmin is a ModelAdmin first and foremost for contrib.AbstractUser and that if you are going to contribute a custom user model to your project, you are essentially expected to contribute a custom ModelAdmin for it to work in the admin.

I do think that two admin levels of admin contract could be potentially documented. Currently what is documented is what your model needs to support the permission machinery of the admin - and so is the first, lower, bar. The next bar would be to document what would be required to use your model with UserAdmin. My current docs modification is conflating those.

So to consider three options:

1. modify UserAdmin to be very flexible to work with custom user models
2. document the UserAdmin contract that custom user models must conform to in order to use UserAdmin
3. document how to subclass or replace UserAdmin to allow your custom model to best work with the admin

1: isn't ultimately possible, there is no way to have UserAdmin support the range of custom user models. It is debatable whether adding a get_username and adding support for USERNAME_FIELD to UserAdmin just falsely increases this expectation, and in the end provide very few custom user models what they need. That is, having UserAdmin reference USERNAME_FIELD is unlikely to make UserAdmin work with many custom user models.

2: The contract for UserAdmin may essentially be too coupled to contrib.AbstractUser, and so isn't an worthwhile contract to document and build against for a AbstractBaseUser subclass, and instead point people to option 3

3: recognize that a common desire will be to extend User, and document the technique of subclassing UserAdmin and registering that subclass for the custom user, and otherwise point people to the need to create their own ModelAdmin for their own custom user.

So I'd say the current doc patch needs to be improved to:

clarify that what is currently documented as admin compliance is not UserAdmin compliance, and that meeting the bar for UserAdmin compat is better achieved by extending User or registering their own ModelAdmin.

I'm also going to put together a little test to see if my hunch is correct that the requirement for a custom user model to have a 'username' field goes beyond just the requirement for UserAdmin - as there are just so many references in the admin code to username.

[Russell Keith-Magee <russell@…>]

In c433fcb3fb34fccd69782979f0e7cd5f2d4a4893:

Fixed #19077, #19079 -- Made USERNAME_FIELD a required field, and modified UserAdmin to match.

[Gavin Wahl <gwahl@…>]

This patch missed one use of username -- on line 136 of django.contrib.auth.admin, in user_change_password, escape(user.username) should be escape(user.get_username()).

[Russell Keith-Magee]

This has been reported as part of the reopen of #19056.