diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index e9980368b..f1740babc 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -799,19 +799,22 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) { b.send(ipn.Notify{LoginFinished: &empty.Message{}}) } - prefsChanged := false - // Lock b once and do only the things that require locking. b.mu.Lock() if st.LogoutFinished != nil { - // Since we're logged out now, our netmap cache is invalid. - // Since st.NetMap==nil means "netmap is unchanged", there is - // no other way to represent this change. - b.setNetMapLocked(nil) - b.e.SetNetworkMap(new(netmap.NetworkMap)) + if p := b.pm.CurrentPrefs(); p.Persist() == nil || p.Persist().LoginName == "" { + b.mu.Unlock() + return + } + if err := b.pm.DeleteProfile(b.pm.CurrentProfile().ID); err != nil { + b.logf("error deleting profile: %v", err) + } + b.resetForProfileChangeLockedOnEntry() + return } + prefsChanged := false prefs := b.pm.CurrentPrefs().AsStruct() netMap := b.netMap interact := b.interact diff --git a/ipn/ipnlocal/profiles.go b/ipn/ipnlocal/profiles.go index c26753d82..f330c2e9a 100644 --- a/ipn/ipnlocal/profiles.go +++ b/ipn/ipnlocal/profiles.go @@ -268,6 +268,11 @@ var errProfileNotFound = errors.New("profile not found") // useful for deleting the last profile. In other cases, it is // recommended to call SwitchProfile() first. func (pm *profileManager) DeleteProfile(id ipn.ProfileID) error { + if id == "" && pm.isNewProfile { + // Deleting the in-memory only new profile, just create a new one. + pm.NewProfile() + return nil + } kp, ok := pm.knownProfiles[id] if !ok { return errProfileNotFound diff --git a/ipn/ipnlocal/state_test.go b/ipn/ipnlocal/state_test.go index 35928e90b..944aeda00 100644 --- a/ipn/ipnlocal/state_test.go +++ b/ipn/ipnlocal/state_test.go @@ -14,6 +14,7 @@ import ( qt "github.com/frankban/quicktest" "tailscale.com/control/controlclient" + "tailscale.com/envknob" "tailscale.com/ipn" "tailscale.com/ipn/store/mem" "tailscale.com/tailcfg" @@ -81,6 +82,7 @@ func (nt *notifyThrottler) drain(count int) []ipn.Notify { // no more notifications expected close(ch) + nt.t.Log(nn) return nn } @@ -125,6 +127,9 @@ func (cc *mockControl) populateKeys() (newKeys bool) { newKeys = true } + if cc.persist == nil { + cc.persist = &persist.Persist{} + } if cc.persist != nil && cc.persist.PrivateNodeKey.IsZero() { cc.logf("Generating a new nodekey.") cc.persist.OldPrivateNodeKey = cc.persist.PrivateNodeKey @@ -281,6 +286,8 @@ func (cc *mockControl) UpdateEndpoints(endpoints []tailcfg.Endpoint) { // predictable, but maybe a bit less thorough. This is more of an overall // state machine test than a test of the wgengine+magicsock integration. func TestStateMachine(t *testing.T) { + envknob.Setenv("TAILSCALE_USE_WIP_CODE", "1") + defer envknob.Setenv("TAILSCALE_USE_WIP_CODE", "") c := qt.New(t) logf := tstest.WhileTestRunningLogger(t) @@ -304,10 +311,10 @@ func TestStateMachine(t *testing.T) { cc.opts = opts cc.logfActual = opts.Logf cc.authBlocked = true - cc.persist = &cc.opts.Persist + cc.persist = cc.opts.Persist.Clone() cc.mu.Unlock() - cc.logf("ccGen: new mockControl.") + t.Logf("ccGen: new mockControl.") cc.called("New") return cc, nil }) @@ -585,25 +592,29 @@ func TestStateMachine(t *testing.T) { // Let's make the logout succeed. t.Logf("\n\nLogout (async) - succeed") - notifies.expect(1) + notifies.expect(3) cc.setAuthBlocked(true) cc.send(nil, "", false, nil) { - nn := notifies.drain(1) - cc.assertCalls("unpause", "unpause") + nn := notifies.drain(3) + cc.assertCalls("unpause", "unpause", "Shutdown", "unpause", "New", "unpause") c.Assert(nn[0].State, qt.IsNotNil) - c.Assert(ipn.NeedsLogin, qt.Equals, *nn[0].State) - c.Assert(b.Prefs().LoggedOut(), qt.IsTrue) + c.Assert(*nn[0].State, qt.Equals, ipn.NoState) + c.Assert(nn[1].Prefs, qt.IsNotNil) // emptyPrefs + c.Assert(nn[2].State, qt.IsNotNil) + c.Assert(*nn[2].State, qt.Equals, ipn.NeedsLogin) + c.Assert(b.Prefs().LoggedOut(), qt.IsFalse) c.Assert(b.Prefs().WantRunning(), qt.IsFalse) - c.Assert(ipn.NeedsLogin, qt.Equals, b.State()) + c.Assert(b.State(), qt.Equals, ipn.NeedsLogin) } - // A second logout should do nothing, since the prefs haven't changed. + // A second logout should reset all prefs. t.Logf("\n\nLogout2 (async)") - notifies.expect(0) + notifies.expect(1) b.Logout() { - notifies.drain(0) + nn := notifies.drain(1) + c.Assert(nn[0].Prefs, qt.IsNotNil) // emptyPrefs // BUG: the backend has already called StartLogout, and we're // still logged out. So it shouldn't call it again. cc.assertCalls("StartLogout", "unpause") @@ -620,7 +631,7 @@ func TestStateMachine(t *testing.T) { cc.send(nil, "", false, nil) { notifies.drain(0) - cc.assertCalls("unpause", "unpause") + cc.assertCalls() c.Assert(b.Prefs().LoggedOut(), qt.IsTrue) c.Assert(b.Prefs().WantRunning(), qt.IsFalse) c.Assert(ipn.NeedsLogin, qt.Equals, b.State()) @@ -647,7 +658,7 @@ func TestStateMachine(t *testing.T) { cc.send(nil, "", false, nil) { notifies.drain(0) - cc.assertCalls("unpause", "unpause") + cc.assertCalls() c.Assert(b.Prefs().LoggedOut(), qt.IsTrue) c.Assert(b.Prefs().WantRunning(), qt.IsFalse) c.Assert(ipn.NeedsLogin, qt.Equals, b.State()) @@ -679,10 +690,7 @@ func TestStateMachine(t *testing.T) { c.Assert(ipn.NeedsLogin, qt.Equals, b.State()) } - // Let's break the rules a little. Our control server accepts - // your invalid login attempt, with no need for an interactive login. - // (This simulates an admin reviving a key that you previously - // disabled.) + b.Login(nil) t.Logf("\n\nLoginFinished3") notifies.expect(3) cc.setAuthBlocked(false) @@ -692,9 +700,10 @@ func TestStateMachine(t *testing.T) { }) { nn := notifies.drain(3) - cc.assertCalls("unpause", "unpause", "unpause") + cc.assertCalls("Login", "unpause", "unpause", "unpause") c.Assert(nn[0].LoginFinished, qt.IsNotNil) c.Assert(nn[1].Prefs, qt.IsNotNil) + 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") @@ -737,7 +746,7 @@ func TestStateMachine(t *testing.T) { c.Assert(nn[1].State, qt.IsNotNil) c.Assert(nn[0].Prefs.WantRunning(), qt.IsFalse) c.Assert(nn[0].Prefs.LoggedOut(), qt.IsFalse) - c.Assert(ipn.Stopped, qt.Equals, *nn[1].State) + c.Assert(*nn[1].State, qt.Equals, ipn.Stopped) } // When logged in but !WantRunning, ipn leaves us unpaused to retrieve diff --git a/tstest/integration/integration_test.go b/tstest/integration/integration_test.go index a7c6dde33..504597586 100644 --- a/tstest/integration/integration_test.go +++ b/tstest/integration/integration_test.go @@ -514,6 +514,7 @@ func TestLogoutRemovesAllPeers(t *testing.T) { nodes[i].AwaitIP() nodes[i].AwaitRunning() } + expectedPeers := len(nodes) - 1 // Make every node ping every other node. // This makes sure magicsock is fully populated. @@ -543,12 +544,15 @@ func TestLogoutRemovesAllPeers(t *testing.T) { } } - wantNode0PeerCount(len(nodes) - 1) // all other nodes are peers + wantNode0PeerCount(expectedPeers) // all other nodes are peers nodes[0].MustLogOut() wantNode0PeerCount(0) // node[0] is logged out, so it should not have any peers - nodes[0].MustUp() + + nodes[0].MustUp() // This will create a new node + expectedPeers++ + nodes[0].AwaitIP() - wantNode0PeerCount(len(nodes) - 1) // all other nodes are peers again + wantNode0PeerCount(expectedPeers) // all existing peers and the new node } // testEnv contains the test environment (set of servers) used by one