changelog 0.25.1

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>

.github/workflows/test-integration.yaml

@@ -24,6 +24,7 @@ jobs:
           - TestPolicyUpdateWhileRunningWithCLIInDatabase
           - TestAuthKeyLogoutAndReloginSameUser
           - TestAuthKeyLogoutAndReloginNewUser
+          - TestAuthKeyLogoutAndReloginSameUserExpiredKey
           - TestOIDCAuthenticationPingAll
           - TestOIDCExpireNodesBasedOnTokenExpiry
           - TestOIDC024UserCreation
@@ -68,6 +69,7 @@ jobs:
           - TestEnableDisableAutoApprovedRoute
           - TestAutoApprovedSubRoute2068
           - TestSubnetRouteACL
+          - TestEnablingExitRoutes
           - TestHeadscale
           - TestCreateTailscale
           - TestTailscaleNodesJoiningHeadcale

CHANGELOG.md

@@ -1,9 +1,17 @@
 # CHANGELOG
 
-## Next
+## 0.25.1 (2025-02-25)
 
+### Changes
 
-## 0.25.0 (2025-02-xx)
+- Fix issue where registration errors are sent correctly
+  [#2435](https://github.com/juanfont/headscale/pull/2435)
+- Fix issue where routes passed on registration were not saved
+  [#2444](https://github.com/juanfont/headscale/pull/2444)
+- Fix issue where registration page was displayed twice
+  [#2445](https://github.com/juanfont/headscale/pull/2445)
+
+## 0.25.0 (2025-02-11)
 
 ### BREAKING
 

hscontrol/auth.go

@@ -43,10 +43,7 @@ func (h *Headscale) handleRegister(
 	}
 
 	if regReq.Followup != "" {
-		// TODO(kradalby): Does this need to return an error of some sort?
-		// Maybe if the registration fails down the line it can be sent
-		// on the channel and returned here?
-		h.waitForFollowup(ctx, regReq)
+		return h.waitForFollowup(ctx, regReq)
 	}
 
 	if regReq.Auth != nil && regReq.Auth.AuthKey != "" {
@@ -117,42 +114,51 @@ func (h *Headscale) handleExistingNode(
 		h.nodeNotifier.NotifyWithIgnore(ctx, types.StateUpdateExpire(node.ID, requestExpiry), node.ID)
 	}
 
+	return nodeToRegisterResponse(node), nil
+}
+
+func nodeToRegisterResponse(node *types.Node) *tailcfg.RegisterResponse {
 	return &tailcfg.RegisterResponse{
 		// TODO(kradalby): Only send for user-owned nodes
 		// and not tagged nodes when tags is working.
 		User:           *node.User.TailscaleUser(),
 		Login:          *node.User.TailscaleLogin(),
-		NodeKeyExpired: expired,
+		NodeKeyExpired: node.IsExpired(),
 
 		// Headscale does not implement the concept of machine authorization
 		// so we always return true here.
 		// Revisit this if #2176 gets implemented.
 		MachineAuthorized: true,
-	}, nil
+	}
 }
 
 func (h *Headscale) waitForFollowup(
 	ctx context.Context,
 	regReq tailcfg.RegisterRequest,
-) {
+) (*tailcfg.RegisterResponse, error) {
 	fu, err := url.Parse(regReq.Followup)
 	if err != nil {
-		return
+		return nil, NewHTTPError(http.StatusUnauthorized, "invalid followup URL", err)
 	}
 
 	followupReg, err := types.RegistrationIDFromString(strings.ReplaceAll(fu.Path, "/register/", ""))
 	if err != nil {
-		return
+		return nil, NewHTTPError(http.StatusUnauthorized, "invalid registration ID", err)
 	}
 
 	if reg, ok := h.registrationCache.Get(followupReg); ok {
 		select {
 		case <-ctx.Done():
-			return
-		case <-reg.Registered:
-			return
+			return nil, NewHTTPError(http.StatusUnauthorized, "registration timed out", err)
+		case node := <-reg.Registered:
+			if node == nil {
+				return nil, NewHTTPError(http.StatusUnauthorized, "node not found", nil)
+			}
+			return nodeToRegisterResponse(node), nil
 		}
 	}
+
+	return nil, NewHTTPError(http.StatusNotFound, "followup registration not found", nil)
 }
 
 // canUsePreAuthKey checks if a pre auth key can be used.
@@ -277,7 +283,7 @@ func (h *Headscale) handleRegisterInteractive(
 			Hostinfo:   regReq.Hostinfo,
 			LastSeen:   ptr.To(time.Now()),
 		},
-		Registered: make(chan struct{}),
+		Registered: make(chan *types.Node),
 	}
 
 	if !regReq.Expiry.IsZero() {

hscontrol/db/node.go

@@ -371,7 +371,12 @@ func (hsdb *HSDatabase) HandleNodeFromAuthPath(
 				}
 
 				// Signal to waiting clients that the machine has been registered.
+				select {
+				case reg.Registered <- node:
+				default:
+				}
 				close(reg.Registered)
+
 				newNode = true
 				return node, err
 			} else {
@@ -452,6 +457,10 @@ func RegisterNode(tx *gorm.DB, node types.Node, ipv4 *netip.Addr, ipv6 *netip.Ad
 		return nil, fmt.Errorf("failed register(save) node in the database: %w", err)
 	}
 
+	if _, err := SaveNodeRoutes(tx, &node); err != nil {
+		return nil, fmt.Errorf("failed to save node routes: %w", err)
+	}
+
 	log.Trace().
 		Caller().
 		Str("node", node.Hostname).

hscontrol/db/node_test.go

@@ -744,6 +744,7 @@ func TestRenameNode(t *testing.T) {
 		Hostname:       "test",
 		UserID:         user.ID,
 		RegisterMethod: util.RegisterMethodAuthKey,
+		Hostinfo:       &tailcfg.Hostinfo{},
 	}
 
 	node2 := types.Node{
@@ -753,6 +754,7 @@ func TestRenameNode(t *testing.T) {
 		Hostname:       "test",
 		UserID:         user2.ID,
 		RegisterMethod: util.RegisterMethodAuthKey,
+		Hostinfo:       &tailcfg.Hostinfo{},
 	}
 
 	err = db.DB.Save(&node).Error

hscontrol/grpcv1.go

@@ -866,7 +866,7 @@ func (api headscaleV1APIServer) DebugCreateNode(
 
 			Hostinfo: &hostinfo,
 		},
-		Registered: make(chan struct{}),
+		Registered: make(chan *types.Node),
 	}
 
 	log.Debug().

hscontrol/noise.go

@@ -226,6 +226,10 @@ func (ns *noiseServer) NoisePollNetMapHandler(
 	}
 }
 
+func regErr(err error) *tailcfg.RegisterResponse {
+	return &tailcfg.RegisterResponse{Error: err.Error()}
+}
+
 // NoiseRegistrationHandler handles the actual registration process of a node.
 func (ns *noiseServer) NoiseRegistrationHandler(
 	writer http.ResponseWriter,
@@ -237,52 +241,47 @@ func (ns *noiseServer) NoiseRegistrationHandler(
 		return
 	}
 
-	registerRequest, registerResponse, err := func() (*tailcfg.RegisterRequest, []byte, error) {
+	registerRequest, registerResponse := func() (*tailcfg.RegisterRequest, *tailcfg.RegisterResponse) {
+		var resp *tailcfg.RegisterResponse
 		body, err := io.ReadAll(req.Body)
 		if err != nil {
-			return nil, nil, err
+			return &tailcfg.RegisterRequest{}, regErr(err)
 		}
-		var registerRequest tailcfg.RegisterRequest
-		if err := json.Unmarshal(body, &registerRequest); err != nil {
-			return nil, nil, err
+		var regReq tailcfg.RegisterRequest
+		if err := json.Unmarshal(body, &regReq); err != nil {
+			return &regReq, regErr(err)
 		}
 
-		ns.nodeKey = registerRequest.NodeKey
+		ns.nodeKey = regReq.NodeKey
 
-		resp, err := ns.headscale.handleRegister(req.Context(), registerRequest, ns.conn.Peer())
-		// TODO(kradalby): Here we could have two error types, one that is surfaced to the client
-		// and one that returns 500.
+		resp, err = ns.headscale.handleRegister(req.Context(), regReq, ns.conn.Peer())
 		if err != nil {
-			return nil, nil, err
+			var httpErr HTTPError
+			if errors.As(err, &httpErr) {
+				resp = &tailcfg.RegisterResponse{
+					Error: httpErr.Msg,
+				}
+				return &regReq, resp
+			} else {
+			}
+			return &regReq, regErr(err)
 		}
 
-		respBody, err := json.Marshal(resp)
-		if err != nil {
-			return nil, nil, err
-		}
-
-		return &registerRequest, respBody, nil
+		return &regReq, resp
 	}()
-	if err != nil {
-		log.Error().
-			Caller().
-			Err(err).
-			Msg("Error handling registration")
-		http.Error(writer, "Internal server error", http.StatusInternalServerError)
-	}
 
 	// Reject unsupported versions
 	if rejectUnsupported(writer, registerRequest.Version, ns.machineKey, registerRequest.NodeKey) {
 		return
 	}
 
+	respBody, err := json.Marshal(registerResponse)
+	if err != nil {
+		httpError(writer, err)
+		return
+	}
+
 	writer.Header().Set("Content-Type", "application/json; charset=utf-8")
 	writer.WriteHeader(http.StatusOK)
-	_, err = writer.Write(registerResponse)
-	if err != nil {
-		log.Error().
-			Caller().
-			Err(err).
-			Msg("Failed to write response")
-	}
+	writer.Write(respBody)
 }

hscontrol/types/common.go

@@ -174,5 +174,5 @@ func (r RegistrationID) String() string {
 
 type RegisterNode struct {
 	Node       Node
-	Registered chan struct{}
+	Registered chan *Node
 }

hscontrol/util/util.go

@@ -1,6 +1,13 @@
 package util
 
-import "tailscale.com/util/cmpver"
+import (
+	"errors"
+	"fmt"
+	"net/url"
+	"strings"
+
+	"tailscale.com/util/cmpver"
+)
 
 func TailscaleVersionNewerOrEqual(minimum, toCheck string) bool {
 	if cmpver.Compare(minimum, toCheck) <= 0 ||
@@ -11,3 +18,31 @@ func TailscaleVersionNewerOrEqual(minimum, toCheck string) bool {
 
 	return false
 }
+
+// ParseLoginURLFromCLILogin parses the output of the tailscale up command to extract the login URL.
+// It returns an error if not exactly one URL is found.
+func ParseLoginURLFromCLILogin(output string) (*url.URL, error) {
+	lines := strings.Split(output, "\n")
+	var urlStr string
+
+	for _, line := range lines {
+		line = strings.TrimSpace(line)
+		if strings.HasPrefix(line, "http://") || strings.HasPrefix(line, "https://") {
+			if urlStr != "" {
+				return nil, fmt.Errorf("multiple URLs found: %s and %s", urlStr, line)
+			}
+			urlStr = line
+		}
+	}
+
+	if urlStr == "" {
+		return nil, errors.New("no URL found")
+	}
+
+	loginURL, err := url.Parse(urlStr)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse URL: %w", err)
+	}
+
+	return loginURL, nil
+}

hscontrol/util/util_test.go

@@ -93,3 +93,88 @@ func TestTailscaleVersionNewerOrEqual(t *testing.T) {
 		})
 	}
 }
+
+func TestParseLoginURLFromCLILogin(t *testing.T) {
+	tests := []struct {
+		name    string
+		output  string
+		wantURL string
+		wantErr string
+	}{
+		{
+			name: "valid https URL",
+			output: `
+To authenticate, visit:
+
+        https://headscale.example.com/register/3oYCOZYA2zZmGB4PQ7aHBaMi
+
+Success.`,
+			wantURL: "https://headscale.example.com/register/3oYCOZYA2zZmGB4PQ7aHBaMi",
+			wantErr: "",
+		},
+		{
+			name: "valid http URL",
+			output: `
+To authenticate, visit:
+
+        http://headscale.example.com/register/3oYCOZYA2zZmGB4PQ7aHBaMi
+
+Success.`,
+			wantURL: "http://headscale.example.com/register/3oYCOZYA2zZmGB4PQ7aHBaMi",
+			wantErr: "",
+		},
+		{
+			name: "no URL",
+			output: `
+To authenticate, visit:
+
+Success.`,
+			wantURL: "",
+			wantErr: "no URL found",
+		},
+		{
+			name: "multiple URLs",
+			output: `
+To authenticate, visit:
+
+        https://headscale.example.com/register/3oYCOZYA2zZmGB4PQ7aHBaMi
+
+To authenticate, visit:
+
+        http://headscale.example.com/register/dv1l2k5FackOYl-7-V3mSd_E
+
+Success.`,
+			wantURL: "",
+			wantErr: "multiple URLs found: https://headscale.example.com/register/3oYCOZYA2zZmGB4PQ7aHBaMi and http://headscale.example.com/register/dv1l2k5FackOYl-7-V3mSd_E",
+		},
+		{
+			name: "invalid URL",
+			output: `
+To authenticate, visit:
+
+        invalid-url
+
+Success.`,
+			wantURL: "",
+			wantErr: "no URL found",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotURL, err := ParseLoginURLFromCLILogin(tt.output)
+			if tt.wantErr != "" {
+				if err == nil || err.Error() != tt.wantErr {
+					t.Errorf("ParseLoginURLFromCLILogin() error = %v, wantErr %v", err, tt.wantErr)
+				}
+			} else {
+				if err != nil {
+					t.Errorf("ParseLoginURLFromCLILogin() error = %v, wantErr %v", err, tt.wantErr)
+				}
+				if gotURL.String() != tt.wantURL {
+					t.Errorf("ParseLoginURLFromCLILogin() = %v, want %v", gotURL, tt.wantURL)
+				}
+			}
+		})
+	}
+}

integration/auth_key_test.go

@@ -228,3 +228,99 @@ func TestAuthKeyLogoutAndReloginNewUser(t *testing.T) {
 		assert.Equal(t, "user1@test.no", status.User[status.Self.UserID].LoginName)
 	}
 }
+
+func TestAuthKeyLogoutAndReloginSameUserExpiredKey(t *testing.T) {
+	IntegrationSkip(t)
+	t.Parallel()
+
+	for _, https := range []bool{true, false} {
+		t.Run(fmt.Sprintf("with-https-%t", https), func(t *testing.T) {
+			scenario, err := NewScenario(dockertestMaxWait())
+			assertNoErr(t, err)
+			defer scenario.ShutdownAssertNoPanics(t)
+
+			spec := map[string]int{
+				"user1": len(MustTestVersions),
+				"user2": len(MustTestVersions),
+			}
+
+			opts := []hsic.Option{hsic.WithTestName("pingallbyip")}
+			if https {
+				opts = append(opts, []hsic.Option{
+					hsic.WithTLS(),
+				}...)
+			}
+
+			err = scenario.CreateHeadscaleEnv(spec, []tsic.Option{}, opts...)
+			assertNoErrHeadscaleEnv(t, err)
+
+			allClients, err := scenario.ListTailscaleClients()
+			assertNoErrListClients(t, err)
+
+			err = scenario.WaitForTailscaleSync()
+			assertNoErrSync(t, err)
+
+			// assertClientsState(t, allClients)
+
+			clientIPs := make(map[TailscaleClient][]netip.Addr)
+			for _, client := range allClients {
+				ips, err := client.IPs()
+				if err != nil {
+					t.Fatalf("failed to get IPs for client %s: %s", client.Hostname(), err)
+				}
+				clientIPs[client] = ips
+			}
+
+			headscale, err := scenario.Headscale()
+			assertNoErrGetHeadscale(t, err)
+
+			listNodes, err := headscale.ListNodes()
+			assert.Equal(t, len(listNodes), len(allClients))
+			nodeCountBeforeLogout := len(listNodes)
+			t.Logf("node count before logout: %d", nodeCountBeforeLogout)
+
+			for _, client := range allClients {
+				err := client.Logout()
+				if err != nil {
+					t.Fatalf("failed to logout client %s: %s", client.Hostname(), err)
+				}
+			}
+
+			err = scenario.WaitForTailscaleLogout()
+			assertNoErrLogout(t, err)
+
+			t.Logf("all clients logged out")
+
+			// if the server is not running with HTTPS, we have to wait a bit before
+			// reconnection as the newest Tailscale client has a measure that will only
+			// reconnect over HTTPS if they saw a noise connection previously.
+			// https://github.com/tailscale/tailscale/commit/1eaad7d3deb0815e8932e913ca1a862afa34db38
+			// https://github.com/juanfont/headscale/issues/2164
+			if !https {
+				time.Sleep(5 * time.Minute)
+			}
+
+			for userName := range spec {
+				key, err := scenario.CreatePreAuthKey(userName, true, false)
+				if err != nil {
+					t.Fatalf("failed to create pre-auth key for user %s: %s", userName, err)
+				}
+
+				// Expire the key so it can't be used
+				_, err = headscale.Execute(
+					[]string{
+						"headscale",
+						"preauthkeys",
+						"--user",
+						userName,
+						"expire",
+						key.Key,
+					})
+				assertNoErr(t, err)
+
+				err = scenario.RunTailscaleUp(userName, headscale.GetEndpoint(), key.GetKey())
+				assert.ErrorContains(t, err, "authkey expired")
+			}
+		})
+	}
+}

integration/route_test.go

@@ -17,6 +17,8 @@ import (
 	"github.com/juanfont/headscale/integration/hsic"
 	"github.com/juanfont/headscale/integration/tsic"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"tailscale.com/net/tsaddr"
 	"tailscale.com/types/ipproto"
 	"tailscale.com/types/views"
 	"tailscale.com/wgengine/filter"
@@ -1316,3 +1318,123 @@ func TestSubnetRouteACL(t *testing.T) {
 		t.Errorf("Subnet (%s) filter, unexpected result (-want +got):\n%s", subRouter1.Hostname(), diff)
 	}
 }
+
+// TestEnablingExitRoutes tests enabling exit routes for clients.
+// Its more or less the same as TestEnablingRoutes, but with the --advertise-exit-node flag
+// set during login instead of set.
+func TestEnablingExitRoutes(t *testing.T) {
+	IntegrationSkip(t)
+	t.Parallel()
+
+	user := "user2"
+
+	scenario, err := NewScenario(dockertestMaxWait())
+	assertNoErrf(t, "failed to create scenario: %s", err)
+	defer scenario.ShutdownAssertNoPanics(t)
+
+	spec := map[string]int{
+		user: 2,
+	}
+
+	err = scenario.CreateHeadscaleEnv(spec, []tsic.Option{
+		tsic.WithExtraLoginArgs([]string{"--advertise-exit-node"}),
+	}, hsic.WithTestName("clienableroute"))
+	assertNoErrHeadscaleEnv(t, err)
+
+	allClients, err := scenario.ListTailscaleClients()
+	assertNoErrListClients(t, err)
+
+	err = scenario.WaitForTailscaleSync()
+	assertNoErrSync(t, err)
+
+	headscale, err := scenario.Headscale()
+	assertNoErrGetHeadscale(t, err)
+
+	err = scenario.WaitForTailscaleSync()
+	assertNoErrSync(t, err)
+
+	var routes []*v1.Route
+	err = executeAndUnmarshal(
+		headscale,
+		[]string{
+			"headscale",
+			"routes",
+			"list",
+			"--output",
+			"json",
+		},
+		&routes,
+	)
+
+	assertNoErr(t, err)
+	assert.Len(t, routes, 4)
+
+	for _, route := range routes {
+		assert.True(t, route.GetAdvertised())
+		assert.False(t, route.GetEnabled())
+		assert.False(t, route.GetIsPrimary())
+	}
+
+	// Verify that no routes has been sent to the client,
+	// they are not yet enabled.
+	for _, client := range allClients {
+		status, err := client.Status()
+		assertNoErr(t, err)
+
+		for _, peerKey := range status.Peers() {
+			peerStatus := status.Peer[peerKey]
+
+			assert.Nil(t, peerStatus.PrimaryRoutes)
+		}
+	}
+
+	// Enable all routes
+	for _, route := range routes {
+		_, err = headscale.Execute(
+			[]string{
+				"headscale",
+				"routes",
+				"enable",
+				"--route",
+				strconv.Itoa(int(route.GetId())),
+			})
+		assertNoErr(t, err)
+	}
+
+	var enablingRoutes []*v1.Route
+	err = executeAndUnmarshal(
+		headscale,
+		[]string{
+			"headscale",
+			"routes",
+			"list",
+			"--output",
+			"json",
+		},
+		&enablingRoutes,
+	)
+	assertNoErr(t, err)
+	assert.Len(t, enablingRoutes, 4)
+
+	for _, route := range enablingRoutes {
+		assert.True(t, route.GetAdvertised())
+		assert.True(t, route.GetEnabled())
+	}
+
+	time.Sleep(5 * time.Second)
+
+	// Verify that the clients can see the new routes
+	for _, client := range allClients {
+		status, err := client.Status()
+		assertNoErr(t, err)
+
+		for _, peerKey := range status.Peers() {
+			peerStatus := status.Peer[peerKey]
+
+			require.NotNil(t, peerStatus.AllowedIPs)
+			assert.Len(t, peerStatus.AllowedIPs.AsSlice(), 4)
+			assert.Contains(t, peerStatus.AllowedIPs.AsSlice(), tsaddr.AllIPv4())
+			assert.Contains(t, peerStatus.AllowedIPs.AsSlice(), tsaddr.AllIPv6())
+		}
+	}
+}

integration/tsic/tsic.go

@@ -80,6 +80,7 @@ type TailscaleInContainer struct {
 	withExtraHosts    []string
 	workdir           string
 	netfilter         string
+	extraLoginArgs    []string
 
 	// build options, solely for HEAD
 	buildConfig TailscaleInContainerBuildConfig
@@ -203,6 +204,14 @@ func WithBuildTag(tag string) Option {
 	}
 }
 
+// WithExtraLoginArgs adds additional arguments to the `tailscale up` command
+// as part of the Login function.
+func WithExtraLoginArgs(args []string) Option {
+	return func(tsic *TailscaleInContainer) {
+		tsic.extraLoginArgs = args
+	}
+}
+
 // New returns a new TailscaleInContainer instance.
 func New(
 	pool *dockertest.Pool,
@@ -436,6 +445,10 @@ func (t *TailscaleInContainer) Login(
 		"--accept-routes=false",
 	}
 
+	if t.extraLoginArgs != nil {
+		command = append(command, t.extraLoginArgs...)
+	}
+
 	if t.withSSH {
 		command = append(command, "--ssh")
 	}
@@ -475,6 +488,10 @@ func (t *TailscaleInContainer) LoginWithURL(
 		"--accept-routes=false",
 	}
 
+	if t.extraLoginArgs != nil {
+		command = append(command, t.extraLoginArgs...)
+	}
+
 	stdout, stderr, err := t.Execute(command)
 	if errors.Is(err, errTailscaleNotLoggedIn) {
 		return nil, errTailscaleCannotUpWithoutAuthkey
@@ -486,15 +503,7 @@ func (t *TailscaleInContainer) LoginWithURL(
 		}
 	}()
 
-	urlStr := strings.ReplaceAll(stdout+stderr, "\nTo authenticate, visit:\n\n\t", "")
-	urlStr = strings.TrimSpace(urlStr)
-
-	if urlStr == "" {
-		return nil, fmt.Errorf("failed to get login URL: stdout: %s, stderr: %s", stdout, stderr)
-	}
-
-	// parse URL
-	loginURL, err = url.Parse(urlStr)
+	loginURL, err = util.ParseLoginURLFromCLILogin(stdout + stderr)
 	if err != nil {
 		return nil, err
 	}