Skip to content

Commit 49687e2

Browse files
H-TTTTTH-TTTTT
committed
fix(clients): persist all editable fields for clients with no inbound
ClientService.Update only wrote most editable fields to the clients table inside the per-inbound loop (UpdateInboundClient -> SyncInbound), so a client with no attached inbound — the external-links / remote- subscription client — silently dropped subId, totalGB, expiryTime, limitIp, tgId, comment, reset, flow, security and the credential fields on edit. Update still returned success, so the panel showed a saved toast while the row was untouched. email/enable/group_name/ad_tag/reverse already had dedicated unconditional direct writes that covered the no-inbound case; the rest did not. This was a known failure shape: commit c4448f4 added the no-inbound fallback for email only, and TestUpdate_PersistsRecordEnable_NoInbound covered enable only — both stayed scoped to a single field. Extract SyncInbound's per-row field merge into applyClientRecordMerge (unconditional for scalar quota/lifecycle/subscription fields, preserve-when-empty for credentials/identifiers, earliest CreatedAt) and reuse it from Update. Update's new branch runs only when the client has no inbound, so it fixes the gap without altering the inbound-attached path (where flow is per-inbound gated via clientWithInboundFlow and SyncInbound already persists every field). email/reverse/group/ad_tag/ enable keep their existing dedicated writes; the new write covers the remaining columns. Fixes #5981
1 parent 16b2bcf commit 49687e2

3 files changed

Lines changed: 294 additions & 47 deletions

File tree

‎internal/web/service/client_crud.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,35 @@ func (s *ClientService) Update(inboundSvc *InboundService, id int, updated model
433433
}
434434
}
435435

436+
if len(inboundIds) == 0 {
437+
merged := *existing
438+
applyClientRecordMerge(&merged, updated.ToRecord())
439+
if err := database.GetDB().Model(&model.ClientRecord{}).
440+
Where("id = ?", id).
441+
Updates(map[string]any{
442+
"sub_id": merged.SubID,
443+
"uuid": merged.UUID,
444+
"password": merged.Password,
445+
"auth": merged.Auth,
446+
"secret": merged.Secret,
447+
"flow": merged.Flow,
448+
"security": merged.Security,
449+
"wg_private_key": merged.PrivateKey,
450+
"wg_public_key": merged.PublicKey,
451+
"wg_allowed_ips": merged.AllowedIPs,
452+
"wg_pre_shared_key": merged.PreSharedKey,
453+
"wg_keep_alive": merged.KeepAlive,
454+
"limit_ip": merged.LimitIP,
455+
"total_gb": merged.TotalGB,
456+
"expiry_time": merged.ExpiryTime,
457+
"tg_id": merged.TgID,
458+
"comment": merged.Comment,
459+
"reset": merged.Reset,
460+
}).Error; err != nil {
461+
return needRestart, err
462+
}
463+
}
464+
436465
reverseStr := ""
437466
if updated.Reverse != nil && strings.TrimSpace(updated.Reverse.Tag) != "" {
438467
if b, mErr := json.Marshal(updated.Reverse); mErr == nil {

‎internal/web/service/client_link.go‎

Lines changed: 60 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,65 @@ import (
99
"gorm.io/gorm"
1010
)
1111

12+
// applyClientRecordMerge merges incoming client-record fields onto row using the
13+
// same rules everywhere a client record is persisted: scalar quota / lifecycle /
14+
// subscription fields are applied unconditionally (so clearing them takes
15+
// effect), while credentials and identifiers are only overwritten when the
16+
// incoming value is non-empty (so a partial update preserves the stored UUID /
17+
// password / keys). CreatedAt keeps the earliest known value. Email, UpdatedAt,
18+
// and the Id primary key are intentionally not touched here — callers handle
19+
// those separately. Shared by SyncInbound (per-inbound persistence) and Update
20+
// (the no-attached-inbound fallback) so the two paths cannot diverge.
21+
func applyClientRecordMerge(row *model.ClientRecord, incoming *model.ClientRecord) {
22+
if incoming.UUID != "" {
23+
row.UUID = incoming.UUID
24+
}
25+
if incoming.Password != "" {
26+
row.Password = incoming.Password
27+
}
28+
if incoming.Auth != "" {
29+
row.Auth = incoming.Auth
30+
}
31+
if incoming.Secret != "" {
32+
row.Secret = incoming.Secret
33+
}
34+
if incoming.AdTag != "" {
35+
row.AdTag = incoming.AdTag
36+
}
37+
row.Flow = incoming.Flow
38+
if incoming.Security != "" {
39+
row.Security = incoming.Security
40+
}
41+
if incoming.Reverse != "" {
42+
row.Reverse = incoming.Reverse
43+
}
44+
if incoming.PrivateKey != "" {
45+
row.PrivateKey = incoming.PrivateKey
46+
}
47+
if incoming.PublicKey != "" {
48+
row.PublicKey = incoming.PublicKey
49+
}
50+
if incoming.AllowedIPs != "" {
51+
row.AllowedIPs = incoming.AllowedIPs
52+
}
53+
row.PreSharedKey = incoming.PreSharedKey
54+
row.KeepAlive = incoming.KeepAlive
55+
row.SubID = incoming.SubID
56+
row.LimitIP = incoming.LimitIP
57+
row.TotalGB = incoming.TotalGB
58+
row.ExpiryTime = incoming.ExpiryTime
59+
row.Enable = incoming.Enable
60+
row.TgID = incoming.TgID
61+
if incoming.Group != "" {
62+
row.Group = incoming.Group
63+
}
64+
row.Comment = incoming.Comment
65+
row.Reset = incoming.Reset
66+
if incoming.CreatedAt > 0 && (row.CreatedAt == 0 || incoming.CreatedAt < row.CreatedAt) {
67+
row.CreatedAt = incoming.CreatedAt
68+
}
69+
}
70+
1271
func (s *ClientService) SyncInbound(tx *gorm.DB, inboundId int, clients []model.Client) error {
1372
if tx == nil {
1473
tx = database.GetDB()
@@ -66,53 +125,7 @@ func (s *ClientService) SyncInbound(tx *gorm.DB, inboundId int, clients []model.
66125
}
67126

68127
before := *row
69-
if incoming.UUID != "" {
70-
row.UUID = incoming.UUID
71-
}
72-
if incoming.Password != "" {
73-
row.Password = incoming.Password
74-
}
75-
if incoming.Auth != "" {
76-
row.Auth = incoming.Auth
77-
}
78-
if incoming.Secret != "" {
79-
row.Secret = incoming.Secret
80-
}
81-
if incoming.AdTag != "" {
82-
row.AdTag = incoming.AdTag
83-
}
84-
row.Flow = incoming.Flow
85-
if incoming.Security != "" {
86-
row.Security = incoming.Security
87-
}
88-
if incoming.Reverse != "" {
89-
row.Reverse = incoming.Reverse
90-
}
91-
if incoming.PrivateKey != "" {
92-
row.PrivateKey = incoming.PrivateKey
93-
}
94-
if incoming.PublicKey != "" {
95-
row.PublicKey = incoming.PublicKey
96-
}
97-
if incoming.AllowedIPs != "" {
98-
row.AllowedIPs = incoming.AllowedIPs
99-
}
100-
row.PreSharedKey = incoming.PreSharedKey
101-
row.KeepAlive = incoming.KeepAlive
102-
row.SubID = incoming.SubID
103-
row.LimitIP = incoming.LimitIP
104-
row.TotalGB = incoming.TotalGB
105-
row.ExpiryTime = incoming.ExpiryTime
106-
row.Enable = incoming.Enable
107-
row.TgID = incoming.TgID
108-
if incoming.Group != "" {
109-
row.Group = incoming.Group
110-
}
111-
row.Comment = incoming.Comment
112-
row.Reset = incoming.Reset
113-
if incoming.CreatedAt > 0 && (row.CreatedAt == 0 || incoming.CreatedAt < row.CreatedAt) {
114-
row.CreatedAt = incoming.CreatedAt
115-
}
128+
applyClientRecordMerge(row, incoming)
116129
preservedUpdatedAt := max(incoming.UpdatedAt, row.UpdatedAt)
117130
row.UpdatedAt = preservedUpdatedAt
118131

‎internal/web/service/client_update_no_inbound_test.go‎

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
package service
2+
3+
import (
4+
"strings"
5+
"testing"
6+
7+
"github.com/mhsanaei/3x-ui/v3/internal/database"
8+
"github.com/mhsanaei/3x-ui/v3/internal/database/model"
9+
)
10+
11+
func TestUpdate_PersistsFields_NoInbound(t *testing.T) {
12+
cases := []struct {
13+
name string
14+
mutate func(c *model.Client)
15+
readBack func(rec *model.ClientRecord) any
16+
want any
17+
}{
18+
{
19+
name: "subId",
20+
mutate: func(c *model.Client) { c.SubID = "new-sub-id" },
21+
readBack: func(rec *model.ClientRecord) any { return rec.SubID },
22+
want: "new-sub-id",
23+
},
24+
{
25+
name: "totalGB cleared to zero",
26+
mutate: func(c *model.Client) { c.TotalGB = 0 },
27+
readBack: func(rec *model.ClientRecord) any { return rec.TotalGB },
28+
want: int64(0),
29+
},
30+
{
31+
name: "expiryTime",
32+
mutate: func(c *model.Client) { c.ExpiryTime = 1700000000 },
33+
readBack: func(rec *model.ClientRecord) any { return rec.ExpiryTime },
34+
want: int64(1700000000),
35+
},
36+
{
37+
name: "limitIp",
38+
mutate: func(c *model.Client) { c.LimitIP = 7 },
39+
readBack: func(rec *model.ClientRecord) any { return rec.LimitIP },
40+
want: 7,
41+
},
42+
{
43+
name: "tgId",
44+
mutate: func(c *model.Client) { c.TgID = 9876543210 },
45+
readBack: func(rec *model.ClientRecord) any { return rec.TgID },
46+
want: int64(9876543210),
47+
},
48+
{
49+
name: "comment cleared to empty",
50+
mutate: func(c *model.Client) { c.Comment = "" },
51+
readBack: func(rec *model.ClientRecord) any { return rec.Comment },
52+
want: "",
53+
},
54+
{
55+
name: "reset",
56+
mutate: func(c *model.Client) { c.Reset = 30 },
57+
readBack: func(rec *model.ClientRecord) any { return rec.Reset },
58+
want: 30,
59+
},
60+
{
61+
name: "flow",
62+
mutate: func(c *model.Client) { c.Flow = "xtls-rprx-vision" },
63+
readBack: func(rec *model.ClientRecord) any { return rec.Flow },
64+
want: "xtls-rprx-vision",
65+
},
66+
{
67+
name: "security",
68+
mutate: func(c *model.Client) { c.Security = "aes-128-gcm" },
69+
readBack: func(rec *model.ClientRecord) any { return rec.Security },
70+
want: "aes-128-gcm",
71+
},
72+
{
73+
name: "uuid rotated",
74+
mutate: func(c *model.Client) { c.ID = "22222222-2222-2222-2222-222222222222" },
75+
readBack: func(rec *model.ClientRecord) any { return rec.UUID },
76+
want: "22222222-2222-2222-2222-222222222222",
77+
},
78+
}
79+
80+
for _, tc := range cases {
81+
t.Run(tc.name, func(t *testing.T) {
82+
setupBulkDB(t)
83+
svc := &ClientService{}
84+
inboundSvc := &InboundService{}
85+
86+
email := "noib-" + strings.ReplaceAll(strings.ToLower(tc.name), " ", "-") + "@x"
87+
rec := &model.ClientRecord{
88+
Email: email,
89+
UUID: "11111111-1111-1111-1111-111111111111",
90+
SubID: email,
91+
TotalGB: 5,
92+
ExpiryTime: 1000,
93+
LimitIP: 1,
94+
TgID: 1,
95+
Comment: "seeded",
96+
Reset: 1,
97+
Flow: "seeded-flow",
98+
Security: "seeded-sec",
99+
}
100+
if err := database.GetDB().Create(rec).Error; err != nil {
101+
t.Fatalf("create record: %v", err)
102+
}
103+
104+
updated := rec.ToClient()
105+
tc.mutate(updated)
106+
if _, err := svc.Update(inboundSvc, rec.Id, *updated); err != nil {
107+
t.Fatalf("Update: %v", err)
108+
}
109+
110+
got, err := svc.GetByID(rec.Id)
111+
if err != nil {
112+
t.Fatalf("GetByID: %v", err)
113+
}
114+
if tc.readBack(got) != tc.want {
115+
t.Fatalf("%s: not persisted for no-inbound client, got %v, want %v", tc.name, tc.readBack(got), tc.want)
116+
}
117+
})
118+
}
119+
}
120+
121+
func TestUpdate_NoInbound_PreservesCredentialsWhenOmitted(t *testing.T) {
122+
setupBulkDB(t)
123+
svc := &ClientService{}
124+
inboundSvc := &InboundService{}
125+
126+
email := "noib-preserve@x"
127+
rec := &model.ClientRecord{
128+
Email: email,
129+
UUID: "11111111-1111-1111-1111-111111111111",
130+
SubID: email,
131+
Password: "seeded-pw",
132+
Auth: "seeded-auth",
133+
Secret: "seeded-secret",
134+
}
135+
if err := database.GetDB().Create(rec).Error; err != nil {
136+
t.Fatalf("create record: %v", err)
137+
}
138+
139+
updated := rec.ToClient()
140+
updated.ID = ""
141+
updated.Password = ""
142+
updated.Auth = ""
143+
updated.Secret = ""
144+
updated.Comment = "only comment changed"
145+
if _, err := svc.Update(inboundSvc, rec.Id, *updated); err != nil {
146+
t.Fatalf("Update: %v", err)
147+
}
148+
149+
got, err := svc.GetByID(rec.Id)
150+
if err != nil {
151+
t.Fatalf("GetByID: %v", err)
152+
}
153+
if got.UUID != "11111111-1111-1111-1111-111111111111" {
154+
t.Fatalf("uuid wiped on partial update, got %q", got.UUID)
155+
}
156+
if got.Password != "seeded-pw" {
157+
t.Fatalf("password wiped on partial update, got %q", got.Password)
158+
}
159+
if got.Auth != "seeded-auth" {
160+
t.Fatalf("auth wiped on partial update, got %q", got.Auth)
161+
}
162+
if got.Secret != "seeded-secret" {
163+
t.Fatalf("secret wiped on partial update, got %q", got.Secret)
164+
}
165+
if got.Comment != "only comment changed" {
166+
t.Fatalf("comment not persisted, got %q", got.Comment)
167+
}
168+
}
169+
170+
func TestApplyClientRecordMerge_MirrorsSyncInboundRules(t *testing.T) {
171+
row := &model.ClientRecord{
172+
UUID: "kept-uuid",
173+
Password: "kept-pw",
174+
Flow: "kept-flow",
175+
TotalGB: 9,
176+
Group: "kept-group",
177+
Comment: "kept-comment",
178+
}
179+
incoming := &model.ClientRecord{
180+
Password: "new-pw",
181+
TotalGB: 0,
182+
Comment: "new-comment",
183+
}
184+
185+
applyClientRecordMerge(row, incoming)
186+
187+
if row.UUID != "kept-uuid" {
188+
t.Fatalf("empty incoming UUID should preserve stored UUID, got %q", row.UUID)
189+
}
190+
if row.Password != "new-pw" {
191+
t.Fatalf("non-empty incoming Password should overwrite, got %q", row.Password)
192+
}
193+
if row.Flow != "" {
194+
t.Fatalf("incoming Flow is unconditional and should overwrite with empty, got %q", row.Flow)
195+
}
196+
if row.TotalGB != 0 {
197+
t.Fatalf("incoming TotalGB is unconditional and should overwrite with zero, got %v", row.TotalGB)
198+
}
199+
if row.Group != "kept-group" {
200+
t.Fatalf("empty incoming Group should preserve stored group, got %q", row.Group)
201+
}
202+
if row.Comment != "new-comment" {
203+
t.Fatalf("incoming Comment is unconditional and should overwrite, got %q", row.Comment)
204+
}
205+
}

0 commit comments

Comments
 (0)