diff --git a/internal/web/runtime/local.go b/internal/web/runtime/local.go index be5a960d8..48af05466 100644 --- a/internal/web/runtime/local.go +++ b/internal/web/runtime/local.go @@ -130,6 +130,10 @@ func (l *Local) DeleteUser(ctx context.Context, ib *model.Inbound, email string) return nil } +func (l *Local) DeleteClient(context.Context, string) error { + return nil +} + func (l *Local) UpdateUser(ctx context.Context, ib *model.Inbound, oldEmail string, payload model.Client) error { if oldEmail != "" { if err := l.RemoveUser(ctx, ib, oldEmail); err != nil && !strings.Contains(err.Error(), "not found") { diff --git a/internal/web/runtime/remote.go b/internal/web/runtime/remote.go index d6d0faf1b..3f529e070 100644 --- a/internal/web/runtime/remote.go +++ b/internal/web/runtime/remote.go @@ -553,6 +553,22 @@ func (r *Remote) DeleteUser(ctx context.Context, ib *model.Inbound, email string return err } +func (r *Remote) DeleteClient(ctx context.Context, email string) error { + if email == "" { + return nil + } + _, err := r.do(ctx, http.MethodPost, + "panel/api/clients/del/"+url.PathEscape(email), nil) + if err == nil { + return nil + } + var apiErr *remoteAPIError + if errors.As(err, &apiErr) && strings.Contains(strings.ToLower(apiErr.msg), "not found") { + return nil + } + return err +} + func (r *Remote) UpdateUser(ctx context.Context, ib *model.Inbound, oldEmail string, payload model.Client) error { if oldEmail == "" { oldEmail = payload.Email diff --git a/internal/web/runtime/runtime.go b/internal/web/runtime/runtime.go index fa3832045..d0540055c 100644 --- a/internal/web/runtime/runtime.go +++ b/internal/web/runtime/runtime.go @@ -23,6 +23,13 @@ type Runtime interface { DeleteUser(ctx context.Context, ib *model.Inbound, email string) error AddClient(ctx context.Context, ib *model.Inbound, client model.Client) error + // DeleteClient removes the client identified by email entirely from the + // runtime's own store: on Remote it hits the node's full-delete endpoint + // (record, attachments, traffic), unlike DeleteUser which only detaches + // from one inbound and leaves the node's client record behind. Local has + // no client store of its own, so it is a no-op there. + DeleteClient(ctx context.Context, email string) error + RestartXray(ctx context.Context) error ResetClientTraffic(ctx context.Context, ib *model.Inbound, email string) error diff --git a/internal/web/service/client_bulk.go b/internal/web/service/client_bulk.go index 97512c041..e3305c8d2 100644 --- a/internal/web/service/client_bulk.go +++ b/internal/web/service/client_bulk.go @@ -1067,9 +1067,12 @@ func (s *ClientService) bulkDelInboundClients( push = false } if push { + // bulkDelInboundClients only runs for full client deletion + // (BulkDelete), so the node must drop its client record too, + // not just detach from this inbound (#5797). pushFailed := false for email := range foundEmails { - if err1 := rt.DeleteUser(context.Background(), oldInbound, email); err1 != nil { + if err1 := rt.DeleteClient(context.Background(), email); err1 != nil { logger.Warning("Error in deleting client on", rt.Name(), ":", err1) pushFailed = true } diff --git a/internal/web/service/client_crud.go b/internal/web/service/client_crud.go index 1171cfa4a..a1b85a2a5 100644 --- a/internal/web/service/client_crud.go +++ b/internal/web/service/client_crud.go @@ -451,7 +451,7 @@ func (s *ClientService) Delete(inboundSvc *InboundService, id int, keepTraffic b if existing.Email == "" { continue } - nr, delErr := s.DelInboundClientByEmail(inboundSvc, ibId, existing.Email, false) + nr, delErr := s.DelInboundClientByEmail(inboundSvc, ibId, existing.Email, false, true) if delErr != nil { // The client is already absent from this inbound (data drift or a // retried delete). Skip it — deletion stays idempotent. @@ -609,7 +609,7 @@ func (s *ClientService) DeleteByEmail(inboundSvc *InboundService, email string, } needRestart := false for _, ibId := range inboundIds { - nr, delErr := s.DelInboundClientByEmail(inboundSvc, ibId, email, false) + nr, delErr := s.DelInboundClientByEmail(inboundSvc, ibId, email, false, true) if delErr != nil { if errors.Is(delErr, ErrClientNotInInbound) { continue @@ -672,7 +672,7 @@ func (s *ClientService) Detach(inboundSvc *InboundService, id int, inboundIds [] if existing.Email == "" { continue } - nr, delErr := s.DelInboundClientByEmail(inboundSvc, ibId, existing.Email, true) + nr, delErr := s.DelInboundClientByEmail(inboundSvc, ibId, existing.Email, true, false) if delErr != nil { if errors.Is(delErr, ErrClientNotInInbound) { continue diff --git a/internal/web/service/client_delete_node_full_test.go b/internal/web/service/client_delete_node_full_test.go new file mode 100644 index 000000000..377b95b1b --- /dev/null +++ b/internal/web/service/client_delete_node_full_test.go @@ -0,0 +1,66 @@ +package service + +import ( + "testing" + + "github.com/google/uuid" + + "github.com/mhsanaei/3x-ui/v3/internal/database/model" +) + +// A full client delete must reach the node as the full-delete RPC so the node +// drops its own client record too — the detach RPC leaves an orphaned record +// that keeps showing in the node's client list (#5797). +func TestDelete_NodeClientDispatchesFullDeleteRPC(t *testing.T) { + setupBulkDB(t) + nodeID, fake := setupNodeRuntime(t) + + clients := []model.Client{{ID: uuid.NewString(), Email: "full-del@x", Enable: true}} + nodeInbound(t, nodeID, 32001, clients) + + svc := &ClientService{} + inboundSvc := &InboundService{} + + rec, err := svc.GetRecordByEmail(nil, "full-del@x") + if err != nil { + t.Fatalf("GetRecordByEmail: %v", err) + } + if _, err := svc.Delete(inboundSvc, rec.Id, false); err != nil { + t.Fatalf("Delete: %v", err) + } + + if got := fake.deleteClient.Load(); got != 1 { + t.Fatalf("full delete dispatched %d DeleteClient RPCs, want 1", got) + } + if got := fake.deleteUser.Load(); got != 0 { + t.Fatalf("full delete dispatched %d DeleteUser (detach) RPCs, want 0", got) + } +} + +// A plain detach must stay scoped to the one inbound via the detach RPC and +// never escalate to the node-wide full delete. +func TestDetach_NodeClientStaysOnDetachRPC(t *testing.T) { + setupBulkDB(t) + nodeID, fake := setupNodeRuntime(t) + + clients := []model.Client{{ID: uuid.NewString(), Email: "detach-me@x", Enable: true}} + ib := nodeInbound(t, nodeID, 32002, clients) + + svc := &ClientService{} + inboundSvc := &InboundService{} + + rec, err := svc.GetRecordByEmail(nil, "detach-me@x") + if err != nil { + t.Fatalf("GetRecordByEmail: %v", err) + } + if _, err := svc.Detach(inboundSvc, rec.Id, []int{ib.Id}); err != nil { + t.Fatalf("Detach: %v", err) + } + + if got := fake.deleteUser.Load(); got != 1 { + t.Fatalf("detach dispatched %d DeleteUser RPCs, want 1", got) + } + if got := fake.deleteClient.Load(); got != 0 { + t.Fatalf("detach dispatched %d DeleteClient RPCs, want 0", got) + } +} diff --git a/internal/web/service/client_inbound_apply.go b/internal/web/service/client_inbound_apply.go index 7303c4364..ad38760ae 100644 --- a/internal/web/service/client_inbound_apply.go +++ b/internal/web/service/client_inbound_apply.go @@ -831,7 +831,7 @@ func (s *ClientService) UpdateInboundClient(inboundSvc *InboundService, data *mo return needRestart, nil } -func (s *ClientService) DelInboundClientByEmail(inboundSvc *InboundService, inboundId int, email string, keepTraffic bool) (bool, error) { +func (s *ClientService) DelInboundClientByEmail(inboundSvc *InboundService, inboundId int, email string, keepTraffic bool, fullDelete bool) (bool, error) { defer lockInbound(inboundId).Unlock() oldInbound, err := inboundSvc.GetInbound(inboundId) @@ -972,9 +972,17 @@ func (s *ClientService) DelInboundClientByEmail(inboundSvc *InboundService, inbo } else { // Node inbound: propagate the delete regardless of the enable flag — // the node's own DB still carries a disabled client and would - // resurrect it on the next snapshot otherwise. + // resurrect it on the next snapshot otherwise. A full client delete + // must remove the node's client record too, not just detach it from + // this inbound (#5797). if push { - if err1 := rt.DeleteUser(context.Background(), oldInbound, email); err1 != nil { + var err1 error + if fullDelete { + err1 = rt.DeleteClient(context.Background(), email) + } else { + err1 = rt.DeleteUser(context.Background(), oldInbound, email) + } + if err1 != nil { logger.Warning("Error in deleting client on", rt.Name(), ":", err1) } else { advancePushedInbound(rt, prevSettings, oldInbound) diff --git a/internal/web/service/client_wireguard_crud_test.go b/internal/web/service/client_wireguard_crud_test.go index 8d2e1b1ae..4db0ae543 100644 --- a/internal/web/service/client_wireguard_crud_test.go +++ b/internal/web/service/client_wireguard_crud_test.go @@ -80,7 +80,7 @@ func TestWireGuardClientAddUpdateDeleteRoundTrip(t *testing.T) { t.Fatalf("settings lost wg fields after metadata edit: %+v", listAfter) } - if _, err := svc.DelInboundClientByEmail(inboundSvc, ib.Id, "alice@wg", false); err != nil { + if _, err := svc.DelInboundClientByEmail(inboundSvc, ib.Id, "alice@wg", false, false); err != nil { t.Fatalf("DelInboundClientByEmail: %v", err) } final, err := svc.ListForInbound(nil, ib.Id) diff --git a/internal/web/service/del_shared_email_runtime_test.go b/internal/web/service/del_shared_email_runtime_test.go index a83a22e1a..504732dbc 100644 --- a/internal/web/service/del_shared_email_runtime_test.go +++ b/internal/web/service/del_shared_email_runtime_test.go @@ -25,7 +25,7 @@ func TestDelInboundClientByEmail_SharedEmailStillRemovesFromRuntime(t *testing.T svc := &ClientService{} inboundSvc := &InboundService{} - if _, err := svc.DelInboundClientByEmail(inboundSvc, ibA.Id, "shared@x", false); err != nil { + if _, err := svc.DelInboundClientByEmail(inboundSvc, ibA.Id, "shared@x", false, false); err != nil { t.Fatalf("DelInboundClientByEmail: %v", err) } diff --git a/internal/web/service/node_bulk_dispatch_test.go b/internal/web/service/node_bulk_dispatch_test.go index f52509fa2..8069ffc31 100644 --- a/internal/web/service/node_bulk_dispatch_test.go +++ b/internal/web/service/node_bulk_dispatch_test.go @@ -16,9 +16,10 @@ import ( // fakeNodeRuntime is a runtime.Runtime stub that counts the per-client dispatch // calls so a test can assert a bulk op does NOT stream one RPC per client. type fakeNodeRuntime struct { - addClient atomic.Int32 - deleteUser atomic.Int32 - updateUser atomic.Int32 + addClient atomic.Int32 + deleteUser atomic.Int32 + deleteClient atomic.Int32 + updateUser atomic.Int32 } func (f *fakeNodeRuntime) Name() string { return "fake-node" } @@ -40,6 +41,11 @@ func (f *fakeNodeRuntime) DeleteUser(context.Context, *model.Inbound, string) er return nil } +func (f *fakeNodeRuntime) DeleteClient(context.Context, string) error { + f.deleteClient.Add(1) + return nil +} + func (f *fakeNodeRuntime) AddClient(context.Context, *model.Inbound, model.Client) error { f.addClient.Add(1) return nil diff --git a/internal/web/service/node_dirty_test.go b/internal/web/service/node_dirty_test.go index 0de264114..26f89d6e3 100644 --- a/internal/web/service/node_dirty_test.go +++ b/internal/web/service/node_dirty_test.go @@ -99,7 +99,7 @@ func TestDelInboundClientByEmail_DisabledNodeClientMarksDirty(t *testing.T) { inboundSvc := &InboundService{} clientSvc := &ClientService{} - if _, err := clientSvc.DelInboundClientByEmail(inboundSvc, central.Id, "a@x", false); err != nil { + if _, err := clientSvc.DelInboundClientByEmail(inboundSvc, central.Id, "a@x", false, false); err != nil { t.Fatalf("DelInboundClientByEmail: %v", err) } diff --git a/internal/web/service/sync_scale_postgres_test.go b/internal/web/service/sync_scale_postgres_test.go index 9b111c32f..bf69e2325 100644 --- a/internal/web/service/sync_scale_postgres_test.go +++ b/internal/web/service/sync_scale_postgres_test.go @@ -201,7 +201,7 @@ func TestAddDelClientPostgresScale(t *testing.T) { delEmail := clients[n/2].Email start = time.Now() - if _, err := svc.DelInboundClientByEmail(inboundSvc, ib.Id, delEmail, false); err != nil { + if _, err := svc.DelInboundClientByEmail(inboundSvc, ib.Id, delEmail, false, false); err != nil { t.Fatalf("DelInboundClientByEmail: %v", err) } delDur := time.Since(start)