Skip to content

Commit ec2cf09

Browse files
EmyrkEmyrk
committed
Implement authorize and some basic tests
1 parent 0876d12 commit ec2cf09

File tree

5 files changed

+155
-131
lines changed

5 files changed

+155
-131
lines changed

‎coderd/authz/authz.go

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,48 @@
11
package authz
22

33
import (
4+
"context"
45
"errors"
6+
"strings"
57

68
"github.com/coder/coder/coderd/authz/rbac"
79
)
810

911
var ErrUnauthorized = errors.New("unauthorized")
1012

11-
// TODO: Implement Authorize
12-
func Authorize(subj Subject, res Object, action rbac.Operation) error {
13-
// TODO: Expand subject roles into their permissions as appropriate. Apply scopes.
14-
13+
func Authorize(ctx context.Context, subj Subject, res Object, action rbac.Operation) error {
1514
if res.ObjectType == "" {
1615
return ErrUnauthorized
1716
}
1817

18+
// Own actions only succeed if the subject owns the resource.
19+
if !isAll(action) {
20+
// Before we reject this, the user could be an admin with "-all" perms.
21+
err := Authorize(ctx, subj, res, rbac.Operation(strings.ReplaceAll(string(action), "-own", "-all")))
22+
if err == nil {
23+
return nil
24+
}
25+
if res.Owner != subj.ID() {
26+
return ErrUnauthorized
27+
}
28+
}
29+
1930
if SiteEnforcer.RolesHavePermission(subj.Roles(), res.ObjectType, action) {
2031
return nil
2132
}
2233

34+
if res.OrgOwner != "" {
35+
orgRoles, err := subj.OrgRoles(ctx, res.OrgOwner)
36+
if err == nil {
37+
if OrganizationEnforcer.RolesHavePermission(orgRoles, res.ObjectType, action) {
38+
return nil
39+
}
40+
}
41+
}
42+
2343
return ErrUnauthorized
2444
}
45+
46+
func isAll(action rbac.Operation) bool {
47+
return strings.HasSuffix(string(action), "-all")
48+
}

‎coderd/authz/authz_test.go

Lines changed: 97 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1,79 +1,113 @@
11
package authz_test
22

33
import (
4+
"context"
45
"testing"
56

7+
"github.com/coder/coder/coderd/authz"
68
"github.com/coder/coder/coderd/authz/rbac"
7-
89
"github.com/stretchr/testify/require"
9-
10-
"github.com/coder/coder/coderd/authz"
1110
)
1211

1312
func TestAuthorize(t *testing.T) {
1413
t.Parallel()
1514

16-
// testCases := []struct {
17-
// Subject authz.Subject
18-
// Resource authz.Object
19-
// Action rbac.Operation
20-
// ExpectedError string
21-
// }{
22-
// {
23-
// Subject: &authz.SubjectTODO{
24-
// UserID: "user",
25-
// Site: []rbac.Role{authz.SiteMember},
26-
// Org: map[string]rbac.Roles{
27-
// "default": {authz.OrganizationAdmin},
28-
// },
29-
// },
30-
// Resource: &authz.Object{},
31-
// },
32-
// }
33-
34-
// user will become an authn object, and can even be a database.User if it
35-
// fulfills the interface. Until then, use a placeholder.
36-
user := authz.SubjectTODO{
37-
UserID: "alice",
38-
// No site perms
39-
Site: []rbac.Role{authz.SiteMember},
40-
Org: map[string]rbac.Roles{
41-
// Admin of org "default".
42-
"default": {authz.OrganizationAdmin},
15+
testCases := []struct {
16+
Name string
17+
Subject authz.Subject
18+
Resource authz.Object
19+
Action rbac.Operation
20+
ExpectedError bool
21+
}{
22+
{
23+
Name: "Org admin trying to read all workspace",
24+
Subject: &authz.SubjectTODO{
25+
UserID: "org-admin",
26+
Site: []rbac.Role{authz.SiteMember},
27+
Org: map[string]rbac.Roles{
28+
"default": {authz.OrganizationAdmin},
29+
},
30+
},
31+
Resource: authz.Object{
32+
ObjectType: authz.Workspaces,
33+
},
34+
Action: authz.ReadAll,
35+
ExpectedError: true,
36+
},
37+
{
38+
Name: "Org admin read org workspace",
39+
Subject: &authz.SubjectTODO{
40+
UserID: "org-admin",
41+
Site: []rbac.Role{authz.SiteMember},
42+
Org: map[string]rbac.Roles{
43+
"default": {authz.OrganizationAdmin},
44+
},
45+
},
46+
Resource: authz.Object{
47+
ObjectType: authz.Workspaces,
48+
OrgOwner: "default",
49+
},
50+
Action: authz.ReadAll,
51+
},
52+
{
53+
Name: "Org admin read someone else's workspace",
54+
Subject: &authz.SubjectTODO{
55+
UserID: "org-admin",
56+
Site: []rbac.Role{authz.SiteMember},
57+
Org: map[string]rbac.Roles{
58+
"default": {authz.OrganizationAdmin},
59+
},
60+
},
61+
Resource: authz.Object{
62+
Owner: "org-member",
63+
ObjectType: authz.Workspaces,
64+
OrgOwner: "default",
65+
},
66+
Action: authz.ReadOwn,
67+
},
68+
{
69+
Name: "Org member read their workspace",
70+
Subject: &authz.SubjectTODO{
71+
UserID: "org-member",
72+
Site: []rbac.Role{authz.SiteMember},
73+
Org: map[string]rbac.Roles{
74+
"default": {authz.OrganizationMember},
75+
},
76+
},
77+
Resource: authz.Object{
78+
Owner: "org-member",
79+
ObjectType: authz.Workspaces,
80+
OrgOwner: "default",
81+
},
82+
Action: authz.ReadOwn,
83+
},
84+
{
85+
Name: "Site member read their workspace in other org",
86+
Subject: &authz.SubjectTODO{
87+
UserID: "site-member",
88+
Site: []rbac.Role{authz.SiteMember},
89+
Org: map[string]rbac.Roles{
90+
"default": {authz.OrganizationMember},
91+
},
92+
},
93+
Resource: authz.Object{
94+
Owner: "site-member",
95+
ObjectType: authz.Workspaces,
96+
OrgOwner: "not-default",
97+
},
98+
Action: authz.ReadOwn,
99+
ExpectedError: true,
43100
},
44101
}
45102

46-
// TODO: Uncomment all assertions when implementation is done.
47-
48-
//nolint:paralleltest
49-
t.Run("ReadAllWorkspaces", func(t *testing.T) {
50-
// To read all workspaces on the site
51-
err := authz.Authorize(user, authz.Object{}, authz.ReadAll)
52-
var _ = err
53-
require.Error(t, err, "this user cannot read all workspaces")
54-
})
55-
56-
// nolint:paralleltest
57-
// t.Run("ReadOrgWorkspaces", func(t *testing.T) {
58-
// To read all workspaces on the org 'default'
59-
// err := authz.Authorize(user, authz.ResourceWorkspace.Org("default"), authz.ActionRead)
60-
// require.NoError(t, err, "this user can read all org workspaces in 'default'")
61-
// })
62-
//
63-
// nolint:paralleltest
64-
// t.Run("ReadMyWorkspace", func(t *testing.T) {
65-
// Note 'database.Workspace' could fulfill the object interface and be passed in directly
66-
// err := authz.Authorize(user, authz.ResourceWorkspace.Org("default").Owner(user.UserID), authz.ActionRead)
67-
// require.NoError(t, err, "this user can their workspace")
68-
//
69-
// err = authz.Authorize(user, authz.ResourceWorkspace.Org("default").Owner(user.UserID).AsID("1234"), authz.ActionRead)
70-
// require.NoError(t, err, "this user can read workspace '1234'")
71-
// })
72-
//
73-
// nolint:paralleltest
74-
// t.Run("CreateNewSiteUser", func(t *testing.T) {
75-
// err := authz.Authorize(user, authz.ResourceUser, authz.ActionCreate)
76-
// var _ = err
77-
// require.Error(t, err, "this user cannot create new users")
78-
// })
103+
for _, c := range testCases {
104+
t.Run(c.Name, func(t *testing.T) {
105+
err := authz.Authorize(context.Background(), c.Subject, c.Resource, c.Action)
106+
if c.ExpectedError {
107+
require.EqualError(t, err, authz.ErrUnauthorized.Error(), "unauth")
108+
} else {
109+
require.NoError(t, err, "exp auth succeed")
110+
}
111+
})
112+
}
79113
}

‎coderd/authz/domain.go

Lines changed: 28 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,10 @@ const (
5050
Workspaces rbac.Resource = "environments"
5151
)
5252

53-
// Operations
53+
// Operations **must** have an -all or -own suffix
5454
const (
55-
Create rbac.Operation = "create" // create a new resource
55+
CreateOwn rbac.Operation = "create-own" // create a new resource owned by me
56+
CreateAll rbac.Operation = "create-all" // create a new resource in this scope
5657
DeleteAll rbac.Operation = "delete-all" // delete any of these resources in this scope
5758
DeleteOwn rbac.Operation = "delete-own" // delete any of these resources owned in this scope
5859
ReadAll rbac.Operation = "read-all" // read all fields in any of these resources in this scope
@@ -77,7 +78,7 @@ var SiteEnforcer = rbac.Enforcer{
7778
ResourcePools: {UpdateAll},
7879
},
7980
SiteAdmin: {
80-
APIKeys: {Create, ReadAll, UpdateAll, DeleteAll},
81+
APIKeys: {CreateAll, ReadAll, UpdateAll, DeleteAll},
8182
},
8283
SiteAuditor: {
8384
AuditLogs: {ReadAll},
@@ -86,28 +87,28 @@ var SiteEnforcer = rbac.Enforcer{
8687
AuditLogs: {ReadAll},
8788
Configs: {ReadAll, UpdateAll},
8889
SecretConfigs: {ReadAll, UpdateAll},
89-
Workspaces: {Create, ReadAll, UpdateAll, DeleteAll},
90-
Extensions: {Create, DeleteAll},
90+
Workspaces: {CreateAll, ReadAll, UpdateAll, DeleteAll},
91+
Extensions: {CreateAll, DeleteAll},
9192
FeatureFlags: {ReadAll, UpdateAll},
92-
ImageTags: {Create, ReadAll, UpdateAll, DeleteAll},
93-
Images: {Create, ReadAll, UpdateAll, DeleteAll},
93+
ImageTags: {CreateAll, ReadAll, UpdateAll, DeleteAll},
94+
Images: {CreateAll, ReadAll, UpdateAll, DeleteAll},
9495
Metrics: {ReadAll},
9596
OAuth: {ReadAll, UpdateAll},
96-
OrganizationMembers: {Create, ReadAll, UpdateAll, DeleteAll},
97-
Organizations: {Create, ReadAll, UpdateAll, DeleteAll},
98-
Registries: {Create, ReadAll, UpdateAll, DeleteAll},
97+
OrganizationMembers: {CreateAll, ReadAll, UpdateAll, DeleteAll},
98+
Organizations: {CreateAll, ReadAll, UpdateAll, DeleteAll},
99+
Registries: {CreateAll, ReadAll, UpdateAll, DeleteAll},
99100
ResourcePools: {UpdateAll, ReadAll},
100-
Satellites: {Create, ReadAll, UpdateAll, DeleteAll},
101-
SystemBanners: {Create, ReadAll, UpdateAll, DeleteAll},
102-
TLSCertificates: {Create, ReadAll, UpdateAll, DeleteAll},
103-
Usage: {Create, ReadAll, UpdateAll, DeleteAll},
104-
Users: {Create, ReadAll, UpdateAll, DeleteAll},
105-
WorkspaceProviders: {Create, ReadAll, UpdateAll, DeleteAll},
101+
Satellites: {CreateAll, ReadAll, UpdateAll, DeleteAll},
102+
SystemBanners: {CreateAll, ReadAll, UpdateAll, DeleteAll},
103+
TLSCertificates: {CreateAll, ReadAll, UpdateAll, DeleteAll},
104+
Usage: {CreateAll, ReadAll, UpdateAll, DeleteAll},
105+
Users: {CreateAll, ReadAll, UpdateAll, DeleteAll},
106+
WorkspaceProviders: {CreateAll, ReadAll, UpdateAll, DeleteAll},
106107
},
107108
SiteMember: {
108-
APIKeys: {Create, ReadOwn, UpdateOwn, DeleteOwn},
109+
APIKeys: {CreateOwn, ReadOwn, UpdateOwn, DeleteOwn},
109110
Configs: {ReadAll},
110-
DevURLs: {Create, ReadOwn, UpdateOwn, DeleteOwn},
111+
DevURLs: {CreateOwn, ReadOwn, UpdateOwn, DeleteOwn},
111112
FeatureFlags: {ReadAll},
112113
ResourcePools: {ReadAll},
113114
Metrics: {ReadOwn},
@@ -124,20 +125,20 @@ var OrganizationEnforcer = rbac.Enforcer{
124125
},
125126
RolePermissions: rbac.RolePermissions{
126127
OrganizationManager: {
127-
Workspaces: {Create, ReadAll, UpdateAll, DeleteAll},
128-
ImageTags: {Create, ReadAll, UpdateAll, DeleteAll},
129-
Images: {Create, ReadAll, UpdateAll, DeleteAll},
128+
Workspaces: {CreateAll, ReadAll, UpdateAll, DeleteAll},
129+
ImageTags: {CreateAll, ReadAll, UpdateAll, DeleteAll},
130+
Images: {CreateAll, ReadAll, UpdateAll, DeleteAll},
130131
Metrics: {ReadAll},
131-
OrganizationMembers: {Create, ReadAll, UpdateAll, DeleteAll},
132+
OrganizationMembers: {CreateAll, ReadAll, UpdateAll, DeleteAll},
132133
Organizations: {ReadAll},
133-
Registries: {Create, ReadAll, UpdateAll, DeleteAll},
134+
Registries: {CreateAll, ReadAll, UpdateAll, DeleteAll},
134135
Users: {ReadAll},
135136
},
136137
OrganizationMember: {
137138
DevURLs: {ReadAll},
138-
Workspaces: {Create, ReadAll, UpdateOwn, DeleteOwn},
139-
ImageTags: {Create, ReadAll},
140-
Images: {Create, ReadAll},
139+
Workspaces: {CreateOwn, ReadAll, UpdateOwn, DeleteOwn},
140+
ImageTags: {CreateOwn, ReadAll},
141+
Images: {CreateOwn, ReadAll},
141142
Metrics: {ReadOwn},
142143
OrganizationMembers: {},
143144
Organizations: {},
@@ -146,7 +147,7 @@ var OrganizationEnforcer = rbac.Enforcer{
146147
Users: {ReadOwn},
147148
},
148149
OrganizationRegistryManager: {
149-
Registries: {Create, ReadAll, UpdateAll, DeleteAll},
150+
Registries: {CreateAll, ReadAll, UpdateAll, DeleteAll},
150151
},
151152
},
152153
}

‎coderd/authz/enforcers_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ func TestResolveSiteEnforcer(t *testing.T) {
1515
assert.Exactly(t, siteEnforcerMap[SiteAdmin][AuditLogs][ReadAll], true)
1616

1717
// SiteAuditor inherited from SiteMember
18-
assert.Exactly(t, siteEnforcerMap[SiteAuditor][DevURLs][Create], true)
18+
assert.Exactly(t, siteEnforcerMap[SiteAuditor][DevURLs][CreateOwn], true)
1919

2020
// SiteManager merged Metrics perms with SiteMember
2121
assert.Exactly(t, siteEnforcerMap[SiteManager][Metrics][ReadOwn], true)
@@ -31,7 +31,7 @@ func TestResolveOrgEnforcer(t *testing.T) {
3131
assert.NotNil(t, orgEnforcerMap[OrganizationAdmin])
3232

3333
// OrganizationManager merged Workspaces perms with OrganizationMember
34-
testOps := rbac.Operations{Create, ReadAll, UpdateAll, UpdateOwn, DeleteAll}
34+
testOps := rbac.Operations{CreateOwn, ReadAll, UpdateAll, UpdateOwn, DeleteAll}
3535
for _, op := range testOps {
3636
assert.Exactly(t, orgEnforcerMap[OrganizationManager][Workspaces][op], true)
3737
}

‎coderd/authz/example_test.go

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

0 commit comments

Comments
 (0)