From 1156414f70127ea5bce53fb66f4dfae5e65d06ad Mon Sep 17 00:00:00 2001 From: Pulse Monitor Date: Sat, 30 Aug 2025 22:30:59 +0000 Subject: [PATCH] fix: enforce admin privileges for proxy auth users on write operations addresses GHSA-wmgw-3g78-89xf - proxy authenticated non-admin users now properly receive 403 Forbidden when attempting write operations - Added RequireAdmin middleware to check proxy auth admin role - Applied admin checks to node add/update/delete operations - Applied admin checks to system settings updates - Applied admin checks to export/import operations - Applied admin checks to API token regeneration - Applied admin checks to password changes - Non-admin proxy auth users now have proper read-only access as documented --- internal/api/auth.go | 55 ++++++++++++ internal/api/router.go | 56 ++++++++++-- internal/api/security_setup_fix.go | 21 +++++ internal/api/system_settings.go | 21 +++++ test-proxy-auth.sh | 140 +++++++++++++++++++++++++++++ 5 files changed, 287 insertions(+), 6 deletions(-) create mode 100755 test-proxy-auth.sh diff --git a/internal/api/auth.go b/internal/api/auth.go index 666620503..398005b2f 100644 --- a/internal/api/auth.go +++ b/internal/api/auth.go @@ -500,4 +500,59 @@ func RequireAuth(cfg *config.Config, handler http.HandlerFunc) http.HandlerFunc http.Error(w, "Unauthorized", http.StatusUnauthorized) } } +} + +// RequireAdmin middleware checks for authentication and admin privileges +// For proxy auth users, it ensures they have the admin role +// For other auth methods, all authenticated users are considered admins +func RequireAdmin(cfg *config.Config, handler http.HandlerFunc) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + // First check if user is authenticated + if !CheckAuth(cfg, w, r) { + // Log the failed attempt + log.Warn(). + Str("ip", r.RemoteAddr). + Str("path", r.URL.Path). + Str("method", r.Method). + Msg("Unauthorized access attempt") + + // Return authentication error + if strings.HasPrefix(r.URL.Path, "/api/") || strings.Contains(r.Header.Get("Accept"), "application/json") { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusUnauthorized) + w.Write([]byte(`{"error":"Authentication required"}`)) + } else { + http.Error(w, "Unauthorized", http.StatusUnauthorized) + } + return + } + + // Check if using proxy auth and if so, verify admin status + if cfg.ProxyAuthSecret != "" { + if valid, username, isAdmin := CheckProxyAuth(cfg, r); valid { + if !isAdmin { + // User is authenticated but not an admin + log.Warn(). + Str("ip", r.RemoteAddr). + Str("path", r.URL.Path). + Str("method", r.Method). + Str("username", username). + Msg("Non-admin user attempted to access admin endpoint") + + // Return forbidden error + if strings.HasPrefix(r.URL.Path, "/api/") || strings.Contains(r.Header.Get("Accept"), "application/json") { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusForbidden) + w.Write([]byte(`{"error":"Admin privileges required"}`)) + } else { + http.Error(w, "Admin privileges required", http.StatusForbidden) + } + return + } + } + } + + // User is authenticated and has admin privileges (or not using proxy auth) + handler(w, r) + } } \ No newline at end of file diff --git a/internal/api/router.go b/internal/api/router.go index c3be18f5d..1ced9a394 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -128,7 +128,7 @@ func (r *Router) setupRoutes() { case http.MethodGet: configHandlers.HandleGetNodes(w, r) case http.MethodPost: - configHandlers.HandleAddNode(w, r) + RequireAdmin(configHandlers.config, configHandlers.HandleAddNode)(w, r) default: http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) } @@ -154,9 +154,9 @@ func (r *Router) setupRoutes() { r.mux.HandleFunc("/api/config/nodes/", func(w http.ResponseWriter, r *http.Request) { switch r.Method { case http.MethodPut: - configHandlers.HandleUpdateNode(w, r) + RequireAdmin(configHandlers.config, configHandlers.HandleUpdateNode)(w, r) case http.MethodDelete: - configHandlers.HandleDeleteNode(w, r) + RequireAdmin(configHandlers.config, configHandlers.HandleDeleteNode)(w, r) case http.MethodPost: // Handle test endpoint if strings.HasSuffix(r.URL.Path, "/test") { @@ -176,7 +176,7 @@ func (r *Router) setupRoutes() { configHandlers.HandleGetSystemSettings(w, r) case http.MethodPut: // DEPRECATED - use /api/system/settings/update instead - configHandlers.HandleUpdateSystemSettingsOLD(w, r) + RequireAdmin(configHandlers.config, configHandlers.HandleUpdateSystemSettingsOLD)(w, r) default: http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) } @@ -484,9 +484,11 @@ func (r *Router) setupRoutes() { if req.Method == http.MethodPost { // Check proxy auth first hasValidProxyAuth := false + proxyAuthIsAdmin := false if r.config.ProxyAuthSecret != "" { - if valid, _, _ := CheckProxyAuth(r.config, req); valid { + if valid, _, isAdmin := CheckProxyAuth(r.config, req); valid { hasValidProxyAuth = true + proxyAuthIsAdmin = isAdmin } } @@ -517,6 +519,16 @@ func (r *Router) setupRoutes() { r.config.APIToken != "" || r.config.ProxyAuthSecret != "" + // Check admin privileges for proxy auth users + if hasValidProxyAuth && !proxyAuthIsAdmin { + log.Warn(). + Str("ip", req.RemoteAddr). + Str("path", req.URL.Path). + Msg("Non-admin proxy auth user attempted export/import") + http.Error(w, "Admin privileges required for export/import", http.StatusForbidden) + return + } + if authRequired && !hasValidAuth { log.Warn(). Str("ip", req.RemoteAddr). @@ -571,9 +583,11 @@ func (r *Router) setupRoutes() { if req.Method == http.MethodPost { // Check proxy auth first hasValidProxyAuth := false + proxyAuthIsAdmin := false if r.config.ProxyAuthSecret != "" { - if valid, _, _ := CheckProxyAuth(r.config, req); valid { + if valid, _, isAdmin := CheckProxyAuth(r.config, req); valid { hasValidProxyAuth = true + proxyAuthIsAdmin = isAdmin } } @@ -604,6 +618,16 @@ func (r *Router) setupRoutes() { r.config.APIToken != "" || r.config.ProxyAuthSecret != "" + // Check admin privileges for proxy auth users + if hasValidProxyAuth && !proxyAuthIsAdmin { + log.Warn(). + Str("ip", req.RemoteAddr). + Str("path", req.URL.Path). + Msg("Non-admin proxy auth user attempted export/import") + http.Error(w, "Admin privileges required for export/import", http.StatusForbidden) + return + } + if authRequired && !hasValidAuth { log.Warn(). Str("ip", req.RemoteAddr). @@ -968,6 +992,26 @@ func (r *Router) handleChangePassword(w http.ResponseWriter, req *http.Request) "Only POST method is allowed", nil) return } + + // Check if using proxy auth and if so, verify admin status + if r.config.ProxyAuthSecret != "" { + if valid, username, isAdmin := CheckProxyAuth(r.config, req); valid { + if !isAdmin { + // User is authenticated but not an admin + log.Warn(). + Str("ip", req.RemoteAddr). + Str("path", req.URL.Path). + Str("method", req.Method). + Str("username", username). + Msg("Non-admin user attempted to change password") + + // Return forbidden error + writeErrorResponse(w, http.StatusForbidden, "forbidden", + "Admin privileges required", nil) + return + } + } + } // Parse request var changeReq struct { diff --git a/internal/api/security_setup_fix.go b/internal/api/security_setup_fix.go index 7cdf5dd7b..4de890b80 100644 --- a/internal/api/security_setup_fix.go +++ b/internal/api/security_setup_fix.go @@ -347,6 +347,27 @@ func (r *Router) HandleRegenerateAPIToken(w http.ResponseWriter, rq *http.Reques return } + // Check if using proxy auth and if so, verify admin status + if r.config.ProxyAuthSecret != "" { + if valid, username, isAdmin := CheckProxyAuth(r.config, rq); valid { + if !isAdmin { + // User is authenticated but not an admin + log.Warn(). + Str("ip", rq.RemoteAddr). + Str("path", rq.URL.Path). + Str("method", rq.Method). + Str("username", username). + Msg("Non-admin user attempted to regenerate API token") + + // Return forbidden error + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusForbidden) + w.Write([]byte(`{"error":"Admin privileges required"}`)) + return + } + } + } + if rq.Method != http.MethodPost { http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) return diff --git a/internal/api/system_settings.go b/internal/api/system_settings.go index a1248e505..f6a748996 100644 --- a/internal/api/system_settings.go +++ b/internal/api/system_settings.go @@ -215,6 +215,27 @@ func (h *SystemSettingsHandler) HandleUpdateSystemSettings(w http.ResponseWriter if !CheckAuth(h.config, w, r) { return } + + // Check if using proxy auth and if so, verify admin status + if h.config.ProxyAuthSecret != "" { + if valid, username, isAdmin := CheckProxyAuth(h.config, r); valid { + if !isAdmin { + // User is authenticated but not an admin + log.Warn(). + Str("ip", r.RemoteAddr). + Str("path", r.URL.Path). + Str("method", r.Method). + Str("username", username). + Msg("Non-admin user attempted to update system settings") + + // Return forbidden error + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusForbidden) + w.Write([]byte(`{"error":"Admin privileges required"}`)) + return + } + } + } // Load existing settings first to preserve fields not in the request existingSettings, err := h.persistence.LoadSystemSettings() diff --git a/test-proxy-auth.sh b/test-proxy-auth.sh new file mode 100755 index 000000000..946270a39 --- /dev/null +++ b/test-proxy-auth.sh @@ -0,0 +1,140 @@ +#!/bin/bash + +# Test script for proxy auth admin permissions +# This simulates what Authentik/Caddy would send + +BASE_URL="http://localhost:7656" +PROXY_SECRET="test-secret-123" + +echo "=== Testing Proxy Auth Admin Permissions ===" +echo + +# First, update the config to enable proxy auth +echo "1. Setting up proxy auth configuration..." +cat > /tmp/proxy-test.env << EOF +PROXY_AUTH_SECRET=$PROXY_SECRET +PROXY_AUTH_USER_HEADER=X-Authentik-Username +PROXY_AUTH_ROLE_HEADER=X-Authentik-Groups +PROXY_AUTH_ADMIN_ROLE=admin +PROXY_AUTH_ROLE_SEPARATOR=| +EOF + +# Copy current env and add proxy auth settings +cp /etc/pulse/.env /tmp/backup.env +cat /tmp/proxy-test.env >> /etc/pulse/.env + +# Restart service to pick up new config +echo "2. Restarting service with proxy auth enabled..." +sudo systemctl restart pulse-dev +sleep 5 + +echo "3. Testing API endpoints with different user roles..." +echo + +# Test as non-admin user (should be blocked from write operations) +echo "=== Testing as non-admin user (alice) ===" +echo "Groups: users|staff (no admin role)" +echo + +echo -n "GET /api/security/status (should work): " +curl -s -X GET "$BASE_URL/api/security/status" \ + -H "X-Proxy-Secret: $PROXY_SECRET" \ + -H "X-Authentik-Username: alice" \ + -H "X-Authentik-Groups: users|staff" \ + | jq -r '.proxyAuthIsAdmin // "ERROR"' + +echo -n "GET /api/config/nodes (should work): " +curl -s -X GET "$BASE_URL/api/config/nodes" \ + -H "X-Proxy-Secret: $PROXY_SECRET" \ + -H "X-Authentik-Username: alice" \ + -H "X-Authentik-Groups: users|staff" \ + -o /dev/null -w "%{http_code}\n" + +echo -n "POST /api/config/nodes (should be 403 Forbidden): " +curl -s -X POST "$BASE_URL/api/config/nodes" \ + -H "X-Proxy-Secret: $PROXY_SECRET" \ + -H "X-Authentik-Username: alice" \ + -H "X-Authentik-Groups: users|staff" \ + -H "Content-Type: application/json" \ + -d '{"name":"test","host":"192.168.1.1","type":"pve"}' \ + -o /dev/null -w "%{http_code}\n" + +echo -n "POST /api/system/settings/update (should be 403 Forbidden): " +curl -s -X POST "$BASE_URL/api/system/settings/update" \ + -H "X-Proxy-Secret: $PROXY_SECRET" \ + -H "X-Authentik-Username: alice" \ + -H "X-Authentik-Groups: users|staff" \ + -H "Content-Type: application/json" \ + -d '{"pollingInterval":30}' \ + -o /dev/null -w "%{http_code}\n" + +echo -n "POST /api/config/export (should be 403 Forbidden): " +curl -s -X POST "$BASE_URL/api/config/export" \ + -H "X-Proxy-Secret: $PROXY_SECRET" \ + -H "X-Authentik-Username: alice" \ + -H "X-Authentik-Groups: users|staff" \ + -H "Content-Type: application/json" \ + -d '{"passphrase":"test123456789"}' \ + -o /dev/null -w "%{http_code}\n" + +echo +echo "=== Testing as admin user (bob) ===" +echo "Groups: users|staff|admin (has admin role)" +echo + +echo -n "GET /api/security/status (should work): " +curl -s -X GET "$BASE_URL/api/security/status" \ + -H "X-Proxy-Secret: $PROXY_SECRET" \ + -H "X-Authentik-Username: bob" \ + -H "X-Authentik-Groups: users|staff|admin" \ + | jq -r '.proxyAuthIsAdmin // "ERROR"' + +echo -n "POST /api/config/nodes (should work - 400 due to incomplete data): " +curl -s -X POST "$BASE_URL/api/config/nodes" \ + -H "X-Proxy-Secret: $PROXY_SECRET" \ + -H "X-Authentik-Username: bob" \ + -H "X-Authentik-Groups: users|staff|admin" \ + -H "Content-Type: application/json" \ + -d '{"name":"test","host":"192.168.1.1","type":"pve"}' \ + -o /dev/null -w "%{http_code}\n" + +echo -n "POST /api/system/settings/update (should work - 200): " +curl -s -X POST "$BASE_URL/api/system/settings/update" \ + -H "X-Proxy-Secret: $PROXY_SECRET" \ + -H "X-Authentik-Username: bob" \ + -H "X-Authentik-Groups: users|staff|admin" \ + -H "Content-Type: application/json" \ + -d '{"darkMode":true}' \ + -o /dev/null -w "%{http_code}\n" + +echo -n "POST /api/config/export (should work - 200): " +curl -s -X POST "$BASE_URL/api/config/export" \ + -H "X-Proxy-Secret: $PROXY_SECRET" \ + -H "X-Authentik-Username: bob" \ + -H "X-Authentik-Groups: users|staff|admin" \ + -H "Content-Type: application/json" \ + -d '{"passphrase":"test123456789"}' \ + -o /dev/null -w "%{http_code}\n" + +echo +echo "=== Testing without proxy auth (should fail) ===" +echo + +echo -n "POST /api/config/nodes (should be 401 Unauthorized): " +curl -s -X POST "$BASE_URL/api/config/nodes" \ + -H "Content-Type: application/json" \ + -d '{"name":"test","host":"192.168.1.1","type":"pve"}' \ + -o /dev/null -w "%{http_code}\n" + +# Restore original config +echo +echo "4. Restoring original configuration..." +mv /tmp/backup.env /etc/pulse/.env +sudo systemctl restart pulse-dev + +echo +echo "=== Test Complete ===" +echo "Summary:" +echo "- Non-admin users should get 403 Forbidden on write operations" +echo "- Admin users should be able to perform all operations" +echo "- Users without proxy auth should get 401 Unauthorized" \ No newline at end of file