From f23d202e0d0996ffa50c3e1565b7fdf4d6464905 Mon Sep 17 00:00:00 2001 From: Michael Reber Date: Fri, 5 Dec 2025 23:54:21 +0100 Subject: [PATCH] Added function if reload fails after enabling any jail, all jails enabled in that request are automatically disabled --- internal/fail2ban/connector_ssh.go | 3 +- pkg/web/handlers.go | 80 ++++++++++++++++++++++++++++-- pkg/web/static/fail2ban-ui.css | 4 ++ pkg/web/static/js/jails.js | 48 ++++++++++++++++-- 4 files changed, 125 insertions(+), 10 deletions(-) diff --git a/internal/fail2ban/connector_ssh.go b/internal/fail2ban/connector_ssh.go index a541ffb..c11bd88 100644 --- a/internal/fail2ban/connector_ssh.go +++ b/internal/fail2ban/connector_ssh.go @@ -4,6 +4,7 @@ import ( "bufio" "context" "encoding/base64" + "errors" "fmt" "os" "os/exec" @@ -856,7 +857,7 @@ PYEOF resolveOut = strings.TrimSpace(resolveOut) if strings.HasPrefix(resolveOut, "ERROR:") { - return originalPath, "", nil, fmt.Errorf(strings.TrimPrefix(resolveOut, "ERROR:")) + return originalPath, "", nil, errors.New(strings.TrimPrefix(resolveOut, "ERROR:")) } if strings.HasPrefix(resolveOut, "RESOLVED:") { resolvedPath = strings.TrimPrefix(resolveOut, "RESOLVED:") diff --git a/pkg/web/handlers.go b/pkg/web/handlers.go index 4eec0f9..1adf723 100644 --- a/pkg/web/handlers.go +++ b/pkg/web/handlers.go @@ -1012,6 +1012,16 @@ func AdvancedActionsTestHandler(c *gin.Context) { // UpdateJailManagementHandler updates the enabled state for each jail. // Expected JSON format: { "JailName1": true, "JailName2": false, ... } +// getJailNames converts a map of jail names to a sorted slice of jail names +func getJailNames(jails map[string]bool) []string { + names := make([]string, 0, len(jails)) + for name := range jails { + names = append(names, name) + } + sort.Strings(names) + return names +} + // After updating, fail2ban is reloaded to apply the changes. func UpdateJailManagementHandler(c *gin.Context) { config.DebugLog("----------------------------") @@ -1034,6 +1044,15 @@ func UpdateJailManagementHandler(c *gin.Context) { c.JSON(http.StatusBadRequest, gin.H{"error": "No jail updates provided"}) return } + + // Track which jails were enabled (for error recovery) + enabledJails := make(map[string]bool) + for jailName, enabled := range updates { + if enabled { + enabledJails[jailName] = true + } + } + // Update jail configuration file(s) with the new enabled states. if err := conn.UpdateJailEnabledStates(c.Request.Context(), updates); err != nil { config.DebugLog("Error updating jail enabled states: %v", err) @@ -1041,13 +1060,66 @@ func UpdateJailManagementHandler(c *gin.Context) { return } config.DebugLog("Successfully updated jail enabled states") + // Reload fail2ban to apply the changes (reload is sufficient for jail enable/disable) if err := conn.Reload(c.Request.Context()); err != nil { - config.DebugLog("Warning: failed to reload fail2ban after updating jail settings: %v", err) - // Still return success but warn about reload failure + config.DebugLog("Error: failed to reload fail2ban after updating jail settings: %v", err) + errMsg := err.Error() + + // If any jails were enabled in this request and reload failed, disable them all + if len(enabledJails) > 0 { + config.DebugLog("Reload failed after enabling %d jail(s), auto-disabling all enabled jails: %v", len(enabledJails), enabledJails) + + // Disable all jails that were just enabled + disableUpdate := make(map[string]bool) + for jailName := range enabledJails { + disableUpdate[jailName] = false + } + + if disableErr := conn.UpdateJailEnabledStates(c.Request.Context(), disableUpdate); disableErr != nil { + config.DebugLog("Error disabling jails after reload failure: %v", disableErr) + c.JSON(http.StatusOK, gin.H{ + "error": fmt.Sprintf("Failed to reload fail2ban: %s. Additionally, failed to auto-disable enabled jails: %v", errMsg, disableErr), + "autoDisabled": false, + "enabledJails": getJailNames(enabledJails), + }) + return + } + + // Reload again after disabling + if reloadErr := conn.Reload(c.Request.Context()); reloadErr != nil { + config.DebugLog("Error: failed to reload fail2ban after disabling jails: %v", reloadErr) + c.JSON(http.StatusOK, gin.H{ + "error": fmt.Sprintf("Failed to reload fail2ban after disabling jails: %v", reloadErr), + "autoDisabled": true, + "enabledJails": getJailNames(enabledJails), + }) + return + } + + config.DebugLog("Successfully disabled %d jail(s) and reloaded fail2ban", len(enabledJails)) + jailNamesList := getJailNames(enabledJails) + if len(jailNamesList) == 1 { + c.JSON(http.StatusOK, gin.H{ + "error": fmt.Sprintf("Jail '%s' was enabled but caused a reload error: %s. It has been automatically disabled.", jailNamesList[0], errMsg), + "autoDisabled": true, + "enabledJails": jailNamesList, + "message": fmt.Sprintf("Jail '%s' was automatically disabled due to configuration error", jailNamesList[0]), + }) + } else { + c.JSON(http.StatusOK, gin.H{ + "error": fmt.Sprintf("Jails %v were enabled but caused a reload error: %s. They have been automatically disabled.", jailNamesList, errMsg), + "autoDisabled": true, + "enabledJails": jailNamesList, + "message": fmt.Sprintf("%d jail(s) were automatically disabled due to configuration error", len(jailNamesList)), + }) + } + return + } + + // Error occurred but no jails were enabled (only disabled), so just report the error c.JSON(http.StatusOK, gin.H{ - "message": "Jail settings updated successfully, but fail2ban reload failed", - "warning": "Please reload fail2ban manually: " + err.Error(), + "error": fmt.Sprintf("Failed to reload fail2ban: %s", errMsg), }) return } diff --git a/pkg/web/static/fail2ban-ui.css b/pkg/web/static/fail2ban-ui.css index 1cda009..50d2157 100644 --- a/pkg/web/static/fail2ban-ui.css +++ b/pkg/web/static/fail2ban-ui.css @@ -152,6 +152,10 @@ mark { background-color: #1d4ed8; } +.toast-warning { + background-color: #d97706; +} + #advancedMikrotikFields, #advancedPfSenseFields { padding: 10px; } \ No newline at end of file diff --git a/pkg/web/static/js/jails.js b/pkg/web/static/js/jails.js index f9a310a..95ef5f0 100644 --- a/pkg/web/static/js/jails.js +++ b/pkg/web/static/js/jails.js @@ -321,15 +321,53 @@ function saveManageJailsSingle(checkbox) { return res.json(); }) .then(function(data) { + // Check if there was an error (including auto-disabled jails) if (data.error) { - showToast("Error saving jail settings: " + data.error, 'error'); - // Revert checkbox state on error - checkbox.checked = !isEnabled; - return; + var errorMsg = data.error; + var toastType = 'error'; + + // If jails were auto-disabled, check if this jail was one of them + var wasAutoDisabled = data.autoDisabled && data.enabledJails && Array.isArray(data.enabledJails) && data.enabledJails.indexOf(jailName) !== -1; + + if (wasAutoDisabled) { + checkbox.checked = false; + toastType = 'warning'; + // Use the message if available, otherwise use the error + errorMsg = data.message || errorMsg; + } else { + // Revert checkbox state on error + checkbox.checked = !isEnabled; + } + + showToast(errorMsg, toastType); + + // Still reload the jail list to reflect the actual state + return fetch(withServerParam('/api/jails/manage'), { + headers: serverHeaders() + }).then(function(res) { return res.json(); }) + .then(function(data) { + if (data.jails && data.jails.length) { + // Update the checkbox state based on server response + const jail = data.jails.find(function(j) { return j.jailName === jailName; }); + if (jail) { + checkbox.checked = jail.enabled; + } + } + loadServers().then(function() { + updateRestartBanner(); + return refreshData({ silent: true }); + }); + }); } + + // Check for warning (legacy support) + if (data.warning) { + showToast(data.warning, 'warning'); + } + console.log('Jail state saved successfully:', data); // Show success toast - showToast('Jail ' + jailName + ' ' + (isEnabled ? 'enabled' : 'disabled') + ' successfully', 'success'); + showToast(data.message || ('Jail ' + jailName + ' ' + (isEnabled ? 'enabled' : 'disabled') + ' successfully'), 'success'); // Reload the jail list to reflect the actual state return fetch(withServerParam('/api/jails/manage'), { headers: serverHeaders()