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

[WIP] Multi User server with session management #391

Closed
wants to merge 3 commits into from

Conversation

echarles
Copy link
Member

Fixes #122

This is WIP and aims to provide foundation session management for a multiuser jupyter server.

This would allow to build applications with authentication like the following example in the screencast.

Kapture 2021-01-21 at 08 54 37

@pierrotsmnrd
Copy link

Following up the Jupyter Server Weekly Dev Meeting :

  • General feedback has been positive on the need for session and the demo with the authentication has been well received
  • There were questions about the use of torndsession, which is seen “old”. One solution is to import their code and maintain it, as it’s a MIT licence.
  • Another question was the trust . My understanding is that anyone connected can run code, which was the main problem. Solutions like a authorization layer could be a way to answer that question : Add authorization layer to server request handlers #165

cc @kevin-bates @vidartf @Zsailer

@echarles
Copy link
Member Author

Thx a lot @pierrotsmnrd for the participation to today meeting which I was not able to attend. I am adding @blink1073 to this PR. I guess this is a pretty important move we have discussed some time ago in #122 and I hope we can get back on this next week during next meeting. Authorization model is key this is why the work @Zsailer has done in #165 could be used in conjunction to this proposal to make Jupyter Server more "auth".

@ellisonbg
Copy link
Contributor

The single user notebook server has a base class for handlers that be be overridden to provide customer authenticators and session management. JupyterHub is doing this for its variant of the single user notebook server here:

https://github.com/jupyterhub/jupyterhub/blob/master/jupyterhub/singleuser/mixins.py#L676

Currently this is a bit monkey-patchy as it requires overriding base classes for the handlers with mixins. If we want to add auth/sessions/identify to the single user server I believe we should do so in a way that Jupyterhub can also use and leverage to simplify this logic and make it less monkey-patchy.

I don't think the jupyter-server needs to be able to reuse all of the JupyterHub authenticators, just that we build it in a manner that addresses JupyterHub's usage cases as well.

@echarles can you comment on how the proposal in this PR helps or interferes with this direction?

@codecov-commenter
Copy link

Codecov Report

Merging #391 (3bf4ce3) into master (c9ee2a4) will decrease coverage by 1.80%.
The diff coverage is 30.98%.

❗ Current head 3bf4ce3 differs from pull request most recent head 5d14708. Consider uploading reports for the commit 5d14708 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #391      +/-   ##
==========================================
- Coverage   77.04%   75.23%   -1.81%     
==========================================
  Files         109      115       +6     
  Lines        9945    10068     +123     
  Branches     1078     1100      +22     
==========================================
- Hits         7662     7575      -87     
- Misses       1907     2114     +207     
- Partials      376      379       +3     
Impacted Files Coverage Δ
jupyter_server/serverapp.py 64.88% <ø> (-1.22%) ⬇️
jupyter_server/torndsession/memorysession.py 0.00% <0.00%> (ø)
jupyter_server/torndsession/session.py 31.61% <31.61%> (ø)
jupyter_server/torndsession/compat.py 37.83% <37.83%> (ø)
jupyter_server/torndsession/driver.py 52.17% <52.17%> (ø)
jupyter_server/base/handlers.py 63.76% <60.00%> (-0.16%) ⬇️
jupyter_server/torndsession/__init__.py 100.00% <100.00%> (ø)
jupyter_server/__main__.py 66.66% <0.00%> (-33.34%) ⬇️
jupyter_server/auth/__main__.py 0.00% <0.00%> (-25.00%) ⬇️
jupyter_server/traittypes.py 55.35% <0.00%> (-8.34%) ⬇️
... and 22 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c9ee2a4...5d14708. Read the comment docs.

@echarles echarles mentioned this pull request Dec 10, 2021
@echarles echarles changed the title [WIP] Add session management [WIP] Multi User server with session management Dec 10, 2021
@echarles
Copy link
Member Author

Referring @Zsailer on #165 (comment) who has waited 2 years to see the Authorization feature merged, I still have hope in this one 1 year later :)

In this multi-user case, the additional issue I think is that the use-case/usage/need for this is not well understood, prolly also not well surfaced/explained.

@echarles can you comment on how the proposal in this PR helps or interferes with this direction?

@ellisonbg I can comment a lot in the thread, but I think a conversation during e.g. the server community meeting would allow easier interactions.

There are ramifications in the work being done by @minrk in #671 and in the whole RTC stories.

@echarles
Copy link
Member Author

@fcollonval

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DISCUSS] Add a Session Management infrastructure for extension developers
5 participants