fix: caching of assets (correct headers and versioned avatar and variables.css url) (#4118)

* fix: caching of assets (correct headers and versioned avatar url)

* serve variables.css versioned and extend shared max age of assets

* fix TestCommandSide_AddHumanAvatar

* refactor: const types

* refactor: return values

Co-authored-by: Fabi <38692350+hifabienne@users.noreply.github.com>
Co-authored-by: adlerhurst <silvan.reusser@gmail.com>

cmd/defaults.yaml

@@ -103,6 +103,15 @@ Machine:
     #   Url: "http://169.254.169.254/metadata/instance?api-version=2021-02-01"
     #   JPath: "$.compute.vmId"
 
+# Storage for assets like user avatar, organization logo, icon, font, ...
+AssetStorage:
+  Type: db
+  # HTTP cache control settings for serving assets in the assets API and login UI
+  # the assets will also be served with an etag and last-modified header
+  Cache:
+    MaxAge: 5s
+    SharedMaxAge: 168h #7d
+
 Projections:
   RequeueEvery: 60s
   RetryFailedAfter: 1s
@@ -177,7 +186,7 @@ Console:
     SharedMaxAge: 5m
   LongCache:
     MaxAge: 12h
-    SharedMaxAge: 168h
+    SharedMaxAge: 168h #7d
 
 Notification:
   Repository:

cmd/start/start.go

@@ -189,7 +189,8 @@ func startAPIs(ctx context.Context, router *mux.Router, commands *command.Comman
 	}
 
 	instanceInterceptor := middleware.InstanceInterceptor(queries, config.HTTP1HostHeader, login.IgnoreInstanceEndpoints...)
-	apis.RegisterHandler(assets.HandlerPrefix, assets.NewHandler(commands, verifier, config.InternalAuthZ, id.SonyFlakeGenerator(), store, queries, instanceInterceptor.Handler))
+	assetsCache := middleware.AssetsCacheInterceptor(config.AssetStorage.Cache.MaxAge, config.AssetStorage.Cache.SharedMaxAge)
+	apis.RegisterHandler(assets.HandlerPrefix, assets.NewHandler(commands, verifier, config.InternalAuthZ, id.SonyFlakeGenerator(), store, queries, instanceInterceptor.Handler, assetsCache.Handler))
 
 	userAgentInterceptor, err := middleware.NewUserAgentHandler(config.UserAgentCookie, keys.UserAgentCookieKey, id.SonyFlakeGenerator(), config.ExternalSecure, login.EndpointResources)
 	if err != nil {
@@ -213,7 +214,7 @@ func startAPIs(ctx context.Context, router *mux.Router, commands *command.Comman
 	}
 	apis.RegisterHandler(console.HandlerPrefix, c)
 
-	l, err := login.CreateLogin(config.Login, commands, queries, authRepo, store, console.HandlerPrefix+"/", op.AuthCallbackURL(oidcProvider), config.ExternalSecure, userAgentInterceptor, op.NewIssuerInterceptor(oidcProvider.IssuerFromRequest).Handler, instanceInterceptor.Handler, keys.User, keys.IDPConfig, keys.CSRFCookieKey)
+	l, err := login.CreateLogin(config.Login, commands, queries, authRepo, store, console.HandlerPrefix+"/", op.AuthCallbackURL(oidcProvider), config.ExternalSecure, userAgentInterceptor, op.NewIssuerInterceptor(oidcProvider.IssuerFromRequest).Handler, instanceInterceptor.Handler, assetsCache.Handler, keys.User, keys.IDPConfig, keys.CSRFCookieKey)
 	if err != nil {
 		return fmt.Errorf("unable to start login: %w", err)
 	}

internal/api/assets/asset.go

@@ -76,7 +76,7 @@ func DefaultErrorHandler(w http.ResponseWriter, r *http.Request, err error, code
 	http.Error(w, err.Error(), code)
 }
 
-func NewHandler(commands *command.Commands, verifier *authz.TokenVerifier, authConfig authz.Config, idGenerator id.Generator, storage static.Storage, queries *query.Queries, instanceInterceptor func(handler http.Handler) http.Handler) http.Handler {
+func NewHandler(commands *command.Commands, verifier *authz.TokenVerifier, authConfig authz.Config, idGenerator id.Generator, storage static.Storage, queries *query.Queries, instanceInterceptor, assetCacheInterceptor func(handler http.Handler) http.Handler) http.Handler {
 	h := &Handler{
 		commands:        commands,
 		errorHandler:    DefaultErrorHandler,
@@ -88,7 +88,7 @@ func NewHandler(commands *command.Commands, verifier *authz.TokenVerifier, authC
 
 	verifier.RegisterServer("Assets-API", "assets", AssetsService_AuthMethods)
 	router := mux.NewRouter()
-	router.Use(instanceInterceptor)
+	router.Use(instanceInterceptor, assetCacheInterceptor)
 	RegisterRoutes(router, h)
 	router.PathPrefix("/{owner}").Methods("GET").HandlerFunc(DownloadHandleFunc(h, h.GetFile()))
 	return http_util.CopyHeadersToContext(http_mw.CORSInterceptor(router))
@@ -190,6 +190,10 @@ func DownloadHandleFunc(s AssetsService, downloader Downloader) func(http.Respon
 }
 
 func GetAsset(w http.ResponseWriter, r *http.Request, resourceOwner, objectName string, storage static.Storage) error {
+	split := strings.Split(objectName, "?v=")
+	if len(split) == 2 {
+		objectName = split[0]
+	}
 	data, getInfo, err := storage.GetObject(r.Context(), authz.GetInstance(r.Context()).InstanceID(), resourceOwner, objectName)
 	if err != nil {
 		return fmt.Errorf("download failed: %v", err)
@@ -198,14 +202,16 @@ func GetAsset(w http.ResponseWriter, r *http.Request, resourceOwner, objectName
 	if err != nil {
 		return fmt.Errorf("download failed: %v", err)
 	}
-	if info.Hash == r.Header.Get(http_util.IfNoneMatch) {
+	if info.Hash == strings.Trim(r.Header.Get(http_util.IfNoneMatch), "\"") {
+		w.Header().Set(http_util.LastModified, info.LastModified.Format(time.RFC1123))
+		w.Header().Set(http_util.Etag, "\""+info.Hash+"\"")
 		w.WriteHeader(304)
 		return nil
 	}
 	w.Header().Set(http_util.ContentLength, strconv.FormatInt(info.Size, 10))
 	w.Header().Set(http_util.ContentType, info.ContentType)
 	w.Header().Set(http_util.LastModified, info.LastModified.Format(time.RFC1123))
-	w.Header().Set(http_util.Etag, info.Hash)
+	w.Header().Set(http_util.Etag, "\""+info.Hash+"\"")
 	_, err = w.Write(data)
 	logging.New().OnError(err).Error("error writing response for asset")
 	return nil

internal/api/http/middleware/cache_interceptor.go

@@ -3,7 +3,6 @@ package middleware
 import (
 	"fmt"
 	"net/http"
-	"regexp"
 	"strings"
 	"time"
 
@@ -24,16 +23,16 @@ type Cacheability string
 
 const (
 	CacheabilityNotSet  Cacheability = ""
-	CacheabilityPublic               = "public"
+	CacheabilityPublic  Cacheability = "public"
-	CacheabilityPrivate              = "private"
+	CacheabilityPrivate Cacheability = "private"
 )
 
 type Revalidation string
 
 const (
 	RevalidationNotSet Revalidation = ""
-	RevalidationMust                = "must-revalidate"
+	RevalidationMust   Revalidation = "must-revalidate"
-	RevalidationProxy               = "proxy-revalidate"
+	RevalidationProxy  Revalidation = "proxy-revalidate"
 )
 
 type CacheConfig struct {
@@ -54,40 +53,42 @@ var (
 	}
 )
 
-func DefaultCacheInterceptor(pattern string, maxAge, sharedMaxAge time.Duration) (func(http.Handler) http.Handler, error) {
+func NoCacheInterceptor() *cacheInterceptor {
-	regex, err := regexp.Compile(pattern)
+	return CacheInterceptorOpts(NeverCacheOptions)
-	if err != nil {
+}
-		return nil, err
+
+func AssetsCacheInterceptor(maxAge, sharedMaxAge time.Duration) *cacheInterceptor {
+	return CacheInterceptorOpts(AssetOptions(maxAge, sharedMaxAge))
+}
+
+func CacheInterceptorOpts(cache *Cache) *cacheInterceptor {
+	return &cacheInterceptor{
+		cache: cache,
 	}
-	return func(handler http.Handler) http.Handler {
-		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			if regex.MatchString(r.URL.Path) {
-				AssetsCacheInterceptor(maxAge, sharedMaxAge, handler).ServeHTTP(w, r)
-				return
-			}
-			NoCacheInterceptor(handler).ServeHTTP(w, r)
-		})
-	}, nil
 }
 
-func NoCacheInterceptor(h http.Handler) http.Handler {
+type cacheInterceptor struct {
-	return CacheInterceptorOpts(h, NeverCacheOptions)
+	cache *Cache
 }
 
-func AssetsCacheInterceptor(maxAge, sharedMaxAge time.Duration, h http.Handler) http.Handler {
+func (c *cacheInterceptor) Handler(next http.Handler) http.Handler {
-	return CacheInterceptorOpts(h, AssetOptions(maxAge, sharedMaxAge))
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-}
+		next.ServeHTTP(&cachingResponseWriter{
-
-func CacheInterceptorOpts(h http.Handler, cache *Cache) http.Handler {
-	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
-		cachingResponseWriter := &cachingResponseWriter{
 			ResponseWriter: w,
-			Cache:          cache,
+			Cache:          c.cache,
-		}
+		}, r)
-		h.ServeHTTP(cachingResponseWriter, req)
 	})
 }
 
+func (c *cacheInterceptor) HandlerFunc(next http.HandlerFunc) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		next.ServeHTTP(&cachingResponseWriter{
+			ResponseWriter: w,
+			Cache:          c.cache,
+		}, r)
+	}
+}
+
 type cachingResponseWriter struct {
 	http.ResponseWriter
 	*Cache
@@ -121,16 +122,16 @@ func (c *Cache) serializeHeaders(w http.ResponseWriter) {
 	expires := time.Now().UTC().Add(maxAge).Format(http.TimeFormat)
 
 	if c.NoCache {
-		control = append(control, fmt.Sprintf("no-cache"))
+		control = append(control, "no-cache")
 		pragma = true
 	}
 
 	if c.NoStore {
-		control = append(control, fmt.Sprintf("no-store"))
+		control = append(control, "no-store")
 		pragma = true
 	}
 	if c.NoTransform {
-		control = append(control, fmt.Sprintf("no-transform"))
+		control = append(control, "no-transform")
 	}
 
 	if c.Revalidation != RevalidationNotSet {

internal/api/oidc/op.go

@@ -123,7 +123,7 @@ func createOptions(config Config, externalSecure bool, userAgentCookie, instance
 		op.WithHttpInterceptors(
 			middleware.MetricsHandler(metricTypes),
 			middleware.TelemetryHandler(),
-			middleware.NoCacheInterceptor,
+			middleware.NoCacheInterceptor().Handler,
 			instanceHandler,
 			userAgentCookie,
 			http_utils.CopyHeadersToContext,

internal/api/ui/console/console.go

@@ -147,12 +147,11 @@ func assetsCacheInterceptorIgnoreManifest(shortMaxAge, shortSharedMaxAge, longMa
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			for _, file := range shortCacheFiles {
 				if r.URL.Path == file || isIndexOrSubPath(r.URL.Path) {
-					middleware.AssetsCacheInterceptor(shortMaxAge, shortSharedMaxAge, handler).ServeHTTP(w, r)
+					middleware.AssetsCacheInterceptor(shortMaxAge, shortSharedMaxAge).Handler(handler).ServeHTTP(w, r)
 					return
 				}
 			}
-			middleware.AssetsCacheInterceptor(longMaxAge, longSharedMaxAge, handler).ServeHTTP(w, r)
+			middleware.AssetsCacheInterceptor(longMaxAge, longSharedMaxAge).Handler(handler).ServeHTTP(w, r)
-			return
 		})
 	}
 }

internal/api/ui/login/login.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"net/http"
 	"strings"
+	"time"
 
 	"github.com/gorilla/csrf"
 	"github.com/gorilla/mux"
@@ -44,6 +45,7 @@ type Config struct {
 	LanguageCookieName string
 	CSRFCookieName     string
 	Cache              middleware.CacheConfig
+	AssetCache         middleware.CacheConfig
 }
 
 const (
@@ -62,7 +64,8 @@ func CreateLogin(config Config,
 	externalSecure bool,
 	userAgentCookie,
 	issuerInterceptor,
-	instanceHandler mux.MiddlewareFunc,
+	instanceHandler,
+	assetCache mux.MiddlewareFunc,
 	userCodeAlg crypto.EncryptionAlgorithm,
 	idpConfigAlg crypto.EncryptionAlgorithm,
 	csrfCookieKey []byte,
@@ -84,14 +87,8 @@ func CreateLogin(config Config,
 		return nil, fmt.Errorf("unable to create filesystem: %w", err)
 	}
 
-	csrfInterceptor, err := createCSRFInterceptor(config.CSRFCookieName, csrfCookieKey, externalSecure, login.csrfErrorHandler())
+	csrfInterceptor := createCSRFInterceptor(config.CSRFCookieName, csrfCookieKey, externalSecure, login.csrfErrorHandler())
-	if err != nil {
+	cacheInterceptor := createCacheInterceptor(config.Cache.MaxAge, config.Cache.SharedMaxAge, assetCache)
-		return nil, fmt.Errorf("unable to create csrfInterceptor: %w", err)
-	}
-	cacheInterceptor, err := middleware.DefaultCacheInterceptor(EndpointResources, config.Cache.MaxAge, config.Cache.SharedMaxAge)
-	if err != nil {
-		return nil, fmt.Errorf("unable to create cacheInterceptor: %w", err)
-	}
 	security := middleware.SecurityHeaders(csp(), login.cspErrorHandler)
 
 	login.router = CreateRouter(login, statikFS, middleware.TelemetryHandler(IgnoreInstanceEndpoints...), instanceHandler, csrfInterceptor, cacheInterceptor, security, userAgentCookie, issuerInterceptor)
@@ -108,7 +105,7 @@ func csp() *middleware.CSP {
 	return &csp
 }
 
-func createCSRFInterceptor(cookieName string, csrfCookieKey []byte, externalSecure bool, errorHandler http.Handler) (func(http.Handler) http.Handler, error) {
+func createCSRFInterceptor(cookieName string, csrfCookieKey []byte, externalSecure bool, errorHandler http.Handler) func(http.Handler) http.Handler {
 	path := "/"
 	return func(handler http.Handler) http.Handler {
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -123,7 +120,23 @@ func createCSRFInterceptor(cookieName string, csrfCookieKey []byte, externalSecu
 				csrf.ErrorHandler(errorHandler),
 			)(handler).ServeHTTP(w, r)
 		})
-	}, nil
+	}
+}
+
+func createCacheInterceptor(maxAge, sharedMaxAge time.Duration, assetCache mux.MiddlewareFunc) func(http.Handler) http.Handler {
+	return func(handler http.Handler) http.Handler {
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			if strings.HasPrefix(r.URL.Path, EndpointDynamicResources) {
+				assetCache.Middleware(handler).ServeHTTP(w, r)
+				return
+			}
+			if strings.HasPrefix(r.URL.Path, EndpointResources) {
+				middleware.AssetsCacheInterceptor(maxAge, sharedMaxAge).Handler(handler).ServeHTTP(w, r)
+				return
+			}
+			middleware.NoCacheInterceptor().Handler(handler).ServeHTTP(w, r)
+		})
+	}
 }
 
 func (l *Login) Handler() http.Handler {

internal/api/ui/login/renderer.go

@@ -8,6 +8,7 @@ import (
 	"net/http"
 	"path"
 	"strings"
+	"time"
 
 	"github.com/gorilla/csrf"
 	"github.com/zitadel/logging"
@@ -84,19 +85,13 @@ func CreateRenderer(pathPrefix string, staticDir http.FileSystem, staticStorage
 			return path.Join(r.pathPrefix, EndpointResources, "themes", theme, file)
 		},
 		"hasCustomPolicy": func(policy *domain.LabelPolicy) bool {
-			if policy != nil {
+			return policy != nil
-				return true
-			}
-			return false
 		},
 		"hasWatermark": func(policy *domain.LabelPolicy) bool {
-			if policy != nil && policy.DisableWatermark {
+			return policy == nil || !policy.DisableWatermark
-				return false
-			}
-			return true
 		},
 		"variablesCssFileUrl": func(orgID string, policy *domain.LabelPolicy) string {
-			cssFile := domain.CssPath + "/" + domain.CssVariablesFileName
+			cssFile := domain.CssPath + "/" + domain.CssVariablesFileName + "?v=" + policy.ChangeDate.Format(time.RFC3339)
 			return path.Join(r.pathPrefix, fmt.Sprintf("%s?%s=%s&%s=%v&%s=%s", EndpointDynamicResources, "orgId", orgID, "default-policy", policy.Default, "filename", cssFile))
 		},
 		"customLogoResource": func(orgID string, policy *domain.LabelPolicy, darkMode bool) string {

internal/command/user_human_avatar.go

@@ -25,7 +25,7 @@ func (c *Commands) AddHumanAvatar(ctx context.Context, orgID, userID string, upl
 		return nil, caos_errs.ThrowInternal(err, "USER-1Xyud", "Errors.Assets.Object.PutFailed")
 	}
 	userAgg := UserAggregateFromWriteModel(&existingUser.WriteModel)
-	pushedEvents, err := c.eventstore.Push(ctx, user.NewHumanAvatarAddedEvent(ctx, userAgg, asset.Name))
+	pushedEvents, err := c.eventstore.Push(ctx, user.NewHumanAvatarAddedEvent(ctx, userAgg, asset.VersionedName()))
 	if err != nil {
 		return nil, err
 	}

internal/command/user_human_avatar_test.go

@@ -155,7 +155,7 @@ func TestCommandSide_AddHumanAvatar(t *testing.T) {
 							eventFromEventPusher(
 								user.NewHumanAvatarAddedEvent(context.Background(),
 									&user.NewAggregate("user1", "org1").Aggregate,
-									"avatar",
+									"avatar?v=test",
 								),
 							),
 						},

internal/static/config/config.go

@@ -3,6 +3,7 @@ package config
 import (
 	"database/sql"
 
+	"github.com/zitadel/zitadel/internal/api/http/middleware"
 	"github.com/zitadel/zitadel/internal/errors"
 	"github.com/zitadel/zitadel/internal/static"
 	"github.com/zitadel/zitadel/internal/static/database"
@@ -11,6 +12,7 @@ import (
 
 type AssetStorageConfig struct {
 	Type   string
+	Cache  middleware.CacheConfig
 	Config map[string]interface{} `mapstructure:",remain"`
 }
 

internal/static/storage.go

@@ -35,3 +35,7 @@ type Asset struct {
 	Location      string
 	ContentType   string
 }
+
+func (a *Asset) VersionedName() string {
+	return a.Name + "?v=" + a.Hash
+}