ipn/ipnlocal: don't dup-suppress UserProfiles on IPNBus on profile switches

Fixes #19889

Change-Id: I324a735c13772c0c79ed7392c0baa5064b34823b
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

ipn/ipnlocal/local.go

@@ -165,6 +165,12 @@ type watchSession struct {
 	// "control re-announces the same profile" case is a pointer-cheap
 	// equality check.
 	lastSentUserProfile map[tailcfg.UserID]tailcfg.UserProfileView
+
+	// lastSentSelf identifies the self node most recently delivered to
+	// this session via [Notify.SelfChange]. A change to self is a profile
+	// boundary for implicit bus state, so per-session dedup state such as
+	// lastSentUserProfile must be reset when this identity changes.
+	lastSentSelf tailcfg.NodeView
 }
 
 var (
@@ -3873,6 +3879,10 @@ func (b *LocalBackend) notifyForSessionLocked(sess *watchSession, n *ipn.Notify)
 	stripPatches := len(n.PeerChangedPatch) > 0 && !wantsPeerPatches
 	promotePatches := len(n.PeerChangedPatch) > 0 && wantsPeerChanges && !wantsPeerPatches
 
+	if sess.selfChangeResetsImplicitState(n.SelfChange.View()) {
+		sess.lastSentUserProfile = nil
+	}
+
 	// UserProfiles ride alongside peer changes and are gated on the
 	// same opt-in. Sessions that didn't ask for peer changes get the
 	// field stripped entirely; opted-in sessions get a per-session
@@ -3934,6 +3944,28 @@ func (b *LocalBackend) notifyForSessionLocked(sess *watchSession, n *ipn.Notify)
 	return &nCopy
 }
 
+// selfChangeResetsImplicitState reports whether self changes the identity of
+// the self node last delivered to sess. WatchIPNBus sessions survive profile
+// switches, but a self-node identity change is a boundary for implicit bus
+// state: publishers must resend any implicit state, such as UserProfiles,
+// needed to interpret subsequent peer deltas.
+func (sess *watchSession) selfChangeResetsImplicitState(self tailcfg.NodeView) bool {
+	if !self.Valid() {
+		return false
+	}
+	reset := !sess.lastSentSelf.Valid() || !sameSelfNode(sess.lastSentSelf, self)
+	sess.lastSentSelf = self
+	return reset
+}
+
+func sameSelfNode(a, b tailcfg.NodeView) bool {
+	// Include User in the identity even though a node can move users, for
+	// example when an untagged auth-key node is later tagged. Treating that as
+	// a boundary only causes redundant implicit-state delivery; missing the
+	// boundary would leave consumers unable to resolve newly referenced users.
+	return a.ID() == b.ID() && a.StableID() == b.StableID() && a.User() == b.User()
+}
+
 // hasPeerChangeWatcherLocked reports whether any active watcher wants peer-set
 // notifications. b.mu must be held.
 func (b *LocalBackend) hasPeerChangeWatcherLocked() bool {

ipn/ipnlocal/local_test.go

@@ -2319,6 +2319,54 @@ func TestNotifyForSessionUserProfilesGating(t *testing.T) {
 	})
 }
 
+func TestNotifyForSessionUserProfilesDedupResetsOnSelfChange(t *testing.T) {
+	b := newTestLocalBackend(t)
+
+	deliver := func(sess *watchSession, n *ipn.Notify) *ipn.Notify {
+		b.mu.Lock()
+		defer b.mu.Unlock()
+		return b.notifyForSessionLocked(sess, n)
+	}
+
+	sess := &watchSession{mask: ipn.NotifyPeerChanges}
+	self1 := &tailcfg.Node{ID: 1, StableID: "self1", User: 10}
+	self2 := &tailcfg.Node{ID: 2, StableID: "self2", User: 20}
+	profiles1 := map[tailcfg.UserID]tailcfg.UserProfileView{
+		10: (&tailcfg.UserProfile{ID: 10, LoginName: "alice@example.com", DisplayName: "Alice"}).View(),
+		11: (&tailcfg.UserProfile{ID: 11, LoginName: "peer@example.com", DisplayName: "Peer"}).View(),
+	}
+	profiles2 := map[tailcfg.UserID]tailcfg.UserProfileView{
+		20: (&tailcfg.UserProfile{ID: 20, LoginName: "bob@example.com", DisplayName: "Bob"}).View(),
+	}
+
+	n := deliver(sess, &ipn.Notify{SelfChange: self1, UserProfiles: profiles1})
+	if got, want := len(n.UserProfiles), len(profiles1); got != want {
+		t.Fatalf("initial UserProfiles len = %d; want %d", got, want)
+	}
+	n = deliver(sess, &ipn.Notify{SelfChange: self1, UserProfiles: profiles1})
+	if len(n.UserProfiles) != 0 {
+		t.Fatalf("same self duplicate UserProfiles = %v; want empty", n.UserProfiles)
+	}
+	n = deliver(sess, &ipn.Notify{SelfChange: self2, UserProfiles: profiles2})
+	if got, want := len(n.UserProfiles), len(profiles2); got != want {
+		t.Fatalf("new self UserProfiles len = %d; want %d", got, want)
+	}
+	n = deliver(sess, &ipn.Notify{SelfChange: self1, UserProfiles: profiles1})
+	if got, want := len(n.UserProfiles), len(profiles1); got != want {
+		t.Fatalf("returned self UserProfiles len = %d; want %d", got, want)
+	}
+
+	sess = &watchSession{mask: ipn.NotifyPeerChanges}
+	n = deliver(sess, &ipn.Notify{UserProfiles: profiles1})
+	if got, want := len(n.UserProfiles), len(profiles1); got != want {
+		t.Fatalf("pre-self UserProfiles len = %d; want %d", got, want)
+	}
+	n = deliver(sess, &ipn.Notify{SelfChange: self1, UserProfiles: profiles1})
+	if got, want := len(n.UserProfiles), len(profiles1); got != want {
+		t.Fatalf("first self UserProfiles len = %d; want %d", got, want)
+	}
+}
+
 // tests LocalBackend.updateNetmapDeltaLocked
 func TestUpdateNetmapDelta(t *testing.T) {
 	b := newTestLocalBackend(t)