refactor: Return the display_name and name in the roles endpoint (#1328)

coderd/rbac/builtin.go

@@ -51,7 +51,8 @@ var (
 		// admin grants all actions to all resources.
 		admin: func(_ string) Role {
 			return Role{
-				Name: admin,
+				Name:        admin,
+				DisplayName: "Admin",
 				Site: permissions(map[Object][]Action{
 					ResourceWildcard: {WildcardSymbol},
 				}),
@@ -61,7 +62,8 @@ var (
 		// member grants all actions to all resources owned by the user
 		member: func(_ string) Role {
 			return Role{
-				Name: member,
+				Name:        member,
+				DisplayName: "Member",
 				User: permissions(map[Object][]Action{
 					ResourceWildcard: {WildcardSymbol},
 				}),
@@ -73,7 +75,8 @@ var (
 		// TODO: Finish the auditor as we add resources.
 		auditor: func(_ string) Role {
 			return Role{
-				Name: "auditor",
+				Name:        "auditor",
+				DisplayName: "Auditor",
 				Site: permissions(map[Object][]Action{
 					// Should be able to read all template details, even in orgs they
 					// are not in.
@@ -86,7 +89,8 @@ var (
 		// organization scope.
 		orgAdmin: func(organizationID string) Role {
 			return Role{
-				Name: roleName(orgAdmin, organizationID),
+				Name:        roleName(orgAdmin, organizationID),
+				DisplayName: "Organization Admin",
 				Org: map[string][]Permission{
 					organizationID: {
 						{
@@ -104,7 +108,8 @@ var (
 		// in an organization.
 		orgMember: func(organizationID string) Role {
 			return Role{
-				Name: roleName(orgMember, organizationID),
+				Name:        roleName(orgMember, organizationID),
+				DisplayName: "Organization Member",
 				Org: map[string][]Permission{
 					organizationID: {},
 				},
@@ -151,11 +156,11 @@ func IsOrgRole(roleName string) (string, bool) {
 //
 // This should be a list in a database, but until then we build
 // the list from the builtins.
-func OrganizationRoles(organizationID uuid.UUID) []string {
+func OrganizationRoles(organizationID uuid.UUID) []Role {
-	var roles []string
+	var roles []Role
 	for _, roleF := range builtInRoles {
-		role := roleF(organizationID.String()).Name
+		role := roleF(organizationID.String())
-		_, scope, err := roleSplit(role)
+		_, scope, err := roleSplit(role.Name)
 		if err != nil {
 			// This should never happen
 			continue
@@ -172,8 +177,8 @@ func OrganizationRoles(organizationID uuid.UUID) []string {
 //
 // This should be a list in a database, but until then we build
 // the list from the builtins.
-func SiteRoles() []string {
+func SiteRoles() []Role {
-	var roles []string
+	var roles []Role
 	for _, roleF := range builtInRoles {
 		role := roleF("random")
 		_, scope, err := roleSplit(role.Name)
@@ -182,7 +187,7 @@ func SiteRoles() []string {
 			continue
 		}
 		if scope == "" {
-			roles = append(roles, role.Name)
+			roles = append(roles, role)
 		}
 	}
 	return roles

coderd/rbac/builtin_test.go

@@ -65,6 +65,12 @@ func TestIsOrgRole(t *testing.T) {
 func TestListRoles(t *testing.T) {
 	t.Parallel()
 
+	siteRoles := rbac.SiteRoles()
+	siteRoleNames := make([]string, 0, len(siteRoles))
+	for _, role := range siteRoles {
+		siteRoleNames = append(siteRoleNames, role.Name)
+	}
+
 	// If this test is ever failing, just update the list to the roles
 	// expected from the builtin set.
 	require.ElementsMatch(t, []string{
@@ -72,12 +78,18 @@ func TestListRoles(t *testing.T) {
 		"member",
 		"auditor",
 	},
-		rbac.SiteRoles())
+		siteRoleNames)
 
 	orgID := uuid.New()
+	orgRoles := rbac.OrganizationRoles(orgID)
+	orgRoleNames := make([]string, 0, len(orgRoles))
+	for _, role := range orgRoles {
+		orgRoleNames = append(orgRoleNames, role.Name)
+	}
+
 	require.ElementsMatch(t, []string{
 		fmt.Sprintf("organization-admin:%s", orgID.String()),
 		fmt.Sprintf("organization-member:%s", orgID.String()),
 	},
-		rbac.OrganizationRoles(orgID))
+		orgRoleNames)
 }

coderd/rbac/role.go

@@ -17,8 +17,9 @@ type Permission struct {
 // Users of this package should instead **only** use the role names, and
 // this package will expand the role names into their json payloads.
 type Role struct {
-	Name string       `json:"name"`
+	Name        string       `json:"name"`
-	Site []Permission `json:"site"`
+	DisplayName string       `json:"display_name"`
+	Site        []Permission `json:"site"`
 	// Org is a map of orgid to permissions. We represent orgid as a string.
 	// We scope the organizations in the role so we can easily combine all the
 	// roles.

coderd/roles.go

@@ -4,6 +4,7 @@ import (
 	"net/http"
 
 	"github.com/coder/coder/coderd/httpmw"
+	"github.com/coder/coder/codersdk"
 
 	"github.com/coder/coder/coderd/httpapi"
 	"github.com/coder/coder/coderd/rbac"
@@ -14,7 +15,7 @@ func (*api) assignableSiteRoles(rw http.ResponseWriter, _ *http.Request) {
 	// TODO: @emyrk in the future, allow granular subsets of roles to be returned based on the
 	// 	role of the user.
 	roles := rbac.SiteRoles()
-	httpapi.Write(rw, http.StatusOK, roles)
+	httpapi.Write(rw, http.StatusOK, codersdk.ConvertRoles(roles))
 }
 
 // assignableSiteRoles returns all site wide roles that can be assigned.
@@ -23,5 +24,5 @@ func (*api) assignableOrgRoles(rw http.ResponseWriter, r *http.Request) {
 	// 	role of the user.
 	organization := httpmw.OrganizationParam(r)
 	roles := rbac.OrganizationRoles(organization.ID)
-	httpapi.Write(rw, http.StatusOK, roles)
+	httpapi.Write(rw, http.StatusOK, codersdk.ConvertRoles(roles))
 }

coderd/roles_test.go

@@ -45,13 +45,13 @@ func TestListRoles(t *testing.T) {
 	testCases := []struct {
 		Name            string
 		Client          *codersdk.Client
-		APICall         func() ([]string, error)
+		APICall         func() ([]codersdk.Role, error)
-		ExpectedRoles   []string
+		ExpectedRoles   []codersdk.Role
 		AuthorizedError string
 	}{
 		{
 			Name: "MemberListSite",
-			APICall: func() ([]string, error) {
+			APICall: func() ([]codersdk.Role, error) {
 				x, err := member.ListSiteRoles(ctx)
 				return x, err
 			},
@@ -59,14 +59,14 @@ func TestListRoles(t *testing.T) {
 		},
 		{
 			Name: "OrgMemberListOrg",
-			APICall: func() ([]string, error) {
+			APICall: func() ([]codersdk.Role, error) {
 				return member.ListOrganizationRoles(ctx, admin.OrganizationID)
 			},
 			AuthorizedError: unauth,
 		},
 		{
 			Name: "NonOrgMemberListOrg",
-			APICall: func() ([]string, error) {
+			APICall: func() ([]codersdk.Role, error) {
 				return member.ListOrganizationRoles(ctx, otherOrg.ID)
 			},
 			AuthorizedError: notMember,
@@ -74,21 +74,21 @@ func TestListRoles(t *testing.T) {
 		// Org admin
 		{
 			Name: "OrgAdminListSite",
-			APICall: func() ([]string, error) {
+			APICall: func() ([]codersdk.Role, error) {
 				return orgAdmin.ListSiteRoles(ctx)
 			},
 			AuthorizedError: unauth,
 		},
 		{
 			Name: "OrgAdminListOrg",
-			APICall: func() ([]string, error) {
+			APICall: func() ([]codersdk.Role, error) {
 				return orgAdmin.ListOrganizationRoles(ctx, admin.OrganizationID)
 			},
-			ExpectedRoles: rbac.OrganizationRoles(admin.OrganizationID),
+			ExpectedRoles: codersdk.ConvertRoles(rbac.OrganizationRoles(admin.OrganizationID)),
 		},
 		{
 			Name: "OrgAdminListOtherOrg",
-			APICall: func() ([]string, error) {
+			APICall: func() ([]codersdk.Role, error) {
 				return orgAdmin.ListOrganizationRoles(ctx, otherOrg.ID)
 			},
 			AuthorizedError: notMember,
@@ -96,17 +96,17 @@ func TestListRoles(t *testing.T) {
 		// Admin
 		{
 			Name: "AdminListSite",
-			APICall: func() ([]string, error) {
+			APICall: func() ([]codersdk.Role, error) {
 				return client.ListSiteRoles(ctx)
 			},
-			ExpectedRoles: rbac.SiteRoles(),
+			ExpectedRoles: codersdk.ConvertRoles(rbac.SiteRoles()),
 		},
 		{
 			Name: "AdminListOrg",
-			APICall: func() ([]string, error) {
+			APICall: func() ([]codersdk.Role, error) {
 				return client.ListOrganizationRoles(ctx, admin.OrganizationID)
 			},
-			ExpectedRoles: rbac.OrganizationRoles(admin.OrganizationID),
+			ExpectedRoles: codersdk.ConvertRoles(rbac.OrganizationRoles(admin.OrganizationID)),
 		},
 	}
 

codersdk/roles.go

@@ -6,12 +6,18 @@ import (
 	"fmt"
 	"net/http"
 
+	"github.com/coder/coder/coderd/rbac"
 	"github.com/google/uuid"
 )
 
+type Role struct {
+	Name        string `json:"name"`
+	DisplayName string `json:"display_name"`
+}
+
 // ListSiteRoles lists all available site wide roles.
 // This is not user specific.
-func (c *Client) ListSiteRoles(ctx context.Context) ([]string, error) {
+func (c *Client) ListSiteRoles(ctx context.Context) ([]Role, error) {
 	res, err := c.request(ctx, http.MethodGet, "/api/v2/users/roles", nil)
 	if err != nil {
 		return nil, err
@@ -20,13 +26,13 @@ func (c *Client) ListSiteRoles(ctx context.Context) ([]string, error) {
 	if res.StatusCode != http.StatusOK {
 		return nil, readBodyAsError(res)
 	}
-	var roles []string
+	var roles []Role
 	return roles, json.NewDecoder(res.Body).Decode(&roles)
 }
 
 // ListOrganizationRoles lists all available roles for a given organization.
 // This is not user specific.
-func (c *Client) ListOrganizationRoles(ctx context.Context, org uuid.UUID) ([]string, error) {
+func (c *Client) ListOrganizationRoles(ctx context.Context, org uuid.UUID) ([]Role, error) {
 	res, err := c.request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/organizations/%s/members/roles/", org.String()), nil)
 	if err != nil {
 		return nil, err
@@ -35,6 +41,17 @@ func (c *Client) ListOrganizationRoles(ctx context.Context, org uuid.UUID) ([]st
 	if res.StatusCode != http.StatusOK {
 		return nil, readBodyAsError(res)
 	}
-	var roles []string
+	var roles []Role
 	return roles, json.NewDecoder(res.Body).Decode(&roles)
 }
+
+func ConvertRoles(roles []rbac.Role) []Role {
+	converted := make([]Role, 0, len(roles))
+	for _, role := range roles {
+		converted = append(converted, Role{
+			DisplayName: role.DisplayName,
+			Name:        role.Name,
+		})
+	}
+	return converted
+}