Merge branch 'main' into issue-68-slack-url-validation

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) {
+		if err != nil {
-			// Redirect to login if not authenticated
+			h.log.Error("failed to get session", "error", err)
-			http.Redirect(w, r, "/pages/login", http.StatusSeeOther)
+			http.Error(w, "Internal server error", http.StatusInternalServerError)
 
 			return
 		}

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"))
+}

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.
+	// writes of the response. It must stay above the router's
-	httpWriteTimeout = 10 * time.Second
+	// 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.

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())
 	})
 }