From 66fa7221b6ed7fcf1534bb607951a8b2e692c205 Mon Sep 17 00:00:00 2001 From: Alex Chan Date: Mon, 15 Sep 2025 23:20:38 +0100 Subject: [PATCH] WIP WIP WIP WIP WIP WIP tbr tbr tbr --- cmd/tailscale/cli/up.go | 48 ++++++++++++++++++++------ ipn/ipnlocal/local.go | 3 +- ipn/ipnstate/ipnstate.go | 4 +++ tstest/integration/integration_test.go | 16 ++++----- 4 files changed, 48 insertions(+), 23 deletions(-) diff --git a/cmd/tailscale/cli/up.go b/cmd/tailscale/cli/up.go index 097af725b..e0a2ad9af 100644 --- a/cmd/tailscale/cli/up.go +++ b/cmd/tailscale/cli/up.go @@ -446,6 +446,7 @@ func runUp(ctx context.Context, cmd string, args []string, upArgs upArgsT) (retE return fixTailscaledConnectError(err) } origAuthURL := st.AuthURL + origPublicNodeKey := st.PublicNodeKey // printAuthURL reports whether we should print out the // provided auth URL from an IPN notify. @@ -540,9 +541,17 @@ func runUp(ctx context.Context, cmd string, args []string, upArgs upArgsT) (retE } }() - running := make(chan bool, 1) // gets value once in state ipn.Running + upComplete := make(chan bool, 1) watchErr := make(chan error, 1) + // Start watching the IPN bus before we call Start() or StartLoginInteractive(), + // or we could miss IPN status messages. + watcher, err := localClient.WatchIPNBus(watchCtx, ipn.NotifyInitialState) + if err != nil { + return err + } + defer watcher.Close() + // Special case: bare "tailscale up" means to just start // running, if there's ever been a login. if simpleUp { @@ -583,12 +592,6 @@ func runUp(ctx context.Context, cmd string, args []string, upArgs upArgsT) (retE } } - watcher, err := localClient.WatchIPNBus(watchCtx, ipn.NotifyInitialState) - if err != nil { - return err - } - defer watcher.Close() - go func() { var printed bool // whether we've yet printed anything to stdout or stderr var lastURLPrinted string @@ -613,6 +616,18 @@ func runUp(ctx context.Context, cmd string, args []string, upArgs upArgsT) (retE fmt.Fprintf(Stderr, "\nTo approve your machine, visit (as admin):\n\n\t%s\n\n", prefs.AdminPageURL(policyclient.Get())) } case ipn.Running: + // If we've entered the running state and we're doing a force reauth, + // check if the node key has changed -- this tells us the user has + // completed a new auth process. + // + // TODO(alexc): it would be nice if we could get the public key + // in the ipn.Notify message rather than fetching the status + // again, but the key change happens very deep inside ipn and + // it wasn't obvious how we could emit the "key change" event. + if upArgs.forceReauth && !hasNodeKeyChanged(ctx, origPublicNodeKey) { + continue + } + // Done full authentication process if env.upArgs.json { printUpDoneJSON(ipn.Running, "") @@ -621,7 +636,7 @@ func runUp(ctx context.Context, cmd string, args []string, upArgs upArgsT) (retE fmt.Fprintf(Stderr, "Success.\n") } select { - case running <- true: + case upComplete <- true: default: } cancelWatch() @@ -680,18 +695,18 @@ func runUp(ctx context.Context, cmd string, args []string, upArgs upArgsT) (retE timeoutCh = timeoutTimer.C } select { - case <-running: + case <-upComplete: return nil case <-watchCtx.Done(): select { - case <-running: + case <-upComplete: return nil default: } return watchCtx.Err() case err := <-watchErr: select { - case <-running: + case <-upComplete: return nil default: } @@ -701,6 +716,17 @@ func runUp(ctx context.Context, cmd string, args []string, upArgs upArgsT) (retE } } +// hasNodeKeyChanged reports if the node key has changed since the initial state. +// we got from ipn. +func hasNodeKeyChanged(ctx context.Context, origPublicNodeKey string) bool { + st, err := localClient.Status(ctx) + if err != nil { + return false + } + + return st.PublicNodeKey != origPublicNodeKey +} + // upWorthWarning reports whether the health check message s is worth warning // about during "tailscale up". Many of the health checks are noisy or confusing // or very ephemeral and happen especially briefly at startup. diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 6108aa830..6ba59f87b 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -1217,6 +1217,7 @@ func (b *LocalBackend) UpdateStatus(sb *ipnstate.StatusBuilder) { } s.Health = b.health.Strings() s.HaveNodeKey = b.hasNodeKeyLocked() + s.PublicNodeKey = b.NodeKey().String() // TODO(bradfitz): move this health check into a health.Warnable // and remove from here. @@ -5789,8 +5790,6 @@ func (b *LocalBackend) hasNodeKeyLocked() bool { // NodeKey returns the public node key. func (b *LocalBackend) NodeKey() key.NodePublic { - b.mu.Lock() - defer b.mu.Unlock() if !b.hasNodeKeyLocked() { return key.NodePublic{} } diff --git a/ipn/ipnstate/ipnstate.go b/ipn/ipnstate/ipnstate.go index e7ae2d62b..bbb617cab 100644 --- a/ipn/ipnstate/ipnstate.go +++ b/ipn/ipnstate/ipnstate.go @@ -45,6 +45,10 @@ type Status struct { // HaveNodeKey is whether the current profile has a node key configured. HaveNodeKey bool `json:",omitempty"` + // PublicNodeKey contains a string representation of the public node key, + // if configured + PublicNodeKey string `json:",omitempty"` + AuthURL string // current URL provided by control to authorize client TailscaleIPs []netip.Addr // Tailscale IP(s) assigned to this node Self *PeerStatus diff --git a/tstest/integration/integration_test.go b/tstest/integration/integration_test.go index 841ceceaf..2d3b88427 100644 --- a/tstest/integration/integration_test.go +++ b/tstest/integration/integration_test.go @@ -305,16 +305,12 @@ func TestOneNodeUpAuth(t *testing.T) { alreadyLoggedIn: true, needsNewLogin: false, }, - // TODO(alexc): This test is failing because of a bug in `tailscale up` where - // it waits for ipn to enter the "Running" state. If we're already logged in - // and running, this completes immediately, before we've had a chance to show - // the user the auth URL. - // { - // name: "up-with-force-reauth-after-login", - // args: []string{"up", "--force-reauth"}, - // alreadyLoggedIn: true, - // needsNewLogin: true, - // }, + { + name: "up-with-force-reauth-after-login", + args: []string{"up", "--force-reauth"}, + alreadyLoggedIn: true, + needsNewLogin: true, + }, { name: "up-with-auth-key-after-login", args: []string{"up", "--auth-key=opensesame"},