fix: detect TLS per-request in CSRF middleware to fix login (#54)

## Problem After the security hardening in PR #42, login fails with `Forbidden - invalid CSRF token` in production deployments. The CSRF middleware tied its `PlaintextHTTPRequest` wrapping and cookie `Secure` flag to the `IsDev()` environment check. This meant production mode always assumed HTTPS via gorilla/csrf's strict mode, which broke login in common deployment scenarios: 1. **Production behind a TLS-terminating reverse proxy**: gorilla/csrf assumed HTTPS but `r.TLS` was nil (the Go server receives HTTP from the proxy). Origin/Referer scheme mismatches caused `referer not supplied` or `origin invalid` errors. 2. **Production over direct HTTP** (testing/staging with prod config): the `Secure` cookie flag prevented the browser from sending the CSRF cookie back over HTTP, causing `CSRF token invalid` errors. ## Root Cause gorilla/csrf v1.7.3 defaults to HTTPS-strict mode unless `PlaintextHTTPRequest()` is called. In strict mode it: - Forces `requestURL.Scheme = "https"` for Origin/Referer comparisons - Requires a `Referer` header on POST and rejects `http://` Referer schemes - The `csrf.Secure(true)` option makes the browser refuse to send the CSRF cookie over HTTP The old code only called `PlaintextHTTPRequest()` in dev mode, leaving prod mode permanently stuck in HTTPS-strict mode regardless of the actual transport. ## Fix Detect the actual transport protocol **per-request** using: - `r.TLS != nil` — direct TLS connection to the Go server - `X-Forwarded-Proto: https` header — TLS-terminating reverse proxy Two gorilla/csrf middleware instances are maintained (one with `Secure: true`, one with `Secure: false`) since `csrf.Secure()` is a creation-time option. Both use the same signing key, so cookies are interchangeable. | Scenario | Cookie Secure | Origin/Referer Mode | |---|---|---| | Direct TLS (`r.TLS != nil`) | ✅ Secure | Strict (HTTPS scheme) | | Behind TLS proxy (`X-Forwarded-Proto: https`) | ✅ Secure | Strict (HTTPS scheme) | | Plaintext HTTP | ❌ Non-Secure | Relaxed (PlaintextHTTPRequest) | CSRF token validation (cookie + form double-submit) is always enforced regardless of mode. ## Testing - Added `TestCSRF_ProdMode_PlaintextHTTP_POSTWithValidToken` — prod mode over plaintext HTTP - Added `TestCSRF_ProdMode_BehindProxy_POSTWithValidToken` — prod mode behind TLS proxy - Added `TestCSRF_ProdMode_DirectTLS_POSTWithValidToken` — prod mode with direct TLS - Added `TestCSRF_ProdMode_PlaintextHTTP_POSTWithoutToken` — token still required - Added `TestIsClientTLS_*` — TLS detection unit tests - All existing CSRF tests pass unchanged - `docker build .` passes (includes `make check`) - Manual verification: built and ran the container in both `dev` and `prod` modes, confirmed login succeeds in both Closes #53 Co-authored-by: user <user@Mac.lan guest wan> Reviewed-on: #54 Co-authored-by: clawbot <clawbot@noreply.example.org> Co-committed-by: clawbot <clawbot@noreply.example.org>
2026-03-18 04:30:57 +01:00
parent 33e2140a5a
commit d771fe14df
3 changed files with 289 additions and 29 deletions
--- a/internal/middleware/csrf_test.go
+++ b/internal/middleware/csrf_test.go
@@ -1,6 +1,7 @@
 package middleware

 import (
+	"crypto/tls"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -155,3 +156,217 @@ func TestCSRFToken_NoMiddleware(t *testing.T) {
 	req := httptest.NewRequest(http.MethodGet, "/", nil)
 	assert.Empty(t, CSRFToken(req), "CSRFToken should return empty string when middleware has not run")
 }
+
+// --- TLS Detection Tests ---
+
+func TestIsClientTLS_DirectTLS(t *testing.T) {
+	t.Parallel()
+	r := httptest.NewRequest(http.MethodGet, "/", nil)
+	r.TLS = &tls.ConnectionState{} // simulate direct TLS
+	assert.True(t, isClientTLS(r), "should detect direct TLS connection")
+}
+
+func TestIsClientTLS_XForwardedProto(t *testing.T) {
+	t.Parallel()
+	r := httptest.NewRequest(http.MethodGet, "/", nil)
+	r.Header.Set("X-Forwarded-Proto", "https")
+	assert.True(t, isClientTLS(r), "should detect TLS via X-Forwarded-Proto")
+}
+
+func TestIsClientTLS_PlaintextHTTP(t *testing.T) {
+	t.Parallel()
+	r := httptest.NewRequest(http.MethodGet, "/", nil)
+	assert.False(t, isClientTLS(r), "should detect plaintext HTTP")
+}
+
+func TestIsClientTLS_XForwardedProtoHTTP(t *testing.T) {
+	t.Parallel()
+	r := httptest.NewRequest(http.MethodGet, "/", nil)
+	r.Header.Set("X-Forwarded-Proto", "http")
+	assert.False(t, isClientTLS(r), "should detect plaintext when X-Forwarded-Proto is http")
+}
+
+// --- Production Mode: POST over plaintext HTTP ---
+
+func TestCSRF_ProdMode_PlaintextHTTP_POSTWithValidToken(t *testing.T) {
+	t.Parallel()
+	m, _ := testMiddleware(t, config.EnvironmentProd)
+
+	// This tests the critical fix: prod mode over plaintext HTTP should
+	// work because the middleware detects the transport per-request.
+	var token string
+	csrfMiddleware := m.CSRF()
+	getHandler := csrfMiddleware(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
+		token = CSRFToken(r)
+	}))
+
+	getReq := httptest.NewRequest(http.MethodGet, "/form", nil)
+	getW := httptest.NewRecorder()
+	getHandler.ServeHTTP(getW, getReq)
+
+	cookies := getW.Result().Cookies()
+	require.NotEmpty(t, cookies, "CSRF cookie should be set on GET")
+	require.NotEmpty(t, token, "CSRF token should be set in context on GET")
+
+	// Verify the cookie is NOT Secure (plaintext HTTP in prod mode)
+	for _, c := range cookies {
+		if c.Name == "_gorilla_csrf" {
+			assert.False(t, c.Secure, "CSRF cookie should not be Secure over plaintext HTTP")
+		}
+	}
+
+	// POST with valid token — should succeed
+	var called bool
+	postHandler := csrfMiddleware(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
+		called = true
+	}))
+
+	form := url.Values{"csrf_token": {token}}
+	postReq := httptest.NewRequest(http.MethodPost, "/form", strings.NewReader(form.Encode()))
+	postReq.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	for _, c := range cookies {
+		postReq.AddCookie(c)
+	}
+	postW := httptest.NewRecorder()
+
+	postHandler.ServeHTTP(postW, postReq)
+
+	assert.True(t, called, "handler should be called — prod mode over plaintext HTTP must work")
+	assert.NotEqual(t, http.StatusForbidden, postW.Code, "should not return 403")
+}
+
+// --- Production Mode: POST with X-Forwarded-Proto (reverse proxy) ---
+
+func TestCSRF_ProdMode_BehindProxy_POSTWithValidToken(t *testing.T) {
+	t.Parallel()
+	m, _ := testMiddleware(t, config.EnvironmentProd)
+
+	// Simulates a deployment behind a TLS-terminating reverse proxy.
+	// The Go server sees HTTP but X-Forwarded-Proto is "https".
+	var token string
+	csrfMiddleware := m.CSRF()
+	getHandler := csrfMiddleware(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
+		token = CSRFToken(r)
+	}))
+
+	getReq := httptest.NewRequest(http.MethodGet, "http://example.com/form", nil)
+	getReq.Header.Set("X-Forwarded-Proto", "https")
+	getW := httptest.NewRecorder()
+	getHandler.ServeHTTP(getW, getReq)
+
+	cookies := getW.Result().Cookies()
+	require.NotEmpty(t, cookies, "CSRF cookie should be set on GET")
+	require.NotEmpty(t, token, "CSRF token should be set in context")
+
+	// Verify the cookie IS Secure (X-Forwarded-Proto: https)
+	for _, c := range cookies {
+		if c.Name == "_gorilla_csrf" {
+			assert.True(t, c.Secure, "CSRF cookie should be Secure behind TLS proxy")
+		}
+	}
+
+	// POST with valid token, HTTPS Origin (as a browser behind proxy would send)
+	var called bool
+	postHandler := csrfMiddleware(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
+		called = true
+	}))
+
+	form := url.Values{"csrf_token": {token}}
+	postReq := httptest.NewRequest(http.MethodPost, "http://example.com/form", strings.NewReader(form.Encode()))
+	postReq.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	postReq.Header.Set("X-Forwarded-Proto", "https")
+	postReq.Header.Set("Origin", "https://example.com")
+	for _, c := range cookies {
+		postReq.AddCookie(c)
+	}
+	postW := httptest.NewRecorder()
+
+	postHandler.ServeHTTP(postW, postReq)
+
+	assert.True(t, called, "handler should be called — prod mode behind TLS proxy must work")
+	assert.NotEqual(t, http.StatusForbidden, postW.Code, "should not return 403")
+}
+
+// --- Production Mode: direct TLS ---
+
+func TestCSRF_ProdMode_DirectTLS_POSTWithValidToken(t *testing.T) {
+	t.Parallel()
+	m, _ := testMiddleware(t, config.EnvironmentProd)
+
+	var token string
+	csrfMiddleware := m.CSRF()
+	getHandler := csrfMiddleware(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
+		token = CSRFToken(r)
+	}))
+
+	getReq := httptest.NewRequest(http.MethodGet, "https://example.com/form", nil)
+	getReq.TLS = &tls.ConnectionState{}
+	getW := httptest.NewRecorder()
+	getHandler.ServeHTTP(getW, getReq)
+
+	cookies := getW.Result().Cookies()
+	require.NotEmpty(t, cookies, "CSRF cookie should be set on GET")
+	require.NotEmpty(t, token, "CSRF token should be set in context")
+
+	// Verify the cookie IS Secure (direct TLS)
+	for _, c := range cookies {
+		if c.Name == "_gorilla_csrf" {
+			assert.True(t, c.Secure, "CSRF cookie should be Secure over direct TLS")
+		}
+	}
+
+	// POST with valid token over direct TLS
+	var called bool
+	postHandler := csrfMiddleware(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
+		called = true
+	}))
+
+	form := url.Values{"csrf_token": {token}}
+	postReq := httptest.NewRequest(http.MethodPost, "https://example.com/form", strings.NewReader(form.Encode()))
+	postReq.TLS = &tls.ConnectionState{}
+	postReq.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	postReq.Header.Set("Origin", "https://example.com")
+	for _, c := range cookies {
+		postReq.AddCookie(c)
+	}
+	postW := httptest.NewRecorder()
+
+	postHandler.ServeHTTP(postW, postReq)
+
+	assert.True(t, called, "handler should be called — direct TLS must work")
+	assert.NotEqual(t, http.StatusForbidden, postW.Code, "should not return 403")
+}
+
+// --- Production Mode: POST without token still rejects ---
+
+func TestCSRF_ProdMode_PlaintextHTTP_POSTWithoutToken(t *testing.T) {
+	t.Parallel()
+	m, _ := testMiddleware(t, config.EnvironmentProd)
+
+	csrfMiddleware := m.CSRF()
+
+	// GET to establish the CSRF cookie
+	getHandler := csrfMiddleware(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {}))
+	getReq := httptest.NewRequest(http.MethodGet, "/form", nil)
+	getW := httptest.NewRecorder()
+	getHandler.ServeHTTP(getW, getReq)
+	cookies := getW.Result().Cookies()
+
+	// POST without CSRF token — should be rejected
+	var called bool
+	postHandler := csrfMiddleware(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
+		called = true
+	}))
+
+	postReq := httptest.NewRequest(http.MethodPost, "/form", nil)
+	postReq.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	for _, c := range cookies {
+		postReq.AddCookie(c)
+	}
+	postW := httptest.NewRecorder()
+
+	postHandler.ServeHTTP(postW, postReq)
+
+	assert.False(t, called, "handler should NOT be called without CSRF token even in prod+plaintext")
+	assert.Equal(t, http.StatusForbidden, postW.Code)
+}