From 48dd4bbe21e1b4ade2ca23acce86a115cb0f59ff Mon Sep 17 00:00:00 2001
From: Nick Khyl <nickk@tailscale.com>
Date: Fri, 7 Feb 2025 11:18:57 -0600
Subject: [PATCH] ipn/ipn{local,server}: remove ResetForClientDisconnect in
 favor of SetCurrentUser(nil)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

There’s (*LocalBackend).ResetForClientDisconnect, and there’s also (*LocalBackend).resetForProfileChangeLockedOnEntry.
Both methods essentially did the same thing but in slightly different ways. For example, resetForProfileChangeLockedOnEntry didn’t reset the control client until (*LocalBackend).Start() was called at the very end and didn’t reset the keyExpired flag, while ResetForClientDisconnect didn’t reinitialize TKA.

Since SetCurrentUser can be called with a nil argument to reset the currently connected user and internally calls resetForProfileChangeLockedOnEntry, we can remove ResetForClientDisconnect and let SetCurrentUser and resetForProfileChangeLockedOnEntry handle it.

Updates #14823

Signed-off-by: Nick Khyl <nickk@tailscale.com>
---
 ipn/ipnlocal/local.go   | 41 ++++++-----------------------------------
 ipn/ipnserver/server.go |  2 +-
 2 files changed, 7 insertions(+), 36 deletions(-)

diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go
index 38bcfaaa2..811b978f7 100644
--- a/ipn/ipnlocal/local.go
+++ b/ipn/ipnlocal/local.go
@@ -5615,41 +5615,6 @@ func (b *LocalBackend) resetAuthURLLocked() {
 	b.authActor = nil
 }
 
-// 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
-// connect to tailscaled next. But it does not trigger a logout; we
-// don't want to the user to have to reauthenticate in the future
-// when they restart the GUI.
-func (b *LocalBackend) ResetForClientDisconnect() {
-	b.logf("LocalBackend.ResetForClientDisconnect")
-
-	unlock := b.lockAndGetUnlock()
-	defer unlock()
-
-	prevCC := b.resetControlClientLocked()
-	if prevCC != nil {
-		// Needs to happen without b.mu held.
-		defer prevCC.Shutdown()
-	}
-
-	b.setNetMapLocked(nil)
-	b.pm.Reset()
-	if b.currentUser != nil {
-		if c, ok := b.currentUser.(ipnauth.ActorCloser); ok {
-			c.Close()
-		}
-		b.currentUser = nil
-	}
-	b.keyExpired = false
-	b.resetAuthURLLocked()
-	b.activeLogin = ""
-	b.resetDialPlan()
-	b.resetAlwaysOnOverrideLocked()
-	b.setAtomicValuesFromPrefsLocked(ipn.PrefsView{})
-	b.enterStateLockedOnEntry(ipn.Stopped, unlock)
-}
-
 func (b *LocalBackend) ShouldRunSSH() bool { return b.sshAtomicBool.Load() && envknob.CanSSHD() }
 
 // ShouldRunWebClient reports whether the web client is being run
@@ -7178,13 +7143,19 @@ func (b *LocalBackend) resetForProfileChangeLockedOnEntry(unlock unlockOnce) err
 	b.setNetMapLocked(nil) // Reset netmap.
 	// Reset the NetworkMap in the engine
 	b.e.SetNetworkMap(new(netmap.NetworkMap))
+	if prevCC := b.resetControlClientLocked(); prevCC != nil {
+		// Needs to happen without b.mu held.
+		defer prevCC.Shutdown()
+	}
 	if err := b.initTKALocked(); err != nil {
 		return err
 	}
 	b.lastServeConfJSON = mem.B(nil)
 	b.serveConfig = ipn.ServeConfigView{}
 	b.lastSuggestedExitNode = ""
+	b.keyExpired = false
 	b.resetAlwaysOnOverrideLocked()
+	b.setAtomicValuesFromPrefsLocked(b.pm.CurrentPrefs())
 	b.enterStateLockedOnEntry(ipn.NoState, unlock) // Reset state; releases b.mu
 	b.health.SetLocalLogConfigHealth(nil)
 	return b.Start(ipn.Options{})
diff --git a/ipn/ipnserver/server.go b/ipn/ipnserver/server.go
index 7bc2c7b3e..436b8404d 100644
--- a/ipn/ipnserver/server.go
+++ b/ipn/ipnserver/server.go
@@ -434,7 +434,7 @@ func (s *Server) addActiveHTTPRequest(req *http.Request, actor ipnauth.Actor) (o
 				s.logf("client disconnected; staying alive in server mode")
 			} else {
 				s.logf("client disconnected; stopping server")
-				lb.ResetForClientDisconnect()
+				lb.SetCurrentUser(nil)
 			}
 		}