From 5be05f2c0d243c9ae4987137a17f009662af33a0 Mon Sep 17 00:00:00 2001
From: Gesa Stupperich <gesa@tailscale.com>
Date: Thu, 21 May 2026 14:05:49 +0100
Subject: [PATCH] control/controlclient: discard stale auth results in
 authRoutine

authRoutine snapshots c.loginGoal, runs TryLogin without the lock,
then writes back loggedIn/loginGoal under the lock. If a concurrent
Login() or Logout() changes the goal during the in-flight request,
the write-back overwrites the new intent: the more recent login goal
is silently dropped, or a logout is reverted to logged-in.

Gate both the URL-followup and success commits on c.loginGoal still
matching the goal we were processing. Stale results are ignored and
the next iteration runs with the current goal.

Updates #19326

Signed-off-by: Gesa Stupperich <gesa@tailscale.com>
---
 control/controlclient/auto.go | 34 ++++++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/control/controlclient/auto.go b/control/controlclient/auto.go
index 109a82e79..421bc0def 100644
--- a/control/controlclient/auto.go
+++ b/control/controlclient/auto.go
@@ -380,9 +380,15 @@ func (c *Auto) authRoutine() {
 			}
 			c.mu.Lock()
 			c.urlToVisit = url
-			c.loginGoal = &LoginGoal{
-				flags: LoginDefault,
-				url:   url,
+			// Only store the URL follow-up goal if no concurrent Login() call has
+			// replaced the goal we were processing while our control plane request
+			// was in flight. Otherwise, the intent from the more recent goal gets
+			// lost.
+			if c.loginGoal == goal {
+				c.loginGoal = &LoginGoal{
+					flags: LoginDefault,
+					url:   url,
+				}
 			}
 			c.mu.Unlock()
 
@@ -400,13 +406,25 @@ func (c *Auto) authRoutine() {
 		// success
 		c.direct.health.SetAuthRoutineInError(nil)
 		c.mu.Lock()
-		c.urlToVisit = ""
-		c.loggedIn = true
-		c.loginGoal = nil
+		// Only commit the login success if no concurrent Login()
+		// call has reset the goal and no Logout() has moved on
+		// while our control plane request was in flight. In the
+		// first case, clearing the goal would prevent the next
+		// iteration from picking it up and running with it. In
+		// the second case, we would record that we're loggedIn
+		// even though we're logged out.
+		goalStillCurrentGoal := c.loginGoal == goal
+		if goalStillCurrentGoal {
+			c.urlToVisit = ""
+			c.loggedIn = true
+			c.loginGoal = nil
+		}
 		c.mu.Unlock()
 
-		c.sendStatus("authRoutine-success", nil, "", nil)
-		c.restartMap()
+		if goalStillCurrentGoal {
+			c.sendStatus("authRoutine-success", nil, "", nil)
+			c.restartMap()
+		}
 		bo.Reset()
 	}
 }