Merge pull request #5067 from kreuzert/main

Add Authenticator.refresh_pre_stop option

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.

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

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