diff --git a/client/tailscale/localclient.go b/client/tailscale/localclient.go index a9e6edc28..5dad2d667 100644 --- a/client/tailscale/localclient.go +++ b/client/tailscale/localclient.go @@ -548,6 +548,11 @@ func (lc *LocalClient) Logout(ctx context.Context) error { return err } +func (lc *LocalClient) LogoutAsync(ctx context.Context) error { + _, err := lc.send(ctx, "POST", "/localapi/v0/logout?async=true", http.StatusNoContent, nil) + return err +} + // SetDNS adds a DNS TXT record for the given domain name, containing // the provided TXT value. The intended use case is answering // LetsEncrypt/ACME dns-01 challenges. diff --git a/cmd/tailscale/cli/logout.go b/cmd/tailscale/cli/logout.go index 0bce01fda..a46409b54 100644 --- a/cmd/tailscale/cli/logout.go +++ b/cmd/tailscale/cli/logout.go @@ -6,6 +6,7 @@ package cli import ( "context" + "flag" "fmt" "strings" @@ -23,11 +24,23 @@ the current node key, forcing a future use of it to cause a reauthentication. `), Exec: runLogout, + FlagSet: (func() *flag.FlagSet { + fs := newFlagSet("logout") + fs.BoolVar(&logoutArgs.async, "async", false, "Does not wait for logout to be complete (status can be queried to determine success)") + return fs + })(), +} + +var logoutArgs struct { + async bool } func runLogout(ctx context.Context, args []string) error { if len(args) > 0 { return fmt.Errorf("too many non-flag arguments: %q", args) } + if logoutArgs.async { + return localClient.LogoutAsync(ctx) + } return localClient.Logout(ctx) } diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index a407e7242..6c56d5ac4 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -810,7 +810,9 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) { if err := b.pm.DeleteProfile(b.pm.CurrentProfile().ID); err != nil { b.logf("error deleting profile: %v", err) } - b.resetForProfileChangeLockedOnEntry() + // Restart backend asychonously, so that we avoid reentrancy in the + // controclient (we're currently in a callback from it). + b.resetForProfileChangeLockedOnEntry(profileChangeStartAsync) return } @@ -2110,7 +2112,7 @@ func (b *LocalBackend) SetCurrentUserID(uid string) { b.mu.Unlock() return } - b.resetForProfileChangeLockedOnEntry() + b.resetForProfileChangeLockedOnEntry(profileChangeStartSync) } func (b *LocalBackend) CheckPrefs(p *ipn.Prefs) error { @@ -4045,16 +4047,32 @@ func (b *LocalBackend) SwitchProfile(profile ipn.ProfileID) error { b.mu.Unlock() return err } - return b.resetForProfileChangeLockedOnEntry() + return b.resetForProfileChangeLockedOnEntry(profileChangeStartSync) } +type profileChangeStartMode int + +const ( + profileChangeStartSync profileChangeStartMode = iota + profileChangeStartAsync +) + // resetForProfileChangeLockedOnEntry resets the backend for a profile change. -func (b *LocalBackend) resetForProfileChangeLockedOnEntry() error { +// It requires b.mu be held to call it, but it unlocks b.mu when done. +func (b *LocalBackend) resetForProfileChangeLockedOnEntry(startMode profileChangeStartMode) error { b.setNetMapLocked(nil) // Reset netmap. // Reset the NetworkMap in the engine b.e.SetNetworkMap(new(netmap.NetworkMap)) b.enterStateLockedOnEntry(ipn.NoState) // Reset state. - return b.Start(ipn.Options{}) + switch startMode { + case profileChangeStartSync: + return b.Start(ipn.Options{}) + case profileChangeStartAsync: + go b.Start(ipn.Options{}) + return nil + default: + panic("unreachable") + } } // DeleteProfile deletes a profile with the given ID. @@ -4072,7 +4090,7 @@ func (b *LocalBackend) DeleteProfile(p ipn.ProfileID) error { if !needToRestart { return nil } - return b.resetForProfileChangeLockedOnEntry() + return b.resetForProfileChangeLockedOnEntry(profileChangeStartSync) } // CurrentProfile returns the current LoginProfile. @@ -4087,7 +4105,7 @@ func (b *LocalBackend) CurrentProfile() ipn.LoginProfile { func (b *LocalBackend) NewProfile() error { b.mu.Lock() b.pm.NewProfile() - return b.resetForProfileChangeLockedOnEntry() + return b.resetForProfileChangeLockedOnEntry(profileChangeStartSync) } // ListProfiles returns a list of all LoginProfiles. diff --git a/ipn/localapi/localapi.go b/ipn/localapi/localapi.go index c04b716b5..f236a131f 100644 --- a/ipn/localapi/localapi.go +++ b/ipn/localapi/localapi.go @@ -552,7 +552,12 @@ func (h *Handler) serveLogout(w http.ResponseWriter, r *http.Request) { http.Error(w, "want POST", 400) return } - err := h.b.LogoutSync(r.Context()) + var err error + if defBool(r.FormValue("async"), false) { + h.b.Logout() + } else { + err = h.b.LogoutSync(r.Context()) + } if err == nil { w.WriteHeader(http.StatusNoContent) return diff --git a/tstest/integration/integration_test.go b/tstest/integration/integration_test.go index 504597586..ccb2d122a 100644 --- a/tstest/integration/integration_test.go +++ b/tstest/integration/integration_test.go @@ -555,6 +555,23 @@ func TestLogoutRemovesAllPeers(t *testing.T) { wantNode0PeerCount(expectedPeers) // all existing peers and the new node } +func TestLogoutAsyncState(t *testing.T) { + t.Parallel() + env := newTestEnv(t) + node := newTestNode(t, env) + node.StartDaemon() + node.AwaitResponding() + node.MustUp() + node.AwaitIP() + node.AwaitRunning() + + log.Printf("running logout CLI") + if err := node.Tailscale("logout", "--async").Run(); err != nil { + t.Fatalf("logout: %v", err) + } + node.AwaitNeedsLogin() +} + // testEnv contains the test environment (set of servers) used by one // or more nodes. type testEnv struct {