fix: Role assign ui fixes (#3521)

Co-authored-by: Kira Pilot <kira@coder.com>

coderd/rbac/builtin.go

@@ -123,7 +123,10 @@ var (
 				Name:        userAdmin,
 				DisplayName: "User Admin",
 				Site: permissions(map[Object][]Action{
-					ResourceUser: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
+					ResourceRoleAssignment: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
+					ResourceUser:           {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
+					// Full perms to manage org members
+					ResourceOrganizationMember: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
 				}),
 			}
 		},
@@ -196,6 +199,10 @@ var (
 			templateAdmin: true,
 			userAdmin:     true,
 		},
+		userAdmin: {
+			member:    true,
+			orgMember: true,
+		},
 		orgAdmin: {
 			orgAdmin:  true,
 			orgMember: true,

coderd/rbac/builtin_test.go

@@ -119,7 +119,7 @@ func TestRolePermissions(t *testing.T) {
 	memberMe := authSubject{Name: "member_me", UserID: currentUser.String(), Roles: []string{rbac.RoleMember()}}
 	orgMemberMe := authSubject{Name: "org_member_me", UserID: currentUser.String(), Roles: []string{rbac.RoleMember(), rbac.RoleOrgMember(orgID)}}
 
-	admin := authSubject{Name: "admin", UserID: adminID.String(), Roles: []string{rbac.RoleMember(), rbac.RoleOwner()}}
+	owner := authSubject{Name: "owner", UserID: adminID.String(), Roles: []string{rbac.RoleMember(), rbac.RoleOwner()}}
 	orgAdmin := authSubject{Name: "org_admin", UserID: adminID.String(), Roles: []string{rbac.RoleMember(), rbac.RoleOrgMember(orgID), rbac.RoleOrgAdmin(orgID)}}
 
 	otherOrgMember := authSubject{Name: "org_member_other", UserID: uuid.NewString(), Roles: []string{rbac.RoleMember(), rbac.RoleOrgMember(otherOrg)}}
@@ -130,7 +130,7 @@ func TestRolePermissions(t *testing.T) {
 
 	// requiredSubjects are required to be asserted in each test case. This is
 	// to make sure one is not forgotten.
-	requiredSubjects := []authSubject{memberMe, admin, orgMemberMe, orgAdmin, otherOrgAdmin, otherOrgMember, templateAdmin, userAdmin}
+	requiredSubjects := []authSubject{memberMe, owner, orgMemberMe, orgAdmin, otherOrgAdmin, otherOrgMember, templateAdmin, userAdmin}
 
 	testCases := []struct {
 		// Name the test case to better locate the failing test case.
@@ -150,7 +150,7 @@ func TestRolePermissions(t *testing.T) {
 			Actions:  []rbac.Action{rbac.ActionRead},
 			Resource: rbac.ResourceUser,
 			AuthorizeMap: map[bool][]authSubject{
-				true:  {admin, memberMe, orgMemberMe, orgAdmin, otherOrgMember, otherOrgAdmin, templateAdmin, userAdmin},
+				true:  {owner, memberMe, orgMemberMe, orgAdmin, otherOrgMember, otherOrgAdmin, templateAdmin, userAdmin},
 				false: {},
 			},
 		},
@@ -159,7 +159,7 @@ func TestRolePermissions(t *testing.T) {
 			Actions:  []rbac.Action{rbac.ActionCreate, rbac.ActionUpdate, rbac.ActionDelete},
 			Resource: rbac.ResourceUser,
 			AuthorizeMap: map[bool][]authSubject{
-				true:  {admin, userAdmin},
+				true:  {owner, userAdmin},
 				false: {memberMe, orgMemberMe, orgAdmin, otherOrgMember, otherOrgAdmin, templateAdmin},
 			},
 		},
@@ -169,7 +169,7 @@ func TestRolePermissions(t *testing.T) {
 			Actions:  []rbac.Action{rbac.ActionCreate, rbac.ActionRead, rbac.ActionUpdate, rbac.ActionDelete},
 			Resource: rbac.ResourceWorkspace.InOrg(orgID).WithOwner(currentUser.String()),
 			AuthorizeMap: map[bool][]authSubject{
-				true:  {admin, orgMemberMe, orgAdmin, templateAdmin},
+				true:  {owner, orgMemberMe, orgAdmin, templateAdmin},
 				false: {memberMe, otherOrgAdmin, otherOrgMember, userAdmin},
 			},
 		},
@@ -179,7 +179,7 @@ func TestRolePermissions(t *testing.T) {
 			Actions:  []rbac.Action{rbac.ActionCreate, rbac.ActionRead, rbac.ActionUpdate, rbac.ActionDelete},
 			Resource: rbac.ResourceWorkspaceExecution.InOrg(orgID).WithOwner(currentUser.String()),
 			AuthorizeMap: map[bool][]authSubject{
-				true:  {admin, orgAdmin, orgMemberMe},
+				true:  {owner, orgAdmin, orgMemberMe},
 				false: {memberMe, otherOrgAdmin, otherOrgMember, templateAdmin, userAdmin},
 			},
 		},
@@ -188,7 +188,7 @@ func TestRolePermissions(t *testing.T) {
 			Actions:  []rbac.Action{rbac.ActionCreate, rbac.ActionUpdate, rbac.ActionDelete},
 			Resource: rbac.ResourceTemplate.InOrg(orgID),
 			AuthorizeMap: map[bool][]authSubject{
-				true:  {admin, orgAdmin, templateAdmin},
+				true:  {owner, orgAdmin, templateAdmin},
 				false: {memberMe, orgMemberMe, otherOrgAdmin, otherOrgMember, userAdmin},
 			},
 		},
@@ -197,7 +197,7 @@ func TestRolePermissions(t *testing.T) {
 			Actions:  []rbac.Action{rbac.ActionRead},
 			Resource: rbac.ResourceTemplate.InOrg(orgID),
 			AuthorizeMap: map[bool][]authSubject{
-				true:  {admin, orgMemberMe, orgAdmin, templateAdmin},
+				true:  {owner, orgMemberMe, orgAdmin, templateAdmin},
 				false: {memberMe, otherOrgAdmin, otherOrgMember, userAdmin},
 			},
 		},
@@ -206,7 +206,7 @@ func TestRolePermissions(t *testing.T) {
 			Actions:  []rbac.Action{rbac.ActionCreate},
 			Resource: rbac.ResourceFile,
 			AuthorizeMap: map[bool][]authSubject{
-				true:  {admin, templateAdmin},
+				true:  {owner, templateAdmin},
 				false: {orgMemberMe, orgAdmin, memberMe, otherOrgAdmin, otherOrgMember, userAdmin},
 			},
 		},
@@ -215,7 +215,7 @@ func TestRolePermissions(t *testing.T) {
 			Actions:  []rbac.Action{rbac.ActionRead, rbac.ActionUpdate, rbac.ActionDelete},
 			Resource: rbac.ResourceFile.WithOwner(currentUser.String()),
 			AuthorizeMap: map[bool][]authSubject{
-				true:  {admin, memberMe, orgMemberMe, templateAdmin},
+				true:  {owner, memberMe, orgMemberMe, templateAdmin},
 				false: {orgAdmin, otherOrgAdmin, otherOrgMember, userAdmin},
 			},
 		},
@@ -224,7 +224,7 @@ func TestRolePermissions(t *testing.T) {
 			Actions:  []rbac.Action{rbac.ActionCreate},
 			Resource: rbac.ResourceOrganization,
 			AuthorizeMap: map[bool][]authSubject{
-				true:  {admin},
+				true:  {owner},
 				false: {orgAdmin, otherOrgAdmin, otherOrgMember, memberMe, orgMemberMe, templateAdmin, userAdmin},
 			},
 		},
@@ -233,7 +233,7 @@ func TestRolePermissions(t *testing.T) {
 			Actions:  []rbac.Action{rbac.ActionUpdate, rbac.ActionDelete},
 			Resource: rbac.ResourceOrganization.InOrg(orgID),
 			AuthorizeMap: map[bool][]authSubject{
-				true:  {admin, orgAdmin},
+				true:  {owner, orgAdmin},
 				false: {otherOrgAdmin, otherOrgMember, memberMe, orgMemberMe, templateAdmin, userAdmin},
 			},
 		},
@@ -242,7 +242,7 @@ func TestRolePermissions(t *testing.T) {
 			Actions:  []rbac.Action{rbac.ActionRead},
 			Resource: rbac.ResourceOrganization.InOrg(orgID),
 			AuthorizeMap: map[bool][]authSubject{
-				true:  {admin, orgAdmin, orgMemberMe},
+				true:  {owner, orgAdmin, orgMemberMe},
 				false: {otherOrgAdmin, otherOrgMember, memberMe, templateAdmin, userAdmin},
 			},
 		},
@@ -251,8 +251,8 @@ func TestRolePermissions(t *testing.T) {
 			Actions:  []rbac.Action{rbac.ActionCreate, rbac.ActionUpdate, rbac.ActionDelete},
 			Resource: rbac.ResourceRoleAssignment,
 			AuthorizeMap: map[bool][]authSubject{
-				true:  {admin},
-				false: {orgAdmin, orgMemberMe, otherOrgAdmin, otherOrgMember, memberMe, templateAdmin, userAdmin},
+				true:  {owner, userAdmin},
+				false: {orgAdmin, orgMemberMe, otherOrgAdmin, otherOrgMember, memberMe, templateAdmin},
 			},
 		},
 		{
@@ -260,7 +260,7 @@ func TestRolePermissions(t *testing.T) {
 			Actions:  []rbac.Action{rbac.ActionRead},
 			Resource: rbac.ResourceRoleAssignment,
 			AuthorizeMap: map[bool][]authSubject{
-				true:  {admin, orgAdmin, orgMemberMe, otherOrgAdmin, otherOrgMember, memberMe, templateAdmin, userAdmin},
+				true:  {owner, orgAdmin, orgMemberMe, otherOrgAdmin, otherOrgMember, memberMe, templateAdmin, userAdmin},
 				false: {},
 			},
 		},
@@ -269,7 +269,7 @@ func TestRolePermissions(t *testing.T) {
 			Actions:  []rbac.Action{rbac.ActionCreate, rbac.ActionUpdate, rbac.ActionDelete},
 			Resource: rbac.ResourceOrgRoleAssignment.InOrg(orgID),
 			AuthorizeMap: map[bool][]authSubject{
-				true:  {admin, orgAdmin},
+				true:  {owner, orgAdmin},
 				false: {orgMemberMe, otherOrgAdmin, otherOrgMember, memberMe, templateAdmin, userAdmin},
 			},
 		},
@@ -278,7 +278,7 @@ func TestRolePermissions(t *testing.T) {
 			Actions:  []rbac.Action{rbac.ActionRead},
 			Resource: rbac.ResourceOrgRoleAssignment.InOrg(orgID),
 			AuthorizeMap: map[bool][]authSubject{
-				true:  {admin, orgAdmin, orgMemberMe},
+				true:  {owner, orgAdmin, orgMemberMe},
 				false: {otherOrgAdmin, otherOrgMember, memberMe, templateAdmin, userAdmin},
 			},
 		},
@@ -287,7 +287,7 @@ func TestRolePermissions(t *testing.T) {
 			Actions:  []rbac.Action{rbac.ActionCreate, rbac.ActionRead, rbac.ActionUpdate, rbac.ActionDelete},
 			Resource: rbac.ResourceAPIKey.WithOwner(currentUser.String()),
 			AuthorizeMap: map[bool][]authSubject{
-				true:  {admin, orgMemberMe, memberMe},
+				true:  {owner, orgMemberMe, memberMe},
 				false: {orgAdmin, otherOrgAdmin, otherOrgMember, templateAdmin, userAdmin},
 			},
 		},
@@ -296,7 +296,7 @@ func TestRolePermissions(t *testing.T) {
 			Actions:  []rbac.Action{rbac.ActionCreate, rbac.ActionRead, rbac.ActionUpdate, rbac.ActionDelete},
 			Resource: rbac.ResourceUserData.WithOwner(currentUser.String()),
 			AuthorizeMap: map[bool][]authSubject{
-				true:  {admin, orgMemberMe, memberMe},
+				true:  {owner, orgMemberMe, memberMe},
 				false: {orgAdmin, otherOrgAdmin, otherOrgMember, templateAdmin, userAdmin},
 			},
 		},
@@ -305,8 +305,8 @@ func TestRolePermissions(t *testing.T) {
 			Actions:  []rbac.Action{rbac.ActionCreate, rbac.ActionUpdate, rbac.ActionDelete},
 			Resource: rbac.ResourceOrganizationMember.InOrg(orgID),
 			AuthorizeMap: map[bool][]authSubject{
-				true:  {admin, orgAdmin},
-				false: {orgMemberMe, memberMe, otherOrgAdmin, otherOrgMember, templateAdmin, userAdmin},
+				true:  {owner, orgAdmin, userAdmin},
+				false: {orgMemberMe, memberMe, otherOrgAdmin, otherOrgMember, templateAdmin},
 			},
 		},
 		{
@@ -314,8 +314,8 @@ func TestRolePermissions(t *testing.T) {
 			Actions:  []rbac.Action{rbac.ActionRead},
 			Resource: rbac.ResourceOrganizationMember.InOrg(orgID),
 			AuthorizeMap: map[bool][]authSubject{
-				true:  {admin, orgAdmin, orgMemberMe},
-				false: {memberMe, otherOrgAdmin, otherOrgMember, templateAdmin, userAdmin},
+				true:  {owner, orgAdmin, orgMemberMe, userAdmin},
+				false: {memberMe, otherOrgAdmin, otherOrgMember, templateAdmin},
 			},
 		},
 	}

coderd/roles.go

@@ -20,14 +20,7 @@ func (api *API) assignableSiteRoles(rw http.ResponseWriter, r *http.Request) {
 	}
 
 	roles := rbac.SiteRoles()
-	assignable := make([]rbac.Role, 0)
-	for _, role := range roles {
-		if rbac.CanAssignRole(actorRoles.Roles, role.Name) {
-			assignable = append(assignable, role)
-		}
-	}
-
-	httpapi.Write(rw, http.StatusOK, convertRoles(assignable))
+	httpapi.Write(rw, http.StatusOK, assignableRoles(actorRoles.Roles, roles))
 }
 
 // assignableSiteRoles returns all site wide roles that can be assigned.
@@ -41,14 +34,7 @@ func (api *API) assignableOrgRoles(rw http.ResponseWriter, r *http.Request) {
 	}
 
 	roles := rbac.OrganizationRoles(organization.ID)
-	assignable := make([]rbac.Role, 0)
-	for _, role := range roles {
-		if rbac.CanAssignRole(actorRoles.Roles, role.Name) {
-			assignable = append(assignable, role)
-		}
-	}
-
-	httpapi.Write(rw, http.StatusOK, convertRoles(assignable))
+	httpapi.Write(rw, http.StatusOK, assignableRoles(actorRoles.Roles, roles))
 }
 
 func (api *API) checkPermissions(rw http.ResponseWriter, r *http.Request) {
@@ -102,14 +88,19 @@ func convertRole(role rbac.Role) codersdk.Role {
 	}
 }
 
-func convertRoles(roles []rbac.Role) []codersdk.Role {
-	converted := make([]codersdk.Role, 0, len(roles))
+func assignableRoles(actorRoles []string, roles []rbac.Role) []codersdk.AssignableRoles {
+	assignable := make([]codersdk.AssignableRoles, 0)
 	for _, role := range roles {
-		// Roles without display names should never be shown to the ui.
 		if role.DisplayName == "" {
 			continue
 		}
-		converted = append(converted, convertRole(role))
+		assignable = append(assignable, codersdk.AssignableRoles{
+			Role: codersdk.Role{
+				Name:        role.Name,
+				DisplayName: role.DisplayName,
+			},
+			Assignable: rbac.CanAssignRole(actorRoles, role.Name),
+		})
 	}
-	return converted
+	return assignable
 }

coderd/roles_test.go

@@ -120,35 +120,39 @@ func TestListRoles(t *testing.T) {
 	require.NoError(t, err, "create org")
 
 	const forbidden = "Forbidden"
-	siteRoles := convertRoles(rbac.RoleOwner(), "auditor", "template-admin", "user-admin")
-	orgRoles := convertRoles(rbac.RoleOrgAdmin(admin.OrganizationID))
-
 	testCases := []struct {
 		Name            string
 		Client          *codersdk.Client
-		APICall         func(context.Context) ([]codersdk.Role, error)
-		ExpectedRoles   []codersdk.Role
+		APICall         func(context.Context) ([]codersdk.AssignableRoles, error)
+		ExpectedRoles   []codersdk.AssignableRoles
 		AuthorizedError string
 	}{
 		{
 			// Members cannot assign any roles
 			Name: "MemberListSite",
-			APICall: func(ctx context.Context) ([]codersdk.Role, error) {
+			APICall: func(ctx context.Context) ([]codersdk.AssignableRoles, error) {
 				x, err := member.ListSiteRoles(ctx)
 				return x, err
 			},
-			ExpectedRoles: []codersdk.Role{},
+			ExpectedRoles: convertRoles(map[string]bool{
+				"owner":          false,
+				"auditor":        false,
+				"template-admin": false,
+				"user-admin":     false,
+			}),
 		},
 		{
 			Name: "OrgMemberListOrg",
-			APICall: func(ctx context.Context) ([]codersdk.Role, error) {
+			APICall: func(ctx context.Context) ([]codersdk.AssignableRoles, error) {
 				return member.ListOrganizationRoles(ctx, admin.OrganizationID)
 			},
-			ExpectedRoles: []codersdk.Role{},
+			ExpectedRoles: convertRoles(map[string]bool{
+				rbac.RoleOrgAdmin(admin.OrganizationID): false,
+			}),
 		},
 		{
 			Name: "NonOrgMemberListOrg",
-			APICall: func(ctx context.Context) ([]codersdk.Role, error) {
+			APICall: func(ctx context.Context) ([]codersdk.AssignableRoles, error) {
 				return member.ListOrganizationRoles(ctx, otherOrg.ID)
 			},
 			AuthorizedError: forbidden,
@@ -156,21 +160,28 @@ func TestListRoles(t *testing.T) {
 		// Org admin
 		{
 			Name: "OrgAdminListSite",
-			APICall: func(ctx context.Context) ([]codersdk.Role, error) {
+			APICall: func(ctx context.Context) ([]codersdk.AssignableRoles, error) {
 				return orgAdmin.ListSiteRoles(ctx)
 			},
-			ExpectedRoles: []codersdk.Role{},
+			ExpectedRoles: convertRoles(map[string]bool{
+				"owner":          false,
+				"auditor":        false,
+				"template-admin": false,
+				"user-admin":     false,
+			}),
 		},
 		{
 			Name: "OrgAdminListOrg",
-			APICall: func(ctx context.Context) ([]codersdk.Role, error) {
+			APICall: func(ctx context.Context) ([]codersdk.AssignableRoles, error) {
 				return orgAdmin.ListOrganizationRoles(ctx, admin.OrganizationID)
 			},
-			ExpectedRoles: orgRoles,
+			ExpectedRoles: convertRoles(map[string]bool{
+				rbac.RoleOrgAdmin(admin.OrganizationID): true,
+			}),
 		},
 		{
 			Name: "OrgAdminListOtherOrg",
-			APICall: func(ctx context.Context) ([]codersdk.Role, error) {
+			APICall: func(ctx context.Context) ([]codersdk.AssignableRoles, error) {
 				return orgAdmin.ListOrganizationRoles(ctx, otherOrg.ID)
 			},
 			AuthorizedError: forbidden,
@@ -178,17 +189,24 @@ func TestListRoles(t *testing.T) {
 		// Admin
 		{
 			Name: "AdminListSite",
-			APICall: func(ctx context.Context) ([]codersdk.Role, error) {
+			APICall: func(ctx context.Context) ([]codersdk.AssignableRoles, error) {
 				return client.ListSiteRoles(ctx)
 			},
-			ExpectedRoles: siteRoles,
+			ExpectedRoles: convertRoles(map[string]bool{
+				"owner":          true,
+				"auditor":        true,
+				"template-admin": true,
+				"user-admin":     true,
+			}),
 		},
 		{
 			Name: "AdminListOrg",
-			APICall: func(ctx context.Context) ([]codersdk.Role, error) {
+			APICall: func(ctx context.Context) ([]codersdk.AssignableRoles, error) {
 				return client.ListOrganizationRoles(ctx, admin.OrganizationID)
 			},
-			ExpectedRoles: orgRoles,
+			ExpectedRoles: convertRoles(map[string]bool{
+				rbac.RoleOrgAdmin(admin.OrganizationID): true,
+			}),
 		},
 	}
 
@@ -222,10 +240,14 @@ func convertRole(roleName string) codersdk.Role {
 	}
 }
 
-func convertRoles(roleNames ...string) []codersdk.Role {
-	converted := make([]codersdk.Role, 0, len(roleNames))
-	for _, roleName := range roleNames {
-		converted = append(converted, convertRole(roleName))
+func convertRoles(assignableRoles map[string]bool) []codersdk.AssignableRoles {
+	converted := make([]codersdk.AssignableRoles, 0, len(assignableRoles))
+	for roleName, assignable := range assignableRoles {
+		role := convertRole(roleName)
+		converted = append(converted, codersdk.AssignableRoles{
+			Role:       role,
+			Assignable: assignable,
+		})
 	}
 	return converted
 }