mirror of
https://github.com/MHSanaei/3x-ui.git
synced 2026-09-11 04:37:16 +00:00
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.
This commit is contained in:
@@ -1147,6 +1147,8 @@
|
||||
"type": "string"
|
||||
},
|
||||
"keepAlive": {
|
||||
"description": "Seconds between PersistentKeepalive packets; 0 sends none, omit to keep the stored value",
|
||||
"nullable": true,
|
||||
"type": "integer"
|
||||
},
|
||||
"limitIp": {
|
||||
|
||||
@@ -275,7 +275,7 @@ export const EXAMPLES: Record<string, unknown> = {
|
||||
"forwardedPorts": "",
|
||||
"group": "",
|
||||
"id": "",
|
||||
"keepAlive": 0,
|
||||
"keepAlive": null,
|
||||
"limitIp": 0,
|
||||
"password": "",
|
||||
"preSharedKey": "",
|
||||
|
||||
@@ -1121,6 +1121,8 @@ export const SCHEMAS: Record<string, unknown> = {
|
||||
"type": "string"
|
||||
},
|
||||
"keepAlive": {
|
||||
"description": "Seconds between PersistentKeepalive packets; 0 sends none, omit to keep the stored value",
|
||||
"nullable": true,
|
||||
"type": "integer"
|
||||
},
|
||||
"limitIp": {
|
||||
|
||||
@@ -271,7 +271,7 @@ export interface Client {
|
||||
forwardedPorts?: string;
|
||||
group?: string;
|
||||
id?: string;
|
||||
keepAlive?: number;
|
||||
keepAlive?: number | null;
|
||||
limitIp: number;
|
||||
password?: string;
|
||||
preSharedKey?: string;
|
||||
|
||||
@@ -292,7 +292,7 @@ export const ClientSchema = z.object({
|
||||
forwardedPorts: z.string().optional(),
|
||||
group: z.string().optional(),
|
||||
id: z.string().optional(),
|
||||
keepAlive: z.number().int().optional(),
|
||||
keepAlive: z.number().int().nullable().optional(),
|
||||
limitIp: z.number().int(),
|
||||
password: z.string().optional(),
|
||||
preSharedKey: z.string().optional(),
|
||||
|
||||
@@ -1279,6 +1279,7 @@ export default function ClientFormModal({
|
||||
name="wgKeepAlive"
|
||||
label={t('pages.clients.tunnelKeepAlive')}
|
||||
extra={t('pages.clients.tunnelKeepAliveHint')}
|
||||
transform={{ output: (v) => Number(v) || 0 }}
|
||||
>
|
||||
<InputNumber min={0} max={65535} style={{ width: '100%' }} />
|
||||
</FormField>
|
||||
|
||||
@@ -441,8 +441,8 @@ func WireguardPeerFromClient(c Client) map[string]any {
|
||||
if c.PreSharedKey != "" {
|
||||
peer["preSharedKey"] = c.PreSharedKey
|
||||
}
|
||||
if c.KeepAlive > 0 {
|
||||
peer["keepAlive"] = c.KeepAlive
|
||||
if ka := c.KeepAliveSeconds(); ka > 0 {
|
||||
peer["keepAlive"] = ka
|
||||
}
|
||||
return peer
|
||||
}
|
||||
@@ -890,7 +890,7 @@ type Client struct {
|
||||
// before -- fully backward compatible for callers that never set this.
|
||||
AllowedIPsByInbound map[int][]string `json:"allowedIPsByInbound,omitempty"`
|
||||
PreSharedKey string `json:"preSharedKey,omitempty"`
|
||||
KeepAlive int `json:"keepAlive,omitempty"`
|
||||
KeepAlive *int `json:"keepAlive,omitempty"` // Seconds between PersistentKeepalive packets; 0 sends none, omit to keep the stored value
|
||||
ForwardedPorts string `json:"forwardedPorts,omitempty"` // AmneziaWG per-client port-forwarding spec, e.g. "80,443,8000-8100"
|
||||
Secret string `json:"secret,omitempty" example:"ee1234567890abcdef1234567890abcd7777772e636c6f7564666c6172652e636f6d"`
|
||||
AdTag string `json:"adTag,omitempty" example:"0123456789abcdef0123456789abcdef"`
|
||||
@@ -1113,6 +1113,18 @@ type Host struct {
|
||||
|
||||
func (Host) TableName() string { return "hosts" }
|
||||
|
||||
// KeepAliveSeconds is the client's PersistentKeepalive, 0 when unset.
|
||||
func (c Client) KeepAliveSeconds() int {
|
||||
if c.KeepAlive == nil {
|
||||
return 0
|
||||
}
|
||||
return *c.KeepAlive
|
||||
}
|
||||
|
||||
// KeepAlivePtr wraps an explicit PersistentKeepalive, 0 included -- distinct
|
||||
// from a nil KeepAlive, which means the field was never sent.
|
||||
func KeepAlivePtr(v int) *int { return &v }
|
||||
|
||||
func (c *Client) ToRecord() *ClientRecord {
|
||||
rec := &ClientRecord{
|
||||
Email: c.Email,
|
||||
@@ -1141,7 +1153,7 @@ func (c *Client) ToRecord() *ClientRecord {
|
||||
PublicKey: c.PublicKey,
|
||||
AllowedIPs: strings.Join(c.AllowedIPs, ","),
|
||||
PreSharedKey: c.PreSharedKey,
|
||||
KeepAlive: c.KeepAlive,
|
||||
KeepAlive: c.KeepAliveSeconds(),
|
||||
ForwardedPorts: c.ForwardedPorts,
|
||||
Secret: c.Secret,
|
||||
AdTag: c.AdTag,
|
||||
@@ -1199,7 +1211,7 @@ func (r *ClientRecord) ToClient() *Client {
|
||||
PublicKey: r.PublicKey,
|
||||
AllowedIPs: splitWireguardAllowedIPs(r.AllowedIPs),
|
||||
PreSharedKey: r.PreSharedKey,
|
||||
KeepAlive: r.KeepAlive,
|
||||
KeepAlive: KeepAlivePtr(r.KeepAlive),
|
||||
ForwardedPorts: r.ForwardedPorts,
|
||||
Secret: r.Secret,
|
||||
AdTag: r.AdTag,
|
||||
|
||||
@@ -13,7 +13,7 @@ func TestClientToRecordRoundTripWireGuard(t *testing.T) {
|
||||
PublicKey: "cGVlci1wdWJsaWMta2V5LWJhc2U2NC0zMmJ5dGVzISE=",
|
||||
AllowedIPs: []string{"10.0.0.2/32", "fd00::2/128"},
|
||||
PreSharedKey: "cHNrLWJhc2U2NC0zMmJ5dGVzLXBsYWNlaG9sZGVyISE=",
|
||||
KeepAlive: 25,
|
||||
KeepAlive: KeepAlivePtr(25),
|
||||
}
|
||||
|
||||
rec := c.ToRecord()
|
||||
@@ -29,7 +29,7 @@ func TestClientToRecordRoundTripWireGuard(t *testing.T) {
|
||||
{"PrivateKey", c.PrivateKey, got.PrivateKey},
|
||||
{"PublicKey", c.PublicKey, got.PublicKey},
|
||||
{"PreSharedKey", c.PreSharedKey, got.PreSharedKey},
|
||||
{"KeepAlive", c.KeepAlive, got.KeepAlive},
|
||||
{"KeepAlive", c.KeepAliveSeconds(), got.KeepAliveSeconds()},
|
||||
} {
|
||||
if f.a != f.b {
|
||||
t.Errorf("%s round-trip = %v, want %v", f.name, f.b, f.a)
|
||||
|
||||
@@ -407,8 +407,8 @@ func (s *SubClashService) buildWireguardProxy(subReq *SubService, inbound *model
|
||||
if client.PreSharedKey != "" {
|
||||
proxy["pre-shared-key"] = client.PreSharedKey
|
||||
}
|
||||
if client.KeepAlive > 0 {
|
||||
proxy["persistent-keepalive"] = client.KeepAlive
|
||||
if ka := client.KeepAliveSeconds(); ka > 0 {
|
||||
proxy["persistent-keepalive"] = ka
|
||||
}
|
||||
for _, addr := range client.AllowedIPs {
|
||||
ip := stripCIDR(addr)
|
||||
|
||||
@@ -828,7 +828,7 @@ func TestBuildWireguardProxyForClash(t *testing.T) {
|
||||
Email: "user",
|
||||
PrivateKey: clientPriv,
|
||||
PreSharedKey: "psk-value",
|
||||
KeepAlive: 25,
|
||||
KeepAlive: model.KeepAlivePtr(25),
|
||||
AllowedIPs: []string{"10.0.0.2/32", "fd00::2/128"},
|
||||
}
|
||||
|
||||
|
||||
@@ -866,8 +866,8 @@ func (s *SubJsonService) genWireguard(inbound *model.Inbound, client model.Clien
|
||||
if client.PreSharedKey != "" {
|
||||
peer["preSharedKey"] = client.PreSharedKey
|
||||
}
|
||||
if client.KeepAlive > 0 {
|
||||
peer["keepAlive"] = client.KeepAlive
|
||||
if ka := client.KeepAliveSeconds(); ka > 0 {
|
||||
peer["keepAlive"] = ka
|
||||
}
|
||||
|
||||
settings := map[string]any{
|
||||
|
||||
@@ -380,7 +380,7 @@ func TestSubJsonServiceWireguard(t *testing.T) {
|
||||
Email: "user",
|
||||
PrivateKey: clientPriv,
|
||||
PreSharedKey: "psk-value",
|
||||
KeepAlive: 25,
|
||||
KeepAlive: model.KeepAlivePtr(25),
|
||||
AllowedIPs: []string{"10.0.0.2/32", "fd00::2/128"},
|
||||
}
|
||||
|
||||
|
||||
@@ -677,8 +677,8 @@ func (s *SubService) genWireguardLink(inbound *model.Inbound, email string) stri
|
||||
if client.PreSharedKey != "" {
|
||||
params["presharedkey"] = client.PreSharedKey
|
||||
}
|
||||
if client.KeepAlive > 0 {
|
||||
params["keepalive"] = strconv.Itoa(client.KeepAlive)
|
||||
if ka := client.KeepAliveSeconds(); ka > 0 {
|
||||
params["keepalive"] = strconv.Itoa(ka)
|
||||
}
|
||||
return buildLinkWithParams(link, params, s.genRemark(inbound, email, "", ""))
|
||||
}
|
||||
@@ -779,8 +779,8 @@ func amneziaWGConfigText(server *amneziawg.ServerSettings, client *model.Client,
|
||||
}
|
||||
b.WriteString("AllowedIPs = 0.0.0.0/0, ::/0\n")
|
||||
fmt.Fprintf(&b, "Endpoint = %s:%d", host, port)
|
||||
if client.KeepAlive > 0 {
|
||||
fmt.Fprintf(&b, "\nPersistentKeepalive = %d", client.KeepAlive)
|
||||
if ka := client.KeepAliveSeconds(); ka > 0 {
|
||||
fmt.Fprintf(&b, "\nPersistentKeepalive = %d", ka)
|
||||
}
|
||||
|
||||
return b.String()
|
||||
|
||||
@@ -214,7 +214,7 @@ func TestAmneziaWGConfigTextPeerFieldOrder(t *testing.T) {
|
||||
server := &amneziawg.ServerSettings{PublicKey: "serverPub", PrimaryDNS: "8.8.8.8", MTU: 1420}
|
||||
|
||||
t.Run("every optional field set", func(t *testing.T) {
|
||||
client := &model.Client{PrivateKey: "clientPriv", AllowedIPs: []string{"10.8.1.2/32"}, PreSharedKey: "psk", KeepAlive: 25}
|
||||
client := &model.Client{PrivateKey: "clientPriv", AllowedIPs: []string{"10.8.1.2/32"}, PreSharedKey: "psk", KeepAlive: model.KeepAlivePtr(25)}
|
||||
conf := amneziaWGConfigText(server, client, "203.0.113.7", 51820, "remark")
|
||||
if got := peerFields(t, conf); !slices.Equal(got, peerFieldOrder) {
|
||||
t.Fatalf("peer fields = %v, want %v\n%s", got, peerFieldOrder, conf)
|
||||
|
||||
@@ -241,7 +241,7 @@ func (l *Local) AddClient(ctx context.Context, ib *model.Inbound, client model.C
|
||||
"publicKey": client.PublicKey,
|
||||
"allowedIPs": client.AllowedIPs,
|
||||
"preSharedKey": client.PreSharedKey,
|
||||
"keepAlive": wgKeepAlive(client.KeepAlive),
|
||||
"keepAlive": wgKeepAlive(client.KeepAliveSeconds()),
|
||||
}
|
||||
return l.AddUser(ctx, ib, user)
|
||||
}
|
||||
@@ -282,7 +282,7 @@ func (l *Local) UpdateUser(ctx context.Context, ib *model.Inbound, oldEmail stri
|
||||
"publicKey": payload.PublicKey,
|
||||
"allowedIPs": payload.AllowedIPs,
|
||||
"preSharedKey": payload.PreSharedKey,
|
||||
"keepAlive": wgKeepAlive(payload.KeepAlive),
|
||||
"keepAlive": wgKeepAlive(payload.KeepAliveSeconds()),
|
||||
}
|
||||
return l.AddUser(ctx, ib, user)
|
||||
}
|
||||
|
||||
@@ -583,7 +583,7 @@ func (s *ClientService) AddInboundClient(inboundSvc *InboundService, data *model
|
||||
"publicKey": client.PublicKey,
|
||||
"allowedIPs": client.AllowedIPs,
|
||||
"preSharedKey": client.PreSharedKey,
|
||||
"keepAlive": keepAliveStr(client.KeepAlive),
|
||||
"keepAlive": keepAliveStr(client.KeepAliveSeconds()),
|
||||
})
|
||||
if err1 == nil {
|
||||
logger.Debug("Client added on", rt.Name(), ":", client.Email)
|
||||
@@ -727,7 +727,7 @@ func (s *ClientService) UpdateInboundClient(inboundSvc *InboundService, data *mo
|
||||
if clients[0].PreSharedKey == "" {
|
||||
clients[0].PreSharedKey = old.PreSharedKey
|
||||
}
|
||||
if clients[0].KeepAlive == 0 {
|
||||
if clients[0].KeepAlive == nil {
|
||||
clients[0].KeepAlive = old.KeepAlive
|
||||
}
|
||||
// ForwardedPorts is AmneziaWG-only (WireGuard's own inbound never
|
||||
@@ -794,8 +794,8 @@ func (s *ClientService) UpdateInboundClient(inboundSvc *InboundService, data *mo
|
||||
if clients[0].PreSharedKey != "" {
|
||||
newMap["preSharedKey"] = clients[0].PreSharedKey
|
||||
}
|
||||
if clients[0].KeepAlive > 0 {
|
||||
newMap["keepAlive"] = clients[0].KeepAlive
|
||||
if ka := clients[0].KeepAliveSeconds(); ka > 0 {
|
||||
newMap["keepAlive"] = ka
|
||||
}
|
||||
if oldInbound.Protocol == model.AmneziaWG && clients[0].ForwardedPorts != "" {
|
||||
newMap["forwardedPorts"] = clients[0].ForwardedPorts
|
||||
@@ -1007,7 +1007,7 @@ func (s *ClientService) UpdateInboundClient(inboundSvc *InboundService, data *mo
|
||||
"publicKey": clients[0].PublicKey,
|
||||
"allowedIPs": clients[0].AllowedIPs,
|
||||
"preSharedKey": clients[0].PreSharedKey,
|
||||
"keepAlive": keepAliveStr(clients[0].KeepAlive),
|
||||
"keepAlive": keepAliveStr(clients[0].KeepAliveSeconds()),
|
||||
})
|
||||
if err1 == nil {
|
||||
logger.Debug("Client edited on", rt.Name(), ":", clients[0].Email)
|
||||
|
||||
@@ -0,0 +1,104 @@
|
||||
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
|
||||
}
|
||||
|
||||
// The update path restores the stored keepalive whenever the incoming one is
|
||||
// zero. That was a 0 -> 0 no-op while no UI could set the field; once the
|
||||
// client form could, "0 disables it" became unreachable on an existing client.
|
||||
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 same contract: a payload that never mentions
|
||||
// keepAlive (a metadata-only edit from the bot or the API) must still leave
|
||||
// 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)
|
||||
}
|
||||
}
|
||||
@@ -249,8 +249,8 @@ func defaultWireguardClients(settingsJSON string, existing, clients []model.Clie
|
||||
if c.PreSharedKey != "" {
|
||||
m["preSharedKey"] = c.PreSharedKey
|
||||
}
|
||||
if c.KeepAlive > 0 {
|
||||
m["keepAlive"] = c.KeepAlive
|
||||
if ka := c.KeepAliveSeconds(); ka > 0 {
|
||||
m["keepAlive"] = ka
|
||||
}
|
||||
interfaceClients[i] = m
|
||||
}
|
||||
|
||||
@@ -77,7 +77,7 @@ func wgPeerList(t *testing.T, settings map[string]any) []map[string]any {
|
||||
|
||||
func TestGetXrayConfigWireGuardPeers(t *testing.T) {
|
||||
clients := []model.Client{
|
||||
{Email: "alice@wg.test", Enable: true, PublicKey: "pub-alice", AllowedIPs: []string{"10.0.0.2/32"}, KeepAlive: 25},
|
||||
{Email: "alice@wg.test", Enable: true, PublicKey: "pub-alice", AllowedIPs: []string{"10.0.0.2/32"}, KeepAlive: model.KeepAlivePtr(25)},
|
||||
{Email: "bob@wg.test", Enable: true, PublicKey: "pub-bob", AllowedIPs: []string{"10.0.0.3/32"}},
|
||||
}
|
||||
seedWGInbound(t, "wg-multi", 51820, clients)
|
||||
|
||||
Reference in New Issue
Block a user