wip

ipn/ipnlocal/local.go

@@ -206,8 +206,6 @@ type LocalBackend struct {
 	activeLogin      string // last logged LoginName from netMap
 	engineStatus     ipn.EngineStatus
 	endpoints        []tailcfg.Endpoint
-	blocked          bool
-	keyExpired       bool
 	authURL          string // cleared on Notify
 	authURLSticky    string // not cleared on Notify
 	interact         bool
@@ -917,28 +915,10 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) {
 			})
 		}
 	}
-
-	wasBlocked := b.blocked
-	keyExpiryExtended := false
-	if st.NetMap != nil {
-		wasExpired := b.keyExpired
-		isExpired := !st.NetMap.Expiry.IsZero() && st.NetMap.Expiry.Before(time.Now())
-		if wasExpired && !isExpired {
-			keyExpiryExtended = true
-		}
-		b.keyExpired = isExpired
-	}
 	b.mu.Unlock()
 
-	if keyExpiryExtended && wasBlocked {
+	if st.LoginFinished != nil {
-		// Key extended, unblock the engine
-		b.blockEngineUpdates(false)
-	}
-
-	if st.LoginFinished != nil && wasBlocked {
 		// Auth completed, unblock the engine
-		b.blockEngineUpdates(false)
-		b.authReconfig()
 		b.send(ipn.Notify{LoginFinished: &empty.Message{}})
 	}
 
@@ -983,7 +963,7 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) {
 		b.authURL = st.URL
 		b.authURLSticky = st.URL
 	}
-	if wasBlocked && st.LoginFinished != nil {
+	if st.LoginFinished != nil {
 		// Interactive login finished successfully (URL visited).
 		// After an interactive login, the user always wants
 		// WantRunning.
@@ -2068,8 +2048,6 @@ func (b *LocalBackend) popBrowserAuthNow() {
 
 	b.logf("popBrowserAuthNow: url=%v", url != "")
 
-	b.blockEngineUpdates(true)
-	b.stopEngineAndWait()
 	b.tellClientToBrowseToURL(url)
 	if b.State() == ipn.Running {
 		b.enterState(ipn.Starting)
@@ -2900,39 +2878,17 @@ func (b *LocalBackend) NetMap() *netmap.NetworkMap {
 	return b.netMap
 }
 
-func (b *LocalBackend) isEngineBlocked() bool {
-	b.mu.Lock()
-	defer b.mu.Unlock()
-	return b.blocked
-}
-
-// blockEngineUpdate sets b.blocked to block, while holding b.mu. Its
-// indirect effect is to turn b.authReconfig() into a no-op if block
-// is true.
-func (b *LocalBackend) blockEngineUpdates(block bool) {
-	b.logf("blockEngineUpdates(%v)", block)
-
-	b.mu.Lock()
-	b.blocked = block
-	b.mu.Unlock()
-}
-
 // authReconfig pushes a new configuration into wgengine, if engine
 // updates are not currently blocked, based on the cached netmap and
 // user prefs.
 func (b *LocalBackend) authReconfig() {
 	b.mu.Lock()
-	blocked := b.blocked
 	prefs := b.pm.CurrentPrefs()
 	nm := b.netMap
 	hasPAC := b.prevIfState.HasPAC()
 	disableSubnetsIfPAC := nm != nil && nm.Debug != nil && nm.Debug.DisableSubnetsIfPAC.EqualBool(true)
 	b.mu.Unlock()
 
-	if blocked {
-		b.logf("[v1] authReconfig: blocked, skipping.")
-		return
-	}
 	if nm == nil {
 		b.logf("[v1] authReconfig: netmap not yet valid. Skipping.")
 		return
@@ -3590,7 +3546,6 @@ func (b *LocalBackend) enterStateLockedOnEntry(newState ipn.State) {
 	switch newState {
 	case ipn.NeedsLogin:
 		systemd.Status("Needs login: %s", authURL)
-		b.blockEngineUpdates(true)
 		fallthrough
 	case ipn.Stopped:
 		err := b.e.Reconfig(&wgcfg.Config{}, &router.Config{}, &dns.Config{}, nil)
@@ -3632,12 +3587,10 @@ func (b *LocalBackend) hasNodeKey() bool {
 func (b *LocalBackend) nextState() ipn.State {
 	b.mu.Lock()
 	var (
-		cc         = b.cc
+		cc     = b.cc
-		netMap     = b.netMap
+		netMap = b.netMap
-		state      = b.state
+		state  = b.state
-		blocked    = b.blocked
+		st     = b.engineStatus
-		st         = b.engineStatus
-		keyExpired = b.keyExpired
 
 		wantRunning = false
 		loggedOut   = false
@@ -3649,7 +3602,7 @@ func (b *LocalBackend) nextState() ipn.State {
 	b.mu.Unlock()
 
 	switch {
-	case !wantRunning && !loggedOut && !blocked && b.hasNodeKey():
+	case !wantRunning && !loggedOut && b.hasNodeKey():
 		return ipn.Stopped
 	case netMap == nil:
 		if (cc != nil && cc.AuthCantContinue()) || loggedOut {
@@ -3677,10 +3630,6 @@ func (b *LocalBackend) nextState() ipn.State {
 		}
 	case !wantRunning:
 		return ipn.Stopped
-	case keyExpired:
-		// NetMap must be non-nil for us to get here.
-		// The node key expired, need to relogin.
-		return ipn.NeedsLogin
 	case netMap.MachineStatus != tailcfg.MachineAuthorized:
 		// TODO(crawshaw): handle tailcfg.MachineInvalid
 		return ipn.NeedsMachineAuth
@@ -3781,7 +3730,6 @@ func (b *LocalBackend) ResetForClientDisconnect() {
 	b.resetControlClientLockedAsync()
 	b.setNetMapLocked(nil)
 	b.pm.Reset()
-	b.keyExpired = false
 	b.authURL = ""
 	b.authURLSticky = ""
 	b.activeLogin = ""

ipn/ipnlocal/state_test.go

@@ -327,6 +327,7 @@ func TestStateMachine(t *testing.T) {
 	notifies.expect(0)
 
 	b.SetNotifyCallback(func(n ipn.Notify) {
+		t.Helper()
 		if n.State != nil ||
 			(n.Prefs != nil && n.Prefs.Valid()) ||
 			n.BrowseToURL != nil ||
@@ -407,7 +408,7 @@ func TestStateMachine(t *testing.T) {
 	// Attempted non-interactive login with no key; indicate that
 	// the user needs to visit a login URL.
 	t.Logf("\n\nLogin (url response)")
-	notifies.expect(1)
+	notifies.expect(2)
 	url1 := "https://localhost:1/1"
 	cc.send(nil, url1, false, nil)
 	{
@@ -416,12 +417,12 @@ func TestStateMachine(t *testing.T) {
 		// ...but backend eats that notification, because the user
 		// didn't explicitly request interactive login yet, and
 		// we're already in NeedsLogin state.
-		nn := notifies.drain(1)
+		nn := notifies.drain(2)
 
 		c.Assert(nn[0].Prefs, qt.IsNotNil)
 		c.Assert(nn[0].Prefs.LoggedOut(), qt.IsFalse)
 		c.Assert(nn[0].Prefs.WantRunning(), qt.IsFalse)
-		c.Assert(ipn.NeedsLogin, qt.Equals, b.State())
+		c.Assert(ipn.Stopped, qt.Equals, b.State())
 	}
 
 	// Now we'll try an interactive login.
@@ -456,15 +457,19 @@ func TestStateMachine(t *testing.T) {
 
 	// Provide a new interactive login URL.
 	t.Logf("\n\nLogin2 (url response)")
-	notifies.expect(1)
+	notifies.expect(2)
+	t.Logf("Dasdfasdf")
 	url2 := "https://localhost:1/2"
+	t.Logf("Dasdfasdf")
 	cc.send(nil, url2, false, nil)
+	t.Logf("Dasdfasdf")
 	{
 		cc.assertCalls()
 
 		// This time, backend should emit it to the UI right away,
 		// because the UI is anxiously awaiting a new URL to visit.
-		nn := notifies.drain(1)
+		t.Logf("Dasdfasdf")
+		nn := notifies.drain(2)
 		c.Assert(nn[0].BrowseToURL, qt.IsNotNil)
 		c.Assert(url2, qt.Equals, *nn[0].BrowseToURL)
 		c.Assert(ipn.NeedsLogin, qt.Equals, b.State())
@@ -914,7 +919,6 @@ func TestStateMachine(t *testing.T) {
 		c.Assert(nn[0].State, qt.IsNotNil)
 		c.Assert(ipn.NeedsLogin, qt.Equals, *nn[0].State)
 		c.Assert(ipn.NeedsLogin, qt.Equals, b.State())
-		c.Assert(b.isEngineBlocked(), qt.IsTrue)
 	}
 
 	t.Logf("\n\nExtendKey")
@@ -929,7 +933,6 @@ func TestStateMachine(t *testing.T) {
 		c.Assert(nn[0].State, qt.IsNotNil)
 		c.Assert(ipn.Starting, qt.Equals, *nn[0].State)
 		c.Assert(ipn.Starting, qt.Equals, b.State())
-		c.Assert(b.isEngineBlocked(), qt.IsFalse)
 	}
 	notifies.expect(1)
 	// Fake a DERP connection.