diff --git a/jupyterhub/auth.py b/jupyterhub/auth.py index fab1cf43..99dfc148 100644 --- a/jupyterhub/auth.py +++ b/jupyterhub/auth.py @@ -86,7 +86,7 @@ class Authenticator(LoggingConfigurable): auth info will never be considered stale. Set `auth_refresh_age = 0` to disable time-based calls to `refresh_user`. - You can still use :attr:`refresh_pre_spawn` if `auth_refresh_age` is disabled. + You can still use :attr:`refresh_pre_spawn` or :attr:`refresh_pre_stop` if `auth_refresh_age` is disabled. """, ) @@ -106,6 +106,25 @@ class Authenticator(LoggingConfigurable): """, ) + refresh_pre_stop = Bool( + False, + config=True, + help="""Force refresh of auth prior to stop. + + This forces :meth:`.refresh_user` to be called prior to stopping + a server, to ensure that auth state is up-to-date. + + This can be important when e.g. auth tokens stored in auth_state may have expired, + but are a required part of the Spawner's shutdown steps. + + If refresh_user cannot refresh the user auth data, + stop will fail until the user logs in again. + If an admin initiates the stop, it will proceed regardless. + + .. versionadded:: 5.4 + """, + ) + admin_users = Set( help=""" Set of users that will be granted admin rights on this JupyterHub. diff --git a/jupyterhub/handlers/base.py b/jupyterhub/handlers/base.py index 5eaa6800..cfa76d06 100644 --- a/jupyterhub/handlers/base.py +++ b/jupyterhub/handlers/base.py @@ -1326,6 +1326,22 @@ class BaseHandler(RequestHandler): spawner = user.spawners[server_name] if spawner.pending: raise RuntimeError(f"{spawner._log_name} pending {spawner.pending}") + + if self.authenticator.refresh_pre_stop: + auth_user = await self.refresh_auth(user, force=True) + if auth_user is None: + if ( + self.current_user.kind == "user" + and self.current_user.name == user.name + ): + raise web.HTTPError( + 403, "auth has expired for %s, login again", user.name + ) + else: + self.log.warning( + "User %s may have stale auth info. Stopping anyway.", user.name + ) + # set user._stop_pending before doing anything async # to avoid races spawner._stop_pending = True diff --git a/jupyterhub/tests/test_auth_expiry.py b/jupyterhub/tests/test_auth_expiry.py index 9e5e1b89..6a7d5f04 100644 --- a/jupyterhub/tests/test_auth_expiry.py +++ b/jupyterhub/tests/test_auth_expiry.py @@ -37,6 +37,14 @@ def refresh_pre_spawn(app): app.authenticator.refresh_pre_spawn = False +@pytest.fixture +def refresh_pre_stop(app): + """Fixture enabling auth refresh pre stop""" + app.authenticator.refresh_pre_stop = True + yield + app.authenticator.refresh_pre_stop = False + + async def test_auth_refresh_at_login(app, user): # auth_refreshed starts unset: assert not user._auth_refreshed @@ -175,3 +183,85 @@ async def test_refresh_pre_spawn_expired_admin_request( ) # api requests can't do login redirects assert r.status_code == 403 + + +async def test_refresh_pre_stop(app, user, refresh_pre_stop): + cookies = await app.login_user(user.name) + assert user._auth_refreshed + user._auth_refreshed -= 10 + before = user._auth_refreshed + + r = await api_request( + app, f'users/{user.name}/server', method='post', name=user.name + ) + + assert user._auth_refreshed == before + assert 200 <= r.status_code < 300 + + # auth is fresh, but should be forced to refresh by stop + r = await api_request( + app, f'users/{user.name}/server', method='delete', name=user.name + ) + assert 200 <= r.status_code < 300 + assert user._auth_refreshed > before + + +async def test_refresh_pre_stop_expired(app, user, refresh_pre_stop, disable_refresh): + cookies = await app.login_user(user.name) + assert user._auth_refreshed + user._auth_refreshed -= 10 + before = user._auth_refreshed + + r = await api_request( + app, f'users/{user.name}/server', method='post', name=user.name + ) + assert user._auth_refreshed == before + assert 200 <= r.status_code < 300 + + # auth is fresh, doesn't trigger expiry + r = await api_request( + app, f'users/{user.name}/server', method='delete', name=user.name + ) + assert r.status_code == 403 + assert user._auth_refreshed == before + + +async def test_refresh_pre_stop_admin_request(app, user, admin_user, refresh_pre_stop): + await app.login_user(user.name) + await app.login_user(admin_user.name) + user._auth_refreshed -= 10 + before = user._auth_refreshed + + r = await api_request( + app, 'users', user.name, 'server', method='post', name=admin_user.name + ) + assert user._auth_refreshed == before + assert 200 <= r.status_code < 300 + + # admin request, auth is fresh. Should still refresh user auth. + r = await api_request( + app, 'users', user.name, 'server', method='delete', name=admin_user.name + ) + assert 200 <= r.status_code < 300 + assert user._auth_refreshed > before + + +async def test_refresh_pre_stop_expired_admin_request( + app, user, admin_user, refresh_pre_stop, disable_refresh +): + await app.login_user(user.name) + await app.login_user(admin_user.name) + user._auth_refreshed -= 10 + + r = await api_request( + app, 'users', user.name, 'server', method='post', name=admin_user.name + ) + assert 200 <= r.status_code < 300 + + # auth needs refresh but can't without a new login; stop should be forced + user._auth_refreshed -= app.authenticator.auth_refresh_age + r = await api_request( + app, 'users', user.name, 'server', method='delete', name=admin_user.name + ) + + assert 200 <= r.status_code < 300