fix(auth): set the session cookie's Secure flag behind a TLS-terminating proxy (M462 #4977)
The cookie set Secure from r.TLS, which is nil whenever TLS terminates at Traefik or Cloudflare, so a public deployment's session cookie went out without Secure. auth.IsHTTPS now decides it from X-Forwarded-Proto, believed only through the trusted-hop count (rule 94) and read positionally like X-Forwarded-For. Plain-HTTP and LAN logins are unchanged; nothing redirects. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -140,7 +140,7 @@ func (h *handlers) handleLogin(w http.ResponseWriter, r *http.Request) {
|
||||
Value: token,
|
||||
Path: "/",
|
||||
HttpOnly: true,
|
||||
Secure: r.TLS != nil, // dev over http stays functional; prod over https gets Secure
|
||||
Secure: auth.IsHTTPS(r, h.netSettings.Hops()), // plain HTTP and LAN stay functional (rule 94)
|
||||
SameSite: http.SameSiteStrictMode,
|
||||
MaxAge: int(sessionCookieMaxAge.Seconds()),
|
||||
})
|
||||
|
||||
@@ -195,7 +195,7 @@ func (h *handlers) handleRegister(w http.ResponseWriter, r *http.Request) {
|
||||
Value: sessionToken,
|
||||
Path: "/",
|
||||
HttpOnly: true,
|
||||
Secure: r.TLS != nil,
|
||||
Secure: auth.IsHTTPS(r, h.netSettings.Hops()), // plain HTTP and LAN stay functional (rule 94)
|
||||
SameSite: http.SameSiteStrictMode,
|
||||
MaxAge: int(sessionCookieMaxAge.Seconds()),
|
||||
})
|
||||
|
||||
@@ -65,6 +65,36 @@ func ClientIP(r *http.Request, trustedProxyHops int) string {
|
||||
return remote
|
||||
}
|
||||
|
||||
// IsHTTPS reports whether the client reached us over HTTPS, reading through
|
||||
// trustedProxyHops reverse proxies by the same rule as ClientIP.
|
||||
//
|
||||
// TLS terminates at the operator's proxy (rule 94), so r.TLS is nil in every
|
||||
// real deployment and only X-Forwarded-Proto can say what the client used.
|
||||
// That header is believed only when hops >= 1: with no trusted proxy it is
|
||||
// just something any client can send. Proxies that append rather than
|
||||
// overwrite produce a comma list, read positionally like X-Forwarded-For.
|
||||
//
|
||||
// This decides only HTTPS-only behaviour (the cookie Secure flag, HSTS). It
|
||||
// must never drive a redirect: plain-HTTP and LAN use keep working.
|
||||
func IsHTTPS(r *http.Request, trustedProxyHops int) bool {
|
||||
if r.TLS != nil {
|
||||
return true
|
||||
}
|
||||
if trustedProxyHops <= 0 {
|
||||
return false
|
||||
}
|
||||
raw := r.Header.Get("X-Forwarded-Proto")
|
||||
if strings.TrimSpace(raw) == "" {
|
||||
return false
|
||||
}
|
||||
parts := strings.Split(raw, ",")
|
||||
idx := len(parts) - trustedProxyHops
|
||||
if idx < 0 {
|
||||
idx = 0
|
||||
}
|
||||
return strings.EqualFold(strings.TrimSpace(parts[idx]), "https")
|
||||
}
|
||||
|
||||
// forwardedChain returns the X-Forwarded-For entries in wire order, or the
|
||||
// single X-Real-IP value when XFF is absent.
|
||||
//
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package auth
|
||||
|
||||
import (
|
||||
"crypto/tls"
|
||||
"net/http"
|
||||
"testing"
|
||||
)
|
||||
@@ -168,3 +169,45 @@ func TestClientIP(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsHTTPS(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
hops int
|
||||
proto string
|
||||
tls bool
|
||||
want bool
|
||||
}{
|
||||
// The three cases rule 94 names.
|
||||
{name: "hops 0 ignores a forwarded https", hops: 0, proto: "https", want: false},
|
||||
{name: "hops 1 believes a forwarded https", hops: 1, proto: "https", want: true},
|
||||
{name: "plain request is not https", hops: 1, want: false},
|
||||
|
||||
{name: "forwarded http is not https", hops: 1, proto: "http", want: false},
|
||||
{name: "case-insensitive", hops: 1, proto: "HTTPS", want: true},
|
||||
{name: "direct TLS is https whatever the hops", hops: 0, tls: true, want: true},
|
||||
// An appending chain is read positionally: with one trusted hop the
|
||||
// last entry is our proxy's word, and a client-supplied first entry
|
||||
// cannot flip it.
|
||||
{name: "appended chain reads our proxy's entry", hops: 1, proto: "https, http", want: false},
|
||||
{name: "two hops read the CDN's entry", hops: 2, proto: "https, http", want: true},
|
||||
{name: "over-counted hops clamp to the first entry", hops: 5, proto: "https", want: true},
|
||||
}
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
r, err := http.NewRequest(http.MethodGet, "/api/auth/login", nil)
|
||||
if err != nil {
|
||||
t.Fatalf("NewRequest: %v", err)
|
||||
}
|
||||
if tc.proto != "" {
|
||||
r.Header.Set("X-Forwarded-Proto", tc.proto)
|
||||
}
|
||||
if tc.tls {
|
||||
r.TLS = &tls.ConnectionState{}
|
||||
}
|
||||
if got := IsHTTPS(r, tc.hops); got != tc.want {
|
||||
t.Errorf("IsHTTPS(hops=%d, proto=%q) = %v, want %v", tc.hops, tc.proto, got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user