From cfa8350d10adf343f4611ac685246860f9aebb42 Mon Sep 17 00:00:00 2001 From: BlindMaster24 <375291171150z@gmail.com> Date: Tue, 15 Sep 2026 18:00:54 +0300 Subject: [PATCH] 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 | 1 + internal/web/service/client_inbound_apply.go | 2 + .../web/service/client_reverse_readd_test.go | 149 ++++++++++++++++++ internal/web/service/inbound_traffic.go | 1 + internal/xray/api.go | 24 +++ internal/xray/api_reverse_test.go | 54 +++++++ 6 files changed, 231 insertions(+) create mode 100644 internal/web/service/client_reverse_readd_test.go create mode 100644 internal/xray/api_reverse_test.go diff --git a/internal/web/service/client_bulk.go b/internal/web/service/client_bulk.go index c040f9ddb..8bc44564e 100644 --- a/internal/web/service/client_bulk.go +++ b/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) diff --git a/internal/web/service/client_inbound_apply.go b/internal/web/service/client_inbound_apply.go index 4f9cd9c1a..13fd14318 100644 --- a/internal/web/service/client_inbound_apply.go +++ b/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) diff --git a/internal/web/service/client_reverse_readd_test.go b/internal/web/service/client_reverse_readd_test.go new file mode 100644 index 000000000..19b8694fb --- /dev/null +++ b/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) + } + } +} diff --git a/internal/web/service/inbound_traffic.go b/internal/web/service/inbound_traffic.go index 64133c8ba..f91336498 100644 --- a/internal/web/service/inbound_traffic.go +++ b/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 diff --git a/internal/xray/api.go b/internal/xray/api.go index 5f9fcee1c..714d14dd6 100644 --- a/internal/xray/api.go +++ b/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)) diff --git a/internal/xray/api_reverse_test.go b/internal/xray/api_reverse_test.go new file mode 100644 index 000000000..5e390dfeb --- /dev/null +++ b/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) + } + }) + } +}