1 Commits

Author SHA1 Message Date
b04bc2cc7b Add inactivity-based session timeout (closes #66)
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.
2026-08-09 05:59:54 +00:00
16 changed files with 918 additions and 540 deletions

View File

@@ -92,6 +92,27 @@ TTY detection, and security headers are always applied.
| `METRICS_USERNAME` | Basic auth username for `/metrics` | `""` | | `METRICS_USERNAME` | Basic auth username for `/metrics` | `""` |
| `METRICS_PASSWORD` | Basic auth password for `/metrics` | `""` | | `METRICS_PASSWORD` | Basic auth password for `/metrics` | `""` |
| `SENTRY_DSN` | Sentry error reporting DSN | `""` | | `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 On first startup, webhooker automatically generates a cryptographically
secure session encryption key and stores it in the database. This key secure session encryption key and stores it in the database. This key

12
TODO.md
View File

@@ -28,12 +28,10 @@ databases currently grow without bound.
# Completed Steps # Completed Steps
- 2026-08-09 Root the delivery engine's worker pool and the retention - 2026-08-09 Inactivity-based session timeout: sliding idle expiry
reaper's sweep loop at `context.Background()` rather than the fx (`SESSION_IDLE_TIMEOUT`, default `24h`) refreshed on authenticated
`OnStart` hook context (#97), which carries fx's 15s start timeout and requests, with the 7-day absolute cap kept as an independent
killed both roughly fifteen seconds after boot: the proxy silently backstop that activity never extends (#66)
stopped delivering webhooks entirely, and the reaper never ran a
single sweep under its default one-hour interval
- 2026-08-07 Update golangci-lint to v2.12.2 (Docker image digest in - 2026-08-07 Update golangci-lint to v2.12.2 (Docker image digest in
`Dockerfile`, release-archive sha256 pins in `script/bootstrap`), `Dockerfile`, release-archive sha256 pins in `script/bootstrap`),
adopt the canonical `.golangci.yml` (v2 `linters.settings` layout so adopt the canonical `.golangci.yml` (v2 `linters.settings` layout so
@@ -77,7 +75,7 @@ databases currently grow without bound.
- event redelivery endpoint - event redelivery endpoint
- OpenAPI specification - OpenAPI specification
- Analytics dashboard: success rates, response times, volume - 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 - Password change and reset flow
- Later, nice to have - Later, nice to have
- email delivery target type - email delivery target type

View File

@@ -31,6 +31,10 @@ const (
// defaultRetentionSweepInterval is how often the retention // defaultRetentionSweepInterval is how often the retention
// reaper deletes events older than each webhook's RetentionDays. // reaper deletes events older than each webhook's RetentionDays.
defaultRetentionSweepInterval = time.Hour 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 // ErrInvalidEnvironment is returned when WEBHOOKER_ENVIRONMENT
@@ -60,6 +64,10 @@ type Config struct {
// RetentionSweepInterval is how often the retention reaper runs. // RetentionSweepInterval is how often the retention reaper runs.
RetentionSweepInterval time.Duration RetentionSweepInterval time.Duration
// SessionIdleTimeout is the sliding inactivity window after
// which a session expires. Non-positive disables idle expiry.
SessionIdleTimeout time.Duration
params *ConfigParams params *ConfigParams
log *slog.Logger log *slog.Logger
} }
@@ -162,6 +170,15 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
return nil, err 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 // Load configuration values from environment variables
s := &Config{ s := &Config{
DataDir: envString("DATA_DIR"), DataDir: envString("DATA_DIR"),
@@ -173,6 +190,7 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
Port: envInt("PORT", defaultPort), Port: envInt("PORT", defaultPort),
SentryDSN: envString("SENTRY_DSN"), SentryDSN: envString("SENTRY_DSN"),
RetentionSweepInterval: retentionSweepInterval, RetentionSweepInterval: retentionSweepInterval,
SessionIdleTimeout: sessionIdleTimeout,
log: log, log: log,
params: &params, params: &params,
} }

View File

@@ -163,7 +163,7 @@ func TestRetentionSweepInterval(t *testing.T) {
} }
if tt.expectError { if tt.expectError {
testRetentionSweepIntervalError(t) expectStartupError(t)
} else { } else {
testRetentionSweepIntervalSuccess(t, tt.expected) 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() t.Helper()
var cfg *config.Config var cfg *config.Config
@@ -215,6 +217,82 @@ func testRetentionSweepIntervalSuccess(
assert.Equal(t, expected, cfg.RetentionSweepInterval) 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) { func TestDefaultDataDir(t *testing.T) {
for _, env := range []string{"", "dev", "prod"} { for _, env := range []string{"", "dev", "prod"} {
name := env name := env

View File

@@ -5,8 +5,6 @@ import (
"log/slog" "log/slog"
"os" "os"
"time" "time"
"go.uber.org/fx"
) )
// NewTestRetentionReaper builds a RetentionReaper backed by the given // NewTestRetentionReaper builds a RetentionReaper backed by the given
@@ -31,26 +29,3 @@ func NewTestRetentionReaper(
func (r *RetentionReaper) ExportSweep(ctx context.Context) { func (r *RetentionReaper) ExportSweep(ctx context.Context) {
r.sweep(ctx) r.sweep(ctx)
} }
// ExportRegisterHooks registers the reaper's real fx lifecycle hooks
// on a lifecycle supplied by a test, so a test can drive the exact
// OnStart/OnStop functions the application runs and hand OnStart the
// kind of context fx actually supplies.
func (r *RetentionReaper) ExportRegisterHooks(lc fx.Lifecycle) {
r.registerHooks(lc)
}
// ExportStart starts the reaper's background loop for tests.
func (r *RetentionReaper) ExportStart() {
r.start()
}
// ExportStop stops the reaper's background loop for tests.
func (r *RetentionReaper) ExportStop() {
r.stop()
}
// ExportSetInterval overrides the sweep interval for tests.
func (r *RetentionReaper) ExportSetInterval(d time.Duration) {
r.interval = d
}

View File

@@ -56,20 +56,9 @@ func NewRetentionReaper(
interval: params.Config.RetentionSweepInterval, interval: params.Config.RetentionSweepInterval,
} }
r.registerHooks(lc)
return r
}
// registerHooks wires the reaper's start and stop into the fx
// lifecycle. The start hook's context is deliberately ignored: see
// start for why the sweep loop must not inherit it.
func (r *RetentionReaper) registerHooks(lc fx.Lifecycle) {
lc.Append(fx.Hook{ lc.Append(fx.Hook{
//nolint:contextcheck // Not inheriting the hook context is OnStart: func(ctx context.Context) error {
// the point: see start. r.start(ctx)
OnStart: func(_ context.Context) error {
r.start()
return nil return nil
}, },
@@ -79,20 +68,12 @@ func (r *RetentionReaper) registerHooks(lc fx.Lifecycle) {
return nil return nil
}, },
}) })
return r
} }
// start launches the background sweep loop. func (r *RetentionReaper) start(ctx context.Context) {
// ctx, cancel := context.WithCancel(ctx)
// The loop's context is derived from context.Background(), NOT from
// the fx OnStart hook context. The hook context carries fx's start
// timeout (15s by default) and is cancelled once the start phase
// completes, so a loop derived from it dies 45 minutes before its
// first tick under the default one-hour sweep interval, leaving a
// reaper that never reaps. A long-lived goroutine must outlive the
// startup phase, so its lifetime is bounded by OnStop instead: stop
// cancels this context and waits on the WaitGroup.
func (r *RetentionReaper) start() {
ctx, cancel := context.WithCancel(context.Background())
r.cancel = cancel r.cancel = cancel
r.wg.Add(1) r.wg.Add(1)

View File

@@ -1,209 +0,0 @@
package database_test
import (
"context"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/fx"
"gorm.io/gorm"
"sneak.berlin/go/webhooker/internal/database"
)
const (
// reaperTestInterval is the sweep interval a lifecycle test
// runs the reaper at, so a loop that survives startup produces
// an observable sweep quickly.
reaperTestInterval = 10 * time.Millisecond
// reaperStopTimeout bounds how long a lifecycle test waits for
// the reaper's OnStop hook to return before declaring the
// shutdown hung.
reaperStopTimeout = 10 * time.Second
// reaperTestRetentionDays is the retention policy the lifecycle
// tests give their webhook.
reaperTestRetentionDays = 30
)
// recordingLifecycle is a minimal fx.Lifecycle that records the
// hooks a component registers, so a test can invoke the real
// OnStart/OnStop functions with a context of its choosing.
type recordingLifecycle struct {
hooks []fx.Hook
}
func (l *recordingLifecycle) Append(h fx.Hook) {
l.hooks = append(l.hooks, h)
}
// startReaperViaHook drives the genuine fx hooks the application
// registers for the reaper, handing OnStart a context that is
// already done. It returns the recorded lifecycle so the caller
// can drive OnStop too.
func startReaperViaHook(
t *testing.T, r *database.RetentionReaper,
) *recordingLifecycle {
t.Helper()
lc := &recordingLifecycle{}
r.ExportRegisterHooks(lc)
require.Len(t, lc.hooks, 1)
// fx hands OnStart a context carrying the application start
// timeout, and cancels it when the start phase ends. An
// already-cancelled context is that same defect taken to its
// limit, and unlike a plain context.Background() it actually
// distinguishes a correctly rooted loop from a broken one.
hookCtx, cancel := context.WithCancel(context.Background())
cancel()
require.NoError(t, lc.hooks[0].OnStart(hookCtx))
return lc
}
// eventGone reports whether an event row has been removed. It
// takes no *testing.T because it is polled from an
// assert.Eventually condition, which runs off the test goroutine
// where testify assertions must not be used.
func eventGone(db *gorm.DB, eventID string) bool {
var n int64
err := db.Unscoped().Model(&database.Event{}).
Where("id = ?", eventID).Count(&n).Error
if err != nil {
return false
}
return n == 0
}
// seedExpiredWebhook creates a webhook with a finite retention
// policy plus one long-expired event chain, and returns the
// webhook's database and the chain's event ID.
func seedExpiredWebhook(
t *testing.T, env *retentionTestEnv,
) (*gorm.DB, string) {
t.Helper()
webhookID := createWebhook(
t, env.mainDB.DB(), reaperTestRetentionDays,
)
db, err := env.mgr.GetDB(webhookID)
require.NoError(t, err)
chain := seedEventChain(
t, db, webhookID,
time.Now().Add(-365*24*time.Hour),
)
return db, chain.eventID
}
// TestRetentionReaper_LoopOutlivesStartHookContext is the
// regression test for a reaper that never reaped. fx calls
// OnStart with a context carrying the application's start timeout
// (15s by default) and cancels it when the start phase ends, so a
// sweep loop rooted in it is dead three quarters of an hour
// before its first tick under the default one-hour interval, and
// per-webhook event databases grow without bound exactly as they
// did before retention existed.
//
// Driving OnStart with an already-cancelled context is that
// defect taken to its limit: a loop that inherits the hook
// context never ticks once, while a correctly rooted loop keeps
// sweeping for as long as the process lives.
func TestRetentionReaper_LoopOutlivesStartHookContext(
t *testing.T,
) {
t.Parallel()
env := setupRetentionTest(t)
db, eventID := seedExpiredWebhook(t, env)
env.reaper.ExportSetInterval(reaperTestInterval)
lc := startReaperViaHook(t, env.reaper)
t.Cleanup(func() {
_ = lc.hooks[0].OnStop(context.Background())
})
assert.Eventually(
t,
func() bool { return eventGone(db, eventID) },
5*time.Second,
reaperTestInterval,
"the sweep loop must keep running after the start "+
"hook's context is done; it reaped nothing, so it "+
"inherited the hook context and died",
)
}
// TestRetentionReaper_StopHookStopsLoop proves the fix did not
// trade a startup bug for a shutdown hang: now that the sweep
// loop no longer observes the start hook's cancellation, OnStop
// is the only thing that can stop it, and it must both return
// promptly and actually leave the loop stopped.
func TestRetentionReaper_StopHookStopsLoop(t *testing.T) {
t.Parallel()
env := setupRetentionTest(t)
db, eventID := seedExpiredWebhook(t, env)
env.reaper.ExportSetInterval(reaperTestInterval)
lc := startReaperViaHook(t, env.reaper)
// Let the loop prove it is running before stopping it, so a
// fast OnStop cannot pass by stopping something already dead.
require.Eventually(
t,
func() bool { return eventGone(db, eventID) },
5*time.Second,
reaperTestInterval,
)
var stopErr error
stopped := make(chan struct{})
go func() {
defer close(stopped)
// stop blocks on the loop's WaitGroup, so returning at all
// proves the goroutine observed the cancellation.
stopErr = lc.hooks[0].OnStop(context.Background())
}()
select {
case <-stopped:
case <-time.After(reaperStopTimeout):
t.Fatal(
"OnStop did not return: the retention reaper's " +
"WaitGroup is still waiting on a loop that never " +
"observed cancellation",
)
}
require.NoError(t, stopErr)
// With the loop gone, a newly expired chain must survive.
survivor := seedEventChain(
t, db, "stopped-webhook",
time.Now().Add(-365*24*time.Hour),
)
time.Sleep(20 * reaperTestInterval)
assert.False(
t,
eventGone(db, survivor.eventID),
"a stopped reaper must not sweep anything",
)
}

View File

@@ -149,7 +149,18 @@ func New(
Transport: NewSSRFSafeTransport(), Transport: NewSSRFSafeTransport(),
}) })
e.registerHooks(lc) lc.Append(fx.Hook{
OnStart: func(ctx context.Context) error {
e.start(ctx)
return nil
},
OnStop: func(_ context.Context) error {
e.stop()
return nil
},
})
return e return e
} }
@@ -199,40 +210,8 @@ func (e *Engine) ScheduleRetry(
}) })
} }
// registerHooks wires the engine's start and stop into the fx func (e *Engine) start(ctx context.Context) {
// lifecycle. The start hook's context is deliberately ignored: ctx, cancel := context.WithCancel(ctx)
// see start for why the worker pool must not inherit it.
func (e *Engine) registerHooks(lc fx.Lifecycle) {
lc.Append(fx.Hook{
//nolint:contextcheck // Not inheriting the hook context
// is the point: see start.
OnStart: func(_ context.Context) error {
e.start()
return nil
},
OnStop: func(_ context.Context) error {
e.stop()
return nil
},
})
}
// start launches the worker pool, restart recovery, and the
// periodic retry sweep.
//
// Their context is derived from context.Background(), NOT from
// the fx OnStart hook context. The hook context carries fx's
// start timeout (15s by default) and is cancelled once the start
// phase completes, so goroutines derived from it stop a few
// seconds into the process: every worker would return and the
// engine would silently stop delivering webhooks entirely. A
// long-lived goroutine must outlive the startup phase, so its
// lifetime is bounded by OnStop instead: stop cancels this
// context and waits on the WaitGroup.
func (e *Engine) start() {
ctx, cancel := context.WithCancel(context.Background())
e.cancel = cancel e.cancel = cancel
for range e.workers { for range e.workers {

View File

@@ -476,7 +476,7 @@ func TestWorkerLifecycle_StartStop(t *testing.T) {
t.Parallel() t.Parallel()
s := newISetup(t) s := newISetup(t)
s.Engine.ExportStart() s.Engine.ExportStart(context.Background())
event := iSeedEvent( event := iSeedEvent(
t, s.WebhookDB, s.WebhookID, t, s.WebhookDB, s.WebhookID,
@@ -499,17 +499,21 @@ func TestWorkerLifecycle_StartStop(t *testing.T) {
s.Engine.Notify([]delivery.Task{task}) s.Engine.Notify([]delivery.Task{task})
iWaitForDelivered(t, s.WebhookDB, d.ID) iWaitForStatus(
t, s.WebhookDB, d.ID,
database.DeliveryStatusDelivered,
)
s.Engine.ExportStop() s.Engine.ExportStop()
} }
// iWaitForDelivered polls until the delivery reaches the // iWaitForStatus polls until the delivery reaches the
// delivered status. // expected status.
func iWaitForDelivered( func iWaitForStatus(
t *testing.T, t *testing.T,
db *gorm.DB, db *gorm.DB,
deliveryID string, deliveryID string,
expected database.DeliveryStatus,
) { ) {
t.Helper() t.Helper()
@@ -523,7 +527,7 @@ func iWaitForDelivered(
return false return false
} }
return d.Status == database.DeliveryStatusDelivered return d.Status == expected
}, 5*time.Second, 50*time.Millisecond) }, 5*time.Second, 50*time.Millisecond)
} }
@@ -554,7 +558,7 @@ func TestWorkerLifecycle_ProcessesRetryChannel(
database.DeliveryStatusRetrying, database.DeliveryStatusRetrying,
) )
s.Engine.ExportStart() s.Engine.ExportStart(context.Background())
bodyStr := event.Body bodyStr := event.Body
cfg := iHTTPConfig(ts.URL) cfg := iHTTPConfig(ts.URL)
@@ -565,7 +569,10 @@ func TestWorkerLifecycle_ProcessesRetryChannel(
s.Engine.ExportRetryCh() <- task s.Engine.ExportRetryCh() <- task
iWaitForDelivered(t, s.WebhookDB, d.ID) iWaitForStatus(
t, s.WebhookDB, d.ID,
database.DeliveryStatusDelivered,
)
s.Engine.ExportStop() s.Engine.ExportStop()
} }

View File

@@ -1,199 +0,0 @@
package delivery_test
import (
"context"
"testing"
"time"
"github.com/google/uuid"
"github.com/stretchr/testify/require"
"go.uber.org/fx"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
)
const (
// hookStopTimeout bounds how long a lifecycle test waits for
// the engine's OnStop hook to return before declaring the
// shutdown hung.
hookStopTimeout = 10 * time.Second
// hookSettleDelay is how long startEngineViaHook waits after
// OnStart before the caller may enqueue work. A worker pool
// wrongly rooted in the already-done hook context has nothing
// but ctx.Done() ready in its select, so it is deterministically
// gone by the end of this window. Without the wait, Notify would
// race the pool's very first select, in which a ready ctx.Done()
// and a ready deliveryCh are chosen between at random and a
// doomed pool still delivers.
hookSettleDelay = 250 * time.Millisecond
)
// recordingLifecycle is a minimal fx.Lifecycle that records the
// hooks a component registers, so a test can invoke the real
// OnStart/OnStop functions with a context of its choosing.
type recordingLifecycle struct {
hooks []fx.Hook
}
func (l *recordingLifecycle) Append(h fx.Hook) {
l.hooks = append(l.hooks, h)
}
// startEngineViaHook drives the genuine fx hooks the application
// registers for the engine, handing OnStart a context that is
// already done, and returns only once a pool that inherited that
// context would have exited. It returns the recorded lifecycle so
// the caller can drive OnStop too.
//
// Callers must not seed pending or retrying deliveries before
// calling this: restart recovery enqueues those during startup,
// which would put work in the queue while the pool is still
// racing its first select.
func startEngineViaHook(
t *testing.T, eng *delivery.Engine,
) *recordingLifecycle {
t.Helper()
lc := &recordingLifecycle{}
eng.ExportRegisterHooks(lc)
require.Len(t, lc.hooks, 1)
// fx hands OnStart a context carrying the application start
// timeout, and cancels it when the start phase ends. An
// already-cancelled context is that same defect taken to its
// limit, and unlike a plain context.Background() it actually
// distinguishes a correctly rooted loop from a broken one.
hookCtx, cancel := context.WithCancel(context.Background())
cancel()
require.NoError(t, lc.hooks[0].OnStart(hookCtx))
time.Sleep(hookSettleDelay)
return lc
}
// seedLogTask seeds a pending delivery for a log target and
// returns its ID together with the task that drives it. The log
// target needs no network, so a delivery completing proves only
// that a worker picked the task up.
func seedLogTask(
t *testing.T, s iSetup,
) (string, delivery.Task) {
t.Helper()
event := iSeedEvent(
t, s.WebhookDB, s.WebhookID,
`{"lifecycle":"hook-context"}`,
)
targetID := uuid.New().String()
d := iSeedDelivery(
t, s.WebhookDB, event.ID, targetID,
database.DeliveryStatusPending,
)
bodyStr := event.Body
task := iTask(
d, event, s.WebhookID, targetID,
"hook-context-test", "", 0, 1, &bodyStr,
)
task.TargetType = database.TargetTypeLog
return d.ID, task
}
// TestEngine_WorkersOutliveStartHookContext is the regression
// test for a delivery engine that stopped delivering roughly
// fifteen seconds after boot. fx calls OnStart with a context
// carrying the application's start timeout (15s by default) and
// cancels it when the start phase ends, so a worker pool rooted
// in it exits shortly after startup: the process keeps accepting
// and persisting events while nothing at all forwards them.
//
// Driving OnStart with an already-cancelled context is that
// defect taken to its limit. A pool that inherits the hook
// context is gone before the task is even enqueued; a correctly
// rooted pool keeps working for as long as the process lives.
func TestEngine_WorkersOutliveStartHookContext(t *testing.T) {
t.Parallel()
s := newISetup(t)
lc := startEngineViaHook(t, s.Engine)
t.Cleanup(func() {
_ = lc.hooks[0].OnStop(context.Background())
})
// Seeded only after the pool has settled, so restart recovery
// cannot enqueue it during startup.
deliveryID, task := seedLogTask(t, s)
s.Engine.Notify([]delivery.Task{task})
iWaitForDelivered(t, s.WebhookDB, deliveryID)
}
// TestEngine_StopHookStopsWorkers proves the fix did not trade a
// startup bug for a shutdown hang: now that the worker pool no
// longer observes the start hook's cancellation, OnStop is the
// only thing that can stop it, and it must both return promptly
// and actually leave the pool drained.
func TestEngine_StopHookStopsWorkers(t *testing.T) {
t.Parallel()
s := newISetup(t)
lc := startEngineViaHook(t, s.Engine)
// Let the pool prove it is running before stopping it, so a
// fast OnStop cannot pass by stopping something already dead.
firstID, firstTask := seedLogTask(t, s)
s.Engine.Notify([]delivery.Task{firstTask})
iWaitForDelivered(t, s.WebhookDB, firstID)
var stopErr error
stopped := make(chan struct{})
go func() {
defer close(stopped)
// stop blocks on the workers' WaitGroup, so returning at
// all proves every goroutine observed the cancellation.
stopErr = lc.hooks[0].OnStop(context.Background())
}()
select {
case <-stopped:
case <-time.After(hookStopTimeout):
t.Fatal(
"OnStop did not return: the delivery engine's " +
"WaitGroup is still waiting on a goroutine that " +
"never observed cancellation",
)
}
require.NoError(t, stopErr)
// With every worker gone, a freshly notified task must sit
// untouched in the queue rather than being delivered.
secondID, secondTask := seedLogTask(t, s)
s.Engine.Notify([]delivery.Task{secondTask})
time.Sleep(200 * time.Millisecond)
var after database.Delivery
require.NoError(
t,
s.WebhookDB.First(&after, "id = ?", secondID).Error,
)
require.Equal(
t,
database.DeliveryStatusPending,
after.Status,
"a stopped engine must not deliver anything",
)
}

View File

@@ -7,7 +7,6 @@ import (
"net/http" "net/http"
"time" "time"
"go.uber.org/fx"
"gorm.io/gorm" "gorm.io/gorm"
"sneak.berlin/go/webhooker/internal/database" "sneak.berlin/go/webhooker/internal/database"
) )
@@ -190,16 +189,8 @@ func (e *Engine) ExportRecoverInFlight(
} }
// ExportStart exposes start for testing. // ExportStart exposes start for testing.
func (e *Engine) ExportStart() { func (e *Engine) ExportStart(ctx context.Context) {
e.start() e.start(ctx)
}
// ExportRegisterHooks registers the engine's real fx lifecycle
// hooks on a lifecycle supplied by a test, so a test can drive
// the exact OnStart/OnStop functions the application runs and
// hand OnStart the kind of context fx actually supplies.
func (e *Engine) ExportRegisterHooks(lc fx.Lifecycle) {
e.registerHooks(lc)
} }
// ExportStop exposes stop for testing. // ExportStop exposes stop for testing.

View File

@@ -186,6 +186,10 @@ func (s *Middleware) RequireAuth() func(http.Handler) http.Handler {
return 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) { if !s.session.IsAuthenticated(sess) {
s.log.Debug( s.log.Debug(
"auth middleware: unauthenticated request", "auth middleware: unauthenticated request",
@@ -199,6 +203,26 @@ func (s *Middleware) RequireAuth() func(http.Handler) http.Handler {
return 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) next.ServeHTTP(w, r)
}) })
} }

View File

@@ -8,6 +8,7 @@ import (
"net/http/httptest" "net/http/httptest"
"os" "os"
"testing" "testing"
"time"
"github.com/gorilla/sessions" "github.com/gorilla/sessions"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
@@ -28,6 +29,22 @@ func testMiddleware(
) (*middleware.Middleware, *session.Session) { ) (*middleware.Middleware, *session.Session) {
t.Helper() 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( log := slog.New(slog.NewTextHandler(
os.Stderr, os.Stderr,
&slog.HandlerOptions{Level: slog.LevelDebug}, &slog.HandlerOptions{Level: slog.LevelDebug},
@@ -35,6 +52,7 @@ func testMiddleware(
cfg := &config.Config{ cfg := &config.Config{
Environment: env, Environment: env,
SessionIdleTimeout: idleTimeout,
} }
// Create a real session manager with a known key // Create a real session manager with a known key
@@ -53,11 +71,40 @@ func testMiddleware(
SameSite: http.SameSiteLaxMode, 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) 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 --- // --- Logging Middleware Tests ---
@@ -387,6 +434,181 @@ func TestRequireAuth_UnauthenticatedSession_RedirectsToLogin(
assert.Equal(t, "/pages/login", w.Header().Get("Location")) 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 --- // --- NoCache Middleware Tests ---
func TestNoCache_SetsHeaders(t *testing.T) { func TestNoCache_SetsHeaders(t *testing.T) {
@@ -479,7 +701,7 @@ func metricsAuthMiddleware(
store := sessions.NewCookieStore(key) store := sessions.NewCookieStore(key)
store.Options = &sessions.Options{Path: "/", MaxAge: 86400} 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) return middleware.NewForTest(log, cfg, sessManager)
} }

View File

@@ -10,6 +10,7 @@ import (
"log/slog" "log/slog"
"maps" "maps"
"net/http" "net/http"
"time"
"github.com/gorilla/sessions" "github.com/gorilla/sessions"
"go.uber.org/fx" "go.uber.org/fx"
@@ -32,6 +33,18 @@ const (
// status. // status.
AuthenticatedKey = "authenticated" 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 // sessionKeyLength is the required length in bytes for the
// session authentication key. // session authentication key.
sessionKeyLength = 32 sessionKeyLength = 32
@@ -41,6 +54,19 @@ const (
// secondsPerDay is the number of seconds in a day. // secondsPerDay is the number of seconds in a day.
secondsPerDay = 86400 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 // 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 key []byte // raw 32-byte auth key, also used for CSRF cookie signing
log *slog.Logger log *slog.Logger
config *config.Config 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 // New creates a new session manager. The cookie store is
@@ -75,6 +111,8 @@ func New(
s := &Session{ s := &Session{
log: params.Logger.Get(), log: params.Logger.Get(),
config: params.Config, config: params.Config,
idleTimeout: params.Config.SessionIdleTimeout,
now: time.Now,
} }
lc.Append(fx.Hook{ lc.Append(fx.Hook{
@@ -149,29 +187,98 @@ func (s *Session) Save(
return sess.Save(r, w) 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( func (s *Session) SetUser(
sess *sessions.Session, sess *sessions.Session,
userID, username string, userID, username string,
) { ) {
now := s.now().Unix()
sess.Values[UserIDKey] = userID sess.Values[UserIDKey] = userID
sess.Values[UsernameKey] = username sess.Values[UsernameKey] = username
sess.Values[AuthenticatedKey] = true 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) { func (s *Session) ClearUser(sess *sessions.Session) {
delete(sess.Values, UserIDKey) delete(sess.Values, UserIDKey)
delete(sess.Values, UsernameKey) delete(sess.Values, UsernameKey)
delete(sess.Values, AuthenticatedKey) delete(sess.Values, AuthenticatedKey)
delete(sess.Values, CreatedAtKey)
delete(sess.Values, LastSeenKey)
} }
// IsAuthenticated checks if the session has an authenticated // sessionTime reads a Unix-second timestamp stored under key.
// user. 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 { func (s *Session) IsAuthenticated(sess *sessions.Session) bool {
auth, ok := sess.Values[AuthenticatedKey].(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. // GetUserID retrieves the user ID from the session.
@@ -253,3 +360,41 @@ func (s *Session) Regenerate(
return newSess, nil 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))
}

View File

@@ -7,6 +7,7 @@ import (
"net/http/httptest" "net/http/httptest"
"os" "os"
"testing" "testing"
"time"
"github.com/gorilla/sessions" "github.com/gorilla/sessions"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
@@ -17,11 +18,47 @@ import (
const testKeySize = 32 const testKeySize = 32
// testSession creates a Session with a real cookie store for // testIdleTimeout is the idle window used by the expiry tests.
// testing. 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 { func testSession(t *testing.T) *session.Session {
t.Helper() 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) key := make([]byte, testKeySize)
for i := range key { for i := range key {
@@ -39,6 +76,7 @@ func testSession(t *testing.T) *session.Session {
cfg := &config.Config{ cfg := &config.Config{
Environment: config.EnvironmentDev, Environment: config.EnvironmentDev,
SessionIdleTimeout: idleTimeout,
} }
log := slog.New(slog.NewTextHandler( log := slog.New(slog.NewTextHandler(
@@ -46,7 +84,46 @@ func testSession(t *testing.T) *session.Session {
&slog.HandlerOptions{Level: slog.LevelDebug}, &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 --- // --- Get and Save Tests ---
@@ -430,6 +507,263 @@ func TestSessionConstants(t *testing.T) {
assert.Equal(t, "user_id", session.UserIDKey) assert.Equal(t, "user_id", session.UserIDKey)
assert.Equal(t, "username", session.UsernameKey) assert.Equal(t, "username", session.UsernameKey)
assert.Equal(t, "authenticated", session.AuthenticatedKey) 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 --- // --- Edge Cases ---

View File

@@ -2,6 +2,7 @@ package session
import ( import (
"log/slog" "log/slog"
"time"
"github.com/gorilla/sessions" "github.com/gorilla/sessions"
"sneak.berlin/go/webhooker/internal/config" "sneak.berlin/go/webhooker/internal/config"
@@ -12,16 +13,28 @@ import (
// middleware and handler tests to use real session functionality. The key // middleware and handler tests to use real session functionality. The key
// parameter is the raw 32-byte authentication key used for session encryption // parameter is the raw 32-byte authentication key used for session encryption
// and CSRF cookie signing. // 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( func NewForTest(
store *sessions.CookieStore, store *sessions.CookieStore,
cfg *config.Config, cfg *config.Config,
log *slog.Logger, log *slog.Logger,
key []byte, key []byte,
now func() time.Time,
) *Session { ) *Session {
if now == nil {
now = time.Now
}
return &Session{ return &Session{
store: store, store: store,
key: key, key: key,
config: cfg, config: cfg,
log: log, log: log,
idleTimeout: cfg.SessionIdleTimeout,
now: now,
} }
} }