Skip to content
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

Use default user token when running under JupyterHub #21

Merged
merged 1 commit into from
Nov 19, 2021

Conversation

yuvipanda
Copy link
Collaborator

When running under a JupyterHub, only the currently running
notebook server process is accessible from the browser (via
the proxy), so only the token set for the current server will work.
Any additional servers running (via jupyter-server-proxy) will
still need this token, so multiple servers running in a JupyterHub
(a currently unknown pattern) can't work anyway.

So when we are running in a JupyterHub, we just use the provided
token in the share URL. No network call is made.

Fixes #10

When running under a JupyterHub, only the currently running
notebook server process is accessible from the browser (via
the proxy), so only the token set for the current server will work.
Any additional servers running (via jupyter-server-proxy) will
still need this token, so multiple servers running in a JupyterHub
(a currently unknown pattern) can't work anyway.

So when we are running in a JupyterHub, we just use the provided
token in the share URL. No network call is made.

Fixes jupyterlab-contrib#10
yuvipanda added a commit to yuvipanda/datahub that referenced this pull request Nov 19, 2021
To use real time collaboration in JupyterLab, we
need jupyterlab-contrib/jupyterlab-link-share#10
to be fixed.
jupyterlab-contrib/jupyterlab-link-share#21 is
a proposed solution, and I'm testing it out here to see if it
works properly.

Ref berkeley-dsep-infra#3027
@yuvipanda
Copy link
Collaborator Author

The test failure seems unrelated?

@jtpio
Copy link
Member

jtpio commented Nov 19, 2021

The test failure seems unrelated?

Right, this is tracked in jupyter-server/jupyter_releaser#131

Copy link
Member

@jtpio jtpio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks Yuvi for the fix!!

@jtpio jtpio merged commit 526163e into jupyterlab-contrib:main Nov 19, 2021
@jtpio
Copy link
Member

jtpio commented Nov 19, 2021

Making a new release with this change.

@consideRatio
Copy link

This is related to jupyterhub/jupyterhub#3683 I think. I find the topic quite complicated overall, but here is my attempted summary.

When a user visits JupyterHub with a browser, they are assigned a session id. This session id is planned to be connected to the token in the future (via jupyterhub/jupyterhub#3683), but, this PR will make us make use of a token that was associated with another user's/browser's session id - and that could perhaps break when the JupyterHub behaviour change...

Hmmm, something like that. I'm +1 for this change and such, but I just wanted to highlight there may be issues down the line in this implementation, or maybe not --- I'm not sure: this is so tricky!

@jtpio
Copy link
Member

jtpio commented Nov 19, 2021

Thanks @consideRatio for the input.

Right, this extension was first created just so we could easily test RTC on the JupyterLab repo, which uses jupyter-server-proxy on Binder to spawn a lab in dev mode. Need to check this is still working with this change.

@jtpio
Copy link
Member

jtpio commented Nov 19, 2021

@yuvipanda
Copy link
Collaborator Author

Based on my understanding of @minrk's comment in jupyterhub/jupyterhub#3683 (comment), that PR landing will break the behavior I am relying on in this PR. Thoughts on how to mitigate this, @minrk?

@yuvipanda
Copy link
Collaborator Author

Thanks @jtpio! I'll wait for #24 to land too before rebuilding :D

@minrk
Copy link
Contributor

minrk commented Nov 19, 2021

It is not true that a token can be assumed to be available in PageConfig when running under JupyterHub, at least not until jupyterlab/jupyterlab_server#220 lands and JupyterHub is updated to make use of it. After that's fixed, though, I'm not sure using that value would be right, but I think you've discussed the "sharing with a token granting you access to become me" in #24).

Prior to JupyterHub 1.5, JupyterLab would always put its server token on the page, which isn't the right token to use, especially in 2.0 with granular permissions. JupyterHub 1.5 disables this, at least when running with jupyter_server, and JupyterLab should disable it as well.

We need jupyterlab/jupyterlab_server#220 to give us a hook to set the token on a per request basis. But this token would not be super appropriate to use in a share link.

@consideRatio
Copy link

I've verified this to work with Z2JH 1.1.3 that runs JupyterHub 1.4.2 as a server and with a user environment having JupyterHub 1.5.0 on the image, jupyterlab 3.2.4, and jupyterlab-link-share 0.2.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Token set to an empty string as /servers returned an empty string - why?
4 participants