make_singleuser_app: patch-in HubAuthenticatedHandler at lower priority

apply patch directly to BaseHandler instead of each handler instance
so that overrides can still take effect (i.e. APIHandler raising 403 instead of redirecting)

jupyterhub/services/auth.py

@@ -19,6 +19,7 @@ import string
 import time
 import uuid
 import warnings
+from unittest import mock
 from urllib.parse import quote
 from urllib.parse import urlencode
 
@@ -832,8 +833,12 @@ class HubAuthenticated(object):
             # add state argument to OAuth url
             state = self.hub_auth.set_state_cookie(self, next_url=self.request.uri)
             login_url = url_concat(login_url, {'state': state})
+        # override at setting level,
+        # to allow any subclass overrides of get_login_url to preserve their effect
+        # for example, APIHandler raises 403 to prevent redirects
+        with mock.patch.dict(self.application.settings, {"login_url": login_url}):
             app_log.debug("Redirecting to login url: %s", login_url)
-        return login_url
+            return super().get_login_url()
 
     def check_hub_user(self, model):
         """Check whether Hub-authenticated user or service should be allowed.

jupyterhub/singleuser/mixins.py

@@ -9,11 +9,10 @@ with JupyterHub authentication mixins enabled.
 # Copyright (c) Jupyter Development Team.
 # Distributed under the terms of the Modified BSD License.
 import asyncio
-import importlib
 import json
 import os
 import random
-from datetime import datetime
+import warnings
 from datetime import timezone
 from textwrap import dedent
 from urllib.parse import urlparse
@@ -23,7 +22,6 @@ from jinja2 import FunctionLoader
 from tornado import ioloop
 from tornado.httpclient import AsyncHTTPClient
 from tornado.httpclient import HTTPRequest
-from tornado.web import HTTPError
 from tornado.web import RequestHandler
 from traitlets import Any
 from traitlets import Bool
@@ -94,9 +92,18 @@ class JupyterHubLoginHandlerMixin:
 
     @staticmethod
     def get_user(handler):
-        """alternative get_current_user to query the Hub"""
+        """alternative get_current_user to query the Hub
+
+        Thus shouldn't be called anymore because HubAuthenticatedHandler
+        should have already overridden get_current_user()
+        """
         # patch in HubAuthenticated class for querying the Hub for cookie authentication
-        if HubAuthenticatedHandler not in handler.__class__.__bases__:
+        if HubAuthenticatedHandler not in handler.__class__.mro():
+            warnings.warn(
+                f"Expected to see HubAuthenticatedHandler in {handler.__class__}.mro()",
+                RuntimeWarning,
+                stacklevel=2,
+            )
             handler.__class__ = type(
                 handler.__class__.__name__,
                 (HubAuthenticatedHandler, handler.__class__),
@@ -691,6 +698,7 @@ def make_singleuser_app(App):
     """
 
     empty_parent_app = App()
+    log = empty_parent_app.log
 
     # detect base classes
     LoginHandler = empty_parent_app.login_handler_class
@@ -707,6 +715,26 @@ def make_singleuser_app(App):
                 "{}.base_handler_class must be defined".format(App.__name__)
             )
 
+    # patch-in hub HubOAuthCallbackHandler to BaseHandler,
+    # so anything inheriting from BaseHandler uses Hub authentication
+    if HubAuthenticatedHandler not in BaseHandler.__bases__:
+        log.debug(f"Patching {HubAuthenticatedHandler} into {BaseHandler}")
+        BaseHandler.__bases__ = (HubAuthenticatedHandler,) + BaseHandler.__bases__
+        # adding it to bases isn't enough to override methods defined on BaseHandler, though.
+        # we still need to override any methods defined on BaseHandler *itself* that we should override
+        # since bases come immediately *after* the class itself
+        # as of writing, there are no collisions on the default classes
+        # so this block has no effect for ServerApp or NotebookApp
+        seen = set()
+        for cls in HubAuthenticatedHandler.mro():
+            for name, method in cls.__dict__.items():
+                if name in seen or name.startswith("__"):
+                    continue
+                seen.add(name)
+                if name in BaseHandler.__dict__:
+                    log.debug(f"Overriding {BaseHandler}.{name} with {cls}.{name}")
+                    setattr(BaseHandler, name, method)
+
     # create Handler classes from mixins + bases
     class JupyterHubLoginHandler(JupyterHubLoginHandlerMixin, LoginHandler):
         pass

jupyterhub/tests/mocking.py

@@ -404,9 +404,10 @@ class StubSingleUserSpawner(MockSpawner):
         Should be:
 
         - authenticated, so we are testing auth
-        - always available (i.e. in base ServerApp and NotebookApp
+        - always available (i.e. in mocked ServerApp and NotebookApp)
+        - *not* an API handler that raises 403 instead of redirecting
         """
-        return "/api/status"
+        return "/tree"
 
     _thread = None
 

jupyterhub/tests/test_singleuser.py

@@ -21,6 +21,7 @@ async def test_singleuser_auth(app):
     user = app.users['nandy']
     if not user.running:
         await user.spawn()
+        await app.proxy.add_user(user)
     url = public_url(app, user)
 
     # no cookies, redirects to login page
@@ -28,6 +29,11 @@ async def test_singleuser_auth(app):
     r.raise_for_status()
     assert '/hub/login' in r.url
 
+    # unauthenticated /api/ should 403, not redirect
+    api_url = url_path_join(url, "api/status")
+    r = await async_requests.get(api_url, allow_redirects=False)
+    assert r.status_code == 403
+
     # with cookies, login successful
     r = await async_requests.get(url, cookies=cookies)
     r.raise_for_status()