Opened 12 months ago

Closed 9 months ago

#34542 closed Bug (fixed)

Required fields allowed to be blank are not accepted non-interactively using createsuperuser

Reported by: Lantizia Owned by: Mateusz Więckowski
Component: contrib.auth Version: dev
Severity: Normal Keywords: auth createsuperuser superuser email
Cc: 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 (last modified by Lantizia)

I've only encountered Django once before (installing mailman3), but this time I was trying to install NetBox and I noticed this sentence in their documentation...

"Specifying an email address for the user is not required" (search for that line in the URL below for better context)

https://docs.netbox.dev/en/stable/installation/3-netbox/

According to these lines... https://github.com/django/django/blob/main/django/contrib/auth/models.py#L378 and https://github.com/django/django/blob/main/django/contrib/auth/models.py#L358

The 'email' field is marked as required, but is also marked in a way to allow it to be empty.

This section of the code validates required fields when createsuperuser is called interactively... https://github.com/django/django/blob/main/django/contrib/auth/management/commands/createsuperuser.py#L143

This section of the code validates required fields when createsuperuser is called non-interactively... https://github.com/django/django/blob/main/django/contrib/auth/management/commands/createsuperuser.py#L219

Although required fields are enforced, the non-interactive code doesn't allow for those to be blank, where blank required fields are permitted. I've tried to set a field like 'email' to blank non-interactively but nothing works, for example...

a) just don't set the email field either through an argument or an environment variable... it'll complain it is needed
b) set the email field via a variable as just 'DJANGO_SUPERUSER_EMAIL=' (i.e. setting the variable to null)... it still complains it is needed
c) set the email field via an argument of --email (i.e. double quotes) or --email "" (i.e. double single quotes) or even --email \ (i.e. passing a single space)... it still complains it is needed

Hope this makes sense :)

Change History (28)

comment:1 by Lantizia, 12 months ago

Description: modified (diff)

comment:2 by Natalia Bidart, 12 months ago

Resolution: invalid
Status: newclosed

Hello! The email field for the provided User class from django.contrib.auth does indeed require the email. But the email is allowed to be the empty string (blank=True in the model), so when creating users via a django web form or interactively, one can pass the empty string as value and that is accepted.

In summary, the email field is required but it can be blank.

comment:3 by Lantizia, 12 months ago

Hi, then the bug is that noninteractively... it is not allowed to be blank.

comment:4 by Lantizia, 12 months ago

Resolution: invalid
Status: closednew

comment:5 by Lantizia, 12 months ago

Description: modified (diff)
Summary: Required fields not enforced when interactively using createsuperuserRequired fields allowed to be blank are not accepted non-interactively using createsuperuser

comment:6 by Lantizia, 12 months ago

Description: modified (diff)

comment:7 by Natalia Bidart, 12 months ago

Triage Stage: UnreviewedAccepted

Thanks Lantizia, you have a valid point, we could make the noninteractively process allow for the empty string for email, to match the behavior of the interactive and web experience, but only when either the --email or env var should be passed as such though.

Would you like to prepare a patch?

comment:8 by Anvansh Singh, 12 months ago

Owner: changed from nobody to Anvansh Singh
Status: newassigned

I will be preparing a patch for it. Will send it out by next wednesday.

in reply to:  7 ; comment:9 by Mohit Singh Sinsniwal, 11 months ago

Replying to Natalia Bidart:

Thanks Lantizia, you have a valid point, we could make the noninteractively process allow for the empty string for email, to match the behavior of the interactive and web experience, but only when either the --email or env var should be passed as such though.

Would you like to prepare a patch?

I want to work on this. However, I would like to understand what we want to solve. If we give an email and username with --noinput, it does not require the password.

python3.11 manage.py createsuperuser --email "a@b.com"  --user mohitd --noinput

And if we dont want to write the email, it will require the password.

python3.11 manage.py createsuperuser --email ""  --user mohite

Do we want to create --noinput with no email? Because that means creating a superuser without giving an email and password

in reply to:  9 comment:10 by Natalia Bidart, 11 months ago

Replying to Mohit Singh Sinsniwal:

Replying to Natalia Bidart:

Thanks Lantizia, you have a valid point, we could make the noninteractively process allow for the empty string for email, to match the behavior of the interactive and web experience, but only when either the --email or env var should be passed as such though.

Would you like to prepare a patch?

I want to work on this. However, I would like to understand what we want to solve. If we give an email and username with --noinput, it does not require the password.

python3.11 manage.py createsuperuser --email "a@b.com"  --user mohitd --noinput

And if we dont want to write the email, it will require the password.

python3.11 manage.py createsuperuser --email ""  --user mohite

Hi! I'm not entirely sure what you are asking, since your original report was about using --noinput without passing an email explicitely. In the --noinput case, password is not asked via the prompt (but it's used from the env var if given), so we want something similar. So if the command is called with something like this:

python -Wall manage.py createsuperuser --username=adminusername --email --noinput

or this:

DJANGO_SUPERUSER_EMAIL= python -Wall manage.py createsuperuser --username=adminusername --noinput

the superuser should be created with no email. Password can be provided via the DJANGO_SUPERUSER_PASSWORD env var.

Do we want to create --noinput with no email? Because that means creating a superuser without giving an email and password

We understood this was your ask in the bug report. If it wasn't, what's your expected outcome then?
Thanks!

in reply to:  description comment:11 by Syed Abdul Mateen, 10 months ago

Replying to Lantizia:

I've only encountered Django once before (installing mailman3), but this time I was trying to install NetBox and I noticed this sentence in their documentation...

"Specifying an email address for the user is not required" (search for that line in the URL below for better context)

https://docs.netbox.dev/en/stable/installation/3-netbox/

According to these lines... https://github.com/django/django/blob/main/django/contrib/auth/models.py#L378 and https://github.com/django/django/blob/main/django/contrib/auth/models.py#L358

The 'email' field is marked as required, but is also marked in a way to allow it to be empty.

This section of the code validates required fields when createsuperuser is called interactively... https://github.com/django/django/blob/main/django/contrib/auth/management/commands/createsuperuser.py#L143

This section of the code validates required fields when createsuperuser is called non-interactively... https://github.com/django/django/blob/main/django/contrib/auth/management/commands/createsuperuser.py#L219

Although required fields are enforced, the non-interactive code doesn't allow for those to be blank, where blank required fields are permitted. I've tried to set a field like 'email' to blank non-interactively but nothing works, for example...

a) just don't set the email field either through an argument or an environment variable... it'll complain it is needed
b) set the email field via a variable as just 'DJANGO_SUPERUSER_EMAIL=' (i.e. setting the variable to null)... it still complains it is needed
c) set the email field via an argument of --email (i.e. double quotes) or --email "" (i.e. double single quotes) or even --email \ (i.e. passing a single space)... it still complains it is needed

Hope this makes sense :)

I just tried making a superuser non-interactively using the command

python manage.py createsuperuser --email "" --user admin

and it works without any error.

Is this bug still valid? @Natalia Bidart

Last edited 10 months ago by Syed Abdul Mateen (previous) (diff)

comment:12 by Natalia Bidart, 10 months ago

Syed, can you please your whole invocation and shell result? For me, your command prompts for password:

$ python manage.py createsuperuser --email "" --user admin
Password:
Operation cancelled.

And when passing --nopinput, I get an error:

$ python manage.py createsuperuser --email "" --user admin --noinput
CommandError: You must use --email with --noinput.

comment:13 by Mateusz Więckowski, 10 months ago

Hi! I'm not entirely sure that I understood the problem, but if so, I do believe I have created a solution.

In the 'non-interactive' part of the code where the error is thrown (e.g., CommandError: You must use --email with --noinput), I added a block of code that checks if the field is allowed to be blank. If that's the case, I simply do not raise the error.

This is how it works for me now:

py manage.py createsuperuser --username username --noinput

Superuser created successfully.

Let me know if that's what was the issue with this one.

comment:14 by Natalia Bidart, 10 months ago

Hello Mateusz, the goal for the fix is to allow the email value to be blank, not to allow it to be missing. So basically an invocation with --email "" should succeed but not passing --email at all should fail for noinput or should prompt for email if interactive.

Does that make sense?

comment:15 by Mateusz Więckowski, 9 months ago

Okay, got It. This is how it works for me now:

py manage.py createsuperuser --username testing --noinput
CommandError: You must use --email with --noinput.

py manage.py createsuperuser --email "" --username testing --noinput
Superuser created successfully.

I did not change anything for the "interactive" part of the process - as far as I've understood from the comments it works as intended.

in reply to:  15 comment:16 by Natalia Bidart, 9 months ago

Replying to Mateusz Więckowski:

Okay, got It. This is how it works for me now:

py manage.py createsuperuser --username testing --noinput
CommandError: You must use --email with --noinput.

py manage.py createsuperuser --email "" --username testing --noinput
Superuser created successfully.

I did not change anything for the "interactive" part of the process - as far as I've understood from the comments it works as intended.

Thanks for sharing this. What shell are you using? I'm using bash with Python 3.11, and I get this:

$ python -Wall manage.py createsuperuser --email "" --username testing --noinput
CommandError: You must use --email with --noinput.
Last edited 9 months ago by Natalia Bidart (previous) (diff)

comment:17 by Mateusz Więckowski, 9 months ago

I'm using cmd. I did some changes to "non-interactive" part in Django's createsuperuser.py

It's rather quick, so you could do it yourself and check:
Move this line:
https://github.com/django/django/blob/main/django/contrib/auth/management/commands/createsuperuser.py#L226
above the if statement at the line 222
In the if itself add the following:

if field.blank and options[field_name] is not None: 
    continue

in reply to:  17 comment:18 by Natalia Bidart, 9 months ago

Replying to Mateusz Więckowski:

I'm using cmd. I did some changes to "non-interactive" part in Django's createsuperuser.py

It's rather quick, so you could do it yourself and check:
Move this line:
https://github.com/django/django/blob/main/django/contrib/auth/management/commands/createsuperuser.py#L226
above the if statement at the line 222
In the if itself add the following:

if field.blank and options[field_name] is not None: 
    continue

Would you like to prepare a patch/PR? Please note that the solution should also work when passing the env var DJANGO_SUPERUSER_EMAIL (on both interactive and --noinput mode).

comment:19 by Mateusz Więckowski, 9 months ago

When passing DJANGO_SUPERUSER_EMAIL:

py manage.py createsuperuser --username username

Prompts for email, allows it to be blank, doesn't get value from DJANGO_SUPERUSER_EMAIL.

py manage.py createsuperuser --email "" --username username

Doesn't prompt for email, doesn't get value from DJANGO_SUPERUSER_EMAIL, sets email blank.

py manage.py createsuperuser --email "" --username username --noinput
py manage.py createsuperuser --username username --noinput

Doesn't prompt, gets value from DJANGO_SUPERUSER_EMAIL.

I believe it works how it's supposed to. Is that correct?

comment:20 by Natalia Bidart, 9 months ago

Hello Mateusz,

Did you run the examples above with or without your changes? Using Django main, this is what I get:

$ DJANGO_SUPERUSER_EMAIL="" python -Wall manage.py createsuperuser --username testing --noinput
CommandError: You must use --email with --noinput.

If you are confident that your proposal is correct and works for every case, then please create a PR and I'll happily review. Thanks!

comment:21 by Lufafa Joshua, 9 months ago

Replying to Mateusz Więckowski:

I'm using cmd. I did some changes to "non-interactive" part in Django's createsuperuser.py
It's rather quick, so you could do it yourself and check:
Move this line:
https://github.com/django/django/blob/main/django/contrib/auth/management/commands/createsuperuser.py#L226
above the if statement at the line 222
In the if itself add the following:

if field.blank and options[field_name] is not None: 
continue

Hey, Considering your approach, instead of using:

if field.blank and options[field_name] is not None: 
    continue

I have used

if field.blank: 
    continue

and it seems to bypass the exception if the field is allowed to be blank while using environment variable or --noinput

in reply to:  21 comment:22 by Mateusz Więckowski, 9 months ago

Replying to Lufafa Joshua:

Replying to Mateusz Więckowski:

I'm using cmd. I did some changes to "non-interactive" part in Django's createsuperuser.py
It's rather quick, so you could do it yourself and check:
Move this line:
https://github.com/django/django/blob/main/django/contrib/auth/management/commands/createsuperuser.py#L226
above the if statement at the line 222
In the if itself add the following:

if field.blank and options[field_name] is not None: 
continue

Hey, Considering your approach, instead of using:

if field.blank and options[field_name] is not None: 
    continue

I have used

if field.blank: 
    continue

and it seems to bypass the exception if the field is allowed to be blank while using environment variable or --noinput

Hey, that was my initial approach, but it creates unwanted behavior that is described in comments: #comment:13 and #comment:14.

I've also found that this:

if field.blank and options[field_name] is not None: 
    continue

Doesn't work for all cases when setting DJANGO_SUPERUSER_EMAIL.

My current approach looks like this and works for all cases I could've found.

if not value:
    if (field.blank and
            (options[field_name] is not None or 
                os.environ.get(env_var) is not None)): 
        continue

in reply to:  20 ; comment:23 by Mateusz Więckowski, 9 months ago

Replying to Natalia Bidart:

Hello Mateusz,

Did you run the examples above with or without your changes? Using Django main, this is what I get:

$ DJANGO_SUPERUSER_EMAIL="" python -Wall manage.py createsuperuser --username testing --noinput
CommandError: You must use --email with --noinput.

If you are confident that your proposal is correct and works for every case, then please create a PR and I'll happily review. Thanks!

I ran the examples with my changes. I found one case with environmental variables that wasn't working, but managed to fix it.

At the moment I am confident that my current solution works, but before doing the PR I have just one question - should I write tests as well? In the ticket details, it says 'Needs tests: no', but in the guide about creating a PR, it's mentioned that tests are always needed, except for documentation changes.

in reply to:  23 comment:24 by Sarah Boyce, 9 months ago

Replying to Mateusz Więckowski:

At the moment I am confident that my current solution works, but before doing the PR I have just one question - should I write tests as well? In the ticket details, it says 'Needs tests: no', but in the guide about creating a PR, it's mentioned that tests are always needed, except for documentation changes.

Yes you should write tests. The 'Needs tests' area of the ticket is marked as 'Yes' when there was a patch that got reviewed and it was missing tests, then this is part of the feedback as to why the patch has not been accepted. 👍

comment:25 by Mateusz Więckowski, 9 months ago

Has patch: set
Owner: changed from Anvansh Singh to Mateusz Więckowski

comment:26 by Natalia Bidart, 9 months ago

Patch needs improvement: set

comment:27 by Mariusz Felisiak, 9 months ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:28 by Mariusz Felisiak <felisiak.mariusz@…>, 9 months ago

Resolution: fixed
Status: assignedclosed

In 5aa4c0b:

Fixed #34542 -- Made createsuperuser handle required blank fields in non-interactive mode.

Note: See TracTickets for help on using tickets.
Back to Top