From 6aaf1d48dfc9c75818194cba99aa91c86c478b63 Mon Sep 17 00:00:00 2001 From: Maisem Ali Date: Thu, 3 Aug 2023 18:38:28 -0600 Subject: [PATCH] types/persist: drop duplicated Persist.LoginName It was duplicated from Persist.UserProfile.LoginName, drop it. Updates #7726 Signed-off-by: Maisem Ali --- cmd/tailscale/cli/cli_test.go | 25 +++++++++++++------------ cmd/tailscale/cli/up.go | 2 +- control/controlclient/direct.go | 5 +---- ipn/ipnlocal/local.go | 18 +++++++++--------- ipn/ipnlocal/profiles.go | 8 +------- ipn/ipnlocal/profiles_test.go | 4 ---- ipn/ipnlocal/state_test.go | 9 +++------ ipn/prefs.go | 15 ++++----------- ipn/prefs_test.go | 16 ++++++++++++---- types/persist/persist.go | 4 +--- types/persist/persist_clone.go | 1 - types/persist/persist_test.go | 13 +------------ types/persist/persist_view.go | 2 -- 13 files changed, 46 insertions(+), 76 deletions(-) diff --git a/cmd/tailscale/cli/cli_test.go b/cmd/tailscale/cli/cli_test.go index 5a29f177d..30165c295 100644 --- a/cmd/tailscale/cli/cli_test.go +++ b/cmd/tailscale/cli/cli_test.go @@ -18,6 +18,7 @@ "tailscale.com/health/healthmsg" "tailscale.com/ipn" "tailscale.com/ipn/ipnstate" + "tailscale.com/tailcfg" "tailscale.com/tka" "tailscale.com/tstest" "tailscale.com/types/persist" @@ -834,7 +835,7 @@ func TestUpdatePrefs(t *testing.T) { flags: []string{}, curPrefs: &ipn.Prefs{ ControlURL: ipn.DefaultControlURL, - Persist: &persist.Persist{LoginName: "crawshaw.github"}, + Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, }, env: upCheckEnv{ backendState: "Stopped", @@ -846,7 +847,7 @@ func TestUpdatePrefs(t *testing.T) { flags: []string{}, curPrefs: &ipn.Prefs{ ControlURL: ipn.DefaultControlURL, - Persist: &persist.Persist{LoginName: "crawshaw.github"}, + Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, }, env: upCheckEnv{backendState: "Running"}, wantSimpleUp: true, @@ -857,7 +858,7 @@ func TestUpdatePrefs(t *testing.T) { flags: []string{"--reset"}, curPrefs: &ipn.Prefs{ ControlURL: ipn.DefaultControlURL, - Persist: &persist.Persist{LoginName: "crawshaw.github"}, + Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, }, env: upCheckEnv{backendState: "Running"}, wantJustEditMP: &ipn.MaskedPrefs{ @@ -884,7 +885,7 @@ func TestUpdatePrefs(t *testing.T) { flags: []string{}, curPrefs: &ipn.Prefs{ ControlURL: "https://login.tailscale.com", - Persist: &persist.Persist{LoginName: "crawshaw.github"}, + Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, }, env: upCheckEnv{backendState: "Running"}, wantSimpleUp: true, @@ -895,7 +896,7 @@ func TestUpdatePrefs(t *testing.T) { flags: []string{"--login-server=https://localhost:1000"}, curPrefs: &ipn.Prefs{ ControlURL: "https://login.tailscale.com", - Persist: &persist.Persist{LoginName: "crawshaw.github"}, + Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, AllowSingleHosts: true, CorpDNS: true, NetfilterMode: preftype.NetfilterOn, @@ -910,7 +911,7 @@ func TestUpdatePrefs(t *testing.T) { flags: []string{"--advertise-tags=tag:foo"}, curPrefs: &ipn.Prefs{ ControlURL: "https://login.tailscale.com", - Persist: &persist.Persist{LoginName: "crawshaw.github"}, + Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, AllowSingleHosts: true, CorpDNS: true, NetfilterMode: preftype.NetfilterOn, @@ -944,7 +945,7 @@ func TestUpdatePrefs(t *testing.T) { flags: []string{"--ssh"}, curPrefs: &ipn.Prefs{ ControlURL: "https://login.tailscale.com", - Persist: &persist.Persist{LoginName: "crawshaw.github"}, + Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, AllowSingleHosts: true, CorpDNS: true, NetfilterMode: preftype.NetfilterOn, @@ -965,7 +966,7 @@ func TestUpdatePrefs(t *testing.T) { flags: []string{"--ssh=false"}, curPrefs: &ipn.Prefs{ ControlURL: "https://login.tailscale.com", - Persist: &persist.Persist{LoginName: "crawshaw.github"}, + Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, AllowSingleHosts: true, CorpDNS: true, RunSSH: true, @@ -990,7 +991,7 @@ func TestUpdatePrefs(t *testing.T) { sshOverTailscale: true, curPrefs: &ipn.Prefs{ ControlURL: "https://login.tailscale.com", - Persist: &persist.Persist{LoginName: "crawshaw.github"}, + Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, AllowSingleHosts: true, CorpDNS: true, NetfilterMode: preftype.NetfilterOn, @@ -1014,7 +1015,7 @@ func TestUpdatePrefs(t *testing.T) { sshOverTailscale: true, curPrefs: &ipn.Prefs{ ControlURL: "https://login.tailscale.com", - Persist: &persist.Persist{LoginName: "crawshaw.github"}, + Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, AllowSingleHosts: true, CorpDNS: true, NetfilterMode: preftype.NetfilterOn, @@ -1037,7 +1038,7 @@ func TestUpdatePrefs(t *testing.T) { sshOverTailscale: true, curPrefs: &ipn.Prefs{ ControlURL: "https://login.tailscale.com", - Persist: &persist.Persist{LoginName: "crawshaw.github"}, + Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, AllowSingleHosts: true, CorpDNS: true, NetfilterMode: preftype.NetfilterOn, @@ -1059,7 +1060,7 @@ func TestUpdatePrefs(t *testing.T) { sshOverTailscale: true, curPrefs: &ipn.Prefs{ ControlURL: "https://login.tailscale.com", - Persist: &persist.Persist{LoginName: "crawshaw.github"}, + Persist: &persist.Persist{UserProfile: tailcfg.UserProfile{LoginName: "crawshaw.github"}}, AllowSingleHosts: true, CorpDNS: true, RunSSH: true, diff --git a/cmd/tailscale/cli/up.go b/cmd/tailscale/cli/up.go index 581ea4406..d713050b1 100644 --- a/cmd/tailscale/cli/up.go +++ b/cmd/tailscale/cli/up.go @@ -424,7 +424,7 @@ func updatePrefs(prefs, curPrefs *ipn.Prefs, env upCheckEnv) (simpleUp bool, jus simpleUp = env.flagSet.NFlag() == 0 && curPrefs.Persist != nil && - curPrefs.Persist.LoginName != "" && + curPrefs.Persist.UserProfile.LoginName != "" && env.backendState != ipn.NeedsLogin.String() justEdit := env.backendState == ipn.Running.String() && diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index 571356e58..84d14d67b 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -560,7 +560,7 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new request.NodeKey.ShortString(), opt.URL != "", len(nodeKeySignature) > 0) request.Auth.Oauth2Token = opt.Token request.Auth.Provider = persist.Provider - request.Auth.LoginName = persist.LoginName + request.Auth.LoginName = persist.UserProfile.LoginName request.Auth.AuthKey = authKey err = signRegisterRequest(&request, c.serverURL, c.serverKey, machinePrivKey.Public()) if err != nil { @@ -646,9 +646,6 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new if resp.Login.Provider != "" { persist.Provider = resp.Login.Provider } - if resp.Login.LoginName != "" { - persist.LoginName = resp.Login.LoginName - } persist.UserProfile = tailcfg.UserProfile{ ID: resp.User.ID, DisplayName: resp.Login.DisplayName, diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 43619a62a..2d23761f2 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -957,7 +957,7 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) { b.mu.Lock() if st.LogoutFinished != nil { - if p := b.pm.CurrentPrefs(); !p.Persist().Valid() || p.Persist().LoginName() == "" { + if p := b.pm.CurrentPrefs(); !p.Persist().Valid() || p.Persist().UserProfile().LoginName() == "" { b.mu.Unlock() return } @@ -2784,16 +2784,16 @@ func (b *LocalBackend) setPrefsLockedOnEntry(caller string, newp *ipn.Prefs) ipn } } if netMap != nil { - up := netMap.UserProfiles[netMap.User] - if login := up.LoginName; login != "" { - if newp.Persist == nil { - b.logf("active login: %s", login) + newProfile := netMap.UserProfiles[netMap.User] + if newLoginName := newProfile.LoginName; newLoginName != "" { + if !oldp.Persist().Valid() { + b.logf("active login: %s", newLoginName) } else { - if newp.Persist.LoginName != login { - b.logf("active login: %q (changed from %q)", login, newp.Persist.LoginName) - newp.Persist.LoginName = login + oldLoginName := oldp.Persist().UserProfile().LoginName() + if oldLoginName != newLoginName { + b.logf("active login: %q (changed from %q)", newLoginName, oldLoginName) } - newp.Persist.UserProfile = up + newp.Persist.UserProfile = newProfile } } } diff --git a/ipn/ipnlocal/profiles.go b/ipn/ipnlocal/profiles.go index 26e0517cd..e1544f7a2 100644 --- a/ipn/ipnlocal/profiles.go +++ b/ipn/ipnlocal/profiles.go @@ -208,16 +208,10 @@ func init() { func (pm *profileManager) SetPrefs(prefsIn ipn.PrefsView) error { prefs := prefsIn.AsStruct().View() newPersist := prefs.Persist().AsStruct() - if newPersist == nil || newPersist.NodeID == "" { + if newPersist == nil || newPersist.NodeID == "" || newPersist.UserProfile.LoginName == "" { return pm.setPrefsLocked(prefs) } up := newPersist.UserProfile - if up.LoginName == "" { - // Backwards compatibility with old prefs files. - up.LoginName = newPersist.LoginName - } else { - newPersist.LoginName = up.LoginName - } if up.DisplayName == "" { up.DisplayName = up.LoginName } diff --git a/ipn/ipnlocal/profiles_test.go b/ipn/ipnlocal/profiles_test.go index 7090b8a0f..9f9f593bb 100644 --- a/ipn/ipnlocal/profiles_test.go +++ b/ipn/ipnlocal/profiles_test.go @@ -32,7 +32,6 @@ func TestProfileCurrentUserSwitch(t *testing.T) { p := pm.CurrentPrefs().AsStruct() p.Persist = &persist.Persist{ NodeID: tailcfg.StableNodeID(fmt.Sprint(id)), - LoginName: loginName, PrivateNodeKey: key.NewNode(), UserProfile: tailcfg.UserProfile{ ID: tailcfg.UserID(id), @@ -88,7 +87,6 @@ func TestProfileList(t *testing.T) { p := pm.CurrentPrefs().AsStruct() p.Persist = &persist.Persist{ NodeID: tailcfg.StableNodeID(fmt.Sprint(id)), - LoginName: loginName, PrivateNodeKey: key.NewNode(), UserProfile: tailcfg.UserProfile{ ID: tailcfg.UserID(id), @@ -211,7 +209,6 @@ func TestProfileManagement(t *testing.T) { nodeIDs[loginName] = nid } p.Persist = &persist.Persist{ - LoginName: loginName, PrivateNodeKey: key.NewNode(), UserProfile: tailcfg.UserProfile{ ID: uid, @@ -340,7 +337,6 @@ func TestProfileManagementWindows(t *testing.T) { p := pm.CurrentPrefs().AsStruct() p.ForceDaemon = forceDaemon p.Persist = &persist.Persist{ - LoginName: loginName, UserProfile: tailcfg.UserProfile{ ID: id, LoginName: loginName, diff --git a/ipn/ipnlocal/state_test.go b/ipn/ipnlocal/state_test.go index 22e4ada82..c924abd5a 100644 --- a/ipn/ipnlocal/state_test.go +++ b/ipn/ipnlocal/state_test.go @@ -476,7 +476,6 @@ func TestStateMachine(t *testing.T) { // The backend should propagate this upward for the UI. t.Logf("\n\nLoginFinished") notifies.expect(3) - cc.persist.LoginName = "user1" cc.persist.UserProfile.LoginName = "user1" cc.persist.NodeID = "node1" cc.send(nil, "", true, &netmap.NetworkMap{}) @@ -494,7 +493,7 @@ func TestStateMachine(t *testing.T) { c.Assert(nn[0].LoginFinished, qt.IsNotNil) c.Assert(nn[1].Prefs, qt.IsNotNil) c.Assert(nn[2].State, qt.IsNotNil) - c.Assert(nn[1].Prefs.Persist().LoginName(), qt.Equals, "user1") + c.Assert(nn[1].Prefs.Persist().UserProfile().LoginName(), qt.Equals, "user1") c.Assert(ipn.NeedsMachineAuth, qt.Equals, *nn[2].State) c.Assert(ipn.NeedsMachineAuth, qt.Equals, b.State()) } @@ -703,7 +702,6 @@ func TestStateMachine(t *testing.T) { b.Login(nil) t.Logf("\n\nLoginFinished3") notifies.expect(3) - cc.persist.LoginName = "user2" cc.persist.UserProfile.LoginName = "user2" cc.persist.NodeID = "node2" cc.send(nil, "", true, &netmap.NetworkMap{ @@ -717,7 +715,7 @@ func TestStateMachine(t *testing.T) { c.Assert(nn[1].Prefs.Persist(), qt.IsNotNil) c.Assert(nn[2].State, qt.IsNotNil) // Prefs after finishing the login, so LoginName updated. - c.Assert(nn[1].Prefs.Persist().LoginName(), qt.Equals, "user2") + c.Assert(nn[1].Prefs.Persist().UserProfile().LoginName(), qt.Equals, "user2") c.Assert(nn[1].Prefs.LoggedOut(), qt.IsFalse) c.Assert(nn[1].Prefs.WantRunning(), qt.IsTrue) c.Assert(ipn.Starting, qt.Equals, *nn[2].State) @@ -840,7 +838,6 @@ func TestStateMachine(t *testing.T) { // interactive login, so we end up unpaused. t.Logf("\n\nLoginDifferent URL visited") notifies.expect(3) - cc.persist.LoginName = "user3" cc.persist.UserProfile.LoginName = "user3" cc.persist.NodeID = "node3" cc.send(nil, "", true, &netmap.NetworkMap{ @@ -859,7 +856,7 @@ func TestStateMachine(t *testing.T) { c.Assert(nn[1].Prefs, qt.IsNotNil) c.Assert(nn[2].State, qt.IsNotNil) // Prefs after finishing the login, so LoginName updated. - c.Assert(nn[1].Prefs.Persist().LoginName(), qt.Equals, "user3") + c.Assert(nn[1].Prefs.Persist().UserProfile().LoginName(), qt.Equals, "user3") c.Assert(nn[1].Prefs.LoggedOut(), qt.IsFalse) c.Assert(nn[1].Prefs.WantRunning(), qt.IsTrue) c.Assert(ipn.Starting, qt.Equals, *nn[2].State) diff --git a/ipn/prefs.go b/ipn/prefs.go index e79da0d09..e81c44ad2 100644 --- a/ipn/prefs.go +++ b/ipn/prefs.go @@ -651,18 +651,11 @@ func PrefsFromBytes(b []byte) (*Prefs, error) { if len(b) == 0 { return p, nil } - persist := &persist.Persist{} - err := json.Unmarshal(b, persist) - if err == nil && (persist.Provider != "" || persist.LoginName != "") { - // old-style relaynode config; import it - p.Persist = persist - } else { - err = json.Unmarshal(b, &p) - if err != nil { - log.Printf("Prefs parse: %v: %v\n", err, b) - } + + if err := json.Unmarshal(b, p); err != nil { + return nil, err } - return p, err + return p, nil } var jsonEscapedZero = []byte(`\u0000`) diff --git a/ipn/prefs_test.go b/ipn/prefs_test.go index 3a1e30619..150d74098 100644 --- a/ipn/prefs_test.go +++ b/ipn/prefs_test.go @@ -264,12 +264,18 @@ func TestPrefsEqual(t *testing.T) { { &Prefs{Persist: &persist.Persist{}}, - &Prefs{Persist: &persist.Persist{LoginName: "dave"}}, + &Prefs{Persist: &persist.Persist{ + UserProfile: tailcfg.UserProfile{LoginName: "dave"}, + }}, false, }, { - &Prefs{Persist: &persist.Persist{LoginName: "dave"}}, - &Prefs{Persist: &persist.Persist{LoginName: "dave"}}, + &Prefs{Persist: &persist.Persist{ + UserProfile: tailcfg.UserProfile{LoginName: "dave"}, + }}, + &Prefs{Persist: &persist.Persist{ + UserProfile: tailcfg.UserProfile{LoginName: "dave"}, + }}, true, }, { @@ -345,7 +351,9 @@ func TestPrefsPersist(t *testing.T) { tstest.PanicOnLog() c := persist.Persist{ - LoginName: "test@example.com", + UserProfile: tailcfg.UserProfile{ + LoginName: "test@example.com", + }, } p := Prefs{ ControlURL: "https://controlplane.tailscale.com", diff --git a/types/persist/persist.go b/types/persist/persist.go index 4ed4c6070..19df45dcb 100644 --- a/types/persist/persist.go +++ b/types/persist/persist.go @@ -35,7 +35,6 @@ type Persist struct { PrivateNodeKey key.NodePrivate OldPrivateNodeKey key.NodePrivate // needed to request key rotation Provider string - LoginName string UserProfile tailcfg.UserProfile NetworkLockKey key.NLPrivate NodeID tailcfg.StableNodeID @@ -80,7 +79,6 @@ func (p *Persist) Equals(p2 *Persist) bool { p.PrivateNodeKey.Equal(p2.PrivateNodeKey) && p.OldPrivateNodeKey.Equal(p2.OldPrivateNodeKey) && p.Provider == p2.Provider && - p.LoginName == p2.LoginName && p.UserProfile.Equal(&p2.UserProfile) && p.NetworkLockKey.Equal(p2.NetworkLockKey) && p.NodeID == p2.NodeID && @@ -102,5 +100,5 @@ func (p *Persist) Pretty() string { nk = p.PublicNodeKey() } return fmt.Sprintf("Persist{lm=%v, o=%v, n=%v u=%#v}", - mk.ShortString(), ok.ShortString(), nk.ShortString(), p.LoginName) + mk.ShortString(), ok.ShortString(), nk.ShortString(), p.UserProfile.LoginName) } diff --git a/types/persist/persist_clone.go b/types/persist/persist_clone.go index 9e3385112..121d906ec 100644 --- a/types/persist/persist_clone.go +++ b/types/persist/persist_clone.go @@ -31,7 +31,6 @@ func (src *Persist) Clone() *Persist { PrivateNodeKey key.NodePrivate OldPrivateNodeKey key.NodePrivate Provider string - LoginName string UserProfile tailcfg.UserProfile NetworkLockKey key.NLPrivate NodeID tailcfg.StableNodeID diff --git a/types/persist/persist_test.go b/types/persist/persist_test.go index 2207c6d88..bfbc92c7e 100644 --- a/types/persist/persist_test.go +++ b/types/persist/persist_test.go @@ -21,7 +21,7 @@ func fieldsOf(t reflect.Type) (fields []string) { } func TestPersistEqual(t *testing.T) { - persistHandles := []string{"LegacyFrontendPrivateMachineKey", "PrivateNodeKey", "OldPrivateNodeKey", "Provider", "LoginName", "UserProfile", "NetworkLockKey", "NodeID", "DisallowedTKAStateIDs"} + persistHandles := []string{"LegacyFrontendPrivateMachineKey", "PrivateNodeKey", "OldPrivateNodeKey", "Provider", "UserProfile", "NetworkLockKey", "NodeID", "DisallowedTKAStateIDs"} if have := fieldsOf(reflect.TypeOf(Persist{})); !reflect.DeepEqual(have, persistHandles) { t.Errorf("Persist.Equal check might be out of sync\nfields: %q\nhandled: %q\n", have, persistHandles) @@ -82,17 +82,6 @@ func TestPersistEqual(t *testing.T) { &Persist{Provider: "google"}, true, }, - - { - &Persist{LoginName: "foo@tailscale.com"}, - &Persist{LoginName: "bar@tailscale.com"}, - false, - }, - { - &Persist{LoginName: "foo@tailscale.com"}, - &Persist{LoginName: "foo@tailscale.com"}, - true, - }, { &Persist{UserProfile: tailcfg.UserProfile{ ID: tailcfg.UserID(3), diff --git a/types/persist/persist_view.go b/types/persist/persist_view.go index c05b7fd1b..8c3173a23 100644 --- a/types/persist/persist_view.go +++ b/types/persist/persist_view.go @@ -68,7 +68,6 @@ func (v PersistView) LegacyFrontendPrivateMachineKey() key.MachinePrivate { func (v PersistView) PrivateNodeKey() key.NodePrivate { return v.ж.PrivateNodeKey } func (v PersistView) OldPrivateNodeKey() key.NodePrivate { return v.ж.OldPrivateNodeKey } func (v PersistView) Provider() string { return v.ж.Provider } -func (v PersistView) LoginName() string { return v.ж.LoginName } func (v PersistView) UserProfile() tailcfg.UserProfileView { return v.ж.UserProfile.View() } func (v PersistView) NetworkLockKey() key.NLPrivate { return v.ж.NetworkLockKey } func (v PersistView) NodeID() tailcfg.StableNodeID { return v.ж.NodeID } @@ -83,7 +82,6 @@ func (v PersistView) DisallowedTKAStateIDs() views.Slice[string] { PrivateNodeKey key.NodePrivate OldPrivateNodeKey key.NodePrivate Provider string - LoginName string UserProfile tailcfg.UserProfile NetworkLockKey key.NLPrivate NodeID tailcfg.StableNodeID