From 1a2e5d2e9db2fed23018b3bf0e3fa683f0e62a0c Mon Sep 17 00:00:00 2001
From: Min RK <benjaminrk@gmail.com>
Date: Fri, 21 Mar 2025 17:18:24 +0100
Subject: [PATCH] improve xsrf errors on login

- show login form for trying again, just like a password failure
- nicer, but more vague "try again" error for expired xsrf (original error still logged)
  because users logging in don't need to know or understand xsrf stuff
- set fresh xsrf cookie when login page loads, to maximize time until expiration
---
 jupyterhub/handlers/login.py   | 43 +++++++++++++++++++++++++++++++++-
 jupyterhub/tests/test_pages.py | 38 +++++++++++++++++++++++++++++-
 2 files changed, 79 insertions(+), 2 deletions(-)

diff --git a/jupyterhub/handlers/login.py b/jupyterhub/handlers/login.py
index 3f6c6912..b8e4e9a0 100644
--- a/jupyterhub/handlers/login.py
+++ b/jupyterhub/handlers/login.py
@@ -9,6 +9,7 @@ from tornado import web
 from tornado.escape import url_escape
 from tornado.httputil import url_concat
 
+from .._xsrf_utils import _set_xsrf_cookie
 from ..utils import maybe_future
 from .base import BaseHandler
 
@@ -94,7 +95,37 @@ class LogoutHandler(BaseHandler):
 class LoginHandler(BaseHandler):
     """Render the login page."""
 
-    def _render(self, login_error=None, username=None):
+    def render_template(self, name, **ns):
+        # intercept error page rendering for form submissions
+        if (
+            name == "error.html"
+            and self.request.method.lower() == "post"
+            and self.request.headers.get("Sec-Fetch-Mode", "navigate") == "navigate"
+        ):
+            # regular login form submission
+            # render login form with error message
+            ns["login_error"] = ns.get("message") or ns.get("status_message", "")
+            ns["username"] = self.get_argument("username", strip=True, default="")
+            return self._render(**ns)
+        else:
+            return super().render_template(name, **ns)
+
+    def check_xsrf_cookie(self):
+        try:
+            return super().check_xsrf_cookie()
+        except web.HTTPError as e:
+            # rewrite xsrf error on login form for nicer message
+            # suggest retry, which is likely to succeed
+            # log the original error so admins can debug
+            self.log.error("XSRF error on login form: %s", e)
+            if self.request.headers.get("Sec-Fetch-Mode", "navigate") == "navigate":
+                raise web.HTTPError(
+                    e.status_code, "Login form invalid or expired. Try again."
+                )
+            else:
+                raise
+
+    def _render(self, login_error=None, username=None, **kwargs):
         context = {
             "next": url_escape(self.get_argument('next', default='')),
             "username": username,
@@ -116,6 +147,7 @@ class LoginHandler(BaseHandler):
             'login.html',
             **context,
             custom_html=custom_html,
+            **kwargs,
         )
 
     async def get(self):
@@ -147,6 +179,14 @@ class LoginHandler(BaseHandler):
                     self.redirect(auto_login_url)
                 return
             username = self.get_argument('username', default='')
+            # always set a fresh xsrf cookie when the login page is rendered
+            # ensures we are as far from expiration as possible
+            xsrf_token = self.xsrf_token
+            if self.request.headers.get("Sec-Fetch-Mode", "navigate") == "navigate":
+                _set_xsrf_cookie(
+                    self, self._xsrf_token_id, cookie_path=self.hub.base_url
+                )
+            # to restart the timer
             self.finish(await self._render(username=username))
 
     async def post(self):
@@ -169,6 +209,7 @@ class LoginHandler(BaseHandler):
             self._jupyterhub_user = user
             self.redirect(self.get_next_url(user))
         else:
+            self.set_status(403)
             html = await self._render(
                 login_error='Invalid username or password', username=data['username']
             )
diff --git a/jupyterhub/tests/test_pages.py b/jupyterhub/tests/test_pages.py
index 1b653d39..fa524ef7 100644
--- a/jupyterhub/tests/test_pages.py
+++ b/jupyterhub/tests/test_pages.py
@@ -723,14 +723,50 @@ async def test_page_with_token(app, user, url, token_in):
 
 
 async def test_login_fail(app):
+    name = 'wash'
+    base_url = public_url(app)
+    login_url = base_url + 'hub/login'
+    r = await async_requests.get(login_url)
+    r.raise_for_status()
+    xsrf = r.cookies['_xsrf']
+    r = await async_requests.get(login_url)
+    assert set(r.cookies.keys()).issubset({"_xsrf"})
+    r = await async_requests.post(
+        login_url,
+        data={'username': name, 'password': 'wrong', '_xsrf': xsrf},
+        allow_redirects=False,
+        cookies=r.cookies,
+    )
+    assert r.status_code == 403
+    assert set(r.cookies.keys()).issubset({"_xsrf"})
+    page = BeautifulSoup(r.content, "html.parser")
+    assert "Sign in" in page.text
+    login = page.find("form")
+    login_error = login.find(class_="login_error")
+    assert login_error
+    assert "Invalid user" in login_error.text
+
+
+async def test_login_fail_xsrf_expired(app):
     name = 'wash'
     base_url = public_url(app)
     r = await async_requests.post(
         base_url + 'hub/login',
-        data={'username': name, 'password': 'wrong'},
+        data={
+            'username': name,
+            'password': name,
+            '_xsrf': "wrong",
+        },
         allow_redirects=False,
     )
+    assert r.status_code == 403
     assert set(r.cookies.keys()).issubset({"_xsrf"})
+    page = BeautifulSoup(r.content, "html.parser")
+    assert "Sign in" in page.text
+    login = page.find("form")
+    login_error = login.find(class_="login_error")
+    assert login_error
+    assert "Try again" in login_error.text
 
 
 @pytest.mark.parametrize(