From 48c9f4a2eb4705c196f8d59f47612cf123946571 Mon Sep 17 00:00:00 2001 From: flongford Date: Tue, 13 Jul 2021 15:20:11 +0100 Subject: [PATCH 01/64] MAINT: bump jupyterhub package to 0.8.1 --- requirements.txt | 3 +-- setup.py | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/requirements.txt b/requirements.txt index 1d7b2b5f..0bcacece 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,8 +6,7 @@ requests==2.10.0 # on ubuntu uses 1.23, so they don't like each other. docker-py==1.8 escapism==0.0.1 -# Pinned to 0.8.0.dev0 due to issues with spawners (fails with 0.8.0) -git+http://github.com/jupyterhub/jupyterhub.git@2d1a45f0190059ef436c2f97dc8d6e391eb2d139#egg=jupyterhub +jupyterhub==0.8.1 jupyter_client==4.3.0 click==6.6 tabulate==0.7.5 diff --git a/setup.py b/setup.py index 91ab2672..ad2fabf5 100644 --- a/setup.py +++ b/setup.py @@ -49,7 +49,7 @@ def write_version_py(): requirements.extend(["sqlalchemy>=1.0"]) else: requirements.extend([ - "jupyterhub>0.7", + "jupyterhub>0.8", "docker-py==1.8", "tornadowebapi>=0.5.0"]) From cf35e776e3f1a9c49e4f061c61e5d3a15d3a2ce6 Mon Sep 17 00:00:00 2001 From: flongford Date: Tue, 13 Jul 2021 15:21:57 +0100 Subject: [PATCH 02/64] FIX: replace jupyterhub.orm.Proxy with jupyterhub.proxy.Proxy --- Makefile | 1 + jupyterhub/jupyterhub_config.py | 8 ++++++-- remoteappmanager/jupyterhub/spawners.py | 22 ++++++++++----------- remoteappmanager/services/reverse_proxy.py | 23 ++++++---------------- 4 files changed, 23 insertions(+), 31 deletions(-) diff --git a/Makefile b/Makefile index 561ea908..11c285ef 100644 --- a/Makefile +++ b/Makefile @@ -113,6 +113,7 @@ npmdeps: fi npm install + npm install configurable-http-proxy npm --version .PHONY: pythondeps diff --git a/jupyterhub/jupyterhub_config.py b/jupyterhub/jupyterhub_config.py index 503db0f7..18d1de7b 100644 --- a/jupyterhub/jupyterhub_config.py +++ b/jupyterhub/jupyterhub_config.py @@ -9,6 +9,10 @@ c.JupyterHub.hub_ip = public_ips()[0] if len(public_ips()) else '127.0.0.1' +c.ConfigurableHTTPProxy.command = [ + 'configurable-http-proxy', + f'--default-target=http://{c.JupyterHub.hub_ip}:8081'] + c.JupyterHub.logo_file = os.path.join( os.path.dirname(os.path.dirname(os.path.realpath(__file__))), 'remoteappmanager/static/images/header_logo.png' @@ -18,7 +22,7 @@ setting_mode = ('system_user', 'virtual_user')[1] if setting_mode == 'virtual_user': - c.JupyterHub.spawner_class = ('remoteappmanager.jupyterhub.spawners.' + + c.JupyterHub.spawner_class = ('remoteappmanager.jupyterhub.spawners.' 'VirtualUserSpawner') # Parent directory in which temporary directory is created for @@ -37,6 +41,6 @@ c.Authenticator.admin_users = {"admin"} elif setting_mode == 'system_user': - c.JupyterHub.spawner_class = ('remoteappmanager.jupyterhub.spawners.' + + c.JupyterHub.spawner_class = ('remoteappmanager.jupyterhub.spawners.' 'SystemUserSpawner') c.Authenticator.admin_users = {os.environ["USER"]} diff --git a/remoteappmanager/jupyterhub/spawners.py b/remoteappmanager/jupyterhub/spawners.py index 682b5b83..3859cd46 100644 --- a/remoteappmanager/jupyterhub/spawners.py +++ b/remoteappmanager/jupyterhub/spawners.py @@ -2,11 +2,11 @@ import escapism import string -from traitlets import Any, Unicode +from traitlets import Any, Unicode, Instance from tornado import gen from jupyterhub.spawner import LocalProcessSpawner -from jupyterhub import orm +from jupyterhub.proxy import Proxy, ConfigurableHTTPProxy class BaseSpawner(LocalProcessSpawner): @@ -14,9 +14,8 @@ class BaseSpawner(LocalProcessSpawner): the actual spawners """ - #: The instance of the orm Proxy. - #: We use Any in agreement with base class practice. - proxy = Any() + #: The instance of the proxy.Proxy. + proxy = Instance(Proxy) #: The path of the configuration file for the cmd executable config_file_path = Unicode(config=True) @@ -32,11 +31,7 @@ def cmd(self): def __init__(self, **kwargs): super().__init__(**kwargs) - - # We get the first one. Strangely enough, jupyterhub has a table - # containing potentially multiple proxies, but it's enforced to - # contain only one. - self.proxy = self.db.query(orm.Proxy).first() + self.proxy = ConfigurableHTTPProxy() def get_args(self): args = super().get_args() @@ -44,12 +39,15 @@ def get_args(self): for iarg, arg in enumerate(args): args[iarg] = arg.replace('--base-url=', '--base-urlpath=') + args.append("--cookie_name={}".format( + self.server.cookie_name)) + args.append("--proxy-api-url={}".format( - self.proxy.api_server.url)) + self.proxy.api_url)) args.append("--logout_url={}".format( self.authenticator.logout_url( - self.hub.server.base_url))) + self.hub.base_url))) if self.config_file_path: args.append("--config-file={}".format(self.config_file_path)) diff --git a/remoteappmanager/services/reverse_proxy.py b/remoteappmanager/services/reverse_proxy.py index 938f621e..002adc60 100644 --- a/remoteappmanager/services/reverse_proxy.py +++ b/remoteappmanager/services/reverse_proxy.py @@ -1,7 +1,7 @@ from urllib import parse from tornado import gen, httpclient -from jupyterhub import orm as jupyterhub_orm +from jupyterhub.proxy import Proxy, ConfigurableHTTPProxy from traitlets import HasTraits, Unicode, Instance from remoteappmanager.logging.logging_mixin import LoggingMixin @@ -17,7 +17,7 @@ class ReverseProxy(LoggingMixin, HasTraits): api_token = Unicode() #: Internal instance of the reverse proxy API - _reverse_proxy = Instance(jupyterhub_orm.Proxy) + _reverse_proxy = Instance(Proxy) def __init__(self, *args, **kwargs): """Initializes the reverse proxy connection object.""" @@ -35,11 +35,11 @@ def __init__(self, *args, **kwargs): self.log.error(message) raise ValueError(message) - # Note, we use jupyterhub orm Proxy, but not for database access, - # just for interface convenience. - self._reverse_proxy = jupyterhub_orm.Proxy( + # Note: we just use the jupyterhub ConfigurableHTTPProxy for + # interface convenience + self._reverse_proxy = ConfigurableHTTPProxy( auth_token=self.api_token, - api_server=_server_from_url(self.endpoint_url) + api_server=self.endpoint_url ) self.log.info("Reverse proxy setup on {}".format( @@ -94,14 +94,3 @@ def unregister(self, urlpath): urlpath)) else: raise e - - -def _server_from_url(url): - """Creates a orm.Server from a given url""" - parsed = parse.urlparse(url) - return jupyterhub_orm.Server( - proto=parsed.scheme, - ip=parsed.hostname, - port=parsed.port, - base_url=parsed.path - ) From d90ac0cc34e5060564bc316f9c70b35eb2d90483 Mon Sep 17 00:00:00 2001 From: flongford Date: Tue, 13 Jul 2021 15:23:20 +0100 Subject: [PATCH 03/64] FIX: include missing X-JupyterHib-Version headers --- doc/source/mock_missing.py | 2 +- remoteappmanager/base_application.py | 37 +++++++++++++++++++++-- remoteappmanager/handlers/base_handler.py | 9 ++++++ 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/doc/source/mock_missing.py b/doc/source/mock_missing.py index 56582234..07c020e1 100644 --- a/doc/source/mock_missing.py +++ b/doc/source/mock_missing.py @@ -49,7 +49,7 @@ def mock_modules(): MOCK_MODULES.append('jupyterhub.auth') MOCK_MODULES.append('jupyterhub.spawner') MOCK_TYPES.append( - ("jupyterhub.orm", "Proxy", (object, )) + ("jupyterhub.proxy", "Proxy", (object, )) ) else: del jupyterhub diff --git a/remoteappmanager/base_application.py b/remoteappmanager/base_application.py index 42fcdf9b..385931a2 100644 --- a/remoteappmanager/base_application.py +++ b/remoteappmanager/base_application.py @@ -2,9 +2,11 @@ from remoteappmanager.handlers.handler_authenticator import HubAuthenticator from traitlets import Instance, default -from tornado import web +from tornado import web, gen import tornado.ioloop +from jupyterhub._version import __version__, _check_version +from tornado.httpclient import AsyncHTTPClient from tornadowebapi.registry import Registry from remoteappmanager.db.interfaces import ABCDatabase @@ -80,7 +82,8 @@ def __init__(self, settings.update(as_dict(command_line_config)) settings.update(as_dict(file_config)) settings["static_url_prefix"] = ( - self._command_line_config.base_urlpath + "static/") + self._command_line_config.base_urlpath + "static/") + settings['X-JupyterHub-Version'] = __version__ handlers = self._get_handlers() @@ -145,7 +148,8 @@ def _registry_default(self): # Public def start(self): """Start the application and the ioloop""" - + self.log.info("Starting SimPhoNy-Remote using JupyterHub" + " server version %s", __version__) self.log.info("Adding demo apps to User registry:") self._add_demo_apps() @@ -162,8 +166,35 @@ def start(self): self.listen(self.command_line_config.port) + tornado.ioloop.IOLoop.current().run_sync(self.check_hub_version) tornado.ioloop.IOLoop.current().start() + @gen.coroutine + def check_hub_version(self): + """Test a connection to my Hub + + - exit if I can't connect at all + - check version and warn on sufficient mismatch + """ + client = AsyncHTTPClient() + RETRIES = 5 + for i in range(1, RETRIES + 1): + try: + resp = yield client.fetch( + self.command_line_config.hub_api_url) + except Exception: + self.log.exception( + "Failed to connect to my Hub at %s (attempt %i/%i). Is it running?", + self.command_line_config.hub_api_url, i, RETRIES) + yield gen.sleep(min(2 ** i, 16)) + else: + break + else: + self.exit(1) + + hub_version = resp.headers.get('X-JupyterHub-Version') + _check_version(hub_version, __version__, self.log) + # Private def _add_demo_apps(self): """Grant access to any demo applications provided for user""" diff --git a/remoteappmanager/handlers/base_handler.py b/remoteappmanager/handlers/base_handler.py index 298afbee..d2aed73c 100644 --- a/remoteappmanager/handlers/base_handler.py +++ b/remoteappmanager/handlers/base_handler.py @@ -1,6 +1,7 @@ from http.client import responses import hashlib +from jupyterhub._version import __version__ from tornado import web, gen from remoteappmanager.logging.logging_mixin import LoggingMixin @@ -78,3 +79,11 @@ def write_error(self, status_code, **kwargs): self.render('error.html', status_code=status_code, status_message=status_message, message=message) + + def set_default_headers(self): + """Overload existing RequestHandler.set_default_headers method to include a + header containing the local version of JupyterHub. + Required by jupyterhub >= 0.8.0 + """ + super().set_default_headers() + self.set_header('X-JupyterHub-Version', __version__) From 8e5b2905561b499481afe92e1a93d385db4750a6 Mon Sep 17 00:00:00 2001 From: flongford Date: Tue, 13 Jul 2021 15:23:54 +0100 Subject: [PATCH 04/64] TST: add dummy-user option for dev testing --- dev-requirements.txt | 2 ++ jupyterhub/jupyterhub_config.py | 9 ++++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index bf7cc541..f06d137e 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -1,3 +1,5 @@ flake8 coverage selenium==3.141.0 +notebook +jupyterhub-simplespawner diff --git a/jupyterhub/jupyterhub_config.py b/jupyterhub/jupyterhub_config.py index 18d1de7b..568705b2 100644 --- a/jupyterhub/jupyterhub_config.py +++ b/jupyterhub/jupyterhub_config.py @@ -19,7 +19,7 @@ ) # Choose between system-user mode and virtual-user mode -setting_mode = ('system_user', 'virtual_user')[1] +setting_mode = ('system_user', 'virtual_user', 'dummy_user')[1] if setting_mode == 'virtual_user': c.JupyterHub.spawner_class = ('remoteappmanager.jupyterhub.spawners.' @@ -44,3 +44,10 @@ c.JupyterHub.spawner_class = ('remoteappmanager.jupyterhub.spawners.' 'SystemUserSpawner') c.Authenticator.admin_users = {os.environ["USER"]} + +elif setting_mode == 'dummy_user': + # Make sure to run: `pip install dev-requirements.txt` first + c.JupyterHub.spawner_class = ( + 'simplespawner.SimpleLocalProcessSpawner') + c.JupyterHub.authenticator_class = ( + 'remoteappmanager.jupyterhub.auth.WorldAuthenticator') From 56b76efba50d35fdda272f66637dc27ec5c7ea3a Mon Sep 17 00:00:00 2001 From: flongford Date: Wed, 5 Jan 2022 21:58:37 -0300 Subject: [PATCH 05/64] CLN: remove uneeded override of BaseHandler.set_default_headers --- remoteappmanager/handlers/base_handler.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/remoteappmanager/handlers/base_handler.py b/remoteappmanager/handlers/base_handler.py index d2aed73c..298afbee 100644 --- a/remoteappmanager/handlers/base_handler.py +++ b/remoteappmanager/handlers/base_handler.py @@ -1,7 +1,6 @@ from http.client import responses import hashlib -from jupyterhub._version import __version__ from tornado import web, gen from remoteappmanager.logging.logging_mixin import LoggingMixin @@ -79,11 +78,3 @@ def write_error(self, status_code, **kwargs): self.render('error.html', status_code=status_code, status_message=status_message, message=message) - - def set_default_headers(self): - """Overload existing RequestHandler.set_default_headers method to include a - header containing the local version of JupyterHub. - Required by jupyterhub >= 0.8.0 - """ - super().set_default_headers() - self.set_header('X-JupyterHub-Version', __version__) From 92e694bdb597df5b85922338478458c3694efdc8 Mon Sep 17 00:00:00 2001 From: flongford Date: Wed, 5 Jan 2022 22:01:31 -0300 Subject: [PATCH 06/64] STY: flake8 fixes --- remoteappmanager/base_application.py | 2 +- remoteappmanager/cli/remoteappdb/__main__.py | 8 ++++---- remoteappmanager/jupyterhub/spawners.py | 2 +- remoteappmanager/services/reverse_proxy.py | 2 -- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/remoteappmanager/base_application.py b/remoteappmanager/base_application.py index a94f26b2..41650874 100644 --- a/remoteappmanager/base_application.py +++ b/remoteappmanager/base_application.py @@ -197,7 +197,7 @@ def check_hub_version(self): self.command_line_config.hub_api_url) except Exception: self.log.exception( - "Failed to connect to my Hub at %s (attempt %i/%i). Is it running?", + "Failed to connect to my Hub at %s (attempt %i/%i). Is it running?", # noqa: E501 self.command_line_config.hub_api_url, i, RETRIES) yield gen.sleep(min(2 ** i, 16)) else: diff --git a/remoteappmanager/cli/remoteappdb/__main__.py b/remoteappmanager/cli/remoteappdb/__main__.py index e67c4b78..69fbfcd7 100644 --- a/remoteappmanager/cli/remoteappdb/__main__.py +++ b/remoteappmanager/cli/remoteappdb/__main__.py @@ -172,7 +172,7 @@ def user(ctx): @user.command() @click.argument("user") @click.pass_context -def create(ctx, user): +def create_user(ctx, user): """Creates a user USER in the database.""" session = ctx.obj.session orm_user = orm.User(name=user) @@ -212,7 +212,7 @@ def remove(ctx, user): help="Shows the applications each user " "is allowed to run") @click.pass_context -def list(ctx, no_decoration, show_apps): +def list_users(ctx, no_decoration, show_apps): """Show a list of the available users.""" if no_decoration: @@ -275,7 +275,7 @@ def app(ctx): default=True, help="Verify image name against docker.") @click.pass_context -def create(ctx, image, verify): +def create_image(ctx, image, verify): """Creates a new application for image IMAGE.""" # Verify if `image` is an existing docker image @@ -322,7 +322,7 @@ def remove(ctx, image): @click.option('--no-decoration', is_flag=True, help="Disable table decorations") @click.pass_context -def list(ctx, no_decoration): +def list_applications(ctx, no_decoration): """List all registered applications.""" if no_decoration: tablefmt = "plain" diff --git a/remoteappmanager/jupyterhub/spawners.py b/remoteappmanager/jupyterhub/spawners.py index 3859cd46..05990933 100644 --- a/remoteappmanager/jupyterhub/spawners.py +++ b/remoteappmanager/jupyterhub/spawners.py @@ -2,7 +2,7 @@ import escapism import string -from traitlets import Any, Unicode, Instance +from traitlets import Unicode, Instance from tornado import gen from jupyterhub.spawner import LocalProcessSpawner diff --git a/remoteappmanager/services/reverse_proxy.py b/remoteappmanager/services/reverse_proxy.py index 002adc60..d664e176 100644 --- a/remoteappmanager/services/reverse_proxy.py +++ b/remoteappmanager/services/reverse_proxy.py @@ -1,5 +1,3 @@ -from urllib import parse - from tornado import gen, httpclient from jupyterhub.proxy import Proxy, ConfigurableHTTPProxy from traitlets import HasTraits, Unicode, Instance From e4aa2ec2b5755ef6084439cd603aad71229d8bb0 Mon Sep 17 00:00:00 2001 From: flongford Date: Wed, 5 Jan 2022 22:14:34 -0300 Subject: [PATCH 07/64] FIX: revert cli function name changes in favor of flake exceptions --- remoteappmanager/cli/remoteappdb/__main__.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/remoteappmanager/cli/remoteappdb/__main__.py b/remoteappmanager/cli/remoteappdb/__main__.py index 69fbfcd7..54591b06 100644 --- a/remoteappmanager/cli/remoteappdb/__main__.py +++ b/remoteappmanager/cli/remoteappdb/__main__.py @@ -172,7 +172,7 @@ def user(ctx): @user.command() @click.argument("user") @click.pass_context -def create_user(ctx, user): +def create(ctx, user): # noqa: F811 """Creates a user USER in the database.""" session = ctx.obj.session orm_user = orm.User(name=user) @@ -190,7 +190,7 @@ def create_user(ctx, user): @user.command() @click.argument("user") @click.pass_context -def remove(ctx, user): +def remove(ctx, user): # noqa: F811 """Removes a user.""" session = ctx.obj.session @@ -212,7 +212,7 @@ def remove(ctx, user): help="Shows the applications each user " "is allowed to run") @click.pass_context -def list_users(ctx, no_decoration, show_apps): +def list(ctx, no_decoration, show_apps): # noqa: F811 """Show a list of the available users.""" if no_decoration: @@ -275,7 +275,7 @@ def app(ctx): default=True, help="Verify image name against docker.") @click.pass_context -def create_image(ctx, image, verify): +def create(ctx, image, verify): # noqa: F811 """Creates a new application for image IMAGE.""" # Verify if `image` is an existing docker image @@ -304,7 +304,7 @@ def create_image(ctx, image, verify): @app.command() # noqa @click.argument("image") @click.pass_context -def remove(ctx, image): +def remove(ctx, image): # noqa: F811 """Removes an application from the list.""" session = ctx.obj.session @@ -322,7 +322,7 @@ def remove(ctx, image): @click.option('--no-decoration', is_flag=True, help="Disable table decorations") @click.pass_context -def list_applications(ctx, no_decoration): +def list(ctx, no_decoration): # noqa: F811 """List all registered applications.""" if no_decoration: tablefmt = "plain" From 8d6ae2bd070d23005f50f6e8b9a43dea6a07ef8e Mon Sep 17 00:00:00 2001 From: flongford Date: Wed, 5 Jan 2022 22:17:03 -0300 Subject: [PATCH 08/64] FIX: tidy up deprecated uses of orm.Proxy --- nginx/nginx.conf.template | 2 +- remoteappmanager/jupyterhub/tests/test_spawners.py | 4 ++-- remoteappmanager/services/tests/test_reverse_proxy.py | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/nginx/nginx.conf.template b/nginx/nginx.conf.template index 1a87da22..c46e894b 100644 --- a/nginx/nginx.conf.template +++ b/nginx/nginx.conf.template @@ -38,7 +38,7 @@ server { # Managing literal requests to the JupyterHub front end location / { # Note: this url uses https, since we are working on an older version of JupyterHub (< 0.8.0), which - # internall uses the deprecated jupyterhub.orm.Proxy class. It is expected that updating the version + # internal uses the deprecated jupyterhub.proxy.Proxy class. It is expected that updating the version # to > 0.8.0 will mean using a http address instead, since this is replaced by the # jupyterhub.proxy.ConfigurableHTTPProxy class instead proxy_pass https://127.0.0.1:8000; diff --git a/remoteappmanager/jupyterhub/tests/test_spawners.py b/remoteappmanager/jupyterhub/tests/test_spawners.py index bc185128..ebb43834 100644 --- a/remoteappmanager/jupyterhub/tests/test_spawners.py +++ b/remoteappmanager/jupyterhub/tests/test_spawners.py @@ -7,7 +7,7 @@ from unittest import mock from tornado.testing import AsyncTestCase -from jupyterhub import orm +from jupyterhub import orm, proxy from remoteappmanager.jupyterhub.spawners import ( SystemUserSpawner, @@ -47,7 +47,7 @@ def new_spawner(spawner_class): db = mock.Mock() db.query = mock.Mock() db.query().first = mock.Mock( - return_value=orm.Proxy( + return_value=proxy.Proxy( auth_token="whatever", api_server=orm.Server(proto="http", ip="127.0.0.1", diff --git a/remoteappmanager/services/tests/test_reverse_proxy.py b/remoteappmanager/services/tests/test_reverse_proxy.py index d51b170d..d3fa4cdb 100644 --- a/remoteappmanager/services/tests/test_reverse_proxy.py +++ b/remoteappmanager/services/tests/test_reverse_proxy.py @@ -1,6 +1,6 @@ from unittest.mock import Mock -from jupyterhub import orm +from jupyterhub import proxy from remoteappmanager.services.reverse_proxy import ReverseProxy from tornado import gen from tornado.testing import gen_test, AsyncTestCase, ExpectLog @@ -21,7 +21,7 @@ def mock_api_request(self, *args, **kwargs): reverse_proxy = ReverseProxy( endpoint_url="http://fake/api", api_token="token") - reverse_proxy._reverse_proxy = Mock(spec=orm.Proxy) + reverse_proxy._reverse_proxy = Mock(spec=proxy.Proxy) reverse_proxy._reverse_proxy.api_request = mock_api_request yield reverse_proxy.register("/hello/from/me/", From f73361cac18dc073a4bf8f21f3cb90bef4c23fd5 Mon Sep 17 00:00:00 2001 From: flongford Date: Wed, 5 Jan 2022 22:22:53 -0300 Subject: [PATCH 09/64] TST: formatting fixed to returned auth dicts --- .../auth/tests/test_github_whitelist_authenticator.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/remoteappmanager/jupyterhub/auth/tests/test_github_whitelist_authenticator.py b/remoteappmanager/jupyterhub/auth/tests/test_github_whitelist_authenticator.py index ebc8d059..b933661e 100644 --- a/remoteappmanager/jupyterhub/auth/tests/test_github_whitelist_authenticator.py +++ b/remoteappmanager/jupyterhub/auth/tests/test_github_whitelist_authenticator.py @@ -36,12 +36,12 @@ def test_basic_auth_with_whitelist_file(self): auth.whitelist_file = whitelist_path response = yield auth.get_authenticated_user( Mock(), {"username": "foo"}) - self.assertEqual(response, "foo") + self.assertEqual(response['name'], "foo") # Check again to touch the code that does not trigger another load response = yield auth.get_authenticated_user( Mock(), {"username": "foo"}) - self.assertEqual(response, "foo") + self.assertEqual(response["name"], "foo") # wait one second, so that we see a change in mtime. time.sleep(1) @@ -64,7 +64,7 @@ def test_basic_auth_without_whitelist_file(self): {"username": "foo"}) # Should be equivalent to no whitelist, so everybody allowed - self.assertEqual(response, "foo") + self.assertEqual(response['name'], "foo") @gen_test def test_exception_during_read(self): From b023dd9ea6ebc0ea013069641ca539cbf51dccd9 Mon Sep 17 00:00:00 2001 From: flongford Date: Wed, 5 Jan 2022 22:30:12 -0300 Subject: [PATCH 10/64] TST: replace mocking of deprecated orm.Hub --- remoteappmanager/jupyterhub/tests/test_spawners.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/remoteappmanager/jupyterhub/tests/test_spawners.py b/remoteappmanager/jupyterhub/tests/test_spawners.py index ebb43834..ddb2c2fd 100644 --- a/remoteappmanager/jupyterhub/tests/test_spawners.py +++ b/remoteappmanager/jupyterhub/tests/test_spawners.py @@ -7,7 +7,7 @@ from unittest import mock from tornado.testing import AsyncTestCase -from jupyterhub import orm, proxy +from jupyterhub import orm, proxy, objects from remoteappmanager.jupyterhub.spawners import ( SystemUserSpawner, @@ -62,7 +62,12 @@ def new_spawner(spawner_class): user.server = generic_server # Mock hub - hub = orm.Hub(server=generic_server) + hub = objects.Hub( + ip="127.0.0.2", + proto="http", + port=31337, + base_url="/" + ) # Mock authenticator authenticator = mock.Mock() From 6be89c5d1ec03b06369105f1f9850ad2cd20b122 Mon Sep 17 00:00:00 2001 From: flongford Date: Thu, 6 Jan 2022 09:27:11 -0300 Subject: [PATCH 11/64] FIX: update EnvironmentConfig to source hub settings relocated to os environ --- remoteappmanager/environment_config.py | 24 ++++++++++++++++--- .../tests/test_environment_config.py | 15 ++++++++---- 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/remoteappmanager/environment_config.py b/remoteappmanager/environment_config.py index 3cf49513..79d58b82 100644 --- a/remoteappmanager/environment_config.py +++ b/remoteappmanager/environment_config.py @@ -6,13 +6,25 @@ class EnvironmentConfig(HasTraits): """Configuration options for the application server, originating from environment variables.""" - #: Token for JupyterHub API. Originates from JPY_API_TOKEN + #: Token for JupyterHub API. Originates from JUPYTERHUB_API_TOKEN jpy_api_token = Unicode(help="The JupyterHub API token") #: Token for the ReverseProxy API. Originates from #: PROXY_API_TOKEN proxy_api_token = Unicode(help="The Reverse Proxy API token") + # This is the host of the hub. It's always empty (jupyterhub decision). + # Originates from JUPYTERHUB_HOST + hub_host = Unicode(help="The url of the jupyterhub server") + + # This is a url path that sends the request to jupyterhub. + # It's normally /hub/. Originates from JUPYTERHUB_SERVICE_PREFIX + hub_prefix = Unicode(help="The url prefix of the jupyterhub") + + # This is a full url to reach the hub api (e.g. for authentication check) + # Originates from JUPYTERHUB_API_URL + hub_api_url = Unicode(help="The url of the jupyterhub REST API") + # Home is not part of it because we change it along the way, # so we can't rely on the value at startup. @@ -20,7 +32,13 @@ def parse_config(self): """Parses the environment variables, and assign their values to our local traits. """ - for traitlet_name in self.traits().keys(): - envname = traitlet_name.upper() + mapping = { + "JUPYTERHUB_API_TOKEN": 'jpy_api_token', + "PROXY_API_TOKEN": 'proxy_api_token', + "JUPYTERHUB_HOST": 'hub_host', + "JUPYTERHUB_SERVICE_PREFIX": 'hub_prefix', + "JUPYTERHUB_API_URL": 'hub_api_url' + } + for envname, traitlet_name in mapping.items(): setattr(self, traitlet_name, os.environ.get(envname, "")) diff --git a/remoteappmanager/tests/test_environment_config.py b/remoteappmanager/tests/test_environment_config.py index ad21daed..f17bafb8 100644 --- a/remoteappmanager/tests/test_environment_config.py +++ b/remoteappmanager/tests/test_environment_config.py @@ -8,10 +8,17 @@ class TestEnvironmentConfig(unittest.TestCase): def test_init(self): # We don't want to tinker with os.environ config = EnvironmentConfig() - with patch.dict('os.environ', - {'JPY_API_TOKEN': 'jpy_token', - 'PROXY_API_TOKEN': 'proxy_token' - }): + env = { + 'JUPYTERHUB_API_TOKEN': 'jpy_token', + 'PROXY_API_TOKEN': 'proxy_token', + 'JUPYTERHUB_HOST': 'jpy_host', + 'JUPYTERHUB_SERVICE_PREFIX': 'jpy_proxy', + 'JUPYTERHUB_API_URL': 'jpy_api_url', + } + with patch.dict('os.environ', env): config.parse_config() self.assertEqual(config.jpy_api_token, "jpy_token") self.assertEqual(config.proxy_api_token, "proxy_token") + self.assertEqual(config.hub_host, "jpy_host") + self.assertEqual(config.hub_prefix, "jpy_proxy") + self.assertEqual(config.hub_api_url, "jpy_api_url") From f6d617e3dae7e08e3b4dcea540f7d95f2ebf5181 Mon Sep 17 00:00:00 2001 From: flongford Date: Thu, 6 Jan 2022 09:53:47 -0300 Subject: [PATCH 12/64] TST: remove hub_* traits from CommandLineConfig replace source in BaseApplication --- remoteappmanager/base_application.py | 14 ++++++++++---- remoteappmanager/command_line_config.py | 10 ---------- remoteappmanager/tests/test_environment_config.py | 12 ++++++------ remoteappmanager/tests/utils.py | 3 --- 4 files changed, 16 insertions(+), 23 deletions(-) diff --git a/remoteappmanager/base_application.py b/remoteappmanager/base_application.py index 41650874..ce13fbd0 100644 --- a/remoteappmanager/base_application.py +++ b/remoteappmanager/base_application.py @@ -111,7 +111,7 @@ def _reverse_proxy_default(self): @default("hub") def _hub_default(self): """Initializes the Hub instance.""" - return Hub(endpoint_url=self.command_line_config.hub_api_url, + return Hub(endpoint_url=self.environment_config.hub_api_url, api_token=self.environment_config.jpy_api_token, ) @@ -173,13 +173,19 @@ def start(self): getattr(self._command_line_config, trait_name) ) ) + for trait_name in self._environment_config.trait_names(): + self.log.info("{}: {}".format( + trait_name, + getattr(self._environment_config, trait_name) + ) + ) self.log.info("Listening for connections on {}:{}".format( self.command_line_config.ip, self.command_line_config.port)) self.listen(self.command_line_config.port) - tornado.ioloop.IOLoop.current().run_sync(self.check_hub_version) + #tornado.ioloop.IOLoop.current().run_sync(self.check_hub_version) tornado.ioloop.IOLoop.current().start() @gen.coroutine @@ -194,11 +200,11 @@ def check_hub_version(self): for i in range(1, RETRIES + 1): try: resp = yield client.fetch( - self.command_line_config.hub_api_url) + self.environment_config.hub_api_url) except Exception: self.log.exception( "Failed to connect to my Hub at %s (attempt %i/%i). Is it running?", # noqa: E501 - self.command_line_config.hub_api_url, i, RETRIES) + self.environment_config.hub_api_url, i, RETRIES) yield gen.sleep(min(2 ** i, 16)) else: break diff --git a/remoteappmanager/command_line_config.py b/remoteappmanager/command_line_config.py index 5e45cba0..96b26fdd 100644 --- a/remoteappmanager/command_line_config.py +++ b/remoteappmanager/command_line_config.py @@ -24,16 +24,6 @@ class CommandLineConfig(HasTraits): # typically, it's /user/username base_urlpath = Unicode(help="The base url where the server resides") - # This is the host of the hub. It's always empty (jupyterhub decision) - hub_host = Unicode(help="The url of the jupyterhub server") - - # This is a url path that sends the request to jupyterhub. - # It's normally /hub/ - hub_prefix = Unicode(help="The url prefix of the jupyterhub") - - # This is a full url to reach the hub api (e.g. for authentication check) - hub_api_url = Unicode(help="The url of the jupyterhub REST API") - # The full URL where to access the reverse proxy API. proxy_api_url = Unicode(help="The url of the reverse proxy API") diff --git a/remoteappmanager/tests/test_environment_config.py b/remoteappmanager/tests/test_environment_config.py index f17bafb8..5b0bccef 100644 --- a/remoteappmanager/tests/test_environment_config.py +++ b/remoteappmanager/tests/test_environment_config.py @@ -11,14 +11,14 @@ def test_init(self): env = { 'JUPYTERHUB_API_TOKEN': 'jpy_token', 'PROXY_API_TOKEN': 'proxy_token', - 'JUPYTERHUB_HOST': 'jpy_host', - 'JUPYTERHUB_SERVICE_PREFIX': 'jpy_proxy', - 'JUPYTERHUB_API_URL': 'jpy_api_url', + 'JUPYTERHUB_HOST': '', + 'JUPYTERHUB_SERVICE_PREFIX': '/hub/', + 'JUPYTERHUB_API_URL': 'http://172.17.5.167:8081/hub/api', } with patch.dict('os.environ', env): config.parse_config() self.assertEqual(config.jpy_api_token, "jpy_token") self.assertEqual(config.proxy_api_token, "proxy_token") - self.assertEqual(config.hub_host, "jpy_host") - self.assertEqual(config.hub_prefix, "jpy_proxy") - self.assertEqual(config.hub_api_url, "jpy_api_url") + self.assertEqual(config.hub_host, "") + self.assertEqual(config.hub_prefix, '/hub/') + self.assertEqual(config.hub_api_url, 'http://172.17.5.167:8081/hub/api') diff --git a/remoteappmanager/tests/utils.py b/remoteappmanager/tests/utils.py index cddfb195..2e85a5b8 100644 --- a/remoteappmanager/tests/utils.py +++ b/remoteappmanager/tests/utils.py @@ -16,9 +16,6 @@ "port": 57022, "cookie-name": "jupyter-hub-token-johndoe", "base-urlpath": "\"/user/johndoe/\"", - "hub-host": "", - "hub-prefix": "/hub/", - "hub-api-url": "http://172.17.5.167:8081/hub/api", "proxy-api-url": "http://192.168.100.99/proxy/api", "ip": "127.0.0.1", "config-file": fixtures.get("remoteappmanager_config.py") From 5e00472f8b0095a96d5b498878e69603c4d6af8d Mon Sep 17 00:00:00 2001 From: flongford Date: Thu, 6 Jan 2022 09:55:24 -0300 Subject: [PATCH 13/64] FIX: various fixes to BaseSpawner class --- remoteappmanager/jupyterhub/spawners.py | 11 +++++++---- .../jupyterhub/tests/test_spawners.py | 17 +++++++++++------ 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/remoteappmanager/jupyterhub/spawners.py b/remoteappmanager/jupyterhub/spawners.py index 05990933..bf6c0c62 100644 --- a/remoteappmanager/jupyterhub/spawners.py +++ b/remoteappmanager/jupyterhub/spawners.py @@ -31,16 +31,19 @@ def cmd(self): def __init__(self, **kwargs): super().__init__(**kwargs) - self.proxy = ConfigurableHTTPProxy() + # We get the first one. Strangely enough, jupyterhub has a table + # containing potentially multiple proxies, but it's enforced to + # contain only one. + self.proxy = self.db.query(ConfigurableHTTPProxy).first() def get_args(self): args = super().get_args() - for iarg, arg in enumerate(args): - args[iarg] = arg.replace('--base-url=', '--base-urlpath=') + args.append('--base-urlpath="{}"'.format( + self.server.base_url)) args.append("--cookie_name={}".format( - self.server.cookie_name)) + self.hub.cookie_name)) args.append("--proxy-api-url={}".format( self.proxy.api_url)) diff --git a/remoteappmanager/jupyterhub/tests/test_spawners.py b/remoteappmanager/jupyterhub/tests/test_spawners.py index ddb2c2fd..fb101a20 100644 --- a/remoteappmanager/jupyterhub/tests/test_spawners.py +++ b/remoteappmanager/jupyterhub/tests/test_spawners.py @@ -47,16 +47,16 @@ def new_spawner(spawner_class): db = mock.Mock() db.query = mock.Mock() db.query().first = mock.Mock( - return_value=proxy.Proxy( + return_value=proxy.ConfigurableHTTPProxy( auth_token="whatever", - api_server=orm.Server(proto="http", - ip="127.0.0.1", - port=12345, - base_url="/foo/bar/"))) + api_url="http://127.0.0.1:12345/foo/bar/" + ) + ) # Mock user user = mock.Mock() user.name = username() + user.url = 'http://my-callback.com' user.admin = False user.state = None user.server = generic_server @@ -75,8 +75,13 @@ def new_spawner(spawner_class): return_value='/logout_test') authenticator.login_service = 'TEST' - return spawner_class( + spawner = spawner_class( db=db, user=user, hub=hub, authenticator=authenticator) + # As of Jupyter 0.8.1, Spawner classes do not assign server + # property from user instance and the setter does not seem + # to work during instantiation + spawner.server = generic_server + return spawner class TestSystemUserSpawner(TempMixin, AsyncTestCase): From 5c18531d2ab3d8d3fa4efd781fa3be5cf9f0233b Mon Sep 17 00:00:00 2001 From: flongford Date: Thu, 6 Jan 2022 10:39:24 -0300 Subject: [PATCH 14/64] FIX: wrong assignment of ConfigurableHTTPProxy.api_server trait --- remoteappmanager/services/reverse_proxy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/remoteappmanager/services/reverse_proxy.py b/remoteappmanager/services/reverse_proxy.py index d664e176..3227e121 100644 --- a/remoteappmanager/services/reverse_proxy.py +++ b/remoteappmanager/services/reverse_proxy.py @@ -37,7 +37,7 @@ def __init__(self, *args, **kwargs): # interface convenience self._reverse_proxy = ConfigurableHTTPProxy( auth_token=self.api_token, - api_server=self.endpoint_url + api_url=self.endpoint_url ) self.log.info("Reverse proxy setup on {}".format( From 373fee34804773b3ad05a1434f2576ddf712e6b4 Mon Sep 17 00:00:00 2001 From: flongford Date: Thu, 6 Jan 2022 10:40:16 -0300 Subject: [PATCH 15/64] TST: Spawner.ip no longer has a default value, manually assigning 127.0.0.1 --- remoteappmanager/jupyterhub/tests/test_spawners.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/remoteappmanager/jupyterhub/tests/test_spawners.py b/remoteappmanager/jupyterhub/tests/test_spawners.py index fb101a20..1c677bea 100644 --- a/remoteappmanager/jupyterhub/tests/test_spawners.py +++ b/remoteappmanager/jupyterhub/tests/test_spawners.py @@ -76,7 +76,7 @@ def new_spawner(spawner_class): authenticator.login_service = 'TEST' spawner = spawner_class( - db=db, user=user, hub=hub, authenticator=authenticator) + db=db, ip='127.0.0.1', user=user, hub=hub, authenticator=authenticator) # As of Jupyter 0.8.1, Spawner classes do not assign server # property from user instance and the setter does not seem # to work during instantiation @@ -93,12 +93,14 @@ def test_args(self): path = fixtures.get("remoteappmanager_config.py") self.spawner.config_file_path = path args = self.spawner.get_args() + self.assertIn("--ip=\"127.0.0.1\"", args) self.assertIn("--proxy-api-url=http://127.0.0.1:12345/foo/bar/", args) self.assertIn("--config-file={}".format(path), args) self.assertIn("--base-urlpath=\"/\"", args) def test_args_without_config_file_path(self): args = self.spawner.get_args() + self.assertIn("--ip=\"127.0.0.1\"", args) self.assertIn("--proxy-api-url=http://127.0.0.1:12345/foo/bar/", args) self.assertFalse(any("--config-file=" in arg for arg in args)) self.assertIn("--base-urlpath=\"/\"", args) From eff19bc345e7275e910a9c02d4c8e53fa2b404aa Mon Sep 17 00:00:00 2001 From: flongford Date: Thu, 6 Jan 2022 10:40:46 -0300 Subject: [PATCH 16/64] FIX: further fixes to BaseSpawner get_args --- remoteappmanager/jupyterhub/spawners.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/remoteappmanager/jupyterhub/spawners.py b/remoteappmanager/jupyterhub/spawners.py index bf6c0c62..412f3013 100644 --- a/remoteappmanager/jupyterhub/spawners.py +++ b/remoteappmanager/jupyterhub/spawners.py @@ -39,11 +39,14 @@ def __init__(self, **kwargs): def get_args(self): args = super().get_args() + args.append('--user="{}"'.format( + self.user.name)) + args.append('--base-urlpath="{}"'.format( self.server.base_url)) args.append("--cookie_name={}".format( - self.hub.cookie_name)) + self.user.server.cookie_name)) args.append("--proxy-api-url={}".format( self.proxy.api_url)) From 4c4e01302589524c96e2a9ee7e6ee3305f733600 Mon Sep 17 00:00:00 2001 From: flongford Date: Thu, 6 Jan 2022 10:57:57 -0300 Subject: [PATCH 17/64] STY: flake8 fixes --- remoteappmanager/base_application.py | 2 +- remoteappmanager/tests/test_environment_config.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/remoteappmanager/base_application.py b/remoteappmanager/base_application.py index ce13fbd0..565f7f1e 100644 --- a/remoteappmanager/base_application.py +++ b/remoteappmanager/base_application.py @@ -185,7 +185,7 @@ def start(self): self.listen(self.command_line_config.port) - #tornado.ioloop.IOLoop.current().run_sync(self.check_hub_version) + tornado.ioloop.IOLoop.current().run_sync(self.check_hub_version) tornado.ioloop.IOLoop.current().start() @gen.coroutine diff --git a/remoteappmanager/tests/test_environment_config.py b/remoteappmanager/tests/test_environment_config.py index 5b0bccef..62c560f7 100644 --- a/remoteappmanager/tests/test_environment_config.py +++ b/remoteappmanager/tests/test_environment_config.py @@ -21,4 +21,5 @@ def test_init(self): self.assertEqual(config.proxy_api_token, "proxy_token") self.assertEqual(config.hub_host, "") self.assertEqual(config.hub_prefix, '/hub/') - self.assertEqual(config.hub_api_url, 'http://172.17.5.167:8081/hub/api') + self.assertEqual( + config.hub_api_url, 'http://172.17.5.167:8081/hub/api') From 05c49e44819a7610290e0fec9f9df413afa205cf Mon Sep 17 00:00:00 2001 From: flongford Date: Thu, 6 Jan 2022 11:11:56 -0300 Subject: [PATCH 18/64] TST: fix test environment for BaseApplication --- .../tests/test_environment_config.py | 10 ++-------- remoteappmanager/tests/utils.py | 16 +++++++++++++--- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/remoteappmanager/tests/test_environment_config.py b/remoteappmanager/tests/test_environment_config.py index 62c560f7..72cd57d7 100644 --- a/remoteappmanager/tests/test_environment_config.py +++ b/remoteappmanager/tests/test_environment_config.py @@ -2,20 +2,14 @@ from unittest.mock import patch from remoteappmanager.environment_config import EnvironmentConfig +from remoteappmanager.tests.utils import env_config class TestEnvironmentConfig(unittest.TestCase): def test_init(self): # We don't want to tinker with os.environ config = EnvironmentConfig() - env = { - 'JUPYTERHUB_API_TOKEN': 'jpy_token', - 'PROXY_API_TOKEN': 'proxy_token', - 'JUPYTERHUB_HOST': '', - 'JUPYTERHUB_SERVICE_PREFIX': '/hub/', - 'JUPYTERHUB_API_URL': 'http://172.17.5.167:8081/hub/api', - } - with patch.dict('os.environ', env): + with patch.dict('os.environ', env_config): config.parse_config() self.assertEqual(config.jpy_api_token, "jpy_token") self.assertEqual(config.proxy_api_token, "proxy_token") diff --git a/remoteappmanager/tests/utils.py b/remoteappmanager/tests/utils.py index 2e85a5b8..76a776ee 100644 --- a/remoteappmanager/tests/utils.py +++ b/remoteappmanager/tests/utils.py @@ -21,6 +21,14 @@ "config-file": fixtures.get("remoteappmanager_config.py") } +env_config = { + 'JUPYTERHUB_API_TOKEN': 'jpy_token', + 'PROXY_API_TOKEN': 'proxy_token', + 'JUPYTERHUB_HOST': '', + 'JUPYTERHUB_SERVICE_PREFIX': '/hub/', + 'JUPYTERHUB_API_URL': 'http://172.17.5.167:8081/hub/api', +} + def init_sqlite_db(path): """Initializes the sqlite database at a given path. @@ -45,9 +53,11 @@ def basic_file_config(): def basic_environment_config(): config = EnvironmentConfig() - config.proxy_api_token = "dummy_token" - config.jpy_api_token = "dummy_token" - + config.proxy_api_token = env_config['PROXY_API_TOKEN'] + config.jpy_api_token = env_config['JUPYTERHUB_API_TOKEN'] + config.hub_host = env_config['JUPYTERHUB_HOST'] + config.hub_prefix = env_config['JUPYTERHUB_SERVICE_PREFIX'] + config.hub_api_url = env_config['JUPYTERHUB_API_URL'] return config From c1d74a71411aecf0b65b0760de7993ec4adeae67 Mon Sep 17 00:00:00 2001 From: flongford Date: Thu, 6 Jan 2022 14:25:44 -0300 Subject: [PATCH 19/64] FIX: pass Proxy API status to Spawner in config, rather than during runtime --- jupyterhub/jupyterhub_config.py | 10 ++++++++ remoteappmanager/jupyterhub/spawners.py | 25 +++++++++---------- .../jupyterhub/tests/test_spawners.py | 16 ++++++------ 3 files changed, 30 insertions(+), 21 deletions(-) diff --git a/jupyterhub/jupyterhub_config.py b/jupyterhub/jupyterhub_config.py index 568705b2..7de464dd 100644 --- a/jupyterhub/jupyterhub_config.py +++ b/jupyterhub/jupyterhub_config.py @@ -1,6 +1,7 @@ # Configuration file for jupyterhub. import os from jupyter_client.localinterfaces import public_ips +from jupyterhub.utils import new_token c = get_config() # noqa @@ -12,6 +13,8 @@ c.ConfigurableHTTPProxy.command = [ 'configurable-http-proxy', f'--default-target=http://{c.JupyterHub.hub_ip}:8081'] +c.ConfigurableHTTPProxy.api_url = "http://127.0.0.1:8001" +c.ConfigurableHTTPProxy.auth_token = new_token() c.JupyterHub.logo_file = os.path.join( os.path.dirname(os.path.dirname(os.path.realpath(__file__))), @@ -35,6 +38,13 @@ # file for the spawned service, instead of using the defaults. # c.Spawner.config_file_path = 'remoteappmanager_config.py' + # As of jupyterhub>0.8.0, there's no convenient way to obtain a + # reference between the Spawner and ConfigurableHTTProxy objects + # during runtime, so we simply assign common traits once and do + # not expect them to change + c.Spawner.proxy_api_url = c.ConfigurableHTTPProxy.api_url + c.Spawner.proxy_auth_token = c.ConfigurableHTTPProxy.auth_token + # FIXME: replace me with other authenticator (e.g. GitHub OAuth...) c.JupyterHub.authenticator_class = ( 'remoteappmanager.jupyterhub.auth.WorldAuthenticator') diff --git a/remoteappmanager/jupyterhub/spawners.py b/remoteappmanager/jupyterhub/spawners.py index 412f3013..ca5fd1ed 100644 --- a/remoteappmanager/jupyterhub/spawners.py +++ b/remoteappmanager/jupyterhub/spawners.py @@ -6,7 +6,6 @@ from tornado import gen from jupyterhub.spawner import LocalProcessSpawner -from jupyterhub.proxy import Proxy, ConfigurableHTTPProxy class BaseSpawner(LocalProcessSpawner): @@ -14,12 +13,19 @@ class BaseSpawner(LocalProcessSpawner): the actual spawners """ - #: The instance of the proxy.Proxy. - proxy = Instance(Proxy) - #: The path of the configuration file for the cmd executable config_file_path = Unicode(config=True) + #: The URL for JupyterHub's Proxy API server. We currently expect + #: this to be set manually in the jupyterhub_config.py file + #: (along with ConfigurableHTTPProxy.api_url) + proxy_api_url = Unicode(config=True) + + #: The token for JupyterHub's Proxy API server. We currently expect + #: this to be set manually in the jupyterhub_config.py file + #: (along with ConfigurableHTTPProxy.auth_token) + proxy_auth_token = Unicode(config=True) + @property def cmd(self): """Overrides the base class traitlet so that we take full control @@ -29,13 +35,6 @@ def cmd(self): if self.user.admin is True else ["remoteappmanager"]) - def __init__(self, **kwargs): - super().__init__(**kwargs) - # We get the first one. Strangely enough, jupyterhub has a table - # containing potentially multiple proxies, but it's enforced to - # contain only one. - self.proxy = self.db.query(ConfigurableHTTPProxy).first() - def get_args(self): args = super().get_args() @@ -49,7 +48,7 @@ def get_args(self): self.user.server.cookie_name)) args.append("--proxy-api-url={}".format( - self.proxy.api_url)) + self.proxy_api_url)) args.append("--logout_url={}".format( self.authenticator.logout_url( @@ -65,7 +64,7 @@ def get_args(self): def get_env(self): env = super().get_env() - env["PROXY_API_TOKEN"] = self.proxy.auth_token + env["PROXY_API_TOKEN"] = self.proxy_auth_token env.update(_docker_envvars()) return env diff --git a/remoteappmanager/jupyterhub/tests/test_spawners.py b/remoteappmanager/jupyterhub/tests/test_spawners.py index 1c677bea..4b8b24f2 100644 --- a/remoteappmanager/jupyterhub/tests/test_spawners.py +++ b/remoteappmanager/jupyterhub/tests/test_spawners.py @@ -45,13 +45,6 @@ def new_spawner(spawner_class): # Mock db db = mock.Mock() - db.query = mock.Mock() - db.query().first = mock.Mock( - return_value=proxy.ConfigurableHTTPProxy( - auth_token="whatever", - api_url="http://127.0.0.1:12345/foo/bar/" - ) - ) # Mock user user = mock.Mock() @@ -76,7 +69,14 @@ def new_spawner(spawner_class): authenticator.login_service = 'TEST' spawner = spawner_class( - db=db, ip='127.0.0.1', user=user, hub=hub, authenticator=authenticator) + ip='127.0.0.1', + db=db, + user=user, + hub=hub, + authenticator=authenticator, + proxy_api_url="http://127.0.0.1:12345/foo/bar/", + proxy_auth_token="whatever", + ) # As of Jupyter 0.8.1, Spawner classes do not assign server # property from user instance and the setter does not seem # to work during instantiation From 9299d3c9ab85b8f07624f252d575adc9606e22c0 Mon Sep 17 00:00:00 2001 From: flongford Date: Thu, 6 Jan 2022 14:26:39 -0300 Subject: [PATCH 20/64] STY: flake8 fixes --- remoteappmanager/jupyterhub/spawners.py | 2 +- remoteappmanager/jupyterhub/tests/test_spawners.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/remoteappmanager/jupyterhub/spawners.py b/remoteappmanager/jupyterhub/spawners.py index ca5fd1ed..7dc1bf4c 100644 --- a/remoteappmanager/jupyterhub/spawners.py +++ b/remoteappmanager/jupyterhub/spawners.py @@ -2,7 +2,7 @@ import escapism import string -from traitlets import Unicode, Instance +from traitlets import Unicode from tornado import gen from jupyterhub.spawner import LocalProcessSpawner diff --git a/remoteappmanager/jupyterhub/tests/test_spawners.py b/remoteappmanager/jupyterhub/tests/test_spawners.py index 4b8b24f2..48ef1b4f 100644 --- a/remoteappmanager/jupyterhub/tests/test_spawners.py +++ b/remoteappmanager/jupyterhub/tests/test_spawners.py @@ -7,7 +7,7 @@ from unittest import mock from tornado.testing import AsyncTestCase -from jupyterhub import orm, proxy, objects +from jupyterhub import orm, objects from remoteappmanager.jupyterhub.spawners import ( SystemUserSpawner, From 35e551454f07f733f8d26a5b3f586341146d6a45 Mon Sep 17 00:00:00 2001 From: flongford Date: Thu, 6 Jan 2022 16:44:40 -0300 Subject: [PATCH 21/64] FIX: hack-y fix for breaking changes to Spawner class introduced in v0.8.x --- remoteappmanager/jupyterhub/spawners.py | 29 +++++++++++++++++-- .../jupyterhub/tests/test_spawners.py | 20 ++++++------- 2 files changed, 35 insertions(+), 14 deletions(-) diff --git a/remoteappmanager/jupyterhub/spawners.py b/remoteappmanager/jupyterhub/spawners.py index 7dc1bf4c..52d467b5 100644 --- a/remoteappmanager/jupyterhub/spawners.py +++ b/remoteappmanager/jupyterhub/spawners.py @@ -2,7 +2,7 @@ import escapism import string -from traitlets import Unicode +from traitlets import Unicode, default from tornado import gen from jupyterhub.spawner import LocalProcessSpawner @@ -26,6 +26,10 @@ class BaseSpawner(LocalProcessSpawner): #: (along with ConfigurableHTTPProxy.auth_token) proxy_auth_token = Unicode(config=True) + @default('ip') + def _ip_default(self): + return "127.0.0.1" + @property def cmd(self): """Overrides the base class traitlet so that we take full control @@ -35,14 +39,34 @@ def cmd(self): if self.user.admin is True else ["remoteappmanager"]) + def __init__(self, **kwargs): + super(LocalProcessSpawner, self).__init__(**kwargs) + # FIXME: This is a hack since get_args method contains a bug in v0.8.1 + # that means it cannot handle a non-assigned port + # Note tat we assume that the self.user.server attribute will have + # a non-None value for this to work + server = self.user.server + if server: + self.port = server.port + # We can obtain a reference to the JupyterHub.proxy object + # through the tornado settings passed onto the User + proxy = self.user.settings.get('proxy') + if proxy is not None: + self.proxy_api_url = proxy.api_url + self.proxy_auth_token = proxy.auth_token + def get_args(self): args = super().get_args() args.append('--user="{}"'.format( self.user.name)) + if not self.port: + args.append("--port={}".format( + self.user.server.port)) + args.append('--base-urlpath="{}"'.format( - self.server.base_url)) + self.user.server.base_url)) args.append("--cookie_name={}".format( self.user.server.cookie_name)) @@ -160,7 +184,6 @@ def start(self): else: self.log.info("Created %s's temporary workspace in: %s", self.user.name, self._virtual_workspace) - return (yield super().start()) @gen.coroutine diff --git a/remoteappmanager/jupyterhub/tests/test_spawners.py b/remoteappmanager/jupyterhub/tests/test_spawners.py index 48ef1b4f..d8467189 100644 --- a/remoteappmanager/jupyterhub/tests/test_spawners.py +++ b/remoteappmanager/jupyterhub/tests/test_spawners.py @@ -7,7 +7,7 @@ from unittest import mock from tornado.testing import AsyncTestCase -from jupyterhub import orm, objects +from jupyterhub import proxy, orm, objects from remoteappmanager.jupyterhub.spawners import ( SystemUserSpawner, @@ -53,6 +53,12 @@ def new_spawner(spawner_class): user.admin = False user.state = None user.server = generic_server + user.settings = { + 'proxy': proxy.ConfigurableHTTPProxy( + api_url="http://127.0.0.1:12345/foo/bar/", + auth_token="whatever" + ) + } # Mock hub hub = objects.Hub( @@ -68,20 +74,12 @@ def new_spawner(spawner_class): return_value='/logout_test') authenticator.login_service = 'TEST' - spawner = spawner_class( - ip='127.0.0.1', + return spawner_class( db=db, user=user, hub=hub, - authenticator=authenticator, - proxy_api_url="http://127.0.0.1:12345/foo/bar/", - proxy_auth_token="whatever", + authenticator=authenticator ) - # As of Jupyter 0.8.1, Spawner classes do not assign server - # property from user instance and the setter does not seem - # to work during instantiation - spawner.server = generic_server - return spawner class TestSystemUserSpawner(TempMixin, AsyncTestCase): From d1cf7604d890c6e0eecb84f516d713c28791a6dd Mon Sep 17 00:00:00 2001 From: flongford Date: Thu, 6 Jan 2022 16:58:09 -0300 Subject: [PATCH 22/64] FIX: simply overwrite LocalProcessSpawner.get_args --- remoteappmanager/jupyterhub/spawners.py | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/remoteappmanager/jupyterhub/spawners.py b/remoteappmanager/jupyterhub/spawners.py index 52d467b5..6dfe8328 100644 --- a/remoteappmanager/jupyterhub/spawners.py +++ b/remoteappmanager/jupyterhub/spawners.py @@ -41,13 +41,6 @@ def cmd(self): def __init__(self, **kwargs): super(LocalProcessSpawner, self).__init__(**kwargs) - # FIXME: This is a hack since get_args method contains a bug in v0.8.1 - # that means it cannot handle a non-assigned port - # Note tat we assume that the self.user.server attribute will have - # a non-None value for this to work - server = self.user.server - if server: - self.port = server.port # We can obtain a reference to the JupyterHub.proxy object # through the tornado settings passed onto the User proxy = self.user.settings.get('proxy') @@ -56,14 +49,20 @@ def __init__(self, **kwargs): self.proxy_auth_token = proxy.auth_token def get_args(self): - args = super().get_args() - + # FIXME: we can't call the super get_args method since it contains a + # bug in v0.8.1 that cannot handle an unassigned self.port attribute. + # Note that we assume that the self.user.server attribute will have + # a non-None value for this to work + args = [] + args.extend(self.args) args.append('--user="{}"'.format( self.user.name)) - if not self.port: - args.append("--port={}".format( - self.user.server.port)) + args.append('--ip="{}"'.format( + self.ip)) + + args.append('--port="{}"'.format( + self.user.server.port)) args.append('--base-urlpath="{}"'.format( self.user.server.base_url)) From 45addc397d29f93dc37d5d11812a6bfd382c1107 Mon Sep 17 00:00:00 2001 From: flongford Date: Thu, 6 Jan 2022 17:03:57 -0300 Subject: [PATCH 23/64] FIX: remove default BaseSpawner.ip value --- remoteappmanager/jupyterhub/spawners.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/remoteappmanager/jupyterhub/spawners.py b/remoteappmanager/jupyterhub/spawners.py index 6dfe8328..9751bf7f 100644 --- a/remoteappmanager/jupyterhub/spawners.py +++ b/remoteappmanager/jupyterhub/spawners.py @@ -2,7 +2,7 @@ import escapism import string -from traitlets import Unicode, default +from traitlets import Unicode from tornado import gen from jupyterhub.spawner import LocalProcessSpawner @@ -26,10 +26,6 @@ class BaseSpawner(LocalProcessSpawner): #: (along with ConfigurableHTTPProxy.auth_token) proxy_auth_token = Unicode(config=True) - @default('ip') - def _ip_default(self): - return "127.0.0.1" - @property def cmd(self): """Overrides the base class traitlet so that we take full control From 50011c1d59815bb4731db1811db865b0b298b261 Mon Sep 17 00:00:00 2001 From: flongford Date: Thu, 6 Jan 2022 17:11:35 -0300 Subject: [PATCH 24/64] FIX: handle None value for LocalProcessSpawner.ip --- remoteappmanager/jupyterhub/spawners.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/remoteappmanager/jupyterhub/spawners.py b/remoteappmanager/jupyterhub/spawners.py index 9751bf7f..de0c4b5c 100644 --- a/remoteappmanager/jupyterhub/spawners.py +++ b/remoteappmanager/jupyterhub/spawners.py @@ -45,17 +45,18 @@ def __init__(self, **kwargs): self.proxy_auth_token = proxy.auth_token def get_args(self): - # FIXME: we can't call the super get_args method since it contains a - # bug in v0.8.1 that cannot handle an unassigned self.port attribute. - # Note that we assume that the self.user.server attribute will have - # a non-None value for this to work + # FIXME: we can't call the super get_args method before starting the + # spawner, a bug exists in jupyterhub v0.8.1 that means it cannot + # handle an unassigned self.port attribute. + # Note that this workaround assumes the self.user.server attribute + # has a non-None value args = [] args.extend(self.args) args.append('--user="{}"'.format( self.user.name)) args.append('--ip="{}"'.format( - self.ip)) + self.ip or "127.0.0.1")) args.append('--port="{}"'.format( self.user.server.port)) From 5877724e30748b3ae629a27d3c71334d06a18657 Mon Sep 17 00:00:00 2001 From: flongford Date: Thu, 6 Jan 2022 17:24:10 -0300 Subject: [PATCH 25/64] MAINT: changes to Nginx conf file --- nginx/nginx.conf.template | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/nginx/nginx.conf.template b/nginx/nginx.conf.template index c46e894b..f6a869ac 100644 --- a/nginx/nginx.conf.template +++ b/nginx/nginx.conf.template @@ -37,11 +37,7 @@ server { # Managing literal requests to the JupyterHub front end location / { - # Note: this url uses https, since we are working on an older version of JupyterHub (< 0.8.0), which - # internal uses the deprecated jupyterhub.proxy.Proxy class. It is expected that updating the version - # to > 0.8.0 will mean using a http address instead, since this is replaced by the - # jupyterhub.proxy.ConfigurableHTTPProxy class instead - proxy_pass https://127.0.0.1:8000; + proxy_pass http://127.0.0.1:8000; proxy_set_header X-Real-IP $remote_addr; proxy_set_header Host $host; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; From 3a71e900d70e11765653e06128afd40d7aac54d3 Mon Sep 17 00:00:00 2001 From: flongford Date: Thu, 6 Jan 2022 17:30:46 -0300 Subject: [PATCH 26/64] FIX: set LocalProcessSpawner.server upon instantiation from User attribute --- remoteappmanager/jupyterhub/spawners.py | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/remoteappmanager/jupyterhub/spawners.py b/remoteappmanager/jupyterhub/spawners.py index de0c4b5c..950f6998 100644 --- a/remoteappmanager/jupyterhub/spawners.py +++ b/remoteappmanager/jupyterhub/spawners.py @@ -36,7 +36,11 @@ def cmd(self): else ["remoteappmanager"]) def __init__(self, **kwargs): - super(LocalProcessSpawner, self).__init__(**kwargs) + super(BaseSpawner, self).__init__(**kwargs) + # FIXME: this is a workaround to breaking changes that were introduced + # in jupyterhub v0.8.1 and assumes the self.user.server attribute + # has a non-None value + self.server = self.user.server # We can obtain a reference to the JupyterHub.proxy object # through the tornado settings passed onto the User proxy = self.user.settings.get('proxy') @@ -45,22 +49,14 @@ def __init__(self, **kwargs): self.proxy_auth_token = proxy.auth_token def get_args(self): - # FIXME: we can't call the super get_args method before starting the - # spawner, a bug exists in jupyterhub v0.8.1 that means it cannot - # handle an unassigned self.port attribute. - # Note that this workaround assumes the self.user.server attribute - # has a non-None value - args = [] - args.extend(self.args) + args = super(BaseSpawner, self).get_args() + args.append('--user="{}"'.format( self.user.name)) args.append('--ip="{}"'.format( self.ip or "127.0.0.1")) - args.append('--port="{}"'.format( - self.user.server.port)) - args.append('--base-urlpath="{}"'.format( self.user.server.base_url)) From 1ad490a407fd823759e938fc5da25320a25aa7b6 Mon Sep 17 00:00:00 2001 From: flongford Date: Thu, 6 Jan 2022 18:02:24 -0300 Subject: [PATCH 27/64] FIX: avoid recursion loop between Spawner and User server assignments --- remoteappmanager/jupyterhub/spawners.py | 16 ++++++++++------ .../jupyterhub/tests/test_spawners.py | 8 ++++++++ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/remoteappmanager/jupyterhub/spawners.py b/remoteappmanager/jupyterhub/spawners.py index 950f6998..a6b04ad0 100644 --- a/remoteappmanager/jupyterhub/spawners.py +++ b/remoteappmanager/jupyterhub/spawners.py @@ -2,10 +2,11 @@ import escapism import string -from traitlets import Unicode +from traitlets import Unicode, default from tornado import gen from jupyterhub.spawner import LocalProcessSpawner +from jupyterhub import orm class BaseSpawner(LocalProcessSpawner): @@ -26,11 +27,14 @@ class BaseSpawner(LocalProcessSpawner): #: (along with ConfigurableHTTPProxy.auth_token) proxy_auth_token = Unicode(config=True) + @default('ip') + def _ip_default(self): + return "127.0.0.1" + @property def cmd(self): """Overrides the base class traitlet so that we take full control of the spawned command according to user admin status""" - return (["remoteappadmin"] if self.user.admin is True else ["remoteappmanager"]) @@ -40,7 +44,10 @@ def __init__(self, **kwargs): # FIXME: this is a workaround to breaking changes that were introduced # in jupyterhub v0.8.1 and assumes the self.user.server attribute # has a non-None value - self.server = self.user.server + if not self.server: + self.server = self.db.query(orm.Server).filter( + orm.Server.base_url == self.user.server.base_url + ) # We can obtain a reference to the JupyterHub.proxy object # through the tornado settings passed onto the User proxy = self.user.settings.get('proxy') @@ -54,9 +61,6 @@ def get_args(self): args.append('--user="{}"'.format( self.user.name)) - args.append('--ip="{}"'.format( - self.ip or "127.0.0.1")) - args.append('--base-urlpath="{}"'.format( self.user.server.base_url)) diff --git a/remoteappmanager/jupyterhub/tests/test_spawners.py b/remoteappmanager/jupyterhub/tests/test_spawners.py index d8467189..07e41c2c 100644 --- a/remoteappmanager/jupyterhub/tests/test_spawners.py +++ b/remoteappmanager/jupyterhub/tests/test_spawners.py @@ -45,6 +45,14 @@ def new_spawner(spawner_class): # Mock db db = mock.Mock() + db.query().filter = mock.Mock( + return_value=orm.Server( + proto="http", + ip="127.0.0.1", + port=12345, + base_url="/foo/bar/" + ) + ) # Mock user user = mock.Mock() From 6ed6dc6eb2f3c16e3484a02b7da3bc88aff38104 Mon Sep 17 00:00:00 2001 From: flongford Date: Thu, 6 Jan 2022 23:20:41 -0300 Subject: [PATCH 28/64] FIX: amend Spawner test suite to instantiate from orm.Server instance as in production --- remoteappmanager/jupyterhub/spawners.py | 47 ++++++------------- .../jupyterhub/tests/test_spawners.py | 19 ++++---- 2 files changed, 24 insertions(+), 42 deletions(-) diff --git a/remoteappmanager/jupyterhub/spawners.py b/remoteappmanager/jupyterhub/spawners.py index a6b04ad0..d2e568ea 100644 --- a/remoteappmanager/jupyterhub/spawners.py +++ b/remoteappmanager/jupyterhub/spawners.py @@ -2,11 +2,10 @@ import escapism import string -from traitlets import Unicode, default +from traitlets import Any, Unicode from tornado import gen from jupyterhub.spawner import LocalProcessSpawner -from jupyterhub import orm class BaseSpawner(LocalProcessSpawner): @@ -14,23 +13,13 @@ class BaseSpawner(LocalProcessSpawner): the actual spawners """ + #: The instance of the JupyterHub Proxy. + #: We use Any in agreement with base class practice. + proxy = Any() + #: The path of the configuration file for the cmd executable config_file_path = Unicode(config=True) - #: The URL for JupyterHub's Proxy API server. We currently expect - #: this to be set manually in the jupyterhub_config.py file - #: (along with ConfigurableHTTPProxy.api_url) - proxy_api_url = Unicode(config=True) - - #: The token for JupyterHub's Proxy API server. We currently expect - #: this to be set manually in the jupyterhub_config.py file - #: (along with ConfigurableHTTPProxy.auth_token) - proxy_auth_token = Unicode(config=True) - - @default('ip') - def _ip_default(self): - return "127.0.0.1" - @property def cmd(self): """Overrides the base class traitlet so that we take full control @@ -41,34 +30,28 @@ def cmd(self): def __init__(self, **kwargs): super(BaseSpawner, self).__init__(**kwargs) - # FIXME: this is a workaround to breaking changes that were introduced - # in jupyterhub v0.8.1 and assumes the self.user.server attribute - # has a non-None value - if not self.server: - self.server = self.db.query(orm.Server).filter( - orm.Server.base_url == self.user.server.base_url - ) # We can obtain a reference to the JupyterHub.proxy object # through the tornado settings passed onto the User - proxy = self.user.settings.get('proxy') - if proxy is not None: - self.proxy_api_url = proxy.api_url - self.proxy_auth_token = proxy.auth_token + self.proxy = self.user.settings.get('proxy') def get_args(self): - args = super(BaseSpawner, self).get_args() + args = super().get_args() + + # Handle no default IP value (removed in jupyterhub v0.8.0) + if not self.ip: + args.append('--ip="127.0.0.1"') args.append('--user="{}"'.format( self.user.name)) args.append('--base-urlpath="{}"'.format( - self.user.server.base_url)) + self.server.base_url)) args.append("--cookie_name={}".format( - self.user.server.cookie_name)) + self.server.cookie_name)) args.append("--proxy-api-url={}".format( - self.proxy_api_url)) + self.proxy.api_url)) args.append("--logout_url={}".format( self.authenticator.logout_url( @@ -84,7 +67,7 @@ def get_args(self): def get_env(self): env = super().get_env() - env["PROXY_API_TOKEN"] = self.proxy_auth_token + env["PROXY_API_TOKEN"] = self.proxy.auth_token env.update(_docker_envvars()) return env diff --git a/remoteappmanager/jupyterhub/tests/test_spawners.py b/remoteappmanager/jupyterhub/tests/test_spawners.py index 07e41c2c..5c17f02c 100644 --- a/remoteappmanager/jupyterhub/tests/test_spawners.py +++ b/remoteappmanager/jupyterhub/tests/test_spawners.py @@ -40,19 +40,12 @@ def new_spawner(spawner_class): proto="http", ip="127.0.0.2", port=31337, - base_url="/" + base_url="/", + cookie_name='my_cookie' ) # Mock db db = mock.Mock() - db.query().filter = mock.Mock( - return_value=orm.Server( - proto="http", - ip="127.0.0.1", - port=12345, - base_url="/foo/bar/" - ) - ) # Mock user user = mock.Mock() @@ -60,7 +53,6 @@ def new_spawner(spawner_class): user.url = 'http://my-callback.com' user.admin = False user.state = None - user.server = generic_server user.settings = { 'proxy': proxy.ConfigurableHTTPProxy( api_url="http://127.0.0.1:12345/foo/bar/", @@ -82,9 +74,16 @@ def new_spawner(spawner_class): return_value='/logout_test') authenticator.login_service = 'TEST' + # Mocks instantiating Spawner instance via an orm.Spawner instance + # in the User._new_spawner method, as would happen in production + orm_spawner = orm.Spawner( + name='', + server=generic_server + ) return spawner_class( db=db, user=user, + orm_spawner=orm_spawner, hub=hub, authenticator=authenticator ) From 47a75f242887d9083304ae48c839721e2a57ac88 Mon Sep 17 00:00:00 2001 From: flongford Date: Thu, 6 Jan 2022 23:23:41 -0300 Subject: [PATCH 29/64] CLN: tidy up jupyterhub config --- jupyterhub/jupyterhub_config.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/jupyterhub/jupyterhub_config.py b/jupyterhub/jupyterhub_config.py index 7de464dd..fc389c5b 100644 --- a/jupyterhub/jupyterhub_config.py +++ b/jupyterhub/jupyterhub_config.py @@ -13,8 +13,6 @@ c.ConfigurableHTTPProxy.command = [ 'configurable-http-proxy', f'--default-target=http://{c.JupyterHub.hub_ip}:8081'] -c.ConfigurableHTTPProxy.api_url = "http://127.0.0.1:8001" -c.ConfigurableHTTPProxy.auth_token = new_token() c.JupyterHub.logo_file = os.path.join( os.path.dirname(os.path.dirname(os.path.realpath(__file__))), @@ -38,13 +36,6 @@ # file for the spawned service, instead of using the defaults. # c.Spawner.config_file_path = 'remoteappmanager_config.py' - # As of jupyterhub>0.8.0, there's no convenient way to obtain a - # reference between the Spawner and ConfigurableHTTProxy objects - # during runtime, so we simply assign common traits once and do - # not expect them to change - c.Spawner.proxy_api_url = c.ConfigurableHTTPProxy.api_url - c.Spawner.proxy_auth_token = c.ConfigurableHTTPProxy.auth_token - # FIXME: replace me with other authenticator (e.g. GitHub OAuth...) c.JupyterHub.authenticator_class = ( 'remoteappmanager.jupyterhub.auth.WorldAuthenticator') From 50939f8f4e4cf82fc5e273aecefe28e539d9ebd1 Mon Sep 17 00:00:00 2001 From: flongford Date: Thu, 6 Jan 2022 23:58:35 -0300 Subject: [PATCH 30/64] EXP: attempt to fix redirect loop by patching RequestHandler headers --- remoteappmanager/base_application.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/remoteappmanager/base_application.py b/remoteappmanager/base_application.py index 565f7f1e..9dea80e2 100644 --- a/remoteappmanager/base_application.py +++ b/remoteappmanager/base_application.py @@ -8,6 +8,7 @@ from jupyterhub._version import __version__, _check_version from tornado.httpclient import AsyncHTTPClient from tornadowebapi.registry import Registry +from tornado.web import RequestHandler from remoteappmanager.db.interfaces import ABCDatabase from remoteappmanager.logging.logging_mixin import LoggingMixin @@ -88,6 +89,7 @@ def __init__(self, handlers = self._get_handlers() super().__init__(handlers, **settings) + self.patch_default_headers() # Initializers @default("container_manager") @@ -214,6 +216,16 @@ def check_hub_version(self): hub_version = resp.headers.get('X-JupyterHub-Version') _check_version(hub_version, __version__, self.log) + def patch_default_headers(self): + if hasattr(RequestHandler, '_orig_set_default_headers'): + return + RequestHandler._orig_set_default_headers = RequestHandler.set_default_headers + def set_jupyterhub_header(self): + self._orig_set_default_headers() + self.set_header('X-JupyterHub-Version', __version__) + + RequestHandler.set_default_headers = set_jupyterhub_header + # Private def _add_demo_apps(self, user): """Grant access to any demo applications provided for user""" From 498b980e1ea662df6468fbf4406c055634ec2875 Mon Sep 17 00:00:00 2001 From: flongford Date: Fri, 7 Jan 2022 00:03:25 -0300 Subject: [PATCH 31/64] STY: flake8 fixes --- jupyterhub/jupyterhub_config.py | 3 +-- remoteappmanager/base_application.py | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/jupyterhub/jupyterhub_config.py b/jupyterhub/jupyterhub_config.py index fc389c5b..4c3f943a 100644 --- a/jupyterhub/jupyterhub_config.py +++ b/jupyterhub/jupyterhub_config.py @@ -1,7 +1,6 @@ # Configuration file for jupyterhub. import os from jupyter_client.localinterfaces import public_ips -from jupyterhub.utils import new_token c = get_config() # noqa @@ -16,7 +15,7 @@ c.JupyterHub.logo_file = os.path.join( os.path.dirname(os.path.dirname(os.path.realpath(__file__))), - 'remoteappmanager/static/images/header_logo.png' + 'remoteappmanager/static/images/header_logo.Ëšpng' ) # Choose between system-user mode and virtual-user mode diff --git a/remoteappmanager/base_application.py b/remoteappmanager/base_application.py index 9dea80e2..8dd4ba67 100644 --- a/remoteappmanager/base_application.py +++ b/remoteappmanager/base_application.py @@ -219,7 +219,8 @@ def check_hub_version(self): def patch_default_headers(self): if hasattr(RequestHandler, '_orig_set_default_headers'): return - RequestHandler._orig_set_default_headers = RequestHandler.set_default_headers + RequestHandler._orig_set_default_headers = RequestHandler.set_default_headers # noqa: E501 + def set_jupyterhub_header(self): self._orig_set_default_headers() self.set_header('X-JupyterHub-Version', __version__) From 5cc5478e9193b7583309326458a0a0885471df3d Mon Sep 17 00:00:00 2001 From: flongford Date: Fri, 7 Jan 2022 11:29:00 -0300 Subject: [PATCH 32/64] FIX: use correct assignment of cookie name from JupyterHub --- remoteappmanager/jupyterhub/spawners.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/remoteappmanager/jupyterhub/spawners.py b/remoteappmanager/jupyterhub/spawners.py index d2e568ea..d8b6d140 100644 --- a/remoteappmanager/jupyterhub/spawners.py +++ b/remoteappmanager/jupyterhub/spawners.py @@ -48,7 +48,7 @@ def get_args(self): self.server.base_url)) args.append("--cookie_name={}".format( - self.server.cookie_name)) + self.hub.cookie_name)) args.append("--proxy-api-url={}".format( self.proxy.api_url)) From c2f8bce5cb00416852c81c19f309af0d04a0aa32 Mon Sep 17 00:00:00 2001 From: flongford Date: Fri, 7 Jan 2022 11:55:45 -0300 Subject: [PATCH 33/64] FIX: use correct env variable for EnvironmentConfig.hub_prefix --- remoteappmanager/environment_config.py | 4 ++-- remoteappmanager/tests/utils.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/remoteappmanager/environment_config.py b/remoteappmanager/environment_config.py index 79d58b82..d14d092d 100644 --- a/remoteappmanager/environment_config.py +++ b/remoteappmanager/environment_config.py @@ -18,7 +18,7 @@ class EnvironmentConfig(HasTraits): hub_host = Unicode(help="The url of the jupyterhub server") # This is a url path that sends the request to jupyterhub. - # It's normally /hub/. Originates from JUPYTERHUB_SERVICE_PREFIX + # It's normally /hub/. Originates from JUPYTERHUB_BASE_URL hub_prefix = Unicode(help="The url prefix of the jupyterhub") # This is a full url to reach the hub api (e.g. for authentication check) @@ -36,7 +36,7 @@ def parse_config(self): "JUPYTERHUB_API_TOKEN": 'jpy_api_token', "PROXY_API_TOKEN": 'proxy_api_token', "JUPYTERHUB_HOST": 'hub_host', - "JUPYTERHUB_SERVICE_PREFIX": 'hub_prefix', + "JUPYTERHUB_BASE_URL": 'hub_prefix', "JUPYTERHUB_API_URL": 'hub_api_url' } for envname, traitlet_name in mapping.items(): diff --git a/remoteappmanager/tests/utils.py b/remoteappmanager/tests/utils.py index 76a776ee..904f6dcc 100644 --- a/remoteappmanager/tests/utils.py +++ b/remoteappmanager/tests/utils.py @@ -25,7 +25,7 @@ 'JUPYTERHUB_API_TOKEN': 'jpy_token', 'PROXY_API_TOKEN': 'proxy_token', 'JUPYTERHUB_HOST': '', - 'JUPYTERHUB_SERVICE_PREFIX': '/hub/', + 'JUPYTERHUB_BASE_URL': '/hub/', 'JUPYTERHUB_API_URL': 'http://172.17.5.167:8081/hub/api', } @@ -56,7 +56,7 @@ def basic_environment_config(): config.proxy_api_token = env_config['PROXY_API_TOKEN'] config.jpy_api_token = env_config['JUPYTERHUB_API_TOKEN'] config.hub_host = env_config['JUPYTERHUB_HOST'] - config.hub_prefix = env_config['JUPYTERHUB_SERVICE_PREFIX'] + config.hub_prefix = env_config['JUPYTERHUB_BASE_URL'] config.hub_api_url = env_config['JUPYTERHUB_API_URL'] return config From 0032c50d2c08eb4ad2921e79431df0af096ac2be Mon Sep 17 00:00:00 2001 From: flongford Date: Fri, 7 Jan 2022 13:07:36 -0300 Subject: [PATCH 34/64] FIX: more parsing of hub_prefix argument back to CommandLineConfig --- remoteappmanager/command_line_config.py | 4 ++++ remoteappmanager/environment_config.py | 5 ----- remoteappmanager/jupyterhub/spawners.py | 3 +++ remoteappmanager/jupyterhub/tests/test_spawners.py | 4 ++-- remoteappmanager/tests/test_environment_config.py | 1 - remoteappmanager/tests/utils.py | 3 +-- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/remoteappmanager/command_line_config.py b/remoteappmanager/command_line_config.py index 96b26fdd..d088be67 100644 --- a/remoteappmanager/command_line_config.py +++ b/remoteappmanager/command_line_config.py @@ -24,6 +24,10 @@ class CommandLineConfig(HasTraits): # typically, it's /user/username base_urlpath = Unicode(help="The base url where the server resides") + # This is a url path that sends the request to jupyterhub. + # It's normally /hub/. + hub_prefix = Unicode(help="The url prefix of the jupyterhub") + # The full URL where to access the reverse proxy API. proxy_api_url = Unicode(help="The url of the reverse proxy API") diff --git a/remoteappmanager/environment_config.py b/remoteappmanager/environment_config.py index d14d092d..feea91b4 100644 --- a/remoteappmanager/environment_config.py +++ b/remoteappmanager/environment_config.py @@ -17,10 +17,6 @@ class EnvironmentConfig(HasTraits): # Originates from JUPYTERHUB_HOST hub_host = Unicode(help="The url of the jupyterhub server") - # This is a url path that sends the request to jupyterhub. - # It's normally /hub/. Originates from JUPYTERHUB_BASE_URL - hub_prefix = Unicode(help="The url prefix of the jupyterhub") - # This is a full url to reach the hub api (e.g. for authentication check) # Originates from JUPYTERHUB_API_URL hub_api_url = Unicode(help="The url of the jupyterhub REST API") @@ -36,7 +32,6 @@ def parse_config(self): "JUPYTERHUB_API_TOKEN": 'jpy_api_token', "PROXY_API_TOKEN": 'proxy_api_token', "JUPYTERHUB_HOST": 'hub_host', - "JUPYTERHUB_BASE_URL": 'hub_prefix', "JUPYTERHUB_API_URL": 'hub_api_url' } for envname, traitlet_name in mapping.items(): diff --git a/remoteappmanager/jupyterhub/spawners.py b/remoteappmanager/jupyterhub/spawners.py index d8b6d140..e53398ca 100644 --- a/remoteappmanager/jupyterhub/spawners.py +++ b/remoteappmanager/jupyterhub/spawners.py @@ -47,6 +47,9 @@ def get_args(self): args.append('--base-urlpath="{}"'.format( self.server.base_url)) + args.append('--hub-prefix={}'.format( + self.hub.base_url)) + args.append("--cookie_name={}".format( self.hub.cookie_name)) diff --git a/remoteappmanager/jupyterhub/tests/test_spawners.py b/remoteappmanager/jupyterhub/tests/test_spawners.py index 5c17f02c..0460c322 100644 --- a/remoteappmanager/jupyterhub/tests/test_spawners.py +++ b/remoteappmanager/jupyterhub/tests/test_spawners.py @@ -41,7 +41,7 @@ def new_spawner(spawner_class): ip="127.0.0.2", port=31337, base_url="/", - cookie_name='my_cookie' + cookie_name='cookie' ) # Mock db @@ -65,7 +65,7 @@ def new_spawner(spawner_class): ip="127.0.0.2", proto="http", port=31337, - base_url="/" + base_url="/hub/", ) # Mock authenticator diff --git a/remoteappmanager/tests/test_environment_config.py b/remoteappmanager/tests/test_environment_config.py index 72cd57d7..6ecfc895 100644 --- a/remoteappmanager/tests/test_environment_config.py +++ b/remoteappmanager/tests/test_environment_config.py @@ -14,6 +14,5 @@ def test_init(self): self.assertEqual(config.jpy_api_token, "jpy_token") self.assertEqual(config.proxy_api_token, "proxy_token") self.assertEqual(config.hub_host, "") - self.assertEqual(config.hub_prefix, '/hub/') self.assertEqual( config.hub_api_url, 'http://172.17.5.167:8081/hub/api') diff --git a/remoteappmanager/tests/utils.py b/remoteappmanager/tests/utils.py index 904f6dcc..db751e94 100644 --- a/remoteappmanager/tests/utils.py +++ b/remoteappmanager/tests/utils.py @@ -16,6 +16,7 @@ "port": 57022, "cookie-name": "jupyter-hub-token-johndoe", "base-urlpath": "\"/user/johndoe/\"", + "hub-prefix": '/hub/', "proxy-api-url": "http://192.168.100.99/proxy/api", "ip": "127.0.0.1", "config-file": fixtures.get("remoteappmanager_config.py") @@ -25,7 +26,6 @@ 'JUPYTERHUB_API_TOKEN': 'jpy_token', 'PROXY_API_TOKEN': 'proxy_token', 'JUPYTERHUB_HOST': '', - 'JUPYTERHUB_BASE_URL': '/hub/', 'JUPYTERHUB_API_URL': 'http://172.17.5.167:8081/hub/api', } @@ -56,7 +56,6 @@ def basic_environment_config(): config.proxy_api_token = env_config['PROXY_API_TOKEN'] config.jpy_api_token = env_config['JUPYTERHUB_API_TOKEN'] config.hub_host = env_config['JUPYTERHUB_HOST'] - config.hub_prefix = env_config['JUPYTERHUB_BASE_URL'] config.hub_api_url = env_config['JUPYTERHUB_API_URL'] return config From de60c6ddb0d9c4f2946dc7adf029d9014ecab851 Mon Sep 17 00:00:00 2001 From: flongford Date: Fri, 7 Jan 2022 13:09:04 -0300 Subject: [PATCH 35/64] CLN: remove typo in jupyterhub_config.py --- jupyterhub/jupyterhub_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jupyterhub/jupyterhub_config.py b/jupyterhub/jupyterhub_config.py index 4c3f943a..568705b2 100644 --- a/jupyterhub/jupyterhub_config.py +++ b/jupyterhub/jupyterhub_config.py @@ -15,7 +15,7 @@ c.JupyterHub.logo_file = os.path.join( os.path.dirname(os.path.dirname(os.path.realpath(__file__))), - 'remoteappmanager/static/images/header_logo.Ëšpng' + 'remoteappmanager/static/images/header_logo.png' ) # Choose between system-user mode and virtual-user mode From e3df7448711fd09ea54614f1cb75980ea1a24d86 Mon Sep 17 00:00:00 2001 From: flongford Date: Fri, 7 Jan 2022 13:37:20 -0300 Subject: [PATCH 36/64] FIX: correctly parse cookie-name argument --- remoteappmanager/jupyterhub/spawners.py | 6 ++++-- remoteappmanager/jupyterhub/tests/test_spawners.py | 4 ++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/remoteappmanager/jupyterhub/spawners.py b/remoteappmanager/jupyterhub/spawners.py index e53398ca..d83358ae 100644 --- a/remoteappmanager/jupyterhub/spawners.py +++ b/remoteappmanager/jupyterhub/spawners.py @@ -1,6 +1,7 @@ import os import escapism import string +from urllib.parse import quote from traitlets import Any, Unicode from tornado import gen @@ -50,8 +51,9 @@ def get_args(self): args.append('--hub-prefix={}'.format( self.hub.base_url)) - args.append("--cookie_name={}".format( - self.hub.cookie_name)) + args.append("--cookie-name={}-{}".format( + self.hub.cookie_name, + quote(self.user.name, safe=''))) args.append("--proxy-api-url={}".format( self.proxy.api_url)) diff --git a/remoteappmanager/jupyterhub/tests/test_spawners.py b/remoteappmanager/jupyterhub/tests/test_spawners.py index 0460c322..5357b300 100644 --- a/remoteappmanager/jupyterhub/tests/test_spawners.py +++ b/remoteappmanager/jupyterhub/tests/test_spawners.py @@ -98,14 +98,18 @@ def test_args(self): path = fixtures.get("remoteappmanager_config.py") self.spawner.config_file_path = path args = self.spawner.get_args() + user = self.spawner.user.name self.assertIn("--ip=\"127.0.0.1\"", args) + self.assertIn(f"--cookie-name=jupyter-hub-token-{user}", args) self.assertIn("--proxy-api-url=http://127.0.0.1:12345/foo/bar/", args) self.assertIn("--config-file={}".format(path), args) self.assertIn("--base-urlpath=\"/\"", args) def test_args_without_config_file_path(self): args = self.spawner.get_args() + user = self.spawner.user.name self.assertIn("--ip=\"127.0.0.1\"", args) + self.assertIn(f"--cookie-name=jupyter-hub-token-{user}", args) self.assertIn("--proxy-api-url=http://127.0.0.1:12345/foo/bar/", args) self.assertFalse(any("--config-file=" in arg for arg in args)) self.assertIn("--base-urlpath=\"/\"", args) From 1ce224077465733ec837bb6b82eae99c2d13cfd4 Mon Sep 17 00:00:00 2001 From: flongford Date: Fri, 7 Jan 2022 13:55:28 -0300 Subject: [PATCH 37/64] FIX: use cookie-name as of jupyterhub 0.8.0 --- remoteappmanager/jupyterhub/spawners.py | 5 ++--- remoteappmanager/jupyterhub/tests/test_spawners.py | 6 ++---- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/remoteappmanager/jupyterhub/spawners.py b/remoteappmanager/jupyterhub/spawners.py index d83358ae..05a0e794 100644 --- a/remoteappmanager/jupyterhub/spawners.py +++ b/remoteappmanager/jupyterhub/spawners.py @@ -51,9 +51,8 @@ def get_args(self): args.append('--hub-prefix={}'.format( self.hub.base_url)) - args.append("--cookie-name={}-{}".format( - self.hub.cookie_name, - quote(self.user.name, safe=''))) + args.append("--cookie-name={}".format( + self.hub.cookie_name)) args.append("--proxy-api-url={}".format( self.proxy.api_url)) diff --git a/remoteappmanager/jupyterhub/tests/test_spawners.py b/remoteappmanager/jupyterhub/tests/test_spawners.py index 5357b300..92e7d3b0 100644 --- a/remoteappmanager/jupyterhub/tests/test_spawners.py +++ b/remoteappmanager/jupyterhub/tests/test_spawners.py @@ -98,18 +98,16 @@ def test_args(self): path = fixtures.get("remoteappmanager_config.py") self.spawner.config_file_path = path args = self.spawner.get_args() - user = self.spawner.user.name self.assertIn("--ip=\"127.0.0.1\"", args) - self.assertIn(f"--cookie-name=jupyter-hub-token-{user}", args) + self.assertIn(f"--cookie-name=jupyter-hub-token", args) self.assertIn("--proxy-api-url=http://127.0.0.1:12345/foo/bar/", args) self.assertIn("--config-file={}".format(path), args) self.assertIn("--base-urlpath=\"/\"", args) def test_args_without_config_file_path(self): args = self.spawner.get_args() - user = self.spawner.user.name self.assertIn("--ip=\"127.0.0.1\"", args) - self.assertIn(f"--cookie-name=jupyter-hub-token-{user}", args) + self.assertIn(f"--cookie-name=jupyter-hub-token", args) self.assertIn("--proxy-api-url=http://127.0.0.1:12345/foo/bar/", args) self.assertFalse(any("--config-file=" in arg for arg in args)) self.assertIn("--base-urlpath=\"/\"", args) From d0a3f19de016e2b687e40b112fb772b1a2543c3e Mon Sep 17 00:00:00 2001 From: flongford Date: Fri, 7 Jan 2022 16:50:11 -0300 Subject: [PATCH 38/64] EXP: run selenium tests in debug mode --- jupyterhub/start.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jupyterhub/start.sh b/jupyterhub/start.sh index 20b622a8..07ef6225 100644 --- a/jupyterhub/start.sh +++ b/jupyterhub/start.sh @@ -5,4 +5,4 @@ else SCRIPT_DIR=`dirname "$cwd/$0"` fi export PATH=$SCRIPT_DIR/../node_modules/.bin/:$PATH -jupyterhub --ssl-key test.key --ssl-cert test.crt +jupyterhub --ssl-key test.key --ssl-cert test.crt --debug From 3c28a92ee90fc49b0e6dc09c86b3f5a6a23e4233 Mon Sep 17 00:00:00 2001 From: flongford Date: Fri, 7 Jan 2022 16:54:04 -0300 Subject: [PATCH 39/64] STY: flake8 fixes --- remoteappmanager/jupyterhub/spawners.py | 1 - remoteappmanager/jupyterhub/tests/test_spawners.py | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/remoteappmanager/jupyterhub/spawners.py b/remoteappmanager/jupyterhub/spawners.py index 05a0e794..673680a3 100644 --- a/remoteappmanager/jupyterhub/spawners.py +++ b/remoteappmanager/jupyterhub/spawners.py @@ -1,7 +1,6 @@ import os import escapism import string -from urllib.parse import quote from traitlets import Any, Unicode from tornado import gen diff --git a/remoteappmanager/jupyterhub/tests/test_spawners.py b/remoteappmanager/jupyterhub/tests/test_spawners.py index 92e7d3b0..4d5cf136 100644 --- a/remoteappmanager/jupyterhub/tests/test_spawners.py +++ b/remoteappmanager/jupyterhub/tests/test_spawners.py @@ -99,7 +99,7 @@ def test_args(self): self.spawner.config_file_path = path args = self.spawner.get_args() self.assertIn("--ip=\"127.0.0.1\"", args) - self.assertIn(f"--cookie-name=jupyter-hub-token", args) + self.assertIn("--cookie-name=jupyter-hub-token", args) self.assertIn("--proxy-api-url=http://127.0.0.1:12345/foo/bar/", args) self.assertIn("--config-file={}".format(path), args) self.assertIn("--base-urlpath=\"/\"", args) @@ -107,7 +107,7 @@ def test_args(self): def test_args_without_config_file_path(self): args = self.spawner.get_args() self.assertIn("--ip=\"127.0.0.1\"", args) - self.assertIn(f"--cookie-name=jupyter-hub-token", args) + self.assertIn("--cookie-name=jupyter-hub-token", args) self.assertIn("--proxy-api-url=http://127.0.0.1:12345/foo/bar/", args) self.assertFalse(any("--config-file=" in arg for arg in args)) self.assertIn("--base-urlpath=\"/\"", args) From 435cf94973818aaa35110e5971331eedc0c35475 Mon Sep 17 00:00:00 2001 From: flongford Date: Wed, 12 Jan 2022 10:28:55 -0300 Subject: [PATCH 40/64] EXP: change url for tornado RedirectHandlers in application --- remoteappmanager/admin_application.py | 2 +- remoteappmanager/application.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/remoteappmanager/admin_application.py b/remoteappmanager/admin_application.py index dfe5fb00..03d07ff5 100644 --- a/remoteappmanager/admin_application.py +++ b/remoteappmanager/admin_application.py @@ -22,5 +22,5 @@ def _web_handlers(self): return [ (base_urlpath, AdminHomeHandler), (base_urlpath.rstrip('/'), - web.RedirectHandler, {"url": base_urlpath}), + web.RedirectHandler, {"url": '/hub/user-redirect'}), ] diff --git a/remoteappmanager/application.py b/remoteappmanager/application.py index bf0b96b6..8cb874b0 100644 --- a/remoteappmanager/application.py +++ b/remoteappmanager/application.py @@ -22,5 +22,5 @@ def _web_handlers(self): )+"/?", RegisterContainerHandler), (base_urlpath, UserHomeHandler), (base_urlpath.rstrip('/'), web.RedirectHandler, { - "url": base_urlpath}), + "url": '/hub/user-redirect'}), ] From 740598afe3c58f0f94e321bb1506c4554be49d1f Mon Sep 17 00:00:00 2001 From: flongford Date: Wed, 12 Jan 2022 10:54:46 -0300 Subject: [PATCH 41/64] EXP: revert change to RedirectHandler for admin application to avoid unit test failure (for now) --- remoteappmanager/admin_application.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/remoteappmanager/admin_application.py b/remoteappmanager/admin_application.py index 03d07ff5..dfe5fb00 100644 --- a/remoteappmanager/admin_application.py +++ b/remoteappmanager/admin_application.py @@ -22,5 +22,5 @@ def _web_handlers(self): return [ (base_urlpath, AdminHomeHandler), (base_urlpath.rstrip('/'), - web.RedirectHandler, {"url": '/hub/user-redirect'}), + web.RedirectHandler, {"url": base_urlpath}), ] From 92cb197e63252b6dfadb19a47b5621e44034e25b Mon Sep 17 00:00:00 2001 From: flongford Date: Wed, 12 Jan 2022 11:53:20 -0300 Subject: [PATCH 42/64] EXP: revert change to RedirectHandler for user application to avoid unit test failures (for now) --- remoteappmanager/application.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/remoteappmanager/application.py b/remoteappmanager/application.py index 8cb874b0..bf0b96b6 100644 --- a/remoteappmanager/application.py +++ b/remoteappmanager/application.py @@ -22,5 +22,5 @@ def _web_handlers(self): )+"/?", RegisterContainerHandler), (base_urlpath, UserHomeHandler), (base_urlpath.rstrip('/'), web.RedirectHandler, { - "url": '/hub/user-redirect'}), + "url": base_urlpath}), ] From e14fce448b480951bb9c14631308ac0755a83073 Mon Sep 17 00:00:00 2001 From: flongford Date: Sun, 22 May 2022 13:45:18 +0100 Subject: [PATCH 43/64] DEV: enable user authentication with remoteappmanager via JupyterHub as an OAuth provider --- remoteappmanager/base_application.py | 38 +++++++++++++++++-- remoteappmanager/handlers/base_handler.py | 17 ++++++--- .../handlers/handler_authenticator.py | 25 +++++++++++- 3 files changed, 71 insertions(+), 9 deletions(-) diff --git a/remoteappmanager/base_application.py b/remoteappmanager/base_application.py index 8dd4ba67..e0caa746 100644 --- a/remoteappmanager/base_application.py +++ b/remoteappmanager/base_application.py @@ -1,11 +1,13 @@ import importlib +from secrets import token_urlsafe +from urllib.parse import urlparse -from remoteappmanager.handlers.handler_authenticator import HubAuthenticator from traitlets import Instance, default from tornado import web, gen import tornado.ioloop from jupyterhub._version import __version__, _check_version +from jupyterhub.services.auth import HubOAuth, HubOAuthCallbackHandler from tornado.httpclient import AsyncHTTPClient from tornadowebapi.registry import Registry from tornado.web import RequestHandler @@ -13,6 +15,7 @@ from remoteappmanager.db.interfaces import ABCDatabase from remoteappmanager.logging.logging_mixin import LoggingMixin from remoteappmanager.docker.container_manager import ContainerManager +from remoteappmanager.handlers.handler_authenticator import HubOAuthenticator from remoteappmanager.user import User from remoteappmanager.traitlets import as_dict from remoteappmanager.services.hub import Hub @@ -40,8 +43,12 @@ class BaseApplication(web.Application, LoggingMixin): reverse_proxy = Instance(ReverseProxy) #: API access to jupyterhub (for auth requests) + #: Deprecated as of remoteappmanager 2.2.0 hub = Instance(Hub) + #: API access to JupyterHub for OAuth requests + hub_auth = Instance(HubOAuth) + #: Manages the docker interface container_manager = Instance(ContainerManager) @@ -86,11 +93,21 @@ def __init__(self, self._command_line_config.base_urlpath + "static/") settings['X-JupyterHub-Version'] = __version__ + # Since we are using JupyterHub as an OAuth provider we'll also + # need to create our own cookies to keep track of user logins + settings['cookie_secret'] = token_urlsafe(64) + handlers = self._get_handlers() super().__init__(handlers, **settings) self.patch_default_headers() + # Add callback url to enable OAuth with JupyterHub + self.add_handlers(r".*$", [( + urlparse(self.hub_auth.oauth_redirect_uri).path, + HubOAuthCallbackHandler + )]) + # Initializers @default("container_manager") def _container_manager_default(self): @@ -110,9 +127,24 @@ def _reverse_proxy_default(self): api_token=self.environment_config.proxy_api_token, ) + @default("hub_auth") + def _hub_auth_default(self): + """Initializes the HubOAuth instance used to authenticate with + JupyterHub. + """ + return HubOAuth( + hub_api_url=self.environment_config.hub_api_url, + api_token=self.environment_config.jpy_api_token, + base_url=self.command_line_config.base_urlpath, + hub_prefix=self.command_line_config.hub_prefix, + ) + @default("hub") def _hub_default(self): - """Initializes the Hub instance.""" + """Initializes the Hub instance. + + Deprecated as of remoteappmanager 2.2.0 + """ return Hub(endpoint_url=self.environment_config.hub_api_url, api_token=self.environment_config.jpy_api_token, ) @@ -158,7 +190,7 @@ def _user_default(self): @default("registry") def _registry_default(self): reg = Registry() - reg.authenticator = HubAuthenticator + reg.authenticator = HubOAuthenticator for resource_handler in self._webapi_resources(): reg.register(resource_handler) return reg diff --git a/remoteappmanager/handlers/base_handler.py b/remoteappmanager/handlers/base_handler.py index 298afbee..a75446ae 100644 --- a/remoteappmanager/handlers/base_handler.py +++ b/remoteappmanager/handlers/base_handler.py @@ -3,22 +3,29 @@ from tornado import web, gen +from jupyterhub.services.auth import HubOAuthenticated + from remoteappmanager.logging.logging_mixin import LoggingMixin -from remoteappmanager.handlers.handler_authenticator import HubAuthenticator +from remoteappmanager.handlers.handler_authenticator import HubOAuthenticator -class BaseHandler(web.RequestHandler, LoggingMixin): +class BaseHandler(web.RequestHandler, HubOAuthenticated, LoggingMixin): """Base class for the request handler.""" #: The authenticator that is used to recognize the user. - authenticator = HubAuthenticator + authenticator = HubOAuthenticator + @web.authenticated @gen.coroutine def prepare(self): """Runs before any specific handler. """ - # Authenticate the user against the hub. We can't use get_current_user - # because we want to do it asynchronously. + # Note that this additional authentication layer is only + # required to support the tornadowebapi framework - the + # actual handshake with JupyterHub is handled by the + # HubOAuthenticated mixin + + # Authenticate the user against the hub. self.current_user = yield self.authenticator.authenticate(self) def render(self, template_name, **kwargs): diff --git a/remoteappmanager/handlers/handler_authenticator.py b/remoteappmanager/handlers/handler_authenticator.py index cdc72b8c..1f039dc8 100644 --- a/remoteappmanager/handlers/handler_authenticator.py +++ b/remoteappmanager/handlers/handler_authenticator.py @@ -14,7 +14,10 @@ def authenticate(cls, handler): class HubAuthenticator(Authenticator): """Authenticator that uses the remote JupyterHub to validate - the request.""" + the request. + + Deprecated as of remoteappmanager 2.2.0 + """ @classmethod @gen.coroutine def authenticate(cls, handler): @@ -32,3 +35,23 @@ def authenticate(cls, handler): user = webapp.user return user + + +class HubOAuthenticator(Authenticator): + """Authenticator that uses the remote JupyterHub as an OAuth + provider to validate the request.""" + @classmethod + @gen.coroutine + def authenticate(cls, handler): + # Authenticate the user against the hub. Expects the handler + # to inherit from the jupyterhub.services.auth.HubOAuthenticated + # mixin + webapp = handler.application + hub_auth = webapp.hub + user = None + + user_data = hub_auth.get_user(handler) + if user_data.get('name', '') == webapp.user.name: + user = webapp.user + + return user From c45a7684b21230850ec6838bd7e4cf2947094c41 Mon Sep 17 00:00:00 2001 From: flongford Date: Sun, 22 May 2022 14:21:01 +0100 Subject: [PATCH 44/64] CLN: move connection with JupyterHub OAuth service into remoteappmanager.services.hub module --- remoteappmanager/base_application.py | 33 ++------ .../handlers/handler_authenticator.py | 4 +- remoteappmanager/services/hub.py | 75 ++++++++++++++++++- remoteappmanager/tests/mocking/dummy.py | 1 + 4 files changed, 83 insertions(+), 30 deletions(-) diff --git a/remoteappmanager/base_application.py b/remoteappmanager/base_application.py index e0caa746..d85ac8b5 100644 --- a/remoteappmanager/base_application.py +++ b/remoteappmanager/base_application.py @@ -1,13 +1,11 @@ import importlib from secrets import token_urlsafe -from urllib.parse import urlparse from traitlets import Instance, default from tornado import web, gen import tornado.ioloop from jupyterhub._version import __version__, _check_version -from jupyterhub.services.auth import HubOAuth, HubOAuthCallbackHandler from tornado.httpclient import AsyncHTTPClient from tornadowebapi.registry import Registry from tornado.web import RequestHandler @@ -18,7 +16,7 @@ from remoteappmanager.handlers.handler_authenticator import HubOAuthenticator from remoteappmanager.user import User from remoteappmanager.traitlets import as_dict -from remoteappmanager.services.hub import Hub +from remoteappmanager.services.hub import HubOAuth from remoteappmanager.services.reverse_proxy import ReverseProxy @@ -42,12 +40,8 @@ class BaseApplication(web.Application, LoggingMixin): #: API access to the configurable-http-proxy reverse_proxy = Instance(ReverseProxy) - #: API access to jupyterhub (for auth requests) - #: Deprecated as of remoteappmanager 2.2.0 - hub = Instance(Hub) - #: API access to JupyterHub for OAuth requests - hub_auth = Instance(HubOAuth) + hub = Instance(HubOAuth) #: Manages the docker interface container_manager = Instance(ContainerManager) @@ -102,12 +96,6 @@ def __init__(self, super().__init__(handlers, **settings) self.patch_default_headers() - # Add callback url to enable OAuth with JupyterHub - self.add_handlers(r".*$", [( - urlparse(self.hub_auth.oauth_redirect_uri).path, - HubOAuthCallbackHandler - )]) - # Initializers @default("container_manager") def _container_manager_default(self): @@ -127,8 +115,8 @@ def _reverse_proxy_default(self): api_token=self.environment_config.proxy_api_token, ) - @default("hub_auth") - def _hub_auth_default(self): + @default("hub") + def _hub_default(self): """Initializes the HubOAuth instance used to authenticate with JupyterHub. """ @@ -139,16 +127,6 @@ def _hub_auth_default(self): hub_prefix=self.command_line_config.hub_prefix, ) - @default("hub") - def _hub_default(self): - """Initializes the Hub instance. - - Deprecated as of remoteappmanager 2.2.0 - """ - return Hub(endpoint_url=self.environment_config.hub_api_url, - api_token=self.environment_config.jpy_api_token, - ) - @default("db") def _db_default(self): """Initializes the database connection.""" @@ -297,6 +275,7 @@ def _web_handlers(self): def _get_handlers(self): """Returns the registered handlers""" base_urlpath = self.command_line_config.base_urlpath + web_auth = self.hub.callback_handlers() web_api = self.registry.api_handlers(base_urlpath) web_handlers = self._web_handlers() - return web_api+web_handlers + return web_auth+web_api+web_handlers diff --git a/remoteappmanager/handlers/handler_authenticator.py b/remoteappmanager/handlers/handler_authenticator.py index 1f039dc8..79250b34 100644 --- a/remoteappmanager/handlers/handler_authenticator.py +++ b/remoteappmanager/handlers/handler_authenticator.py @@ -47,10 +47,10 @@ def authenticate(cls, handler): # to inherit from the jupyterhub.services.auth.HubOAuthenticated # mixin webapp = handler.application - hub_auth = webapp.hub + hub = webapp.hub user = None - user_data = hub_auth.get_user(handler) + user_data = hub.get_user(handler) if user_data.get('name', '') == webapp.user.name: user = webapp.user diff --git a/remoteappmanager/services/hub.py b/remoteappmanager/services/hub.py index cd611063..370756a1 100644 --- a/remoteappmanager/services/hub.py +++ b/remoteappmanager/services/hub.py @@ -1,9 +1,12 @@ -from urllib.parse import quote +from urllib.parse import quote, urlparse from tornado import gen, escape from tornado.httpclient import AsyncHTTPClient from traitlets import HasTraits, Unicode +from jupyterhub.services.auth import HubOAuth as _HubOAuth +from jupyterhub.services.auth import HubOAuthCallbackHandler + from remoteappmanager.logging.logging_mixin import LoggingMixin from remoteappmanager.utils import url_path_join @@ -69,3 +72,73 @@ def verify_token(self, cookie_name, encrypted_cookie): return escape.json_decode(r.body) else: return {} + + +class HubOAuth(LoggingMixin, HasTraits): + """Provides access to JupyterHub OAuth authenticator services.""" + + #: The url at which the Hub can be reached + hub_api_url = Unicode() + + #: The api token to authenticate the request + api_token = Unicode() + + base_url = Unicode() + + hub_prefix = Unicode() + + def __init__(self, *args, **kwargs): + """Initializes the hub connection object.""" + super().__init__(*args, **kwargs) + + if not self.api_token: + message = ("Invalid API Token to initialise " + "the hub connection.") + self.log.error(message) + raise ValueError(message) + + if not self.hub_api_url: + message = ("Invalid API url to initialise " + "the hub connection.") + self.log.error(message) + raise ValueError(message) + + if not self.base_url: + message = ("Invalid base url to query " + "the hub connection.") + self.log.error(message) + raise ValueError(message) + + self._hub_auth = _HubOAuth( + hub_api_url=self.hub_api_url, + api_token=self.api_token, + base_url=self.base_url, + hub_prefix=self.hub_prefix, + ) + + @gen.coroutine + def get_user(self, handler): + """Verify the authentication details present in the handler + session + + Parameters + ---------- + handler : tornado.web.RequestHandler + Request handler to be inspected. Expected to have inherited + from the JupyterHub HubOAuthenticated mixin + + Returns + ------- + user_data : dict + If authentication is successful, user_data contains the user's + information from jupyterhub associated with the given encrypted + cookie. Otherwise the dictionary is empty. + """ + return self._hub_auth.get_user(handler) + + def callback_handlers(self): + # Add callback url to enable OAuth with JupyterHub + return [( + urlparse(self._hub_auth.oauth_redirect_uri).path, + HubOAuthCallbackHandler + )] diff --git a/remoteappmanager/tests/mocking/dummy.py b/remoteappmanager/tests/mocking/dummy.py index cd7287ab..2ffdddce 100644 --- a/remoteappmanager/tests/mocking/dummy.py +++ b/remoteappmanager/tests/mocking/dummy.py @@ -219,6 +219,7 @@ def create_hub(params=None): hub = Hub(**params) hub.verify_token = mock_coro_factory({}) + hub.callback_handlers = lambda: [] return hub From 1879aa1b57c1b6ac78540be0e876600717a0f248 Mon Sep 17 00:00:00 2001 From: flongford Date: Sun, 22 May 2022 14:23:09 +0100 Subject: [PATCH 45/64] TST: include mocking of new HubOAuth.get_user method for testing --- remoteappmanager/tests/mocking/dummy.py | 1 + 1 file changed, 1 insertion(+) diff --git a/remoteappmanager/tests/mocking/dummy.py b/remoteappmanager/tests/mocking/dummy.py index 2ffdddce..4e72b1d6 100644 --- a/remoteappmanager/tests/mocking/dummy.py +++ b/remoteappmanager/tests/mocking/dummy.py @@ -219,6 +219,7 @@ def create_hub(params=None): hub = Hub(**params) hub.verify_token = mock_coro_factory({}) + hub.get_user = mock_coro_factory({}) hub.callback_handlers = lambda: [] return hub From 8af268482384342744d2511dc0063c58310b7930 Mon Sep 17 00:00:00 2001 From: flongford Date: Mon, 23 May 2022 10:08:43 +0100 Subject: [PATCH 46/64] DEV: fold HubOAuth class into Hub --- remoteappmanager/base_application.py | 9 ++-- remoteappmanager/services/hub.py | 61 +++++++--------------------- 2 files changed, 20 insertions(+), 50 deletions(-) diff --git a/remoteappmanager/base_application.py b/remoteappmanager/base_application.py index d85ac8b5..09645676 100644 --- a/remoteappmanager/base_application.py +++ b/remoteappmanager/base_application.py @@ -16,7 +16,7 @@ from remoteappmanager.handlers.handler_authenticator import HubOAuthenticator from remoteappmanager.user import User from remoteappmanager.traitlets import as_dict -from remoteappmanager.services.hub import HubOAuth +from remoteappmanager.services.hub import Hub from remoteappmanager.services.reverse_proxy import ReverseProxy @@ -41,7 +41,7 @@ class BaseApplication(web.Application, LoggingMixin): reverse_proxy = Instance(ReverseProxy) #: API access to JupyterHub for OAuth requests - hub = Instance(HubOAuth) + hub = Instance(Hub) #: Manages the docker interface container_manager = Instance(ContainerManager) @@ -117,10 +117,10 @@ def _reverse_proxy_default(self): @default("hub") def _hub_default(self): - """Initializes the HubOAuth instance used to authenticate with + """Initializes the Hub instance used to authenticate with JupyterHub. """ - return HubOAuth( + return Hub( hub_api_url=self.environment_config.hub_api_url, api_token=self.environment_config.jpy_api_token, base_url=self.command_line_config.base_urlpath, @@ -275,6 +275,7 @@ def _web_handlers(self): def _get_handlers(self): """Returns the registered handlers""" base_urlpath = self.command_line_config.base_urlpath + # Must include callback handlers to complete OAuth flow web_auth = self.hub.callback_handlers() web_api = self.registry.api_handlers(base_urlpath) web_handlers = self._web_handlers() diff --git a/remoteappmanager/services/hub.py b/remoteappmanager/services/hub.py index 370756a1..49066d90 100644 --- a/remoteappmanager/services/hub.py +++ b/remoteappmanager/services/hub.py @@ -4,8 +4,8 @@ from tornado.httpclient import AsyncHTTPClient from traitlets import HasTraits, Unicode -from jupyterhub.services.auth import HubOAuth as _HubOAuth -from jupyterhub.services.auth import HubOAuthCallbackHandler +from jupyterhub.services.auth import ( + HubOAuth, HubOAuthCallbackHandler) from remoteappmanager.logging.logging_mixin import LoggingMixin from remoteappmanager.utils import url_path_join @@ -20,6 +20,10 @@ class Hub(LoggingMixin, HasTraits): #: The api token to authenticate the request api_token = Unicode() + base_url = Unicode() + + hub_prefix = Unicode() + def __init__(self, *args, **kwargs): """Initializes the hub connection object.""" super().__init__(*args, **kwargs) @@ -36,6 +40,13 @@ def __init__(self, *args, **kwargs): self.log.error(message) raise ValueError(message) + self._hub_auth = HubOAuth( + hub_api_url=self.endpoint_url, + api_token=self.api_token, + base_url=self.base_url, + hub_prefix=self.hub_prefix, + ) + @gen.coroutine def verify_token(self, cookie_name, encrypted_cookie): """Verify the authentication token and grants access to the user @@ -73,49 +84,6 @@ def verify_token(self, cookie_name, encrypted_cookie): else: return {} - -class HubOAuth(LoggingMixin, HasTraits): - """Provides access to JupyterHub OAuth authenticator services.""" - - #: The url at which the Hub can be reached - hub_api_url = Unicode() - - #: The api token to authenticate the request - api_token = Unicode() - - base_url = Unicode() - - hub_prefix = Unicode() - - def __init__(self, *args, **kwargs): - """Initializes the hub connection object.""" - super().__init__(*args, **kwargs) - - if not self.api_token: - message = ("Invalid API Token to initialise " - "the hub connection.") - self.log.error(message) - raise ValueError(message) - - if not self.hub_api_url: - message = ("Invalid API url to initialise " - "the hub connection.") - self.log.error(message) - raise ValueError(message) - - if not self.base_url: - message = ("Invalid base url to query " - "the hub connection.") - self.log.error(message) - raise ValueError(message) - - self._hub_auth = _HubOAuth( - hub_api_url=self.hub_api_url, - api_token=self.api_token, - base_url=self.base_url, - hub_prefix=self.hub_prefix, - ) - @gen.coroutine def get_user(self, handler): """Verify the authentication details present in the handler @@ -137,7 +105,8 @@ def get_user(self, handler): return self._hub_auth.get_user(handler) def callback_handlers(self): - # Add callback url to enable OAuth with JupyterHub + """Add callback url to enable OAuth with JupyterHub + """ return [( urlparse(self._hub_auth.oauth_redirect_uri).path, HubOAuthCallbackHandler From 28656decf5be7cf9e50713d45d4d56d3470790f1 Mon Sep 17 00:00:00 2001 From: flongford Date: Mon, 23 May 2022 10:17:47 +0100 Subject: [PATCH 47/64] CLN: minor tidy up --- remoteappmanager/base_application.py | 2 +- remoteappmanager/handlers/base_handler.py | 3 ++- remoteappmanager/services/hub.py | 7 ++++++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/remoteappmanager/base_application.py b/remoteappmanager/base_application.py index 09645676..62e17565 100644 --- a/remoteappmanager/base_application.py +++ b/remoteappmanager/base_application.py @@ -40,7 +40,7 @@ class BaseApplication(web.Application, LoggingMixin): #: API access to the configurable-http-proxy reverse_proxy = Instance(ReverseProxy) - #: API access to JupyterHub for OAuth requests + #: API access to JupyterHub for requests hub = Instance(Hub) #: Manages the docker interface diff --git a/remoteappmanager/handlers/base_handler.py b/remoteappmanager/handlers/base_handler.py index a75446ae..fc0624ff 100644 --- a/remoteappmanager/handlers/base_handler.py +++ b/remoteappmanager/handlers/base_handler.py @@ -12,7 +12,8 @@ class BaseHandler(web.RequestHandler, HubOAuthenticated, LoggingMixin): """Base class for the request handler.""" - #: The authenticator that is used to recognize the user. + #: The authenticator that is used by tornadowebapi to recognize + #: the user. authenticator = HubOAuthenticator @web.authenticated diff --git a/remoteappmanager/services/hub.py b/remoteappmanager/services/hub.py index 49066d90..f7528633 100644 --- a/remoteappmanager/services/hub.py +++ b/remoteappmanager/services/hub.py @@ -14,14 +14,16 @@ class Hub(LoggingMixin, HasTraits): """Provides access to JupyterHub authenticator services.""" - #: The url at which the Hub can be reached + #: The url at which the JupyterHub can be reached endpoint_url = Unicode() #: The api token to authenticate the request api_token = Unicode() + #: The base urlpath for the current user. base_url = Unicode() + #: The url prefix of the JupyterHub hub_prefix = Unicode() def __init__(self, *args, **kwargs): @@ -40,6 +42,7 @@ def __init__(self, *args, **kwargs): self.log.error(message) raise ValueError(message) + # Create connection to JupyterHub's OAuth services self._hub_auth = HubOAuth( hub_api_url=self.endpoint_url, api_token=self.api_token, @@ -52,6 +55,8 @@ def verify_token(self, cookie_name, encrypted_cookie): """Verify the authentication token and grants access to the user if verified. + Deprecated as of remoteappmanager 2.2.0 + Parameters ---------- cookie_name: str From 3b1d1c326574bc79c7e6777d545d158c11a9a869 Mon Sep 17 00:00:00 2001 From: flongford Date: Mon, 23 May 2022 12:26:48 +0100 Subject: [PATCH 48/64] FIX: typo in keyword argument --- remoteappmanager/base_application.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/remoteappmanager/base_application.py b/remoteappmanager/base_application.py index 62e17565..925832a9 100644 --- a/remoteappmanager/base_application.py +++ b/remoteappmanager/base_application.py @@ -121,7 +121,7 @@ def _hub_default(self): JupyterHub. """ return Hub( - hub_api_url=self.environment_config.hub_api_url, + endpoint_url=self.environment_config.hub_api_url, api_token=self.environment_config.jpy_api_token, base_url=self.command_line_config.base_urlpath, hub_prefix=self.command_line_config.hub_prefix, From 795f8fa69117f9996b629f8c3c3856ba4d8b4d88 Mon Sep 17 00:00:00 2001 From: flongford Date: Mon, 23 May 2022 13:03:47 +0100 Subject: [PATCH 49/64] FIX: minor fixes to auth process --- remoteappmanager/handlers/base_handler.py | 4 +++- remoteappmanager/handlers/handler_authenticator.py | 2 +- remoteappmanager/services/hub.py | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/remoteappmanager/handlers/base_handler.py b/remoteappmanager/handlers/base_handler.py index fc0624ff..a7942ddf 100644 --- a/remoteappmanager/handlers/base_handler.py +++ b/remoteappmanager/handlers/base_handler.py @@ -9,7 +9,7 @@ from remoteappmanager.handlers.handler_authenticator import HubOAuthenticator -class BaseHandler(web.RequestHandler, HubOAuthenticated, LoggingMixin): +class BaseHandler(HubOAuthenticated, web.RequestHandler, LoggingMixin): """Base class for the request handler.""" #: The authenticator that is used by tornadowebapi to recognize @@ -28,6 +28,8 @@ def prepare(self): # Authenticate the user against the hub. self.current_user = yield self.authenticator.authenticate(self) + if self.current_user is None: + self.log.warn("Failed to authenticate user session with JupyterHub") def render(self, template_name, **kwargs): """Reimplements render to pass well known information to the rendering diff --git a/remoteappmanager/handlers/handler_authenticator.py b/remoteappmanager/handlers/handler_authenticator.py index 79250b34..98d58439 100644 --- a/remoteappmanager/handlers/handler_authenticator.py +++ b/remoteappmanager/handlers/handler_authenticator.py @@ -50,7 +50,7 @@ def authenticate(cls, handler): hub = webapp.hub user = None - user_data = hub.get_user(handler) + user_data = yield hub.get_user(handler) if user_data.get('name', '') == webapp.user.name: user = webapp.user diff --git a/remoteappmanager/services/hub.py b/remoteappmanager/services/hub.py index f7528633..d2f97a32 100644 --- a/remoteappmanager/services/hub.py +++ b/remoteappmanager/services/hub.py @@ -105,7 +105,7 @@ def get_user(self, handler): user_data : dict If authentication is successful, user_data contains the user's information from jupyterhub associated with the given encrypted - cookie. Otherwise the dictionary is empty. + cookie. Otherwise the dictionary is empty. """ return self._hub_auth.get_user(handler) From e487a86f59f443afaf7cfc1ac54c2e22a7cf4461 Mon Sep 17 00:00:00 2001 From: flongford Date: Mon, 23 May 2022 13:50:03 +0100 Subject: [PATCH 50/64] TST: replace mocked calls to verify_token with get_user --- remoteappmanager/handlers/admin/tests/test_admin_handlers.py | 4 ++-- remoteappmanager/handlers/tests/test_base_handler.py | 2 +- .../handlers/tests/test_register_container_handler.py | 4 ++-- remoteappmanager/handlers/tests/test_user_home_handler.py | 4 ++-- remoteappmanager/tests/mocking/dummy.py | 2 +- remoteappmanager/webapi/admin/tests/test_accounting.py | 4 ++-- remoteappmanager/webapi/admin/tests/test_application.py | 4 ++-- remoteappmanager/webapi/admin/tests/test_container.py | 2 +- remoteappmanager/webapi/admin/tests/test_stats.py | 2 +- remoteappmanager/webapi/admin/tests/test_user.py | 4 ++-- remoteappmanager/webapi/tests/test_container.py | 4 ++-- 11 files changed, 18 insertions(+), 18 deletions(-) diff --git a/remoteappmanager/handlers/admin/tests/test_admin_handlers.py b/remoteappmanager/handlers/admin/tests/test_admin_handlers.py index a6c7e570..6215ea87 100644 --- a/remoteappmanager/handlers/admin/tests/test_admin_handlers.py +++ b/remoteappmanager/handlers/admin/tests/test_admin_handlers.py @@ -12,7 +12,7 @@ class TestBaseAccess(AsyncHTTPTestCase): def get_app(self): app = dummy.create_admin_application() - app.hub.verify_token.return_value = { + app.hub.get_user.return_value = { 'pending': None, 'name': app.settings['user'], 'admin': False, @@ -30,7 +30,7 @@ def test_access(self): self.assertIn(self.body_string, str(res.body)) def test_failed_auth(self): - self._app.hub.verify_token.return_value = {} + self._app.hub.get_user.return_value = {} with ExpectLog('tornado.access', ''): res = self.fetch( self.url, diff --git a/remoteappmanager/handlers/tests/test_base_handler.py b/remoteappmanager/handlers/tests/test_base_handler.py index badbb8ea..719ae900 100644 --- a/remoteappmanager/handlers/tests/test_base_handler.py +++ b/remoteappmanager/handlers/tests/test_base_handler.py @@ -17,7 +17,7 @@ def get_app(self): file_config = self.get_file_config() app = dummy.create_application(file_config=file_config) - app.hub.verify_token.return_value = { + app.hub.get_user.return_value = { 'pending': None, 'name': app.settings['user'], 'admin': False, diff --git a/remoteappmanager/handlers/tests/test_register_container_handler.py b/remoteappmanager/handlers/tests/test_register_container_handler.py index 96d8cf6a..c1f5b100 100644 --- a/remoteappmanager/handlers/tests/test_register_container_handler.py +++ b/remoteappmanager/handlers/tests/test_register_container_handler.py @@ -9,7 +9,7 @@ class TestRegisterContainerHandler(TempMixin, AsyncHTTPTestCase): def get_app(self): app = dummy.create_application() - app.hub.verify_token.return_value = { + app.hub.get_user.return_value = { 'pending': None, 'name': app.settings['user'], 'admin': False, @@ -37,7 +37,7 @@ def test_present_url_id(self): self.assertTrue(self._app.reverse_proxy.register.called) def test_failed_auth(self): - self._app.hub.verify_token.return_value = {} + self._app.hub.get_user.return_value = {} res = self.fetch("/user/johndoe/containers/url_id", headers={ "Cookie": "jupyter-hub-token-johndoe=foo" diff --git a/remoteappmanager/handlers/tests/test_user_home_handler.py b/remoteappmanager/handlers/tests/test_user_home_handler.py index 362cf2fb..bb51d0d7 100644 --- a/remoteappmanager/handlers/tests/test_user_home_handler.py +++ b/remoteappmanager/handlers/tests/test_user_home_handler.py @@ -7,7 +7,7 @@ class TestUserHomeHandler(TempMixin, AsyncHTTPTestCase): def get_app(self): app = dummy.create_application() - app.hub.verify_token.return_value = { + app.hub.get_user.return_value = { 'pending': None, 'name': app.settings['user'], 'admin': False, @@ -25,7 +25,7 @@ def test_home(self): self.assertIn("applist", str(res.body)) def test_failed_auth(self): - self._app.hub.verify_token.return_value = {} + self._app.hub.get_user.return_value = {} with ExpectLog('tornado.access', ''): res = self.fetch("/user/johndoe/", headers={ diff --git a/remoteappmanager/tests/mocking/dummy.py b/remoteappmanager/tests/mocking/dummy.py index 4e72b1d6..3c1f3637 100644 --- a/remoteappmanager/tests/mocking/dummy.py +++ b/remoteappmanager/tests/mocking/dummy.py @@ -312,7 +312,7 @@ def _create_application_from_class( if command_line_config is None: command_line_config = basic_command_line_config() - if environment_config is not None: + if environment_config is None: environment_config = basic_environment_config() app = application_class( diff --git a/remoteappmanager/webapi/admin/tests/test_accounting.py b/remoteappmanager/webapi/admin/tests/test_accounting.py index 9f4ab24c..eabb41bc 100644 --- a/remoteappmanager/webapi/admin/tests/test_accounting.py +++ b/remoteappmanager/webapi/admin/tests/test_accounting.py @@ -11,7 +11,7 @@ class TestAccounting(WebAPITestCase): def get_app(self): app = dummy.create_admin_application() - app.hub.verify_token.return_value = { + app.hub.get_user.return_value = { 'pending': None, 'name': app.settings['user'], 'admin': False, @@ -126,7 +126,7 @@ def test_items(self): self.assertEqual(len(data["identifiers"]), 2) def test_delete_failed_auth(self): - self._app.hub.verify_token.return_value = {} + self._app.hub.get_user.return_value = {} self.delete("/user/johndoe/api/v1/accounting/cbaee2e8ef414f9fb0f1c97416b8aa6c/", # noqa httpstatus.NOT_FOUND) diff --git a/remoteappmanager/webapi/admin/tests/test_application.py b/remoteappmanager/webapi/admin/tests/test_application.py index f4be9b11..dc2de7e6 100644 --- a/remoteappmanager/webapi/admin/tests/test_application.py +++ b/remoteappmanager/webapi/admin/tests/test_application.py @@ -10,7 +10,7 @@ class TestApplication(WebAPITestCase): def get_app(self): app = dummy.create_admin_application() - app.hub.verify_token.return_value = { + app.hub.get_user.return_value = { 'pending': None, 'name': app.settings['user'], 'admin': False, @@ -63,7 +63,7 @@ def test_create_invalid_representation(self): httpstatus.BAD_REQUEST) def test_delete_failed_auth(self): - self._app.hub.verify_token.return_value = {} + self._app.hub.get_user.return_value = {} self.delete("/user/johndoe/api/v1/applications/0/", httpstatus.NOT_FOUND) diff --git a/remoteappmanager/webapi/admin/tests/test_container.py b/remoteappmanager/webapi/admin/tests/test_container.py index eff382be..defd64ea 100644 --- a/remoteappmanager/webapi/admin/tests/test_container.py +++ b/remoteappmanager/webapi/admin/tests/test_container.py @@ -10,7 +10,7 @@ class TestContainer(WebAPITestCase): def get_app(self): app = dummy.create_admin_application() - app.hub.verify_token.return_value = { + app.hub.get_user.return_value = { 'pending': None, 'name': app.settings['user'], 'admin': False, diff --git a/remoteappmanager/webapi/admin/tests/test_stats.py b/remoteappmanager/webapi/admin/tests/test_stats.py index fd0c1079..8f06cbab 100644 --- a/remoteappmanager/webapi/admin/tests/test_stats.py +++ b/remoteappmanager/webapi/admin/tests/test_stats.py @@ -8,7 +8,7 @@ class TestStats(WebAPITestCase): def get_app(self): app = dummy.create_admin_application() - app.hub.verify_token.return_value = { + app.hub.get_user.return_value = { 'pending': None, 'name': app.settings['user'], 'admin': False, diff --git a/remoteappmanager/webapi/admin/tests/test_user.py b/remoteappmanager/webapi/admin/tests/test_user.py index 731b9848..1785057d 100644 --- a/remoteappmanager/webapi/admin/tests/test_user.py +++ b/remoteappmanager/webapi/admin/tests/test_user.py @@ -10,7 +10,7 @@ class TestUser(WebAPITestCase): def get_app(self): app = dummy.create_admin_application() - app.hub.verify_token.return_value = { + app.hub.get_user.return_value = { 'pending': None, 'name': app.settings['user'], 'admin': False, @@ -62,7 +62,7 @@ def test_unable_to_create(self): httpstatus.INTERNAL_SERVER_ERROR) def test_delete_failed_auth(self): - self._app.hub.verify_token.return_value = {} + self._app.hub.get_user.return_value = {} self.delete("/user/johndoe/api/v1/users/0/", httpstatus.NOT_FOUND) diff --git a/remoteappmanager/webapi/tests/test_container.py b/remoteappmanager/webapi/tests/test_container.py index c2c726a8..c9932ae5 100644 --- a/remoteappmanager/webapi/tests/test_container.py +++ b/remoteappmanager/webapi/tests/test_container.py @@ -15,7 +15,7 @@ class TestContainer(WebAPITestCase): def get_app(self): app = dummy.create_application() - app.hub.verify_token.return_value = { + app.hub.get_user.return_value = { 'pending': None, 'name': app.settings['user'], 'admin': False, @@ -346,7 +346,7 @@ def test_post_start(self): self.assertTrue(self._app.reverse_proxy.register.called) def test_post_failed_auth(self): - self._app.hub.verify_token.return_value = {} + self._app.hub.get_user.return_value = {} self.post("/user/johndoe/api/v1/containers/", {"mapping_id": "b7ca425a51bf40acbd305b3f782714b6"}, From fb6d6d11fa792ef437939d9b8cafb7160b472477 Mon Sep 17 00:00:00 2001 From: flongford Date: Mon, 23 May 2022 13:54:22 +0100 Subject: [PATCH 51/64] STY: flake8 fix --- remoteappmanager/handlers/base_handler.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/remoteappmanager/handlers/base_handler.py b/remoteappmanager/handlers/base_handler.py index a7942ddf..af9884ab 100644 --- a/remoteappmanager/handlers/base_handler.py +++ b/remoteappmanager/handlers/base_handler.py @@ -29,7 +29,8 @@ def prepare(self): # Authenticate the user against the hub. self.current_user = yield self.authenticator.authenticate(self) if self.current_user is None: - self.log.warn("Failed to authenticate user session with JupyterHub") + self.log.warn( + "Failed to authenticate user session with JupyterHub") def render(self, template_name, **kwargs): """Reimplements render to pass well known information to the rendering From 5563cefcae8e33185d68840dfbee16c3db188b46 Mon Sep 17 00:00:00 2001 From: flongford Date: Mon, 23 May 2022 16:37:28 +0100 Subject: [PATCH 52/64] TST: fix python tests via mocking jupyterhub HubOAuth calls (for now) --- .../handlers/admin/tests/test_admin_handlers.py | 8 ++++++-- remoteappmanager/handlers/tests/test_base_handler.py | 10 ++++++++-- .../tests/test_register_container_handler.py | 12 ++++++++---- .../handlers/tests/test_user_home_handler.py | 8 ++++++-- remoteappmanager/jupyterhub/tests/test_spawners.py | 6 ++++-- 5 files changed, 32 insertions(+), 12 deletions(-) diff --git a/remoteappmanager/handlers/admin/tests/test_admin_handlers.py b/remoteappmanager/handlers/admin/tests/test_admin_handlers.py index 6215ea87..ae02f681 100644 --- a/remoteappmanager/handlers/admin/tests/test_admin_handlers.py +++ b/remoteappmanager/handlers/admin/tests/test_admin_handlers.py @@ -1,8 +1,12 @@ +from unittest import mock + from tornado.testing import AsyncHTTPTestCase, ExpectLog from remoteappmanager.tests.mocking import dummy +@mock.patch.dict('os.environ', {"JUPYTERHUB_CLIENT_ID": 'client-id'}) +@mock.patch('jupyterhub.services.auth.HubOAuth.get_user') class TestBaseAccess(AsyncHTTPTestCase): #: which url to poke url = "/user/johndoe" @@ -19,7 +23,7 @@ def get_app(self): 'server': app.settings['base_urlpath']} return app - def test_access(self): + def test_access(self, mock_get_user): res = self.fetch(self.url, headers={ "Cookie": "jupyter-hub-token-johndoe=foo" @@ -29,7 +33,7 @@ def test_access(self): self.assertEqual(res.code, 200) self.assertIn(self.body_string, str(res.body)) - def test_failed_auth(self): + def test_failed_auth(self, mock_get_user): self._app.hub.get_user.return_value = {} with ExpectLog('tornado.access', ''): res = self.fetch( diff --git a/remoteappmanager/handlers/tests/test_base_handler.py b/remoteappmanager/handlers/tests/test_base_handler.py index 719ae900..e62c9238 100644 --- a/remoteappmanager/handlers/tests/test_base_handler.py +++ b/remoteappmanager/handlers/tests/test_base_handler.py @@ -1,3 +1,5 @@ +from unittest import mock + from tornado.testing import AsyncHTTPTestCase, ExpectLog from remoteappmanager.file_config import FileConfig @@ -45,13 +47,15 @@ def test_home_internal_error(self): self.assertIn(" Ref.:", str(res.body)) +@mock.patch.dict('os.environ', {"JUPYTERHUB_CLIENT_ID": 'client-id'}) +@mock.patch('jupyterhub.services.auth.HubOAuth.get_user') class TestBaseHandlerDatabaseError(TestBaseHandler): def get_file_config(self): file_config = super().get_file_config() file_config.database_class = "remoteappmanager.db.orm.ORMDatabase" return file_config - def test_home_internal_error(self): + def test_home_internal_error(self, mock_get_user): with ExpectLog('tornado.application', ''), \ ExpectLog('tornado.access', ''): res = self.fetch("/user/johndoe/", @@ -66,13 +70,15 @@ def test_home_internal_error(self): str(res.body)) +@mock.patch.dict('os.environ', {"JUPYTERHUB_CLIENT_ID": 'client-id'}) +@mock.patch('jupyterhub.services.auth.HubOAuth.get_user') class TestBaseHandlerGATracking(TestBaseHandler): def get_file_config(self): file_config = super().get_file_config() file_config.ga_tracking_id = "UA-12345-6" return file_config - def test_ga_tracking(self): + def test_ga_tracking(self, mock_get_user): res = self.fetch("/user/johndoe/", headers={ "Cookie": "jupyter-hub-token-johndoe=foo" diff --git a/remoteappmanager/handlers/tests/test_register_container_handler.py b/remoteappmanager/handlers/tests/test_register_container_handler.py index c1f5b100..b475c2a4 100644 --- a/remoteappmanager/handlers/tests/test_register_container_handler.py +++ b/remoteappmanager/handlers/tests/test_register_container_handler.py @@ -1,3 +1,5 @@ +from unittest import mock + from tornado.testing import ExpectLog, AsyncHTTPTestCase from remoteappmanager.tests.mocking import dummy @@ -5,6 +7,8 @@ from remoteappmanager.tests.utils import mock_coro_factory +@mock.patch.dict('os.environ', {"JUPYTERHUB_CLIENT_ID": 'client-id'}) +@mock.patch('jupyterhub.services.auth.HubOAuth.get_user') class TestRegisterContainerHandler(TempMixin, AsyncHTTPTestCase): def get_app(self): @@ -16,7 +20,7 @@ def get_app(self): 'server': app.settings['base_urlpath']} return app - def test_absent_url_id(self): + def test_absent_url_id(self, mock_get_user): with ExpectLog('tornado.access', ''): res = self.fetch( "/user/johndoe/containers/99999/", @@ -25,7 +29,7 @@ def test_absent_url_id(self): self.assertEqual(res.code, 404) - def test_present_url_id(self): + def test_present_url_id(self, mock_get_user): res = self.fetch("/user/johndoe/containers/20dcb84cdbea4b1899447246789093d0/", # noqa headers={ "Cookie": "jupyter-hub-token-johndoe=foo" @@ -36,7 +40,7 @@ def test_present_url_id(self): self.assertEqual(res.code, 302) self.assertTrue(self._app.reverse_proxy.register.called) - def test_failed_auth(self): + def test_failed_auth(self, mock_get_user): self._app.hub.get_user.return_value = {} res = self.fetch("/user/johndoe/containers/url_id", headers={ @@ -49,7 +53,7 @@ def test_failed_auth(self): self.assertFalse(self._app.reverse_proxy.register.called) self.assertEqual(res.code, 302) - def test_failure_of_reverse_proxy(self): + def test_failure_of_reverse_proxy(self, mock_get_user): self._app.reverse_proxy.register = mock_coro_factory( side_effect=Exception("BOOM")) diff --git a/remoteappmanager/handlers/tests/test_user_home_handler.py b/remoteappmanager/handlers/tests/test_user_home_handler.py index bb51d0d7..1d1663dd 100644 --- a/remoteappmanager/handlers/tests/test_user_home_handler.py +++ b/remoteappmanager/handlers/tests/test_user_home_handler.py @@ -1,9 +1,13 @@ +from unittest import mock + from tornado.testing import AsyncHTTPTestCase, ExpectLog from remoteappmanager.tests.mocking import dummy from remoteappmanager.tests.temp_mixin import TempMixin +@mock.patch.dict('os.environ', {"JUPYTERHUB_CLIENT_ID": 'client-id'}) +@mock.patch('jupyterhub.services.auth.HubOAuth.get_user') class TestUserHomeHandler(TempMixin, AsyncHTTPTestCase): def get_app(self): app = dummy.create_application() @@ -14,7 +18,7 @@ def get_app(self): 'server': app.settings['base_urlpath']} return app - def test_home(self): + def test_home(self, mock_get_user): res = self.fetch("/user/johndoe/", headers={ "Cookie": "jupyter-hub-token-johndoe=foo" @@ -24,7 +28,7 @@ def test_home(self): self.assertEqual(res.code, 200) self.assertIn("applist", str(res.body)) - def test_failed_auth(self): + def test_failed_auth(self, mock_get_user): self._app.hub.get_user.return_value = {} with ExpectLog('tornado.access', ''): res = self.fetch("/user/johndoe/", diff --git a/remoteappmanager/jupyterhub/tests/test_spawners.py b/remoteappmanager/jupyterhub/tests/test_spawners.py index cb2de2df..e7baac74 100644 --- a/remoteappmanager/jupyterhub/tests/test_spawners.py +++ b/remoteappmanager/jupyterhub/tests/test_spawners.py @@ -84,7 +84,8 @@ def new_spawner(spawner_class): user=user, orm_spawner=orm_spawner, hub=hub, - authenticator=authenticator + authenticator=authenticator, + api_token="dummy_token" ) @@ -131,7 +132,8 @@ def test_env_has_docker_vars(self): def test_cmd_spawning(self): env = os.environ.copy() - env["PROXY_API_TOKEN"] = "dummy_token" + env["JUPYTERHUB_API_TOKEN"] = "dummy_token" + env["JUPYTERHUB_API_URL"] = 'http://172.17.5.167:8081/hub/api' path = fixtures.get("remoteappmanager_config.py") self.spawner.config_file_path = path From 73f4b634e242511857ce049d2bbd4c6836383be4 Mon Sep 17 00:00:00 2001 From: flongford Date: Mon, 23 May 2022 16:53:50 +0100 Subject: [PATCH 53/64] CLN: remove unnecessary dev changes --- dev-requirements.txt | 2 -- jupyterhub/jupyterhub_config.py | 13 +------------ 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index f06d137e..bf7cc541 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -1,5 +1,3 @@ flake8 coverage selenium==3.141.0 -notebook -jupyterhub-simplespawner diff --git a/jupyterhub/jupyterhub_config.py b/jupyterhub/jupyterhub_config.py index 568705b2..b975d5b0 100644 --- a/jupyterhub/jupyterhub_config.py +++ b/jupyterhub/jupyterhub_config.py @@ -9,17 +9,13 @@ c.JupyterHub.hub_ip = public_ips()[0] if len(public_ips()) else '127.0.0.1' -c.ConfigurableHTTPProxy.command = [ - 'configurable-http-proxy', - f'--default-target=http://{c.JupyterHub.hub_ip}:8081'] - c.JupyterHub.logo_file = os.path.join( os.path.dirname(os.path.dirname(os.path.realpath(__file__))), 'remoteappmanager/static/images/header_logo.png' ) # Choose between system-user mode and virtual-user mode -setting_mode = ('system_user', 'virtual_user', 'dummy_user')[1] +setting_mode = ('system_user', 'virtual_user')[1] if setting_mode == 'virtual_user': c.JupyterHub.spawner_class = ('remoteappmanager.jupyterhub.spawners.' @@ -44,10 +40,3 @@ c.JupyterHub.spawner_class = ('remoteappmanager.jupyterhub.spawners.' 'SystemUserSpawner') c.Authenticator.admin_users = {os.environ["USER"]} - -elif setting_mode == 'dummy_user': - # Make sure to run: `pip install dev-requirements.txt` first - c.JupyterHub.spawner_class = ( - 'simplespawner.SimpleLocalProcessSpawner') - c.JupyterHub.authenticator_class = ( - 'remoteappmanager.jupyterhub.auth.WorldAuthenticator') From c8cb0091411c780e8ca19a3d09354c5ddf7b7646 Mon Sep 17 00:00:00 2001 From: flongford Date: Tue, 24 May 2022 09:42:56 +0100 Subject: [PATCH 54/64] FIX: remove deprecated code from logout workaround --- remoteappmanager/jupyterhub/auth/simphony_remote_auth_mixin.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/remoteappmanager/jupyterhub/auth/simphony_remote_auth_mixin.py b/remoteappmanager/jupyterhub/auth/simphony_remote_auth_mixin.py index 1abb32eb..76521b0b 100644 --- a/remoteappmanager/jupyterhub/auth/simphony_remote_auth_mixin.py +++ b/remoteappmanager/jupyterhub/auth/simphony_remote_auth_mixin.py @@ -22,9 +22,6 @@ def get(self): yield gen.maybe_future(self.stop_single_user(user)) self.log.info("User logged out: %s", user.name) self.clear_login_cookie() - for name in user.other_user_cookies: - self.clear_login_cookie(name) - user.other_user_cookies = set([]) self.statsd.incr('logout') self.redirect(self.settings['login_url'], permanent=False) From 760768eb56016ff5917dfb0d5e895a458e042932 Mon Sep 17 00:00:00 2001 From: flongford Date: Tue, 24 May 2022 17:45:01 +0100 Subject: [PATCH 55/64] TST: fix admin login for selenium tests --- selenium_tests/AdminDriverTest.py | 1 - selenium_tests/test_spawner_options_form.py | 2 -- 2 files changed, 3 deletions(-) diff --git a/selenium_tests/AdminDriverTest.py b/selenium_tests/AdminDriverTest.py index 4efc372f..0175f9ac 100644 --- a/selenium_tests/AdminDriverTest.py +++ b/selenium_tests/AdminDriverTest.py @@ -14,7 +14,6 @@ def admin_login(self): self.click_first_element_located(By.ID, "start") self.click_first_element_located(By.CSS_SELECTOR, "input.btn") - self.click_first_element_located(By.ID, "start") def setUp(self): RemoteAppDriverTest.setUp(self) diff --git a/selenium_tests/test_spawner_options_form.py b/selenium_tests/test_spawner_options_form.py index f2a3d472..bfaf8ad8 100644 --- a/selenium_tests/test_spawner_options_form.py +++ b/selenium_tests/test_spawner_options_form.py @@ -10,7 +10,6 @@ def test_admin_login_default_session(self): self.click_first_element_located(By.ID, "start") self.click_first_element_located(By.CSS_SELECTOR, "input.btn") - self.click_first_element_located(By.ID, "start") self.wait_until_text_inside_element_located( By.CSS_SELECTOR, ".header", "ADMIN") @@ -22,7 +21,6 @@ def test_admin_login_user_session(self): self.click_first_element_located( By.CSS_SELECTOR, "#session_form > option:nth-child(2)") self.click_first_element_located(By.CSS_SELECTOR, "input.btn") - self.click_first_element_located(By.ID, "start") self.wait_until_text_inside_element_located( By.CSS_SELECTOR, ".header", "APPLICATIONS") From 9ecf5149b2c043203c4419cd08aef7993a0c5f88 Mon Sep 17 00:00:00 2001 From: flongford Date: Tue, 24 May 2022 17:47:32 +0100 Subject: [PATCH 56/64] FIX: error handling for shutting down server upon user logout --- .../jupyterhub/auth/simphony_remote_auth_mixin.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/remoteappmanager/jupyterhub/auth/simphony_remote_auth_mixin.py b/remoteappmanager/jupyterhub/auth/simphony_remote_auth_mixin.py index 76521b0b..866258ea 100644 --- a/remoteappmanager/jupyterhub/auth/simphony_remote_auth_mixin.py +++ b/remoteappmanager/jupyterhub/auth/simphony_remote_auth_mixin.py @@ -1,4 +1,5 @@ from tornado import gen +from tornado.httpclient import HTTPClientError from jupyterhub.handlers import LogoutHandler as _LogoutHandler from jupyterhub.handlers import LoginHandler @@ -19,7 +20,10 @@ def get(self): # once running on jupyterhub>=1.0.0 if user.admin and user.spawner is not None: self.log.info(f"Shutting down {user.name}'s server") - yield gen.maybe_future(self.stop_single_user(user)) + try: + yield gen.maybe_future(self.stop_single_user(user)) + except HTTPClientError: + self.log.warning("Failed to shut down server") self.log.info("User logged out: %s", user.name) self.clear_login_cookie() self.statsd.incr('logout') From dc24c2d290eb49d0b5716f0a3bf4ab94cc0ad6c6 Mon Sep 17 00:00:00 2001 From: flongford Date: Wed, 25 May 2022 14:48:52 +0100 Subject: [PATCH 57/64] CLN: separate JupyterHub authentication framework into separate method that can be mocked --- .../admin/tests/test_admin_handlers.py | 6 ++-- remoteappmanager/handlers/base_handler.py | 29 +++++++++++++++---- .../handlers/tests/test_base_handler.py | 8 ++--- .../tests/test_register_container_handler.py | 10 +++---- .../handlers/tests/test_user_home_handler.py | 6 ++-- 5 files changed, 39 insertions(+), 20 deletions(-) diff --git a/remoteappmanager/handlers/admin/tests/test_admin_handlers.py b/remoteappmanager/handlers/admin/tests/test_admin_handlers.py index ae02f681..fe01d77d 100644 --- a/remoteappmanager/handlers/admin/tests/test_admin_handlers.py +++ b/remoteappmanager/handlers/admin/tests/test_admin_handlers.py @@ -6,7 +6,7 @@ @mock.patch.dict('os.environ', {"JUPYTERHUB_CLIENT_ID": 'client-id'}) -@mock.patch('jupyterhub.services.auth.HubOAuth.get_user') +@mock.patch('remoteappmanager.handlers.base_handler.BaseHandler._verify_jupyterhub_oauth') #: noqa:501 class TestBaseAccess(AsyncHTTPTestCase): #: which url to poke url = "/user/johndoe" @@ -23,7 +23,7 @@ def get_app(self): 'server': app.settings['base_urlpath']} return app - def test_access(self, mock_get_user): + def test_access(self, mock_verify): res = self.fetch(self.url, headers={ "Cookie": "jupyter-hub-token-johndoe=foo" @@ -33,7 +33,7 @@ def test_access(self, mock_get_user): self.assertEqual(res.code, 200) self.assertIn(self.body_string, str(res.body)) - def test_failed_auth(self, mock_get_user): + def test_failed_auth(self, mock_verify): self._app.hub.get_user.return_value = {} with ExpectLog('tornado.access', ''): res = self.fetch( diff --git a/remoteappmanager/handlers/base_handler.py b/remoteappmanager/handlers/base_handler.py index af9884ab..484eeaed 100644 --- a/remoteappmanager/handlers/base_handler.py +++ b/remoteappmanager/handlers/base_handler.py @@ -17,16 +17,35 @@ class BaseHandler(HubOAuthenticated, web.RequestHandler, LoggingMixin): authenticator = HubOAuthenticator @web.authenticated + def _verify_jupyterhub_oauth(self): + """ + Internal method that just serves to separate the JupyterHub + OAuth flow from the tornadowebapi framework. The JupyterHub + HubAuth / HubAuthenticated framework is designed to be + interacted with directly from the tornado web.Request object, + rather than outsourced to a separate class (as it is in + tornadowebapi) + + Note that this introduces some unwanted coupling between the + request handlers and thier authenticators, but with the trade + off that we expect the jupyterhub.services.auth module to provide + a stable API + + Raises + ------ + HTTPError(403) if authentication fails + """ + @gen.coroutine def prepare(self): """Runs before any specific handler. """ + # Authenticate the user against the hub. + self._verify_jupyterhub_oauth() - # Note that this additional authentication layer is only - # required to support the tornadowebapi framework - the - # actual handshake with JupyterHub is handled by the + # Note that this step is only required to load the internal + # user model used by the tornadowebapi framework - the + # actual handshake with JupyterHub is performed by the # HubOAuthenticated mixin - - # Authenticate the user against the hub. self.current_user = yield self.authenticator.authenticate(self) if self.current_user is None: self.log.warn( diff --git a/remoteappmanager/handlers/tests/test_base_handler.py b/remoteappmanager/handlers/tests/test_base_handler.py index e62c9238..1a907299 100644 --- a/remoteappmanager/handlers/tests/test_base_handler.py +++ b/remoteappmanager/handlers/tests/test_base_handler.py @@ -48,14 +48,14 @@ def test_home_internal_error(self): @mock.patch.dict('os.environ', {"JUPYTERHUB_CLIENT_ID": 'client-id'}) -@mock.patch('jupyterhub.services.auth.HubOAuth.get_user') +@mock.patch('remoteappmanager.handlers.base_handler.BaseHandler._verify_jupyterhub_oauth') #: noqa:501 class TestBaseHandlerDatabaseError(TestBaseHandler): def get_file_config(self): file_config = super().get_file_config() file_config.database_class = "remoteappmanager.db.orm.ORMDatabase" return file_config - def test_home_internal_error(self, mock_get_user): + def test_home_internal_error(self, mock_verify): with ExpectLog('tornado.application', ''), \ ExpectLog('tornado.access', ''): res = self.fetch("/user/johndoe/", @@ -71,14 +71,14 @@ def test_home_internal_error(self, mock_get_user): @mock.patch.dict('os.environ', {"JUPYTERHUB_CLIENT_ID": 'client-id'}) -@mock.patch('jupyterhub.services.auth.HubOAuth.get_user') +@mock.patch('remoteappmanager.handlers.base_handler.BaseHandler._verify_jupyterhub_oauth') #: noqa:501 class TestBaseHandlerGATracking(TestBaseHandler): def get_file_config(self): file_config = super().get_file_config() file_config.ga_tracking_id = "UA-12345-6" return file_config - def test_ga_tracking(self, mock_get_user): + def test_ga_tracking(self, mock_verify): res = self.fetch("/user/johndoe/", headers={ "Cookie": "jupyter-hub-token-johndoe=foo" diff --git a/remoteappmanager/handlers/tests/test_register_container_handler.py b/remoteappmanager/handlers/tests/test_register_container_handler.py index b475c2a4..aeac5387 100644 --- a/remoteappmanager/handlers/tests/test_register_container_handler.py +++ b/remoteappmanager/handlers/tests/test_register_container_handler.py @@ -8,7 +8,7 @@ @mock.patch.dict('os.environ', {"JUPYTERHUB_CLIENT_ID": 'client-id'}) -@mock.patch('jupyterhub.services.auth.HubOAuth.get_user') +@mock.patch('remoteappmanager.handlers.base_handler.BaseHandler._verify_jupyterhub_oauth') #: noqa:501 class TestRegisterContainerHandler(TempMixin, AsyncHTTPTestCase): def get_app(self): @@ -20,7 +20,7 @@ def get_app(self): 'server': app.settings['base_urlpath']} return app - def test_absent_url_id(self, mock_get_user): + def test_absent_url_id(self, mock_verify): with ExpectLog('tornado.access', ''): res = self.fetch( "/user/johndoe/containers/99999/", @@ -29,7 +29,7 @@ def test_absent_url_id(self, mock_get_user): self.assertEqual(res.code, 404) - def test_present_url_id(self, mock_get_user): + def test_present_url_id(self, mock_verify): res = self.fetch("/user/johndoe/containers/20dcb84cdbea4b1899447246789093d0/", # noqa headers={ "Cookie": "jupyter-hub-token-johndoe=foo" @@ -40,7 +40,7 @@ def test_present_url_id(self, mock_get_user): self.assertEqual(res.code, 302) self.assertTrue(self._app.reverse_proxy.register.called) - def test_failed_auth(self, mock_get_user): + def test_failed_auth(self, mock_verify): self._app.hub.get_user.return_value = {} res = self.fetch("/user/johndoe/containers/url_id", headers={ @@ -53,7 +53,7 @@ def test_failed_auth(self, mock_get_user): self.assertFalse(self._app.reverse_proxy.register.called) self.assertEqual(res.code, 302) - def test_failure_of_reverse_proxy(self, mock_get_user): + def test_failure_of_reverse_proxy(self, mock_verify): self._app.reverse_proxy.register = mock_coro_factory( side_effect=Exception("BOOM")) diff --git a/remoteappmanager/handlers/tests/test_user_home_handler.py b/remoteappmanager/handlers/tests/test_user_home_handler.py index 1d1663dd..ce1aa604 100644 --- a/remoteappmanager/handlers/tests/test_user_home_handler.py +++ b/remoteappmanager/handlers/tests/test_user_home_handler.py @@ -7,7 +7,7 @@ @mock.patch.dict('os.environ', {"JUPYTERHUB_CLIENT_ID": 'client-id'}) -@mock.patch('jupyterhub.services.auth.HubOAuth.get_user') +@mock.patch('remoteappmanager.handlers.base_handler.BaseHandler._verify_jupyterhub_oauth') #: noqa:501 class TestUserHomeHandler(TempMixin, AsyncHTTPTestCase): def get_app(self): app = dummy.create_application() @@ -18,7 +18,7 @@ def get_app(self): 'server': app.settings['base_urlpath']} return app - def test_home(self, mock_get_user): + def test_home(self, mock_verify): res = self.fetch("/user/johndoe/", headers={ "Cookie": "jupyter-hub-token-johndoe=foo" @@ -28,7 +28,7 @@ def test_home(self, mock_get_user): self.assertEqual(res.code, 200) self.assertIn("applist", str(res.body)) - def test_failed_auth(self, mock_get_user): + def test_failed_auth(self, mock_verify): self._app.hub.get_user.return_value = {} with ExpectLog('tornado.access', ''): res = self.fetch("/user/johndoe/", From d40c60774782809a973883e26723e55a9246bd0e Mon Sep 17 00:00:00 2001 From: flongford Date: Wed, 25 May 2022 15:10:48 +0100 Subject: [PATCH 58/64] Revert "CLN: separate JupyterHub authentication framework into separate method that can be mocked" This reverts commit dc24c2d290eb49d0b5716f0a3bf4ab94cc0ad6c6. --- .../admin/tests/test_admin_handlers.py | 6 ++-- remoteappmanager/handlers/base_handler.py | 29 ++++--------------- .../handlers/tests/test_base_handler.py | 8 ++--- .../tests/test_register_container_handler.py | 10 +++---- .../handlers/tests/test_user_home_handler.py | 6 ++-- 5 files changed, 20 insertions(+), 39 deletions(-) diff --git a/remoteappmanager/handlers/admin/tests/test_admin_handlers.py b/remoteappmanager/handlers/admin/tests/test_admin_handlers.py index fe01d77d..ae02f681 100644 --- a/remoteappmanager/handlers/admin/tests/test_admin_handlers.py +++ b/remoteappmanager/handlers/admin/tests/test_admin_handlers.py @@ -6,7 +6,7 @@ @mock.patch.dict('os.environ', {"JUPYTERHUB_CLIENT_ID": 'client-id'}) -@mock.patch('remoteappmanager.handlers.base_handler.BaseHandler._verify_jupyterhub_oauth') #: noqa:501 +@mock.patch('jupyterhub.services.auth.HubOAuth.get_user') class TestBaseAccess(AsyncHTTPTestCase): #: which url to poke url = "/user/johndoe" @@ -23,7 +23,7 @@ def get_app(self): 'server': app.settings['base_urlpath']} return app - def test_access(self, mock_verify): + def test_access(self, mock_get_user): res = self.fetch(self.url, headers={ "Cookie": "jupyter-hub-token-johndoe=foo" @@ -33,7 +33,7 @@ def test_access(self, mock_verify): self.assertEqual(res.code, 200) self.assertIn(self.body_string, str(res.body)) - def test_failed_auth(self, mock_verify): + def test_failed_auth(self, mock_get_user): self._app.hub.get_user.return_value = {} with ExpectLog('tornado.access', ''): res = self.fetch( diff --git a/remoteappmanager/handlers/base_handler.py b/remoteappmanager/handlers/base_handler.py index 484eeaed..af9884ab 100644 --- a/remoteappmanager/handlers/base_handler.py +++ b/remoteappmanager/handlers/base_handler.py @@ -17,35 +17,16 @@ class BaseHandler(HubOAuthenticated, web.RequestHandler, LoggingMixin): authenticator = HubOAuthenticator @web.authenticated - def _verify_jupyterhub_oauth(self): - """ - Internal method that just serves to separate the JupyterHub - OAuth flow from the tornadowebapi framework. The JupyterHub - HubAuth / HubAuthenticated framework is designed to be - interacted with directly from the tornado web.Request object, - rather than outsourced to a separate class (as it is in - tornadowebapi) - - Note that this introduces some unwanted coupling between the - request handlers and thier authenticators, but with the trade - off that we expect the jupyterhub.services.auth module to provide - a stable API - - Raises - ------ - HTTPError(403) if authentication fails - """ - @gen.coroutine def prepare(self): """Runs before any specific handler. """ - # Authenticate the user against the hub. - self._verify_jupyterhub_oauth() - # Note that this step is only required to load the internal - # user model used by the tornadowebapi framework - the - # actual handshake with JupyterHub is performed by the + # Note that this additional authentication layer is only + # required to support the tornadowebapi framework - the + # actual handshake with JupyterHub is handled by the # HubOAuthenticated mixin + + # Authenticate the user against the hub. self.current_user = yield self.authenticator.authenticate(self) if self.current_user is None: self.log.warn( diff --git a/remoteappmanager/handlers/tests/test_base_handler.py b/remoteappmanager/handlers/tests/test_base_handler.py index 1a907299..e62c9238 100644 --- a/remoteappmanager/handlers/tests/test_base_handler.py +++ b/remoteappmanager/handlers/tests/test_base_handler.py @@ -48,14 +48,14 @@ def test_home_internal_error(self): @mock.patch.dict('os.environ', {"JUPYTERHUB_CLIENT_ID": 'client-id'}) -@mock.patch('remoteappmanager.handlers.base_handler.BaseHandler._verify_jupyterhub_oauth') #: noqa:501 +@mock.patch('jupyterhub.services.auth.HubOAuth.get_user') class TestBaseHandlerDatabaseError(TestBaseHandler): def get_file_config(self): file_config = super().get_file_config() file_config.database_class = "remoteappmanager.db.orm.ORMDatabase" return file_config - def test_home_internal_error(self, mock_verify): + def test_home_internal_error(self, mock_get_user): with ExpectLog('tornado.application', ''), \ ExpectLog('tornado.access', ''): res = self.fetch("/user/johndoe/", @@ -71,14 +71,14 @@ def test_home_internal_error(self, mock_verify): @mock.patch.dict('os.environ', {"JUPYTERHUB_CLIENT_ID": 'client-id'}) -@mock.patch('remoteappmanager.handlers.base_handler.BaseHandler._verify_jupyterhub_oauth') #: noqa:501 +@mock.patch('jupyterhub.services.auth.HubOAuth.get_user') class TestBaseHandlerGATracking(TestBaseHandler): def get_file_config(self): file_config = super().get_file_config() file_config.ga_tracking_id = "UA-12345-6" return file_config - def test_ga_tracking(self, mock_verify): + def test_ga_tracking(self, mock_get_user): res = self.fetch("/user/johndoe/", headers={ "Cookie": "jupyter-hub-token-johndoe=foo" diff --git a/remoteappmanager/handlers/tests/test_register_container_handler.py b/remoteappmanager/handlers/tests/test_register_container_handler.py index aeac5387..b475c2a4 100644 --- a/remoteappmanager/handlers/tests/test_register_container_handler.py +++ b/remoteappmanager/handlers/tests/test_register_container_handler.py @@ -8,7 +8,7 @@ @mock.patch.dict('os.environ', {"JUPYTERHUB_CLIENT_ID": 'client-id'}) -@mock.patch('remoteappmanager.handlers.base_handler.BaseHandler._verify_jupyterhub_oauth') #: noqa:501 +@mock.patch('jupyterhub.services.auth.HubOAuth.get_user') class TestRegisterContainerHandler(TempMixin, AsyncHTTPTestCase): def get_app(self): @@ -20,7 +20,7 @@ def get_app(self): 'server': app.settings['base_urlpath']} return app - def test_absent_url_id(self, mock_verify): + def test_absent_url_id(self, mock_get_user): with ExpectLog('tornado.access', ''): res = self.fetch( "/user/johndoe/containers/99999/", @@ -29,7 +29,7 @@ def test_absent_url_id(self, mock_verify): self.assertEqual(res.code, 404) - def test_present_url_id(self, mock_verify): + def test_present_url_id(self, mock_get_user): res = self.fetch("/user/johndoe/containers/20dcb84cdbea4b1899447246789093d0/", # noqa headers={ "Cookie": "jupyter-hub-token-johndoe=foo" @@ -40,7 +40,7 @@ def test_present_url_id(self, mock_verify): self.assertEqual(res.code, 302) self.assertTrue(self._app.reverse_proxy.register.called) - def test_failed_auth(self, mock_verify): + def test_failed_auth(self, mock_get_user): self._app.hub.get_user.return_value = {} res = self.fetch("/user/johndoe/containers/url_id", headers={ @@ -53,7 +53,7 @@ def test_failed_auth(self, mock_verify): self.assertFalse(self._app.reverse_proxy.register.called) self.assertEqual(res.code, 302) - def test_failure_of_reverse_proxy(self, mock_verify): + def test_failure_of_reverse_proxy(self, mock_get_user): self._app.reverse_proxy.register = mock_coro_factory( side_effect=Exception("BOOM")) diff --git a/remoteappmanager/handlers/tests/test_user_home_handler.py b/remoteappmanager/handlers/tests/test_user_home_handler.py index ce1aa604..1d1663dd 100644 --- a/remoteappmanager/handlers/tests/test_user_home_handler.py +++ b/remoteappmanager/handlers/tests/test_user_home_handler.py @@ -7,7 +7,7 @@ @mock.patch.dict('os.environ', {"JUPYTERHUB_CLIENT_ID": 'client-id'}) -@mock.patch('remoteappmanager.handlers.base_handler.BaseHandler._verify_jupyterhub_oauth') #: noqa:501 +@mock.patch('jupyterhub.services.auth.HubOAuth.get_user') class TestUserHomeHandler(TempMixin, AsyncHTTPTestCase): def get_app(self): app = dummy.create_application() @@ -18,7 +18,7 @@ def get_app(self): 'server': app.settings['base_urlpath']} return app - def test_home(self, mock_verify): + def test_home(self, mock_get_user): res = self.fetch("/user/johndoe/", headers={ "Cookie": "jupyter-hub-token-johndoe=foo" @@ -28,7 +28,7 @@ def test_home(self, mock_verify): self.assertEqual(res.code, 200) self.assertIn("applist", str(res.body)) - def test_failed_auth(self, mock_verify): + def test_failed_auth(self, mock_get_user): self._app.hub.get_user.return_value = {} with ExpectLog('tornado.access', ''): res = self.fetch("/user/johndoe/", From 8dbc7ed8c0abf9ac717b6575646b8fd1bb353822 Mon Sep 17 00:00:00 2001 From: flongford Date: Wed, 25 May 2022 15:21:07 +0100 Subject: [PATCH 59/64] DOC: add more documentation to describe the new authentication flows --- remoteappmanager/handlers/base_handler.py | 20 +++++++++---------- .../handlers/handler_authenticator.py | 18 ++++++++++++++--- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/remoteappmanager/handlers/base_handler.py b/remoteappmanager/handlers/base_handler.py index af9884ab..3a315d3f 100644 --- a/remoteappmanager/handlers/base_handler.py +++ b/remoteappmanager/handlers/base_handler.py @@ -10,23 +10,23 @@ class BaseHandler(HubOAuthenticated, web.RequestHandler, LoggingMixin): - """Base class for the request handler.""" + """Base class for the request handler. - #: The authenticator that is used by tornadowebapi to recognize - #: the user. + Each request will be authenticated using JupyterHub as an OAuth + provider using the HubOAuthenticated mixin first before + being independently validated against the application's user model. + https://jupyterhub.readthedocs.io/en/0.8.1/api/services.auth.html + """ + + #: The authenticator that is used to recognize and load + #: the internal user model. authenticator = HubOAuthenticator @web.authenticated @gen.coroutine def prepare(self): """Runs before any specific handler. """ - - # Note that this additional authentication layer is only - # required to support the tornadowebapi framework - the - # actual handshake with JupyterHub is handled by the - # HubOAuthenticated mixin - - # Authenticate the user against the hub. + # Authenticate the user against the hub self.current_user = yield self.authenticator.authenticate(self) if self.current_user is None: self.log.warn( diff --git a/remoteappmanager/handlers/handler_authenticator.py b/remoteappmanager/handlers/handler_authenticator.py index 98d58439..3452872d 100644 --- a/remoteappmanager/handlers/handler_authenticator.py +++ b/remoteappmanager/handlers/handler_authenticator.py @@ -43,9 +43,21 @@ class HubOAuthenticator(Authenticator): @classmethod @gen.coroutine def authenticate(cls, handler): - # Authenticate the user against the hub. Expects the handler - # to inherit from the jupyterhub.services.auth.HubOAuthenticated - # mixin + """Authenticate the handler application's user session + against the hub. + + Parameters + ---------- + handler : tornado.web.RequestHandler or + tornadowebapi.resource_handler.ResourceHandler + Handler to authenticate against the JupyterHub + + Returns + ------- + user : remoteappmanager.user.User or None + Internal model representing authenticated user + """ + # Authenticate the user against the hub. webapp = handler.application hub = webapp.hub user = None From f00778b04ccf726a4fe357f93b4c230d67788e49 Mon Sep 17 00:00:00 2001 From: flongford Date: Wed, 25 May 2022 15:24:47 +0100 Subject: [PATCH 60/64] CLN: minor tidy up --- remoteappmanager/base_application.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/remoteappmanager/base_application.py b/remoteappmanager/base_application.py index 925832a9..b0d0a6b6 100644 --- a/remoteappmanager/base_application.py +++ b/remoteappmanager/base_application.py @@ -40,7 +40,7 @@ class BaseApplication(web.Application, LoggingMixin): #: API access to the configurable-http-proxy reverse_proxy = Instance(ReverseProxy) - #: API access to JupyterHub for requests + #: API access to jupyterhub (for auth requests) hub = Instance(Hub) #: Manages the docker interface From 1be503bfcafad7bafa4b0acafe6c9f6b0b082ebe Mon Sep 17 00:00:00 2001 From: flongford Date: Wed, 25 May 2022 16:20:46 +0100 Subject: [PATCH 61/64] CLN: reverse change to Nginx conf template --- nginx/nginx.conf.template | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nginx/nginx.conf.template b/nginx/nginx.conf.template index f6a869ac..36a75b24 100644 --- a/nginx/nginx.conf.template +++ b/nginx/nginx.conf.template @@ -37,7 +37,7 @@ server { # Managing literal requests to the JupyterHub front end location / { - proxy_pass http://127.0.0.1:8000; + proxy_pass https://127.0.0.1:8000; proxy_set_header X-Real-IP $remote_addr; proxy_set_header Host $host; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; From b6c7ebce51de7685c34178a0b6a9a1f4cefd8da9 Mon Sep 17 00:00:00 2001 From: flongford Date: Wed, 25 May 2022 16:48:31 +0100 Subject: [PATCH 62/64] CLN: replace HubAuthenticator with HubOAuthenticator (since it wont work on versions of jupyterhub > 0.8.0 anyway) --- remoteappmanager/base_application.py | 4 +-- remoteappmanager/handlers/base_handler.py | 4 +-- .../handlers/handler_authenticator.py | 27 +------------------ remoteappmanager/tests/mocking/dummy.py | 1 - 4 files changed, 5 insertions(+), 31 deletions(-) diff --git a/remoteappmanager/base_application.py b/remoteappmanager/base_application.py index b0d0a6b6..01b3b694 100644 --- a/remoteappmanager/base_application.py +++ b/remoteappmanager/base_application.py @@ -13,7 +13,7 @@ from remoteappmanager.db.interfaces import ABCDatabase from remoteappmanager.logging.logging_mixin import LoggingMixin from remoteappmanager.docker.container_manager import ContainerManager -from remoteappmanager.handlers.handler_authenticator import HubOAuthenticator +from remoteappmanager.handlers.handler_authenticator import HubAuthenticator from remoteappmanager.user import User from remoteappmanager.traitlets import as_dict from remoteappmanager.services.hub import Hub @@ -168,7 +168,7 @@ def _user_default(self): @default("registry") def _registry_default(self): reg = Registry() - reg.authenticator = HubOAuthenticator + reg.authenticator = HubAuthenticator for resource_handler in self._webapi_resources(): reg.register(resource_handler) return reg diff --git a/remoteappmanager/handlers/base_handler.py b/remoteappmanager/handlers/base_handler.py index 3a315d3f..75db87c9 100644 --- a/remoteappmanager/handlers/base_handler.py +++ b/remoteappmanager/handlers/base_handler.py @@ -6,7 +6,7 @@ from jupyterhub.services.auth import HubOAuthenticated from remoteappmanager.logging.logging_mixin import LoggingMixin -from remoteappmanager.handlers.handler_authenticator import HubOAuthenticator +from remoteappmanager.handlers.handler_authenticator import HubAuthenticator class BaseHandler(HubOAuthenticated, web.RequestHandler, LoggingMixin): @@ -20,7 +20,7 @@ class BaseHandler(HubOAuthenticated, web.RequestHandler, LoggingMixin): #: The authenticator that is used to recognize and load #: the internal user model. - authenticator = HubOAuthenticator + authenticator = HubAuthenticator @web.authenticated @gen.coroutine diff --git a/remoteappmanager/handlers/handler_authenticator.py b/remoteappmanager/handlers/handler_authenticator.py index 3452872d..79d934da 100644 --- a/remoteappmanager/handlers/handler_authenticator.py +++ b/remoteappmanager/handlers/handler_authenticator.py @@ -13,32 +13,7 @@ def authenticate(cls, handler): class HubAuthenticator(Authenticator): - """Authenticator that uses the remote JupyterHub to validate - the request. - - Deprecated as of remoteappmanager 2.2.0 - """ - @classmethod - @gen.coroutine - def authenticate(cls, handler): - # Authenticate the user against the hub. We can't use get_current_user - # because we want to do it asynchronously. - webapp = handler.application - hub = webapp.hub - cookie_name = handler.settings["cookie_name"] - user_cookie = handler.get_cookie(cookie_name) - user = None - - if user_cookie: - user_data = (yield hub.verify_token(cookie_name, user_cookie)) - if user_data.get('name', '') == webapp.user.name: - user = webapp.user - - return user - - -class HubOAuthenticator(Authenticator): - """Authenticator that uses the remote JupyterHub as an OAuth + """Authenticator that uses the remote JupyterHub as an Auth provider to validate the request.""" @classmethod @gen.coroutine diff --git a/remoteappmanager/tests/mocking/dummy.py b/remoteappmanager/tests/mocking/dummy.py index 3c1f3637..70b0e614 100644 --- a/remoteappmanager/tests/mocking/dummy.py +++ b/remoteappmanager/tests/mocking/dummy.py @@ -218,7 +218,6 @@ def create_hub(params=None): hub = Hub(**params) - hub.verify_token = mock_coro_factory({}) hub.get_user = mock_coro_factory({}) hub.callback_handlers = lambda: [] return hub From da197ae35ab868f352d839df33de8751d79b3108 Mon Sep 17 00:00:00 2001 From: flongford Date: Wed, 25 May 2022 17:11:34 +0100 Subject: [PATCH 63/64] DOC: tidy up docstrings --- remoteappmanager/base_application.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/remoteappmanager/base_application.py b/remoteappmanager/base_application.py index 01b3b694..8930b82d 100644 --- a/remoteappmanager/base_application.py +++ b/remoteappmanager/base_application.py @@ -202,10 +202,8 @@ def start(self): @gen.coroutine def check_hub_version(self): - """Test a connection to my Hub - - - exit if I can't connect at all - - check version and warn on sufficient mismatch + """Test a connection to the JupyterHub warn on sufficient + mismatch between versions """ client = AsyncHTTPClient() RETRIES = 5 @@ -215,7 +213,8 @@ def check_hub_version(self): self.environment_config.hub_api_url) except Exception: self.log.exception( - "Failed to connect to my Hub at %s (attempt %i/%i). Is it running?", # noqa: E501 + "Failed to connect to my Hub at %s (attempt %i/%i)." + " Is it running?", self.environment_config.hub_api_url, i, RETRIES) yield gen.sleep(min(2 ** i, 16)) else: @@ -227,6 +226,10 @@ def check_hub_version(self): _check_version(hub_version, __version__, self.log) def patch_default_headers(self): + """Ensure the current JupyterHub version has been added to + each request handler header, since this will be checked for + compatibility by the hub + """ if hasattr(RequestHandler, '_orig_set_default_headers'): return RequestHandler._orig_set_default_headers = RequestHandler.set_default_headers # noqa: E501 From 746a13e160961586a46334126f3371b720b23852 Mon Sep 17 00:00:00 2001 From: flongford Date: Wed, 25 May 2022 17:30:54 +0100 Subject: [PATCH 64/64] MAINT: reduce constraints on dependencies --- requirements.txt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/requirements.txt b/requirements.txt index 03e3ae9c..cc921978 100644 --- a/requirements.txt +++ b/requirements.txt @@ -8,10 +8,9 @@ requests==2.10.0 docker-py==1.8 escapism==0.0.1 jupyterhub==0.8.1 -jupyter_client==4.3.0 +jupyter_client>=4.3.0 click==6.6 tabulate==0.7.5 tornadowebapi @ git+http://github.com/simphony/tornado-webapi.git@8b6846faae23657a04cf97ca5229ce8ea083d000#egg=tornadowebapi -# Pinned to 0.10.0 since 0.11.0 does not seem to be compatible with jupyterhub 0.7.x -oauthenticator==0.10.0 +oauthenticator>=0.10.0 jinja2>=2.8