Skip to content

Commit d60f65a

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

File tree

13 files changed

+94
-84
lines changed

13 files changed

+94
-84
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: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ var (
169169
rbac.ResourceTemplate.Type: {policy.ActionRead, policy.ActionUpdate},
170170
// Unsure why provisionerd needs update and read personal
171171
rbac.ResourceUser.Type: {policy.ActionRead, policy.ActionReadPersonal, policy.ActionUpdatePersonal},
172-
rbac.ResourceWorkspace.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionDelete, policy.ActionWorkspaceBuild},
172+
rbac.ResourceWorkspace.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionDelete, policy.ActionWorkspaceStart, policy.ActionWorkspaceStop},
173173
rbac.ResourceApiKey.Type: {policy.WildcardSymbol},
174174
// When org scoped provisioner credentials are implemented,
175175
// this can be reduced to read a specific org.
@@ -193,7 +193,7 @@ var (
193193
Site: rbac.Permissions(map[string][]policy.Action{
194194
rbac.ResourceSystem.Type: {policy.WildcardSymbol},
195195
rbac.ResourceTemplate.Type: {policy.ActionRead, policy.ActionUpdate},
196-
rbac.ResourceWorkspace.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionWorkspaceBuild},
196+
rbac.ResourceWorkspace.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionWorkspaceStart, policy.ActionWorkspaceStop},
197197
rbac.ResourceUser.Type: {policy.ActionRead},
198198
}),
199199
Org: map[string][]rbac.Permission{},
@@ -232,7 +232,7 @@ var (
232232
DisplayName: "Coder",
233233
Site: rbac.Permissions(map[string][]policy.Action{
234234
rbac.ResourceWildcard.Type: {policy.ActionRead},
235-
rbac.ResourceApiKey.Type: {policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete},
235+
rbac.ResourceApiKey.Type: rbac.ResourceApiKey.AvailableActions(),
236236
rbac.ResourceGroup.Type: {policy.ActionCreate, policy.ActionUpdate},
237237
rbac.ResourceAssignRole.Type: rbac.ResourceAssignRole.AvailableActions(),
238238
rbac.ResourceSystem.Type: {policy.WildcardSymbol},
@@ -241,7 +241,7 @@ var (
241241
rbac.ResourceAssignOrgRole.Type: {policy.ActionRead, policy.ActionCreate, policy.ActionDelete},
242242
rbac.ResourceProvisionerDaemon.Type: {policy.ActionCreate, policy.ActionUpdate},
243243
rbac.ResourceUser.Type: rbac.ResourceUser.AvailableActions(),
244-
rbac.ResourceWorkspace.Type: {policy.ActionUpdate, policy.ActionDelete, policy.ActionWorkspaceBuild, policy.ActionSSH},
244+
rbac.ResourceWorkspace.Type: {policy.ActionUpdate, policy.ActionDelete, policy.ActionWorkspaceStart, policy.ActionWorkspaceStop, policy.ActionSSH},
245245
rbac.ResourceWorkspaceProxy.Type: {policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete},
246246
}),
247247
Org: map[string][]rbac.Permission{},
@@ -2531,9 +2531,11 @@ func (q *querier) InsertWorkspaceBuild(ctx context.Context, arg database.InsertW
25312531
return xerrors.Errorf("get workspace by id: %w", err)
25322532
}
25332533

2534-
var action policy.Action = policy.ActionWorkspaceBuild
2534+
var action policy.Action = policy.ActionWorkspaceStart
25352535
if arg.Transition == database.WorkspaceTransitionDelete {
25362536
action = policy.ActionDelete
2537+
} else if arg.Transition == database.WorkspaceTransitionStop {
2538+
action = policy.ActionWorkspaceStop
25372539
}
25382540

25392541
if err = q.authorizeContext(ctx, action, w); err != nil {
@@ -3280,7 +3282,7 @@ func (q *querier) UpsertAppSecurityKey(ctx context.Context, data string) error {
32803282
}
32813283

32823284
func (q *querier) UpsertApplicationName(ctx context.Context, value string) error {
3283-
if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceDeploymentConfig); err != nil {
3285+
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceDeploymentConfig); err != nil {
32843286
return err
32853287
}
32863288
return q.db.UpsertApplicationName(ctx, value)
@@ -3294,7 +3296,7 @@ func (q *querier) UpsertDefaultProxy(ctx context.Context, arg database.UpsertDef
32943296
}
32953297

32963298
func (q *querier) UpsertHealthSettings(ctx context.Context, value string) error {
3297-
if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceDeploymentConfig); err != nil {
3299+
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceDeploymentConfig); err != nil {
32983300
return err
32993301
}
33003302
return q.db.UpsertHealthSettings(ctx, value)

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -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) {

‎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: 17 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,22 @@
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":"build",
3+
"object":{
4+
"id":"c6ae3dfb-0e55-4eca-b263-8ceaf95ad056",
5+
"owner":"76df8ecb-a179-45cc-9a67-acc8b0512ee5",
6+
"org_owner":"021dcc96-562b-4a43-8405-0d287b5298df",
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": [
16-
{
17-
"name": "owner",
18-
"display_name": "Owner",
19-
"site": [
20-
{
21-
"negate": false,
22-
"resource_type": "*",
23-
"action": "*"
24-
}
25-
],
26-
"org": {},
27-
"user": []
28-
}
11+
"subject":{
12+
"FriendlyName":"testuser",
13+
"ID":"76df8ecb-a179-45cc-9a67-acc8b0512ee5",
14+
"Roles":[
15+
"owner",
16+
"member",
17+
"organization-member:021dcc96-562b-4a43-8405-0d287b5298df"
2918
],
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-
}
19+
"Groups":null,
20+
"Scope":"all"
4521
}
4622
}

‎coderd/rbac/object.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func (z Object) ValidAction(action policy.Action) error {
3939
return fmt.Errorf("invalid type %q", z.Type)
4040
}
4141
if _, ok := perms.Actions[action]; !ok {
42-
return fmt.Errorf("invalid action %q", action)
42+
return fmt.Errorf("invalid action %q for type %q", action, z.Type)
4343
}
4444

4545
return nil

‎coderd/rbac/object_gen.go

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/rbac/policy/policy.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ const (
1616
ActionApplicationConnect Action = "application_connect"
1717
ActionViewInsights Action = "view_insights"
1818

19-
ActionWorkspaceBuild Action = "build"
19+
ActionWorkspaceStart Action = "start"
20+
ActionWorkspaceStop Action = "stop"
2021

2122
ActionAssign Action = "assign"
2223

@@ -51,8 +52,10 @@ var workspaceActions = map[Action]ActionDefinition{
5152
ActionUpdate: actDef("edit workspace settings (scheduling, permissions, parameters)"),
5253
ActionDelete: actDef("delete workspace"),
5354

54-
// Workspace provisioning
55-
ActionWorkspaceBuild: actDef("allows starting, stopping, and updating a workspace"),
55+
// Workspace provisioning. Start & stop are different so dormant workspaces can be
56+
// stopped, but not stared.
57+
ActionWorkspaceStart: actDef("allows starting a workspace"),
58+
ActionWorkspaceStop: actDef("allows stopping a workspace"),
5659

5760
// Running a workspace
5861
ActionSSH: actDef("ssh into a given workspace"),
@@ -104,12 +107,14 @@ var RBACPermissions = map[string]PermissionDefinition{
104107
},
105108
"audit_log": {
106109
Actions: map[Action]ActionDefinition{
107-
ActionRead: actDef("read audit logs"),
110+
ActionRead: actDef("read audit logs"),
111+
ActionCreate: actDef("create new audit log entries"),
108112
},
109113
},
110114
"deployment_config": {
111115
Actions: map[Action]ActionDefinition{
112-
ActionRead: actDef("read deployment config"),
116+
ActionRead: actDef("read deployment config"),
117+
ActionUpdate: actDef("updating health information"),
113118
},
114119
},
115120
"deployment_stats": {
@@ -173,7 +178,7 @@ var RBACPermissions = map[string]PermissionDefinition{
173178
},
174179
"debug_info": {
175180
Actions: map[Action]ActionDefinition{
176-
ActionUse: actDef("access to debug routes"),
181+
ActionRead: actDef("access to debug routes"),
177182
},
178183
},
179184
"system": {
@@ -189,6 +194,7 @@ var RBACPermissions = map[string]PermissionDefinition{
189194
ActionCreate: actDef("create an api key"),
190195
ActionRead: actDef("read api key details (secrets are not stored)"),
191196
ActionDelete: actDef("delete an api key"),
197+
ActionUpdate: actDef("update an api key, eg expires"),
192198
},
193199
},
194200
"tailnet_coordinator": {

‎coderd/rbac/roles.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
146146
// This adds back in the Workspace permissions.
147147
Permissions(map[string][]policy.Action{
148148
ResourceWorkspace.Type: ownerWorkspaceActions,
149-
ResourceWorkspaceDormant.Type: {policy.ActionRead, policy.ActionDelete, policy.ActionCreate, policy.ActionUpdate},
149+
ResourceWorkspaceDormant.Type: {policy.ActionRead, policy.ActionDelete, policy.ActionCreate, policy.ActionUpdate, policy.ActionWorkspaceStop},
150150
})...),
151151
Org: map[string][]Permission{},
152152
User: []Permission{},
@@ -167,7 +167,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
167167
User: append(allPermsExcept(ResourceWorkspaceDormant, ResourceUser, ResourceOrganizationMember),
168168
Permissions(map[string][]policy.Action{
169169
// Reduced permission set on dormant workspaces. No build, ssh, or exec
170-
ResourceWorkspaceDormant.Type: {policy.ActionRead, policy.ActionDelete, policy.ActionCreate, policy.ActionUpdate},
170+
ResourceWorkspaceDormant.Type: {policy.ActionRead, policy.ActionDelete, policy.ActionCreate, policy.ActionUpdate, policy.ActionWorkspaceStop},
171171

172172
// Users cannot do create/update/delete on themselves, but they
173173
// can read their own details.
@@ -272,7 +272,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
272272
Org: map[string][]Permission{
273273
// Org admins should not have workspace exec perms.
274274
organizationID: append(allPermsExcept(ResourceWorkspace, ResourceWorkspaceDormant), Permissions(map[string][]policy.Action{
275-
ResourceWorkspaceDormant.Type: {policy.ActionRead, policy.ActionDelete, policy.ActionCreate, policy.ActionUpdate},
275+
ResourceWorkspaceDormant.Type: {policy.ActionRead, policy.ActionDelete, policy.ActionCreate, policy.ActionUpdate, policy.ActionWorkspaceStop},
276276
ResourceWorkspace.Type: slice.Omit(ResourceWorkspace.AvailableActions(), policy.ActionApplicationConnect, policy.ActionSSH),
277277
})...),
278278
},

0 commit comments

Comments
 (0)