Opened 7 years ago

Closed 7 years ago

#27524 closed Bug (wontfix)

Using user instance (instead of get_user_model()) leads to errors when user model is overridden

Reported by: Andy Martin Owned by: nobody
Component: contrib.auth Version: 1.8
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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)).

Attachments (1)

0001-Use-get_user_model-instead-of-user-instance-to-call-.patch (1.5 KB ) - added by Andy Martin 7 years ago.
Patch file for proposed fix (will also be submitted as a PR on Github)

Download all attachments as: .zip

Change History (5)

by Andy Martin, 7 years ago

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

comment:1 by Tim Graham, 7 years ago

Component: Uncategorizedcontrib.auth
Needs tests: set

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 PR is enough.

comment:2 by Andy Martin, 7 years ago

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.

comment:3 by Simon Charette, 7 years ago

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.

comment:4 by Tim Graham, 7 years ago

Resolution: wontfix
Status: newclosed

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

Note: See TracTickets for help on using tickets.
Back to Top