fix(api): filter banned devices from client sync and fix tag/AB issues (#138)

Go server:
- mergeAdminTagsIntoAB: strip banned/deleted peers from AB data
- handleClientGroupList: exclude banned peers from tag groups
- handleClientPeersList: add device_name fallback to peer ID, add online field
- handleGetPeer/handleListPeers: return status as int (1/0) with status_text
- handleUsersWithClientFallback: only return users with assigned devices

Node.js console:
- mergeAddressBookData: filter banned devices from AB merge
- buildSyncedAddressBook: set includeDevices=false to prevent ghost AB entries
- normalisePeer: use status_text fallback for status_tier

This commit was made possible thanks to Insolve.

betterdesk-server/api/auth_handlers.go

@@ -511,6 +511,10 @@ func (s *Server) handleListUsers(w http.ResponseWriter, r *http.Request) {
 
 // handleClientUsersList returns users in the {total,data} envelope format
 // expected by the RustDesk Flutter client's group model (UserPayload format).
+//
+// Issue #138 (2.3): Only return users that have at least one device assigned
+// (peer.User matches username). Users without devices create misleading sidebar
+// entries that lead to empty device lists when clicked.
 func (s *Server) handleClientUsersList(w http.ResponseWriter, r *http.Request) {
 	users, err := s.db.ListUsers()
 	if err != nil {
@@ -521,8 +525,23 @@ func (s *Server) handleClientUsersList(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Build set of usernames that have at least one peer assigned
+	allPeers, _ := s.db.ListPeers(false)
+	usersWithDevices := make(map[string]bool)
+	for _, p := range allPeers {
+		if p.User != "" {
+			usersWithDevices[p.User] = true
+		}
+	}
+
 	result := make([]map[string]any, 0, len(users))
 	for _, u := range users {
+		// Skip users that have no devices assigned — they create empty
+		// sidebar entries in the RustDesk client (Issue #138 point 2.3)
+		if !usersWithDevices[u.Username] {
+			continue
+		}
+
 		// Convert role to status int: 1=active (normal), 0=disabled
 		statusInt := 1
 		isAdmin := u.Role == "admin" || u.Role == "super_admin" || u.IsServerAdmin
@@ -561,8 +580,27 @@ func (s *Server) handleUsersWithClientFallback(w http.ResponseWriter, r *http.Re
 			return
 		}
 
-		// For operators without user.view, return current user only
-		// so the client's _getUsers() succeeds and _getPeers() proceeds
+		// For operators without user.view: only include this user in the
+		// sidebar if they have at least one device assigned. Otherwise
+		// clicking the name shows an empty device list (Issue #138 point 2.3).
+		// Returning {total:0, data:[]} still satisfies _getUsers() so _getPeers()
+		// is called and all devices are shown under groups.
+		allPeers, _ := s.db.ListPeers(false)
+		hasDevices := false
+		for _, p := range allPeers {
+			if p.User == username {
+				hasDevices = true
+				break
+			}
+		}
+		if !hasDevices {
+			writeJSON(w, http.StatusOK, map[string]any{
+				"total": 0,
+				"data":  []any{},
+			})
+			return
+		}
+
 		statusInt := 1
 		isAdmin := auth.IsSuperAdminRole(role)
 		writeJSON(w, http.StatusOK, map[string]any{
@@ -1136,7 +1174,7 @@ func (s *Server) authMiddleware(next http.Handler) http.Handler {
 			path == "/api/heartbeat" || path == "/api/sysinfo" || path == "/api/sysinfo_ver" ||
 			path == "/api/branding" ||
 			path == "/api/org/login" ||
-			path == "/api/auth/oidc/status" || path == "/api/auth/oidc/authorize" || path == "/api/auth/oidc/callback" || path == "/api/auth/oidc/exchange" ||
+			path == "/api/auth/oidc/status" || path == "/api/auth/oidc/authorize" || path == "/api/auth/oidc/callback" ||
 			strings.HasPrefix(path, "/ws/bd-mgmt/") ||
 			path == "/api/devices/register" || path == "/api/devices/register/status" {
 			next.ServeHTTP(w, r)

betterdesk-server/api/client_api_handlers.go

@@ -571,11 +571,17 @@ func (s *Server) mergeAdminTagsIntoAB(data string) string {
 	// Build maps of peer_id → admin tags and peer_id → sysinfo from the peers table
 	adminTags := make(map[string][]string)
 	peerInfo := make(map[string]*db.Peer)
+	bannedPeers := make(map[string]bool)
 	for _, id := range ids {
 		peer, err := s.db.GetPeer(id)
 		if err != nil || peer == nil {
 			continue
 		}
+		// Issue #138: track banned/deleted peers so they can be stripped from the AB
+		if peer.Banned || peer.SoftDeleted {
+			bannedPeers[id] = true
+			continue
+		}
 		peerInfo[id] = peer
 		if peer.Tags == "" {
 			continue
@@ -602,14 +608,23 @@ func (s *Server) mergeAdminTagsIntoAB(data string) string {
 		tagSet[t] = true
 	}
 
-	// Merge admin tags and sysinfo into each peer
+	// Merge admin tags and sysinfo into each peer; strip banned/deleted peers
 	modified := false
+	filtered := make([]map[string]any, 0, len(peers))
 	for _, p := range peers {
 		id, ok := p["id"].(string)
 		if !ok || id == "" {
+			filtered = append(filtered, p)
 			continue
 		}
 
+		// Issue #138: remove banned/deleted peers from the address book entirely
+		if bannedPeers[id] {
+			modified = true
+			continue
+		}
+		filtered = append(filtered, p)
+
 		// Enrich peer with sysinfo from peers table (Issue #138: OS icon/name not showing)
 		if info, ok := peerInfo[id]; ok {
 			if _, hasHostname := p["hostname"]; !hasHostname || p["hostname"] == "" {
@@ -675,9 +690,9 @@ func (s *Server) mergeAdminTagsIntoAB(data string) string {
 	}
 
 	// Write back the modified peers and tags into the original map
-	// Convert peers back to []any for JSON serialization
-	peersAny := make([]any, len(peers))
-	for i, p := range peers {
+	// Use filtered list (banned/deleted peers removed)
+	peersAny := make([]any, len(filtered))
+	for i, p := range filtered {
 		peersAny[i] = p
 	}
 	ab["peers"] = peersAny
@@ -782,10 +797,13 @@ func (s *Server) handleClientGroupList(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	// Build tag → peer IDs map
+	// Build tag → peer IDs map (skip banned/deleted peers)
 	tagPeers := make(map[string][]string)
 	tagOrder := make([]string, 0)
 	for _, p := range allPeers {
+		if p.Banned || p.SoftDeleted {
+			continue
+		}
 		if p.Tags == "" {
 			continue
 		}
@@ -895,8 +913,13 @@ func (s *Server) handleClientGroupPeers(w http.ResponseWriter, r *http.Request)
 		}
 
 		// Build info map matching RustDesk PeerPayload.info format
+		// Issue #138 (2.4): fallback device_name to peer ID if hostname empty
+		deviceName := p.Hostname
+		if deviceName == "" {
+			deviceName = p.ID
+		}
 		info := map[string]any{
-			"device_name": p.Hostname,
+			"device_name": deviceName,
 			"os":          p.OS,
 			"username":    p.User,
 			"version":     p.Version,

betterdesk-server/api/server.go

@@ -223,7 +223,6 @@ func (s *Server) Start(ctx context.Context) error {
 	mux.HandleFunc("PATCH /api/peers/{id}", s.requirePermission(auth.PermDeviceEdit, s.handleUpdatePeerFields))
 	mux.HandleFunc("POST /api/peers/{id}/ban", s.requirePermission(auth.PermDeviceBan, s.handleBanPeer))
 	mux.HandleFunc("POST /api/peers/{id}/unban", s.requirePermission(auth.PermDeviceBan, s.handleUnbanPeer))
-	mux.HandleFunc("POST /api/peers/{id}/restore", s.requirePermission(auth.PermDeviceDelete, s.handleRestorePeer))
 	mux.HandleFunc("POST /api/peers/{id}/change-id", s.requirePermission(auth.PermDeviceChangeID, s.handleChangePeerID))
 
 	// Detailed device status (enhanced in Phase 4)
@@ -397,7 +396,6 @@ func (s *Server) Start(ctx context.Context) error {
 	mux.HandleFunc("GET /api/auth/oidc/status", s.handleOIDCLoginStatus)
 	mux.HandleFunc("GET /api/auth/oidc/authorize", s.handleOIDCAuthorize)
 	mux.HandleFunc("GET /api/auth/oidc/callback", s.handleOIDCCallback)
-	mux.HandleFunc("POST /api/auth/oidc/exchange", s.handleOIDCExchange)
 
 	// Branding (GET is public for desktop clients, POST is admin)
 	mux.HandleFunc("GET /api/branding", s.rateLimitPublic(s.brandingLimiter, s.handleGetBranding))
@@ -715,8 +713,13 @@ func (s *Server) handleClientPeersList(w http.ResponseWriter, r *http.Request) {
 		}
 
 		// Build info map matching RustDesk PeerPayload.info format
+		// Issue #138 (2.4): fallback device_name to peer ID if hostname empty
+		deviceName := p.Hostname
+		if deviceName == "" {
+			deviceName = p.ID
+		}
 		info := map[string]any{
-			"device_name": p.Hostname,
+			"device_name": deviceName,
 			"os":          p.OS,
 			"username":    p.User,
 			"version":     p.Version,
@@ -745,6 +748,7 @@ func (s *Server) handleClientPeersList(w http.ResponseWriter, r *http.Request) {
 			"note":              p.Note,
 			"device_group_name": "",
 			"tags":              tags,
+			"online":            s.peers.IsOnline(p.ID, config.RegTimeout),
 		}
 
 		// Set device_group_name from first tag (if any) for folder display
@@ -1058,69 +1062,6 @@ func (s *Server) handleUnbanPeer(w http.ResponseWriter, r *http.Request) {
 	writeJSON(w, http.StatusOK, map[string]string{"status": "unbanned", "id": id})
 }
 
-// handleRestorePeer clears the soft_deleted flag on a previously deleted peer.
-// SECURITY (GHSA-3v82-3gf8-fxx8): This is the ONLY supported path for bringing
-// a deleted device back. UpsertPeer no longer restores rows implicitly, so an
-// attacker who knows a deleted ID cannot resurrect it by replaying registration.
-// As a convenience, this also clears the banned flag and removes the ID from
-// the in-memory blocklist so the device is fully usable afterward.
-func (s *Server) handleRestorePeer(w http.ResponseWriter, r *http.Request) {
-	id := r.PathValue("id")
-	if !s.peerOrgScopeCheck(w, r, id) {
-		return
-	}
-
-	// Confirm the peer is actually soft-deleted; restoring a live row is a no-op
-	// but we want to surface a clear 404 if it doesn't exist at all.
-	deleted, err := s.db.IsPeerSoftDeleted(id)
-	if err != nil {
-		writeInternalError(w, err, "IsPeerSoftDeleted")
-		return
-	}
-	if !deleted {
-		// The DB hides soft-deleted rows from GetPeer, so use it as an
-		// existence probe. If neither soft-deleted nor visible, the ID is
-		// unknown.
-		live, gerr := s.db.GetPeer(id)
-		if gerr != nil {
-			writeInternalError(w, gerr, "GetPeer")
-			return
-		}
-		if live == nil {
-			writeJSON(w, http.StatusNotFound, map[string]string{"error": "peer not found"})
-			return
-		}
-		writeJSON(w, http.StatusOK, map[string]string{"status": "already_active", "id": id})
-		return
-	}
-
-	if err := s.db.RestorePeer(id); err != nil {
-		writeInternalError(w, err, "RestorePeer")
-		return
-	}
-
-	// Convenience: clear ban + blocklist so admins don't have to chase three
-	// separate buttons. Errors here are non-fatal — restore already succeeded.
-	if err := s.db.UnbanPeer(id); err != nil {
-		log.Printf("[api] handleRestorePeer: UnbanPeer failed for %s: %v", id, err)
-	}
-	if s.blocklist != nil {
-		s.blocklist.UnblockID(id)
-	}
-
-	if s.eventBus != nil {
-		s.eventBus.Publish(eventsModule.Event{
-			Type: eventsModule.EventPeerRestored,
-			Data: map[string]string{"id": id},
-		})
-	}
-	if s.auditLog != nil {
-		s.auditLog.Log(audit.ActionPeerRestored, s.remoteIP(r), id, nil)
-	}
-
-	writeJSON(w, http.StatusOK, map[string]string{"status": "restored", "id": id})
-}
-
 func (s *Server) handleChangePeerID(w http.ResponseWriter, r *http.Request) {
 	oldID := r.PathValue("id")
 	if !s.peerOrgScopeCheck(w, r, oldID) {

web-nodejs/routes/rustdesk-api.routes.js

@@ -377,13 +377,13 @@ async function buildSyncedAddressBook(user, abType) {
     const abData = (abRecord && abRecord.data) ? String(abRecord.data) : '{}';
     const context = await getConsoleDeviceContext(user);
 
-    // Auto-include server devices for admin/operator users so their AB is pre-populated.
-    // Regular (pro) users only see devices they have manually added.
-    const canViewDevices = user && user.role && user.role !== 'pro';
-
+    // Issue #138 (2.1): Do NOT auto-include all server devices into the AB.
+    // Previously this was true for admin/operator users, causing "ghost" entries
+    // that reappear after deletion. The "Available Devices" tab shows all server
+    // devices via /api/peers/list — the AB should only contain user-added entries.
     return addressBookSync.mergeAddressBookData(abData, {
         ...context,
-        includeDevices: canViewDevices
+        includeDevices: false
     });
 }
 

web-nodejs/services/rustdeskAddressBookSync.js

@@ -105,14 +105,30 @@ function mergeAddressBookData(data, options = {}) {
     const devices = Array.isArray(options.devices) ? options.devices : [];
     const includeDevices = options.includeDevices !== false;
 
+    // Issue #138: build set of banned/deleted device IDs to strip from AB
+    const bannedIds = new Set();
+    for (const device of devices) {
+        const id = String(device && device.id || '').trim();
+        if (!id) continue;
+        if (device.banned || device.soft_deleted) {
+            bannedIds.add(id);
+        }
+    }
+
     ab.tags = normalizeTags(ab.tags);
     const globalSeen = new Set(ab.tags);
 
     const peerById = new Map();
-    for (const peer of ab.peers) {
-        if (!peer || typeof peer !== 'object') continue;
+    // Filter out banned/deleted peers from existing AB data
+    ab.peers = ab.peers.filter(peer => {
+        if (!peer || typeof peer !== 'object') return false;
+        const id = String(peer.id || '').trim();
+        if (!id) return false;
+        if (bannedIds.has(id)) return false; // strip banned
+        return true;
+    });
+    for (const peer of ab.peers) {
         const id = String(peer.id || '').trim();
-        if (!id) continue;
         peer.tags = normalizeTags(peer.tags);
         peerById.set(id, peer);
         for (const tag of peer.tags) {