Code

Opened 17 months ago

Closed 17 months ago

Last modified 6 months ago

#19822 closed Cleanup/optimization (fixed)

USERNAME_FIELD should be validated as unique=True

Reported by: russellm Owned by: nobody
Component: contrib.auth Version: 1.5-rc-1
Severity: Release blocker Keywords:
Cc: chris.jerdonek@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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.

Attachments (1)

19822-1.diff (2.7 KB) - added by claudep 17 months ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 17 months ago by russellm

This was raised in response to some 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).

Changed 17 months ago by claudep

comment:2 Changed 17 months ago by claudep

  • Has patch set
  • Version changed from 1.4 to 1.5-rc-1

comment:3 Changed 17 months ago by Russell Keith-Magee <russell@…>

  • Resolution set to fixed
  • Status changed from new to closed

In f5e4a699ca0f58818acbdf9081164060cee910fa:

Fixed #19822 -- Added validation for uniqueness on USERNAME_FIELD on custom User models.

Thanks to Claude Peroz for the draft patch.

comment:4 Changed 17 months ago by Russell Keith-Magee <russell@…>

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

comment:5 Changed 17 months ago 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.

comment:6 Changed 16 months ago 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?

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

comment:8 Changed 6 months ago by cjerdonek

  • Cc chris.jerdonek@… added

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.