[Django] #19822: USERNAME_FIELD should be validated as unique=True
Django
------------------------------------------------+------------------------
Reporter: russellm | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.auth | Version: 1.4
Severity: Release blocker | Keywords:
Triage Stage: Accepted | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
The docs describe the fact that USERNAME_FIELD on a custom User model must
be unique. Lots of code (e.g., login forms) work on the assumption that
USERNAME_FIELD is unique.
However, nothing actually enforces this requirement.
There should be a validation step to enforce the uniqueness of
USERNAME_FIELD. It would also be advisable to enforce db_index=True (since
username will be a common lookup field)
Marking as release blocker because it's an easy mistake for end-developers
to make, and will cause all sorts of weird bugs if it isn't caught.
--
Ticket URL: <https://code.djangoproject.com/ticket/19822>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: russellm | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.auth | Version: 1.4
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by russellm):
This was raised in response to some [https://groups.google.com/d/msg
/django-users/S8lGqkOf8o0/r7PSZsgmDO0J mailing list activity]; the OP
suggested that the unique and db_index properties should be auto applied
to USERNAME_FIELD. We could do this, but I think explicit is better than
implicit.
If being named in USERNAME_FIELD magically made the field unique and
indexed, the developer needs to implicitly know that this will happen. If
a developer actually *wanted* the username field to be non-unique, they
might get a surprise when they find that it isn't. Of course, this points
at a design problem on their end; the difference is that if it is
implicit, they will discover the problem by accident when a non-unique
username is rejected; if it is raised as a validation error, they'll get
an error when they sync their tables (giving them an indication that the
problem must be fixed).
--
Ticket URL: <https://code.djangoproject.com/ticket/19822#comment:1>
Django
Reporter: russellm | Owner: nobody
Type: Cleanup/optimization | Status: new
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* has_patch: 0 => 1
* version: 1.4 => 1.5-rc-1
--
Ticket URL: <https://code.djangoproject.com/ticket/19822#comment:2>
Django
Reporter: russellm | Owner: nobody
Type: Cleanup/optimization | Status: closed
Component: contrib.auth | Version: 1.5-rc-1
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Russell Keith-Magee <russell@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"f5e4a699ca0f58818acbdf9081164060cee910fa"]:
{{{
#!CommitTicketReference repository=""
revision="f5e4a699ca0f58818acbdf9081164060cee910fa"
Fixed #19822 -- Added validation for uniqueness on USERNAME_FIELD on
custom User models.
Thanks to Claude Peroz for the draft patch.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/19822#comment:3>
Django
Reporter: russellm | Owner: nobody
Type: Cleanup/optimization | Status: closed
Component: contrib.auth | Version: 1.5-rc-1
Severity: Release blocker | Resolution: fixed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Russell Keith-Magee <russell@…>):
In [changeset:"bc6746ac303ab625f2bc6fc878bd63661c784a59"]:
{{{
#!CommitTicketReference repository=""
revision="bc6746ac303ab625f2bc6fc878bd63661c784a59"
[1.5.x] Fixed #19822 -- Added validation for uniqueness on USERNAME_FIELD
on custom User models.
Thanks to Claude Peroz for the draft patch.
(cherry picked from commit f5e4a699ca0f58818acbdf9081164060cee910fa)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/19822#comment:4>
Django
Reporter: russellm | Owner: nobody
Type: Cleanup/optimization | Status: closed
Component: contrib.auth | Version: 1.5-rc-1
Severity: Release blocker | Resolution: fixed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by russellm):
For the record -- I ommitted the db_index check because (1) on most
databases, a unique field will be automatically indexed, and (2) Django's
default User doesn't have db_index=True.
--
Ticket URL: <https://code.djangoproject.com/ticket/19822#comment:5>
Django
Reporter: russellm | Owner: nobody
Type: Cleanup/optimization | Status: closed
Component: contrib.auth | Version: 1.5-rc-1
Severity: Release blocker | Resolution: fixed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by anonymous):
> [...] Of course, this points at a design problem on their end; [...]
I must disagree on that (and yes, I got the surprise since everything was
working fine with the release candidate). I actually ran into a scenario
where the username alone should not be unique. I am currently implementing
a system where a reseller can have labels and labels have users. In this
specific case, a user must be unique for a label (which is basically a
company), but not for a reseller. Users don't have to know that 2 labels
are owned by a single reseller. In short; our unique constraint is based
on the label and the USERNAME_FIELD.
I can imagine there are more valid cases where this validation check would
be blocking. Wouldn't it be better to make this validation option optional
by i.e. a config setting?
--
Ticket URL: <https://code.djangoproject.com/ticket/19822#comment:6>
Django
Reporter: russellm | Owner: nobody
Type: Cleanup/optimization | Status: closed
Component: contrib.auth | Version: 1.5-rc-1
Severity: Release blocker | Resolution: fixed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by cjerdonek):
I also think this enforcement should be kept optional. I created issue
#21735 for this. There I describe a second use case, in addition to the
one by the anonymous poster above.
--
Ticket URL: <https://code.djangoproject.com/ticket/19822#comment:7>
Django
Reporter: russellm | Owner: nobody
Type: Cleanup/optimization | Status: closed
Component: contrib.auth | Version: 1.5-rc-1
Severity: Release blocker | Resolution: fixed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by cjerdonek):
* cc: chris.jerdonek@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/19822#comment:8>