Files
3x-ui/internal/database/model/model_wireguard_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

101 lines
3.2 KiB
Go

package model
import (
"encoding/json"
"reflect"
"strings"
"testing"
)
func TestClientToRecordRoundTripWireGuard(t *testing.T) {
c := &Client{
Email: "alice@example.test",
Enable: true,
PrivateKey: "cGVlci1wcml2YXRlLWtleS1iYXNlNjQtMzJieXRlcw==",
PublicKey: "cGVlci1wdWJsaWMta2V5LWJhc2U2NC0zMmJ5dGVzISE=",
AllowedIPs: []string{"10.0.0.2/32", "fd00::2/128"},
PreSharedKey: "cHNrLWJhc2U2NC0zMmJ5dGVzLXBsYWNlaG9sZGVyISE=",
KeepAlive: KeepAlivePtr(25),
}
rec := c.ToRecord()
if rec.AllowedIPs != "10.0.0.2/32,fd00::2/128" {
t.Fatalf("AllowedIPs CSV = %q, want %q", rec.AllowedIPs, "10.0.0.2/32,fd00::2/128")
}
got := rec.ToClient()
for _, f := range []struct {
name string
a, b any
}{
{"PrivateKey", c.PrivateKey, got.PrivateKey},
{"PublicKey", c.PublicKey, got.PublicKey},
{"PreSharedKey", c.PreSharedKey, got.PreSharedKey},
{"KeepAlive", c.KeepAliveSeconds(), got.KeepAliveSeconds()},
} {
if f.a != f.b {
t.Errorf("%s round-trip = %v, want %v", f.name, f.b, f.a)
}
}
if !reflect.DeepEqual(got.AllowedIPs, c.AllowedIPs) {
t.Errorf("AllowedIPs round-trip = %v, want %v", got.AllowedIPs, c.AllowedIPs)
}
}
// ToClient feeds the settings JSON of every protocol, not just the tunnels, and
// that JSON reaches xray-core verbatim through GenXrayInboundConfig.
func TestClientToClientOmitsUnsetKeepAlive(t *testing.T) {
rec := &ClientRecord{Email: "vless@example.test", UUID: "11111111-2222-3333-4444-555555555555", Enable: true}
if got := rec.ToClient().KeepAlive; got != nil {
t.Fatalf("KeepAlive for a record that never set one = %d, want nil", *got)
}
blob, err := json.Marshal(map[string][]Client{"clients": {*rec.ToClient()}})
if err != nil {
t.Fatalf("marshal settings payload: %v", err)
}
if strings.Contains(string(blob), "keepAlive") {
t.Fatalf("settings payload carries keepAlive for a non-tunnel client: %s", blob)
}
}
func TestClientRecordEmptyAllowedIPs(t *testing.T) {
rec := &ClientRecord{Email: "bob@example.test", AllowedIPs: ""}
if got := rec.ToClient().AllowedIPs; got != nil {
t.Fatalf("empty CSV → AllowedIPs = %v, want nil", got)
}
rec.AllowedIPs = " 10.0.0.5/32 , ,"
if got := rec.ToClient().AllowedIPs; !reflect.DeepEqual(got, []string{"10.0.0.5/32"}) {
t.Fatalf("trimmed CSV → AllowedIPs = %v, want [10.0.0.5/32]", got)
}
}
func TestMergeClientRecordWireGuardKeysPreserved(t *testing.T) {
existing := &ClientRecord{
Email: "carol@example.test",
PrivateKey: "existing-private",
PublicKey: "existing-public",
AllowedIPs: "10.0.0.7/32",
UpdatedAt: 100,
}
incomingEmpty := &ClientRecord{Email: "carol@example.test", UpdatedAt: 200}
MergeClientRecord(existing, incomingEmpty)
if existing.PrivateKey != "existing-private" || existing.PublicKey != "existing-public" {
t.Fatalf("empty incoming wiped keys: priv=%q pub=%q", existing.PrivateKey, existing.PublicKey)
}
if existing.AllowedIPs != "10.0.0.7/32" {
t.Fatalf("empty incoming wiped allowedIPs: %q", existing.AllowedIPs)
}
incomingNewer := &ClientRecord{
Email: "carol@example.test",
AllowedIPs: "10.0.0.8/32",
UpdatedAt: 300,
}
MergeClientRecord(existing, incomingNewer)
if existing.AllowedIPs != "10.0.0.8/32" {
t.Fatalf("newer allowedIPs not applied: %q", existing.AllowedIPs)
}
}