Skip to content

Commit b678309

Browse files
EmyrkEmyrk
authored
chore: Authz should support non-named roles (#5855)
* chore: Authz should support non-named roles Named roles are a construct for users to assign/interact with roles. For authzlayer implementation, we need to create "system" users. To enforce strict security, we are making specific roles with the exact required permissions for the system action. These new roles should not be available to the user. There is a clear code divide with this implementation that allows a RoleNames implemenation for users to user, and system users can create their own implementation
1 parent de66f0d commit b678309

File tree

7 files changed

+54
-21
lines changed

7 files changed

+54
-21
lines changed

‎coderd/coderdtest/authorize.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,7 @@ func (a *AuthTester) Test(ctx context.Context, assertRoute map[string]RouteCheck
529529

530530
type authCall struct {
531531
SubjectID string
532-
Roles []string
532+
Roles rbac.ExpandableRoles
533533
Groups []string
534534
Scope rbac.ScopeName
535535
Action rbac.Action
@@ -545,11 +545,11 @@ var _ rbac.Authorizer = (*RecordingAuthorizer)(nil)
545545

546546
// ByRoleNameSQL does not record the call. This matches the postgres behavior
547547
// of not calling Authorize()
548-
func (r *RecordingAuthorizer) ByRoleNameSQL(_ context.Context, _ string, _ []string, _ rbac.ScopeName, _ []string, _ rbac.Action, _ rbac.Object) error {
548+
func (r *RecordingAuthorizer) ByRoleNameSQL(_ context.Context, _ string, _ rbac.ExpandableRoles, _ rbac.ScopeName, _ []string, _ rbac.Action, _ rbac.Object) error {
549549
return r.AlwaysReturn
550550
}
551551

552-
func (r *RecordingAuthorizer) ByRoleName(_ context.Context, subjectID string, roleNames []string, scope rbac.ScopeName, groups []string, action rbac.Action, object rbac.Object) error {
552+
func (r *RecordingAuthorizer) ByRoleName(_ context.Context, subjectID string, roleNames rbac.ExpandableRoles, scope rbac.ScopeName, groups []string, action rbac.Action, object rbac.Object) error {
553553
r.Called = &authCall{
554554
SubjectID: subjectID,
555555
Roles: roleNames,
@@ -561,7 +561,7 @@ func (r *RecordingAuthorizer) ByRoleName(_ context.Context, subjectID string, ro
561561
return r.AlwaysReturn
562562
}
563563

564-
func (r *RecordingAuthorizer) PrepareByRoleName(_ context.Context, subjectID string, roles []string, scope rbac.ScopeName, groups []string, action rbac.Action, _ string) (rbac.PreparedAuthorized, error) {
564+
func (r *RecordingAuthorizer) PrepareByRoleName(_ context.Context, subjectID string, roles rbac.ExpandableRoles, scope rbac.ScopeName, groups []string, action rbac.Action, _ string) (rbac.PreparedAuthorized, error) {
565565
return &fakePreparedAuthorizer{
566566
Original: r,
567567
SubjectID: subjectID,
@@ -580,7 +580,7 @@ func (r *RecordingAuthorizer) reset() {
580580
type fakePreparedAuthorizer struct {
581581
Original *RecordingAuthorizer
582582
SubjectID string
583-
Roles []string
583+
Roles rbac.ExpandableRoles
584584
Scope rbac.ScopeName
585585
Action rbac.Action
586586
Groups []string

‎coderd/httpmw/apikey.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020

2121
"github.com/coder/coder/coderd/database"
2222
"github.com/coder/coder/coderd/httpapi"
23+
"github.com/coder/coder/coderd/rbac"
2324
"github.com/coder/coder/codersdk"
2425
)
2526

@@ -53,7 +54,7 @@ type userAuthKey struct{}
5354
type Authorization struct {
5455
ID uuid.UUID
5556
Username string
56-
Roles []string
57+
Roles rbac.RoleNames
5758
Groups []string
5859
Scope database.APIKeyScope
5960
}

‎coderd/rbac/authz.go

+25-10
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,24 @@ import (
1717
"github.com/coder/coder/coderd/tracing"
1818
)
1919

20+
// ExpandableRoles is any type that can be expanded into a []Role. This is implemented
21+
// as an interface so we can have RoleNames for user defined roles, and implement
22+
// custom ExpandableRoles for system type users (eg autostart/autostop system role).
23+
// We want a clear divide between the two types of roles so users have no codepath
24+
// to interact or assign system roles.
25+
//
26+
// Note: We may also want to do the same thing with scopes to allow custom scope
27+
// support unavailable to the user. Eg: Scope to a single resource.
28+
type ExpandableRoles interface {
29+
Expand() ([]Role, error)
30+
// Names is for logging and tracing purposes, we want to know the human
31+
// names of the expanded roles.
32+
Names() []string
33+
}
34+
2035
type Authorizer interface {
21-
ByRoleName(ctx context.Context, subjectID string, roleNames []string, scope ScopeName, groups []string, action Action, object Object) error
22-
PrepareByRoleName(ctx context.Context, subjectID string, roleNames []string, scope ScopeName, groups []string, action Action, objectType string) (PreparedAuthorized, error)
36+
ByRoleName(ctx context.Context, subjectID string, roleNames ExpandableRoles, scope ScopeName, groups []string, action Action, object Object) error
37+
PrepareByRoleName(ctx context.Context, subjectID string, roleNames ExpandableRoles, scope ScopeName, groups []string, action Action, objectType string) (PreparedAuthorized, error)
2338
}
2439

2540
type PreparedAuthorized interface {
@@ -33,7 +48,7 @@ type PreparedAuthorized interface {
3348
//
3449
// Ideally the 'CompileToSQL' is used instead for large sets. This cost scales
3550
// linearly with the number of objects passed in.
36-
func Filter[O Objecter](ctx context.Context, auth Authorizer, subjID string, subjRoles []string, scope ScopeName, groups []string, action Action, objects []O) ([]O, error) {
51+
func Filter[O Objecter](ctx context.Context, auth Authorizer, subjID string, subjRoles ExpandableRoles, scope ScopeName, groups []string, action Action, objects []O) ([]O, error) {
3752
if len(objects) == 0 {
3853
// Nothing to filter
3954
return objects, nil
@@ -45,7 +60,7 @@ func Filter[O Objecter](ctx context.Context, auth Authorizer, subjID string, sub
4560
// objects, then the span is not interesting. It would just add excessive
4661
// 0 time spans that provide no insight.
4762
ctx, span := tracing.StartSpan(ctx,
48-
rbacTraceAttributes(subjRoles, len(groups), scope, action, objectType,
63+
rbacTraceAttributes(subjRoles.Names(), len(groups), scope, action, objectType,
4964
// For filtering, we are only measuring the total time for the entire
5065
// set of objects. This and the 'PrepareByRoleName' span time
5166
// is all that is required to measure the performance of this
@@ -179,11 +194,11 @@ type authSubject struct {
179194
// ByRoleName will expand all roleNames into roles before calling Authorize().
180195
// This is the function intended to be used outside this package.
181196
// The role is fetched from the builtin map located in memory.
182-
func (a RegoAuthorizer) ByRoleName(ctx context.Context, subjectID string, roleNames []string, scope ScopeName, groups []string, action Action, object Object) error {
197+
func (a RegoAuthorizer) ByRoleName(ctx context.Context, subjectID string, roleNames ExpandableRoles, scope ScopeName, groups []string, action Action, object Object) error {
183198
start := time.Now()
184199
ctx, span := tracing.StartSpan(ctx,
185200
trace.WithTimestamp(start), // Reuse the time.Now for metric and trace
186-
rbacTraceAttributes(roleNames, len(groups), scope, action, object.Type,
201+
rbacTraceAttributes(roleNames.Names(), len(groups), scope, action, object.Type,
187202
// For authorizing a single object, this data is useful to know how
188203
// complex our objects are getting.
189204
attribute.Int("object_num_groups", len(object.ACLGroupList)),
@@ -192,7 +207,7 @@ func (a RegoAuthorizer) ByRoleName(ctx context.Context, subjectID string, roleNa
192207
)
193208
defer span.End()
194209

195-
roles, err := RolesByNames(roleNames)
210+
roles, err := roleNames.Expand()
196211
if err != nil {
197212
return err
198213
}
@@ -239,15 +254,15 @@ func (a RegoAuthorizer) Authorize(ctx context.Context, subjectID string, roles [
239254
return nil
240255
}
241256

242-
func (a RegoAuthorizer) PrepareByRoleName(ctx context.Context, subjectID string, roleNames []string, scope ScopeName, groups []string, action Action, objectType string) (PreparedAuthorized, error) {
257+
func (a RegoAuthorizer) PrepareByRoleName(ctx context.Context, subjectID string, roleNames ExpandableRoles, scope ScopeName, groups []string, action Action, objectType string) (PreparedAuthorized, error) {
243258
start := time.Now()
244259
ctx, span := tracing.StartSpan(ctx,
245260
trace.WithTimestamp(start),
246-
rbacTraceAttributes(roleNames, len(groups), scope, action, objectType),
261+
rbacTraceAttributes(roleNames.Names(), len(groups), scope, action, objectType),
247262
)
248263
defer span.End()
249264

250-
roles, err := RolesByNames(roleNames)
265+
roles, err := roleNames.Expand()
251266
if err != nil {
252267
return nil, err
253268
}

‎coderd/rbac/authz_internal_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func TestFilterError(t *testing.T) {
4444
t.Parallel()
4545
auth := NewAuthorizer(prometheus.NewRegistry())
4646

47-
_, err := Filter(context.Background(), auth, uuid.NewString(), []string{}, ScopeAll, []string{}, ActionRead, []Object{ResourceUser, ResourceWorkspace})
47+
_, err := Filter(context.Background(), auth, uuid.NewString(), RoleNames{}, ScopeAll, []string{}, ActionRead, []Object{ResourceUser, ResourceWorkspace})
4848
require.ErrorContains(t, err, "object types must be uniform")
4949
}
5050

@@ -75,7 +75,7 @@ func TestFilter(t *testing.T) {
7575
testCases := []struct {
7676
Name string
7777
SubjectID string
78-
Roles []string
78+
Roles RoleNames
7979
Action Action
8080
Scope ScopeName
8181
ObjectType string

‎coderd/rbac/authz_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import (
1313

1414
type benchmarkCase struct {
1515
Name string
16-
Roles []string
16+
Roles rbac.RoleNames
1717
Groups []string
1818
UserID uuid.UUID
1919
Scope rbac.ScopeName

‎coderd/rbac/builtin.go

+18-1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,19 @@ const (
1919
orgMember string = "organization-member"
2020
)
2121

22+
// RoleNames is a list of user assignable role names. The role names must be
23+
// in the builtInRoles map. Any non-user assignable roles will generate an
24+
// error on Expand.
25+
type RoleNames []string
26+
27+
func (names RoleNames) Expand() ([]Role, error) {
28+
return rolesByNames(names)
29+
}
30+
31+
func (names RoleNames) Names() []string {
32+
return names
33+
}
34+
2235
// The functions below ONLY need to exist for roles that are "defaulted" in some way.
2336
// Any other roles (like auditor), can be listed and let the user select/assigned.
2437
// Once we have a database implementation, the "default" roles can be defined on the
@@ -244,6 +257,10 @@ func CanAssignRole(roles []string, assignedRole string) bool {
244257

245258
// RoleByName returns the permissions associated with a given role name.
246259
// This allows just the role names to be stored and expanded when required.
260+
//
261+
// This function is exported so that the Display name can be returned to the
262+
// api. We should maybe make an exported function that returns just the
263+
// human-readable content of the Role struct (name + display name).
247264
func RoleByName(name string) (Role, error) {
248265
roleName, orgID, err := roleSplit(name)
249266
if err != nil {
@@ -266,7 +283,7 @@ func RoleByName(name string) (Role, error) {
266283
return role, nil
267284
}
268285

269-
func RolesByNames(roleNames []string) ([]Role, error) {
286+
func rolesByNames(roleNames []string) ([]Role, error) {
270287
roles := make([]Role, 0, len(roleNames))
271288
for _, n := range roleNames {
272289
r, err := RoleByName(n)

‎coderd/rbac/builtin_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ type authSubject struct {
1717
// Name is helpful for test assertions
1818
Name string
1919
UserID string
20-
Roles []string
20+
Roles rbac.RoleNames
2121
Groups []string
2222
}
2323

0 commit comments

Comments
 (0)