#15371 closed (fixed)
createsuperuser with --noinput creates users with empty password
Reported by: | yishaibeeri | Owned by: | nobody |
---|---|---|---|
Component: | contrib.auth | Version: | dev |
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: | no | UI/UX: | no |
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.
Attachments (1)
Change History (11)
by , 14 years ago
Attachment: | createsuperuser_no_pwd.patch added |
---|
follow-up: 3 comment:1 by , 14 years ago
Keywords: | blocker added |
---|
comment:2 by , 14 years ago
Triage Stage: | Unreviewed → Accepted |
---|
follow-up: 4 comment:3 by , 14 years ago
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):
- 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).
- 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?
- 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 by , 14 years ago
Cc: | added |
---|
I apologise - forgot to login before posting my reply above.
Replying to anonymous:
comment:5 by , 14 years ago
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 by , 14 years ago
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 by , 14 years ago
Triage Stage: | Accepted → 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.
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.