From dcac08b1d5107b8ce96e8083ec99c3c3662a5764 Mon Sep 17 00:00:00 2001 From: Livio Spring Date: Tue, 16 Aug 2022 07:04:36 +0200 Subject: [PATCH] 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 --- cmd/defaults.yaml | 11 ++- cmd/start/start.go | 5 +- internal/api/assets/asset.go | 14 ++-- .../api/http/middleware/cache_interceptor.go | 67 ++++++++++--------- internal/api/oidc/op.go | 2 +- internal/api/ui/console/console.go | 5 +- internal/api/ui/login/login.go | 35 +++++++--- internal/api/ui/login/renderer.go | 13 ++-- internal/command/user_human_avatar.go | 2 +- internal/command/user_human_avatar_test.go | 2 +- internal/static/config/config.go | 2 + internal/static/storage.go | 4 ++ 12 files changed, 96 insertions(+), 66 deletions(-) diff --git a/cmd/defaults.yaml b/cmd/defaults.yaml index ce9fa26d97..2034957f51 100644 --- a/cmd/defaults.yaml +++ b/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: diff --git a/cmd/start/start.go b/cmd/start/start.go index 9d152aae8a..6df8501bd6 100644 --- a/cmd/start/start.go +++ b/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) } diff --git a/internal/api/assets/asset.go b/internal/api/assets/asset.go index 3ff3feeda7..c417230b24 100644 --- a/internal/api/assets/asset.go +++ b/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 diff --git a/internal/api/http/middleware/cache_interceptor.go b/internal/api/http/middleware/cache_interceptor.go index 5a8af8b802..04dbf2b6c9 100644 --- a/internal/api/http/middleware/cache_interceptor.go +++ b/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" - CacheabilityPrivate = "private" + CacheabilityPublic Cacheability = "public" + CacheabilityPrivate Cacheability = "private" ) type Revalidation string const ( RevalidationNotSet Revalidation = "" - RevalidationMust = "must-revalidate" - RevalidationProxy = "proxy-revalidate" + RevalidationMust Revalidation = "must-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) { - regex, err := regexp.Compile(pattern) - if err != nil { - return nil, err +func NoCacheInterceptor() *cacheInterceptor { + return CacheInterceptorOpts(NeverCacheOptions) +} + +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 { - return CacheInterceptorOpts(h, NeverCacheOptions) +type cacheInterceptor struct { + cache *Cache } -func AssetsCacheInterceptor(maxAge, sharedMaxAge time.Duration, h http.Handler) http.Handler { - return CacheInterceptorOpts(h, AssetOptions(maxAge, sharedMaxAge)) -} - -func CacheInterceptorOpts(h http.Handler, cache *Cache) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - cachingResponseWriter := &cachingResponseWriter{ +func (c *cacheInterceptor) Handler(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + next.ServeHTTP(&cachingResponseWriter{ ResponseWriter: w, - Cache: cache, - } - h.ServeHTTP(cachingResponseWriter, req) + Cache: c.cache, + }, r) }) } +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 { diff --git a/internal/api/oidc/op.go b/internal/api/oidc/op.go index b2db6c5233..a3ab352a28 100644 --- a/internal/api/oidc/op.go +++ b/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, diff --git a/internal/api/ui/console/console.go b/internal/api/ui/console/console.go index ede686fc04..04785576fe 100644 --- a/internal/api/ui/console/console.go +++ b/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) - return + middleware.AssetsCacheInterceptor(longMaxAge, longSharedMaxAge).Handler(handler).ServeHTTP(w, r) }) } } diff --git a/internal/api/ui/login/login.go b/internal/api/ui/login/login.go index 43aa6d6f69..19964a1a39 100644 --- a/internal/api/ui/login/login.go +++ b/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()) - if err != nil { - 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) - } + csrfInterceptor := createCSRFInterceptor(config.CSRFCookieName, csrfCookieKey, externalSecure, login.csrfErrorHandler()) + cacheInterceptor := createCacheInterceptor(config.Cache.MaxAge, config.Cache.SharedMaxAge, assetCache) 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 { diff --git a/internal/api/ui/login/renderer.go b/internal/api/ui/login/renderer.go index 2de8692f7d..c083a4f479 100644 --- a/internal/api/ui/login/renderer.go +++ b/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 true - } - return false + return policy != nil }, "hasWatermark": func(policy *domain.LabelPolicy) bool { - if policy != nil && policy.DisableWatermark { - return false - } - return true + return policy == nil || !policy.DisableWatermark }, "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 { diff --git a/internal/command/user_human_avatar.go b/internal/command/user_human_avatar.go index b70fabdd77..8994bda945 100644 --- a/internal/command/user_human_avatar.go +++ b/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 } diff --git a/internal/command/user_human_avatar_test.go b/internal/command/user_human_avatar_test.go index 4c2a62dfbf..0f6f19d327 100644 --- a/internal/command/user_human_avatar_test.go +++ b/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", ), ), }, diff --git a/internal/static/config/config.go b/internal/static/config/config.go index 334c3b6fbb..2ace6a731e 100644 --- a/internal/static/config/config.go +++ b/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"` } diff --git a/internal/static/storage.go b/internal/static/storage.go index a8520320e8..ba42575edb 100644 --- a/internal/static/storage.go +++ b/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 +}