Improve session expiration handling (#43)

* Account for session timeout changes in cleanup; fix default timeout

* Upgrade some dependencies

* Upgrade goreleaser

.github/workflows/build.yml

@@ -26,7 +26,7 @@ jobs:
 
       - uses: open-policy-agent/setup-opa@v2
         with:
-          version: 0.45
+          version: 0.48
 
       - name: Build
         run: make ci

database/conf.go

@@ -21,7 +21,7 @@ type Config struct {
 func GetConfig() (c Config, err error) {
 	c.URI = env.GetString(envPrefix+"URI", "file:db.sqlite3")
 
-	c.Flags = sqlite.OpenURI | sqlite.OpenNoMutex | sqlite.OpenSharedCache | sqlite.OpenWAL
+	c.Flags = sqlite.OpenURI | sqlite.OpenSharedCache | sqlite.OpenWAL
 	memory, err := env.GetBool(envPrefix + "MEMORY")
 	if err != nil {
 		return Config{}, errors.Wrap(err, "couldn't make SQLite in-memory config")

go.mod

@@ -5,44 +5,41 @@ go 1.19
 require (
 	github.com/alexedwards/argon2id v0.0.0-20211130144151-3585854a6387
 	github.com/benbjohnson/hashfs v0.2.1
-	github.com/bmatcuk/doublestar/v4 v4.4.0
+	github.com/bmatcuk/doublestar/v4 v4.6.0
 	github.com/dgraph-io/ristretto v0.1.1
 	github.com/gorilla/csrf v1.7.1
 	github.com/gorilla/securecookie v1.1.1
 	github.com/gorilla/sessions v1.2.1
 	github.com/gorilla/websocket v1.5.0
-	github.com/labstack/echo/v4 v4.9.1
-	github.com/open-policy-agent/opa v0.46.1
+	github.com/labstack/echo/v4 v4.10.0
+	github.com/open-policy-agent/opa v0.49.0
 	github.com/pkg/errors v0.9.1
 	github.com/quasoft/memstore v0.0.0-20191010062613-2bce066d2b0b
 	github.com/twmb/murmur3 v1.1.6
 	github.com/vmihailenco/msgpack/v5 v5.3.5
 	golang.org/x/sync v0.1.0
-	zombiezen.com/go/sqlite v0.10.1
+	zombiezen.com/go/sqlite v0.12.0
 )
 
 require (
 	github.com/OneOfOne/xxhash v1.2.8 // indirect
 	github.com/agnivade/levenshtein v1.1.1 // indirect
-	github.com/cespare/xxhash/v2 v2.1.2 // indirect
-	github.com/dgraph-io/badger/v3 v3.2103.4 // indirect
-	github.com/dustin/go-humanize v1.0.0 // indirect
+	github.com/cespare/xxhash/v2 v2.2.0 // indirect
+	github.com/dustin/go-humanize v1.0.1 // indirect
 	github.com/ghodss/yaml v1.0.0 // indirect
 	github.com/gobwas/glob v0.2.3 // indirect
 	github.com/golang/glog v1.0.0 // indirect
-	github.com/google/flatbuffers v22.10.26+incompatible // indirect
-	github.com/google/go-cmp v0.5.9 // indirect
+	github.com/google/flatbuffers v23.1.21+incompatible // indirect
 	github.com/google/uuid v1.3.0 // indirect
-	github.com/klauspost/compress v1.15.12 // indirect
+	github.com/klauspost/compress v1.15.15 // indirect
 	github.com/kr/pretty v0.1.0 // indirect
 	github.com/labstack/gommon v0.4.0 // indirect
 	github.com/mattn/go-colorable v0.1.13 // indirect
-	github.com/mattn/go-isatty v0.0.16 // indirect
-	github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect
-	github.com/prometheus/client_model v0.3.0 // indirect
+	github.com/mattn/go-isatty v0.0.17 // indirect
+	github.com/prometheus/common v0.39.0 // indirect
+	github.com/prometheus/procfs v0.9.0 // indirect
 	github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475 // indirect
 	github.com/remyoudompheng/bigfft v0.0.0-20200410134404-eec4a21b6bb0 // indirect
-	github.com/stretchr/testify v1.8.1 // indirect
 	github.com/tchap/go-patricia/v2 v2.3.1 // indirect
 	github.com/valyala/bytebufferpool v1.0.0 // indirect
 	github.com/valyala/fasttemplate v1.2.2 // indirect
@@ -51,13 +48,13 @@ require (
 	github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect
 	github.com/yashtewari/glob-intersection v0.1.0 // indirect
 	go.opencensus.io v0.24.0 // indirect
-	golang.org/x/crypto v0.1.0 // indirect
-	golang.org/x/net v0.1.0 // indirect
-	golang.org/x/sys v0.2.0 // indirect
-	golang.org/x/text v0.4.0 // indirect
+	golang.org/x/crypto v0.6.0 // indirect
+	golang.org/x/net v0.6.0 // indirect
+	golang.org/x/sys v0.5.0 // indirect
+	golang.org/x/text v0.7.0 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect
-	modernc.org/libc v1.16.7 // indirect
-	modernc.org/mathutil v1.4.1 // indirect
-	modernc.org/memory v1.1.1 // indirect
-	modernc.org/sqlite v1.17.3 // indirect
+	modernc.org/libc v1.21.5 // indirect
+	modernc.org/mathutil v1.5.0 // indirect
+	modernc.org/memory v1.4.0 // indirect
+	modernc.org/sqlite v1.20.0 // indirect
 )

handling/time.go

@@ -12,13 +12,27 @@ func Repeat(ctx context.Context, interval time.Duration, f Worker) error {
 	if interval == 0 {
 		return repeatInstantly(ctx, f)
 	}
-	ticker := time.NewTicker(interval)
-	defer ticker.Stop()
+	return repeatDelayed(ctx, interval, f)
+}
+
+func RepeatImmediate(ctx context.Context, interval time.Duration, f Worker) error {
+	done, err := f()
+	if err != nil {
+		return err
+	}
+	if done {
+		return nil
+	}
+
+	return Repeat(ctx, interval, f)
+}
+
+func repeatInstantly(ctx context.Context, f Worker) error {
 	for {
 		select {
 		case <-ctx.Done():
 			return ctx.Err()
-		case <-ticker.C:
+		default:
 			if err := ctx.Err(); err != nil {
 				// Context was also canceled and it should have priority
 				return err
@@ -35,12 +49,14 @@ func Repeat(ctx context.Context, interval time.Duration, f Worker) error {
 	}
 }
 
-func repeatInstantly(ctx context.Context, f Worker) error {
+func repeatDelayed(ctx context.Context, interval time.Duration, f Worker) error {
+	ticker := time.NewTicker(interval)
+	defer ticker.Stop()
 	for {
 		select {
 		case <-ctx.Done():
 			return ctx.Err()
-		default:
+		case <-ticker.C:
 			if err := ctx.Err(); err != nil {
 				// Context was also canceled and it should have priority
 				return err

opa/errors.rego

@@ -20,9 +20,7 @@ new(message) := {
 is_error(result) if {
 	result.type == "error"
 	some "message", _ in result
-} else = false if {
-	true
-}
+} else = false
 
 # METADATA
 # description: |

session/conf.go

@@ -69,7 +69,7 @@ func GetConfig() (c Config, err error) {
 }
 
 func getTimeouts() (t Timeouts, err error) {
-	const defaultAbsolute = 60 * 24 * 7 * 24 // default: 1 week
+	const defaultAbsolute = 60 * 24 * 7 // default: 1 week
 	rawAbsolute, err := env.GetInt64(envPrefix+"TIMEOUTS_ABSOLUTE", defaultAbsolute)
 	if err != nil {
 		return Timeouts{}, errors.Wrap(err, "couldn't make absolute timeout config")

session/sqlitestore/models.go

@@ -40,6 +40,13 @@ func (s Session) newDelete() map[string]interface{} {
 	}
 }
 
+func (s Session) newDeletePastCreationTimeout(timeout time.Duration) map[string]interface{} {
+	return map[string]interface{}{
+		"$expiration_timeout":        timeout.Milliseconds(),
+		"$expiration_time_threshold": s.ExpirationTime.UnixMilli(),
+	}
+}
+
 func (s Session) newDeletePastExpiration() map[string]interface{} {
 	return map[string]interface{}{
 		"$expiration_time_threshold": s.ExpirationTime.UnixMilli(),

session/sqlitestore/queries/delete-sessions-past-creation-timeout.sql

@@ -0,0 +1,2 @@
+delete from sessions_session
+where sessions_session.creation_time + $expiration_timeout < $expiration_time_threshold

session/sqlitestore/store.go

@@ -35,10 +35,10 @@ type SqliteStore struct {
 func NewSqliteStore(
 	db *database.DB, absoluteTimeout time.Duration, keyPairs ...[]byte,
 ) *SqliteStore {
-	// TODO: also take a parameter for a function to modify a Session with *sssion.Session.Valuesa,
+	// TODO: also take a parameter for a function to modify a Session with *sssion.Session.Values,
 	// which would enable e.g. storing the user identity in a column rather than an encoded string,
 	// to enable selecting all sessions fo a user
-	// We don't allow specifying the table name programatically (which would require turning our
+	// We don't allow specifying the table name programmatically (which would require turning our
 	// embedded migrations and queries into Go templates to render into sql), because renaming the
 	// table would require its own dedicated migration, and it's too much complexity to deal with that
 	// use case.
@@ -230,6 +230,23 @@ func (ss *SqliteStore) DeleteSession(ctx context.Context, id string) (err error)
 	)
 }
 
+//go:embed queries/delete-sessions-past-creation-timeout.sql
+var rawDeleteOldCreatedSessionsQuery string
+var deleteOldCreatedSessionsQuery string = strings.TrimSpace(rawDeleteOldCreatedSessionsQuery)
+
+func (ss *SqliteStore) DeleteOldCreatedSessions(
+	ctx context.Context, timeout time.Duration, threshold time.Time,
+) (err error) {
+	return errors.Wrapf(
+		ss.db.ExecuteDelete(
+			ctx, deleteOldCreatedSessionsQuery, Session{
+				ExpirationTime: threshold,
+			}.newDeletePastCreationTimeout(timeout),
+		),
+		"couldn't delete sessions which expired before %s", threshold,
+	)
+}
+
 //go:embed queries/delete-sessions-past-expiration.sql
 var rawDeleteExpiredSessionsQuery string
 var deleteExpiredSessionsQuery string = strings.TrimSpace(rawDeleteExpiredSessionsQuery)
@@ -243,14 +260,29 @@ func (ss *SqliteStore) DeleteExpiredSessions(ctx context.Context, threshold time
 	)
 }
 
+func (ss *SqliteStore) Cleanup(ctx context.Context) (done bool, err error) {
+	// This will delete any session based on its absolute timeout at the time of creation, even if the
+	// absolute timeout later increased such that the session's age is greater than the current
+	// timeout.
+	if err := ss.DeleteExpiredSessions(ctx, time.Now()); err != nil {
+		return false, errors.Wrap(err, "couldn't perform periodic deletion of expired sessions")
+	}
+	// This will delete any session based on the current absolute timeout, even if the absolute
+	// timeout was decreased such that current timeout is less than the session's expiration time in
+	// the database.
+	if err := ss.DeleteOldCreatedSessions(ctx, ss.AbsoluteTimeout, time.Now()); err != nil {
+		return false, errors.Wrap(
+			err, "couldn't perform periodic deletion of sessions created a long time ago",
+		)
+	}
+	return false, nil
+}
+
 func (ss *SqliteStore) PeriodicallyCleanup(
 	ctx context.Context, interval time.Duration,
 ) error {
 	return handling.Repeat(ctx, interval, func() (done bool, err error) {
-		return false, errors.Wrap(
-			ss.DeleteExpiredSessions(ctx, time.Now()),
-			"couldn't perform periodic deletion of expired sessions",
-		)
+		return ss.Cleanup(ctx)
 	})
 }