Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#19354 closed Bug (fixed)

Unable to set PK for custom user model

Reported by: markteisman@… Owned by: nobody
Component: contrib.auth Version: 1.5-alpha-1
Severity: Normal Keywords: PK Primary Key Custom User
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description (last modified by Claude Paroz)

Hello,
I've written a custom user model, and wished to set a field as the primary key. See the example below.

class User(AbstractBaseUser):
    email = models.EmailField(_('email'), max_length=75, unique=True, primary_key=True)

    USERNAME_FIELD = 'email'
    ...

Registering the user works fine, but when I authenticate the user using django.contrib.auth.views.login, I get an error stating that the 'User' object has no attribute 'id'.

I discussed this on IRC, where it was suggested to report this as a bug. We believe it is desirable to use .pk instead, for reasons related to portability.

A quick search has shown that there are 12 instances of 'user.id'.

Here's a summary of the error report:

Exception Type:	AttributeError
Exception Value: 'User' object has no attribute 'id'
Exception Location:	<path>/lib/python2.6/site-packages/django/contrib/auth/__init__.py in login, line 84
Python Executable:	<path>/bin/python
Python Version:	2.6.5

Regards,
Mark

Attachments (1)

19354-1.diff (7.1 KB ) - added by Claude Paroz 11 years ago.
Do not assume user.id == user.pk

Download all attachments as: .zip

Change History (8)

comment:1 by Claude Paroz, 11 years ago

Description: modified (diff)

Reformatted, please use preview.

comment:2 by Claude Paroz, 11 years ago

Easy pickings: set
Triage Stage: UnreviewedAccepted

I think that in this case, replacing id by pk is trivial.

In the more general case, I'm not sure if we want to be completely id-agnostic for the User model. There are places where it is currently required to be an integer. It would also be useful to know why you couldn't keep an autoincrement id, while keeping email unique.

comment:3 by markteisman@…, 11 years ago

In my case it was in no way necessary, and I reverted to using id as pk. To make sure the email address gets indexed I turned to db_index=True instead.

I'm trying to think of reasons why to become id-agnostic though. What if developers would be insistent in specifying a pk, and attempted to solve the occurring issue by setting a non-unique id. The following test may then pass for several distinct users:

if request.session[SESSION_KEY] != user.id:
    # To avoid reusing another user's session, create a new, empty
    # session if the existing session corresponds to a different
    # authenticated user.
    request.session.flush()

I suspect that this potentially could lead to unfortunate situations. Relying on fields that are unique by definition this wouldn't pass(?)

In all honesty I'm quite a novice developer though, and I'm not sure if this truly could lead to problems.

I've achieved my goal if I made you aware of this, so that you can make an informed decision about what to do.

by Claude Paroz, 11 years ago

Attachment: 19354-1.diff added

Do not assume user.id == user.pk

comment:4 by Claude Paroz, 11 years ago

Has patch: set

You are right in that as long as the user model is no longer under our control, it is a mistake to assume id == pk. Attached a patch with proposed updates.

comment:5 by markteisman@…, 11 years ago

Thank you for your adequate response

comment:6 by Claude Paroz <claude@…>, 11 years ago

Resolution: fixed
Status: newclosed

In 0eeae15056edf07f786d3be5b47c14ab62eacd31:

Fixed #19354 -- Do not assume usermodel.pk == usermodel.id

Thanks markteisman at hotmail.com for the report.

comment:7 by Claude Paroz <claude@…>, 11 years ago

In 47c5b50d346237f30584ad9303dea8f6933ffae3:

[1.5.x] Fixed #19354 -- Do not assume usermodel.pk == usermodel.id

Thanks markteisman at hotmail.com for the report.
Backport of 0eeae1505 from master.

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