From 326629494e3792c77526afb2d7b0f5751f32b4f8 Mon Sep 17 00:00:00 2001 From: sneak Date: Fri, 7 Aug 2026 18:48:28 +0700 Subject: [PATCH] Wrap /user/{username} in RequireAuth middleware (closes #60) --- 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()) }) }