From ee6949c2d99b1390adde9e316375c729bb82f057 Mon Sep 17 00:00:00 2001 From: Alex Chan Date: Mon, 15 Sep 2025 11:44:00 +0100 Subject: [PATCH] tstest/integration: expand the tests for `tailscale up` Expand the integration tests to cover a wider range of scenarios, including: * Auth URLs and auth keys * With and without the `--force-reauth` flag * Before and after a successful initial login These tests expose a race condition when using `--force-reauth` on an already-logged in device. The command completes too quickly, preventing the auth URL from being displayed. This issue is identified and will be fixed in the next commit. Fixes #17108 --- tstest/integration/integration_test.go | 185 ++++++++++++++---- tstest/integration/testcontrol/testcontrol.go | 17 ++ 2 files changed, 164 insertions(+), 38 deletions(-) diff --git a/tstest/integration/integration_test.go b/tstest/integration/integration_test.go index de464108c..841ceceaf 100644 --- a/tstest/integration/integration_test.go +++ b/tstest/integration/integration_test.go @@ -23,6 +23,7 @@ import ( "regexp" "runtime" "strconv" + "strings" "sync/atomic" "testing" "time" @@ -263,52 +264,160 @@ func TestStateSavedOnStart(t *testing.T) { } func TestOneNodeUpAuth(t *testing.T) { - tstest.Shard(t) - tstest.Parallel(t) - env := NewTestEnv(t, ConfigureControl(func(control *testcontrol.Server) { - control.RequireAuth = true - })) - - n1 := NewTestNode(t, env) - d1 := n1.StartDaemon() + for _, tt := range []struct { + name string + args []string + // + // What auth key should we use for control? + authKey string + // + // Is tailscaled already logged in before we run this `up` command? + alreadyLoggedIn bool + // + // Do we need to log in again with an /auth/ URL? + needsNewLogin bool + }{ + { + name: "up", + args: []string{"up"}, + needsNewLogin: true, + }, + { + name: "up-with-force-reauth", + args: []string{"up", "--force-reauth"}, + needsNewLogin: true, + }, + { + name: "up-with-auth-key", + args: []string{"up", "--auth-key=opensesame"}, + authKey: "opensesame", + needsNewLogin: false, + }, + { + name: "up-with-force-reauth-and-auth-key", + args: []string{"up", "--force-reauth", "--auth-key=opensesame"}, + authKey: "opensesame", + needsNewLogin: false, + }, + { + name: "up-after-login", + args: []string{"up"}, + 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-auth-key-after-login", + args: []string{"up", "--auth-key=opensesame"}, + authKey: "opensesame", + alreadyLoggedIn: true, + needsNewLogin: false, + }, + { + name: "up-with-force-reauth-and-auth-key-after-login", + args: []string{"up", "--force-reauth", "--auth-key=opensesame"}, + authKey: "opensesame", + alreadyLoggedIn: true, + needsNewLogin: false, + }, + } { + t.Run(tt.name, func(t *testing.T) { + tstest.Shard(t) + tstest.Parallel(t) + + env := NewTestEnv(t, ConfigureControl( + func(control *testcontrol.Server) { + if tt.authKey != "" { + control.RequireAuthKey = tt.authKey + } else { + control.RequireAuth = true + } - n1.AwaitListening() + control.AllNodesSameUser = true + }, + )) + + n1 := NewTestNode(t, env) + d1 := n1.StartDaemon() + defer d1.MustCleanShutdown(t) + + cmdArgs := append(tt.args, "--login-server="+env.ControlURL()) + + // This handler looks for /auth/ URLs in the stdout from "tailscale up", + // and it it sees them, completes the auth process. + // + // It counts how many auth URLs it's seen. + var authCountAtomic atomic.Int32 + authURLHandler := &authURLParserWriter{fn: func(urlStr string) error { + t.Logf("saw auth URL %q", urlStr) + if env.Control.CompleteAuth(urlStr) { + if authCountAtomic.Add(1) > 1 { + err := errors.New("completed multiple auth URLs") + t.Error(err) + return err + } + t.Logf("completed login to %s", urlStr) + return nil + } else { + err := fmt.Errorf("Failed to complete initial login to %q", urlStr) + t.Fatal(err) + return err + } + }} + + // If we should be logged in at the start of the test case, go ahead + // and run the login command. + // + // Otherwise, just wait for tailscaled to be listening. + if tt.alreadyLoggedIn { + t.Logf("Running initial login: %s", strings.Join(cmdArgs, " ")) + cmd := n1.Tailscale(cmdArgs...) + cmd.Stdout = authURLHandler + cmd.Stderr = cmd.Stdout + if err := cmd.Run(); err != nil { + t.Fatalf("up: %v", err) + } + authCountAtomic.Store(0) + n1.AwaitRunning() + } else { + n1.AwaitListening() + } - st := n1.MustStatus() - t.Logf("Status: %s", st.BackendState) + st := n1.MustStatus() + t.Logf("Status: %s", st.BackendState) - t.Logf("Running up --login-server=%s ...", env.ControlURL()) + t.Logf("Running command: %s", strings.Join(cmdArgs, " ")) + cmd := n1.Tailscale(cmdArgs...) + cmd.Stdout = authURLHandler + cmd.Stderr = cmd.Stdout - cmd := n1.Tailscale("up", "--login-server="+env.ControlURL()) - var authCountAtomic atomic.Int32 - cmd.Stdout = &authURLParserWriter{fn: func(urlStr string) error { - t.Logf("saw auth URL %q", urlStr) - if env.Control.CompleteAuth(urlStr) { - if authCountAtomic.Add(1) > 1 { - err := errors.New("completed multple auth URLs") - t.Error(err) - return err + if err := cmd.Run(); err != nil { + t.Fatalf("up: %v", err) } - t.Logf("completed auth path %s", urlStr) - return nil - } - err := fmt.Errorf("Failed to complete auth path to %q", urlStr) - t.Error(err) - return err - }} - cmd.Stderr = cmd.Stdout - if err := cmd.Run(); err != nil { - t.Fatalf("up: %v", err) - } - t.Logf("Got IP: %v", n1.AwaitIP4()) + t.Logf("Got IP: %v", n1.AwaitIP4()) - n1.AwaitRunning() + cmd.Wait() - if n := authCountAtomic.Load(); n != 1 { - t.Errorf("Auth URLs completed = %d; want 1", n) - } + n1.AwaitRunning() - d1.MustCleanShutdown(t) + var expectedAuthUrls int32 + if tt.needsNewLogin { + expectedAuthUrls = 1 + } + if n := authCountAtomic.Load(); n != expectedAuthUrls { + t.Errorf("Auth URLs completed = %d; want %d", n, expectedAuthUrls) + } + }) + } } func TestConfigFileAuthKey(t *testing.T) { diff --git a/tstest/integration/testcontrol/testcontrol.go b/tstest/integration/testcontrol/testcontrol.go index 2fbf37de9..0f90f8dc1 100644 --- a/tstest/integration/testcontrol/testcontrol.go +++ b/tstest/integration/testcontrol/testcontrol.go @@ -644,6 +644,23 @@ func (s *Server) serveRegister(w http.ResponseWriter, r *http.Request, mkey key. // some follow-ups? For now all are successes. } + // The in-memory list of nodes, users, and logins is keyed by + // the node key. If the node key changes, update all the data stores + // to use the new node key. + // if _, oldNodeKeyOk := s.nodes[req.OldNodeKey]; oldNodeKeyOk { + // if _, newNodeKeyOk := s.nodes[req.NodeKey]; !newNodeKeyOk { + // s.nodes[req.OldNodeKey].Key = req.NodeKey + // s.nodes[req.NodeKey] = s.nodes[req.OldNodeKey] + + // s.users[req.NodeKey] = s.users[req.OldNodeKey] + // s.logins[req.NodeKey] = s.logins[req.OldNodeKey] + + // delete(s.nodes, req.OldNodeKey) + // delete(s.users, req.OldNodeKey) + // delete(s.logins, req.OldNodeKey) + // } + // } + nk := req.NodeKey user, login := s.getUser(nk)