From 04e1ce00340fe2a3d74d4bca7e0e9e307deb3779 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 30 Aug 2023 18:33:58 -0700 Subject: [PATCH] control/controlclient: remove unused StartLogout Updates #cleanup Co-authored-by: Maisem Ali Change-Id: I9d052fdbee787f1e8c872124e4bee61c7f04d142 Signed-off-by: Brad Fitzpatrick --- cmd/tsconnect/wasm/wasm_js.go | 6 +++++- control/controlclient/auto.go | 11 ----------- control/controlclient/client.go | 4 ---- ipn/ipnlocal/local.go | 19 +------------------ ipn/ipnlocal/local_test.go | 3 --- ipn/ipnlocal/state_test.go | 25 ++++++++++--------------- tstest/log.go | 4 ++-- 7 files changed, 18 insertions(+), 54 deletions(-) diff --git a/cmd/tsconnect/wasm/wasm_js.go b/cmd/tsconnect/wasm/wasm_js.go index 356194586..baf5d0160 100644 --- a/cmd/tsconnect/wasm/wasm_js.go +++ b/cmd/tsconnect/wasm/wasm_js.go @@ -326,7 +326,11 @@ func (i *jsIPN) logout() { if i.lb.State() == ipn.NoState { log.Printf("Backend not running") } - go i.lb.Logout() + go func() { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + i.lb.LogoutSync(ctx) + }() } func (i *jsIPN) ssh(host, username string, termConfig js.Value) map[string]any { diff --git a/control/controlclient/auto.go b/control/controlclient/auto.go index 3b4da21e9..dc8a7f210 100644 --- a/control/controlclient/auto.go +++ b/control/controlclient/auto.go @@ -679,17 +679,6 @@ func (c *Auto) Login(t *tailcfg.Oauth2Token, flags LoginFlags) { c.cancelAuth() } -func (c *Auto) StartLogout() { - c.logf("client.StartLogout()") - - c.mu.Lock() - c.loginGoal = &LoginGoal{ - wantLoggedIn: false, - } - c.mu.Unlock() - c.cancelAuth() -} - func (c *Auto) Logout(ctx context.Context) error { c.logf("client.Logout()") diff --git a/control/controlclient/client.go b/control/controlclient/client.go index 80e56790e..f942676ae 100644 --- a/control/controlclient/client.go +++ b/control/controlclient/client.go @@ -34,10 +34,6 @@ type Client interface { // LoginFinished flag (on success) or an auth URL (if further // interaction is needed). Login(*tailcfg.Oauth2Token, LoginFlags) - // StartLogout starts an asynchronous logout process. - // When it finishes, the Status callback will be called while - // AuthCantContinue()==true. - StartLogout() // Logout starts a synchronous logout process. It doesn't return // until the logout operation has been completed. Logout(context.Context) error diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 0342affd0..1d386dd84 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -3910,18 +3910,7 @@ func (b *LocalBackend) ShouldHandleViaIP(ip netip.Addr) bool { return false } -// Logout tells the controlclient that we want to log out, and -// transitions the local engine to the logged-out state without -// waiting for controlclient to be in that state. -func (b *LocalBackend) Logout() { - b.logout(context.Background(), false) -} - func (b *LocalBackend) LogoutSync(ctx context.Context) error { - return b.logout(ctx, true) -} - -func (b *LocalBackend) logout(ctx context.Context, sync bool) error { b.mu.Lock() cc := b.cc b.mu.Unlock() @@ -3946,13 +3935,7 @@ func (b *LocalBackend) logout(ctx context.Context, sync bool) error { return errors.New("no controlclient") } - var err error - if sync { - err = cc.Logout(ctx) - } else { - cc.StartLogout() - } - + err := cc.Logout(ctx) b.stateMachine() return err } diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 4f77d9c85..a9422433e 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -826,9 +826,6 @@ type legacyBackend interface { StartLoginInteractive() // Login logs in with an OAuth2 token. Login(token *tailcfg.Oauth2Token) - // Logout terminates the current login session and stops the - // wireguard engine. - Logout() // SetPrefs installs a new set of user preferences, including // WantRunning. This may cause the wireguard engine to // reconfigure or stop. diff --git a/ipn/ipnlocal/state_test.go b/ipn/ipnlocal/state_test.go index 4ce904eba..cff6b175c 100644 --- a/ipn/ipnlocal/state_test.go +++ b/ipn/ipnlocal/state_test.go @@ -217,11 +217,6 @@ func (cc *mockControl) Login(t *tailcfg.Oauth2Token, flags controlclient.LoginFl cc.authBlocked = interact || newKeys } -func (cc *mockControl) StartLogout() { - cc.logf("StartLogout") - cc.called("StartLogout") -} - func (cc *mockControl) Logout(ctx context.Context) error { cc.logf("Logout") cc.called("Logout") @@ -329,10 +324,10 @@ func TestStateMachine(t *testing.T) { (n.Prefs != nil && n.Prefs.Valid()) || n.BrowseToURL != nil || n.LoginFinished != nil { - logf("\n%v\n\n", n) + logf("%v\n\n", n) notifies.put(n) } else { - logf("\n(ignored) %v\n\n", n) + logf("(ignored) %v\n\n", n) } }) @@ -583,12 +578,12 @@ func TestStateMachine(t *testing.T) { // User wants to logout. store.awaitWrite() - t.Logf("\n\nLogout (async)") + t.Logf("\n\nLogout") notifies.expect(2) - b.Logout() + b.LogoutSync(context.Background()) { nn := notifies.drain(2) - cc.assertCalls("pause", "StartLogout") + cc.assertCalls("pause", "Logout") c.Assert(nn[0].State, qt.IsNotNil) c.Assert(nn[1].Prefs, qt.IsNotNil) c.Assert(ipn.Stopped, qt.Equals, *nn[0].State) @@ -599,7 +594,7 @@ func TestStateMachine(t *testing.T) { } // Let's make the logout succeed. - t.Logf("\n\nLogout (async) - succeed") + t.Logf("\n\nLogout - succeed") notifies.expect(3) cc.send(nil, "", false, nil) { @@ -617,15 +612,15 @@ func TestStateMachine(t *testing.T) { } // A second logout should reset all prefs. - t.Logf("\n\nLogout2 (async)") + t.Logf("\n\nLogout2") notifies.expect(1) - b.Logout() + b.LogoutSync(context.Background()) { nn := notifies.drain(1) c.Assert(nn[0].Prefs, qt.IsNotNil) // emptyPrefs // BUG: the backend has already called StartLogout, and we're // still logged out. So it shouldn't call it again. - cc.assertCalls("StartLogout") + cc.assertCalls("Logout") cc.assertCalls() c.Assert(b.Prefs().LoggedOut(), qt.IsTrue) c.Assert(b.Prefs().WantRunning(), qt.IsFalse) @@ -645,7 +640,7 @@ func TestStateMachine(t *testing.T) { } // Try the synchronous logout feature. - t.Logf("\n\nLogout3 (sync)") + t.Logf("\n\nLogout (sync)") notifies.expect(0) b.LogoutSync(context.Background()) // NOTE: This returns as soon as cc.Logout() returns, which is okay diff --git a/tstest/log.go b/tstest/log.go index fe309d1df..86639357e 100644 --- a/tstest/log.go +++ b/tstest/log.go @@ -152,7 +152,7 @@ func WhileTestRunningLogger(t testing.TB) logger.Logf { mu sync.RWMutex done bool ) - + tlogf := logger.TestLogger(t) logger := func(format string, args ...any) { t.Helper() @@ -162,7 +162,7 @@ func WhileTestRunningLogger(t testing.TB) logger.Logf { if done { return } - t.Logf(format, args...) + tlogf(format, args...) } // t.Cleanup is run before the test is marked as done, so by acquiring