Code

Opened 7 years ago

Closed 19 months ago

#3011 closed New feature (fixed)

Allow for extendable auth_user module

Reported by: nowell strite Owned by: nobody
Component: contrib.auth Version:
Severity: Normal Keywords: auth_user
Cc: say4ne@…, bram@…, mike.lopez@…, osborn.steven@…, michael@…, simon@…, treborhudson@…, goliath.mailinglist@…, hanne.moa@…, michal@…, marinho, ville@…, sylvaintersideral@…, david, dan@…, waylan@…, albrecht.andi@…, nreilly@…, tomasare@…, youngj@…, richard@…, seocam, alexkoshelev, sfllaw, robin, ramusus@…, stv@…, tinodb@…, danfairs, miracle2k, msaelices@…, mmitar@…, anball@…, remco@…, tom@…, frankoid, wdoekes, Kronuz, tgecho, hwaara@…, chris+django@…, alex@…, lancelotj, drdee, jeverling@…, honyczek@…, j.arnds@…, mindsocket, sergzach, jeroen@…, ivan_virabyan Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

I have been using the AUTH_PROFILE_MODULE setting paired with the get_profile() feature of the Django auth system and have run into many cases where it would be nice to directly access extra arbitrary user information without having to call get_profile() for each record.

I have patched my Django installation (with the attached .diff) to allow the developer to provide their own auth_user module, with the requirement that any developer created user model contains the base fields provided by the default auth_user model. This not only allows you to continue to use the build-in Django admin modules that rely on the auth_user system, but it allows you to store information like a users address, or override the username field to accept emails, etc.

My solution to this problem involves a few things

  • Adding a settings AUTH_USER_MODULE that specifies the path to the custom auth_user model. (i.e. 'myproj.apps.users.models.User')
  • Moving the existing User class out of the ./django/contrib/auth/models.py into a subdirectory './django/contrib/auth/users/models.py' that will not automatically be registered as a model when someone adds the 'django.contrib.auth' module to their INSTALLED_APPS. The AUTH_USER_MODULE setting in global settings points to this class by default 'django.contrib.auth.users.models.DefaultUser'
  • Creating a UserTemplate class that every user auth system must inherit from. This ensures that the default functions (like check_password, has_permission, etc.) are available (all of which can be overridden in the custom User class.

Attachments (16)

auth_user_module.diff (8.1 KB) - added by nowell strite 7 years ago.
auth_user_module patch
auth_user_module2.diff (8.1 KB) - added by nowell strite 7 years ago.
updated patch, fixes typo in previous file
auth_user_module3.diff (8.7 KB) - added by nowell strite 7 years ago.
Bugfix: This version plays nicely with syncdb and works with the django.db.models.loading.get_model(....)
3011.diff (8.6 KB) - added by Gary Wilson <gary.wilson@…> 7 years ago.
patch against [4462]
3011.2.diff (6.5 KB) - added by Densetsu no Ero-sennin <densetsu.no.ero.sennin@…> 7 years ago.
patch against [6023]
3011.3.diff (6.5 KB) - added by Densetsu no Ero-sennin <densetsu.no.ero.sennin@…> 7 years ago.
oh sorry, my previous patch was against newforms-admin branch, here's the patch against trunk [6023]
3011.4.diff (9.6 KB) - added by Michael Richardson <michael@…> 6 years ago.
Brought proposed patch up to trunk and moved some methods around; added stub documentation.
3011.5.diff (15.5 KB) - added by Michael Richardson <michael@…> 6 years ago.
Last patch didn't include django/contrib/auth/default_user.py, which is where the, er, well, default user lives. Kind of important.
3011.6.diff (15.7 KB) - added by Michael Richardson <michael@…> 6 years ago.
Patch against [7832] and added a quick fix so that apps like contenttypes wouldn't think that the imported module was part of contrib.auth.
3011.7.diff (18.1 KB) - added by hvendelbo 6 years ago.
Nearly there
3011-r10115.diff (2.9 KB) - added by seocam 5 years ago.
New patch against the trunk [10115]
#3011-extendable auth_user.diff (3.5 KB) - added by Kronuz 3 years ago.
#3011-extendable_auth_user.diff (40.0 KB) - added by Kronuz 2 years ago.
#3011-extendable_auth_user-1.3.diff (41.5 KB) - added by Kronuz 2 years ago.
Updated for django 1.3
#3011-extendable_auth_user-1.4.diff (35.8 KB) - added by Kronuz 2 years ago.
Updated for django 1.4
t3011_swappable_validation.diff (2.4 KB) - added by mindsocket 20 months ago.
Make model validation more robust (checks that swappable referred to in setting exists)

Download all attachments as: .zip

Change History (139)

comment:1 Changed 7 years ago by anonymous

  • Type changed from defect to enhancement

Changed 7 years ago by nowell strite

auth_user_module patch

Changed 7 years ago by nowell strite

updated patch, fixes typo in previous file

Changed 7 years ago by nowell strite

Bugfix: This version plays nicely with syncdb and works with the django.db.models.loading.get_model(....)

comment:2 Changed 7 years ago by anonymous

  • milestone Version 1.0 deleted

Milestone Version 1.0 deleted

comment:3 Changed 7 years ago by Gary Wilson <gary.wilson@…>

  • Has patch set
  • Needs documentation set
  • Needs tests set
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

Cool, I didn't know this ticket existed. This was something I have been wanting to do but never found the time.

Thanks for the patch, I think you are headed in the right direction. I would say most of the existing methods that were left in UserTemplate should move along with the User model though. Take set_password for example, this would only work for a user model with a password attribute. By default, set_password should do nothing. Other methods like get_absolute_url and is_authenticated could stay since they would be sensible defaults the way they exist now.

Marking this as accepted as this has been a highly desired feature for quite a while.

Changed 7 years ago by Gary Wilson <gary.wilson@…>

patch against [4462]

comment:4 Changed 7 years ago by Gary Wilson <gary.wilson@…>

#3669 marked as a duplicate, and has some other ideas.

comment:5 Changed 7 years ago by anonymous

  • Cc densetsu.no.ero.sennin@… added

Changed 7 years ago by Densetsu no Ero-sennin <densetsu.no.ero.sennin@…>

patch against [6023]

Changed 7 years ago by Densetsu no Ero-sennin <densetsu.no.ero.sennin@…>

oh sorry, my previous patch was against newforms-admin branch, here's the patch against trunk [6023]

comment:6 Changed 7 years ago by esaj

+1 on adding this feature.

comment:7 Changed 6 years ago by Simon Litchfield <simon@…>

  • Cc simon@… added

Email, rather than username, certainly seems to be the standard these days for web authentication. Problem is, the User model only gives us a 30 character primary key field to work with.

I'm a big +1 for any improvement that would allow us to easily use our own model in place of the User model.

comment:8 Changed 6 years ago by michael@…

  • Cc michael@… added

I'm +1 on this. It would be extremely useful to have custom user models especially when dealing with things like openid.

comment:9 Changed 6 years ago by Rob Hudson <treborhudson@…>

  • Cc treborhudson@… added

+1. This would be great. What does it need to be included? Tests? Docs? How can we help?

comment:10 Changed 6 years ago by mrts

  • milestone set to 1.0

Looks to be in scope for 1.0.

comment:11 Changed 6 years ago by David Danier <goliath.mailinglist@…>

  • Cc goliath.mailinglist@… added

Funny I found this ticket (again). This is something I wanted to add to django for ages now, but going an rather radical approach. Instead of just being able to plug another User-model in I decided it would be nice to create a modular auth-framework, which lets you replace the permission-system as well, as I think "auth" are two parts: AUTHentication (who is requesting a page) and AUTHorization (may he request that page).

I filed a bug about this, too (#3669)...but it got closed (duplicate of this here). So - because it looks like things are getting forward here - I just wanted to get some discussion up again about this.

What do you think about going one step further and also create a modular permission system? What I implemeneted so far is a simple hook-system, using some ideas of the generic-auth-branch.

Would it be possible to make it a user-choice to include (or exclude) thinks like the existing permission, group and message-models in auth or even replace this parts, too?

Perhaps there can be shared some code or ideas? Some outdated code is available at https://svn.webmasterpro.de/django-auth-rewrite/

Anyway, I'm +1 for getting this integrated of course. ;-)

comment:12 Changed 6 years ago by emulbreh

+1

I'd like abstract models for permissions, groups and the admin stuff (is_superuser, is_staff, ..). Don't require any particular fields (auth backends may provide suitable mixins).

Something like the following would then be equivalent to the current User implementation:

from django.contrib.admin import AdminUserMixin
from django.contrib.authorization import UserWithGroupMixin, UserWithPermissionsMixin
from django.contrib.auth.backends.model import ModelBackendUserMixin

class User(UserWithPermissionsMixin, UserWithGroupMixin, AdminUserMixin, ModelBackendUserMixin):
    first_name = models.CharField(_('first name'), max_length=30, blank=True)
    last_name = models.CharField(_('last name'), max_length=30, blank=True)
    email = models.EmailField(_('e-mail address'), blank=True)
    last_login = models.DateTimeField(_('last login'), default=datetime.datetime.now)
    date_joined = models.DateTimeField(_('date joined'), default=datetime.datetime.now)
    ...

(Not thought through:
ModelBackendUserMixin could be a factory: class User(ModelBackendUserMixin(identity='email', password='password')), or would a metaclass solution be nicer ..)

comment:13 Changed 6 years ago by piranha

  • Cc piranha@… added

Changed 6 years ago by Michael Richardson <michael@…>

Brought proposed patch up to trunk and moved some methods around; added stub documentation.

Changed 6 years ago by Michael Richardson <michael@…>

Last patch didn't include django/contrib/auth/default_user.py, which is where the, er, well, default user lives. Kind of important.

comment:14 Changed 6 years ago by gwilson

  • milestone 1.0 deleted

An enhancement, not 1.0.

comment:15 Changed 6 years ago by anonymous

  • Cc hanne.moa@… added

Changed 6 years ago by Michael Richardson <michael@…>

Patch against [7832] and added a quick fix so that apps like contenttypes wouldn't think that the imported module was part of contrib.auth.

comment:16 Changed 6 years ago by Michael Richardson <michael@…>

So there are several major issues here, probably the largest being how we can make this integrate with the admin interface (perhaps we should be doing this on newforms-admin?) and current generic apps.

I would like to find a way to make it dynamic rather than just saying, "Oh, you're using your own user model, sorry you're SOL if you want to use this and this and this."

I'm also in now way sure that the current method of doing it is the proper one, but now that the diff is current we can at least figure out where to go from here.

Anybody volunteering or have any ideas?

comment:17 Changed 6 years ago by anonymous

  • Cc osborn.steven@… added

comment:18 Changed 6 years ago by Rob Hudson <treborhudson@…>

I like the idea of duck typing. If your provided user model has x, y, and z fields or properties, and X, Y, and Z methods, then the default admin and other apps will accept it and use it as if it were the default. That way, if you want your own model to be used by the admin, you make sure those properties and methods are available and work. We can even provide an AuthUserTemplate that have stubs that raise a NotImplementedError. (I don't know what you normally do for the properties though.) Similar to how the HttpResponse method has checks for _is_string, I can imagine that the user module loading mechanism checks these properties and fields and adds a _is_auth (or something) that the various other subsystems can check against.

comment:19 Changed 6 years ago by David Danier <goliath.mailinglist@…>

So, this one will get a little shorter, I way right through with my comment when firefox crashed...great.

What I did, when trying to solve this (see #comment:11) was to strip down django.contrib.auth to only provide some generic API for all common auth-related stuff. So django.contrib.auth would only provide some API, which functions/methods just wraping the real functions/methods provided by the module(s) defined in django.settings. Additionally I was able to split the rest of django.contrib.auth into separate applications, so django got independent modules for users (auth_user) and permissions (auth_model_perm), but is not limited to this applications (for example groups could get its own application or the whole auth-backends-system could get its own user-application, removing the need of supporting backends in the main auth_user-application). Users could then use the django-users in combination with some self-written permission-system (supporting object-level-permissions for example), or do it the other way round (thats what the current patch provides).

However my code was never ready and did not solve every problem, first of all when dealing with backwards compability. I tried to set sane defaults (current applications as default selected modules) and provide support for old code, like imports (from django.contrib.auth.models import User), but thats only the tip of the iceberg.

But I think the direction my approach went was not all bad, it provided the developer with an API for all auth-related stuff and supported him with sane defauls (thats what the current auth-application does). Anyway someone could replace this with its own implementation while keeping third-party applications supported (google code lists many projects, many of them use django.contrib.auth some way), even for permission tasks (thats something generic-auth tried to solve).

Of course there needs to be some work done to provide some defaults (e.g. users should at least be displayable in templates using {{ user|escape }}), but not everything is really needed. For example the message-system is nice, but most applications won't use it. Perhaps a cleanup of the auth-application could make this optional and provide some simple way to test for message-support in third-party-applications (if hasattr(request.user, 'send_message')?). But that would be post-1.0. ;-)
(And btw: I really like the idea of using mixins here)

comment:20 Changed 6 years ago by anonymous

  • Cc michal@… added

comment:21 Changed 6 years ago by marinho

  • Cc marinho added

comment:22 Changed 6 years ago by hvendelbo

patch number 6 won't work if you don't override the user model. I guess it needs to conditionally delete the entry.

comment:23 Changed 6 years ago by hvendelbo

might also want to set the table name on the default user to auth_user

comment:24 Changed 6 years ago by Uninen

  • Cc ville@… added

comment:25 Changed 6 years ago by hvendelbo

My current change includes making UserTemplate abstract, and setting db_table = "auth_user" on DefaultUser. This should ensure that this patch doesn't change default installation table names. I haven't verified that yet though.

comment:26 Changed 6 years ago by anonymous

  • Cc sylvaintersideral@… added

comment:27 Changed 6 years ago by david

  • Cc larlet@… added

comment:28 Changed 6 years ago by dan

  • Owner changed from nobody to dan
  • Status changed from new to assigned

This is great. Just what I've been looking for. Can we change the name of the settings variable, though? I think good naming is really important, and calling it AUTH_USER_MODULE when it doesn't name a module is a bad idea. Can we call it AUTH_USER_MODEL instead?

comment:29 Changed 6 years ago by dan

Uh oh... I just submitted a comment and left everything else set to the defaults, which appears to have inadvertently changed the ticket state. Can someone undo that?

comment:30 Changed 6 years ago by dan

  • Cc dan@… added

Changed 6 years ago by hvendelbo

Nearly there

comment:31 Changed 6 years ago by wayla

  • Cc waylan@… added

comment:32 Changed 6 years ago by aalbrecht

  • Cc albrecht.andi@… added

comment:33 Changed 6 years ago by darkpixel

  • Cc django@… added

comment:34 Changed 6 years ago by anonymous

  • Cc nreilly@… added

comment:35 Changed 6 years ago by anonymous

  • Cc tomasare@… added

comment:36 Changed 6 years ago by adunar

  • Cc youngj@… added

comment:37 Changed 5 years ago by david

After reviewing your patch, I just wonder why don't you put default fields and methods in the abstract TemplateUser class? This way, you don't have to copy/paste most of the content of User class in your custom one and you can still override some of them with your own if you want. Make sense?

ps: maybe AbstractUser is more descriptive than TemplateUser?

comment:38 Changed 5 years ago by siddhi

  • Cc siddhartag@… added

comment:39 follow-up: Changed 5 years ago by cornbread

  • Cc richard@… added

I'd like to see #3400, and #9463 in with this. For sorting and filtering user data across the user profile field.

comment:40 in reply to: ↑ 39 Changed 5 years ago by HM

Replying to cornbread:

I'd like to see #3400, and #9463 in with this. For sorting and filtering user data across the user profile field.

With extendable User-models, profile-models are no longer needed making #9463 redundant.

comment:41 Changed 5 years ago by david

Too bad that it has been rejected for "too many potential downsides" from 1.1 roadmap http://code.djangoproject.com/wiki/Version1.1Features

What's the status of the latest patch? Is there somebody still active on this feature?

comment:42 follow-up: Changed 5 years ago by HM

Strange development, that. I thought I was paying close attention to the 1.1 process, this feature getting +0 and then one -1 so I expected it to be deferred. I can't find any list of the many potential downsides anywhere, and the comment to the guy who set it -1 seemed like a comment to a -0-feature so I thought it was just a goof...

The "many potential downsides" ought to be listed here, anyway.

comment:43 in reply to: ↑ 42 ; follow-up: Changed 5 years ago by HM

Replying to HM:

Strange development, that. I thought I was paying close attention to the 1.1 process, this feature getting +0 and then one -1 so I expected it to be deferred.

More than one -1 now (from http://spreadsheets.google.com/ccc?key=pSqnCvef6OXmGWQ9qbEVMeA):

  • AH: Any sort of framework-in-the-framework like this makes me suspicious, and the implementation and design would have to be incredibly solid.
  • JB: Doesn't affect me one way or another.

There's also one +1, from GW.

But from this to "too many potential downsides"? *scratches head*

comment:44 in reply to: ↑ 43 Changed 5 years ago by ramiro

Replying to HM:

But from this to "too many potential downsides"? *scratches head*

You might want to take a look at this #django-dev IRC channel log for a related discussion: http://oebfare.com/logger/django-dev/2008/11/25/1/#17:32-977590

comment:45 follow-up: Changed 5 years ago by wkornewald

  • Cc wkornewald added

So, what's going to happen with this ticket? It's on the list of ideas for Summer of Code 2009.

We really need this feature and we'd like to have an officially supported API for this. Django's default user comes with first_name and last_name which is too specific for us. We only have full_name and calculated @property for first_name and last_name. Also, we don't have usernames. In a lot of cases email is sufficient, so why should the user bother about inventing a username? Of course, this means that in our case email should be a required and unique field.

Moreover, code that uses get_profile() is just more annoying to write and you have to call select_related(). It's much easier to have all user fields in the User model. Our code has definitely become easier to work with since we eliminated the profile model and put everything into the User.

In our app-engine-patch project (a Django port to App Engine) we have something similar:
http://code.google.com/p/app-engine-patch/wiki/CustomUserModel

We also allow for overriding the AnonymousUser (which sometimes needs additional attributes, too).

Now that I think about it, the AUTH_ADMIN_MODULE stuff isn't necessary because you can easily register this in some other app's admin.py.

comment:46 Changed 5 years ago by Alex

One thing I'll note for this is that with the new managed option it's very easy to make these pure python changes in your own code.

comment:47 Changed 5 years ago by wkornewald

And how would I replace the User model using that managed option without modifying the auth app?

comment:48 in reply to: ↑ 45 ; follow-up: Changed 5 years ago by David Danier <goliath.mailinglist@…>

Replying to wkornewald:

We really need this feature and we'd like to have an officially supported API for this. Django's default user comes with first_name and last_name which is too specific for us. We only have full_name and calculated @property for first_name and last_name. Also, we don't have usernames. In a lot of cases email is sufficient, so why should the user bother about inventing a username? Of course, this means that in our case email should be a required and unique field.

Moreover, code that uses get_profile() is just more annoying to write and you have to call select_related(). It's much easier to have all user fields in the User model. Our code has definitely become easier to work with since we eliminated the profile model and put everything into the User.

There are more fundamental flaws in the current approach. For example one might need to have AnonymousUser saved in the database, which normalized the way permission control works (permissions for the AnonymousUser can be saved to the database).

One bit problem I see with the current contrib.auth-module is, that it combines different things. There is no need to put code for authorization (who is accessing the page?) and authentication (is someone allowed to access something?) into the same app. I would prefer having these things in different applications so they could be replaced separately. contrib.auth could provide some kind of generic API for accessing these new applications. Having all of these different apps combined in contrib.auth makes redesigning these thing much more complicated. Perhaps a first step might be creating independent apps here.

comment:49 in reply to: ↑ 48 Changed 5 years ago by wkornewald

Replying to David Danier <goliath.mailinglist@gmx.de>:

There are more fundamental flaws in the current approach. For example one might need to have AnonymousUser saved in the database, which normalized the way permission control works (permissions for the AnonymousUser can be saved to the database).

One bit problem I see with the current contrib.auth-module is, that it combines different things. There is no need to put code for authorization (who is accessing the page?) and authentication (is someone allowed to access something?) into the same app. I would prefer having these things in different applications so they could be replaced separately. contrib.auth could provide some kind of generic API for accessing these new applications. Having all of these different apps combined in contrib.auth makes redesigning these thing much more complicated. Perhaps a first step might be creating independent apps here.

I like that. Moving out the permission system would also be very useful for an App Engine port because the current Permission model complicates queries unnecessarily. You can as well just store a string like "auth.can_delete" (or a whole list of such strings) which allows for much more efficient querying on non-relational DBs. This works very well in app-engine-patch.

comment:50 Changed 5 years ago by mtredinnick

Design discussion in bug tickets is a really bad place, since you won't reach the right audience. We have a mailing list (django-dev) for that, please use it. be aware, though, that now (as we're entering the push to get 1.1 out the door) is not a great time for getting a lot of attention on new designs, so be prepared to raise it again in the ealry 1.2 period.

comment:51 follow-up: Changed 5 years ago by david

  • Cc david added; larlet@… removed

What about the new proxy Meta attribute? Did someone try to extends a User for real with it? http://docs.djangoproject.com/en/dev/topics/db/models/#proxy-models

comment:52 in reply to: ↑ 51 Changed 5 years ago by David Danier <goliath.mailinglist@…>

Replying to david:

What about the new proxy Meta attribute?

I think this may help with some of the problems described here, at least some of the problems that are avoidable in pure python. Many of these things can be worked around using the advantages a dynamic language like python provides, too. For example someone could just import User and then do somethink like:

User.my_new_method = lambda self: self.something_i_want_to_do_here

This may not be as clean as using some proxy-class, but works fine and has the advantage of not needing to import some "other" user.

As said above I see more fundamental flaws in current contrib.auth. One might be that you have no clean way to replace AnonymousUser (and put it into the database for example). Another is the combination of four or five different applications into contrib.auth (authentication, authorization, groups, messages, permissions) which goes against everything you try to teach new django-users when creating applications and makes creation of new applications for only parts of this more complicated.

There are other tickets adressing some of these issues, see #4604 for example. You may also notice, that the current patch tries to put a combination of session and user-based messages into the template context. This really would not be necessary if you could just choose between two decoupled message-apps.

@mtredinnick: I'm not sure if restructuring the whole contrib.auth-app would be appropriate in 1.x anyway, so perhaps this is something that needs to be done for some 2.x-version. I will perhaps raise this issue on the mailinglist again, when 1.2 is released.

comment:53 Changed 5 years ago by bswenson

  • Cc bram@…, mike.lopez@… added

comment:54 Changed 5 years ago by gonz

  • Cc gonz added

Changed 5 years ago by seocam

New patch against the trunk [10115]

comment:55 follow-up: Changed 5 years ago by seocam

  • Cc seocam added
  • Owner changed from dan to seocam
  • Status changed from assigned to new

The patch above implements the UserTemplate keeping the original User but using an abstract model. This implementation it's not so flexible but it keeps the compatibility with 1.0 versions. Even if this patch it's not what we are looking for I'd like to the this ticket.

comment:56 in reply to: ↑ 55 ; follow-up: Changed 5 years ago by seocam

  • Status changed from new to assigned

Replying to seocam:

The patch above implements the UserTemplate keeping the original User but using an abstract model. This implementation it's not so flexible but it keeps the compatibility with 1.0 versions. Even if this patch it's not what we are looking for I'd like to the this ticket.

to get this ticket*

comment:57 in reply to: ↑ 56 ; follow-up: Changed 5 years ago by David Danier <goliath.mailinglist@…>

Replying to seocam:

Replying to seocam:

Even if this patch it's not what we are looking for I'd like to the this ticket.

to get this ticket*

I like to have some fix for this in Django, too....but I see one major downside on the approach of patching everything together to support these things: If you look at my proposal above you may agree, that - for example - having AUTH_BACKENDS is not neccessary to support different backends. One could just implement some kind of LdapUser-Model (which only has fields, that are really needed for LDAP) and use this. You can say similar things about AUTH_PROFILE_MODULE, which would be not neccessary any more.

So perhaps instead of creating an continuous growing contrib.auth-module it would be a good idea to concentrate on replacing it by something that reduces the overall overhead (in code and management). For me it looks like currently many things get patched in whithout getting down to the real problems and by providing many half-ready solutions on the road down to a flexible authentication system.

Why half-ready? AUTH_PROFILE_MODULE is a good example here. You may use user.get_profile() to load the profile for a particular user. But there is no way to auto-create the profile for new users (like get_or_create()). So you have to write your own register-view to get this working, or use some other way to work around this (I think there is some closed ticket for this, too).

comment:58 in reply to: ↑ 57 ; follow-up: Changed 5 years ago by seocam

I like to have some fix for this in Django, too....but I see one major downside on the approach of patching everything together to support these things: If you look at my proposal above you may agree, that - for example - having AUTH_BACKENDS is not neccessary to support different backends. One could just implement some kind of LdapUser-Model (which only has fields, that are really needed for LDAP) and use this. You can say similar things about AUTH_PROFILE_MODULE, which would be not neccessary any more.

So perhaps instead of creating an continuous growing contrib.auth-module it would be a good idea to concentrate on replacing it by something that reduces the overall overhead (in code and management). For me it looks like currently many things get patched in whithout getting down to the real problems and by providing many half-ready solutions on the road down to a flexible authentication system.

Why half-ready? AUTH_PROFILE_MODULE is a good example here. You may use user.get_profile() to load the profile for a particular user. But there is no way to auto-create the profile for new users (like get_or_create()). So you have to write your own register-view to get this working, or use some other way to work around this (I think there is some closed ticket for this, too).

I agree that the AUTH_PROFILE_MODULE is a half-ready solution, and I know that the patch I'm proposing it's not a final solution as well, but I can't see any solution which don't break the backwards compatibility and would fix everything here.

Also add a feature that we already know it's going to be deprecated it's not a good idea either.

So the idea here is have a long term solution which provides more much flexibility. I'll try to create a draft spec based on what I've read until now, and as soon as a have something I'll send in the list.

comment:59 Changed 5 years ago by cornbread

I'd also love to see a username option with this. ie The ability to use the email as username.

comment:60 in reply to: ↑ 58 ; follow-up: Changed 5 years ago by David Danier <goliath.mailinglist@…>

Replying to seocam:

[...] , but I can't see any solution which don't break the backwards compatibility and would fix everything here.

I would suggest adding some kind of contrib.newauth (like newforms) and keeping the old contrib.auth for some time (until 2.0?). One problem I see with this would be foreign keys, but perhaps we could provide some (hackish) workaround for this.

So the idea here is have a long term solution which provides more much flexibility. I'll try to create a draft spec based on what I've read until now, and as soon as a have something I'll send in the list.

Perhaps we could put up a wiki-page to work on this proposal. I would like to write down some ideas, too. ;-)

comment:61 in reply to: ↑ 60 Changed 5 years ago by hvendelbo

Replying to David Danier <goliath.mailinglist@gmx.de>:

I'm using the patch (might have tweaked it slightly), and without setting an override Django works as before. I'm just about to put the patch on GitHub git://github.com/thepian/django.git
I've been thinking that a general model override mechanism would be preferable, I'm not sure what the best approach would be.
I'll be at Euro DjangoCon if you want to discuss this.

comment:62 Changed 5 years ago by alexkoshelev

  • Cc alexkoshelev added

comment:63 Changed 5 years ago by gonz

comment:64 Changed 5 years ago by sfllaw

  • Cc sfllaw added

comment:65 Changed 5 years ago by robin

  • Cc robin added

comment:66 Changed 5 years ago by sayane

  • Cc say4ne@… added

I'm waiting for it too ;-)

comment:67 Changed 5 years ago by ramusus

  • Cc ramusus@… added

comment:68 Changed 5 years ago by veena

  • Cc stv@… added

Maybe you can inspire in the API design of authentication and authorization module in Nette Framework. I think, it's designed enough abstract and with huge practical view http://api.nettephp.com/0.9/li_Nette-Security.html and http://api.nettephp.com/0.9/Nette-Web/User.html However, storage for credentials, permissions, user profile etc. is not implemented in this API. It could be in database or anywhere else.

comment:69 Changed 5 years ago by tinodb

  • Cc tinodb@… added

comment:70 Changed 5 years ago by leond

*bump*
Any new thoughts? The newauth-idea sounds good.

comment:71 Changed 5 years ago by HM

I use the function get_user_model() which reads from a setting what user-model to use. Then instead of

    from django.contrib.auth.models import User
    ...
    user = models.ForeignKey(User...

I do

    User = get_user_model()
    ...
    user = models.ForeignKey(User...

The apps using the user-model must not have conflicting needs of course, but we are after all using Python and are all consenting adults, right? No need for handcuffs and bullet-proof contracts.

comment:72 Changed 4 years ago by danfairs

  • Cc danfairs added

comment:73 Changed 4 years ago by anonymous

  • Cc miracle2k added

comment:74 follow-up: Changed 4 years ago by patrys

How about doing exactly what the contrib.comments app does?

Provide:

  • contrib.auth.get_model
  • contrib.auth.get_registration_form
  • etc.

comment:75 in reply to: ↑ 74 Changed 4 years ago by HM

Replying to patrys:

How about doing exactly what the contrib.comments app does?

Provide:

  • contrib.auth.get_model
  • contrib.auth.get_registration_form
  • etc.

I tried extending contrib.comments as per those functions and etc., and wound up writing my own from scratch. When one does not want, under any circumstance whatsoever until the heat-death of the universe, a non-authenticated non-authorized person to ever leave a comment, it just can't be built around.

Better to remove complexity than pile on another layer.

comment:76 Changed 4 years ago by patrys

HM:

Should be as easy as pointing get_form_target to the classic view decorated with @login_required or even @user_passes_test.

comment:77 Changed 4 years ago by msaelices

  • Cc msaelices@… added

comment:78 Changed 4 years ago by mitar

  • Cc mmitar@… added

comment:79 Changed 4 years ago by aball

  • Cc anball@… added

comment:80 Changed 4 years ago by shanx

  • Cc remco@… added

comment:81 Changed 3 years ago by rizumu

  • Cc tom@… added

comment:82 Changed 3 years ago by gabrielhurley

  • Component changed from Contrib apps to contrib.auth

comment:83 Changed 3 years ago by lrekucki

  • Severity changed from normal to Normal
  • Type changed from enhancement to New feature

Changed 3 years ago by Kronuz

comment:84 Changed 3 years ago by Kronuz

I found the current patch produces a bug if used alongside the contrib.admin app under certain circumstances when the new User model has foreign key fields with lazy relations to other app models. This problem arises since those models are still unresolved when django later attempts to import the INSTALLED_APP modules, while fetching the locales during the creation of the languages translation object, at that point, while importing the admin app, contrib/admin/__init__.py tries to import contrib.admin.sites, which results in the importing of AuthenticationForm... __new__() then tries to use formfield() on the User's available fields for the User model, but since those foreign key fields still contain unresolved strings, it throws an exception (AttributeError: 'str' object has no attribute '_default_manager'). I'm attaching a new patch which fixes this issue.

comment:85 Changed 3 years ago by rizumu

  • Easy pickings unset

Kronuz: I too ran into this. Kept hitting the error, cannot import name 'actions' in the admin whenever running createsuperuser, or custom mgmnt commands that relied on User.

Your patch resolved this issue for me, but I had to change one line. Specifically, this was failing because the attribute couldn't be found.

getattr(auth_user_module, auth_user_module_parts[0])

Importing the '.modules' resolved it:

auth_user_module = __import__(auth_user_module_parts[0] + '.models', fromlist=['dummy'])
Last edited 3 years ago by rizumu (previous) (diff)

comment:86 Changed 3 years ago by rizumu

  • Easy pickings set

comment:87 follow-up: Changed 3 years ago by jezdez

  • Easy pickings unset

With all due respect, this isn't easy at all.

comment:88 in reply to: ↑ 87 Changed 3 years ago by rizumu

Replying to jezdez:

With all due respect, this isn't easy at all.

I thought I unset it on accident, it's definitely not easy.

comment:89 Changed 3 years ago by wim@…

  • Owner changed from seocam to nobody
  • Status changed from assigned to new
  • UI/UX unset

comment:90 Changed 3 years ago by frankoid

  • Cc frankoid added

comment:91 Changed 2 years ago by wdoekes

  • Cc wdoekes added

comment:92 Changed 2 years ago by aaugustin

#17254 was a duplicate.

comment:93 Changed 2 years ago by Kronuz

  • Cc Kronuz added

comment:94 Changed 2 years ago by gonz

  • Cc gonz removed

comment:95 Changed 2 years ago by wkornewald

  • Cc wkornewald removed

comment:96 Changed 2 years ago by siddhi

  • Cc siddhartag@… removed

comment:97 Changed 2 years ago by tgecho

  • Cc tgecho added

comment:98 Changed 2 years ago by hwaara@…

  • Cc hwaara@… added

comment:99 Changed 2 years ago by Aaron C. de Bruyn <django@…>

  • Cc django@… removed

comment:100 Changed 2 years ago by EroSennin

  • Cc densetsu.no.ero.sennin@… removed

comment:101 Changed 2 years ago by piranha

  • Cc piranha@… removed

comment:102 Changed 2 years ago by Kronuz

There are problems some times when you need to import the UserTemplate (circular imports) ...for this reason, I split the file to put the models in a new file called base.py (perhaps abstract.py could be a better name) and left the model.py file to be only for the creation of the default User from the template, or loading of the custom User model class from some other module (as set up in AUTH_USER_MODULE, which should be a string in the form: custom_auth.models.User) The custom user model could (and maybe should) have the abstract contrib.auth.base.UserTemplate model as its base. I believe we could also have a BaseUserAbstractModel in addition to (or instead of) `UserTemplate.
I'm attaching the patch now.

Changed 2 years ago by Kronuz

Changed 2 years ago by Kronuz

Updated for django 1.3

Changed 2 years ago by Kronuz

Updated for django 1.4

comment:103 Changed 2 years ago by Kronuz

This solution it's not perfect... I know, but it works well for me and others... so I updated it to work with both django 1.3 and 1.4. What I currently changed from the past versions of the patch is that AUTH_USER_MODULE now works more like AUTH_PROFILE_MODULE, where you supply the app_label and model, and not the full python import to the model (i.e. it now expects custom_auth.User instead of custom_auth.models.User). Custom User models should still have contrib.auth.base.UserTemplate as its base (note UserTemplatemust be loaded from contrib.auth.base.UserTemplate, not contrib.auth.models.UserTemplate, to avoid possible circular imports where nasty things happen).

It'd probably be cleaner if the patch used models.get_model() instead of importing it using __import__(), but for some reason (most likely more circular imports) I couldn't make it work reliably.

comment:104 Changed 2 years ago by gcc

  • Cc chris+django@… added

comment:105 Changed 2 years ago by alex@…

  • Cc alex@… added

comment:106 Changed 2 years ago by clay

  • Cc clay added

Here's a fork of 1.4rc1 that allows for pluggable auth apps using the strategy some have already suggested in this ticket, namely, defining User at run-time to be either the default implementation or a user-provided implementation:

https://github.com/claymation/django/compare/pluggable-auth-apps

In addition to User, all of the companion models, forms, backends, decorators, and middleware classes that depend on User are similarly defined at run-time. The complete set of objects defined in this manner represents what I consider to be the public interface to the existing auth.User model:

   models:      User, UserManager, AnonymousUser, Group, Permission, SiteProfileNotAvailable
   admin:       UserAdmin, GroupAdmin
   backends:    ModelBackend, RemoteUserBackend
   decorators:  user_passes_test, login_required, permission_required
   middleware:  RemoteUserMiddleware
   forms:       AuthenticationForm, UserCreationForm, UserChangeForm,
                AdminPasswordChangeForm, PasswordChangeForm, PasswordResetForm, SetPasswordForm

Developers are free to define their own pluggable auth apps, by setting AUTH_APP and adding the app to INSTALLED_APPS, following the COMMENTS_APP pattern. Here is one such pluggable auth app, which provides a User model with no username field and forms/backends/middleware that allow users to login with their email address:

https://github.com/claymation/django_email_auth

Is this the same approach that has been rejected for inclusion in core previously, and if so, why?

comment:107 Changed 2 years ago by lancelotj

  • Cc lancelotj added

comment:108 Changed 2 years ago by drdee

  • Cc drdee added

comment:109 Changed 2 years ago by clay

  • Cc clay removed

comment:110 Changed 2 years ago by carljm

There was recently a lengthy thread on the django-developers mailing list about this issue. The proposed solution that was eventually selected by the BDFLs was essentially a swappable User model, as summarized in option 2a in ContribAuthImprovements.

comment:111 Changed 2 years ago by jeverling

  • Cc jeverling@… added

comment:112 Changed 23 months ago by russellm

I've just added a draft branch implementing the BDFL-approved solution: https://github.com/freakboy3742/django/tree/t3011

comment:113 Changed 22 months ago by honyczek@…

  • Cc honyczek@… added

comment:114 Changed 22 months ago by jelko

  • Cc j.arnds@… added

comment:115 Changed 20 months ago by anonymous

NB to test with a custom User model that has an FK('self') defined.

Changed 20 months ago by mindsocket

Make model validation more robust (checks that swappable referred to in setting exists)

comment:116 Changed 20 months ago by mindsocket

First time contributor, so please be firm but gentle :)

Attached a patch (applies on to Russell's t3011 branch) to make model validation and get_user_model function fail more gracefully if swappable setting is malformed or referenced model does not exist. Feedback welcome. In particular I'm conscious that there isn't a unit test for this addition (though not sure if that's applicable in this case)

Last edited 20 months ago by mindsocket (previous) (diff)

comment:117 Changed 20 months ago by mindsocket

  • Cc mindsocket added

comment:118 Changed 20 months ago by sergzach

  • Cc sergzach added

comment:119 Changed 19 months ago by dekkers

  • Cc jeroen@… added

comment:120 Changed 19 months ago by danellis

  • Cc danellis added

comment:121 Changed 19 months ago by danellis

  • Cc danellis removed

comment:122 Changed 19 months ago by ivan_virabyan

  • Cc ivan_virabyan added

comment:123 Changed 19 months ago by Russell Keith-Magee <russell@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 70a0de37d132e5f1514fb939875f69649f103124:

Fixed #3011 -- Added swappable auth.User models.

Thanks to the many people that contributed to the development and review of
this patch, including (but not limited to) Jacob Kaplan-Moss, Anssi
Kääriäinen, Ramiro Morales, Preston Holmes, Josh Ourisman, Thomas Sutton,
and Roger Barnes, as well as the many, many people who have contributed to
the design discussion around this ticket over many years.

Squashed commit of the following:

commit d84749a0f034a0a6906d20df047086b1219040d0
Merge: 531e771 7c11b1a
Author: Russell Keith-Magee <russell@…>
Date: Wed Sep 26 18:37:04 2012 +0800

Merge remote-tracking branch 'django/master' into t3011

commit 531e7715da545f930c49919a19e954d41c59b446
Merge: 29d1abb 1f84b04
Author: Russell Keith-Magee <russell@…>
Date: Wed Sep 26 07:09:23 2012 +0800

Merged recent trunk changes.

commit 29d1abbe351fd5da855fe5ce09e24227d90ddc91
Merge: 8a527dd 54c81a1
Author: Russell Keith-Magee <russell@…>
Date: Mon Sep 24 07:49:46 2012 +0800

Merge remote-tracking branch 'django/master' into t3011

commit 8a527dda13c9bec955b1f7e8db5822d1d9b32a01
Author: Russell Keith-Magee <russell@…>
Date: Mon Sep 24 07:48:05 2012 +0800

Ensure sequences are reset correctly in the presence of swapped models.

commit e2b6e22f298eb986d74d28b8d9906f37f5ff8eb8
Author: Russell Keith-Magee <russell@…>
Date: Sun Sep 23 17:53:05 2012 +0800

Modifications to the handling and docs for auth forms.

commit 98aba856b534620aea9091f824b442b47d2fdb3c
Author: Russell Keith-Magee <russell@…>
Date: Sun Sep 23 15:28:57 2012 +0800

Improved error handling and docs for get_user_model()

commit 0229209c844f06dfeb33b0b8eeec000c127695b6
Merge: 6494bf9 8599f64
Author: Russell Keith-Magee <russell@…>
Date: Sun Sep 23 14:50:11 2012 +0800

Merged recent Django trunk changes.

commit 6494bf91f2ddaaabec3ec017f2e3131937c35517
Author: Russell Keith-Magee <russell@…>
Date: Mon Sep 17 21:38:44 2012 +0800

Improved validation of swappable model settings.

commit 5a04cde342cc860384eb844cfda5af55204564ad
Author: Russell Keith-Magee <russell@…>
Date: Mon Sep 17 07:15:14 2012 +0800

Removed some unused imports.

commit ffd535e4136dc54f084b6ac467e81444696e1c8a
Author: Russell Keith-Magee <russell@…>
Date: Sun Sep 16 20:31:28 2012 +0800

Corrected attribute access on for get_by_natural_key

commit 913e1ac84c3d9c7c58a9b3bdbbb15ebccd8a8c0a
Author: Russell Keith-Magee <russell@…>
Date: Sun Sep 16 20:12:34 2012 +0800

Added test for proxy model safeguards on swappable models.

commit 280bf19e94d0d534d0e51bae485c1842558f4ff4
Merge: dbb3900 935a863
Author: Russell Keith-Magee <russell@…>
Date: Sun Sep 16 18:16:49 2012 +0800

Merge remote-tracking branch 'django/master' into t3011

commit dbb3900775a99df8b6cb1d7063cf364eab55621a
Author: Russell Keith-Magee <russell@…>
Date: Sun Sep 16 18:09:27 2012 +0800

Fixes for Python 3 compatibility.

commit dfd72131d8664615e245aa0f95b82604ba6b3821
Author: Russell Keith-Magee <russell@…>
Date: Sun Sep 16 15:54:30 2012 +0800

Added protection against proxying swapped models.

commit abcb027190e53613e7f1734e77ee185b2587de31
Author: Russell Keith-Magee <russell@…>
Date: Sun Sep 16 15:11:10 2012 +0800

Cleanup and documentation of AbstractUser base class.

commit a9491a87763e307f0eb0dc246f54ac865a6ffb34
Merge: fd8bb4e 08bcb4a
Author: Russell Keith-Magee <russell@…>
Date: Sun Sep 16 14:46:49 2012 +0800

Merge commit '08bcb4aec1ed154cefc631b8510ee13e9af0c19d' into t3011

commit fd8bb4e3e498a92d7a8b340f0684d5f088aa4c92
Author: Russell Keith-Magee <russell@…>
Date: Sun Sep 16 14:20:14 2012 +0800

Documentation improvements coming from community review.

commit b550a6d06d016ab6a0198c4cb2dffe9cceabe8a5
Author: Russell Keith-Magee <russell@…>
Date: Sun Sep 16 13:52:47 2012 +0800

Refactored skipIfCustomUser into the contrib.auth tests.

commit 52a02f11107c3f0d711742b8ca65b75175b79d6a
Author: Russell Keith-Magee <russell@…>
Date: Sun Sep 16 13:46:10 2012 +0800

Refactored common 'get' pattern into manager method.

commit b441a6bbc7d6065175715cb09316b9f13268171b
Author: Russell Keith-Magee <russell@…>
Date: Sun Sep 16 13:41:33 2012 +0800

Added note about backwards incompatible change to admin login messages.

commit 08bcb4aec1ed154cefc631b8510ee13e9af0c19d
Author: Anssi Kääriäinen <akaariai@…>
Date: Sat Sep 15 18:30:33 2012 +0300

Splitted User to AbstractUser and User

commit d9f5e5addbad5e1a01f67e7358e4f5091c3cad81
Author: Anssi Kääriäinen <akaariai@…>
Date: Sat Sep 15 18:30:02 2012 +0300

Reworked REQUIRED_FIELDS + create_user() interaction

commit 579f152e4a6e06671e1ac1e59e2b43cf4d764bf4
Merge: 9184972 93e6733
Author: Russell Keith-Magee <russell@…>
Date: Sat Sep 15 20:18:37 2012 +0800

Merge remote-tracking branch 'django/master' into t3011

commit 918497218c58227f5032873ff97261627b2ceab2
Author: Russell Keith-Magee <russell@…>
Date: Sat Sep 15 20:18:19 2012 +0800

Deprecate AUTH_PROFILE_MODULE and get_profile().

commit 334cdfc1bb6a6794791497cdefda843bca2ea57a
Author: Russell Keith-Magee <russell@…>
Date: Sat Sep 15 20:00:12 2012 +0800

Added release notes for new swappable User feature.

commit 5d7bb22e8d913b51aba1c3360e7af8b01b6c0ab6
Author: Russell Keith-Magee <russell@…>
Date: Sat Sep 15 19:59:49 2012 +0800

Ensure swapped models can't be queried.

commit 57ac6e3d32605a67581e875b37ec5b2284711a32
Merge: f2ec915 abfba3b
Author: Russell Keith-Magee <russell@…>
Date: Sat Sep 15 14:31:54 2012 +0800

Merge remote-tracking branch 'django/master' into t3011

commit f2ec915b20f81c8afeaa3df25f80689712f720f8
Merge: 1952656 5e99a3d
Author: Russell Keith-Magee <russell@…>
Date: Sun Sep 9 08:29:51 2012 +0800

Merge remote-tracking branch 'django/master' into t3011

commit 19526563b54fa300785c49cfb625c0c6158ced67
Merge: 2c5e833 c4aa26a
Author: Russell Keith-Magee <russell@…>
Date: Sun Sep 9 08:22:26 2012 +0800

Merge recent changes from master.

commit 2c5e833a30bef4305d55eacc0703533152f5c427
Author: Russell Keith-Magee <russell@…>
Date: Sun Sep 9 07:53:46 2012 +0800

Corrected admin_views tests following removal of the email fallback on admin logins.

commit 20d1892491839d6ef21f37db4ca136935c2076bf
Author: Russell Keith-Magee <russell@…>
Date: Sun Sep 9 01:00:37 2012 +0800

Added conditional skips for all tests dependent on the default User model

commit 40ea8b888284775481fc1eaadeff267dbd7e3dfa
Author: Russell Keith-Magee <russell@…>
Date: Sat Sep 8 23:47:02 2012 +0800

Added documentation for REQUIRED_FIELDS in custom auth.

commit e6aaf659708cf6491f5485d3edfa616cb9214cc0
Author: Russell Keith-Magee <russell@…>
Date: Sat Sep 8 23:20:02 2012 +0800

Added first draft of custom User docs.

Thanks to Greg Turner for the initial text.

commit 75118bd242eec87649da2859e8c50a199a8a1dca
Author: Thomas Sutton <me@…>
Date: Mon Aug 20 11:17:26 2012 +0800

Admin app should not allow username discovery

The admin app login form should not allow users to discover the username
associated with an email address.

commit d088b3af58dad7449fc58493193a327725c57c22
Author: Thomas Sutton <me@…>
Date: Mon Aug 20 10:32:13 2012 +0800

Admin app login form should use swapped user model

commit 7e82e83d67ee0871a72e1a3a723afdd214fcefc3
Merge: e29c010 39aa890
Author: Russell Keith-Magee <russell@…>
Date: Fri Sep 7 23:45:03 2012 +0800

Merged master changes.

commit e29c010beb96ca07697c4e3e0c0d5d3ffdc4c0a3
Merge: 8e3fd70 30bdf22
Author: Russell Keith-Magee <russell@…>
Date: Mon Aug 20 13:12:57 2012 +0800

Merge remote-tracking branch 'django/master' into t3011

commit 8e3fd703d02c31a4c3ac9f51f5011d03c0bd47f6
Merge: 507bb50 26e0ba0
Author: Russell Keith-Magee <russell@…>
Date: Mon Aug 20 13:09:09 2012 +0800

Merged recent changes from trunk.

commit 507bb50a9291bfcdcfa1198f9fea21d4e3b1e762
Author: Russell Keith-Magee <russell@…>
Date: Mon Jun 4 20:41:37 2012 +0800

Modified auth app so that login with alternate auth app is possible.

commit dabe3628362ab7a4a6c9686dd874803baa997eaa
Author: Russell Keith-Magee <russell@…>
Date: Mon Jun 4 20:10:51 2012 +0800

Modified auth management commands to handle custom user definitions.

commit 7cc0baf89d490c92ef3f1dc909b8090191a1294b
Author: Russell Keith-Magee <russell@…>
Date: Mon Jun 4 14:17:28 2012 +0800

Added model Meta option for swappable models, and made auth.User a swappable model

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.