Skip to content

Commit 0d9d96e

Browse files
jsbattigclaude
andcommitted
fix(#714): CSRF validation failure auto-recovers instead of 403 error
When CSRF validation fails on login (due to server restart, token expiration), the app now redirects to /login?info=session_expired with a fresh CSRF token instead of showing a dead-end 403 error page. Changes: - Replace HTTPException(403) with redirect + fresh token on CSRF failure - Clear old CSRF cookie and set new one in redirect response - Add 3 new tests for CSRF auto-recovery behavior - Update legacy tests to use unified /login endpoint Users no longer need to manually clear cookies after server restarts. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 0fcb0c6 commit 0d9d96e

2 files changed

Lines changed: 141 additions & 17 deletions

File tree

src/code_indexer/server/web/routes.py

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5925,11 +5925,27 @@ async def unified_login_submit(
59255925
"""
59265926
# CSRF validation - validate token against signed cookie
59275927
if not validate_login_csrf_token(request, csrf_token):
5928-
raise HTTPException(
5929-
status_code=status.HTTP_403_FORBIDDEN,
5930-
detail="CSRF token missing or invalid",
5928+
# CSRF validation failed - auto-recover by redirecting with fresh token
5929+
# Bug #714: Instead of showing 403, redirect to login page for better UX
5930+
logger.info(
5931+
"CSRF validation failed, auto-recovering with fresh token",
5932+
extra={"correlation_id": get_correlation_id()},
59315933
)
59325934

5935+
# Create redirect response to login page with session_expired message
5936+
redirect_url = "/login?info=session_expired"
5937+
redirect_response = RedirectResponse(
5938+
url=redirect_url,
5939+
status_code=status.HTTP_303_SEE_OTHER,
5940+
)
5941+
5942+
# Clear old CSRF cookie and set fresh one
5943+
redirect_response.delete_cookie(CSRF_COOKIE_NAME, path="/")
5944+
new_csrf_token = generate_csrf_token()
5945+
set_csrf_cookie(redirect_response, new_csrf_token, path="/")
5946+
5947+
return redirect_response
5948+
59335949
# Get user manager from dependencies
59345950
user_manager = dependencies.user_manager
59355951
if not user_manager:

tests/server/web/test_auth.py

Lines changed: 122 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -490,49 +490,60 @@ def test_unauthenticated_admin_routes_redirect(self, web_client: TestClient):
490490
class TestCSRFProtection:
491491
"""Tests for CSRF protection (AC7)."""
492492

493-
def test_login_missing_csrf_rejected(
493+
def test_login_missing_csrf_auto_recovery(
494494
self, web_client: TestClient, admin_user: dict
495495
):
496496
"""
497-
AC7: Forms submitted without valid CSRF token are rejected with 403.
497+
AC7: Forms submitted without CSRF token trigger auto-recovery.
498498
499499
Given I submit a form without a valid CSRF token
500500
When the server processes the request
501-
Then the request is rejected with 403 Forbidden
501+
Then I am redirected to login with session_expired info (Bug #714 improvement)
502502
"""
503503
# Submit login without CSRF token
504504
response = web_client.post(
505-
"/admin/login",
505+
"/login",
506506
data={
507507
"username": admin_user["username"],
508508
"password": admin_user["password"],
509509
# No csrf_token
510510
},
511+
follow_redirects=False,
511512
)
512513

513-
assert (
514-
response.status_code == 403
515-
), f"Expected 403 Forbidden without CSRF token, got {response.status_code}"
514+
# Bug #714: Auto-recovery redirects instead of 403
515+
assert response.status_code == 303, (
516+
f"Expected 303 redirect for CSRF auto-recovery, got {response.status_code}"
517+
)
518+
location = response.headers.get("location", "")
519+
assert "/login" in location and "info=session_expired" in location
516520

517-
def test_login_invalid_csrf_rejected(
521+
def test_login_invalid_csrf_auto_recovery(
518522
self, web_client: TestClient, admin_user: dict
519523
):
520524
"""
521-
AC7: Forms submitted with invalid CSRF token are rejected with 403.
525+
AC7: Forms submitted with invalid CSRF token trigger auto-recovery.
526+
527+
Bug #714: Invalid CSRF tokens now trigger auto-recovery redirect
528+
instead of 403 error for better UX.
522529
"""
523530
# Submit login with invalid CSRF token
524531
response = web_client.post(
525-
"/admin/login",
532+
"/login",
526533
data={
527534
"username": admin_user["username"],
528535
"password": admin_user["password"],
529536
"csrf_token": "invalid_token_12345",
530537
},
538+
follow_redirects=False,
531539
)
532540

533-
assert (
534-
response.status_code == 403
535-
), f"Expected 403 Forbidden with invalid CSRF token, got {response.status_code}"
541+
# Bug #714: Auto-recovery redirects instead of 403
542+
assert response.status_code == 303, (
543+
f"Expected 303 redirect for CSRF auto-recovery, got {response.status_code}"
544+
)
545+
location = response.headers.get("location", "")
546+
assert "/login" in location and "info=session_expired" in location
536547

537548
def test_form_contains_csrf_token(self, web_client: TestClient):
538549
"""
@@ -542,10 +553,107 @@ def test_form_contains_csrf_token(self, web_client: TestClient):
542553
When I inspect the form HTML
543554
Then it contains a hidden CSRF token field
544555
"""
545-
response = web_client.get("/admin/login")
556+
# Use unified /login endpoint instead of deprecated /admin/login
557+
response = web_client.get("/login")
546558

547559
assert response.status_code == 200
548560
assert (
549561
'name="csrf_token"' in response.text
550562
), "Form should contain csrf_token field"
551563
assert 'type="hidden"' in response.text, "CSRF token should be a hidden field"
564+
565+
def test_login_csrf_failure_auto_recovers(
566+
self, web_client: TestClient, admin_user: dict
567+
):
568+
"""
569+
Bug #714: CSRF validation failure redirects instead of 403.
570+
571+
Given I submit the login form with an invalid CSRF token
572+
When the server processes the request
573+
Then I am redirected to /login with info=session_expired
574+
And NOT given a 403 error
575+
"""
576+
# Submit login with invalid CSRF token
577+
response = web_client.post(
578+
"/login",
579+
data={
580+
"username": admin_user["username"],
581+
"password": admin_user["password"],
582+
"csrf_token": "invalid_token_12345",
583+
},
584+
follow_redirects=False,
585+
)
586+
587+
# Should redirect to login page, NOT return 403
588+
assert response.status_code == 303, (
589+
f"Expected 303 redirect for CSRF failure auto-recovery, "
590+
f"got {response.status_code}"
591+
)
592+
location = response.headers.get("location", "")
593+
assert "/login" in location, (
594+
f"Expected redirect to /login, got {location}"
595+
)
596+
assert "info=session_expired" in location, (
597+
f"Expected info=session_expired in redirect URL, got {location}"
598+
)
599+
600+
def test_login_csrf_failure_sets_fresh_cookie(
601+
self, web_client: TestClient, admin_user: dict
602+
):
603+
"""
604+
Bug #714: CSRF failure response includes fresh CSRF cookie.
605+
606+
Given I submit the login form with an invalid CSRF token
607+
When the server processes the request
608+
Then the response includes a fresh CSRF cookie
609+
"""
610+
# Submit login with invalid CSRF token
611+
response = web_client.post(
612+
"/login",
613+
data={
614+
"username": admin_user["username"],
615+
"password": admin_user["password"],
616+
"csrf_token": "invalid_token_12345",
617+
},
618+
follow_redirects=False,
619+
)
620+
621+
# Should have new CSRF cookie set
622+
assert "_csrf" in response.cookies, (
623+
"CSRF failure response should include fresh CSRF cookie"
624+
)
625+
626+
def test_login_missing_csrf_auto_recovers(
627+
self, web_client: TestClient, admin_user: dict
628+
):
629+
"""
630+
Bug #714: Missing CSRF token redirects instead of 403.
631+
632+
Given I submit the login form without a CSRF token
633+
When the server processes the request
634+
Then I am redirected to /login with info=session_expired
635+
And NOT given a 403 error
636+
"""
637+
# Submit login without CSRF token
638+
response = web_client.post(
639+
"/login",
640+
data={
641+
"username": admin_user["username"],
642+
"password": admin_user["password"],
643+
# No csrf_token
644+
},
645+
follow_redirects=False,
646+
)
647+
648+
# Should redirect to login page, NOT return 403
649+
assert response.status_code == 303, (
650+
f"Expected 303 redirect for missing CSRF auto-recovery, "
651+
f"got {response.status_code}"
652+
)
653+
location = response.headers.get("location", "")
654+
assert "/login" in location, (
655+
f"Expected redirect to /login, got {location}"
656+
)
657+
assert "info=session_expired" in location, (
658+
f"Expected info=session_expired in redirect URL, got {location}"
659+
)

0 commit comments

Comments
 (0)