From 96d0291848f1699fa0f94d37f92f90607d31afc8 Mon Sep 17 00:00:00 2001 From: Livio Spring Date: Tue, 16 Jan 2024 11:28:56 +0100 Subject: [PATCH] fix: enable iframe use on http://localhost (#7152) * fix: enable iframe use on http://localhost * docs(iframe): add info about cookies * improve comments --- .../solution-scenarios/configurations.mdx | 10 ++++- internal/api/http/cookie.go | 41 ++++++++++++------- .../api/http/middleware/user_agent_cookie.go | 1 + internal/api/ui/login/login.go | 15 +++++-- 4 files changed, 47 insertions(+), 20 deletions(-) diff --git a/docs/docs/guides/solution-scenarios/configurations.mdx b/docs/docs/guides/solution-scenarios/configurations.mdx index 090572e898..e13cc6cd3d 100644 --- a/docs/docs/guides/solution-scenarios/configurations.mdx +++ b/docs/docs/guides/solution-scenarios/configurations.mdx @@ -105,7 +105,7 @@ Go to the "Advanced" section, per default login with phone number should be allo ## Embedding ZITADEL in an iFrame To maximise the security during login and in the Console UI, ZITADEL follows security best practices by setting a -Content-Security-Policy (CSP) and X-Frame-Options: +Content-Security-Policy (CSP), X-Frame-Options and cookies with SameSite Lax: ``` Content-Security-Policy: frame-ancestors 'none' @@ -136,7 +136,13 @@ This will change the CSP to the following: Content-Security-Policy: frame-ancestors https://custom-domain.com ``` -and remove the X-Frame-Options header. +remove the X-Frame-Options header and change the SameSite to `None`. + +:::note +Please note, that SameSite None requires the cookie to be flagged `secure`, which means it must be sent over TLS (HTTPS) or localhost. +This also means that domains other than localhost must use TLS for this option to work. +This is due to browser restrictions: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#none +::: ### Disable Multi-factor (MFA) Prompt diff --git a/internal/api/http/cookie.go b/internal/api/http/cookie.go index e40cfdaa3a..ab055c20d2 100644 --- a/internal/api/http/cookie.go +++ b/internal/api/http/cookie.go @@ -10,10 +10,12 @@ import ( ) const ( - prefixSecure = "__Secure-" - prefixHost = "__Host-" + PrefixSecure cookiePrefix = "__Secure-" + PrefixHost cookiePrefix = "__Host-" ) +type cookiePrefix string + type CookieHandler struct { securecookie *securecookie.SecureCookie secureOnly bool @@ -21,6 +23,7 @@ type CookieHandler struct { sameSite http.SameSite path string maxAge int + prefix cookiePrefix } func NewCookieHandler(opts ...CookieHandlerOpt) *CookieHandler { @@ -78,14 +81,17 @@ func WithMaxAge(maxAge int) CookieHandlerOpt { } } -func SetCookiePrefix(name, domain, path string, secureOnly bool) string { +func WithPrefix(prefix cookiePrefix) CookieHandlerOpt { + return func(c *CookieHandler) { + c.prefix = prefix + } +} + +func SetCookiePrefix(name string, secureOnly bool, prefix cookiePrefix) string { if !secureOnly { return name } - if domain != "" || path != "/" { - return prefixSecure + name - } - return prefixHost + name + return string(prefix) + name } func (c *CookieHandler) GetCookieValue(r *http.Request, name string) (string, error) { @@ -97,7 +103,7 @@ func (c *CookieHandler) GetCookieValue(r *http.Request, name string) (string, er } func (c *CookieHandler) GetEncryptedCookieValue(r *http.Request, name string, value interface{}) error { - cookie, err := r.Cookie(SetCookiePrefix(name, r.Host, c.path, c.secureOnly)) + cookie, err := r.Cookie(SetCookiePrefix(name, c.secureOnly, c.prefix)) if err != nil { return err } @@ -131,19 +137,24 @@ func (c *CookieHandler) DeleteCookie(w http.ResponseWriter, name string) { c.httpSet(w, name, "", "", -1) } -func (c *CookieHandler) httpSet(w http.ResponseWriter, name, domain, value string, maxage int) { - c.httpSetWithSameSite(w, name, domain, value, maxage, c.sameSite) +func (c *CookieHandler) httpSet(w http.ResponseWriter, name, domain, value string, maxAge int) { + c.httpSetWithSameSite(w, name, domain, value, maxAge, c.sameSite) } -func (c *CookieHandler) httpSetWithSameSite(w http.ResponseWriter, name, domain, value string, maxage int, sameSite http.SameSite) { +func (c *CookieHandler) httpSetWithSameSite(w http.ResponseWriter, name, host, value string, maxAge int, sameSite http.SameSite) { + domain := strings.Split(host, ":")[0] + // same site none requires the secure flag, so we'll set it even if the cookie is set on non-TLS for localhost + secure := c.secureOnly || (sameSite == http.SameSiteNoneMode && domain == "localhost") + // prefix the cookie for secure cookies (TLS only, therefore not for samesite none on http://localhost) + prefixedName := SetCookiePrefix(name, c.secureOnly, c.prefix) http.SetCookie(w, &http.Cookie{ - Name: SetCookiePrefix(name, domain, c.path, c.secureOnly), + Name: prefixedName, Value: value, - Domain: strings.Split(domain, ":")[0], + Domain: domain, Path: c.path, - MaxAge: maxage, + MaxAge: maxAge, HttpOnly: c.httpOnly, - Secure: c.secureOnly, + Secure: secure, SameSite: sameSite, }) varyValues := w.Header().Values("vary") diff --git a/internal/api/http/middleware/user_agent_cookie.go b/internal/api/http/middleware/user_agent_cookie.go index c156e6d942..5eddfb4439 100644 --- a/internal/api/http/middleware/user_agent_cookie.go +++ b/internal/api/http/middleware/user_agent_cookie.go @@ -46,6 +46,7 @@ func NewUserAgentHandler(config *UserAgentCookieConfig, cookieKey []byte, idGene opts := []http_utils.CookieHandlerOpt{ http_utils.WithEncryption(cookieKey, cookieKey), http_utils.WithMaxAge(int(config.MaxAge.Seconds())), + http_utils.WithPrefix(http_utils.PrefixSecure), } if !externalSecure { opts = append(opts, http_utils.WithUnsecure()) diff --git a/internal/api/ui/login/login.go b/internal/api/ui/login/login.go index 71bba44d5d..af6e555468 100644 --- a/internal/api/ui/login/login.go +++ b/internal/api/ui/login/login.go @@ -8,6 +8,7 @@ import ( "github.com/gorilla/csrf" "github.com/gorilla/mux" + "github.com/zitadel/zitadel/feature" "github.com/zitadel/zitadel/internal/api/authz" http_utils "github.com/zitadel/zitadel/internal/api/http" @@ -122,13 +123,21 @@ func createCSRFInterceptor(cookieName string, csrfCookieKey []byte, externalSecu handler.ServeHTTP(w, r) return } + // by default we use SameSite Lax and the externalSecure (TLS) for the secure flag sameSiteMode := csrf.SameSiteLaxMode - if len(authz.GetInstance(r.Context()).SecurityPolicyAllowedOrigins()) > 0 { + secureOnly := externalSecure + instance := authz.GetInstance(r.Context()) + // in case of `allow iframe`... + if len(instance.SecurityPolicyAllowedOrigins()) > 0 { + // ... we need to change to SameSite none ... sameSiteMode = csrf.SameSiteNoneMode + // ... and since SameSite none requires the secure flag, we'll set it for TLS and for localhost + // (regardless of the TLS / externalSecure settings) + secureOnly = externalSecure || instance.RequestedDomain() == "localhost" } csrf.Protect(csrfCookieKey, - csrf.Secure(externalSecure), - csrf.CookieName(http_utils.SetCookiePrefix(cookieName, "", path, externalSecure)), + csrf.Secure(secureOnly), + csrf.CookieName(http_utils.SetCookiePrefix(cookieName, externalSecure, http_utils.PrefixHost)), csrf.Path(path), csrf.ErrorHandler(errorHandler), csrf.SameSite(sameSiteMode),