diff --git a/README.md b/README.md index 1514b61..dc00b71 100644 --- a/README.md +++ b/README.md @@ -92,6 +92,27 @@ TTY detection, and security headers are always applied. | `METRICS_USERNAME` | Basic auth username for `/metrics` | `""` | | `METRICS_PASSWORD` | Basic auth password for `/metrics` | `""` | | `SENTRY_DSN` | Sentry error reporting DSN | `""` | +| `SESSION_IDLE_TIMEOUT` | Idle session timeout (Go duration) | `24h` | + +Sessions are bounded by two independent clocks, and end at whichever +one runs out first: + +- **Idle expiry** (`SESSION_IDLE_TIMEOUT`, default `24h`) is a sliding + window. Every authenticated request pushes it forward, so a session + in continuous use never hits it, while an abandoned one expires a day + after its last use. Set it to `0` to disable idle expiry entirely; + the absolute cap below still applies. A set-but-unparseable value + aborts startup rather than silently falling back to the default. +- **Absolute expiry** is a fixed 7 days from login. Activity does + **not** extend it: after a week, every session ends and the user + authenticates again. + +Only requests that authenticate with the session count as activity, so +an unauthenticated request carrying the cookie cannot keep a session +alive. The idle timestamp is rewritten at most once per tenth of the +idle window rather than on every request, which means a session may +expire up to 10% early relative to the user's true last request, but +never late. On first startup, webhooker automatically generates a cryptographically secure session encryption key and stores it in the database. This key diff --git a/TODO.md b/TODO.md index c869649..0751920 100644 --- a/TODO.md +++ b/TODO.md @@ -28,6 +28,10 @@ databases currently grow without bound. # Completed Steps +- 2026-08-09 Inactivity-based session timeout: sliding idle expiry + (`SESSION_IDLE_TIMEOUT`, default `24h`) refreshed on authenticated + requests, with the 7-day absolute cap kept as an independent + backstop that activity never extends (#66) - 2026-08-07 Update golangci-lint to v2.12.2 (Docker image digest in `Dockerfile`, release-archive sha256 pins in `script/bootstrap`), adopt the canonical `.golangci.yml` (v2 `linters.settings` layout so @@ -71,7 +75,7 @@ databases currently grow without bound. - event redelivery endpoint - OpenAPI specification - Analytics dashboard: success rates, response times, volume -- Session expiration tuning and a remember-me option +- A remember-me option at login - Password change and reset flow - Later, nice to have - email delivery target type diff --git a/internal/config/config.go b/internal/config/config.go index 414a3bb..0e659ed 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -31,6 +31,10 @@ const ( // defaultRetentionSweepInterval is how often the retention // reaper deletes events older than each webhook's RetentionDays. defaultRetentionSweepInterval = time.Hour + + // defaultSessionIdleTimeout is how long a session may go without + // authenticated activity before it expires. + defaultSessionIdleTimeout = 24 * time.Hour ) // ErrInvalidEnvironment is returned when WEBHOOKER_ENVIRONMENT @@ -60,6 +64,10 @@ type Config struct { // RetentionSweepInterval is how often the retention reaper runs. RetentionSweepInterval time.Duration + // SessionIdleTimeout is the sliding inactivity window after + // which a session expires. Non-positive disables idle expiry. + SessionIdleTimeout time.Duration + params *ConfigParams log *slog.Logger } @@ -162,6 +170,15 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) { return nil, err } + // Same fail-loud treatment for the session idle timeout. + sessionIdleTimeout, err := envDuration( + "SESSION_IDLE_TIMEOUT", + defaultSessionIdleTimeout, + ) + if err != nil { + return nil, err + } + // Load configuration values from environment variables s := &Config{ DataDir: envString("DATA_DIR"), @@ -173,6 +190,7 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) { Port: envInt("PORT", defaultPort), SentryDSN: envString("SENTRY_DSN"), RetentionSweepInterval: retentionSweepInterval, + SessionIdleTimeout: sessionIdleTimeout, log: log, params: ¶ms, } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 7e3c2c8..571d16d 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -163,7 +163,7 @@ func TestRetentionSweepInterval(t *testing.T) { } if tt.expectError { - testRetentionSweepIntervalError(t) + expectStartupError(t) } else { testRetentionSweepIntervalSuccess(t, tt.expected) } @@ -171,7 +171,9 @@ func TestRetentionSweepInterval(t *testing.T) { } } -func testRetentionSweepIntervalError(t *testing.T) { +// expectStartupError asserts that fx refuses to build the app, +// which is what a set-but-unparseable duration must cause. +func expectStartupError(t *testing.T) { t.Helper() var cfg *config.Config @@ -215,6 +217,82 @@ func testRetentionSweepIntervalSuccess( assert.Equal(t, expected, cfg.RetentionSweepInterval) } +func TestSessionIdleTimeout(t *testing.T) { + tests := []struct { + name string + set bool + value string + expectError bool + expected time.Duration + }{ + { + name: "unset uses default", + set: false, + expected: 24 * time.Hour, + }, + { + name: "valid value is parsed", + set: true, + value: "30m", + expected: 30 * time.Minute, + }, + { + name: "unparseable value fails startup", + set: true, + value: "not-a-duration", + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Cannot use t.Parallel() here because t.Setenv + // is incompatible with parallel subtests. + t.Setenv("WEBHOOKER_ENVIRONMENT", "dev") + + if tt.set { + t.Setenv("SESSION_IDLE_TIMEOUT", tt.value) + } else { + require.NoError(t, os.Unsetenv( + "SESSION_IDLE_TIMEOUT", + )) + } + + if tt.expectError { + expectStartupError(t) + } else { + testSessionIdleTimeoutSuccess(t, tt.expected) + } + }) + } +} + +func testSessionIdleTimeoutSuccess( + t *testing.T, + expected time.Duration, +) { + t.Helper() + + var cfg *config.Config + + app := fxtest.New( + t, + fx.Provide( + globals.New, + logger.New, + config.New, + ), + fx.Populate(&cfg), + ) + require.NoError(t, app.Err()) + + app.RequireStart() + + defer app.RequireStop() + + assert.Equal(t, expected, cfg.SessionIdleTimeout) +} + func TestDefaultDataDir(t *testing.T) { for _, env := range []string{"", "dev", "prod"} { name := env diff --git a/internal/middleware/middleware.go b/internal/middleware/middleware.go index 5f30912..206cd7b 100644 --- a/internal/middleware/middleware.go +++ b/internal/middleware/middleware.go @@ -186,6 +186,10 @@ func (s *Middleware) RequireAuth() func(http.Handler) http.Handler { return } + // IsAuthenticated also enforces both session expiry + // deadlines, so an idle-expired or absolutely-expired + // session lands here and is sent back to the login + // page. if !s.session.IsAuthenticated(sess) { s.log.Debug( "auth middleware: unauthenticated request", @@ -199,6 +203,26 @@ func (s *Middleware) RequireAuth() func(http.Handler) http.Handler { return } + // This request authenticated with the session, so it + // counts as activity: push the idle deadline forward. + // This is the only place sessions are refreshed, which + // is what keeps an unauthenticated request from + // extending someone else's session. Touch advances the + // idle clock only -- the absolute cap is untouched -- + // and reports false when nothing changed, so most + // requests do not re-issue the cookie. Save before the + // handler runs, while the headers are still ours to + // write. + if s.session.Touch(sess) { + err = s.session.Save(r, w, sess) + if err != nil { + s.log.Error( + "auth middleware: failed to refresh session", + "error", err, + ) + } + } + next.ServeHTTP(w, r) }) } diff --git a/internal/middleware/middleware_test.go b/internal/middleware/middleware_test.go index fafec86..71e49ad 100644 --- a/internal/middleware/middleware_test.go +++ b/internal/middleware/middleware_test.go @@ -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) } diff --git a/internal/session/session.go b/internal/session/session.go index b9f181b..1a2d2e1 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -10,6 +10,7 @@ import ( "log/slog" "maps" "net/http" + "time" "github.com/gorilla/sessions" "go.uber.org/fx" @@ -32,6 +33,18 @@ const ( // status. AuthenticatedKey = "authenticated" + // CreatedAtKey is the session key holding the Unix timestamp at + // which the session was authenticated. It anchors the ABSOLUTE + // expiry clock and is written exactly once, by SetUser. Nothing + // refreshes it: an absolute deadline that moved with activity + // would not be a cap at all. + CreatedAtKey = "created_at" + + // LastSeenKey is the session key holding the Unix timestamp of + // the most recent authenticated request. It anchors the IDLE + // expiry clock and is pushed forward by Touch. + LastSeenKey = "last_seen" + // sessionKeyLength is the required length in bytes for the // session authentication key. sessionKeyLength = 32 @@ -41,6 +54,19 @@ const ( // secondsPerDay is the number of seconds in a day. secondsPerDay = 86400 + + // sessionAbsoluteMaxAge is the hard upper bound on how long a + // session may live, measured from CreatedAtKey. Activity never + // extends it, so even a continuously used session ends here and + // the user has to authenticate again. + sessionAbsoluteMaxAge = sessionMaxAgeDays * secondsPerDay * time.Second + + // idleRefreshDivisor rate-limits idle-deadline refreshes. Touch + // only rewrites LastSeenKey once the stored value is older than + // idleTimeout/idleRefreshDivisor, so an active session is + // re-saved at most this many times per idle window instead of + // once per request. See Touch for the tradeoff this buys. + idleRefreshDivisor = 10 ) // ErrSessionKeyLength is returned when the decoded session key @@ -62,6 +88,16 @@ type Session struct { key []byte // raw 32-byte auth key, also used for CSRF cookie signing log *slog.Logger config *config.Config + + // idleTimeout is the sliding inactivity window. A session that + // sees no authenticated request within this window expires, + // independently of the absolute cap. Non-positive disables idle + // expiry and leaves sessionAbsoluteMaxAge as the only bound. + idleTimeout time.Duration + + // now reads the current time. Injected so expiry can be tested + // without sleeping. + now func() time.Time } // New creates a new session manager. The cookie store is @@ -73,8 +109,10 @@ func New( params Params, ) (*Session, error) { s := &Session{ - log: params.Logger.Get(), - config: params.Config, + log: params.Logger.Get(), + config: params.Config, + idleTimeout: params.Config.SessionIdleTimeout, + now: time.Now, } lc.Append(fx.Hook{ @@ -149,29 +187,98 @@ func (s *Session) Save( return sess.Save(r, w) } -// SetUser sets the user information in the session. +// SetUser sets the user information in the session. It starts both +// expiry clocks: CreatedAtKey (absolute, never refreshed again) and +// LastSeenKey (idle, refreshed by Touch). func (s *Session) SetUser( sess *sessions.Session, userID, username string, ) { + now := s.now().Unix() + sess.Values[UserIDKey] = userID sess.Values[UsernameKey] = username sess.Values[AuthenticatedKey] = true + sess.Values[CreatedAtKey] = now + sess.Values[LastSeenKey] = now } -// ClearUser removes user information from the session. +// ClearUser removes user information from the session, including +// both expiry timestamps. func (s *Session) ClearUser(sess *sessions.Session) { delete(sess.Values, UserIDKey) delete(sess.Values, UsernameKey) delete(sess.Values, AuthenticatedKey) + delete(sess.Values, CreatedAtKey) + delete(sess.Values, LastSeenKey) } -// IsAuthenticated checks if the session has an authenticated -// user. +// sessionTime reads a Unix-second timestamp stored under key. +func sessionTime( + sess *sessions.Session, + key string, +) (time.Time, bool) { + secs, ok := sess.Values[key].(int64) + if !ok { + return time.Time{}, false + } + + return time.Unix(secs, 0), true +} + +// IsAuthenticated checks if the session has an authenticated user +// whose session has not passed either expiry deadline. Every +// authentication decision goes through here, so neither clock can +// be bypassed by a caller that forgets to check it. func (s *Session) IsAuthenticated(sess *sessions.Session) bool { auth, ok := sess.Values[AuthenticatedKey].(bool) + if !ok || !auth { + return false + } - return ok && auth + return !s.expired(sess) +} + +// Touch records authenticated activity by pushing the IDLE deadline +// forward. It writes LastSeenKey only; CreatedAtKey is left alone so +// the absolute cap keeps counting down even for a user who never +// stops clicking. +// +// Callers must only invoke Touch for a request that authenticated +// with this session. Refreshing on an unauthenticated request would +// let anyone holding a stolen or abandoned cookie keep the session +// alive by polling a public endpoint. Touch enforces that itself by +// returning false for any session that is not currently +// authenticated and unexpired. +// +// To avoid re-encrypting and re-emitting the session cookie on every +// single request, the timestamp is advanced only once it is older +// than idleTimeout/idleRefreshDivisor. The tradeoff is that +// LastSeenKey lags real activity by up to that much, so a session +// can expire slightly early relative to the user's true last +// request -- never late. +// +// Touch reports whether it changed the session; only then does the +// caller need to save it. +func (s *Session) Touch(sess *sessions.Session) bool { + if s.idleTimeout <= 0 { + return false + } + + if !s.IsAuthenticated(sess) { + return false + } + + now := s.now() + + lastSeen, ok := sessionTime(sess, LastSeenKey) + if ok && now.Sub(lastSeen) < s.idleTimeout/idleRefreshDivisor { + return false + } + + sess.Values[LastSeenKey] = now.Unix() + + return true } // GetUserID retrieves the user ID from the session. @@ -253,3 +360,41 @@ func (s *Session) Regenerate( return newSess, nil } + +// expired reports whether the session has passed either of its two +// independent deadlines. They are deliberately kept apart: +// +// - the ABSOLUTE deadline is CreatedAtKey + sessionAbsoluteMaxAge. +// It is fixed at login and no amount of activity moves it. +// - the IDLE deadline is LastSeenKey + idleTimeout. Activity moves +// it forward via Touch. +// +// Whichever comes first ends the session. +// +// A session that claims to be authenticated but carries no +// timestamps predates this check; it is treated as expired so the +// user re-authenticates rather than being granted an unbounded +// session. +func (s *Session) expired(sess *sessions.Session) bool { + now := s.now() + + createdAt, ok := sessionTime(sess, CreatedAtKey) + if !ok { + return true + } + + if !now.Before(createdAt.Add(sessionAbsoluteMaxAge)) { + return true + } + + if s.idleTimeout <= 0 { + return false + } + + lastSeen, ok := sessionTime(sess, LastSeenKey) + if !ok { + return true + } + + return !now.Before(lastSeen.Add(s.idleTimeout)) +} diff --git a/internal/session/session_test.go b/internal/session/session_test.go index ba1e8c0..f269149 100644 --- a/internal/session/session_test.go +++ b/internal/session/session_test.go @@ -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 --- diff --git a/internal/session/testing.go b/internal/session/testing.go index 5906ddc..0080a62 100644 --- a/internal/session/testing.go +++ b/internal/session/testing.go @@ -2,6 +2,7 @@ package session import ( "log/slog" + "time" "github.com/gorilla/sessions" "sneak.berlin/go/webhooker/internal/config" @@ -12,16 +13,28 @@ import ( // middleware and handler tests to use real session functionality. The key // parameter is the raw 32-byte authentication key used for session encryption // and CSRF cookie signing. +// +// The idle timeout is taken from cfg.SessionIdleTimeout, exactly as in +// production. The now parameter supplies the clock used for expiry +// checks so tests can advance time without sleeping; pass nil for the +// real clock. func NewForTest( store *sessions.CookieStore, cfg *config.Config, log *slog.Logger, key []byte, + now func() time.Time, ) *Session { + if now == nil { + now = time.Now + } + return &Session{ - store: store, - key: key, - config: cfg, - log: log, + store: store, + key: key, + config: cfg, + log: log, + idleTimeout: cfg.SessionIdleTimeout, + now: now, } }