Code

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#15371 closed (fixed)

createsuperuser with --noinput creates users with empty password

Reported by: yishaibeeri Owned by: nobody
Component: contrib.auth Version: master
Severity: Keywords: createsuperuser blocker
Cc: yishaibeeri Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

As of [14053], empty passwords are allowed by the contrib.auth User model and helper functions.

As a side effect, when running the createsuperuser management command in non-interactive mode, the created superuser is given the (valid) empty password. However, the usage string displayed when running createsuperuser --help promises that the user will be created with auth.models.UNUSABLE_PASSWORD.

IMHO, the previous behavior (create the user with UNUSABLE_PASSWORD) is the correct one - I'm attaching a patch that fixes this.

If the new, empty-password behavior, is decided to be the right one, then the usage string for createsuperuser should be fixed to reflect that.

Also see https://groups.google.com/group/django-developers/browse_thread/thread/36da4515d621fb78/e07fa8b7b876bea5

Attachments (1)

createsuperuser_no_pwd.patch (1.9 KB) - added by yishaibeeri 3 years ago.

Download all attachments as: .zip

Change History (11)

Changed 3 years ago by yishaibeeri

comment:1 follow-up: Changed 3 years ago by russellm

  • Keywords blocker added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

As was discussed at the time, there's no particular reason to reject "" (empty string) as a password -- in fact, that very point is made in the mailing list thread you reference.

Accepting, but with a different patch required.

It's also a blocker, because this is a recent change that requires tweaking.

comment:2 Changed 3 years ago by russellm

  • Triage Stage changed from Unreviewed to Accepted

comment:3 in reply to: ↑ 1 ; follow-up: Changed 3 years ago by anonymous

Replying to russellm:

As was discussed at the time, there's no particular reason to reject "" (empty string) as a password -- in fact, that very point is made in the mailing list thread you reference.

I agree - explicitly setting an empty password should be allowed.
However, currently in createsuperuser the empty password is created implicitly, as there is no way to provide a password in --noinput mode.

I see three options (maybe more that I'm missing):

  1. Allow empty passwords, but have createsuperuser create users with UNUSABLE_PASSWORD when run in --noinput mode - as documented in the usage string (this is the current patch).
  2. Add the option to provide a password (including the empty one) to the createsuperuser command line. If not provided, fall back to the behavior in (1) above. I think this would be a useful addition - but perhaps there is good reason this option was not added to createsuperuser?
  3. Keep the current behavior (default to empty password) - in which case the patch is simply to fix the usage string. I find this the most problematic solution - as it gives no way to automatically create a superuser with any kind of secure password. This would force users to write their custom script or management command that additionally (re)sets the password.

Accepting, but with a different patch required.

I'll be happy to write one - which of the above should it be? or yet something else?

It's also a blocker, because this is a recent change that requires tweaking.

comment:4 in reply to: ↑ 3 Changed 3 years ago by yishaibeeri

  • Cc yishaibeeri added

I apologise - forgot to login before posting my reply above.

Replying to anonymous:

comment:5 Changed 3 years ago by russellm

  • Patch needs improvement set

I'd say (1) is the right approach here. Preventing a user from logging in until they have actually provided a password isn't surprising behavior, and in this case, it *documented* unsurprising behavior. The actual implementation has drifted so the drift needs to be corrected.

comment:6 Changed 3 years ago by yishaibeeri

  • Patch needs improvement unset

In this case I believe the patch I have attached does exactly this. It only fixes the createsuperuser wrong behavior, and does not modify the recent change of allowing empty passwords in create_user and set_password.

I'm unsetting the patch_needs_improvement flag - is there something else missing?

comment:7 Changed 3 years ago by russellm

  • Triage Stage changed from Accepted to Ready for checkin

My apologies -- I misinterpreted your comments about the patch, and didn't pay enough attention to the patch itself.

In which case, nothing else is required here; marking the patch as RFC.

comment:8 Changed 3 years ago by russellm

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

In [15631]:

Fixed #15371 -- Ensure that a superuser created with the createsuperuser management command with --noinput has an invalid password, not a blank password. Thanks to yishaibeeri for the report and patch.

comment:9 Changed 3 years ago by russellm

In [15632]:

[1.2.X] Fixed #15371 -- Ensure that a superuser created with the createsuperuser management command with --noinput has an invalid password, not a blank password. Thanks to yishaibeeri for the report and patch.

Backport of r15631 from trunk.

comment:10 Changed 3 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

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.