From 2e459c911031669080bc110059cf2b4b19c5379d Mon Sep 17 00:00:00 2001 From: Adam Mathes Date: Mon, 16 Feb 2026 14:02:48 -0800 Subject: Enhance CSRF protection for login page Login form now includes a CSRF token from the cookie as a hidden form field. The CSRF middleware accepts tokens from either the X-CSRF-Token header (for JS clients) or the csrf_token form field (for HTML forms). Removed /login from the CSRF exclusion list so login POSTs are now validated. Co-Authored-By: Claude Opus 4.6 --- web/login_test.go | 77 ++++++++++++++++++++++++++++++++------------------- web/static/login.html | 7 ++++- web/web.go | 10 +++++-- 3 files changed, 61 insertions(+), 33 deletions(-) diff --git a/web/login_test.go b/web/login_test.go index cd3cb01..f4931b2 100644 --- a/web/login_test.go +++ b/web/login_test.go @@ -6,63 +6,82 @@ import ( "strings" "testing" - "golang.org/x/crypto/bcrypt" - "adammathes.com/neko/config" ) -func TestCSRFLoginExclusion(t *testing.T) { - // Setup password +func TestCSRFLoginWithFormToken(t *testing.T) { pw := "secret" - hashed, _ := bcrypt.GenerateFromPassword([]byte(pw), bcrypt.DefaultCost) - // We need to set the global config since loginHandler uses it - config.Config.DigestPassword = string(hashed) // wait, config stores the hashed password? - // let's check web.go loginHandler: - // if password == config.Config.DigestPassword { - // v, _ := bcrypt.GenerateFromPassword([]byte(password), 0) - - // Ah, it compares PLAIN TEXT password with config.Config.DigestPassword. - // wait, line 113: if password == config.Config.DigestPassword { - // So config stores the PLAIN TEXT password? That's... odd, but okay for this test. originalPw := config.Config.DigestPassword defer func() { config.Config.DigestPassword = originalPw }() config.Config.DigestPassword = pw - // Create a mux with login handler mux := http.NewServeMux() mux.HandleFunc("/login/", loginHandler) mux.HandleFunc("/other", func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) }) - // Wrap with CSRF middleware - // We need to pass a pointer to Settings handler := CSRFMiddleware(&config.Config, mux) - // Test 1: POST /login/ without CSRF token - // Should NOT return 403 Forbidden. - // Since we provide correct password, it should redirect (307) - req := httptest.NewRequest("POST", "/login/", strings.NewReader("password="+pw)) + // Step 1: GET /login/ to obtain the CSRF token cookie + getReq := httptest.NewRequest("GET", "/login/", nil) + getRR := httptest.NewRecorder() + handler.ServeHTTP(getRR, getReq) + + var csrfToken string + for _, c := range getRR.Result().Cookies() { + if c.Name == "csrf_token" { + csrfToken = c.Value + break + } + } + if csrfToken == "" { + t.Fatal("Expected csrf_token cookie to be set on GET /login/") + } + + // Test 1: POST /login/ with CSRF token in form field should succeed + body := "password=" + pw + "&csrf_token=" + csrfToken + req := httptest.NewRequest("POST", "/login/", strings.NewReader(body)) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.AddCookie(&http.Cookie{Name: "csrf_token", Value: csrfToken}) rr := httptest.NewRecorder() handler.ServeHTTP(rr, req) - if rr.Code == http.StatusForbidden { - t.Errorf("Expected /login/ POST to be allowed without CSRF token, got 403 Forbidden") - } if rr.Code != http.StatusSeeOther { - t.Errorf("Expected 303 Redirect on successful login, got %d", rr.Code) + t.Errorf("Expected 303 Redirect on successful login with CSRF token, got %d", rr.Code) } - // Test 2: POST /other without CSRF token - // Should fail with 403 Forbidden - req2 := httptest.NewRequest("POST", "/other", nil) + // Test 2: POST /login/ without CSRF token should be rejected + req2 := httptest.NewRequest("POST", "/login/", strings.NewReader("password="+pw)) + req2.Header.Set("Content-Type", "application/x-www-form-urlencoded") rr2 := httptest.NewRecorder() handler.ServeHTTP(rr2, req2) if rr2.Code != http.StatusForbidden { - t.Errorf("Expected 403 Forbidden for POST /other, got %d", rr2.Code) + t.Errorf("Expected 403 Forbidden for POST /login/ without CSRF token, got %d", rr2.Code) + } + + // Test 3: POST /other without CSRF token should fail + req3 := httptest.NewRequest("POST", "/other", nil) + rr3 := httptest.NewRecorder() + + handler.ServeHTTP(rr3, req3) + + if rr3.Code != http.StatusForbidden { + t.Errorf("Expected 403 Forbidden for POST /other, got %d", rr3.Code) + } + + // Test 4: POST /other with CSRF token in header should succeed + req4 := httptest.NewRequest("POST", "/other", nil) + req4.AddCookie(&http.Cookie{Name: "csrf_token", Value: csrfToken}) + req4.Header.Set("X-CSRF-Token", csrfToken) + rr4 := httptest.NewRecorder() + + handler.ServeHTTP(rr4, req4) + + if rr4.Code == http.StatusForbidden { + t.Errorf("Expected POST /other with valid CSRF header to succeed, got 403") } } diff --git a/web/static/login.html b/web/static/login.html index c7d0a03..c469528 100644 --- a/web/static/login.html +++ b/web/static/login.html @@ -115,7 +115,8 @@