MAINT: Update JupyterHub to v0.8.1 (#552)

* MAINT: bump jupyterhub package to 0.8.1

* FIX: replace jupyterhub.orm.Proxy with jupyterhub.proxy.Proxy

* FIX: include missing X-JupyterHib-Version headers

* TST: add dummy-user option for dev testing

* CLN: remove uneeded override of BaseHandler.set_default_headers

* STY: flake8 fixes

* FIX: revert cli function name changes in favor of flake exceptions

* FIX: tidy up deprecated uses of orm.Proxy

* TST: formatting fixed to returned auth dicts

* TST: replace mocking of deprecated orm.Hub

* FIX: update EnvironmentConfig to source hub settings relocated to os environ

* TST: remove hub_* traits from CommandLineConfig replace source in BaseApplication

* FIX: various fixes to BaseSpawner class

* FIX: wrong assignment of ConfigurableHTTPProxy.api_server trait

* TST: Spawner.ip no longer has a default value, manually assigning 127.0.0.1

* FIX: further fixes to BaseSpawner get_args

* STY: flake8 fixes

* TST: fix test environment for BaseApplication

* FIX: pass Proxy API status to Spawner in config, rather than during runtime

* STY: flake8 fixes

* FIX: hack-y fix for breaking changes to Spawner class introduced in v0.8.x

* FIX: simply overwrite LocalProcessSpawner.get_args

* FIX: remove default BaseSpawner.ip value

* FIX: handle None value for LocalProcessSpawner.ip

* MAINT: changes to Nginx  conf file

* FIX: set LocalProcessSpawner.server upon instantiation from User attribute

* FIX: avoid recursion loop between Spawner and User server assignments

* FIX: amend Spawner test suite to instantiate from orm.Server instance as in production

* CLN: tidy up jupyterhub config

* EXP: attempt to fix redirect loop by patching RequestHandler headers

* STY: flake8 fixes

* FIX: use correct assignment of cookie name from JupyterHub

* FIX: use correct env variable for EnvironmentConfig.hub_prefix

* FIX: more parsing of hub_prefix argument back to CommandLineConfig

* CLN: remove typo in jupyterhub_config.py

* FIX: correctly parse cookie-name argument

* FIX: use cookie-name as of jupyterhub 0.8.0

* EXP: run selenium tests in debug mode

* STY: flake8 fixes

* EXP: change url for tornado RedirectHandlers in application

* EXP: revert change to RedirectHandler for admin application to avoid unit test failure (for now)

* EXP: revert change to RedirectHandler for user application to avoid unit test failures (for now)

* DEV: enable user authentication with remoteappmanager via JupyterHub as an OAuth provider

* CLN: move connection with JupyterHub OAuth service into remoteappmanager.services.hub module

* TST: include mocking of new HubOAuth.get_user method for testing

* DEV: fold HubOAuth class into Hub

* CLN: minor tidy up

* FIX: typo in keyword argument

* FIX: minor fixes to auth process

* TST: replace mocked calls to verify_token with get_user

* STY: flake8 fix

* TST: fix python tests via mocking jupyterhub HubOAuth calls (for now)

* CLN: remove unnecessary dev changes

* FIX: remove deprecated code from logout workaround

* TST: fix admin login for selenium tests

* FIX: error handling for shutting down server upon user logout

* CLN: separate JupyterHub authentication framework into separate method that can be mocked

* Revert "CLN: separate JupyterHub authentication framework into separate method that can be mocked"

This reverts commit dc24c2d.

* DOC: add more documentation to describe the new authentication flows

* CLN: minor tidy up

* CLN: reverse change to Nginx conf template

* CLN: replace HubAuthenticator with HubOAuthenticator (since it wont work on versions of jupyterhub > 0.8.0 anyway)

* DOC: tidy up docstrings

* MAINT: reduce constraints on dependencies

Makefile

@@ -113,6 +113,7 @@ npmdeps:
 	fi
 
 	npm install
+	npm install configurable-http-proxy
 	npm --version
 
 .PHONY: pythondeps

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

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"]}

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

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;

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,22 +176,70 @@ 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(
                 trait_name,
                 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

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"

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")
 

remoteappmanager/environment_config.py

@@ -6,21 +6,34 @@ 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.
 
     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, ""))

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,

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