From 0c9c885d51a4e78f2fee0d47d89be245cd02d9b8 Mon Sep 17 00:00:00 2001 From: clawbot Date: Fri, 7 Aug 2026 13:58:28 +0200 Subject: [PATCH 1/2] Raise HTTP WriteTimeout above the request middleware timeout (closes #62) (#72) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Raise `httpWriteTimeout` in `internal/server/http.go` from 10s to `65 * time.Second` so it comfortably exceeds the router's 60s `requestTimeout`. This makes the `middleware.Timeout(60s)` the effective request limit — a slow response now returns a clean 503 from the middleware instead of being cut at the socket write deadline by the transport. `httpReadTimeout` stays at 10s. A comment on `httpWriteTimeout` documents that it must remain above the 60s request timeout. Change is confined to `internal/server/http.go`; `routes.go` is untouched. Closes #62 Co-authored-by: sneak Reviewed-on: https://git.eeqj.de/sneak/webhooker/pulls/72 Co-authored-by: clawbot Co-committed-by: clawbot --- internal/server/http.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/internal/server/http.go b/internal/server/http.go index efb7705..4c99ba5 100644 --- a/internal/server/http.go +++ b/internal/server/http.go @@ -13,8 +13,11 @@ const ( httpReadTimeout = 10 * time.Second // httpWriteTimeout is the maximum duration before timing out - // writes of the response. - httpWriteTimeout = 10 * time.Second + // writes of the response. It must stay above the router's + // requestTimeout (60s, in routes.go) so the middleware timeout + // fires first and returns a clean 503, rather than the transport + // cutting the connection at the socket write deadline. + httpWriteTimeout = 65 * time.Second // httpMaxHeaderBytes is the maximum number of bytes the // server will read parsing the request headers. From 07fc63d9fad792201b306d135d4a619d918c0129 Mon Sep 17 00:00:00 2001 From: clawbot Date: Fri, 7 Aug 2026 14:00:16 +0200 Subject: [PATCH 2/2] Wrap /user/{username} in RequireAuth middleware (closes #60) (#71) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enforces authentication for the `/user/{username}` route group at the middleware layer, matching every other authenticated route group. ## Changes - **`internal/server/routes.go`** (`setupUserRoutes`): added `r.Use(s.mw.RequireAuth())` immediately after the existing `r.Use(s.mw.CSRF())` on the `/user/{username}` group, so auth is enforced by design (CSRF first, then RequireAuth) — consistent with `/sources` and `/source/{sourceID}`. - **`internal/handlers/profile.go`** (`HandleProfile`): removed the now-dead unauthenticated-redirect branch (RequireAuth guarantees an authenticated session before the handler runs). The handler still reads the username and user id from the session for the own-profile-only check; a request for another user's profile still returns 403. The session-retrieval error is now handled as a 500. ## Tests (`internal/handlers/profile_test.go`) - own profile returns 200 - another user's profile returns 403 - an unauthenticated request to `/user/{username}` is redirected to `/pages/login` at the middleware layer and never reaches the endpoint handler (routing-level test replicating the CSRF + RequireAuth chain) ## Validation `docker build .` (fmt-check, lint, test, build) passes. Closes #60 Co-authored-by: sneak Co-authored-by: Jeffrey Paul Reviewed-on: https://git.eeqj.de/sneak/webhooker/pulls/71 Co-authored-by: clawbot Co-committed-by: clawbot --- internal/handlers/profile.go | 10 +- internal/handlers/profile_test.go | 159 ++++++++++++++++++++++++++++++ internal/server/routes.go | 1 + 3 files changed, 166 insertions(+), 4 deletions(-) create mode 100644 internal/handlers/profile_test.go diff --git a/internal/handlers/profile.go b/internal/handlers/profile.go index 95c8a0f..40c9b50 100644 --- a/internal/handlers/profile.go +++ b/internal/handlers/profile.go @@ -17,11 +17,13 @@ func (h *Handlers) HandleProfile() http.HandlerFunc { return } - // Get session + // Get session. RequireAuth middleware guarantees an + // authenticated session before this handler runs, so we + // only need to guard against an unexpected retrieval error. sess, err := h.session.Get(r) - if err != nil || !h.session.IsAuthenticated(sess) { - // Redirect to login if not authenticated - http.Redirect(w, r, "/pages/login", http.StatusSeeOther) + if err != nil { + h.log.Error("failed to get session", "error", err) + http.Error(w, "Internal server error", http.StatusInternalServerError) return } diff --git a/internal/handlers/profile_test.go b/internal/handlers/profile_test.go new file mode 100644 index 0000000..7c1d282 --- /dev/null +++ b/internal/handlers/profile_test.go @@ -0,0 +1,159 @@ +package handlers_test + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/go-chi/chi" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sneak.berlin/go/webhooker/internal/config" + "sneak.berlin/go/webhooker/internal/handlers" + "sneak.berlin/go/webhooker/internal/logger" + "sneak.berlin/go/webhooker/internal/middleware" + "sneak.berlin/go/webhooker/internal/session" +) + +// authenticatedCookies creates an authenticated session for the given +// user and returns the resulting cookies for use on a later request. +func authenticatedCookies( + t *testing.T, + sess *session.Session, + userID, username string, +) []*http.Cookie { + t.Helper() + + setupReq := httptest.NewRequestWithContext( + context.Background(), http.MethodGet, "/setup", nil, + ) + setupW := httptest.NewRecorder() + + s, err := sess.Get(setupReq) + require.NoError(t, err) + + sess.SetUser(s, userID, username) + require.NoError(t, sess.Save(setupReq, setupW, s)) + + cookies := setupW.Result().Cookies() + require.NotEmpty(t, cookies, "session cookie should be set") + + return cookies +} + +// profileRequest builds a GET request for the given profile username, +// attaching the supplied cookies and the chi URL parameter that the +// handler reads via chi.URLParam. +func profileRequest( + username string, + cookies []*http.Cookie, +) *http.Request { + req := httptest.NewRequestWithContext( + context.Background(), http.MethodGet, "/user/"+username, nil, + ) + + for _, c := range cookies { + req.AddCookie(c) + } + + rctx := chi.NewRouteContext() + rctx.URLParams.Add("username", username) + + return req.WithContext( + context.WithValue(req.Context(), chi.RouteCtxKey, rctx), + ) +} + +func TestHandleProfile_OwnProfile_OK(t *testing.T) { + t.Parallel() + + var h *handlers.Handlers + + var sess *session.Session + + app := newTestApp(t, &h, &sess) + app.RequireStart() + + t.Cleanup(app.RequireStop) + + cookies := authenticatedCookies(t, sess, "test-user-id", "testuser") + + req := profileRequest("testuser", cookies) + w := httptest.NewRecorder() + + h.HandleProfile().ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) +} + +func TestHandleProfile_OtherProfile_Forbidden(t *testing.T) { + t.Parallel() + + var h *handlers.Handlers + + var sess *session.Session + + app := newTestApp(t, &h, &sess) + app.RequireStart() + + t.Cleanup(app.RequireStop) + + cookies := authenticatedCookies(t, sess, "test-user-id", "testuser") + + req := profileRequest("otheruser", cookies) + w := httptest.NewRecorder() + + h.HandleProfile().ServeHTTP(w, req) + + assert.Equal(t, http.StatusForbidden, w.Code) +} + +// TestUserRoute_Unauthenticated_RedirectedByMiddleware exercises the +// /user/{username} route group's middleware chain (CSRF then +// RequireAuth, matching setupUserRoutes) and proves that an +// unauthenticated request is redirected to /pages/login at the +// middleware layer, never reaching the endpoint handler. +func TestUserRoute_Unauthenticated_RedirectedByMiddleware(t *testing.T) { + t.Parallel() + + var log *logger.Logger + + var cfg *config.Config + + var sess *session.Session + + app := newTestApp(t, &log, &cfg, &sess) + app.RequireStart() + + t.Cleanup(app.RequireStop) + + mw := middleware.NewForTest(log.Get(), cfg, sess) + + var handlerReached bool + + router := chi.NewRouter() + router.Route("/user/{username}", func(r chi.Router) { + r.Use(mw.CSRF()) + r.Use(mw.RequireAuth()) + r.Get("/", func(w http.ResponseWriter, _ *http.Request) { + handlerReached = true + + w.WriteHeader(http.StatusOK) + }) + }) + + req := httptest.NewRequestWithContext( + context.Background(), http.MethodGet, "/user/testuser", nil, + ) + w := httptest.NewRecorder() + + router.ServeHTTP(w, req) + + assert.False( + t, handlerReached, + "handler must not be reached for unauthenticated request", + ) + assert.Equal(t, http.StatusSeeOther, w.Code) + assert.Equal(t, "/pages/login", w.Header().Get("Location")) +} diff --git a/internal/server/routes.go b/internal/server/routes.go index 82b67cd..adb9f16 100644 --- a/internal/server/routes.go +++ b/internal/server/routes.go @@ -106,6 +106,7 @@ func (s *Server) setupPageRoutes() { func (s *Server) setupUserRoutes() { s.router.Route("/user/{username}", func(r chi.Router) { r.Use(s.mw.CSRF()) + r.Use(s.mw.RequireAuth()) r.Get("/", s.h.HandleProfile()) }) }