ipn/ipnstate: add PeerStatus.AltSharerUserID, stop mangling Node.User

In b987b2ab18 (2021-01-12) when we introduced sharing we mapped
the sharer to the userid at a low layer, mostly to fix the display of
"tailscale status" and the client UIs, but also some tests.

The commit earlier today, 7dec09d169, removed the 2.5yo option
to let clients disable that automatic mapping, as clearly we were never
getting around to it.

This plumbs the Sharer UserID all the way to ipnstatus so the CLI
itself can choose to print out the Sharer's identity over the node's
original owner.

Then we stop mangling Node.User and let clients decide how they want
to render things.

To ease the migration for the Windows GUI (which currently operates on
tailcfg.Node via the NetMap from WatchIPNBus, instead of PeerStatus),
a new method Node.SharerOrUser is added to do the mapping of
Sharer-else-User.

Updates #1909
Updates tailscale/corp#1183

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

cmd/tailscale/cli/status.go

@@ -23,6 +23,7 @@ import (
 	"tailscale.com/ipn"
 	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/net/interfaces"
+	"tailscale.com/util/cmpx"
 	"tailscale.com/util/dnsname"
 )
 
@@ -308,12 +309,20 @@ func dnsOrQuoteHostname(st *ipnstate.Status, ps *ipnstate.PeerStatus) string {
 }
 
 func ownerLogin(st *ipnstate.Status, ps *ipnstate.PeerStatus) string {
-	if ps.UserID.IsZero() {
+	// We prioritize showing the name of the sharer as the owner of a node if
+	// it's different from the node's user. This is less surprising: if user B
+	// from a company shares user's C node from the same company with user A who
+	// don't know user C, user A might be surprised to see user C listed in
+	// their netmap. We've historically (2021-01..2023-08) always shown the
+	// sharer's name in the UI. Perhaps we want to show both here? But the CLI's
+	// a bit space constrained.
+	uid := cmpx.Or(ps.AltSharerUserID, ps.UserID)
+	if uid.IsZero() {
 		return "-"
 	}
-	u, ok := st.User[ps.UserID]
+	u, ok := st.User[uid]
 	if !ok {
-		return fmt.Sprint(ps.UserID)
+		return fmt.Sprint(uid)
 	}
 	if i := strings.Index(u.LoginName, "@"); i != -1 {
 		return u.LoginName[:i+1]

control/controlclient/map.go

@@ -203,7 +203,7 @@ func (ms *mapSession) netmapForResponse(resp *tailcfg.MapResponse) *netmap.Netwo
 	for _, peer := range resp.Peers {
 		peer.InitDisplayNames(magicDNSSuffix)
 		if !peer.Sharer.IsZero() {
-			peer.User = peer.Sharer
+			ms.addUserProfile(peer.Sharer)
 		}
 		ms.addUserProfile(peer.User)
 	}

ipn/ipnlocal/local.go

@@ -747,18 +747,19 @@ func (b *LocalBackend) populatePeerStatusLocked(sb *ipnstate.StatusBuilder) {
 		}
 		online := p.Online()
 		ps := &ipnstate.PeerStatus{
-			InNetworkMap: true,
-			UserID:       p.User(),
-			TailscaleIPs: tailscaleIPs,
-			HostName:     p.Hostinfo().Hostname(),
-			DNSName:      p.Name(),
-			OS:           p.Hostinfo().OS(),
-			LastSeen:     lastSeen,
-			Online:       online != nil && *online,
-			ShareeNode:   p.Hostinfo().ShareeNode(),
-			ExitNode:     p.StableID() != "" && p.StableID() == exitNodeID,
-			SSH_HostKeys: p.Hostinfo().SSH_HostKeys().AsSlice(),
-			Location:     p.Hostinfo().Location(),
+			InNetworkMap:    true,
+			UserID:          p.User(),
+			AltSharerUserID: p.Sharer(),
+			TailscaleIPs:    tailscaleIPs,
+			HostName:        p.Hostinfo().Hostname(),
+			DNSName:         p.Name(),
+			OS:              p.Hostinfo().OS(),
+			LastSeen:        lastSeen,
+			Online:          online != nil && *online,
+			ShareeNode:      p.Hostinfo().ShareeNode(),
+			ExitNode:        p.StableID() != "" && p.StableID() == exitNodeID,
+			SSH_HostKeys:    p.Hostinfo().SSH_HostKeys().AsSlice(),
+			Location:        p.Hostinfo().Location(),
 		}
 		peerStatusFromNode(ps, p)
 

ipn/ipnstate/ipnstate.go

@@ -199,6 +199,10 @@ type PeerStatus struct {
 	OS      string // HostInfo.OS
 	UserID  tailcfg.UserID
 
+	// AltSharerUserID is the user who shared this node
+	// if it's different than UserID. Otherwise it's zero.
+	AltSharerUserID tailcfg.UserID `json:",omitempty"`
+
 	// TailscaleIPs are the IP addresses assigned to the node.
 	TailscaleIPs []netip.Addr
 
@@ -387,6 +391,9 @@ func (sb *StatusBuilder) AddPeer(peer key.NodePublic, st *PeerStatus) {
 	if v := st.UserID; v != 0 {
 		e.UserID = v
 	}
+	if v := st.AltSharerUserID; v != 0 {
+		e.AltSharerUserID = v
+	}
 	if v := st.TailscaleIPs; v != nil {
 		e.TailscaleIPs = v
 	}

tailcfg/tailcfg.go

@@ -21,6 +21,7 @@ import (
 	"tailscale.com/types/opt"
 	"tailscale.com/types/structs"
 	"tailscale.com/types/tkatype"
+	"tailscale.com/util/cmpx"
 	"tailscale.com/util/dnsname"
 )
 
@@ -223,9 +224,9 @@ type Node struct {
 	// e.g. "host.tail-scale.ts.net."
 	Name string
 
-	// User is the user who created the node. If ACL tags are in
-	// use for the node then it doesn't reflect the ACL identity
-	// that the node is running as.
+	// User is the user who created the node. If ACL tags are in use for the
+	// node then it doesn't reflect the ACL identity that the node is running
+	// as.
 	User UserID
 
 	// Sharer, if non-zero, is the user who shared this node, if different than User.
@@ -386,12 +387,20 @@ func (n *Node) IsTagged() bool {
 	return len(n.Tags) > 0
 }
 
+// SharerOrUser Sharer if set, else User.
+func (n *Node) SharerOrUser() UserID {
+	return cmpx.Or(n.Sharer, n.User)
+}
+
 // IsTagged reports whether the node has any tags.
 func (n NodeView) IsTagged() bool { return n.ж.IsTagged() }
 
 // DisplayName wraps Node.DisplayName.
 func (n NodeView) DisplayName(forOwner bool) string { return n.ж.DisplayName(forOwner) }
 
+// SharerOrUser wraps Node.SharerOrUser.
+func (n NodeView) SharerOrUser() UserID { return n.ж.SharerOrUser() }
+
 // InitDisplayNames computes and populates n's display name
 // fields: n.ComputedName, n.computedHostIfDifferent, and
 // n.ComputedNameWithHost.