1 Commits

Author SHA1 Message Date
clawbot
855b9de866 Allow retention_days of 0 to mean retain forever (closes #79)
All checks were successful
check / check (push) Successful in 3m4s
RetentionDays carried gorm:"default:30", so GORM substituted 30 for a
zero value while building the insert. A webhook could therefore never
be configured to keep its events indefinitely: the reaper's
retain-forever branch existed but was unreachable from the normal
create and edit flows.

Introduce database.RetentionForeverDays = 365 * 1000 as the sentinel
for "retain forever" and a Webhook.BeforeSave hook that rewrites any
non-positive RetentionDays to it. The rewrite has to live in the hook
rather than at the call sites: GORM applies the column default while
converting the model to insert values, which happens after BeforeSave,
so anything later loses that race. Putting it on the model also means
a future call site, such as the planned REST API, cannot bypass it.

The reaper now skips a webhook when Webhook.RetainsForever reports
true, which recognises the sentinel and keeps honouring the old <= 0
values for rows written before it existed. Without this the sentinel,
being positive, would have produced a cutoff a thousand years in the
past and a DELETE matching nothing on every sweep.

Form handling is shared by create and edit through parseRetentionDays
so the two cannot drift: an empty field keeps the previous behaviour
(default on create, unchanged on edit), 0 is honoured, and an
unparseable or negative value is a 400 that re-renders the form rather
than a silently substituted default.

The retention inputs drop max="365". That cap was not cosmetic: the
edit form pre-fills the stored value, so a retain-forever webhook
rendered 365000 into an input capped at 365 and browser validation
would have blocked saving any edit to it. min becomes 0 with a hint
explaining what 0 does, and the list and detail views render a
RetentionLabel of "forever" instead of a raw day count.

The 30-day default is consolidated into database.DefaultRetentionDays,
referenced from the handler and from the create form's pre-filled
value, with a test asserting it agrees with the struct tag that cannot
reference it.
2026-08-09 02:32:34 +00:00
21 changed files with 954 additions and 933 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
@@ -328,13 +307,20 @@ event routing.
| `user_id` | UUID | Foreign key → User |
| `name` | string | Human-readable name |
| `description` | string | Optional description |
| `retention_days` | integer | Days to retain events (default: 30) |
| `retention_days` | integer | Days to retain events (default: 30; 0 means retain forever) |
**Relations:** Belongs to User. Has many Entrypoints. Has many Targets.
The `retention_days` field controls how long event data is kept in the
webhook's dedicated database before automatic cleanup.
Setting `retention_days` to `0` means "retain events forever". Because
the column carries a default of 30, a literal zero cannot survive an
insert, so a zero is rewritten on save to a sentinel of `365 * 1000`
days (`database.RetentionForeverDays`). The retention reaper recognises
that sentinel and skips the webhook entirely, and the web UI displays
such a webhook's retention as "forever" rather than as a day count.
#### Entrypoint
A receiver URL where external services POST webhook events. Each
@@ -530,7 +516,7 @@ This separation provides:
DB; the event database file is hard-deleted (permanently removed).
- **Per-webhook retention** — the `retention_days` field on each webhook
controls automatic cleanup of old events in that webhook's database
only.
only, or disables cleanup entirely when set to `0` (retain forever).
- **Performance** — each webhook's database has its own WAL, its own
page cache, and its own lock, so concurrent event ingestion across
webhooks won't contend.

12
TODO.md
View File

@@ -28,10 +28,12 @@ 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 Make retain-forever reachable from the normal create and
edit flows (#79): a `RetentionForeverDays = 365 * 1000` sentinel, a
`Webhook.BeforeSave` hook rewriting any non-positive `retention_days`
to it ahead of GORM's own column defaulting, a reaper that skips such
webhooks outright, form validation that honours `0` and rejects
garbage with a 400, and a retention UI that says "forever"
- 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 +77,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

@@ -18,6 +18,11 @@ const (
testVersion = "test"
// testContentType is the event content type used in tests.
testContentType = "application/json"
// testWebhookName is the Webhook.Name used in tests.
testWebhookName = "test-webhook"
// testForeverLabel is Webhook.RetentionLabel for a retain-forever
// webhook.
testForeverLabel = "forever"
)
func setupTestDB(

View File

@@ -1,6 +1,39 @@
package database
import (
"strconv"
"gorm.io/gorm"
)
const (
// DefaultRetentionDays is the event retention period applied to a
// webhook created without an explicit retention value. It is the
// single source of truth for that policy and must stay in sync
// with the `gorm:"default:30"` column default on
// Webhook.RetentionDays below; a struct tag cannot reference a
// constant, so a test asserts the two agree.
DefaultRetentionDays = 30
// RetentionForeverDays is the sentinel RetentionDays value meaning
// "retain events forever". Users express that intent as 0, which
// Webhook.BeforeSave rewrites to this value: the column default
// substitutes DefaultRetentionDays for a zero value at insert
// time, so a zero can never survive a round trip to the database.
// Nothing outside this file may hardcode the number.
RetentionForeverDays = 365 * 1000
)
// Webhook represents a webhook processing unit that groups entrypoints and targets
//
// The receiver kinds below are deliberately mixed. BeforeSave has to
// take a pointer because it mutates the record, and GORM only invokes
// hooks declared that way. RetainsForever and RetentionLabel have to
// take values because html/template calls them on webhooks held in a
// template data map, which reflection cannot address; a pointer
// receiver there fails at render time rather than at compile time.
//
//nolint:recvcheck // GORM needs a pointer hook; templates need values.
type Webhook struct {
BaseModel
@@ -8,7 +41,9 @@ type Webhook struct {
Name string `gorm:"not null" json:"name"`
Description string `json:"description"`
// RetentionDays is the number of days to retain events.
// RetentionDays is the number of days to retain events. A value of
// RetentionForeverDays means retain forever. The column default
// must equal DefaultRetentionDays.
RetentionDays int `gorm:"default:30" json:"retentionDays"`
// Relations
@@ -16,3 +51,44 @@ type Webhook struct {
Entrypoints []Entrypoint `json:"entrypoints,omitempty"`
Targets []Target `json:"targets,omitempty"`
}
// BeforeSave normalises RetentionDays on every insert and update. A
// non-positive value is the user's way of asking for "retain forever",
// which is stored as the RetentionForeverDays sentinel.
//
// This has to happen in a hook rather than at the call sites. GORM
// substitutes the column default (DefaultRetentionDays) for a zero
// value while building the insert statement, which runs after
// BeforeSave; rewriting any later than this loses that race and the
// row lands at 30 days. Living on the model also means a future call
// site — a REST API, a fixture, a migration — cannot bypass it.
func (w *Webhook) BeforeSave(_ *gorm.DB) error {
if w.RetentionDays <= 0 {
w.RetentionDays = RetentionForeverDays
}
return nil
}
// RetainsForever reports whether this webhook's events are kept
// indefinitely. It accepts the RetentionForeverDays sentinel written by
// BeforeSave and, defensively, the non-positive values that rows
// written before the sentinel existed may still carry.
func (w Webhook) RetainsForever() bool {
return w.RetentionDays <= 0 ||
w.RetentionDays >= RetentionForeverDays
}
// RetentionLabel returns the webhook's retention policy as display
// text, so that no template has to know about the sentinel value.
func (w Webhook) RetentionLabel() string {
if w.RetainsForever() {
return "forever"
}
if w.RetentionDays == 1 {
return "1 day"
}
return strconv.Itoa(w.RetentionDays) + " days"
}

View File

@@ -0,0 +1,184 @@
package database_test
import (
"context"
"reflect"
"strconv"
"testing"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gorm.io/gorm"
"gorm.io/gorm/clause"
"sneak.berlin/go/webhooker/internal/database"
)
// startedTestDB returns a started main database for model-level tests.
func startedTestDB(t *testing.T) *gorm.DB {
t.Helper()
db, lc := setupTestDB(t)
ctx := context.Background()
require.NoError(t, lc.Start(ctx))
t.Cleanup(func() { require.NoError(t, lc.Stop(ctx)) })
return db.DB()
}
// storedRetention reads the retention_days column straight out of the
// row, so the assertion is about what was persisted rather than about
// whatever the in-memory struct happens to hold.
func storedRetention(t *testing.T, db *gorm.DB, id string) int {
t.Helper()
var got int
require.NoError(
t,
db.Model(&database.Webhook{}).
Where("id = ?", id).
Pluck("retention_days", &got).Error,
)
return got
}
// newWebhookWithRetention creates a webhook through the ordinary Create
// path, so the BeforeSave hook and the GORM column default both apply
// exactly as they do in production.
func newWebhookWithRetention(
t *testing.T,
db *gorm.DB,
wh *database.Webhook,
) string {
t.Helper()
wh.UserID = uuid.New().String()
wh.Name = testWebhookName
require.NoError(
t,
db.Omit(clause.Associations).Create(wh).Error,
)
return wh.ID
}
func TestWebhookBeforeSave_ZeroBecomesForeverSentinel(t *testing.T) {
t.Parallel()
db := startedTestDB(t)
wh := &database.Webhook{RetentionDays: 0}
id := newWebhookWithRetention(t, db, wh)
assert.Equal(
t,
database.RetentionForeverDays,
storedRetention(t, db, id),
"a zero retention must be stored as the sentinel, "+
"not replaced by the column default",
)
}
func TestWebhookBeforeSave_NegativeBecomesForeverSentinel(t *testing.T) {
t.Parallel()
db := startedTestDB(t)
wh := &database.Webhook{RetentionDays: -5}
id := newWebhookWithRetention(t, db, wh)
assert.Equal(
t,
database.RetentionForeverDays,
storedRetention(t, db, id),
)
}
func TestWebhookBeforeSave_PositiveIsPreserved(t *testing.T) {
t.Parallel()
db := startedTestDB(t)
wh := &database.Webhook{RetentionDays: 7}
id := newWebhookWithRetention(t, db, wh)
assert.Equal(t, 7, storedRetention(t, db, id))
}
// TestWebhookBeforeSave_UpdateToZeroBecomesSentinel proves the hook
// fires on update as well as insert, via the same Save call the edit
// handler makes.
func TestWebhookBeforeSave_UpdateToZeroBecomesSentinel(t *testing.T) {
t.Parallel()
db := startedTestDB(t)
wh := &database.Webhook{RetentionDays: 30}
id := newWebhookWithRetention(t, db, wh)
require.Equal(t, 30, storedRetention(t, db, id))
wh.RetentionDays = 0
require.NoError(t, db.Omit(clause.Associations).Save(wh).Error)
assert.Equal(
t,
database.RetentionForeverDays,
storedRetention(t, db, id),
)
}
// TestWebhookRetentionColumnDefaultMatchesConstant guards the one place
// the default lives twice: a struct tag cannot reference a constant, so
// this asserts the tag and DefaultRetentionDays agree.
func TestWebhookRetentionColumnDefaultMatchesConstant(t *testing.T) {
t.Parallel()
field, ok := reflect.TypeFor[database.Webhook]().
FieldByName("RetentionDays")
require.True(t, ok, "Webhook.RetentionDays must exist")
assert.Equal(
t,
"default:"+strconv.Itoa(database.DefaultRetentionDays),
field.Tag.Get("gorm"),
)
}
func TestWebhookRetainsForeverAndLabel(t *testing.T) {
t.Parallel()
cases := []struct {
name string
days int
forever bool
label string
}{
{
"sentinel",
database.RetentionForeverDays, true, testForeverLabel,
},
{
"above sentinel",
database.RetentionForeverDays + 1, true, testForeverLabel,
},
{"legacy zero", 0, true, testForeverLabel},
{"legacy negative", -1, true, testForeverLabel},
{"default", database.DefaultRetentionDays, false, "30 days"},
{"one day", 1, false, "1 day"},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
wh := database.Webhook{RetentionDays: tc.days}
assert.Equal(t, tc.forever, wh.RetainsForever())
assert.Equal(t, tc.label, wh.RetentionLabel())
})
}
}

View File

@@ -114,7 +114,8 @@ func (r *RetentionReaper) run(ctx context.Context) {
}
// sweep lists every webhook from the main database and reaps expired
// rows from each per-webhook database whose RetentionDays is positive.
// rows from each per-webhook database that has a finite retention
// policy. Webhooks set to retain forever are skipped entirely.
func (r *RetentionReaper) sweep(ctx context.Context) {
var webhooks []Webhook
@@ -139,8 +140,13 @@ func (r *RetentionReaper) sweep(ctx context.Context) {
wh := webhooks[i]
// RetentionDays of zero or less means retain forever.
if wh.RetentionDays <= 0 {
// Skip retain-forever webhooks before building any query.
// RetainsForever covers both the RetentionForeverDays
// sentinel and the non-positive values that predate it: the
// sentinel is a positive number, so without this the reaper
// would compute a cutoff a thousand years in the past and
// issue a DELETE matching nothing on every single sweep.
if wh.RetainsForever() {
continue
}

View File

@@ -77,7 +77,7 @@ func createWebhook(
wh := &database.Webhook{
UserID: uuid.New().String(),
Name: "test-webhook",
Name: testWebhookName,
RetentionDays: retentionDays,
}
require.NoError(
@@ -85,10 +85,11 @@ func createWebhook(
db.Omit(clause.Associations).Create(wh).Error,
)
// The RetentionDays column carries a GORM default of 30, so a
// zero (or negative) value passed to Create is replaced by that
// default. Force the requested value explicitly so the
// retain-forever (<= 0) path can be exercised.
// Webhook.BeforeSave rewrites a non-positive RetentionDays to the
// retain-forever sentinel, and the column's GORM default would
// otherwise substitute 30. Force the requested value with a
// column-level update so tests can plant legacy rows that predate
// the sentinel and still carry a literal 0 or negative value.
require.NoError(
t,
db.Model(wh).
@@ -98,6 +99,30 @@ func createWebhook(
return wh.ID
}
// createWebhookNormally inserts a webhook through the ordinary Create
// path, with no column-level forcing, so Webhook.BeforeSave applies
// exactly as it does in production. Passing 0 therefore yields a row
// holding the RetentionForeverDays sentinel.
func createWebhookNormally(
t *testing.T,
db *gorm.DB,
retentionDays int,
) string {
t.Helper()
wh := &database.Webhook{
UserID: uuid.New().String(),
Name: testWebhookName,
RetentionDays: retentionDays,
}
require.NoError(
t,
db.Omit(clause.Associations).Create(wh).Error,
)
return wh.ID
}
// eventChain is the set of row IDs seeded for a single event.
type eventChain struct {
eventID string
@@ -256,12 +281,59 @@ func TestRetentionReaper_ReapsExpiredKeepsRecent(t *testing.T) {
assertChainPresent(t, db, recent)
}
// TestRetentionReaper_SkipsSentinelReapsFiniteInSameSweep covers the
// end-to-end retain-forever path: a webhook created the normal way with
// a requested retention of 0 lands on the RetentionForeverDays
// sentinel, and the reaper leaves its ancient events alone while still
// reaping a finite-retention webhook in the very same sweep.
func TestRetentionReaper_SkipsSentinelReapsFiniteInSameSweep(
t *testing.T,
) {
t.Parallel()
env := setupRetentionTest(t)
foreverID := createWebhookNormally(t, env.mainDB.DB(), 0)
var stored database.Webhook
require.NoError(
t,
env.mainDB.DB().Where("id = ?", foreverID).
First(&stored).Error,
)
require.Equal(
t,
database.RetentionForeverDays,
stored.RetentionDays,
"a requested retention of 0 must persist as the sentinel",
)
finiteID := createWebhookNormally(t, env.mainDB.DB(), 30)
foreverDB, err := env.mgr.GetDB(foreverID)
require.NoError(t, err)
finiteDB, err := env.mgr.GetDB(finiteID)
require.NoError(t, err)
ancient := time.Now().Add(-365 * 24 * time.Hour)
kept := seedEventChain(t, foreverDB, foreverID, ancient)
doomed := seedEventChain(t, finiteDB, finiteID, ancient)
env.reaper.ExportSweep(context.Background())
assertChainPresent(t, foreverDB, kept)
assertChainGone(t, finiteDB, doomed)
}
func TestRetentionReaper_RetainsForeverWhenNonPositive(t *testing.T) {
t.Parallel()
env := setupRetentionTest(t)
// RetentionDays of zero means retain forever.
// A legacy row written before the sentinel existed still carries a
// literal 0; the <= 0 guard must keep honouring it.
webhookID := createWebhook(t, env.mainDB.DB(), 0)
db, err := env.mgr.GetDB(webhookID)

View File

@@ -26,8 +26,6 @@ const (
maxBodyShift = 20
// recentEventLimit is the number of recent events to show.
recentEventLimit = 20
// defaultRetentionDays is the default event retention period.
defaultRetentionDays = 30
// paginationPerPage is the number of items per page.
paginationPerPage = 25

View File

@@ -25,6 +25,37 @@ type WebhookListItem struct {
// errMissingURL signals that a required URL was not provided.
var errMissingURL = errors.New("missing URL")
// errInvalidRetention signals a retention_days form value that is not
// a non-negative whole number.
var errInvalidRetention = errors.New("invalid retention days")
// retentionErrorMessage is what the create and edit forms show the user
// when parseRetentionDays returns errInvalidRetention.
const retentionErrorMessage = "Retention must be a whole number of " +
"days, or 0 to retain events forever."
// parseRetentionDays interprets a retention_days form value.
//
// An empty value yields fallback, which lets the create path apply the
// default and the edit path leave the stored value unchanged. A value
// of 0 is returned as 0 and is rewritten to the retain-forever
// sentinel by database.Webhook's BeforeSave hook, so no handler needs
// to know the sentinel. Anything unparseable or negative is an error
// rather than a silently substituted default.
func parseRetentionDays(raw string, fallback int) (int, error) {
raw = strings.TrimSpace(raw)
if raw == "" {
return fallback, nil
}
v, err := strconv.Atoi(raw)
if err != nil || v < 0 {
return 0, errInvalidRetention
}
return v, nil
}
// EventWithDeliveries holds an event and its deliveries.
type EventWithDeliveries struct {
database.Event
@@ -106,11 +137,20 @@ func (h *Handlers) buildWebhookListItems(
// HandleSourceCreate shows the form to create a new webhook.
func (h *Handlers) HandleSourceCreate() http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
data := map[string]any{
tmplKeyError: "",
h.renderTemplate(
w, r, "sources_new.html", newSourceFormData(""),
)
}
}
h.renderTemplate(w, r, "sources_new.html", data)
// newSourceFormData builds the template data for the webhook creation
// form, carrying the retention default so the pre-filled value comes
// from database.DefaultRetentionDays rather than being a third
// hardcoded copy of the same policy.
func newSourceFormData(errMsg string) map[string]any {
return map[string]any{
tmplKeyError: errMsg,
"DefaultRetentionDays": database.DefaultRetentionDays,
}
}
@@ -145,23 +185,26 @@ func (h *Handlers) HandleSourceCreateSubmit() http.HandlerFunc {
retentionStr := r.FormValue("retention_days")
if name == "" {
data := map[string]any{
tmplKeyError: "Name is required",
}
w.WriteHeader(http.StatusBadRequest)
h.renderTemplate(w, r, "sources_new.html", data)
h.renderTemplate(
w, r, "sources_new.html",
newSourceFormData("Name is required"),
)
return
}
retentionDays := defaultRetentionDays
retentionDays, retErr := parseRetentionDays(
retentionStr, database.DefaultRetentionDays,
)
if retErr != nil {
w.WriteHeader(http.StatusBadRequest)
h.renderTemplate(
w, r, "sources_new.html",
newSourceFormData(retentionErrorMessage),
)
if retentionStr != "" {
v, convErr := strconv.Atoi(retentionStr)
if convErr == nil && v > 0 {
retentionDays = v
}
return
}
h.createWebhookWithEntrypoint(
@@ -428,7 +471,25 @@ func (h *Handlers) applyWebhookEdit(
webhook.Name = name
webhook.Description = r.FormValue("description")
h.parseRetention(r, webhook)
// An empty field falls back to the stored value, so submitting the
// form without touching retention leaves the policy alone.
retentionDays, retErr := parseRetentionDays(
r.FormValue("retention_days"), webhook.RetentionDays,
)
if retErr != nil {
data := map[string]any{
tmplKeyWebhook: *webhook,
tmplKeyError: retentionErrorMessage,
}
w.WriteHeader(http.StatusBadRequest)
h.renderTemplate(w, r, "source_edit.html", data)
return
}
webhook.RetentionDays = retentionDays
err := h.db.DB().Save(webhook).Error
if err != nil {
@@ -442,23 +503,6 @@ func (h *Handlers) applyWebhookEdit(
)
}
// parseRetention parses and applies retention_days from the
// form.
func (h *Handlers) parseRetention(
r *http.Request,
webhook *database.Webhook,
) {
retStr := r.FormValue("retention_days")
if retStr == "" {
return
}
v, err := strconv.Atoi(retStr)
if err == nil && v > 0 {
webhook.RetentionDays = v
}
}
// HandleSourceDelete handles webhook deletion.
func (h *Handlers) HandleSourceDelete() http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {

View File

@@ -0,0 +1,481 @@
package handlers_test
import (
"context"
"net/http"
"net/http/httptest"
"net/url"
"strconv"
"strings"
"testing"
"github.com/go-chi/chi"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gorm.io/gorm/clause"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/handlers"
"sneak.berlin/go/webhooker/internal/session"
)
const (
// sourceTestUserID is the session user id used by the webhook
// management tests.
sourceTestUserID = "source-test-user"
// sourceIDParam is the chi URL parameter naming a webhook.
sourceIDParam = "sourceID"
)
// formRequest builds an urlencoded POST to path carrying the given
// cookies, plus any chi URL parameters the handler reads.
func formRequest(
path string,
cookies []*http.Cookie,
form url.Values,
urlParams map[string]string,
) *http.Request {
req := httptest.NewRequestWithContext(
context.Background(),
http.MethodPost,
path,
strings.NewReader(form.Encode()),
)
req.Header.Set(
"Content-Type", "application/x-www-form-urlencoded",
)
for _, c := range cookies {
req.AddCookie(c)
}
rctx := chi.NewRouteContext()
for k, v := range urlParams {
rctx.URLParams.Add(k, v)
}
return req.WithContext(
context.WithValue(req.Context(), chi.RouteCtxKey, rctx),
)
}
// getRequest builds a GET to path carrying the given cookies, plus any
// chi URL parameters the handler reads.
func getRequest(
t *testing.T,
path string,
cookies []*http.Cookie,
urlParams map[string]string,
) *http.Request {
t.Helper()
req := httptest.NewRequestWithContext(
context.Background(), http.MethodGet, path, nil,
)
for _, c := range cookies {
req.AddCookie(c)
}
rctx := chi.NewRouteContext()
for k, v := range urlParams {
rctx.URLParams.Add(k, v)
}
return req.WithContext(
context.WithValue(req.Context(), chi.RouteCtxKey, rctx),
)
}
// submitCreate posts the webhook creation form with the given
// retention_days value (omitted entirely when retention is nil) and
// returns the recorder.
func submitCreate(
t *testing.T,
h *handlers.Handlers,
cookies []*http.Cookie,
name string,
retention *string,
) *httptest.ResponseRecorder {
t.Helper()
form := url.Values{}
form.Set("name", name)
if retention != nil {
form.Set("retention_days", *retention)
}
req := formRequest("/sources/new", cookies, form, nil)
w := httptest.NewRecorder()
h.HandleSourceCreateSubmit().ServeHTTP(w, req)
return w
}
// onlyWebhook loads the single webhook belonging to the test user.
func onlyWebhook(
t *testing.T,
db *database.Database,
) database.Webhook {
t.Helper()
var webhooks []database.Webhook
require.NoError(
t,
db.DB().Where("user_id = ?", sourceTestUserID).
Find(&webhooks).Error,
)
require.Len(t, webhooks, 1)
return webhooks[0]
}
// seedWebhook inserts a webhook owned by the test user with an exact
// stored retention value, bypassing Webhook.BeforeSave via a
// column-level update so that legacy rows can be planted too.
func seedWebhook(
t *testing.T,
db *database.Database,
retentionDays int,
) database.Webhook {
t.Helper()
wh := &database.Webhook{
UserID: sourceTestUserID,
Name: "seeded",
RetentionDays: retentionDays,
}
require.NoError(
t,
db.DB().Omit(clause.Associations).Create(wh).Error,
)
require.NoError(
t,
db.DB().Model(wh).
Update("retention_days", retentionDays).Error,
)
wh.RetentionDays = retentionDays
return *wh
}
// storedRetentionDays reads the retention_days column for a webhook.
func storedRetentionDays(
t *testing.T,
db *database.Database,
id string,
) int {
t.Helper()
var got int
require.NoError(
t,
db.DB().Model(&database.Webhook{}).
Where("id = ?", id).
Pluck("retention_days", &got).Error,
)
return got
}
// sourceTestEnv bundles the handler, session, and database a webhook
// management test drives.
type sourceTestEnv struct {
handlers *handlers.Handlers
db *database.Database
cookies []*http.Cookie
}
func setupSourceTest(t *testing.T) *sourceTestEnv {
t.Helper()
var h *handlers.Handlers
var sess *session.Session
var db *database.Database
app := newTestApp(t, &h, &sess, &db)
app.RequireStart()
t.Cleanup(app.RequireStop)
return &sourceTestEnv{
handlers: h,
db: db,
cookies: authenticatedCookies(
t, sess, sourceTestUserID, "sourceuser",
),
}
}
// TestHandleSourceCreateSubmit_ZeroRetentionPersistsForever is the core
// regression test for the bug: the create form's 0 must reach the
// database as the retain-forever sentinel rather than being replaced by
// the column's default of 30.
func TestHandleSourceCreateSubmit_ZeroRetentionPersistsForever(
t *testing.T,
) {
t.Parallel()
env := setupSourceTest(t)
zero := "0"
w := submitCreate(t, env.handlers, env.cookies, "forever", &zero)
require.Equal(t, http.StatusSeeOther, w.Code)
wh := onlyWebhook(t, env.db)
assert.Equal(
t,
database.RetentionForeverDays,
storedRetentionDays(t, env.db, wh.ID),
)
assert.True(t, wh.RetainsForever())
}
func TestHandleSourceCreateSubmit_OmittedRetentionUsesDefault(
t *testing.T,
) {
t.Parallel()
env := setupSourceTest(t)
w := submitCreate(t, env.handlers, env.cookies, "defaulted", nil)
require.Equal(t, http.StatusSeeOther, w.Code)
wh := onlyWebhook(t, env.db)
assert.Equal(
t,
database.DefaultRetentionDays,
storedRetentionDays(t, env.db, wh.ID),
)
}
// TestHandleSourceCreate_PrefillsDefaultFromConstant keeps the create
// form's pre-filled retention from becoming a third hardcoded copy of
// the 30-day policy.
func TestHandleSourceCreate_PrefillsDefaultFromConstant(t *testing.T) {
t.Parallel()
env := setupSourceTest(t)
w := httptest.NewRecorder()
env.handlers.HandleSourceCreate().ServeHTTP(
w, getRequest(t, "/sources/new", env.cookies, nil),
)
require.Equal(t, http.StatusOK, w.Code)
body := w.Body.String()
assert.Contains(
t, body,
`value="`+strconv.Itoa(database.DefaultRetentionDays)+`"`,
)
assert.NotContains(
t, body, `max="365"`,
"a max below the sentinel would block retain-forever",
)
assert.Contains(t, body, `min="0"`)
}
func TestHandleSourceCreateSubmit_InvalidRetentionIsRejected(
t *testing.T,
) {
t.Parallel()
for _, raw := range []string{"abc", "-1", "3.5"} {
t.Run(raw, func(t *testing.T) {
t.Parallel()
env := setupSourceTest(t)
w := submitCreate(
t, env.handlers, env.cookies, "bad", &raw,
)
assert.Equal(t, http.StatusBadRequest, w.Code)
assert.Contains(
t, w.Body.String(), "Retention must be",
)
var count int64
require.NoError(
t,
env.db.DB().Model(&database.Webhook{}).
Where("user_id = ?", sourceTestUserID).
Count(&count).Error,
)
assert.Zero(
t, count,
"no webhook may be created from a rejected form",
)
})
}
}
// submitEdit posts the webhook edit form for the given webhook.
func submitEdit(
t *testing.T,
env *sourceTestEnv,
wh database.Webhook,
retention string,
) *httptest.ResponseRecorder {
t.Helper()
form := url.Values{}
form.Set("name", wh.Name)
form.Set("description", wh.Description)
form.Set("retention_days", retention)
req := formRequest(
"/source/"+wh.ID+"/edit",
env.cookies,
form,
map[string]string{sourceIDParam: wh.ID},
)
w := httptest.NewRecorder()
env.handlers.HandleSourceEditSubmit().ServeHTTP(w, req)
return w
}
func TestHandleSourceEditSubmit_ZeroRetentionPersistsForever(
t *testing.T,
) {
t.Parallel()
env := setupSourceTest(t)
wh := seedWebhook(t, env.db, database.DefaultRetentionDays)
w := submitEdit(t, env, wh, "0")
require.Equal(t, http.StatusSeeOther, w.Code)
assert.Equal(
t,
database.RetentionForeverDays,
storedRetentionDays(t, env.db, wh.ID),
)
}
func TestHandleSourceEditSubmit_InvalidRetentionIsRejected(
t *testing.T,
) {
t.Parallel()
env := setupSourceTest(t)
wh := seedWebhook(t, env.db, database.DefaultRetentionDays)
w := submitEdit(t, env, wh, "not-a-number")
assert.Equal(t, http.StatusBadRequest, w.Code)
assert.Contains(t, w.Body.String(), "Retention must be")
assert.Equal(
t,
database.DefaultRetentionDays,
storedRetentionDays(t, env.db, wh.ID),
"a rejected form must not change the stored retention",
)
}
func TestHandleSourceEditSubmit_EmptyRetentionLeavesValueUnchanged(
t *testing.T,
) {
t.Parallel()
env := setupSourceTest(t)
wh := seedWebhook(t, env.db, 7)
w := submitEdit(t, env, wh, "")
require.Equal(t, http.StatusSeeOther, w.Code)
assert.Equal(t, 7, storedRetentionDays(t, env.db, wh.ID))
}
// TestSourceEditForm_ForeverWebhookRoundTrips walks the exact path that
// the removed max="365" cap used to break: render the edit form for a
// retain-forever webhook, confirm the pre-filled sentinel is not capped
// by browser validation, then submit that pre-filled value straight
// back and confirm the retention policy survives untouched.
func TestSourceEditForm_ForeverWebhookRoundTrips(t *testing.T) {
t.Parallel()
env := setupSourceTest(t)
wh := seedWebhook(t, env.db, database.RetentionForeverDays)
req := getRequest(
t, "/source/"+wh.ID+"/edit", env.cookies,
map[string]string{sourceIDParam: wh.ID},
)
w := httptest.NewRecorder()
env.handlers.HandleSourceEdit().ServeHTTP(w, req)
require.Equal(t, http.StatusOK, w.Code)
sentinel := strconv.Itoa(database.RetentionForeverDays)
body := w.Body.String()
assert.Contains(
t, body, `value="`+sentinel+`"`,
"the edit form pre-fills the stored retention",
)
assert.NotContains(
t, body, `max="365"`,
"a max below the sentinel would block saving any edit",
)
assert.Contains(
t, body, "forever",
"the form explains what the sentinel means",
)
// Submit the pre-filled value back, exactly as a browser would.
post := submitEdit(t, env, wh, sentinel)
require.Equal(t, http.StatusSeeOther, post.Code)
assert.Equal(
t,
database.RetentionForeverDays,
storedRetentionDays(t, env.db, wh.ID),
)
}
// TestSourceListAndDetail_ShowForeverNotTheSentinelNumber checks that
// the retain-forever value is never rendered to the user as a raw day
// count on either read-only view.
func TestSourceListAndDetail_ShowForeverNotTheSentinelNumber(
t *testing.T,
) {
t.Parallel()
env := setupSourceTest(t)
wh := seedWebhook(t, env.db, database.RetentionForeverDays)
sentinel := strconv.Itoa(database.RetentionForeverDays)
listW := httptest.NewRecorder()
env.handlers.HandleSourceList().ServeHTTP(
listW, getRequest(t, "/sources", env.cookies, nil),
)
require.Equal(t, http.StatusOK, listW.Code)
assert.Contains(t, listW.Body.String(), "Retention: forever")
assert.NotContains(t, listW.Body.String(), sentinel)
detailW := httptest.NewRecorder()
env.handlers.HandleSourceDetail().ServeHTTP(
detailW,
getRequest(
t, "/source/"+wh.ID, env.cookies,
map[string]string{sourceIDParam: wh.ID},
),
)
require.Equal(t, http.StatusOK, detailW.Code)
assert.Contains(t, detailW.Body.String(), "Retention: forever")
assert.NotContains(t, detailW.Body.String(), sentinel)
}

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,
}
}

View File

@@ -181,7 +181,7 @@
<!-- Info -->
<div class="mt-4 text-sm text-gray-400">
<p>Retention: {{.Webhook.RetentionDays}} days &middot; Created: {{.Webhook.CreatedAt.Format "2006-01-02 15:04:05 UTC"}}</p>
<p>Retention: {{.Webhook.RetentionLabel}} &middot; Created: {{.Webhook.CreatedAt.Format "2006-01-02 15:04:05 UTC"}}</p>
</div>
</div>
{{end}}

View File

@@ -28,7 +28,8 @@
<div class="form-group">
<label for="retention_days" class="label">Retention (days)</label>
<input type="number" id="retention_days" name="retention_days" value="{{.Webhook.RetentionDays}}" min="1" max="365" class="input">
<input type="number" id="retention_days" name="retention_days" value="{{.Webhook.RetentionDays}}" min="0" class="input">
<p class="text-xs text-gray-500 mt-1">Currently {{.Webhook.RetentionLabel}}. Enter 0 to retain events forever.</p>
</div>
<div class="flex gap-3">

View File

@@ -25,7 +25,7 @@
<p class="text-sm text-gray-500 mt-1">{{.Description}}</p>
{{end}}
</div>
<span class="badge-info">{{.RetentionDays}}d retention</span>
<span class="badge-info">Retention: {{.RetentionLabel}}</span>
</div>
<div class="flex gap-6 mt-4 text-sm text-gray-500">
<span>{{.EntrypointCount}} entrypoint{{if ne .EntrypointCount 1}}s{{end}}</span>

View File

@@ -28,8 +28,8 @@
<div class="form-group">
<label for="retention_days" class="label">Retention (days)</label>
<input type="number" id="retention_days" name="retention_days" value="30" min="1" max="365" class="input">
<p class="text-xs text-gray-500 mt-1">How long to keep event data.</p>
<input type="number" id="retention_days" name="retention_days" value="{{.DefaultRetentionDays}}" min="0" class="input">
<p class="text-xs text-gray-500 mt-1">How long to keep event data. Enter 0 to retain events forever.</p>
</div>
<div class="flex gap-3">