diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ae74290..6ab0abe2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ tags; any other tag is rejected, for new and re-registering nodes alike. See - Expiring or deleting a non-existent pre-auth key now returns an error instead of silently succeeding [#3324](https://github.com/juanfont/headscale/pull/3324) - Improve systemd service file hardening [#3341](https://github.com/juanfont/headscale/pull/3341) +- Fix `headscale users destroy`/`rename` reporting "multiple users match query" when no user matches; an ambiguous match now lists the matching users [#3476](https://github.com/juanfont/headscale/pull/3476) - Headscale now requires Go 1.27 to build ## 0.29.4 (2026-09-23) diff --git a/cmd/headscale/cli/users.go b/cmd/headscale/cli/users.go index 2edac555..5aef4b71 100644 --- a/cmd/headscale/cli/users.go +++ b/cmd/headscale/cli/users.go @@ -7,6 +7,7 @@ import ( "net/http" "net/url" "strconv" + "strings" clientv1 "github.com/juanfont/headscale/gen/client/v1" "github.com/juanfont/headscale/hscontrol/util" @@ -19,6 +20,8 @@ import ( var ( errFlagRequired = errors.New("--name or --identifier flag is required") errMultipleUsersMatch = errors.New("multiple users match query, specify an ID") + errUserNotFound = errors.New("no user matches query") + errInvalidIdentifier = errors.New("--identifier must be a positive user ID") ) func usernameAndIDFlag(cmd *cobra.Command) { @@ -31,6 +34,13 @@ func usernameAndIDFromFlag(cmd *cobra.Command) (uint64, string, error) { username, _ := cmd.Flags().GetString("name") identifier, _ := cmd.Flags().GetInt64("identifier") + + // An explicit zero or negative identifier never matches a user; the + // API treats id=0 as "no filter", which would list every user. + if cmd.Flags().Changed("identifier") && identifier <= 0 { + return 0, "", errInvalidIdentifier + } + if username == "" && identifier < 0 { return 0, "", errFlagRequired } @@ -74,11 +84,29 @@ func resolveSingleUser( } users := resp.JSON200.Users - if len(users) != 1 { - return "", nil, errMultipleUsersMatch + + switch len(users) { + case 0: + return "", nil, errUserNotFound + case 1: + return users[0].Id, &users[0], nil + default: + return "", nil, fmt.Errorf("%w: %s", errMultipleUsersMatch, describeUsers(users)) + } +} + +// describeUsers renders the users that matched an ambiguous query so the +// operator can pick one by ID. +func describeUsers(users []clientv1.User) string { + parts := make([]string, len(users)) + for i, user := range users { + parts[i] = fmt.Sprintf( + "id=%s name=%s email=%s provider=%s", + user.Id, user.Name, user.Email, user.Provider, + ) } - return users[0].Id, &users[0], nil + return strings.Join(parts, "; ") } func init() { @@ -162,7 +190,7 @@ var destroyUserCmd = &cobra.Command{ } if !confirmAction(cmd, fmt.Sprintf( - "Do you want to remove the user %q (%s) and any associated preauthkeys?", + "Do you want to remove the user %q (%s) and its pre-auth keys? Nodes owned by the user must be deleted or moved first.", user.Name, user.Id, )) { return printOutput(cmd, map[string]string{colResult: "User not destroyed"}, "User not destroyed") diff --git a/cmd/headscale/cli/users_test.go b/cmd/headscale/cli/users_test.go index f3d3a3a8..5ed0a7db 100644 --- a/cmd/headscale/cli/users_test.go +++ b/cmd/headscale/cli/users_test.go @@ -3,8 +3,10 @@ package cli import ( "context" "encoding/json" + "errors" "net/http" "net/http/httptest" + "strings" "testing" clientv1 "github.com/juanfont/headscale/gen/client/v1" @@ -81,6 +83,9 @@ func TestResolveSingleUser(t *testing.T) { flagName string wantId string wantErr bool + wantErrIs error + // wantErrHas are substrings the error message must contain. + wantErrHas []string }{ { // Regression: renaming by name used to return the raw flag @@ -97,17 +102,40 @@ func TestResolveSingleUser(t *testing.T) { wantId: "9", }, { - name: "no match is an error", - users: []clientv1.User{lukas}, - flagName: "nobody@example.com", - wantErr: true, + // Regression: zero matches used to be reported as + // "multiple users match query". + name: "no match is a not-found error", + users: []clientv1.User{lukas}, + flagName: "nobody@example.com", + wantErr: true, + wantErrIs: errUserNotFound, }, { // OIDC users can share a name, see issue #3429. - name: "multiple matches are an error", - users: []clientv1.User{hannes, hannesDup}, - flagName: "hannes@rueger.events", - wantErr: true, + name: "multiple matches are an ambiguity error listing the matches", + users: []clientv1.User{hannes, hannesDup}, + flagName: "hannes@rueger.events", + wantErr: true, + wantErrIs: errMultipleUsersMatch, + wantErrHas: []string{ + "id=9 name=hannes@rueger.events email=hannes@rueger.events", + "id=10 name=hannes@rueger.events email=other@example.com", + }, + }, + { + // Regression: --identifier 0 was sent to the API as "no + // filter", listing every user and failing as ambiguous. + name: "identifier zero is rejected before calling the API", + users: []clientv1.User{lukas, hannes}, + identifier: "0", + wantErr: true, + wantErrIs: errInvalidIdentifier, + }, + { + name: "no flags is a usage error", + users: []clientv1.User{lukas}, + wantErr: true, + wantErrIs: errFlagRequired, }, } @@ -129,6 +157,16 @@ func TestResolveSingleUser(t *testing.T) { t.Fatalf("resolveSingleUser() error = nil, want error") } + if tt.wantErrIs != nil && !errors.Is(err, tt.wantErrIs) { + t.Fatalf("resolveSingleUser() error = %v, want %v", err, tt.wantErrIs) + } + + for _, want := range tt.wantErrHas { + if !strings.Contains(err.Error(), want) { + t.Errorf("resolveSingleUser() error = %q, want it to contain %q", err, want) + } + } + return }