Skip to content

Commit 47c5bf2

Browse files
committed
chore: Rework roles to be expandable by name alone
1 parent ed6fd1f commit 47c5bf2

File tree

4 files changed

+190
-151
lines changed

4 files changed

+190
-151
lines changed

coderd/rbac/authz_test.go

+42-11
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package rbac_test
33
import (
44
"context"
55
"encoding/json"
6+
"fmt"
67
"testing"
78

89
"golang.org/x/xerrors"
@@ -14,8 +15,11 @@ import (
1415

1516
// subject is required because rego needs
1617
type subject struct {
17-
UserID string `json:"id"`
18-
Roles []rbac.Role `json:"roles"`
18+
UserID string `json:"id"`
19+
// For the unit test we want to pass in the roles directly, instead of just
20+
// by name. This allows us to test custom roles that do not exist in the product,
21+
// but test edge cases of the implementation.
22+
Roles []rbac.Role `json:"roles"`
1923
}
2024

2125
// TestAuthorizeDomain test the very basic roles that are commonly used.
@@ -26,7 +30,10 @@ func TestAuthorizeDomain(t *testing.T) {
2630

2731
user := subject{
2832
UserID: "me",
29-
Roles: []rbac.Role{rbac.RoleMember, rbac.RoleOrgMember(defOrg)},
33+
Roles: []rbac.Role{
34+
must(rbac.RoleByName(rbac.RoleMember())),
35+
must(rbac.RoleByName(rbac.RoleOrgMember(defOrg))),
36+
},
3037
}
3138

3239
testAuthorize(t, "Member", user, []authTestCase{
@@ -126,8 +133,8 @@ func TestAuthorizeDomain(t *testing.T) {
126133
user = subject{
127134
UserID: "me",
128135
Roles: []rbac.Role{
129-
rbac.RoleOrgAdmin(defOrg),
130-
rbac.RoleMember,
136+
must(rbac.RoleByName(rbac.RoleOrgAdmin(defOrg))),
137+
must(rbac.RoleByName(rbac.RoleMember())),
131138
},
132139
}
133140

@@ -173,8 +180,8 @@ func TestAuthorizeDomain(t *testing.T) {
173180
user = subject{
174181
UserID: "me",
175182
Roles: []rbac.Role{
176-
rbac.RoleAdmin,
177-
rbac.RoleMember,
183+
must(rbac.RoleByName(rbac.RoleAdmin())),
184+
must(rbac.RoleByName(rbac.RoleMember())),
178185
},
179186
}
180187

@@ -221,7 +228,19 @@ func TestAuthorizeDomain(t *testing.T) {
221228
user = subject{
222229
UserID: "me",
223230
Roles: []rbac.Role{
224-
rbac.RoleWorkspaceAgent(wrkID),
231+
{
232+
Name: fmt.Sprintf("agent-%s", wrkID),
233+
// This is at the site level to prevent the token from losing access if the user
234+
// is kicked from the org
235+
Site: []rbac.Permission{
236+
{
237+
Negate: false,
238+
ResourceType: rbac.ResourceWorkspace.Type,
239+
ResourceID: wrkID,
240+
Action: rbac.ActionRead,
241+
},
242+
},
243+
},
225244
},
226245
}
227246

@@ -439,8 +458,20 @@ func TestAuthorizeLevels(t *testing.T) {
439458
user := subject{
440459
UserID: "me",
441460
Roles: []rbac.Role{
442-
rbac.RoleAdmin,
443-
rbac.RoleOrgDenyAll(defOrg),
461+
must(rbac.RoleByName(rbac.RoleAdmin())),
462+
{
463+
Name: "org-deny" + defOrg,
464+
Org: map[string][]rbac.Permission{
465+
defOrg: {
466+
{
467+
Negate: true,
468+
ResourceType: "*",
469+
ResourceID: "*",
470+
Action: "*",
471+
},
472+
},
473+
},
474+
},
444475
{
445476
Name: "user-deny-all",
446477
// List out deny permissions explicitly
@@ -514,7 +545,7 @@ func TestAuthorizeLevels(t *testing.T) {
514545
},
515546
},
516547
},
517-
rbac.RoleOrgAdmin(defOrg),
548+
must(rbac.RoleByName(rbac.RoleOrgAdmin(defOrg))),
518549
{
519550
Name: "user-deny-all",
520551
// List out deny permissions explicitly

coderd/rbac/builtin.go

+140-23
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,113 @@ import (
77
)
88

99
const (
10-
Admin = "admin"
11-
Member = "member"
10+
admin string = "admin"
11+
member string = "member"
12+
auditor string = "auditor"
1213

13-
OrganizationMember = "organization-member"
14-
OrganizationAdmin = "organization-admin"
14+
orgAdmin string = "organization-admin"
15+
orgMember string = "organization-member"
16+
)
17+
18+
// RoleName is a string that represents a registered rbac role. We want to store
19+
// strings in the database to allow the underlying role permissions to be migrated
20+
// or modified.
21+
// All RoleNames should have an entry in 'builtInRoles'.
22+
// We use functions to retrieve the name incase we need to add a scope.
23+
type RoleName = string
24+
25+
func RoleAdmin() string {
26+
return roleName(admin, "")
27+
}
28+
29+
func RoleMember() string {
30+
return roleName(member, "")
31+
}
32+
33+
func RoleOrgAdmin(organizationID string) RoleName {
34+
return roleName(orgAdmin, organizationID)
35+
}
36+
37+
func RoleOrgMember(organizationID string) RoleName {
38+
return roleName(orgMember, organizationID)
39+
}
40+
41+
var (
42+
// builtInRoles are just a hard coded set for now. Ideally we store these in
43+
// the database. Right now they are functions because the org id should scope
44+
// certain roles. If we store them in the database, we will need to store
45+
// them such that the "org" permissions are dynamically changed by the
46+
// scopeID passed in. This isn't a hard problem to solve, it's just easier
47+
// as a function right now.
48+
builtInRoles = map[string]func(scopeID string) Role{
49+
// admin grants all actions to all resources.
50+
admin: func(_ string) Role {
51+
return Role{
52+
Name: admin,
53+
Site: permissions(map[Object][]Action{
54+
ResourceWildcard: {WildcardSymbol},
55+
}),
56+
}
57+
},
58+
59+
// member grants all actions to all resources owned by the user
60+
member: func(_ string) Role {
61+
return Role{
62+
Name: member,
63+
User: permissions(map[Object][]Action{
64+
ResourceWildcard: {WildcardSymbol},
65+
}),
66+
}
67+
},
68+
69+
// auditor provides all permissions required to effectively read and understand
70+
// audit log events.
71+
// TODO: Finish the auditor as we add resources.
72+
auditor: func(_ string) Role {
73+
return Role{
74+
Name: "auditor",
75+
Site: permissions(map[Object][]Action{
76+
//ResourceAuditLogs: {ActionRead},
77+
// Should be able to read all template details, even in orgs they
78+
// are not in.
79+
ResourceTemplate: {ActionRead},
80+
}),
81+
}
82+
},
83+
84+
// orgAdmin returns a role with all actions allows in a given
85+
// organization scope.
86+
orgAdmin: func(organizationID string) Role {
87+
return Role{
88+
Name: roleName(orgAdmin, organizationID),
89+
Org: map[string][]Permission{
90+
organizationID: {
91+
{
92+
Negate: false,
93+
ResourceType: "*",
94+
ResourceID: "*",
95+
Action: "*",
96+
},
97+
},
98+
},
99+
}
100+
},
101+
102+
// orgMember has an empty set of permissions, this just implies their membership
103+
// in an organization.
104+
orgMember: func(organizationID string) Role {
105+
return Role{
106+
Name: roleName(orgMember, organizationID),
107+
Org: map[string][]Permission{
108+
organizationID: {},
109+
},
110+
}
111+
},
112+
}
15113
)
16114

17115
// RoleByName returns the permissions associated with a given role name.
18-
// This allows just the role names to be stored.
116+
// This allows just the role names to be stored and expanded when required.
19117
func RoleByName(name string) (Role, error) {
20118
arr := strings.Split(name, ":")
21119
if len(arr) > 2 {
@@ -28,34 +126,53 @@ func RoleByName(name string) (Role, error) {
28126
scopeID = arr[1]
29127
}
30128

31-
// If the role requires a scope, the scope will be checked at the end
32-
// of the switch statement.
33-
var scopedRole Role
34-
switch roleName {
35-
case Admin:
36-
return RoleAdmin, nil
37-
case Member:
38-
return RoleMember, nil
39-
case OrganizationMember:
40-
scopedRole = RoleOrgMember(scopeID)
41-
case OrganizationAdmin:
42-
scopedRole = RoleOrgAdmin(scopeID)
43-
default:
129+
roleFunc, ok := builtInRoles[roleName]
130+
if !ok {
44131
// No role found
45132
return Role{}, xerrors.Errorf("role %q not found", roleName)
46133
}
47134

48-
// Scoped roles should be checked their scope is set
49-
if scopeID == "" {
50-
return Role{}, xerrors.Errorf("%q requires a scope id", roleName)
135+
// Ensure all org roles are properly scoped a non-empty organization id.
136+
// This is just some defensive programming.
137+
role := roleFunc(scopeID)
138+
if len(role.Org) > 0 && scopeID == "" {
139+
return Role{}, xerrors.Errorf("expect a scope id for role %q", roleName)
51140
}
52141

53-
return scopedRole, nil
142+
return role, nil
54143
}
55144

56-
func RoleName(name string, scopeID string) string {
145+
// roleName is a quick helper function to return
146+
// role_name:scopeID
147+
// If no scopeID is required, only 'role_name' is returned
148+
func roleName(name string, scopeID string) string {
57149
if scopeID == "" {
58150
return name
59151
}
60152
return name + ":" + scopeID
61153
}
154+
155+
// permissions is just a helper function to make building roles that list out resources
156+
// and actions a bit easier.
157+
func permissions(perms map[Object][]Action) []Permission {
158+
list := make([]Permission, 0, len(perms))
159+
for k, actions := range perms {
160+
for _, act := range actions {
161+
act := act
162+
list = append(list, Permission{
163+
Negate: false,
164+
ResourceType: k.Type,
165+
ResourceID: WildcardSymbol,
166+
Action: act,
167+
})
168+
}
169+
}
170+
return list
171+
}
172+
173+
func must[T any](value T, err error) T {
174+
if err != nil {
175+
panic(err)
176+
}
177+
return value
178+
}

coderd/rbac/example_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ func TestExample(t *testing.T) {
2222
user := subject{
2323
UserID: "alice",
2424
Roles: []rbac.Role{
25-
must(rbac.RoleByName(rbac.Member)),
26-
must(rbac.RoleByName(rbac.RoleName(rbac.OrganizationMember, "default"))),
25+
must(rbac.RoleByName(rbac.RoleMember())),
26+
must(rbac.RoleByName(rbac.RoleOrgAdmin("default"))),
2727
},
2828
}
2929

0 commit comments

Comments
 (0)