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
This commit is contained in:
Min RK
2025-03-21 17:18:24 +01:00
parent 6619524d1f
commit 1a2e5d2e9d
2 changed files with 79 additions and 2 deletions

View File

@@ -9,6 +9,7 @@ from tornado import web
from tornado.escape import url_escape from tornado.escape import url_escape
from tornado.httputil import url_concat from tornado.httputil import url_concat
from .._xsrf_utils import _set_xsrf_cookie
from ..utils import maybe_future from ..utils import maybe_future
from .base import BaseHandler from .base import BaseHandler
@@ -94,7 +95,37 @@ class LogoutHandler(BaseHandler):
class LoginHandler(BaseHandler): class LoginHandler(BaseHandler):
"""Render the login page.""" """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 = { context = {
"next": url_escape(self.get_argument('next', default='')), "next": url_escape(self.get_argument('next', default='')),
"username": username, "username": username,
@@ -116,6 +147,7 @@ class LoginHandler(BaseHandler):
'login.html', 'login.html',
**context, **context,
custom_html=custom_html, custom_html=custom_html,
**kwargs,
) )
async def get(self): async def get(self):
@@ -147,6 +179,14 @@ class LoginHandler(BaseHandler):
self.redirect(auto_login_url) self.redirect(auto_login_url)
return return
username = self.get_argument('username', default='') 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)) self.finish(await self._render(username=username))
async def post(self): async def post(self):
@@ -169,6 +209,7 @@ class LoginHandler(BaseHandler):
self._jupyterhub_user = user self._jupyterhub_user = user
self.redirect(self.get_next_url(user)) self.redirect(self.get_next_url(user))
else: else:
self.set_status(403)
html = await self._render( html = await self._render(
login_error='Invalid username or password', username=data['username'] login_error='Invalid username or password', username=data['username']
) )

View File

@@ -723,14 +723,50 @@ async def test_page_with_token(app, user, url, token_in):
async def test_login_fail(app): 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' name = 'wash'
base_url = public_url(app) base_url = public_url(app)
r = await async_requests.post( r = await async_requests.post(
base_url + 'hub/login', base_url + 'hub/login',
data={'username': name, 'password': 'wrong'}, data={
'username': name,
'password': name,
'_xsrf': "wrong",
},
allow_redirects=False, allow_redirects=False,
) )
assert r.status_code == 403
assert set(r.cookies.keys()).issubset({"_xsrf"}) 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( @pytest.mark.parametrize(