Compare commits
1 Commits
issue-102-
...
issue-108-
| Author | SHA1 | Date | |
|---|---|---|---|
| 618b07ca0f |
12
README.md
12
README.md
@@ -150,9 +150,10 @@ 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.
|
||||
after its last use. Any non-positive value (`0`, or a negative
|
||||
duration such as `-1s`) disables 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.
|
||||
@@ -164,6 +165,11 @@ 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.
|
||||
|
||||
Both clocks are anchored by timestamps stored in the session cookie.
|
||||
Sessions issued before this feature existed carry neither, so they are
|
||||
treated as expired: upgrading to a build that has it logs every
|
||||
existing session out once, and those users sign in again.
|
||||
|
||||
#### Invalid values abort startup
|
||||
|
||||
The defaults above apply **only** to variables that are unset (or set
|
||||
|
||||
@@ -46,19 +46,8 @@ func (r *RetentionReaper) ExportStart() {
|
||||
}
|
||||
|
||||
// ExportStop stops the reaper's background loop for tests.
|
||||
func (r *RetentionReaper) ExportStop(ctx context.Context) error {
|
||||
return r.stop(ctx)
|
||||
}
|
||||
|
||||
// ExportWedgeLoop adds a goroutine to the reaper's WaitGroup that
|
||||
// never observes cancellation and returns only when release is
|
||||
// closed. It stands in for a sweep stuck on a locked database.
|
||||
func (r *RetentionReaper) ExportWedgeLoop(
|
||||
release <-chan struct{},
|
||||
) {
|
||||
r.wg.Go(func() {
|
||||
<-release
|
||||
})
|
||||
func (r *RetentionReaper) ExportStop() {
|
||||
r.stop()
|
||||
}
|
||||
|
||||
// ExportSetInterval overrides the sweep interval for tests.
|
||||
|
||||
@@ -10,7 +10,6 @@ import (
|
||||
"go.uber.org/fx"
|
||||
"gorm.io/gorm"
|
||||
"sneak.berlin/go/webhooker/internal/config"
|
||||
"sneak.berlin/go/webhooker/internal/lifecycle"
|
||||
"sneak.berlin/go/webhooker/internal/logger"
|
||||
)
|
||||
|
||||
@@ -63,9 +62,8 @@ func NewRetentionReaper(
|
||||
}
|
||||
|
||||
// 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); the stop hook's
|
||||
// context is honoured (see stop).
|
||||
// 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{
|
||||
//nolint:contextcheck // Not inheriting the hook context is
|
||||
@@ -75,8 +73,10 @@ func (r *RetentionReaper) registerHooks(lc fx.Lifecycle) {
|
||||
|
||||
return nil
|
||||
},
|
||||
OnStop: func(ctx context.Context) error {
|
||||
return r.stop(ctx)
|
||||
OnStop: func(_ context.Context) error {
|
||||
r.stop()
|
||||
|
||||
return nil
|
||||
},
|
||||
})
|
||||
}
|
||||
@@ -105,27 +105,15 @@ func (r *RetentionReaper) start() {
|
||||
)
|
||||
}
|
||||
|
||||
// stop cancels the sweep loop's context and waits for it to
|
||||
// exit, bounded by the stop hook's context: a sweep wedged on a
|
||||
// locked database must not hang the process past fx's stop
|
||||
// timeout.
|
||||
func (r *RetentionReaper) stop(ctx context.Context) error {
|
||||
func (r *RetentionReaper) stop() {
|
||||
r.log.Info("retention reaper stopping")
|
||||
|
||||
if r.cancel != nil {
|
||||
r.cancel()
|
||||
}
|
||||
|
||||
err := lifecycle.WaitForShutdown(
|
||||
ctx, r.log, "retention reaper", &r.wg,
|
||||
)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
r.wg.Wait()
|
||||
r.log.Info("retention reaper stopped")
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func (r *RetentionReaper) run(ctx context.Context) {
|
||||
|
||||
@@ -26,13 +26,6 @@ const (
|
||||
// reaperTestRetentionDays is the retention policy the lifecycle
|
||||
// tests give their webhook.
|
||||
reaperTestRetentionDays = 30
|
||||
|
||||
// reaperWedgeStopTimeout is the stop timeout the wedged-shutdown
|
||||
// test hands OnStop, standing in for fx's StopTimeout. The test
|
||||
// asserts only that the hook returns at all, and allows it
|
||||
// reaperStopTimeout — forty times this budget — to do so, so no
|
||||
// assertion races the wall clock.
|
||||
reaperWedgeStopTimeout = 250 * time.Millisecond
|
||||
)
|
||||
|
||||
// recordingLifecycle is a minimal fx.Lifecycle that records the
|
||||
@@ -214,59 +207,3 @@ func TestRetentionReaper_StopHookStopsLoop(t *testing.T) {
|
||||
"a stopped reaper must not sweep anything",
|
||||
)
|
||||
}
|
||||
|
||||
// TestRetentionReaper_StopHookHonoursStopTimeout is the
|
||||
// regression test for a shutdown that could never complete. fx
|
||||
// hands OnStop a context carrying the application's stop timeout;
|
||||
// an OnStop that discards it and calls wg.Wait() bare hangs the
|
||||
// process forever on a sweep blocked on a locked SQLite database
|
||||
// — precisely when a bounded shutdown matters most.
|
||||
//
|
||||
// The wedged goroutine here never observes cancellation, so the
|
||||
// hook can only return by honouring its context, and it must say
|
||||
// so rather than reporting a clean stop.
|
||||
func TestRetentionReaper_StopHookHonoursStopTimeout(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupRetentionTest(t)
|
||||
|
||||
env.reaper.ExportSetInterval(reaperTestInterval)
|
||||
|
||||
lc := startReaperViaHook(t, env.reaper)
|
||||
|
||||
release := make(chan struct{})
|
||||
|
||||
t.Cleanup(func() { close(release) })
|
||||
|
||||
env.reaper.ExportWedgeLoop(release)
|
||||
|
||||
stopCtx, cancel := context.WithTimeout(
|
||||
context.Background(), reaperWedgeStopTimeout,
|
||||
)
|
||||
defer cancel()
|
||||
|
||||
var stopErr error
|
||||
|
||||
stopped := make(chan struct{})
|
||||
|
||||
go func() {
|
||||
defer close(stopped)
|
||||
|
||||
stopErr = lc.hooks[0].OnStop(stopCtx)
|
||||
}()
|
||||
|
||||
select {
|
||||
case <-stopped:
|
||||
case <-time.After(reaperStopTimeout):
|
||||
t.Fatal(
|
||||
"OnStop did not return: it discarded the stop " +
|
||||
"context and is waiting on a wedged goroutine " +
|
||||
"that will never observe cancellation",
|
||||
)
|
||||
}
|
||||
|
||||
require.ErrorIs(t, stopErr, context.DeadlineExceeded)
|
||||
require.ErrorContains(t, stopErr, "retention reaper")
|
||||
}
|
||||
|
||||
@@ -10,7 +10,6 @@ import (
|
||||
"go.uber.org/fx"
|
||||
"sneak.berlin/go/webhooker/internal/config"
|
||||
"sneak.berlin/go/webhooker/internal/database"
|
||||
"sneak.berlin/go/webhooker/internal/lifecycle"
|
||||
"sneak.berlin/go/webhooker/internal/logger"
|
||||
)
|
||||
|
||||
@@ -68,9 +67,10 @@ func NewArchiveSweeper(
|
||||
}
|
||||
|
||||
// registerHooks wires the sweeper's start and stop into the fx
|
||||
// lifecycle. The start hook's context is deliberately ignored
|
||||
// (see start for why the background loop must not inherit it);
|
||||
// the stop hook's context is honoured (see stop).
|
||||
// lifecycle. Both hook contexts are deliberately ignored: see
|
||||
// start for why the background loop must not inherit the start
|
||||
// hook's context, and stop for why shutdown blocks on the loop
|
||||
// rather than on the stop hook's deadline.
|
||||
func (s *ArchiveSweeper) registerHooks(lc fx.Lifecycle) {
|
||||
lc.Append(fx.Hook{
|
||||
//nolint:contextcheck // Not passing the hook context is
|
||||
@@ -80,8 +80,10 @@ func (s *ArchiveSweeper) registerHooks(lc fx.Lifecycle) {
|
||||
|
||||
return nil
|
||||
},
|
||||
OnStop: func(ctx context.Context) error {
|
||||
return s.stop(ctx)
|
||||
OnStop: func(_ context.Context) error {
|
||||
s.stop()
|
||||
|
||||
return nil
|
||||
},
|
||||
})
|
||||
}
|
||||
@@ -111,27 +113,15 @@ func (s *ArchiveSweeper) start() {
|
||||
)
|
||||
}
|
||||
|
||||
// stop cancels the sweep loop's context and waits for it to
|
||||
// exit, bounded by the stop hook's context: a prune wedged on a
|
||||
// locked archive must not hang the process past fx's stop
|
||||
// timeout.
|
||||
func (s *ArchiveSweeper) stop(ctx context.Context) error {
|
||||
func (s *ArchiveSweeper) stop() {
|
||||
s.log.Info("archive sweeper stopping")
|
||||
|
||||
if s.cancel != nil {
|
||||
s.cancel()
|
||||
}
|
||||
|
||||
err := lifecycle.WaitForShutdown(
|
||||
ctx, s.log, "archive sweeper", &s.wg,
|
||||
)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
s.wg.Wait()
|
||||
s.log.Info("archive sweeper stopped")
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func (s *ArchiveSweeper) run(ctx context.Context) {
|
||||
|
||||
@@ -14,6 +14,7 @@ import (
|
||||
"github.com/google/uuid"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"go.uber.org/fx"
|
||||
"gorm.io/driver/sqlite"
|
||||
"gorm.io/gorm"
|
||||
"gorm.io/gorm/clause"
|
||||
@@ -225,6 +226,17 @@ func countArchivedRows(path string) (int64, error) {
|
||||
return count, nil
|
||||
}
|
||||
|
||||
// captureLifecycle 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 captureLifecycle struct {
|
||||
hooks []fx.Hook
|
||||
}
|
||||
|
||||
func (l *captureLifecycle) Append(h fx.Hook) {
|
||||
l.hooks = append(l.hooks, h)
|
||||
}
|
||||
|
||||
// TestArchiveSweeper_LoopOutlivesStartHookContext is the
|
||||
// regression test for a sweeper that never swept. fx calls
|
||||
// OnStart with a context carrying the application's start
|
||||
@@ -258,7 +270,7 @@ func TestArchiveSweeper_LoopOutlivesStartHookContext(
|
||||
|
||||
// Drive the genuine fx hooks the application registers,
|
||||
// rather than a test-only entry point.
|
||||
lc := &recordingLifecycle{}
|
||||
lc := &captureLifecycle{}
|
||||
env.sweeper.ExportRegisterHooks(lc)
|
||||
require.Len(t, lc.hooks, 1)
|
||||
|
||||
@@ -912,36 +924,7 @@ func TestArchiveSweeper_StopsCleanly(t *testing.T) {
|
||||
env.sweeper.ExportSetInterval(time.Millisecond)
|
||||
env.sweeper.ExportStart()
|
||||
|
||||
// stop blocks on the loop's WaitGroup, so returning without
|
||||
// error proves the loop observed the cancellation and exited
|
||||
// well inside the stop context.
|
||||
require.NoError(
|
||||
t, env.sweeper.ExportStop(context.Background()),
|
||||
)
|
||||
}
|
||||
|
||||
// TestArchiveSweeper_StopHookHonoursStopTimeout is the sweeper's
|
||||
// half of the same shutdown defect the engine and the retention
|
||||
// reaper carried: an OnStop that discards its context and waits
|
||||
// on the WaitGroup bare hangs the process forever on a prune
|
||||
// wedged inside a locked archive.
|
||||
func TestArchiveSweeper_StopHookHonoursStopTimeout(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSweeperTest(t)
|
||||
|
||||
lc := &recordingLifecycle{}
|
||||
env.sweeper.ExportRegisterHooks(lc)
|
||||
require.Len(t, lc.hooks, 1)
|
||||
require.NoError(t, lc.hooks[0].OnStart(context.Background()))
|
||||
|
||||
release := make(chan struct{})
|
||||
|
||||
t.Cleanup(func() { close(release) })
|
||||
|
||||
env.sweeper.ExportWedgeLoop(release)
|
||||
|
||||
requireStopHookExpires(t, lc.hooks[0], "archive sweeper")
|
||||
// stop blocks on the loop's WaitGroup, so returning at all
|
||||
// proves the loop observed the cancellation and exited.
|
||||
env.sweeper.ExportStop()
|
||||
}
|
||||
|
||||
@@ -13,7 +13,6 @@ import (
|
||||
"go.uber.org/fx"
|
||||
"gorm.io/gorm"
|
||||
"sneak.berlin/go/webhooker/internal/database"
|
||||
"sneak.berlin/go/webhooker/internal/lifecycle"
|
||||
"sneak.berlin/go/webhooker/internal/logger"
|
||||
)
|
||||
|
||||
@@ -235,9 +234,8 @@ func (e *Engine) ScheduleRetry(
|
||||
}
|
||||
|
||||
// registerHooks wires the engine's start and stop into the fx
|
||||
// lifecycle. The start hook's context is deliberately ignored
|
||||
// (see start for why the worker pool must not inherit it); the
|
||||
// stop hook's context is honoured (see stop).
|
||||
// lifecycle. The start hook's context is deliberately ignored:
|
||||
// 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
|
||||
@@ -247,8 +245,10 @@ func (e *Engine) registerHooks(lc fx.Lifecycle) {
|
||||
|
||||
return nil
|
||||
},
|
||||
OnStop: func(ctx context.Context) error {
|
||||
return e.stop(ctx)
|
||||
OnStop: func(_ context.Context) error {
|
||||
e.stop()
|
||||
|
||||
return nil
|
||||
},
|
||||
})
|
||||
}
|
||||
@@ -289,26 +289,11 @@ func (e *Engine) start() {
|
||||
)
|
||||
}
|
||||
|
||||
// stop cancels the worker pool's context and waits for the pool
|
||||
// to drain, bounded by the stop hook's context: a wedged worker
|
||||
// must not hang the process past fx's stop timeout.
|
||||
func (e *Engine) stop(ctx context.Context) error {
|
||||
func (e *Engine) stop() {
|
||||
e.log.Info("delivery engine stopping")
|
||||
|
||||
if e.cancel != nil {
|
||||
e.cancel()
|
||||
}
|
||||
|
||||
err := lifecycle.WaitForShutdown(
|
||||
ctx, e.log, "delivery engine", &e.wg,
|
||||
)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
e.wg.Wait()
|
||||
e.log.Info("delivery engine stopped")
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func (e *Engine) worker(ctx context.Context) {
|
||||
|
||||
@@ -501,7 +501,7 @@ func TestWorkerLifecycle_StartStop(t *testing.T) {
|
||||
|
||||
iWaitForDelivered(t, s.WebhookDB, d.ID)
|
||||
|
||||
require.NoError(t, s.Engine.ExportStop(context.Background()))
|
||||
s.Engine.ExportStop()
|
||||
}
|
||||
|
||||
// iWaitForDelivered polls until the delivery reaches the
|
||||
@@ -567,7 +567,7 @@ func TestWorkerLifecycle_ProcessesRetryChannel(
|
||||
|
||||
iWaitForDelivered(t, s.WebhookDB, d.ID)
|
||||
|
||||
require.NoError(t, s.Engine.ExportStop(context.Background()))
|
||||
s.Engine.ExportStop()
|
||||
}
|
||||
|
||||
// --- processDelivery: unknown target type ---
|
||||
|
||||
@@ -27,13 +27,6 @@ const (
|
||||
// and a ready deliveryCh are chosen between at random and a
|
||||
// doomed pool still delivers.
|
||||
hookSettleDelay = 250 * time.Millisecond
|
||||
|
||||
// wedgeStopTimeout is the stop timeout a wedged-shutdown test
|
||||
// hands OnStop, standing in for fx's StopTimeout. The test
|
||||
// asserts only that the hook returns at all, and allows it
|
||||
// hookStopTimeout — forty times this budget — to do so, so no
|
||||
// assertion here races the wall clock.
|
||||
wedgeStopTimeout = 250 * time.Millisecond
|
||||
)
|
||||
|
||||
// recordingLifecycle is a minimal fx.Lifecycle that records the
|
||||
@@ -47,44 +40,6 @@ func (l *recordingLifecycle) Append(h fx.Hook) {
|
||||
l.hooks = append(l.hooks, h)
|
||||
}
|
||||
|
||||
// requireStopHookExpires drives hook.OnStop with a stop context
|
||||
// that expires while a wedged goroutine is still running, and
|
||||
// requires the hook to return the deadline error naming
|
||||
// component instead of blocking on the WaitGroup forever.
|
||||
func requireStopHookExpires(
|
||||
t *testing.T, hook fx.Hook, component string,
|
||||
) {
|
||||
t.Helper()
|
||||
|
||||
stopCtx, cancel := context.WithTimeout(
|
||||
context.Background(), wedgeStopTimeout,
|
||||
)
|
||||
defer cancel()
|
||||
|
||||
var stopErr error
|
||||
|
||||
stopped := make(chan struct{})
|
||||
|
||||
go func() {
|
||||
defer close(stopped)
|
||||
|
||||
stopErr = hook.OnStop(stopCtx)
|
||||
}()
|
||||
|
||||
select {
|
||||
case <-stopped:
|
||||
case <-time.After(hookStopTimeout):
|
||||
t.Fatal(
|
||||
"OnStop did not return: it discarded the stop " +
|
||||
"context and is waiting on a wedged goroutine " +
|
||||
"that will never observe cancellation",
|
||||
)
|
||||
}
|
||||
|
||||
require.ErrorIs(t, stopErr, context.DeadlineExceeded)
|
||||
require.ErrorContains(t, stopErr, component)
|
||||
}
|
||||
|
||||
// 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
|
||||
@@ -242,30 +197,3 @@ func TestEngine_StopHookStopsWorkers(t *testing.T) {
|
||||
"a stopped engine must not deliver anything",
|
||||
)
|
||||
}
|
||||
|
||||
// TestEngine_StopHookHonoursStopTimeout is the regression test
|
||||
// for a shutdown that could never complete. fx hands OnStop a
|
||||
// context carrying the application's stop timeout; an OnStop
|
||||
// that discards it and calls wg.Wait() bare hangs the process
|
||||
// forever on a single worker stuck inside a delivery target that
|
||||
// never returns — precisely when a bounded shutdown matters
|
||||
// most.
|
||||
//
|
||||
// The wedged goroutine here never observes cancellation, so the
|
||||
// hook can only return by honouring its context, and it must say
|
||||
// so rather than reporting a clean stop.
|
||||
func TestEngine_StopHookHonoursStopTimeout(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
s := newISetup(t)
|
||||
|
||||
lc := startEngineViaHook(t, s.Engine)
|
||||
|
||||
release := make(chan struct{})
|
||||
|
||||
t.Cleanup(func() { close(release) })
|
||||
|
||||
s.Engine.ExportWedgeWorker(release)
|
||||
|
||||
requireStopHookExpires(t, lc.hooks[0], "delivery engine")
|
||||
}
|
||||
|
||||
@@ -216,19 +216,8 @@ func (e *Engine) ExportRegisterHooks(lc fx.Lifecycle) {
|
||||
}
|
||||
|
||||
// ExportStop exposes stop for testing.
|
||||
func (e *Engine) ExportStop(ctx context.Context) error {
|
||||
return e.stop(ctx)
|
||||
}
|
||||
|
||||
// ExportWedgeWorker adds a goroutine to the engine's WaitGroup
|
||||
// that never observes cancellation and returns only when release
|
||||
// is closed. It stands in for a worker stuck inside a delivery
|
||||
// target that never returns, which is the only way stop can be
|
||||
// made to outlast its context.
|
||||
func (e *Engine) ExportWedgeWorker(release <-chan struct{}) {
|
||||
e.wg.Go(func() {
|
||||
<-release
|
||||
})
|
||||
func (e *Engine) ExportStop() {
|
||||
e.stop()
|
||||
}
|
||||
|
||||
// ExportDeliveryCh returns the delivery channel.
|
||||
@@ -529,19 +518,8 @@ func (s *ArchiveSweeper) ExportRegisterHooks(lc fx.Lifecycle) {
|
||||
}
|
||||
|
||||
// ExportStop stops the sweeper's background loop for tests.
|
||||
func (s *ArchiveSweeper) ExportStop(ctx context.Context) error {
|
||||
return s.stop(ctx)
|
||||
}
|
||||
|
||||
// ExportWedgeLoop adds a goroutine to the sweeper's WaitGroup
|
||||
// that never observes cancellation and returns only when release
|
||||
// is closed. It stands in for a prune stuck on a locked archive.
|
||||
func (s *ArchiveSweeper) ExportWedgeLoop(
|
||||
release <-chan struct{},
|
||||
) {
|
||||
s.wg.Go(func() {
|
||||
<-release
|
||||
})
|
||||
func (s *ArchiveSweeper) ExportStop() {
|
||||
s.stop()
|
||||
}
|
||||
|
||||
// ExportSetInterval overrides the sweep interval for tests.
|
||||
|
||||
@@ -1,57 +0,0 @@
|
||||
// Package lifecycle holds helpers shared by the components that
|
||||
// register fx start and stop hooks.
|
||||
package lifecycle
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"sync"
|
||||
)
|
||||
|
||||
// WaitForShutdown waits for wg to drain, bounded by ctx.
|
||||
//
|
||||
// fx hands OnStop a context carrying the application's stop
|
||||
// timeout. A bare wg.Wait() discards that deadline, so a single
|
||||
// goroutine that never observes cancellation — a delivery target
|
||||
// that never returns, a SQLite operation blocked on a lock —
|
||||
// hangs the process forever instead of letting it exit when the
|
||||
// timeout expires, which is exactly when a clean shutdown matters
|
||||
// most.
|
||||
//
|
||||
// On timeout it logs at error naming component and returns an
|
||||
// error: the goroutines are still running, and reporting success
|
||||
// would hide an unclean shutdown from the operator. The waiting
|
||||
// goroutine outlives this call and exits when (if) wg drains; it
|
||||
// holds nothing but the channel it closes.
|
||||
func WaitForShutdown(
|
||||
ctx context.Context,
|
||||
log *slog.Logger,
|
||||
component string,
|
||||
wg *sync.WaitGroup,
|
||||
) error {
|
||||
done := make(chan struct{})
|
||||
|
||||
go func() {
|
||||
defer close(done)
|
||||
|
||||
wg.Wait()
|
||||
}()
|
||||
|
||||
select {
|
||||
case <-done:
|
||||
return nil
|
||||
case <-ctx.Done():
|
||||
log.Error(
|
||||
"shutdown timed out, goroutines still running",
|
||||
"component", component,
|
||||
"error", ctx.Err(),
|
||||
)
|
||||
|
||||
return fmt.Errorf(
|
||||
"%s: shutdown timed out, "+
|
||||
"goroutines still running: %w",
|
||||
component, ctx.Err(),
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -1,62 +0,0 @@
|
||||
package lifecycle_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"log/slog"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
"sneak.berlin/go/webhooker/internal/lifecycle"
|
||||
)
|
||||
|
||||
// waitTimeout is the stop budget the timeout case gives a
|
||||
// goroutine that never returns. The test's own patience is the
|
||||
// go test deadline, so the only thing this value affects is how
|
||||
// long the case takes.
|
||||
const waitTimeout = 100 * time.Millisecond
|
||||
|
||||
func discardLogger() *slog.Logger {
|
||||
return slog.New(slog.DiscardHandler)
|
||||
}
|
||||
|
||||
func TestWaitForShutdown_DrainedGroup(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var wg sync.WaitGroup
|
||||
|
||||
wg.Go(func() {})
|
||||
|
||||
require.NoError(
|
||||
t,
|
||||
lifecycle.WaitForShutdown(
|
||||
context.Background(), discardLogger(),
|
||||
"test component", &wg,
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
func TestWaitForShutdown_ContextExpires(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
release := make(chan struct{})
|
||||
|
||||
t.Cleanup(func() { close(release) })
|
||||
|
||||
var wg sync.WaitGroup
|
||||
|
||||
wg.Go(func() { <-release })
|
||||
|
||||
ctx, cancel := context.WithTimeout(
|
||||
context.Background(), waitTimeout,
|
||||
)
|
||||
defer cancel()
|
||||
|
||||
err := lifecycle.WaitForShutdown(
|
||||
ctx, discardLogger(), "test component", &wg,
|
||||
)
|
||||
|
||||
require.ErrorIs(t, err, context.DeadlineExceeded)
|
||||
require.ErrorContains(t, err, "test component")
|
||||
}
|
||||
@@ -214,11 +214,11 @@ func (s *Middleware) RequireAuth() func(http.Handler) http.Handler {
|
||||
// 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 {
|
||||
saveErr := s.session.Save(r, w, sess)
|
||||
if saveErr != nil {
|
||||
s.log.Error(
|
||||
"auth middleware: failed to refresh session",
|
||||
"error", err,
|
||||
"error", saveErr,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
150
internal/session/codec_test.go
Normal file
150
internal/session/codec_test.go
Normal file
@@ -0,0 +1,150 @@
|
||||
package session_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"crypto/hmac"
|
||||
"crypto/sha256"
|
||||
"encoding/base64"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/gorilla/sessions"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"sneak.berlin/go/webhooker/internal/session"
|
||||
)
|
||||
|
||||
// The tests below exercise the securecookie codecs underneath the
|
||||
// store and nothing else: Session.Get only decodes, so no server-side
|
||||
// expiry check takes part in the result. They exist because
|
||||
// NewCookieStore gives its codecs a 30-day max age that assigning
|
||||
// store.Options does not override, which would let the codec accept a
|
||||
// cookie weeks past the cap the cookie attribute advertises.
|
||||
|
||||
// issuedCookie returns a session cookie the store itself wrote.
|
||||
func issuedCookie(t *testing.T, s *session.Session) string {
|
||||
t.Helper()
|
||||
|
||||
req := httptest.NewRequestWithContext(
|
||||
context.Background(), http.MethodGet, "/", nil)
|
||||
w := httptest.NewRecorder()
|
||||
|
||||
sess, err := s.Get(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
sess.Values["probe"] = "value"
|
||||
require.NoError(t, s.Save(req, w, sess))
|
||||
|
||||
cookies := w.Result().Cookies()
|
||||
require.Len(t, cookies, 1)
|
||||
|
||||
return cookies[0].Value
|
||||
}
|
||||
|
||||
// restamp rewrites the timestamp inside an encoded session cookie and
|
||||
// re-signs it, yielding the cookie the store would have written at
|
||||
// that instant. securecookie stamps the encoding time itself and
|
||||
// exposes no seam to move it, so its wire format is reproduced here:
|
||||
// the base64url payload is "date|value|mac", where mac is HMAC-SHA256
|
||||
// of "name|date|value" under the store's key.
|
||||
func restamp(
|
||||
t *testing.T,
|
||||
encoded string,
|
||||
at time.Time,
|
||||
) string {
|
||||
t.Helper()
|
||||
|
||||
raw, err := base64.URLEncoding.DecodeString(encoded)
|
||||
require.NoError(t, err)
|
||||
|
||||
parts := strings.SplitN(string(raw), "|", 3)
|
||||
require.Len(t, parts, 3)
|
||||
|
||||
stamped := fmt.Sprintf("%d|%s", at.Unix(), parts[1])
|
||||
|
||||
mac := hmac.New(sha256.New, testKey())
|
||||
_, err = mac.Write([]byte(session.SessionName + "|" + stamped))
|
||||
require.NoError(t, err)
|
||||
|
||||
payload := append([]byte(stamped+"|"), mac.Sum(nil)...)
|
||||
|
||||
return base64.URLEncoding.EncodeToString(payload)
|
||||
}
|
||||
|
||||
// decodeCookie feeds value back through the store's decode path.
|
||||
func decodeCookie(
|
||||
t *testing.T,
|
||||
s *session.Session,
|
||||
value string,
|
||||
) (*sessions.Session, error) {
|
||||
t.Helper()
|
||||
|
||||
req := httptest.NewRequestWithContext(
|
||||
context.Background(), http.MethodGet, "/", nil)
|
||||
req.AddCookie(&http.Cookie{
|
||||
Name: session.SessionName,
|
||||
Value: value,
|
||||
Path: "/",
|
||||
HttpOnly: true,
|
||||
Secure: true,
|
||||
SameSite: http.SameSiteLaxMode,
|
||||
})
|
||||
|
||||
sess, err := s.Get(req)
|
||||
require.NotNil(t, sess)
|
||||
|
||||
return sess, err
|
||||
}
|
||||
|
||||
func TestCodec_AcceptsCookieInsideAbsoluteCap(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
s := testSession(t)
|
||||
|
||||
sess, err := decodeCookie(t, s, restamp(
|
||||
t,
|
||||
issuedCookie(t, s),
|
||||
time.Now().Add(-(testAbsoluteMaxAge-time.Hour)),
|
||||
))
|
||||
require.NoError(t, err)
|
||||
assert.False(
|
||||
t, sess.IsNew,
|
||||
"a cookie inside the cap must still decode",
|
||||
)
|
||||
assert.Equal(
|
||||
t, "value", sess.Values["probe"],
|
||||
"decoding must yield the values that were saved",
|
||||
)
|
||||
}
|
||||
|
||||
func TestCodec_RejectsCookiePastAbsoluteCap(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
s := testSession(t)
|
||||
|
||||
sess, err := decodeCookie(t, s, restamp(
|
||||
t,
|
||||
issuedCookie(t, s),
|
||||
time.Now().Add(-(testAbsoluteMaxAge+time.Hour)),
|
||||
))
|
||||
require.Error(
|
||||
t, err,
|
||||
"the codec must refuse a cookie older than the cap",
|
||||
)
|
||||
assert.Contains(
|
||||
t, err.Error(), "expired timestamp",
|
||||
"rejection must come from the codec's age check",
|
||||
)
|
||||
assert.True(
|
||||
t, sess.IsNew,
|
||||
"a cookie past the cap must not populate a session",
|
||||
)
|
||||
assert.Nil(
|
||||
t, sess.Values["probe"],
|
||||
"a cookie past the cap must not yield its values",
|
||||
)
|
||||
}
|
||||
10
internal/session/export_test.go
Normal file
10
internal/session/export_test.go
Normal file
@@ -0,0 +1,10 @@
|
||||
package session
|
||||
|
||||
import "github.com/gorilla/sessions"
|
||||
|
||||
// NewStore exposes the production cookie-store constructor so tests
|
||||
// exercise the store the application actually runs with, rather than a
|
||||
// lookalike assembled in the test.
|
||||
func NewStore(key []byte, secure bool) *sessions.CookieStore {
|
||||
return newStore(key, secure)
|
||||
}
|
||||
@@ -100,6 +100,35 @@ type Session struct {
|
||||
now func() time.Time
|
||||
}
|
||||
|
||||
// cookieOptions returns the cookie attributes used for every session
|
||||
// cookie. MaxAge is deliberately left at its zero value: for a store
|
||||
// it is set through CookieStore.MaxAge (see newStore), and for a
|
||||
// single session it is copied from the store's options.
|
||||
func cookieOptions(secure bool) *sessions.Options {
|
||||
return &sessions.Options{
|
||||
Path: "/",
|
||||
HttpOnly: true,
|
||||
Secure: secure,
|
||||
SameSite: http.SameSiteLaxMode,
|
||||
}
|
||||
}
|
||||
|
||||
// newStore builds the session cookie store.
|
||||
//
|
||||
// The absolute cap MUST be applied with store.MaxAge and not by
|
||||
// assigning store.Options.MaxAge. NewCookieStore gives the underlying
|
||||
// securecookie codecs a 30-day max age of their own, and assigning
|
||||
// Options never touches Codecs -- so a store configured that way still
|
||||
// decodes a 30-day-old cookie, leaving the cookie attribute and the
|
||||
// codec disagreeing about the same policy. store.MaxAge sets both.
|
||||
func newStore(key []byte, secure bool) *sessions.CookieStore {
|
||||
store := sessions.NewCookieStore(key)
|
||||
store.Options = cookieOptions(secure)
|
||||
store.MaxAge(secondsPerDay * sessionMaxAgeDays)
|
||||
|
||||
return store
|
||||
}
|
||||
|
||||
// New creates a new session manager. The cookie store is
|
||||
// initialized during the fx OnStart phase after the database is
|
||||
// connected, using a session key that is auto-generated and stored
|
||||
@@ -142,19 +171,8 @@ func New(
|
||||
)
|
||||
}
|
||||
|
||||
store := sessions.NewCookieStore(keyBytes)
|
||||
|
||||
// Configure cookie options for security
|
||||
store.Options = &sessions.Options{
|
||||
Path: "/",
|
||||
MaxAge: secondsPerDay * sessionMaxAgeDays,
|
||||
HttpOnly: true,
|
||||
Secure: !params.Config.IsDev(),
|
||||
SameSite: http.SameSiteLaxMode,
|
||||
}
|
||||
|
||||
s.key = keyBytes
|
||||
s.store = store
|
||||
s.store = newStore(keyBytes, !params.Config.IsDev())
|
||||
s.log.Info("session manager initialized")
|
||||
|
||||
return nil
|
||||
@@ -350,13 +368,8 @@ func (s *Session) Regenerate(
|
||||
// Apply the standard session options (the destroyed old
|
||||
// session had MaxAge = -1, which store.New might inherit
|
||||
// from the cookie).
|
||||
newSess.Options = &sessions.Options{
|
||||
Path: "/",
|
||||
MaxAge: secondsPerDay * sessionMaxAgeDays,
|
||||
HttpOnly: true,
|
||||
Secure: !s.config.IsDev(),
|
||||
SameSite: http.SameSiteLaxMode,
|
||||
}
|
||||
newSess.Options = cookieOptions(!s.config.IsDev())
|
||||
newSess.Options.MaxAge = secondsPerDay * sessionMaxAgeDays
|
||||
|
||||
return newSess, nil
|
||||
}
|
||||
|
||||
@@ -39,6 +39,19 @@ func (c *fakeClock) Advance(d time.Duration) {
|
||||
c.t = c.t.Add(d)
|
||||
}
|
||||
|
||||
// testKey returns the fixed session key the tests sign with. The
|
||||
// codec tests re-sign cookies with it, so it must be the same key the
|
||||
// store was built from.
|
||||
func testKey() []byte {
|
||||
key := make([]byte, testKeySize)
|
||||
|
||||
for i := range key {
|
||||
key[i] = byte(i + 42)
|
||||
}
|
||||
|
||||
return key
|
||||
}
|
||||
|
||||
// testSession creates a Session with a real cookie store and the
|
||||
// real clock.
|
||||
func testSession(t *testing.T) *session.Session {
|
||||
@@ -59,20 +72,8 @@ func testSessionWithClock(
|
||||
) (*session.Session, *fakeClock) {
|
||||
t.Helper()
|
||||
|
||||
key := make([]byte, testKeySize)
|
||||
|
||||
for i := range key {
|
||||
key[i] = byte(i + 42)
|
||||
}
|
||||
|
||||
store := sessions.NewCookieStore(key)
|
||||
store.Options = &sessions.Options{
|
||||
Path: "/",
|
||||
MaxAge: 86400 * 7,
|
||||
HttpOnly: true,
|
||||
Secure: false,
|
||||
SameSite: http.SameSiteLaxMode,
|
||||
}
|
||||
key := testKey()
|
||||
store := session.NewStore(key, false)
|
||||
|
||||
cfg := &config.Config{
|
||||
Environment: config.EnvironmentDev,
|
||||
@@ -645,6 +646,34 @@ func TestTouch_LazyBelowRefreshThreshold(t *testing.T) {
|
||||
)
|
||||
}
|
||||
|
||||
func TestTouch_RefreshThresholdIsOneTenthOfIdleWindow(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// testRefreshDivisor restates the documented bound independently
|
||||
// of the implementation constant: the idle timestamp is rewritten
|
||||
// once it is a tenth of the idle window old, which is what makes
|
||||
// "expires up to 10% early, never late" true. Both assertions are
|
||||
// needed to pin it -- a larger divisor fails the first, a smaller
|
||||
// one fails the second.
|
||||
const testRefreshDivisor = 10
|
||||
|
||||
threshold := testIdleTimeout / testRefreshDivisor
|
||||
|
||||
s, sess, clock := authenticatedSession(t, testIdleTimeout)
|
||||
|
||||
clock.Advance(threshold - time.Second)
|
||||
assert.False(
|
||||
t, s.Touch(sess),
|
||||
"Touch must not rewrite the session below a tenth of the window",
|
||||
)
|
||||
|
||||
clock.Advance(time.Second)
|
||||
assert.True(
|
||||
t, s.Touch(sess),
|
||||
"Touch must rewrite the session at a tenth of the window",
|
||||
)
|
||||
}
|
||||
|
||||
func TestTouch_UnauthenticatedSessionIsNotRefreshed(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user