From 8092eaed805ec2baac601e5a93ed4fe2fbad4a80 Mon Sep 17 00:00:00 2001 From: Marwan Sulaiman Date: Fri, 1 Dec 2023 02:39:14 -0500 Subject: [PATCH] ipn/ipnlocal,others: add tailnet display name to user profile This PR adds the new editable display name for a tailnet which will be the preferred display in client UIs and CLIs when fast user switching between profiles. Updates #9286 Signed-off-by: Marwan Sulaiman --- cmd/tailscale/cli/switch.go | 6 +++++- control/controlclient/map.go | 5 +++++ ipn/ipnlocal/local.go | 21 +++++++++++++++------ ipn/ipnlocal/network-lock.go | 1 + ipn/ipnlocal/profiles.go | 4 ++-- ipn/prefs.go | 14 ++++++-------- tailcfg/tailcfg.go | 3 +++ types/netmap/netmap.go | 11 +++++++++++ types/netmap/nodemut_test.go | 2 +- 9 files changed, 49 insertions(+), 18 deletions(-) diff --git a/cmd/tailscale/cli/switch.go b/cmd/tailscale/cli/switch.go index 857b88d8b..b504536d9 100644 --- a/cmd/tailscale/cli/switch.go +++ b/cmd/tailscale/cli/switch.go @@ -59,9 +59,13 @@ func listProfiles(ctx context.Context) error { if prof.ID == curP.ID { name += "*" } + tailnet := prof.NetworkProfile.DomainName + if prof.NetworkProfile.DisplayName != "" { + tailnet = prof.NetworkProfile.DisplayName + } printRow( string(prof.ID), - prof.NetworkProfile.DomainName, + tailnet, name, ) } diff --git a/control/controlclient/map.go b/control/controlclient/map.go index 90bf83213..67cf8cd90 100644 --- a/control/controlclient/map.go +++ b/control/controlclient/map.go @@ -83,6 +83,7 @@ type mapSession struct { lastSSHPolicy *tailcfg.SSHPolicy collectServices bool lastDomain string + lastDisplayName string lastDomainAuditLogID string lastHealth []string lastPopBrowserURL string @@ -312,6 +313,9 @@ func (ms *mapSession) updateStateFromResponse(resp *tailcfg.MapResponse) { if resp.Domain != "" { ms.lastDomain = resp.Domain } + if resp.DisplayName != "" { + ms.lastDisplayName = resp.DisplayName + } if resp.DomainDataPlaneAuditLogID != "" { ms.lastDomainAuditLogID = resp.DomainDataPlaneAuditLogID } @@ -756,6 +760,7 @@ func (ms *mapSession) netmap() *netmap.NetworkMap { Peers: peerViews, UserProfiles: make(map[tailcfg.UserID]tailcfg.UserProfile), Domain: ms.lastDomain, + DisplayName: ms.lastDisplayName, DomainAuditLogID: ms.lastDomainAuditLogID, DNS: *ms.lastDNSConfig, PacketFilter: ms.lastParsedPacketFilter, diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index eadf78f49..841ad22bf 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -1119,17 +1119,22 @@ func (b *LocalBackend) SetControlClientStatus(c controlclient.Client, st control // Until recently, we did not store the account's tailnet name. So check if this is the case, // and backfill it on incoming status update. - if b.pm.requiresBackfill() && st.NetMap != nil && st.NetMap.Domain != "" { - prefsChanged = true + var np ipn.NetworkProfile + if st.NetMap != nil { + np = ipn.NetworkProfile{ + MagicDNSName: st.NetMap.MagicDNSSuffix(), + DomainName: st.NetMap.DomainName(), + DisplayName: st.NetMap.GetDisplayName(), + } + if b.pm.requiresBackfill(np) { + prefsChanged = true + } } // Perform all mutations of prefs based on the netmap here. if prefsChanged { // Prefs will be written out if stale; this is not safe unless locked or cloned. - if err := b.pm.SetPrefs(prefs.View(), ipn.NetworkProfile{ - MagicDNSName: st.NetMap.MagicDNSSuffix(), - DomainName: st.NetMap.DomainName(), - }); err != nil { + if err := b.pm.SetPrefs(prefs.View(), np); err != nil { b.logf("Failed to save new controlclient state: %v", err) } } @@ -1188,6 +1193,7 @@ func (b *LocalBackend) SetControlClientStatus(c controlclient.Client, st control if err := b.pm.SetPrefs(p, ipn.NetworkProfile{ MagicDNSName: st.NetMap.MagicDNSSuffix(), DomainName: st.NetMap.DomainName(), + DisplayName: st.NetMap.GetDisplayName(), }); err != nil { b.logf("Failed to save new controlclient state: %v", err) } @@ -1618,6 +1624,7 @@ func (b *LocalBackend) Start(opts ipn.Options) error { if err := b.pm.SetPrefs(pv, ipn.NetworkProfile{ MagicDNSName: b.netMap.MagicDNSSuffix(), DomainName: b.netMap.DomainName(), + DisplayName: b.netMap.GetDisplayName(), }); err != nil { b.logf("failed to save UpdatePrefs state: %v", err) } @@ -2527,6 +2534,7 @@ func (b *LocalBackend) migrateStateLocked(prefs *ipn.Prefs) (err error) { if err := b.pm.SetPrefs(prefs.View(), ipn.NetworkProfile{ MagicDNSName: b.netMap.MagicDNSSuffix(), DomainName: b.netMap.DomainName(), + DisplayName: b.netMap.GetDisplayName(), }); err != nil { return fmt.Errorf("store.WriteState: %v", err) } @@ -3111,6 +3119,7 @@ func (b *LocalBackend) setPrefsLockedOnEntry(caller string, newp *ipn.Prefs) ipn if err := b.pm.SetPrefs(prefs, ipn.NetworkProfile{ MagicDNSName: b.netMap.MagicDNSSuffix(), DomainName: b.netMap.DomainName(), + DisplayName: b.netMap.GetDisplayName(), }); err != nil { b.logf("failed to save new controlclient state: %v", err) } diff --git a/ipn/ipnlocal/network-lock.go b/ipn/ipnlocal/network-lock.go index cfe33147b..381d9c3a1 100644 --- a/ipn/ipnlocal/network-lock.go +++ b/ipn/ipnlocal/network-lock.go @@ -581,6 +581,7 @@ func (b *LocalBackend) NetworkLockForceLocalDisable() error { if err := b.pm.SetPrefs(newPrefs.View(), ipn.NetworkProfile{ MagicDNSName: b.netMap.MagicDNSSuffix(), DomainName: b.netMap.DomainName(), + DisplayName: b.netMap.GetDisplayName(), }); err != nil { return fmt.Errorf("saving prefs: %w", err) } diff --git a/ipn/ipnlocal/profiles.go b/ipn/ipnlocal/profiles.go index 55bb6a1a1..442ed5277 100644 --- a/ipn/ipnlocal/profiles.go +++ b/ipn/ipnlocal/profiles.go @@ -608,10 +608,10 @@ func (pm *profileManager) migrateFromLegacyPrefs() error { return nil } -func (pm *profileManager) requiresBackfill() bool { +func (pm *profileManager) requiresBackfill(np ipn.NetworkProfile) bool { return pm != nil && pm.currentProfile != nil && - pm.currentProfile.NetworkProfile.RequiresBackfill() + pm.currentProfile.NetworkProfile.RequiresBackfill(np) } var ( diff --git a/ipn/prefs.go b/ipn/prefs.go index 5e8033a28..6417a6f62 100644 --- a/ipn/prefs.go +++ b/ipn/prefs.go @@ -795,16 +795,14 @@ type WindowsUserID string type NetworkProfile struct { MagicDNSName string DomainName string + DisplayName string } -// RequiresBackfill returns whether this object does not have all the data -// expected. This is because this struct is a later addition to LoginProfile and -// this method can be checked to see if it's been backfilled to the current -// expectation or not. Note that for now, it just checks if the struct is empty. -// In the future, if we have new optional fields, this method can be changed to -// do more explicit checks to return whether it's apt for a backfill or not. -func (n NetworkProfile) RequiresBackfill() bool { - return n == NetworkProfile{} +// RequiresBackfill returns whether the new NetworkProfile has changed +// from the already existing one and therefore requires updating the +// local storage. +func (n NetworkProfile) RequiresBackfill(np NetworkProfile) bool { + return n != np } // LoginProfile represents a single login profile as managed diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 5d4e5598d..5a56a168f 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -1782,6 +1782,9 @@ type MapResponse struct { // If empty, the value is unchanged. Domain string `json:",omitempty"` + // DisplayName is the user editable version of the Domain. + DisplayName string `json:",omitempty"` + // CollectServices reports whether this node's Tailnet has // requested that info about services be included in HostInfo. // If unset, the most recent non-empty MapResponse value in diff --git a/types/netmap/netmap.go b/types/netmap/netmap.go index 233dcc656..e92a63278 100644 --- a/types/netmap/netmap.go +++ b/types/netmap/netmap.go @@ -68,6 +68,8 @@ type NetworkMap struct { // Domain is the current Tailnet name. Domain string + // DisplayName is the user editable version of the Domain. + DisplayName string `json:",omitempty"` // DomainAuditLogID is an audit log ID provided by control and // only populated if the domain opts into data-plane audit logging. @@ -187,6 +189,15 @@ func (nm *NetworkMap) DomainName() string { return nm.Domain } +// DisplayName returns the name of the NetworkMap's current tailnet user +// editable display name. If the map is nil, it returns an empty string. +func (nm *NetworkMap) GetDisplayName() string { + if nm == nil { + return "" + } + return nm.DisplayName +} + // SelfCapabilities returns SelfNode.Capabilities if nm and nm.SelfNode are // non-nil. This is a method so we can use it in envknob/logknob without a // circular dependency. diff --git a/types/netmap/nodemut_test.go b/types/netmap/nodemut_test.go index f11a303af..353f9524a 100644 --- a/types/netmap/nodemut_test.go +++ b/types/netmap/nodemut_test.go @@ -45,7 +45,7 @@ func TestMapResponseContainsNonPatchFields(t *testing.T) { var want bool switch f.Name { - case "MapSessionHandle", "Seq", "KeepAlive", "PingRequest", "PopBrowserURL", "ControlTime": + case "MapSessionHandle", "Seq", "KeepAlive", "PingRequest", "PopBrowserURL", "ControlTime", "DisplayName": // There are meta fields that apply to all MapResponse values. // They should be ignored. want = false