fix(inbounds): keep the stored client list and enable on inbound save

Invariant: saving an inbound's configuration never changes which clients it
holds nor whether it is enabled; both have their own endpoints. The edit
modal posts back the clients and the enable flag it loaded when it opened.
A client added meanwhile (another admin, the bot, the API, LDAP) was
detached and its stats deleted; a client deleted meanwhile came back with
its credentials, restoring access that had been revoked; an inbound
switched off meanwhile was switched back on.

For every save but a master's node-sync push, UpdateInbound now takes the
client list and enable from the row it re-reads inside the writer; this
replaces the lifecycle-only carry from the previous commit. Client
validation (renewal schedule, Hysteria auth, TUIC credentials) moves after
that swap so it judges the clients actually saved: a protocol switch keeps
the stored clients, and #6268's refusal must apply to them.

The edit form no longer loads or sends clients, so neither the JSON editor
nor validation sees a copy the server ignores, and the enable switch shows
only when adding; the list toggle (/setEnable) covers existing inbounds.

Tests that added or re-keyed clients through a panel inbound save pinned
the old rule; they now drive the master-push path, where payload clients
still apply.
This commit is contained in:
MHSanaei
2026-09-28 13:23:48 +02:00
parent fb7418f7bd
commit 823db05966
13 changed files with 207 additions and 96 deletions
@@ -48,7 +48,8 @@ func TestClientRenewalWriteValidation(t *testing.T) {
}
_, _, err = inboundSvc.AddInbound(&update)
case "update inbound":
_, _, err = inboundSvc.UpdateInbound(&update)
// A panel save keeps the stored clients; only a master's push writes them.
_, _, err = (&InboundService{FromNodeSync: true}).UpdateInbound(&update)
case "add inbound client":
client.Email = "invalid-new-client"
update.Settings = clientsSettings(t, []model.Client{client})
+37 -29
View File
@@ -1684,6 +1684,35 @@ func (s *InboundService) SetInboundEnable(id int, enable bool) (bool, error) {
return needRestart, nil
}
func (s *InboundService) validateUpdatedInboundClients(inbound *model.Inbound) error {
clients, err := s.GetClients(inbound)
if err != nil {
return err
}
if err := validateClientsRenewal(clients); err != nil {
return err
}
for _, client := range clients {
switch inbound.Protocol {
case model.Hysteria:
if client.Auth == "" {
return common.NewError("empty client ID")
}
case model.TUIC:
if client.ID == "" {
return common.NewError("empty client ID")
}
if client.Password == "" {
return common.NewError("tuic client requires a password")
}
if client.Email == "" {
return common.NewError("empty client email")
}
}
}
return nil
}
func (s *InboundService) UpdateInbound(inbound *model.Inbound) (*model.Inbound, bool, error) {
legacyShareAddr := legacyMtprotoShareAddr(inbound)
inbound.TrafficResetDay = normalizeTrafficResetDay(inbound.TrafficResetDay)
@@ -1706,34 +1735,6 @@ func (s *InboundService) UpdateInbound(inbound *model.Inbound) (*model.Inbound,
}
inbound.SubSortIndex = normalizeSubSortIndex(inbound.SubSortIndex)
clients, err := s.GetClients(inbound)
if err != nil {
return inbound, false, err
}
if err := validateClientsRenewal(clients); err != nil {
return inbound, false, err
}
if inbound.Protocol == model.Hysteria {
for _, client := range clients {
if client.Auth == "" {
return inbound, false, common.NewError("empty client ID")
}
}
}
if inbound.Protocol == model.TUIC {
for _, client := range clients {
if client.ID == "" {
return inbound, false, common.NewError("empty client ID")
}
if client.Password == "" {
return inbound, false, common.NewError("tuic client requires a password")
}
if client.Email == "" {
return inbound, false, common.NewError("empty client email")
}
}
}
// Grandfather a row that was already stored incomplete so it stays editable;
// only a save that breaks a previously valid TLS block is refused.
if !s.FromNodeSync {
@@ -1776,8 +1777,15 @@ func (s *InboundService) UpdateInbound(inbound *model.Inbound) (*model.Inbound,
return err
}
oldInbound = stored
// The form posts back the clients and enable it loaded; both have their
// own endpoints, so only a master's push may change them here.
if !s.FromNodeSync {
inbound.Settings = keepStoredClientLifecycle(inbound.Settings, stored.Settings)
inbound.Settings = keepStoredClients(inbound.Settings, stored.Settings)
inbound.Enable = stored.Enable
}
// On the clients actually saved: a protocol switch keeps the stored ones.
if err := s.validateUpdatedInboundClients(inbound); err != nil {
return err
}
conflict, cErr := checkPortConflictTx(tx, inbound, inbound.Id)
if cErr != nil {
@@ -8,10 +8,12 @@ import (
"github.com/mhsanaei/3x-ui/v3/internal/database/model"
)
// An inbound save keeps the stored clients, so switching to Hysteria is judged
// on them: ones with no auth would leave an inbound nobody can connect to.
func TestUpdateInbound_RejectsHysteriaClientWithoutAuth(t *testing.T) {
setupConflictDB(t)
seedInboundConflict(t, "in-45001-tcp", "0.0.0.0", 45001, model.VLESS,
`{"network":"tcp"}`, `{"clients":[]}`)
`{"network":"tcp"}`, `{"clients":[{"email":"hysteria@x","enable":true,"password":"not-hysteria-auth"}]}`)
var existing model.Inbound
if err := database.GetDB().Where("tag = ?", "in-45001-tcp").First(&existing).Error; err != nil {
@@ -20,7 +22,7 @@ func TestUpdateInbound_RejectsHysteriaClientWithoutAuth(t *testing.T) {
update := existing
update.Protocol = model.Hysteria
update.Settings = `{"clients":[{"email":"hysteria@x","enable":true,"password":"not-hysteria-auth"}]}`
update.Settings = `{"clients":[]}`
svc := &InboundService{}
if _, _, err := svc.UpdateInbound(&update); err == nil || !strings.Contains(err.Error(), "empty client ID") {
@@ -54,7 +56,8 @@ func TestUpdateInbound_PreservesHysteriaClientAuth(t *testing.T) {
update := existing
update.Settings = `{"clients":[{"email":"hysteria@x","enable":true,"password":"` + password + `","auth":"` + wantAuth + `"}]}`
svc := &InboundService{}
// Only a master's push still carries clients through an inbound save.
svc := &InboundService{FromNodeSync: true}
if _, _, err := svc.UpdateInbound(&update); err != nil {
t.Fatalf("UpdateInbound: %v", err)
}
@@ -84,7 +84,8 @@ func TestUpdateInboundMtprotoUnchangedDoesNotRestart(t *testing.T) {
if !strings.Contains(update.Settings, mtprotoTestSecretD) {
t.Fatal("fixture must contain the re-keyed secret")
}
_, needRestart, err := svc.UpdateInbound(&update)
// Clients reach an inbound save only as a master's push to its node.
_, needRestart, err := (&InboundService{FromNodeSync: true}).UpdateInbound(&update)
if err != nil {
t.Fatalf("UpdateInbound: %v", err)
}
+10 -32
View File
@@ -140,44 +140,22 @@ func mergeClientLists(base, ours, current []any) []any {
return out
}
// Client limits and state the inbound form never owns: the client endpoints and
// traffic jobs change them, so a form opened earlier must not write them back.
var storedClientLifecycleKeys = []string{"enable", "expiryTime", "totalGB", "reset", "resetDay", "resetWeekday", "resetMax"}
// keepStoredClientLifecycle copies those keys from the stored settings onto every
// payload client the inbound already holds; new clients keep what they carry.
func keepStoredClientLifecycle(payload, stored string) string {
// keepStoredClients puts the stored client list back into an inbound save's
// payload: clients change through the client endpoints, never this form.
func keepStoredClients(payload, stored string) string {
var payloadM, storedM map[string]any
if json.Unmarshal([]byte(payload), &payloadM) != nil || json.Unmarshal([]byte(stored), &storedM) != nil {
return payload
}
storedClients, _ := storedM["clients"].([]any)
storedBy := indexClientsByEmail(storedClients)
payloadClients, _ := payloadM["clients"].([]any)
changed := false
for _, entry := range payloadClients {
p, email := clientEntryEmail(entry)
s, known := storedBy[email]
if !known {
continue
}
for _, key := range storedClientLifecycleKeys {
sv, inStored := s[key]
pv, inPayload := p[key]
if inStored == inPayload && reflect.DeepEqual(sv, pv) {
continue
}
changed = true
if inStored {
p[key] = sv
} else {
delete(p, key)
}
}
}
if !changed {
storedClients, has := storedM["clients"]
if reflect.DeepEqual(payloadM["clients"], storedClients) {
return payload
}
if has {
payloadM["clients"] = storedClients
} else {
delete(payloadM, "clients")
}
b, err := json.MarshalIndent(payloadM, "", " ")
if err != nil {
return payload
@@ -5,6 +5,7 @@ import (
"time"
"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"
"gorm.io/gorm"
@@ -92,3 +93,78 @@ func TestInboundUpdateKeepsTrafficAddedMidEdit(t *testing.T) {
t.Fatalf("inbound after edit: up=%d down=%d remark=%q, want 100/50/edited", saved.Up, saved.Down, saved.Remark)
}
}
func inboundLinksEmail(t *testing.T, inboundId int, email string) bool {
t.Helper()
var n int64
if err := database.GetDB().Table("client_inbounds").
Joins("JOIN clients ON clients.id = client_inbounds.client_id").
Where("client_inbounds.inbound_id = ? AND clients.email = ?", inboundId, email).
Count(&n).Error; err != nil {
t.Fatalf("count links: %v", err)
}
return n > 0
}
// A client added while the modal was open is not in the list it posts back;
// saving the inbound must not detach it.
func TestInboundFormSaveKeepsClientAddedWhileOpen(t *testing.T) {
setupBulkDB(t)
ib := seedRenewableNeighbour(t, 23204, nil)
form := *ib
if _, err := (&ClientService{}).AddInboundClient(&InboundService{}, &model.Inbound{
Id: ib.Id, Settings: clientsSettings(t, []model.Client{{Email: "z@stale", ID: "aaaaaaaa-0000-0000-0000-00000000000c", Enable: true}}),
}); err != nil {
t.Fatalf("AddInboundClient: %v", err)
}
form.Remark = "edited"
if _, _, err := (&InboundService{}).UpdateInbound(&form); err != nil {
t.Fatalf("UpdateInbound: %v", err)
}
if _, ok := settingsClient(t, ib.Id, "z@stale"); !ok || !inboundLinksEmail(t, ib.Id, "z@stale") {
t.Fatalf("client added while the form was open was dropped: in settings=%v linked=%v", ok, inboundLinksEmail(t, ib.Id, "z@stale"))
}
}
// A client deleted while the modal was open is still in the list it posts
// back; saving must not restore its access.
func TestInboundFormSaveDoesNotResurrectDeletedClient(t *testing.T) {
setupBulkDB(t)
ib := seedRenewableNeighbour(t, 23205, nil)
form := *ib
if _, err := (&ClientService{}).DelInboundClientByEmail(&InboundService{}, ib.Id, "x@stale", false, true); err != nil {
t.Fatalf("DelInboundClientByEmail: %v", err)
}
form.Remark = "edited"
if _, _, err := (&InboundService{}).UpdateInbound(&form); err != nil {
t.Fatalf("UpdateInbound: %v", err)
}
if _, ok := settingsClient(t, ib.Id, "x@stale"); ok || inboundLinksEmail(t, ib.Id, "x@stale") {
t.Fatalf("deleted client came back: in settings=%v linked=%v", ok, inboundLinksEmail(t, ib.Id, "x@stale"))
}
}
// An inbound switched off while the modal was open stays off when the form,
// which still holds enable=true, is saved.
func TestInboundFormSaveKeepsEnableToggledWhileOpen(t *testing.T) {
setupBulkDB(t)
ib := seedRenewableNeighbour(t, 23206, nil)
form := *ib
if _, err := (&InboundService{}).SetInboundEnable(ib.Id, false); err != nil {
t.Fatalf("SetInboundEnable: %v", err)
}
form.Remark = "edited"
if _, _, err := (&InboundService{}).UpdateInbound(&form); err != nil {
t.Fatalf("UpdateInbound: %v", err)
}
saved, err := (&InboundService{}).GetInbound(ib.Id)
if err != nil {
t.Fatalf("GetInbound: %v", err)
}
if saved.Enable || saved.Remark != "edited" {
t.Fatalf("after save: enable=%v remark=%q, want disabled and edited", saved.Enable, saved.Remark)
}
}