ipn/ipnlocal: set WantRunning upon an interactive login, but not during a seamless renewal or a profile switch

The LocalBackend's state machine starts in NoState and soon transitions to NeedsLogin if there's no auto-start profile,
with the profileManager starting with a new empty profile. Notably, entering the NeedsLogin state blocks engine updates.
We expect the user to transition out of this state by logging in interactively, and we set WantRunning to true when
controlclient enters the StateAuthenticated state.

While our intention is correct, and completing an interactive login should set WantRunning to true, our assumption
that logging into the current Tailscale profile is the only way to transition out of the NeedsLogin state is not accurate.
Another common transition path includes an explicit profile switch (via LocalBackend.SwitchProfile) or an implicit switch
when a Windows user connects to the backend. This results in a bug where WantRunning is set to true even when it was
previously set to false, and the user expressed no intention of changing it.

A similar issue occurs when switching from (sic) a Tailnet that has seamlessRenewalEnabled, regardless of the current state
of the LocalBackend's state machine, and also results in unexpectedly set WantRunning. While this behavior is generally
undesired, it is also incorrect that it depends on the control knobs of the Tailnet we're switching from rather than
the Tailnet we're switching to. However, this issue needs to be addressed separately.

This PR updates LocalBackend.SetControlClientStatus to only set WantRunning to true in response to an interactive login
as indicated by a non-empty authURL.

Fixes #6668
Fixes #11280
Updates #12756

Signed-off-by: Nick Khyl <nickk@tailscale.com>

ipn/ipnlocal/local.go

@@ -1342,20 +1342,26 @@ func (b *LocalBackend) SetControlClientStatus(c controlclient.Client, st control
 			prefs.Persist = st.Persist.AsStruct()
 		}
 	}
+	if st.LoginFinished() {
+		if b.authURL != "" {
+			b.resetAuthURLLocked()
+			// Interactive login finished successfully (URL visited).
+			// After an interactive login, the user always wants
+			// WantRunning.
+			if !prefs.WantRunning {
+				prefs.WantRunning = true
+				prefsChanged = true
+			}
+		}
+		if prefs.LoggedOut {
+			prefs.LoggedOut = false
+			prefsChanged = true
+		}
+	}
 	if st.URL != "" {
 		b.authURL = st.URL
 		b.authURLTime = b.clock.Now()
 	}
-	if (wasBlocked || b.seamlessRenewalEnabled()) && st.LoginFinished() {
-		// Interactive login finished successfully (URL visited).
-		// After an interactive login, the user always wants
-		// WantRunning.
-		if !prefs.WantRunning || prefs.LoggedOut {
-			prefsChanged = true
-		}
-		prefs.WantRunning = true
-		prefs.LoggedOut = false
-	}
 	if shouldAutoExitNode() {
 		// Re-evaluate exit node suggestion in case circumstances have changed.
 		_, err := b.suggestExitNodeLocked(curNetMap)
@@ -4704,8 +4710,7 @@ func (b *LocalBackend) enterStateLockedOnEntry(newState ipn.State, unlock unlock
 	activeLogin := b.activeLogin
 	authURL := b.authURL
 	if newState == ipn.Running {
-		b.authURL = ""
-		b.authURLTime = time.Time{}
+		b.resetAuthURLLocked()
 
 		// Start a captive portal detection loop if none has been
 		// started. Create a new context if none is present, since it
@@ -4987,7 +4992,7 @@ func (b *LocalBackend) resetControlClientLocked() controlclient.Client {
 		return nil
 	}
 
-	b.authURL = ""
+	b.resetAuthURLLocked()
 
 	// When we clear the control client, stop any outstanding netmap expiry
 	// timer; synthesizing a new netmap while we don't have a control
@@ -5007,6 +5012,11 @@ func (b *LocalBackend) resetControlClientLocked() controlclient.Client {
 	return prev
 }
 
+func (b *LocalBackend) resetAuthURLLocked() {
+	b.authURL = ""
+	b.authURLTime = time.Time{}
+}
+
 // ResetForClientDisconnect resets the backend for GUI clients running
 // in interactive (non-headless) mode. This is currently used only by
 // Windows. This causes all state to be cleared, lest an unrelated user
@@ -5034,8 +5044,7 @@ func (b *LocalBackend) ResetForClientDisconnect() {
 		b.currentUser = nil
 	}
 	b.keyExpired = false
-	b.authURL = ""
-	b.authURLTime = time.Time{}
+	b.resetAuthURLLocked()
 	b.activeLogin = ""
 	b.resetDialPlan()
 	b.setAtomicValuesFromPrefsLocked(ipn.PrefsView{})

ipn/ipnlocal/state_test.go

@@ -651,25 +651,55 @@ func TestStateMachine(t *testing.T) {
 		c.Assert(ipn.NeedsLogin, qt.Equals, b.State())
 	}
 
+	// Explicitly set the ControlURL to avoid defaulting to [ipn.DefaultControlURL].
+	// This prevents [LocalBackend] from using the production control server during tests
+	// and ensures that [LocalBackend.validPopBrowserURL] returns true for the
+	// fake interactive login URLs used below. Otherwise, we won't be receiving
+	// BrowseToURL notifications as expected.
+	// See tailscale/tailscale#11393.
+	notifies.expect(1)
+	b.EditPrefs(&ipn.MaskedPrefs{
+		ControlURLSet: true,
+		Prefs: ipn.Prefs{
+			ControlURL: "https://localhost:1/",
+		},
+	})
+	notifies.drain(1)
+
+	t.Logf("\n\nStartLoginInteractive3")
 	b.StartLoginInteractive(context.Background())
-	t.Logf("\n\nLoginFinished3")
+	// We've been logged out, and the previously created profile is now deleted.
+	// We're attempting an interactive login for the first time with the new profile,
+	// this should result in a call to the control server, which in turn should provide
+	// an interactive login URL to visit.
+	notifies.expect(2)
+	url3 := "https://localhost:1/3"
+	cc.send(nil, url3, false, nil)
+	{
+		nn := notifies.drain(2)
+		cc.assertCalls("Login")
+		c.Assert(nn[1].BrowseToURL, qt.IsNotNil)
+		c.Assert(*nn[1].BrowseToURL, qt.Equals, url3)
+	}
+	t.Logf("%q visited", url3)
 	notifies.expect(3)
 	cc.persist.UserProfile.LoginName = "user2"
 	cc.persist.NodeID = "node2"
 	cc.send(nil, "", true, &netmap.NetworkMap{
 		SelfNode: (&tailcfg.Node{MachineAuthorized: true}).View(),
 	})
+	t.Logf("\n\nLoginFinished3")
 	{
 		nn := notifies.drain(3)
-		cc.assertCalls("Login")
 		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().UserProfile().LoginName, qt.Equals, "user2")
 		c.Assert(nn[1].Prefs.LoggedOut(), qt.IsFalse)
+		// If a user initiates an interactive login, they also expect WantRunning to become true.
 		c.Assert(nn[1].Prefs.WantRunning(), qt.IsTrue)
+		c.Assert(nn[2].State, qt.IsNotNil)
 		c.Assert(ipn.Starting, qt.Equals, *nn[2].State)
 	}
 
@@ -767,18 +797,12 @@ func TestStateMachine(t *testing.T) {
 	// We want to try logging in as a different user, while Stopped.
 	// First, start the login process (without logging out first).
 	t.Logf("\n\nLoginDifferent")
-	notifies.expect(2)
-	b.EditPrefs(&ipn.MaskedPrefs{
-		ControlURLSet: true,
-		Prefs: ipn.Prefs{
-			ControlURL: "https://localhost:1/",
-		},
-	})
+	notifies.expect(1)
 	b.StartLoginInteractive(context.Background())
-	url3 := "https://localhost:1/3"
-	cc.send(nil, url3, false, nil)
+	url4 := "https://localhost:1/4"
+	cc.send(nil, url4, false, nil)
 	{
-		nn := notifies.drain(2)
+		nn := notifies.drain(1)
 		// It might seem like WantRunning should switch to true here,
 		// but that would be risky since we already have a valid
 		// user account. It might try to reconnect to the old account
@@ -787,8 +811,8 @@ func TestStateMachine(t *testing.T) {
 		// Because the login hasn't yet completed, the old login
 		// is still valid, so it's correct that we stay paused.
 		cc.assertCalls("Login")
-		c.Assert(nn[1].BrowseToURL, qt.IsNotNil)
-		c.Assert(*nn[1].BrowseToURL, qt.Equals, url3)
+		c.Assert(nn[0].BrowseToURL, qt.IsNotNil)
+		c.Assert(*nn[0].BrowseToURL, qt.Equals, url4)
 	}
 
 	// Now, let's complete the interactive login, using a different