Repository: jupyterhub/jupyterhub

Issue:
Services are moved to a subdomain if subdomain_host is set
### Bug description

Specifying `c.JupyterHub.subdomain_host` moves the services URL to `//services.<subdomain_host>/services/`. This was a bit tricky to figure out, and I in the end only discovered it by querying the proxy via its API.

#### Expected behaviour

- Either services should stay at the main hub domain
- Or the behavior should be documented when describing the service behavior and additionally the admin panel should take this into account when computing service URLs (now it does not).

### How to reproduce

1. Setup a whoami service
2. Configure the jupyterhub to use a subdomain_host
3. Check proxy paths

### Your personal set up

Jupyterhub 3.1.0

Discussion:

Patch:
diff --git a/jupyterhub/app.py b/jupyterhub/app.py
index 430564bf..4c59e6c0 100644
--- a/jupyterhub/app.py
+++ b/jupyterhub/app.py
@@ -93,6 +93,8 @@ from .utils import (
     maybe_future,
     print_ps_info,
     print_stacks,
+    subdomain_hook_idna,
+    subdomain_hook_legacy,
     url_path_join,
 )
 
@@ -739,6 +741,72 @@ class JupyterHub(Application):
             return ''
         return urlparse(self.subdomain_host).hostname
 
+    subdomain_hook = Union(
+        [Callable(), Unicode()],
+        default_value="idna",
+        config=True,
+        help="""
+        Hook for constructing subdomains for users and services.
+        Only used when `JupyterHub.subdomain_host` is set.
+
+        There are two predefined hooks, which can be selected by name:
+
+        - 'legacy' (deprecated)
+        - 'idna' (default, more robust. No change for _most_ usernames)
+
+        Otherwise, should be a function which must not be async.
+        A custom subdomain_hook should have the signature:
+
+        def subdomain_hook(name, domain, kind) -> str:
+            ...
+
+        and should return a unique, valid domain name for all usernames.
+
+        - `name` is the original name, which may need escaping to be safe as a domain name label
+        - `domain` is the domain of the Hub itself
+        - `kind` will be one of 'user' or 'service'
+
+        JupyterHub itself puts very little limit on usernames
+        to accommodate a wide variety of Authenticators,
+        but your identity provider is likely much more strict,
+        allowing you to make assumptions about the name.
+
+        The default behavior is to have all services
+        on a single `services.{domain}` subdomain,
+        and each user on `{username}.{domain}`.
+        This is the 'legacy' scheme,
+        and doesn't work for all usernames.
+
+        The 'idna' scheme is a new scheme that should produce a valid domain name for any user,
+        using IDNA encoding for unicode usernames, and a truncate-and-hash approach for
+        any usernames that can't be easily encoded into a domain component.
+
+        .. versionadded:: 5.0
+        """,
+    )
+
+    @default("subdomain_hook")
+    def _default_subdomain_hook(self):
+        return subdomain_hook_idna
+
+    @validate("subdomain_hook")
+    def _subdomain_hook(self, proposal):
+        # shortcut `subdomain_hook = "idna"` config
+        hook = proposal.value
+        if hook == "idna":
+            return subdomain_hook_idna
+        if hook == "legacy":
+            if self.subdomain_host:
+                self.log.warning(
+                    "Using deprecated 'legacy' subdomain hook. JupyterHub.subdomain_hook = 'idna' is the new default, added in JupyterHub 5."
+                )
+            return subdomain_hook_legacy
+        if not callable(hook):
+            raise ValueError(
+                f"subdomain_hook must be 'idna', 'legacy', or a callable, got {hook!r}"
+            )
+        return hook
+
     logo_file = Unicode(
         '',
         help="Specify path to a logo image to override the Jupyter logo in the banner.",
@@ -2401,14 +2469,16 @@ class JupyterHub(Application):
             Service: the created service
         """
 
+        name = orm_service.name
         if self.domain:
-            domain = 'services.' + self.domain
-            parsed = urlparse(self.subdomain_host)
-            host = f'{parsed.scheme}://services.{parsed.netloc}'
+            parsed_host = urlparse(self.subdomain_host)
+            domain = self.subdomain_hook(name, self.domain, kind="service")
+            host = f"{parsed_host.scheme}://{domain}"
+            if parsed_host.port:
+                host = f"{host}:{parsed_host.port}"
         else:
             domain = host = ''
 
-        name = orm_service.name
         service = Service(
             parent=self,
             app=self,
@@ -2454,17 +2524,20 @@ class JupyterHub(Application):
             Optional[Service]: The created service
         """
 
+        if 'name' not in spec:
+            raise ValueError(f'service spec must have a name: {spec}')
+
+        name = spec['name']
+
         if self.domain:
-            domain = 'services.' + self.domain
-            parsed = urlparse(self.subdomain_host)
-            host = f'{parsed.scheme}://services.{parsed.netloc}'
+            parsed_host = urlparse(self.subdomain_host)
+            domain = self.subdomain_hook(name, self.domain, kind="service")
+            host = f"{parsed_host.scheme}://{domain}"
+            if parsed_host.port:
+                host = f"{host}:{parsed_host.port}"
         else:
             domain = host = ''
 
-        if 'name' not in spec:
-            raise ValueError('service spec must have a name: %r' % spec)
-
-        name = spec['name']
         # get/create orm
         orm_service = orm.Service.find(self.db, name=name)
         if orm_service is None:
@@ -2895,6 +2968,7 @@ class JupyterHub(Application):
             static_path=os.path.join(self.data_files_path, 'static'),
             static_url_prefix=url_path_join(self.hub.base_url, 'static/'),
             static_handler_class=CacheControlStaticFilesHandler,
+            subdomain_hook=self.subdomain_hook,
             template_path=self.template_paths,
             template_vars=self.template_vars,
             jinja2_env=jinja_env,
diff --git a/jupyterhub/services/service.py b/jupyterhub/services/service.py
index 24c3f37c..77932e90 100644
--- a/jupyterhub/services/service.py
+++ b/jupyterhub/services/service.py
@@ -365,6 +365,14 @@ class Service(LoggingConfigurable):
     def prefix(self):
         return url_path_join(self.base_url, 'services', self.name + '/')
 
+    @property
+    def href(self):
+        """Convenient 'href' to use for links to this service"""
+        if self.domain:
+            return f"//{self.domain}{self.prefix}"
+        else:
+            return self.prefix
+
     @property
     def proxy_spec(self):
         if not self.server:
diff --git a/jupyterhub/user.py b/jupyterhub/user.py
index 101ee4f5..fe10b7bc 100644
--- a/jupyterhub/user.py
+++ b/jupyterhub/user.py
@@ -1,11 +1,9 @@
 # Copyright (c) Jupyter Development Team.
 # Distributed under the terms of the Modified BSD License.
 import json
-import string
 import warnings
 from collections import defaultdict
 from datetime import datetime, timedelta
-from functools import lru_cache
 from urllib.parse import quote, urlparse
 
 from sqlalchemy import inspect
@@ -21,8 +19,10 @@ from .objects import Server
 from .spawner import LocalProcessSpawner
 from .utils import (
     AnyTimeoutError,
+    _strict_dns_safe,
     make_ssl_context,
     maybe_future,
+    subdomain_hook_legacy,
     url_escape_path,
     url_path_join,
 )
@@ -55,42 +55,6 @@ Common causes of this timeout, and debugging tips:
    to a number of seconds that is enough for servers to become responsive.
 """
 
-# set of chars that are safe in dns labels
-# (allow '.' because we don't mind multiple levels of subdomains)
-_dns_safe = set(string.ascii_letters + string.digits + '-.')
-# don't escape % because it's the escape char and we handle it separately
-_dns_needs_replace = _dns_safe | {"%"}
-
-
-@lru_cache()
-def _dns_quote(name):
-    """Escape a name for use in a dns label
-
-    this is _NOT_ fully domain-safe, but works often enough for realistic usernames.
-    Fully safe would be full IDNA encoding,
-    PLUS escaping non-IDNA-legal ascii,
-    PLUS some encoding of boundary conditions
-    """
-    # escape name for subdomain label
-    label = quote(name, safe="").lower()
-    # some characters are not handled by quote,
-    # because they are legal in URLs but not domains,
-    # specifically _ and ~ (starting in 3.7).
-    # Escape these in the same way (%{hex_codepoint}).
-    unique_chars = set(label)
-    for c in unique_chars:
-        if c not in _dns_needs_replace:
-            label = label.replace(c, f"%{ord(c):x}")
-
-    # underscore is our escape char -
-    # it's not officially legal in hostnames,
-    # but is valid in _domain_ names (?),
-    # and always works in practice.
-    # FIXME: We should consider switching to proper IDNA encoding
-    # for 3.0.
-    label = label.replace("%", "_")
-    return label
-
 
 class UserDict(dict):
     """Like defaultdict, but for users
@@ -559,8 +523,19 @@ class User:
     @property
     def domain(self):
         """Get the domain for my server."""
+        hook = self.settings.get("subdomain_hook", subdomain_hook_legacy)
+        return hook(self.name, self.settings['domain'], kind='user')
 
-        return _dns_quote(self.name) + '.' + self.settings['domain']
+    @property
+    def dns_safe_name(self):
+        """Get a dns-safe encoding of my name
+
+        - always safe value for a single DNS label
+        - max 40 characters, leaving room for additional components
+
+        .. versionadded:: 5.0
+        """
+        return _strict_dns_safe(self.name, max_length=40)
 
     @property
     def host(self):
diff --git a/jupyterhub/utils.py b/jupyterhub/utils.py
index 4c19cf23..4db7ffe0 100644
--- a/jupyterhub/utils.py
+++ b/jupyterhub/utils.py
@@ -8,19 +8,23 @@ import functools
 import hashlib
 import inspect
 import random
+import re
 import secrets
 import socket
 import ssl
+import string
 import sys
 import threading
 import uuid
 import warnings
 from binascii import b2a_hex
 from datetime import datetime, timezone
+from functools import lru_cache
 from hmac import compare_digest
 from operator import itemgetter
 from urllib.parse import quote
 
+import idna
 from async_generator import aclosing
 from sqlalchemy.exc import SQLAlchemyError
 from tornado import gen, ioloop, web
@@ -779,3 +783,153 @@ def get_browser_protocol(request):
 
     # no forwarded headers
     return request.protocol
+
+
+# set of chars that are safe in dns labels
+# (allow '.' because we don't mind multiple levels of subdomains)
+_dns_safe = set(string.ascii_letters + string.digits + '-.')
+# don't escape % because it's the escape char and we handle it separately
+_dns_needs_replace = _dns_safe | {"%"}
+
+
+@lru_cache()
+def _dns_quote(name):
+    """Escape a name for use in a dns label
+
+    this is _NOT_ fully domain-safe, but works often enough for realistic usernames.
+    Fully safe would be full IDNA encoding,
+    PLUS escaping non-IDNA-legal ascii,
+    PLUS some encoding of boundary conditions
+    """
+    # escape name for subdomain label
+    label = quote(name, safe="").lower()
+    # some characters are not handled by quote,
+    # because they are legal in URLs but not domains,
+    # specifically _ and ~ (starting in 3.7).
+    # Escape these in the same way (%{hex_codepoint}).
+    unique_chars = set(label)
+    for c in unique_chars:
+        if c not in _dns_needs_replace:
+            label = label.replace(c, f"%{ord(c):x}")
+
+    # underscore is our escape char -
+    # it's not officially legal in hostnames,
+    # but is valid in _domain_ names (?),
+    # and seems to always work in practice.
+    label = label.replace("%", "_")
+    return label
+
+
+def subdomain_hook_legacy(name, domain, kind):
+    """Legacy (default) hook for subdomains
+
+    Users are at '$user.$host' where $user is _mostly_ DNS-safe.
+    Services are all simultaneously on 'services.$host`.
+    """
+    if kind == "user":
+        # backward-compatibility
+        return f"{_dns_quote(name)}.{domain}"
+    elif kind == "service":
+        return f"services.{domain}"
+    else:
+        raise ValueError(f"kind must be 'service' or 'user', not {kind!r}")
+
+
+# strict dns-safe characters (excludes '-')
+_strict_dns_safe = set(string.ascii_lowercase) | set(string.digits)
+
+
+def _trim_and_hash(name):
+    """Always-safe fallback for a DNS label
+
+    Produces a valid and unique DNS label for any string
+
+    - prefix with 'u-' to avoid collisions and first-character rules
+    - Selects the first N characters that are safe ('x' if none are safe)
+    - suffix with truncated hash of true name
+    - length is guaranteed to be < 32 characters
+      leaving room for additional components to build a DNS label.
+      Will currently be between 12-19 characters:
+        4 (prefix, delimiters) + 7 (hash) + 1-8 (name stub)
+    """
+    name_hash = hashlib.sha256(name.encode('utf8')).hexdigest()[:7]
+
+    safe_chars = [c for c in name.lower() if c in _strict_dns_safe]
+    name_stub = ''.join(safe_chars[:8])
+    # We MUST NOT put the `--` in the 3rd and 4th position (RFC 5891)
+    # which is reserved for IDNs
+    # It would be if name_stub were empty, so put 'x' here
+    # (value doesn't matter, as uniqueness is in the hash - the stub is more of a hint, anyway)
+    if not name_stub:
+        name_stub = "x"
+    return f"u-{name_stub}--{name_hash}"
+
+
+# A host name (label) can start or end with a letter or a number
+# this pattern doesn't need to handle the boundary conditions,
+# which are handled more simply with starts/endswith
+_dns_re = re.compile(r'^[a-z0-9-]{1,63}$', flags=re.IGNORECASE)
+
+
+def _is_dns_safe(label, max_length=63):
+    # A host name (label) MUST NOT consist of all numeric values
+    if label.isnumeric():
+        return False
+    # A host name (label) can be up to 63 characters
+    if not 0 < len(label) <= max_length:
+        return False
+    # A host name (label) MUST NOT start or end with a '-' (dash)
+    if label.startswith('-') or label.endswith('-'):
+        return False
+    return bool(_dns_re.match(label))
+
+
+def _strict_dns_safe_encode(name, max_length=63):
+    """Will encode a username to a guaranteed-safe DNS label
+
+    - if it contains '--' at all, jump to the end and take the hash route to avoid collisions with escaped
+    - if safe, use it
+    - if not, use IDNA encoding
+    - if a safe encoding cannot be produced, use stripped safe characters + '--{hash}`
+    - allow specifying a max_length, to give room for additional components,
+      if used as only a _part_ of a DNS label.
+    """
+    # short-circuit: avoid accepting already-encoded results
+    # which all include '--'
+    if '--' in name:
+        return _trim_and_hash(name)
+
+    # if name is already safe (and can't collide with an escaped result) use it
+    if _is_dns_safe(name, max_length=max_length):
+        return name
+
+    # next: use IDNA encoding, if applicable
+    try:
+        idna_name = idna.encode(name).decode("ascii")
+    except ValueError:
+        idna_name = None
+
+    if idna_name and idna_name != name and _is_dns_safe(idna_name):
+        return idna_name
+
+    # fallback, always works: trim to safe characters and hash
+    return _trim_and_hash(name)
+
+
+def subdomain_hook_idna(name, domain, kind):
+    """New, reliable subdomain hook
+
+    More reliable than previous, should always produce valid domains
+
+    - uses IDNA encoding for simple unicode names
+    - separate domain for each service
+    - uses stripped name and hash, where above schemes fail to produce a valid domain
+    """
+    safe_name = _strict_dns_safe_encode(name)
+    if kind == 'user':
+        # 'user' namespace is special-cased as the default
+        # for aesthetics and backward-compatibility for names that don't need escaping
+        suffix = ""
+    else:
+        suffix = f"--{kind}"
+    return f"{safe_name}{suffix}.{domain}"
diff --git a/requirements.txt b/requirements.txt
index 84bda83e..0d07a703 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,6 +1,7 @@
 alembic>=1.4
 async_generator>=1.9
 certipy>=0.1.2
+idna
 importlib_metadata>=3.6; python_version < '3.10'
 jinja2>=2.11.0
 jupyter_telemetry>=0.1.0
diff --git a/share/jupyterhub/templates/page.html b/share/jupyterhub/templates/page.html
index a4247b94..e59b2d31 100644
--- a/share/jupyterhub/templates/page.html
+++ b/share/jupyterhub/templates/page.html
@@ -132,7 +132,7 @@
               <ul class="dropdown-menu">
               {% for service in services %}
               {% block service scoped %}
-                <li><a class="dropdown-item" href="{{service.prefix}}">{{service.name}}</a></li>
+                <li><a class="dropdown-item" href="{{service.href}}">{{service.name}}</a></li>
               {% endblock %}
               {% endfor %}
               </ul>


Test patch:
diff --git a/jupyterhub/tests/test_utils.py b/jupyterhub/tests/test_utils.py
index 129bc0fa..d13564c3 100644
--- a/jupyterhub/tests/test_utils.py
+++ b/jupyterhub/tests/test_utils.py
@@ -122,3 +122,46 @@ def test_browser_protocol(x_scheme, x_forwarded_proto, forwarded, expected):
 
     proto = utils.get_browser_protocol(request)
     assert proto == expected
+
+
+@pytest.mark.parametrize(
+    "name, expected",
+    [
+        ("safe", "safe"),
+        ("has--doubledash", "u-hasdoubl--cb052ae"),
+        ("uhasdoubl--cb052ae", "u-uhasdoub--3c0d1c9"),
+        ("üni", "xn--ni-wka"),
+        ("xn--ni-wka", "u-xnniwka--ceb4edd"),
+        ("x", "x"),
+        ("-pre", "u-pre--0e46e7b"),
+        ("É", "u-x--a755f65"),
+        ("é", "xn--9ca"),
+        ("a" * 64, "u-aaaaaaaa--ffe054f"),
+        ("a.b", "u-ab--2e7336d"),
+    ],
+)
+def test_subdomain_hook_idna(name, expected):
+    expected_domain = expected + ".domain"
+    resolved = utils.subdomain_hook_idna(name, "domain", "user")
+    assert resolved == expected_domain
+
+
+@pytest.mark.parametrize(
+    "name, expected",
+    [
+        ("safe", "safe"),
+        ("üni", "_c3_bcni"),
+        ("x", "x"),
+        ("É", "_c3_89"),
+        ("é", "_c3_a9"),
+        # bad cases:
+        ("a.b", "a.b"),
+        ("has--doubledash", "has--doubledash"),
+        ("-pre", "-pre"),
+        ("a" * 64, "a" * 64),
+    ],
+)
+def test_subdomain_hook_legacy(name, expected):
+    expected_domain = expected + ".domain"
+    resolved = utils.subdomain_hook_legacy(name, "domain", "user")
+    assert resolved == expected_domain