Skip to content

Commit cf91eff

Browse files
EmyrkEmyrk
authored
chore: implement databased backend for custom roles (#13295)
Includes db schema and dbauthz layer for upserting custom roles. Unit test in `customroles_test.go` verify against escalating permissions through this feature.
1 parent 194be12 commit cf91eff

21 files changed

+854
-19
lines changed

‎coderd/database/dbauthz/customroles_test.go

+258
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,258 @@
1+
package dbauthz_test
2+
3+
import (
4+
"encoding/json"
5+
"testing"
6+
7+
"github.com/google/uuid"
8+
"github.com/prometheus/client_golang/prometheus"
9+
"github.com/stretchr/testify/require"
10+
11+
"cdr.dev/slog"
12+
"github.com/coder/coder/v2/coderd/coderdtest"
13+
"github.com/coder/coder/v2/coderd/database"
14+
"github.com/coder/coder/v2/coderd/database/dbauthz"
15+
"github.com/coder/coder/v2/coderd/database/dbmem"
16+
"github.com/coder/coder/v2/coderd/rbac"
17+
"github.com/coder/coder/v2/coderd/rbac/policy"
18+
"github.com/coder/coder/v2/testutil"
19+
)
20+
21+
// TestUpsertCustomRoles verifies creating custom roles cannot escalate permissions.
22+
func TestUpsertCustomRoles(t *testing.T) {
23+
t.Parallel()
24+
25+
userID := uuid.New()
26+
subjectFromRoles := func(roles rbac.ExpandableRoles) rbac.Subject {
27+
return rbac.Subject{
28+
FriendlyName: "Test user",
29+
ID: userID.String(),
30+
Roles: roles,
31+
Groups: nil,
32+
Scope: rbac.ScopeAll,
33+
}
34+
}
35+
36+
canAssignRole := rbac.Role{
37+
Name: "can-assign",
38+
DisplayName: "",
39+
Site: rbac.Permissions(map[string][]policy.Action{
40+
rbac.ResourceAssignRole.Type: {policy.ActionCreate},
41+
}),
42+
}
43+
44+
merge := func(u ...interface{}) rbac.Roles {
45+
all := make([]rbac.Role, 0)
46+
for _, v := range u {
47+
v := v
48+
switch t := v.(type) {
49+
case rbac.Role:
50+
all = append(all, t)
51+
case rbac.ExpandableRoles:
52+
all = append(all, must(t.Expand())...)
53+
case string:
54+
all = append(all, must(rbac.RoleByName(t)))
55+
default:
56+
panic("unknown type")
57+
}
58+
}
59+
60+
return all
61+
}
62+
63+
orgID := uuid.New()
64+
testCases := []struct {
65+
name string
66+
67+
subject rbac.ExpandableRoles
68+
69+
// 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+
}{
75+
{
76+
// No roles, so no assign role
77+
name: "no-roles",
78+
subject: rbac.RoleNames([]string{}),
79+
errorContains: "forbidden",
80+
},
81+
{
82+
// This works because the new role has 0 perms
83+
name: "empty",
84+
subject: merge(canAssignRole),
85+
},
86+
{
87+
name: "mixed-scopes",
88+
subject: merge(canAssignRole, rbac.RoleOwner()),
89+
site: rbac.Permissions(map[string][]policy.Action{
90+
rbac.ResourceWorkspace.Type: {policy.ActionRead},
91+
}),
92+
org: map[string][]rbac.Permission{
93+
uuid.New().String(): rbac.Permissions(map[string][]policy.Action{
94+
rbac.ResourceWorkspace.Type: {policy.ActionRead},
95+
}),
96+
},
97+
errorContains: "cannot assign both org and site permissions",
98+
},
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+
},
112+
{
113+
name: "invalid-action",
114+
subject: merge(canAssignRole, rbac.RoleOwner()),
115+
site: rbac.Permissions(map[string][]policy.Action{
116+
// Action does not go with resource
117+
rbac.ResourceWorkspace.Type: {policy.ActionViewInsights},
118+
}),
119+
errorContains: "invalid action",
120+
},
121+
{
122+
name: "invalid-resource",
123+
subject: merge(canAssignRole, rbac.RoleOwner()),
124+
site: rbac.Permissions(map[string][]policy.Action{
125+
"foobar": {policy.ActionViewInsights},
126+
}),
127+
errorContains: "invalid resource",
128+
},
129+
{
130+
// Not allowing these at this time.
131+
name: "negative-permission",
132+
subject: merge(canAssignRole, rbac.RoleOwner()),
133+
site: []rbac.Permission{
134+
{
135+
Negate: true,
136+
ResourceType: rbac.ResourceWorkspace.Type,
137+
Action: policy.ActionRead,
138+
},
139+
},
140+
errorContains: "no negative permissions",
141+
},
142+
{
143+
name: "wildcard", // not allowed
144+
subject: merge(canAssignRole, rbac.RoleOwner()),
145+
site: rbac.Permissions(map[string][]policy.Action{
146+
rbac.ResourceWorkspace.Type: {policy.WildcardSymbol},
147+
}),
148+
errorContains: "no wildcard symbols",
149+
},
150+
// escalation checks
151+
{
152+
name: "read-workspace-escalation",
153+
subject: merge(canAssignRole),
154+
site: rbac.Permissions(map[string][]policy.Action{
155+
rbac.ResourceWorkspace.Type: {policy.ActionRead},
156+
}),
157+
errorContains: "not allowed to grant this permission",
158+
},
159+
{
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+
}),
167+
},
168+
errorContains: "not allowed to grant this permission",
169+
},
170+
{
171+
name: "user-escalation",
172+
// 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},
176+
}),
177+
errorContains: "not allowed to grant this permission",
178+
},
179+
{
180+
name: "template-admin-escalation",
181+
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!
185+
}),
186+
user: rbac.Permissions(map[string][]policy.Action{
187+
rbac.ResourceWorkspace.Type: {policy.ActionRead}, // ok!
188+
}),
189+
errorContains: "deployment_config",
190+
},
191+
// ok!
192+
{
193+
name: "read-workspace-template-admin",
194+
subject: merge(canAssignRole, rbac.RoleTemplateAdmin()),
195+
site: rbac.Permissions(map[string][]policy.Action{
196+
rbac.ResourceWorkspace.Type: {policy.ActionRead},
197+
}),
198+
},
199+
{
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+
},
208+
},
209+
{
210+
name: "user-perms",
211+
// This is weird, but is ok
212+
subject: merge(canAssignRole, rbac.RoleMember()),
213+
user: rbac.Permissions(map[string][]policy.Action{
214+
rbac.ResourceWorkspace.Type: {policy.ActionRead},
215+
}),
216+
},
217+
{
218+
name: "site+user-perms",
219+
subject: merge(canAssignRole, rbac.RoleMember(), rbac.RoleTemplateAdmin()),
220+
site: rbac.Permissions(map[string][]policy.Action{
221+
rbac.ResourceWorkspace.Type: {policy.ActionRead},
222+
}),
223+
user: rbac.Permissions(map[string][]policy.Action{
224+
rbac.ResourceWorkspace.Type: {policy.ActionRead},
225+
}),
226+
},
227+
}
228+
229+
for _, tc := range testCases {
230+
tc := tc
231+
232+
t.Run(tc.name, func(t *testing.T) {
233+
t.Parallel()
234+
db := dbmem.New()
235+
rec := &coderdtest.RecordingAuthorizer{
236+
Wrapped: rbac.NewAuthorizer(prometheus.NewRegistry()),
237+
}
238+
az := dbauthz.New(db, rec, slog.Make(), coderdtest.AccessControlStorePointer())
239+
240+
subject := subjectFromRoles(tc.subject)
241+
ctx := testutil.Context(t, testutil.WaitMedium)
242+
ctx = dbauthz.As(ctx, subject)
243+
244+
_, err := az.UpsertCustomRole(ctx, database.UpsertCustomRoleParams{
245+
Name: "test-role",
246+
DisplayName: "",
247+
SitePermissions: must(json.Marshal(tc.site)),
248+
OrgPermissions: must(json.Marshal(tc.org)),
249+
UserPermissions: must(json.Marshal(tc.user)),
250+
})
251+
if tc.errorContains != "" {
252+
require.ErrorContains(t, err, tc.errorContains)
253+
} else {
254+
require.NoError(t, err)
255+
}
256+
})
257+
}
258+
}

0 commit comments

Comments
 (0)