mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-10 02:25:56 +00:00
Stop auto-escalating SameSite to None on proxied requests
getCookieSettings auto-set SameSite=None on the session and CSRF cookies whenever a request arrived via a trusted proxy with X-Forwarded-Proto: https. The original intent was "be more permissive for proxied deployments," but in practice that disabled the browser-side CSRF defence for every Pulse deployment behind a reverse proxy — which is nearly all of them. SameSite=None tells the browser to attach the cookie on arbitrary cross-site requests. That is only required for cross-origin iframe embedding scenarios Pulse does not document or support. The top-level-navigation cases that actually matter (OIDC/SAML callback landing, Cloudflare-tunnel access from a bookmark) all work under Lax, which still attaches cookies on top-level navigations and only blocks cross-site sub-resource requests. Always return SameSiteLaxMode. The Secure flag still tracks the actual connection state via isConnectionSecure(r). Together with the earlier CSRF bypass fix (require token regardless of Authorization header), the browser-side and server-side defences are now both in place. Test expectations updated: three cases that previously asserted SameSiteNoneMode for the proxied-HTTPS / Cloudflare-tunnel / direct- TLS-with-Forwarded paths now assert SameSiteLaxMode, with comments flagging them as regression coverage.
This commit is contained in:
+17
-17
@@ -288,7 +288,21 @@ func isWebSocketUpgrade(r *http.Request) bool {
|
||||
return strings.EqualFold(r.Header.Get("Upgrade"), "websocket")
|
||||
}
|
||||
|
||||
// getCookieSettings returns the appropriate cookie settings based on proxy detection
|
||||
// getCookieSettings returns the appropriate cookie settings based on proxy detection.
|
||||
//
|
||||
// SameSite defaults to Lax for ALL requests, regardless of proxy mode. The
|
||||
// previous behaviour auto-escalated to SameSite=None whenever the request
|
||||
// arrived via a trusted proxy on https — intended to "be more permissive"
|
||||
// for proxied deployments, but in practice that disabled the browser-side
|
||||
// CSRF defence for every deployment behind a reverse proxy (which is nearly
|
||||
// all of them). SameSite=None tells the browser to send the cookie on
|
||||
// arbitrary cross-site requests, which is only required for cross-origin
|
||||
// iframe embedding scenarios Pulse does not document or support. Lax
|
||||
// continues to send cookies on top-level navigations (the OIDC/SAML
|
||||
// callback case), so the proxied-login flow is unaffected.
|
||||
//
|
||||
// Secure is still set automatically based on the actual connection state
|
||||
// (TLS, or X-Forwarded-Proto: https from a trusted proxy).
|
||||
func getCookieSettings(r *http.Request) (secure bool, sameSite http.SameSite) {
|
||||
isProxied := detectProxy(r)
|
||||
isSecure := isConnectionSecure(r)
|
||||
@@ -302,24 +316,10 @@ func getCookieSettings(r *http.Request) (secure bool, sameSite http.SameSite) {
|
||||
Str("cf_connecting_ip", r.Header.Get("CF-Connecting-IP")).
|
||||
Str("x_forwarded_for", r.Header.Get("X-Forwarded-For")).
|
||||
Str("x_forwarded_proto", r.Header.Get("X-Forwarded-Proto")).
|
||||
Msg("Proxy/tunnel detected - adjusting cookie settings")
|
||||
Msg("Proxy/tunnel detected - cookies use Lax SameSite")
|
||||
}
|
||||
|
||||
// Default to Lax for better compatibility
|
||||
sameSitePolicy := http.SameSiteLaxMode
|
||||
|
||||
if isProxied {
|
||||
// For proxied connections, we need to be more permissive
|
||||
// But only use None if connection is secure (required by browsers)
|
||||
if isSecure {
|
||||
sameSitePolicy = http.SameSiteNoneMode
|
||||
} else {
|
||||
// For HTTP proxies, stay with Lax for compatibility
|
||||
sameSitePolicy = http.SameSiteLaxMode
|
||||
}
|
||||
}
|
||||
|
||||
return isSecure, sameSitePolicy
|
||||
return isSecure, http.SameSiteLaxMode
|
||||
}
|
||||
|
||||
// Cookie name constants. The session cookie uses the __Host- prefix when served
|
||||
|
||||
@@ -255,6 +255,10 @@ func TestGetCookieSettings(t *testing.T) {
|
||||
wantSameSite: http.SameSiteLaxMode,
|
||||
},
|
||||
{
|
||||
// Regression: previously auto-escalated SameSite to None here,
|
||||
// which disabled the browser-side CSRF defence for every
|
||||
// proxied-HTTPS deployment. SameSite must stay Lax; Secure
|
||||
// still tracks the proxied protocol.
|
||||
name: "HTTPS through proxy",
|
||||
useTLS: false,
|
||||
headers: map[string]string{
|
||||
@@ -262,7 +266,7 @@ func TestGetCookieSettings(t *testing.T) {
|
||||
"X-Forwarded-Proto": "https",
|
||||
},
|
||||
wantSecure: true,
|
||||
wantSameSite: http.SameSiteNoneMode,
|
||||
wantSameSite: http.SameSiteLaxMode,
|
||||
},
|
||||
{
|
||||
name: "HTTP through proxy",
|
||||
@@ -275,6 +279,8 @@ func TestGetCookieSettings(t *testing.T) {
|
||||
wantSameSite: http.SameSiteLaxMode,
|
||||
},
|
||||
{
|
||||
// Regression: same pattern as the HTTPS-through-proxy case via
|
||||
// Cloudflare's CF-* headers — must NOT escalate to SameSite=None.
|
||||
name: "Cloudflare tunnel HTTPS",
|
||||
useTLS: false,
|
||||
headers: map[string]string{
|
||||
@@ -283,7 +289,7 @@ func TestGetCookieSettings(t *testing.T) {
|
||||
"X-Forwarded-Proto": "https",
|
||||
},
|
||||
wantSecure: true,
|
||||
wantSameSite: http.SameSiteNoneMode,
|
||||
wantSameSite: http.SameSiteLaxMode,
|
||||
},
|
||||
{
|
||||
name: "proxy detected but no proto header",
|
||||
@@ -295,13 +301,15 @@ func TestGetCookieSettings(t *testing.T) {
|
||||
wantSameSite: http.SameSiteLaxMode,
|
||||
},
|
||||
{
|
||||
// Regression: previously SameSite=None when TLS + trusted-proxy
|
||||
// Forwarded header. Must stay Lax.
|
||||
name: "direct TLS with Forwarded header (trusted proxy)",
|
||||
useTLS: true,
|
||||
headers: map[string]string{
|
||||
"Forwarded": "for=192.168.1.1;proto=https",
|
||||
},
|
||||
wantSecure: true,
|
||||
wantSameSite: http.SameSiteNoneMode,
|
||||
wantSameSite: http.SameSiteLaxMode,
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user