diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d952c07..59963e03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ - View of config, policy, filter, ssh policy per node, connected nodes and DERPmap -## 0.25.1 (2025-02-24) +## 0.25.1 (2025-02-25) ### Changes @@ -22,6 +22,8 @@ [#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) diff --git a/hscontrol/auth.go b/hscontrol/auth.go index 4cc7058b..0a8602cd 100644 --- a/hscontrol/auth.go +++ b/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 != "" { @@ -111,42 +108,51 @@ func (h *Headscale) handleExistingNode( h.nodeNotifier.NotifyWithIgnore(ctx, types.UpdateExpire(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. @@ -271,7 +277,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() { diff --git a/hscontrol/db/node.go b/hscontrol/db/node.go index c9244095..74cd7a9f 100644 --- a/hscontrol/db/node.go +++ b/hscontrol/db/node.go @@ -372,7 +372,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 { diff --git a/hscontrol/grpcv1.go b/hscontrol/grpcv1.go index 59fe4ebd..7368083c 100644 --- a/hscontrol/grpcv1.go +++ b/hscontrol/grpcv1.go @@ -838,7 +838,7 @@ func (api headscaleV1APIServer) DebugCreateNode( Hostinfo: &hostinfo, }, - Registered: make(chan struct{}), + Registered: make(chan *types.Node), } log.Debug(). diff --git a/hscontrol/types/common.go b/hscontrol/types/common.go index c8d696af..c4cc8a2e 100644 --- a/hscontrol/types/common.go +++ b/hscontrol/types/common.go @@ -194,5 +194,5 @@ func (r RegistrationID) String() string { type RegisterNode struct { Node Node - Registered chan struct{} + Registered chan *Node } diff --git a/hscontrol/util/util.go b/hscontrol/util/util.go index 7cb7f453..569af354 100644 --- a/hscontrol/util/util.go +++ b/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 +} diff --git a/hscontrol/util/util_test.go b/hscontrol/util/util_test.go index 282b52e6..1e331fe2 100644 --- a/hscontrol/util/util_test.go +++ b/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) + } + } + }) + } +} diff --git a/integration/tsic/tsic.go b/integration/tsic/tsic.go index 964b2662..8bfd4f60 100644 --- a/integration/tsic/tsic.go +++ b/integration/tsic/tsic.go @@ -503,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 }