diff --git a/coderd/coderdtest/authorize.go b/coderd/coderdtest/authorize.go index 462dcc644f56f..03ad70ed338e6 100644 --- a/coderd/coderdtest/authorize.go +++ b/coderd/coderdtest/authorize.go @@ -529,7 +529,7 @@ func (a *AuthTester) Test(ctx context.Context, assertRoute map[string]RouteCheck type authCall struct { SubjectID string - Roles []string + Roles rbac.ExpandableRoles Groups []string Scope rbac.ScopeName Action rbac.Action @@ -545,11 +545,11 @@ var _ rbac.Authorizer = (*RecordingAuthorizer)(nil) // ByRoleNameSQL does not record the call. This matches the postgres behavior // of not calling Authorize() -func (r *RecordingAuthorizer) ByRoleNameSQL(_ context.Context, _ string, _ []string, _ rbac.ScopeName, _ []string, _ rbac.Action, _ rbac.Object) error { +func (r *RecordingAuthorizer) ByRoleNameSQL(_ context.Context, _ string, _ rbac.ExpandableRoles, _ rbac.ScopeName, _ []string, _ rbac.Action, _ rbac.Object) error { return r.AlwaysReturn } -func (r *RecordingAuthorizer) ByRoleName(_ context.Context, subjectID string, roleNames []string, scope rbac.ScopeName, groups []string, action rbac.Action, object rbac.Object) error { +func (r *RecordingAuthorizer) ByRoleName(_ context.Context, subjectID string, roleNames rbac.ExpandableRoles, scope rbac.ScopeName, groups []string, action rbac.Action, object rbac.Object) error { r.Called = &authCall{ SubjectID: subjectID, Roles: roleNames, @@ -561,7 +561,7 @@ func (r *RecordingAuthorizer) ByRoleName(_ context.Context, subjectID string, ro return r.AlwaysReturn } -func (r *RecordingAuthorizer) PrepareByRoleName(_ context.Context, subjectID string, roles []string, scope rbac.ScopeName, groups []string, action rbac.Action, _ string) (rbac.PreparedAuthorized, error) { +func (r *RecordingAuthorizer) PrepareByRoleName(_ context.Context, subjectID string, roles rbac.ExpandableRoles, scope rbac.ScopeName, groups []string, action rbac.Action, _ string) (rbac.PreparedAuthorized, error) { return &fakePreparedAuthorizer{ Original: r, SubjectID: subjectID, @@ -580,7 +580,7 @@ func (r *RecordingAuthorizer) reset() { type fakePreparedAuthorizer struct { Original *RecordingAuthorizer SubjectID string - Roles []string + Roles rbac.ExpandableRoles Scope rbac.ScopeName Action rbac.Action Groups []string diff --git a/coderd/httpmw/apikey.go b/coderd/httpmw/apikey.go index c72d0d40e606a..14250c3e59583 100644 --- a/coderd/httpmw/apikey.go +++ b/coderd/httpmw/apikey.go @@ -20,6 +20,7 @@ import ( "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/httpapi" + "github.com/coder/coder/coderd/rbac" "github.com/coder/coder/codersdk" ) @@ -53,7 +54,7 @@ type userAuthKey struct{} type Authorization struct { ID uuid.UUID Username string - Roles []string + Roles rbac.RoleNames Groups []string Scope database.APIKeyScope } diff --git a/coderd/rbac/authz.go b/coderd/rbac/authz.go index d3e43b4e181a3..888176c212780 100644 --- a/coderd/rbac/authz.go +++ b/coderd/rbac/authz.go @@ -17,9 +17,24 @@ import ( "github.com/coder/coder/coderd/tracing" ) +// ExpandableRoles is any type that can be expanded into a []Role. This is implemented +// as an interface so we can have RoleNames for user defined roles, and implement +// custom ExpandableRoles for system type users (eg autostart/autostop system role). +// We want a clear divide between the two types of roles so users have no codepath +// to interact or assign system roles. +// +// Note: We may also want to do the same thing with scopes to allow custom scope +// support unavailable to the user. Eg: Scope to a single resource. +type ExpandableRoles interface { + Expand() ([]Role, error) + // Names is for logging and tracing purposes, we want to know the human + // names of the expanded roles. + Names() []string +} + type Authorizer interface { - ByRoleName(ctx context.Context, subjectID string, roleNames []string, scope ScopeName, groups []string, action Action, object Object) error - PrepareByRoleName(ctx context.Context, subjectID string, roleNames []string, scope ScopeName, groups []string, action Action, objectType string) (PreparedAuthorized, error) + ByRoleName(ctx context.Context, subjectID string, roleNames ExpandableRoles, scope ScopeName, groups []string, action Action, object Object) error + PrepareByRoleName(ctx context.Context, subjectID string, roleNames ExpandableRoles, scope ScopeName, groups []string, action Action, objectType string) (PreparedAuthorized, error) } type PreparedAuthorized interface { @@ -33,7 +48,7 @@ type PreparedAuthorized interface { // // Ideally the 'CompileToSQL' is used instead for large sets. This cost scales // linearly with the number of objects passed in. -func Filter[O Objecter](ctx context.Context, auth Authorizer, subjID string, subjRoles []string, scope ScopeName, groups []string, action Action, objects []O) ([]O, error) { +func Filter[O Objecter](ctx context.Context, auth Authorizer, subjID string, subjRoles ExpandableRoles, scope ScopeName, groups []string, action Action, objects []O) ([]O, error) { if len(objects) == 0 { // Nothing to filter return objects, nil @@ -45,7 +60,7 @@ func Filter[O Objecter](ctx context.Context, auth Authorizer, subjID string, sub // objects, then the span is not interesting. It would just add excessive // 0 time spans that provide no insight. ctx, span := tracing.StartSpan(ctx, - rbacTraceAttributes(subjRoles, len(groups), scope, action, objectType, + rbacTraceAttributes(subjRoles.Names(), len(groups), scope, action, objectType, // For filtering, we are only measuring the total time for the entire // set of objects. This and the 'PrepareByRoleName' span time // is all that is required to measure the performance of this @@ -179,11 +194,11 @@ type authSubject struct { // ByRoleName will expand all roleNames into roles before calling Authorize(). // This is the function intended to be used outside this package. // The role is fetched from the builtin map located in memory. -func (a RegoAuthorizer) ByRoleName(ctx context.Context, subjectID string, roleNames []string, scope ScopeName, groups []string, action Action, object Object) error { +func (a RegoAuthorizer) ByRoleName(ctx context.Context, subjectID string, roleNames ExpandableRoles, scope ScopeName, groups []string, action Action, object Object) error { start := time.Now() ctx, span := tracing.StartSpan(ctx, trace.WithTimestamp(start), // Reuse the time.Now for metric and trace - rbacTraceAttributes(roleNames, len(groups), scope, action, object.Type, + rbacTraceAttributes(roleNames.Names(), len(groups), scope, action, object.Type, // For authorizing a single object, this data is useful to know how // complex our objects are getting. attribute.Int("object_num_groups", len(object.ACLGroupList)), @@ -192,7 +207,7 @@ func (a RegoAuthorizer) ByRoleName(ctx context.Context, subjectID string, roleNa ) defer span.End() - roles, err := RolesByNames(roleNames) + roles, err := roleNames.Expand() if err != nil { return err } @@ -239,15 +254,15 @@ func (a RegoAuthorizer) Authorize(ctx context.Context, subjectID string, roles [ return nil } -func (a RegoAuthorizer) PrepareByRoleName(ctx context.Context, subjectID string, roleNames []string, scope ScopeName, groups []string, action Action, objectType string) (PreparedAuthorized, error) { +func (a RegoAuthorizer) PrepareByRoleName(ctx context.Context, subjectID string, roleNames ExpandableRoles, scope ScopeName, groups []string, action Action, objectType string) (PreparedAuthorized, error) { start := time.Now() ctx, span := tracing.StartSpan(ctx, trace.WithTimestamp(start), - rbacTraceAttributes(roleNames, len(groups), scope, action, objectType), + rbacTraceAttributes(roleNames.Names(), len(groups), scope, action, objectType), ) defer span.End() - roles, err := RolesByNames(roleNames) + roles, err := roleNames.Expand() if err != nil { return nil, err } diff --git a/coderd/rbac/authz_internal_test.go b/coderd/rbac/authz_internal_test.go index 204d65551f47e..0afff5226ec38 100644 --- a/coderd/rbac/authz_internal_test.go +++ b/coderd/rbac/authz_internal_test.go @@ -44,7 +44,7 @@ func TestFilterError(t *testing.T) { t.Parallel() auth := NewAuthorizer(prometheus.NewRegistry()) - _, err := Filter(context.Background(), auth, uuid.NewString(), []string{}, ScopeAll, []string{}, ActionRead, []Object{ResourceUser, ResourceWorkspace}) + _, err := Filter(context.Background(), auth, uuid.NewString(), RoleNames{}, ScopeAll, []string{}, ActionRead, []Object{ResourceUser, ResourceWorkspace}) require.ErrorContains(t, err, "object types must be uniform") } @@ -75,7 +75,7 @@ func TestFilter(t *testing.T) { testCases := []struct { Name string SubjectID string - Roles []string + Roles RoleNames Action Action Scope ScopeName ObjectType string diff --git a/coderd/rbac/authz_test.go b/coderd/rbac/authz_test.go index eb36fdb665537..f8588fa9baae5 100644 --- a/coderd/rbac/authz_test.go +++ b/coderd/rbac/authz_test.go @@ -13,7 +13,7 @@ import ( type benchmarkCase struct { Name string - Roles []string + Roles rbac.RoleNames Groups []string UserID uuid.UUID Scope rbac.ScopeName diff --git a/coderd/rbac/builtin.go b/coderd/rbac/builtin.go index 964eec3a7386f..76d7f373a16f9 100644 --- a/coderd/rbac/builtin.go +++ b/coderd/rbac/builtin.go @@ -19,6 +19,19 @@ const ( orgMember string = "organization-member" ) +// RoleNames is a list of user assignable role names. The role names must be +// in the builtInRoles map. Any non-user assignable roles will generate an +// error on Expand. +type RoleNames []string + +func (names RoleNames) Expand() ([]Role, error) { + return rolesByNames(names) +} + +func (names RoleNames) Names() []string { + return names +} + // The functions below ONLY need to exist for roles that are "defaulted" in some way. // Any other roles (like auditor), can be listed and let the user select/assigned. // 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 { // RoleByName returns the permissions associated with a given role name. // This allows just the role names to be stored and expanded when required. +// +// This function is exported so that the Display name can be returned to the +// api. We should maybe make an exported function that returns just the +// human-readable content of the Role struct (name + display name). func RoleByName(name string) (Role, error) { roleName, orgID, err := roleSplit(name) if err != nil { @@ -266,7 +283,7 @@ func RoleByName(name string) (Role, error) { return role, nil } -func RolesByNames(roleNames []string) ([]Role, error) { +func rolesByNames(roleNames []string) ([]Role, error) { roles := make([]Role, 0, len(roleNames)) for _, n := range roleNames { r, err := RoleByName(n) diff --git a/coderd/rbac/builtin_test.go b/coderd/rbac/builtin_test.go index 6a15e77f5415d..cdbf652367ca1 100644 --- a/coderd/rbac/builtin_test.go +++ b/coderd/rbac/builtin_test.go @@ -17,7 +17,7 @@ type authSubject struct { // Name is helpful for test assertions Name string UserID string - Roles []string + Roles rbac.RoleNames Groups []string }