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>
This commit is contained in:
committed by
Gesa Stupperich
parent
6a822dcc36
commit
5be05f2c0d
@@ -380,9 +380,15 @@ func (c *Auto) authRoutine() {
|
|||||||
}
|
}
|
||||||
c.mu.Lock()
|
c.mu.Lock()
|
||||||
c.urlToVisit = url
|
c.urlToVisit = url
|
||||||
c.loginGoal = &LoginGoal{
|
// Only store the URL follow-up goal if no concurrent Login() call has
|
||||||
flags: LoginDefault,
|
// replaced the goal we were processing while our control plane request
|
||||||
url: url,
|
// 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()
|
c.mu.Unlock()
|
||||||
|
|
||||||
@@ -400,13 +406,25 @@ func (c *Auto) authRoutine() {
|
|||||||
// success
|
// success
|
||||||
c.direct.health.SetAuthRoutineInError(nil)
|
c.direct.health.SetAuthRoutineInError(nil)
|
||||||
c.mu.Lock()
|
c.mu.Lock()
|
||||||
c.urlToVisit = ""
|
// Only commit the login success if no concurrent Login()
|
||||||
c.loggedIn = true
|
// call has reset the goal and no Logout() has moved on
|
||||||
c.loginGoal = nil
|
// 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.mu.Unlock()
|
||||||
|
|
||||||
c.sendStatus("authRoutine-success", nil, "", nil)
|
if goalStillCurrentGoal {
|
||||||
c.restartMap()
|
c.sendStatus("authRoutine-success", nil, "", nil)
|
||||||
|
c.restartMap()
|
||||||
|
}
|
||||||
bo.Reset()
|
bo.Reset()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user