Autogenerate SECRET_KEY or use given one

[ObadaS]

A brief description of the purpose of the changes contained in this PR.

Automatically generate Django's Secret Key if it does not exist inside the .env file, otherwise use the given secret key

Alternative to #2232

Checklist

• Code review by me
• Hand tested by me
• I'm proud of my work
• Code review by reviewer
• Hand tested by reviewer
• CircleCi tests are passing
• Ready to merge

[Didayolo]

We need to update the documentation to indicate that admins should add a SECRET_KEY variable in .env

[ObadaS]

The variable is added automatically in the .env file at launch if no secret key is defined.
But I can also update the documentation to document these changes

[Didayolo]

The variable is added automatically in the .env file at launch if no secret key is defined. But I can also update the documentation to document these changes

Ah yes I forgot about this. I thought it was going to be re-generated everytime instead of being written. Not sure if we want to document it then.

[Didayolo]

@ObadaS

Why do we use config = configobj.ConfigObj('.env') ?

I feel like it just adds a useless package. We are already manually parsing the .env file.

I think we could simply checks for the environment variable.

Maybe something like:

env_secret = os.environ.get("SECRET_KEY")
if env_secret:
    SECRET_KEY = env_secret
else:
      SECRET_KEY = get_random_secret_key()
      with open(".env", "a") as f:
          f.write(f"\nSECRET_KEY={SECRET_KEY}\n")

---

Also, a nice call from #2232 is that it was updating .env at Docker entrypoint, not in base/settings.py, so it may be safer if other services use the key.

[ObadaS]

@ObadaS

Why do we use config = configobj.ConfigObj('.env') ?

I feel like it just adds a useless package. We are already manually parsing the .env file.

I think we could simply checks for the environment variable.

While we do read the lines in the files, it stores the whole line into the variable, so if we find the SECRET_KEY we would need to remove SECRET_KEY=' and the last ' before assigning it in Django

Also, I saw from your last commit that you removed f.close(), was that on purpose ?

[Didayolo]

@ObadaS
Why do we use config = configobj.ConfigObj('.env') ?
I feel like it just adds a useless package. We are already manually parsing the .env file.
I think we could simply checks for the environment variable.

While we do read the lines in the files, it stores the whole line into the variable, so if we find the SECRET_KEY we would need to remove SECRET_KEY=' and the last ' before assigning it in Django

Also, I saw from your last commit that you removed f.close(), was that on purpose ?

I just edited my comment for more details.

About f.close() yes it is on purpose, you don't need it when you use with open(".env", "a+") as f:.

[Didayolo]

By the way I tested it and it seems to work fine, I am just nitpicking on details

[ObadaS]

@ObadaS

Why do we use config = configobj.ConfigObj('.env') ?

I feel like it just adds a useless package. We are already manually parsing the .env file.

I think we could simply checks for the environment variable.

Maybe something like:

env_secret = os.environ.get("SECRET_KEY")
if env_secret:
    SECRET_KEY = env_secret
else:
      SECRET_KEY = get_random_secret_key()
      with open(".env", "a") as f:
          f.write(f"\nSECRET_KEY={SECRET_KEY}\n")

Also, a nice call from #2232 is that it was updating .env at Docker entrypoint, not in base/settings.py, so it may be safer if other services use the key.

This code would not work since the base.py is loaded multiple times during launch (and everytime we do so Django commands through manage.py like shell_plus or migrations)

With your code, the secret key will keep getting written in the .env file until you restart the Django containers and make the secret key variable an environnement variable. So you end up with multiple secret keys in the .env

As for #2232 putting the generation inside a docker entry point, it means that in the future, when we make Codabench binaries and don't rely on docker (or podman), the secret key generation won't work.

[Didayolo]

@ObadaS OK, thank you for the explanation. On my side I think it is ready to be merged.