Compare commits
1 Commits
issue-90-b
...
issue-97-l
| Author | SHA1 | Date | |
|---|---|---|---|
| 20a050b49d |
17
README.md
17
README.md
@@ -867,17 +867,9 @@ Applied to all routes in this order:
|
|||||||
8. **Sentry** — Error reporting to Sentry (if `SENTRY_DSN` is set;
|
8. **Sentry** — Error reporting to Sentry (if `SENTRY_DSN` is set;
|
||||||
configured with `Repanic: true` so panics still reach Recoverer)
|
configured with `Repanic: true` so panics still reach Recoverer)
|
||||||
|
|
||||||
Additionally, form endpoints (`/pages`, `/user/*`, `/sources`,
|
Additionally, form endpoints (`/pages`, `/sources`, `/source/*`) apply a
|
||||||
`/source/*`) apply a **MaxBodySize** middleware that limits
|
**MaxBodySize** middleware that limits POST/PUT/PATCH request bodies to
|
||||||
POST/PUT/PATCH request bodies to 1 MB. It is registered ahead of the
|
1 MB using `http.MaxBytesReader`, preventing oversized form submissions.
|
||||||
CSRF middleware in every one of those route groups, because
|
|
||||||
gorilla/csrf parses the form; if the cap were installed after it, form
|
|
||||||
parsing would run under net/http's 10 MB default and the 1 MB limit
|
|
||||||
would never apply. A request that declares a `Content-Length` over the
|
|
||||||
limit is answered with `413 Request Entity Too Large` before any other
|
|
||||||
middleware or handler runs; a chunked request, or one that lies about
|
|
||||||
its length, is hard-capped by `http.MaxBytesReader` and fails
|
|
||||||
downstream at form-parse time.
|
|
||||||
|
|
||||||
### Authentication
|
### Authentication
|
||||||
|
|
||||||
@@ -899,8 +891,7 @@ downstream at form-parse time.
|
|||||||
- Production security headers on all responses: HSTS, X-Content-Type-Options
|
- Production security headers on all responses: HSTS, X-Content-Type-Options
|
||||||
(`nosniff`), X-Frame-Options (`DENY`), Content-Security-Policy, Referrer-Policy,
|
(`nosniff`), X-Frame-Options (`DENY`), Content-Security-Policy, Referrer-Policy,
|
||||||
and Permissions-Policy
|
and Permissions-Policy
|
||||||
- Request body size limits (1 MB) on all form POST endpoints, enforced
|
- Request body size limits (1 MB) on all form POST endpoints
|
||||||
by middleware that runs before CSRF parses the form
|
|
||||||
- **CSRF protection** via [gorilla/csrf](https://github.com/gorilla/csrf)
|
- **CSRF protection** via [gorilla/csrf](https://github.com/gorilla/csrf)
|
||||||
on all state-changing forms (cookie-based double-submit tokens with
|
on all state-changing forms (cookie-based double-submit tokens with
|
||||||
HMAC authentication). Applied to `/pages`, `/sources`, `/source`, and
|
HMAC authentication). Applied to `/pages`, `/sources`, `/source`, and
|
||||||
|
|||||||
13
TODO.md
13
TODO.md
@@ -28,13 +28,12 @@ databases currently grow without bound.
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
- 2026-08-09 Enforce the request body size limit before the CSRF
|
- 2026-08-09 Root the delivery engine's worker pool and the retention
|
||||||
middleware parses the form (#90): `MaxBodySize` is now registered
|
reaper's sweep loop at `context.Background()` rather than the fx
|
||||||
ahead of `CSRF()` in every form route group, the `/user/{username}`
|
`OnStart` hook context (#97), which carries fx's 15s start timeout and
|
||||||
group gained the cap it never had (which is where `POST /password`
|
killed both roughly fifteen seconds after boot: the proxy silently
|
||||||
lives), the middleware rejects a declared-oversize body with a real
|
stopped delivering webhooks entirely, and the reaper never ran a
|
||||||
413 up front, and the redundant handler-local
|
single sweep under its default one-hour interval
|
||||||
`http.MaxBytesReader` calls were removed
|
|
||||||
- 2026-08-07 Update golangci-lint to v2.12.2 (Docker image digest in
|
- 2026-08-07 Update golangci-lint to v2.12.2 (Docker image digest in
|
||||||
`Dockerfile`, release-archive sha256 pins in `script/bootstrap`),
|
`Dockerfile`, release-archive sha256 pins in `script/bootstrap`),
|
||||||
adopt the canonical `.golangci.yml` (v2 `linters.settings` layout so
|
adopt the canonical `.golangci.yml` (v2 `linters.settings` layout so
|
||||||
|
|||||||
@@ -5,6 +5,8 @@ import (
|
|||||||
"log/slog"
|
"log/slog"
|
||||||
"os"
|
"os"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
"go.uber.org/fx"
|
||||||
)
|
)
|
||||||
|
|
||||||
// NewTestRetentionReaper builds a RetentionReaper backed by the given
|
// NewTestRetentionReaper builds a RetentionReaper backed by the given
|
||||||
@@ -29,3 +31,26 @@ func NewTestRetentionReaper(
|
|||||||
func (r *RetentionReaper) ExportSweep(ctx context.Context) {
|
func (r *RetentionReaper) ExportSweep(ctx context.Context) {
|
||||||
r.sweep(ctx)
|
r.sweep(ctx)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ExportRegisterHooks registers the reaper's real fx lifecycle hooks
|
||||||
|
// on a lifecycle supplied by a test, so a test can drive the exact
|
||||||
|
// OnStart/OnStop functions the application runs and hand OnStart the
|
||||||
|
// kind of context fx actually supplies.
|
||||||
|
func (r *RetentionReaper) ExportRegisterHooks(lc fx.Lifecycle) {
|
||||||
|
r.registerHooks(lc)
|
||||||
|
}
|
||||||
|
|
||||||
|
// ExportStart starts the reaper's background loop for tests.
|
||||||
|
func (r *RetentionReaper) ExportStart() {
|
||||||
|
r.start()
|
||||||
|
}
|
||||||
|
|
||||||
|
// ExportStop stops the reaper's background loop for tests.
|
||||||
|
func (r *RetentionReaper) ExportStop() {
|
||||||
|
r.stop()
|
||||||
|
}
|
||||||
|
|
||||||
|
// ExportSetInterval overrides the sweep interval for tests.
|
||||||
|
func (r *RetentionReaper) ExportSetInterval(d time.Duration) {
|
||||||
|
r.interval = d
|
||||||
|
}
|
||||||
|
|||||||
@@ -56,9 +56,20 @@ func NewRetentionReaper(
|
|||||||
interval: params.Config.RetentionSweepInterval,
|
interval: params.Config.RetentionSweepInterval,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
r.registerHooks(lc)
|
||||||
|
|
||||||
|
return r
|
||||||
|
}
|
||||||
|
|
||||||
|
// registerHooks wires the reaper's start and stop into the fx
|
||||||
|
// lifecycle. The start hook's context is deliberately ignored: see
|
||||||
|
// start for why the sweep loop must not inherit it.
|
||||||
|
func (r *RetentionReaper) registerHooks(lc fx.Lifecycle) {
|
||||||
lc.Append(fx.Hook{
|
lc.Append(fx.Hook{
|
||||||
OnStart: func(ctx context.Context) error {
|
//nolint:contextcheck // Not inheriting the hook context is
|
||||||
r.start(ctx)
|
// the point: see start.
|
||||||
|
OnStart: func(_ context.Context) error {
|
||||||
|
r.start()
|
||||||
|
|
||||||
return nil
|
return nil
|
||||||
},
|
},
|
||||||
@@ -68,12 +79,20 @@ func NewRetentionReaper(
|
|||||||
return nil
|
return nil
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
|
|
||||||
return r
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func (r *RetentionReaper) start(ctx context.Context) {
|
// start launches the background sweep loop.
|
||||||
ctx, cancel := context.WithCancel(ctx)
|
//
|
||||||
|
// The loop's context is derived from context.Background(), NOT from
|
||||||
|
// the fx OnStart hook context. The hook context carries fx's start
|
||||||
|
// timeout (15s by default) and is cancelled once the start phase
|
||||||
|
// completes, so a loop derived from it dies 45 minutes before its
|
||||||
|
// first tick under the default one-hour sweep interval, leaving a
|
||||||
|
// reaper that never reaps. A long-lived goroutine must outlive the
|
||||||
|
// startup phase, so its lifetime is bounded by OnStop instead: stop
|
||||||
|
// cancels this context and waits on the WaitGroup.
|
||||||
|
func (r *RetentionReaper) start() {
|
||||||
|
ctx, cancel := context.WithCancel(context.Background())
|
||||||
r.cancel = cancel
|
r.cancel = cancel
|
||||||
|
|
||||||
r.wg.Add(1)
|
r.wg.Add(1)
|
||||||
|
|||||||
209
internal/database/retention_lifecycle_test.go
Normal file
209
internal/database/retention_lifecycle_test.go
Normal file
@@ -0,0 +1,209 @@
|
|||||||
|
package database_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
"go.uber.org/fx"
|
||||||
|
"gorm.io/gorm"
|
||||||
|
"sneak.berlin/go/webhooker/internal/database"
|
||||||
|
)
|
||||||
|
|
||||||
|
const (
|
||||||
|
// reaperTestInterval is the sweep interval a lifecycle test
|
||||||
|
// runs the reaper at, so a loop that survives startup produces
|
||||||
|
// an observable sweep quickly.
|
||||||
|
reaperTestInterval = 10 * time.Millisecond
|
||||||
|
|
||||||
|
// reaperStopTimeout bounds how long a lifecycle test waits for
|
||||||
|
// the reaper's OnStop hook to return before declaring the
|
||||||
|
// shutdown hung.
|
||||||
|
reaperStopTimeout = 10 * time.Second
|
||||||
|
|
||||||
|
// reaperTestRetentionDays is the retention policy the lifecycle
|
||||||
|
// tests give their webhook.
|
||||||
|
reaperTestRetentionDays = 30
|
||||||
|
)
|
||||||
|
|
||||||
|
// recordingLifecycle is a minimal fx.Lifecycle that records the
|
||||||
|
// hooks a component registers, so a test can invoke the real
|
||||||
|
// OnStart/OnStop functions with a context of its choosing.
|
||||||
|
type recordingLifecycle struct {
|
||||||
|
hooks []fx.Hook
|
||||||
|
}
|
||||||
|
|
||||||
|
func (l *recordingLifecycle) Append(h fx.Hook) {
|
||||||
|
l.hooks = append(l.hooks, h)
|
||||||
|
}
|
||||||
|
|
||||||
|
// startReaperViaHook drives the genuine fx hooks the application
|
||||||
|
// registers for the reaper, handing OnStart a context that is
|
||||||
|
// already done. It returns the recorded lifecycle so the caller
|
||||||
|
// can drive OnStop too.
|
||||||
|
func startReaperViaHook(
|
||||||
|
t *testing.T, r *database.RetentionReaper,
|
||||||
|
) *recordingLifecycle {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
lc := &recordingLifecycle{}
|
||||||
|
r.ExportRegisterHooks(lc)
|
||||||
|
require.Len(t, lc.hooks, 1)
|
||||||
|
|
||||||
|
// fx hands OnStart a context carrying the application start
|
||||||
|
// timeout, and cancels it when the start phase ends. An
|
||||||
|
// already-cancelled context is that same defect taken to its
|
||||||
|
// limit, and unlike a plain context.Background() it actually
|
||||||
|
// distinguishes a correctly rooted loop from a broken one.
|
||||||
|
hookCtx, cancel := context.WithCancel(context.Background())
|
||||||
|
cancel()
|
||||||
|
|
||||||
|
require.NoError(t, lc.hooks[0].OnStart(hookCtx))
|
||||||
|
|
||||||
|
return lc
|
||||||
|
}
|
||||||
|
|
||||||
|
// eventGone reports whether an event row has been removed. It
|
||||||
|
// takes no *testing.T because it is polled from an
|
||||||
|
// assert.Eventually condition, which runs off the test goroutine
|
||||||
|
// where testify assertions must not be used.
|
||||||
|
func eventGone(db *gorm.DB, eventID string) bool {
|
||||||
|
var n int64
|
||||||
|
|
||||||
|
err := db.Unscoped().Model(&database.Event{}).
|
||||||
|
Where("id = ?", eventID).Count(&n).Error
|
||||||
|
if err != nil {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
|
return n == 0
|
||||||
|
}
|
||||||
|
|
||||||
|
// seedExpiredWebhook creates a webhook with a finite retention
|
||||||
|
// policy plus one long-expired event chain, and returns the
|
||||||
|
// webhook's database and the chain's event ID.
|
||||||
|
func seedExpiredWebhook(
|
||||||
|
t *testing.T, env *retentionTestEnv,
|
||||||
|
) (*gorm.DB, string) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
webhookID := createWebhook(
|
||||||
|
t, env.mainDB.DB(), reaperTestRetentionDays,
|
||||||
|
)
|
||||||
|
|
||||||
|
db, err := env.mgr.GetDB(webhookID)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
chain := seedEventChain(
|
||||||
|
t, db, webhookID,
|
||||||
|
time.Now().Add(-365*24*time.Hour),
|
||||||
|
)
|
||||||
|
|
||||||
|
return db, chain.eventID
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestRetentionReaper_LoopOutlivesStartHookContext is the
|
||||||
|
// regression test for a reaper that never reaped. fx calls
|
||||||
|
// OnStart with a context carrying the application's start timeout
|
||||||
|
// (15s by default) and cancels it when the start phase ends, so a
|
||||||
|
// sweep loop rooted in it is dead three quarters of an hour
|
||||||
|
// before its first tick under the default one-hour interval, and
|
||||||
|
// per-webhook event databases grow without bound exactly as they
|
||||||
|
// did before retention existed.
|
||||||
|
//
|
||||||
|
// Driving OnStart with an already-cancelled context is that
|
||||||
|
// defect taken to its limit: a loop that inherits the hook
|
||||||
|
// context never ticks once, while a correctly rooted loop keeps
|
||||||
|
// sweeping for as long as the process lives.
|
||||||
|
func TestRetentionReaper_LoopOutlivesStartHookContext(
|
||||||
|
t *testing.T,
|
||||||
|
) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
env := setupRetentionTest(t)
|
||||||
|
|
||||||
|
db, eventID := seedExpiredWebhook(t, env)
|
||||||
|
|
||||||
|
env.reaper.ExportSetInterval(reaperTestInterval)
|
||||||
|
|
||||||
|
lc := startReaperViaHook(t, env.reaper)
|
||||||
|
t.Cleanup(func() {
|
||||||
|
_ = lc.hooks[0].OnStop(context.Background())
|
||||||
|
})
|
||||||
|
|
||||||
|
assert.Eventually(
|
||||||
|
t,
|
||||||
|
func() bool { return eventGone(db, eventID) },
|
||||||
|
5*time.Second,
|
||||||
|
reaperTestInterval,
|
||||||
|
"the sweep loop must keep running after the start "+
|
||||||
|
"hook's context is done; it reaped nothing, so it "+
|
||||||
|
"inherited the hook context and died",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestRetentionReaper_StopHookStopsLoop proves the fix did not
|
||||||
|
// trade a startup bug for a shutdown hang: now that the sweep
|
||||||
|
// loop no longer observes the start hook's cancellation, OnStop
|
||||||
|
// is the only thing that can stop it, and it must both return
|
||||||
|
// promptly and actually leave the loop stopped.
|
||||||
|
func TestRetentionReaper_StopHookStopsLoop(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
env := setupRetentionTest(t)
|
||||||
|
|
||||||
|
db, eventID := seedExpiredWebhook(t, env)
|
||||||
|
|
||||||
|
env.reaper.ExportSetInterval(reaperTestInterval)
|
||||||
|
|
||||||
|
lc := startReaperViaHook(t, env.reaper)
|
||||||
|
|
||||||
|
// Let the loop prove it is running before stopping it, so a
|
||||||
|
// fast OnStop cannot pass by stopping something already dead.
|
||||||
|
require.Eventually(
|
||||||
|
t,
|
||||||
|
func() bool { return eventGone(db, eventID) },
|
||||||
|
5*time.Second,
|
||||||
|
reaperTestInterval,
|
||||||
|
)
|
||||||
|
|
||||||
|
var stopErr error
|
||||||
|
|
||||||
|
stopped := make(chan struct{})
|
||||||
|
|
||||||
|
go func() {
|
||||||
|
defer close(stopped)
|
||||||
|
|
||||||
|
// stop blocks on the loop's WaitGroup, so returning at all
|
||||||
|
// proves the goroutine observed the cancellation.
|
||||||
|
stopErr = lc.hooks[0].OnStop(context.Background())
|
||||||
|
}()
|
||||||
|
|
||||||
|
select {
|
||||||
|
case <-stopped:
|
||||||
|
case <-time.After(reaperStopTimeout):
|
||||||
|
t.Fatal(
|
||||||
|
"OnStop did not return: the retention reaper's " +
|
||||||
|
"WaitGroup is still waiting on a loop that never " +
|
||||||
|
"observed cancellation",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
require.NoError(t, stopErr)
|
||||||
|
|
||||||
|
// With the loop gone, a newly expired chain must survive.
|
||||||
|
survivor := seedEventChain(
|
||||||
|
t, db, "stopped-webhook",
|
||||||
|
time.Now().Add(-365*24*time.Hour),
|
||||||
|
)
|
||||||
|
|
||||||
|
time.Sleep(20 * reaperTestInterval)
|
||||||
|
|
||||||
|
assert.False(
|
||||||
|
t,
|
||||||
|
eventGone(db, survivor.eventID),
|
||||||
|
"a stopped reaper must not sweep anything",
|
||||||
|
)
|
||||||
|
}
|
||||||
@@ -149,18 +149,7 @@ func New(
|
|||||||
Transport: NewSSRFSafeTransport(),
|
Transport: NewSSRFSafeTransport(),
|
||||||
})
|
})
|
||||||
|
|
||||||
lc.Append(fx.Hook{
|
e.registerHooks(lc)
|
||||||
OnStart: func(ctx context.Context) error {
|
|
||||||
e.start(ctx)
|
|
||||||
|
|
||||||
return nil
|
|
||||||
},
|
|
||||||
OnStop: func(_ context.Context) error {
|
|
||||||
e.stop()
|
|
||||||
|
|
||||||
return nil
|
|
||||||
},
|
|
||||||
})
|
|
||||||
|
|
||||||
return e
|
return e
|
||||||
}
|
}
|
||||||
@@ -210,8 +199,40 @@ func (e *Engine) ScheduleRetry(
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
func (e *Engine) start(ctx context.Context) {
|
// registerHooks wires the engine's start and stop into the fx
|
||||||
ctx, cancel := context.WithCancel(ctx)
|
// lifecycle. The start hook's context is deliberately ignored:
|
||||||
|
// see start for why the worker pool must not inherit it.
|
||||||
|
func (e *Engine) registerHooks(lc fx.Lifecycle) {
|
||||||
|
lc.Append(fx.Hook{
|
||||||
|
//nolint:contextcheck // Not inheriting the hook context
|
||||||
|
// is the point: see start.
|
||||||
|
OnStart: func(_ context.Context) error {
|
||||||
|
e.start()
|
||||||
|
|
||||||
|
return nil
|
||||||
|
},
|
||||||
|
OnStop: func(_ context.Context) error {
|
||||||
|
e.stop()
|
||||||
|
|
||||||
|
return nil
|
||||||
|
},
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
// start launches the worker pool, restart recovery, and the
|
||||||
|
// periodic retry sweep.
|
||||||
|
//
|
||||||
|
// Their context is derived from context.Background(), NOT from
|
||||||
|
// the fx OnStart hook context. The hook context carries fx's
|
||||||
|
// start timeout (15s by default) and is cancelled once the start
|
||||||
|
// phase completes, so goroutines derived from it stop a few
|
||||||
|
// seconds into the process: every worker would return and the
|
||||||
|
// engine would silently stop delivering webhooks entirely. A
|
||||||
|
// long-lived goroutine must outlive the startup phase, so its
|
||||||
|
// lifetime is bounded by OnStop instead: stop cancels this
|
||||||
|
// context and waits on the WaitGroup.
|
||||||
|
func (e *Engine) start() {
|
||||||
|
ctx, cancel := context.WithCancel(context.Background())
|
||||||
e.cancel = cancel
|
e.cancel = cancel
|
||||||
|
|
||||||
for range e.workers {
|
for range e.workers {
|
||||||
|
|||||||
@@ -476,7 +476,7 @@ func TestWorkerLifecycle_StartStop(t *testing.T) {
|
|||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
s := newISetup(t)
|
s := newISetup(t)
|
||||||
s.Engine.ExportStart(context.Background())
|
s.Engine.ExportStart()
|
||||||
|
|
||||||
event := iSeedEvent(
|
event := iSeedEvent(
|
||||||
t, s.WebhookDB, s.WebhookID,
|
t, s.WebhookDB, s.WebhookID,
|
||||||
@@ -499,21 +499,17 @@ func TestWorkerLifecycle_StartStop(t *testing.T) {
|
|||||||
|
|
||||||
s.Engine.Notify([]delivery.Task{task})
|
s.Engine.Notify([]delivery.Task{task})
|
||||||
|
|
||||||
iWaitForStatus(
|
iWaitForDelivered(t, s.WebhookDB, d.ID)
|
||||||
t, s.WebhookDB, d.ID,
|
|
||||||
database.DeliveryStatusDelivered,
|
|
||||||
)
|
|
||||||
|
|
||||||
s.Engine.ExportStop()
|
s.Engine.ExportStop()
|
||||||
}
|
}
|
||||||
|
|
||||||
// iWaitForStatus polls until the delivery reaches the
|
// iWaitForDelivered polls until the delivery reaches the
|
||||||
// expected status.
|
// delivered status.
|
||||||
func iWaitForStatus(
|
func iWaitForDelivered(
|
||||||
t *testing.T,
|
t *testing.T,
|
||||||
db *gorm.DB,
|
db *gorm.DB,
|
||||||
deliveryID string,
|
deliveryID string,
|
||||||
expected database.DeliveryStatus,
|
|
||||||
) {
|
) {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
|
|
||||||
@@ -527,7 +523,7 @@ func iWaitForStatus(
|
|||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|
||||||
return d.Status == expected
|
return d.Status == database.DeliveryStatusDelivered
|
||||||
}, 5*time.Second, 50*time.Millisecond)
|
}, 5*time.Second, 50*time.Millisecond)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -558,7 +554,7 @@ func TestWorkerLifecycle_ProcessesRetryChannel(
|
|||||||
database.DeliveryStatusRetrying,
|
database.DeliveryStatusRetrying,
|
||||||
)
|
)
|
||||||
|
|
||||||
s.Engine.ExportStart(context.Background())
|
s.Engine.ExportStart()
|
||||||
|
|
||||||
bodyStr := event.Body
|
bodyStr := event.Body
|
||||||
cfg := iHTTPConfig(ts.URL)
|
cfg := iHTTPConfig(ts.URL)
|
||||||
@@ -569,10 +565,7 @@ func TestWorkerLifecycle_ProcessesRetryChannel(
|
|||||||
|
|
||||||
s.Engine.ExportRetryCh() <- task
|
s.Engine.ExportRetryCh() <- task
|
||||||
|
|
||||||
iWaitForStatus(
|
iWaitForDelivered(t, s.WebhookDB, d.ID)
|
||||||
t, s.WebhookDB, d.ID,
|
|
||||||
database.DeliveryStatusDelivered,
|
|
||||||
)
|
|
||||||
|
|
||||||
s.Engine.ExportStop()
|
s.Engine.ExportStop()
|
||||||
}
|
}
|
||||||
|
|||||||
199
internal/delivery/engine_lifecycle_test.go
Normal file
199
internal/delivery/engine_lifecycle_test.go
Normal file
@@ -0,0 +1,199 @@
|
|||||||
|
package delivery_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"github.com/google/uuid"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
"go.uber.org/fx"
|
||||||
|
"sneak.berlin/go/webhooker/internal/database"
|
||||||
|
"sneak.berlin/go/webhooker/internal/delivery"
|
||||||
|
)
|
||||||
|
|
||||||
|
const (
|
||||||
|
// hookStopTimeout bounds how long a lifecycle test waits for
|
||||||
|
// the engine's OnStop hook to return before declaring the
|
||||||
|
// shutdown hung.
|
||||||
|
hookStopTimeout = 10 * time.Second
|
||||||
|
|
||||||
|
// hookSettleDelay is how long startEngineViaHook waits after
|
||||||
|
// OnStart before the caller may enqueue work. A worker pool
|
||||||
|
// wrongly rooted in the already-done hook context has nothing
|
||||||
|
// but ctx.Done() ready in its select, so it is deterministically
|
||||||
|
// gone by the end of this window. Without the wait, Notify would
|
||||||
|
// race the pool's very first select, in which a ready ctx.Done()
|
||||||
|
// and a ready deliveryCh are chosen between at random and a
|
||||||
|
// doomed pool still delivers.
|
||||||
|
hookSettleDelay = 250 * time.Millisecond
|
||||||
|
)
|
||||||
|
|
||||||
|
// recordingLifecycle is a minimal fx.Lifecycle that records the
|
||||||
|
// hooks a component registers, so a test can invoke the real
|
||||||
|
// OnStart/OnStop functions with a context of its choosing.
|
||||||
|
type recordingLifecycle struct {
|
||||||
|
hooks []fx.Hook
|
||||||
|
}
|
||||||
|
|
||||||
|
func (l *recordingLifecycle) Append(h fx.Hook) {
|
||||||
|
l.hooks = append(l.hooks, h)
|
||||||
|
}
|
||||||
|
|
||||||
|
// startEngineViaHook drives the genuine fx hooks the application
|
||||||
|
// registers for the engine, handing OnStart a context that is
|
||||||
|
// already done, and returns only once a pool that inherited that
|
||||||
|
// context would have exited. It returns the recorded lifecycle so
|
||||||
|
// the caller can drive OnStop too.
|
||||||
|
//
|
||||||
|
// Callers must not seed pending or retrying deliveries before
|
||||||
|
// calling this: restart recovery enqueues those during startup,
|
||||||
|
// which would put work in the queue while the pool is still
|
||||||
|
// racing its first select.
|
||||||
|
func startEngineViaHook(
|
||||||
|
t *testing.T, eng *delivery.Engine,
|
||||||
|
) *recordingLifecycle {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
lc := &recordingLifecycle{}
|
||||||
|
eng.ExportRegisterHooks(lc)
|
||||||
|
require.Len(t, lc.hooks, 1)
|
||||||
|
|
||||||
|
// fx hands OnStart a context carrying the application start
|
||||||
|
// timeout, and cancels it when the start phase ends. An
|
||||||
|
// already-cancelled context is that same defect taken to its
|
||||||
|
// limit, and unlike a plain context.Background() it actually
|
||||||
|
// distinguishes a correctly rooted loop from a broken one.
|
||||||
|
hookCtx, cancel := context.WithCancel(context.Background())
|
||||||
|
cancel()
|
||||||
|
|
||||||
|
require.NoError(t, lc.hooks[0].OnStart(hookCtx))
|
||||||
|
|
||||||
|
time.Sleep(hookSettleDelay)
|
||||||
|
|
||||||
|
return lc
|
||||||
|
}
|
||||||
|
|
||||||
|
// seedLogTask seeds a pending delivery for a log target and
|
||||||
|
// returns its ID together with the task that drives it. The log
|
||||||
|
// target needs no network, so a delivery completing proves only
|
||||||
|
// that a worker picked the task up.
|
||||||
|
func seedLogTask(
|
||||||
|
t *testing.T, s iSetup,
|
||||||
|
) (string, delivery.Task) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
event := iSeedEvent(
|
||||||
|
t, s.WebhookDB, s.WebhookID,
|
||||||
|
`{"lifecycle":"hook-context"}`,
|
||||||
|
)
|
||||||
|
targetID := uuid.New().String()
|
||||||
|
|
||||||
|
d := iSeedDelivery(
|
||||||
|
t, s.WebhookDB, event.ID, targetID,
|
||||||
|
database.DeliveryStatusPending,
|
||||||
|
)
|
||||||
|
|
||||||
|
bodyStr := event.Body
|
||||||
|
task := iTask(
|
||||||
|
d, event, s.WebhookID, targetID,
|
||||||
|
"hook-context-test", "", 0, 1, &bodyStr,
|
||||||
|
)
|
||||||
|
task.TargetType = database.TargetTypeLog
|
||||||
|
|
||||||
|
return d.ID, task
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestEngine_WorkersOutliveStartHookContext is the regression
|
||||||
|
// test for a delivery engine that stopped delivering roughly
|
||||||
|
// fifteen seconds after boot. fx calls OnStart with a context
|
||||||
|
// carrying the application's start timeout (15s by default) and
|
||||||
|
// cancels it when the start phase ends, so a worker pool rooted
|
||||||
|
// in it exits shortly after startup: the process keeps accepting
|
||||||
|
// and persisting events while nothing at all forwards them.
|
||||||
|
//
|
||||||
|
// Driving OnStart with an already-cancelled context is that
|
||||||
|
// defect taken to its limit. A pool that inherits the hook
|
||||||
|
// context is gone before the task is even enqueued; a correctly
|
||||||
|
// rooted pool keeps working for as long as the process lives.
|
||||||
|
func TestEngine_WorkersOutliveStartHookContext(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
s := newISetup(t)
|
||||||
|
|
||||||
|
lc := startEngineViaHook(t, s.Engine)
|
||||||
|
t.Cleanup(func() {
|
||||||
|
_ = lc.hooks[0].OnStop(context.Background())
|
||||||
|
})
|
||||||
|
|
||||||
|
// Seeded only after the pool has settled, so restart recovery
|
||||||
|
// cannot enqueue it during startup.
|
||||||
|
deliveryID, task := seedLogTask(t, s)
|
||||||
|
|
||||||
|
s.Engine.Notify([]delivery.Task{task})
|
||||||
|
|
||||||
|
iWaitForDelivered(t, s.WebhookDB, deliveryID)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestEngine_StopHookStopsWorkers proves the fix did not trade a
|
||||||
|
// startup bug for a shutdown hang: now that the worker pool no
|
||||||
|
// longer observes the start hook's cancellation, OnStop is the
|
||||||
|
// only thing that can stop it, and it must both return promptly
|
||||||
|
// and actually leave the pool drained.
|
||||||
|
func TestEngine_StopHookStopsWorkers(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
s := newISetup(t)
|
||||||
|
|
||||||
|
lc := startEngineViaHook(t, s.Engine)
|
||||||
|
|
||||||
|
// Let the pool prove it is running before stopping it, so a
|
||||||
|
// fast OnStop cannot pass by stopping something already dead.
|
||||||
|
firstID, firstTask := seedLogTask(t, s)
|
||||||
|
s.Engine.Notify([]delivery.Task{firstTask})
|
||||||
|
iWaitForDelivered(t, s.WebhookDB, firstID)
|
||||||
|
|
||||||
|
var stopErr error
|
||||||
|
|
||||||
|
stopped := make(chan struct{})
|
||||||
|
|
||||||
|
go func() {
|
||||||
|
defer close(stopped)
|
||||||
|
|
||||||
|
// stop blocks on the workers' WaitGroup, so returning at
|
||||||
|
// all proves every goroutine observed the cancellation.
|
||||||
|
stopErr = lc.hooks[0].OnStop(context.Background())
|
||||||
|
}()
|
||||||
|
|
||||||
|
select {
|
||||||
|
case <-stopped:
|
||||||
|
case <-time.After(hookStopTimeout):
|
||||||
|
t.Fatal(
|
||||||
|
"OnStop did not return: the delivery engine's " +
|
||||||
|
"WaitGroup is still waiting on a goroutine that " +
|
||||||
|
"never observed cancellation",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
require.NoError(t, stopErr)
|
||||||
|
|
||||||
|
// With every worker gone, a freshly notified task must sit
|
||||||
|
// untouched in the queue rather than being delivered.
|
||||||
|
secondID, secondTask := seedLogTask(t, s)
|
||||||
|
s.Engine.Notify([]delivery.Task{secondTask})
|
||||||
|
|
||||||
|
time.Sleep(200 * time.Millisecond)
|
||||||
|
|
||||||
|
var after database.Delivery
|
||||||
|
|
||||||
|
require.NoError(
|
||||||
|
t,
|
||||||
|
s.WebhookDB.First(&after, "id = ?", secondID).Error,
|
||||||
|
)
|
||||||
|
require.Equal(
|
||||||
|
t,
|
||||||
|
database.DeliveryStatusPending,
|
||||||
|
after.Status,
|
||||||
|
"a stopped engine must not deliver anything",
|
||||||
|
)
|
||||||
|
}
|
||||||
@@ -7,6 +7,7 @@ import (
|
|||||||
"net/http"
|
"net/http"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
"go.uber.org/fx"
|
||||||
"gorm.io/gorm"
|
"gorm.io/gorm"
|
||||||
"sneak.berlin/go/webhooker/internal/database"
|
"sneak.berlin/go/webhooker/internal/database"
|
||||||
)
|
)
|
||||||
@@ -189,8 +190,16 @@ func (e *Engine) ExportRecoverInFlight(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// ExportStart exposes start for testing.
|
// ExportStart exposes start for testing.
|
||||||
func (e *Engine) ExportStart(ctx context.Context) {
|
func (e *Engine) ExportStart() {
|
||||||
e.start(ctx)
|
e.start()
|
||||||
|
}
|
||||||
|
|
||||||
|
// ExportRegisterHooks registers the engine's real fx lifecycle
|
||||||
|
// hooks on a lifecycle supplied by a test, so a test can drive
|
||||||
|
// the exact OnStart/OnStop functions the application runs and
|
||||||
|
// hand OnStart the kind of context fx actually supplies.
|
||||||
|
func (e *Engine) ExportRegisterHooks(lc fx.Lifecycle) {
|
||||||
|
e.registerHooks(lc)
|
||||||
}
|
}
|
||||||
|
|
||||||
// ExportStop exposes stop for testing.
|
// ExportStop exposes stop for testing.
|
||||||
|
|||||||
@@ -29,8 +29,10 @@ func (h *Handlers) HandleLoginPage() http.HandlerFunc {
|
|||||||
// HandleLoginSubmit handles the login form submission (POST)
|
// HandleLoginSubmit handles the login form submission (POST)
|
||||||
func (h *Handlers) HandleLoginSubmit() http.HandlerFunc {
|
func (h *Handlers) HandleLoginSubmit() http.HandlerFunc {
|
||||||
return func(w http.ResponseWriter, r *http.Request) {
|
return func(w http.ResponseWriter, r *http.Request) {
|
||||||
// The body size cap is enforced by the MaxBodySize
|
// Limit request body to prevent memory exhaustion
|
||||||
// middleware, which runs before CSRF parses the form.
|
r.Body = http.MaxBytesReader(w, r.Body, 1<<maxBodyShift)
|
||||||
|
|
||||||
|
// Parse form data
|
||||||
err := r.ParseForm()
|
err := r.ParseForm()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
h.log.Error("failed to parse form", "error", err)
|
h.log.Error("failed to parse form", "error", err)
|
||||||
|
|||||||
@@ -31,8 +31,9 @@ func (h *Handlers) HandlePasswordChange() http.HandlerFunc {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
// The body size cap is enforced by the MaxBodySize
|
// Limit request body to prevent memory exhaustion.
|
||||||
// middleware, which runs before CSRF parses the form.
|
r.Body = http.MaxBytesReader(w, r.Body, 1<<maxBodyShift)
|
||||||
|
|
||||||
err := r.ParseForm()
|
err := r.ParseForm()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
h.log.Error("failed to parse form", "error", err)
|
h.log.Error("failed to parse form", "error", err)
|
||||||
|
|||||||
@@ -127,8 +127,10 @@ func (h *Handlers) HandleSourceCreateSubmit() http.HandlerFunc {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
// The body size cap is enforced by the MaxBodySize
|
r.Body = http.MaxBytesReader(
|
||||||
// middleware, which runs before CSRF parses the form.
|
w, r.Body, 1<<maxBodyShift,
|
||||||
|
)
|
||||||
|
|
||||||
err := r.ParseForm()
|
err := r.ParseForm()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
http.Error(
|
http.Error(
|
||||||
@@ -384,8 +386,10 @@ func (h *Handlers) HandleSourceEditSubmit() http.HandlerFunc {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
// The body size cap is enforced by the MaxBodySize
|
r.Body = http.MaxBytesReader(
|
||||||
// middleware, which runs before CSRF parses the form.
|
w, r.Body, 1<<maxBodyShift,
|
||||||
|
)
|
||||||
|
|
||||||
err = r.ParseForm()
|
err = r.ParseForm()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
http.Error(
|
http.Error(
|
||||||
@@ -405,8 +409,10 @@ func (h *Handlers) applyWebhookEdit(
|
|||||||
r *http.Request,
|
r *http.Request,
|
||||||
webhook *database.Webhook,
|
webhook *database.Webhook,
|
||||||
) {
|
) {
|
||||||
// The body size cap is enforced by the MaxBodySize middleware,
|
r.Body = http.MaxBytesReader(
|
||||||
// which runs before CSRF parses the form.
|
w, r.Body, 1<<maxBodyShift,
|
||||||
|
)
|
||||||
|
|
||||||
name := r.FormValue("name")
|
name := r.FormValue("name")
|
||||||
if name == "" {
|
if name == "" {
|
||||||
data := map[string]any{
|
data := map[string]any{
|
||||||
@@ -719,8 +725,10 @@ func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
// The body size cap is enforced by the MaxBodySize
|
r.Body = http.MaxBytesReader(
|
||||||
// middleware, which runs before CSRF parses the form.
|
w, r.Body, 1<<maxBodyShift,
|
||||||
|
)
|
||||||
|
|
||||||
err = r.ParseForm()
|
err = r.ParseForm()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
http.Error(
|
http.Error(
|
||||||
@@ -777,8 +785,10 @@ func (h *Handlers) HandleTargetCreate() http.HandlerFunc {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
// The body size cap is enforced by the MaxBodySize
|
r.Body = http.MaxBytesReader(
|
||||||
// middleware, which runs before CSRF parses the form.
|
w, r.Body, 1<<maxBodyShift,
|
||||||
|
)
|
||||||
|
|
||||||
err = r.ParseForm()
|
err = r.ParseForm()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
http.Error(
|
http.Error(
|
||||||
@@ -798,8 +808,10 @@ func (h *Handlers) processTargetCreate(
|
|||||||
r *http.Request,
|
r *http.Request,
|
||||||
webhook database.Webhook,
|
webhook database.Webhook,
|
||||||
) {
|
) {
|
||||||
// The body size cap is enforced by the MaxBodySize middleware,
|
r.Body = http.MaxBytesReader(
|
||||||
// which runs before CSRF parses the form.
|
w, r.Body, 1<<maxBodyShift,
|
||||||
|
)
|
||||||
|
|
||||||
name := r.FormValue("name")
|
name := r.FormValue("name")
|
||||||
targetType := database.TargetType(r.FormValue("type"))
|
targetType := database.TargetType(r.FormValue("type"))
|
||||||
targetURL := r.FormValue("url")
|
targetURL := r.FormValue("url")
|
||||||
|
|||||||
@@ -285,36 +285,10 @@ func (s *Middleware) NoCache() func(http.Handler) http.Handler {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// bodyLimitedMethod reports whether the request method carries a
|
// MaxBodySize returns middleware that limits the request body size
|
||||||
// body that the MaxBodySize middleware should cap.
|
// for POST requests. If the body exceeds the given limit in
|
||||||
func bodyLimitedMethod(method string) bool {
|
// bytes, the server returns 413 Request Entity Too Large. This
|
||||||
return method == http.MethodPost ||
|
// prevents clients from sending arbitrarily large form bodies.
|
||||||
method == http.MethodPut ||
|
|
||||||
method == http.MethodPatch
|
|
||||||
}
|
|
||||||
|
|
||||||
// MaxBodySize returns middleware that limits the size of
|
|
||||||
// POST/PUT/PATCH request bodies to maxBytes. It must be registered
|
|
||||||
// before any middleware that parses the body — notably CSRF, which
|
|
||||||
// calls r.PostFormValue — so that form parsing happens under this
|
|
||||||
// cap rather than net/http's 10 MB default.
|
|
||||||
//
|
|
||||||
// Two enforcement paths exist, because http.MaxBytesReader alone
|
|
||||||
// cannot produce a 413: it reports the overflow as an error from
|
|
||||||
// Read, by which point the body parser downstream has already
|
|
||||||
// converted that error into its own response.
|
|
||||||
//
|
|
||||||
// - Declared oversize: the request announces a Content-Length
|
|
||||||
// greater than maxBytes. The middleware answers 413 Request
|
|
||||||
// Entity Too Large immediately and does not call the next
|
|
||||||
// handler, so neither CSRF nor the endpoint handler runs.
|
|
||||||
// - Undeclared oversize: the request is chunked (Content-Length
|
|
||||||
// of -1) or lies about its Content-Length. There is nothing to
|
|
||||||
// check up front, so http.MaxBytesReader hard-caps the body at
|
|
||||||
// maxBytes and the request fails downstream — the form parse
|
|
||||||
// errors out and CSRF rejects it with 403. The response is less
|
|
||||||
// precise than a 413, but the body is still never buffered
|
|
||||||
// beyond the cap, which is the property that matters.
|
|
||||||
func (s *Middleware) MaxBodySize(
|
func (s *Middleware) MaxBodySize(
|
||||||
maxBytes int64,
|
maxBytes int64,
|
||||||
) func(http.Handler) http.Handler {
|
) func(http.Handler) http.Handler {
|
||||||
@@ -323,31 +297,14 @@ func (s *Middleware) MaxBodySize(
|
|||||||
w http.ResponseWriter,
|
w http.ResponseWriter,
|
||||||
r *http.Request,
|
r *http.Request,
|
||||||
) {
|
) {
|
||||||
if !bodyLimitedMethod(r.Method) {
|
if r.Method == http.MethodPost ||
|
||||||
next.ServeHTTP(w, r)
|
r.Method == http.MethodPut ||
|
||||||
|
r.Method == http.MethodPatch {
|
||||||
return
|
r.Body = http.MaxBytesReader(
|
||||||
}
|
w, r.Body, maxBytes,
|
||||||
|
|
||||||
if r.ContentLength > maxBytes {
|
|
||||||
s.log.Warn(
|
|
||||||
"request body exceeds limit",
|
|
||||||
"method", r.Method,
|
|
||||||
"path", r.URL.Path,
|
|
||||||
"content_length", r.ContentLength,
|
|
||||||
"limit", maxBytes,
|
|
||||||
)
|
)
|
||||||
http.Error(
|
|
||||||
w,
|
|
||||||
"Request Entity Too Large",
|
|
||||||
http.StatusRequestEntityTooLarge,
|
|
||||||
)
|
|
||||||
|
|
||||||
return
|
|
||||||
}
|
}
|
||||||
|
|
||||||
r.Body = http.MaxBytesReader(w, r.Body, maxBytes)
|
|
||||||
|
|
||||||
next.ServeHTTP(w, r)
|
next.ServeHTTP(w, r)
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -3,12 +3,10 @@ package middleware_test
|
|||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"encoding/base64"
|
"encoding/base64"
|
||||||
"io"
|
|
||||||
"log/slog"
|
"log/slog"
|
||||||
"net/http"
|
"net/http"
|
||||||
"net/http/httptest"
|
"net/http/httptest"
|
||||||
"os"
|
"os"
|
||||||
"strings"
|
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"github.com/gorilla/sessions"
|
"github.com/gorilla/sessions"
|
||||||
@@ -428,153 +426,6 @@ func TestNoCache_SetsHeaders(t *testing.T) {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
// --- MaxBodySize Middleware Tests ---
|
|
||||||
|
|
||||||
const testBodyLimit int64 = 64
|
|
||||||
|
|
||||||
// maxBodySizeHandler wraps a sentinel handler in MaxBodySize with
|
|
||||||
// testBodyLimit. The sentinel records whether it ran and how much of
|
|
||||||
// the body it managed to read, so tests can distinguish "never
|
|
||||||
// reached" from "reached but truncated".
|
|
||||||
type maxBodySizeResult struct {
|
|
||||||
called bool
|
|
||||||
read int
|
|
||||||
readErr error
|
|
||||||
response *httptest.ResponseRecorder
|
|
||||||
}
|
|
||||||
|
|
||||||
func runMaxBodySize(
|
|
||||||
t *testing.T,
|
|
||||||
req *http.Request,
|
|
||||||
) *maxBodySizeResult {
|
|
||||||
t.Helper()
|
|
||||||
|
|
||||||
m, _ := testMiddleware(t, config.EnvironmentDev)
|
|
||||||
res := &maxBodySizeResult{response: httptest.NewRecorder()}
|
|
||||||
|
|
||||||
handler := m.MaxBodySize(testBodyLimit)(http.HandlerFunc(
|
|
||||||
func(w http.ResponseWriter, r *http.Request) {
|
|
||||||
res.called = true
|
|
||||||
|
|
||||||
body, err := io.ReadAll(r.Body)
|
|
||||||
res.read = len(body)
|
|
||||||
res.readErr = err
|
|
||||||
|
|
||||||
w.WriteHeader(http.StatusOK)
|
|
||||||
},
|
|
||||||
))
|
|
||||||
|
|
||||||
handler.ServeHTTP(res.response, req)
|
|
||||||
|
|
||||||
return res
|
|
||||||
}
|
|
||||||
|
|
||||||
// postWithBody builds a POST request whose Content-Length is
|
|
||||||
// accurate for the given payload size.
|
|
||||||
func postWithBody(size int) *http.Request {
|
|
||||||
req := httptest.NewRequestWithContext(
|
|
||||||
context.Background(),
|
|
||||||
http.MethodPost, "/pages/login",
|
|
||||||
strings.NewReader(strings.Repeat("a", size)),
|
|
||||||
)
|
|
||||||
req.Header.Set(
|
|
||||||
"Content-Type", "application/x-www-form-urlencoded",
|
|
||||||
)
|
|
||||||
|
|
||||||
return req
|
|
||||||
}
|
|
||||||
|
|
||||||
func TestMaxBodySize_DeclaredOversize_413AndHandlerNotReached(
|
|
||||||
t *testing.T,
|
|
||||||
) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
res := runMaxBodySize(t, postWithBody(int(testBodyLimit)+1))
|
|
||||||
|
|
||||||
assert.False(
|
|
||||||
t, res.called,
|
|
||||||
"handler must not be reached for an oversized body",
|
|
||||||
)
|
|
||||||
assert.Equal(
|
|
||||||
t, http.StatusRequestEntityTooLarge, res.response.Code,
|
|
||||||
)
|
|
||||||
}
|
|
||||||
|
|
||||||
func TestMaxBodySize_AtLimit_PassesThrough(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
res := runMaxBodySize(t, postWithBody(int(testBodyLimit)))
|
|
||||||
|
|
||||||
assert.True(
|
|
||||||
t, res.called,
|
|
||||||
"handler should be reached for a body at the limit",
|
|
||||||
)
|
|
||||||
require.NoError(t, res.readErr)
|
|
||||||
assert.Equal(t, int(testBodyLimit), res.read)
|
|
||||||
assert.Equal(t, http.StatusOK, res.response.Code)
|
|
||||||
}
|
|
||||||
|
|
||||||
func TestMaxBodySize_UnderLimit_PassesThrough(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
res := runMaxBodySize(t, postWithBody(1))
|
|
||||||
|
|
||||||
assert.True(t, res.called)
|
|
||||||
require.NoError(t, res.readErr)
|
|
||||||
assert.Equal(t, 1, res.read)
|
|
||||||
assert.Equal(t, http.StatusOK, res.response.Code)
|
|
||||||
}
|
|
||||||
|
|
||||||
func TestMaxBodySize_GetWithOversizeBody_NotCapped(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
req := httptest.NewRequestWithContext(
|
|
||||||
context.Background(),
|
|
||||||
http.MethodGet, "/pages/login",
|
|
||||||
strings.NewReader(
|
|
||||||
strings.Repeat("a", int(testBodyLimit)+1),
|
|
||||||
),
|
|
||||||
)
|
|
||||||
|
|
||||||
res := runMaxBodySize(t, req)
|
|
||||||
|
|
||||||
assert.True(
|
|
||||||
t, res.called,
|
|
||||||
"GET requests are not subject to the POST body cap",
|
|
||||||
)
|
|
||||||
require.NoError(t, res.readErr)
|
|
||||||
assert.Equal(t, int(testBodyLimit)+1, res.read)
|
|
||||||
}
|
|
||||||
|
|
||||||
// TestMaxBodySize_UndeclaredOversize_TruncatedAtCap covers the
|
|
||||||
// chunked / lying-Content-Length case: there is nothing to check up
|
|
||||||
// front, so the request reaches the handler but MaxBytesReader
|
|
||||||
// hard-caps the body and the read fails at the limit.
|
|
||||||
func TestMaxBodySize_UndeclaredOversize_TruncatedAtCap(
|
|
||||||
t *testing.T,
|
|
||||||
) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
req := postWithBody(int(testBodyLimit) + 1)
|
|
||||||
// Simulate a chunked request: no declared length.
|
|
||||||
req.ContentLength = -1
|
|
||||||
|
|
||||||
res := runMaxBodySize(t, req)
|
|
||||||
|
|
||||||
assert.True(
|
|
||||||
t, res.called,
|
|
||||||
"an undeclared oversize body cannot be rejected up front",
|
|
||||||
)
|
|
||||||
require.Error(
|
|
||||||
t, res.readErr,
|
|
||||||
"reading past the cap must fail",
|
|
||||||
)
|
|
||||||
assert.Equal(
|
|
||||||
t, int(testBodyLimit), res.read,
|
|
||||||
"the handler must not see more than the cap",
|
|
||||||
)
|
|
||||||
}
|
|
||||||
|
|
||||||
// --- Helper Tests ---
|
// --- Helper Tests ---
|
||||||
|
|
||||||
func TestIpFromHostPort(t *testing.T) {
|
func TestIpFromHostPort(t *testing.T) {
|
||||||
|
|||||||
@@ -1,36 +0,0 @@
|
|||||||
package server
|
|
||||||
|
|
||||||
import (
|
|
||||||
"log/slog"
|
|
||||||
"net/http"
|
|
||||||
|
|
||||||
"sneak.berlin/go/webhooker/internal/config"
|
|
||||||
"sneak.berlin/go/webhooker/internal/handlers"
|
|
||||||
"sneak.berlin/go/webhooker/internal/middleware"
|
|
||||||
)
|
|
||||||
|
|
||||||
// MaxFormBodySizeForTest exposes the form body cap so tests can
|
|
||||||
// build requests that sit exactly at, below, and above it.
|
|
||||||
const MaxFormBodySizeForTest = maxFormBodySize
|
|
||||||
|
|
||||||
// NewRouterForTest builds the real route tree via SetupRoutes with
|
|
||||||
// the supplied middleware and handlers, bypassing the fx lifecycle
|
|
||||||
// and the HTTP listener. Tests use it so that route-group middleware
|
|
||||||
// registration order is exercised exactly as it ships, rather than
|
|
||||||
// against a hand-rebuilt chain that could drift from routes.go.
|
|
||||||
func NewRouterForTest(
|
|
||||||
log *slog.Logger,
|
|
||||||
cfg *config.Config,
|
|
||||||
mw *middleware.Middleware,
|
|
||||||
h *handlers.Handlers,
|
|
||||||
) http.Handler {
|
|
||||||
s := &Server{
|
|
||||||
log: log,
|
|
||||||
mw: mw,
|
|
||||||
h: h,
|
|
||||||
params: ServerParams{Config: cfg},
|
|
||||||
}
|
|
||||||
s.SetupRoutes()
|
|
||||||
|
|
||||||
return s.router
|
|
||||||
}
|
|
||||||
@@ -90,11 +90,9 @@ func (s *Server) setupRoutes() {
|
|||||||
|
|
||||||
func (s *Server) setupPageRoutes() {
|
func (s *Server) setupPageRoutes() {
|
||||||
s.router.Route("/pages", func(r chi.Router) {
|
s.router.Route("/pages", func(r chi.Router) {
|
||||||
// MaxBodySize must precede CSRF: gorilla/csrf parses the
|
|
||||||
// form, so the cap has to be installed before it runs.
|
|
||||||
r.Use(s.mw.MaxBodySize(maxFormBodySize))
|
|
||||||
r.Use(s.mw.CSRF())
|
r.Use(s.mw.CSRF())
|
||||||
r.Use(s.mw.NoCache())
|
r.Use(s.mw.NoCache())
|
||||||
|
r.Use(s.mw.MaxBodySize(maxFormBodySize))
|
||||||
|
|
||||||
r.Group(func(r chi.Router) {
|
r.Group(func(r chi.Router) {
|
||||||
r.Use(s.mw.LoginRateLimit())
|
r.Use(s.mw.LoginRateLimit())
|
||||||
@@ -108,9 +106,6 @@ func (s *Server) setupPageRoutes() {
|
|||||||
|
|
||||||
func (s *Server) setupUserRoutes() {
|
func (s *Server) setupUserRoutes() {
|
||||||
s.router.Route("/user/{username}", func(r chi.Router) {
|
s.router.Route("/user/{username}", func(r chi.Router) {
|
||||||
// MaxBodySize must precede CSRF: gorilla/csrf parses the
|
|
||||||
// form, so the cap has to be installed before it runs.
|
|
||||||
r.Use(s.mw.MaxBodySize(maxFormBodySize))
|
|
||||||
r.Use(s.mw.CSRF())
|
r.Use(s.mw.CSRF())
|
||||||
r.Use(s.mw.NoCache())
|
r.Use(s.mw.NoCache())
|
||||||
r.Use(s.mw.RequireAuth())
|
r.Use(s.mw.RequireAuth())
|
||||||
@@ -123,24 +118,20 @@ func (s *Server) setupUserRoutes() {
|
|||||||
|
|
||||||
func (s *Server) setupSourceRoutes() {
|
func (s *Server) setupSourceRoutes() {
|
||||||
s.router.Route("/sources", func(r chi.Router) {
|
s.router.Route("/sources", func(r chi.Router) {
|
||||||
// MaxBodySize must precede CSRF: gorilla/csrf parses the
|
|
||||||
// form, so the cap has to be installed before it runs.
|
|
||||||
r.Use(s.mw.MaxBodySize(maxFormBodySize))
|
|
||||||
r.Use(s.mw.CSRF())
|
r.Use(s.mw.CSRF())
|
||||||
r.Use(s.mw.NoCache())
|
r.Use(s.mw.NoCache())
|
||||||
r.Use(s.mw.RequireAuth())
|
r.Use(s.mw.RequireAuth())
|
||||||
|
r.Use(s.mw.MaxBodySize(maxFormBodySize))
|
||||||
r.Get("/", s.h.HandleSourceList())
|
r.Get("/", s.h.HandleSourceList())
|
||||||
r.Get("/new", s.h.HandleSourceCreate())
|
r.Get("/new", s.h.HandleSourceCreate())
|
||||||
r.Post("/new", s.h.HandleSourceCreateSubmit())
|
r.Post("/new", s.h.HandleSourceCreateSubmit())
|
||||||
})
|
})
|
||||||
|
|
||||||
s.router.Route("/source/{sourceID}", func(r chi.Router) {
|
s.router.Route("/source/{sourceID}", func(r chi.Router) {
|
||||||
// MaxBodySize must precede CSRF: gorilla/csrf parses the
|
|
||||||
// form, so the cap has to be installed before it runs.
|
|
||||||
r.Use(s.mw.MaxBodySize(maxFormBodySize))
|
|
||||||
r.Use(s.mw.CSRF())
|
r.Use(s.mw.CSRF())
|
||||||
r.Use(s.mw.NoCache())
|
r.Use(s.mw.NoCache())
|
||||||
r.Use(s.mw.RequireAuth())
|
r.Use(s.mw.RequireAuth())
|
||||||
|
r.Use(s.mw.MaxBodySize(maxFormBodySize))
|
||||||
r.Get("/", s.h.HandleSourceDetail())
|
r.Get("/", s.h.HandleSourceDetail())
|
||||||
r.Get("/edit", s.h.HandleSourceEdit())
|
r.Get("/edit", s.h.HandleSourceEdit())
|
||||||
r.Post("/edit", s.h.HandleSourceEditSubmit())
|
r.Post("/edit", s.h.HandleSourceEditSubmit())
|
||||||
|
|||||||
@@ -1,375 +0,0 @@
|
|||||||
package server_test
|
|
||||||
|
|
||||||
import (
|
|
||||||
"context"
|
|
||||||
"html"
|
|
||||||
"net/http"
|
|
||||||
"net/http/httptest"
|
|
||||||
"net/url"
|
|
||||||
"regexp"
|
|
||||||
"strings"
|
|
||||||
"testing"
|
|
||||||
|
|
||||||
"github.com/stretchr/testify/assert"
|
|
||||||
"github.com/stretchr/testify/require"
|
|
||||||
"go.uber.org/fx"
|
|
||||||
"go.uber.org/fx/fxtest"
|
|
||||||
"sneak.berlin/go/webhooker/internal/config"
|
|
||||||
"sneak.berlin/go/webhooker/internal/database"
|
|
||||||
"sneak.berlin/go/webhooker/internal/delivery"
|
|
||||||
"sneak.berlin/go/webhooker/internal/globals"
|
|
||||||
"sneak.berlin/go/webhooker/internal/handlers"
|
|
||||||
"sneak.berlin/go/webhooker/internal/healthcheck"
|
|
||||||
"sneak.berlin/go/webhooker/internal/logger"
|
|
||||||
"sneak.berlin/go/webhooker/internal/middleware"
|
|
||||||
"sneak.berlin/go/webhooker/internal/server"
|
|
||||||
"sneak.berlin/go/webhooker/internal/session"
|
|
||||||
)
|
|
||||||
|
|
||||||
// csrfCookieName is the cookie gorilla/csrf issues when it runs. Its
|
|
||||||
// presence or absence on a response is how these tests tell whether
|
|
||||||
// the CSRF middleware executed.
|
|
||||||
const csrfCookieName = "_gorilla_csrf"
|
|
||||||
|
|
||||||
type noopNotifier struct{}
|
|
||||||
|
|
||||||
func (n *noopNotifier) Notify([]delivery.Task) {}
|
|
||||||
|
|
||||||
// testEnv is the real router from routes.go plus the collaborators
|
|
||||||
// tests need to seed users and forge sessions.
|
|
||||||
type testEnv struct {
|
|
||||||
router http.Handler
|
|
||||||
sess *session.Session
|
|
||||||
db *database.Database
|
|
||||||
}
|
|
||||||
|
|
||||||
// newTestEnv wires the dependency graph with fx and builds the
|
|
||||||
// production route tree, so middleware registration order is
|
|
||||||
// exercised exactly as it ships.
|
|
||||||
func newTestEnv(t *testing.T) *testEnv {
|
|
||||||
t.Helper()
|
|
||||||
|
|
||||||
var (
|
|
||||||
log *logger.Logger
|
|
||||||
cfg *config.Config
|
|
||||||
mw *middleware.Middleware
|
|
||||||
hnd *handlers.Handlers
|
|
||||||
sess *session.Session
|
|
||||||
db *database.Database
|
|
||||||
)
|
|
||||||
|
|
||||||
app := fxtest.New(
|
|
||||||
t,
|
|
||||||
fx.Provide(
|
|
||||||
globals.New,
|
|
||||||
logger.New,
|
|
||||||
func() *config.Config {
|
|
||||||
return &config.Config{
|
|
||||||
DataDir: t.TempDir(),
|
|
||||||
Environment: config.EnvironmentDev,
|
|
||||||
}
|
|
||||||
},
|
|
||||||
database.New,
|
|
||||||
database.NewWebhookDBManager,
|
|
||||||
healthcheck.New,
|
|
||||||
session.New,
|
|
||||||
func() delivery.Notifier { return &noopNotifier{} },
|
|
||||||
middleware.New,
|
|
||||||
handlers.New,
|
|
||||||
),
|
|
||||||
fx.Populate(&log, &cfg, &mw, &hnd, &sess, &db),
|
|
||||||
)
|
|
||||||
app.RequireStart()
|
|
||||||
t.Cleanup(app.RequireStop)
|
|
||||||
|
|
||||||
return &testEnv{
|
|
||||||
router: server.NewRouterForTest(log.Get(), cfg, mw, hnd),
|
|
||||||
sess: sess,
|
|
||||||
db: db,
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// oversizeValue returns a form value one byte past the route-group
|
|
||||||
// body cap, so an encoded form containing it is guaranteed oversize.
|
|
||||||
func oversizeValue() string {
|
|
||||||
return strings.Repeat("a", int(server.MaxFormBodySizeForTest)+1)
|
|
||||||
}
|
|
||||||
|
|
||||||
// csrfCookieSet reports whether the response issued a gorilla/csrf
|
|
||||||
// cookie, which only happens if the CSRF middleware ran.
|
|
||||||
func csrfCookieSet(w *httptest.ResponseRecorder) bool {
|
|
||||||
for _, c := range w.Result().Cookies() {
|
|
||||||
if c.Name == csrfCookieName {
|
|
||||||
return true
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
return false
|
|
||||||
}
|
|
||||||
|
|
||||||
// get issues a GET through the router with the supplied cookies.
|
|
||||||
func (e *testEnv) get(
|
|
||||||
path string,
|
|
||||||
cookies []*http.Cookie,
|
|
||||||
) *httptest.ResponseRecorder {
|
|
||||||
req := httptest.NewRequestWithContext(
|
|
||||||
context.Background(), http.MethodGet, path, nil,
|
|
||||||
)
|
|
||||||
|
|
||||||
for _, c := range cookies {
|
|
||||||
req.AddCookie(c)
|
|
||||||
}
|
|
||||||
|
|
||||||
w := httptest.NewRecorder()
|
|
||||||
e.router.ServeHTTP(w, req)
|
|
||||||
|
|
||||||
return w
|
|
||||||
}
|
|
||||||
|
|
||||||
// post issues a urlencoded form POST through the router. The body is
|
|
||||||
// a strings.Reader, so the request carries an accurate
|
|
||||||
// Content-Length — the signal MaxBodySize checks up front.
|
|
||||||
func (e *testEnv) post(
|
|
||||||
path string,
|
|
||||||
form url.Values,
|
|
||||||
cookies []*http.Cookie,
|
|
||||||
) *httptest.ResponseRecorder {
|
|
||||||
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)
|
|
||||||
}
|
|
||||||
|
|
||||||
w := httptest.NewRecorder()
|
|
||||||
e.router.ServeHTTP(w, req)
|
|
||||||
|
|
||||||
return w
|
|
||||||
}
|
|
||||||
|
|
||||||
// csrfFrom renders the page at path and returns the CSRF token from
|
|
||||||
// its form together with every cookie needed for the follow-up POST.
|
|
||||||
func (e *testEnv) csrfFrom(
|
|
||||||
t *testing.T,
|
|
||||||
path string,
|
|
||||||
cookies []*http.Cookie,
|
|
||||||
) (string, []*http.Cookie) {
|
|
||||||
t.Helper()
|
|
||||||
|
|
||||||
w := e.get(path, cookies)
|
|
||||||
require.Equal(t, http.StatusOK, w.Code)
|
|
||||||
|
|
||||||
pattern := regexp.MustCompile(
|
|
||||||
`name="csrf_token" value="([^"]+)"`,
|
|
||||||
)
|
|
||||||
|
|
||||||
match := pattern.FindStringSubmatch(w.Body.String())
|
|
||||||
require.Len(t, match, 2, "form must embed a CSRF token")
|
|
||||||
|
|
||||||
// html/template escapes "+" and "=" in attribute values, and
|
|
||||||
// gorilla/csrf tokens are standard base64, so the value read
|
|
||||||
// out of the markup has to be unescaped before it is submitted.
|
|
||||||
token := html.UnescapeString(match[1])
|
|
||||||
|
|
||||||
combined := make([]*http.Cookie, 0, len(cookies))
|
|
||||||
combined = append(combined, cookies...)
|
|
||||||
combined = append(combined, w.Result().Cookies()...)
|
|
||||||
|
|
||||||
return token, combined
|
|
||||||
}
|
|
||||||
|
|
||||||
// authCookies forges an authenticated session for the given user.
|
|
||||||
func (e *testEnv) authCookies(
|
|
||||||
t *testing.T,
|
|
||||||
userID, username string,
|
|
||||||
) []*http.Cookie {
|
|
||||||
t.Helper()
|
|
||||||
|
|
||||||
req := httptest.NewRequestWithContext(
|
|
||||||
context.Background(), http.MethodGet, "/setup", nil,
|
|
||||||
)
|
|
||||||
w := httptest.NewRecorder()
|
|
||||||
|
|
||||||
s, err := e.sess.Get(req)
|
|
||||||
require.NoError(t, err)
|
|
||||||
|
|
||||||
e.sess.SetUser(s, userID, username)
|
|
||||||
require.NoError(t, e.sess.Save(req, w, s))
|
|
||||||
|
|
||||||
cookies := w.Result().Cookies()
|
|
||||||
require.NotEmpty(t, cookies, "session cookie should be set")
|
|
||||||
|
|
||||||
return cookies
|
|
||||||
}
|
|
||||||
|
|
||||||
// seedUser creates a user with the given password and returns the
|
|
||||||
// stored hash so tests can assert whether it later changed.
|
|
||||||
func (e *testEnv) seedUser(
|
|
||||||
t *testing.T,
|
|
||||||
username, password string,
|
|
||||||
) (string, string) {
|
|
||||||
t.Helper()
|
|
||||||
|
|
||||||
hash, err := database.HashPassword(password)
|
|
||||||
require.NoError(t, err)
|
|
||||||
|
|
||||||
user := &database.User{Username: username, Password: hash}
|
|
||||||
require.NoError(t, e.db.DB().Create(user).Error)
|
|
||||||
|
|
||||||
return user.ID, hash
|
|
||||||
}
|
|
||||||
|
|
||||||
// storedHash reads the current password hash for a username.
|
|
||||||
func (e *testEnv) storedHash(t *testing.T, username string) string {
|
|
||||||
t.Helper()
|
|
||||||
|
|
||||||
var user database.User
|
|
||||||
|
|
||||||
require.NoError(t,
|
|
||||||
e.db.DB().Where("username = ?", username).
|
|
||||||
First(&user).Error,
|
|
||||||
)
|
|
||||||
|
|
||||||
return user.Password
|
|
||||||
}
|
|
||||||
|
|
||||||
// --- /pages group ---
|
|
||||||
|
|
||||||
// TestPagesLogin_OversizeBody_RejectedBeforeCSRF proves the cap runs
|
|
||||||
// ahead of gorilla/csrf: the response is a clean 413 and no CSRF
|
|
||||||
// cookie was issued, so neither the CSRF middleware nor the login
|
|
||||||
// handler ran.
|
|
||||||
func TestPagesLogin_OversizeBody_RejectedBeforeCSRF(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
env := newTestEnv(t)
|
|
||||||
|
|
||||||
form := url.Values{}
|
|
||||||
form.Set("username", oversizeValue())
|
|
||||||
form.Set("password", "irrelevant")
|
|
||||||
|
|
||||||
w := env.post("/pages/login", form, nil)
|
|
||||||
|
|
||||||
assert.Equal(
|
|
||||||
t, http.StatusRequestEntityTooLarge, w.Code,
|
|
||||||
)
|
|
||||||
assert.False(
|
|
||||||
t, csrfCookieSet(w),
|
|
||||||
"CSRF middleware must not run for an oversized body",
|
|
||||||
)
|
|
||||||
}
|
|
||||||
|
|
||||||
// TestPagesLogin_UnderLimit_NoToken_CSRFRejects is the control for
|
|
||||||
// the test above: an identically shaped but under-limit POST does
|
|
||||||
// reach gorilla/csrf, which rejects it and issues its cookie. Without
|
|
||||||
// this, the missing-cookie assertion above would prove nothing.
|
|
||||||
func TestPagesLogin_UnderLimit_NoToken_CSRFRejects(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
env := newTestEnv(t)
|
|
||||||
|
|
||||||
form := url.Values{}
|
|
||||||
form.Set("username", "someone")
|
|
||||||
form.Set("password", "irrelevant")
|
|
||||||
|
|
||||||
w := env.post("/pages/login", form, nil)
|
|
||||||
|
|
||||||
assert.Equal(t, http.StatusForbidden, w.Code)
|
|
||||||
assert.True(
|
|
||||||
t, csrfCookieSet(w),
|
|
||||||
"CSRF middleware should run for an under-limit body",
|
|
||||||
)
|
|
||||||
}
|
|
||||||
|
|
||||||
// TestPagesLogin_UnderLimit_ValidToken_ReachesHandler proves the
|
|
||||||
// reorder did not break CSRF token handling: a token harvested from
|
|
||||||
// the rendered login form is still accepted and the request lands in
|
|
||||||
// the handler.
|
|
||||||
func TestPagesLogin_UnderLimit_ValidToken_ReachesHandler(
|
|
||||||
t *testing.T,
|
|
||||||
) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
env := newTestEnv(t)
|
|
||||||
|
|
||||||
token, cookies := env.csrfFrom(t, "/pages/login", nil)
|
|
||||||
|
|
||||||
form := url.Values{}
|
|
||||||
form.Set("csrf_token", token)
|
|
||||||
form.Set("username", "nosuchuser")
|
|
||||||
form.Set("password", "wrongpassword")
|
|
||||||
|
|
||||||
w := env.post("/pages/login", form, cookies)
|
|
||||||
|
|
||||||
assert.Equal(t, http.StatusUnauthorized, w.Code)
|
|
||||||
assert.Contains(
|
|
||||||
t, w.Body.String(), "Invalid username or password",
|
|
||||||
"request should reach the login handler",
|
|
||||||
)
|
|
||||||
}
|
|
||||||
|
|
||||||
// --- /user/{username} group ---
|
|
||||||
|
|
||||||
// TestPasswordChange_OversizeBody_RejectedAndPasswordUnchanged
|
|
||||||
// covers the route that previously had no middleware body cap at
|
|
||||||
// all. The request carries a valid session and a valid CSRF token,
|
|
||||||
// so the only thing that can stop it is the size cap; the unchanged
|
|
||||||
// password hash is the observable proof the handler never ran.
|
|
||||||
func TestPasswordChange_OversizeBody_RejectedAndPasswordUnchanged(
|
|
||||||
t *testing.T,
|
|
||||||
) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
env := newTestEnv(t)
|
|
||||||
|
|
||||||
userID, originalHash := env.seedUser(t, "pwuser", "oldpassword")
|
|
||||||
cookies := env.authCookies(t, userID, "pwuser")
|
|
||||||
token, cookies := env.csrfFrom(t, "/user/pwuser/", cookies)
|
|
||||||
|
|
||||||
form := url.Values{}
|
|
||||||
form.Set("csrf_token", token)
|
|
||||||
form.Set("current_password", "oldpassword")
|
|
||||||
form.Set("new_password", oversizeValue())
|
|
||||||
form.Set("confirm_password", oversizeValue())
|
|
||||||
|
|
||||||
w := env.post("/user/pwuser/password", form, cookies)
|
|
||||||
|
|
||||||
assert.Equal(
|
|
||||||
t, http.StatusRequestEntityTooLarge, w.Code,
|
|
||||||
)
|
|
||||||
assert.Equal(
|
|
||||||
t, originalHash, env.storedHash(t, "pwuser"),
|
|
||||||
"handler must not run, so the password must be unchanged",
|
|
||||||
)
|
|
||||||
}
|
|
||||||
|
|
||||||
// TestPasswordChange_UnderLimit_Succeeds proves that adding the cap
|
|
||||||
// to the /user/{username} group did not break the route it guards.
|
|
||||||
func TestPasswordChange_UnderLimit_Succeeds(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
env := newTestEnv(t)
|
|
||||||
|
|
||||||
userID, originalHash := env.seedUser(t, "okuser", "oldpassword")
|
|
||||||
cookies := env.authCookies(t, userID, "okuser")
|
|
||||||
token, cookies := env.csrfFrom(t, "/user/okuser/", cookies)
|
|
||||||
|
|
||||||
form := url.Values{}
|
|
||||||
form.Set("csrf_token", token)
|
|
||||||
form.Set("current_password", "oldpassword")
|
|
||||||
form.Set("new_password", "brandnewpassword")
|
|
||||||
form.Set("confirm_password", "brandnewpassword")
|
|
||||||
|
|
||||||
w := env.post("/user/okuser/password", form, cookies)
|
|
||||||
|
|
||||||
assert.Equal(t, http.StatusOK, w.Code)
|
|
||||||
assert.NotEqual(
|
|
||||||
t, originalHash, env.storedHash(t, "okuser"),
|
|
||||||
"an under-limit password change should still apply",
|
|
||||||
)
|
|
||||||
}
|
|
||||||
Reference in New Issue
Block a user