Files
3x-ui/internal/web/service/client_update_keepalive_test.go
YoungReckless4 8f162994ef feat(clients): let admins set PersistentKeepalive on tunnel clients (#6377)
* feat(clients): let admins set PersistentKeepalive on tunnel clients

model.Client already carries KeepAlive, and every AmneziaWG/WireGuard client
config emitter already writes PersistentKeepalive when it is above zero -- but
nothing in the UI could set it, so it stayed 0 and the line was never emitted.

Without it a peer that goes quiet has nothing to trigger a handshake: WireGuard
only initiates when it has data to send. An idle client stays disconnected
after any interruption -- a NAT mapping timing out, a device sleeping, the
panel restarting -- until the user generates traffic themselves.

New clients default to 25, the conventional value, which also keeps the NAT
mapping open. Existing clients keep whatever they have, and 0 remains valid and
means "do not send keepalives".

* fix(clients): let an explicit 0 actually disable PersistentKeepalive

Addresses review feedback on the previous commit.

UpdateInboundClient carries a stored keepalive forward whenever the incoming
one is zero, so the settings JSON and the running peer survive a metadata-only
edit that omits the field. That was a 0 -> 0 no-op while no UI could set a
nonzero value. Now that the client form can, the carry-forward became reachable
in the other direction: a client created at the form's default of 25 could
never be returned to 0, and the hint text shipped to all 13 locales -- "0
disables it" -- described something the backend silently refused. The save even
reported success, because a settings blob that came back byte-identical skips
the transaction entirely.

The zero value cannot carry that distinction, so model.Client.KeepAlive becomes
*int: nil means the field was never sent, &0 means "send no keepalives". The
pointer survives the internal marshal in ClientService.Update, which is where an
explicit 0 was being erased by omitempty before UpdateInboundClient ever saw it.
ClientRecord.KeepAlive stays a plain int -- it is the stored column, where
"unset" has no meaning -- and the conversions bridge the two.

Two tests, both red before this change in the direction they cover: an explicit
0 must reach wg_keep_alive, and an update that omits the field must still leave
a stored 25 alone.

Also adds the output transform every other numeric field in the client form
already has, so a cleared box sends 0 rather than null.

* fix(clients): repair the keepalive pointer conversion after the main merge

Merging main brought buildAmneziaWGProxy (#6326) in beside the
Client.KeepAlive int -> *int change without reconciling the new call site,
so internal/sub stopped compiling and took every package importing it with
it. The two sides touched different lines, so git merged them without a
conflict -- the green `make verify` on 112b19a8 predates the break.

ToClient also wrapped a stored 0 in a pointer, so omitempty stopped
omitting: a VLESS client's settings JSON gained "keepAlive": 0 on the
attach and bulk-attach paths, and that JSON reaches xray-core verbatim
through GenXrayInboundConfig. wg_keep_alive cannot tell "off" from "never
set", so a stored 0 now stays nil.

Also copies the regenerated openapi.json over the docs mirror, which
nothing in CI checks, and trims two comment blocks to the two-line cap.

---------

Co-authored-by: Sanaei <ho3ein.sanaei@gmail.com>
2026-09-10 22:32:42 +02:00

103 lines
3.1 KiB
Go

package service
import (
"testing"
"github.com/mhsanaei/3x-ui/v3/internal/database/model"
)
func inboundKeepAlive(t *testing.T, inboundSvc *InboundService, ibId int, email string) int {
t.Helper()
ib, err := inboundSvc.GetInbound(ibId)
if err != nil {
t.Fatalf("GetInbound %d: %v", ibId, err)
}
clients, err := inboundSvc.GetClients(ib)
if err != nil {
t.Fatalf("GetClients %d: %v", ibId, err)
}
for i := range clients {
if clients[i].Email == email {
return clients[i].KeepAliveSeconds()
}
}
t.Fatalf("email %q not found on inbound %d", email, ibId)
return 0
}
// seedKeepAliveClient attaches one WireGuard client already carrying a
// PersistentKeepalive, and returns its inbound and client-record id.
func seedKeepAliveClient(t *testing.T, email string, keepAlive int) (*model.Inbound, int) {
t.Helper()
svc := &ClientService{}
seeded := model.Client{
Email: email,
SubID: "sub-" + email,
Enable: true,
AllowedIPs: []string{"10.0.0.5/32"},
KeepAlive: model.KeepAlivePtr(keepAlive),
}
ib := mkInbound(t, 51820, model.WireGuard, clientsSettings(t, []model.Client{seeded}))
if err := svc.SyncInbound(nil, ib.Id, []model.Client{seeded}); err != nil {
t.Fatalf("seed linkage: %v", err)
}
return ib, lookupClientRecord(t, email).Id
}
// Carrying forward on a zero incoming keepalive was a 0 -> 0 no-op while no UI
// could set the field; once the client form could, "0 disables it" was unreachable.
func TestUpdateCanClearKeepAliveOnAnExistingClient(t *testing.T) {
setupBulkDB(t)
inboundSvc := &InboundService{}
svc := &ClientService{}
ib, recId := seedKeepAliveClient(t, "ka@x", 25)
if got := inboundKeepAlive(t, inboundSvc, ib.Id, "ka@x"); got != 25 {
t.Fatalf("seeded keepAlive = %d, want 25", got)
}
updated := model.Client{
Email: "ka@x",
Enable: true,
AllowedIPs: []string{"10.0.0.5/32"},
KeepAlive: model.KeepAlivePtr(0),
}
if _, err := svc.Update(inboundSvc, recId, updated, 0); err != nil {
t.Fatalf("Update: %v", err)
}
if got := inboundKeepAlive(t, inboundSvc, ib.Id, "ka@x"); got != 0 {
t.Fatalf("inbound keepAlive after an explicit 0 = %d, want 0", got)
}
if got := lookupClientRecord(t, "ka@x").KeepAlive; got != 0 {
t.Fatalf("stored wg_keep_alive after an explicit 0 = %d, want 0", got)
}
}
// The other half of the contract: a payload that never mentions keepAlive (a
// metadata-only edit from the bot or the API) leaves the stored value alone.
func TestUpdateWithoutKeepAlivePreservesTheStoredValue(t *testing.T) {
setupBulkDB(t)
inboundSvc := &InboundService{}
svc := &ClientService{}
ib, recId := seedKeepAliveClient(t, "ka@x", 25)
updated := model.Client{
Email: "ka@x",
Enable: true,
AllowedIPs: []string{"10.0.0.5/32"},
}
if _, err := svc.Update(inboundSvc, recId, updated, 0); err != nil {
t.Fatalf("Update: %v", err)
}
if got := inboundKeepAlive(t, inboundSvc, ib.Id, "ka@x"); got != 25 {
t.Fatalf("inbound keepAlive after an edit that omitted it = %d, want 25", got)
}
if got := lookupClientRecord(t, "ka@x").KeepAlive; got != 25 {
t.Fatalf("stored wg_keep_alive after an edit that omitted it = %d, want 25", got)
}
}