-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow user to access their Magpie cookie programmatically #407
Conversation
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2320/Result : success BIRDHOUSE_DEPLOY_BRANCH : add-magpie-cookie-as-env-var DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-133.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/1449/NOTEBOOK TEST RESULTS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks, this will be useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a bad idea to instruct users to use env variables for cookies (note: plain text password are not better either). It can give the false impression that this is more secure.
If the cookie is printed in the output or stored anywhere in the notebook, it could leak access.
Another issue is that cookies are not persistent. Once a refresh occurs, the value set in MAGPIE_COOKIES
will not be valid anymore. This will confuse the users even more.
I'm not sure what would be the best solution yet, but my first impression is that MAGPIE_COOKIES
trades a problem for another.
I think the cookie is supposed to be temporary. If user does another login, it will invalidate the old cookie. It's about the same risk as all the secrets that CI pipeline made available to tests. Any rogues tests running on the same CI system can potentially steal/log all those secrets.
Ah this one I never though of ! Given the tighter integration between JupyterHub session and Magpie (login to JupyterHub or Magpie will also login to the other one automatically) I thought this issue should not arise. If indeed |
I don't know if the Jupyter authenticator would be able to dynamically update the value of |
Ok so just so I understand correctly, the situation that you're worried about is:
This is a valid problem which we should consider. It could be mitigated with good documentation but if we can make this transparent to the user that would be better.
It can't, but we can dynamically access it using a different method, let me update this PR with the alternative method and you can let me know what you think. |
@fmigneault note that the new method accesses the cookie value stored in the jupyterhub database directly. So this scenario:
becomes:
|
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2334/Result : failure BIRDHOUSE_DEPLOY_BRANCH : add-magpie-cookie-as-env-var DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-154.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/1459/NOTEBOOK TEST RESULTS |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2335/Result : failure BIRDHOUSE_DEPLOY_BRANCH : add-magpie-cookie-as-env-var DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-46.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/1460/NOTEBOOK TEST RESULTS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -24,6 +24,26 @@ if os.getenv("JUPYTERHUB_CRYPT_KEY"): | |||
c.MagpieAuthenticator.refresh_pre_spawn = True | |||
c.MagpieAuthenticator.auth_refresh_age = int("${JUPYTERHUB_AUTHENTICATOR_REFRESH_AGE}") | |||
|
|||
# Allow users to access their own auth_state in order to get their own magpie cookie | |||
# See https://github.com/jupyterhub/jupyterhub/issues/3588 for details | |||
c.JupyterHub.load_roles = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume JUPYTER_API_TOKEN
now exists because of this new config?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No I think it always existed in the environment
Curious, when the user logs back into Jupyterhub, isn't the |
No the problem is that the environment variables are only set once when the jupyterlab server first spawns |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the new JupyterHub API alternative is a good solution.
Please make it slightly more explicit in the CHANGES which code (first) is to avoid and the new "STRONGLY RECOMMENDED" approach to use instead.
That should also be described somewhere else in docs because it will get lost over time with future releases.
I'll make sure we document it in the jupyterhub tutorials (in progress: DACCS-Climate/marble-tutorials#18) |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2346/Result : failure BIRDHOUSE_DEPLOY_BRANCH : add-magpie-cookie-as-env-var DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-154.rdext.crim.ca Infrastructure deployment failed. Instance has not been destroyed. @matprov |
Overview
When the user logs in to jupyterhub, their Magpie cookie is stored in the jupyterhub database. This allows the user to access this variable to programmatically access resources protected by magpie without having to copy/paste these cookies from their browser session or add a username and password in plaintext to the file.
For example, to access a dataset behind a secured URL with
xarray.open_dataset
using a username and password:And to do the same thing using the current magpie cookie already used to log in the current user (no need to include username and password)
Note that users who are already logged in to jupyterhub will need to log out and log in for these changes to take effect.
Changes
Non-breaking changes
Breaking changes
Related Issue / Discussion
Additional Information
birdhouse_daccs_configs_branch: master
birdhouse_skip_ci: false