From fdfaba4166e8a1af7a98ef20b07900ba5757deb6 Mon Sep 17 00:00:00 2001 From: Min RK Date: Fri, 22 Apr 2022 14:54:01 +0200 Subject: [PATCH] allow get_user to be async careful to deprecate overridden get_current_user without ignoring auth Needs some changes due to early steps that are called before prepare, but must now be moved to prepare due to the reliance on auth info. - setting CORS headers (set_default_headers) - check_xsrf_cookie - check_origin now that get_user is async, we have to re-run these bits in prepare after user is authenticated --- jupyter_server/auth/identity.py | 2 + jupyter_server/auth/login.py | 2 +- jupyter_server/base/handlers.py | 74 ++++++++++++++++++++++++------ jupyter_server/base/zmqhandlers.py | 2 +- jupyter_server/gateway/handlers.py | 2 +- 5 files changed, 66 insertions(+), 16 deletions(-) diff --git a/jupyter_server/auth/identity.py b/jupyter_server/auth/identity.py index da8ca56685..d3cca77911 100644 --- a/jupyter_server/auth/identity.py +++ b/jupyter_server/auth/identity.py @@ -95,6 +95,8 @@ class IdentityProvider(LoggingConfigurable): """ Interface for providing identity + _may_ be a coroutine. + Two principle methods: - :meth:`~.IdentityProvider.get_user` returns a :class:`~.User` object diff --git a/jupyter_server/auth/login.py b/jupyter_server/auth/login.py index deb60b2a7f..6eb07e5748 100644 --- a/jupyter_server/auth/login.py +++ b/jupyter_server/auth/login.py @@ -152,7 +152,7 @@ def is_token_authenticated(cls, handler): """ if getattr(handler, "_user_id", None) is None: # ensure get_user has been called, so we know if we're token-authenticated - handler.get_current_user() + handler.current_user return getattr(handler, "_token_authenticated", False) @classmethod diff --git a/jupyter_server/base/handlers.py b/jupyter_server/base/handlers.py index d51e65f2c9..b3c827bc6d 100644 --- a/jupyter_server/base/handlers.py +++ b/jupyter_server/base/handlers.py @@ -3,6 +3,7 @@ # Distributed under the terms of the Modified BSD License. import datetime import functools +import inspect import ipaddress import json import mimetypes @@ -134,7 +135,21 @@ def clear_login_cookie(self): self.force_clear_cookie(self.cookie_name) def get_current_user(self): - return self.identity_provider.get_user(self) + clsname = self.__class__.__name__ + msg = ( + f"Calling `{clsname}.get_current_user()` directly is deprecated in jupyter-server 2.0." + " Use `self.current_user` instead (works in all versions)." + ) + if hasattr(self, "_jupyter_current_user"): + # backward-compat: return _jupyter_current_user + warnings.warn( + msg, + DeprecationWarning, + stacklevel=2, + ) + return self._jupyter_current_user + # haven't called get_user in prepare, raise + raise RuntimeError(msg) def skip_check_origin(self): """Ask my login_handler if I should skip the origin_check @@ -164,7 +179,7 @@ def cookie_name(self): @property def logged_in(self): """Is a user currently logged in?""" - user = self.get_current_user() + user = self.current_user return user and not user == "anonymous" @property @@ -346,6 +361,13 @@ def allow_credentials(self): def set_default_headers(self): """Add CORS headers, if defined""" super().set_default_headers() + + def set_cors_headers(self): + """Add CORS headers, if defined + + Now that current_user is async (jupyter-server 2.0), + must be called at the end of prepare(), instead of in set_default_headers. + """ if self.allow_origin: self.set_header("Access-Control-Allow-Origin", self.allow_origin) elif self.allow_origin_pat: @@ -484,6 +506,9 @@ def check_referer(self): def check_xsrf_cookie(self): """Bypass xsrf cookie checks when token-authenticated""" + if not hasattr(self, "_jupyter_current_user"): + # Called too early, will be checked later + return if self.token_authenticated or self.settings.get("disable_check_xsrf", False): # Token-authenticated requests do not need additional XSRF-check # Servers without authentication are vulnerable to XSRF @@ -543,9 +568,40 @@ def check_host(self): ) return allow - def prepare(self): + async def prepare(self): if not self.check_host(): raise web.HTTPError(403) + + from jupyter_server.auth import IdentityProvider + + if ( + type(self.identity_provider) is IdentityProvider + and inspect.getmodule(self.get_current_user).__name__ != __name__ + ): + # check for overridden get_current_user + default IdentityProvider + # deprecated way to override auth (e.g. JupyterHub < 3.0) + # allow deprecated, overridden get_current_user + warnings.warn( + "Overriding JupyterHandler.get_current_user is deprecated in jupyter-server 2.0." + " Use an IdentityProvider class.", + DeprecationWarning, + # stacklevel not useful here + ) + user = self.get_current_user() + else: + user = self.identity_provider.get_user(self) + if inspect.isawaitable(user): + # IdentityProvider.get_user _may_ be async + user = await user + + # self.current_user for tornado's @web.authenticated + # self._jupyter_current_user for backward-compat in deprecated get_current_user calls + # and our own private checks for whether .current_user has been set + self.current_user = self._jupyter_current_user = user + # complete initial steps which require auth to resolve first: + self.set_cors_headers() + if self.request.method not in {"GET", "HEAD", "OPTIONS"}: + self.check_xsrf_cookie() return super().prepare() # --------------------------------------------------------------- @@ -638,10 +694,10 @@ def write_error(self, status_code, **kwargs): class APIHandler(JupyterHandler): """Base class for API handlers""" - def prepare(self): + async def prepare(self): + await super().prepare() if not self.check_origin(): raise web.HTTPError(404) - return super().prepare() def write_error(self, status_code, **kwargs): """APIHandler errors are JSON, not human pages""" @@ -663,14 +719,6 @@ def write_error(self, status_code, **kwargs): self.log.warning(reply["message"]) self.finish(json.dumps(reply)) - def get_current_user(self): - """Raise 403 on API handlers instead of redirecting to human login page""" - # preserve _user_cache so we don't raise more than once - if hasattr(self, "_user_cache"): - return self._user_cache - self._user_cache = user = super().get_current_user() - return user - def get_login_url(self): # if get_login_url is invoked in an API handler, # that means @web.authenticated is trying to trigger a redirect. diff --git a/jupyter_server/base/zmqhandlers.py b/jupyter_server/base/zmqhandlers.py index 826c22663c..ad6342af85 100644 --- a/jupyter_server/base/zmqhandlers.py +++ b/jupyter_server/base/zmqhandlers.py @@ -313,7 +313,7 @@ def pre_get(self): the websocket finishes completing. """ # authenticate the request before opening the websocket - user = self.get_current_user() + user = self.current_user if user is None: self.log.warning("Couldn't authenticate WebSocket connection") raise web.HTTPError(403) diff --git a/jupyter_server/gateway/handlers.py b/jupyter_server/gateway/handlers.py index a36f2d4faf..e31302c464 100644 --- a/jupyter_server/gateway/handlers.py +++ b/jupyter_server/gateway/handlers.py @@ -48,7 +48,7 @@ def authenticate(self): the websocket finishes completing. """ # authenticate the request before opening the websocket - if self.get_current_user() is None: + if self.current_user is None: self.log.warning("Couldn't authenticate WebSocket connection") raise web.HTTPError(403)