Skip to content

Commit

Permalink
consolidate cookie config on IdentityProvider
Browse files Browse the repository at this point in the history 
- 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
  • Loading branch information
@minrk
minrk committed May 9, 2022
1 parent 4777871 commit c965d30
Show file tree
Hide file tree
Showing 3 changed files with 149 additions and 67 deletions.
116 changes: 106 additions & 10 deletions jupyter_server/auth/identity.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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:
Expand Down Expand Up @@ -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>",
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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()
Expand Down
69 changes: 28 additions & 41 deletions jupyter_server/base/handlers.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand All @@ -46,7 +44,6 @@
# -----------------------------------------------------------------------------
# Top-level handlers
# -----------------------------------------------------------------------------
non_alphanum = re.compile(r"[^A-Za-z0-9]")

_sys_info_cache = None

Expand Down Expand Up @@ -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__
Expand Down Expand Up @@ -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?"""
Expand Down
31 changes: 15 additions & 16 deletions jupyter_server/serverapp.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit c965d30

Please sign in to comment.