From 7d69335b4a8d880dbedad2b78f18ae0682a04ccd Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 24 May 2024 10:42:02 -1000 Subject: [PATCH 1/9] Add edit org role --- cli/cliui/parameter.go | 5 +- cli/cliui/select.go | 17 +- cli/cliui/select_test.go | 5 +- cli/organizationroles.go | 309 ++++++++++++++++++++++++++++++-- coderd/util/slice/slice.go | 9 + codersdk/rbacresources_gen.go | 30 ++++ scripts/rbacgen/codersdk.gotmpl | 12 ++ 7 files changed, 365 insertions(+), 22 deletions(-) diff --git a/cli/cliui/parameter.go b/cli/cliui/parameter.go index 897ddec4de4d6..8080ef1a96906 100644 --- a/cli/cliui/parameter.go +++ b/cli/cliui/parameter.go @@ -43,7 +43,10 @@ func RichParameter(inv *serpent.Invocation, templateVersionParameter codersdk.Te return "", err } - values, err := MultiSelect(inv, options) + values, err := MultiSelect(inv, MultiSelectOptions{ + Options: options, + Defaults: options, + }) if err == nil { v, err := json.Marshal(&values) if err != nil { diff --git a/cli/cliui/select.go b/cli/cliui/select.go index 3ae27ee811e50..a67ad42bcfeda 100644 --- a/cli/cliui/select.go +++ b/cli/cliui/select.go @@ -56,6 +56,7 @@ type SelectOptions struct { Options []string // Default will be highlighted first if it's a valid option. Default string + Message string Size int HideSearch bool } @@ -122,6 +123,7 @@ func Select(inv *serpent.Invocation, opts SelectOptions) (string, error) { Options: opts.Options, Default: defaultOption, PageSize: opts.Size, + Message: opts.Message, }, &value, survey.WithIcons(func(is *survey.IconSet) { is.Help.Text = "Type to search" if opts.HideSearch { @@ -138,15 +140,22 @@ func Select(inv *serpent.Invocation, opts SelectOptions) (string, error) { return value, err } -func MultiSelect(inv *serpent.Invocation, items []string) ([]string, error) { +type MultiSelectOptions struct { + Message string + Options []string + Defaults []string +} + +func MultiSelect(inv *serpent.Invocation, opts MultiSelectOptions) ([]string, error) { // Similar hack is applied to Select() if flag.Lookup("test.v") != nil { - return items, nil + return opts.Defaults, nil } prompt := &survey.MultiSelect{ - Options: items, - Default: items, + Options: opts.Options, + Default: opts.Defaults, + Message: opts.Message, } var values []string diff --git a/cli/cliui/select_test.go b/cli/cliui/select_test.go index c399121adb6ec..c0da49714fc40 100644 --- a/cli/cliui/select_test.go +++ b/cli/cliui/select_test.go @@ -107,7 +107,10 @@ func newMultiSelect(ptty *ptytest.PTY, items []string) ([]string, error) { var values []string cmd := &serpent.Command{ Handler: func(inv *serpent.Invocation) error { - selectedItems, err := cliui.MultiSelect(inv, items) + selectedItems, err := cliui.MultiSelect(inv, cliui.MultiSelectOptions{ + Options: items, + Defaults: items, + }) if err == nil { values = selectedItems } diff --git a/cli/organizationroles.go b/cli/organizationroles.go index 91d1b20f54dd4..59c9eb4fa006f 100644 --- a/cli/organizationroles.go +++ b/cli/organizationroles.go @@ -1,13 +1,19 @@ package cli import ( + "encoding/json" "fmt" + "io" + "os" "slices" "strings" + "github.com/google/uuid" "golang.org/x/xerrors" "github.com/coder/coder/v2/cli/cliui" + "github.com/coder/coder/v2/coderd/database/db2sdk" + "github.com/coder/coder/v2/coderd/util/slice" "github.com/coder/coder/v2/codersdk" "github.com/coder/serpent" ) @@ -23,6 +29,7 @@ func (r *RootCmd) organizationRoles() *serpent.Command { Hidden: true, Children: []*serpent.Command{ r.showOrganizationRoles(), + r.editOrganizationRole(), }, } return cmd @@ -31,24 +38,16 @@ func (r *RootCmd) organizationRoles() *serpent.Command { func (r *RootCmd) showOrganizationRoles() *serpent.Command { formatter := cliui.NewOutputFormatter( cliui.ChangeFormatterData( - cliui.TableFormat([]assignableRolesTableRow{}, []string{"name", "display_name", "built_in", "site_permissions", "org_permissions", "user_permissions"}), + cliui.TableFormat([]roleTableRow{}, []string{"name", "display_name", "site_permissions", "org_permissions", "user_permissions"}), func(data any) (any, error) { input, ok := data.([]codersdk.AssignableRoles) if !ok { return nil, xerrors.Errorf("expected []codersdk.AssignableRoles got %T", data) } - rows := make([]assignableRolesTableRow, 0, len(input)) - for _, role := range input { - rows = append(rows, assignableRolesTableRow{ - Name: role.Name, - DisplayName: role.DisplayName, - SitePermissions: fmt.Sprintf("%d permissions", len(role.SitePermissions)), - OrganizationPermissions: fmt.Sprintf("%d organizations", len(role.OrganizationPermissions)), - UserPermissions: fmt.Sprintf("%d permissions", len(role.UserPermissions)), - Assignable: role.Assignable, - BuiltIn: role.BuiltIn, - }) - } + + rows := db2sdk.List(input, func(f codersdk.AssignableRoles) roleTableRow { + return roleToTableView(f.Role) + }) return rows, nil }, ), @@ -101,13 +100,291 @@ func (r *RootCmd) showOrganizationRoles() *serpent.Command { return cmd } -type assignableRolesTableRow struct { +func (r *RootCmd) editOrganizationRole() *serpent.Command { + formatter := cliui.NewOutputFormatter( + cliui.ChangeFormatterData( + cliui.TableFormat([]roleTableRow{}, []string{"name", "display_name", "site_permissions", "org_permissions", "user_permissions"}), + func(data any) (any, error) { + typed, _ := data.(codersdk.Role) + return []roleTableRow{roleToTableView(typed)}, nil + }, + ), + cliui.JSONFormat(), + ) + + var ( + dryRun bool + ) + + client := new(codersdk.Client) + cmd := &serpent.Command{ + Use: "edit ", + Short: "Edit an organization custom role", + Long: FormatExamples( + Example{ + Description: "Run with an input.json file", + Command: "coder roles edit custom_name < role.json", + }, + ), + Options: []serpent.Option{ + cliui.SkipPromptOption(), + { + Name: "dry-run", + Description: "Does all the work, but does not submit the final updated role.", + Flag: "dry-run", + Value: serpent.BoolOf(&dryRun), + }, + }, + Middleware: serpent.Chain( + serpent.RequireRangeArgs(0, 1), + r.InitClient(client), + ), + Handler: func(inv *serpent.Invocation) error { + ctx := inv.Context() + org, err := CurrentOrganization(r, inv, client) + if err != nil { + return err + } + + var customRole codersdk.Role + fi, _ := os.Stdin.Stat() + if (fi.Mode() & os.ModeCharDevice) == 0 { + // JSON Upload mode + bytes, err := io.ReadAll(os.Stdin) + if err != nil { + return xerrors.Errorf("reading stdin: %w", err) + } + + err = json.Unmarshal(bytes, &customRole) + if err != nil { + return xerrors.Errorf("parsing stdin json: %w", err) + } + + if customRole.Name == "" { + arr := make([]json.RawMessage, 0) + err = json.Unmarshal(bytes, &arr) + if err == nil && len(arr) > 0 { + return xerrors.Errorf("the input appears to be an array, only 1 role can be sent at a time") + } + return xerrors.Errorf("json input does not appear to be a valid role") + } + } else { + interactiveRole, err := interactiveOrgRoleEdit(inv, org.ID, client) + if err != nil { + return xerrors.Errorf("editing role: %w", err) + } + + customRole = *interactiveRole + + // Only the interactive can answer prompts. + totalOrg := 0 + for _, o := range customRole.OrganizationPermissions { + totalOrg += len(o) + } + preview := fmt.Sprintf("perms: %d site, %d over %d orgs, %d user", + len(customRole.SitePermissions), totalOrg, len(customRole.OrganizationPermissions), len(customRole.UserPermissions)) + _, err = cliui.Prompt(inv, cliui.PromptOptions{ + Text: "Are you sure you wish to update the role? " + preview, + Default: "yes", + IsConfirm: true, + }) + if err != nil { + return xerrors.Errorf("abort: %w", err) + } + } + + var updated codersdk.Role + if dryRun { + // Do not actually post + updated = customRole + } else { + updated, err = client.PatchOrganizationRole(ctx, org.ID, customRole) + if err != nil { + return fmt.Errorf("patch role: %w", err) + } + } + + output, err := formatter.Format(ctx, updated) + if err != nil { + return xerrors.Errorf("formatting: %w", err) + } + + _, err = fmt.Fprintln(inv.Stdout, output) + return err + }, + } + + formatter.AttachOptions(&cmd.Options) + return cmd +} + +func interactiveOrgRoleEdit(inv *serpent.Invocation, orgID uuid.UUID, client *codersdk.Client) (*codersdk.Role, error) { + ctx := inv.Context() + roles, err := client.ListOrganizationRoles(ctx, orgID) + if err != nil { + return nil, xerrors.Errorf("listing roles: %w", err) + } + + // Make sure the role actually exists first + var originalRole codersdk.AssignableRoles + for _, r := range roles { + if strings.EqualFold(inv.Args[0], r.Name) { + originalRole = r + break + } + } + + if originalRole.Name == "" { + _, err = cliui.Prompt(inv, cliui.PromptOptions{ + Text: "No organization role exists with that name, do you want to create one?", + Default: "yes", + IsConfirm: true, + }) + if err != nil { + return nil, xerrors.Errorf("abort: %w", err) + } + + originalRole.Role = codersdk.Role{ + Name: inv.Args[0], + OrganizationID: orgID.String(), + } + } + + // Some checks since interactive mode is limited in what it currently sees + if len(originalRole.SitePermissions) > 0 { + return nil, xerrors.Errorf("unable to edit role in interactive mode, it contains site wide permissions") + } + + if len(originalRole.UserPermissions) > 0 { + return nil, xerrors.Errorf("unable to edit role in interactive mode, it contains user permissions") + } + + role := &originalRole.Role + allowedResources := []codersdk.RBACResource{ + codersdk.ResourceTemplate, + codersdk.ResourceWorkspace, + codersdk.ResourceUser, + codersdk.ResourceGroup, + } + + const done = "Finish and submit changes" + const abort = "Cancel changes" + + // Now starts the role editing "game". +customRoleLoop: + for { + selected, err := cliui.Select(inv, cliui.SelectOptions{ + Message: "Select which resources to edit permissions", + Options: append(permissionPreviews(role, allowedResources), done, abort), + }) + if err != nil { + return role, xerrors.Errorf("selecting resource: %w", err) + } + switch selected { + case done: + break customRoleLoop + case abort: + return role, xerrors.Errorf("edit role %q aborted", role.Name) + default: + strs := strings.Split(selected, "::") + resource := strings.TrimSpace(strs[0]) + + actions, err := cliui.MultiSelect(inv, cliui.MultiSelectOptions{ + Message: fmt.Sprintf("Select actions to allow across the whole deployment for resources=%q", resource), + Options: slice.ToStrings(codersdk.RBACResourceActions[codersdk.RBACResource(resource)]), + Defaults: defaultActions(role, resource), + }) + if err != nil { + return role, xerrors.Errorf("selecting actions for resource %q: %w", resource, err) + } + applyOrgResourceActions(role, orgID, resource, actions) + // back to resources! + } + } + // This println is required because the prompt ends us on the same line as some text. + _, _ = fmt.Println() + + return role, nil +} + +func applyOrgResourceActions(role *codersdk.Role, orgID uuid.UUID, resource string, actions []string) { + if _, ok := role.OrganizationPermissions[orgID.String()]; !ok { + role.OrganizationPermissions[orgID.String()] = []codersdk.Permission{} + } + + // Construct new site perms with only new perms for the resource + keep := make([]codersdk.Permission, 0) + for _, perm := range role.OrganizationPermissions[orgID.String()] { + perm := perm + if string(perm.ResourceType) != resource { + keep = append(keep, perm) + } + } + + // Add new perms + for _, action := range actions { + keep = append(keep, codersdk.Permission{ + Negate: false, + ResourceType: codersdk.RBACResource(resource), + Action: codersdk.RBACAction(action), + }) + } + + role.OrganizationPermissions[orgID.String()] = keep +} + +func defaultActions(role *codersdk.Role, resource string) []string { + defaults := make([]string, 0) + for _, perm := range role.SitePermissions { + if string(perm.ResourceType) == resource { + defaults = append(defaults, string(perm.Action)) + } + } + return defaults +} + +func permissionPreviews(role *codersdk.Role, resources []codersdk.RBACResource) []string { + previews := make([]string, 0, len(resources)) + for _, resource := range resources { + previews = append(previews, permissionPreview(role, resource)) + } + return previews +} + +func permissionPreview(role *codersdk.Role, resource codersdk.RBACResource) string { + count := 0 + for _, perm := range role.SitePermissions { + if perm.ResourceType == resource { + count++ + } + } + return fmt.Sprintf("%s :: %d permissions", resource, count) +} + +func roleToTableView(role codersdk.Role) roleTableRow { + return roleTableRow{ + Name: role.Name, + DisplayName: role.DisplayName, + SitePermissions: fmt.Sprintf("%d permissions", len(role.SitePermissions)), + OrganizationPermissions: fmt.Sprintf("%s", orgPermissionString(role.OrganizationPermissions)), + UserPermissions: fmt.Sprintf("%d permissions", len(role.UserPermissions)), + } +} + +func orgPermissionString(perms map[string][]codersdk.Permission) string { + totalOrg := 0 + for _, o := range perms { + totalOrg += len(o) + } + return fmt.Sprintf("%d over %d organizations", + totalOrg, len(perms)) +} + +type roleTableRow struct { Name string `table:"name,default_sort"` DisplayName string `table:"display_name"` SitePermissions string ` table:"site_permissions"` // map[] -> Permissions OrganizationPermissions string `table:"org_permissions"` UserPermissions string `table:"user_permissions"` - Assignable bool `table:"assignable"` - BuiltIn bool `table:"built_in"` } diff --git a/coderd/util/slice/slice.go b/coderd/util/slice/slice.go index f06930f373557..9bb1da930ff45 100644 --- a/coderd/util/slice/slice.go +++ b/coderd/util/slice/slice.go @@ -4,6 +4,15 @@ import ( "golang.org/x/exp/constraints" ) +// ToStrings works for any type where the base type is a string. +func ToStrings[T ~string](a []T) []string { + tmp := make([]string, 0, len(a)) + for _, v := range a { + tmp = append(tmp, string(v)) + } + return tmp +} + // Omit creates a new slice with the arguments omitted from the list. func Omit[T comparable](a []T, omits ...T) []T { tmp := make([]T, 0, len(a)) diff --git a/codersdk/rbacresources_gen.go b/codersdk/rbacresources_gen.go index 9c7d9cc485128..42db5449c29f4 100644 --- a/codersdk/rbacresources_gen.go +++ b/codersdk/rbacresources_gen.go @@ -48,3 +48,33 @@ const ( ActionWorkspaceStart RBACAction = "start" ActionWorkspaceStop RBACAction = "stop" ) + +// RBACResourceActions is the mapping of resources to which actions are valid for +// said resource type. +var RBACResourceActions = map[RBACResource][]RBACAction{ + ResourceWildcard: []RBACAction{}, + ResourceApiKey: []RBACAction{ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceAssignOrgRole: []RBACAction{ActionAssign, ActionDelete, ActionRead}, + ResourceAssignRole: []RBACAction{ActionAssign, ActionCreate, ActionDelete, ActionRead}, + ResourceAuditLog: []RBACAction{ActionCreate, ActionRead}, + ResourceDebugInfo: []RBACAction{ActionRead}, + ResourceDeploymentConfig: []RBACAction{ActionRead, ActionUpdate}, + ResourceDeploymentStats: []RBACAction{ActionRead}, + ResourceFile: []RBACAction{ActionCreate, ActionRead}, + ResourceGroup: []RBACAction{ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceLicense: []RBACAction{ActionCreate, ActionDelete, ActionRead}, + ResourceOauth2App: []RBACAction{ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceOauth2AppCodeToken: []RBACAction{ActionCreate, ActionDelete, ActionRead}, + ResourceOauth2AppSecret: []RBACAction{ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceOrganization: []RBACAction{ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceOrganizationMember: []RBACAction{ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceProvisionerDaemon: []RBACAction{ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceReplicas: []RBACAction{ActionRead}, + ResourceSystem: []RBACAction{ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceTailnetCoordinator: []RBACAction{ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceTemplate: []RBACAction{ActionCreate, ActionDelete, ActionRead, ActionUpdate, ActionViewInsights}, + ResourceUser: []RBACAction{ActionCreate, ActionDelete, ActionRead, ActionReadPersonal, ActionUpdate, ActionUpdatePersonal}, + ResourceWorkspace: []RBACAction{ActionApplicationConnect, ActionCreate, ActionDelete, ActionRead, ActionSSH, ActionWorkspaceStart, ActionWorkspaceStop, ActionUpdate}, + ResourceWorkspaceDormant: []RBACAction{ActionApplicationConnect, ActionCreate, ActionDelete, ActionRead, ActionSSH, ActionWorkspaceStart, ActionWorkspaceStop, ActionUpdate}, + ResourceWorkspaceProxy: []RBACAction{ActionCreate, ActionDelete, ActionRead, ActionUpdate}, +} diff --git a/scripts/rbacgen/codersdk.gotmpl b/scripts/rbacgen/codersdk.gotmpl index 1492eaf86c2bf..dff4e165b1df5 100644 --- a/scripts/rbacgen/codersdk.gotmpl +++ b/scripts/rbacgen/codersdk.gotmpl @@ -16,3 +16,15 @@ const ( {{ $element.Enum }} RBACAction = "{{ $element.Value }}" {{- end }} ) + +// RBACResourceActions is the mapping of resources to which actions are valid for +// said resource type. +var RBACResourceActions = map[RBACResource][]RBACAction{ + {{- range $element := . }} + Resource{{ pascalCaseName $element.FunctionName }}: []RBACAction{ + {{- range $actionValue, $_ := $element.Actions }} + {{- actionEnum $actionValue -}}, + {{- end -}} + }, + {{- end }} +} From b005cdc1dbceda8c5c2736d0f1c7e6133bcd4f80 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 24 May 2024 10:50:27 -1000 Subject: [PATCH 2/9] Fix imports --- cli/organizationroles.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/cli/organizationroles.go b/cli/organizationroles.go index 59c9eb4fa006f..111e3f4ab70e7 100644 --- a/cli/organizationroles.go +++ b/cli/organizationroles.go @@ -12,7 +12,6 @@ import ( "golang.org/x/xerrors" "github.com/coder/coder/v2/cli/cliui" - "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/util/slice" "github.com/coder/coder/v2/codersdk" "github.com/coder/serpent" @@ -40,15 +39,17 @@ func (r *RootCmd) showOrganizationRoles() *serpent.Command { cliui.ChangeFormatterData( cliui.TableFormat([]roleTableRow{}, []string{"name", "display_name", "site_permissions", "org_permissions", "user_permissions"}), func(data any) (any, error) { - input, ok := data.([]codersdk.AssignableRoles) + inputs, ok := data.([]codersdk.AssignableRoles) if !ok { return nil, xerrors.Errorf("expected []codersdk.AssignableRoles got %T", data) } - rows := db2sdk.List(input, func(f codersdk.AssignableRoles) roleTableRow { - return roleToTableView(f.Role) - }) - return rows, nil + tableRows := make([]roleTableRow, 0) + for _, input := range inputs { + tableRows = append(tableRows, roleToTableView(input.Role)) + } + + return tableRows, nil }, ), cliui.JSONFormat(), From 677855a0ab164b09b23e66187561d9d36f504925 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 24 May 2024 11:43:13 -1000 Subject: [PATCH 3/9] add unit test --- cli/organizationroles.go | 57 +++++++++++++++++-------- enterprise/cli/organization_test.go | 64 +++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 16 deletions(-) create mode 100644 enterprise/cli/organization_test.go diff --git a/cli/organizationroles.go b/cli/organizationroles.go index 111e3f4ab70e7..42fff2b7b5a41 100644 --- a/cli/organizationroles.go +++ b/cli/organizationroles.go @@ -4,7 +4,6 @@ import ( "encoding/json" "fmt" "io" - "os" "slices" "strings" @@ -114,7 +113,8 @@ func (r *RootCmd) editOrganizationRole() *serpent.Command { ) var ( - dryRun bool + dryRun bool + jsonInput bool ) client := new(codersdk.Client) @@ -135,6 +135,12 @@ func (r *RootCmd) editOrganizationRole() *serpent.Command { Flag: "dry-run", Value: serpent.BoolOf(&dryRun), }, + { + Name: "stdin", + Description: "Reads stdin for the json role definition to upload.", + Flag: "stdin", + Value: serpent.BoolOf(&jsonInput), + }, }, Middleware: serpent.Chain( serpent.RequireRangeArgs(0, 1), @@ -148,10 +154,9 @@ func (r *RootCmd) editOrganizationRole() *serpent.Command { } var customRole codersdk.Role - fi, _ := os.Stdin.Stat() - if (fi.Mode() & os.ModeCharDevice) == 0 { + if jsonInput { // JSON Upload mode - bytes, err := io.ReadAll(os.Stdin) + bytes, err := io.ReadAll(inv.Stdin) if err != nil { return xerrors.Errorf("reading stdin: %w", err) } @@ -170,6 +175,10 @@ func (r *RootCmd) editOrganizationRole() *serpent.Command { return xerrors.Errorf("json input does not appear to be a valid role") } } else { + if len(inv.Args) == 0 { + return xerrors.Errorf("missing role name argument, usage: \"coder organizations roles edit \"") + } + interactiveRole, err := interactiveOrgRoleEdit(inv, org.ID, client) if err != nil { return xerrors.Errorf("editing role: %w", err) @@ -182,7 +191,7 @@ func (r *RootCmd) editOrganizationRole() *serpent.Command { for _, o := range customRole.OrganizationPermissions { totalOrg += len(o) } - preview := fmt.Sprintf("perms: %d site, %d over %d orgs, %d user", + preview := fmt.Sprintf("permissions: %d site, %d over %d orgs, %d user", len(customRole.SitePermissions), totalOrg, len(customRole.OrganizationPermissions), len(customRole.UserPermissions)) _, err = cliui.Prompt(inv, cliui.PromptOptions{ Text: "Are you sure you wish to update the role? " + preview, @@ -276,7 +285,7 @@ customRoleLoop: for { selected, err := cliui.Select(inv, cliui.SelectOptions{ Message: "Select which resources to edit permissions", - Options: append(permissionPreviews(role, allowedResources), done, abort), + Options: append(permissionPreviews(role, orgID, allowedResources), done, abort), }) if err != nil { return role, xerrors.Errorf("selecting resource: %w", err) @@ -293,7 +302,7 @@ customRoleLoop: actions, err := cliui.MultiSelect(inv, cliui.MultiSelectOptions{ Message: fmt.Sprintf("Select actions to allow across the whole deployment for resources=%q", resource), Options: slice.ToStrings(codersdk.RBACResourceActions[codersdk.RBACResource(resource)]), - Defaults: defaultActions(role, resource), + Defaults: defaultActions(role, orgID, resource), }) if err != nil { return role, xerrors.Errorf("selecting actions for resource %q: %w", resource, err) @@ -309,6 +318,10 @@ customRoleLoop: } func applyOrgResourceActions(role *codersdk.Role, orgID uuid.UUID, resource string, actions []string) { + if role.OrganizationPermissions == nil { + role.OrganizationPermissions = map[string][]codersdk.Permission{} + } + if _, ok := role.OrganizationPermissions[orgID.String()]; !ok { role.OrganizationPermissions[orgID.String()] = []codersdk.Permission{} } @@ -334,9 +347,13 @@ func applyOrgResourceActions(role *codersdk.Role, orgID uuid.UUID, resource stri role.OrganizationPermissions[orgID.String()] = keep } -func defaultActions(role *codersdk.Role, resource string) []string { +func defaultActions(role *codersdk.Role, orgID uuid.UUID, resource string) []string { + if role.OrganizationPermissions == nil { + role.OrganizationPermissions = map[string][]codersdk.Permission{} + } + defaults := make([]string, 0) - for _, perm := range role.SitePermissions { + for _, perm := range role.OrganizationPermissions[orgID.String()] { if string(perm.ResourceType) == resource { defaults = append(defaults, string(perm.Action)) } @@ -344,17 +361,21 @@ func defaultActions(role *codersdk.Role, resource string) []string { return defaults } -func permissionPreviews(role *codersdk.Role, resources []codersdk.RBACResource) []string { +func permissionPreviews(role *codersdk.Role, orgID uuid.UUID, resources []codersdk.RBACResource) []string { previews := make([]string, 0, len(resources)) for _, resource := range resources { - previews = append(previews, permissionPreview(role, resource)) + previews = append(previews, permissionPreview(role, orgID, resource)) } return previews } -func permissionPreview(role *codersdk.Role, resource codersdk.RBACResource) string { +func permissionPreview(role *codersdk.Role, orgID uuid.UUID, resource codersdk.RBACResource) string { + if role.OrganizationPermissions == nil { + role.OrganizationPermissions = map[string][]codersdk.Permission{} + } + count := 0 - for _, perm := range role.SitePermissions { + for _, perm := range role.OrganizationPermissions[orgID.String()] { if perm.ResourceType == resource { count++ } @@ -377,8 +398,12 @@ func orgPermissionString(perms map[string][]codersdk.Permission) string { for _, o := range perms { totalOrg += len(o) } - return fmt.Sprintf("%d over %d organizations", - totalOrg, len(perms)) + plural := "" + if len(perms) > 1 { + plural = "s" + } + return fmt.Sprintf("%d over %d organization%s", + totalOrg, len(perms), plural) } type roleTableRow struct { diff --git a/enterprise/cli/organization_test.go b/enterprise/cli/organization_test.go new file mode 100644 index 0000000000000..ea8df0addf8af --- /dev/null +++ b/enterprise/cli/organization_test.go @@ -0,0 +1,64 @@ +package cli + +import ( + "bytes" + "fmt" + "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/codersdk" + "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" + "github.com/coder/coder/v2/enterprise/coderd/license" + "github.com/coder/coder/v2/testutil" +) + +func TestEditOrganizationRoles(t *testing.T) { + t.Parallel() + + t.Run("JSON", func(t *testing.T) { + t.Parallel() + + dv := coderdtest.DeploymentValues(t) + dv.Experiments = []string{string(codersdk.ExperimentCustomRoles)} + client, owner := coderdenttest.New(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + DeploymentValues: dv, + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureCustomRoles: 1, + }, + }}) + + ctx := testutil.Context(t, testutil.WaitMedium) + inv, root := clitest.New(t, "organization", "roles", "edit", "--stdin") + // Use json input, as interactive mode would be challenging to control + inv.Stdin = bytes.NewBufferString(fmt.Sprintf(`{ + "name": "new-role", + "organization_id": "%[1]s", + "display_name": "", + "site_permissions": [], + "organization_permissions": { + "%[1]s": [ + { + "resource_type": "workspace", + "action": "read" + } + ] + }, + "user_permissions": [], + "assignable": false, + "built_in": false + }`, owner.OrganizationID.String())) + clitest.SetupConfig(t, client, root) + + buf := new(bytes.Buffer) + inv.Stdout = buf + err := inv.WithContext(ctx).Run() + require.NoError(t, err) + require.Contains(t, buf.String(), "new-role") + }) +} From 57723a6773ae6ad8dc324705972620b8d2f6474f Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 24 May 2024 12:00:16 -1000 Subject: [PATCH 4/9] linting --- cli/organizationroles.go | 2 +- enterprise/cli/organization_test.go | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/cli/organizationroles.go b/cli/organizationroles.go index 42fff2b7b5a41..f501f39e7d4a6 100644 --- a/cli/organizationroles.go +++ b/cli/organizationroles.go @@ -388,7 +388,7 @@ func roleToTableView(role codersdk.Role) roleTableRow { Name: role.Name, DisplayName: role.DisplayName, SitePermissions: fmt.Sprintf("%d permissions", len(role.SitePermissions)), - OrganizationPermissions: fmt.Sprintf("%s", orgPermissionString(role.OrganizationPermissions)), + OrganizationPermissions: orgPermissionString(role.OrganizationPermissions), UserPermissions: fmt.Sprintf("%d permissions", len(role.UserPermissions)), } } diff --git a/enterprise/cli/organization_test.go b/enterprise/cli/organization_test.go index ea8df0addf8af..f3c0459a3eec2 100644 --- a/enterprise/cli/organization_test.go +++ b/enterprise/cli/organization_test.go @@ -1,4 +1,4 @@ -package cli +package cli_test import ( "bytes" @@ -31,7 +31,8 @@ func TestEditOrganizationRoles(t *testing.T) { Features: license.Features{ codersdk.FeatureCustomRoles: 1, }, - }}) + }, + }) ctx := testutil.Context(t, testutil.WaitMedium) inv, root := clitest.New(t, "organization", "roles", "edit", "--stdin") @@ -53,6 +54,7 @@ func TestEditOrganizationRoles(t *testing.T) { "assignable": false, "built_in": false }`, owner.OrganizationID.String())) + //nolint:gocritic // only owners can edit roles clitest.SetupConfig(t, client, root) buf := new(bytes.Buffer) From fbd8cf351200e555c19972b2ee1be85e75557016 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 29 May 2024 15:43:32 -0500 Subject: [PATCH 5/9] compile issue --- cli/organizationroles.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/cli/organizationroles.go b/cli/organizationroles.go index f501f39e7d4a6..d92c60395a677 100644 --- a/cli/organizationroles.go +++ b/cli/organizationroles.go @@ -186,13 +186,8 @@ func (r *RootCmd) editOrganizationRole() *serpent.Command { customRole = *interactiveRole - // Only the interactive can answer prompts. - totalOrg := 0 - for _, o := range customRole.OrganizationPermissions { - totalOrg += len(o) - } - preview := fmt.Sprintf("permissions: %d site, %d over %d orgs, %d user", - len(customRole.SitePermissions), totalOrg, len(customRole.OrganizationPermissions), len(customRole.UserPermissions)) + preview := fmt.Sprintf("permissions: %d site, %d org, %d user", + len(customRole.SitePermissions), len(customRole.OrganizationPermissions), len(customRole.UserPermissions)) _, err = cliui.Prompt(inv, cliui.PromptOptions{ Text: "Are you sure you wish to update the role? " + preview, Default: "yes", From 2bc99cdac1b7df50043bb51c032fad7334b41648 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 30 May 2024 10:10:33 -0500 Subject: [PATCH 6/9] Fix compile --- cli/organizationroles.go | 35 ++++++++++------------------------- 1 file changed, 10 insertions(+), 25 deletions(-) diff --git a/cli/organizationroles.go b/cli/organizationroles.go index d92c60395a677..29a82cac0a366 100644 --- a/cli/organizationroles.go +++ b/cli/organizationroles.go @@ -314,16 +314,12 @@ customRoleLoop: func applyOrgResourceActions(role *codersdk.Role, orgID uuid.UUID, resource string, actions []string) { if role.OrganizationPermissions == nil { - role.OrganizationPermissions = map[string][]codersdk.Permission{} - } - - if _, ok := role.OrganizationPermissions[orgID.String()]; !ok { - role.OrganizationPermissions[orgID.String()] = []codersdk.Permission{} + role.OrganizationPermissions = make([]codersdk.Permission, 0) } // Construct new site perms with only new perms for the resource keep := make([]codersdk.Permission, 0) - for _, perm := range role.OrganizationPermissions[orgID.String()] { + for _, perm := range role.OrganizationPermissions { perm := perm if string(perm.ResourceType) != resource { keep = append(keep, perm) @@ -339,16 +335,16 @@ func applyOrgResourceActions(role *codersdk.Role, orgID uuid.UUID, resource stri }) } - role.OrganizationPermissions[orgID.String()] = keep + role.OrganizationPermissions = keep } func defaultActions(role *codersdk.Role, orgID uuid.UUID, resource string) []string { if role.OrganizationPermissions == nil { - role.OrganizationPermissions = map[string][]codersdk.Permission{} + role.OrganizationPermissions = []codersdk.Permission{} } defaults := make([]string, 0) - for _, perm := range role.OrganizationPermissions[orgID.String()] { + for _, perm := range role.OrganizationPermissions { if string(perm.ResourceType) == resource { defaults = append(defaults, string(perm.Action)) } @@ -366,11 +362,11 @@ func permissionPreviews(role *codersdk.Role, orgID uuid.UUID, resources []coders func permissionPreview(role *codersdk.Role, orgID uuid.UUID, resource codersdk.RBACResource) string { if role.OrganizationPermissions == nil { - role.OrganizationPermissions = map[string][]codersdk.Permission{} + role.OrganizationPermissions = []codersdk.Permission{} } count := 0 - for _, perm := range role.OrganizationPermissions[orgID.String()] { + for _, perm := range role.OrganizationPermissions { if perm.ResourceType == resource { count++ } @@ -382,28 +378,17 @@ func roleToTableView(role codersdk.Role) roleTableRow { return roleTableRow{ Name: role.Name, DisplayName: role.DisplayName, + OrganizationID: role.OrganizationID, SitePermissions: fmt.Sprintf("%d permissions", len(role.SitePermissions)), - OrganizationPermissions: orgPermissionString(role.OrganizationPermissions), + OrganizationPermissions: fmt.Sprintf("%d permissions", len(role.OrganizationPermissions)), UserPermissions: fmt.Sprintf("%d permissions", len(role.UserPermissions)), } } -func orgPermissionString(perms map[string][]codersdk.Permission) string { - totalOrg := 0 - for _, o := range perms { - totalOrg += len(o) - } - plural := "" - if len(perms) > 1 { - plural = "s" - } - return fmt.Sprintf("%d over %d organization%s", - totalOrg, len(perms), plural) -} - type roleTableRow struct { Name string `table:"name,default_sort"` DisplayName string `table:"display_name"` + OrganizationID string `table:"organization_id"` SitePermissions string ` table:"site_permissions"` // map[] -> Permissions OrganizationPermissions string `table:"org_permissions"` From da0107c3cb2bdd19de050ccfa6f0d15eb94eac58 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 30 May 2024 10:27:11 -0500 Subject: [PATCH 7/9] Add custom role cli command --- cli/organizationroles.go | 16 ++++++++-------- enterprise/cli/organization_test.go | 16 +++++++--------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/cli/organizationroles.go b/cli/organizationroles.go index 29a82cac0a366..14ccce48159f6 100644 --- a/cli/organizationroles.go +++ b/cli/organizationroles.go @@ -280,7 +280,7 @@ customRoleLoop: for { selected, err := cliui.Select(inv, cliui.SelectOptions{ Message: "Select which resources to edit permissions", - Options: append(permissionPreviews(role, orgID, allowedResources), done, abort), + Options: append(permissionPreviews(role, allowedResources), done, abort), }) if err != nil { return role, xerrors.Errorf("selecting resource: %w", err) @@ -297,12 +297,12 @@ customRoleLoop: actions, err := cliui.MultiSelect(inv, cliui.MultiSelectOptions{ Message: fmt.Sprintf("Select actions to allow across the whole deployment for resources=%q", resource), Options: slice.ToStrings(codersdk.RBACResourceActions[codersdk.RBACResource(resource)]), - Defaults: defaultActions(role, orgID, resource), + Defaults: defaultActions(role, resource), }) if err != nil { return role, xerrors.Errorf("selecting actions for resource %q: %w", resource, err) } - applyOrgResourceActions(role, orgID, resource, actions) + applyOrgResourceActions(role, resource, actions) // back to resources! } } @@ -312,7 +312,7 @@ customRoleLoop: return role, nil } -func applyOrgResourceActions(role *codersdk.Role, orgID uuid.UUID, resource string, actions []string) { +func applyOrgResourceActions(role *codersdk.Role, resource string, actions []string) { if role.OrganizationPermissions == nil { role.OrganizationPermissions = make([]codersdk.Permission, 0) } @@ -338,7 +338,7 @@ func applyOrgResourceActions(role *codersdk.Role, orgID uuid.UUID, resource stri role.OrganizationPermissions = keep } -func defaultActions(role *codersdk.Role, orgID uuid.UUID, resource string) []string { +func defaultActions(role *codersdk.Role, resource string) []string { if role.OrganizationPermissions == nil { role.OrganizationPermissions = []codersdk.Permission{} } @@ -352,15 +352,15 @@ func defaultActions(role *codersdk.Role, orgID uuid.UUID, resource string) []str return defaults } -func permissionPreviews(role *codersdk.Role, orgID uuid.UUID, resources []codersdk.RBACResource) []string { +func permissionPreviews(role *codersdk.Role, resources []codersdk.RBACResource) []string { previews := make([]string, 0, len(resources)) for _, resource := range resources { - previews = append(previews, permissionPreview(role, orgID, resource)) + previews = append(previews, permissionPreview(role, resource)) } return previews } -func permissionPreview(role *codersdk.Role, orgID uuid.UUID, resource codersdk.RBACResource) string { +func permissionPreview(role *codersdk.Role, resource codersdk.RBACResource) string { if role.OrganizationPermissions == nil { role.OrganizationPermissions = []codersdk.Permission{} } diff --git a/enterprise/cli/organization_test.go b/enterprise/cli/organization_test.go index f3c0459a3eec2..f848e72b275ca 100644 --- a/enterprise/cli/organization_test.go +++ b/enterprise/cli/organization_test.go @@ -39,17 +39,15 @@ func TestEditOrganizationRoles(t *testing.T) { // Use json input, as interactive mode would be challenging to control inv.Stdin = bytes.NewBufferString(fmt.Sprintf(`{ "name": "new-role", - "organization_id": "%[1]s", + "organization_id": "%s", "display_name": "", "site_permissions": [], - "organization_permissions": { - "%[1]s": [ - { - "resource_type": "workspace", - "action": "read" - } - ] - }, + "organization_permissions": [ + { + "resource_type": "workspace", + "action": "read" + } + ], "user_permissions": [], "assignable": false, "built_in": false From ce841f99cc43596eca66b18b2fa673300ddc552c Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 30 May 2024 10:43:24 -0500 Subject: [PATCH 8/9] fixup example --- cli/organizationroles.go | 2 +- enterprise/cli/organization_test.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/cli/organizationroles.go b/cli/organizationroles.go index 14ccce48159f6..8cf557eecf9c3 100644 --- a/cli/organizationroles.go +++ b/cli/organizationroles.go @@ -124,7 +124,7 @@ func (r *RootCmd) editOrganizationRole() *serpent.Command { Long: FormatExamples( Example{ Description: "Run with an input.json file", - Command: "coder roles edit custom_name < role.json", + Command: "coder roles edit --stdin < role.json", }, ), Options: []serpent.Option{ diff --git a/enterprise/cli/organization_test.go b/enterprise/cli/organization_test.go index f848e72b275ca..aed1a32a44770 100644 --- a/enterprise/cli/organization_test.go +++ b/enterprise/cli/organization_test.go @@ -18,6 +18,8 @@ import ( func TestEditOrganizationRoles(t *testing.T) { t.Parallel() + // Unit test uses --stdin and json as the role input. The interactive cli would + // be hard to drive from a unit test. t.Run("JSON", func(t *testing.T) { t.Parallel() @@ -36,7 +38,6 @@ func TestEditOrganizationRoles(t *testing.T) { ctx := testutil.Context(t, testutil.WaitMedium) inv, root := clitest.New(t, "organization", "roles", "edit", "--stdin") - // Use json input, as interactive mode would be challenging to control inv.Stdin = bytes.NewBufferString(fmt.Sprintf(`{ "name": "new-role", "organization_id": "%s", From e5082ae40f2f493410b517548c1b4e7aa9562b99 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 31 May 2024 10:41:23 -0500 Subject: [PATCH 9/9] Add invalid role test --- enterprise/cli/organization_test.go | 47 +++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/enterprise/cli/organization_test.go b/enterprise/cli/organization_test.go index aed1a32a44770..51571602d05e5 100644 --- a/enterprise/cli/organization_test.go +++ b/enterprise/cli/organization_test.go @@ -62,4 +62,51 @@ func TestEditOrganizationRoles(t *testing.T) { require.NoError(t, err) require.Contains(t, buf.String(), "new-role") }) + + t.Run("InvalidRole", func(t *testing.T) { + t.Parallel() + + dv := coderdtest.DeploymentValues(t) + dv.Experiments = []string{string(codersdk.ExperimentCustomRoles)} + client, owner := coderdenttest.New(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + DeploymentValues: dv, + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureCustomRoles: 1, + }, + }, + }) + + ctx := testutil.Context(t, testutil.WaitMedium) + inv, root := clitest.New(t, "organization", "roles", "edit", "--stdin") + inv.Stdin = bytes.NewBufferString(fmt.Sprintf(`{ + "name": "new-role", + "organization_id": "%s", + "display_name": "", + "site_permissions": [ + { + "resource_type": "workspace", + "action": "read" + } + ], + "organization_permissions": [ + { + "resource_type": "workspace", + "action": "read" + } + ], + "user_permissions": [], + "assignable": false, + "built_in": false + }`, owner.OrganizationID.String())) + //nolint:gocritic // only owners can edit roles + clitest.SetupConfig(t, client, root) + + buf := new(bytes.Buffer) + inv.Stdout = buf + err := inv.WithContext(ctx).Run() + require.ErrorContains(t, err, "not allowed to assign site wide permissions for an organization role") + }) }