WIP

WIP

WIP

WIP

WIP

WIP

tbr

tbr

tbr

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.

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{}
 	}

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

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
+			name:            "up-with-force-reauth-after-login",
-		// and running, this completes immediately, before we've had a chance to show
+			args:            []string{"up", "--force-reauth"},
-		// the user the auth URL.
+			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"},