Skip to content

Commit e320661

Browse files
EmyrkEmyrk
authored
chore: implement typed database for custom permissions (breaks existing custom roles) (#13457)
* chore: typed database custom permissions * add migration to fix any custom roles out there
1 parent 168d2d6 commit e320661

17 files changed

+256
-266
lines changed

‎cli/organizationroles.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ func (r *RootCmd) organizationRoles() *serpent.Command {
3636
func (r *RootCmd) showOrganizationRoles() *serpent.Command {
3737
formatter := cliui.NewOutputFormatter(
3838
cliui.ChangeFormatterData(
39-
cliui.TableFormat([]roleTableRow{}, []string{"name", "display_name", "site_permissions", "org_permissions", "user_permissions"}),
39+
cliui.TableFormat([]roleTableRow{}, []string{"name", "display_name", "site_permissions", "organization_permissions", "user_permissions"}),
4040
func(data any) (any, error) {
4141
inputs, ok := data.([]codersdk.AssignableRoles)
4242
if !ok {
@@ -103,7 +103,7 @@ func (r *RootCmd) showOrganizationRoles() *serpent.Command {
103103
func (r *RootCmd) editOrganizationRole() *serpent.Command {
104104
formatter := cliui.NewOutputFormatter(
105105
cliui.ChangeFormatterData(
106-
cliui.TableFormat([]roleTableRow{}, []string{"name", "display_name", "site_permissions", "org_permissions", "user_permissions"}),
106+
cliui.TableFormat([]roleTableRow{}, []string{"name", "display_name", "site_permissions", "organization_permissions", "user_permissions"}),
107107
func(data any) (any, error) {
108108
typed, _ := data.(codersdk.Role)
109109
return []roleTableRow{roleToTableView(typed)}, nil
@@ -391,6 +391,6 @@ type roleTableRow struct {
391391
OrganizationID string `table:"organization_id"`
392392
SitePermissions string ` table:"site_permissions"`
393393
// map[<org_id>] -> Permissions
394-
OrganizationPermissions string `table:"org_permissions"`
394+
OrganizationPermissions string `table:"organization_permissions"`
395395
UserPermissions string `table:"user_permissions"`
396396
}

‎coderd/database/db2sdk/db2sdk.go

Lines changed: 29 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import (
1818
"github.com/coder/coder/v2/coderd/database"
1919
"github.com/coder/coder/v2/coderd/parameter"
2020
"github.com/coder/coder/v2/coderd/rbac"
21-
"github.com/coder/coder/v2/coderd/rbac/policy"
2221
"github.com/coder/coder/v2/coderd/workspaceapps/appurl"
2322
"github.com/coder/coder/v2/codersdk"
2423
"github.com/coder/coder/v2/provisionersdk/proto"
@@ -526,54 +525,51 @@ func ProvisionerDaemon(dbDaemon database.ProvisionerDaemon) codersdk.Provisioner
526525
return result
527526
}
528527

529-
func Role(role rbac.Role) codersdk.Role {
528+
func RBACRole(role rbac.Role) codersdk.Role {
530529
roleName, orgIDStr, err := rbac.RoleSplit(role.Name)
531530
if err != nil {
532531
roleName = role.Name
533532
}
533+
orgPerms := role.Org[orgIDStr]
534534

535535
return codersdk.Role{
536-
Name: roleName,
537-
OrganizationID: orgIDStr,
538-
DisplayName: role.DisplayName,
539-
SitePermissions: List(role.Site, Permission),
540-
// This is not perfect. If there are organization permissions in another
541-
// organization, they will be omitted. This should not be allowed, so
542-
// should never happen.
543-
OrganizationPermissions: List(role.Org[orgIDStr], Permission),
544-
UserPermissions: List(role.User, Permission),
536+
Name: roleName,
537+
OrganizationID: orgIDStr,
538+
DisplayName: role.DisplayName,
539+
SitePermissions: List(role.Site, RBACPermission),
540+
OrganizationPermissions: List(orgPerms, RBACPermission),
541+
UserPermissions: List(role.User, RBACPermission),
545542
}
546543
}
547544

548-
func Permission(permission rbac.Permission) codersdk.Permission {
549-
return codersdk.Permission{
550-
Negate: permission.Negate,
551-
ResourceType: codersdk.RBACResource(permission.ResourceType),
552-
Action: codersdk.RBACAction(permission.Action),
545+
func Role(role database.CustomRole) codersdk.Role {
546+
orgID := ""
547+
if role.OrganizationID.UUID != uuid.Nil {
548+
orgID = role.OrganizationID.UUID.String()
553549
}
554-
}
555550

556-
func RoleToRBAC(role codersdk.Role) rbac.Role {
557-
orgPerms := map[string][]rbac.Permission{}
558-
if role.OrganizationID != "" {
559-
orgPerms = map[string][]rbac.Permission{
560-
role.OrganizationID: List(role.OrganizationPermissions, PermissionToRBAC),
561-
}
551+
return codersdk.Role{
552+
Name: role.Name,
553+
OrganizationID: orgID,
554+
DisplayName: role.DisplayName,
555+
SitePermissions: List(role.SitePermissions, Permission),
556+
OrganizationPermissions: List(role.OrgPermissions, Permission),
557+
UserPermissions: List(role.UserPermissions, Permission),
562558
}
559+
}
563560

564-
return rbac.Role{
565-
Name: rbac.RoleName(role.Name, role.OrganizationID),
566-
DisplayName: role.DisplayName,
567-
Site: List(role.SitePermissions, PermissionToRBAC),
568-
Org: orgPerms,
569-
User: List(role.UserPermissions, PermissionToRBAC),
561+
func Permission(permission database.CustomRolePermission) codersdk.Permission {
562+
return codersdk.Permission{
563+
Negate: permission.Negate,
564+
ResourceType: codersdk.RBACResource(permission.ResourceType),
565+
Action: codersdk.RBACAction(permission.Action),
570566
}
571567
}
572568

573-
func PermissionToRBAC(permission codersdk.Permission) rbac.Permission {
574-
return rbac.Permission{
569+
func RBACPermission(permission rbac.Permission) codersdk.Permission {
570+
return codersdk.Permission{
575571
Negate: permission.Negate,
576-
ResourceType: string(permission.ResourceType),
577-
Action: policy.Action(permission.Action),
572+
ResourceType: codersdk.RBACResource(permission.ResourceType),
573+
Action: codersdk.RBACAction(permission.Action),
578574
}
579575
}

‎coderd/database/dbauthz/customroles_test.go

Lines changed: 72 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package dbauthz_test
22

33
import (
4-
"encoding/json"
54
"testing"
65

76
"github.com/google/uuid"
@@ -11,10 +10,12 @@ import (
1110
"cdr.dev/slog"
1211
"github.com/coder/coder/v2/coderd/coderdtest"
1312
"github.com/coder/coder/v2/coderd/database"
13+
"github.com/coder/coder/v2/coderd/database/db2sdk"
1414
"github.com/coder/coder/v2/coderd/database/dbauthz"
1515
"github.com/coder/coder/v2/coderd/database/dbmem"
1616
"github.com/coder/coder/v2/coderd/rbac"
1717
"github.com/coder/coder/v2/coderd/rbac/policy"
18+
"github.com/coder/coder/v2/codersdk"
1819
"github.com/coder/coder/v2/testutil"
1920
)
2021

@@ -60,17 +61,21 @@ func TestUpsertCustomRoles(t *testing.T) {
6061
return all
6162
}
6263

63-
orgID := uuid.New()
64+
orgID := uuid.NullUUID{
65+
UUID: uuid.New(),
66+
Valid: true,
67+
}
6468
testCases := []struct {
6569
name string
6670

6771
subject rbac.ExpandableRoles
6872

6973
// Perms to create on new custom role
70-
site []rbac.Permission
71-
org map[string][]rbac.Permission
72-
user []rbac.Permission
73-
errorContains string
74+
organizationID uuid.NullUUID
75+
site []codersdk.Permission
76+
org []codersdk.Permission
77+
user []codersdk.Permission
78+
errorContains string
7479
}{
7580
{
7681
// No roles, so no assign role
@@ -84,144 +89,129 @@ func TestUpsertCustomRoles(t *testing.T) {
8489
subject: merge(canAssignRole),
8590
},
8691
{
87-
name: "mixed-scopes",
88-
subject: merge(canAssignRole, rbac.RoleOwner()),
89-
site: rbac.Permissions(map[string][]policy.Action{
90-
rbac.ResourceWorkspace.Type: {policy.ActionRead},
92+
name: "mixed-scopes",
93+
subject: merge(canAssignRole, rbac.RoleOwner()),
94+
organizationID: orgID,
95+
site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
96+
codersdk.ResourceWorkspace: {codersdk.ActionRead},
97+
}),
98+
org: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
99+
codersdk.ResourceWorkspace: {codersdk.ActionRead},
91100
}),
92-
org: map[string][]rbac.Permission{
93-
uuid.New().String(): rbac.Permissions(map[string][]policy.Action{
94-
rbac.ResourceWorkspace.Type: {policy.ActionRead},
95-
}),
96-
},
97101
errorContains: "cannot assign both org and site permissions",
98102
},
99-
{
100-
name: "multiple-org",
101-
subject: merge(canAssignRole, rbac.RoleOwner()),
102-
org: map[string][]rbac.Permission{
103-
uuid.New().String(): rbac.Permissions(map[string][]policy.Action{
104-
rbac.ResourceWorkspace.Type: {policy.ActionRead},
105-
}),
106-
uuid.New().String(): rbac.Permissions(map[string][]policy.Action{
107-
rbac.ResourceWorkspace.Type: {policy.ActionRead},
108-
}),
109-
},
110-
errorContains: "cannot assign permissions to more than 1",
111-
},
112103
{
113104
name: "invalid-action",
114105
subject: merge(canAssignRole, rbac.RoleOwner()),
115-
site: rbac.Permissions(map[string][]policy.Action{
106+
site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
116107
// Action does not go with resource
117-
rbac.ResourceWorkspace.Type: {policy.ActionViewInsights},
108+
codersdk.ResourceWorkspace: {codersdk.ActionViewInsights},
118109
}),
119110
errorContains: "invalid action",
120111
},
121112
{
122113
name: "invalid-resource",
123114
subject: merge(canAssignRole, rbac.RoleOwner()),
124-
site: rbac.Permissions(map[string][]policy.Action{
125-
"foobar": {policy.ActionViewInsights},
115+
site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
116+
"foobar": {codersdk.ActionViewInsights},
126117
}),
127118
errorContains: "invalid resource",
128119
},
129120
{
130121
// Not allowing these at this time.
131122
name: "negative-permission",
132123
subject: merge(canAssignRole, rbac.RoleOwner()),
133-
site: []rbac.Permission{
124+
site: []codersdk.Permission{
134125
{
135126
Negate: true,
136-
ResourceType: rbac.ResourceWorkspace.Type,
137-
Action: policy.ActionRead,
127+
ResourceType: codersdk.ResourceWorkspace,
128+
Action: codersdk.ActionRead,
138129
},
139130
},
140131
errorContains: "no negative permissions",
141132
},
142133
{
143134
name: "wildcard", // not allowed
144135
subject: merge(canAssignRole, rbac.RoleOwner()),
145-
site: rbac.Permissions(map[string][]policy.Action{
146-
rbac.ResourceWorkspace.Type: {policy.WildcardSymbol},
136+
site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
137+
codersdk.ResourceWorkspace: {"*"},
147138
}),
148139
errorContains: "no wildcard symbols",
149140
},
150141
// escalation checks
151142
{
152143
name: "read-workspace-escalation",
153144
subject: merge(canAssignRole),
154-
site: rbac.Permissions(map[string][]policy.Action{
155-
rbac.ResourceWorkspace.Type: {policy.ActionRead},
145+
site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
146+
codersdk.ResourceWorkspace: {codersdk.ActionRead},
156147
}),
157148
errorContains: "not allowed to grant this permission",
158149
},
159150
{
160-
name: "read-workspace-outside-org",
161-
subject: merge(canAssignRole, rbac.RoleOrgAdmin(orgID)),
162-
org: map[string][]rbac.Permission{
163-
// The org admin is for a different org
164-
uuid.NewString(): rbac.Permissions(map[string][]policy.Action{
165-
rbac.ResourceWorkspace.Type: {policy.ActionRead},
166-
}),
151+
name: "read-workspace-outside-org",
152+
organizationID: uuid.NullUUID{
153+
UUID: uuid.New(),
154+
Valid: true,
167155
},
156+
subject: merge(canAssignRole, rbac.RoleOrgAdmin(orgID.UUID)),
157+
org: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
158+
codersdk.ResourceWorkspace: {codersdk.ActionRead},
159+
}),
168160
errorContains: "not allowed to grant this permission",
169161
},
170162
{
171163
name: "user-escalation",
172164
// These roles do not grant user perms
173-
subject: merge(canAssignRole, rbac.RoleOrgAdmin(orgID)),
174-
user: rbac.Permissions(map[string][]policy.Action{
175-
rbac.ResourceWorkspace.Type: {policy.ActionRead},
165+
subject: merge(canAssignRole, rbac.RoleOrgAdmin(orgID.UUID)),
166+
user: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
167+
codersdk.ResourceWorkspace: {codersdk.ActionRead},
176168
}),
177169
errorContains: "not allowed to grant this permission",
178170
},
179171
{
180172
name: "template-admin-escalation",
181173
subject: merge(canAssignRole, rbac.RoleTemplateAdmin()),
182-
site: rbac.Permissions(map[string][]policy.Action{
183-
rbac.ResourceWorkspace.Type: {policy.ActionRead}, // ok!
184-
rbac.ResourceDeploymentConfig.Type: {policy.ActionUpdate}, // not ok!
174+
site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
175+
codersdk.ResourceWorkspace: {codersdk.ActionRead}, // ok!
176+
codersdk.ResourceDeploymentConfig: {codersdk.ActionUpdate}, // not ok!
185177
}),
186-
user: rbac.Permissions(map[string][]policy.Action{
187-
rbac.ResourceWorkspace.Type: {policy.ActionRead}, // ok!
178+
user: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
179+
codersdk.ResourceWorkspace: {codersdk.ActionRead}, // ok!
188180
}),
189181
errorContains: "deployment_config",
190182
},
191183
// ok!
192184
{
193185
name: "read-workspace-template-admin",
194186
subject: merge(canAssignRole, rbac.RoleTemplateAdmin()),
195-
site: rbac.Permissions(map[string][]policy.Action{
196-
rbac.ResourceWorkspace.Type: {policy.ActionRead},
187+
site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
188+
codersdk.ResourceWorkspace: {codersdk.ActionRead},
197189
}),
198190
},
199191
{
200-
name: "read-workspace-in-org",
201-
subject: merge(canAssignRole, rbac.RoleOrgAdmin(orgID)),
202-
org: map[string][]rbac.Permission{
203-
// Org admin of this org, this is ok!
204-
orgID.String(): rbac.Permissions(map[string][]policy.Action{
205-
rbac.ResourceWorkspace.Type: {policy.ActionRead},
206-
}),
207-
},
192+
name: "read-workspace-in-org",
193+
subject: merge(canAssignRole, rbac.RoleOrgAdmin(orgID.UUID)),
194+
organizationID: orgID,
195+
org: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
196+
codersdk.ResourceWorkspace: {codersdk.ActionRead},
197+
}),
208198
},
209199
{
210200
name: "user-perms",
211201
// This is weird, but is ok
212202
subject: merge(canAssignRole, rbac.RoleMember()),
213-
user: rbac.Permissions(map[string][]policy.Action{
214-
rbac.ResourceWorkspace.Type: {policy.ActionRead},
203+
user: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
204+
codersdk.ResourceWorkspace: {codersdk.ActionRead},
215205
}),
216206
},
217207
{
218208
name: "site+user-perms",
219209
subject: merge(canAssignRole, rbac.RoleMember(), rbac.RoleTemplateAdmin()),
220-
site: rbac.Permissions(map[string][]policy.Action{
221-
rbac.ResourceWorkspace.Type: {policy.ActionRead},
210+
site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
211+
codersdk.ResourceWorkspace: {codersdk.ActionRead},
222212
}),
223-
user: rbac.Permissions(map[string][]policy.Action{
224-
rbac.ResourceWorkspace.Type: {policy.ActionRead},
213+
user: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
214+
codersdk.ResourceWorkspace: {codersdk.ActionRead},
225215
}),
226216
},
227217
}
@@ -244,9 +234,10 @@ func TestUpsertCustomRoles(t *testing.T) {
244234
_, err := az.UpsertCustomRole(ctx, database.UpsertCustomRoleParams{
245235
Name: "test-role",
246236
DisplayName: "",
247-
SitePermissions: must(json.Marshal(tc.site)),
248-
OrgPermissions: must(json.Marshal(tc.org)),
249-
UserPermissions: must(json.Marshal(tc.user)),
237+
OrganizationID: tc.organizationID,
238+
SitePermissions: db2sdk.List(tc.site, convertSDKPerm),
239+
OrgPermissions: db2sdk.List(tc.org, convertSDKPerm),
240+
UserPermissions: db2sdk.List(tc.user, convertSDKPerm),
250241
})
251242
if tc.errorContains != "" {
252243
require.ErrorContains(t, err, tc.errorContains)
@@ -256,3 +247,11 @@ func TestUpsertCustomRoles(t *testing.T) {
256247
})
257248
}
258249
}
250+
251+
func convertSDKPerm(perm codersdk.Permission) database.CustomRolePermission {
252+
return database.CustomRolePermission{
253+
Negate: perm.Negate,
254+
ResourceType: string(perm.ResourceType),
255+
Action: policy.Action(perm.Action),
256+
}
257+
}

0 commit comments

Comments
 (0)