ensure refresh_user still works before user state has been persisted to db #23
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
1 task
tlvu
approved these changes
Nov 13, 2023
mishaschwartz
added a commit
to bird-house/birdhouse-deploy
that referenced
this pull request
Nov 30, 2023
…permission to access (#402) ## Overview By setting the `JUPYTERHUB_CRYPT_KEY` environment variable in the `env.local` file, jupyterhub will store user's authentication information (session cookie) in the database. This allows jupyterhub to periodically check whether the user still has permission to access jupyterhub (the session cookie is not expired and the permission have not changed). The minimum duration between checks can be set with the `JUPYTERHUB_AUTHENTICATOR_REFRESH_AGE` variable which is an integer (in seconds). Note that users who are already logged in to jupyterhub will need to log out and log in for these changes to take effect. To forcibly log out all users currently logged in to jupyterhub you can run the following command to force the recreation of the cookie secret: ```shell docker exec jupyterhub rm /persist/jupyterhub_cookie_secret && docker restart jupyterhub ``` First discussed here: #358 (comment) ## Changes **Non-breaking changes** - Adds two new environment variables to configure additional jupyterhub authentication - New jupyterhub version pavics/jupyterhub:4.0.2-20231024 **Breaking changes** ## Related Issue / Discussion Related to #334 - [x] Note that this PR requires Ouranosinc/jupyterhub#23 to be merged in first and the jupyterhub version updated to match. ## Additional Information <!-- The test suite can be run using a different DACCS config with ``birdhouse_daccs_configs_branch: branch_name`` in the PR description. To globally skip the test suite regardless of the commit message use ``birdhouse_skip_ci: true`` in the PR description. --> birdhouse_daccs_configs_branch: master birdhouse_skip_ci: false
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.
When a user logs in, the
refresh_userseems to be called before the user'sauth_stateis persisted to the database.We assumed that this could not happen in this order so we assumed that
auth_statecould not be None whenrefresh_userwas called. This assumption seems to be false.To fix this, we should use a pattern suggested by the documentation to simply check whether the auth_state is None directly.
Note, this has been tested in the birdhouse stack and I can confirm that the auth_state does eventually (later in the log-in chain) get persisted to the database and is used to refresh the user as expected.