fix: Ban new IPs with fail2ban instead of disconnected the client. (#3919)

* fix: Ban new IPs with fail2ban  instead of disconnected the client.

* fix: Remove unused strconv import

* fix: Revert log fail2ban format

1 files changed, 9 insertions, 70 deletions

diff --git a/web/job/check_client_ip_job.go b/web/job/check_client_ip_job.go
index d3c1a1d1..cbc352dc 100644
--- a/web/job/check_client_ip_job.go
+++ b/web/job/check_client_ip_job.go
@@ -10,7 +10,6 @@ import (
 	"regexp"
 	"runtime"
 	"sort"
-	"strconv"
 	"time"
 
 	"github.com/mhsanaei/3x-ui/v2/database"
@@ -319,13 +318,14 @@ func (j *CheckClientIpJob) updateInboundClientIps(inboundClientIps *model.Inboun
 		}
 	}
 
-	// Convert back to slice and sort by timestamp (newest first)
+	// Convert back to slice and sort by timestamp (oldest first)
+	// This ensures we always protect the original/current connections and ban new excess ones.
 	allIps := make([]IPWithTimestamp, 0, len(ipMap))
 	for ip, timestamp := range ipMap {
 		allIps = append(allIps, IPWithTimestamp{IP: ip, Timestamp: timestamp})
 	}
 	sort.Slice(allIps, func(i, j int) bool {
-		return allIps[i].Timestamp > allIps[j].Timestamp // Descending order (newest first)
+		return allIps[i].Timestamp < allIps[j].Timestamp // Ascending order (oldest first)
 	})
 
 	shouldCleanLog := false
@@ -345,23 +345,17 @@ func (j *CheckClientIpJob) updateInboundClientIps(inboundClientIps *model.Inboun
 	if len(allIps) > limitIp {
 		shouldCleanLog = true
 
-		// Keep only the newest IPs (up to limitIp)
+		// Keep the oldest IPs (currently active connections) and ban the new excess ones.
 		keptIps := allIps[:limitIp]
-		disconnectedIps := allIps[limitIp:]
+		bannedIps := allIps[limitIp:]
 
-		// Log the disconnected IPs (old ones)
-		for _, ipTime := range disconnectedIps {
+		// Log banned IPs in the format fail2ban filters expect: [LIMIT_IP] Email = X || Disconnecting OLD IP = Y || Timestamp = Z
+		for _, ipTime := range bannedIps {
 			j.disAllowedIps = append(j.disAllowedIps, ipTime.IP)
 			log.Printf("[LIMIT_IP] Email = %s || Disconnecting OLD IP = %s || Timestamp = %d", clientEmail, ipTime.IP, ipTime.Timestamp)
 		}
 
-		// Actually disconnect old IPs by temporarily removing and re-adding user
-		// This forces Xray to drop existing connections from old IPs
-		if len(disconnectedIps) > 0 {
-			j.disconnectClientTemporarily(inbound, clientEmail, clients)
-		}
-
-		// Update database with only the newest IPs
+		// Update database with only the currently active (kept) IPs
 		jsonIps, _ := json.Marshal(keptIps)
 		inboundClientIps.Ips = string(jsonIps)
 	} else {
@@ -378,67 +372,12 @@ func (j *CheckClientIpJob) updateInboundClientIps(inboundClientIps *model.Inboun
 	}
 
 	if len(j.disAllowedIps) > 0 {
-		logger.Infof("[LIMIT_IP] Client %s: Kept %d newest IPs, disconnected %d old IPs", clientEmail, limitIp, len(j.disAllowedIps))
+		logger.Infof("[LIMIT_IP] Client %s: Kept %d current IPs, queued %d new IPs for fail2ban", clientEmail, limitIp, len(j.disAllowedIps))
 	}
 
 	return shouldCleanLog
 }
 
-// disconnectClientTemporarily removes and re-adds a client to force disconnect old connections
-func (j *CheckClientIpJob) disconnectClientTemporarily(inbound *model.Inbound, clientEmail string, clients []model.Client) {
-	var xrayAPI xray.XrayAPI
-
-	// Get panel settings for API port
-	db := database.GetDB()
-	var apiPort int
-	var apiPortSetting model.Setting
-	if err := db.Where("key = ?", "xrayApiPort").First(&apiPortSetting).Error; err == nil {
-		apiPort, _ = strconv.Atoi(apiPortSetting.Value)
-	}
-
-	if apiPort == 0 {
-		apiPort = 10085 // Default API port
-	}
-
-	err := xrayAPI.Init(apiPort)
-	if err != nil {
-		logger.Warningf("[LIMIT_IP] Failed to init Xray API for disconnection: %v", err)
-		return
-	}
-	defer xrayAPI.Close()
-
-	// Find the client config
-	var clientConfig map[string]any
-	for _, client := range clients {
-		if client.Email == clientEmail {
-			// Convert client to map for API
-			clientBytes, _ := json.Marshal(client)
-			json.Unmarshal(clientBytes, &clientConfig)
-			break
-		}
-	}
-
-	if clientConfig == nil {
-		return
-	}
-
-	// Remove user to disconnect all connections
-	err = xrayAPI.RemoveUser(inbound.Tag, clientEmail)
-	if err != nil {
-		logger.Warningf("[LIMIT_IP] Failed to remove user %s: %v", clientEmail, err)
-		return
-	}
-
-	// Wait a moment for disconnection to take effect
-	time.Sleep(100 * time.Millisecond)
-
-	// Re-add user to allow new connections
-	err = xrayAPI.AddUser(string(inbound.Protocol), inbound.Tag, clientConfig)
-	if err != nil {
-		logger.Warningf("[LIMIT_IP] Failed to re-add user %s: %v", clientEmail, err)
-	}
-}
-
 func (j *CheckClientIpJob) getInboundByEmail(clientEmail string) (*model.Inbound, error) {
 	db := database.GetDB()
 	inbound := &model.Inbound{}