Skip to content

Commit 70bcd3d

Browse files
committed
fix site <-> user perms bug
1 parent 095ad88 commit 70bcd3d

File tree

5 files changed

+113
-74
lines changed

5 files changed

+113
-74
lines changed

coderd/database/db2sdk/db2sdk.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -532,7 +532,7 @@ func Role(role rbac.Role) codersdk.Role {
532532
DisplayName: role.DisplayName,
533533
SitePermissions: List(role.Site, Permission),
534534
OrganizationPermissions: Map(role.Org, ListLazy(Permission)),
535-
UserPermissions: List(role.Site, Permission),
535+
UserPermissions: List(role.User, Permission),
536536
}
537537
}
538538

enterprise/cli/roleedit.go

+80-62
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,9 @@ import (
1919
func (r *RootCmd) editRole() *serpent.Command {
2020
formatter := cliui.NewOutputFormatter(
2121
cliui.ChangeFormatterData(
22-
cliui.TableFormat([]codersdk.Role{}, []string{"name", "display_name", "site_permissions", "org_permissions", "user_permissions"}),
22+
cliui.TableFormat([]roleTableView{}, []string{"name", "display_name", "site_permissions", "org_permissions", "user_permissions"}),
2323
func(data any) (any, error) {
24-
return []codersdk.Role{data.(codersdk.Role)}, nil
24+
return []roleTableView{roleToTableView(data.(codersdk.Role))}, nil
2525
},
2626
),
2727
cliui.JSONFormat(),
@@ -51,108 +51,126 @@ func (r *RootCmd) editRole() *serpent.Command {
5151
},
5252
},
5353
Middleware: serpent.Chain(
54-
serpent.RequireNArgs(1),
54+
serpent.RequireRangeArgs(0, 1),
5555
r.InitClient(client),
5656
),
5757
Handler: func(inv *serpent.Invocation) error {
5858
ctx := inv.Context()
59-
roles, err := client.ListSiteRoles(ctx)
60-
if err != nil {
61-
return xerrors.Errorf("listing roles: %w", err)
62-
}
63-
64-
// Make sure the role actually exists first
65-
var originalRole codersdk.AssignableRoles
66-
for _, r := range roles {
67-
if strings.EqualFold(inv.Args[0], r.Name) {
68-
originalRole = r
69-
break
70-
}
71-
}
72-
73-
if originalRole.Name == "" {
74-
_, err = cliui.Prompt(inv, cliui.PromptOptions{
75-
Text: "No role exists with that name, do you want to create one?",
76-
Default: "yes",
77-
IsConfirm: true,
78-
})
79-
if err != nil {
80-
return xerrors.Errorf("abort: %w", err)
81-
}
8259

83-
originalRole.Role = codersdk.Role{
84-
Name: inv.Args[0],
85-
}
86-
}
87-
88-
var customRole *codersdk.Role
89-
// Either interactive, or take input mode.
60+
var customRole codersdk.Role
9061
fi, _ := os.Stdin.Stat()
9162
if (fi.Mode() & os.ModeCharDevice) == 0 {
63+
// JSON Upload mode
9264
bytes, err := io.ReadAll(os.Stdin)
9365
if err != nil {
9466
return xerrors.Errorf("reading stdin: %w", err)
9567
}
9668

97-
err = json.Unmarshal(bytes, customRole)
69+
err = json.Unmarshal(bytes, &customRole)
9870
if err != nil {
9971
return xerrors.Errorf("parsing stdin json: %w", err)
10072
}
101-
} else {
102-
// Interactive mode
103-
if len(originalRole.OrganizationPermissions) > 0 {
104-
return xerrors.Errorf("unable to edit role in interactive mode, it contains organization permissions")
105-
}
10673

107-
if len(originalRole.UserPermissions) > 0 {
108-
return xerrors.Errorf("unable to edit role in interactive mode, it contains user permissions")
74+
if customRole.Name == "" {
75+
arr := make([]json.RawMessage, 0)
76+
err = json.Unmarshal(bytes, &arr)
77+
if err == nil && len(arr) > 0 {
78+
return xerrors.Errorf("the input appears to be an array, only 1 role can be sent at a time")
79+
}
80+
return xerrors.Errorf("json input does not appear to be a valid role")
10981
}
110-
111-
customRole, err = interactiveEdit(inv, &originalRole.Role)
82+
} else {
83+
interactiveRole, err := interactiveEdit(inv, client)
11284
if err != nil {
11385
return xerrors.Errorf("editing role: %w", err)
11486
}
115-
}
11687

117-
totalOrg := 0
118-
for _, o := range customRole.OrganizationPermissions {
119-
totalOrg += len(o)
120-
}
121-
preview := fmt.Sprintf("perms: %d site, %d over %d orgs, %d user",
122-
len(customRole.SitePermissions), totalOrg, len(customRole.OrganizationPermissions), len(customRole.UserPermissions))
123-
_, err = cliui.Prompt(inv, cliui.PromptOptions{
124-
Text: "Are you sure you wish to update the role? " + preview,
125-
Default: "yes",
126-
IsConfirm: true,
127-
})
128-
if err != nil {
129-
return xerrors.Errorf("abort: %w", err)
88+
customRole = *interactiveRole
89+
90+
// Only the interactive can answer prompts.
91+
totalOrg := 0
92+
for _, o := range customRole.OrganizationPermissions {
93+
totalOrg += len(o)
94+
}
95+
preview := fmt.Sprintf("perms: %d site, %d over %d orgs, %d user",
96+
len(customRole.SitePermissions), totalOrg, len(customRole.OrganizationPermissions), len(customRole.UserPermissions))
97+
_, err = cliui.Prompt(inv, cliui.PromptOptions{
98+
Text: "Are you sure you wish to update the role? " + preview,
99+
Default: "yes",
100+
IsConfirm: true,
101+
})
102+
if err != nil {
103+
return xerrors.Errorf("abort: %w", err)
104+
}
130105
}
131106

107+
var err error
132108
var updated codersdk.Role
133109
if dryRun {
134110
// Do not actually post
135-
updated = *customRole
111+
updated = customRole
136112
} else {
137-
updated, err = client.PatchRole(ctx, *customRole)
113+
updated, err = client.PatchRole(ctx, customRole)
138114
if err != nil {
139115
return fmt.Errorf("patch role: %w", err)
140116
}
141117
}
142118

143-
_, err = formatter.Format(ctx, updated)
119+
output, err := formatter.Format(ctx, updated)
144120
if err != nil {
145121
return xerrors.Errorf("formatting: %w", err)
146122
}
147-
return nil
123+
124+
_, err = fmt.Fprintln(inv.Stdout, output)
125+
return err
148126
},
149127
}
150128

151129
formatter.AttachOptions(&cmd.Options)
152130
return cmd
153131
}
154132

155-
func interactiveEdit(inv *serpent.Invocation, role *codersdk.Role) (*codersdk.Role, error) {
133+
func interactiveEdit(inv *serpent.Invocation, client *codersdk.Client) (*codersdk.Role, error) {
134+
ctx := inv.Context()
135+
roles, err := client.ListSiteRoles(ctx)
136+
if err != nil {
137+
return nil, xerrors.Errorf("listing roles: %w", err)
138+
}
139+
140+
// Make sure the role actually exists first
141+
var originalRole codersdk.AssignableRoles
142+
for _, r := range roles {
143+
if strings.EqualFold(inv.Args[0], r.Name) {
144+
originalRole = r
145+
break
146+
}
147+
}
148+
149+
if originalRole.Name == "" {
150+
_, err = cliui.Prompt(inv, cliui.PromptOptions{
151+
Text: "No role exists with that name, do you want to create one?",
152+
Default: "yes",
153+
IsConfirm: true,
154+
})
155+
if err != nil {
156+
return nil, xerrors.Errorf("abort: %w", err)
157+
}
158+
159+
originalRole.Role = codersdk.Role{
160+
Name: inv.Args[0],
161+
}
162+
}
163+
164+
// Some checks since interactive mode is limited in what it currently sees
165+
if len(originalRole.OrganizationPermissions) > 0 {
166+
return nil, xerrors.Errorf("unable to edit role in interactive mode, it contains organization permissions")
167+
}
168+
169+
if len(originalRole.UserPermissions) > 0 {
170+
return nil, xerrors.Errorf("unable to edit role in interactive mode, it contains user permissions")
171+
}
172+
173+
role := &originalRole.Role
156174
allowedResources := []codersdk.RBACResource{
157175
codersdk.ResourceTemplate,
158176
codersdk.ResourceWorkspace,

enterprise/cli/rolescmd.go

+20-10
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,9 @@ func (r *RootCmd) showRole() *serpent.Command {
4444
rows := make([]assignableRolesTableRow, 0, len(input))
4545
for _, role := range input {
4646
rows = append(rows, assignableRolesTableRow{
47-
Name: role.Name,
48-
DisplayName: role.DisplayName,
49-
SitePermissions: fmt.Sprintf("%d permissions", len(role.SitePermissions)),
50-
OrganizationPermissions: fmt.Sprintf("%d organizations", len(role.OrganizationPermissions)),
51-
UserPermissions: fmt.Sprintf("%d permissions", len(role.UserPermissions)),
52-
Assignable: role.Assignable,
53-
BuiltIn: role.BuiltIn,
47+
roleTableView: roleToTableView(role.Role),
48+
Assignable: role.Assignable,
49+
BuiltIn: role.BuiltIn,
5450
})
5551
}
5652
return rows, nil
@@ -100,13 +96,27 @@ func (r *RootCmd) showRole() *serpent.Command {
10096
return cmd
10197
}
10298

103-
type assignableRolesTableRow struct {
99+
func roleToTableView(role codersdk.Role) roleTableView {
100+
return roleTableView{
101+
Name: role.Name,
102+
DisplayName: role.DisplayName,
103+
SitePermissions: fmt.Sprintf("%d permissions", len(role.SitePermissions)),
104+
OrganizationPermissions: fmt.Sprintf("%d organizations", len(role.OrganizationPermissions)),
105+
UserPermissions: fmt.Sprintf("%d permissions", len(role.UserPermissions)),
106+
}
107+
}
108+
109+
type roleTableView struct {
104110
Name string `table:"name,default_sort"`
105111
DisplayName string `table:"display_name"`
106112
SitePermissions string ` table:"site_permissions"`
107113
// map[<org_id>] -> Permissions
108114
OrganizationPermissions string `table:"org_permissions"`
109115
UserPermissions string `table:"user_permissions"`
110-
Assignable bool `table:"assignable"`
111-
BuiltIn bool `table:"built_in"`
116+
}
117+
118+
type assignableRolesTableRow struct {
119+
roleTableView `table:"r,recursive_inline"`
120+
Assignable bool `table:"assignable"`
121+
BuiltIn bool `table:"built_in"`
112122
}

enterprise/cmd/coder/role.json

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
{
2+
}

enterprise/coderd/roles_test.go

+10-1
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,18 @@ func TestCustomRole(t *testing.T) {
6767
allRoles, err := tmplAdmin.ListSiteRoles(ctx)
6868
require.NoError(t, err)
6969

70+
var foundRole codersdk.AssignableRoles
7071
require.True(t, slices.ContainsFunc(allRoles, func(selected codersdk.AssignableRoles) bool {
71-
return selected.Name == role.Name
72+
if selected.Name == role.Name {
73+
foundRole = selected
74+
return true
75+
}
76+
return false
7277
}), "role missing from site role list")
78+
79+
require.Len(t, foundRole.SitePermissions, 7)
80+
require.Len(t, foundRole.OrganizationPermissions, 0)
81+
require.Len(t, foundRole.UserPermissions, 0)
7382
})
7483

7584
// Revoked licenses cannot modify/create custom roles, but they can

0 commit comments

Comments
 (0)