Skip to content

Commit 44423b3

Browse files
EmyrkEmyrk
committed
chore: Drop resource_id support in rbac system
resource_id was implemented to support: - Workspace agent tokens - Roles as a resource - Workspace sharing The additional complexity is not required and will ease support of rbac moving forward.
1 parent c0cc8b9 commit 44423b3

15 files changed

+155
-339
lines changed

‎coderd/coderd_test.go

+25-25
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
215215
authorizer.AlwaysReturn = rbac.ForbiddenWithInternal(xerrors.New("fake implementation"), nil, nil)
216216

217217
// Some quick reused objects
218-
workspaceRBACObj := rbac.ResourceWorkspace.InOrg(organization.ID).WithID(workspace.ID.String()).WithOwner(workspace.OwnerID.String())
218+
workspaceRBACObj := rbac.ResourceWorkspace.InOrg(organization.ID).WithOwner(workspace.OwnerID.String())
219219

220220
// skipRoutes allows skipping routes from being checked.
221221
skipRoutes := map[string]string{
@@ -342,107 +342,107 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
342342
"GET:/api/v2/organizations/{organization}/templates": {
343343
StatusCode: http.StatusOK,
344344
AssertAction: rbac.ActionRead,
345-
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
345+
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID),
346346
},
347347
"POST:/api/v2/organizations/{organization}/templates": {
348348
AssertAction: rbac.ActionCreate,
349349
AssertObject: rbac.ResourceTemplate.InOrg(organization.ID),
350350
},
351351
"DELETE:/api/v2/templates/{template}": {
352352
AssertAction: rbac.ActionDelete,
353-
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
353+
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID),
354354
},
355355
"GET:/api/v2/templates/{template}": {
356356
AssertAction: rbac.ActionRead,
357-
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
357+
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID),
358358
},
359359
"POST:/api/v2/files": {AssertAction: rbac.ActionCreate, AssertObject: rbac.ResourceFile},
360360
"GET:/api/v2/files/{fileHash}": {
361361
AssertAction: rbac.ActionRead,
362-
AssertObject: rbac.ResourceFile.WithOwner(admin.UserID.String()).WithID(file.Hash),
362+
AssertObject: rbac.ResourceFile.WithOwner(admin.UserID.String()),
363363
},
364364
"GET:/api/v2/templates/{template}/versions": {
365365
AssertAction: rbac.ActionRead,
366-
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
366+
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID),
367367
},
368368
"PATCH:/api/v2/templates/{template}/versions": {
369369
AssertAction: rbac.ActionUpdate,
370-
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
370+
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID),
371371
},
372372
"GET:/api/v2/templates/{template}/versions/{templateversionname}": {
373373
AssertAction: rbac.ActionRead,
374-
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
374+
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID),
375375
},
376376
"GET:/api/v2/templateversions/{templateversion}": {
377377
AssertAction: rbac.ActionRead,
378-
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
378+
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID),
379379
},
380380
"PATCH:/api/v2/templateversions/{templateversion}/cancel": {
381381
AssertAction: rbac.ActionUpdate,
382-
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
382+
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID),
383383
},
384384
"GET:/api/v2/templateversions/{templateversion}/logs": {
385385
AssertAction: rbac.ActionRead,
386-
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
386+
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID),
387387
},
388388
"GET:/api/v2/templateversions/{templateversion}/parameters": {
389389
AssertAction: rbac.ActionRead,
390-
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
390+
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID),
391391
},
392392
"GET:/api/v2/templateversions/{templateversion}/resources": {
393393
AssertAction: rbac.ActionRead,
394-
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
394+
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID),
395395
},
396396
"GET:/api/v2/templateversions/{templateversion}/schema": {
397397
AssertAction: rbac.ActionRead,
398-
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
398+
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID),
399399
},
400400
"POST:/api/v2/templateversions/{templateversion}/dry-run": {
401401
// The first check is to read the template
402402
AssertAction: rbac.ActionRead,
403-
AssertObject: rbac.ResourceTemplate.InOrg(version.OrganizationID).WithID(template.ID.String()),
403+
AssertObject: rbac.ResourceTemplate.InOrg(version.OrganizationID),
404404
},
405405
"GET:/api/v2/templateversions/{templateversion}/dry-run/{templateversiondryrun}": {
406406
AssertAction: rbac.ActionRead,
407-
AssertObject: rbac.ResourceTemplate.InOrg(version.OrganizationID).WithID(template.ID.String()),
407+
AssertObject: rbac.ResourceTemplate.InOrg(version.OrganizationID),
408408
},
409409
"GET:/api/v2/templateversions/{templateversion}/dry-run/{templateversiondryrun}/resources": {
410410
AssertAction: rbac.ActionRead,
411-
AssertObject: rbac.ResourceTemplate.InOrg(version.OrganizationID).WithID(template.ID.String()),
411+
AssertObject: rbac.ResourceTemplate.InOrg(version.OrganizationID),
412412
},
413413
"GET:/api/v2/templateversions/{templateversion}/dry-run/{templateversiondryrun}/logs": {
414414
AssertAction: rbac.ActionRead,
415-
AssertObject: rbac.ResourceTemplate.InOrg(version.OrganizationID).WithID(template.ID.String()),
415+
AssertObject: rbac.ResourceTemplate.InOrg(version.OrganizationID),
416416
},
417417
"PATCH:/api/v2/templateversions/{templateversion}/dry-run/{templateversiondryrun}/cancel": {
418418
AssertAction: rbac.ActionRead,
419-
AssertObject: rbac.ResourceTemplate.InOrg(version.OrganizationID).WithID(template.ID.String()),
419+
AssertObject: rbac.ResourceTemplate.InOrg(version.OrganizationID),
420420
},
421421
"GET:/api/v2/provisionerdaemons": {
422422
StatusCode: http.StatusOK,
423-
AssertObject: rbac.ResourceProvisionerDaemon.WithID(provisionerds[0].ID.String()),
423+
AssertObject: rbac.ResourceProvisionerDaemon,
424424
},
425425

426426
"POST:/api/v2/parameters/{scope}/{id}": {
427427
AssertAction: rbac.ActionUpdate,
428-
AssertObject: rbac.ResourceTemplate.WithID(template.ID.String()),
428+
AssertObject: rbac.ResourceTemplate,
429429
},
430430
"GET:/api/v2/parameters/{scope}/{id}": {
431431
AssertAction: rbac.ActionRead,
432-
AssertObject: rbac.ResourceTemplate.WithID(template.ID.String()),
432+
AssertObject: rbac.ResourceTemplate,
433433
},
434434
"DELETE:/api/v2/parameters/{scope}/{id}/{name}": {
435435
AssertAction: rbac.ActionUpdate,
436-
AssertObject: rbac.ResourceTemplate.WithID(template.ID.String()),
436+
AssertObject: rbac.ResourceTemplate,
437437
},
438438
"GET:/api/v2/organizations/{organization}/templates/{templatename}": {
439439
AssertAction: rbac.ActionRead,
440-
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID).WithID(template.ID.String()),
440+
AssertObject: rbac.ResourceTemplate.InOrg(template.OrganizationID),
441441
},
442442
"POST:/api/v2/organizations/{organization}/workspaces": {
443443
AssertAction: rbac.ActionCreate,
444444
// No ID when creating
445-
AssertObject: workspaceRBACObj.WithID(""),
445+
AssertObject: workspaceRBACObj,
446446
},
447447
"GET:/api/v2/workspaces/{workspace}/watch": {
448448
AssertAction: rbac.ActionRead,

‎coderd/database/modelmethods.go

+8-8
Original file line numberDiff line numberDiff line change
@@ -5,37 +5,37 @@ import (
55
)
66

77
func (t Template) RBACObject() rbac.Object {
8-
return rbac.ResourceTemplate.InOrg(t.OrganizationID).WithID(t.ID.String())
8+
return rbac.ResourceTemplate.InOrg(t.OrganizationID)
99
}
1010

1111
func (t TemplateVersion) RBACObject() rbac.Object {
1212
// Just use the parent template resource for controlling versions
13-
return rbac.ResourceTemplate.InOrg(t.OrganizationID).WithID(t.TemplateID.UUID.String())
13+
return rbac.ResourceTemplate.InOrg(t.OrganizationID)
1414
}
1515

1616
func (w Workspace) RBACObject() rbac.Object {
17-
return rbac.ResourceWorkspace.InOrg(w.OrganizationID).WithID(w.ID.String()).WithOwner(w.OwnerID.String())
17+
return rbac.ResourceWorkspace.InOrg(w.OrganizationID).WithOwner(w.OwnerID.String())
1818
}
1919

2020
func (m OrganizationMember) RBACObject() rbac.Object {
21-
return rbac.ResourceOrganizationMember.InOrg(m.OrganizationID).WithID(m.UserID.String())
21+
return rbac.ResourceOrganizationMember.InOrg(m.OrganizationID)
2222
}
2323

2424
func (o Organization) RBACObject() rbac.Object {
25-
return rbac.ResourceOrganization.InOrg(o.ID).WithID(o.ID.String())
25+
return rbac.ResourceOrganization.InOrg(o.ID)
2626
}
2727

2828
func (d ProvisionerDaemon) RBACObject() rbac.Object {
29-
return rbac.ResourceProvisionerDaemon.WithID(d.ID.String())
29+
return rbac.ResourceProvisionerDaemon
3030
}
3131

3232
func (f File) RBACObject() rbac.Object {
33-
return rbac.ResourceFile.WithID(f.Hash).WithOwner(f.CreatedBy.String())
33+
return rbac.ResourceFile.WithOwner(f.CreatedBy.String())
3434
}
3535

3636
// RBACObject returns the RBAC object for the site wide user resource.
3737
// If you are trying to get the RBAC object for the UserData, use
3838
// rbac.ResourceUserData
3939
func (u User) RBACObject() rbac.Object {
40-
return rbac.ResourceUser.WithID(u.ID.String())
40+
return rbac.ResourceUser
4141
}

‎coderd/files.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ func (api *API) fileByHash(rw http.ResponseWriter, r *http.Request) {
9999
}
100100

101101
if !api.Authorize(r, rbac.ActionRead,
102-
rbac.ResourceFile.WithOwner(file.CreatedBy.String()).WithID(file.Hash)) {
102+
rbac.ResourceFile.WithOwner(file.CreatedBy.String())) {
103103
// Return 404 to not leak the file exists
104104
httpapi.ResourceNotFound(rw)
105105
return

‎coderd/members.go

+12-12
Original file line numberDiff line numberDiff line change
@@ -37,19 +37,19 @@ func (api *API) putMemberRoles(rw http.ResponseWriter, r *http.Request) {
3737
// The org-member role is always implied.
3838
impliedTypes := append(params.Roles, rbac.RoleOrgMember(organization.ID))
3939
added, removed := rbac.ChangeRoleSet(member.Roles, impliedTypes)
40-
for _, roleName := range added {
41-
// Assigning a role requires the create permission.
42-
if !api.Authorize(r, rbac.ActionCreate, rbac.ResourceOrgRoleAssignment.WithID(roleName).InOrg(organization.ID)) {
43-
httpapi.Forbidden(rw)
44-
return
45-
}
40+
41+
// TODO: Handle added and removed roles.
42+
43+
// Assigning a role requires the create permission.
44+
if !api.Authorize(r, rbac.ActionCreate, rbac.ResourceOrgRoleAssignment.InOrg(organization.ID)) {
45+
httpapi.Forbidden(rw)
46+
return
4647
}
47-
for _, roleName := range removed {
48-
// Removing a role requires the delete permission.
49-
if !api.Authorize(r, rbac.ActionDelete, rbac.ResourceOrgRoleAssignment.WithID(roleName).InOrg(organization.ID)) {
50-
httpapi.Forbidden(rw)
51-
return
52-
}
48+
49+
// Removing a role requires the delete permission.
50+
if !api.Authorize(r, rbac.ActionDelete, rbac.ResourceOrgRoleAssignment.InOrg(organization.ID)) {
51+
httpapi.Forbidden(rw)
52+
return
5353
}
5454

5555
updatedUser, err := api.updateOrganizationMemberRoles(r.Context(), database.UpdateMemberRolesParams{

‎coderd/organizations.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,7 @@ func (api *API) organization(rw http.ResponseWriter, r *http.Request) {
2020
organization := httpmw.OrganizationParam(r)
2121

2222
if !api.Authorize(r, rbac.ActionRead, rbac.ResourceOrganization.
23-
InOrg(organization.ID).
24-
WithID(organization.ID.String())) {
23+
InOrg(organization.ID)) {
2524
httpapi.ResourceNotFound(rw)
2625
return
2726
}

0 commit comments

Comments
 (0)