1 Commits

Author SHA1 Message Date
clawbot
0384de4a7b Terminally fail retrying deliveries with a non-retry target type (closes #82)
All checks were successful
check / check (push) Successful in 4m4s
Restart recovery and the 60s retry sweep both looked an orphaned
`retrying` delivery's target up in the registry and silently returned
when it did not implement `rescheduler`. If a target's type was edited
from a retry type (`http`/`slack`) to a fire-and-forget type
(`database`/`log`) or an unknown one while a delivery was still
retrying, that delivery stayed `retrying` forever.

Both sites now hand the delivery to one shared helper,
`failUnretryableRetry`, which records a `DeliveryResult` naming the
current target type as the reason and marks the delivery `failed`. It
logs at warn, not error: this is operator-caused state, not a system
fault.

Re-dispatching under the new type was rejected as it would perform a
delivery the operator never asked for; the event itself stays in the
per-webhook event database, so manual redelivery can recover it
deliberately.

Fire-and-forget targets never set status `retrying` under normal
operation, so this path stays unreachable for them in practice.
2026-08-09 05:51:51 +00:00
12 changed files with 298 additions and 885 deletions

View File

@@ -92,27 +92,6 @@ 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
@@ -657,6 +636,18 @@ This means:
durable fallback that ensures no retry is permanently lost, even under
extreme backpressure.
**Changing a target's type does not migrate in-flight deliveries.** Only
`http` and `slack` targets own durable retries; `database` and `log`
targets are fire-and-forget and never produce a `retrying` delivery. If a
target's `type` is edited from a retrying type to a non-retrying (or
unknown) one while one of its deliveries is still `retrying`, both
recovery paths above terminally mark that delivery `failed` and record a
`DeliveryResult` naming the current target type as the reason, logging it
at warn level. The delivery is not re-dispatched under the new type — the
operator never asked for that delivery — and the event itself remains
stored in the per-webhook event database, so it can be redelivered
manually.
### Circuit Breaker (HTTP Targets with Retries)
HTTP targets with `max_retries` > 0 are protected by a **per-target circuit breaker** that

10
TODO.md
View File

@@ -28,10 +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-09 Restart recovery and the 60s retry sweep terminally fail an
orphaned `retrying` delivery whose target type no longer supports
retries, recording a `DeliveryResult` with the reason instead of
leaving the delivery stuck forever (#82)
- 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
@@ -75,7 +75,7 @@ databases currently grow without bound.
- event redelivery endpoint
- OpenAPI specification
- Analytics dashboard: success rates, response times, volume
- A remember-me option at login
- Session expiration tuning and a remember-me option
- Password change and reset flow
- Later, nice to have
- email delivery target type

View File

@@ -31,10 +31,6 @@ 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
@@ -64,10 +60,6 @@ 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
}
@@ -170,15 +162,6 @@ 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"),
@@ -190,7 +173,6 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
Port: envInt("PORT", defaultPort),
SentryDSN: envString("SENTRY_DSN"),
RetentionSweepInterval: retentionSweepInterval,
SessionIdleTimeout: sessionIdleTimeout,
log: log,
params: &params,
}

View File

@@ -163,7 +163,7 @@ func TestRetentionSweepInterval(t *testing.T) {
}
if tt.expectError {
expectStartupError(t)
testRetentionSweepIntervalError(t)
} else {
testRetentionSweepIntervalSuccess(t, tt.expected)
}
@@ -171,9 +171,7 @@ func TestRetentionSweepInterval(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) {
func testRetentionSweepIntervalError(t *testing.T) {
t.Helper()
var cfg *config.Config
@@ -217,82 +215,6 @@ 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

View File

@@ -453,8 +453,9 @@ func (e *Engine) recoverRetryingDeliveries(
// recoverSingleRetry hands an orphaned retrying delivery back
// to its target to recompute the remaining backoff, then
// reschedules it. Targets that do not own durable retries
// (fire-and-forget) never produce retrying deliveries, so
// they are skipped.
// (fire-and-forget) never produce retrying deliveries, so a
// delivery found in that state has had its target's type
// changed underneath it and is terminally failed.
func (e *Engine) recoverSingleRetry(
webhookDB *gorm.DB,
webhookID string,
@@ -475,6 +476,10 @@ func (e *Engine) recoverSingleRetry(
rs, ok := e.targets[target.Type].(rescheduler)
if !ok {
e.failUnretryableRetry(
webhookDB, webhookID, d, &target,
)
return
}
@@ -649,8 +654,8 @@ func (e *Engine) sweepWebhookRetries(
// sweepSingleRetry re-enqueues an orphaned retrying delivery
// whose backoff window has elapsed, delegating the backoff
// decision to the delivery's target. Targets that do not own
// durable retries are skipped.
// decision to the delivery's target. A delivery whose target
// no longer owns durable retries is terminally failed.
func (e *Engine) sweepSingleRetry(
webhookDB *gorm.DB,
webhookID string,
@@ -670,6 +675,10 @@ func (e *Engine) sweepSingleRetry(
rs, ok := e.targets[target.Type].(rescheduler)
if !ok {
e.failUnretryableRetry(
webhookDB, webhookID, d, &target,
)
return
}
@@ -710,6 +719,59 @@ func (e *Engine) sweepSingleRetry(
}
}
// failUnretryableRetry terminally fails an orphaned retrying
// delivery whose target type no longer supports retries. Both
// restart recovery and the periodic sweep call it, so the
// terminal transition exists once.
//
// This is only reachable when a target's type has been changed
// out from under an in-flight retrying delivery (or the type is
// unknown to the registry): fire-and-forget targets never set
// status retrying themselves. Re-dispatching under the new type
// would be a delivery the operator never asked for, and leaving
// the row retrying strands it forever, so the delivery is
// failed with a recorded reason and can be redelivered
// manually. Logged at warn, not error: this is operator-caused
// state, not a system fault.
func (e *Engine) failUnretryableRetry(
webhookDB *gorm.DB,
webhookID string,
d *database.Delivery,
target *database.Target,
) {
e.log.Warn(
"failing orphaned retrying delivery: target "+
"type no longer supports retries",
"webhook_id", webhookID,
"delivery_id", d.ID,
"target_id", target.ID,
"target_name", target.Name,
"target_type", target.Type,
)
reason := fmt.Sprintf(
"target type %q does not support retries; "+
"delivery was left retrying by a previous "+
"target type and has been failed terminally",
target.Type,
)
e.recordResult(
webhookDB,
d,
e.countAttempts(webhookDB, d.ID)+1,
false,
0,
"",
reason,
0,
)
e.updateDeliveryStatus(
webhookDB, d, database.DeliveryStatusFailed,
)
}
// processDelivery dispatches a delivery to the target that
// owns its type. Unknown target types fail the delivery.
func (e *Engine) processDelivery(

View File

@@ -748,6 +748,193 @@ func TestRecoverWebhookDeliveries_RetryingDeliveries(
case <-time.After(5 * time.Second):
t.Fatal("expected retry task from recovery")
}
// Regression guard: a target that still supports retries
// must be rescheduled, never terminally failed, and must
// not gain a synthetic result row.
iAssertStatus(
t, s.WebhookDB, d.ID,
database.DeliveryStatusRetrying,
)
assert.Len(t, iResults(t, s.WebhookDB, d.ID), 1)
}
// --- Retrying deliveries whose target type changed ---
// iSeedRetryingWithType seeds a retrying delivery with one
// recorded failed attempt against a target of the given type,
// standing in for a target whose type was edited in the main
// database while the delivery was still retrying.
func iSeedRetryingWithType(
t *testing.T,
s iSetup,
targetType database.TargetType,
) string {
t.Helper()
targetID := uuid.New().String()
iCreateTarget(t, s.MainDB, targetID,
s.WebhookID, "mutated-target", targetType,
iHTTPConfig("http://example.com/hook"), 5,
)
event := iSeedEvent(
t, s.WebhookDB, s.WebhookID,
`{"orphaned":"retry"}`,
)
d := iSeedDelivery(
t, s.WebhookDB, event.ID, targetID,
database.DeliveryStatusRetrying,
)
iSeedFailedResult(t, s.WebhookDB, d.ID)
return d.ID
}
// iResults loads a delivery's results in attempt order.
func iResults(
t *testing.T, db *gorm.DB, deliveryID string,
) []database.DeliveryResult {
t.Helper()
var results []database.DeliveryResult
require.NoError(t, db.
Where("delivery_id = ?", deliveryID).
Order("attempt_num").
Find(&results).Error)
return results
}
// iAssertTerminallyFailed asserts the delivery ended failed
// with a result row recording why, and was not rescheduled.
func iAssertTerminallyFailed(
t *testing.T,
s iSetup,
deliveryID string,
targetType database.TargetType,
) {
t.Helper()
iAssertStatus(
t, s.WebhookDB, deliveryID,
database.DeliveryStatusFailed,
)
results := iResults(t, s.WebhookDB, deliveryID)
require.Len(t, results, 2)
last := results[1]
assert.False(t, last.Success)
assert.Equal(t, 2, last.AttemptNum)
assert.Contains(
t, last.Error, string(targetType),
)
assert.Contains(
t, last.Error, "does not support retries",
)
assert.Empty(t, s.Engine.ExportRetryCh())
}
func TestRecoverSingleRetry_TypeNoLongerRetries(
t *testing.T,
) {
t.Parallel()
s := newISetup(t)
iCreateWebhook(
t, s.MainDB, s.WebhookID, "mutated-type",
)
deliveryID := iSeedRetryingWithType(
t, s, database.TargetTypeLog,
)
s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID,
)
iAssertTerminallyFailed(
t, s, deliveryID, database.TargetTypeLog,
)
}
func TestSweepSingleRetry_TypeNoLongerRetries(
t *testing.T,
) {
t.Parallel()
s := newISetup(t)
iCreateWebhook(
t, s.MainDB, s.WebhookID, "mutated-type-sweep",
)
deliveryID := iSeedRetryingWithType(
t, s, database.TargetTypeDatabase,
)
s.Engine.ExportSweepWebhookRetries(
context.Background(), s.WebhookID,
)
iAssertTerminallyFailed(
t, s, deliveryID, database.TargetTypeDatabase,
)
}
func TestRecoverSingleRetry_UnknownTargetType(
t *testing.T,
) {
t.Parallel()
s := newISetup(t)
iCreateWebhook(
t, s.MainDB, s.WebhookID, "unknown-type",
)
unknown := database.TargetType("not-a-target-type")
deliveryID := iSeedRetryingWithType(t, s, unknown)
s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID,
)
iAssertTerminallyFailed(t, s, deliveryID, unknown)
}
func TestSweepSingleRetry_UnknownTargetType(
t *testing.T,
) {
t.Parallel()
s := newISetup(t)
iCreateWebhook(
t, s.MainDB, s.WebhookID, "unknown-type-sweep",
)
unknown := database.TargetType("not-a-target-type")
deliveryID := iSeedRetryingWithType(t, s, unknown)
s.Engine.ExportSweepWebhookRetries(
context.Background(), s.WebhookID,
)
iAssertTerminallyFailed(t, s, deliveryID, unknown)
}
// iSeedFailedResult creates a failed delivery result.

View File

@@ -188,6 +188,13 @@ func (e *Engine) ExportRecoverInFlight(
e.recoverInFlight(ctx)
}
// ExportSweepWebhookRetries exposes sweepWebhookRetries.
func (e *Engine) ExportSweepWebhookRetries(
ctx context.Context, webhookID string,
) {
e.sweepWebhookRetries(ctx, webhookID)
}
// ExportStart exposes start for testing.
func (e *Engine) ExportStart(ctx context.Context) {
e.start(ctx)

View File

@@ -186,10 +186,6 @@ 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",
@@ -203,26 +199,6 @@ 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)
})
}

View File

@@ -8,7 +8,6 @@ import (
"net/http/httptest"
"os"
"testing"
"time"
"github.com/gorilla/sessions"
"github.com/stretchr/testify/assert"
@@ -29,22 +28,6 @@ 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},
@@ -52,7 +35,6 @@ func testMiddlewareWithSessionClock(
cfg := &config.Config{
Environment: env,
SessionIdleTimeout: idleTimeout,
}
// Create a real session manager with a known key
@@ -71,40 +53,11 @@ func testMiddlewareWithSessionClock(
SameSite: http.SameSiteLaxMode,
}
var now func() time.Time
if clock != nil {
now = clock.Now
}
sessManager := session.NewForTest(store, cfg, log, key, now)
sessManager := session.NewForTest(store, cfg, log, key)
m := middleware.NewForTest(log, cfg, 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,
),
}
return m, sessManager
}
// --- Logging Middleware Tests ---
@@ -434,181 +387,6 @@ 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) {
@@ -701,7 +479,7 @@ func metricsAuthMiddleware(
store := sessions.NewCookieStore(key)
store.Options = &sessions.Options{Path: "/", MaxAge: 86400}
sessManager := session.NewForTest(store, cfg, log, key, nil)
sessManager := session.NewForTest(store, cfg, log, key)
return middleware.NewForTest(log, cfg, sessManager)
}

View File

@@ -10,7 +10,6 @@ import (
"log/slog"
"maps"
"net/http"
"time"
"github.com/gorilla/sessions"
"go.uber.org/fx"
@@ -33,18 +32,6 @@ 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
@@ -54,19 +41,6 @@ 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
@@ -88,16 +62,6 @@ 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
@@ -111,8 +75,6 @@ func New(
s := &Session{
log: params.Logger.Get(),
config: params.Config,
idleTimeout: params.Config.SessionIdleTimeout,
now: time.Now,
}
lc.Append(fx.Hook{
@@ -187,98 +149,29 @@ func (s *Session) Save(
return sess.Save(r, w)
}
// SetUser sets the user information in the session. It starts both
// expiry clocks: CreatedAtKey (absolute, never refreshed again) and
// LastSeenKey (idle, refreshed by Touch).
// SetUser sets the user information in the session.
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, including
// both expiry timestamps.
// ClearUser removes user information from the session.
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)
}
// 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.
// IsAuthenticated checks if the session has an authenticated
// user.
func (s *Session) IsAuthenticated(sess *sessions.Session) bool {
auth, ok := sess.Values[AuthenticatedKey].(bool)
if !ok || !auth {
return false
}
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
return ok && auth
}
// GetUserID retrieves the user ID from the session.
@@ -360,41 +253,3 @@ 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))
}

View File

@@ -7,7 +7,6 @@ import (
"net/http/httptest"
"os"
"testing"
"time"
"github.com/gorilla/sessions"
"github.com/stretchr/testify/assert"
@@ -18,47 +17,11 @@ import (
const testKeySize = 32
// 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.
// testSession creates a Session with a real cookie store for
// testing.
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 {
@@ -76,7 +39,6 @@ func testSessionWithClock(
cfg := &config.Config{
Environment: config.EnvironmentDev,
SessionIdleTimeout: idleTimeout,
}
log := slog.New(slog.NewTextHandler(
@@ -84,46 +46,7 @@ func testSessionWithClock(
&slog.HandlerOptions{Level: slog.LevelDebug},
))
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
return session.NewForTest(store, cfg, log, key)
}
// --- Get and Save Tests ---
@@ -507,263 +430,6 @@ 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 ---

View File

@@ -2,7 +2,6 @@ package session
import (
"log/slog"
"time"
"github.com/gorilla/sessions"
"sneak.berlin/go/webhooker/internal/config"
@@ -13,28 +12,16 @@ 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,
idleTimeout: cfg.SessionIdleTimeout,
now: now,
}
}