AbstractUser to get more abstract

[Abdulaziz Alfoudari]

This is a continuation of my post on stackoverflow.

With the introduction of Django 1.5, it was possible to create a custom User model which is flexible enough to have any user profile the developer wants created. However, looking at a very common problem which is using the email as the primary user identifier instead of username, the solution requires copying most of Django's internal definition of AbstractUser and that is only to remove the username field.

A better solution in my opinion is make AbstractUser even more abstract by removing username field, and allowing the developer to explicitly specify the field to be used as the user identifier. This will require a tiny extra work for those that use the current default behavior, but it will also greatly reduce the work needed for the very common problem of using email as the user identifier.

Please share your thoughts and opinions on this.

[Cal Leeming [Simplicity Media Ltd]]

Although I cannot comment on the specific of how this could be done better, I do at least concur that implementing a custom AbstractUser could be done in a much better way!!

Cal

--
You received this message because you are subscribed to the Google Groups "Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
For more options, visit https://groups.google.com/groups/opt_out.

[Russell Keith-Magee]

The short answer: this isn't going to happen. AbstractUser is a released and documented API, so we're not in a position to change it in the way you describe without causing massive inconvenience to everyone that is using it at present (at least, I don't see an obvious way that this could be done).

However, ticket #20824 describes a proposal to add an email-login analog of Django's built-in user. This would make introduction of email-based login a matter of 2 lines of configuration. This ticket is really just waiting on someone to prepare a patch… and it should be a relatively simple patch to prepare. If you're looking to get involved in Django development, this would be an easy place to start.

Yours,

Russ Magee %-)

[Aaron Merriam]

Check out django-authtools

https://django-authtools.readthedocs.org/en/latest/

Provides a few abstract base classes that make this very easy to accomplish.  I'm sure there are other 3rd party apps doing the same.

[tanderegg]

Hey folks - I took a crack at implementing this, please check out my comment here (which contains a link to the branch in my fork): https://code.djangoproject.com/ticket/20824#comment:4

Let me know if I missed anything!

Tim

[James Pic]

Why not just override the username class attribute in your subclass ?
by None or even something which returns instance.email.

In reality I have no idea, some of my users are using their email
address as usernames and it's not a problem for django, so anything
project-specific would sound a little overkill imho.

Did I miss anything critical ?

[Jorge Cardoso Leitão]

Hi Tim.

Thanks for posting this here. I was not aware of this ticket.

Thank you also for taking the initiative and starting moving on this front.

First of all, like you, I'm also recent in Django dev. This means that most likely, we will both make mistakes in the approach.

The important is that we learn with them, and that is also why we are in a community of more experience users.

Let me try to convey why is important that we take into account what other people have already done (i.e. existing apps).

I'm using django-authtools; I found the the app is has tests and it is well documented.

I'm using it in production and so far I didn't found any issue.

My point is: the main reason we try to use existing code is not because the code is there, but because it is normally tested (both UnitTests and people using it and reporting issues) and documented, which means it is less prone to errors and more robust to integration.

That said, I would start by comparing APIs.

That allow us to have a solid ground on what there is already out there, to see if any of the existing apps fulfils our expectations of what we would like to have in the core.

I tried to clone your branch ticket_20824 from github to compare your app with django-authtools, but it is (far) behind Django master branch, which is causing merge conflicts that I don't know how to resolve (because I don't know what you are exactly doing).

Maybe I'm doing something wrong or I don't know enough of git, but could you please tell us (or maybe is just me that need to be told!) how can we compare your patch with Django-master?

Let me try to give an example:

After taking a look at github.com/Liberationtech/django-libtech-emailuser, the following are the main differences and similarities with django-authtools.

Please notice that I'm a (so far) 1-time comitter of django-authtools, so take this comparison with appropriate care as I'm certainly biased.

Where they are similar:

1. Both are subclasses of AbstractBaseUser and PermissionsMixin (thus allow permissions API to be used)

2. Both implement the required forms for creation, change, passwordReset.

3. Both implement admin interface

Where they are different:

1. Models implementation:

emailuser uses one model with 3 fields: email, first_name, last_name (USERNAME_FIELD = 'email)

django-authtools uses defines three models: 

A. AbstractUserMail (with one field 'email', like emailuser),

B. AbstractNamedUser, derived from AbstractUserMail with an additional field "name". 

C. User, derived from AbstractNamedUser, which is swappable and non-abstract.

2. django-authtools has tests, emailuser doesn't

3. django-authtools is documented, emailuser doesn't

4. django-authtools implements views, emailuser doesn't.

5. they have different licences, but I don't know enough about them to know what are the differences.

6. django-authtools passes on travis-ci.org tests, and, as far as I know, enforces clean commit history (don't know if it is relevant thought).

7. usermail has a form to ask reset-password (by email?), but I didn't found any implementation of the actual reset.

Tom, would you be willing to provide the differences and similarities of your API with django-authtools and emailuser, or teach me how can I compare it against django-master so I can make a comparison?

It would be nice to have your API documented here so we can take the most out of the all existing implementations.

Best regards,

Jorge

[tanderegg]

Hi Jorge -

Thanks for the response.  Rookie mistake: I had cloned from 1.6.x stable instead of master, the branch is now fixed to be branched from django/master from 10 minutes ago, that should resolve your conflicts.

In terms of API, the API I went with is basically identical to the API used in django-libtech-emailuser, with the exception that I did not include a new password_reset form.  Testing showed this was not necessary, the one in django.contrib.auth works fine with my model.  The reason our API's are the same is because we both copied the existing API for django.contrib.auth.User exactly.  This means first_name and last_name, along with the email, password, is_active, is_staff, and is_superuser all exist in the auth_email.AbstractUser model, just like they do in auth.AbstractUser.

I went this route one the principle that simplicity is best, and I wanted folks to get the exact same behaviour from auth_email.AbstractUser, and auth_email.User, that they would expect from auth.AbstractUser and auth.User.

django-libtech-emailuser did not includ any tests, so my code goes beyond that to include a small test suite that covers everything I thought was nesseccary to cover.

I did not go the route of django-authtools because I felt that it went above and beyond the scope of this ticket, since it included multiple layers of abstraction, a rewrite of the views to use Class Based Views, etc.  I felt that if I were to go that route, then the re-write should occur in django.contrib.auth, not in my extension of it.  Hence, out of scope for this ticket since that could potentially affect an established API.

Let me know if that logic makes sense!

Tim

[tanderegg]

Quick addendum: I forgot that django-libtech-emailuser does not use an AbstractUser intermediary step, it simply has one model.  My approach has an abstract model, just like auth.User, with a concrete model that inherits from it and is swappable.

-Tim

[Aaron Merriam]

Up front, I'm one of the developers of django-authtools

I'm feel strongly that a new contrib application is the wrong way to accomplish this.  Based on my memory of auth and django-authtools, most of `auth` will just work with the AbstractEmailUser class provided by django-authtools.  Why not just include a new swappable user class in django.contrib.auth that uses email as username?  I believe that all of the built in django.contrib.auth views will work with the AbstractEmailUser model in django-authtools, All of the forms from django.contrib.auth except UserCreationForm and UserChangeForm work with AbstractEmailUser.  There would need to be changes to the ModelAdmin provided by django.contrib.auth. django-authtools has a ModelAdmin that works for the normal auth.User and with AbstractEmailUser, which would be a very minor change to django.contrib.auth.admin

Adding a new contrib application for this seems very heavy weight and to duplicate a lot of logic that already has a well established home.  Is there other support for doing this more inline with the way it was done in django-authtools.

[tanderegg]

Hi Aaron -

I set it up as a separate contrib app primariliy because that was the method suggested by Russel in the ticket.  It doesn't actually recreate any of the functionality of contrib.auth, it only does exactly what you describe: create an AbstractUser model (with a new manager), new versions of UserCreationForm and UserChangeForm, and a new ModelAdmin that inherits from contrib.auth.admin.UserAdmin and overrides the necessary members/methods.  In other words, my contrib.auth_email depends heavily on existing functionality in contrib.auth.

I'm personally agnostic on whether this should be in contrib.auth or as a separate contrib app.  The argument for a second contrib app would just be that it keeps unesseccary code out of the main auth app for people that don't want to use an email user.  Either way, I'll look into just altering contrib.auth.admin.UserAdmin to handle both cases, because that would be simpler than creating a new model admin that overrides functionality (as long as it doesn't alter existing behavior).

-Tim

[Aaron Merriam]

Russel, I'm curious if you could expand/explain your motivation on having this implemented as a separate contrib application rather than including it with django.contrib.auth

[Russell Keith-Magee]

Hi Aaron,

The motivation? Simple practicality. 

contrib.auth ships with a known manifest of models. If you add extra User models to contrib.auth, *every* Django project in existence will  gain extra User models -- unless we develop a whole lot of extra infrastructure to *not* install certain models.

Putting the "email-User' model in a separate app should mean it's a 2-line configuration to opt into a "standardised" email-based User model, and no special "don't install this model" infrastructure will be required.

As for the argument that there will be code duplication -- there shouldn't be. A contrib email-auth app isn't going to *replace* contrib.auth. contrib.auth has to be there for all the AUTH_USER_MODEL stuff to work. There might be a need for some refactoring of contrib.auth to provide better extension points, but we certainly aren't going to be reproducing the entirety of contrib.auth in contrib.emailauth -- emailauth will be extending the base provided by auth.

Yours,

Russ Magee %-)

[Russell Keith-Magee]

Hi all

It's great to see this discussion is happening -- however, given that we're debating the merits of different architectural approaches, with the aim of presenting a single approach for final inclusion in Django master, it would be worthwhile formalising the discussion. This is something we've done on a number of other occasions -- the discussion about pluggable contrib.auth:

https://code.djangoproject.com/wiki/ContribAuthImprovements

And the introduction of contrib.messages:

https://code.djangoproject.com/wiki/SessionMessages

are two good examples. This sort of summary is very helpful for the core team because it means we don't need to track hundreds of messages over dozens of threads -- we can focus on the end point of the discussion and make a clear decision.

I've set up a stub page here:

https://code.djangoproject.com/wiki/ContribEmailAuth

Feel free to fill in details. Try to keep the discussion objective; if there's a matter of opinion/taste in discussion, flag it as such.

Yours,

Russ Magee %-)

[Russell Keith-Magee]

On Wed, Sep 18, 2013 at 1:27 PM, Luke Sneeringer <lu...@sneeringer.com> wrote:

Russell,

I would love to do the work for the email-login analogue you describe. I actually proposed just such a thing a few months ago but was rebuffed.

I'm sorry to hear this. Out of interest, did a member of the core team actually say "no", or was it just a matter of proposing something and not getting traction? 

If it was the latter, it's important to remember that the core team are all volunteers, and sometimes the spare time of the core team doesn't necessarily match up with the spare time of volunteers in the wider community. As a result, well intentioned and desired work sometimes gets ignored. It's not (necessarily) being ignored because it was bad -- often it's just "we don't have enough cycles *right now*.

 

However, I think this would be extremely useful. Also, I am, in fact, looking to get involved with Django development, as I haven't quite navigated the hurdles successfully.

I do have one request, though. Is there a core developer that would be willing to "mentor" my work on this, so I can make sure I am writing something worthy of acceptance?

 

Well, I'm willing to mentor the effort. 

Like I've indicated, the first step isn't to write code at all - it's to get a good summary of the state of play of existing implementations. Multiple people have already taken a swing at an implementation; before we commit to one particular codebase, we need to understand what has already been offered by the community.

Yours,

Russ Magee %-)

[Luke Sneeringer]

Russell,

I would love to do the work for the email-login analogue you describe. I actually proposed just such a thing a few months ago but was rebuffed. However, I think this would be extremely useful. Also, I am, in fact, looking to get involved with Django development, as I haven't quite navigated the hurdles successfully.

I do have one request, though. Is there a core developer that would be willing to "mentor" my work on this, so I can make sure I am writing something worthy of acceptance?

L

[Timothy Anderegg]

Hi all - I updated Russ's new wiki page to include the work I've done so far: https://code.djangoproject.com/wiki/ContribEmailAuth  Again, the patch I've been working on is here: https://github.com/tanderegg/django/tree/ticket_20824_master  Please let me know if you have any feedback.

The only real other option (that I can see) would be to do something more extensive like django-authtools (https://django-authtools.readthedocs.org/en/latest/).  I can write up a description of this approach as well if that would be useful, although I did write up a description of the difference between my code and the two other existing django apps in a post earlier in this thread.

Thanks,

Tim

--

You received this message because you are subscribed to a topic in the Google Groups "Django developers" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/django-developers/rb7v9kVAK3I/unsubscribe.
To unsubscribe from this group and all its topics, send an email to django-develop...@googlegroups.com.

[Luke Sneeringer]

I added the authtools approach to the wiki for completion, although I believe it to be an inferior approach.

One thing I dislike is having a separate app (e.g. d.c.auth_email) that has to be installed separately. That feels pretty impure to me. I'm doing a thought exercise about potential solutions, though, and not exactly coming up aces.

The best solution I can currently think of is to have User and EmailUser which are both models that live in django.contrib.auth. Then, we would have to add code to our model detection that says that *if* a model is a subclass of AbstractBaseUser, include it if and only if it is the AUTH_USER_MODEL.

I can't decide if that solution is better or worse than the disease. It makes for a much more attractive set of steps to follow for people who want to use it, though -- basically, just set AUTH_USER_MODEL to 'auth.EmailUser', and done.

Thoughts?

Best Regards,

Luke Sneeringer

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

To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.

[Aaron Merriam]

Luke, I'm +1 on wanting a solution that allows "just set AUTH_USER_MODEL to 'auth.EmailUser', and done".

It'd be nice if 'swappable' could accomplish this for us.  Something along the lines of for any set of swappable models which are swappable for the same settings value, only load the one that is named by the settings value.  I am taking some time to read the source surrounding this setting and model loading to even figure out if this is a possible modification.  Russel, curious about your thoughts on if this approach would be something you would sign off on.

This seems like a good compromise over adding infrastructure for not installing certain models.  This would instead create infrastructure to declare a certain 'type' of model must be present, but that you may decide which one based on a settings value.

This approach feels cleaner to me because we don't want to 'turn off' a model, but rather only load a single 'User' model from some set of 'User' models.  In the case of 'User', we need at least one 'User' model to be loaded, and this enforces that, where as the ability to turn off certain models could introduce situations in which someone turns off a model without replacing it.

Aaron Merriam

[Russell Keith-Magee]

On Thu, Sep 19, 2013 at 3:39 AM, Luke Sneeringer <lu...@sneeringer.com> wrote:

I added the authtools approach to the wiki for completion, although I believe it to be an inferior approach.

One thing I dislike is having a separate app (e.g. d.c.auth_email) that has to be installed separately. That feels pretty impure to me. I'm doing a thought exercise about potential solutions, though, and not exactly coming up aces.

The best solution I can currently think of is to have User and EmailUser which are both models that live in django.contrib.auth. Then, we would have to add code to our model detection that says that *if* a model is a subclass of AbstractBaseUser, include it if and only if it is the AUTH_USER_MODEL.

I can't decide if that solution is better or worse than the disease. It makes for a much more attractive set of steps to follow for people who want to use it, though -- basically, just set AUTH_USER_MODEL to 'auth.EmailUser', and done.

My opinion - it's worse than the disease. 

Option 1 involves a clean auth app that just contains a stub user, and a clean extension app providing an alternative user. You install the extension app, and say you want to use it.

Option 2 makes a special case of *one particular* extension user, and makes all the internals of models, forms, views, etc embedded inside an if statement.

Option 1 strikes me as Django eating it's own dog food. Option 2 is making the internals of contrib.auth a mess to support one specific use case. Option 1 lets us provide implicit documentation of "this is how you build a pluggable User app". Option 2 means we spend our life making sure we're editing the right code at the right level of indentation.

If it isn't clear by now, I see very little to like in Option 2 :-)

Yours,

Russ Magee %-)

[Russell Keith-Magee]

On Thu, Sep 19, 2013 at 8:37 AM, Aaron Merriam <aaronm...@gmail.com> wrote:

Luke, I'm +1 on wanting a solution that allows "just set AUTH_USER_MODEL to 'auth.EmailUser', and done".

It'd be nice if 'swappable' could accomplish this for us.  Something along the lines of for any set of swappable models which are swappable for the same settings value, only load the one that is named by the settings value.  I am taking some time to read the source surrounding this setting and model loading to even figure out if this is a possible modification.  Russel, curious about your thoughts on if this approach would be something you would sign off on.

 

Can't say I'm wild about it. Explicit is better that implicit. 

Regarding your technical suggestion -- I'm not sure I see how that would work. auth.User nominates itself as swappable; and then you use AUTH_USER_MODEL to point myauth.EmailUser as the new User. There's nothing on EmailUser itself to say "I'm intended for use as a swappable User model", so there's no way to do the retrospective "don't install me if I'm not AUTH_USER_MODEL". EmailUser is, in every respect, a "normal" Django model. 

We could add another Meta setting for this, I suppose, but that doesn't fix the problems with picking the right forms, admin registration, views, and so on. And we'd be adding that setting for one specific reason - so that you can have 2 user models in contrib.auth, and only have one installed in the app cache. The setting wouldn't be any use to anyone else developing a third-party User model, because they have to do things "the right way" - install and app, and point the user model at model in the app.

If we were going to go down the "single setting" model, I'd rather see it as "you put auth_email in your INSTALLED_APPS, and that automatically sets AUTH_USER_MODEL". However, that requires infrastructure that we don't have at the moment. See my permanent refrain to App Refactor :-) And even then, I'm not necessarily convinced it's the right way to go. I don't see 2 explicit settings to be especially onerous, or difficult to explain. One setting installs the app. The other tells Django which model in that app to use. They really are separate concerns.

Oh - and Russell, not Russel :-)

 

This seems like a good compromise over adding infrastructure for not installing certain models.  This would instead create infrastructure to declare a certain 'type' of model must be present, but that you may decide which one based on a settings value.

This approach feels cleaner to me because we don't want to 'turn off' a model, but rather only load a single 'User' model from some set of 'User' models.  In the case of 'User', we need at least one 'User' model to be loaded, and this enforces that, where as the ability to turn off certain models could introduce situations in which someone turns off a model without replacing it.

Remember - It's not just about models. It's about forms, admin registrations, views, possibly even URLs.

Yours.

Russ Magee %-)

[Luke Sneeringer]

On Wednesday, September 18, 2013 6:39:13 PM UTC-6, Russell Keith-Magee wrote:

On Thu, Sep 19, 2013 at 3:39 AM, Luke Sneeringer <lu...@sneeringer.com> wrote:

I added the authtools approach to the wiki for completion, although I believe it to be an inferior approach.

One thing I dislike is having a separate app (e.g. d.c.auth_email) that has to be installed separately. That feels pretty impure to me. I'm doing a thought exercise about potential solutions, though, and not exactly coming up aces.

The best solution I can currently think of is to have User and EmailUser which are both models that live in django.contrib.auth. Then, we would have to add code to our model detection that says that *if* a model is a subclass of AbstractBaseUser, include it if and only if it is the AUTH_USER_MODEL.

I can't decide if that solution is better or worse than the disease. It makes for a much more attractive set of steps to follow for people who want to use it, though -- basically, just set AUTH_USER_MODEL to 'auth.EmailUser', and done.

My opinion - it's worse than the disease. 

Option 1 involves a clean auth app that just contains a stub user, and a clean extension app providing an alternative user. You install the extension app, and say you want to use it.

Option 2 makes a special case of *one particular* extension user, and makes all the internals of models, forms, views, etc embedded inside an if statement.

I think where I part from you is on the "one particular" aspect of the discussion. I actually see the "option 2" approach as having very clean side effects for some other use cases.

Allow me to explain:

There is, as you have pointed out, a lot more to auth than just the User model. There are forms and the like, which you mentioned, but there are also other aspects: the permissions models and functionality, for instance. Subclasses of AbstractBaseUser which also subclass PermissionsMixin still get the permissions things out of the box, and that is a documented use case in the official documentation.

So, right now (if I understand correctly), if you install the auth app, you get the Group and Permission models (and supporting code, natch), as well as the User model. If you decide to set a different User model using AUTH_USER_MODEL, but you keep auth installed for these other aspects (Group, Permission, etc.) then Django installs an auth_user table that isn't used (note to self: verify this).

If you set AUTH_USER_MODEL to something other than auth.User, you are making a statement that you do not want the stock model. This is true whether you change it to the upcoming, included "EmailUser" model, or to something else entirely.

So, I don't think that option 2 "makes a special case of one particular extension user". If anything, I assert it would do the opposite: it actually performs the most expected behavior in all cases. If the plain User model is the AUTH_USER_MODEL and d.c.auth is in INSTALLED APPS, then it is installed. If you choose to use an included-in-core e-mail model and install d.c.auth, you get that model and table instead of the current User model. And, if you use your own, special custom User model, but install auth for the remaining aspects, then you get your custom user model, and not either of the stock ones.

Basically, my understanding is that you start to get unclean and counter-intuitive behavior when you want to install the auth app for everything other than the User model (and forms, etc.), that this would correct.

Best Regards,

Luke

P. S. To be clear, I am not trying in any way to be argumentative, and I am happy to implement the separate app if that is what the core developers decide on.

[Luke Sneeringer]

On Wednesday, September 18, 2013 6:59:17 PM UTC-6, Russell Keith-Magee wrote:

On Thu, Sep 19, 2013 at 8:37 AM, Aaron Merriam <aaronm...@gmail.com> wrote:

Luke, I'm +1 on wanting a solution that allows "just set AUTH_USER_MODEL to 'auth.EmailUser', and done".

It'd be nice if 'swappable' could accomplish this for us.  Something along the lines of for any set of swappable models which are swappable for the same settings value, only load the one that is named by the settings value.  I am taking some time to read the source surrounding this setting and model loading to even figure out if this is a possible modification.  Russel, curious about your thoughts on if this approach would be something you would sign off on.

 

Can't say I'm wild about it. Explicit is better that implicit. 

I think I might be missing something salient. How do these solutions vary regarding explicit vs. implicit? I guess I don't see why that particular line is germane.

 

If we were going to go down the "single setting" model, I'd rather see it as "you put auth_email in your INSTALLED_APPS, and that automatically sets AUTH_USER_MODEL". However, that requires infrastructure that we don't have at the moment. See my permanent refrain to App Refactor :-) And even then, I'm not necessarily convinced it's the right way to go. I don't see 2 explicit settings to be especially onerous, or difficult to explain. One setting installs the app. The other tells Django which model in that app to use. They really are separate concerns.

Are they? I guess I see the User model to be used as a setting within auth...

Best Regards,

Luke

[Luke Sneeringer]

Bah, I should have planned my e-mailing better. Sorry for sending a third in such rapid succession.

I just did some checking into my assumptions (quoted below) on this. It turns out that Django core already does some of what I suggest: in particular, it doesn't install the User model if django.contrib.auth is installed but another AUTH_USER_MODEL is invoked, based on a Meta option called "swappable" (see [here][1] and [here][2]).

Am I misunderstanding? It seems like we are already pretty well down my proposed path.

Best Regards,

Luke

  [1]: https://github.com/django/django/blob/b2b763448f726ee952743596e9a34fcb154bdb12/django/contrib/auth/models.py#L406-L414

  [2]: https://github.com/django/django/blob/master/django/db/models/loading.py#L296

[Marc Tamlyn]

The problem you've got here is how it knows to *not* install EmailUser. If it's a model defined in d.c.auth.models, it will get installed, irrespective of whether it is AUTH_USER_MODEL or not. This is where we have to special case the new model somehow so it is only installed if we are using it. This is far from straightforwards! By putting it in a separate app which is not installed by default, then we solve this issue trivially without more magic.

[Jorge Cardoso Leitão]

Hi all.

I summarise the options with some of the issues raised, and I add my own concern.

One option presented here is to have both models in d.c.auth. As pointed out by Russell and others, this causes the problem on how to define the models such that they are not installed if they are not being the AUTH_USER_MODEL. Assuming that this problem could be solved, the user's procedure would be:

To use User (+ Permissions):

install django.contrib.auth

To use MailUser (+ Permissions):

install django.contrib.auth

set AUTH_USER_MODEL to MailUser

The second option presented here is to add a separate app (e.g. d.c.mailuser), which would require some imports from django.contrib.auth (forms, views). The user's procedure would be:

To use User (+ Permissions):

install django.contrib.auth

To use MailUser (+ Permissions):

install django.contrib.auth

install django.contrib.mailauth

set AUTH_USER_MODEL to MailUser

Is this correct or I'm missing something important here?

If correct, I think the second option is cleaner, even though there is the issue to minimise the repetition of code for forms, views, which was pointed out to be easier to solve than the problem of model installation.

What I want to emphasise here is that django.contrib.auth has to be installed in the case the custom model (email or other) is subclassed from PermissionsMixin.

This is something that I'm bringing because it seems odd that d.c.auth has to be installed even if we want to use mailuser (or any user). Wouldn't be possible to detach permissions to a new app, say django.contrib.perms, and remove the step of installing d.c.auth? I mean, we are in authentication; permissions are not related with authentication.

Even though I think this is because historically auth was the contrib for all these, which is good, I don't see why it should still be since now there are custom models: now there is a specific reason where I can see an advantage in separating these things; we could have a third option:

Django's state is:

django.contrib.auth for the standard user shipped by Django

django.contrib.mail_auth for the mail user shipped by Django

django.contrib.perms for permissions

To use AnyUser (shipped or not shipped):

install app_for_anyuser

set AUTH_USER_MODEL to AnyUser

To use AnyUser + Permissions:

install app_for_user

install django.contrib.perms

set AUTH_USER_MODEL to AnyUser

The settings for a new project would have AUTH_USER_MODEL = 'django.contrib.auth.models.User' and would have a new installed app "django.contrib.perms"

I think this is nice and clean because:

1. is explicit

2. is intuitive how to change the auth model just by looking at the settings

3. keeps the separation that Russell pointed out between installing an app and setting the auth model

4. (not sure, don't know enough): "swappable" can be deprecated: we just use the one set in AUTH_USER_MODEL (and install its app, naturally)

5. Django's shipped user is no longer "special"; it is just the default in the settings.

I have to admit this can be more serious than I'm seeing, but from the code I saw in auth, it does seem to make some sense. Naturally there are required migrations that have to be applied and so on, so, I'm not sure this should deserve a discussion on its own.

In conclusion, I think that the discussion we are having is very related with the fact that the standard User model is defined on the same app as Permissions. Because permissions are required for any User that uses Admin, d.c.auth has to be installed. Thus, we need to avoid installing User if we want another User (i.e. we have to swap it).

My claim is that If we detach permissions from auth, we no longer have to install User in the first place (we just install permissions), which means we can select the user (custom or shipped) by installing its app and selecting it in AUTH_USER_MODEL. I think this deprecates swappable, and makes the whole thing more explicit and flexible.

Dos this make any sense?

Meanwhile or not, I agree that having separate apps is a solution worth pursuing.

Regards,

Jorge

[Bruno Ribeiro da Silva]

Hi, I develop applications in Django and I started to read the developer mailing list and got interested by this topic about MailUser problem and I want to add my 50 cents, sorry if I'm taking a completely wrong approach, since I never developed the internals of Django.

But what if we just add a AUTH_USERNAME_FIELD to django settings and with this modify the behaviour of User model?

I did some fast coding to show you what I'm thinking:
https://github.com/loop0/django/compare/email_user

Isn't this a simpler approach? Does it have too much implications? I tested changing AUTH_USERNAME_FIELD on a sample project settings to email and it worked login to admin.

I like Jorge's idea to separate permissions into it's own app too.

Sorry if I wasn't supposed to post to this mailing group.

Regars,

Bruno

Bruno Ribeiro da Silva

Python/Django/Java Developer

Homebrewer

[Aaron Merriam]

I and my colleague (gavi...@gmail.com) have made some edits to the wiki to clarify the purpose of authtools, and more accurately explain what the 'authtools' approach would look like.  If you previously have examined 'option 2', I would ask that you go and reread that section in the wiki.

https://code.djangoproject.com/wiki/ContribEmailAuth

The problem you've got here is how it knows to *not* install EmailUser. If it's a model defined in d.c.auth.models, it will get installed, irrespective of whether it is AUTH_USER_MODEL or not.

I wanted to verify this so I ran some basic tests and turns out that swappable already does the correct thing.  If two models have the same swappable value, only the one referenced in settings will be used/installed.  This makes it practical to include a second user model in d.c.auth.models without any changes to model loading or the swappable mechanism.

Refactoring d.c.auth is a low impact way to facilitate custom user models including and beyond email as username.  We would much rather see d.c.auth become more generic and more abstract, than see a new contrib app built specifically for email as username.  Russell stated that this issue was more than just about the user model, that it included forms, views, urls, etc.  If there is a solution which allows for views, forms, urls, etc to be shared, I would prefer that approach over adding a new app.

[Luke Sneeringer]

But Django already has the mechanism to know not to install the model. It's the "swappable" Meta option. That is what causes User not to be installed if it is not used (contra my initial statement yesterday; I was flat wrong).

Right now, User has a meta option:

    swappable = 'AUTH_USER_MODEL'

...that does exactly what we are describing. It seems EmailUser (or whatever we call it) should just do this also.

[Luke Sneeringer]

On Sep 19, 2013, at 11:33 AM, Aaron Merriam <aaronm...@gmail.com> wrote:

I and my colleague (gavi...@gmail.com) have made some edits to the wiki to clarify the purpose of authtools, and more accurately explain what the 'authtools' approach would look like.  If you previously have examined 'option 2', I would ask that you go and reread that section in the wiki.

https://code.djangoproject.com/wiki/ContribEmailAuth

The problem you've got here is how it knows to *not* install EmailUser. If it's a model defined in d.c.auth.models, it will get installed, irrespective of whether it is AUTH_USER_MODEL or not.

I wanted to verify this so I ran some basic tests and turns out that swappable already does the correct thing.  If two models have the same swappable value, only the one referenced in settings will be used/installed.  This makes it practical to include a second user model in d.c.auth.models without any changes to model loading or the swappable mechanism.

Precisely. I think I originally stated my thinking quite poorly, which caused a great deal of confusion (sorry about that). I didn't know that swappable existed, and was proposing a mechanism that acted similarly to that. But, since swappable *does* exist, it seems like it's the best mechanism. I had originally made the same error that Marc made.

[Marc Tamlyn]

Whilst this works, I'm not sure it was the intention in the design of swappable. Clever though and it would allow the single-app approach.

However, we still have the issue of different forms, urls, views etc all based off the changed model. Making these change automatically would be horrible.

I still think a second app is a better plan.

If we are going to do this I think it is worth revisiting class basing the with views. A decent chunk of work was done on this at some point, and there are other implementations in the wild. It makes having the two separate versions much simpler in terms of swapping out form classes etc. One if the major issues was the lack of quality tests for some of the views.

Marc

--

[Aaron Merriam]

I'm not sure it was the intention in the design of swappable.

I don't know the intended use of swappable, but this seems like exactly the type of thing that a 'swappable' model should be able to do.

 

However, we still have the issue of different forms, urls, views etc all based off the changed model.

Marc, can you explain why you feel we need different forms, urls, views, etc.  The only forms that would not work appropriately with EmailUser would be UserCreation and UserChange forms, both of which can be modified to work correctly with EmailUser or other custom user models .  The views in d.c.auth all work correctly with the proposed EmailUser and most custom user models.   The urls in d.c.auth are not concerned with the username field so I don't see an issue there.  The ModelAdmin class in d.c.auth can also be modified to respect USERNAME_FIELD and work with EmailUser.

 Making these change automatically would be horrible.

Can you explain why this would be horrible.  The changes proposed in the 'authtools' style approach make d.c.auth more abstract in ways that make using a custom user model easier, while still keeping d.c.auth functioning the same when the user has not been swapped out.  To me, this is a step towards making custom user models easier and thus, a bonus.

[Marc Tamlyn]

I think you'll find that at least the Authentication form is wrong as well - at least in the sense that it calls the field username and it doesn't validate as an email. Once we get better at html5 fields as well, it won't be the correct input type.

Admin registration, the management commands, and the manager would also all need to be different.

I'd rather see auth get less magical and abstract, not more. I already find the existence of USERNAME_FIELD rather ugly. This whole approach is very implicit.

--

[Russell Keith-Magee]

On Thu, Sep 19, 2013 at 9:41 PM, Marc Tamlyn <marc....@gmail.com> wrote:

The problem you've got here is how it knows to *not* install EmailUser. If it's a model defined in d.c.auth.models, it will get installed, irrespective of whether it is AUTH_USER_MODEL or not. This is where we have to special case the new model somehow so it is only installed if we are using it. This is far from straightforwards! By putting it in a separate app which is not installed by default, then we solve this issue trivially without more magic.

Elaborating on this point: it's not just the models, either -- it's views, admin registrations, forms, and potentially URLs.

There's also a matter of consistency. We have to ship a default User model. We've set up a system that encourages people to develop their own user models. If you develop a third party User model, you need to do two things -- register the app, and declare AUTH_USER_MODELS. It seems completely logical to me that an EmailUser app that we're shipping as part of Django should be expected to do the same. 

If nothing else, it means we can point at contrib.email_auth and say "that's how you do it!".

 

On 19 Sep 2013 05:27, "Luke Sneeringer" <lu...@sneeringer.com> wrote:

Bah, I should have planned my e-mailing better. Sorry for sending a third in such rapid succession.

I just did some checking into my assumptions (quoted below) on this. It turns out that Django core already does some of what I suggest: in particular, it doesn't install the User model if django.contrib.auth is installed but another AUTH_USER_MODEL is invoked, based on a Meta option called "swappable" (see [here][1] and [here][2]).

Am I misunderstanding? It seems like we are already pretty well down my proposed path.

 

You've correctly identified the current behaviour, but you're extrapolating in a direction that isn't possible (at least, not with the current infrastructure).

You're correct in saying auth.User won't be installed if another model is set as AUTH_USER_MODEL. However, if auth.User (or any other model for that matter) is set as AUTH_USER_MODEL, nothing prevents EmailUser from being installed. This is because there's no artefact on the "plugged" model that says "I'm a candidate for being a pluggable User. *Any* model *could* be installed as a User -- you don't have to predeclare "I'm a candidate for being a User". Therefore, we have no metadata basis on which to say "Oh, I'm not being used as a pluggable model, don't install me." 

We *could* add this metadata, but again -- we'd be adding metadata and a bunch of complex internals as a workaround for asking users to put an app in INSTALLED_APPS, and set AUTH_USER_MODEL. For my money, the cost vastly exceeds the questionable benefit.

Yours,

Russ Magee %-)

[Aaron Merriam]

However, if auth.User (or any other model for that matter) is set as AUTH_USER_MODEL, nothing prevents EmailUser from being installed.

The current behavior of swappable is that if two models are swappable on the same settings value, only the one designated by the settings value will be installed.  So, currently, in django if I add the concrete model EmailUser to django.contrib.auth.models with swappable='AUTH_USER_MODEL', and do nothing else, this model is not installed.  If I reset my database and set my settings.AUTH_USER_MODEL to 'auth.EmailUser', then EmailUser will be installed and 'django.contrib.auth.User' will not be installed.  This is the current behavior of swappable based on my testing.

So there is no need for new internal mechanisms to control whether a model is installed or not, as that mechanism already exists within the current behavior of swappable.

[Aaron Merriam]

Russell, my reply there isn't meant to be confrontational and yet re-reading it I can see it being easily interpreted as such.  My intention is to ask whether there is something about swappable that I am missing.  It seems to work the way I stated above but your statement seems to imply that their is some missing piece that needs extra mechanisms surrounding model loading.  So, am I missing something?

Marc,

Thank you for your reply, and I respect the difference of opinion.  If I understand you correctly, you feel that this belongs in a separate contrib application because to refactor auth to be generic enough to handle both the built in User and EmailUser will involve too much magic.

I'm curious if you've taken a look at the code behind django-authtools.  I feel that it is a good example of what some of the changes to auth.forms or auth.views might look like, and that there is less 'magic' involved than you might think.

[Russell Keith-Magee]

Hi Aaron.

On Fri, Sep 20, 2013 at 8:00 AM, Aaron Merriam <aaronm...@gmail.com> wrote:

Russell, my reply there isn't meant to be confrontational and yet re-reading it I can see it being easily interpreted as such.  

No worries - I didn't read your response as confrontational, but I'll make sure I continue to not do so :-)

 

My intention is to ask whether there is something about swappable that I am missing.  It seems to work the way I stated above but your statement seems to imply that their is some missing piece that needs extra mechanisms surrounding model loading.  So, am I missing something?

I think you are.

Lets try via example. django.contrib.auth contains 2 User models -- User, and EmailUser. The (massively truncated) definitions for these models are:

class User(Model):

   username = models.CharField()

   USERNAME_FIELD = 'username'

   class Meta:

       swappable = 'AUTH_USER_MODEL'

class EmailUser(Model):

    email = models.CharField()

   USERNAME_FIELD = 'email'

Note that EmailUser *doesn't* have a Meta: swappable definition. There is nothing on this model that says "I am swappable", or "I am an appropriate substitute for User".

So - Lets look at two cases. 

Case 1: You set AUTH_USER_MODEL = 'auth.EmailModel'. You run syncdb. EmailUser is synchronised because it's a model in an app. User *isn't* synchronised because it's explicitly marked as being swappable, and the swappable attribute doesn't point at itself. Problem solved, right? We've got the right models syncronized!

Well, no - we still have a problem -- We can't just say "contrib.auth.forms.AuthenticationForm", because this form needs to change depending on the model that is in use. We have similar problems with admin, and potentially with views and URLs.

Case 2: The default case: I just want the default User model. I install django.contrib.auth in INSTALLED_APPS. I run syncdb. User is synchronised, as we expect.

Question: How does Django identify that EmailUser *shouldn't* be synchronised to the database? EmailUser isn't mentioned in any setting (because AUTH_USER_MODEL == 'auth.User'). There's nothing on EmailUser that flags it as being a "User" model. It's in a models file with other models (Group, Permission) which *will* be synchronized. Absent of any additional information, EmailUser will be synchronised, because it looks like a normal model.

Answer: We have 2 options. We need to modify contrib/auth/models.py to read:

class User(Model):

   …

if settings.AUTH_USER_MODEL == 'auth.EmailUser':

    class EmailUser(Model):

        ….

and make the same changes in forms, admin, views, URLs etc. 

Alternatively, we need to add something to the definition of EmailUser that says "I'm a User Model. Don't sync me unless I'm active":

class EmailUser(Model):

    email = models.CharField()

    class Meta:

        swappable_using = 'AUTH_USER_MODEL'

And then we *still* need to do the "if settings.AUTH_USER_MODEL == auth.EmailUser" dance with views, forms, admin, etc. 

However, this also means:

 1) We need to a bunch of internal plumbing in the app loader, Meta classes and so on to determine whether or not a model should be synchronised, and "hide" it in some way if it shouldn't be there. If you think this logic is simple, I've got a bridge to sell you :-)

 2) We have to introduce a backwards incompatible change to pluggable models. At present you don't have to pre-declare "my model is a User model". All existing swappable User models in the wild need to be updated to be "1.7 compliant".

 3) We introduce an implicit requirement for predeclaration. I need to know ahead of time that my model is going to be a User model. In the case of User, this probably isn't a huge impediment -- after all, I'm gonna know if I'm a User model. But looking longer term: the infrastructure for swappable Users isn't User specific. By design, swappable is a generic mechanism. You *could* use it as a limited substitute for generic foreign keys (e.g., a comments model pointing at a specific content type). But now you have to pre-declare that my model "can be swapped for X" -- which means that as a model designer, you need to know all the models that you *might* be swapped out as. Consider the comments example -- I want people to be able to comment on Users.  But I can't, because I haven't pre-declared User to be "swappable as" a content object for comments. Predeclaration imposes a restriction on one side of the swappable agreement that isn't always under your control.

The alternative -- we put EmailUser in a separate app. That app contains it's own forms, admin, views and URLs. No extra Meta attributes are required. No special "if" logic is required internal to models, views, forms, admin, etc. It's completely backwards compatible. It doesn't impose any pre-declaration restrictions. And it requires no modifications to the app loading internals. The only overhead - you have to declare an extra app in INSTALLED_APPS -- which is hardly surprising to me, given that it's being *distributed* as an app.

Does that clarify the problem for you?

Yours,

Russ Magee %-)

[Russell Keith-Magee]

Hi Bruno,

On Fri, Sep 20, 2013 at 1:26 AM, Bruno Ribeiro da Silva <con...@brunoribeiro.org> wrote:

Hi, I develop applications in Django and I started to read the developer mailing list and got interested by this topic about MailUser problem and I want to add my 50 cents, sorry if I'm taking a completely wrong approach, since I never developed the internals of Django.

But what if we just add a AUTH_USERNAME_FIELD to django settings and with this modify the behaviour of User model?

I did some fast coding to show you what I'm thinking:
https://github.com/loop0/django/compare/email_user

Isn't this a simpler approach? Does it have too much implications? I tested changing AUTH_USERNAME_FIELD on a sample project settings to email and it worked login to admin.

I like Jorge's idea to separate permissions into it's own app too.

Sorry if I wasn't supposed to post to this mailing group.

You're certainly allowed to post to the group -- we accept ideas from anyone.

However, in this case, I'm gonna say No. We've deliberately introduced a setting to allow people to swap in an arbitrary User model, because while the username field is a *common* thing to want to change, it isn't the *only* thing people want to change, and email vs username isn't the only choice. Introducing a second setting that covers part of the potential use case doesn't appeal to me at all.

Yours,

Russ Magee %-)

 

[Russell Keith-Magee]

Hi Jorge,

On Thu, Sep 19, 2013 at 11:45 PM, Jorge Cardoso Leitão <jo...@sapo.pt> wrote:

Hi all.

I summarise the options with some of the issues raised, and I add my own concern.

One option presented here is to have both models in d.c.auth. As pointed out by Russell and others, this causes the problem on how to define the models such that they are not installed if they are not being the AUTH_USER_MODEL. Assuming that this problem could be solved, the user's procedure would be:

To use User (+ Permissions):

install django.contrib.auth

To use MailUser (+ Permissions):

install django.contrib.auth

set AUTH_USER_MODEL to MailUser

The second option presented here is to add a separate app (e.g. d.c.mailuser), which would require some imports from django.contrib.auth (forms, views). The user's procedure would be:

To use User (+ Permissions):

install django.contrib.auth

To use MailUser (+ Permissions):

install django.contrib.auth

install django.contrib.mailauth

set AUTH_USER_MODEL to MailUser

Is this correct or I'm missing something important here?

You've correctly described the two options under discussion. There is an open question on exactly how to implement option 1, but from a public API perspective, you're correct.

 

If correct, I think the second option is cleaner, even though there is the issue to minimise the repetition of code for forms, views, which was pointed out to be easier to solve than the problem of model installation.

What I want to emphasise here is that django.contrib.auth has to be installed in the case the custom model (email or other) is subclassed from PermissionsMixin.

This is something that I'm bringing because it seems odd that d.c.auth has to be installed even if we want to use mailuser (or any user). Wouldn't be possible to detach permissions to a new app, say django.contrib.perms, and remove the step of installing d.c.auth? I mean, we are in authentication; permissions are not related with authentication.

This has been discussed in the past -- largely because there's a difference between authentication and authorisation. If we had our time over, we'd probably make this distinction better, and put permissions into a separate app. However, we're stuck with 8 years of legacy now, and it's hard to break this. 

Unless you can propose a backwards-compatible approach to this problem - i.e., making sure that every existing project that *doesn't* have contrib.permissions in INSTALLED_APPS continues to work -- splitting permissions into a separate app isn't really a viable option.

Also - I'm not sure I completely agree with the argument that if you're using contrib.emailauth, you shouldn't have to put contrib.auth in INSTALLED_APPS. This comes down to the idea of "what is an app" -- is it just models, or is it views, URLS, templates, and more? If it's just models I might agree with you, but an app is much more than that. If you're using contrib.emailauth, you're still using auth views, middlewares, and so on.

Yours,

Russ Magee %-)

[Russell Keith-Magee]

On Fri, Sep 20, 2013 at 2:21 AM, Luke Sneeringer <lu...@sneeringer.com> wrote:

But Django already has the mechanism to know not to install the model. It's the "swappable" Meta option. That is what causes User not to be installed if it is not used (contra my initial statement yesterday; I was flat wrong).

Right now, User has a meta option:

    swappable = 'AUTH_USER_MODEL'

...that does exactly what we are describing. It seems EmailUser (or whatever we call it) should just do this also.

No, it doesn't -- at least, not completely and in both directions. See my full answer to Aaron.

Yours,

Russ Magee %-)

[gavi...@gmail.com]

> Note that EmailUser *doesn't* have a Meta: swappable definition. There is nothing on this model that says "I am swappable", or "I am an appropriate substitute for User".

Ah, this is were the misunderstanding was. authtools does actually set swappable on its replacement user, too[1]. The two definitions look like this:

    class User(Model):

        username = models.CharField()

        USERNAME_FIELD = 'username'

        class Meta:

            swappable = 'AUTH_USER_MODEL'

    class EmailUser(Model):

        email = models.CharField()

        USERNAME_FIELD = 'email'

        class Meta:

           swappable = 'AUTH_USER_MODEL'

> How does Django identify that EmailUser *shouldn't* be synchronised to the database? 

The existing swappable mechanism takes care of it. Only the model referenced by settings.AUTH_USER_MODEL will be installed, not both. 

> we still have a problem -- We can't just say "contrib.auth.forms.AuthenticationForm", because this form needs to change depending on the model that is in use. We have similar problems with admin, and potentially with views and URLs.

It's straightforward to make the forms, views, and URLs work without checking what user model is installed, and this is the approach authtools takes. We can agree that code like `if settings.AUTH_USER_MODEL == auth.EmailUser"` is absolutely awful, so it's nice that it's not required to implement forms that work with custom user models. The forms from authtools will work with any user model, not just authtools.User and auth.User. It doesn't use any ugly switches on the type of the installed user model to do it either. (Note: the views and URLs don't actually have to change to accommodate EmailUser. authtools ships with the CBV auth views simply because #17209[2] has stalled.)

[1]: https://github.com/fusionbox/django-authtools/blob/master/authtools/models.py#L86

[2]: https://code.djangoproject.com/ticket/17209

[Russell Keith-Magee]

On Fri, Sep 20, 2013 at 10:53 AM, <gavi...@gmail.com> wrote:

> Note that EmailUser *doesn't* have a Meta: swappable definition. There is nothing on this model that says "I am swappable", or "I am an appropriate substitute for User".

Ah, this is were the misunderstanding was. authtools does actually set swappable on its replacement user, too[1]. The two definitions look like this:

    class User(Model):

        username = models.CharField()

        USERNAME_FIELD = 'username'

        class Meta:

            swappable = 'AUTH_USER_MODEL'

    class EmailUser(Model):

        email = models.CharField()

        USERNAME_FIELD = 'email'

        class Meta:

           swappable = 'AUTH_USER_MODEL'

> How does Django identify that EmailUser *shouldn't* be synchronised to the database? 

The existing swappable mechanism takes care of it. Only the model referenced by settings.AUTH_USER_MODEL will be installed, not both. 

Ah. Thats… interesting. I'm moderately surprised it works at all. I'll need to dig in to the internals to work out *why* it works… swappable certainly wasn't intended to work like that. There might be some interesting land mines lying in wait.

 

> we still have a problem -- We can't just say "contrib.auth.forms.AuthenticationForm", because this form needs to change depending on the model that is in use. We have similar problems with admin, and potentially with views and URLs.

It's straightforward to make the forms, views, and URLs work without checking what user model is installed, and this is the approach authtools takes. We can agree that code like `if settings.AUTH_USER_MODEL == auth.EmailUser"` is absolutely awful, so it's nice that it's not required to implement forms that work with custom user models. The forms from authtools will work with any user model, not just authtools.User and auth.User.

Well… no, they won't. They'll work with a *very* wide subset of User models, but not *every* user model. Most notably, you're relying on the ModelForms providing the right widgets for all fields, which will be a valid assumption for *most* models, but not *all* models. Given that Django aims for a 90% solution, I'm not sure that this is a major problem, however.

What *is* a major problem is the issues you'll have with tests. Your code binds User -- and thus the forms -- at import time. You're going to have fun using those forms in a test environment that is swapping User models.

Yours,

Russ Magee %-)

[Luke Sneeringer]

> On Sep 19, 2013, at 8:06 PM, Russell Keith-Magee <rus...@keith-magee.com> wrote:
>
> Note that EmailUser *doesn't* have a Meta: swappable definition. There is nothing on this model that says "I am swappable", or "I am an appropriate substitute for User".
>

But isn't that assuming your conclusion? The point we are making is that EmailUser *should* have the swappable Meta option.

> Question: How does Django identify that EmailUser *shouldn't* be synchronised to the database? EmailUser isn't mentioned in any setting (because AUTH_USER_MODEL == 'auth.User'). There's nothing on EmailUser that flags it as being a "User" model. It's in a models file with other models (Group, Permission) which *will* be synchronized. Absent of any additional information, EmailUser will be synchronised, because it looks like a normal model.
>
> Answer: We have 2 options. We need to modify contrib/auth/models.py to read:
>
> class User(Model):
> …
>
> if settings.AUTH_USER_MODEL == 'auth.EmailUser':
> class EmailUser(Model):
> ….

No, you just add the swappable Meta option, and then it "just works" as far as the model is concerned, right? (I could test this...)

> 1) We need to a bunch of internal plumbing in the app loader, Meta classes and so on to determine whether or not a model should be synchronised, and "hide" it in some way if it shouldn't be there. If you think this logic is simple, I've got a bridge to sell you :-)

This is where I think I must be missing something. Doesn't swappable already do this? If not, what *does* swappable do?

> 2) We have to introduce a backwards incompatible change to pluggable models. At present you don't have to pre-declare "my model is a User model". All existing swappable User models in the wild need to be updated to be "1.7 compliant".

swappable is opt-in, not opt-out. Since existing models people are writing are not swappable, it would just work. The opt-in is to the ability to be subsumed if the setting does not match, not the ability to be used if it does.

> Does that clarify the problem for you?

Not really, to be honest, because your statements about how the existing system works are divergent from my observations. I am therefore very confused...even though I have mostly lurked here, I respect you highly and am inclined to trust your judgment. On the other hand, I can't figure out why your interpretation of the exists system is so different from what I seem to observe.

Maybe you could help me with something: Could you explain to me what swappable *does* do? Here's what I think it does: I think it provides the mechanism to say, "do not use this model unless the setting says to use me". I infer from both you and Marc that's not what it does (Marc said I was proposing a "clever use"). That's fine. What does it do? Help me out. I feel certain I am just one step away from the lightbulb moment, but I really want to understand.

I have the repo checked out, tests running, and want to work on this. But I want to make sure I do the best work possible, so I want to make sure I understand the problem space as much I can. Aim first, then fire, and all that. :-)

L

[Luke Sneeringer]

Sent from my iPad

On Sep 19, 2013, at 9:24 PM, Russell Keith-Magee <rus...@keith-magee.com> wrote:

On Fri, Sep 20, 2013 at 10:53 AM, <gavi...@gmail.com> wrote:

> Note that EmailUser *doesn't* have a Meta: swappable definition. There is nothing on this model that says "I am swappable", or "I am an appropriate substitute for User".

Ah, this is were the misunderstanding was. authtools does actually set swappable on its replacement user, too[1]. The two definitions look like this:

    class User(Model):

        username = models.CharField()

        USERNAME_FIELD = 'username'

        class Meta:

            swappable = 'AUTH_USER_MODEL'

    class EmailUser(Model):

        email = models.CharField()

        USERNAME_FIELD = 'email'

        class Meta:

           swappable = 'AUTH_USER_MODEL'

> How does Django identify that EmailUser *shouldn't* be synchronised to the database? 

The existing swappable mechanism takes care of it. Only the model referenced by settings.AUTH_USER_MODEL will be installed, not both. 

Ah. Thats… interesting. I'm moderately surprised it works at all. I'll need to dig in to the internals to work out *why* it works… swappable certainly wasn't intended to work like that. There might be some interesting land mines lying in wait.

Aha, I think I found at least some of my disconnect. But I tested this yesterday, and it definitely does do that.

I know I just asked this in a previous email, but I will repeat for ease of others possibly following this thread: can you provide a brief overview of what swappable *does* do? It seems to be on observation it does exactly this. You say it's intended to do something else. What is that something else?

L

[Russell Keith-Magee]

On Fri, Sep 20, 2013 at 12:11 PM, Luke Sneeringer <lu...@sneeringer.com> wrote:

Sent from my iPad

On Sep 19, 2013, at 9:24 PM, Russell Keith-Magee <rus...@keith-magee.com> wrote:

On Fri, Sep 20, 2013 at 10:53 AM, <gavi...@gmail.com> wrote:

> Note that EmailUser *doesn't* have a Meta: swappable definition. There is nothing on this model that says "I am swappable", or "I am an appropriate substitute for User".

Ah, this is were the misunderstanding was. authtools does actually set swappable on its replacement user, too[1]. The two definitions look like this:

    class User(Model):

        username = models.CharField()

        USERNAME_FIELD = 'username'

        class Meta:

            swappable = 'AUTH_USER_MODEL'

    class EmailUser(Model):

        email = models.CharField()

        USERNAME_FIELD = 'email'

        class Meta:

           swappable = 'AUTH_USER_MODEL'

> How does Django identify that EmailUser *shouldn't* be synchronised to the database? 

The existing swappable mechanism takes care of it. Only the model referenced by settings.AUTH_USER_MODEL will be installed, not both. 

Ah. Thats… interesting. I'm moderately surprised it works at all. I'll need to dig in to the internals to work out *why* it works… swappable certainly wasn't intended to work like that. There might be some interesting land mines lying in wait.

Aha, I think I found at least some of my disconnect. But I tested this yesterday, and it definitely does do that.

Agreed - this explains the source of the disconnect.

 

I know I just asked this in a previous email, but I will repeat for ease of others possibly following this thread: can you provide a brief overview of what swappable *does* do? It seems to be on observation it does exactly this. You say it's intended to do something else. What is that something else?

 

The intention was to mark a particular model as a something that can be replaced. It's about marking the endpoint that can be swapped, not predeclaring all possible participants in a swap.

In the case of contrib.auth: Auth requires the concept of a model that represents a User. At the end of the day, it shouldn't matter which model that user actually is -- it just needs to be a model, and we can determine at runtime what that model should be. You don't have to predeclare that you're going to be used as a User - you just point to the model using AUTH_USER_MODEL.

For ease of use (and backwards compatibility) we need to ship a concrete User model. But then we need to be able to:

 * Not install the model if it's been replaced, and

 * Easily answer the runtime question "what have you been swapped out for?"

This is done by checking Meta.swappable, reading the value of the setting, and checking if Meta.model_label matches the value in the setting. This is cached as Meta.swapped. The internals of Django (like syncdb) then check Meta.swapped as part of the checks performed to see if a model should be included in any given activity.

I've had a bit of a closer look at the implementation with your use case in mind, and I'm less concerned about land mines caused your proposed usage of swappable. It works by happy accident :-)

However, I'd still make the argument that while it's *possible* to use swappable in this way, it's still not desirable.

Firstly, it leads to inconsistent usage of API. Under the current scheme, the default User model defines swappable. No other User model needs to. In your proposed usage, you've just made a special case of "Swappable models in the same app". Now, granted - this discrepancy isn't something that needs to be documented (because swappable isn't documented), but if someone goes looking, or if we ever want to formalise swappable (which is certainly my long term hope), we need to explain why some models need swappable defined and others don't. 

Alternatively, if we change the game to say that swappable *is* required on all models, then we have a backwards compatibility problem (because no existing custom user model needs swappable to be defined). *Any* model can be swapped in as a substitute user. 

If we were to go down this path, the logical extension (to my mind) would be to validate that you're not swapping in a model that hasn't declared itself as swappable (otherwise, why would you go to the trouble of predeclaring?). This *isn't* currently being done, and I'm not convinced it has value. This is an instance where duck typing works really well. There's also the example I raised about comments on User - you aren't necessarily in control of the model that you want to use as a swappable alternative.

Secondly, marking a model as swappable doesn't make it zero cost. It's still parsed, loaded and added to the app cache, even if it isn't used. Django's internals then ignore any model that is marked as swapped. If we put EmailUser in contrib.auth, it's *always* going to be part of every project that uses auth, regardless of whether you're using the default User, EmailUser, or a third party User app. That's overhead that isn't present if EmailUser is in a separate opt-in app. I'm willing to accept the overhead for the single endpoint User model, because it's the origin - the default thing that needs to be swapped out. I'm less inclined to say that this is acceptable for all possible alternative User models that might be included in auth.

Lastly, we have consistency with external usage, and the benefits of documentation by example. Current usage says that use a custom User model, you add the app the INSTALLED_APPS and set AUTH_USER_MODEL. By shipping email_auth as a separate app, we'd be continuing the same advice, and the app would serve as a helpful example for how to do it. We'd be eating our own dog food/leading by example.

So far, the only convincing argument I've heard for putting the EmailUser in contrib.auth is that it avoids having to add a value to INSTALLED_APPS. And I'm afraid that, by itself, just doesn't convince me. Everything I see suggests that a standalone app is cleaner and more explicit, and serves as a better example to the wider community. I just don't consider an extra entry in INSTALLED_APPS -- explicitly saying "I want to have access to this extra model", the same way you do for *every* other Django app -- is onerous.

Yours,

Russ Magee %-)

[tanderegg]

Hi Luke - 

I just wanted to let you know that I've already started a patch on this, you can see it here: https://github.com/tanderegg/django/compare/ticket_20824_master  If you want to reduce duplicated effort, please take a look at what I have so far and let me know what you think!

Russ, Aaron -

I tend to agree with Russ here that it makes sense to keep auth_email as a separate app, especially if swappable is not meant to be used on any model other than auth.User.  If EmailUser was also put into auth, not only would it create an unesseccary set of tables, it also would cause m2m accessor clashes with groups and permissions.  Additionally, it provides a "template" of sorts for others to refer to if they want to roll their own user models.

That said, even if we use a separate app, we might benefit from refactoring AbstractUser a bit.  I could see the following chain:

AbstractBaseUser -> 

AbstractPermissionsUser (ass ins Permissions Mixin) -> 

AbstractEmailUser (just adds email field and email_user method) -> 

AbstractNamedUser (adds first_name and last_name) -> 

AbstractUser (retains same API as before)

The advantage of doing this would be to remove some code duplication from auth_email.EmailUser, and to create a few more inheritance opportunities for scenarios where someone wants a user with no name fields, or if they want to implement their own naming conventions, or if they want permissions but no email and name.

Here's an example of what that might look like: https://github.com/tanderegg/django/compare/ticket_20824_refactor_master  I also modified the UserManager class to support any of these abstract models.

The main issue with doing this is that email would now be required for all users (which could be avoided by removing AbstractEmailUser from the inheritance chain and adding an email field to AbstractUser), and to maintain backwards compatibility, UserManager.create_user would require passing the email twice, once as username and once as email, for any model that uses email as the username (which could be avoided by overriding in a sub class when using email as username).

Not sure if this refactoring would help all that much in the grand scheme of things though.

-Tim

On Thursday, September 12, 2013 4:44:53 PM UTC-4, Abdulaziz Alfoudari wrote:

This is a continuation of my post on stackoverflow.

With the introduction of Django 1.5, it was possible to create a custom User model which is flexible enough to have any user profile the developer wants created. However, looking at a very common problem which is using the email as the primary user identifier instead of username, the solution requires copying most of Django's internal definition of AbstractUser and that is only to remove the username field.

A better solution in my opinion is make AbstractUser even more abstract by removing username field, and allowing the developer to explicitly specify the field to be used as the user identifier. This will require a tiny extra work for those that use the current default behavior, but it will also greatly reduce the work needed for the very common problem of using email as the user identifier.

Please share your thoughts and opinions on this.

[gavi...@gmail.com]

> The intention was to mark a particular model as a something that can be replaced.

It's hard to find the original rationale behind swappable, but my mental model was always:

  

  A model with `swappable = 'x'` means that the model should only be loading if settings.x is set to that same model

> it's still not desirable.

Why not? Swappable seems like exactly what we need here.

> No other User model needs to [set swappable]

This would still be the case. Only models that want to conditionally load themselves would set swappable. User models in application code probably wouldn't set it, because the project will always use that one user model.

> ... made a special case of "Swappable models in the same app".

I'm not sure where there's a special case. Swappable works for this without any modifications, see authtools.

> *Any* model can be swapped in as a substitute user. 

Yep. Nothing needs to change to keep this possible.

> If we were to go down this path, the logical extension (to my mind) would be to validate that you're not swapping in a model that hasn't declared itself as swappable

Why would you want to validate this? Swappable only controls loading of the model its set on, there is no action-at-a-distance on other models.

> Secondly, marking a model as swappable doesn't make it zero cost. 

That's fair. I'll add it to the wiki as a disadvantage.

> So far, the only convincing argument I've heard for putting the EmailUser in contrib.auth is that it avoids having to add a value to INSTALLED_APPS.

I don't think we should focus so much on the optional part of the proposal. The really important part is to refactor the existing code to work better with custom users. Once this happens, projects like authtools can trivially add an EmailUser in a few lines of code, so it's not so important that there's one in core.

[Luke Sneeringer]

On Sep 19, 2013, at 11:15 PM, Russell Keith-Magee <rus...@keith-magee.com> wrote:

> Firstly, it leads to inconsistent usage of API. Under the current scheme, the default User model defines swappable. No other User model needs to. In your proposed usage, you've just made a special case of "Swappable models in the same app". Now, granted - this discrepancy isn't something that needs to be documented (because swappable isn't documented), but if someone goes looking, or if we ever want to formalise swappable (which is certainly my long term hope), we need to explain why some models need swappable defined and others don't.

If we went with this method, the explanation would be that a model must define itself as swappable if the intent is for it to be able to be overridden.

So, the idea would be that any user model that we chose to ship in Django should be marked swappable, but the User model that someone wrote for their own app would not be.

Even if we decide to go with an auth_email app, we should almost certainly still mark that User model as swappable. (What if someone wanted the forms and not the model?)

> Alternatively, if we change the game to say that swappable *is* required on all models, then we have a backwards compatibility problem (because no existing custom user model needs swappable to be defined). *Any* model can be swapped in as a substitute user.

I completely agree. Any setup that requires swappable on all models does not make sense. But simply marking swappable on models that *we* ship that are intended to be able to be subbed out does make sense, and doesn't cause back-compat issues.

Right now Django ships one model: User, marked swappable. By declaring your own model and setting it as AUTH_USER_MODEL, the swappable setting makes it so that ours is not used.

I am proposing that Django would ship two models: User and EmailUser, both marked swappable. If you declare your own, neither of ours are used. If you use one of ours, the other is not used. If you go the declare your own route, there is no need to mark your model as swappable. It is, as you state, a regular model.

> If we were to go down this path, the logical extension (to my mind) would be to validate that you're not swapping in a model that hasn't declared itself as swappable (otherwise, why would you go to the trouble of predeclaring?). This *isn't* currently being done, and I'm not convinced it has value. This is an instance where duck typing works really well. There's also the example I raised about comments on User - you aren't necessarily in control of the model that you want to use as a swappable alternative.

I agree here too. I don't think this idea has value. However, I don't think it's a necessary condition, either.

> Lastly, we have consistency with external usage, and the benefits of documentation by example. Current usage says that use a custom User model, you add the app the INSTALLED_APPS and set AUTH_USER_MODEL. By shipping email_auth as a separate app, we'd be continuing the same advice, and the app would serve as a helpful example for how to do it. We'd be eating our own dog food/leading by example.
>
> So far, the only convincing argument I've heard for putting the EmailUser in contrib.auth is that it avoids having to add a value to INSTALLED_APPS. And I'm afraid that, by itself, just doesn't convince me. Everything I see suggests that a standalone app is cleaner and more explicit, and serves as a better example to the wider community. I just don't consider an extra entry in INSTALLED_APPS -- explicitly saying "I want to have access to this extra model", the same way you do for *every* other Django app -- is onerous.

I have two reasons for wanting this method: one is the public API, but the other (maybe bigger) is code duplication. Django is about DRY, after all. If we do an auth_email model, we have to largely duplicate the model, and the form, and any relevant views.

If we ship an email user model, what seems the path of least resistance to me would be to have the existing User subclass EmailUser, add the username field and redefine USERNAME_FIELD and REQUIRED_FIELDS. Things like (for example) UserCreationForm would be defined to have the two password fields and then have the username field derived from the field that is declared as USERNAME_FIELD. This has the expedient side effect of making it useful for more substitution models in the wild, although obviously not all of them. You've been referring to a reference implementation as a valuable aspect of the separate-app route, but one of the virtues of my proposal is that it increases the number of cases where much less has to be implemented at all.

tanderegg's implementation really hits this home for me: he has models, forms, etc. that all look almost-but-not-quite-exactly like the ones already being shipped. That also means double maintenance -- if we want to maintain some aspect that's duplicated between them, we have to maintain it in both spots, to keep them in sync.

I've gone a little ways down toward implementing both paths, and one thing that is clear to me is that the code required to ship an alternate model within the same app is substantially less, and has much less (if any) duplication involved.

L

P. S. I'm not the sharpest knife in the drawer when it comes to social things, so I want to state explicitly: I am continuing to debate the question because I perceive the debate to be moving in a useful way. If I'm, in fact, simply being confrontational, then tell me to shut up and I'll just work on doing it your way / commenting on tanderegg's start. At the end of the day, I recognize that it is your decision to make, and at the end of the day, if I can't convince you, I'll accept it with a smile and get the work done! I definitely want to be offering a positive contribution.

[Luke Sneeringer]

On Sep 20, 2013, at 10:58 AM, gavi...@gmail.com wrote:

> > No other User model needs to [set swappable]
>
> This would still be the case. Only models that want to conditionally load themselves would set swappable. User models in application code probably wouldn't set it, because the project will always use that one user model.

This is my understanding too. If you set a user model in individual project code, there's no point in defining it as swappable, as that project is always going to want to use it.

If you're writing the authtools app, you probably do want to define the user model or models you offer as swappable, but that seems like it's still true now.

> > ... made a special case of "Swappable models in the same app".
>
> I'm not sure where there's a special case. Swappable works for this without any modifications, see authtools.
>
> > *Any* model can be swapped in as a substitute user.
>
> Yep. Nothing needs to change to keep this possible.
>
> > If we were to go down this path, the logical extension (to my mind) would be to validate that you're not swapping in a model that hasn't declared itself as swappable
>
> Why would you want to validate this? Swappable only controls loading of the model its set on, there is no action-at-a-distance on other models.

I, too, don't really understand this assumption. The status quo works just fine, and makes complete sense to me.

> I don't think we should focus so much on the optional part of the proposal. The really important part is to refactor the existing code to work better with custom users. Once this happens, projects like authtools can trivially add an EmailUser in a few lines of code, so it's not so important that there's one in core.

Eh, I pretty passionately believe that this particular use case needs to be in core. It's **really** common. But, the point that you make is why I am passionate about not just making an auth_email app, because I see the other approach as *also* having more value in simplifying some other more-simple user substitutes.

L

[Timothy Anderegg]

Hi Luke -

I just wanted to clarify the approach I'm using - The issue of whether or not the EmailUser is in contrib.auth or in a new app contrib.auth_email is a separate issue from code duplication.  Either way, you'd have to create new versions of UserCreationForm, UserChangeForm, UserAdmin, and UserManager, because each of those classes will only work with the default auth.User model (as currently written).

So there's really two independent questions at play here:

1) Should a new EmailUser model be in contrib.auth or contrib.auth_email?  

Based on the current recommended way to use swappable, EmailUser should be in its own app contrib.auth_email.  This could be changed by "blessing" the usage of swappable that Aaron was talking about (which does seem to work), in which case EmailUser could be in the contrib.auth app without conflicts.  Testing would have to be done to make sure this works in all cases.

2) Should contrib.auth undergo refactoring to reduce the amount of code duplication required to create a custom user model? 

This would require modifying the two forms, admin class, and user manager to support custom models without further subclassing (except to add new functionality) or replacement.  I did this for UserManager in my alternative patch (https://github.com/tanderegg/django/compare/ticket_20824_refactor_master), which was fairly straightforward although a little awkard since the current public api requires a separate parameter passed to create_user for email, even if 'email' is also being used as the username.

The forms and admin class would be a bit harder, it could be done but the result would probably be a bit convoluted, and they still would not cover all use cases of course.  Better would be to create abstract base classes for those forms and UserAdmin that cover essential functionality, and then allow custom models to inhereit from those, which would reduce code duplication fairly substantially.  If we used abstract base classes though, the custom user model would still require defining custom forms and admin classes as well, they would just be much simpler.

-Tim

L

--
You received this message because you are subscribed to a topic in the Google Groups "Django developers" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/django-developers/rb7v9kVAK3I/unsubscribe.
To unsubscribe from this group and all its topics, send an email to django-develop...@googlegroups.com.

[Russell Keith-Magee]

Hi Gavin,

On Sat, Sep 21, 2013 at 12:58 AM, <gavi...@gmail.com> wrote:

> The intention was to mark a particular model as a something that can be replaced.

It's hard to find the original rationale behind swappable, but my mental model was always:

  

  A model with `swappable = 'x'` means that the model should only be loading if settings.x is set to that same model

Well, it's hard to find the rationale because it's an internal API that (like the rest of Meta) isn't documented. However, I can speak with authority on the original rationale, because I was the person who wrote the code, and I had a specific intention in mind :-)

> it's still not desirable.

Why not? Swappable seems like exactly what we need here.

> No other User model needs to [set swappable]

This would still be the case. Only models that want to conditionally load themselves would set swappable. User models in application code probably wouldn't set it, because the project will always use that one user model.

> ... made a special case of "Swappable models in the same app".

I'm not sure where there's a special case. Swappable works for this without any modifications, see authtools.

This all hinges on the interpretation. My intention was *not* that swappable meant "This model might not be loaded". My intention *was* that swappable meant "This particular model is an extension point that might be replaced by a third party." 

As I've indicated elsewhere, the fact that the implementation happens to support the first interpretation is accident, not intention.

 

> *Any* model can be swapped in as a substitute user. 

Yep. Nothing needs to change to keep this possible.

> If we were to go down this path, the logical extension (to my mind) would be to validate that you're not swapping in a model that hasn't declared itself as swappable

Why would you want to validate this? Swappable only controls loading of the model its set on, there is no action-at-a-distance on other models.

> Secondly, marking a model as swappable doesn't make it zero cost. 

That's fair. I'll add it to the wiki as a disadvantage.

And this remains my biggest reason why the first interpretation isn't the right interpretation. We shouldn't be encouraging people to build monolithic apps containing dozens of "swappable" models. We should be encouraging people to write small, tight apps that fill a single purpose.

 

> So far, the only convincing argument I've heard for putting the EmailUser in contrib.auth is that it avoids having to add a value to INSTALLED_APPS.

I don't think we should focus so much on the optional part of the proposal. The really important part is to refactor the existing code to work better with custom users. Once this happens, projects like authtools can trivially add an EmailUser in a few lines of code, so it's not so important that there's one in core.

 

I'm not sure exactly what you mean by the "optional" part of the proposal. Where the code will sit seems like a pretty important question to me. There are other questions to be resolved as well, but it seems to me that those discussions are currently being swamped. I may need to a reboot of this conversation.

Yours,

Russ Magee %-)

[Russell Keith-Magee]

On Sat, Sep 21, 2013 at 2:43 AM, Luke Sneeringer <lu...@sneeringer.com> wrote:

P. S. I'm not the sharpest knife in the drawer when it comes to social things, so I want to state explicitly: I am continuing to debate the question because I perceive the debate to be moving in a useful way. If I'm, in fact, simply being confrontational, then tell me to shut up and I'll just work on doing it your way / commenting on tanderegg's start. At the end of the day, I recognize that it is your decision to make, and at the end of the day, if I can't convince you, I'll accept it with a smile and get the work done! I definitely want to be offering a positive contribution.

Understood -- and FTR, I wasn't getting a confrontational vibe; the fact that you're aware that this concern even exists means you're almost certainly not over the line :-) 

And, flipping it around -- I'm not trying to be confrontational either - also just looking out for the best interests of Django (as I see them).

Yours,

Russ Magee %-)

[Russell Keith-Magee]

On Sat, Sep 21, 2013 at 3:49 AM, Timothy Anderegg <timothy....@gmail.com> wrote:

Hi Luke -

I just wanted to clarify the approach I'm using - The issue of whether or not the EmailUser is in contrib.auth or in a new app contrib.auth_email is a separate issue from code duplication.  

This is a key point, and I think the whole conversation warrants a bit of a reboot. 

We have two key questions here:

 1) Where does the new model live? Is it going to live in contrib.auth, or in a new contrib.email_auth package?

 2) Exactly what does the new class hierarchy look like? Are we going to do any refactoring of AbstractUser? How many composable bits are we going to end up with?

We've had a lot of discussion on question 1, which is risking becoming a bit of a bikeshed on the interpretation of `swappable`. 

What I haven't seen is a whole lot of clear discussion on point 2. Most of the discussion seems to stop at "use package/patch X", rather than a providing a good  summary of the consequences. 

Correct me anyone disagrees, but my impression is that the endpoint isn't under dispute -- we're aiming at adding a new User model that is an exact analog of the existing User model, but with email as the USERNAME_FIELD. However, there are a couple of ways we can make this happen (ranging from simple copy/paste to a complex mixin/inheritance hierarchy). Speaking as the person from core who wants/needs to commit this, I need to see options for *this* part of the discussion.

I've just put a summary of these two points on the wiki; I'd invite anyone with an opinion to weigh in. As always, try to remain impersonal and objective. I'd also appreciate people who have contributed to the 'Implementation' section to tie back to the new design section -- if you've proposed a codebase, describe which design decisions you've made.

Yours,

Russ Magee %-)

[Luke Sneeringer]

I'm taking several different e-mail replies and attempting to conglomerate them. Please forgive me if this is not considered socially acceptable here.

On Sep 20, 2013, at 8:42 PM, Russell Keith-Magee <rus...@keith-magee.com> wrote:

We have two key questions here:

 1) Where does the new model live? Is it going to live in contrib.auth, or in a new contrib.email_auth package?

 2) Exactly what does the new class hierarchy look like? Are we going to do any refactoring of AbstractUser? How many composable bits are we going to end up with?

We've had a lot of discussion on question 1, which is risking becoming a bit of a bikeshed on the interpretation of `swappable`. 

What I haven't seen is a whole lot of clear discussion on point 2. Most of the discussion seems to stop at "use package/patch X", rather than a providing a good  summary of the consequences. 

I think you're making a good case that some of the question 1 discussion has been cart before horse. Perhaps it would be better understood if we did some more discussion on the other half.

Here's what I am thinking we should do (wiki segment forthcoming):

• I believe that we should add an AbstractEmailUser as a subclass of AbstractBaseUser, and make AbstractUser subclass it.
• So, right now we have AbstractBaseUser -> AbstractUser -> User.
• My desired hierarchy would be:
• AbstractBaseUser
• AbstractEmailUser
• AbstractUser
• User
• EmailUser
• I assume this is mostly intuitive, but under my proposed hierarchy, everything that AbstractUser has now would be moved to AbstractEmailUser except for the username field. AbstractEmailUser would have USERNAME_FIELD set to 'email', and REQUIRED_FIELDS set to empty tuple. AbstractUser would have USERNAME_FIELD set to 'username' and REQUIRED_FIELDS set to ('email',).
• Both User and EmailUser would have the swappable Meta option set to 'AUTH_USER_MODEL'.
• I would then propose taking forms that hard-declare username as a field (e.g. UserCreationForm, but there are others) and instead determine their username field using the `fields` meta option in ModelForm.
• An example in the case of the UserCreationForm would be: fields = (model.USERNAME_FIELD, 'password1', 'password2')
• There's a potential land mine in here which is that the `clean_username` method is much less straightforward: we'd either have to determine the method name programmatically in class construction or get around the issue through double-defining it. I would favor the former because then the form is useful without modification to more third-party use cases. However, this is one of the (few, IMO) cases where there actually is a complexity hit, and I believe this problem should be noted as a disadvantage of my approach.
• I haven't looked at views as thoroughly, but I believe them to be more straightforward. I will do this dive soon.

This all hinges on the interpretation. My intention was *not* that swappable meant "This model might not be loaded". My intention *was* that swappable meant "This particular model is an extension point that might be replaced by a third party." 

But my proposed usage of swappable is within this intent.

My argument is that User is marked swappable, meaning that this particular model is an extension point that might be replaced by a third party. My argument is also that EmailUser should be marked swappable, meaning that this model is an extension point that might be replaced by a third party. :-)

The spot where I'm potentially outside original intent is that replacing User with EmailUser isn't *technically* a third party replacement, but any option so far proposed technically goes outside this. Therefore, I honestly believe that my proposal is within your original intent.

And this remains my biggest reason why the first interpretation isn't the right interpretation. We shouldn't be encouraging people to build monolithic apps containing dozens of "swappable" models. We should be encouraging people to write small, tight apps that fill a single purpose.

But does that mean it's the right choice in this case? As best as I can tell, we're talking about a non-trivial amount of code duplication to create a separate app for this (duplicate model, duplicate forms, duplicate views...). And shouldn't we also be encouraging people to follow the DRY principle? A separate app very much doesn't.

And, flipping it around -- I'm not trying to be confrontational either - also just looking out for the best interests of Django (as I see them).

Of course! My motivations are similar...I want to contribute the best patch I can. :-) There is no shortage of good faith here, methinks. And it's a really interesting question. :-)

On another note, I've been on vacation recently, but I am setting aside time to work on this this coming week, whether that involves commenting on / improving Tim's implementation, or doing some alternate implementation if a different approach is accepted.

L

[Timothy Anderegg]

• I believe that we should add an AbstractEmailUser as a subclass of AbstractBaseUser, and make AbstractUser subclass it.
• So, right now we have AbstractBaseUser -> AbstractUser -> User.
• My desired hierarchy would be:
• AbstractBaseUser
• AbstractEmailUser
• AbstractUser
• User
• EmailUser
• I assume this is mostly intuitive, but under my proposed hierarchy, everything that AbstractUser has now would be moved to AbstractEmailUser except for the username field. AbstractEmailUser would have USERNAME_FIELD set to 'email', and REQUIRED_FIELDS set to empty tuple. AbstractUser would have USERNAME_FIELD set to 'username' and REQUIRED_FIELDS set to ('email',).
• Both User and EmailUser would have the swappable Meta option set to 'AUTH_USER_MODEL'.

One thing to note here, is that AbstractEmailUser would have to define the email field with unique=True in order for it to be used as USERNAME_FIELD.  This would mean that AbstractUser and User would fundamentally change so that email has to be unique.  This could definitely wreck havoc with existing apps that don't have this requirement.

One solution would be to have an intermediate class (AbstractNamedUser or something) that both AbstractEmailUser and AbstractUser inherit from, and each would define their own email fields.  The other would be to stub out the email field (email = '') on AbstractEmailUser and actually implement it in each descendent class.  I like the former approach myself, since it abstracts out name handling and would make it easier to create custom users that handle names differently.  It might make sense to put AbstractEmailUser above AbstractNamedUser in the inheritance chain, and use the stubbing technique too.

• I would then propose taking forms that hard-declare username as a field (e.g. UserCreationForm, but there are others) and instead determine their username field using the `fields` meta option in ModelForm.
• An example in the case of the UserCreationForm would be: fields = (model.USERNAME_FIELD, 'password1', 'password2')
• There's a potential land mine in here which is that the `clean_username` method is much less straightforward: we'd either have to determine the method name programmatically in class construction or get around the issue through double-defining it. I would favor the former because then the form is useful without modification to more third-party use cases. However, this is one of the (few, IMO) cases where there actually is a complexity hit, and I believe this problem should be noted as a disadvantage of my approach.

The clean_username method is technically unnesseccary, it just provides a better error message than the ORM would.  There might be a way to generalize this differently.  The other issue is that username is hard coded into these forms as a regex validation.  That could perhaps be moved to the AbstractUser class itself. 

• I haven't looked at views as thoroughly, but I believe them to be more straightforward. I will do this dive soon.

In my experimentation, I haven't had to touch the views at all to make things work.  I may have missed something though!

The only other part that would have to change is UserAdmin, although it may end up being simpler just to require a custom Admin class for every custom user model, since it seems to me abstracting that would be tricky.

-Tim

[Luke Sneeringer]

On Sat, Sep 21, 2013 at 5:49 PM, Timothy Anderegg <timothy....@gmail.com> wrote:

• I believe that we should add an AbstractEmailUser as a subclass of AbstractBaseUser, and make AbstractUser subclass it.
• So, right now we have AbstractBaseUser -> AbstractUser -> User.
• My desired hierarchy would be:
• AbstractBaseUser
• AbstractEmailUser
• AbstractUser
• User
• EmailUser
• I assume this is mostly intuitive, but under my proposed hierarchy, everything that AbstractUser has now would be moved to AbstractEmailUser except for the username field. AbstractEmailUser would have USERNAME_FIELD set to 'email', and REQUIRED_FIELDS set to empty tuple. AbstractUser would have USERNAME_FIELD set to 'username' and REQUIRED_FIELDS set to ('email',).
• Both User and EmailUser would have the swappable Meta option set to 'AUTH_USER_MODEL'.

One thing to note here, is that AbstractEmailUser would have to define the email field with unique=True in order for it to be used as USERNAME_FIELD.  This would mean that AbstractUser and User would fundamentally change so that email has to be unique.  This could definitely wreck havoc with existing apps that don't have this requirement.

One solution would be to have an intermediate class (AbstractNamedUser or something) that both AbstractEmailUser and AbstractUser inherit from, and each would define their own email fields.  The other would be to stub out the email field (email = '') on AbstractEmailUser and actually implement it in each descendent class.  I like the former approach myself, since it abstracts out name handling and would make it easier to create custom users that handle names differently.  It might make sense to put AbstractEmailUser above AbstractNamedUser in the inheritance chain, and use the stubbing technique too.

You're absolutely right about this. Good catch.

 

• I would then propose taking forms that hard-declare username as a field (e.g. UserCreationForm, but there are others) and instead determine their username field using the `fields` meta option in ModelForm.
• An example in the case of the UserCreationForm would be: fields = (model.USERNAME_FIELD, 'password1', 'password2')
• There's a potential land mine in here which is that the `clean_username` method is much less straightforward: we'd either have to determine the method name programmatically in class construction or get around the issue through double-defining it. I would favor the former because then the form is useful without modification to more third-party use cases. However, this is one of the (few, IMO) cases where there actually is a complexity hit, and I believe this problem should be noted as a disadvantage of my approach.

The clean_username method is technically unnesseccary, it just provides a better error message than the ORM would.  There might be a way to generalize this differently.  The other issue is that username is hard coded into these forms as a regex validation.  That could perhaps be moved to the AbstractUser class itself. 

Yup.

 

• I haven't looked at views as thoroughly, but I believe them to be more straightforward. I will do this dive soon.

In my experimentation, I haven't had to touch the views at all to make things work.  I may have missed something though!

The only other part that would have to change is UserAdmin, although it may end up being simpler just to require a custom Admin class for every custom user model, since it seems to me abstracting that would be tricky.

I don't think there's anything that needs to be done in views either, but I didn't want to confidently make a statement to that effect and be wrong. :-) I actually think that the work that would need to be done for the admin is pretty similar in function to what would need to be done in the forms case.

L

[Stefano Crosta]

Hello all,

this thread as gone so fast that I didn't manage to catch up and jump in time probably, but I did write a contrib app for Email-only user model: https://bitbucket.org/3h/django-accounts

My experience: I had to copy-paste a LOT of code to be sure all works, definitely much more that I thought I would at the beginning, to get what sounded basic to me: a User model /without/ the annoying (to me) username.

Had I to do it again I would probably simply work on "save" to copy the email into the username and hack the form bit not to ask for a username.

It sounds to me like going the other way around (the current User model inherits from the email-only user model) would have been simpler, but I understand that the username model is the standard django one.

Anyway, as I said I feel I'm jumping in slightly too late but I hope that the following is somehow useful:

1. I also prefere user models without usernames

2. My implementation (could definitelyl have been better, but maybe it gives ideas as to what could be made more abstract?) https://bitbucket.org/3h/django-accounts and thus my experience/feedback on it (too complex to make wihtout copying a lot of Auth things along!)

I'll be eagerly following this thread to see where it gets, and hope to be able to contribute more.

Thanks!

Stefano

[Luke Sneeringer]

Good evening, Russell, et. al.,

I had some time this afternoon. :-) Since there are already a couple of reference implementations for how to do this with an e-mail app, I decided to take a crack at an implementation that would include an EmailUser within the base auth app. I understand we've far from decided on a method, and I still stand by my promise to do whatever Russell decides, but I figured code to look at would be a good thing, and that it would give me a better idea of the task.

Here's the branch: https://github.com/lukesneeringer/django/tree/ticket-20824-noemailapp

I have a few observations and self-criticisms:

• The big advantage, as I see it, to this approach over a separate-app approach is that it drastically reduces code duplication. Forms, the admin, etc., all use the same classes they did before, just those classes are now a bit more aware of what the USERNAME_FIELD and REQUIRED_FIELDS are. This is really the reason I believe this to be the correct approach. A separate app would have to be maintained, well, separately. :-)
• I have a few criticisms of my approach, now that I've had a chance to see it in action:
• The create_user and create_superuser methods on UserManager were a bit of a land mine. The current (1.6) implementation takes username, email, and password as positional arguments, and making this into an all-kwarg approach would break back-compat. I landed on a solution, but I'm not sure it's the right one.
• In order to maintain the form error message but not have the form itself be opinionated about the presence of "username", I removed the explicit validator on the form, simply letting ModelForm outsource it to the Model. This provides the same error message for both invalid and duplicate usernames / e-mail addresses (as appropriate). I think this actually might be a *good* thing (again, code duplication is reduced), but I don't really know if there's some reason why it was constructed the way it was before.
• There's a special message for duplicate usernames ("A user with that username already exists."), and it's not a trivial thing to just make an e-mail address correlate because of internationalization. I reverted to the previous message, which was translated ("Please correct the duplicate data for email, which must be unique."), because I really don't know what the appropriate procedure is here. (This would be an issue with a separate-app approach too, but it's on my list of "things I didn't know".)
• In starting to write tests (not committed), I hit an interesting situation where overwriting AUTH_USER_MODEL using override_settings may not be sufficient, which is an issue I would need to figure out. I expect this would not be a problem in an email_auth app, since the model wouldn't be loaded until after the override.

Russell, I definitely understand that you're still not sold on this method. I think that at this point, I've made my case, and if you're still not sold, I'm ready to coalesce the email_auth app implementations (two have been offered) and get that tip-top, tested, and documented.

Thoughts?

L

[Marc Tamlyn]

Thanks for compiling a more complete solution.

The biggest disadvantage as I see it is that is adds even more complexity to the implementation of those forms. In particular the `setattr` on the form class of a `clean_` method is a really nasty code smell. The code in the `create_user`/`create_superuser` commands is also a prime example of additional unnecessary complexity in a function which would be trivially avoided by separation.

The authentication form renders a field called "username" which is a CharField, not an EmailField. This seems pretty bad to me.

Other things which appear to be bugs to me:

- You automatically register an admin class for any custom user model, which is likely to break validation.

- If I set first_name and last_name as REQUIRED_FIELDS then your admin class will break validation or render a weird form as the field names appear twice.

- Docstring for AbstractCommonUser is wrong.

- There are a number of style errors in the code - backslashes for line continuation, unnecessarily wrapped lines, spaces in the wrong places etc.

The Admin related issues here are the biggest problem. If admin validation fails for *any custom user model*, then the server will not start if contrib.auth is installed.

Your main aim here seems to be to reduce code duplication by writing clever, quasi-magical code instead. We're not just trying to make this work for a particular new User model - we need to make it not break for *every* user model. Writing custom user models is an inherently duplicatory process in some places - I've done it in several projects and it always involves lifting some components from Django and tweaking them appropriately. The more we complexify the forms, the harder this is to do. Russell's original point about providing a complete implementation of "a custom user model" as an example is still very valid. Introducing an easier way to do say, email based auth, should not make writing a custom social based auth solution harder.

Now I admit there's not as much magic so far as I expected (though I expect it may get worse in the test suite). However I think the points regarding complexity still stand. From a maintenance perspective, changing this code in two places is bad, but always having to consider at least two (in your case actually many) distinct cases whenever changing the code is worse.

--
You received this message because you are subscribed to the Google Groups "Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.

[Luke Sneeringer]

On Sep 25, 2013, at 2:58 AM, Marc Tamlyn <marc....@gmail.com> wrote:

Thanks for compiling a more complete solution.

The biggest disadvantage as I see it is that is adds even more complexity to the implementation of those forms. In particular the `setattr` on the form class of a `clean_` method is a really nasty code smell. The code in the `create_user`/`create_superuser` commands is also a prime example of additional unnecessary complexity in a function which would be trivially avoided by separation.

The authentication form renders a field called "username" which is a CharField, not an EmailField. This seems pretty bad to me.

It does this in 1.5 and 1.6. AuthenticationForm is already used for custom user models; this doesn't represent an alteration on my part.

Other things which appear to be bugs to me:

- You automatically register an admin class for any custom user model, which is likely to break validation.

Rats. I thought about that particular issue, but forgot to address it.

- If I set first_name and last_name as REQUIRED_FIELDS then your admin class will break validation or render a weird form as the field names appear twice.

This is a good point, although it could be avoided using a set instead of a list.

The more we complexify the forms, the harder this is to do. Russell's original point about providing a complete implementation of "a custom user model" as an example is still very valid. Introducing an easier way to do say, email based auth, should not make writing a custom social based auth solution harder.

I'm not clear on how this would make that harder. It would either make it easier, or not introduce a change at all.

Now I admit there's not as much magic so far as I expected (though I expect it may get worse in the test suite). However I think the points regarding complexity still stand. From a maintenance perspective, changing this code in two places is bad, but always having to consider at least two (in your case actually many) distinct cases whenever changing the code is worse.

This is an important point. Backward compatibility changes become more serious when there are more instances of custom user models in the wild that are able to use the basic UserCreationForm (and similar) classes. That actually might be the ultimate reason why this solution won't be tenable (regardless of preferability), because any maintenance on the forms would have back-compat implications for unknown custom User models.

L

[Luke Sneeringer]

On Sep 25, 2013, at 2:58 AM, Marc Tamlyn <marc....@gmail.com> wrote:

> Other things which appear to be bugs to me:
> - You automatically register an admin class for any custom user model, which is likely to break validation.
> - If I set first_name and last_name as REQUIRED_FIELDS then your admin class will break validation or render a weird form as the field names appear twice.
>

> The Admin related issues here are the biggest problem. If admin validation fails for *any custom user model*, then the server will not start if contrib.auth is installed.

I fixed these. :-)

I am by no means claiming that these are the only drawbacks of my approach, but this is a simple fix: only register User or EmailUser.

Thanks!

L

[Rocky Meza]

Hi,

I'm another one of the authtools devs.

This is a problem that we ran into with authtools, what we ended up doing was just running the tests 3 times[1].  I don't think this is a realistic approach within Django core though.  One of the main problems with this is that you use get_user_model at the root level of the forms module, so the Form will be declared against only one model--I don't know what you can do though.  Having email_auth as a separate app would probably clear up this problem for the tests, but if that happens then we would just have to concrete specific sets of forms, instead of more generic Forms, which was the whole idea.  The only situation where you should be switching settings around is in tests, so I do think it is a little unfair to compromise the generic nature of the forms just to appease the tests.

Russell, I definitely understand that you're still not sold on this method. I think that at this point, I've made my case, and if you're still not sold, I'm ready to coalesce the email_auth app implementations (two have been offered) and get that tip-top, tested, and documented.

Thoughts?

L

I left some more code-specific comments on GitHub.

[1]: https://github.com/fusionbox/django-authtools/blob/master/Makefile#L6-L15 

[Luke Sneeringer]

On Sep 26, 2013, at 2:17 AM, Rocky Meza <rock...@gmail.com> wrote:

This is a problem that we ran into with authtools, what we ended up doing was just running the tests 3 times[1].  I don't think this is a realistic approach within Django core though.  One of the main problems with this is that you use get_user_model at the root level of the forms module, so the Form will be declared against only one model--I don't know what you can do though.  Having email_auth as a separate app would probably clear up this problem for the tests, but if that happens then we would just have to concrete specific sets of forms, instead of more generic Forms, which was the whole idea.  The only situation where you should be switching settings around is in tests, so I do think it is a little unfair to compromise the generic nature of the forms just to appease the tests.

To be fair, several people are pretty convinced that there should be concrete sets of forms in another app for other reasons. :-) It's far from decided that generic == better.

L

[Russell Keith-Magee]

On Wed, Sep 25, 2013 at 8:04 AM, Luke Sneeringer <lu...@sneeringer.com> wrote:

Good evening, Russell, et. al.,

I had some time this afternoon. :-) Since there are already a couple of reference implementations for how to do this with an e-mail app, I decided to take a crack at an implementation that would include an EmailUser within the base auth app. I understand we've far from decided on a method, and I still stand by my promise to do whatever Russell decides, but I figured code to look at would be a good thing, and that it would give me a better idea of the task.

Here's the branch: https://github.com/lukesneeringer/django/tree/ticket-20824-noemailapp

I have a few observations and self-criticisms:

• The big advantage, as I see it, to this approach over a separate-app approach is that it drastically reduces code duplication. Forms, the admin, etc., all use the same classes they did before, just those classes are now a bit more aware of what the USERNAME_FIELD and REQUIRED_FIELDS are. This is really the reason I believe this to be the correct approach. A separate app would have to be maintained, well, separately. :-)

I completely fail to see why this is the case. You seem to be implying that by putting the EmailUser model in a separate app, this means we need to duplicate code. Why can't you: 

* Put a base class in contrib.auth and put a subclass in contrib.email_auth

* Put the model and admin registration in contrib.email_auth and point at contrib.auth.forms.

If there truly is common base functionality, then *put it in a base class*, and use that base class. 

DRY doesn't mean you don't *EVER* duplicate code. It means that you only ever express an *idea* once. If you have two ideas, and they're actually different, the fact that they've got largely similar implementations doesn't mean you tie yourself in knots trying to avoid typing the same characters twice. At the end of the day, practicality beats purity. "Duplicating" a handful of lines of code at the cost of a simple, predictable implementation is a hands-down win in my book. 

This is especially true when you consider that we're dealing with user authentication here, which has huge implications for the security of Django as a platform. Authentication is literally the front line of framework security, because it's how you get authenticated to do stuff in the first place. Complex implementations mean edge cases accidentally slip through; simple implementations don't allow those problems.

• I have a few criticisms of my approach, now that I've had a chance to see it in action:
• The create_user and create_superuser methods on UserManager were a bit of a land mine. The current (1.6) implementation takes username, email, and password as positional arguments, and making this into an all-kwarg approach would break back-compat. I landed on a solution, but I'm not sure it's the right one.

• In order to maintain the form error message but not have the form itself be opinionated about the presence of "username", I removed the explicit validator on the form, simply letting ModelForm outsource it to the Model. This provides the same error message for both invalid and duplicate usernames / e-mail addresses (as appropriate). I think this actually might be a *good* thing (again, code duplication is reduced), but I don't really know if there's some reason why it was constructed the way it was before.
• There's a special message for duplicate usernames ("A user with that username already exists."), and it's not a trivial thing to just make an e-mail address correlate because of internationalization. I reverted to the previous message, which was translated ("Please correct the duplicate data for email, which must be unique."), because I really don't know what the appropriate procedure is here. (This would be an issue with a separate-app approach too, but it's on my list of "things I didn't know".)
• In starting to write tests (not committed), I hit an interesting situation where overwriting AUTH_USER_MODEL using override_settings may not be sufficient, which is an issue I would need to figure out. I expect this would not be a problem in an email_auth app, since the model wouldn't be loaded until after the override.

… and all of these are exactly the problems that I predicted -- that you'll end up with complex (and potentially backwards incompatible) internals, or weird logic switching between models, or problems with tests binding the User model to a global context. 

 

Russell, I definitely understand that you're still not sold on this method. I think that at this point, I've made my case, and if you're still not sold, I'm ready to coalesce the email_auth app implementations (two have been offered) and get that tip-top, tested, and documented.

 

I think it's safe to say I'm not sold at this point.

Yours,

Russ Magee %-)

[Russell Keith-Magee]

Hi Rocky,

On Thu, Sep 26, 2013 at 4:17 PM, Rocky Meza <rock...@gmail.com> wrote:

Hi,

I'm another one of the authtools devs.

On Tuesday, September 24, 2013 6:04:17 PM UTC-6, Luke Sneeringer wrote:

Good evening, Russell, et. al.,

This is a problem that we ran into with authtools, what we ended up doing was just running the tests 3 times[1].  I don't think this is a realistic approach within Django core though.  One of the main problems with this is that you use get_user_model at the root level of the forms module, so the Form will be declared against only one model--I don't know what you can do though.  Having email_auth as a separate app would probably clear up this problem for the tests, but if that happens then we would just have to concrete specific sets of forms, instead of more generic Forms, which was the whole idea.  The only situation where you should be switching settings around is in tests, so I do think it is a little unfair to compromise the generic nature of the forms just to appease the tests.

Not only is it a realistic approach -- it is (within limits) what the current Django core already does. In order to validate that Django's pluggable user infrastructure actually does what it is supposed to do (i.e., let people with different user models use default auth views etc), Django's auth app ships 7 test User models, and large chunks of the test suite are run for each of those models. It's not a case of running the *entire* auth test suite repeatedly; it's matter of finding the parts of logic that are User model dependent, and running *those* tests repeatedly. 

So - it's entirely possible and realistic - as long as you don't bind one specific User model into a global context. Which is one of the reasons why having a separate app helps :-)

Yours,

Russ Magee %-)

[Rocky Meza]

Hey Russ,

On Monday, September 30, 2013 7:17:14 PM UTC-6, Russell Keith-Magee wrote:

Hi Rocky,

On Thu, Sep 26, 2013 at 4:17 PM, Rocky Meza <rock...@gmail.com> wrote:

Hi,

I'm another one of the authtools devs.

On Tuesday, September 24, 2013 6:04:17 PM UTC-6, Luke Sneeringer wrote:

Good evening, Russell, et. al.,

This is a problem that we ran into with authtools, what we ended up doing was just running the tests 3 times[1].  I don't think this is a realistic approach within Django core though.  One of the main problems with this is that you use get_user_model at the root level of the forms module, so the Form will be declared against only one model--I don't know what you can do though.  Having email_auth as a separate app would probably clear up this problem for the tests, but if that happens then we would just have to concrete specific sets of forms, instead of more generic Forms, which was the whole idea.  The only situation where you should be switching settings around is in tests, so I do think it is a little unfair to compromise the generic nature of the forms just to appease the tests.

Not only is it a realistic approach -- it is (within limits) what the current Django core already does. In order to validate that Django's pluggable user infrastructure actually does what it is supposed to do (i.e., let people with different user models use default auth views etc), Django's auth app ships 7 test User models, and large chunks of the test suite are run for each of those models. It's not a case of running the *entire* auth test suite repeatedly; it's matter of finding the parts of logic that are User model dependent, and running *those* tests repeatedly. 

Oh that's cool, I didn't know that.  That makes some things easier.

So - it's entirely possible and realistic - as long as you don't bind one specific User model into a global context. Which is one of the reasons why having a separate app helps :-)

Yours,

Russ Magee %-)

-rocky