fix(clients): keep a vless reverse client's handler across a re-add (#6558)

* fix(clients): keep a vless reverse client's handler across a re-add

RemoveUser also drops the client's reverse outbound handler, and the account
every live remove/re-add path rebuilt carried no reverse at all: buildUserAccount
read id/flow/testseed/testpre and nothing else. Editing, bulk re-enabling, quota
renewal and adding a client to an existing inbound therefore left a reverse
client able to connect but not to open its tunnel until Xray restarted, with
nothing logged. A traffic reset is the route operators hit most, since a
depleted client is removed and re-added on every renewal.

buildUserAccount now carries the tag (it accepts either the settings JSON object
or a typed client value), and the five account maps those paths build include
the client's reverse. Core chain, read from the pinned xray-core:
AddUserOperation -> User.ToMemoryUser -> vless.Account.AsAccount copies Reverse
(proxy/vless/account.go:24), and GetReverse rebuilds the handler from the stored
account's tag (proxy/vless/inbound/inbound.go:193-205).

Each path has a test that fails without its fix; the account-level test fails on
both input shapes.

* refactor(clients): drop an account map helper nothing calls

Local.AddClient and Local.UpdateUser are only reachable through runtime.Runtime,
and all four call sites of those two methods sit in a node branch, where the
runtime is a *Remote -- Remote.AddUser ignores the map and pushes the inbound
snapshot instead. So the extraction and its test covered a path no deployment
takes, the reverse key it added could never reach a core, and the previous
commit's claim that the node-push paths go through it was wrong.

The four account maps that do reach buildUserAccount are untouched. Reported by
the PR review.

internal/web/service/client_bulk.go

@@ -1802,6 +1802,7 @@ func (s *ClientService) bulkSetEnableInboundClients(inboundSvc *InboundService,
 						"auth":     ch.client.Auth,
 						"password": ch.client.Password,
 						"cipher":   cipher,
+						"reverse":  ch.client.Reverse,
 					})
 					if err1 != nil {
 						logger.Debug("Error in adding client on", rt.Name(), ":", err1)

internal/web/service/client_inbound_apply.go

@@ -610,6 +610,7 @@ func (s *ClientService) AddInboundClient(inboundSvc *InboundService, data *model
 					"allowedIPs":   client.AllowedIPs,
 					"preSharedKey": client.PreSharedKey,
 					"keepAlive":    keepAliveStr(client.KeepAliveSeconds()),
+					"reverse":      client.Reverse,
 				})
 				if err1 == nil {
 					logger.Debug("Client added on", rt.Name(), ":", client.Email)
@@ -1050,6 +1051,7 @@ func (s *ClientService) UpdateInboundClient(inboundSvc *InboundService, data *mo
 						"allowedIPs":   clients[0].AllowedIPs,
 						"preSharedKey": clients[0].PreSharedKey,
 						"keepAlive":    keepAliveStr(clients[0].KeepAliveSeconds()),
+						"reverse":      clients[0].Reverse,
 					})
 					if err1 == nil {
 						logger.Debug("Client edited on", rt.Name(), ":", clients[0].Email)

internal/web/service/client_reverse_readd_test.go

@@ -0,0 +1,149 @@
+package service
+
+import (
+	"context"
+	"sync"
+	"testing"
+
+	"github.com/mhsanaei/3x-ui/v3/internal/database"
+	"github.com/mhsanaei/3x-ui/v3/internal/database/model"
+	"github.com/mhsanaei/3x-ui/v3/internal/xray"
+)
+
+// reverseUserProbe records the account maps the panel pushes to the core: the
+// last place a stored reverse tag can be dropped before it reaches a listener.
+type reverseUserProbe struct {
+	fakeNodeRuntime
+	mu    sync.Mutex
+	users []map[string]any
+}
+
+func (p *reverseUserProbe) AddUser(_ context.Context, _ *model.Inbound, user map[string]any) error {
+	p.mu.Lock()
+	defer p.mu.Unlock()
+	p.users = append(p.users, user)
+	return nil
+}
+
+func (p *reverseUserProbe) recorded() []map[string]any {
+	p.mu.Lock()
+	defer p.mu.Unlock()
+	return append([]map[string]any(nil), p.users...)
+}
+
+const reverseProbeID = "5f2eb9d6-3a2f-4a55-9812-6ea1e2f7a333"
+
+func reverseProbeClient(email string, enable bool) model.Client {
+	return model.Client{Email: email, ID: reverseProbeID, Enable: enable, Reverse: &model.ClientReverse{Tag: "portal"}}
+}
+
+// seedReverseProbeInbound seeds one local vless inbound holding a single reverse
+// client, and the recording runtime every local apply of it lands on.
+func seedReverseProbeInbound(t *testing.T, tag string, port int, enable bool) (*model.Inbound, string, *reverseUserProbe) {
+	t.Helper()
+	setupConflictDB(t)
+	mgr := useTestRuntimeManager(t)
+	probe := &reverseUserProbe{}
+	mgr.SetLocalRuntimeOverride(probe)
+
+	email := tag + "@example.test"
+	client := reverseProbeClient(email, enable)
+	seedInboundConflict(t, tag, "0.0.0.0", port, model.VLESS, `{"network":"tcp"}`, clientsSettings(t, []model.Client{client}))
+	inbound := loadInboundByTag(t, tag)
+	if err := (&ClientService{}).SyncInbound(nil, inbound.Id, []model.Client{client}); err != nil {
+		t.Fatalf("SyncInbound: %v", err)
+	}
+	return inbound, email, probe
+}
+
+// assertReverseReAdd fails unless the core was handed the client's own tag: the
+// handler is gone the moment RemoveUser runs, and only the tag rebuilds it.
+func assertReverseReAdd(t *testing.T, probe *reverseUserProbe, email string) {
+	t.Helper()
+	users := probe.recorded()
+	if len(users) == 0 {
+		t.Fatalf("%s was never re-added to the core, so its reverse tag was never checked", email)
+	}
+	found := false
+	for _, user := range users {
+		if got, _ := user["email"].(string); got != email {
+			continue
+		}
+		found = true
+		tag, _ := user["reverse"].(*model.ClientReverse)
+		if tag == nil || tag.Tag != "portal" {
+			t.Fatalf("the re-add of %s carries reverse %#v, want its stored tag portal", email, user["reverse"])
+		}
+	}
+	if !found {
+		t.Fatalf("no re-add of %s reached the core: %v", email, users)
+	}
+}
+
+// The panel's most ordinary action on a reverse client: editing it removes the
+// account and adds it back, and the core rebuilds nothing without the tag.
+func TestClientEditKeepsTheReverseTag(t *testing.T) {
+	_, email, probe := seedReverseProbeInbound(t, "rev-edit", 50071, true)
+	rec := lookupClientRecord(t, email)
+
+	edited := reverseProbeClient(email, true)
+	edited.Comment = "edited after the tunnel was up"
+	if _, err := (&ClientService{}).Update(&InboundService{}, rec.Id, edited, 0); err != nil {
+		t.Fatalf("Update: %v", err)
+	}
+	assertReverseReAdd(t, probe, email)
+}
+
+func TestBulkReEnableKeepsTheReverseTag(t *testing.T) {
+	_, email, probe := seedReverseProbeInbound(t, "rev-bulk", 50072, false)
+
+	if _, _, err := (&ClientService{}).BulkSetEnable(&InboundService{}, []string{email}, true); err != nil {
+		t.Fatalf("BulkSetEnable: %v", err)
+	}
+	assertReverseReAdd(t, probe, email)
+}
+
+// The route an operator hits most often: a client that exhausted its quota is
+// removed, then re-added by the reset that renews it.
+func TestTrafficResetKeepsTheReverseTag(t *testing.T) {
+	inbound, email, probe := seedReverseProbeInbound(t, "rev-quota", 50073, true)
+	depleteClientTraffic(t, inbound.Id, email)
+
+	if _, err := (&InboundService{}).ResetClientTraffic(inbound.Id, email); err != nil {
+		t.Fatalf("ResetClientTraffic: %v", err)
+	}
+	assertReverseReAdd(t, probe, email)
+}
+
+func TestAddingClientsKeepsTheReverseTag(t *testing.T) {
+	inbound, _, probe := seedReverseProbeInbound(t, "rev-add", 50074, true)
+	const added = "rev-add-second@example.test"
+	second := reverseProbeClient(added, true)
+	second.ID = "7c3fad07-4b1c-4d66-9f83-7db2f3c8b444"
+
+	if _, err := (&ClientService{}).AddInboundClient(&InboundService{}, &model.Inbound{
+		Id:       inbound.Id,
+		Protocol: model.VLESS,
+		Settings: clientsSettings(t, []model.Client{second}),
+	}); err != nil {
+		t.Fatalf("AddInboundClient: %v", err)
+	}
+	assertReverseReAdd(t, probe, added)
+}
+
+// depleteClientTraffic leaves the client enabled in settings but out of quota,
+// the state a traffic reset re-adds it from.
+func depleteClientTraffic(t *testing.T, inboundId int, email string) {
+	t.Helper()
+	db := database.GetDB()
+	res := db.Model(&xray.ClientTraffic{}).Where("email = ?", email).
+		Updates(map[string]any{"enable": false, "up": 1, "down": 1})
+	if res.Error != nil {
+		t.Fatalf("deplete traffic: %v", res.Error)
+	}
+	if res.RowsAffected == 0 {
+		if err := db.Create(&xray.ClientTraffic{InboundId: inboundId, Email: email, Enable: false, Up: 1, Down: 1}).Error; err != nil {
+			t.Fatalf("create depleted traffic: %v", err)
+		}
+	}
+}

internal/web/service/inbound_traffic.go

@@ -728,6 +728,7 @@ func (s *InboundService) resetClientTrafficLocked(id int, clientEmail string) (b
 					"flow":     client.Flow,
 					"password": client.Password,
 					"cipher":   cipher,
+					"reverse":  client.Reverse,
 				}
 				if inbound.NodeID != nil {
 					reenableNodeID = inbound.NodeID

internal/xray/api.go

@@ -496,6 +496,25 @@ func shadowsocksCipherName(user map[string]any) (string, error) {
 	return getOptionalUserString(user, "method")
 }
 
+// reverseTag reads a vless reverse proxy tag from either shape a caller can
+// carry: the settings JSON object, or a typed client value marshalling alike.
+func reverseTag(value any) string {
+	if value == nil {
+		return ""
+	}
+	raw, err := json.Marshal(value)
+	if err != nil {
+		return ""
+	}
+	var parsed struct {
+		Tag string `json:"tag"`
+	}
+	if json.Unmarshal(raw, &parsed) != nil {
+		return ""
+	}
+	return parsed.Tag
+}
+
 // shadowsocksCipherType mirrors xray-core's infra/conf cipherFromString,
 // aliases and case-insensitivity included, so the account the panel builds for
 // a live user matches the one the core built for that inbound from its config.
@@ -556,6 +575,11 @@ func buildUserAccount(protocolName string, user map[string]any) (*serial.TypedMe
 			Id:   userID,
 			Flow: userFlow,
 		}
+		// RemoveUser also drops the account's reverse outbound handler, and
+		// GetReverse only rebuilds it from the tag a re-added account carries.
+		if tag := reverseTag(user["reverse"]); tag != "" {
+			vlessAccount.Reverse = &vless.Reverse{Tag: tag}
+		}
 		if testseedVal, ok := user["testseed"]; ok {
 			if testseedArr, ok := testseedVal.([]any); ok && len(testseedArr) >= 4 {
 				testseed := make([]uint32, len(testseedArr))

internal/xray/api_reverse_test.go

@@ -0,0 +1,54 @@
+package xray
+
+import (
+	"testing"
+
+	"github.com/xtls/xray-core/proxy/vless"
+	"google.golang.org/protobuf/proto"
+)
+
+// typedReverse stands in for model.ClientReverse, which this package cannot
+// import (model imports xray); it marshals to the same {"tag":"…"} shape.
+type typedReverse struct {
+	Tag string `json:"tag"`
+}
+
+// A reverse client's account has to carry its tag: removing the user drops the
+// outbound handler, and only the tag lets the core rebuild it (GetReverse).
+func TestBuildUserAccountCarriesTheReverseTag(t *testing.T) {
+	cases := []struct {
+		name    string
+		reverse any
+		want    string
+	}{
+		{"settings json shape", map[string]any{"tag": "portal"}, "portal"},
+		{"typed client field", &typedReverse{Tag: "portal"}, "portal"},
+		{"blank tag", map[string]any{"tag": ""}, ""},
+		{"typed nil", (*typedReverse)(nil), ""},
+		{"absent", nil, ""},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			user := map[string]any{
+				"email":   "reverse@example.test",
+				"id":      "5f2eb9d6-3a2f-4a55-9812-6ea1e2f7a333",
+				"reverse": tc.reverse,
+			}
+			tm, err := buildUserAccount("vless", user)
+			if err != nil {
+				t.Fatalf("buildUserAccount: %v", err)
+			}
+			if tm == nil {
+				t.Fatal("buildUserAccount returned no account for vless")
+			}
+			account := new(vless.Account)
+			if err := proto.Unmarshal(tm.Value, account); err != nil {
+				t.Fatalf("unmarshal vless account: %v", err)
+			}
+			if got := account.GetReverse().GetTag(); got != tc.want {
+				t.Fatalf("the account carries reverse tag %q, want %q", got, tc.want)
+			}
+		})
+	}
+}