diff --git a/Makefile b/Makefile index 368049df..f993330e 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/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/jupyterhub/jupyterhub_config.py b/jupyterhub/jupyterhub_config.py index 503db0f7..b975d5b0 100644 --- a/jupyterhub/jupyterhub_config.py +++ b/jupyterhub/jupyterhub_config.py @@ -18,7 +18,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 +37,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/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 diff --git a/nginx/nginx.conf.template b/nginx/nginx.conf.template index 1a87da22..36a75b24 100644 --- a/nginx/nginx.conf.template +++ b/nginx/nginx.conf.template @@ -37,10 +37,6 @@ 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 - # 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_set_header X-Real-IP $remote_addr; proxy_set_header Host $host; diff --git a/remoteappmanager/base_application.py b/remoteappmanager/base_application.py index a82ee931..8930b82d 100644 --- a/remoteappmanager/base_application.py +++ b/remoteappmanager/base_application.py @@ -1,15 +1,19 @@ import importlib +from secrets import token_urlsafe -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 tornado.web import RequestHandler 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 HubAuthenticator from remoteappmanager.user import User from remoteappmanager.traitlets import as_dict from remoteappmanager.services.hub import Hub @@ -80,11 +84,17 @@ 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__ + + # 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() # Initializers @default("container_manager") @@ -107,10 +117,15 @@ 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, - api_token=self.environment_config.jpy_api_token, - ) + """Initializes the Hub instance used to authenticate with + JupyterHub. + """ + return Hub( + 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, + ) @default("db") def _db_default(self): @@ -161,7 +176,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("Starting server with options:") for trait_name in self._command_line_config.trait_names(): self.log.info("{}: {}".format( @@ -169,14 +185,61 @@ 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().start() + @gen.coroutine + def check_hub_version(self): + """Test a connection to the JupyterHub warn on sufficient + mismatch between versions + """ + client = AsyncHTTPClient() + RETRIES = 5 + for i in range(1, RETRIES + 1): + try: + resp = yield client.fetch( + 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?", + self.environment_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) + + 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 + + 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""" @@ -215,6 +278,8 @@ 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() - return web_api+web_handlers + return web_auth+web_api+web_handlers diff --git a/remoteappmanager/cli/remoteappdb/__main__.py b/remoteappmanager/cli/remoteappdb/__main__.py index e67c4b78..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(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(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(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(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(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(ctx, no_decoration): +def list(ctx, no_decoration): # noqa: F811 """List all registered applications.""" if no_decoration: tablefmt = "plain" diff --git a/remoteappmanager/command_line_config.py b/remoteappmanager/command_line_config.py index 5e45cba0..d088be67 100644 --- a/remoteappmanager/command_line_config.py +++ b/remoteappmanager/command_line_config.py @@ -24,16 +24,10 @@ 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/ + # 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/environment_config.py b/remoteappmanager/environment_config.py index 3cf49513..feea91b4 100644 --- a/remoteappmanager/environment_config.py +++ b/remoteappmanager/environment_config.py @@ -6,13 +6,21 @@ 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 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 +28,12 @@ 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_API_URL": 'hub_api_url' + } + for envname, traitlet_name in mapping.items(): setattr(self, traitlet_name, os.environ.get(envname, "")) diff --git a/remoteappmanager/handlers/admin/tests/test_admin_handlers.py b/remoteappmanager/handlers/admin/tests/test_admin_handlers.py index a6c7e570..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" @@ -12,14 +16,14 @@ 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, '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,8 +33,8 @@ def test_access(self): self.assertEqual(res.code, 200) self.assertIn(self.body_string, str(res.body)) - def test_failed_auth(self): - self._app.hub.verify_token.return_value = {} + def test_failed_auth(self, mock_get_user): + self._app.hub.get_user.return_value = {} with ExpectLog('tornado.access', ''): res = self.fetch( self.url, diff --git a/remoteappmanager/handlers/base_handler.py b/remoteappmanager/handlers/base_handler.py index 298afbee..75db87c9 100644 --- a/remoteappmanager/handlers/base_handler.py +++ b/remoteappmanager/handlers/base_handler.py @@ -3,23 +3,34 @@ 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 -class BaseHandler(web.RequestHandler, LoggingMixin): - """Base class for the request handler.""" +class BaseHandler(HubOAuthenticated, web.RequestHandler, LoggingMixin): + """Base class for the request handler. + + 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 the user. + #: The authenticator that is used to recognize and load + #: the internal user model. authenticator = HubAuthenticator + @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. + # 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 cdc72b8c..79d934da 100644 --- a/remoteappmanager/handlers/handler_authenticator.py +++ b/remoteappmanager/handlers/handler_authenticator.py @@ -13,22 +13,32 @@ def authenticate(cls, handler): class HubAuthenticator(Authenticator): - """Authenticator that uses the remote JupyterHub to validate - the request.""" + """Authenticator that uses the remote JupyterHub as an Auth + provider to validate the request.""" @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. + """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 - 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 + user_data = yield hub.get_user(handler) + if user_data.get('name', '') == webapp.user.name: + user = webapp.user return user diff --git a/remoteappmanager/handlers/tests/test_base_handler.py b/remoteappmanager/handlers/tests/test_base_handler.py index badbb8ea..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 @@ -17,7 +19,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, @@ -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 96d8cf6a..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,18 +7,20 @@ 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): app = dummy.create_application() - app.hub.verify_token.return_value = { + app.hub.get_user.return_value = { 'pending': None, 'name': app.settings['user'], 'admin': False, '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,8 +40,8 @@ def test_present_url_id(self): self.assertEqual(res.code, 302) self.assertTrue(self._app.reverse_proxy.register.called) - def test_failed_auth(self): - self._app.hub.verify_token.return_value = {} + def test_failed_auth(self, mock_get_user): + self._app.hub.get_user.return_value = {} res = self.fetch("/user/johndoe/containers/url_id", headers={ "Cookie": "jupyter-hub-token-johndoe=foo" @@ -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 362cf2fb..1d1663dd 100644 --- a/remoteappmanager/handlers/tests/test_user_home_handler.py +++ b/remoteappmanager/handlers/tests/test_user_home_handler.py @@ -1,20 +1,24 @@ +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() - app.hub.verify_token.return_value = { + app.hub.get_user.return_value = { 'pending': None, 'name': app.settings['user'], 'admin': False, '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,8 +28,8 @@ def test_home(self): self.assertEqual(res.code, 200) self.assertIn("applist", str(res.body)) - def test_failed_auth(self): - self._app.hub.verify_token.return_value = {} + def test_failed_auth(self, mock_get_user): + self._app.hub.get_user.return_value = {} with ExpectLog('tornado.access', ''): res = self.fetch("/user/johndoe/", headers={ diff --git a/remoteappmanager/jupyterhub/auth/simphony_remote_auth_mixin.py b/remoteappmanager/jupyterhub/auth/simphony_remote_auth_mixin.py index 1abb32eb..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,12 +20,12 @@ 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() - 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) 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): diff --git a/remoteappmanager/jupyterhub/spawners.py b/remoteappmanager/jupyterhub/spawners.py index 6bd2a533..b28e8a8a 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 import orm ADMIN_CMD = "remoteappadmin" USER_CMD = "remoteappmanager" @@ -17,7 +16,7 @@ class BaseSpawner(LocalProcessSpawner): the actual spawners """ - #: The instance of the orm Proxy. + #: The instance of the JupyterHub Proxy. #: We use Any in agreement with base class practice. proxy = Any() @@ -31,12 +30,10 @@ def cmd(self): return self.user_options.get('cmd', self._default_cmd()) 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() + super(BaseSpawner, self).__init__(**kwargs) + # We can obtain a reference to the JupyterHub.proxy object + # through the tornado settings passed onto the User + self.proxy = self.user.settings.get('proxy') @default("options_form") def _options_form_default(self): @@ -71,15 +68,28 @@ def options_from_form(self, form_data): def get_args(self): args = super().get_args() - for iarg, arg in enumerate(args): - args[iarg] = arg.replace('--base-url=', '--base-urlpath=') + # 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.server.base_url)) + + args.append('--hub-prefix={}'.format( + self.hub.base_url)) + + args.append("--cookie-name={}".format( + self.hub.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)) @@ -187,7 +197,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 735f6dcd..e7baac74 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 proxy, orm, objects from remoteappmanager.jupyterhub.spawners import ( ADMIN_CMD, USER_CMD, SystemUserSpawner, VirtualUserSpawner) @@ -39,29 +39,33 @@ def new_spawner(spawner_class): proto="http", ip="127.0.0.2", port=31337, - base_url="/" + base_url="/", + cookie_name='cookie' ) # Mock db db = mock.Mock() - db.query = mock.Mock() - db.query().first = mock.Mock( - return_value=orm.Proxy( - auth_token="whatever", - api_server=orm.Server(proto="http", - ip="127.0.0.1", - port=12345, - base_url="/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 + user.settings = { + 'proxy': proxy.ConfigurableHTTPProxy( + api_url="http://127.0.0.1:12345/foo/bar/", + auth_token="whatever" + ) + } # Mock hub - hub = orm.Hub(server=generic_server) + hub = objects.Hub( + ip="127.0.0.2", + proto="http", + port=31337, + base_url="/hub/", + ) # Mock authenticator authenticator = mock.Mock() @@ -69,8 +73,20 @@ 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, hub=hub, authenticator=authenticator) + db=db, + user=user, + orm_spawner=orm_spawner, + hub=hub, + authenticator=authenticator, + api_token="dummy_token" + ) class TestSystemUserSpawner(TempMixin, AsyncTestCase): @@ -82,12 +98,16 @@ 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("--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() + self.assertIn("--ip=\"127.0.0.1\"", 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) @@ -112,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 diff --git a/remoteappmanager/services/hub.py b/remoteappmanager/services/hub.py index cd611063..d2f97a32 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, HubOAuthCallbackHandler) + from remoteappmanager.logging.logging_mixin import LoggingMixin from remoteappmanager.utils import url_path_join @@ -11,12 +14,18 @@ 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): """Initializes the hub connection object.""" super().__init__(*args, **kwargs) @@ -33,11 +42,21 @@ 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, + 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 if verified. + Deprecated as of remoteappmanager 2.2.0 + Parameters ---------- cookie_name: str @@ -69,3 +88,31 @@ def verify_token(self, cookie_name, encrypted_cookie): return escape.json_decode(r.body) else: return {} + + @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/services/reverse_proxy.py b/remoteappmanager/services/reverse_proxy.py index 938f621e..3227e121 100644 --- a/remoteappmanager/services/reverse_proxy.py +++ b/remoteappmanager/services/reverse_proxy.py @@ -1,7 +1,5 @@ -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 +15,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 +33,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_url=self.endpoint_url ) self.log.info("Reverse proxy setup on {}".format( @@ -94,14 +92,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 - ) 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/", diff --git a/remoteappmanager/tests/mocking/dummy.py b/remoteappmanager/tests/mocking/dummy.py index cd7287ab..70b0e614 100644 --- a/remoteappmanager/tests/mocking/dummy.py +++ b/remoteappmanager/tests/mocking/dummy.py @@ -218,7 +218,8 @@ 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 @@ -310,7 +311,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/tests/test_environment_config.py b/remoteappmanager/tests/test_environment_config.py index ad21daed..6ecfc895 100644 --- a/remoteappmanager/tests/test_environment_config.py +++ b/remoteappmanager/tests/test_environment_config.py @@ -2,16 +2,17 @@ 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() - with patch.dict('os.environ', - {'JPY_API_TOKEN': 'jpy_token', - 'PROXY_API_TOKEN': 'proxy_token' - }): + 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") + self.assertEqual(config.hub_host, "") + 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..db751e94 100644 --- a/remoteappmanager/tests/utils.py +++ b/remoteappmanager/tests/utils.py @@ -16,14 +16,19 @@ "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", + "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") } +env_config = { + 'JUPYTERHUB_API_TOKEN': 'jpy_token', + 'PROXY_API_TOKEN': 'proxy_token', + 'JUPYTERHUB_HOST': '', + 'JUPYTERHUB_API_URL': 'http://172.17.5.167:8081/hub/api', +} + def init_sqlite_db(path): """Initializes the sqlite database at a given path. @@ -48,9 +53,10 @@ 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_api_url = env_config['JUPYTERHUB_API_URL'] return config 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"}, diff --git a/requirements.txt b/requirements.txt index df3c9bfa..cc921978 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,12 +7,10 @@ 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 jupyterhub 0.8.0.dev0 due to issues with spawners (fails with 0.8.0) -jupyterhub @ git+http://github.com/jupyterhub/jupyterhub.git@2d1a45f0190059ef436c2f97dc8d6e391eb2d139#egg=jupyterhub -jupyter_client==4.3.0 +jupyterhub==0.8.1 +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 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")