consolidate cookie config on IdentityProvider

- JupyterHandler.cookie_name -> IdentityProvider.get_cookie_name(handler)
- JupyterHandler.clear_login_cookie -> IdentityProvider.clear_login_cookie
- JupyterHandler.force_clear_cookie -> IdentityProvider._force_clear_cookie
- ServerApp.cookie_options -> IdentityProvider.cookie_options
- ServerApp.get_secure_cookie_kwargs -> IdentityProvider.get_secure_cookie_kwargs
- ServerApp.tornado_settings[secure_cookie] -> IdentityProvider.secure_cookie

Author: minrk

jupyter_server/auth/identity.py

@@ -8,14 +8,16 @@
 from __future__ import annotations
 
 import binascii
+import datetime
 import os
 import re
 import sys
 import uuid
 from dataclasses import asdict, dataclass
+from http.cookies import Morsel
 from typing import TYPE_CHECKING, Any, Awaitable
 
-from tornado import web
+from tornado import escape, httputil, web
 from traitlets import Bool, Dict, Type, Unicode, default
 from traitlets.config import LoggingConfigurable
 
@@ -28,6 +30,8 @@
     from jupyter_server.base.handlers import JupyterHandler
     from jupyter_server.serverapp import ServerApp
 
+_non_alphanum = re.compile(r"[^A-Za-z0-9]")
+
 
 @dataclass
 class User:
@@ -120,7 +124,37 @@ class IdentityProvider(LoggingConfigurable):
     .. versionadded:: 2.0
     """
 
-    cookie_name = Unicode(config=True)
+    cookie_name = Unicode(
+        "",
+        config=True,
+        help=_i18n("Name of the cookie to set for persisting login. Default: username-${Host}."),
+    )
+
+    cookie_options = Dict(
+        config=True,
+        help=_i18n(
+            "Extra keyword arguments to pass to `set_secure_cookie`."
+            " See tornado's set_secure_cookie docs for details."
+        ),
+    )
+
+    secure_cookie = Bool(
+        None,
+        allow_none=True,
+        config=True,
+        help=_i18n(
+            "Specify whether login cookie should have the `secure` property (HTTPS-only)."
+            "Only needed when protocol-detection gives the wrong answer due to proxies."
+        ),
+    )
+
+    get_secure_cookie_kwargs = Dict(
+        config=True,
+        help=_i18n(
+            "Extra keyword arguments to pass to `get_secure_cookie`."
+            " See tornado's get_secure_cookie docs for details."
+        ),
+    )
 
     token = Unicode(
         "<generated>",
@@ -219,9 +253,11 @@ async def _get_user(self, handler: JupyterHandler) -> User | None:
             # If an invalid cookie was sent, clear it to prevent unnecessary
             # extra warnings. But don't do this on a request with *no* cookie,
             # because that can erroneously log you out (see gh-3365)
-            if handler.get_cookie(handler.cookie_name) is not None:
-                handler.log.warning("Clearing invalid/expired login cookie %s", handler.cookie_name)
-                handler.clear_login_cookie()
+            cookie_name = self.get_cookie_name(handler)
+            cookie = handler.get_cookie(cookie_name)
+            if cookie is not None:
+                self.log.warning(f"Clearing invalid/expired login cookie {cookie_name}")
+                self.clear_login_cookie(handler)
             if not self.auth_enabled:
                 # Completely insecure! No authentication at all.
                 # No need to warn here, though; validate_security will have already done that.
@@ -260,24 +296,84 @@ def user_from_cookie(self, cookie_value: str) -> User | None:
         """Inverse of user_to_cookie"""
         return User(username=cookie_value)
 
+    def get_cookie_name(self, handler: JupyterHandler) -> str:
+        """Return the login cookie name
+
+        Uses IdentityProvider.cookie_name, if defined.
+        Default is to generate a string taking host into account to avoid
+        collisions for multiple servers on one hostname with different ports.
+        """
+        if self.cookie_name:
+            return self.cookie_name
+        else:
+            return _non_alphanum.sub("-", f"username-{handler.request.host}")
+
     def set_login_cookie(self, handler: JupyterHandler, user: User) -> None:
         """Call this on handlers to set the login cookie for success"""
-        cookie_options = handler.settings.get("cookie_options", {})
+        cookie_options = {}
+        cookie_options.update(self.cookie_options)
         cookie_options.setdefault("httponly", True)
         # tornado <4.2 has a bug that considers secure==True as soon as
         # 'secure' kwarg is passed to set_secure_cookie
-        if handler.settings.get("secure_cookie", handler.request.protocol == "https"):
+        secure_cookie = self.secure_cookie
+        if secure_cookie is None:
+            secure_cookie = handler.request.protocol == "https"
+        if secure_cookie:
             cookie_options.setdefault("secure", True)
         cookie_options.setdefault("path", handler.base_url)
-        handler.set_secure_cookie(handler.cookie_name, self.user_to_cookie(user), **cookie_options)
+        cookie_name = self.get_cookie_name(handler)
+        handler.set_secure_cookie(cookie_name, self.user_to_cookie(user), **cookie_options)
+
+    def _force_clear_cookie(
+        self, handler: JupyterHandler, name: str, path: str = "/", domain: str | None = None
+    ) -> None:
+        """Deletes the cookie with the given name.
+
+        Tornado's cookie handling currently (Jan 2018) stores cookies in a dict
+        keyed by name, so it can only modify one cookie with a given name per
+        response. The browser can store multiple cookies with the same name
+        but different domains and/or paths. This method lets us clear multiple
+        cookies with the same name.
+
+        Due to limitations of the cookie protocol, you must pass the same
+        path and domain to clear a cookie as were used when that cookie
+        was set (but there is no way to find out on the server side
+        which values were used for a given cookie).
+        """
+        name = escape.native_str(name)
+        expires = datetime.datetime.utcnow() - datetime.timedelta(days=365)
+
+        morsel: Morsel = Morsel()
+        morsel.set(name, "", '""')
+        morsel["expires"] = httputil.format_timestamp(expires)
+        morsel["path"] = path
+        if domain:
+            morsel["domain"] = domain
+        handler.add_header("Set-Cookie", morsel.OutputString())
+
+    def clear_login_cookie(self, handler: JupyterHandler) -> None:
+        """Clear the login cookie, effectively logging out the session."""
+        cookie_options = {}
+        cookie_options.update(self.cookie_options)
+        path = cookie_options.setdefault("path", self.base_url)
+        cookie_name = self.get_cookie_name(handler)
+        handler.clear_cookie(cookie_name, path=path)
+        if path and path != "/":
+            # also clear cookie on / to ensure old cookies are cleared
+            # after the change in path behavior.
+            # N.B. This bypasses the normal cookie handling, which can't update
+            # two cookies with the same name. See the method above.
+            self._force_clear_cookie(handler, cookie_name)
 
     def get_user_cookie(self, handler: JupyterHandler) -> User | None | Awaitable[User | None]:
         """Get user from a cookie
 
         Calls user_from_cookie to deserialize cookie value
         """
-        get_secure_cookie_kwargs = handler.settings.get("get_secure_cookie_kwargs", {})
-        _user_cookie = handler.get_secure_cookie(handler.cookie_name, **get_secure_cookie_kwargs)
+        _user_cookie = handler.get_secure_cookie(
+            self.get_cookie_name(handler),
+            **self.get_secure_cookie_kwargs,
+        )
         if not _user_cookie:
             return None
         user_cookie = _user_cookie.decode()

jupyter_server/base/handlers.py

@@ -3,7 +3,6 @@
 # Distributed under the terms of the Modified BSD License.
 from __future__ import annotations
 
-import datetime
 import functools
 import inspect
 import ipaddress
@@ -15,14 +14,13 @@
 import types
 import warnings
 from http.client import responses
-from http.cookies import Morsel
 from typing import TYPE_CHECKING, Awaitable
 from urllib.parse import urlparse
 
 import prometheus_client
 from jinja2 import TemplateNotFound
 from jupyter_core.paths import is_hidden
-from tornado import escape, httputil, web
+from tornado import web
 from tornado.log import app_log
 from traitlets.config import Application
 
@@ -46,7 +44,6 @@
 # -----------------------------------------------------------------------------
 # Top-level handlers
 # -----------------------------------------------------------------------------
-non_alphanum = re.compile(r"[^A-Za-z0-9]")
 
 _sys_info_cache = None
 
@@ -104,41 +101,36 @@ def set_default_headers(self):
                 # for example, so just ignore)
                 self.log.debug(e)
 
-    def force_clear_cookie(self, name, path="/", domain=None):
-        """Deletes the cookie with the given name.
-
-        Tornado's cookie handling currently (Jan 2018) stores cookies in a dict
-        keyed by name, so it can only modify one cookie with a given name per
-        response. The browser can store multiple cookies with the same name
-        but different domains and/or paths. This method lets us clear multiple
-        cookies with the same name.
-
-        Due to limitations of the cookie protocol, you must pass the same
-        path and domain to clear a cookie as were used when that cookie
-        was set (but there is no way to find out on the server side
-        which values were used for a given cookie).
-        """
-        name = escape.native_str(name)
-        expires = datetime.datetime.utcnow() - datetime.timedelta(days=365)
+    @property
+    def cookie_name(self):
+        warnings.warn(
+            """JupyterHandler.login_handler is deprecated in 2.0,
+            use JupyterHandler.identity_provider.
+            """,
+            DeprecationWarning,
+            stacklevel=2,
+        )
+        return self.identity_provider.get_cookie_name(self)
 
-        morsel: Morsel = Morsel()
-        morsel.set(name, "", '""')
-        morsel["expires"] = httputil.format_timestamp(expires)
-        morsel["path"] = path
-        if domain:
-            morsel["domain"] = domain
-        self.add_header("Set-Cookie", morsel.OutputString())
+    def force_clear_cookie(self, name, path="/", domain=None):
+        warnings.warn(
+            """JupyterHandler.login_handler is deprecated in 2.0,
+            use JupyterHandler.identity_provider.
+            """,
+            DeprecationWarning,
+            stacklevel=2,
+        )
+        return self.identity_provider._force_clear_cookie(self, name, path=path, domain=domain)
 
     def clear_login_cookie(self):
-        cookie_options = self.settings.get("cookie_options", {})
-        path = cookie_options.setdefault("path", self.base_url)
-        self.clear_cookie(self.cookie_name, path=path)
-        if path and path != "/":
-            # also clear cookie on / to ensure old cookies are cleared
-            # after the change in path behavior.
-            # N.B. This bypasses the normal cookie handling, which can't update
-            # two cookies with the same name. See the method above.
-            self.force_clear_cookie(self.cookie_name)
+        warnings.warn(
+            """JupyterHandler.login_handler is deprecated in 2.0,
+            use JupyterHandler.identity_provider.
+            """,
+            DeprecationWarning,
+            stacklevel=2,
+        )
+        return self.identity_provider.clear_login_cookie(self)
 
     def get_current_user(self):
         clsname = self.__class__.__name__
@@ -173,11 +165,6 @@ def token_authenticated(self):
         """Have I been authenticated with a token?"""
         return self.identity_provider.is_token_authenticated(self)
 
-    @property
-    def cookie_name(self):
-        default_cookie_name = non_alphanum.sub("-", f"username-{self.request.host}")
-        return self.settings.get("cookie_name", default_cookie_name)
-
     @property
     def logged_in(self):
         """Is a user currently logged in?"""

jupyter_server/serverapp.py

@@ -1143,12 +1143,8 @@ def _warn_deprecated_config(self, change, clsname, new_name=None):
     def _deprecated_password(self, change):
         self._warn_deprecated_config(change, "PasswordIdentityProvider", new_name="hashed_password")
 
-    @observe("password_required")
-    def _deprecated_password_required(self, change):
-        self._warn_deprecated_config(change, "PasswordIdentityProvider")
-
-    @observe("allow_password_change")
-    def _deprecated_password_change(self, change):
+    @observe("password_required", "allow_password_change")
+    def _deprecated_password_config(self, change):
         self._warn_deprecated_config(change, "PasswordIdentityProvider")
 
     disable_check_xsrf = Bool(
@@ -1318,18 +1314,17 @@ def _default_allow_remote(self):
 
     cookie_options = Dict(
         config=True,
-        help=_i18n(
-            "Extra keyword arguments to pass to `set_secure_cookie`."
-            " See tornado's set_secure_cookie docs for details."
-        ),
+        help=_i18n("DEPRECATED. Use IdentityProvider.cookie_options"),
     )
     get_secure_cookie_kwargs = Dict(
         config=True,
-        help=_i18n(
-            "Extra keyword arguments to pass to `get_secure_cookie`."
-            " See tornado's get_secure_cookie docs for details."
-        ),
+        help=_i18n("DEPRECATED. Use IdentityProvider.get_secure_cookie_kwargs"),
     )
+
+    @observe("cookie_options", "get_secure_cookie_kwargs")
+    def _deprecated_cookie_config(self, change):
+        self._warn_deprecated_config(change, "IdentityProvider")
+
     ssl_options = Dict(
         allow_none=True,
         config=True,
@@ -1947,8 +1942,12 @@ def init_webapp(self):
             self.tornado_settings["allow_origin_pat"] = re.compile(self.allow_origin_pat)
         self.tornado_settings["allow_credentials"] = self.allow_credentials
         self.tornado_settings["autoreload"] = self.autoreload
-        self.tornado_settings["cookie_options"] = self.cookie_options
-        self.tornado_settings["get_secure_cookie_kwargs"] = self.get_secure_cookie_kwargs
+
+        # deprecate accessing these directly, in favor of identity_provider?
+        self.tornado_settings["cookie_options"] = self.identity_provider.cookie_options
+        self.tornado_settings[
+            "get_secure_cookie_kwargs"
+        ] = self.identity_provider.get_secure_cookie_kwargs
         self.tornado_settings["token"] = self.identity_provider.token
 
         # ensure default_url starts with base_url