🐛 [BUG]: magpie allows special characters in usernames that mess up jupyterhub #393

mishaschwartz · 2023-10-17T18:17:38Z

Summary

Jupyterhub creates jupyterlab docker containers named jupyter-{username} where {username} is the user name of the logged in user.

Magpie permits usernames with . in them but jupyterhub will substitute the . with -2e when creating the jupyterlab container. This means that a user with the name example.user will have a container named jupyter-example-2euser and the username according to jupyterlab will be example-2euser.

This causes problems since we assume that these names will align.

Details

Magpie restricts usernames according to this regex: ^[A-Za-z0-9]+(?:[\s_\-\.][A-Za-z0-9]+)*$
- https://github.com/Ouranosinc/Magpie/blob/3.36.0/magpie/api/management/user/user_utils.py#L868-L882
- https://github.com/Ouranosinc/Magpie/blob/3.36.0/magpie/api/exception.py#L48
docker does not restrict container names (it allows . in container names)
jupyterhub escapes everything except lowercase ascii and digits
- https://github.com/jupyterhub/dockerspawner/blob/12.1.0/dockerspawner/dockerspawner.py#L625

When spawning a new jupyterlab container, it fails with the following traceback:

Traceback (most recent call last):
  File "/opt/conda/envs/birdy/bin/jupyterhub-singleuser", line 10, in <module>
    sys.exit(main())
  File "/opt/conda/envs/birdy/lib/python3.9/site-packages/jupyterhub/singleuser/app.py", line 68, in main
    return SingleUserNotebookApp.launch_instance()
  File "/opt/conda/envs/birdy/lib/python3.9/site-packages/jupyter_core/application.py", line 277, in launch_instance
    return super().launch_instance(argv=argv, **kwargs)
  File "/opt/conda/envs/birdy/lib/python3.9/site-packages/traitlets/config/application.py", line 1040, in launch_instance
    app.initialize(argv)
  File "/opt/conda/envs/birdy/lib/python3.9/site-packages/jupyterhub/singleuser/mixins.py", line 971, in initialize
    result = super().initialize(*args, **kwargs)
  File "/opt/conda/envs/birdy/lib/python3.9/site-packages/jupyterhub/singleuser/mixins.py", line 642, in initialize
    super().initialize(argv)
  File "/opt/conda/envs/birdy/lib/python3.9/site-packages/traitlets/config/application.py", line 113, in inner
    return method(app, *args, **kwargs)
  File "/opt/conda/envs/birdy/lib/python3.9/site-packages/jupyter_server/serverapp.py", line 2461, in initialize
    self.init_configurables()
  File "/opt/conda/envs/birdy/lib/python3.9/site-packages/jupyter_server/serverapp.py", line 1864, in init_configurables
    connection_dir=self.runtime_dir,
  File "/opt/conda/envs/birdy/lib/python3.9/site-packages/traitlets/traitlets.py", line 703, in __get__
    return self.get(obj, cls)
  File "/opt/conda/envs/birdy/lib/python3.9/site-packages/traitlets/traitlets.py", line 659, in get
    default = obj.trait_defaults(self.name)
  File "/opt/conda/envs/birdy/lib/python3.9/site-packages/traitlets/traitlets.py", line 1871, in trait_defaults
    return self._get_trait_default_generator(names[0])(self)
  File "/opt/conda/envs/birdy/lib/python3.9/site-packages/jupyter_core/application.py", line 107, in _runtime_dir_default
    ensure_dir_exists(rd, mode=0o700)
  File "/opt/conda/envs/birdy/lib/python3.9/site-packages/jupyter_core/utils/__init__.py", line 25, in ensure_dir_exists
    os.makedirs(path, mode=mode)
  File "/opt/conda/envs/birdy/lib/python3.9/os.py", line 215, in makedirs
    makedirs(head, exist_ok=exist_ok)
  File "/opt/conda/envs/birdy/lib/python3.9/os.py", line 215, in makedirs
    makedirs(head, exist_ok=exist_ok)
  File "/opt/conda/envs/birdy/lib/python3.9/os.py", line 215, in makedirs
    makedirs(head, exist_ok=exist_ok)
  [Previous line repeated 1 more time]
  File "/opt/conda/envs/birdy/lib/python3.9/os.py", line 225, in makedirs
    mkdir(name, mode)
PermissionError: [Errno 13] Permission denied: '/notebook_dir/writable-workspace/.home'

I'm not entirely sure why the issue is a permission denied error but the cause is obviously due to an unexpected mismatch between the expected username and the username according to the container.

One possible solution would be to further restrict the valid usernames in magpie, either permanently or only when jupyterhub is enabled.

To Reproduce

start birdhouse deploy with the magpie and jupyterhub components enabled
create a user with a . in their name in magpie (eg: user.name)
log in to jupyterhub as that user and spawn a new jupyterlab server
inspect the jupyterhub logs for the error message: docker logs -f jupyter-user.name

Environment

Information	Value
Server/Platform URL	daccs.cs.toronto.edu
Version Tag/Commit	1.35.0
Related issues/PR
Related components	jupyterhub, magpie
Custom configuration
docker version	Docker version 24.0.2, build cb74dfc

Concerned Organizations

The text was updated successfully, but these errors were encountered:

tlvu · 2023-10-18T03:08:12Z

Dupe of Ouranosinc/Magpie#497. I have had this problem before so the current work-around is to manually remember to only use alphanumeric char in a username.

Being able to supply a username validation regex to Magpie would enforce this.

mishaschwartz · 2023-10-18T12:44:05Z

Dupe of Ouranosinc/Magpie#497. I have had this problem before so the current work-around is to manually remember to only use alphanumeric char in a username.

Cool! Thanks I must have missed that one.
I don't think that manually remembering to only use alphanum characters is a sustainable solution, especially once other organizations start to deploy the stack.

Being able to supply a username validation regex to Magpie would enforce this.

This is a good idea. I can work on this unless someone at CRIM has the capacity right now?

mishaschwartz · 2023-10-18T12:44:59Z

I'm going to suggest we don't close this (even though its a duplicate of the Magpie issue) since the solution will end up being a change to Magpie as well as a configuration change to the ini files in this repository as well.

fmigneault · 2023-10-18T17:14:15Z

I think the safest approach would be to use the user_id rather than user_name. This way, we never have to worry about either of Jupyter's or Magpie's allowed characters.

mishaschwartz · 2023-10-18T17:23:31Z

@fmigneault

Can you please clarify what you mean.

Where are we using user_id instead of user_name (which component)?
What configuration files/variables do we need to change to do that?

tlvu · 2023-10-18T18:19:00Z

Where are we using user_id instead of user_name (which component)?
What configuration files/variables do we need to change to do that?
@mishaschwartz

I think @fmigneault means using the user_id in the container name (jupyter-123 instead of jupyter-NAMEHERE) and the folder on disk (/data/jupyterhub_user_data/123 instead of /data/jupyter_user_data/NAMEHERE).

For me this is very user unfriendly and error prone. Often we have to chase down which user is using too much resources and currently it's very easy, the user name is directly in the container name.

Also we have to keep an eye on the disk space usage for each user so again having the user name directly in the folder path make this much easier, since we have a lot of users.

When deleting the user, it's also easy to find and wipe the corresponding persisted data on disk since the user name is in the path name to wipe. With user id, there is a risk we could wipe the wrong data on disk.

mishaschwartz · 2023-10-18T19:31:57Z

Thanks @tlvu that makes sense.

I've added a proposed change to Magpie here (Ouranosinc/Magpie#592) that should allow us to optionally restrict the user names.

If that looks good and gets pulled in, I can make a PR here to update the magpie.ini file.

fmigneault · 2023-10-19T14:31:58Z

For me this is very user unfriendly and error prone. Often we have to chase down which user is using too much resources and currently it's very easy, the user name is directly in the container name.

Also we have to keep an eye on the disk space usage for each user so again having the user name directly in the folder path make this much easier, since we have a lot of users.

I got an idea regarding that. Would it be possible to have system link in the form
/data/jupyterhub_user_data/{user_id} -> /data/jupyterhub_user_data/{user_name}?

This way, a custom renaming/regex logic could be applied to obtain redirects for convenience, but real references would use the generic user IDs? You would have a extra step to check those links to figure out the mapping, but it would still be available somewhere.

The issue I forsee with custom regexes such as in Ouranosinc/Magpie#592 is that the similar logic will have to be replicated everywhere that needs to resolve the directories. This means, all Weaver WPS output hooks, all Cowbird handlers, and so on. I think it would be wiser to abstract away that logic using integers.

I think Ouranosinc/Magpie#592 can still be valid on its own. As an admin, you could ask to restrict certain user names, or even enforce some name conventions. However, I am not in favor of using this as a side-effect that assumes it would resolve all issues elsewhere. It feels more like a hack than a solution.

mishaschwartz · 2023-10-19T14:56:13Z

The issue I forsee with custom regexes such as in Ouranosinc/Magpie#592 is that the similar logic will have to be replicated everywhere that needs to resolve the directories. This means, all Weaver WPS output hooks, all Cowbird handlers, and so on. I think it would be wiser to abstract away that logic using integers.

I'm not sure I understand this.

Right now, weaver WPS output hooks, cowbird handlers, etc. all use the username that is defined in Magpie to create directories. All we're doing is restricting the naming convention for those usernames. I don't understand how this affects weaver or cowbird.

fmigneault · 2023-10-19T15:11:36Z

I guess you're right. I was overthinking it. Usernames would be correct from upstream validation in Magpie.
Just need to add a note in the changes that manual retrofit of all existing user names that do not conform to new naming convention will have to be done.

tlvu · 2023-10-20T15:49:27Z

I got an idea regarding that. Would it be possible to have system link in the form
/data/jupyterhub_user_data/{user_id} -> /data/jupyterhub_user_data/{user_name}?

This means at deletion time, we will still need to match the user_id to user_name to delete both of them, still error-prone to delete the wrong not-matching pair.

Also the user_name is created in Magpie. So if the restriction is done at Magpie level, everything else that use the user_name will be automatically in sync. There are no logic about user_name validation to replicate.

add a note in the changes that manual retrofit of all existing user names that do not conform to new naming convention

I understand that the new optional validation regex only take effect for new user creation. I won't affect any existing users. So what "manual retrofit" are you referring to? Do you mean a DB migration of existing users? That should not happen because we will be changing their login-id and they won't be able to login anymore to Jupyter.

In fact, if they are already able to login to Jupyter, that means their naming convention is already matching the good one. Going forward we are simply able to properly enforce this naming convention.

fmigneault · 2023-10-20T17:09:32Z

Do you mean a DB migration of existing users?

Exactly.
If the new regex is applied, and the function used to validate user properties is called, previously valid usernames would now cause API/UI errors.
If their usernames were already patched to make them work Jupyter, then there shouldn't be any issue.

mishaschwartz · 2023-10-20T17:38:51Z

If the new regex is applied, and the function used to validate user properties is called, previously valid usernames would now cause API/UI errors.

The change was made to the check_user_info function which is only called in Magpie when a user is being created or updated. Magpie will still work as before for old users with invalid usernames, they just won't be able to log in to jupyterhub (but they couldn't do that before).

I don't think there will be any old users with invalid names to migrate. If there were old users with usernames that broke jupyterhub you would know already because they would be complaining that they can't use jupyterhub.

tlvu · 2023-10-20T21:49:49Z

For legacy reasons, we have Magpie users before the time of Jupyter so these users do not use Jupyter so no warranty that their usernames conform to the naming convention.

Is there a way to make this additional regex check apply only for new user creation but not when updating existing user (ex: reset their password because too long ago they forgot it or updating their email). Like the admin password 12 char length enforcement, it only trigger if we actually set a new password. If we set again but to the same existing password, the enforcement is bypassed.

Else can you document the DB migration steps we can run manually only for a very small list of problematic users?

fmigneault · 2023-10-20T23:59:58Z

Is there a way to make this additional regex check apply only for new user creation but not when updating existing user

It is the same function that performs the check in both cases.
https://github.com/Ouranosinc/Magpie/blob/92ff2d2fa1e62b69a3a5edf22ef65d61238eaba9/magpie/api/management/user/user_utils.py#L124
https://github.com/Ouranosinc/Magpie/blob/92ff2d2fa1e62b69a3a5edf22ef65d61238eaba9/magpie/api/management/user/user_utils.py#L241

However, unless the user name is updated, the email/password should be checked on their own without revalidation of the user name.

tlvu · 2023-10-23T01:38:37Z

However, unless the user name is updated, the email/password should be checked on their own without revalidation of the user name.

Super, so no need to DB migrate the existing users with "bad" username format.

Users sometime forget their password so we simply set a new one. We never had to change a username before.

mishaschwartz · 2023-10-25T14:39:39Z

@fmigneault Thanks for merging in Ouranosinc/Magpie#592

Unfortunately, while testing the magpie/jupyterhub interface further it looks like the fact that jupyterhub also converts all uppercase characters to lowercase is a problem.

Magpie currently only does case insensitive regex comparisons so there's no way to restrict usernames to lowercase only. In order to allow us to enforce this, I've added another PR to Magpie here (Ouranosinc/Magpie#594).
Please have a look when you have a minute

## Overview When Jupyterhub spawns a new jupyterlab container, it escapes any non-ascii, non-digit character in the username. This results in a username that may not match the expected username (as defined by Magpie). This mismatch results in the container failing to spawn since expected volumes cannot be mounter to the jupyterlab container. This fixes the issue by ensuring that juptyerhub does not convert the username that is receives from Magpie. ## Changes **Non-breaking changes** - limits usernames in Magpie **Breaking changes** - If any users currently exist with invalid usernames, an admin needs to update their user name in Magpie ## Related Issue / Discussion - Resolves #393 ## Additional Information  birdhouse_daccs_configs_branch: master birdhouse_skip_ci: false

mishaschwartz added the bug Something isn't working label Oct 17, 2023

mishaschwartz assigned tlvu and fmigneault Oct 17, 2023

tlvu mentioned this issue Oct 18, 2023

[Feature] How to specify whitelist of allowed character in username Ouranosinc/Magpie#497

Closed

mishaschwartz mentioned this issue Oct 25, 2023

limit Magpie usernames #396

Merged

mishaschwartz closed this as completed in #396 Nov 29, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🐛 [BUG]: magpie allows special characters in usernames that mess up jupyterhub #393

🐛 [BUG]: magpie allows special characters in usernames that mess up jupyterhub #393

mishaschwartz commented Oct 17, 2023 •

edited

Loading

tlvu commented Oct 18, 2023

mishaschwartz commented Oct 18, 2023

mishaschwartz commented Oct 18, 2023

fmigneault commented Oct 18, 2023

mishaschwartz commented Oct 18, 2023

tlvu commented Oct 18, 2023

mishaschwartz commented Oct 18, 2023

fmigneault commented Oct 19, 2023 •

edited

Loading

mishaschwartz commented Oct 19, 2023

fmigneault commented Oct 19, 2023

tlvu commented Oct 20, 2023

fmigneault commented Oct 20, 2023

mishaschwartz commented Oct 20, 2023

tlvu commented Oct 20, 2023

fmigneault commented Oct 20, 2023

tlvu commented Oct 23, 2023

mishaschwartz commented Oct 25, 2023

🐛 [BUG]: magpie allows special characters in usernames that mess up jupyterhub #393

🐛 [BUG]: magpie allows special characters in usernames that mess up jupyterhub #393

Comments

mishaschwartz commented Oct 17, 2023 • edited Loading

Summary

Details

To Reproduce

Environment

Concerned Organizations

tlvu commented Oct 18, 2023

mishaschwartz commented Oct 18, 2023

mishaschwartz commented Oct 18, 2023

fmigneault commented Oct 18, 2023

mishaschwartz commented Oct 18, 2023

tlvu commented Oct 18, 2023

mishaschwartz commented Oct 18, 2023

fmigneault commented Oct 19, 2023 • edited Loading

mishaschwartz commented Oct 19, 2023

fmigneault commented Oct 19, 2023

tlvu commented Oct 20, 2023

fmigneault commented Oct 20, 2023

mishaschwartz commented Oct 20, 2023

tlvu commented Oct 20, 2023

fmigneault commented Oct 20, 2023

tlvu commented Oct 23, 2023

mishaschwartz commented Oct 25, 2023

mishaschwartz commented Oct 17, 2023 •

edited

Loading

fmigneault commented Oct 19, 2023 •

edited

Loading