From 8d9d9da6f515951607ff14a610944b01a2867b5a Mon Sep 17 00:00:00 2001
From: Kyle Isom <kyle@imap.cc>
Date: Thu, 12 Mar 2026 21:59:02 -0700
Subject: [PATCH] fix UI privilege escalation vulnerability

- Add requireAdminRole middleware to web UI that checks
  claims.HasRole("admin") and returns 403 if absent
- Apply middleware to all admin routes (accounts, policies,
  audit, dashboard, credentials)
- Remove redundant inline admin check from handleAdminResetPassword
- Profile routes correctly require only authentication, not admin

Security: The admin/adminGet middleware wrappers only called
requireCookieAuth (JWT validation) but never verified the admin
role. Any authenticated user could access admin endpoints
including role assignment. Fixed by inserting requireAdminRole
into the middleware chain for all admin routes.
---
 internal/ui/handlers_accounts.go | 22 ++-------------------
 internal/ui/ui.go                | 34 +++++++++++++++++++++++++++-----
 2 files changed, 31 insertions(+), 25 deletions(-)

diff --git a/internal/ui/handlers_accounts.go b/internal/ui/handlers_accounts.go
index 3c78823..20a1637 100644
--- a/internal/ui/handlers_accounts.go
+++ b/internal/ui/handlers_accounts.go
@@ -914,26 +914,8 @@ func (u *UIServer) handleCreatePGCreds(w http.ResponseWriter, r *http.Request) {
 // storage.  The plaintext is never logged or included in any response.
 // Audit event EventPasswordChanged is recorded on success.
 func (u *UIServer) handleAdminResetPassword(w http.ResponseWriter, r *http.Request) {
-	// Security: enforce admin role; requireCookieAuth only validates the token,
-	// it does not check roles.  A non-admin with a valid session must not be
-	// able to reset arbitrary accounts' passwords.
-	callerClaims := claimsFromContext(r.Context())
-	if callerClaims == nil {
-		u.renderError(w, r, http.StatusUnauthorized, "unauthorized")
-		return
-	}
-	isAdmin := false
-	for _, role := range callerClaims.Roles {
-		if role == "admin" {
-			isAdmin = true
-			break
-		}
-	}
-	if !isAdmin {
-		u.renderError(w, r, http.StatusForbidden, "admin role required")
-		return
-	}
-
+	// Security: admin role is enforced by the requireAdminRole middleware in
+	// the route registration (ui.go); no inline check needed here.
 	r.Body = http.MaxBytesReader(w, r.Body, maxFormBytes)
 	if err := r.ParseForm(); err != nil {
 		u.renderError(w, r, http.StatusBadRequest, "invalid form")
diff --git a/internal/ui/ui.go b/internal/ui/ui.go
index e728d58..549b5b0 100644
--- a/internal/ui/ui.go
+++ b/internal/ui/ui.go
@@ -301,12 +301,17 @@ func (u *UIServer) Register(mux *http.ServeMux) {
 	uiMux.HandleFunc("POST /logout", u.handleLogout)
 
 	// Protected routes.
-	auth := u.requireCookieAuth
+	//
+	// Security: three distinct access levels:
+	//   - authed:    any valid session cookie (authenticated user)
+	//   - admin:     authed + admin role in JWT claims (mutating admin ops)
+	//   - adminGet:  authed + admin role (read-only admin pages, no CSRF)
+	authed := u.requireCookieAuth
 	admin := func(h http.HandlerFunc) http.Handler {
-		return auth(u.requireCSRF(http.HandlerFunc(h)))
+		return authed(u.requireAdminRole(u.requireCSRF(http.HandlerFunc(h))))
 	}
 	adminGet := func(h http.HandlerFunc) http.Handler {
-		return auth(http.HandlerFunc(h))
+		return authed(u.requireAdminRole(http.HandlerFunc(h)))
 	}
 
 	uiMux.Handle("GET /dashboard", adminGet(u.handleDashboard))
@@ -335,8 +340,8 @@ func (u *UIServer) Register(mux *http.ServeMux) {
 	uiMux.Handle("PUT /accounts/{id}/password", admin(u.handleAdminResetPassword))
 
 	// Profile routes — accessible to any authenticated user (not admin-only).
-	uiMux.Handle("GET /profile", adminGet(u.handleProfilePage))
-	uiMux.Handle("PUT /profile/password", auth(u.requireCSRF(http.HandlerFunc(u.handleSelfChangePassword))))
+	uiMux.Handle("GET /profile", authed(http.HandlerFunc(u.handleProfilePage)))
+	uiMux.Handle("PUT /profile/password", authed(u.requireCSRF(http.HandlerFunc(u.handleSelfChangePassword))))
 
 	// Mount the wrapped UI mux on the parent mux. The "/" pattern acts as a
 	// catch-all for all UI paths; the more-specific /v1/ API patterns registered
@@ -405,6 +410,25 @@ func (u *UIServer) requireCSRF(next http.Handler) http.Handler {
 	})
 }
 
+// requireAdminRole checks that the authenticated user holds the "admin" role.
+// Must be placed after requireCookieAuth in the middleware chain so that
+// claims are available in the context.
+//
+// Security: This is the authoritative server-side check that prevents
+// non-admin users from accessing admin-only UI endpoints. The JWT claims
+// are populated from the database at login/renewal and signed with the
+// server's Ed25519 private key, so they cannot be forged client-side.
+func (u *UIServer) requireAdminRole(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		claims := claimsFromContext(r.Context())
+		if claims == nil || !claims.HasRole("admin") {
+			u.renderError(w, r, http.StatusForbidden, "admin role required")
+			return
+		}
+		next.ServeHTTP(w, r)
+	})
+}
+
 // ---- Helpers ----
 
 // isHTMX reports whether the request was initiated by HTMX.