Skip to content

Commit

Permalink
fix: issues from SECRET_KEY_LOAD_AND_STORE and settings reorganization (
Browse files Browse the repository at this point in the history
#676)

#668 added a new `SECRET_KEY_LOAD_AND_STORE` boolean setting in the
backend. If `SECRET_KEY_LOAD_AND_STORE` is false, then the `SECRET_KEY`
is generated at startup and not stored. If it is true, the program tries
reading it from `SECRET_KEY_PATH`, and if that fails generate a new one
and write it there.

The intent was to allow configuring components that use the same
settings as the backend server not to write the `SECRET_KEY` on the file
system, because this eases configuration in production.

In #668 this was set to `False` by default. However to match the pre-668
behavior you'd need to set it to `True` by default.

Possibly we could get rid of this setting instead and just write the key
every time.

I don't know _why_ frontend authentication on k8s breaks when this
setting is set to `false`. The `SECRET_KEY` is used for signing JWTs, so
presumably at some point Django is restarted and so previously generated
JWTs aren't valid anymore.. But JWTs are requested (and therefore
generated) right before they're needed, so that's weird.

---

#668 also moved the file containing this line:

```python
BASE_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
```

without updating it, which broke paths


Co-authored-by: Guilhem Barthés <guilhem.barthes@owkin.com>
Signed-off-by: Olivier Léobal <olivier.leobal@owkin.com>
  • Loading branch information
oleobal and guilhem-barthes committed Jun 26, 2023
1 parent 0734f72 commit ab5d604
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 17 deletions.
2 changes: 1 addition & 1 deletion backend/backend/settings/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@
# https://docs.djangoproject.com/en/2.0/howto/static-files/

STATIC_URL = f"/{SUBPATH}static/"
MEDIA_ROOT = os.path.join(PROJECT_ROOT, "medias")
MEDIA_ROOT = PROJECT_ROOT / "medias"
SITE_ID = 1

TASK = {
Expand Down
11 changes: 5 additions & 6 deletions backend/backend/settings/deps/path.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import os
import sys
from pathlib import Path

# Build paths inside the project like this: os.path.join(BASE_DIR, ...)
BASE_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
PROJECT_ROOT = os.path.dirname(BASE_DIR)
BASE_DIR: Path = Path(__file__).resolve().parents[2]
PROJECT_ROOT = BASE_DIR.parent

sys.path.append(PROJECT_ROOT)
sys.path.append(os.path.normpath(os.path.join(PROJECT_ROOT, "libs")))
sys.path.append(str(PROJECT_ROOT))
sys.path.append(str(PROJECT_ROOT / "libs"))
8 changes: 4 additions & 4 deletions backend/backend/settings/deps/secret_key.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,26 @@
"""

import os
import pathlib
import secrets
from pathlib import Path

from . import path
from .utils import to_bool

SECRET_KEY_PATH = os.environ.get("SECRET_KEY_PATH", os.path.normpath(os.path.join(path.PROJECT_ROOT, "SECRET")))
SECRET_KEY_PATH = Path(os.environ.get("SECRET_KEY_PATH", path.PROJECT_ROOT / "SECRET"))


def _generate_secret_key():
return secrets.token_urlsafe() # uses a "reasonable default" length


_SECRET_KEY_LOAD_AND_STORE = to_bool(
os.environ.get("SECRET_KEY_LOAD_AND_STORE", "False")
os.environ.get("SECRET_KEY_LOAD_AND_STORE", "True")
) # Whether to load the secret key from file (and write it there if it doesn't exist)

if _SECRET_KEY_LOAD_AND_STORE:
try:
SECRET_KEY = pathlib.Path(SECRET_KEY_PATH).read_text().strip()
SECRET_KEY = SECRET_KEY_PATH.read_text().strip()
except IOError:
try:
SECRET_KEY = _generate_secret_key()
Expand Down
6 changes: 3 additions & 3 deletions backend/backend/settings/dev.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

DEBUG = True

STATIC_ROOT = os.path.join(BASE_DIR, "statics")
STATIC_ROOT = BASE_DIR / "statics"

# Enable Browsable API
REST_FRAMEWORK["DEFAULT_RENDERER_CLASSES"] += ("rest_framework.renderers.BrowsableAPIRenderer",)
Expand All @@ -25,8 +25,8 @@
MODEL_BUCKET_NAME = "substra-model"
MODEL_STORAGE = MinioStorage(MODEL_BUCKET_NAME)

MEDIA_ROOT = os.environ.get("MEDIA_ROOT", os.path.join(PROJECT_ROOT, f"medias/{ORG_NAME}"))
SERVERMEDIAS_ROOT = os.environ.get("SERVERMEDIAS_ROOT", os.path.join(PROJECT_ROOT, f"servermedias/{ORG_NAME}"))
MEDIA_ROOT = os.environ.get("MEDIA_ROOT", str(PROJECT_ROOT / "medias" / ORG_NAME))
SERVERMEDIAS_ROOT = os.environ.get("SERVERMEDIAS_ROOT", str(PROJECT_ROOT / "servermedias" / ORG_NAME))
FUNCTION_BUCKET_NAME = "substra-function"
FUNCTION_STORAGE = MinioStorage(FUNCTION_BUCKET_NAME)

Expand Down
2 changes: 1 addition & 1 deletion backend/backend/settings/prod.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
SESSION_COOKIE_SECURE = True
CSRF_COOKIE_SECURE = True

STATIC_ROOT = os.path.join(BASE_DIR, "statics")
STATIC_ROOT = BASE_DIR / "statics"

DATASAMPLE_BUCKET_NAME = "substra-datasample"
DATASAMPLE_STORAGE = MinioStorage(DATASAMPLE_BUCKET_NAME)
Expand Down
4 changes: 2 additions & 2 deletions docs/settings.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ Accepted true values for `bool` are: `1`, `ON`, `On`, `on`, `T`, `t`, `TRUE`, `T

| Type | Setting | Default value | Comment |
|------|---------|---------------|---------|
| bool | `SECRET_KEY_LOAD_AND_STORE` | `False` | Whether to load the secret key from file (and write it there if it doesn't exist) |
| string | `SECRET_KEY_PATH` | `os.path.normpath(os.path.join(path.PROJECT_ROOT, 'SECRET'))` | |
| bool | `SECRET_KEY_LOAD_AND_STORE` | `True` | Whether to load the secret key from file (and write it there if it doesn't exist) |
| string | `SECRET_KEY_PATH` | `path.PROJECT_ROOT / 'SECRET'` | |

## JWT settings

Expand Down

0 comments on commit ab5d604

Please sign in to comment.