fix: use static rbac method for getting site roles instead of API call

brettkolodny · brettkolodny · commit a2b349df5b38 · 2025-04-15T20:13:56.000Z
diff --git a/cli/usereditroles.go b/cli/usereditroles.go
@@ -1,20 +1,30 @@
 package cli
 
 import (
+	"fmt"
 	"slices"
 	"sort"
 	"strings"
 
 	"golang.org/x/xerrors"
 
 	"github.com/coder/coder/v2/cli/cliui"
+	"github.com/coder/coder/v2/coderd/rbac"
 	"github.com/coder/coder/v2/codersdk"
 	"github.com/coder/serpent"
 )
 
 func (r *RootCmd) userEditRoles() *serpent.Command {
 	client := new(codersdk.Client)
 
+	roles := rbac.SiteRoles()
+
+	siteRoles := make([]string, 0)
+	for _, role := range roles {
+		siteRoles = append(siteRoles, role.Identifier.Name)
+	}
+	sort.Strings(siteRoles)
+
 	var givenRoles []string
 
 	cmd := &serpent.Command{
@@ -24,7 +34,7 @@ func (r *RootCmd) userEditRoles() *serpent.Command {
 			cliui.SkipPromptOption(),
 			{
 				Name:        "roles",
-				Description: "A list of roles to give to the user. This removes any existing roles the user may have.",
+				Description: fmt.Sprintf("A list of roles to give to the user. This removes any existing roles the user may have. The available roles are: %s.", strings.Join(siteRoles, ", ")),
 				Flag:        "roles",
 				Value:       serpent.StringArrayOf(&givenRoles)},
 		},
@@ -37,42 +47,22 @@ func (r *RootCmd) userEditRoles() *serpent.Command {
 				return xerrors.Errorf("fetch user: %w", err)
 			}
 
-			roles, err := client.ListSiteRoles(ctx)
-			if err != nil {
-				return xerrors.Errorf("fetch site roles: %w", err)
-			}
-
-			siteRoles := make([]string, 0)
-			for _, role := range roles {
-				if role.Assignable {
-					siteRoles = append(siteRoles, role.Name)
-				}
-			}
-			sort.Strings(siteRoles)
-
 			userRoles, err := client.UserRoles(ctx, user.Username)
 			if err != nil {
 				return xerrors.Errorf("fetch user roles: %w", err)
 			}
 
 			var selectedRoles []string
 			if len(givenRoles) > 0 {
-				// If the none role is present ignore all other roles.
-				// This is so there is a way to clear roles from the CLI without making a
-				// new command.
-				if slices.Contains(givenRoles, "none") {
-					selectedRoles = []string{}
-				} else {
-					// Make sure all of the given roles are valid site roles
-					for _, givenRole := range givenRoles {
-						if !slices.Contains(siteRoles, givenRole) {
-							siteRolesPretty := strings.Join(siteRoles, ", ")
-							return xerrors.Errorf("The role %s is not valid. Please use one or more of the following roles: %s, or none\n", givenRole, siteRolesPretty)
-						}
+				// Make sure all of the given roles are valid site roles
+				for _, givenRole := range givenRoles {
+					if !slices.Contains(siteRoles, givenRole) {
+						siteRolesPretty := strings.Join(siteRoles, ", ")
+						return xerrors.Errorf("The role %s is not valid. Please use one or more of the following roles: %s\n", givenRole, siteRolesPretty)
 					}
-
-					selectedRoles = givenRoles
 				}
+
+				selectedRoles = givenRoles
 			} else {
 				selectedRoles, err = cliui.MultiSelect(inv, cliui.MultiSelectOptions{
 					Message:  "Select the roles you'd like to assign to the user",
diff --git a/cli/usereditroles_test.go b/cli/usereditroles_test.go
@@ -0,0 +1,61 @@
+package cli_test
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/coder/coder/v2/cli/clitest"
+	"github.com/coder/coder/v2/coderd/coderdtest"
+	"github.com/coder/coder/v2/coderd/rbac"
+	"github.com/coder/coder/v2/testutil"
+)
+
+var roles = []string{"auditor", "user-admin"}
+
+func TestUserEditRoles(t *testing.T) {
+	t.Parallel()
+
+	t.Run("UpdateUserRoles", func(t *testing.T) {
+		t.Parallel()
+
+		client := coderdtest.New(t, nil)
+		owner := coderdtest.CreateFirstUser(t, client)
+		_, member := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.RoleMember())
+
+		inv, root := clitest.New(t, "users", "edit-roles", member.Username, fmt.Sprintf("--roles=%s", strings.Join(roles, ",")))
+		clitest.SetupConfig(t, client, root)
+
+		// Create context with timeout
+		ctx := testutil.Context(t, testutil.WaitShort)
+
+		err := inv.WithContext(ctx).Run()
+		require.NoError(t, err)
+
+		memberRoles, err := client.UserRoles(ctx, member.Username)
+		require.NoError(t, err)
+
+		require.ElementsMatch(t, memberRoles.Roles, roles)
+	})
+
+	t.Run("UserNotFound", func(t *testing.T) {
+		t.Parallel()
+
+		client := coderdtest.New(t, nil)
+		owner := coderdtest.CreateFirstUser(t, client)
+		userAdmin, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.RoleUserAdmin())
+
+		// Setup command with non-existent user
+		inv, root := clitest.New(t, "users", "edit-roles", "nonexistentuser")
+		clitest.SetupConfig(t, userAdmin, root)
+
+		// Create context with timeout
+		ctx := testutil.Context(t, testutil.WaitShort)
+
+		err := inv.WithContext(ctx).Run()
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "fetch user")
+	})
+}
