From 3084ed545b8ff2b869d732d561a3dfc105bee594 Mon Sep 17 00:00:00 2001 From: sneak Date: Fri, 7 Aug 2026 22:54:44 +0700 Subject: [PATCH 1/2] Add admin password change flow (closes #65) --- internal/handlers/profile.go | 237 ++++++++++++++++++++++++------ internal/handlers/profile_test.go | 134 +++++++++++++++++ internal/server/routes.go | 1 + templates/profile.html | 56 +++++++ 4 files changed, 379 insertions(+), 49 deletions(-) diff --git a/internal/handlers/profile.go b/internal/handlers/profile.go index 40c9b50..abf39b9 100644 --- a/internal/handlers/profile.go +++ b/internal/handlers/profile.go @@ -4,63 +4,202 @@ import ( "net/http" "github.com/go-chi/chi" + "sneak.berlin/go/webhooker/internal/database" ) // HandleProfile returns a handler for the user profile page func (h *Handlers) HandleProfile() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - // Get username from URL - requestedUsername := chi.URLParam(r, "username") - if requestedUsername == "" { - http.NotFound(w, r) - - return - } - - // 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.log.Error("failed to get session", "error", err) - http.Error(w, "Internal server error", http.StatusInternalServerError) - - return - } - - // Get user info from session - sessionUsername, ok := h.session.GetUsername(sess) + sessionUserID, sessionUsername, ok := + h.profileOwnerOrDeny(w, r) if !ok { - h.log.Error("authenticated session missing username") - http.Error(w, "Internal server error", http.StatusInternalServerError) - return } - sessionUserID, ok := h.session.GetUserID(sess) - if !ok { - h.log.Error("authenticated session missing user ID") - http.Error(w, "Internal server error", http.StatusInternalServerError) - - return - } - - // For now, only allow users to view their own profile - if requestedUsername != sessionUsername { - http.Error(w, "Forbidden", http.StatusForbidden) - - return - } - - // Prepare data for template - data := map[string]any{ - "User": &UserInfo{ - ID: sessionUserID, - Username: sessionUsername, - }, - } - - // Render the profile page - h.renderTemplate(w, r, "profile.html", data) + h.renderProfile(w, r, sessionUserID, sessionUsername, "", "") } } + +// HandlePasswordChange returns a handler that lets an authenticated +// user change their own password. It is served by the CSRF- and +// auth-protected POST /password route under /user/{username}. +func (h *Handlers) HandlePasswordChange() http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + sessionUserID, sessionUsername, ok := + h.profileOwnerOrDeny(w, r) + if !ok { + return + } + + // Limit request body to prevent memory exhaustion. + r.Body = http.MaxBytesReader(w, r.Body, 1<

User Profile

+ {{if .SuccessMessage}} +
+ {{.SuccessMessage}} +
+ {{end}} + + {{if .ErrorMessage}} +
+ {{.ErrorMessage}} +
+ {{end}} +
@@ -43,6 +55,50 @@
+
+

Change Password

+
+ +
+ + +
+ +
+ + +
+ +
+ + +
+ + +
+
+ -- 2.49.1 From 8362ce9ee066fcc5104b129eb097e620eb3e163e Mon Sep 17 00:00:00 2001 From: sneak Date: Fri, 7 Aug 2026 16:29:43 +0000 Subject: [PATCH 2/2] Rate-limit the password change endpoint (#65) The password change POST verifies the current password, making it a password-based authentication endpoint that REPO_POLICIES.md requires rate limiting on. Extract the login limiter's POST-only per-IP pattern into a shared postRateLimit helper and apply the same 5-per-minute limit to POST /user/{username}/password. --- internal/middleware/export_test.go | 4 ++ internal/middleware/ratelimit.go | 55 ++++++++++++++++++++++--- internal/middleware/ratelimit_test.go | 58 +++++++++++++++++++++------ internal/server/routes.go | 4 +- 4 files changed, 102 insertions(+), 19 deletions(-) diff --git a/internal/middleware/export_test.go b/internal/middleware/export_test.go index 504cb16..2752f1c 100644 --- a/internal/middleware/export_test.go +++ b/internal/middleware/export_test.go @@ -32,3 +32,7 @@ func IsClientTLS(r *http.Request) bool { // LoginRateLimitConst exposes the loginRateLimit constant. const LoginRateLimitConst = loginRateLimit + +// PasswordChangeRateLimitConst exposes the +// passwordChangeRateLimit constant. +const PasswordChangeRateLimitConst = passwordChangeRateLimit diff --git a/internal/middleware/ratelimit.go b/internal/middleware/ratelimit.go index 5076be7..f3ae469 100644 --- a/internal/middleware/ratelimit.go +++ b/internal/middleware/ratelimit.go @@ -14,6 +14,16 @@ const ( // loginRateInterval is the time window for the rate limit. loginRateInterval = 1 * time.Minute + + // passwordChangeRateLimit is the maximum number of password + // change attempts per interval. Each attempt verifies the + // current password, so the endpoint must be rate-limited + // like any other password-based authentication endpoint. + passwordChangeRateLimit = 5 + + // passwordChangeRateInterval is the time window for the + // password change rate limit. + passwordChangeRateInterval = 1 * time.Minute ) // LoginRateLimit returns middleware that enforces per-IP rate @@ -24,19 +34,53 @@ const ( // honours X-Forwarded-For, X-Real-IP, and True-Client-IP headers // for reverse-proxy setups. func (m *Middleware) LoginRateLimit() func(http.Handler) http.Handler { - limiter := httprate.Limit( + return m.postRateLimit( loginRateLimit, loginRateInterval, + "login rate limit exceeded", + "Too many login attempts. Please try again later.", + ) +} + +// PasswordChangeRateLimit returns middleware that enforces +// per-IP rate limiting on password change attempts. The change +// endpoint verifies the current password, so without a limit a +// stolen session could be used to brute-force it; the limit +// matches the login endpoint's. +func (m *Middleware) PasswordChangeRateLimit() func(http.Handler) http.Handler { + return m.postRateLimit( + passwordChangeRateLimit, + passwordChangeRateInterval, + "password change rate limit exceeded", + "Too many password change attempts. "+ + "Please try again later.", + ) +} + +// postRateLimit builds middleware that enforces a per-IP rate +// limit on POST requests only; all other methods pass through +// unaffected. Requests over the limit receive a 429 with the +// given response message, and each rejection is logged with the +// given log message. IP extraction honours X-Forwarded-For, +// X-Real-IP, and True-Client-IP headers for reverse-proxy +// setups. +func (m *Middleware) postRateLimit( + limit int, + interval time.Duration, + logMessage, responseMessage string, +) func(http.Handler) http.Handler { + limiter := httprate.Limit( + limit, + interval, httprate.WithKeyFuncs(httprate.KeyByRealIP), httprate.WithLimitHandler(http.HandlerFunc( func(w http.ResponseWriter, r *http.Request) { - m.log.Warn("login rate limit exceeded", + m.log.Warn(logMessage, "path", r.URL.Path, ) http.Error( w, - "Too many login attempts. "+ - "Please try again later.", + responseMessage, http.StatusTooManyRequests, ) }, @@ -50,8 +94,7 @@ func (m *Middleware) LoginRateLimit() func(http.Handler) http.Handler { w http.ResponseWriter, r *http.Request, ) { - // Only rate-limit POST requests (actual login - // attempts) + // Only rate-limit POST requests. if r.Method != http.MethodPost { next.ServeHTTP(w, r) diff --git a/internal/middleware/ratelimit_test.go b/internal/middleware/ratelimit_test.go index 731903a..c0f7209 100644 --- a/internal/middleware/ratelimit_test.go +++ b/internal/middleware/ratelimit_test.go @@ -46,14 +46,20 @@ func TestLoginRateLimit_AllowsGET(t *testing.T) { assert.Equal(t, 20, callCount) } -func TestLoginRateLimit_LimitsPOST(t *testing.T) { - t.Parallel() - - m, _ := testMiddleware(t, config.EnvironmentDev) +// runPostLimitTest exercises a POST-only rate limit middleware: +// the first limit POSTs to path from ip must pass, and the next +// one must be rejected with 429 without reaching the handler. +func runPostLimitTest( + t *testing.T, + mw func(http.Handler) http.Handler, + limit int, + path, ip string, +) { + t.Helper() var callCount int - handler := m.LoginRateLimit()(http.HandlerFunc( + handler := mw(http.HandlerFunc( func(w http.ResponseWriter, _ *http.Request) { callCount++ @@ -61,13 +67,13 @@ func TestLoginRateLimit_LimitsPOST(t *testing.T) { }, )) - // First loginRateLimit POST requests should succeed - for i := range middleware.LoginRateLimitConst { + // The first limit POST requests should succeed + for i := range limit { req := httptest.NewRequestWithContext( context.Background(), - http.MethodPost, "/pages/login", nil, + http.MethodPost, path, nil, ) - req.RemoteAddr = "10.0.0.1:12345" + req.RemoteAddr = ip w := httptest.NewRecorder() handler.ServeHTTP(w, req) @@ -81,9 +87,9 @@ func TestLoginRateLimit_LimitsPOST(t *testing.T) { // Next POST should be rate-limited req := httptest.NewRequestWithContext( context.Background(), - http.MethodPost, "/pages/login", nil, + http.MethodPost, path, nil, ) - req.RemoteAddr = "10.0.0.1:12345" + req.RemoteAddr = ip w := httptest.NewRecorder() handler.ServeHTTP(w, req) @@ -92,7 +98,35 @@ func TestLoginRateLimit_LimitsPOST(t *testing.T) { t, http.StatusTooManyRequests, w.Code, "POST after limit should be 429", ) - assert.Equal(t, middleware.LoginRateLimitConst, callCount) + assert.Equal(t, limit, callCount) +} + +func TestLoginRateLimit_LimitsPOST(t *testing.T) { + t.Parallel() + + m, _ := testMiddleware(t, config.EnvironmentDev) + + runPostLimitTest( + t, + m.LoginRateLimit(), + middleware.LoginRateLimitConst, + "/pages/login", + "10.0.0.1:12345", + ) +} + +func TestPasswordChangeRateLimit_LimitsPOST(t *testing.T) { + t.Parallel() + + m, _ := testMiddleware(t, config.EnvironmentDev) + + runPostLimitTest( + t, + m.PasswordChangeRateLimit(), + middleware.PasswordChangeRateLimitConst, + "/user/admin/password", + "10.0.0.2:12345", + ) } func TestLoginRateLimit_IndependentPerIP(t *testing.T) { diff --git a/internal/server/routes.go b/internal/server/routes.go index ef449ea..71e7a32 100644 --- a/internal/server/routes.go +++ b/internal/server/routes.go @@ -110,7 +110,9 @@ func (s *Server) setupUserRoutes() { r.Use(s.mw.NoCache()) r.Use(s.mw.RequireAuth()) r.Get("/", s.h.HandleProfile()) - r.Post("/password", s.h.HandlePasswordChange()) + r.With(s.mw.PasswordChangeRateLimit()).Post( + "/password", s.h.HandlePasswordChange(), + ) }) } -- 2.49.1