diff --git a/CHANGELOG.md b/CHANGELOG.md index 96c41a23..6eb9d303 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -185,6 +185,9 @@ APIs change between minors without a deprecation period. the id; the docs now say to use unguessable ids. - **`/v1/models` lists only what the key may use**, rather than every model the gateway serves to anyone. +- **Portal sessions end after a day unused** (still at most 14 days in + all), and expired or idle sessions and used or expired key claims are + deleted hourly — they were refused, but never removed. ### Everything that can be changed on a running cluster, from the console diff --git a/internal/router/router.go b/internal/router/router.go index c246fee1..60c2e845 100644 --- a/internal/router/router.go +++ b/internal/router/router.go @@ -423,6 +423,38 @@ func (r *Router) Start(ctx context.Context) { } if r.store != nil { go r.refreshTenants(ctx) + go r.purgeSessions(ctx) + } +} + +// purgeSessions deletes sessions and key claims that can no longer be used, +// hourly and once at startup: refused already, but a table that only grows +// is its own problem, and a session row is a credential's hash kept for +// nothing. +func (r *Router) purgeSessions(ctx context.Context) { + purge := func() { + now := time.Now() + if n, err := r.store.PurgeSessions(ctx, now); err != nil { + r.log.Warn("could not remove expired sessions", "err", err) + } else if n > 0 { + r.log.Info("removed expired and idle sessions", "sessions", n) + } + if n, err := r.store.PurgeClaims(ctx, now); err != nil { + r.log.Warn("could not remove used and expired key claims", "err", err) + } else if n > 0 { + r.log.Info("removed used and expired key claims", "claims", n) + } + } + purge() + t := time.NewTicker(time.Hour) + defer t.Stop() + for { + select { + case <-ctx.Done(): + return + case <-t.C: + purge() + } } } diff --git a/internal/store/sessions.go b/internal/store/sessions.go index 6d45115e..7b708680 100644 --- a/internal/store/sessions.go +++ b/internal/store/sessions.go @@ -11,6 +11,11 @@ import ( // SessionTTL is how long a sign-in lasts. const SessionTTL = 14 * 24 * time.Hour +// SessionIdle is how long a sign-in lasts unused. A browser left signed in +// on a shared machine, or a cookie copied off one, stops working a day after +// it was last used rather than two weeks after it was made. +const SessionIdle = 24 * time.Hour + // ErrNoSession says a session cookie names nothing usable. var ErrNoSession = errors.New("not signed in") @@ -51,21 +56,21 @@ func (s *DB) CreateSession(ctx context.Context, userID, token, csrf string, ttl // somebody eventually forgets. func (s *DB) SessionByToken(ctx context.Context, token string) (Session, User, error) { row := s.db.QueryRowContext(ctx, - `SELECT s.user_id, s.csrf_hash, s.expires_at, + `SELECT s.user_id, s.csrf_hash, s.expires_at, s.last_seen, u.id, u.email, u.name, u.password_hash, u.state, COALESCE(u.project_id, ''), u.created_at, u.approved_at, u.approved_by, u.last_login_at, u.is_admin FROM sessions s JOIN users u ON u.id = s.user_id WHERE s.token_hash = ?`, HashKey(token)) var sess Session - var expires int64 + var expires, lastSeen int64 var u User var state string var created int64 var approved, lastLogin sql.NullInt64 var approvedBy sql.NullString - if err := row.Scan(&sess.UserID, &sess.CSRFHash, &expires, + if err := row.Scan(&sess.UserID, &sess.CSRFHash, &expires, &lastSeen, &u.ID, &u.Email, &u.Name, &u.PasswordHash, &state, &u.ProjectID, &created, &approved, &approvedBy, &lastLogin, &u.IsAdmin); err != nil { if errors.Is(err, sql.ErrNoRows) { @@ -75,7 +80,7 @@ func (s *DB) SessionByToken(ctx context.Context, token string) (Session, User, e } sess.ExpiresAt = fromMillis(expires) - if time.Now().After(sess.ExpiresAt) { + if now := time.Now(); now.After(sess.ExpiresAt) || now.Sub(fromMillis(lastSeen)) > SessionIdle { return Session{}, User{}, ErrNoSession } @@ -116,9 +121,10 @@ func (s *DB) DeleteUserSessions(ctx context.Context, userID string) error { return err } -// PurgeSessions removes expired rows. +// PurgeSessions removes rows expired, or idle too long, by a moment. func (s *DB) PurgeSessions(ctx context.Context, before time.Time) (int64, error) { - res, err := s.db.ExecContext(ctx, `DELETE FROM sessions WHERE expires_at < ?`, millis(before)) + res, err := s.db.ExecContext(ctx, `DELETE FROM sessions WHERE expires_at < ? OR last_seen < ?`, + millis(before), millis(before.Add(-SessionIdle))) if err != nil { return 0, err } diff --git a/internal/store/sessions_test.go b/internal/store/sessions_test.go new file mode 100644 index 00000000..a844cd35 --- /dev/null +++ b/internal/store/sessions_test.go @@ -0,0 +1,38 @@ +package store + +import ( + "errors" + "testing" + "time" +) + +func TestASessionUnusedForADayEnds(t *testing.T) { + db := capacityStore(t) + ctx := t.Context() + if err := db.CreateUser(ctx, User{ID: "u1", Email: "a@example.com", Name: "a", + PasswordHash: "x", State: UserActive, CreatedAt: time.Now()}); err != nil { + t.Fatal(err) + } + for _, tok := range []string{"fresh", "idle"} { + if err := db.CreateSession(ctx, "u1", tok, tok+"-csrf", 0); err != nil { + t.Fatal(err) + } + } + if _, err := db.db.ExecContext(ctx, `UPDATE sessions SET last_seen = ? WHERE token_hash = ?`, + millis(time.Now().Add(-SessionIdle-time.Minute)), HashKey("idle")); err != nil { + t.Fatal(err) + } + if _, _, err := db.SessionByToken(ctx, "fresh"); err != nil { + t.Errorf("a session in use: %v", err) + } + if _, _, err := db.SessionByToken(ctx, "idle"); !errors.Is(err, ErrNoSession) { + t.Errorf("a session idle past %s: %v", SessionIdle, err) + } + n, err := db.PurgeSessions(ctx, time.Now()) + if err != nil || n != 1 { + t.Errorf("purged %d, %v; expected the idle one", n, err) + } + if _, _, err := db.SessionByToken(ctx, "fresh"); err != nil { + t.Errorf("the session in use was purged: %v", err) + } +}