Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Adding a UI to let users share their servers to other users and groups #438

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Hyrla
Copy link

@Hyrla Hyrla commented Feb 8, 2025

Hello,
I've added the necessary code related to my issue #437.

image

It replaces the default link share UI in cases where Jupyter is running behind a Hub that allows the creation of shares. It enables users to:

  • Create a share to the current server for a user or a group using a search box.
  • Revoke a share for a user or a group.

The permissions needed (both in Hub and Lab) are:

  • "shares!user" to manage users' shares.
  • "read:users:name" to create shares by designating a user by their name (TODO: check if it's really necessary if list:users is already in scope?).
  • "list:users" (not mandatory) to list users for the search box.
  • "list:groups" (not mandatory) to list groups for the search box.
  • "servers!user" (not mandatory) to allow other users to start and stop the shared server

My code does not:

  • Work if neither list:users nor list:groups is in scope. I wanted to add an option to invite someone by typing their exact username, without having to search.
  • Manage share codes, which may be useful in some scenarios.

This development has been made for my own internal purposes, but I think it could be useful to everyone. Comments and critiques are welcome!

Copy link
Contributor

github-actions bot commented Feb 8, 2025

Binder 👈 Launch a Binder on branch Hyrla/jupyter-collaboration/main

@ykazakov
Copy link

ykazakov commented Mar 16, 2025

Amazing work!
I think determining the server owner should be more robust:

https://github.com/Hyrla/jupyter-collaboration/blob/1743de5c50dcd6372dd7926cfc72b96970544f02/packages/collaboration/src/sharedlink.ts#L48

This does not work if c.JupyterHub.base_url is set.

@Hyrla
Copy link
Author

Hyrla commented Mar 16, 2025

Amazing work! I think determining the server owner should be more robust:

https://github.com/Hyrla/jupyter-collaboration/blob/1743de5c50dcd6372dd7926cfc72b96970544f02/packages/collaboration/src/sharedlink.ts#L48

This does not work if c.JupyterHub.base_url is set.

Thank you for your feedback. Do you have any suggestion to determine the server owner with a cleaner way?

@ykazakov
Copy link

I think, if the server is running under JupyterHub, one can extract JupyterHub.base_url from PageConfig.getOption('hubPrefix') by removing the trailing /hub/.

@ykazakov
Copy link

ykazakov commented Mar 16, 2025

There also appears to be an option PageConfig.getOption('hubServerUser'), which is probably what you need?

@ykazakov
Copy link

Similarly, serverName can be extracted from PageConfig.getOption('hubServerName').

@ykazakov
Copy link

ykazakov commented Mar 16, 2025

You can see all available options of PageConfig by inspecting the jupyter-config-data tag of the HTML source of the lab page.
See https://jupyterlab.readthedocs.io/en/latest/api/functions/coreutils.PageConfig.getOption.html

@Hyrla
Copy link
Author

Hyrla commented Mar 16, 2025

You can see all available options of PageConfig by inspecting the jupyter-config-data tag of the HTML source of the lab page. See https://jupyterlab.readthedocs.io/en/latest/api/functions/coreutils.PageConfig.getOption.html

Omg thank you, I searched hours for this list of PageConfig

@Hyrla
Copy link
Author

Hyrla commented Mar 16, 2025

@ykazakov I've pushed the fix, thanks again for your feedback. Could you please try if it works on your Hub? It works on mine but c.JupyterHub.base_url is not set there.

@ykazakov
Copy link

@Hyrla I tested, now it works with c.JupyterHub.base_url set!
However, these options are probably available only if running under JupyterHub, so the variables should probably be set after the check is done:

// If hubPrefix or hubHost or hubUser is set, we are behind a JupyterHub
if (
PageConfig.getOption('hubPrefix') ||
PageConfig.getOption('hubHost') ||
PageConfig.getOption('hubUser')
) {

Also, if running behind a JupyterHub, shouldn't all these options be set, not just one of them?
I would suggest to instead check the presence of (all) options that you really use for the dialog.

@Hyrla
Copy link
Author

Hyrla commented Mar 17, 2025

@ykazakov thank you for your feedback. I am pushing a fix concerning the two getOptions() that are misplaced. Concerning the detection of JupyterHub, not all these variables are necessarily set. I did set back the original condition as seen before my pull request:

PageConfig.getOption('hubUser') !== '',

@ykazakov
Copy link

I have been further testing, and in general, the dialog works great! 👍

Some (mostly minor) comments:

  1. In the search list one cannot tell users and groups apart. If there is a group named exactly as the user, it is not clear which one is wich.
  2. Clicking on the item in the list immediately creates a share. Perhaps it would make sense to have a confirmation dialogue to prevent unintentional shares (particularly for groups).
  3. What happens if the list is very long (hundreds of users)? Perhaps wait until something is typed in the search box and the number of matches is narrowed down. See the nextcloud share dialogue.
  4. If the user does not have a permission to share a server (example: user Alice shares her server with user Bob, and user Bob tries to re-share the server of Alice), the share button results in the standard dialog, which would not work even if to include the token. Perhaps instead, one should show a dialog that the user does not have permission to share this server (if in doubt, ask admin), or even better, not to show the share icon at all.

@manics
Copy link

manics commented Mar 17, 2025

This should be where the page config vars originate:
https://github.com/jupyterhub/jupyterhub/blob/5.2.1/jupyterhub/singleuser/extension.py#L216-L234

@Hyrla
Copy link
Author

Hyrla commented Mar 17, 2025

I have been further testing, and in general, the dialog works great! 👍

Thank you!

Some (mostly minor) comments:

1. In the search list one cannot tell users and groups apart. If there is a group named exactly as the user, it is not clear which one is wich.

Yes you're right, I will add a "Group" prefix like in the shares list

2. Clicking on the item in the list immediately creates a share. Perhaps it would make sense to have a confirmation dialogue to prevent unintentional shares (particularly for groups).

Do you have any example snippets of code to create such a dialogue ? I wasn't able to find one (I am not an expert at all in TS nor Jupyter extension)

3. What happens if the list is very long (hundreds of users)? Perhaps wait until something is typed in the search box and the number of matches is narrowed down. See the [nextcloud share dialogue](https://nextcloud.com/blog/sharing-in-nextcloud/).

On my Hub there are about 350 users and it works like a charm. I am using the API with pagination, so there is no big query with every users at a single time. But if there is thousands of users it could be a problem. As far as I know, there is no Hub API to search for users or group?

4. If the user does not have a permission to share a server (example: user Alice shares her server with user Bob, and user Bob tries to re-share the server of Alice), the share button results in the standard dialog, which would not work even if to include the token. Perhaps instead, one should show a dialog that the user does not have permission to share this server (if in doubt, ask admin), or even better, not to show the share icon at all.

Like for 2., I don't know (yet) a way to create such dialogue but I will add it asap

@Hyrla
Copy link
Author

Hyrla commented Mar 17, 2025

This should be where the page config vars originate: https://github.com/jupyterhub/jupyterhub/blob/5.2.1/jupyterhub/singleuser/extension.py#L216-L234

Thank you!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants