From a652cb8cea36ddf9020e7da0db309c41d4a8989b Mon Sep 17 00:00:00 2001 From: Sanaei Date: Fri, 24 Jul 2026 22:18:39 +0200 Subject: [PATCH] fix(clients): keep a client editable when its subId is already shared (#6065) The subId collision check in Update ran on every save, unlike the email check above it. Because Update defaults an omitted subId to the stored one, any client already sharing a subId was rejected on every later edit -- even a pure totalGB or expiry change that never mentions subId. Gate the check on an actual change. Pre-existing duplicates are reachable because SyncInbound has no such check, and 88a36773 meant to leave them untouched. Editing a client onto another subscriber's subId is still rejected, so the typo guard is intact. --- internal/web/service/client_crud.go | 2 +- .../web/service/client_update_rename_test.go | 39 +++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/internal/web/service/client_crud.go b/internal/web/service/client_crud.go index e43a73682..56c182524 100644 --- a/internal/web/service/client_crud.go +++ b/internal/web/service/client_crud.go @@ -375,7 +375,7 @@ func (s *ClientService) Update(inboundSvc *InboundService, id int, updated model } } - if updated.SubID != "" { + if updated.SubID != existing.SubID { var subCollision int64 if err := database.GetDB().Model(&model.ClientRecord{}). Where("sub_id = ? AND id <> ?", updated.SubID, id). diff --git a/internal/web/service/client_update_rename_test.go b/internal/web/service/client_update_rename_test.go index 5240c4aeb..163cfd137 100644 --- a/internal/web/service/client_update_rename_test.go +++ b/internal/web/service/client_update_rename_test.go @@ -108,6 +108,45 @@ func TestClientUpdateDuplicateSubIDDoesNotRenameEmail(t *testing.T) { } } +func TestClientUpdateKeepsSharedSubIDEditable(t *testing.T) { + setupBulkDB(t) + svc := &ClientService{} + inboundSvc := &InboundService{} + + source := []model.Client{ + {Email: "a@node", ID: "aaaaaaaa-0000-0000-0000-000000000005", SubID: "sub-shared", Enable: true}, + {Email: "b@node", ID: "aaaaaaaa-0000-0000-0000-000000000006", SubID: "sub-shared", Enable: true}, + } + ib := mkInbound(t, 22004, model.VLESS, clientsSettings(t, source)) + if err := svc.SyncInbound(nil, ib.Id, source); err != nil { + t.Fatalf("seed linkage: %v", err) + } + first := lookupClientRecord(t, "a@node") + if first.SubID != "sub-shared" || lookupClientRecord(t, "b@node").SubID != "sub-shared" { + t.Fatalf("seed did not produce a shared subId") + } + + updated := source[0] + updated.TotalGB = 42 + if _, err := svc.Update(inboundSvc, first.Id, updated); err != nil { + t.Fatalf("Update of a client whose subId is already shared: %v", err) + } + if got := lookupClientRecord(t, "a@node").TotalGB; got != 42 { + t.Fatalf("totalGB after update = %d, want 42", got) + } + + omitted := source[0] + omitted.SubID = "" + omitted.TotalGB = 43 + if _, err := svc.Update(inboundSvc, first.Id, omitted); err != nil { + t.Fatalf("Update with subId omitted entirely: %v", err) + } + other := lookupClientRecord(t, "b@node") + if other.SubID != "sub-shared" { + t.Fatalf("other client subId = %q, want %q", other.SubID, "sub-shared") + } +} + func mustInboundSettings(t *testing.T, inboundSvc *InboundService, id int) string { t.Helper() ib, err := inboundSvc.GetInbound(id)