Skip to content

Commit 512c09e

Browse files
EmyrkEmyrk
committed
Address PR comments
- Drop Resource inteface, use concrete struct for now - Rename some roles - Some comments
1 parent 0868301 commit 512c09e

File tree

7 files changed

+69
-88
lines changed

7 files changed

+69
-88
lines changed

‎coderd/authz/authz.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import "golang.org/x/xerrors"
55
var ErrUnauthorized = xerrors.New("unauthorized")
66

77
// TODO: Implement Authorize. This will be implmented in mainly rego.
8-
func Authorize(subj Subject, obj Resource, action Action) error {
8+
func Authorize(subj Subject, obj Object, action Action) error {
99
// TODO: Expand subject roles into their permissions as appropriate. Apply scopes.
1010
var _, _, _ = subj, obj, action
1111
roles, err := subj.GetRoles()
@@ -28,13 +28,13 @@ func Authorize(subj Subject, obj Resource, action Action) error {
2828
merged.Site = append(merged.Site, r.Site...)
2929
// Only grab user roles if the resource is owned by a user.
3030
// These roles only apply if the subject is said owner.
31-
if obj.OwnerID() != "" && obj.OwnerID() == subj.ID() {
31+
if obj.Owner != "" && obj.Owner == subj.ID() {
3232
merged.User = append(merged.User, r.User...)
3333
}
3434

3535
// Grab org roles if the resource is owned by a given organization.
36-
if obj.OrgOwnerID() != "" {
37-
orgID := obj.OrgOwnerID()
36+
if obj.OrgOwner != "" {
37+
orgID := obj.OrgOwner
3838
if v, ok := r.Org[orgID]; ok {
3939
merged.Org[orgID] = append(merged.Org[orgID], v...)
4040
}

‎coderd/authz/authz_test.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func TestAuthorizeDomain(t *testing.T) {
3131

3232
{resource: authz.ResourceWorkspace.SetID(wrkID), actions: allActions(), allow: false},
3333

34-
{resource: authz.ResourceWorkspace, actions: allActions(), allow: false},
34+
{resource: authz.ResourceWorkspace.All(), actions: allActions(), allow: false},
3535

3636
// Other org + me + id
3737
{resource: authz.ResourceWorkspace.SetOrg("other").SetOwner(user.ID()).SetID(wrkID), actions: allActions(), allow: false},
@@ -75,7 +75,7 @@ func TestAuthorizeDomain(t *testing.T) {
7575

7676
{resource: authz.ResourceWorkspace.SetID(wrkID), actions: allActions(), allow: false},
7777

78-
{resource: authz.ResourceWorkspace, actions: allActions(), allow: false},
78+
{resource: authz.ResourceWorkspace.All(), actions: allActions(), allow: false},
7979

8080
// Other org + me + id
8181
{resource: authz.ResourceWorkspace.SetOrg("other").SetOwner(user.ID()).SetID(wrkID), actions: allActions(), allow: false},
@@ -122,7 +122,7 @@ func TestAuthorizeDomain(t *testing.T) {
122122

123123
{resource: authz.ResourceWorkspace.SetID(wrkID), actions: allActions(), allow: false},
124124

125-
{resource: authz.ResourceWorkspace, actions: allActions(), allow: false},
125+
{resource: authz.ResourceWorkspace.All(), actions: allActions(), allow: false},
126126

127127
// Other org + me + id
128128
{resource: authz.ResourceWorkspace.SetOrg("other").SetOwner(user.ID()).SetID(wrkID), actions: allActions(), allow: false},
@@ -169,7 +169,7 @@ func TestAuthorizeDomain(t *testing.T) {
169169

170170
{resource: authz.ResourceWorkspace.SetID(wrkID), actions: allActions(), allow: true},
171171

172-
{resource: authz.ResourceWorkspace, actions: allActions(), allow: true},
172+
{resource: authz.ResourceWorkspace.All(), actions: allActions(), allow: true},
173173

174174
// Other org + me + id
175175
{resource: authz.ResourceWorkspace.SetOrg("other").SetOwner(user.ID()).SetID(wrkID), actions: allActions(), allow: true},
@@ -216,7 +216,7 @@ func TestAuthorizeDomain(t *testing.T) {
216216

217217
{resource: authz.ResourceWorkspace.SetID(wrkID), actions: allActions(), allow: true},
218218

219-
{resource: authz.ResourceWorkspace, actions: allActions(), allow: false},
219+
{resource: authz.ResourceWorkspace.All(), actions: allActions(), allow: false},
220220

221221
// Other org + me + id
222222
{resource: authz.ResourceWorkspace.SetOrg("other").SetOwner(user.ID()).SetID(wrkID), actions: allActions(), allow: false},
@@ -286,7 +286,7 @@ func TestAuthorizeDomain(t *testing.T) {
286286

287287
{resource: authz.ResourceWorkspace.SetID(wrkID), allow: false},
288288

289-
{resource: authz.ResourceWorkspace, allow: false},
289+
{resource: authz.ResourceWorkspace.All(), allow: false},
290290

291291
// Other org + me + id
292292
{resource: authz.ResourceWorkspace.SetOrg("other").SetOwner(user.ID()).SetID(wrkID), allow: false},
@@ -331,7 +331,7 @@ func TestAuthorizeDomain(t *testing.T) {
331331

332332
{resource: authz.ResourceWorkspace.SetID(wrkID)},
333333

334-
{resource: authz.ResourceWorkspace},
334+
{resource: authz.ResourceWorkspace.All()},
335335

336336
// Other org + me + id
337337
{resource: authz.ResourceWorkspace.SetOrg("other").SetOwner(user.ID()).SetID(wrkID)},
@@ -401,7 +401,7 @@ func TestAuthorizeLevels(t *testing.T) {
401401

402402
{resource: authz.ResourceWorkspace.SetID(wrkID)},
403403

404-
{resource: authz.ResourceWorkspace},
404+
{resource: authz.ResourceWorkspace.All()},
405405

406406
// Other org + me + id
407407
{resource: authz.ResourceWorkspace.SetOrg("other").SetOwner(user.ID()).SetID(wrkID)},
@@ -474,7 +474,7 @@ func TestAuthorizeLevels(t *testing.T) {
474474

475475
{resource: authz.ResourceWorkspace.SetID(wrkID), allow: false},
476476

477-
{resource: authz.ResourceWorkspace, allow: false},
477+
{resource: authz.ResourceWorkspace.All(), allow: false},
478478

479479
// Other org + me + id
480480
{resource: authz.ResourceWorkspace.SetOrg("other").SetOwner(user.ID()).SetID(wrkID), allow: false},
@@ -514,7 +514,7 @@ func cases(opt func(c authTestCase) authTestCase, cases []authTestCase) []authTe
514514
}
515515

516516
type authTestCase struct {
517-
resource authz.Resource
517+
resource authz.Object
518518
actions []authz.Action
519519
allow bool
520520
}

‎coderd/authz/example_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func TestExample(t *testing.T) {
2828
//nolint:paralleltest
2929
t.Run("ReadAllWorkspaces", func(t *testing.T) {
3030
// To read all workspaces on the site
31-
err := authz.Authorize(user, authz.ResourceWorkspace, authz.ActionRead)
31+
err := authz.Authorize(user, authz.ResourceWorkspace.All(), authz.ActionRead)
3232
var _ = err
3333
// require.Error(t, err, "this user cannot read all workspaces")
3434
})
@@ -52,7 +52,7 @@ func TestExample(t *testing.T) {
5252

5353
//nolint:paralleltest
5454
t.Run("CreateNewSiteUser", func(t *testing.T) {
55-
err := authz.Authorize(user, authz.ResourceUser, authz.ActionCreate)
55+
err := authz.Authorize(user, authz.ResourceUser.All(), authz.ActionCreate)
5656
var _ = err
5757
// require.Error(t, err, "this user cannot create new users")
5858
})

‎coderd/authz/object.go

Lines changed: 22 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,62 +1,46 @@
11
package authz
22

3-
type Resource interface {
4-
ID() string
5-
ResourceType() ResourceType
3+
//type Resource interface {
4+
// ID() string
5+
// ResourceType() ResourceType
6+
//
7+
// OwnerID() string
8+
// OrgOwnerID() string
9+
//}
610

7-
OwnerID() string
8-
OrgOwnerID() string
9-
}
10-
11-
var _ Resource = (*zObject)(nil)
11+
//var _ Resource = (*Object)(nil)
1212

13-
// zObject is used to create objects for authz checks when you have none in
13+
// Object is used to create objects for authz checks when you have none in
1414
// hand to run the check on.
15-
// An example is if you want to list all workspaces, you can create a zObject
15+
// An example is if you want to list all workspaces, you can create a Object
1616
// that represents the set of workspaces you are trying to get access too.
1717
// Do not export this type, as it can be created from a resource type constant.
18-
type zObject struct {
19-
id string
20-
owner string
21-
orgOwner string
18+
type Object struct {
19+
ID string
20+
Owner string
21+
OrgOwner string
2222

23-
// objectType is "workspace", "project", "devurl", etc
24-
objectType ResourceType
23+
// ObjectType is "workspace", "project", "devurl", etc
24+
ObjectType ResourceType
2525
// TODO: SharedUsers?
2626
}
2727

28-
func (z zObject) ID() string {
29-
return z.id
30-
}
31-
32-
func (z zObject) ResourceType() ResourceType {
33-
return z.objectType
34-
}
35-
36-
func (z zObject) OwnerID() string {
37-
return z.owner
38-
}
39-
40-
func (z zObject) OrgOwnerID() string {
41-
return z.orgOwner
42-
}
43-
4428
// SetOrg adds an org OwnerID to the resource
4529
//nolint:revive
46-
func (z zObject) SetOrg(orgID string) zObject {
47-
z.orgOwner = orgID
30+
func (z Object) SetOrg(orgID string) Object {
31+
z.OrgOwner = orgID
4832
return z
4933
}
5034

5135
// SetOwner adds an OwnerID to the resource
5236
//nolint:revive
53-
func (z zObject) SetOwner(id string) zObject {
54-
z.owner = id
37+
func (z Object) SetOwner(id string) Object {
38+
z.Owner = id
5539
return z
5640
}
5741

5842
//nolint:revive
59-
func (z zObject) SetID(id string) zObject {
60-
z.id = id
43+
func (z Object) SetID(id string) Object {
44+
z.ID = id
6145
return z
6246
}

‎coderd/authz/permission.go

Lines changed: 0 additions & 9 deletions
This file was deleted.

‎coderd/authz/resources.go

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,46 +5,41 @@ type ResourceType string
55

66
const (
77
ResourceWorkspace ResourceType = "workspace"
8-
ResourceProject ResourceType = "project"
8+
ResourceTemplate ResourceType = "template"
99
ResourceDevURL ResourceType = "devurl"
1010
ResourceUser ResourceType = "user"
1111
ResourceAuditLogs ResourceType = "audit-logs"
1212
)
1313

14-
func (ResourceType) ID() string {
15-
return ""
16-
}
17-
18-
func (t ResourceType) ResourceType() ResourceType {
19-
return t
14+
func (z ResourceType) All() Object {
15+
return Object{
16+
ObjectType: z,
17+
}
2018
}
2119

22-
func (ResourceType) OwnerID() string { return "" }
23-
func (ResourceType) OrgOwnerID() string { return "" }
24-
2520
// SetOrg adds an org OwnerID to the resource
2621
//nolint:revive
27-
func (r ResourceType) SetOrg(orgID string) zObject {
28-
return zObject{
29-
orgOwner: orgID,
30-
objectType: r,
22+
func (r ResourceType) SetOrg(orgID string) Object {
23+
return Object{
24+
OrgOwner: orgID,
25+
ObjectType: r,
3126
}
3227
}
3328

3429
// SetOwner adds an OwnerID to the resource
3530
//nolint:revive
36-
func (r ResourceType) SetOwner(id string) zObject {
37-
return zObject{
38-
owner: id,
39-
objectType: r,
31+
func (r ResourceType) SetOwner(id string) Object {
32+
return Object{
33+
Owner: id,
34+
ObjectType: r,
4035
}
4136
}
4237

4338
// SetID adds a resource ID to the resource
4439
//nolint:revive
45-
func (r ResourceType) SetID(id string) zObject {
46-
return zObject{
47-
id: id,
48-
objectType: r,
40+
func (r ResourceType) SetID(id string) Object {
41+
return Object{
42+
ID: id,
43+
ObjectType: r,
4944
}
5045
}

‎coderd/authz/role.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,14 @@ import "fmt"
44

55
const Wildcard = "*"
66

7+
type Permission struct {
8+
// Negate makes this a negative permission
9+
Negate bool
10+
ResourceType ResourceType
11+
ResourceID string
12+
Action Action
13+
}
14+
715
// Role is a set of permissions at multiple levels:
816
// - Site level permissions apply EVERYWHERE
917
// - Org level permissions apply to EVERYTHING in a given ORG
@@ -13,6 +21,9 @@ const Wildcard = "*"
1321
type Role struct {
1422
Name string
1523
Site []Permission
24+
// Org is a map of orgid to permissions. We represent orgid as a string.
25+
// TODO: Maybe switch to uuid, but tokens might need to support a "wildcard" org
26+
// which could be a special uuid (like all 0s?)
1627
Org map[string][]Permission
1728
User []Permission
1829
}
@@ -22,23 +33,23 @@ type Role struct {
2233
var (
2334
// RoleSiteAdmin is a role that allows everything everywhere.
2435
RoleSiteAdmin = Role{
25-
Name: "site-admin",
36+
Name: "admin",
2637
Site: permissions(map[ResourceType][]Action{
2738
Wildcard: {Wildcard},
2839
}),
2940
}
3041

3142
// RoleSiteMember is a role that allows access to user-level resources.
3243
RoleSiteMember = Role{
33-
Name: "site-member",
44+
Name: "member",
3445
User: permissions(map[ResourceType][]Action{
3546
Wildcard: {Wildcard},
3647
}),
3748
}
3849

3950
// RoleSiteAuditor is an example on how to give more precise permissions
4051
RoleSiteAuditor = Role{
41-
Name: "site-auditor",
52+
Name: "auditor",
4253
Site: permissions(map[ResourceType][]Action{
4354
ResourceAuditLogs: {ActionRead},
4455
// Should be able to read user details to associate with logs.

0 commit comments

Comments
 (0)