From 823db059660dde3123d79fc67928da2e29696903 Mon Sep 17 00:00:00 2001 From: MHSanaei Date: Mon, 28 Sep 2026 13:23:48 +0200 Subject: [PATCH] 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. --- .../docs/en/reference/api/inbounds.mdx | 22 +++--- docs/public/openapi.json | 2 +- frontend/public/openapi.json | 2 +- frontend/src/lib/xray/inbound-form-adapter.ts | 17 ++++- frontend/src/pages/api-docs/endpoints.ts | 2 +- .../pages/inbounds/form/InboundFormModal.tsx | 29 +++---- frontend/src/test/inbound-form-modal.test.tsx | 30 ++++++++ .../client_renewal_write_validation_test.go | 3 +- internal/web/service/inbound.go | 66 +++++++++------- .../web/service/inbound_hysteria_auth_test.go | 9 ++- .../web/service/inbound_mtproto_apply_test.go | 3 +- .../web/service/inbound_settings_commit.go | 42 +++------- .../service/inbound_update_stale_form_test.go | 76 +++++++++++++++++++ 13 files changed, 207 insertions(+), 96 deletions(-) diff --git a/docs/content/docs/en/reference/api/inbounds.mdx b/docs/content/docs/en/reference/api/inbounds.mdx index 1b80a29b6..a27dc3e73 100644 --- a/docs/content/docs/en/reference/api/inbounds.mdx +++ b/docs/content/docs/en/reference/api/inbounds.mdx @@ -60,12 +60,11 @@ _openapi: at most once. url: '#delete-many-inbounds-in-one-call-processes-the-list-sequentially-failures-are-reported-per-id-and-the-rest-still-proceed-restarts-xray-at-most-once' - depth: 2 - title: Replace an inbound’s configuration. Body shape mirrors /add. Clients the - inbound already holds keep their stored enable, expiryTime, totalGB, - reset, resetDay, resetWeekday and resetMax — change those through the - /panel/api/clients endpoints. Heavy on inbounds with thousands of - clients — prefer /setEnable for enable-only flips. - url: '#replace-an-inbounds-configuration-body-shape-mirrors-add-clients-the-inbound-already-holds-keep-their-stored-enable-expirytime-totalgb-reset-resetday-resetweekday-and-resetmax--change-those-through-the-panelapiclients-endpoints-heavy-on-inbounds-with-thousands-of-clients--prefer-setenable-for-enable-only-flips' + title: 'Replace an inbound’s configuration. Body shape mirrors /add, but the + inbound keeps its stored client list and enable flag: settings.clients + and enable in the body are ignored. Manage clients through the + /panel/api/clients endpoints and toggle the inbound with /setEnable.' + url: '#replace-an-inbounds-configuration-body-shape-mirrors-add-but-the-inbound-keeps-its-stored-client-list-and-enable-flag-settingsclients-and-enable-in-the-body-are-ignored-manage-clients-through-the-panelapiclients-endpoints-and-toggle-the-inbound-with-setenable' - depth: 2 title: Toggle only the enable flag without serialising the whole settings JSON. Recommended for UI switches on large inbounds. @@ -153,12 +152,11 @@ _openapi: failures are reported per id and the rest still proceed. Restarts xray at most once. id: delete-many-inbounds-in-one-call-processes-the-list-sequentially-failures-are-reported-per-id-and-the-rest-still-proceed-restarts-xray-at-most-once - - content: Replace an inbound’s configuration. Body shape mirrors /add. Clients - the inbound already holds keep their stored enable, expiryTime, - totalGB, reset, resetDay, resetWeekday and resetMax — change those - through the /panel/api/clients endpoints. Heavy on inbounds with - thousands of clients — prefer /setEnable for enable-only flips. - id: replace-an-inbounds-configuration-body-shape-mirrors-add-clients-the-inbound-already-holds-keep-their-stored-enable-expirytime-totalgb-reset-resetday-resetweekday-and-resetmax--change-those-through-the-panelapiclients-endpoints-heavy-on-inbounds-with-thousands-of-clients--prefer-setenable-for-enable-only-flips + - content: 'Replace an inbound’s configuration. Body shape mirrors /add, but the + inbound keeps its stored client list and enable flag: settings.clients + and enable in the body are ignored. Manage clients through the + /panel/api/clients endpoints and toggle the inbound with /setEnable.' + id: replace-an-inbounds-configuration-body-shape-mirrors-add-but-the-inbound-keeps-its-stored-client-list-and-enable-flag-settingsclients-and-enable-in-the-body-are-ignored-manage-clients-through-the-panelapiclients-endpoints-and-toggle-the-inbound-with-setenable - content: Toggle only the enable flag without serialising the whole settings JSON. Recommended for UI switches on large inbounds. id: toggle-only-the-enable-flag-without-serialising-the-whole-settings-json-recommended-for-ui-switches-on-large-inbounds diff --git a/docs/public/openapi.json b/docs/public/openapi.json index a27056eab..307cf7396 100644 --- a/docs/public/openapi.json +++ b/docs/public/openapi.json @@ -5603,7 +5603,7 @@ "tags": [ "Inbounds" ], - "summary": "Replace an inbound’s configuration. Body shape mirrors /add. Clients the inbound already holds keep their stored enable, expiryTime, totalGB, reset, resetDay, resetWeekday and resetMax — change those through the /panel/api/clients endpoints. Heavy on inbounds with thousands of clients — prefer /setEnable for enable-only flips.", + "summary": "Replace an inbound’s configuration. Body shape mirrors /add, but the inbound keeps its stored client list and enable flag: settings.clients and enable in the body are ignored. Manage clients through the /panel/api/clients endpoints and toggle the inbound with /setEnable.", "operationId": "post_panel_api_inbounds_update_id", "parameters": [ { diff --git a/frontend/public/openapi.json b/frontend/public/openapi.json index a27056eab..307cf7396 100644 --- a/frontend/public/openapi.json +++ b/frontend/public/openapi.json @@ -5603,7 +5603,7 @@ "tags": [ "Inbounds" ], - "summary": "Replace an inbound’s configuration. Body shape mirrors /add. Clients the inbound already holds keep their stored enable, expiryTime, totalGB, reset, resetDay, resetWeekday and resetMax — change those through the /panel/api/clients endpoints. Heavy on inbounds with thousands of clients — prefer /setEnable for enable-only flips.", + "summary": "Replace an inbound’s configuration. Body shape mirrors /add, but the inbound keeps its stored client list and enable flag: settings.clients and enable in the body are ignored. Manage clients through the /panel/api/clients endpoints and toggle the inbound with /setEnable.", "operationId": "post_panel_api_inbounds_update_id", "parameters": [ { diff --git a/frontend/src/lib/xray/inbound-form-adapter.ts b/frontend/src/lib/xray/inbound-form-adapter.ts index cba1fd2cf..7aba62c62 100644 --- a/frontend/src/lib/xray/inbound-form-adapter.ts +++ b/frontend/src/lib/xray/inbound-form-adapter.ts @@ -353,9 +353,22 @@ export function dropLegacyOptionalEmpties( } } -export function formValuesToWirePayload(values: InboundFormValues): WireInboundPayload { +// An existing inbound's clients change only through the client endpoints, so +// the edit form neither loads them nor sends them back. +export function withoutClients(values: InboundFormValues): InboundFormValues { + const settings = { ...(values.settings as Record | undefined) }; + delete settings.clients; + return { ...values, settings } as InboundFormValues; +} + +export function formValuesToWirePayload( + values: InboundFormValues, + options: { omitClients?: boolean } = {}, +): WireInboundPayload { const settingsPruned = (pruneEmpty(values.settings ?? {}) ?? {}) as Record; - if (Array.isArray(settingsPruned.clients)) { + if (options.omitClients) { + delete settingsPruned.clients; + } else if (Array.isArray(settingsPruned.clients)) { settingsPruned.clients = normalizeClients(values.protocol, settingsPruned.clients); } let streamPruned = values.streamSettings diff --git a/frontend/src/pages/api-docs/endpoints.ts b/frontend/src/pages/api-docs/endpoints.ts index b0666a5c6..a8002dde9 100644 --- a/frontend/src/pages/api-docs/endpoints.ts +++ b/frontend/src/pages/api-docs/endpoints.ts @@ -325,7 +325,7 @@ export const sections: readonly Section[] = [ method: 'POST', path: '/panel/api/inbounds/update/:id', summary: - 'Replace an inbound’s configuration. Body shape mirrors /add. Clients the inbound already holds keep their stored enable, expiryTime, totalGB, reset, resetDay, resetWeekday and resetMax — change those through the /panel/api/clients endpoints. Heavy on inbounds with thousands of clients — prefer /setEnable for enable-only flips.', + 'Replace an inbound’s configuration. Body shape mirrors /add, but the inbound keeps its stored client list and enable flag: settings.clients and enable in the body are ignored. Manage clients through the /panel/api/clients endpoints and toggle the inbound with /setEnable.', params: [{ name: 'id', in: 'path', type: 'number', desc: 'Inbound ID.' }], body: inboundBody, }, diff --git a/frontend/src/pages/inbounds/form/InboundFormModal.tsx b/frontend/src/pages/inbounds/form/InboundFormModal.tsx index e9fdcc218..ac0954ea4 100644 --- a/frontend/src/pages/inbounds/form/InboundFormModal.tsx +++ b/frontend/src/pages/inbounds/form/InboundFormModal.tsx @@ -19,7 +19,11 @@ import { Controller, FormProvider, useForm, useWatch } from 'react-hook-form'; import { HttpUtil, NumberFormatter, RandomUtil, SizeFormatter, Wireguard } from '@/utils'; import type { RealityScanResult } from '@/generated/types'; -import { rawInboundToFormValues, formValuesToWirePayload } from '@/lib/xray/inbound-form-adapter'; +import { + rawInboundToFormValues, + formValuesToWirePayload, + withoutClients, +} from '@/lib/xray/inbound-form-adapter'; import { createDefaultInboundSettings } from '@/lib/xray/inbound-defaults'; import { generateAwgObfuscation } from '@/lib/xray/amneziawg-obfuscation'; import { composeInboundTag, isAutoInboundTag, type InboundTagInput } from '@/lib/xray/inbound-tag'; @@ -439,7 +443,9 @@ export default function InboundFormModal({ useEffect(() => { if (!open) return; const initial = - mode === 'edit' && dbInbound ? rawInboundToFormValues(dbInbound) : buildAddModeValues(); + mode === 'edit' && dbInbound + ? withoutClients(rawInboundToFormValues(dbInbound)) + : buildAddModeValues(); methods.reset(initial); setScanResult(null); setActiveTab('basic'); @@ -556,13 +562,8 @@ export default function InboundFormModal({ }, [mode, methods]); const saveValues = async () => { - /* - * getValues() returns the entire form store, including settings.clients and - * settings.fallbacks which have no bound field (clients are managed via the - * standalone Client modal, not this inbound modal). With shouldUnregister - * false those pass-through sub-trees survive from the reset object, so the - * update wire payload never silently drops every client on save. - */ + // settings.fallbacks has no bound field; shouldUnregister=false keeps it from + // the reset object. An edit sends no clients: the server keeps the stored ones. const values = methods.getValues() as InboundFormValues; const parsed = InboundFormSchema.safeParse(values); if (!parsed.success) { @@ -577,7 +578,7 @@ export default function InboundFormModal({ } setSaving(true); try { - const payload = formValuesToWirePayload(parsed.data); + const payload = formValuesToWirePayload(parsed.data, { omitClients: mode === 'edit' }); const url = mode === 'edit' && dbInbound ? `/panel/api/inbounds/update/${dbInbound.id}` @@ -615,9 +616,11 @@ export default function InboundFormModal({ const basicTab = ( <> - - - + {mode === 'add' && ( + + + + )} diff --git a/frontend/src/test/inbound-form-modal.test.tsx b/frontend/src/test/inbound-form-modal.test.tsx index 8790a7a48..93351dd26 100644 --- a/frontend/src/test/inbound-form-modal.test.tsx +++ b/frontend/src/test/inbound-form-modal.test.tsx @@ -317,4 +317,34 @@ describe('InboundFormModal', () => { ); }); }); + + // Clients and enable change through their own endpoints; the server keeps the + // stored ones, so the edit form must neither send nor validate its stale copy. + it('edit save neither sends nor validates the clients it loaded', async () => { + const post = vi.mocked(HttpUtil.post); + post.mockClear(); + const dbInbound = cloneLikeVlessInbound('example.com:443'); + const legacy = new DBInbound({ + ...dbInbound, + settings: { + ...(dbInbound.settings as Record), + clients: [{ email: 'legacy', id: '' }], + }, + }); + renderCloneLikeEdit(legacy); + + fireEvent.click(primaryButton()); + + await waitFor(() => expect(post).toHaveBeenCalled()); + const payload = post.mock.calls[0][1] as { settings: string }; + expect(JSON.parse(payload.settings)).not.toHaveProperty('clients'); + }); + + it('offers the enable switch when adding an inbound but not when editing one', () => { + renderModal(); + expect(document.getElementById('inbound-enable')).not.toBeNull(); + cleanup(); + renderCloneLikeEdit(cloneLikeVlessInbound('example.com:443')); + expect(document.getElementById('inbound-enable')).toBeNull(); + }); }); diff --git a/internal/web/service/client_renewal_write_validation_test.go b/internal/web/service/client_renewal_write_validation_test.go index 5652857aa..ba7a9216b 100644 --- a/internal/web/service/client_renewal_write_validation_test.go +++ b/internal/web/service/client_renewal_write_validation_test.go @@ -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}) diff --git a/internal/web/service/inbound.go b/internal/web/service/inbound.go index cd91e2936..332a51839 100644 --- a/internal/web/service/inbound.go +++ b/internal/web/service/inbound.go @@ -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 { diff --git a/internal/web/service/inbound_hysteria_auth_test.go b/internal/web/service/inbound_hysteria_auth_test.go index 9000876e3..f688b8c85 100644 --- a/internal/web/service/inbound_hysteria_auth_test.go +++ b/internal/web/service/inbound_hysteria_auth_test.go @@ -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) } diff --git a/internal/web/service/inbound_mtproto_apply_test.go b/internal/web/service/inbound_mtproto_apply_test.go index 82bfd5ef0..32edf7d49 100644 --- a/internal/web/service/inbound_mtproto_apply_test.go +++ b/internal/web/service/inbound_mtproto_apply_test.go @@ -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) } diff --git a/internal/web/service/inbound_settings_commit.go b/internal/web/service/inbound_settings_commit.go index 163c57c86..5ad8e48dd 100644 --- a/internal/web/service/inbound_settings_commit.go +++ b/internal/web/service/inbound_settings_commit.go @@ -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 diff --git a/internal/web/service/inbound_update_stale_form_test.go b/internal/web/service/inbound_update_stale_form_test.go index 4cb78bb00..136270107 100644 --- a/internal/web/service/inbound_update_stale_form_test.go +++ b/internal/web/service/inbound_update_stale_form_test.go @@ -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) + } +}