Browse Source

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.
Sanaei 15 hours ago
parent
commit
a652cb8cea

+ 1 - 1
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
 		var subCollision int64
 		if err := database.GetDB().Model(&model.ClientRecord{}).
 		if err := database.GetDB().Model(&model.ClientRecord{}).
 			Where("sub_id = ? AND id <> ?", updated.SubID, id).
 			Where("sub_id = ? AND id <> ?", updated.SubID, id).

+ 39 - 0
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 {
 func mustInboundSettings(t *testing.T, inboundSvc *InboundService, id int) string {
 	t.Helper()
 	t.Helper()
 	ib, err := inboundSvc.GetInbound(id)
 	ib, err := inboundSvc.GetInbound(id)