From 90d7044f184043977f3c425fffe4f1d313831be7 Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Tue, 13 Feb 2024 12:38:34 +0000 Subject: [PATCH] Improve and test tornado.web.authenticated heuristic --- jupyter_server/serverapp.py | 22 ++++++++++++++++++++-- tests/test_serverapp.py | 22 ++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/jupyter_server/serverapp.py b/jupyter_server/serverapp.py index 99d3d558b8..0cdd401296 100644 --- a/jupyter_server/serverapp.py +++ b/jupyter_server/serverapp.py @@ -538,8 +538,7 @@ def _check_handler_auth( method = getattr(handler, method_name.lower()) is_unimplemented = method == web.RequestHandler._unimplemented_method is_allowlisted = hasattr(method, "__allow_unauthenticated") - # TODO: modify `tornado.web.authenticated` upstream? - is_blocklisted = method.__code__.co_qualname.startswith("authenticated") + is_blocklisted = _has_tornado_web_authenticated(method) if not is_unimplemented and not is_allowlisted and not is_blocklisted: missing_authentication.append( f"- {method_name} of {handler.__name__} registered for {matcher}" @@ -547,6 +546,25 @@ def _check_handler_auth( return missing_authentication +def _has_tornado_web_authenticated(method: t.Callable[..., t.Any]) -> bool: + """Check if given method was decorated with @web.authenticated. + + Note: it is ok if we reject on @authorized @web.authenticated + because the correct order is @web.authenticated @authorized. + """ + if not hasattr(method, "__wrapped__"): + return False + if not hasattr(method, "__code__"): + return False + code = method.__code__ + if hasattr(code, "co_qualname"): + # new in 3.11 + return code.co_qualname.startswith("authenticated") # type:ignore[no-any-return] + elif hasattr(code, "co_filename"): + return code.co_filename.replace("\\", "/").endswith("tornado/web.py") + return False + + class JupyterPasswordApp(JupyterApp): """Set a password for the Jupyter server. diff --git a/tests/test_serverapp.py b/tests/test_serverapp.py index eade03a24c..e5fe9ddae8 100644 --- a/tests/test_serverapp.py +++ b/tests/test_serverapp.py @@ -9,16 +9,19 @@ import pytest from jupyter_core.application import NoStart +from tornado import web from traitlets import TraitError from traitlets.config import Config from traitlets.tests.utils import check_help_all_output +from jupyter_server.auth.decorator import allow_unauthenticated, authorized from jupyter_server.auth.security import passwd_check from jupyter_server.serverapp import ( JupyterPasswordApp, JupyterServerListApp, ServerApp, ServerWebApplication, + _has_tornado_web_authenticated, list_running_servers, random_ports, ) @@ -637,3 +640,22 @@ def test_immutable_cache_trait(): serverapp.init_configurables() serverapp.init_webapp() assert serverapp.web_app.settings["static_immutable_cache"] == ["/test/immutable"] + + +def test(): + pass + + +@pytest.mark.parametrize( + "method, expected", + [ + [test, False], + [allow_unauthenticated(test), False], + [authorized(test), False], + [web.authenticated(test), True], + [web.authenticated(authorized(test)), True], + [authorized(web.authenticated(test)), False], # wrong order! + ], +) +def test_tornado_authentication_detection(method, expected): + assert _has_tornado_web_authenticated(method) == expected