Skip to content

Commit 44f9203

Browse files
EmyrkEmyrk
committed
make start/stop workspaces different actions
1 parent dda1b4f commit 44f9203

File tree

13 files changed

+202
-90
lines changed

13 files changed

+202
-90
lines changed

‎coderd/coderdtest/coderdtest.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
221221
}
222222

223223
if options.Authorizer == nil {
224-
defAuth := rbac.NewCachingAuthorizer(prometheus.NewRegistry())
224+
defAuth := rbac.NewStrictCachingAuthorizer(prometheus.NewRegistry())
225225
if _, ok := t.(*testing.T); ok {
226226
options.Authorizer = &RecordingAuthorizer{
227227
Wrapped: defAuth,

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -168,9 +168,10 @@ var (
168168
rbac.ResourceSystem.Type: {policy.WildcardSymbol},
169169
rbac.ResourceTemplate.Type: {policy.ActionRead, policy.ActionUpdate},
170170
// Unsure why provisionerd needs update and read personal
171-
rbac.ResourceUser.Type: {policy.ActionRead, policy.ActionReadPersonal, policy.ActionUpdatePersonal},
172-
rbac.ResourceWorkspace.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionDelete, policy.ActionWorkspaceBuild},
173-
rbac.ResourceApiKey.Type: {policy.WildcardSymbol},
171+
rbac.ResourceUser.Type: {policy.ActionRead, policy.ActionReadPersonal, policy.ActionUpdatePersonal},
172+
rbac.ResourceWorkspaceDormant.Type: {policy.ActionDelete, policy.ActionRead, policy.ActionUpdate, policy.ActionWorkspaceStop},
173+
rbac.ResourceWorkspace.Type: {policy.ActionDelete, policy.ActionRead, policy.ActionUpdate, policy.ActionWorkspaceStart, policy.ActionWorkspaceStop},
174+
rbac.ResourceApiKey.Type: {policy.WildcardSymbol},
174175
// When org scoped provisioner credentials are implemented,
175176
// this can be reduced to read a specific org.
176177
rbac.ResourceOrganization.Type: {policy.ActionRead},
@@ -191,10 +192,11 @@ var (
191192
Name: "autostart",
192193
DisplayName: "Autostart Daemon",
193194
Site: rbac.Permissions(map[string][]policy.Action{
194-
rbac.ResourceSystem.Type: {policy.WildcardSymbol},
195-
rbac.ResourceTemplate.Type: {policy.ActionRead, policy.ActionUpdate},
196-
rbac.ResourceWorkspace.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionWorkspaceBuild},
197-
rbac.ResourceUser.Type: {policy.ActionRead},
195+
rbac.ResourceSystem.Type: {policy.WildcardSymbol},
196+
rbac.ResourceTemplate.Type: {policy.ActionRead, policy.ActionUpdate},
197+
rbac.ResourceWorkspaceDormant.Type: {policy.ActionDelete, policy.ActionRead, policy.ActionUpdate, policy.ActionWorkspaceStop},
198+
rbac.ResourceWorkspace.Type: {policy.ActionDelete, policy.ActionRead, policy.ActionUpdate, policy.ActionWorkspaceStart, policy.ActionWorkspaceStop},
199+
rbac.ResourceUser.Type: {policy.ActionRead},
198200
}),
199201
Org: map[string][]rbac.Permission{},
200202
User: []rbac.Permission{},
@@ -232,7 +234,7 @@ var (
232234
DisplayName: "Coder",
233235
Site: rbac.Permissions(map[string][]policy.Action{
234236
rbac.ResourceWildcard.Type: {policy.ActionRead},
235-
rbac.ResourceApiKey.Type: {policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete},
237+
rbac.ResourceApiKey.Type: rbac.ResourceApiKey.AvailableActions(),
236238
rbac.ResourceGroup.Type: {policy.ActionCreate, policy.ActionUpdate},
237239
rbac.ResourceAssignRole.Type: rbac.ResourceAssignRole.AvailableActions(),
238240
rbac.ResourceSystem.Type: {policy.WildcardSymbol},
@@ -241,7 +243,8 @@ var (
241243
rbac.ResourceAssignOrgRole.Type: {policy.ActionRead, policy.ActionCreate, policy.ActionDelete},
242244
rbac.ResourceProvisionerDaemon.Type: {policy.ActionCreate, policy.ActionUpdate},
243245
rbac.ResourceUser.Type: rbac.ResourceUser.AvailableActions(),
244-
rbac.ResourceWorkspace.Type: {policy.ActionUpdate, policy.ActionDelete, policy.ActionWorkspaceBuild, policy.ActionSSH},
246+
rbac.ResourceWorkspaceDormant.Type: {policy.ActionUpdate, policy.ActionDelete, policy.ActionWorkspaceStop},
247+
rbac.ResourceWorkspace.Type: {policy.ActionUpdate, policy.ActionDelete, policy.ActionWorkspaceStart, policy.ActionWorkspaceStop, policy.ActionSSH},
245248
rbac.ResourceWorkspaceProxy.Type: {policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete},
246249
}),
247250
Org: map[string][]rbac.Permission{},
@@ -2531,9 +2534,11 @@ func (q *querier) InsertWorkspaceBuild(ctx context.Context, arg database.InsertW
25312534
return xerrors.Errorf("get workspace by id: %w", err)
25322535
}
25332536

2534-
var action policy.Action = policy.ActionWorkspaceBuild
2537+
var action policy.Action = policy.ActionWorkspaceStart
25352538
if arg.Transition == database.WorkspaceTransitionDelete {
25362539
action = policy.ActionDelete
2540+
} else if arg.Transition == database.WorkspaceTransitionStop {
2541+
action = policy.ActionWorkspaceStop
25372542
}
25382543

25392544
if err = q.authorizeContext(ctx, action, w); err != nil {
@@ -3280,7 +3285,7 @@ func (q *querier) UpsertAppSecurityKey(ctx context.Context, data string) error {
32803285
}
32813286

32823287
func (q *querier) UpsertApplicationName(ctx context.Context, value string) error {
3283-
if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceDeploymentConfig); err != nil {
3288+
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceDeploymentConfig); err != nil {
32843289
return err
32853290
}
32863291
return q.db.UpsertApplicationName(ctx, value)
@@ -3294,7 +3299,7 @@ func (q *querier) UpsertDefaultProxy(ctx context.Context, arg database.UpsertDef
32943299
}
32953300

32963301
func (q *querier) UpsertHealthSettings(ctx context.Context, value string) error {
3297-
if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceDeploymentConfig); err != nil {
3302+
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceDeploymentConfig); err != nil {
32983303
return err
32993304
}
33003305
return q.db.UpsertHealthSettings(ctx, value)
@@ -3329,14 +3334,14 @@ func (q *querier) UpsertLastUpdateCheck(ctx context.Context, value string) error
33293334
}
33303335

33313336
func (q *querier) UpsertLogoURL(ctx context.Context, value string) error {
3332-
if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceDeploymentConfig); err != nil {
3337+
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceDeploymentConfig); err != nil {
33333338
return err
33343339
}
33353340
return q.db.UpsertLogoURL(ctx, value)
33363341
}
33373342

33383343
func (q *querier) UpsertNotificationBanners(ctx context.Context, value string) error {
3339-
if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceDeploymentConfig); err != nil {
3344+
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceDeploymentConfig); err != nil {
33403345
return err
33413346
}
33423347
return q.db.UpsertNotificationBanners(ctx, value)

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -526,10 +526,10 @@ func (s *MethodTestSuite) TestLicense() {
526526
Asserts(rbac.ResourceLicense, policy.ActionCreate)
527527
}))
528528
s.Run("UpsertLogoURL", s.Subtest(func(db database.Store, check *expects) {
529-
check.Args("value").Asserts(rbac.ResourceDeploymentConfig, policy.ActionCreate)
529+
check.Args("value").Asserts(rbac.ResourceDeploymentConfig, policy.ActionUpdate)
530530
}))
531531
s.Run("UpsertNotificationBanners", s.Subtest(func(db database.Store, check *expects) {
532-
check.Args("value").Asserts(rbac.ResourceDeploymentConfig, policy.ActionCreate)
532+
check.Args("value").Asserts(rbac.ResourceDeploymentConfig, policy.ActionUpdate)
533533
}))
534534
s.Run("GetLicenseByID", s.Subtest(func(db database.Store, check *expects) {
535535
l, err := db.InsertLicense(context.Background(), database.InsertLicenseParams{
@@ -1432,7 +1432,18 @@ func (s *MethodTestSuite) TestWorkspace() {
14321432
WorkspaceID: w.ID,
14331433
Transition: database.WorkspaceTransitionStart,
14341434
Reason: database.BuildReasonInitiator,
1435-
}).Asserts(w, policy.ActionWorkspaceBuild)
1435+
}).Asserts(w, policy.ActionWorkspaceStart)
1436+
}))
1437+
s.Run("Stop/InsertWorkspaceBuild", s.Subtest(func(db database.Store, check *expects) {
1438+
t := dbgen.Template(s.T(), db, database.Template{})
1439+
w := dbgen.Workspace(s.T(), db, database.Workspace{
1440+
TemplateID: t.ID,
1441+
})
1442+
check.Args(database.InsertWorkspaceBuildParams{
1443+
WorkspaceID: w.ID,
1444+
Transition: database.WorkspaceTransitionStop,
1445+
Reason: database.BuildReasonInitiator,
1446+
}).Asserts(w, policy.ActionWorkspaceStop)
14361447
}))
14371448
s.Run("Start/RequireActiveVersion/VersionMismatch/InsertWorkspaceBuild", s.Subtest(func(db database.Store, check *expects) {
14381449
t := dbgen.Template(s.T(), db, database.Template{})
@@ -1454,7 +1465,7 @@ func (s *MethodTestSuite) TestWorkspace() {
14541465
Reason: database.BuildReasonInitiator,
14551466
TemplateVersionID: v.ID,
14561467
}).Asserts(
1457-
w, policy.ActionWorkspaceBuild,
1468+
w, policy.ActionWorkspaceStart,
14581469
t, policy.ActionUpdate,
14591470
)
14601471
}))
@@ -1482,7 +1493,7 @@ func (s *MethodTestSuite) TestWorkspace() {
14821493
Reason: database.BuildReasonInitiator,
14831494
TemplateVersionID: v.ID,
14841495
}).Asserts(
1485-
w, policy.ActionWorkspaceBuild,
1496+
w, policy.ActionWorkspaceStart,
14861497
)
14871498
}))
14881499
s.Run("Delete/InsertWorkspaceBuild", s.Subtest(func(db database.Store, check *expects) {
@@ -2206,13 +2217,13 @@ func (s *MethodTestSuite) TestSystemFunctions() {
22062217
check.Args().Asserts()
22072218
}))
22082219
s.Run("UpsertApplicationName", s.Subtest(func(db database.Store, check *expects) {
2209-
check.Args("").Asserts(rbac.ResourceDeploymentConfig, policy.ActionCreate)
2220+
check.Args("").Asserts(rbac.ResourceDeploymentConfig, policy.ActionUpdate)
22102221
}))
22112222
s.Run("GetHealthSettings", s.Subtest(func(db database.Store, check *expects) {
22122223
check.Args().Asserts()
22132224
}))
22142225
s.Run("UpsertHealthSettings", s.Subtest(func(db database.Store, check *expects) {
2215-
check.Args("foo").Asserts(rbac.ResourceDeploymentConfig, policy.ActionCreate)
2226+
check.Args("foo").Asserts(rbac.ResourceDeploymentConfig, policy.ActionUpdate)
22162227
}))
22172228
s.Run("GetDeploymentWorkspaceAgentStats", s.Subtest(func(db database.Store, check *expects) {
22182229
check.Args(time.Time{}).Asserts()

‎coderd/rbac/authz.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,10 @@ type RegoAuthorizer struct {
214214

215215
authorizeHist *prometheus.HistogramVec
216216
prepareHist prometheus.Histogram
217+
218+
// strict checking also verifies the inputs to the authorizer. Making sure
219+
// the action make sense for the input object.
220+
strict bool
217221
}
218222

219223
var _ Authorizer = (*RegoAuthorizer)(nil)
@@ -235,6 +239,13 @@ func NewCachingAuthorizer(registry prometheus.Registerer) Authorizer {
235239
return Cacher(NewAuthorizer(registry))
236240
}
237241

242+
// NewStrictCachingAuthorizer is mainly just for testing.
243+
func NewStrictCachingAuthorizer(registry prometheus.Registerer) Authorizer {
244+
auth := NewAuthorizer(registry)
245+
auth.strict = true
246+
return Cacher(auth)
247+
}
248+
238249
func NewAuthorizer(registry prometheus.Registerer) *RegoAuthorizer {
239250
queryOnce.Do(func() {
240251
var err error
@@ -321,6 +332,12 @@ type authSubject struct {
321332
// the object.
322333
// If an error is returned, the authorization is denied.
323334
func (a RegoAuthorizer) Authorize(ctx context.Context, subject Subject, action policy.Action, object Object) error {
335+
if a.strict {
336+
if err := object.ValidAction(action); err != nil {
337+
return xerrors.Errorf("strict authz check: %w", err)
338+
}
339+
}
340+
324341
start := time.Now()
325342
ctx, span := tracing.StartSpan(ctx,
326343
trace.WithTimestamp(start), // Reuse the time.Now for metric and trace

‎coderd/rbac/authz_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ func BenchmarkRBACAuthorize(b *testing.B) {
160160

161161
// There is no caching that occurs because a fresh context is used for each
162162
// call. And the context needs 'WithCacheCtx' to work.
163-
authorizer := rbac.NewCachingAuthorizer(prometheus.NewRegistry())
163+
authorizer := rbac.NewStrictCachingAuthorizer(prometheus.NewRegistry())
164164
// This benchmarks all the simple cases using just user permissions. Groups
165165
// are added as noise, but do not do anything.
166166
for _, c := range benchCases {
@@ -187,7 +187,7 @@ func BenchmarkRBACAuthorizeGroups(b *testing.B) {
187187
uuid.MustParse("0632b012-49e0-4d70-a5b3-f4398f1dcd52"),
188188
uuid.MustParse("70dbaa7a-ea9c-4f68-a781-97b08af8461d"),
189189
)
190-
authorizer := rbac.NewCachingAuthorizer(prometheus.NewRegistry())
190+
authorizer := rbac.NewStrictCachingAuthorizer(prometheus.NewRegistry())
191191

192192
// Same benchmark cases, but this time groups will be used to match.
193193
// Some '*' permissions will still match, but using a fake action reduces
@@ -239,7 +239,7 @@ func BenchmarkRBACFilter(b *testing.B) {
239239
uuid.MustParse("70dbaa7a-ea9c-4f68-a781-97b08af8461d"),
240240
)
241241

242-
authorizer := rbac.NewCachingAuthorizer(prometheus.NewRegistry())
242+
authorizer := rbac.NewStrictCachingAuthorizer(prometheus.NewRegistry())
243243

244244
for _, c := range benchCases {
245245
b.Run("PrepareOnly-"+c.Name, func(b *testing.B) {

‎coderd/rbac/input.json

Lines changed: 111 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,121 @@
11
{
2-
"action": "never-match-action",
3-
"object": {
4-
"id": "9046b041-58ed-47a3-9c3a-de302577875a",
5-
"owner": "00000000-0000-0000-0000-000000000000",
6-
"org_owner": "bf7b72bd-a2b1-4ef2-962c-1d698e0483f6",
7-
"type": "workspace",
8-
"acl_user_list": {
9-
"f041847d-711b-40da-a89a-ede39f70dc7f": ["create"]
10-
},
11-
"acl_group_list": {}
2+
"action":"delete",
3+
"object":{
4+
"id":"bf9bba2a-dd6b-4a07-8500-1dac6ddc5171",
5+
"owner":"36e3c780-2ae3-443b-ad3c-ceb410a55d12",
6+
"org_owner":"19285bb2-022d-41c6-a7f2-8ace10825691",
7+
"type":"workspace_dormant",
8+
"acl_user_list":null,
9+
"acl_group_list":null
1210
},
13-
"subject": {
14-
"id": "10d03e62-7703-4df5-a358-4f76577d4e2f",
15-
"roles": [
11+
"subject":{
12+
"FriendlyName":"Provisioner Daemon",
13+
"ID":"00000000-0000-0000-0000-000000000000",
14+
"Roles":[
1615
{
17-
"name": "owner",
18-
"display_name": "Owner",
19-
"site": [
16+
"name":"provisionerd",
17+
"display_name":"Provisioner Daemon",
18+
"site":[
2019
{
21-
"negate": false,
22-
"resource_type": "*",
23-
"action": "*"
20+
"negate":false,
21+
"resource_type":"api_key",
22+
"action":"*"
23+
},
24+
{
25+
"negate":false,
26+
"resource_type":"file",
27+
"action":"read"
28+
},
29+
{
30+
"negate":false,
31+
"resource_type":"group",
32+
"action":"read"
33+
},
34+
{
35+
"negate":false,
36+
"resource_type":"organization",
37+
"action":"read"
38+
},
39+
{
40+
"negate":false,
41+
"resource_type":"system",
42+
"action":"*"
43+
},
44+
{
45+
"negate":false,
46+
"resource_type":"template",
47+
"action":"read"
48+
},
49+
{
50+
"negate":false,
51+
"resource_type":"template",
52+
"action":"update"
53+
},
54+
{
55+
"negate":false,
56+
"resource_type":"user",
57+
"action":"read_personal"
58+
},
59+
{
60+
"negate":false,
61+
"resource_type":"user",
62+
"action":"update_personal"
63+
},
64+
{
65+
"negate":false,
66+
"resource_type":"user",
67+
"action":"read"
68+
},
69+
{
70+
"negate":false,
71+
"resource_type":"workspace",
72+
"action":"read"
73+
},
74+
{
75+
"negate":false,
76+
"resource_type":"workspace",
77+
"action":"stop"
78+
},
79+
{
80+
"negate":false,
81+
"resource_type":"workspace",
82+
"action":"start"
83+
},
84+
{
85+
"negate":false,
86+
"resource_type":"workspace",
87+
"action":"update"
88+
},
89+
{
90+
"negate":false,
91+
"resource_type":"workspace",
92+
"action":"delete"
93+
},
94+
{
95+
"negate":false,
96+
"resource_type":"workspace_dormant",
97+
"action":"read"
98+
},
99+
{
100+
"negate":false,
101+
"resource_type":"workspace_dormant",
102+
"action":"update"
103+
},
104+
{
105+
"negate":false,
106+
"resource_type":"workspace_dormant",
107+
"action":"stop"
24108
}
25109
],
26-
"org": {},
27-
"user": []
110+
"org":{
111+
112+
},
113+
"user":[
114+
115+
]
28116
}
29117
],
30-
"groups": ["b617a647-b5d0-4cbe-9e40-26f89710bf18"],
31-
"scope": {
32-
"name": "Scope_all",
33-
"display_name": "All operations",
34-
"site": [
35-
{
36-
"negate": false,
37-
"resource_type": "*",
38-
"action": "*"
39-
}
40-
],
41-
"org": {},
42-
"user": [],
43-
"allow_list": ["*"]
44-
}
118+
"Groups":null,
119+
"Scope":"all"
45120
}
46121
}

0 commit comments

Comments
 (0)