Keep the SSRF-safe transport in clientForConfig (closes #69) #74
@@ -17,11 +17,13 @@ func (h *Handlers) HandleProfile() http.HandlerFunc {
|
|||||||
return
|
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)
|
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
|
return
|
||||||
}
|
}
|
||||||
|
|||||||
159
internal/handlers/profile_test.go
Normal file
159
internal/handlers/profile_test.go
Normal file
@@ -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"))
|
||||||
|
}
|
||||||
@@ -13,8 +13,11 @@ const (
|
|||||||
httpReadTimeout = 10 * time.Second
|
httpReadTimeout = 10 * time.Second
|
||||||
|
|
||||||
// httpWriteTimeout is the maximum duration before timing out
|
// 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
|
// httpMaxHeaderBytes is the maximum number of bytes the
|
||||||
// server will read parsing the request headers.
|
// server will read parsing the request headers.
|
||||||
|
|||||||
@@ -106,6 +106,7 @@ 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) {
|
||||||
r.Use(s.mw.CSRF())
|
r.Use(s.mw.CSRF())
|
||||||
|
r.Use(s.mw.RequireAuth())
|
||||||
r.Get("/", s.h.HandleProfile())
|
r.Get("/", s.h.HandleProfile())
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user