Code

Opened 3 years ago

Closed 2 years ago

#17046 closed Bug (fixed)

User.objects.create_user() allows empty usernames

Reported by: honza Owned by: nobody
Component: contrib.auth Version: master
Severity: Normal Keywords:
Cc: restless.being@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

This code will work:

User.objects.create_user(username='', email='test@test.com', password='password')

It should raise some kind of exception.

I searched for this in the tracker and didn't find any references. I'm happy to work on this given some direction from the core.

Attachments (5)

validate_create_user.diff (399 bytes) - added by ptone 3 years ago.
prevent_empty_username.diff (2.6 KB) - added by honza 3 years ago.
validate_user_save.diff (2.8 KB) - added by nerdap 2 years ago.
Does validation in the User.save() method, and raises a validation error if the username is empty.
patch_for_17046.diff (2.8 KB) - added by kwadrat 2 years ago.
patch_for_17046_another.diff (2.0 KB) - added by kwadrat 2 years ago.

Download all attachments as: .zip

Change History (14)

Changed 3 years ago by ptone

comment:1 Changed 3 years ago by ptone

  • Has patch set
  • Needs documentation unset
  • Needs tests set
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted
  • Version changed from 1.3 to SVN

I do think the manager function should validate the model - attached is a very quick patch that does this - it is not a complete patch

Changed 3 years ago by honza

comment:2 Changed 3 years ago by honza

  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Accepted to Ready for checkin

I added a better patch. Passing in an empty string will raise an IntegrityError. Includes tests. Marking as ready for checkin.

comment:3 Changed 3 years ago by julien

  • Triage Stage changed from Ready for checkin to Accepted

Please do not promote your own patches to RFC and let someone else do it if it passes review. From looking at the patch, it feels like the validation shouldn't be done in the manager but deeper down at the model level. Also ValidationError would seem more appropriate than IntegrityError.

Changed 2 years ago by nerdap

Does validation in the User.save() method, and raises a validation error if the username is empty.

Changed 2 years ago by kwadrat

comment:4 Changed 2 years ago by ext

  • Cc restless.being@… added

Kwadrat's patch looks OK and works as intended.

I wondered about using self.full_clean() in save, instead of the above. However, according to JKocherhans comment here: https://code.djangoproject.com/ticket/13100 using full_clean would break backward compatibility.

Changed 2 years ago by kwadrat

comment:5 Changed 2 years ago by pbnan

Reviewed the latest patch patch_for_17046_another.diff, it works as intended.

comment:6 Changed 2 years ago by pbnan

  • Triage Stage changed from Accepted to Ready for checkin

comment:7 follow-up: Changed 2 years ago by HM

I'd say "design decision needed" on this one. It is not so uncommon to use the email-address as a "username", possibly copying it to the username-field after the create().

comment:8 in reply to: ↑ 7 Changed 2 years ago by charettes

Replying to HM:

I'd say "design decision needed" on this one. It is not so uncommon to use the email-address as a "username", possibly copying it to the username-field after the create().

Isn't there a unique constraint on the username field? Doing so might raise constraint violations in a multi-threaded environment if you are trying to have two users in the copying email to username state at the same time since both share an empty string username while doing it.

comment:9 Changed 2 years ago by jezdez

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

In [17628]:

Fixed #17046 -- Added a check if the username passed to User.objects.create_user is empty or not. Thanks, kwadrat.

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.