From 2885dabcfe56bdafcc05dceee1bf73489ef98c56 Mon Sep 17 00:00:00 2001 From: kari Date: Thu, 29 Oct 2026 12:56:52 -0700 Subject: [PATCH] ipn, feature/guivisibility: support incremental gui visibility updates This moves gui visibility policy evaluation into an opt ipnext to avoid recalculating and broadcasting full visibility snapshots on every peer update. Previously, peer changes triggered a scan of the entire netmap and sent a complete visibility map to every subscribed watcher even when no visibility policy was configured. Instead, we evaluate HiddenNetworkDevices using a per-session visibility filter, send full visibility snapshots on watcher initialization, policy changes, and self node changes, and avoid visibility processing for watchers without the relevant poliocy. Updates tailscale/corp#44104 Signed-off-by: kari --- cmd/tailscaled/depaware.txt | 1 + .../maybe_guivisibility_windows.go | 8 + feature/guivisibility/doc.go | 6 + feature/guivisibility/guivisibility.go | 60 ++++++ feature/guivisibility/guivisibility_test.go | 125 ++++++++++++ ipn/backend.go | 7 + ipn/ipnext/gui_visibility.go | 20 ++ ipn/ipnlocal/gui_visibility.go | 122 ++++++------ ipn/ipnlocal/gui_visibility_test.go | 153 --------------- ipn/ipnlocal/local.go | 39 ++-- ipn/ipnlocal/local_test.go | 178 +++++++++++++++++- 11 files changed, 479 insertions(+), 240 deletions(-) create mode 100644 feature/condregister/maybe_guivisibility_windows.go create mode 100644 feature/guivisibility/doc.go create mode 100644 feature/guivisibility/guivisibility.go create mode 100644 feature/guivisibility/guivisibility_test.go create mode 100644 ipn/ipnext/gui_visibility.go delete mode 100644 ipn/ipnlocal/gui_visibility_test.go diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 47ecff265..f371b8753 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -319,6 +319,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/feature/exitnodehealth from tailscale.com/feature/condregister tailscale.com/feature/favorites from tailscale.com/feature/condregister tailscale.com/feature/favorites/pintype from tailscale.com/client/local+ + W tailscale.com/feature/guivisibility from tailscale.com/feature/condregister L tailscale.com/feature/linkspeed from tailscale.com/feature/condregister L tailscale.com/feature/linuxdnsfight from tailscale.com/feature/condregister tailscale.com/feature/netlog from tailscale.com/feature/condregister/netlog diff --git a/feature/condregister/maybe_guivisibility_windows.go b/feature/condregister/maybe_guivisibility_windows.go new file mode 100644 index 000000000..17ad6c4dd --- /dev/null +++ b/feature/condregister/maybe_guivisibility_windows.go @@ -0,0 +1,8 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +//go:build windows + +package condregister + +import _ "tailscale.com/feature/guivisibility" diff --git a/feature/guivisibility/doc.go b/feature/guivisibility/doc.go new file mode 100644 index 000000000..b96ef7e2c --- /dev/null +++ b/feature/guivisibility/doc.go @@ -0,0 +1,6 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +// Package guivisibility implements policy-based GUI visibility +// filtering for tailnet devices. +package guivisibility diff --git a/feature/guivisibility/guivisibility.go b/feature/guivisibility/guivisibility.go new file mode 100644 index 000000000..1f194657f --- /dev/null +++ b/feature/guivisibility/guivisibility.go @@ -0,0 +1,60 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +package guivisibility + +import ( + "tailscale.com/feature" + "tailscale.com/ipn/ipnext" + "tailscale.com/tailcfg" + "tailscale.com/types/logger" + "tailscale.com/util/syspolicy/pkey" + "tailscale.com/util/syspolicy/policyclient" +) + +func init() { + if feature.Register("guivisibility") { + ipnext.RegisterExtension("guivisibility", func(logger.Logf, ipnext.SafeBackend) (ipnext.Extension, error) { + return extension{}, nil + }) + } +} + +type extension struct{} + +func (extension) Name() string { return "guivisibility" } +func (extension) Init(ipnext.Host) error { return nil } +func (extension) Shutdown() error { return nil } + +func (extension) ForPolicy(snapshot *policyclient.PolicySnapshot) ipnext.GUIVisibilityFilter { + f := visibilityFilter{} + for _, value := range policyclient.GetStringArrayFromSnapshot(snapshot, pkey.HiddenNetworkDevices) { + switch value { + case "current-user": + f.currentUser = true + case "other-users": + f.otherUsers = true + case "tagged-devices": + f.taggedDevices = true + } + } + if !f.currentUser && !f.otherUsers && !f.taggedDevices { + return nil + } + return f +} + +type visibilityFilter struct{ currentUser, otherUsers, taggedDevices bool } + +func (f visibilityFilter) Hide(peer, self tailcfg.NodeView) bool { + if !peer.Valid() || !self.Valid() || peer.ID() == self.ID() { + return false + } + if peer.IsTagged() { + return f.taggedDevices + } + if peer.SharerOrUser() == self.User() { + return f.currentUser + } + return f.otherUsers +} diff --git a/feature/guivisibility/guivisibility_test.go b/feature/guivisibility/guivisibility_test.go new file mode 100644 index 000000000..847d42a0c --- /dev/null +++ b/feature/guivisibility/guivisibility_test.go @@ -0,0 +1,125 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +package guivisibility + +import ( + "testing" + + "tailscale.com/tailcfg" +) + +func TestVisibilityFilter(t *testing.T) { + self := (&tailcfg.Node{ + ID: 1, + User: 10, + }).View() + sameUser := (&tailcfg.Node{ + ID: 2, + User: 10, + }).View() + otherUser := (&tailcfg.Node{ + ID: 3, + User: 20, + }).View() + tagged := (&tailcfg.Node{ + ID: 4, + User: 20, + Tags: []string{"tag:server"}, + }).View() + sharedByCurrentUser := (&tailcfg.Node{ + ID: 5, + User: 20, + Sharer: 10, + }).View() + + tests := []struct { + name string + peer tailcfg.NodeView + filter visibilityFilter + want bool + }{ + { + name: "no-policy/same-user", + peer: sameUser, + }, + { + name: "no-policy/other-user", + peer: otherUser, + }, + { + name: "no-policy/tagged", + peer: tagged, + }, + { + name: "current-user/self-stays-visible", + peer: self, + filter: visibilityFilter{currentUser: true}, + }, + { + name: "current-user/same-user", + peer: sameUser, + filter: visibilityFilter{currentUser: true}, + want: true, + }, + { + name: "current-user/shared-by-current-user", + peer: sharedByCurrentUser, + filter: visibilityFilter{currentUser: true}, + want: true, + }, + { + name: "current-user/other-user", + peer: otherUser, + filter: visibilityFilter{currentUser: true}, + }, + { + name: "other-users/other-user", + peer: otherUser, + filter: visibilityFilter{otherUsers: true}, + want: true, + }, + { + name: "other-users/tagged-is-separate", + peer: tagged, + filter: visibilityFilter{otherUsers: true}, + }, + { + name: "tagged-devices/tagged", + peer: tagged, + filter: visibilityFilter{taggedDevices: true}, + want: true, + }, + { + name: "tagged-devices/other-user", + peer: otherUser, + filter: visibilityFilter{taggedDevices: true}, + }, + { + name: "all/same-user", + peer: sameUser, + filter: visibilityFilter{true, true, true}, + want: true, + }, + { + name: "all/other-user", + peer: otherUser, + filter: visibilityFilter{true, true, true}, + want: true, + }, + { + name: "all/tagged", + peer: tagged, + filter: visibilityFilter{true, true, true}, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.filter.Hide(tt.peer, self); got != tt.want { + t.Fatalf("Hide() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/ipn/backend.go b/ipn/backend.go index 58f0540c3..d3950c529 100644 --- a/ipn/backend.go +++ b/ipn/backend.go @@ -364,6 +364,13 @@ type Notify struct { // peer notifications, åand the underlying netmap. NodeGUIVisibility map[tailcfg.StableNodeID]NodeGUIVisibility `json:",omitzero"` + // NodeGUIVisibilityChanges contains visibility changes for individual peers. + // Entries update, rather than replace, NodeGUIVisibility on the client. + NodeGUIVisibilityChanges map[tailcfg.StableNodeID]NodeGUIVisibility `json:",omitzero"` + + // NodeGUIVisibilityRemoved lists peers whose visibility entries should be deleted. + NodeGUIVisibilityRemoved []tailcfg.StableNodeID `json:",omitzero"` + // PeerChangedPatch, if non-empty, lists narrow per-field peer patches // since the last Notify (currently Online, LastSeen, DERPHome, // Endpoints). It mirrors [tailcfg.MapResponse.PeersChangedPatch]. diff --git a/ipn/ipnext/gui_visibility.go b/ipn/ipnext/gui_visibility.go new file mode 100644 index 000000000..abe9ab380 --- /dev/null +++ b/ipn/ipnext/gui_visibility.go @@ -0,0 +1,20 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +package ipnext + +import ( + "tailscale.com/tailcfg" + "tailscale.com/util/syspolicy/policyclient" +) + +// GUIVisibilityProvider is an optional extension that interprets GUI +// visibility policy for IPN watchers. +type GUIVisibilityProvider interface { + ForPolicy(*policyclient.PolicySnapshot) GUIVisibilityFilter +} + +// GUIVisibilityFilter classifies peers for one watcher's effective policy. +type GUIVisibilityFilter interface { + Hide(peer, self tailcfg.NodeView) bool +} diff --git a/ipn/ipnlocal/gui_visibility.go b/ipn/ipnlocal/gui_visibility.go index 7d76612a6..8fa3b28d1 100644 --- a/ipn/ipnlocal/gui_visibility.go +++ b/ipn/ipnlocal/gui_visibility.go @@ -5,81 +5,81 @@ package ipnlocal import ( "tailscale.com/ipn" + "tailscale.com/ipn/ipnext" "tailscale.com/tailcfg" - "tailscale.com/util/syspolicy/pkey" + "tailscale.com/util/mak" "tailscale.com/util/syspolicy/policyclient" ) -const ( - hiddenNetworkDevicesCurrentUser = "current-user" - hiddenNetworkDevicesOtherUsers = "other-users" - hiddenNetworkDevicesTaggedDevices = "tagged-devices" -) - -// hiddenNetworkDevices is the parsed HiddenNetworkDevices policy for a watch -// session. Its zero value means no devices are hidden. -type hiddenNetworkDevices struct { - currentUser bool - otherUsers bool - taggedDevices bool -} - -func hiddenNetworkDevicesFromPolicy(snapshot *policyclient.PolicySnapshot) hiddenNetworkDevices { - values := policyclient.GetStringArrayFromSnapshot( - snapshot, - pkey.HiddenNetworkDevices, - ) - - var hidden hiddenNetworkDevices - for _, value := range values { - switch value { - case hiddenNetworkDevicesCurrentUser: - hidden.currentUser = true - case hiddenNetworkDevicesOtherUsers: - hidden.otherUsers = true - case hiddenNetworkDevicesTaggedDevices: - hidden.taggedDevices = true - } - } - return hidden -} - -// nodeGUIVisibilityLocked returns a complete snapshot of GUI visibility -// metadata for self and all current peers. -// +// guiVisibilityFilterLocked gets the optional GUI-specific extension. // b.mu must be held. -func (b *LocalBackend) nodeGUIVisibilityLocked(hidden hiddenNetworkDevices) map[tailcfg.StableNodeID]ipn.NodeGUIVisibility { - cn := b.currentNode() - nm := cn.NetMap() - if nm == nil || !nm.SelfNode.Valid() { +func (b *LocalBackend) guiVisibilityFilterLocked(snapshot *policyclient.PolicySnapshot) ipnext.GUIVisibilityFilter { + var provider ipnext.GUIVisibilityProvider + if !b.FindMatchingExtension(&provider) { return nil } + return provider.ForPolicy(snapshot) +} - visibility := make(map[tailcfg.StableNodeID]ipn.NodeGUIVisibility, len(nm.Peers)+1) - - visibility[nm.SelfNode.StableID()] = ipn.NodeGUIVisibility{} - +// nodeGUIVisibilityLocked returns a snapshot of hidden peers. An empty, +// non-nil map clears the previous snapshot when the policy is disabled. +// b.mu must be held. +func (b *LocalBackend) nodeGUIVisibilityLocked(filter ipnext.GUIVisibilityFilter) (map[tailcfg.StableNodeID]ipn.NodeGUIVisibility, map[tailcfg.NodeID]tailcfg.StableNodeID) { + visibility := make(map[tailcfg.StableNodeID]ipn.NodeGUIVisibility) + if filter == nil { + return visibility, nil + } + nm := b.currentNode().NetMap() + if nm == nil || !nm.SelfNode.Valid() { + return visibility, nil + } + var hidden map[tailcfg.NodeID]tailcfg.StableNodeID for _, peer := range nm.Peers { - visibility[peer.StableID()] = ipn.NodeGUIVisibility{ - HideInGUI: peerHiddenInGUI(peer, nm.SelfNode, hidden), + if filter.Hide(peer, nm.SelfNode) { + visibility[peer.StableID()] = ipn.NodeGUIVisibility{HideInGUI: true} + mak.Set(&hidden, peer.ID(), peer.StableID()) } } - - return visibility + return visibility, hidden } -func peerHiddenInGUI(peer, self tailcfg.NodeView, hidden hiddenNetworkDevices) bool { - if peer.ID() == self.ID() { - return false +// nodeGUIVisibilityDeltaLocked evaluates only peers in this notification. +// It returns nil deltas if effective visibility did not change. +// b.mu must be held. +func (b *LocalBackend) nodeGUIVisibilityDeltaLocked(sess *watchSession, n *ipn.Notify) (map[tailcfg.StableNodeID]ipn.NodeGUIVisibility, []tailcfg.StableNodeID) { + var changes map[tailcfg.StableNodeID]ipn.NodeGUIVisibility + var removed []tailcfg.StableNodeID + nm := b.currentNode().NetMap() + var self tailcfg.NodeView + if nm != nil { + self = nm.SelfNode } - - if peer.IsTagged() { - return hidden.taggedDevices + for _, id := range n.PeersRemoved { + if stable, ok := sess.hiddenPeers[id]; ok { + removed = append(removed, stable) + delete(sess.hiddenPeers, id) + } } - - if peer.SharerOrUser() == self.User() { - return hidden.currentUser + for _, peer := range n.PeersChanged { + if peer == nil { + continue + } + if prev, ok := sess.hiddenPeers[peer.ID]; ok && prev != peer.StableID { + removed = append(removed, prev) + delete(sess.hiddenPeers, peer.ID) + } + hidden := self.Valid() && sess.guiVisibility.Hide(peer.View(), self) + _, wasHidden := sess.hiddenPeers[peer.ID] + if hidden == wasHidden { + continue + } + if hidden { + mak.Set(&changes, peer.StableID, ipn.NodeGUIVisibility{HideInGUI: true}) + mak.Set(&sess.hiddenPeers, peer.ID, peer.StableID) + } else { + removed = append(removed, peer.StableID) + delete(sess.hiddenPeers, peer.ID) + } } - - return hidden.otherUsers + return changes, removed } diff --git a/ipn/ipnlocal/gui_visibility_test.go b/ipn/ipnlocal/gui_visibility_test.go deleted file mode 100644 index 376a1dadc..000000000 --- a/ipn/ipnlocal/gui_visibility_test.go +++ /dev/null @@ -1,153 +0,0 @@ -// Copyright (c) Tailscale Inc & contributors -// SPDX-License-Identifier: BSD-3-Clause - -package ipnlocal - -import ( - "testing" - - "tailscale.com/tailcfg" -) - -func TestPeerHiddenInGUI(t *testing.T) { - self := (&tailcfg.Node{ - ID: 1, - User: 10, - }).View() - sameUser := (&tailcfg.Node{ - ID: 2, - User: 10, - }).View() - otherUser := (&tailcfg.Node{ - ID: 3, - User: 20, - }).View() - tagged := (&tailcfg.Node{ - ID: 4, - User: 20, - Tags: []string{"tag:server"}, - }).View() - sharedByCurrentUser := (&tailcfg.Node{ - ID: 5, - User: 20, - Sharer: 10, - }).View() - - tests := []struct { - name string - peer tailcfg.NodeView - hidden hiddenNetworkDevices - want bool - }{ - { - name: "no-policy/same-user", - peer: sameUser, - }, - { - name: "no-policy/other-user", - peer: otherUser, - }, - { - name: "no-policy/tagged", - peer: tagged, - }, - { - name: "current-user/self-stays-visible", - peer: self, - hidden: hiddenNetworkDevices{ - currentUser: true, - }, - }, - { - name: "current-user/same-user", - peer: sameUser, - hidden: hiddenNetworkDevices{ - currentUser: true, - }, - want: true, - }, - { - name: "current-user/shared-by-current-user", - peer: sharedByCurrentUser, - hidden: hiddenNetworkDevices{ - currentUser: true, - }, - want: true, - }, - { - name: "current-user/other-user", - peer: otherUser, - hidden: hiddenNetworkDevices{ - currentUser: true, - }, - }, - { - name: "other-users/other-user", - peer: otherUser, - hidden: hiddenNetworkDevices{ - otherUsers: true, - }, - want: true, - }, - { - name: "other-users/tagged-is-separate", - peer: tagged, - hidden: hiddenNetworkDevices{ - otherUsers: true, - }, - }, - { - name: "tagged-devices/tagged", - peer: tagged, - hidden: hiddenNetworkDevices{ - taggedDevices: true, - }, - want: true, - }, - { - name: "tagged-devices/other-user", - peer: otherUser, - hidden: hiddenNetworkDevices{ - taggedDevices: true, - }, - }, - { - name: "all/same-user", - peer: sameUser, - hidden: hiddenNetworkDevices{ - currentUser: true, - otherUsers: true, - taggedDevices: true, - }, - want: true, - }, - { - name: "all/other-user", - peer: otherUser, - hidden: hiddenNetworkDevices{ - currentUser: true, - otherUsers: true, - taggedDevices: true, - }, - want: true, - }, - { - name: "all/tagged", - peer: tagged, - hidden: hiddenNetworkDevices{ - currentUser: true, - otherUsers: true, - taggedDevices: true, - }, - want: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := peerHiddenInGUI(tt.peer, self, tt.hidden); got != tt.want { - t.Fatalf("peerHiddenInGUI() = %v, want %v", got, tt.want) - } - }) - } -} diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 4e02bfc94..288459631 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -212,9 +212,11 @@ type watchSession struct { // Empty string means default/device scope. policyUID string - // hiddenNetworkDevices is the parsed GUI visibility policy from the - // most recent effective policy snapshot for this session. - hiddenNetworkDevices hiddenNetworkDevices + // GUI visibility is optional; nil if the extension isn't linked. + guiVisibility ipnext.GUIVisibilityFilter + // Track hidden peers by NodeID so removals can be delivered even after + // they disappear from NodeBackend. + hiddenPeers map[tailcfg.NodeID]tailcfg.StableNodeID } var ( @@ -2494,10 +2496,12 @@ func (b *LocalBackend) sysPolicyChangedForSession(sess *watchSession) { return } - sess.hiddenNetworkDevices = hiddenNetworkDevicesFromPolicy(snapshot) + sess.guiVisibility = b.guiVisibilityFilterLocked(snapshot) + visibility, hidden := b.nodeGUIVisibilityLocked(sess.guiVisibility) + sess.hiddenPeers = hidden n := ipn.Notify{ Policy: snapshot, - NodeGUIVisibility: b.nodeGUIVisibilityLocked(sess.hiddenNetworkDevices), + NodeGUIVisibility: visibility, Version: version.Long(), } for _, f := range b.extHost.Hooks().MutateNotifyLocked { @@ -3878,7 +3882,8 @@ func (b *LocalBackend) WatchNotificationsAs(ctx context.Context, actor ipnauth.A b.mu.Lock() var policyUID string - var hiddenNetworkDevices hiddenNetworkDevices + var guiVisibility ipnext.GUIVisibilityFilter + var hiddenPeers map[tailcfg.NodeID]tailcfg.StableNodeID const initialBits = ipn.NotifyInitialState | ipn.NotifyInitialPrefs | ipn.NotifyInitialStatus | ipn.NotifyInitialDriveShares | ipn.NotifyInitialSuggestedExitNode | @@ -3924,8 +3929,8 @@ func (b *LocalBackend) WatchNotificationsAs(ctx context.Context, actor ipnauth.A if err != nil { b.logf("syspolicy: GetPolicySnapshot(%q): %v", policyUID, err) } else if ini.Policy != nil { - hiddenNetworkDevices = hiddenNetworkDevicesFromPolicy(ini.Policy) - ini.NodeGUIVisibility = b.nodeGUIVisibilityLocked(hiddenNetworkDevices) + guiVisibility = b.guiVisibilityFilterLocked(ini.Policy) + ini.NodeGUIVisibility, hiddenPeers = b.nodeGUIVisibilityLocked(guiVisibility) } } } @@ -3941,7 +3946,8 @@ func (b *LocalBackend) WatchNotificationsAs(ctx context.Context, actor ipnauth.A cancel: cancel, mask: mask, policyUID: policyUID, - hiddenNetworkDevices: hiddenNetworkDevices, + guiVisibility: guiVisibility, + hiddenPeers: hiddenPeers, } mak.Set(&b.notifyWatchers, sessionID, session) if mask&ipn.NotifyPeerWireGuardState != 0 { @@ -4268,11 +4274,9 @@ func (b *LocalBackend) notifyForSessionLocked(sess *watchSession, n *ipn.Notify) stripPatches := len(n.PeerChangedPatch) > 0 && !wantsPeerPatches stripPeerState := len(n.PeerState) > 0 && sess.mask&ipn.NotifyPeerWireGuardState == 0 promotePatches := len(n.PeerChangedPatch) > 0 && wantsPeerChanges && !wantsPeerPatches - refreshNodeGUIVisibility := - sess.mask&ipn.NotifySysPolicyChanges != 0 && - (n.SelfChange != nil || - len(n.PeersChanged) != 0 || - len(n.PeersRemoved) != 0) + refreshNodeGUIVisibility := sess.guiVisibility != nil && n.SelfChange != nil + updateNodeGUIVisibility := sess.guiVisibility != nil && + (len(n.PeersChanged) != 0 || len(n.PeersRemoved) != 0) if sess.selfChangeResetsImplicitState(n.SelfChange.View()) { sess.lastSentUserProfile = nil @@ -4302,12 +4306,15 @@ func (b *LocalBackend) notifyForSessionLocked(sess *watchSession, n *ipn.Notify) } replaceUserProfiles := !stripUserProfiles && len(sessUserProfiles) != len(n.UserProfiles) - if !stripPeersChanged && !stripPeersRemoved && !stripPatches && !stripPeerState && !stripUserProfiles && !replaceUserProfiles && !promotePatches && !refreshNodeGUIVisibility { + if !stripPeersChanged && !stripPeersRemoved && !stripPatches && !stripPeerState && !stripUserProfiles && !replaceUserProfiles && !promotePatches && !refreshNodeGUIVisibility && !updateNodeGUIVisibility { return n } nCopy := *n if refreshNodeGUIVisibility { - nCopy.NodeGUIVisibility = b.nodeGUIVisibilityLocked(sess.hiddenNetworkDevices) + nCopy.NodeGUIVisibility, sess.hiddenPeers = b.nodeGUIVisibilityLocked(sess.guiVisibility) + } else if updateNodeGUIVisibility { + nCopy.NodeGUIVisibilityChanges, nCopy.NodeGUIVisibilityRemoved = + b.nodeGUIVisibilityDeltaLocked(sess, n) } if stripPeersChanged { nCopy.PeersChanged = nil diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 1cf25cd40..13834ea41 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -38,11 +38,13 @@ import ( "tailscale.com/drive/driveimpl" "tailscale.com/feature" _ "tailscale.com/feature/condregister/portmapper" + _ "tailscale.com/feature/guivisibility" "tailscale.com/health" "tailscale.com/hostinfo" "tailscale.com/ipn" "tailscale.com/ipn/conffile" "tailscale.com/ipn/ipnauth" + "tailscale.com/ipn/ipnext" "tailscale.com/ipn/ipnlocal/netmapcache" "tailscale.com/ipn/store/mem" "tailscale.com/net/netcheck" @@ -9174,7 +9176,7 @@ func TestHiddenNetworkDevicesNotifyPerUser(t *testing.T) { userStoreA.SetStringLists( source.TestSettingOf( pkey.HiddenNetworkDevices, - []string{hiddenNetworkDevicesOtherUsers}, + []string{"other-users"}, ), ) rsop.RegisterStoreForTest(t, "UserStoreA", setting.UserScopeOf(uidA), userStoreA) @@ -9185,6 +9187,12 @@ func TestHiddenNetworkDevicesNotifyPerUser(t *testing.T) { sys := tsd.NewSystem() sys.PolicyClient.Set(testPolicyClient{}) lb := newTestLocalBackendWithSys(t, sys) + lb.extHost.Init() + + var provider ipnext.GUIVisibilityProvider + if !lb.FindMatchingExtension(&provider) { + t.Fatal("GUI visibility extension not registered or initialized") + } lb.currentNode().SetNetMap(&netmap.NetworkMap{ SelfNode: (&tailcfg.Node{ @@ -9212,12 +9220,7 @@ func TestHiddenNetworkDevicesNotifyPerUser(t *testing.T) { return false } - got, ok := n.NodeGUIVisibility["peer-10"] - if !ok { - t.Errorf("NodeGUIVisibility missing peer-10") - return true - } - + got := n.NodeGUIVisibility["peer-10"] if got.HideInGUI != wantHidden { t.Errorf( "NodeGUIVisibility[peer-10].HideInGUI = %v; want %v", @@ -9260,6 +9263,7 @@ func TestHiddenNetworkDevicesPolicyChangeUpdatesVisibility(t *testing.T) { sys := tsd.NewSystem() sys.PolicyClient.Set(testPolicyClient{}) lb := newTestLocalBackendWithSys(t, sys) + lb.extHost.Init() lb.currentNode().SetNetMap(&netmap.NetworkMap{ SelfNode: (&tailcfg.Node{ @@ -9286,8 +9290,7 @@ func TestHiddenNetworkDevicesPolicyChangeUpdatesVisibility(t *testing.T) { if n.NodeGUIVisibility == nil { return false } - got, ok := n.NodeGUIVisibility["peer-10"] - return ok && !got.HideInGUI + return !n.NodeGUIVisibility["peer-10"].HideInGUI }, }, { @@ -9320,13 +9323,168 @@ func TestHiddenNetworkDevicesPolicyChangeUpdatesVisibility(t *testing.T) { userStore.SetStringLists( source.TestSettingOf( pkey.HiddenNetworkDevices, - []string{hiddenNetworkDevicesOtherUsers}, + []string{"other-users"}, ), ) nw.check() } +func TestHiddenNetworkDevicesIncrementalPeerUpdates(t *testing.T) { + setting.SetDefinitionsForTest(t, + setting.NewDefinition( + pkey.HiddenNetworkDevices, + setting.UserSetting, + setting.StringListValue, + ), + ) + + uid := "S-1-5-21-1001" + userStore := source.NewTestStore(t) + userStore.SetStringLists( + source.TestSettingOf( + pkey.HiddenNetworkDevices, + []string{"other-users"}, + ), + ) + rsop.RegisterStoreForTest(t, "UserStore", setting.UserScopeOf(uid), userStore) + + sys := tsd.NewSystem() + sys.PolicyClient.Set(testPolicyClient{}) + lb := newTestLocalBackendWithSys(t, sys) + lb.extHost.Init() + + lb.currentNode().SetNetMap(&netmap.NetworkMap{ + SelfNode: (&tailcfg.Node{ + ID: 1, + StableID: "self", + User: 1, + }).View(), + Peers: []tailcfg.NodeView{ + (&tailcfg.Node{ + ID: 10, + StableID: "peer-10", + User: 2, + }).View(), + }, + }) + + actor := &ipnauth.TestActor{UID: ipn.WindowsUserID(uid)} + nw := newNotificationWatcher(t, lb, actor) + + nw.watch(ipn.NotifySysPolicyChanges, []wantedNotification{ + { + name: "initial visibility snapshot", + cond: func(t testing.TB, _ ipnauth.Actor, n *ipn.Notify) bool { + if n.NodeGUIVisibility == nil { + return false + } + if len(n.NodeGUIVisibility) != 1 || + !n.NodeGUIVisibility["peer-10"].HideInGUI { + t.Errorf("unexpected initial visibility: %v", n.NodeGUIVisibility) + } + return true + }, + }, + { + name: "hidden peer added", + cond: func(t testing.TB, _ ipnauth.Actor, n *ipn.Notify) bool { + if len(n.NodeGUIVisibilityChanges) == 0 { + return false + } + if n.NodeGUIVisibility != nil { + t.Error("peer update sent a full visibility snapshot") + } + if len(n.NodeGUIVisibilityChanges) != 1 || + !n.NodeGUIVisibilityChanges["peer-20"].HideInGUI { + t.Errorf("unexpected visibility changes: %v", + n.NodeGUIVisibilityChanges) + } + if len(n.NodeGUIVisibilityRemoved) != 0 { + t.Errorf("unexpected visibility removals: %v", + n.NodeGUIVisibilityRemoved) + } + if len(n.PeersChanged) != 0 { + t.Error("policy-only watcher received PeersChanged") + } + return true + }, + }, + { + name: "hidden peer becomes visible", + cond: func(t testing.TB, _ ipnauth.Actor, n *ipn.Notify) bool { + if len(n.NodeGUIVisibilityRemoved) == 0 { + return false + } + if n.NodeGUIVisibility != nil { + t.Error("peer update sent a full visibility snapshot") + } + if len(n.NodeGUIVisibilityChanges) != 0 { + t.Errorf("unexpected visibility changes: %v", + n.NodeGUIVisibilityChanges) + } + if len(n.NodeGUIVisibilityRemoved) != 1 || + n.NodeGUIVisibilityRemoved[0] != "peer-20" { + t.Errorf("unexpected visibility removals: %v", + n.NodeGUIVisibilityRemoved) + } + if len(n.PeersChanged) != 0 { + t.Error("policy-only watcher received PeersChanged") + } + return true + }, + }, + { + name: "hidden peer removed", + cond: func(t testing.TB, _ ipnauth.Actor, n *ipn.Notify) bool { + if len(n.NodeGUIVisibilityRemoved) == 0 { + return false + } + if len(n.NodeGUIVisibilityRemoved) != 1 || + n.NodeGUIVisibilityRemoved[0] != "peer-10" { + t.Errorf("unexpected visibility removals: %v", + n.NodeGUIVisibilityRemoved) + } + if n.NodeGUIVisibility != nil { + t.Error("peer removal sent a full visibility snapshot") + } + if len(n.NodeGUIVisibilityChanges) != 0 { + t.Errorf("unexpected visibility changes: %v", + n.NodeGUIVisibilityChanges) + } + if len(n.PeersRemoved) != 0 { + t.Error("policy-only watcher received PeersRemoved") + } + return true + }, + }, + }, func(t testing.TB, _ ipnauth.Actor, n *ipn.Notify) bool { + if n.NodeGUIVisibility != nil { + t.Errorf("unexpected full visibility snapshot: %v", + n.NodeGUIVisibility) + } + return false + }) + + lb.SendNotify(ipn.Notify{ + PeersChanged: []*tailcfg.Node{ + {ID: 20, StableID: "peer-20", User: 2}, + }, + }) + + lb.SendNotify(ipn.Notify{ + PeersChanged: []*tailcfg.Node{ + {ID: 20, StableID: "peer-20", User: 1}, + }, + }) + + lb.SendNotify(ipn.Notify{ + PeersRemoved: []tailcfg.NodeID{10}, + }) + + nw.check() +} + func TestPolicySnapshotMergesDeviceAndUserScopes(t *testing.T) { setting.SetDefinitionsForTest(t, setting.NewDefinition(pkey.AdminConsoleVisibility, setting.UserSetting, setting.VisibilityValue),