Skip to content

Commit 4f18d66

Browse files
EmyrkEmyrk
committed
chore: create type for unique role names
Using `string` was confusing when something should be combined with org context, and when not to
1 parent 5b9a65e commit 4f18d66

File tree

1 file changed

+54
-41
lines changed

1 file changed

+54
-41
lines changed

‎coderd/rbac/roles.go

+54-41
Original file line numberDiff line numberDiff line change
@@ -47,29 +47,54 @@ func (names RoleNames) Names() []string {
4747
return names
4848
}
4949

50+
// UniqueRoleName is formatted as <role_name>[:<org_id/scope_id>] to create globally unique
51+
// names for a given role. When passing the roles into rego, this helps disambiguate
52+
// 2 roles with the same common name across organizations.
53+
// TODO: Would be worth refactoring the rego to take a structure of:
54+
// {Name string, Scope string} to avoid needing to do this as a single string.
55+
type UniqueRoleName string
56+
57+
func (u UniqueRoleName) Split() (name string, scope string, err error) {
58+
arr := strings.Split(string(u), ":")
59+
if len(arr) > 2 {
60+
return "", "", xerrors.Errorf("too many colons in role name")
61+
}
62+
if arr[0] == "" {
63+
return "", "", xerrors.Errorf("role cannot be the empty string")
64+
}
65+
if len(arr) == 2 {
66+
return arr[0], arr[1], nil
67+
}
68+
return arr[0], "", nil
69+
}
70+
5071
// The functions below ONLY need to exist for roles that are "defaulted" in some way.
5172
// Any other roles (like auditor), can be listed and let the user select/assigned.
5273
// Once we have a database implementation, the "default" roles can be defined on the
5374
// site and orgs, and these functions can be removed.
5475

55-
func RoleOwner() string {
76+
func RoleOwner() UniqueRoleName {
5677
return RoleName(owner, "")
5778
}
5879

59-
func CustomSiteRole() string { return RoleName(customSiteRole, "") }
80+
func CustomSiteRole() UniqueRoleName { return RoleName(customSiteRole, "") }
6081

61-
func RoleTemplateAdmin() string {
82+
func RoleTemplateAdmin() UniqueRoleName {
6283
return RoleName(templateAdmin, "")
6384
}
6485

65-
func RoleUserAdmin() string {
86+
func RoleUserAdmin() UniqueRoleName {
6687
return RoleName(userAdmin, "")
6788
}
6889

69-
func RoleMember() string {
90+
func RoleMember() UniqueRoleName {
7091
return RoleName(member, "")
7192
}
7293

94+
func RoleAuditor() UniqueRoleName {
95+
return RoleName(auditor, "")
96+
}
97+
7398
func RoleOrgAdmin() string {
7499
return orgAdmin
75100
}
@@ -81,14 +106,14 @@ func RoleOrgMember() string {
81106
// ScopedRoleOrgAdmin is the org role with the organization ID
82107
// Deprecated This was used before organization scope was included as a
83108
// field in all user facing APIs. Usage of 'ScopedRoleOrgAdmin()' is preferred.
84-
func ScopedRoleOrgAdmin(organizationID uuid.UUID) string {
109+
func ScopedRoleOrgAdmin(organizationID uuid.UUID) UniqueRoleName {
85110
return RoleName(orgAdmin, organizationID.String())
86111
}
87112

88113
// ScopedRoleOrgMember is the org role with the organization ID
89114
// Deprecated This was used before organization scope was included as a
90115
// field in all user facing APIs. Usage of 'ScopedRoleOrgMember()' is preferred.
91-
func ScopedRoleOrgMember(organizationID uuid.UUID) string {
116+
func ScopedRoleOrgMember(organizationID uuid.UUID) UniqueRoleName {
92117
return RoleName(orgMember, organizationID.String())
93118
}
94119

@@ -158,7 +183,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
158183
// on every authorize call. 'withCachedRegoValue' can be used as well to
159184
// preallocate the rego value that is used by the rego eval engine.
160185
ownerRole := Role{
161-
Name: owner,
186+
Name: RoleOwner(),
162187
DisplayName: "Owner",
163188
Site: append(
164189
// Workspace dormancy and workspace are omitted.
@@ -174,7 +199,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
174199
}.withCachedRegoValue()
175200

176201
memberRole := Role{
177-
Name: member,
202+
Name: RoleMember(),
178203
DisplayName: "Member",
179204
Site: Permissions(map[string][]policy.Action{
180205
ResourceAssignRole.Type: {policy.ActionRead},
@@ -200,7 +225,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
200225
}.withCachedRegoValue()
201226

202227
auditorRole := Role{
203-
Name: auditor,
228+
Name: RoleAuditor(),
204229
DisplayName: "Auditor",
205230
Site: Permissions(map[string][]policy.Action{
206231
// Should be able to read all template details, even in orgs they
@@ -220,7 +245,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
220245
}.withCachedRegoValue()
221246

222247
templateAdminRole := Role{
223-
Name: templateAdmin,
248+
Name: RoleTemplateAdmin(),
224249
DisplayName: "Template Admin",
225250
Site: Permissions(map[string][]policy.Action{
226251
ResourceTemplate.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete, policy.ActionViewInsights},
@@ -241,7 +266,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
241266
}.withCachedRegoValue()
242267

243268
userAdminRole := Role{
244-
Name: userAdmin,
269+
Name: RoleUserAdmin(),
245270
DisplayName: "User Admin",
246271
Site: Permissions(map[string][]policy.Action{
247272
ResourceAssignRole.Type: {policy.ActionAssign, policy.ActionDelete, policy.ActionRead},
@@ -381,7 +406,7 @@ type ExpandableRoles interface {
381406
Expand() ([]Role, error)
382407
// Names is for logging and tracing purposes, we want to know the human
383408
// names of the expanded roles.
384-
Names() []string
409+
Names() []UniqueRoleName
385410
}
386411

387412
// Permission is the format passed into the rego.
@@ -424,7 +449,7 @@ func (perm Permission) Valid() error {
424449
// Users of this package should instead **only** use the role names, and
425450
// this package will expand the role names into their json payloads.
426451
type Role struct {
427-
Name string `json:"name"`
452+
Name UniqueRoleName `json:"name"`
428453
// DisplayName is used for UI purposes. If the role has no display name,
429454
// that means the UI should never display it.
430455
DisplayName string `json:"display_name"`
@@ -474,8 +499,8 @@ func (roles Roles) Expand() ([]Role, error) {
474499
return roles, nil
475500
}
476501

477-
func (roles Roles) Names() []string {
478-
names := make([]string, 0, len(roles))
502+
func (roles Roles) Names() []UniqueRoleName {
503+
names := make([]UniqueRoleName, 0, len(roles))
479504
for _, r := range roles {
480505
names = append(names, r.Name)
481506
}
@@ -485,17 +510,17 @@ func (roles Roles) Names() []string {
485510
// CanAssignRole is a helper function that returns true if the user can assign
486511
// the specified role. This also can be used for removing a role.
487512
// This is a simple implementation for now.
488-
func CanAssignRole(expandable ExpandableRoles, assignedRole string) bool {
513+
func CanAssignRole(expandable ExpandableRoles, assignedRole UniqueRoleName) bool {
489514
// For CanAssignRole, we only care about the names of the roles.
490515
roles := expandable.Names()
491516

492-
assigned, assignedOrg, err := RoleSplit(assignedRole)
517+
assigned, assignedOrg, err := assignedRole.Split()
493518
if err != nil {
494519
return false
495520
}
496521

497522
for _, longRole := range roles {
498-
role, orgID, err := RoleSplit(longRole)
523+
role, orgID, err := longRole.Split()
499524
if err != nil {
500525
continue
501526
}
@@ -523,8 +548,8 @@ func CanAssignRole(expandable ExpandableRoles, assignedRole string) bool {
523548
// This function is exported so that the Display name can be returned to the
524549
// api. We should maybe make an exported function that returns just the
525550
// human-readable content of the Role struct (name + display name).
526-
func RoleByName(name string) (Role, error) {
527-
roleName, orgID, err := RoleSplit(name)
551+
func RoleByName(name UniqueRoleName) (Role, error) {
552+
roleName, orgID, err := name.Split()
528553
if err != nil {
529554
return Role{}, xerrors.Errorf("parse role name: %w", err)
530555
}
@@ -545,7 +570,7 @@ func RoleByName(name string) (Role, error) {
545570
return role, nil
546571
}
547572

548-
func rolesByNames(roleNames []string) ([]Role, error) {
573+
func rolesByNames(roleNames []UniqueRoleName) ([]Role, error) {
549574
roles := make([]Role, 0, len(roleNames))
550575
for _, n := range roleNames {
551576
r, err := RoleByName(n)
@@ -557,8 +582,8 @@ func rolesByNames(roleNames []string) ([]Role, error) {
557582
return roles, nil
558583
}
559584

560-
func IsOrgRole(roleName string) (string, bool) {
561-
_, orgID, err := RoleSplit(roleName)
585+
func IsOrgRole(roleName UniqueRoleName) (string, bool) {
586+
_, orgID, err := roleName.Split()
562587
if err == nil && orgID != "" {
563588
return orgID, true
564589
}
@@ -644,27 +669,15 @@ func ChangeRoleSet(from []string, to []string) (added []string, removed []string
644669
// role_name:scopeID
645670
//
646671
// If no scopeID is required, only 'role_name' is returned
647-
func RoleName(name string, orgID string) string {
672+
func RoleName(name string, orgID string) UniqueRoleName {
648673
if orgID == "" {
649-
return name
674+
return UniqueRoleName(name)
650675
}
651-
return name + ":" + orgID
676+
return UniqueRoleName(name) + UniqueRoleName(":"+orgID)
652677
}
653678

654-
func RoleSplit(role string) (name string, orgID string, err error) {
655-
arr := strings.Split(role, ":")
656-
if len(arr) > 2 {
657-
return "", "", xerrors.Errorf("too many colons in role name")
658-
}
659-
660-
if arr[0] == "" {
661-
return "", "", xerrors.Errorf("role cannot be the empty string")
662-
}
663-
664-
if len(arr) == 2 {
665-
return arr[0], arr[1], nil
666-
}
667-
return arr[0], "", nil
679+
func RoleSplit(role UniqueRoleName) (name string, orgID string, err error) {
680+
return role.Split()
668681
}
669682

670683
// Permissions is just a helper function to make building roles that list out resources

0 commit comments

Comments
 (0)