Custom user models in 1.5, not flexible/dry enough?

[ionel]

Hello,

I'm trying to make a custom user model so I can login and register
with the email (instead of username). This means the email must become
unique and I don't need the username fields. I don't want to fill it
with random data, unique ids, hashes or whatnot.

Also, I need to have the admin working. Because of this I'm forced to
use the AbstractUser instead of AbstractBaseUser. It seems to me that
this abstract is concerned with two purposes: fields and methods for
the admin and display fluff like username, email, first_name,
last_name. This seems wrong to me, there should be two abstract
models: BaseAbstractAdminUser and AbstractUser.

The other alternative would be allowing abstract model subclasses to
override fields. Not sure about this tho, why is this disallowed in
the first place?

What do you think ?

Thanks,
-- ionel

[Russell Keith-Magee]

On Tue, Nov 6, 2012 at 7:59 AM, ionel <ione...@gmail.com> wrote:

Hello,

I'm trying to make a custom user model so I can login and register
with the email (instead of username). This means the email must become
unique and I don't need the username fields. I don't want to fill it
with random data, unique ids, hashes or whatnot.

Also, I need to have the admin working. Because of this I'm forced to
use the AbstractUser instead of AbstractBaseUser.

Incorrect. I've got several examples -- and the documentation contains a fully worked example -- of a User model extending AbstractBaseUser, using email address for a login, that works with admin. I'm also using an example of a User model that uses email address as a login on a site I'm currently developing, and so far, it's working fine.

 

It seems to me that
this abstract is concerned with two purposes: fields and methods for
the admin and display fluff like username, email, first_name,
last_name. This seems wrong to me, there should be two abstract
models: BaseAbstractAdminUser and AbstractUser.

The other alternative would be allowing abstract model subclasses to
override fields. Not sure about this tho, why is this disallowed in
the first place?

What do you think ?

 

It's not clear to me what exactly your proposing, and how much of that stems from a mistake in your original premise.

Like I said -- admin *does* work with AbstractBaseUser subclasses, and the documentation describes how to do it. 

If you're following that documentation and your hitting specific problems, then you may have found a bug, and I'm certainly interested in hearing what those problems are.

Yours,

Russ Magee %-)

[Ionel Cristian Mărieș]

I think you are referring to
https://docs.djangoproject.com/en/dev/topics/auth/#custom-users-and-django-contrib-admin
But I don't want to re-implement those if they already exist on
AbstractUser. I want exactly the same permission granularity as with
non-custom User so I have to pull in the groups and permission
relations.

Maybe there should be a mixin with those ?

And I'm not saying the admin doesn't work with AbstractBaseUser, I'm
just implying that's too much code needed to get the equivalent admin
features on a custom user. Maybe my usecase is not that common and I
need to accept the fact, but then again, maybe not. I'm just putting
it on the table.

Thanks,
-- ionel

On Nov 6, 3:04 am, Russell Keith-Magee <russ...@keith-magee.com>
wrote:

[Ryan Kaskel]

I implemented a custom User model in my new project so I can sympathize with Ionel a bit. I wanted to change a few fields on User and have an admin with the same functionality as the 1.4 auth.User. 

This necessitated some copying and pasting to define the User's relations to auth.Group and auth.Permission (as well as adding is_staff, is_superuser, and a few help methods related to perms such as get_group_permissions). 

I also had to "re-implement" (i.e. copy, paste, and change a few fields) auth.User's ModelAdmin to get the admin interface looking similar (this included some related forms).

Still though, the benefit of Russ's additions far outweigh some of these minor conveniences, which, to be fair, he mentioned in his draft branch release notes (June 4 - https://groups.google.com/d/msg/django-developers/YwEZUIMuIkE/g9-tAZ15RxMJ):

"""

 * The auth forms are bound to User, and don't adapt to MyUser. 

 * You need to roll your own admin registration for MyUser 

 * The admin template still asks for "username", even if the email 
address is the identity field. 

....

 * A full audit of admin to formally establish the "admin User" interface. 

 * Possibly factor out the fields on User that relate to Permissions, 
so it's easy to add the full set of admin permission bits to a custom 
User model. I know Alex Ogier's branch has done some work on these 
last two points, so that's probably where I'll start (or if someone 
wants to beat me to it…)

"""

Looks like someone needs to take the helm and implement some mixins. Maybe start it as a reusable app and grandmother it into Django? Or use Alex's work (https://github.com/ogier/django/blob/auth-mixins/django/contrib/auth/mixins.py#L120)?

-- Ryan

On Tuesday, 6 November 2012 02:19:47 UTC, donarb wrote:

I think it's basically a bit of an ambiguous reading of that section of the documentation:

"If you want your custom User model to also work with Admin, your User model must define some additional attributes and methods. These methods allow the admin to control access of the User to admin content:"

It's saying that the admin app uses those attributes and methods to determine access. What this means is that they are already defined in AbstractUser, but you can override them if your user has a different definition of is_staff or is_active or the permissions. If you look at the source for AbstractUser, it uses the built-in groups and permissions, just as the old User did.

Don

[Alex Ogier]

I consider the mixins I wrote at https://github.com/ogier/django/blob/auth-mixins/django/contrib/auth/mixins.py helpful because they decompose the old User model into some orthogonal chunks that you can pick and choose from when writing your own user model. Russell went a different direction, making a strictly increasing hierarchy of functionality, AbstractBaseUser -> AbstractUser -> User.

Russell's method has some advantages. It's conceptually simpler, and python has always handled multiple inheritance in a mildly broken way (namely, the super(self, klass) hierarchy is wonky). Also, his contract for a User model guarantees a single, unique field that is a primary identifier, which may be nice for apps trying to depend on a user model. That said, I'm not a huge fan of the lines Russell has chosen for his extensibility points. However, he has certainly written many more Django websites than I have, so maybe he has more insight into what is really needed.

Here's my justification for more fine-grained mixins:

- AbstractBaseUser isn't really minimal. It assumes a database-backed password field for every user. This may or may not make sense: I have written sites that use auth.User but do no internal authentication, instead shelling out entirely to an external service like Kerberos, CAS, OpenID, etc. It still is nice to have a password as a fallback, even if you don't expose it to non-admins, so that if an external service is unavailable you aren't locked out of the site, which may be enough justification for leaving the password field and related authentication mechanisms on every user.

- AbstractUser isn't very flexible. Since you can't actually override or change the fields of an abstract model anyways (so far as I know?) the main benefits the AbstractUser model gives you over the default User+Profile model is that there is one less join when querying the database, and you can avoid the ad-hoc get_profile() calls you previously needed to access extra fields. You are still stuck with the small username field and non-unique email addresses.

- AbstractUser is too heavy. In particular, it makes the same mistake that auth.User does of conflating authorization with identity, when these are actually entirely decomposable. In particular, the choice of identity as email vs. username vs. Windows domain account vs. hidden auto-incrementing primary key etc.... is totally orthogonal to the choice of authorization mechanism as Django content-types/permissions vs. always-yes vs. hand-rolled granular permissions etc....

If you want to codify that last point, I see two major types of mixins that would useful when implementing a user class, corresponding to the two main choices I have to make when implementing a site. How do my users identify themselves? and How do I control what they are authorized to do? Currently, if I choose anything besides "My users use a username and password to log in" for identity, I am forced to re-implement permissions from scratch, which I probably don't want to do. And vice versa.

Moving forwards, I assume Russell is pretty invested in the current class hierarchy. Since the Django codebase has started duck-typing the User model anyways, I could certainly reimplement the mixin hierarchy as a reusable app, though it would be the opposite of DRY since 90% of the code is duplicated exactly. We could also merge the two, and implement AbstractUser as inheriting AbstractBaseUser + UsernameMixin + PermissionsMixin. I might try that on my branch, and see how it works out. My guess is it will work just fine albeit with an absurdly deep inheritance tree.

Best,

Alex Ogier

--
You received this message because you are subscribed to the Google Groups "Django developers" group.

To view this discussion on the web visit https://groups.google.com/d/msg/django-developers/-/7C2RlNKxFGAJ.

To post to this group, send email to django-d...@googlegroups.com.
To unsubscribe from this group, send email to django-develop...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.

[Anssi Kääriäinen]

On 6 marras, 23:05, Alex Ogier <alex.og...@gmail.com> wrote:
<SNIP>
> ... Since you can't actually override or change the fields of an abstract model anyways (so far as I know?)

Allowing override of abstract parent's fields is fairly trivial
(https://github.com/akaariai/django/compare/abstract_overrides).

I don't immediately see a reason to disallow override of abstract
parent's fields.

Anyways it seems there is some room for improvement in model
validation for abstract parent case:

class Foo(models.Model):
username = models.CharField(max_length=100)

class Meta:
abstract = True

class FooOverride(models.Model):
username = models.CharField(max_length=200)

class Meta:
abstract = True

class Bar(Foo, FooOverride):
pass

print(Bar._meta.fields)
[<django.db.models.fields.AutoField: id>,
<django.db.models.fields.CharField: username>,
<django.db.models.fields.CharField: username>]

- Anssi

[Alex Ogier]

So, I went ahead and implemented the most useful mixin of the three that I defined previously, the PermissionsMixin. You can see it at https://github.com/ogier/django/blob/permissions-mixin/django/contrib/auth/models.py#L293

This should dramatically simplify the creation of a custom User model in the case that you are OK with Django's default permissions model (and I think many people are). Now instead of reimplementing dumbed-down versions of the permissions code whenever you want to use admin, you can just add PermissionsMixin to your parent classes and *poof* your user model is compatible with contrib.admin (though you still need to roll your own admin class, maybe we can help here too). This would DRY up some of https://docs.djangoproject.com/en/1.5/topics/auth/#a-full-example which could forget about permission-handling code, and still have fully-fledged app permissions.

I think this covers 80% of what is missing from the current class hierarchy. Forcing every AbstractBaseUser to have a password isn't strictly required, but seeing as Django's implementation is pretty good and we don't want to encourage people to go rolling their own, it seems like an OK limitation. Forms will still have to be overridden, but that was always inevitable.

--
You received this message because you are subscribed to the Google Groups "Django developers" group.

[Christian Jensen]

I think this is the right approach when I was making my custom user model I was thinking the same thing. I hope these mixins make it in. 

Sent from my iPhone

[Florian Apolloner]

Hi Alex,

On Tuesday, November 6, 2012 11:55:39 PM UTC+1, Alex Ogier wrote:

So, I went ahead and implemented the most useful mixin of the three that I defined previously, the PermissionsMixin.

I am not really sold on the idea of having this PermissionMixin, for one reason: If I need a custom user model I usually (from my experience, ymmv) have different permission requirements either way and don't want stuff like is_superuser or is_active. So having those two fields on the mixin class makes it unuseable for me again. What I could live with is probably a simple PermissionMixin which just provides the dispatching to _user_has_perm etc. without checking is_superuser or is_active (If you ask me those checks should move to the backend anyways but that's probably not that easy due to backwards compat).

Regards,
Florian

[Tino de Bruijn]

+1 for the mixin approach. I needed to copy 90% of the AbstractUser model just to make the email field unique and remove the username field. The permission requirements I usually have are compatible with the one's used by the admin, but the username field is superfluous. As soon as your permission requirements differ, as Florian describes, you have to roll our own of course.

If mixins aren't the way to go, it would be nice to have an abstract class in between AbstractBaseUser and AbstractUser, that contains everything that is necessary for the admin (AbstractBaseUserAdmin?), with the notion that you have to add the USERNAME_FIELD yourself. 

Regards,

Tino

--

You received this message because you are subscribed to the Google Groups "Django developers" group.

To view this discussion on the web visit https://groups.google.com/d/msg/django-developers/-/fKKJ7R4tyMUJ.

[Ludwig Kraatz]

If you ask me this just points out to some point i mentioned in the original Custom UserModel Thread. I'm trying to reframe it again.

I think the current django.contrib.auth *app* somehow behaves like some mixture of django.core._mixins_everyone_should_use_to_make_apps_interoperability and contrib.auth

As Ross pointed out in this original thread: it is usefull that everyone uses the AbstractBaseUser - and i'm interpreting now - *because it would be better for interoperability and security*

I think its possible and not bad designed at all to have this kinda stuff as core material.

So - most apps use authorization features as  .has_perm() or .is_superuser...

If decoupling it into clear interfaces this could make custom development much easier without loosing interoperability. i would suggest there shouldn't be a default user attributes like .is_superuser because thats very restrictive and nasty to workaround. And its clearly an authorization and no authentication feature.

So why not having it somehow like this? (pythonic pseudocode)

core.mixins.authentication:

AuthModelMixin:

 -abstract-

 UniqueIdentifier

 get_authentication_id(): return UniqueIdentifier

 get_long_name(): return UniqueIdentifier

 get_short_name(): return UniqueIdentifier

DjangoAuthModelMixin(AuthModelMixin):

 -abstract-

 UniqueIdentifier

 password

 last_login #(for password reset token etc.)

 check_password() ..

 set_password()...

core.mixins.personalyzation:

PersonalDataModelMixin:

 -abstract-

 first_name

 last_name

 

 get_long_name(): return first_name+last_name

 get_short_name(): return first_name

core.mixins.authorization

PermissionModelMixin:

 IS_SUPERUSER = "not_as_an_user_attribute_necessarily__is_superuser"

 IS_STAFF = "not_as_an_user_attribute_necessarily__is_staff"

 -abstractmethod-

 has_perm():

 # or maybe even as property for backwards compatibility

 is_staff():return False

 is_superuser(): return False

DjangoPermissionModelMixin(PermissionMixin):

 has_perm(permission): return self.is_superuser() or self._check_for_permission(permission)

 _check_for_permission(permission):

   if permission == IS_STAFF:

     return self.is_staff()

   else:

     super(django1.5a.contrib.auth.User,self).has_perm(permission)

and contrib.auth:

User(Mixina,b,c,d):

  date_joined

            .....

   

Didn't think abt every detail - just ment to point out what i would suggest to possibly fit all needs.

Best regards and sry for this *little* novel

ludwig

[Russell Keith-Magee]

Ok - so, I've been following this thread, and I should probably shed some light on the other side of the decision making process.

I've got a history with Mixins. I was responsible for the final commit of Django's class-based views, which are very mixin heavy… and haven't been universally well received.

There was a lot of very incoherent "this sucks" criticism (which -- as an aside -- is hideously demotivating). However, some did find a way to express their criticism constructively; one good example that explains the bigger problem is from one of Django's core team:

http://lukeplant.me.uk/blog/posts/class-based-views-and-dry-ravioli/

 

tl;dr - The root of the objection is that yes, introducing a stack of mixins decreases repetition - but at the cost of a greater learning curve. And as a result, it's harder for newcomers to get started. This isn't an inherently bad thing, but it *is* a tradeoff.

So, I've been burned. While I still believe mixins have their place… I'm not rushing to go overboard again.

Where does that leave swappable Users?

Something like a PermissionMixin? I can certainly see a place for that. Django's permissions model is a very specific beast, and the current User model implements them in a way that could definitely be refactored for reuse. I can certainly see the value in splitting that out so that you don't have to repeat the same implementation, Truth be told, it's about 10 lines of code to reimplement if you import the helper functions, but if someone were to work up a patch for this, I'd be inclined to add it.

Something like AdminMixin? I'm not as convinced. The Admin User API interface is duck typing at it's best. Admin doesn't care *how* is_staff is implemented -- it just cares that the attribute is available. Splitting out a mixin class to make it easier to put an is_staff attribute on a model seems like a whole lot of unnecessary complexity

Something like PersonalDataModelMixin? Definitely unconvinced on this one. Implementations of first_name and last_name aren't *that* hard, and flexibility to include or not include these attributes was one of the driving motivators behind swappable users in the first place.

Now, ok - in the case of the latter two, you could argue that *any* repetition is bad repetition. However, there's repetition and there's repetition. Being forced to repeat hundreds of lines of complex business logic… sure - that's bad. Being forced to explicitly declare that your user model has a boolean flag to define the staff status of a user? I'm sorry, but that's really not an onerous burden, and the CBV experience has shown that it can be beneficial for this sort of thing to be clearly declared, not masked in a superclass or mixin.

As for the argument that having the mixins makes it easier to build interoperable components? In this case, I don't buy that at all. Again, duck typing is one of Python's strengths. Interfaces are important, but they don't have to be strong interfaces. It's enough to say "you must have an attribute called X"; you don't have to force a base class with a particular implementation to make that happen.

Regarding the specifics -- I'm inclined to agree with Florian on the scope of the PermissionsMixin -- it needs to be constrained in scope, otherwise you're just importing all of AbstractUser into the mixin. So, if someone wants to pull that together (or make an impassioned case the other way), feel free, and I'll see about getting it into trunk before the 1.5 beta freeze. 

Yours,

Russ Magee %-)

--

You received this message because you are subscribed to the Google Groups "Django developers" group.

To view this discussion on the web visit https://groups.google.com/d/msg/django-developers/-/36x8Ecpj9scJ.

[Alex Ogier]

I agree entirely. The point of mixins is not that they are a particularly healthy abstraction, just that they are one way of exposing the Django permissions system, which is currently only done in full form (that is, with is_active and is_superuser guards) in AbsractUser and User. If you want to use Django permissions with AbstractBaseUser, you have to roll your own, and/or depend on underscore-prefixed functions from contrib.auth.models.

I would be +1 for a solution that exposed those permissions checking functions so that they can be used with less boilerplate. In general I think having an explicit contract for User models in particular contexts is a good thing, better than hiding behind an opaque mixin. That is,

    class MyUser(AbstractBaseUser):

        is_active = BooleanField(...)

        is_staff = BooleanField(...)

        @property

        def is_superuser(self):

            return False

        def has_perm(self, perm, obj=None):

            return user_has_perm(self, perm, obj)

        def has_perms(self, perm_list, obj=None):

            return user_has_perms(self, perm_list, obj)

        def has_module_perms(self, package_name)

            return user_has_module_perms(self, package_name)

is arguably preferable to

    class MyUser(AbstractBaseUser, PermissionsMixin):

        pass

The advantages of the former is that it is more explicit, and perhaps more newbie friendly, but it is currently impossible to do without duplicating logic since user_has_perm etc. don't exist. The advantage of the latter is that it involves considerably less boilerplate, but it freezes is_superuser and is_staff as boolean fields on the ORM (though this could be removed from the mixin if people prefer implementing these themselves as Florian suggested).

The point isn't to go mixin-crazy, it's to find the best way to expose permissions-checking (the most boilerplate-heavy of the requirements of an admin-compatible user). The other contracts are probably better exposed as tests in contrib.admin than as little piece-wise mixins.

Best,

Alex Ogier

[Val Neekman]

OK, I went back and looked at Django 1.5 again. (New Custom User Model)

Here is what I think: (User Django 1.4 as base) 

1. Rename username to userid and increase the length to 255 chars while keeping the rest of attribute the same (e.g db_index ...etc)

2. Remove first_name, last_name and full_name (or not)

3. Have __unicode__ return the userid by default

4. Leave everything else alone (Permissions, Auth, Groups ... etc)

Developers would: (optional)

inherit: MyCustomUser(User):

set: AUTH_USER_MODEL = 'myApp.MyCustomUser'

set: Primary Key to UUID (optional)

use: userid field for storing emails, account numbers, or a username, or anything else if they wish so long it's unique

add: other fields as they wish

overwrite: __unicode__ that prints whatever they want (name, id, etc.) or just leave it alone

let: the developers decide if first name, last name is what they want, if prefix is required ... etc.

just: provider the auth,permission,group functionality and everything else is up to the developer

same goes for UserForms, UserAdmin ... Django provides the basic and developers customize and enhance if they wish

No need for UserProfile anymore of course.

Am I over simplifying thing?

This break backward compatibility? Is it already broken by removing UserProfile anyways?

It would be really nice for Django User to just do Auth, Permissions and Groups and stay out of the developer's way.

One thing I know for sure is that making User customizable is definitely going in the right direction.

Val

 

On Tuesday, 20 November 2012 16:37:17 UTC-5, Val Neekman wrote:

I have used Django extensively on few "big" projects, yet I consider myself a newbie when it comes to Django (core).

As soon as the Class-Based views were released, I read the docs and tried to port my "then" ongoing project to the new Classy views.

I have to say that I agree with Russell on the learning curve. Way too many mixins. Too much options ended up creating a lot of confusions.

However, when I look back, I have come to really appreciate the mixins.

Only if there was a way to make Django work like Google Chrom's Preferences; where you'd only see few things that you'd need to worry about unless

you click the "under the hood" option. This model would save the newbies and allows the experienced room to maneuver while staying DRY.

Both Alex and Russell have valid points, but to further explain my view on this I use the following example.

 

Project "A" ONLY needs to swap out the username with an email address. That's all. (Django 1.5+ of course)

To spice it up,  few (NEW) custom fields need to be added to the Custom User Model. (like Facebook ID, ... etc)

The only thing the developer should need,  is to create a MyUser class that inherits from Django's (Whatever)User class.

Then set USERNAME_FIELD = 'email' and add the custom fields and be done with it.

(BTW: USERNAME_FIELD should be renamed to something more intuitive. e.g. USER_AUTH_UNIQUE_ID_FIELD)

Now that would be easy in a perfect world, but I think it would still be OK to expect the developer to inherit one extra mixin class to achieve the above.

The admin should work just as before with the above example.

If that is not the case and if the developer is required to copy and paste some Django code, modify it, and own it, so he could just bring the basic admin functionally back, then my friends, say goodbye to DRY.

Right now I am on Django 1.4 and I own a piece of code that allows me to login users with emails. I only use User for authentication and everything else is in UserProfile. I wish I didn't have to own a piece of custom code so I "just" could  allow login with an email address.

I know lot of other people are doing the same thing and repeating what I have done. (!DRY)

I would welcome such a mixin.

Ciao,

Val

[Russell Keith-Magee]

On Thu, Nov 22, 2012 at 12:03 AM, Val Neekman <v...@neekman.com> wrote:

OK, I went back and looked at Django 1.5 again. (New Custom User Model)

Here is what I think: (User Django 1.4 as base) 

1. Rename username to userid and increase the length to 255 chars while keeping the rest of attribute the same (e.g db_index ...etc)

2. Remove first_name, last_name and full_name (or not)

3. Have __unicode__ return the userid by default

4. Leave everything else alone (Permissions, Auth, Groups ... etc)

Thank you for your feedback. However, we discussed the design of the User model at length 9 months ago, and the design you see today is the result of those discussions. Search the archives if you want to see those discussion, or check the wiki [1] for the redux version of those discussions.

[1] https://code.djangoproject.com/wiki/ContribAuthImprovements

I'm happy to entertain discussions around minor tweaks to the design that was discussed (such as the permissions mixin that has been proposed in this thread), but we're not about to engage in a major redesign process this late in the release cycle unless someone can point out some *major* problems with the proposed design -- and so far, all that has been demonstrated is some minor annoyances requiring duplicated code, not major problems.

Yours,

Russ Magee %-)