diff --git a/control/controlclient/auto.go b/control/controlclient/auto.go index e0f17e172..7e48a1e70 100644 --- a/control/controlclient/auto.go +++ b/control/controlclient/auto.go @@ -67,13 +67,17 @@ func (s State) String() string { type Status struct { _ structs.Incomparable - LoginFinished *empty.Message + LoginFinished *empty.Message // nonempty when login finishes Err string - URL string + URL string // interactive URL to visit to finish logging in Persist *persist.Persist // locally persisted configuration NetMap *netmap.NetworkMap // server-pushed configuration Hostinfo *tailcfg.Hostinfo // current Hostinfo data - State State + + // the internal state machine is not exposed outside this package. + // This field is here mainly for checking the flow during unit + // tests. + state State } // Equal reports whether s and s2 are equal. @@ -88,7 +92,7 @@ func (s *Status) Equal(s2 *Status) bool { reflect.DeepEqual(s.Persist, s2.Persist) && reflect.DeepEqual(s.NetMap, s2.NetMap) && reflect.DeepEqual(s.Hostinfo, s2.Hostinfo) && - s.State == s2.State + s.state == s2.state } func (s Status) String() string { @@ -96,7 +100,7 @@ func (s Status) String() string { if err != nil { panic(err) } - return s.State.String() + " " + string(b) + return s.state.String() + " " + string(b) } type LoginGoal struct { @@ -596,10 +600,15 @@ func (c *Client) mapRoutine() { } } +// AuthCantContinue() returns true if the auth state machine is ready to try +// logging in, but doesn't have enough information to do so. +// +// After freshly creating the controlclient, this is always true, because +// you haven't called Login() yet (even non-interactively). After you call +// Login(), this function will return false for a while. It may become true +// again after a Status{URL!=""} is emitted, indicating that you need +// to visit an interactive login URL. func (c *Client) AuthCantContinue() bool { - if c == nil { - return true - } c.mu.Lock() defer c.mu.Unlock() @@ -670,7 +679,7 @@ func (c *Client) sendStatus(who string, err error, url string, nm *netmap.Networ Persist: p, NetMap: nm, Hostinfo: hi, - State: state, + state: state, } if err != nil { new.Err = err.Error() @@ -706,6 +715,7 @@ func (c *Client) StartLogout() { wantLoggedIn: false, } c.mu.Unlock() + c.cancelAuth() } @@ -720,6 +730,7 @@ func (c *Client) Logout(ctx context.Context) error { loggedOutResult: errc, } c.mu.Unlock() + c.cancelAuth() timer := time.NewTimer(10 * time.Second) diff --git a/ipn/backend.go b/ipn/backend.go index b5aa78c2d..89356c30b 100644 --- a/ipn/backend.go +++ b/ipn/backend.go @@ -17,13 +17,13 @@ import ( type State int const ( - NoState = State(iota) - InUseOtherUser - NeedsLogin - NeedsMachineAuth - Stopped - Starting - Running + NoState = State(iota) + InUseOtherUser // backend is in use by another user + NeedsLogin // an *interactive* user login is required; URL available + NeedsMachineAuth // logged in, but machine key needs approval + Stopped // user requested WantRunning=false + Starting // in transition from stopped to running + Running // network is connected ) // GoogleIDToken Type is the tailcfg.Oauth2Token.TokenType for the Google diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index b04061b7f..503df874d 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -627,7 +627,6 @@ func (b *LocalBackend) Start(opts ipn.Options) error { return errors.New("no state key or prefs provided") } - defer b.stateMachine() if opts.Prefs != nil { b.logf("Start: %v", opts.Prefs.Pretty()) } else { @@ -786,9 +785,10 @@ func (b *LocalBackend) Start(opts ipn.Options) error { b.send(ipn.Notify{BackendLogID: &blid}) b.send(ipn.Notify{Prefs: prefs}) - if wantRunning { - cc.Login(nil, controlclient.LoginDefault) - } + // Even if we don't want to be *connected* to tailscale right now, + // we can attempt the non-interactive login process right away, + // which will make connecting fast later. + cc.Login(nil, controlclient.LoginDefault) return nil } @@ -2105,7 +2105,7 @@ func (b *LocalBackend) nextState() ipn.State { switch { case netMap == nil: - if cc.AuthCantContinue() { + if cc.AuthCantContinue() && b.authURL != "" { // Auth was interrupted or waiting for URL visit, // so it won't proceed without human help. return ipn.NeedsLogin diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index fa2f7cef2..733a7a066 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -5,20 +5,15 @@ package ipnlocal import ( - "bytes" - "fmt" "net/http" "reflect" - "sync" "testing" "inet.af/netaddr" - "tailscale.com/ipn" "tailscale.com/net/interfaces" "tailscale.com/net/tsaddr" "tailscale.com/tailcfg" "tailscale.com/types/netmap" - "tailscale.com/wgengine" "tailscale.com/wgengine/wgcfg" ) @@ -433,40 +428,3 @@ func (panicOnUseTransport) RoundTrip(*http.Request) (*http.Response, error) { } var nl = []byte("\n") - -func TestStartsInNeedsLoginState(t *testing.T) { - var ( - mu sync.Mutex - logBuf bytes.Buffer - ) - logf := func(format string, a ...interface{}) { - mu.Lock() - defer mu.Unlock() - fmt.Fprintf(&logBuf, format, a...) - if !bytes.HasSuffix(logBuf.Bytes(), nl) { - logBuf.Write(nl) - } - } - store := new(ipn.MemoryStore) - eng, err := wgengine.NewFakeUserspaceEngine(logf, 0) - if err != nil { - t.Fatalf("NewFakeUserspaceEngine: %v", err) - } - lb, err := NewLocalBackend(logf, "logid", store, eng) - if err != nil { - t.Fatalf("NewLocalBackend: %v", err) - } - - lb.SetHTTPTestClient(&http.Client{ - Transport: panicOnUseTransport{}, // validate we don't send HTTP requests - }) - - if err := lb.Start(ipn.Options{ - StateKey: ipn.GlobalDaemonStateKey, - }); err != nil { - t.Fatalf("Start: %v", err) - } - if st := lb.State(); st != ipn.NeedsLogin { - t.Errorf("State = %v; want NeedsLogin", st) - } -}