Skip to content

Commit 52c9813

Browse files
EmyrkEmyrk
committed
fix site <-> user perms bug
1 parent b64b016 commit 52c9813

File tree

5 files changed

+114
-74
lines changed

5 files changed

+114
-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

+81-62
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,10 @@ 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+
typed, _ := data.(codersdk.Role)
25+
return []roleTableView{roleToTableView(typed)}, nil
2526
},
2627
),
2728
cliui.JSONFormat(),
@@ -51,108 +52,126 @@ func (r *RootCmd) editRole() *serpent.Command {
5152
},
5253
},
5354
Middleware: serpent.Chain(
54-
serpent.RequireNArgs(1),
55+
serpent.RequireRangeArgs(0, 1),
5556
r.InitClient(client),
5657
),
5758
Handler: func(inv *serpent.Invocation) error {
5859
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-
}
8260

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.
61+
var customRole codersdk.Role
9062
fi, _ := os.Stdin.Stat()
9163
if (fi.Mode() & os.ModeCharDevice) == 0 {
64+
// JSON Upload mode
9265
bytes, err := io.ReadAll(os.Stdin)
9366
if err != nil {
9467
return xerrors.Errorf("reading stdin: %w", err)
9568
}
9669

97-
err = json.Unmarshal(bytes, customRole)
70+
err = json.Unmarshal(bytes, &customRole)
9871
if err != nil {
9972
return xerrors.Errorf("parsing stdin json: %w", err)
10073
}
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-
}
10674

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

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)
89+
customRole = *interactiveRole
90+
91+
// Only the interactive can answer prompts.
92+
totalOrg := 0
93+
for _, o := range customRole.OrganizationPermissions {
94+
totalOrg += len(o)
95+
}
96+
preview := fmt.Sprintf("perms: %d site, %d over %d orgs, %d user",
97+
len(customRole.SitePermissions), totalOrg, len(customRole.OrganizationPermissions), len(customRole.UserPermissions))
98+
_, err = cliui.Prompt(inv, cliui.PromptOptions{
99+
Text: "Are you sure you wish to update the role? " + preview,
100+
Default: "yes",
101+
IsConfirm: true,
102+
})
103+
if err != nil {
104+
return xerrors.Errorf("abort: %w", err)
105+
}
130106
}
131107

108+
var err error
132109
var updated codersdk.Role
133110
if dryRun {
134111
// Do not actually post
135-
updated = *customRole
112+
updated = customRole
136113
} else {
137-
updated, err = client.PatchRole(ctx, *customRole)
114+
updated, err = client.PatchRole(ctx, customRole)
138115
if err != nil {
139116
return fmt.Errorf("patch role: %w", err)
140117
}
141118
}
142119

143-
_, err = formatter.Format(ctx, updated)
120+
output, err := formatter.Format(ctx, updated)
144121
if err != nil {
145122
return xerrors.Errorf("formatting: %w", err)
146123
}
147-
return nil
124+
125+
_, err = fmt.Fprintln(inv.Stdout, output)
126+
return err
148127
},
149128
}
150129

151130
formatter.AttachOptions(&cmd.Options)
152131
return cmd
153132
}
154133

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