Opened 6 years ago

Closed 6 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 6 years ago.
Patch file for proposed fix (will also be submitted as a PR on Github)

Download all attachments as: .zip

Change History (5)

Changed 6 years ago by Andy Martin

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

comment:1 Changed 6 years ago by Tim Graham

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 Changed 6 years ago 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.

comment:3 Changed 6 years ago 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.

comment:4 Changed 6 years ago by Tim Graham

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