Add inactivity-based session timeout (closes #66)
All checks were successful
check / check (push) Successful in 3m6s
All checks were successful
check / check (push) Successful in 3m6s
Sessions had only a 7-day absolute lifetime, and that cap was enforced only by the cookie's MaxAge -- i.e. only by the browser. An abandoned session stayed usable for the full week. Sessions are now bounded by two independent, server-enforced clocks, and end at whichever expires first: - absolute: created_at + 7 days, stamped once by SetUser and never rewritten, so no amount of activity can extend it - idle: last_seen + SESSION_IDLE_TIMEOUT (default 24h), pushed forward by the new Session.Touch Both deadlines are checked in Session.expired, which IsAuthenticated now consults, so every existing authentication decision honours them without each call site having to remember. Activity means a request that passes RequireAuth, which is the only place Touch is called; an unauthenticated request carrying the cookie cannot keep a session alive. Touch re-checks authentication itself so that guarantee does not depend on the call site. To avoid re-issuing the session cookie on every authenticated request, Touch rewrites last_seen only once it is older than a tenth of the idle window. The session therefore expires up to 10% early relative to the user's true last request, never late. An authenticated session carrying no timestamps (a cookie minted before this change) is treated as expired, so the failure mode of the upgrade is one forced re-login rather than an unbounded session. Tests use an injected clock rather than sleeps and cover idle expiry, refresh on activity, an actively used session still dying at the absolute cap, refusal to refresh unauthenticated or expired sessions, disabled idle expiry, and startup aborting on an unparseable SESSION_IDLE_TIMEOUT.
This commit is contained in:
@@ -8,6 +8,7 @@ import (
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/gorilla/sessions"
|
||||
"github.com/stretchr/testify/assert"
|
||||
@@ -28,13 +29,30 @@ func testMiddleware(
|
||||
) (*middleware.Middleware, *session.Session) {
|
||||
t.Helper()
|
||||
|
||||
m, s, _ := testMiddlewareWithSessionClock(t, env, 0, nil)
|
||||
|
||||
return m, s
|
||||
}
|
||||
|
||||
// testMiddlewareWithSessionClock is testMiddleware with a
|
||||
// configurable session idle timeout and a manually advanced clock,
|
||||
// for the session-expiry tests. A nil clock uses the real one.
|
||||
func testMiddlewareWithSessionClock(
|
||||
t *testing.T,
|
||||
env string,
|
||||
idleTimeout time.Duration,
|
||||
clock *fakeClock,
|
||||
) (*middleware.Middleware, *session.Session, *fakeClock) {
|
||||
t.Helper()
|
||||
|
||||
log := slog.New(slog.NewTextHandler(
|
||||
os.Stderr,
|
||||
&slog.HandlerOptions{Level: slog.LevelDebug},
|
||||
))
|
||||
|
||||
cfg := &config.Config{
|
||||
Environment: env,
|
||||
Environment: env,
|
||||
SessionIdleTimeout: idleTimeout,
|
||||
}
|
||||
|
||||
// Create a real session manager with a known key
|
||||
@@ -53,11 +71,40 @@ func testMiddleware(
|
||||
SameSite: http.SameSiteLaxMode,
|
||||
}
|
||||
|
||||
sessManager := session.NewForTest(store, cfg, log, key)
|
||||
var now func() time.Time
|
||||
|
||||
if clock != nil {
|
||||
now = clock.Now
|
||||
}
|
||||
|
||||
sessManager := session.NewForTest(store, cfg, log, key, now)
|
||||
|
||||
m := middleware.NewForTest(log, cfg, sessManager)
|
||||
|
||||
return m, sessManager
|
||||
return m, sessManager, clock
|
||||
}
|
||||
|
||||
// fakeClock is a manually advanced clock, so session 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)
|
||||
}
|
||||
|
||||
// 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,
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
// --- Logging Middleware Tests ---
|
||||
@@ -387,6 +434,181 @@ func TestRequireAuth_UnauthenticatedSession_RedirectsToLogin(
|
||||
assert.Equal(t, "/pages/login", w.Header().Get("Location"))
|
||||
}
|
||||
|
||||
// --- RequireAuth Session Expiry Tests ---
|
||||
|
||||
// loginCookies authenticates a new session and returns the cookies
|
||||
// a browser would then send back.
|
||||
func loginCookies(
|
||||
t *testing.T,
|
||||
sessManager *session.Session,
|
||||
) []*http.Cookie {
|
||||
t.Helper()
|
||||
|
||||
req := httptest.NewRequestWithContext(
|
||||
context.Background(), http.MethodGet, "/login", nil)
|
||||
w := httptest.NewRecorder()
|
||||
|
||||
sess, err := sessManager.Get(req)
|
||||
require.NoError(t, err)
|
||||
sessManager.SetUser(sess, "user-123", "testuser")
|
||||
require.NoError(t, sessManager.Save(req, w, sess))
|
||||
|
||||
cookies := w.Result().Cookies()
|
||||
require.NotEmpty(t, cookies, "session cookie should be set")
|
||||
|
||||
return cookies
|
||||
}
|
||||
|
||||
// runAuthed sends a request carrying cookies through RequireAuth
|
||||
// and reports whether the protected handler ran, plus the response.
|
||||
func runAuthed(
|
||||
t *testing.T,
|
||||
m *middleware.Middleware,
|
||||
cookies []*http.Cookie,
|
||||
) (bool, *httptest.ResponseRecorder) {
|
||||
t.Helper()
|
||||
|
||||
var called bool
|
||||
|
||||
handler := m.RequireAuth()(http.HandlerFunc(
|
||||
func(_ http.ResponseWriter, _ *http.Request) {
|
||||
called = true
|
||||
},
|
||||
))
|
||||
|
||||
req := httptest.NewRequestWithContext(
|
||||
context.Background(),
|
||||
http.MethodGet, "/dashboard", nil,
|
||||
)
|
||||
|
||||
for _, c := range cookies {
|
||||
req.AddCookie(c)
|
||||
}
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
handler.ServeHTTP(w, req)
|
||||
|
||||
return called, w
|
||||
}
|
||||
|
||||
// sessionCookies filters a response's cookies down to the session
|
||||
// cookie, so tests can tell whether the session was re-issued.
|
||||
func sessionCookies(
|
||||
w *httptest.ResponseRecorder,
|
||||
) []*http.Cookie {
|
||||
var out []*http.Cookie
|
||||
|
||||
for _, c := range w.Result().Cookies() {
|
||||
if c.Name == session.SessionName {
|
||||
out = append(out, c)
|
||||
}
|
||||
}
|
||||
|
||||
return out
|
||||
}
|
||||
|
||||
func TestRequireAuth_IdleExpiredSession_RedirectsToLogin(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
idle := time.Hour
|
||||
|
||||
m, sessManager, clock := testMiddlewareWithSessionClock(
|
||||
t, config.EnvironmentDev, idle, newFakeClock(),
|
||||
)
|
||||
|
||||
cookies := loginCookies(t, sessManager)
|
||||
|
||||
clock.Advance(idle)
|
||||
|
||||
called, w := runAuthed(t, m, cookies)
|
||||
|
||||
assert.False(
|
||||
t, called,
|
||||
"handler should not run for an idle-expired session",
|
||||
)
|
||||
assert.Equal(t, http.StatusSeeOther, w.Code)
|
||||
assert.Equal(t, "/pages/login", w.Header().Get("Location"))
|
||||
assert.Empty(
|
||||
t, sessionCookies(w),
|
||||
"an expired session must not be refreshed",
|
||||
)
|
||||
}
|
||||
|
||||
func TestRequireAuth_RefreshesIdleDeadlineOnActivity(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
idle := time.Hour
|
||||
|
||||
m, sessManager, clock := testMiddlewareWithSessionClock(
|
||||
t, config.EnvironmentDev, idle, newFakeClock(),
|
||||
)
|
||||
|
||||
cookies := loginCookies(t, sessManager)
|
||||
|
||||
// Activity halfway through the idle window.
|
||||
clock.Advance(idle / 2)
|
||||
|
||||
called, w := runAuthed(t, m, cookies)
|
||||
require.True(t, called, "handler should run while valid")
|
||||
|
||||
refreshed := sessionCookies(w)
|
||||
require.NotEmpty(
|
||||
t, refreshed,
|
||||
"activity should re-issue the session cookie",
|
||||
)
|
||||
|
||||
// Past the original deadline. The refreshed cookie is still
|
||||
// good; the original one is not.
|
||||
clock.Advance(idle - time.Second)
|
||||
|
||||
calledRefreshed, _ := runAuthed(t, m, refreshed)
|
||||
assert.True(
|
||||
t, calledRefreshed,
|
||||
"refreshed session should outlive the original deadline",
|
||||
)
|
||||
|
||||
calledStale, staleW := runAuthed(t, m, cookies)
|
||||
assert.False(
|
||||
t, calledStale,
|
||||
"the pre-refresh cookie carries the old idle deadline",
|
||||
)
|
||||
assert.Equal(t, http.StatusSeeOther, staleW.Code)
|
||||
}
|
||||
|
||||
func TestRequireAuth_UnauthenticatedRequestDoesNotRefresh(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
m, sessManager, _ := testMiddlewareWithSessionClock(
|
||||
t, config.EnvironmentDev, time.Hour, newFakeClock(),
|
||||
)
|
||||
|
||||
// A session cookie that exists but was never authenticated.
|
||||
req := httptest.NewRequestWithContext(
|
||||
context.Background(), http.MethodGet, "/setup", nil)
|
||||
setupW := httptest.NewRecorder()
|
||||
|
||||
sess, err := sessManager.Get(req)
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, sessManager.Save(req, setupW, sess))
|
||||
|
||||
cookies := setupW.Result().Cookies()
|
||||
require.NotEmpty(t, cookies)
|
||||
|
||||
called, w := runAuthed(t, m, cookies)
|
||||
|
||||
assert.False(t, called)
|
||||
assert.Empty(
|
||||
t, sessionCookies(w),
|
||||
"an unauthenticated request must not stamp the session",
|
||||
)
|
||||
}
|
||||
|
||||
// --- NoCache Middleware Tests ---
|
||||
|
||||
func TestNoCache_SetsHeaders(t *testing.T) {
|
||||
@@ -479,7 +701,7 @@ func metricsAuthMiddleware(
|
||||
store := sessions.NewCookieStore(key)
|
||||
store.Options = &sessions.Options{Path: "/", MaxAge: 86400}
|
||||
|
||||
sessManager := session.NewForTest(store, cfg, log, key)
|
||||
sessManager := session.NewForTest(store, cfg, log, key, nil)
|
||||
|
||||
return middleware.NewForTest(log, cfg, sessManager)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user