From cae3dabf1ed146f50e8bf8021140a58534faae0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCnter=20Grodotzki?= Date: Thu, 23 Apr 2026 22:45:38 +0200 Subject: [PATCH] Fix status page bugs (#19) * Fix status page bugs * simplify --- handler/api_v1_clients.go | 25 +++++++++++++++---------- handler/api_v1_clients_test.go | 19 +++++++++++++++++++ handler/api_v1_users.go | 6 ------ router/api.go | 3 +-- src/pages/ClientsPage.tsx | 6 +++--- src/pages/StatusPage.tsx | 2 +- 6 files changed, 39 insertions(+), 22 deletions(-) diff --git a/handler/api_v1_clients.go b/handler/api_v1_clients.go index 7fa757a..f20758a 100644 --- a/handler/api_v1_clients.go +++ b/handler/api_v1_clients.go @@ -4,7 +4,6 @@ import ( "encoding/base64" "fmt" "net/http" - "sort" "strings" "time" @@ -25,6 +24,10 @@ import ( // connectedThreshold defines how recently a peer must have handshaked to be considered connected var connectedThreshold = 3 * time.Minute +func isConnected(lastHandshake time.Time) bool { + return !lastHandshake.IsZero() && time.Since(lastHandshake) < connectedThreshold +} + func connectedPeerKeys() map[string]bool { keys := make(map[string]bool) wgClient, err := wgctrl.New() @@ -42,7 +45,7 @@ func connectedPeerKeys() map[string]bool { for _, dev := range devices { for _, peer := range dev.Peers { - if time.Since(peer.LastHandshakeTime) < connectedThreshold { + if isConnected(peer.LastHandshakeTime) { keys[peer.PublicKey.String()] = true } } @@ -89,7 +92,6 @@ func APIListClients(db store.IStore) echo.HandlerFunc { filtered := make([]model.ClientData, 0, len(clientDataList)) for _, clientData := range clientDataList { - clientData = util.FillClientSubnetRange(clientData) cl := clientData.Client // Non-admin users can only see clients matching their email @@ -121,7 +123,7 @@ func APIListClients(db store.IStore) echo.HandlerFunc { } } - filtered = append(filtered, clientData) + filtered = append(filtered, util.FillClientSubnetRange(clientData)) } return c.JSON(http.StatusOK, filtered) } @@ -193,7 +195,10 @@ func APICreateClient(db store.IStore, cw *ConfigWriter) echo.HandlerFunc { } // validate name + public key uniqueness in one pass - existingClients, _ := db.GetClients(false) + existingClients, err := db.GetClients(false) + if err != nil { + return apiInternalError(c, "Cannot check for duplicates") + } for _, ec := range existingClients { if strings.EqualFold(ec.Client.Name, client.Name) { return apiBadRequest(c, "A client with this name already exists") @@ -300,7 +305,10 @@ func APIUpdateClient(db store.IStore, cw *ConfigWriter) echo.HandlerFunc { nameChanged := !strings.EqualFold(_client.Name, client.Name) pubKeyChanged := _client.PublicKey != "" && client.PublicKey != _client.PublicKey if nameChanged || pubKeyChanged { - existingClients, _ := db.GetClients(false) + existingClients, err := db.GetClients(false) + if err != nil { + return apiInternalError(c, "Cannot check for duplicates") + } for _, ec := range existingClients { if ec.Client.ID == client.ID { continue @@ -645,7 +653,6 @@ func APIServerStatus(db store.IStore) echo.HandlerFunc { } } - conv := map[bool]int{true: 1, false: 0} for i := range devices { dev := DeviceStatus{Name: devices[i].Name} for j := range devices[i].Peers { @@ -669,7 +676,7 @@ func APIServerStatus(db store.IStore) echo.HandlerFunc { LastHandshakeRel: handshakeRel, AllocatedIP: allocatedIPs, } - p.Connected = p.LastHandshakeRel < connectedThreshold + p.Connected = isConnected(handshakeTime) if devices[i].Peers[j].Endpoint != nil { p.Endpoint = devices[i].Peers[j].Endpoint.String() @@ -681,8 +688,6 @@ func APIServerStatus(db store.IStore) echo.HandlerFunc { } dev.Peers = append(dev.Peers, p) } - sort.SliceStable(dev.Peers, func(a, b int) bool { return dev.Peers[a].Name < dev.Peers[b].Name }) - sort.SliceStable(dev.Peers, func(a, b int) bool { return conv[dev.Peers[a].Connected] > conv[dev.Peers[b].Connected] }) devicesStatus = append(devicesStatus, dev) } } diff --git a/handler/api_v1_clients_test.go b/handler/api_v1_clients_test.go index dfd853a..d4fa15d 100644 --- a/handler/api_v1_clients_test.go +++ b/handler/api_v1_clients_test.go @@ -2071,3 +2071,22 @@ func TestAPIServerStatus(t *testing.T) { // On systems without, it returns 500 assert.Contains(t, []int{http.StatusOK, http.StatusInternalServerError}, rec.Code) } + +// Regression: peers that never connected (zero handshake time) must not be +// reported as connected. Both connectedPeerKeys and APIServerStatus use +// isConnected() which guards against time.Since(zero). +func TestIsConnected_ZeroHandshake(t *testing.T) { + assert.False(t, isConnected(time.Time{}), "zero time must not be connected") +} + +func TestIsConnected_RecentHandshake(t *testing.T) { + assert.True(t, isConnected(time.Now().Add(-30*time.Second)), "30s ago must be connected") +} + +func TestIsConnected_OldHandshake(t *testing.T) { + assert.False(t, isConnected(time.Now().Add(-10*time.Minute)), "10min ago must be disconnected") +} + +func TestIsConnected_ExactlyAtThreshold(t *testing.T) { + assert.False(t, isConnected(time.Now().Add(-connectedThreshold)), "exactly at threshold must be disconnected") +} diff --git a/handler/api_v1_users.go b/handler/api_v1_users.go index 6e837eb..7312790 100644 --- a/handler/api_v1_users.go +++ b/handler/api_v1_users.go @@ -8,7 +8,6 @@ import ( "github.com/labstack/gommon/log" "github.com/DigitalTolk/wireguard-ui/store" - "github.com/DigitalTolk/wireguard-ui/util" ) // APIListUsers returns all users (read-only, managed via SSO) @@ -48,11 +47,6 @@ func APIPatchUserAdmin(db store.IStore) echo.HandlerFunc { return apiInternalError(c, "Cannot update user") } - // update CRC32 cache so existing sessions reflect the change - util.DBUsersToCRC32Mutex.Lock() - util.DBUsersToCRC32[user.Username] = util.GetDBUserCRC32(user) - util.DBUsersToCRC32Mutex.Unlock() - action := "user.demote" if body.Admin { action = "user.promote" diff --git a/router/api.go b/router/api.go index 490e9cc..ab07095 100644 --- a/router/api.go +++ b/router/api.go @@ -42,8 +42,7 @@ func RegisterAPIv1(g *echo.Group, db store.IStore, mailer emailer.Emailer, cw *h settings.GET("", handler.APIGetSettings(db)) settings.PUT("", handler.APIUpdateSettings(db, cw), handler.ContentTypeJson) - // Users (admin only for list/create/delete) - // Users (read-only — managed via SSO) + // Users (admin only) users := g.Group("/users", handler.APIAuth, handler.APIAdmin) users.GET("", handler.APIListUsers(db)) users.GET("/:username", handler.APIGetUser(db)) diff --git a/src/pages/ClientsPage.tsx b/src/pages/ClientsPage.tsx index 475eaf3..c9fe9a2 100644 --- a/src/pages/ClientsPage.tsx +++ b/src/pages/ClientsPage.tsx @@ -97,7 +97,7 @@ function QrCodeDialog({ client, onClose }: { client: ClientData | null; onClose: queryKey: ["client-qr", client?.Client.id], queryFn: () => apiGet<{ qr_code: string }>(`/clients/${client!.Client.id}/qrcode`), enabled: !!client, - staleTime: Infinity, + staleTime: 60_000, }); return ( @@ -570,7 +570,7 @@ export function ClientsPage() { New Client -
setCreateTouched(true)}> +
setCreateTouched(true)}>
Edit Client -
+