feat: make http session cookie path & duration configurable (fixes #10522) (#10632)

Signed-off-by: Vikram Vaswani <2571660+vvaswani@users.noreply.github.com>
Signed-off-by: Jakob Borg <jakob@kastelo.net>
Co-authored-by: Jakob Borg <jakob@kastelo.net>

lib/api/api_auth.go

@@ -25,7 +25,6 @@ import (
 )
 
 const (
-	maxSessionLifetime  = 7 * 24 * time.Hour
 	maxActiveSessions   = 25
 	randomTokenLength   = 64
 	maxLoginRequestSize = 1 << 10 // one kibibyte for username+password

lib/api/api_auth_test.go

@@ -7,6 +7,7 @@
 package api
 
 import (
+	"math"
 	"testing"
 	"time"
 
@@ -192,3 +193,43 @@ func TestTokenManager(t *testing.T) {
 		t.Errorf("token %q should be invalid", t3)
 	}
 }
+
+func TestTokenManagerNoExpiry(t *testing.T) {
+	t.Parallel()
+
+	mdb, err := sqlite.Open(t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() {
+		mdb.Close()
+	})
+	kdb := db.NewMiscDB(mdb)
+	clock := &mockClock{now: time.Now()}
+
+	tm := newTokenManager("testTokensNoExpiry", kdb, -1, 3)
+	tm.timeNow = clock.Now
+
+	token := tm.New()
+	if expiry, ok := tm.tokens.Tokens[token]; !ok || expiry != 0 {
+		t.Fatalf("token should have no expiry, got %d", expiry)
+	}
+
+	clock.wind(365 * 24 * time.Hour)
+	if !tm.Check(token) {
+		t.Fatal("token should still be valid after long time when no-expiry is enabled")
+	}
+}
+
+func TestSessionCookieMaxAgeNoExpiry(t *testing.T) {
+	t.Parallel()
+
+	m := tokenCookieManager{
+		guiCfg: config.GUIConfiguration{
+			SessionCookieDurationS: -1,
+		},
+	}
+	if got := m.sessionCookieMaxAge(); got != math.MaxInt32 {
+		t.Fatalf("unexpected max age %d", got)
+	}
+}

lib/api/tokenmanager.go

@@ -7,6 +7,7 @@
 package api
 
 import (
+	"math"
 	"net/http"
 	"slices"
 	"strings"
@@ -35,6 +36,8 @@ type tokenManager struct {
 	saveTimer *time.Timer
 }
 
+const defaultSessionCookieDurationS = 7 * 24 * 60 * 60
+
 func newTokenManager(key string, miscDB *db.Typed, lifetime time.Duration, maxItems int) *tokenManager {
 	var tokens apiproto.TokenSet
 	if bs, ok, _ := miscDB.Bytes(key); ok {
@@ -60,18 +63,19 @@ func (m *tokenManager) Check(token string) bool {
 	defer m.mut.Unlock()
 
 	expires, ok := m.tokens.Tokens[token]
-	if ok {
-		if expires < m.timeNow().UnixNano() {
-			// The token is expired.
-			m.saveLocked() // removes expired tokens
-			return false
-		}
-
-		// Give the token further life.
-		m.tokens.Tokens[token] = m.timeNow().Add(m.lifetime).UnixNano()
-		m.saveLocked()
+	if !ok {
+		return false
 	}
-	return ok
+	if expires != 0 && expires < m.timeNow().UnixNano() {
+		// The token is expired.
+		m.saveLocked() // removes expired tokens
+		return false
+	}
+
+	// Give the token further life.
+	m.tokens.Tokens[token] = m.newExpiryNanos()
+	m.saveLocked()
+	return true
 }
 
 // New creates a new token and returns it.
@@ -81,12 +85,19 @@ func (m *tokenManager) New() string {
 	m.mut.Lock()
 	defer m.mut.Unlock()
 
-	m.tokens.Tokens[token] = m.timeNow().Add(m.lifetime).UnixNano()
+	m.tokens.Tokens[token] = m.newExpiryNanos()
 	m.saveLocked()
 
 	return token
 }
 
+func (m *tokenManager) newExpiryNanos() int64 {
+	if m.lifetime <= 0 {
+		return 0
+	}
+	return m.timeNow().Add(m.lifetime).UnixNano()
+}
+
 // Delete removes a token.
 func (m *tokenManager) Delete(token string) {
 	m.mut.Lock()
@@ -100,7 +111,7 @@ func (m *tokenManager) saveLocked() {
 	// Remove expired tokens.
 	now := m.timeNow().UnixNano()
 	for token, expiry := range m.tokens.Tokens {
-		if expiry < now {
+		if expiry != 0 && expiry < now {
 			delete(m.tokens.Tokens, token)
 		}
 	}
@@ -152,12 +163,16 @@ type tokenCookieManager struct {
 }
 
 func newTokenCookieManager(shortID string, guiCfg config.GUIConfiguration, evLogger events.Logger, miscDB *db.Typed) *tokenCookieManager {
+	sessionLifetimeS := guiCfg.SessionCookieDurationS
+	if sessionLifetimeS == 0 {
+		sessionLifetimeS = defaultSessionCookieDurationS
+	}
 	return &tokenCookieManager{
 		cookieName: "sessionid-" + shortID,
 		shortID:    shortID,
 		guiCfg:     guiCfg,
 		evLogger:   evLogger,
-		tokens:     newTokenManager("sessions", miscDB, maxSessionLifetime, maxActiveSessions),
+		tokens:     newTokenManager("sessions", miscDB, time.Duration(sessionLifetimeS)*time.Second, maxActiveSessions),
 	}
 }
 
@@ -176,7 +191,11 @@ func (m *tokenCookieManager) createSession(username string, persistent bool, w h
 
 	maxAge := 0
 	if persistent {
-		maxAge = int(maxSessionLifetime.Seconds())
+		maxAge = m.sessionCookieMaxAge()
+	}
+	path := m.guiCfg.SessionCookiePath
+	if path == "" {
+		path = "/"
 	}
 	http.SetCookie(w, &http.Cookie{
 		Name:  m.cookieName,
@@ -185,12 +204,26 @@ func (m *tokenCookieManager) createSession(username string, persistent bool, w h
 		// but in http.Cookie MaxAge = 0 means unspecified (session) and MaxAge < 0 means delete immediately
 		MaxAge: maxAge,
 		Secure: useSecureCookie,
-		Path:   "/",
+		Path:   path,
 	})
 
 	emitLoginAttempt(true, username, r, m.evLogger)
 }
 
+func (m *tokenCookieManager) sessionCookieMaxAge() int {
+	switch {
+	case m.guiCfg.SessionCookieDurationS < 0:
+		// A negative value means "never expire the cookie". Use a very
+		// large Max-Age to make the browser keep the cookie for a long
+		// time.
+		return math.MaxInt32
+	case m.guiCfg.SessionCookieDurationS == 0:
+		return defaultSessionCookieDurationS
+	default:
+		return m.guiCfg.SessionCookieDurationS
+	}
+}
+
 func (m *tokenCookieManager) hasValidSession(r *http.Request) bool {
 	for _, cookie := range r.Cookies() {
 		// We iterate here since there may, historically, be multiple

lib/config/config_test.go

@@ -786,6 +786,32 @@ func TestGUIConfigURL(t *testing.T) {
 	}
 }
 
+func TestGUISessionCookiePathPrepare(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		name string
+		in   string
+		out  string
+	}{
+		{name: "empty stays empty", in: "", out: ""},
+		{name: "already rooted", in: " /gui ", out: "/gui"},
+		{name: "needs slash", in: "gui", out: "/gui"},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			c := GUIConfiguration{SessionCookiePath: tc.in}
+			c.prepare()
+			if c.SessionCookiePath != tc.out {
+				t.Fatalf("unexpected path %q != %q", c.SessionCookiePath, tc.out)
+			}
+		})
+	}
+}
+
 func TestGUIPasswordHash(t *testing.T) {
 	var c GUIConfiguration
 

lib/config/guiconfiguration.go

@@ -33,6 +33,8 @@ type GUIConfiguration struct {
 	InsecureSkipHostCheck     bool     `json:"insecureSkipHostcheck" xml:"insecureSkipHostcheck,omitempty"`
 	InsecureAllowFrameLoading bool     `json:"insecureAllowFrameLoading" xml:"insecureAllowFrameLoading,omitempty"`
 	SendBasicAuthPrompt       bool     `json:"sendBasicAuthPrompt" xml:"sendBasicAuthPrompt,attr"`
+	SessionCookieDurationS    int      `json:"sessionCookieDurationS" xml:"sessionCookieDurationS,omitempty" default:"604800"`
+	SessionCookiePath         string   `json:"sessionCookiePath" xml:"sessionCookiePath,omitempty" default:"/"`
 }
 
 func (c GUIConfiguration) IsAuthEnabled() bool {
@@ -176,6 +178,12 @@ func (c *GUIConfiguration) prepare() {
 	if c.APIKey == "" {
 		c.APIKey = rand.String(32)
 	}
+	if path := strings.TrimSpace(c.SessionCookiePath); path != "" {
+		if !strings.HasPrefix(path, "/") {
+			path = "/" + path
+		}
+		c.SessionCookiePath = path
+	}
 }
 
 func (c GUIConfiguration) Copy() GUIConfiguration {