[Django] #27524: Using user instance (instead of get_user_model()) leads to errors when user model is overridden

16 views
Skip to first unread message

Django

unread,
Nov 22, 2016, 12:02:59 PM11/22/16
to django-...@googlegroups.com
#27524: Using user instance (instead of get_user_model()) leads to errors when user
model is overridden
-----------------------------------------+------------------------
Reporter: Andy Martin | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.8
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
We've been seeing errors like this on 1.8.16:

{{{
File "[...snip...]/django/contrib/auth/__init__.py", line 111, in login
request.session[SESSION_KEY] = user._meta.pk.value_to_string(user)
AttributeError: 'MetaDict' object has no attribute 'pk'
}}}

We solved them by going through the object returned by get_user_model()
instead of using the user instance directly. This also matches what's done
in other parts of the codebase (for example, in
_get_user_session_key(request)).

--
Ticket URL: <https://code.djangoproject.com/ticket/27524>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Nov 22, 2016, 12:03:27 PM11/22/16
to django-...@googlegroups.com
#27524: Using user instance (instead of get_user_model()) leads to errors when user
model is overridden
-------------------------------+--------------------------------------

Reporter: Andy Martin | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.8
Severity: Normal | Resolution:

Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
Changes (by Andy Martin):

* Attachment "0001-Use-get_user_model-instead-of-user-instance-to-
call-.patch" added.

Patch file for proposed fix (will also be submitted as a PR on Github)

Django

unread,
Nov 22, 2016, 1:16:06 PM11/22/16
to django-...@googlegroups.com
#27524: Using user instance (instead of get_user_model()) leads to errors when user
model is overridden
------------------------------+--------------------------------------

Reporter: Andy Martin | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.8
Severity: Normal | Resolution:

Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
------------------------------+--------------------------------------
Changes (by Tim Graham):

* component: Uncategorized => contrib.auth
* needs_tests: 0 => 1


Comment:

Could you provide details about your custom user model? I don't know how
`user._meta` returns `MetaDict`. If the use case is valid, we'll also need
a regression test. By the way, you don't need to attach a patch to the
ticket, the [https://github.com/django/django/pull/7597 PR] is enough.

--
Ticket URL: <https://code.djangoproject.com/ticket/27524#comment:1>

Django

unread,
Nov 22, 2016, 1:53:55 PM11/22/16
to django-...@googlegroups.com
#27524: Using user instance (instead of get_user_model()) leads to errors when user
model is overridden
------------------------------+--------------------------------------

Reporter: Andy Martin | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
------------------------------+--------------------------------------

Comment (by Andy Martin):

Here's the user model:

{{{
from bson import ObjectId
from mongoengine import Document, StringField, BooleanField, DateTimeField

class User(Document):
id = StringField(required=True, primary_key=True, default=lambda:
str(ObjectId()))
username = StringField(required=True)
password = StringField()
is_active = BooleanField(default=True)
is_staff = BooleanField(default=False)
email = StringField()
first_name = StringField()
last_name = StringField()
last_login = DateTimeField()
}}}

In our settings.py, we have the following values set:
{{{
AUTH_USER_MODEL = 'mongo_auth.MongoUser'
MONGOENGINE_USER_DOCUMENT = '[...snip...].admin.models.User' # (points to
User model defined above)
}}}

If you think it should be the responsibility of the person overriding the
user model to make sure it works fine with Django's contrib.auth
implementation, that's fine with me, although googling for the error
message shows other people seem to be hitting the same issue.

Let me know if you think the use case is valid and, if so, I can look into
making a regression test.

--
Ticket URL: <https://code.djangoproject.com/ticket/27524#comment:2>

Django

unread,
Nov 22, 2016, 3:04:43 PM11/22/16
to django-...@googlegroups.com
#27524: Using user instance (instead of get_user_model()) leads to errors when user
model is overridden
------------------------------+--------------------------------------

Reporter: Andy Martin | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
------------------------------+--------------------------------------

Comment (by Simon Charette):

Hello Andy,

There's many occurrences of access to `instance._meta` through Django's
code base where it's assumed to return the same value as
`instance.__class__._meta`. In order to prevent breakages I think it
should be fixed in the library you use to expose `_meta` API compliant
objects.

--
Ticket URL: <https://code.djangoproject.com/ticket/27524#comment:3>

Django

unread,
Nov 22, 2016, 3:15:29 PM11/22/16
to django-...@googlegroups.com
#27524: Using user instance (instead of get_user_model()) leads to errors when user
model is overridden
------------------------------+--------------------------------------

Reporter: Andy Martin | Owner: nobody
Type: Bug | Status: closed
Component: contrib.auth | Version: 1.8
Severity: Normal | Resolution: wontfix

Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
------------------------------+--------------------------------------
Changes (by Tim Graham):

* status: new => closed
* resolution: => wontfix


Comment:

Agreed, I don't think it's feasible to support a different API and give it
complete test coverage.

--
Ticket URL: <https://code.djangoproject.com/ticket/27524#comment:4>

Reply all
Reply to author
Forward
0 new messages