From 4511e7d64e1b7483f8ac07298923cb27f2a863ce Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sat, 19 Aug 2023 15:06:23 -0700 Subject: [PATCH] ipn/ipnstate: add PeerStatus.AltSharerUserID, stop mangling Node.User In b987b2ab18ff48 (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, 7dec09d1693, 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 --- cmd/tailscale/cli/status.go | 15 ++++++++++++--- control/controlclient/map.go | 2 +- ipn/ipnlocal/local.go | 25 +++++++++++++------------ ipn/ipnstate/ipnstate.go | 7 +++++++ tailcfg/tailcfg.go | 15 ++++++++++++--- 5 files changed, 45 insertions(+), 19 deletions(-) diff --git a/cmd/tailscale/cli/status.go b/cmd/tailscale/cli/status.go index ba2215774..cb8755bc2 100644 --- a/cmd/tailscale/cli/status.go +++ b/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] diff --git a/control/controlclient/map.go b/control/controlclient/map.go index ef99a2609..8975f0577 100644 --- a/control/controlclient/map.go +++ b/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) } diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 7eb1f2188..6724e523c 100644 --- a/ipn/ipnlocal/local.go +++ b/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) diff --git a/ipn/ipnstate/ipnstate.go b/ipn/ipnstate/ipnstate.go index 77dfd376a..7e3a10b4a 100644 --- a/ipn/ipnstate/ipnstate.go +++ b/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 } diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 19db70739..c3dfe43f2 100644 --- a/tailcfg/tailcfg.go +++ b/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.