From 2dc0645368d036c9233909ecfa53df73fa070d3d Mon Sep 17 00:00:00 2001 From: Marwan Sulaiman Date: Thu, 16 Nov 2023 21:40:23 -0500 Subject: [PATCH] ipn/ipnlocal,cmd/tailscale: persist tailnet name in user profile This PR starts to persist the NetMap tailnet name in SetPrefs so that tailscaled clients can use this value to disambiguate fast user switching from one tailnet to another that are under the same exact login. We will also try to backfill this information during backend starts and profile switches so that users don't have to re-authenticate their profile. The first client to use this new information is the CLI in 'tailscale switch -list' which now uses text/tabwriter to display the ID, Tailnet, and Account. Since account names are ambiguous, we allow the user to pass 'tailscale switch ID' to specify the exact tailnet they want to switch to. Updates #9286 Signed-off-by: Marwan Sulaiman --- cmd/tailscale/cli/switch.go | 46 +++++++++++++++++++++++++++---- ipn/ipnlocal/local.go | 33 ++++++++++++++++++---- ipn/ipnlocal/network-lock.go | 5 +++- ipn/ipnlocal/network-lock_test.go | 18 ++++++------ ipn/ipnlocal/peerapi_test.go | 2 +- ipn/ipnlocal/profiles.go | 21 ++++++++------ ipn/ipnlocal/profiles_test.go | 10 +++---- ipn/ipnlocal/state_test.go | 2 +- ipn/prefs.go | 28 +++++++++++++++---- types/netmap/netmap.go | 10 +++++++ 10 files changed, 131 insertions(+), 44 deletions(-) diff --git a/cmd/tailscale/cli/switch.go b/cmd/tailscale/cli/switch.go index ffbf552a9..857b88d8b 100644 --- a/cmd/tailscale/cli/switch.go +++ b/cmd/tailscale/cli/switch.go @@ -8,6 +8,8 @@ import ( "flag" "fmt" "os" + "strings" + "text/tabwriter" "time" "github.com/peterbourgon/ff/v3/ffcli" @@ -25,10 +27,14 @@ var switchCmd = &ffcli.Command{ Exec: switchProfile, UsageFunc: func(*ffcli.Command) string { return `USAGE - switch + switch switch --list -"tailscale switch" switches between logged in accounts. +"tailscale switch" switches between logged in accounts. You can +use the ID that's returned from 'tailnet switch -list' +to pick which profile you want to switch to. Alternatively, you +can use the Tailnet or the account names to switch as well. + This command is currently in alpha and may change in the future.` }, } @@ -42,12 +48,22 @@ func listProfiles(ctx context.Context) error { if err != nil { return err } + tw := tabwriter.NewWriter(os.Stdout, 2, 2, 2, ' ', 0) + defer tw.Flush() + printRow := func(vals ...string) { + fmt.Fprintln(tw, strings.Join(vals, "\t")) + } + printRow("ID", "Tailnet", "Account") for _, prof := range all { + name := prof.Name if prof.ID == curP.ID { - fmt.Printf("%s *\n", prof.Name) - } else { - fmt.Println(prof.Name) + name += "*" } + printRow( + string(prof.ID), + prof.NetworkProfile.DomainName, + name, + ) } return nil } @@ -66,12 +82,30 @@ func switchProfile(ctx context.Context, args []string) error { os.Exit(1) } var profID ipn.ProfileID + // Allow matching by ID, Tailnet, or Account + // in that order. for _, p := range all { - if p.Name == args[0] { + if p.ID == ipn.ProfileID(args[0]) { profID = p.ID break } } + if profID == "" { + for _, p := range all { + if p.NetworkProfile.DomainName == args[0] { + profID = p.ID + break + } + } + } + if profID == "" { + for _, p := range all { + if p.Name == args[0] { + profID = p.ID + break + } + } + } if profID == "" { errf("No profile named %q\n", args[0]) os.Exit(1) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index c1f516533..eca185038 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -341,7 +341,7 @@ func NewLocalBackend(logf logger.Logf, logID logid.PublicID, sys *tsd.System, lo return nil, err } p.ApplyEdits(&mp) - if err := pm.SetPrefs(p.View(), ""); err != nil { + if err := pm.SetPrefs(p.View(), ipn.NetworkProfile{}); err != nil { return nil, err } } @@ -1105,10 +1105,19 @@ func (b *LocalBackend) SetControlClientStatus(c controlclient.Client, st control prefsChanged = true } + // 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 + } + // 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(), st.NetMap.MagicDNSSuffix()); err != nil { + if err := b.pm.SetPrefs(prefs.View(), ipn.NetworkProfile{ + MagicDNSName: st.NetMap.MagicDNSSuffix(), + DomainName: st.NetMap.DomainName(), + }); err != nil { b.logf("Failed to save new controlclient state: %v", err) } } @@ -1164,7 +1173,10 @@ func (b *LocalBackend) SetControlClientStatus(c controlclient.Client, st control b.mu.Lock() prefs.WantRunning = false p := prefs.View() - if err := b.pm.SetPrefs(p, st.NetMap.MagicDNSSuffix()); err != nil { + if err := b.pm.SetPrefs(p, ipn.NetworkProfile{ + MagicDNSName: st.NetMap.MagicDNSSuffix(), + DomainName: st.NetMap.DomainName(), + }); err != nil { b.logf("Failed to save new controlclient state: %v", err) } b.mu.Unlock() @@ -1573,7 +1585,10 @@ func (b *LocalBackend) Start(opts ipn.Options) error { newPrefs := opts.UpdatePrefs.Clone() newPrefs.Persist = oldPrefs.Persist().AsStruct() pv := newPrefs.View() - if err := b.pm.SetPrefs(pv, b.netMap.MagicDNSSuffix()); err != nil { + if err := b.pm.SetPrefs(pv, ipn.NetworkProfile{ + MagicDNSName: b.netMap.MagicDNSSuffix(), + DomainName: b.netMap.DomainName(), + }); err != nil { b.logf("failed to save UpdatePrefs state: %v", err) } b.setAtomicValuesFromPrefsLocked(pv) @@ -2479,7 +2494,10 @@ func (b *LocalBackend) migrateStateLocked(prefs *ipn.Prefs) (err error) { // Backend owns the state, but frontend is trying to migrate // state into the backend. b.logf("importing frontend prefs into backend store; frontend prefs: %s", prefs.Pretty()) - if err := b.pm.SetPrefs(prefs.View(), b.netMap.MagicDNSSuffix()); err != nil { + if err := b.pm.SetPrefs(prefs.View(), ipn.NetworkProfile{ + MagicDNSName: b.netMap.MagicDNSSuffix(), + DomainName: b.netMap.DomainName(), + }); err != nil { return fmt.Errorf("store.WriteState: %v", err) } } @@ -3060,7 +3078,10 @@ func (b *LocalBackend) setPrefsLockedOnEntry(caller string, newp *ipn.Prefs) ipn } prefs := newp.View() - if err := b.pm.SetPrefs(prefs, b.netMap.MagicDNSSuffix()); err != nil { + if err := b.pm.SetPrefs(prefs, ipn.NetworkProfile{ + MagicDNSName: b.netMap.MagicDNSSuffix(), + DomainName: b.netMap.DomainName(), + }); err != nil { b.logf("failed to save new controlclient state: %v", err) } b.lastProfileID = b.pm.CurrentProfile().ID diff --git a/ipn/ipnlocal/network-lock.go b/ipn/ipnlocal/network-lock.go index 11cebcca3..cfe33147b 100644 --- a/ipn/ipnlocal/network-lock.go +++ b/ipn/ipnlocal/network-lock.go @@ -578,7 +578,10 @@ func (b *LocalBackend) NetworkLockForceLocalDisable() error { newPrefs := b.pm.CurrentPrefs().AsStruct().Clone() // .Persist should always be initialized here. newPrefs.Persist.DisallowedTKAStateIDs = append(newPrefs.Persist.DisallowedTKAStateIDs, stateID) - if err := b.pm.SetPrefs(newPrefs.View(), b.netMap.MagicDNSSuffix()); err != nil { + if err := b.pm.SetPrefs(newPrefs.View(), ipn.NetworkProfile{ + MagicDNSName: b.netMap.MagicDNSSuffix(), + DomainName: b.netMap.DomainName(), + }); err != nil { return fmt.Errorf("saving prefs: %w", err) } diff --git a/ipn/ipnlocal/network-lock_test.go b/ipn/ipnlocal/network-lock_test.go index db5a7f0f6..da317f343 100644 --- a/ipn/ipnlocal/network-lock_test.go +++ b/ipn/ipnlocal/network-lock_test.go @@ -151,7 +151,7 @@ func TestTKAEnablementFlow(t *testing.T) { PrivateNodeKey: nodePriv, NetworkLockKey: nlPriv, }, - }).View(), "")) + }).View(), ipn.NetworkProfile{})) b := LocalBackend{ capTailnetLock: true, varRoot: temp, @@ -191,7 +191,7 @@ func TestTKADisablementFlow(t *testing.T) { PrivateNodeKey: nodePriv, NetworkLockKey: nlPriv, }, - }).View(), "")) + }).View(), ipn.NetworkProfile{})) temp := t.TempDir() tkaPath := filepath.Join(temp, "tka-profile", string(pm.CurrentProfile().ID)) @@ -383,7 +383,7 @@ func TestTKASync(t *testing.T) { PrivateNodeKey: nodePriv, NetworkLockKey: nlPriv, }, - }).View(), "")) + }).View(), ipn.NetworkProfile{})) // Setup the tka authority on the control plane. key := tka.Key{Kind: tka.Key25519, Public: nlPriv.Public().Verifier(), Votes: 2} @@ -605,7 +605,7 @@ func TestTKADisable(t *testing.T) { PrivateNodeKey: nodePriv, NetworkLockKey: nlPriv, }, - }).View(), "")) + }).View(), ipn.NetworkProfile{})) temp := t.TempDir() tkaPath := filepath.Join(temp, "tka-profile", string(pm.CurrentProfile().ID)) @@ -696,7 +696,7 @@ func TestTKASign(t *testing.T) { PrivateNodeKey: nodePriv, NetworkLockKey: nlPriv, }, - }).View(), "")) + }).View(), ipn.NetworkProfile{})) // Make a fake TKA authority, to seed local state. disablementSecret := bytes.Repeat([]byte{0xa5}, 32) @@ -785,7 +785,7 @@ func TestTKAForceDisable(t *testing.T) { PrivateNodeKey: nodePriv, NetworkLockKey: nlPriv, }, - }).View(), "")) + }).View(), ipn.NetworkProfile{})) temp := t.TempDir() tkaPath := filepath.Join(temp, "tka-profile", string(pm.CurrentProfile().ID)) @@ -880,7 +880,7 @@ func TestTKAAffectedSigs(t *testing.T) { PrivateNodeKey: nodePriv, NetworkLockKey: nlPriv, }, - }).View(), "")) + }).View(), ipn.NetworkProfile{})) // Make a fake TKA authority, to seed local state. disablementSecret := bytes.Repeat([]byte{0xa5}, 32) @@ -1013,7 +1013,7 @@ func TestTKARecoverCompromisedKeyFlow(t *testing.T) { PrivateNodeKey: nodePriv, NetworkLockKey: nlPriv, }, - }).View(), "")) + }).View(), ipn.NetworkProfile{})) // Make a fake TKA authority, to seed local state. disablementSecret := bytes.Repeat([]byte{0xa5}, 32) @@ -1104,7 +1104,7 @@ func TestTKARecoverCompromisedKeyFlow(t *testing.T) { PrivateNodeKey: nodePriv, NetworkLockKey: cosignPriv, }, - }).View(), "")) + }).View(), ipn.NetworkProfile{})) b := LocalBackend{ varRoot: temp, logf: t.Logf, diff --git a/ipn/ipnlocal/peerapi_test.go b/ipn/ipnlocal/peerapi_test.go index 8f54affc9..74e1f4e25 100644 --- a/ipn/ipnlocal/peerapi_test.go +++ b/ipn/ipnlocal/peerapi_test.go @@ -657,7 +657,7 @@ func TestPeerAPIReplyToDNSQueries(t *testing.T) { netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"), }, - }).View(), "") + }).View(), ipn.NetworkProfile{}) if !h.ps.b.OfferingExitNode() { t.Fatal("unexpectedly not offering exit node") } diff --git a/ipn/ipnlocal/profiles.go b/ipn/ipnlocal/profiles.go index 74e5c52bd..55bb6a1a1 100644 --- a/ipn/ipnlocal/profiles.go +++ b/ipn/ipnlocal/profiles.go @@ -207,11 +207,10 @@ func init() { // It also saves the prefs to the StateStore. It stores a copy of the // provided prefs, which may be accessed via CurrentPrefs. // -// If tailnetMagicDNSName is provided non-empty, it will be used to -// enrich the profile with the tailnet's MagicDNS name. The MagicDNS -// name cannot be pulled from prefsIn directly because it is not saved -// on ipn.Prefs (since it's not a field that is configurable by nodes). -func (pm *profileManager) SetPrefs(prefsIn ipn.PrefsView, tailnetMagicDNSName string) error { +// NetworkProfile stores additional information about the tailnet the user +// is logged into so that we can keep track of things like their domain name +// across user switches to disambiguate the same account but a different tailnet. +func (pm *profileManager) SetPrefs(prefsIn ipn.PrefsView, np ipn.NetworkProfile) error { prefs := prefsIn.AsStruct() newPersist := prefs.Persist if newPersist == nil || newPersist.NodeID == "" || newPersist.UserProfile.LoginName == "" { @@ -255,9 +254,7 @@ func (pm *profileManager) SetPrefs(prefsIn ipn.PrefsView, tailnetMagicDNSName st cp.ControlURL = prefs.ControlURL cp.UserProfile = newPersist.UserProfile cp.NodeID = newPersist.NodeID - if tailnetMagicDNSName != "" { - cp.TailnetMagicDNSName = tailnetMagicDNSName - } + cp.NetworkProfile = np pm.knownProfiles[cp.ID] = cp pm.currentProfile = cp if err := pm.writeKnownProfiles(); err != nil { @@ -601,7 +598,7 @@ func (pm *profileManager) migrateFromLegacyPrefs() error { return fmt.Errorf("load legacy prefs: %w", err) } pm.dlogf("loaded legacy preferences; sentinel=%q", sentinel) - if err := pm.SetPrefs(prefs, ""); err != nil { + if err := pm.SetPrefs(prefs, ipn.NetworkProfile{}); err != nil { metricMigrationError.Add(1) return fmt.Errorf("migrating _daemon profile: %w", err) } @@ -611,6 +608,12 @@ func (pm *profileManager) migrateFromLegacyPrefs() error { return nil } +func (pm *profileManager) requiresBackfill() bool { + return pm != nil && + pm.currentProfile != nil && + pm.currentProfile.NetworkProfile.RequiresBackfill() +} + var ( metricNewProfile = clientmetric.NewCounter("profiles_new") metricSwitchProfile = clientmetric.NewCounter("profiles_switch") diff --git a/ipn/ipnlocal/profiles_test.go b/ipn/ipnlocal/profiles_test.go index fa5c89b63..47f75baba 100644 --- a/ipn/ipnlocal/profiles_test.go +++ b/ipn/ipnlocal/profiles_test.go @@ -41,7 +41,7 @@ func TestProfileCurrentUserSwitch(t *testing.T) { LoginName: loginName, }, } - if err := pm.SetPrefs(p.View(), ""); err != nil { + if err := pm.SetPrefs(p.View(), ipn.NetworkProfile{}); err != nil { t.Fatal(err) } return p.View() @@ -96,7 +96,7 @@ func TestProfileList(t *testing.T) { LoginName: loginName, }, } - if err := pm.SetPrefs(p.View(), ""); err != nil { + if err := pm.SetPrefs(p.View(), ipn.NetworkProfile{}); err != nil { t.Fatal(err) } return p.View() @@ -157,7 +157,7 @@ func TestProfileDupe(t *testing.T) { reauth := func(pm *profileManager, p *persist.Persist) { prefs := ipn.NewPrefs() prefs.Persist = p - must.Do(pm.SetPrefs(prefs.View(), "")) + must.Do(pm.SetPrefs(prefs.View(), ipn.NetworkProfile{})) } login := func(pm *profileManager, p *persist.Persist) { pm.NewProfile() @@ -379,7 +379,7 @@ func TestProfileManagement(t *testing.T) { }, NodeID: nid, } - if err := pm.SetPrefs(p.View(), ""); err != nil { + if err := pm.SetPrefs(p.View(), ipn.NetworkProfile{}); err != nil { t.Fatal(err) } return p.View() @@ -506,7 +506,7 @@ func TestProfileManagementWindows(t *testing.T) { }, NodeID: tailcfg.StableNodeID(strconv.Itoa(int(id))), } - if err := pm.SetPrefs(p.View(), ""); err != nil { + if err := pm.SetPrefs(p.View(), ipn.NetworkProfile{}); err != nil { t.Fatal(err) } return p.View() diff --git a/ipn/ipnlocal/state_test.go b/ipn/ipnlocal/state_test.go index 0ac687bb5..6e7791acd 100644 --- a/ipn/ipnlocal/state_test.go +++ b/ipn/ipnlocal/state_test.go @@ -923,7 +923,7 @@ func TestEditPrefsHasNoKeys(t *testing.T) { LegacyFrontendPrivateMachineKey: key.NewMachine(), }, - }).View(), "") + }).View(), ipn.NetworkProfile{}) if p := b.pm.CurrentPrefs().Persist(); !p.Valid() || p.PrivateNodeKey().IsZero() { t.Fatalf("PrivateNodeKey not set") } diff --git a/ipn/prefs.go b/ipn/prefs.go index 6f4d5fb06..71aef0733 100644 --- a/ipn/prefs.go +++ b/ipn/prefs.go @@ -790,6 +790,23 @@ type ProfileID string // tests. type WindowsUserID string +// NetworkProfile is a subset of netmap.NetworkMap +// that should be saved with each user profile. +type NetworkProfile struct { + MagicDNSName string + DomainName 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{} +} + // LoginProfile represents a single login profile as managed // by the ProfileManager. type LoginProfile struct { @@ -804,13 +821,12 @@ type LoginProfile struct { // It is filled in from the UserProfile.LoginName field. Name string - // TailnetMagicDNSName is filled with the MagicDNS suffix for this - // profile's node (even if MagicDNS isn't necessarily in use). - // It will neither start nor end with a period. + // NetworkProfile is a subset of netmap.NetworkMap that we + // store to remember information about the tailnet that this + // profile was logged in with. // - // TailnetMagicDNSName is only filled from 2023-09-09 forward, - // and will only get backfilled when a profile is the current profile. - TailnetMagicDNSName string + // This field was added on 2023-11-17. + NetworkProfile NetworkProfile // Key is the StateKey under which the profile is stored. // It is assigned once at profile creation time and never changes. diff --git a/types/netmap/netmap.go b/types/netmap/netmap.go index 7511544a5..233dcc656 100644 --- a/types/netmap/netmap.go +++ b/types/netmap/netmap.go @@ -177,6 +177,16 @@ func (nm *NetworkMap) MagicDNSSuffix() string { return MagicDNSSuffixOfNodeName(nm.Name) } +// DomainName returns the name of the NetworkMap's +// current tailnet. If the map is nil, it returns +// an empty string. +func (nm *NetworkMap) DomainName() string { + if nm == nil { + return "" + } + return nm.Domain +} + // 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.