Add inactivity-based session timeout (closes #66) (#105)
All checks were successful
check / check (push) Successful in 4s
All checks were successful
check / check (push) Successful in 4s
Sessions now carry a server-enforced idle deadline (SESSION_IDLE_TIMEOUT, default 24h) alongside the 7-day absolute cap, refreshed on authenticated activity. Activity never extends the absolute cap.
This commit was merged in pull request #105.
This commit is contained in:
@@ -7,6 +7,7 @@ import (
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/gorilla/sessions"
|
||||
"github.com/stretchr/testify/assert"
|
||||
@@ -17,11 +18,47 @@ import (
|
||||
|
||||
const testKeySize = 32
|
||||
|
||||
// testSession creates a Session with a real cookie store for
|
||||
// testing.
|
||||
// testIdleTimeout is the idle window used by the expiry tests.
|
||||
const testIdleTimeout = time.Hour
|
||||
|
||||
// testAbsoluteMaxAge restates the documented absolute session cap
|
||||
// independently of the implementation constant.
|
||||
const testAbsoluteMaxAge = 7 * 24 * time.Hour
|
||||
|
||||
// fakeClock is a manually advanced clock, so expiry can be tested
|
||||
// without sleeping.
|
||||
type fakeClock struct {
|
||||
t time.Time
|
||||
}
|
||||
|
||||
func (c *fakeClock) Now() time.Time {
|
||||
return c.t
|
||||
}
|
||||
|
||||
func (c *fakeClock) Advance(d time.Duration) {
|
||||
c.t = c.t.Add(d)
|
||||
}
|
||||
|
||||
// testSession creates a Session with a real cookie store and the
|
||||
// real clock.
|
||||
func testSession(t *testing.T) *session.Session {
|
||||
t.Helper()
|
||||
|
||||
s, _ := testSessionWithClock(t, testIdleTimeout, nil)
|
||||
|
||||
return s
|
||||
}
|
||||
|
||||
// testSessionWithClock creates a Session with a real cookie store,
|
||||
// the given idle timeout, and a manually advanced clock. Passing a
|
||||
// nil clock uses the real one.
|
||||
func testSessionWithClock(
|
||||
t *testing.T,
|
||||
idleTimeout time.Duration,
|
||||
clock *fakeClock,
|
||||
) (*session.Session, *fakeClock) {
|
||||
t.Helper()
|
||||
|
||||
key := make([]byte, testKeySize)
|
||||
|
||||
for i := range key {
|
||||
@@ -38,7 +75,8 @@ func testSession(t *testing.T) *session.Session {
|
||||
}
|
||||
|
||||
cfg := &config.Config{
|
||||
Environment: config.EnvironmentDev,
|
||||
Environment: config.EnvironmentDev,
|
||||
SessionIdleTimeout: idleTimeout,
|
||||
}
|
||||
|
||||
log := slog.New(slog.NewTextHandler(
|
||||
@@ -46,7 +84,46 @@ func testSession(t *testing.T) *session.Session {
|
||||
&slog.HandlerOptions{Level: slog.LevelDebug},
|
||||
))
|
||||
|
||||
return session.NewForTest(store, cfg, log, key)
|
||||
var now func() time.Time
|
||||
|
||||
if clock != nil {
|
||||
now = clock.Now
|
||||
}
|
||||
|
||||
return session.NewForTest(store, cfg, log, key, now), clock
|
||||
}
|
||||
|
||||
// newFakeClock returns a clock started at a fixed instant.
|
||||
func newFakeClock() *fakeClock {
|
||||
return &fakeClock{
|
||||
t: time.Date(
|
||||
2026, time.January, 2, 3, 4, 5, 0, time.UTC,
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
// authenticatedSession returns a fresh session that has just been
|
||||
// logged in, along with its manager and clock.
|
||||
func authenticatedSession(
|
||||
t *testing.T,
|
||||
idleTimeout time.Duration,
|
||||
) (*session.Session, *sessions.Session, *fakeClock) {
|
||||
t.Helper()
|
||||
|
||||
s, clock := testSessionWithClock(
|
||||
t, idleTimeout, newFakeClock(),
|
||||
)
|
||||
|
||||
req := httptest.NewRequestWithContext(
|
||||
context.Background(), http.MethodGet, "/", nil)
|
||||
|
||||
sess, err := s.Get(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
s.SetUser(sess, "user-123", "alice")
|
||||
require.True(t, s.IsAuthenticated(sess))
|
||||
|
||||
return s, sess, clock
|
||||
}
|
||||
|
||||
// --- Get and Save Tests ---
|
||||
@@ -430,6 +507,263 @@ func TestSessionConstants(t *testing.T) {
|
||||
assert.Equal(t, "user_id", session.UserIDKey)
|
||||
assert.Equal(t, "username", session.UsernameKey)
|
||||
assert.Equal(t, "authenticated", session.AuthenticatedKey)
|
||||
assert.Equal(t, "created_at", session.CreatedAtKey)
|
||||
assert.Equal(t, "last_seen", session.LastSeenKey)
|
||||
}
|
||||
|
||||
// --- Expiry Tests ---
|
||||
|
||||
func TestSetUser_StartsBothClocks(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
_, sess, clock := authenticatedSession(t, testIdleTimeout)
|
||||
|
||||
assert.Equal(
|
||||
t, clock.Now().Unix(), sess.Values[session.CreatedAtKey],
|
||||
"SetUser should anchor the absolute clock",
|
||||
)
|
||||
assert.Equal(
|
||||
t, clock.Now().Unix(), sess.Values[session.LastSeenKey],
|
||||
"SetUser should anchor the idle clock",
|
||||
)
|
||||
}
|
||||
|
||||
func TestIsAuthenticated_WithinIdleWindow(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
s, sess, clock := authenticatedSession(t, testIdleTimeout)
|
||||
|
||||
clock.Advance(testIdleTimeout - time.Second)
|
||||
|
||||
assert.True(
|
||||
t, s.IsAuthenticated(sess),
|
||||
"session should still be valid just inside the idle window",
|
||||
)
|
||||
}
|
||||
|
||||
func TestIsAuthenticated_IdleExpired(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
s, sess, clock := authenticatedSession(t, testIdleTimeout)
|
||||
|
||||
clock.Advance(testIdleTimeout)
|
||||
|
||||
assert.False(
|
||||
t, s.IsAuthenticated(sess),
|
||||
"session should expire once the idle window lapses",
|
||||
)
|
||||
}
|
||||
|
||||
// TestTouch_DoesNotExtendAbsoluteCap is the regression test for the
|
||||
// refresh-the-wrong-clock bug: a session that is used continuously
|
||||
// must survive well past the idle window and still die at the
|
||||
// absolute cap.
|
||||
func TestTouch_DoesNotExtendAbsoluteCap(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
s, sess, clock := authenticatedSession(t, testIdleTimeout)
|
||||
|
||||
createdAt := sess.Values[session.CreatedAtKey]
|
||||
|
||||
// Stay active: a request every half idle window, right up to
|
||||
// the absolute cap.
|
||||
step := testIdleTimeout / 2
|
||||
steps := int(testAbsoluteMaxAge/step) - 1
|
||||
|
||||
for i := range steps {
|
||||
clock.Advance(step)
|
||||
s.Touch(sess)
|
||||
|
||||
require.True(
|
||||
t, s.IsAuthenticated(sess),
|
||||
"active session should survive the idle window "+
|
||||
"(step %d of %d)", i+1, steps,
|
||||
)
|
||||
}
|
||||
|
||||
// One more step of activity takes the session to exactly the
|
||||
// absolute cap, measured from login. Nothing that happened in
|
||||
// the loop may have moved that deadline.
|
||||
clock.Advance(step)
|
||||
s.Touch(sess)
|
||||
|
||||
assert.False(
|
||||
t, s.IsAuthenticated(sess),
|
||||
"activity must not extend the absolute cap",
|
||||
)
|
||||
assert.Equal(
|
||||
t, createdAt, sess.Values[session.CreatedAtKey],
|
||||
"Touch must never rewrite the absolute-clock anchor",
|
||||
)
|
||||
}
|
||||
|
||||
func TestTouch_RefreshesIdleDeadline(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
s, sess, clock := authenticatedSession(t, testIdleTimeout)
|
||||
|
||||
// Halfway through the window, activity happens.
|
||||
clock.Advance(testIdleTimeout / 2)
|
||||
assert.True(
|
||||
t, s.Touch(sess),
|
||||
"Touch should refresh once past the lazy-refresh threshold",
|
||||
)
|
||||
|
||||
// Past the original deadline, but inside the refreshed one.
|
||||
clock.Advance(testIdleTimeout - time.Second)
|
||||
assert.True(
|
||||
t, s.IsAuthenticated(sess),
|
||||
"refreshed session should outlive the original deadline",
|
||||
)
|
||||
|
||||
// And it still expires an idle window after that activity.
|
||||
clock.Advance(time.Second)
|
||||
assert.False(
|
||||
t, s.IsAuthenticated(sess),
|
||||
"refreshed session should expire one window after activity",
|
||||
)
|
||||
}
|
||||
|
||||
func TestTouch_LazyBelowRefreshThreshold(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
s, sess, clock := authenticatedSession(t, testIdleTimeout)
|
||||
|
||||
before := sess.Values[session.LastSeenKey]
|
||||
|
||||
// A request arriving almost immediately is not worth a cookie
|
||||
// rewrite.
|
||||
clock.Advance(time.Second)
|
||||
|
||||
assert.False(
|
||||
t, s.Touch(sess),
|
||||
"Touch should not rewrite the session below the threshold",
|
||||
)
|
||||
assert.Equal(
|
||||
t, before, sess.Values[session.LastSeenKey],
|
||||
"last-seen should be unchanged below the threshold",
|
||||
)
|
||||
}
|
||||
|
||||
func TestTouch_UnauthenticatedSessionIsNotRefreshed(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
s, clock := testSessionWithClock(
|
||||
t, testIdleTimeout, newFakeClock(),
|
||||
)
|
||||
|
||||
req := httptest.NewRequestWithContext(
|
||||
context.Background(), http.MethodGet, "/", nil)
|
||||
|
||||
sess, err := s.Get(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
clock.Advance(testIdleTimeout / 2)
|
||||
|
||||
assert.False(
|
||||
t, s.Touch(sess),
|
||||
"an unauthenticated session must not be refreshed",
|
||||
)
|
||||
|
||||
_, hasLastSeen := sess.Values[session.LastSeenKey]
|
||||
assert.False(
|
||||
t, hasLastSeen,
|
||||
"Touch must not stamp an unauthenticated session",
|
||||
)
|
||||
}
|
||||
|
||||
func TestTouch_IdleExpiredSessionIsNotRevived(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
s, sess, clock := authenticatedSession(t, testIdleTimeout)
|
||||
|
||||
clock.Advance(testIdleTimeout)
|
||||
require.False(t, s.IsAuthenticated(sess))
|
||||
|
||||
assert.False(
|
||||
t, s.Touch(sess),
|
||||
"an already expired session must not be refreshed",
|
||||
)
|
||||
assert.False(
|
||||
t, s.IsAuthenticated(sess),
|
||||
"Touch must not revive an expired session",
|
||||
)
|
||||
}
|
||||
|
||||
func TestIsAuthenticated_MissingTimestamps(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
s, _ := testSessionWithClock(
|
||||
t, testIdleTimeout, newFakeClock(),
|
||||
)
|
||||
|
||||
req := httptest.NewRequestWithContext(
|
||||
context.Background(), http.MethodGet, "/", nil)
|
||||
|
||||
sess, err := s.Get(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
// A session from before idle expiry existed: authenticated,
|
||||
// but with no timestamps. Fail closed.
|
||||
sess.Values[session.AuthenticatedKey] = true
|
||||
|
||||
assert.False(
|
||||
t, s.IsAuthenticated(sess),
|
||||
"a session with no timestamps should be rejected",
|
||||
)
|
||||
}
|
||||
|
||||
func TestIsAuthenticated_MissingLastSeen(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
s, sess, _ := authenticatedSession(t, testIdleTimeout)
|
||||
|
||||
delete(sess.Values, session.LastSeenKey)
|
||||
|
||||
assert.False(
|
||||
t, s.IsAuthenticated(sess),
|
||||
"a session with no idle anchor should be rejected",
|
||||
)
|
||||
}
|
||||
|
||||
func TestIdleTimeoutDisabled_AbsoluteCapStillApplies(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
s, sess, clock := authenticatedSession(t, 0)
|
||||
|
||||
// Idle expiry is off, so an untouched session survives an
|
||||
// arbitrary idle stretch.
|
||||
clock.Advance(testAbsoluteMaxAge - time.Second)
|
||||
assert.True(
|
||||
t, s.IsAuthenticated(sess),
|
||||
"idle expiry should be disabled by a non-positive timeout",
|
||||
)
|
||||
|
||||
assert.False(
|
||||
t, s.Touch(sess),
|
||||
"Touch should be a no-op when idle expiry is disabled",
|
||||
)
|
||||
|
||||
// The absolute cap still ends it.
|
||||
clock.Advance(time.Second)
|
||||
assert.False(
|
||||
t, s.IsAuthenticated(sess),
|
||||
"the absolute cap must still apply with idle expiry off",
|
||||
)
|
||||
}
|
||||
|
||||
func TestClearUser_RemovesTimestamps(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
s, sess, _ := authenticatedSession(t, testIdleTimeout)
|
||||
|
||||
s.ClearUser(sess)
|
||||
|
||||
_, hasCreatedAt := sess.Values[session.CreatedAtKey]
|
||||
assert.False(t, hasCreatedAt, "CreatedAtKey should be removed")
|
||||
|
||||
_, hasLastSeen := sess.Values[session.LastSeenKey]
|
||||
assert.False(t, hasLastSeen, "LastSeenKey should be removed")
|
||||
}
|
||||
|
||||
// --- Edge Cases ---
|
||||
|
||||
Reference in New Issue
Block a user