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),