fix(sessions): periodically cleanup expired sessions (#1939)

Signed-off-by: Petu Eusebiu <peusebiu@cisco.com>

pkg/api/authn.go

@@ -6,13 +6,11 @@ import (
 	"crypto/sha256"
 	"crypto/x509"
 	"encoding/base64"
-	"encoding/gob"
 	"errors"
 	"fmt"
 	"net"
 	"net/http"
 	"os"
-	"path"
 	"strconv"
 	"strings"
 	"time"
@@ -39,7 +37,6 @@ import (
 	zcommon "zotregistry.io/zot/pkg/common"
 	"zotregistry.io/zot/pkg/log"
 	reqCtx "zotregistry.io/zot/pkg/requestcontext"
-	storageConstants "zotregistry.io/zot/pkg/storage/constants"
 )
 
 const (
@@ -260,38 +257,6 @@ func (amw *AuthnMiddleware) TryAuthnHandlers(ctlr *Controller) mux.MiddlewareFun
 
 	delay := ctlr.Config.HTTP.Auth.FailDelay
 
-	// setup sessions cookie store used to preserve logged in user in web sessions
-	if ctlr.Config.IsBasicAuthnEnabled() {
-		// To store custom types in our cookies
-		// we must first register them using gob.Register
-		gob.Register(map[string]interface{}{})
-
-		cookieStoreHashKey := securecookie.GenerateRandomKey(64)
-		if cookieStoreHashKey == nil {
-			panic(zerr.ErrHashKeyNotCreated)
-		}
-
-		// if storage is filesystem then use zot's rootDir to store sessions
-		if ctlr.Config.Storage.StorageDriver == nil {
-			sessionsDir := path.Join(ctlr.Config.Storage.RootDirectory, "_sessions")
-			if err := os.MkdirAll(sessionsDir, storageConstants.DefaultDirPerms); err != nil {
-				panic(err)
-			}
-
-			cookieStore := sessions.NewFilesystemStore(sessionsDir, cookieStoreHashKey)
-
-			cookieStore.MaxAge(cookiesMaxAge)
-
-			ctlr.CookieStore = cookieStore
-		} else {
-			cookieStore := sessions.NewCookieStore(cookieStoreHashKey)
-
-			cookieStore.MaxAge(cookiesMaxAge)
-
-			ctlr.CookieStore = cookieStore
-		}
-	}
-
 	// ldap and htpasswd based authN
 	if ctlr.Config.IsLdapAuthEnabled() {
 		ldapConfig := ctlr.Config.HTTP.Auth.LDAP

pkg/api/controller.go

@@ -16,7 +16,6 @@ import (
 
 	"github.com/gorilla/handlers"
 	"github.com/gorilla/mux"
-	"github.com/gorilla/sessions"
 	"github.com/zitadel/oidc/pkg/client/rp"
 
 	"zotregistry.io/zot/errors"
@@ -35,7 +34,6 @@ import (
 const (
 	idleTimeout       = 120 * time.Second
 	readHeaderTimeout = 5 * time.Second
-	cookiesMaxAge     = 86400 // seconds
 )
 
 type Controller struct {
@@ -50,7 +48,7 @@ type Controller struct {
 	CveScanner      ext.CveScanner
 	SyncOnDemand    SyncOnDemand
 	RelyingParties  map[string]rp.RelyingParty
-	CookieStore     sessions.Store
+	CookieStore     *CookieStore
 	// runtime params
 	chosenPort int // kernel-chosen port
 }
@@ -96,6 +94,10 @@ func (c *Controller) GetPort() int {
 }
 
 func (c *Controller) Run(reloadCtx context.Context) error {
+	if err := c.initCookieStore(); err != nil {
+		return err
+	}
+
 	c.StartBackgroundTasks(reloadCtx)
 
 	// setup HTTP API router
@@ -259,6 +261,20 @@ func (c *Controller) InitImageStore() error {
 	return nil
 }
 
+func (c *Controller) initCookieStore() error {
+	// setup sessions cookie store used to preserve logged in user in web sessions
+	if c.Config.IsBasicAuthnEnabled() {
+		cookieStore, err := NewCookieStore(c.StoreController)
+		if err != nil {
+			return err
+		}
+
+		c.CookieStore = cookieStore
+	}
+
+	return nil
+}
+
 func (c *Controller) InitMetaDB(reloadCtx context.Context) error {
 	// init metaDB if search is enabled or we need to store user profiles, api keys or signatures
 	if c.Config.IsSearchEnabled() || c.Config.IsBasicAuthnEnabled() || c.Config.IsImageTrustEnabled() {
@@ -395,6 +411,10 @@ func (c *Controller) StartBackgroundTasks(reloadCtx context.Context) {
 		c.SyncOnDemand = syncOnDemand
 	}
 
+	if c.CookieStore != nil {
+		c.CookieStore.RunSessionCleaner(taskScheduler)
+	}
+
 	// we can later move enabling the other scheduled tasks inside the call below
 	ext.EnableScheduledTasks(c.Config, taskScheduler, c.MetaDB, c.Log) //nolint: contextcheck
 }

pkg/api/controller_test.go

@@ -3409,7 +3409,7 @@ func TestAuthnSessionErrors(t *testing.T) {
 				session.IsNew = false
 				session.Values["authStatus"] = true
 
-				cookieStore, ok := ctlr.CookieStore.(*sessions.FilesystemStore)
+				cookieStore, ok := ctlr.CookieStore.Store.(*sessions.FilesystemStore)
 				So(ok, ShouldBeTrue)
 
 				// first encode sessionID
@@ -3450,7 +3450,7 @@ func TestAuthnSessionErrors(t *testing.T) {
 				session.Values["authStatus"] = false
 				session.Values["username"] = username
 
-				cookieStore, ok := ctlr.CookieStore.(*sessions.FilesystemStore)
+				cookieStore, ok := ctlr.CookieStore.Store.(*sessions.FilesystemStore)
 				So(ok, ShouldBeTrue)
 
 				// first encode sessionID

pkg/api/cookiestore.go

@@ -0,0 +1,159 @@
+package api
+
+import (
+	"context"
+	"encoding/gob"
+	"io/fs"
+	"os"
+	"path"
+	"path/filepath"
+	"strings"
+	"time"
+
+	"github.com/gorilla/securecookie"
+	"github.com/gorilla/sessions"
+
+	zerr "zotregistry.io/zot/errors"
+	"zotregistry.io/zot/pkg/scheduler"
+	"zotregistry.io/zot/pkg/storage"
+	storageConstants "zotregistry.io/zot/pkg/storage/constants"
+)
+
+const cookiesMaxAge = 7200 // 2h
+
+type CookieStore struct {
+	needsCleanup bool // if store should be periodically cleaned
+	rootDir      string
+	sessions.Store
+}
+
+func (c *CookieStore) RunSessionCleaner(sch *scheduler.Scheduler) {
+	if c.needsCleanup {
+		sch.SubmitGenerator(&SessionCleanup{rootDir: c.rootDir}, cookiesMaxAge, scheduler.LowPriority)
+	}
+}
+
+func NewCookieStore(storeController storage.StoreController) (*CookieStore, error) {
+	// To store custom types in our cookies
+	// we must first register them using gob.Register
+	gob.Register(map[string]interface{}{})
+
+	hashKey, err := getHashKey()
+	if err != nil {
+		return &CookieStore{}, err
+	}
+
+	var store sessions.Store
+
+	var sessionsDir string
+
+	var needsCleanup bool
+
+	if storeController.DefaultStore.Name() == storageConstants.LocalStorageDriverName {
+		sessionsDir = path.Join(storeController.DefaultStore.RootDir(), "_sessions")
+		if err := os.MkdirAll(sessionsDir, storageConstants.DefaultDirPerms); err != nil {
+			return &CookieStore{}, err
+		}
+
+		localStore := sessions.NewFilesystemStore(sessionsDir, hashKey)
+
+		localStore.MaxAge(cookiesMaxAge)
+
+		store = localStore
+		needsCleanup = true
+	} else {
+		memStore := sessions.NewCookieStore(hashKey)
+
+		memStore.MaxAge(cookiesMaxAge)
+
+		store = memStore
+	}
+
+	return &CookieStore{
+		Store:        store,
+		rootDir:      sessionsDir,
+		needsCleanup: needsCleanup,
+	}, nil
+}
+
+func getHashKey() ([]byte, error) {
+	hashKey := securecookie.GenerateRandomKey(64)
+	if hashKey == nil {
+		return nil, zerr.ErrHashKeyNotCreated
+	}
+
+	return hashKey, nil
+}
+
+func getExpiredSessions(dir string) ([]string, error) {
+	sessions := make([]string, 0)
+
+	err := filepath.WalkDir(dir, func(_ string, dirEntry fs.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+
+		fileInfo, err := dirEntry.Info()
+		if err != nil { // may have been deleted in the meantime
+			return nil //nolint: nilerr
+		}
+
+		if !strings.HasPrefix(fileInfo.Name(), "session_") {
+			return nil
+		}
+
+		if fileInfo.ModTime().Add(cookiesMaxAge * time.Second).Before(time.Now()) {
+			sessions = append(sessions, path.Join(dir, fileInfo.Name()))
+		}
+
+		return nil
+	})
+
+	return sessions, err
+}
+
+type SessionCleanup struct {
+	rootDir string
+	done    bool
+}
+
+func (gen *SessionCleanup) Next() (scheduler.Task, error) {
+	sessions, err := getExpiredSessions(gen.rootDir)
+	if err != nil {
+		return nil, err
+	}
+
+	if len(sessions) == 0 {
+		gen.done = true
+
+		return nil, nil
+	}
+
+	return &CleanTask{sessions: sessions}, nil
+}
+
+func (gen *SessionCleanup) IsDone() bool {
+	return gen.done
+}
+
+func (gen *SessionCleanup) IsReady() bool {
+	return true
+}
+
+func (gen *SessionCleanup) Reset() {
+	gen.done = false
+}
+
+type CleanTask struct {
+	sessions []string
+}
+
+func (cleanTask *CleanTask) DoWork(ctx context.Context) error {
+	for _, session := range cleanTask.sessions {
+		if err := os.Remove(session); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}

pkg/storage/imagestore/imagestore.go

@@ -52,6 +52,10 @@ type ImageStore struct {
 	commit      bool
 }
 
+func (is *ImageStore) Name() string {
+	return is.storeDriver.Name()
+}
+
 func (is *ImageStore) RootDir() string {
 	return is.rootDir
 }

pkg/storage/types/types.go

@@ -19,6 +19,7 @@ type StoreController interface {
 }
 
 type ImageStore interface { //nolint:interfacebloat
+	Name() string
 	DirExists(d string) bool
 	RootDir() string
 	RLock(*time.Time)

pkg/test/mocks/image_store_mock.go

@@ -12,6 +12,7 @@ import (
 )
 
 type MockedImageStore struct {
+	NameFn              func() string
 	DirExistsFn         func(d string) bool
 	RootDirFn           func() string
 	InitRepoFn          func(name string) error
@@ -68,6 +69,14 @@ func (is MockedImageStore) RUnlock(t *time.Time) {
 func (is MockedImageStore) RLock(t *time.Time) {
 }
 
+func (is MockedImageStore) Name() string {
+	if is.NameFn != nil {
+		return is.NameFn()
+	}
+
+	return ""
+}
+
 func (is MockedImageStore) DirExists(d string) bool {
 	if is.DirExistsFn != nil {
 		return is.DirExistsFn(d)