chore: make secret key configurable and reorganize settings #668
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
documentation
|
How about moving JWT related statements into a dedicated module? Benefits:
Disclaimer:
|
Why not, I think that would fit under this PR. I didn't think of it because our settings files are usually a bit bigger than this but really splitting |
oleobal
force-pushed
the
chore/jwt-key
branch
2 times, most recently
from
de28649
to
a85ecd8
Compare
SdgJlbl
reviewed
Jun 13, 2023
oleobal
force-pushed
the
chore/jwt-key
branch
6 times, most recently
from
de9e301
to
5fcdb66
Compare
oleobal
changed the title
oleobal
force-pushed
the
chore/jwt-key
branch
3 times, most recently
from
d481f20
to
aad6230
Compare
oleobal
changed the title
|
/e2e |
|
End to end tests: ✔️ SUCCESS |
SdgJlbl
reviewed
Jun 13, 2023
| @@ -1,11 +1,11 @@ | |||
| import os | |||
|
|
|||
| from .. import common | |||
| from .utils import to_bool | |||
SdgJlbl
approved these changes
Jun 13, 2023
2 tasks
Signed-off-by: Olivier Léobal <olivier.leobal@owkin.com>
Signed-off-by: Olivier Léobal <olivier.leobal@owkin.com>
Signed-off-by: Olivier Léobal <olivier.leobal@owkin.com>
Signed-off-by: Olivier Léobal <olivier.leobal@owkin.com>
Signed-off-by: Olivier Léobal <olivier.leobal@owkin.com>
|
/e2e --tests sdk,frontend |
oleobal
added a commit
that referenced
this pull request
Jun 20, 2023
oleobal
added a commit
that referenced
this pull request
Jun 20, 2023
2 tasks
oleobal
added a commit
that referenced
this pull request
Jun 26, 2023
#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>
Merged
thbcmlowk
added a commit
that referenced
this pull request
Jun 30, 2023
## [0.39.0](https://github.com/Substra/substra-backend/releases/tag/0.39.0) 2023-06-27 ### Added - New `SECRET_KEY_PATH` and `SECRET_KEY_LOAD_AND_STORE` environment variables ([#668](#668)) Signed-off-by: Thibault Camalon <135698225+thbcmlowk@users.noreply.github.com>
oleobal
added a commit
that referenced
this pull request
Aug 14, 2023
Writing on the root filesystem fails when `readOnlyRootFilesystem` is set, which is the case in some environments, such as OpenShift. This PR fixes this. Since `SECRET_KEY` is now stored in a secret, it should also mostly fix "Given token is not valid for any token type" errors, which were due to application restart where `SECRET_KEY` was regenerated and users had to refresh their tokens (which were signed with the old `SECRET_KEY`) Reverts many changes from #668 Fixes FL-1000 Signed-off-by: Olivier Léobal <olivier.leobal@owkin.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Rename some files and variables to be a bit clearer. Reorganize the settings to lower the size of common.py. Split off CORS and OIDC settings into their own category of settings (
mods) because they change settings from common and therefore must be applied afterwards.Add two new configuration options:
SECRET_KEY_PATHandSECRET_KEY_LOAD_AND_STOREInline
gen_secret_key.pyThis is a backwards compatible prerequisite PR to: #657
Checklist