Compare commits
1 Commits
df1f76b006
...
issue-90-b
| Author | SHA1 | Date | |
|---|---|---|---|
| 08c9c1a5d8 |
17
README.md
17
README.md
@@ -867,9 +867,17 @@ Applied to all routes in this order:
|
||||
8. **Sentry** — Error reporting to Sentry (if `SENTRY_DSN` is set;
|
||||
configured with `Repanic: true` so panics still reach Recoverer)
|
||||
|
||||
Additionally, form endpoints (`/pages`, `/sources`, `/source/*`) apply a
|
||||
**MaxBodySize** middleware that limits POST/PUT/PATCH request bodies to
|
||||
1 MB using `http.MaxBytesReader`, preventing oversized form submissions.
|
||||
Additionally, form endpoints (`/pages`, `/user/*`, `/sources`,
|
||||
`/source/*`) apply a **MaxBodySize** middleware that limits
|
||||
POST/PUT/PATCH request bodies to 1 MB. It is registered ahead of the
|
||||
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
|
||||
|
||||
@@ -891,7 +899,8 @@ Additionally, form endpoints (`/pages`, `/sources`, `/source/*`) apply a
|
||||
- Production security headers on all responses: HSTS, X-Content-Type-Options
|
||||
(`nosniff`), X-Frame-Options (`DENY`), Content-Security-Policy, Referrer-Policy,
|
||||
and Permissions-Policy
|
||||
- Request body size limits (1 MB) on all form POST endpoints
|
||||
- Request body size limits (1 MB) on all form POST endpoints, enforced
|
||||
by middleware that runs before CSRF parses the form
|
||||
- **CSRF protection** via [gorilla/csrf](https://github.com/gorilla/csrf)
|
||||
on all state-changing forms (cookie-based double-submit tokens with
|
||||
HMAC authentication). Applied to `/pages`, `/sources`, `/source`, and
|
||||
|
||||
7
TODO.md
7
TODO.md
@@ -28,6 +28,13 @@ databases currently grow without bound.
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 2026-08-09 Enforce the request body size limit before the CSRF
|
||||
middleware parses the form (#90): `MaxBodySize` is now registered
|
||||
ahead of `CSRF()` in every form route group, the `/user/{username}`
|
||||
group gained the cap it never had (which is where `POST /password`
|
||||
lives), the middleware rejects a declared-oversize body with a real
|
||||
413 up front, and the redundant handler-local
|
||||
`http.MaxBytesReader` calls were removed
|
||||
- 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
|
||||
|
||||
@@ -29,10 +29,8 @@ func (h *Handlers) HandleLoginPage() http.HandlerFunc {
|
||||
// HandleLoginSubmit handles the login form submission (POST)
|
||||
func (h *Handlers) HandleLoginSubmit() http.HandlerFunc {
|
||||
return func(w http.ResponseWriter, r *http.Request) {
|
||||
// Limit request body to prevent memory exhaustion
|
||||
r.Body = http.MaxBytesReader(w, r.Body, 1<<maxBodyShift)
|
||||
|
||||
// Parse form data
|
||||
// The body size cap is enforced by the MaxBodySize
|
||||
// middleware, which runs before CSRF parses the form.
|
||||
err := r.ParseForm()
|
||||
if err != nil {
|
||||
h.log.Error("failed to parse form", "error", err)
|
||||
|
||||
@@ -31,9 +31,8 @@ func (h *Handlers) HandlePasswordChange() http.HandlerFunc {
|
||||
return
|
||||
}
|
||||
|
||||
// Limit request body to prevent memory exhaustion.
|
||||
r.Body = http.MaxBytesReader(w, r.Body, 1<<maxBodyShift)
|
||||
|
||||
// The body size cap is enforced by the MaxBodySize
|
||||
// middleware, which runs before CSRF parses the form.
|
||||
err := r.ParseForm()
|
||||
if err != nil {
|
||||
h.log.Error("failed to parse form", "error", err)
|
||||
|
||||
@@ -127,10 +127,8 @@ func (h *Handlers) HandleSourceCreateSubmit() http.HandlerFunc {
|
||||
return
|
||||
}
|
||||
|
||||
r.Body = http.MaxBytesReader(
|
||||
w, r.Body, 1<<maxBodyShift,
|
||||
)
|
||||
|
||||
// The body size cap is enforced by the MaxBodySize
|
||||
// middleware, which runs before CSRF parses the form.
|
||||
err := r.ParseForm()
|
||||
if err != nil {
|
||||
http.Error(
|
||||
@@ -386,10 +384,8 @@ func (h *Handlers) HandleSourceEditSubmit() http.HandlerFunc {
|
||||
return
|
||||
}
|
||||
|
||||
r.Body = http.MaxBytesReader(
|
||||
w, r.Body, 1<<maxBodyShift,
|
||||
)
|
||||
|
||||
// The body size cap is enforced by the MaxBodySize
|
||||
// middleware, which runs before CSRF parses the form.
|
||||
err = r.ParseForm()
|
||||
if err != nil {
|
||||
http.Error(
|
||||
@@ -409,10 +405,8 @@ func (h *Handlers) applyWebhookEdit(
|
||||
r *http.Request,
|
||||
webhook *database.Webhook,
|
||||
) {
|
||||
r.Body = http.MaxBytesReader(
|
||||
w, r.Body, 1<<maxBodyShift,
|
||||
)
|
||||
|
||||
// The body size cap is enforced by the MaxBodySize middleware,
|
||||
// which runs before CSRF parses the form.
|
||||
name := r.FormValue("name")
|
||||
if name == "" {
|
||||
data := map[string]any{
|
||||
@@ -725,10 +719,8 @@ func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
|
||||
return
|
||||
}
|
||||
|
||||
r.Body = http.MaxBytesReader(
|
||||
w, r.Body, 1<<maxBodyShift,
|
||||
)
|
||||
|
||||
// The body size cap is enforced by the MaxBodySize
|
||||
// middleware, which runs before CSRF parses the form.
|
||||
err = r.ParseForm()
|
||||
if err != nil {
|
||||
http.Error(
|
||||
@@ -785,10 +777,8 @@ func (h *Handlers) HandleTargetCreate() http.HandlerFunc {
|
||||
return
|
||||
}
|
||||
|
||||
r.Body = http.MaxBytesReader(
|
||||
w, r.Body, 1<<maxBodyShift,
|
||||
)
|
||||
|
||||
// The body size cap is enforced by the MaxBodySize
|
||||
// middleware, which runs before CSRF parses the form.
|
||||
err = r.ParseForm()
|
||||
if err != nil {
|
||||
http.Error(
|
||||
@@ -808,10 +798,8 @@ func (h *Handlers) processTargetCreate(
|
||||
r *http.Request,
|
||||
webhook database.Webhook,
|
||||
) {
|
||||
r.Body = http.MaxBytesReader(
|
||||
w, r.Body, 1<<maxBodyShift,
|
||||
)
|
||||
|
||||
// The body size cap is enforced by the MaxBodySize middleware,
|
||||
// which runs before CSRF parses the form.
|
||||
name := r.FormValue("name")
|
||||
targetType := database.TargetType(r.FormValue("type"))
|
||||
targetURL := r.FormValue("url")
|
||||
|
||||
@@ -285,10 +285,36 @@ func (s *Middleware) NoCache() func(http.Handler) http.Handler {
|
||||
}
|
||||
}
|
||||
|
||||
// MaxBodySize returns middleware that limits the request body size
|
||||
// for POST requests. If the body exceeds the given limit in
|
||||
// bytes, the server returns 413 Request Entity Too Large. This
|
||||
// prevents clients from sending arbitrarily large form bodies.
|
||||
// bodyLimitedMethod reports whether the request method carries a
|
||||
// body that the MaxBodySize middleware should cap.
|
||||
func bodyLimitedMethod(method string) bool {
|
||||
return method == http.MethodPost ||
|
||||
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(
|
||||
maxBytes int64,
|
||||
) func(http.Handler) http.Handler {
|
||||
@@ -297,14 +323,31 @@ func (s *Middleware) MaxBodySize(
|
||||
w http.ResponseWriter,
|
||||
r *http.Request,
|
||||
) {
|
||||
if r.Method == http.MethodPost ||
|
||||
r.Method == http.MethodPut ||
|
||||
r.Method == http.MethodPatch {
|
||||
r.Body = http.MaxBytesReader(
|
||||
w, r.Body, maxBytes,
|
||||
)
|
||||
if !bodyLimitedMethod(r.Method) {
|
||||
next.ServeHTTP(w, r)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
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)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -3,10 +3,12 @@ package middleware_test
|
||||
import (
|
||||
"context"
|
||||
"encoding/base64"
|
||||
"io"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/gorilla/sessions"
|
||||
@@ -426,6 +428,153 @@ 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 ---
|
||||
|
||||
func TestIpFromHostPort(t *testing.T) {
|
||||
|
||||
36
internal/server/export_test.go
Normal file
36
internal/server/export_test.go
Normal file
@@ -0,0 +1,36 @@
|
||||
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,9 +90,11 @@ func (s *Server) setupRoutes() {
|
||||
|
||||
func (s *Server) setupPageRoutes() {
|
||||
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.NoCache())
|
||||
r.Use(s.mw.MaxBodySize(maxFormBodySize))
|
||||
|
||||
r.Group(func(r chi.Router) {
|
||||
r.Use(s.mw.LoginRateLimit())
|
||||
@@ -106,6 +108,9 @@ func (s *Server) setupPageRoutes() {
|
||||
|
||||
func (s *Server) setupUserRoutes() {
|
||||
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.NoCache())
|
||||
r.Use(s.mw.RequireAuth())
|
||||
@@ -118,20 +123,24 @@ func (s *Server) setupUserRoutes() {
|
||||
|
||||
func (s *Server) setupSourceRoutes() {
|
||||
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.NoCache())
|
||||
r.Use(s.mw.RequireAuth())
|
||||
r.Use(s.mw.MaxBodySize(maxFormBodySize))
|
||||
r.Get("/", s.h.HandleSourceList())
|
||||
r.Get("/new", s.h.HandleSourceCreate())
|
||||
r.Post("/new", s.h.HandleSourceCreateSubmit())
|
||||
})
|
||||
|
||||
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.NoCache())
|
||||
r.Use(s.mw.RequireAuth())
|
||||
r.Use(s.mw.MaxBodySize(maxFormBodySize))
|
||||
r.Get("/", s.h.HandleSourceDetail())
|
||||
r.Get("/edit", s.h.HandleSourceEdit())
|
||||
r.Post("/edit", s.h.HandleSourceEditSubmit())
|
||||
|
||||
375
internal/server/routes_test.go
Normal file
375
internal/server/routes_test.go
Normal file
@@ -0,0 +1,375 @@
|
||||
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