diff --git a/cmd/headscale/cli/users.go b/cmd/headscale/cli/users.go index 5a0b87e16..2edac5556 100644 --- a/cmd/headscale/cli/users.go +++ b/cmd/headscale/cli/users.go @@ -43,15 +43,15 @@ func usernameAndIDFromFlag(cmd *cobra.Command) (uint64, string, error) { } // resolveSingleUser resolves exactly one user from the --name/--id flags, -// returning the raw flag id and the matched user. +// returning the identifier of the matched user and the user itself. func resolveSingleUser( ctx context.Context, client *clientv1.ClientWithResponses, cmd *cobra.Command, -) (uint64, *clientv1.User, error) { +) (string, *clientv1.User, error) { id, username, err := usernameAndIDFromFlag(cmd) if err != nil { - return 0, nil, err + return "", nil, err } params := &clientv1.ListUsersParams{} @@ -66,19 +66,19 @@ func resolveSingleUser( resp, err := client.ListUsersWithResponse(ctx, params) if err != nil { - return 0, nil, fmt.Errorf("listing users: %w", err) + return "", nil, fmt.Errorf("listing users: %w", err) } if resp.StatusCode() != http.StatusOK { - return 0, nil, apiError(resp.StatusCode(), resp.ApplicationproblemJSONDefault) + return "", nil, apiError(resp.StatusCode(), resp.ApplicationproblemJSONDefault) } users := resp.JSON200.Users if len(users) != 1 { - return 0, nil, errMultipleUsersMatch + return "", nil, errMultipleUsersMatch } - return id, &users[0], nil + return users[0].Id, &users[0], nil } func init() { @@ -239,14 +239,14 @@ var renameUserCmd = &cobra.Command{ Short: "Renames a user", Aliases: []string{"mv"}, RunE: clientRunE(func(ctx context.Context, client *clientv1.ClientWithResponses, cmd *cobra.Command, args []string) error { - id, _, err := resolveSingleUser(ctx, client, cmd) + userId, _, err := resolveSingleUser(ctx, client, cmd) if err != nil { return err } newName, _ := cmd.Flags().GetString("new-name") - resp, err := client.RenameUserWithResponse(ctx, strconv.FormatUint(id, util.Base10), newName) + resp, err := client.RenameUserWithResponse(ctx, userId, newName) if err != nil { return fmt.Errorf("renaming user: %w", err) } diff --git a/cmd/headscale/cli/users_test.go b/cmd/headscale/cli/users_test.go new file mode 100644 index 000000000..54b2467e6 --- /dev/null +++ b/cmd/headscale/cli/users_test.go @@ -0,0 +1,140 @@ +package cli + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + clientv1 "github.com/juanfont/headscale/gen/client/v1" + "github.com/spf13/cobra" +) + +// filterUsersServer serves GET /api/v1/user responses filtered like the +// real API: by the id, name, and email query parameters. +func filterUsersServer(t *testing.T, users []clientv1.User) *httptest.Server { + t.Helper() + + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + query := r.URL.Query() + + filtered := make([]clientv1.User, 0, len(users)) + for _, user := range users { + if name := query.Get("name"); name != "" && user.Name != name { + continue + } + if id := query.Get("id"); id != "" && user.Id != id { + continue + } + if email := query.Get("email"); email != "" && user.Email != email { + continue + } + filtered = append(filtered, user) + } + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + if err := json.NewEncoder(w).Encode(clientv1.ListUsersOutputBody{Users: filtered}); err != nil { + t.Errorf("encoding response: %v", err) + } + })) +} + +func commandWithUserFlags(t *testing.T, identifier, name string) *cobra.Command { + t.Helper() + + cmd := &cobra.Command{} + usernameAndIDFlag(cmd) + + if identifier != "" { + if err := cmd.Flags().Set("identifier", identifier); err != nil { + t.Fatalf("setting identifier flag: %v", err) + } + } + + if name != "" { + if err := cmd.Flags().Set("name", name); err != nil { + t.Fatalf("setting name flag: %v", err) + } + } + + return cmd +} + +func TestResolveSingleUser(t *testing.T) { + lukas := clientv1.User{Id: "6", Name: "lukas", Email: "login@lukasrunge.de"} + hannes := clientv1.User{Id: "9", Name: "hannes@rueger.events", Email: "hannes@rueger.events"} + hannesDup := clientv1.User{Id: "10", Name: "hannes@rueger.events", Email: "other@example.com"} + + tests := []struct { + name string + users []clientv1.User + identifier string + flagName string + wantId string + wantErr bool + }{ + { + // Regression: renaming by name used to return the raw flag + // id (0 when unset), so the rename request targeted user 0. + name: "resolves by name to the matched user's identifier", + users: []clientv1.User{lukas, hannes}, + flagName: "hannes@rueger.events", + wantId: "9", + }, + { + name: "resolves by identifier", + users: []clientv1.User{hannes}, + identifier: "9", + wantId: "9", + }, + { + name: "no match is an error", + users: []clientv1.User{lukas}, + flagName: "nobody@example.com", + wantErr: true, + }, + { + // 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, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + server := filterUsersServer(t, tt.users) + defer server.Close() + + client, err := clientv1.NewClientWithResponses(server.URL) + if err != nil { + t.Fatalf("creating client: %v", err) + } + + cmd := commandWithUserFlags(t, tt.identifier, tt.flagName) + + id, user, err := resolveSingleUser(context.Background(), client, cmd) + if tt.wantErr { + if err == nil { + t.Fatalf("resolveSingleUser() error = nil, want error") + } + return + } + + if err != nil { + t.Fatalf("resolveSingleUser() error = %v", err) + } + + if id != tt.wantId { + t.Errorf("resolveSingleUser() id = %q, want %q", id, tt.wantId) + } + + if user.Id != tt.wantId { + t.Errorf("resolveSingleUser() user.Id = %q, want %q", user.Id, tt.wantId) + } + }) + } +}