diff --git a/jupyterhub/services/auth.py b/jupyterhub/services/auth.py index 6eaa4caa..18597231 100644 --- a/jupyterhub/services/auth.py +++ b/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}) - app_log.debug("Redirecting to login url: %s", login_url) - return login_url + # 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 super().get_login_url() def check_hub_user(self, model): """Check whether Hub-authenticated user or service should be allowed. diff --git a/jupyterhub/singleuser/mixins.py b/jupyterhub/singleuser/mixins.py index c0912710..9b10677f 100755 --- a/jupyterhub/singleuser/mixins.py +++ b/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 diff --git a/jupyterhub/tests/mocking.py b/jupyterhub/tests/mocking.py index e0f97cac..145ef2a9 100644 --- a/jupyterhub/tests/mocking.py +++ b/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 diff --git a/jupyterhub/tests/test_singleuser.py b/jupyterhub/tests/test_singleuser.py index 49b366c9..8252f1d2 100644 --- a/jupyterhub/tests/test_singleuser.py +++ b/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()