diff --git a/coderd/authorize.go b/coderd/authorize.go index 3acdd3d1d9..ab1f3a39fd 100644 --- a/coderd/authorize.go +++ b/coderd/authorize.go @@ -19,14 +19,15 @@ import ( // This is faster than calling Authorize() on each object. func AuthorizeFilter[O rbac.Objecter](h *HTTPAuthorizer, r *http.Request, action rbac.Action, objects []O) ([]O, error) { roles := httpmw.UserAuthorization(r) - objects, err := rbac.Filter(r.Context(), h.Authorizer, roles.ID.String(), roles.Roles, roles.Scope.ToRBAC(), roles.Groups, action, objects) + objects, err := rbac.Filter(r.Context(), h.Authorizer, roles.Actor, action, objects) if err != nil { // Log the error as Filter should not be erroring. h.Logger.Error(r.Context(), "filter failed", slog.Error(err), - slog.F("user_id", roles.ID), + slog.F("user_id", roles.Actor.ID), slog.F("username", roles.Username), - slog.F("scope", roles.Scope), + slog.F("roles", roles.Actor.SafeRoleNames()), + slog.F("scope", roles.Actor.SafeScopeName()), slog.F("route", r.URL.Path), slog.F("action", action), ) @@ -64,7 +65,7 @@ func (api *API) Authorize(r *http.Request, action rbac.Action, object rbac.Objec // } func (h *HTTPAuthorizer) Authorize(r *http.Request, action rbac.Action, object rbac.Objecter) bool { roles := httpmw.UserAuthorization(r) - err := h.Authorizer.ByRoleName(r.Context(), roles.ID.String(), roles.Roles, roles.Scope.ToRBAC(), roles.Groups, action, object.RBACObject()) + err := h.Authorizer.Authorize(r.Context(), roles.Actor, action, object.RBACObject()) if err != nil { // Log the errors for debugging internalError := new(rbac.UnauthorizedError) @@ -75,10 +76,10 @@ func (h *HTTPAuthorizer) Authorize(r *http.Request, action rbac.Action, object r // Log information for debugging. This will be very helpful // in the early days logger.Warn(r.Context(), "unauthorized", - slog.F("roles", roles.Roles), - slog.F("user_id", roles.ID), + slog.F("roles", roles.Actor.SafeRoleNames()), + slog.F("user_id", roles.Actor.ID), slog.F("username", roles.Username), - slog.F("scope", roles.Scope), + slog.F("scope", roles.Actor.SafeScopeName()), slog.F("route", r.URL.Path), slog.F("action", action), slog.F("object", object), @@ -96,7 +97,7 @@ func (h *HTTPAuthorizer) Authorize(r *http.Request, action rbac.Action, object r // Note the authorization is only for the given action and object type. func (h *HTTPAuthorizer) AuthorizeSQLFilter(r *http.Request, action rbac.Action, objectType string) (rbac.PreparedAuthorized, error) { roles := httpmw.UserAuthorization(r) - prepared, err := h.Authorizer.PrepareByRoleName(r.Context(), roles.ID.String(), roles.Roles, roles.Scope.ToRBAC(), roles.Groups, action, objectType) + prepared, err := h.Authorizer.Prepare(r.Context(), roles.Actor, action, objectType) if err != nil { return nil, xerrors.Errorf("prepare filter: %w", err) } @@ -127,9 +128,10 @@ func (api *API) checkAuthorization(rw http.ResponseWriter, r *http.Request) { api.Logger.Debug(ctx, "check-auth", slog.F("my_id", httpmw.APIKey(r).UserID), - slog.F("got_id", auth.ID), + slog.F("got_id", auth.Actor.ID), slog.F("name", auth.Username), - slog.F("roles", auth.Roles), slog.F("scope", auth.Scope), + slog.F("roles", auth.Actor.SafeRoleNames()), + slog.F("scope", auth.Actor.SafeScopeName()), ) response := make(codersdk.AuthorizationResponse) @@ -169,7 +171,7 @@ func (api *API) checkAuthorization(rw http.ResponseWriter, r *http.Request) { Type: v.Object.ResourceType, } if obj.Owner == "me" { - obj.Owner = auth.ID.String() + obj.Owner = auth.Actor.ID } // If a resource ID is specified, fetch that specific resource. @@ -217,7 +219,7 @@ func (api *API) checkAuthorization(rw http.ResponseWriter, r *http.Request) { obj = dbObj.RBACObject() } - err := api.Authorizer.ByRoleName(ctx, auth.ID.String(), auth.Roles, auth.Scope.ToRBAC(), auth.Groups, rbac.Action(v.Action), obj) + err := api.Authorizer.Authorize(ctx, auth.Actor, rbac.Action(v.Action), obj) response[k] = err == nil } diff --git a/coderd/coderdtest/authorize.go b/coderd/coderdtest/authorize.go index 783ef5e964..ab6a5a2db9 100644 --- a/coderd/coderdtest/authorize.go +++ b/coderd/coderdtest/authorize.go @@ -533,12 +533,9 @@ func (a *AuthTester) Test(ctx context.Context, assertRoute map[string]RouteCheck } type authCall struct { - SubjectID string - Roles rbac.ExpandableRoles - Groups []string - Scope rbac.ScopeName - Action rbac.Action - Object rbac.Object + Subject rbac.Subject + Action rbac.Action + Object rbac.Object } type RecordingAuthorizer struct { @@ -548,33 +545,27 @@ type RecordingAuthorizer struct { var _ rbac.Authorizer = (*RecordingAuthorizer)(nil) -// ByRoleNameSQL does not record the call. This matches the postgres behavior +// AuthorizeSQL does not record the call. This matches the postgres behavior // of not calling Authorize() -func (r *RecordingAuthorizer) ByRoleNameSQL(_ context.Context, _ string, _ rbac.ExpandableRoles, _ rbac.ScopeName, _ []string, _ rbac.Action, _ rbac.Object) error { +func (r *RecordingAuthorizer) AuthorizeSQL(_ context.Context, _ rbac.Subject, _ rbac.Action, _ rbac.Object) error { return r.AlwaysReturn } -func (r *RecordingAuthorizer) ByRoleName(_ context.Context, subjectID string, roleNames rbac.ExpandableRoles, scope rbac.ScopeName, groups []string, action rbac.Action, object rbac.Object) error { +func (r *RecordingAuthorizer) Authorize(_ context.Context, subject rbac.Subject, action rbac.Action, object rbac.Object) error { r.Called = &authCall{ - SubjectID: subjectID, - Roles: roleNames, - Groups: groups, - Scope: scope, - Action: action, - Object: object, + Subject: subject, + Action: action, + Object: object, } return r.AlwaysReturn } -func (r *RecordingAuthorizer) PrepareByRoleName(_ context.Context, subjectID string, roles rbac.ExpandableRoles, scope rbac.ScopeName, groups []string, action rbac.Action, _ string) (rbac.PreparedAuthorized, error) { +func (r *RecordingAuthorizer) Prepare(_ context.Context, subject rbac.Subject, action rbac.Action, _ string) (rbac.PreparedAuthorized, error) { return &fakePreparedAuthorizer{ Original: r, - SubjectID: subjectID, - Roles: roles, - Scope: scope, + Subject: subject, Action: action, HardCodedSQLString: "true", - Groups: groups, }, nil } @@ -584,17 +575,14 @@ func (r *RecordingAuthorizer) reset() { type fakePreparedAuthorizer struct { Original *RecordingAuthorizer - SubjectID string - Roles rbac.ExpandableRoles - Scope rbac.ScopeName + Subject rbac.Subject Action rbac.Action - Groups []string HardCodedSQLString string HardCodedRegoString string } func (f *fakePreparedAuthorizer) Authorize(ctx context.Context, object rbac.Object) error { - return f.Original.ByRoleName(ctx, f.SubjectID, f.Roles, f.Scope, f.Groups, f.Action, object) + return f.Original.Authorize(ctx, f.Subject, f.Action, object) } // CompileToSQL returns a compiled version of the authorizer that will work for @@ -604,7 +592,7 @@ func (fakePreparedAuthorizer) CompileToSQL(_ context.Context, _ regosql.ConvertC } func (f *fakePreparedAuthorizer) Eval(object rbac.Object) bool { - return f.Original.ByRoleNameSQL(context.Background(), f.SubjectID, f.Roles, f.Scope, f.Groups, f.Action, object) == nil + return f.Original.AuthorizeSQL(context.Background(), f.Subject, f.Action, object) == nil } func (f fakePreparedAuthorizer) RegoString() string { diff --git a/coderd/httpmw/apikey.go b/coderd/httpmw/apikey.go index 14250c3e59..ceb974c92c 100644 --- a/coderd/httpmw/apikey.go +++ b/coderd/httpmw/apikey.go @@ -52,11 +52,10 @@ func APIKey(r *http.Request) database.APIKey { type userAuthKey struct{} type Authorization struct { - ID uuid.UUID + Actor rbac.Subject + // Username is required for logging and human friendly related + // identification. Username string - Roles rbac.RoleNames - Groups []string - Scope database.APIKeyScope } // UserAuthorizationOptional may return the roles and scope used for @@ -343,11 +342,13 @@ func ExtractAPIKey(cfg ExtractAPIKeyConfig) func(http.Handler) http.Handler { ctx = context.WithValue(ctx, apiKeyContextKey{}, key) ctx = context.WithValue(ctx, userAuthKey{}, Authorization{ - ID: key.UserID, Username: roles.Username, - Roles: roles.Roles, - Scope: key.Scope, - Groups: roles.Groups, + Actor: rbac.Subject{ + ID: key.UserID.String(), + Roles: rbac.RoleNames(roles.Roles), + Groups: roles.Groups, + Scope: rbac.ScopeName(key.Scope), + }, }) next.ServeHTTP(rw, r.WithContext(ctx)) diff --git a/coderd/httpmw/authorize_test.go b/coderd/httpmw/authorize_test.go index 576ef9fd50..46678cb884 100644 --- a/coderd/httpmw/authorize_test.go +++ b/coderd/httpmw/authorize_test.go @@ -126,8 +126,8 @@ func TestExtractUserRoles(t *testing.T) { ) rtr.Get("/", func(_ http.ResponseWriter, r *http.Request) { roles := httpmw.UserAuthorization(r) - require.ElementsMatch(t, user.ID, roles.ID) - require.ElementsMatch(t, expRoles, roles.Roles) + require.Equal(t, user.ID.String(), roles.Actor.ID) + require.ElementsMatch(t, expRoles, roles.Actor.Roles.Names()) }) req := httptest.NewRequest("GET", "/", nil) diff --git a/coderd/httpmw/ratelimit.go b/coderd/httpmw/ratelimit.go index 1b5890196b..ff4e888232 100644 --- a/coderd/httpmw/ratelimit.go +++ b/coderd/httpmw/ratelimit.go @@ -47,7 +47,7 @@ func RateLimit(count int, window time.Duration) func(http.Handler) http.Handler // We avoid using rbac.Authorizer since rego is CPU-intensive // and undermines the DoS-prevention goal of the rate limiter. - for _, role := range auth.Roles { + for _, role := range auth.Actor.SafeRoleNames() { if role == rbac.RoleOwner() { // HACK: use a random key each time to // de facto disable rate limiting. The diff --git a/coderd/members.go b/coderd/members.go index aaf839e987..c67937423d 100644 --- a/coderd/members.go +++ b/coderd/members.go @@ -67,7 +67,7 @@ func (api *API) putMemberRoles(rw http.ResponseWriter, r *http.Request) { // Just treat adding & removing as "assigning" for now. for _, roleName := range append(added, removed...) { - if !rbac.CanAssignRole(actorRoles.Roles, roleName) { + if !rbac.CanAssignRole(actorRoles.Actor.Roles, roleName) { httpapi.Forbidden(rw) return } diff --git a/coderd/rbac/authz.go b/coderd/rbac/authz.go index 888176c212..e11f419a78 100644 --- a/coderd/rbac/authz.go +++ b/coderd/rbac/authz.go @@ -17,24 +17,34 @@ 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 +// Subject is a struct that contains all the elements of a subject in an rbac +// authorize. +type Subject struct { + ID string + Roles ExpandableRoles + Groups []string + Scope ExpandableScope +} + +// SafeScopeName prevent nil pointer dereference. +func (s Subject) SafeScopeName() string { + if s.Scope == nil { + return "no-scope" + } + return s.Scope.Name() +} + +// SafeRoleNames prevent nil pointer dereference. +func (s Subject) SafeRoleNames() []string { + if s.Roles == nil { + return []string{} + } + return s.Roles.Names() } type Authorizer interface { - 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) + Authorize(ctx context.Context, subject Subject, action Action, object Object) error + Prepare(ctx context.Context, subject Subject, action Action, objectType string) (PreparedAuthorized, error) } type PreparedAuthorized interface { @@ -48,7 +58,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 ExpandableRoles, scope ScopeName, groups []string, action Action, objects []O) ([]O, error) { +func Filter[O Objecter](ctx context.Context, auth Authorizer, subject Subject, action Action, objects []O) ([]O, error) { if len(objects) == 0 { // Nothing to filter return objects, nil @@ -60,9 +70,9 @@ 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.Names(), len(groups), scope, action, objectType, + rbacTraceAttributes(subject, action, objectType, // For filtering, we are only measuring the total time for the entire - // set of objects. This and the 'PrepareByRoleName' span time + // set of objects. This and the 'Prepare' span time // is all that is required to measure the performance of this // function on a per-object basis. attribute.Int("num_objects", len(objects)), @@ -71,8 +81,8 @@ func Filter[O Objecter](ctx context.Context, auth Authorizer, subjID string, sub defer span.End() // Running benchmarks on this function, it is **always** faster to call - // auth.ByRoleName on <10 objects. This is because the overhead of - // 'PrepareByRoleName'. Once we cross 10 objects, then it starts to become + // auth.Authorize on <10 objects. This is because the overhead of + // 'Prepare'. Once we cross 10 objects, then it starts to become // faster if len(objects) < 10 { for _, o := range objects { @@ -80,7 +90,7 @@ func Filter[O Objecter](ctx context.Context, auth Authorizer, subjID string, sub if rbacObj.Type != objectType { return nil, xerrors.Errorf("object types must be uniform across the set (%s), found %s", objectType, rbacObj) } - err := auth.ByRoleName(ctx, subjID, subjRoles, scope, groups, action, o.RBACObject()) + err := auth.Authorize(ctx, subject, action, o.RBACObject()) if err == nil { filtered = append(filtered, o) } @@ -88,7 +98,7 @@ func Filter[O Objecter](ctx context.Context, auth Authorizer, subjID string, sub return filtered, nil } - prepared, err := auth.PrepareByRoleName(ctx, subjID, subjRoles, scope, groups, action, objectType) + prepared, err := auth.Prepare(ctx, subject, action, objectType) if err != nil { return nil, xerrors.Errorf("prepare: %w", err) } @@ -191,14 +201,15 @@ type authSubject struct { Scope Scope `json:"scope"` } -// 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 ExpandableRoles, scope ScopeName, groups []string, action Action, object Object) error { +// Authorize is the intended function to be used outside this package. +// It returns `nil` if the subject is authorized to perform the action on +// the object. +// If an error is returned, the authorization is denied. +func (a RegoAuthorizer) Authorize(ctx context.Context, subject Subject, 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.Names(), len(groups), scope, action, object.Type, + rbacTraceAttributes(subject, 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)), @@ -207,18 +218,9 @@ func (a RegoAuthorizer) ByRoleName(ctx context.Context, subjectID string, roleNa ) defer span.End() - roles, err := roleNames.Expand() - if err != nil { - return err - } + err := a.authorize(ctx, subject, action, object) + span.SetAttributes(attribute.Bool("authorized", err == nil)) - scopeRole, err := ExpandScope(scope) - if err != nil { - return err - } - - err = a.Authorize(ctx, subjectID, roles, scopeRole, groups, action, object) - span.AddEvent("authorized", trace.WithAttributes(attribute.Bool("authorized", err == nil))) dur := time.Since(start) if err != nil { a.authorizeHist.WithLabelValues("false").Observe(dur.Seconds()) @@ -229,15 +231,34 @@ func (a RegoAuthorizer) ByRoleName(ctx context.Context, subjectID string, roleNa return nil } -// Authorize allows passing in custom Roles. -// This is really helpful for unit testing, as we can create custom roles to exercise edge cases. -func (a RegoAuthorizer) Authorize(ctx context.Context, subjectID string, roles []Role, scope Scope, groups []string, action Action, object Object) error { +// authorize is the internal function that does the actual authorization. +// It is a different function so the exported one can add tracing + metrics. +// That code tends to clutter up the actual logic, so it's separated out. +// nolint:revive +func (a RegoAuthorizer) authorize(ctx context.Context, subject Subject, action Action, object Object) error { + if subject.Roles == nil { + return xerrors.Errorf("subject must have roles") + } + if subject.Scope == nil { + return xerrors.Errorf("subject must have a scope") + } + + subjRoles, err := subject.Roles.Expand() + if err != nil { + return xerrors.Errorf("expand roles: %w", err) + } + + subjScope, err := subject.Scope.Expand() + if err != nil { + return xerrors.Errorf("expand scope: %w", err) + } + input := map[string]interface{}{ "subject": authSubject{ - ID: subjectID, - Roles: roles, - Groups: groups, - Scope: scope, + ID: subject.ID, + Roles: subjRoles, + Groups: subject.Groups, + Scope: subjScope, }, "object": object, "action": action, @@ -254,27 +275,19 @@ func (a RegoAuthorizer) Authorize(ctx context.Context, subjectID string, roles [ return nil } -func (a RegoAuthorizer) PrepareByRoleName(ctx context.Context, subjectID string, roleNames ExpandableRoles, scope ScopeName, groups []string, action Action, objectType string) (PreparedAuthorized, error) { +// Prepare will partially execute the rego policy leaving the object fields unknown (except for the type). +// This will vastly speed up performance if batch authorization on the same type of objects is needed. +func (a RegoAuthorizer) Prepare(ctx context.Context, subject Subject, action Action, objectType string) (PreparedAuthorized, error) { start := time.Now() ctx, span := tracing.StartSpan(ctx, trace.WithTimestamp(start), - rbacTraceAttributes(roleNames.Names(), len(groups), scope, action, objectType), + rbacTraceAttributes(subject, action, objectType), ) defer span.End() - roles, err := roleNames.Expand() + prepared, err := newPartialAuthorizer(ctx, subject, action, objectType) if err != nil { - return nil, err - } - - scopeRole, err := ExpandScope(scope) - if err != nil { - return nil, err - } - - prepared, err := a.Prepare(ctx, subjectID, roles, scopeRole, groups, action, objectType) - if err != nil { - return nil, err + return nil, xerrors.Errorf("new partial authorizer: %w", err) } // Add attributes of the Prepare results. This will help understand the @@ -287,14 +300,3 @@ func (a RegoAuthorizer) PrepareByRoleName(ctx context.Context, subjectID string, a.prepareHist.Observe(time.Since(start).Seconds()) return prepared, nil } - -// Prepare will partially execute the rego policy leaving the object fields unknown (except for the type). -// This will vastly speed up performance if batch authorization on the same type of objects is needed. -func (RegoAuthorizer) Prepare(ctx context.Context, subjectID string, roles []Role, scope Scope, groups []string, action Action, objectType string) (*PartialAuthorizer, error) { - auth, err := newPartialAuthorizer(ctx, subjectID, roles, scope, groups, action, objectType) - if err != nil { - return nil, xerrors.Errorf("new partial authorizer: %w", err) - } - - return auth, nil -} diff --git a/coderd/rbac/authz_internal_test.go b/coderd/rbac/authz_internal_test.go index 0afff5226e..29195ad179 100644 --- a/coderd/rbac/authz_internal_test.go +++ b/coderd/rbac/authz_internal_test.go @@ -15,16 +15,6 @@ import ( "github.com/coder/coder/testutil" ) -type subject struct { - UserID string `json:"id"` - // For the unit test we want to pass in the roles directly, instead of just - // by name. This allows us to test custom roles that do not exist in the product, - // but test edge cases of the implementation. - Roles []Role `json:"roles"` - Groups []string `json:"groups"` - Scope Scope `json:"scope"` -} - type fakeObject struct { Owner uuid.UUID OrgOwner uuid.UUID @@ -43,14 +33,20 @@ func (w fakeObject) RBACObject() Object { func TestFilterError(t *testing.T) { t.Parallel() auth := NewAuthorizer(prometheus.NewRegistry()) + subject := Subject{ + ID: uuid.NewString(), + Roles: RoleNames{}, + Groups: []string{}, + Scope: ScopeAll, + } - _, err := Filter(context.Background(), auth, uuid.NewString(), RoleNames{}, ScopeAll, []string{}, ActionRead, []Object{ResourceUser, ResourceWorkspace}) + _, err := Filter(context.Background(), auth, subject, ActionRead, []Object{ResourceUser, ResourceWorkspace}) require.ErrorContains(t, err, "object types must be uniform") } // TestFilter ensures the filter acts the same as an individual authorize. // It generates a random set of objects, then runs the Filter batch function -// against the singular ByRoleName function. +// against the singular Authorize function. func TestFilter(t *testing.T) { t.Parallel() @@ -74,78 +70,92 @@ func TestFilter(t *testing.T) { testCases := []struct { Name string - SubjectID string - Roles RoleNames + Actor Subject Action Action - Scope ScopeName ObjectType string }{ { - Name: "NoRoles", - SubjectID: userIDs[0].String(), - Roles: []string{}, - ObjectType: ResourceWorkspace.Type, - Action: ActionRead, - }, - { - Name: "Admin", - SubjectID: userIDs[0].String(), - Roles: []string{RoleOrgMember(orgIDs[0]), "auditor", RoleOwner(), RoleMember()}, - ObjectType: ResourceWorkspace.Type, - Action: ActionRead, - }, - { - Name: "OrgAdmin", - SubjectID: userIDs[0].String(), - Roles: []string{RoleOrgMember(orgIDs[0]), RoleOrgAdmin(orgIDs[0]), RoleMember()}, - ObjectType: ResourceWorkspace.Type, - Action: ActionRead, - }, - { - Name: "OrgMember", - SubjectID: userIDs[0].String(), - Roles: []string{RoleOrgMember(orgIDs[0]), RoleOrgMember(orgIDs[1]), RoleMember()}, - ObjectType: ResourceWorkspace.Type, - Action: ActionRead, - }, - { - Name: "ManyRoles", - SubjectID: userIDs[0].String(), - Roles: []string{ - RoleOrgMember(orgIDs[0]), RoleOrgAdmin(orgIDs[0]), - RoleOrgMember(orgIDs[1]), RoleOrgAdmin(orgIDs[1]), - RoleOrgMember(orgIDs[2]), RoleOrgAdmin(orgIDs[2]), - RoleOrgMember(orgIDs[4]), - RoleOrgMember(orgIDs[5]), - RoleMember(), + Name: "NoRoles", + Actor: Subject{ + ID: userIDs[0].String(), + Roles: RoleNames{}, }, ObjectType: ResourceWorkspace.Type, Action: ActionRead, }, { - Name: "SiteMember", - SubjectID: userIDs[0].String(), - Roles: []string{RoleMember()}, + Name: "Admin", + Actor: Subject{ + ID: userIDs[0].String(), + Roles: RoleNames{RoleOrgMember(orgIDs[0]), "auditor", RoleOwner(), RoleMember()}, + }, + ObjectType: ResourceWorkspace.Type, + Action: ActionRead, + }, + { + Name: "OrgAdmin", + Actor: Subject{ + ID: userIDs[0].String(), + Roles: RoleNames{RoleOrgMember(orgIDs[0]), RoleOrgAdmin(orgIDs[0]), RoleMember()}, + }, + ObjectType: ResourceWorkspace.Type, + Action: ActionRead, + }, + { + Name: "OrgMember", + Actor: Subject{ + ID: userIDs[0].String(), + Roles: RoleNames{RoleOrgMember(orgIDs[0]), RoleOrgMember(orgIDs[1]), RoleMember()}, + }, + ObjectType: ResourceWorkspace.Type, + Action: ActionRead, + }, + { + Name: "ManyRoles", + Actor: Subject{ + ID: userIDs[0].String(), + Roles: RoleNames{ + RoleOrgMember(orgIDs[0]), RoleOrgAdmin(orgIDs[0]), + RoleOrgMember(orgIDs[1]), RoleOrgAdmin(orgIDs[1]), + RoleOrgMember(orgIDs[2]), RoleOrgAdmin(orgIDs[2]), + RoleOrgMember(orgIDs[4]), + RoleOrgMember(orgIDs[5]), + RoleMember(), + }, + }, + ObjectType: ResourceWorkspace.Type, + Action: ActionRead, + }, + { + Name: "SiteMember", + Actor: Subject{ + ID: userIDs[0].String(), + Roles: RoleNames{RoleMember()}, + }, ObjectType: ResourceUser.Type, Action: ActionRead, }, { - Name: "ReadOrgs", - SubjectID: userIDs[0].String(), - Roles: []string{ - RoleOrgMember(orgIDs[0]), - RoleOrgMember(orgIDs[1]), - RoleOrgMember(orgIDs[2]), - RoleOrgMember(orgIDs[3]), - RoleMember(), + Name: "ReadOrgs", + Actor: Subject{ + ID: userIDs[0].String(), + Roles: RoleNames{ + RoleOrgMember(orgIDs[0]), + RoleOrgMember(orgIDs[1]), + RoleOrgMember(orgIDs[2]), + RoleOrgMember(orgIDs[3]), + RoleMember(), + }, }, ObjectType: ResourceOrganization.Type, Action: ActionRead, }, { - Name: "ScopeApplicationConnect", - SubjectID: userIDs[0].String(), - Roles: []string{RoleOrgMember(orgIDs[0]), "auditor", RoleOwner(), RoleMember()}, + Name: "ScopeApplicationConnect", + Actor: Subject{ + ID: userIDs[0].String(), + Roles: RoleNames{RoleOrgMember(orgIDs[0]), "auditor", RoleOwner(), RoleMember()}, + }, ObjectType: ResourceWorkspace.Type, Action: ActionRead, }, @@ -155,6 +165,7 @@ func TestFilter(t *testing.T) { tc := tc t.Run(tc.Name, func(t *testing.T) { t.Parallel() + actor := tc.Actor localObjects := make([]fakeObject, len(objects)) copy(localObjects, objects) @@ -163,16 +174,16 @@ func TestFilter(t *testing.T) { defer cancel() auth := NewAuthorizer(prometheus.NewRegistry()) - scope := ScopeAll - if tc.Scope != "" { - scope = tc.Scope + if actor.Scope == nil { + // Default to ScopeAll + actor.Scope = ScopeAll } // Run auth 1 by 1 var allowedCount int for i, obj := range localObjects { obj.Type = tc.ObjectType - err := auth.ByRoleName(ctx, tc.SubjectID, tc.Roles, scope, []string{}, ActionRead, obj.RBACObject()) + err := auth.Authorize(ctx, actor, ActionRead, obj.RBACObject()) obj.Allowed = err == nil if err == nil { allowedCount++ @@ -181,7 +192,7 @@ func TestFilter(t *testing.T) { } // Run by filter - list, err := Filter(ctx, auth, tc.SubjectID, tc.Roles, scope, []string{}, tc.Action, localObjects) + list, err := Filter(ctx, auth, actor, tc.Action, localObjects) require.NoError(t, err) require.Equal(t, allowedCount, len(list), "expected number of allowed") for _, obj := range list { @@ -198,11 +209,11 @@ func TestAuthorizeDomain(t *testing.T) { unuseID := uuid.New() allUsersGroup := "Everyone" - user := subject{ - UserID: "me", + user := Subject{ + ID: "me", Scope: must(ExpandScope(ScopeAll)), Groups: []string{allUsersGroup}, - Roles: []Role{ + Roles: Roles{ must(RoleByName(RoleMember())), must(RoleByName(RoleOrgMember(defOrg))), }, @@ -211,21 +222,21 @@ func TestAuthorizeDomain(t *testing.T) { testAuthorize(t, "UserACLList", user, []authTestCase{ { resource: ResourceWorkspace.WithOwner(unuseID.String()).InOrg(unuseID).WithACLUserList(map[string][]Action{ - user.UserID: allActions(), + user.ID: allActions(), }), actions: allActions(), allow: true, }, { resource: ResourceWorkspace.WithOwner(unuseID.String()).InOrg(unuseID).WithACLUserList(map[string][]Action{ - user.UserID: {WildcardSymbol}, + user.ID: {WildcardSymbol}, }), actions: allActions(), allow: true, }, { resource: ResourceWorkspace.WithOwner(unuseID.String()).InOrg(unuseID).WithACLUserList(map[string][]Action{ - user.UserID: {ActionRead, ActionUpdate}, + user.ID: {ActionRead, ActionUpdate}, }), actions: []Action{ActionCreate, ActionDelete}, allow: false, @@ -233,7 +244,7 @@ func TestAuthorizeDomain(t *testing.T) { { // By default users cannot update templates resource: ResourceTemplate.InOrg(defOrg).WithACLUserList(map[string][]Action{ - user.UserID: {ActionUpdate}, + user.ID: {ActionUpdate}, }), actions: []Action{ActionUpdate}, allow: true, @@ -274,15 +285,15 @@ func TestAuthorizeDomain(t *testing.T) { testAuthorize(t, "Member", user, []authTestCase{ // Org + me - {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.UserID), actions: allActions(), allow: true}, + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID), actions: allActions(), allow: true}, {resource: ResourceWorkspace.InOrg(defOrg), actions: allActions(), allow: false}, - {resource: ResourceWorkspace.WithOwner(user.UserID), actions: allActions(), allow: true}, + {resource: ResourceWorkspace.WithOwner(user.ID), actions: allActions(), allow: true}, {resource: ResourceWorkspace.All(), actions: allActions(), allow: false}, // Other org + me - {resource: ResourceWorkspace.InOrg(unuseID).WithOwner(user.UserID), actions: allActions(), allow: false}, + {resource: ResourceWorkspace.InOrg(unuseID).WithOwner(user.ID), actions: allActions(), allow: false}, {resource: ResourceWorkspace.InOrg(unuseID), actions: allActions(), allow: false}, // Other org + other user @@ -297,10 +308,10 @@ func TestAuthorizeDomain(t *testing.T) { {resource: ResourceWorkspace.WithOwner("not-me"), actions: allActions(), allow: false}, }) - user = subject{ - UserID: "me", - Scope: must(ExpandScope(ScopeAll)), - Roles: []Role{{ + user = Subject{ + ID: "me", + Scope: must(ExpandScope(ScopeAll)), + Roles: Roles{{ Name: "deny-all", // List out deny permissions explicitly Site: []Permission{ @@ -315,15 +326,15 @@ func TestAuthorizeDomain(t *testing.T) { testAuthorize(t, "DeletedMember", user, []authTestCase{ // Org + me - {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.UserID), actions: allActions(), allow: false}, + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID), actions: allActions(), allow: false}, {resource: ResourceWorkspace.InOrg(defOrg), actions: allActions(), allow: false}, - {resource: ResourceWorkspace.WithOwner(user.UserID), actions: allActions(), allow: false}, + {resource: ResourceWorkspace.WithOwner(user.ID), actions: allActions(), allow: false}, {resource: ResourceWorkspace.All(), actions: allActions(), allow: false}, // Other org + me - {resource: ResourceWorkspace.InOrg(unuseID).WithOwner(user.UserID), actions: allActions(), allow: false}, + {resource: ResourceWorkspace.InOrg(unuseID).WithOwner(user.ID), actions: allActions(), allow: false}, {resource: ResourceWorkspace.InOrg(unuseID), actions: allActions(), allow: false}, // Other org + other user @@ -338,10 +349,10 @@ func TestAuthorizeDomain(t *testing.T) { {resource: ResourceWorkspace.WithOwner("not-me"), actions: allActions(), allow: false}, }) - user = subject{ - UserID: "me", - Scope: must(ExpandScope(ScopeAll)), - Roles: []Role{ + user = Subject{ + ID: "me", + Scope: must(ExpandScope(ScopeAll)), + Roles: Roles{ must(RoleByName(RoleOrgAdmin(defOrg))), must(RoleByName(RoleMember())), }, @@ -349,15 +360,15 @@ func TestAuthorizeDomain(t *testing.T) { testAuthorize(t, "OrgAdmin", user, []authTestCase{ // Org + me - {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.UserID), actions: allActions(), allow: true}, + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID), actions: allActions(), allow: true}, {resource: ResourceWorkspace.InOrg(defOrg), actions: allActions(), allow: true}, - {resource: ResourceWorkspace.WithOwner(user.UserID), actions: allActions(), allow: true}, + {resource: ResourceWorkspace.WithOwner(user.ID), actions: allActions(), allow: true}, {resource: ResourceWorkspace.All(), actions: allActions(), allow: false}, // Other org + me - {resource: ResourceWorkspace.InOrg(unuseID).WithOwner(user.UserID), actions: allActions(), allow: false}, + {resource: ResourceWorkspace.InOrg(unuseID).WithOwner(user.ID), actions: allActions(), allow: false}, {resource: ResourceWorkspace.InOrg(unuseID), actions: allActions(), allow: false}, // Other org + other user @@ -372,10 +383,10 @@ func TestAuthorizeDomain(t *testing.T) { {resource: ResourceWorkspace.WithOwner("not-me"), actions: allActions(), allow: false}, }) - user = subject{ - UserID: "me", - Scope: must(ExpandScope(ScopeAll)), - Roles: []Role{ + user = Subject{ + ID: "me", + Scope: must(ExpandScope(ScopeAll)), + Roles: Roles{ must(RoleByName(RoleOwner())), must(RoleByName(RoleMember())), }, @@ -383,15 +394,15 @@ func TestAuthorizeDomain(t *testing.T) { testAuthorize(t, "SiteAdmin", user, []authTestCase{ // Org + me - {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.UserID), actions: allActions(), allow: true}, + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID), actions: allActions(), allow: true}, {resource: ResourceWorkspace.InOrg(defOrg), actions: allActions(), allow: true}, - {resource: ResourceWorkspace.WithOwner(user.UserID), actions: allActions(), allow: true}, + {resource: ResourceWorkspace.WithOwner(user.ID), actions: allActions(), allow: true}, {resource: ResourceWorkspace.All(), actions: allActions(), allow: true}, // Other org + me - {resource: ResourceWorkspace.InOrg(unuseID).WithOwner(user.UserID), actions: allActions(), allow: true}, + {resource: ResourceWorkspace.InOrg(unuseID).WithOwner(user.ID), actions: allActions(), allow: true}, {resource: ResourceWorkspace.InOrg(unuseID), actions: allActions(), allow: true}, // Other org + other user @@ -406,10 +417,10 @@ func TestAuthorizeDomain(t *testing.T) { {resource: ResourceWorkspace.WithOwner("not-me"), actions: allActions(), allow: true}, }) - user = subject{ - UserID: "me", - Scope: must(ExpandScope(ScopeApplicationConnect)), - Roles: []Role{ + user = Subject{ + ID: "me", + Scope: must(ExpandScope(ScopeApplicationConnect)), + Roles: Roles{ must(RoleByName(RoleOrgMember(defOrg))), must(RoleByName(RoleMember())), }, @@ -422,15 +433,15 @@ func TestAuthorizeDomain(t *testing.T) { return c }, []authTestCase{ // Org + me - {resource: ResourceWorkspaceApplicationConnect.InOrg(defOrg).WithOwner(user.UserID), allow: true}, + {resource: ResourceWorkspaceApplicationConnect.InOrg(defOrg).WithOwner(user.ID), allow: true}, {resource: ResourceWorkspaceApplicationConnect.InOrg(defOrg), allow: false}, - {resource: ResourceWorkspaceApplicationConnect.WithOwner(user.UserID), allow: true}, + {resource: ResourceWorkspaceApplicationConnect.WithOwner(user.ID), allow: true}, {resource: ResourceWorkspaceApplicationConnect.All(), allow: false}, // Other org + me - {resource: ResourceWorkspaceApplicationConnect.InOrg(unuseID).WithOwner(user.UserID), allow: false}, + {resource: ResourceWorkspaceApplicationConnect.InOrg(unuseID).WithOwner(user.ID), allow: false}, {resource: ResourceWorkspaceApplicationConnect.InOrg(unuseID), allow: false}, // Other org + other user @@ -451,15 +462,15 @@ func TestAuthorizeDomain(t *testing.T) { return c }, []authTestCase{ // Org + me - {resource: ResourceWorkspaceApplicationConnect.InOrg(defOrg).WithOwner(user.UserID)}, + {resource: ResourceWorkspaceApplicationConnect.InOrg(defOrg).WithOwner(user.ID)}, {resource: ResourceWorkspaceApplicationConnect.InOrg(defOrg)}, - {resource: ResourceWorkspaceApplicationConnect.WithOwner(user.UserID)}, + {resource: ResourceWorkspaceApplicationConnect.WithOwner(user.ID)}, {resource: ResourceWorkspaceApplicationConnect.All()}, // Other org + me - {resource: ResourceWorkspaceApplicationConnect.InOrg(unuseID).WithOwner(user.UserID)}, + {resource: ResourceWorkspaceApplicationConnect.InOrg(unuseID).WithOwner(user.ID)}, {resource: ResourceWorkspaceApplicationConnect.InOrg(unuseID)}, // Other org + other user @@ -480,15 +491,15 @@ func TestAuthorizeDomain(t *testing.T) { return c }, []authTestCase{ // Org + me - {resource: ResourceTemplate.InOrg(defOrg).WithOwner(user.UserID)}, + {resource: ResourceTemplate.InOrg(defOrg).WithOwner(user.ID)}, {resource: ResourceTemplate.InOrg(defOrg)}, - {resource: ResourceTemplate.WithOwner(user.UserID)}, + {resource: ResourceTemplate.WithOwner(user.ID)}, {resource: ResourceTemplate.All()}, // Other org + me - {resource: ResourceTemplate.InOrg(unuseID).WithOwner(user.UserID)}, + {resource: ResourceTemplate.InOrg(unuseID).WithOwner(user.ID)}, {resource: ResourceTemplate.InOrg(unuseID)}, // Other org + other user @@ -505,10 +516,10 @@ func TestAuthorizeDomain(t *testing.T) { ) // In practice this is a token scope on a regular subject - user = subject{ - UserID: "me", - Scope: must(ExpandScope(ScopeAll)), - Roles: []Role{ + user = Subject{ + ID: "me", + Scope: must(ExpandScope(ScopeAll)), + Roles: Roles{ { Name: "ReadOnlyOrgAndUser", Site: []Permission{}, @@ -537,15 +548,15 @@ func TestAuthorizeDomain(t *testing.T) { }, []authTestCase{ // Read // Org + me - {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.UserID), allow: true}, + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID), allow: true}, {resource: ResourceWorkspace.InOrg(defOrg), allow: true}, - {resource: ResourceWorkspace.WithOwner(user.UserID), allow: true}, + {resource: ResourceWorkspace.WithOwner(user.ID), allow: true}, {resource: ResourceWorkspace.All(), allow: false}, // Other org + me - {resource: ResourceWorkspace.InOrg(unuseID).WithOwner(user.UserID), allow: false}, + {resource: ResourceWorkspace.InOrg(unuseID).WithOwner(user.ID), allow: false}, {resource: ResourceWorkspace.InOrg(unuseID), allow: false}, // Other org + other user @@ -568,15 +579,15 @@ func TestAuthorizeDomain(t *testing.T) { }, []authTestCase{ // Read // Org + me - {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.UserID)}, + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID)}, {resource: ResourceWorkspace.InOrg(defOrg)}, - {resource: ResourceWorkspace.WithOwner(user.UserID)}, + {resource: ResourceWorkspace.WithOwner(user.ID)}, {resource: ResourceWorkspace.All()}, // Other org + me - {resource: ResourceWorkspace.InOrg(unuseID).WithOwner(user.UserID)}, + {resource: ResourceWorkspace.InOrg(unuseID).WithOwner(user.ID)}, {resource: ResourceWorkspace.InOrg(unuseID)}, // Other org + other user @@ -598,10 +609,10 @@ func TestAuthorizeLevels(t *testing.T) { defOrg := uuid.New() unusedID := uuid.New() - user := subject{ - UserID: "me", - Scope: must(ExpandScope(ScopeAll)), - Roles: []Role{ + user := Subject{ + ID: "me", + Scope: must(ExpandScope(ScopeAll)), + Roles: Roles{ must(RoleByName(RoleOwner())), { Name: "org-deny:" + defOrg.String(), @@ -636,15 +647,15 @@ func TestAuthorizeLevels(t *testing.T) { return c }, []authTestCase{ // Org + me - {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.UserID)}, + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID)}, {resource: ResourceWorkspace.InOrg(defOrg)}, - {resource: ResourceWorkspace.WithOwner(user.UserID)}, + {resource: ResourceWorkspace.WithOwner(user.ID)}, {resource: ResourceWorkspace.All()}, // Other org + me - {resource: ResourceWorkspace.InOrg(unusedID).WithOwner(user.UserID)}, + {resource: ResourceWorkspace.InOrg(unusedID).WithOwner(user.ID)}, {resource: ResourceWorkspace.InOrg(unusedID)}, // Other org + other user @@ -659,10 +670,10 @@ func TestAuthorizeLevels(t *testing.T) { {resource: ResourceWorkspace.WithOwner("not-me")}, })) - user = subject{ - UserID: "me", - Scope: must(ExpandScope(ScopeAll)), - Roles: []Role{ + user = Subject{ + ID: "me", + Scope: must(ExpandScope(ScopeAll)), + Roles: Roles{ { Name: "site-noise", Site: []Permission{ @@ -694,15 +705,15 @@ func TestAuthorizeLevels(t *testing.T) { return c }, []authTestCase{ // Org + me - {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.UserID), allow: true}, + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID), allow: true}, {resource: ResourceWorkspace.InOrg(defOrg), allow: true}, - {resource: ResourceWorkspace.WithOwner(user.UserID), allow: false}, + {resource: ResourceWorkspace.WithOwner(user.ID), allow: false}, {resource: ResourceWorkspace.All(), allow: false}, // Other org + me - {resource: ResourceWorkspace.InOrg(unusedID).WithOwner(user.UserID), allow: false}, + {resource: ResourceWorkspace.InOrg(unusedID).WithOwner(user.ID), allow: false}, {resource: ResourceWorkspace.InOrg(unusedID), allow: false}, // Other org + other user @@ -723,10 +734,10 @@ func TestAuthorizeScope(t *testing.T) { defOrg := uuid.New() unusedID := uuid.New() - user := subject{ - UserID: "me", - Roles: []Role{must(RoleByName(RoleOwner()))}, - Scope: must(ExpandScope(ScopeApplicationConnect)), + user := Subject{ + ID: "me", + Roles: Roles{must(RoleByName(RoleOwner()))}, + Scope: must(ExpandScope(ScopeApplicationConnect)), } testAuthorize(t, "Admin_ScopeApplicationConnect", user, @@ -734,11 +745,11 @@ func TestAuthorizeScope(t *testing.T) { c.actions = []Action{ActionRead, ActionUpdate, ActionDelete} return c }, []authTestCase{ - {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.UserID), allow: false}, + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID), allow: false}, {resource: ResourceWorkspace.InOrg(defOrg), allow: false}, - {resource: ResourceWorkspace.WithOwner(user.UserID), allow: false}, + {resource: ResourceWorkspace.WithOwner(user.ID), allow: false}, {resource: ResourceWorkspace.All(), allow: false}, - {resource: ResourceWorkspace.InOrg(unusedID).WithOwner(user.UserID), allow: false}, + {resource: ResourceWorkspace.InOrg(unusedID).WithOwner(user.ID), allow: false}, {resource: ResourceWorkspace.InOrg(unusedID), allow: false}, {resource: ResourceWorkspace.InOrg(defOrg).WithOwner("not-me"), allow: false}, {resource: ResourceWorkspace.WithOwner("not-me"), allow: false}, @@ -749,14 +760,14 @@ func TestAuthorizeScope(t *testing.T) { // Allowed by scope: []authTestCase{ {resource: ResourceWorkspaceApplicationConnect.InOrg(defOrg).WithOwner("not-me"), actions: []Action{ActionCreate}, allow: true}, - {resource: ResourceWorkspaceApplicationConnect.InOrg(defOrg).WithOwner(user.UserID), actions: []Action{ActionCreate}, allow: true}, + {resource: ResourceWorkspaceApplicationConnect.InOrg(defOrg).WithOwner(user.ID), actions: []Action{ActionCreate}, allow: true}, {resource: ResourceWorkspaceApplicationConnect.InOrg(unusedID).WithOwner("not-me"), actions: []Action{ActionCreate}, allow: true}, }, ) - user = subject{ - UserID: "me", - Roles: []Role{ + user = Subject{ + ID: "me", + Roles: Roles{ must(RoleByName(RoleMember())), must(RoleByName(RoleOrgMember(defOrg))), }, @@ -769,11 +780,11 @@ func TestAuthorizeScope(t *testing.T) { c.allow = false return c }, []authTestCase{ - {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.UserID)}, + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID)}, {resource: ResourceWorkspace.InOrg(defOrg)}, - {resource: ResourceWorkspace.WithOwner(user.UserID)}, + {resource: ResourceWorkspace.WithOwner(user.ID)}, {resource: ResourceWorkspace.All()}, - {resource: ResourceWorkspace.InOrg(unusedID).WithOwner(user.UserID)}, + {resource: ResourceWorkspace.InOrg(unusedID).WithOwner(user.ID)}, {resource: ResourceWorkspace.InOrg(unusedID)}, {resource: ResourceWorkspace.InOrg(defOrg).WithOwner("not-me")}, {resource: ResourceWorkspace.WithOwner("not-me")}, @@ -783,16 +794,16 @@ func TestAuthorizeScope(t *testing.T) { }), // Allowed by scope: []authTestCase{ - {resource: ResourceWorkspaceApplicationConnect.InOrg(defOrg).WithOwner(user.UserID), actions: []Action{ActionCreate}, allow: true}, + {resource: ResourceWorkspaceApplicationConnect.InOrg(defOrg).WithOwner(user.ID), actions: []Action{ActionCreate}, allow: true}, {resource: ResourceWorkspaceApplicationConnect.InOrg(defOrg).WithOwner("not-me"), actions: []Action{ActionCreate}, allow: false}, {resource: ResourceWorkspaceApplicationConnect.InOrg(unusedID).WithOwner("not-me"), actions: []Action{ActionCreate}, allow: false}, }, ) workspaceID := uuid.New() - user = subject{ - UserID: "me", - Roles: []Role{ + user = Subject{ + ID: "me", + Roles: Roles{ must(RoleByName(RoleMember())), must(RoleByName(RoleOrgMember(defOrg))), }, @@ -818,11 +829,11 @@ func TestAuthorizeScope(t *testing.T) { c.allow = false return c }, []authTestCase{ - {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.UserID)}, + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID)}, {resource: ResourceWorkspace.InOrg(defOrg)}, - {resource: ResourceWorkspace.WithOwner(user.UserID)}, + {resource: ResourceWorkspace.WithOwner(user.ID)}, {resource: ResourceWorkspace.All()}, - {resource: ResourceWorkspace.InOrg(unusedID).WithOwner(user.UserID)}, + {resource: ResourceWorkspace.InOrg(unusedID).WithOwner(user.ID)}, {resource: ResourceWorkspace.InOrg(unusedID)}, {resource: ResourceWorkspace.InOrg(defOrg).WithOwner("not-me")}, {resource: ResourceWorkspace.WithOwner("not-me")}, @@ -838,11 +849,11 @@ func TestAuthorizeScope(t *testing.T) { c.resource.WithID(workspaceID) return c }, []authTestCase{ - {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.UserID)}, + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID)}, {resource: ResourceWorkspace.InOrg(defOrg)}, - {resource: ResourceWorkspace.WithOwner(user.UserID)}, + {resource: ResourceWorkspace.WithOwner(user.ID)}, {resource: ResourceWorkspace.All()}, - {resource: ResourceWorkspace.InOrg(unusedID).WithOwner(user.UserID)}, + {resource: ResourceWorkspace.InOrg(unusedID).WithOwner(user.ID)}, {resource: ResourceWorkspace.InOrg(unusedID)}, {resource: ResourceWorkspace.InOrg(defOrg).WithOwner("not-me")}, {resource: ResourceWorkspace.WithOwner("not-me")}, @@ -857,11 +868,11 @@ func TestAuthorizeScope(t *testing.T) { c.resource.WithID(uuid.New()) return c }, []authTestCase{ - {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.UserID)}, + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID)}, {resource: ResourceWorkspace.InOrg(defOrg)}, - {resource: ResourceWorkspace.WithOwner(user.UserID)}, + {resource: ResourceWorkspace.WithOwner(user.ID)}, {resource: ResourceWorkspace.All()}, - {resource: ResourceWorkspace.InOrg(unusedID).WithOwner(user.UserID)}, + {resource: ResourceWorkspace.InOrg(unusedID).WithOwner(user.ID)}, {resource: ResourceWorkspace.InOrg(unusedID)}, {resource: ResourceWorkspace.InOrg(defOrg).WithOwner("not-me")}, {resource: ResourceWorkspace.WithOwner("not-me")}, @@ -871,7 +882,7 @@ func TestAuthorizeScope(t *testing.T) { }), // Allowed by scope: []authTestCase{ - {resource: ResourceWorkspace.WithID(workspaceID).InOrg(defOrg).WithOwner(user.UserID), actions: []Action{ActionRead}, allow: true}, + {resource: ResourceWorkspace.WithID(workspaceID).InOrg(defOrg).WithOwner(user.ID), actions: []Action{ActionRead}, allow: true}, // The scope will return true, but the user perms return false for resources not owned by the user. {resource: ResourceWorkspace.WithID(workspaceID).InOrg(defOrg).WithOwner("not-me"), actions: []Action{ActionRead}, allow: false}, {resource: ResourceWorkspace.WithID(workspaceID).InOrg(unusedID).WithOwner("not-me"), actions: []Action{ActionRead}, allow: false}, @@ -879,9 +890,9 @@ func TestAuthorizeScope(t *testing.T) { ) // This scope can only create workspaces - user = subject{ - UserID: "me", - Roles: []Role{ + user = Subject{ + ID: "me", + Roles: Roles{ must(RoleByName(RoleMember())), must(RoleByName(RoleOrgMember(defOrg))), }, @@ -909,11 +920,11 @@ func TestAuthorizeScope(t *testing.T) { c.resource.ID = uuid.NewString() return c }, []authTestCase{ - {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.UserID)}, + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID)}, {resource: ResourceWorkspace.InOrg(defOrg)}, - {resource: ResourceWorkspace.WithOwner(user.UserID)}, + {resource: ResourceWorkspace.WithOwner(user.ID)}, {resource: ResourceWorkspace.All()}, - {resource: ResourceWorkspace.InOrg(unusedID).WithOwner(user.UserID)}, + {resource: ResourceWorkspace.InOrg(unusedID).WithOwner(user.ID)}, {resource: ResourceWorkspace.InOrg(unusedID)}, {resource: ResourceWorkspace.InOrg(defOrg).WithOwner("not-me")}, {resource: ResourceWorkspace.WithOwner("not-me")}, @@ -924,7 +935,7 @@ func TestAuthorizeScope(t *testing.T) { // Test create allowed by scope: []authTestCase{ - {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.UserID), actions: []Action{ActionCreate}, allow: true}, + {resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID), actions: []Action{ActionCreate}, allow: true}, // The scope will return true, but the user perms return false for resources not owned by the user. {resource: ResourceWorkspace.InOrg(defOrg).WithOwner("not-me"), actions: []Action{ActionCreate}, allow: false}, {resource: ResourceWorkspace.InOrg(unusedID).WithOwner("not-me"), actions: []Action{ActionCreate}, allow: false}, @@ -949,7 +960,7 @@ type authTestCase struct { allow bool } -func testAuthorize(t *testing.T, name string, subject subject, sets ...[]authTestCase) { +func testAuthorize(t *testing.T, name string, subject Subject, sets ...[]authTestCase) { t.Helper() authorizer := NewAuthorizer(prometheus.NewRegistry()) for _, cases := range sets { @@ -962,9 +973,10 @@ func testAuthorize(t *testing.T, name string, subject subject, sets ...[]authTes ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) t.Cleanup(cancel) - authError := authorizer.Authorize(ctx, subject.UserID, subject.Roles, subject.Scope, subject.Groups, a, c.resource) + authError := authorizer.Authorize(ctx, subject, a, c.resource) d, _ := json.Marshal(map[string]interface{}{ + // This is not perfect marshal, but it is good enough for debugging this test. "subject": subject, "object": c.resource, "action": a, @@ -985,9 +997,14 @@ func testAuthorize(t *testing.T, name string, subject subject, sets ...[]authTes assert.Error(t, authError, "expected unauthorized") } - partialAuthz, err := authorizer.Prepare(ctx, subject.UserID, subject.Roles, subject.Scope, subject.Groups, a, c.resource.Type) + prepared, err := authorizer.Prepare(ctx, subject, a, c.resource.Type) require.NoError(t, err, "make prepared authorizer") + // For unit testing logging and assertions, we want the PartialAuthorizer + // struct. + partialAuthz, ok := prepared.(*PartialAuthorizer) + require.True(t, ok, "prepared authorizer is partial") + // Ensure the partial can compile to a SQL clause. // This does not guarantee that the clause is valid SQL. _, err = Compile(ConfigWithACL(), partialAuthz) diff --git a/coderd/rbac/authz_test.go b/coderd/rbac/authz_test.go index f8588fa9ba..31f2f6f298 100644 --- a/coderd/rbac/authz_test.go +++ b/coderd/rbac/authz_test.go @@ -12,11 +12,8 @@ import ( ) type benchmarkCase struct { - Name string - Roles rbac.RoleNames - Groups []string - UserID uuid.UUID - Scope rbac.ScopeName + Name string + Actor rbac.Subject } // benchmarkUserCases builds a set of users with different roles and groups. @@ -36,54 +33,66 @@ func benchmarkUserCases() (cases []benchmarkCase, users uuid.UUID, orgs []uuid.U benchCases := []benchmarkCase{ { - Name: "NoRoles", - Roles: []string{}, - UserID: user, - Scope: rbac.ScopeAll, + Name: "NoRoles", + Actor: rbac.Subject{ + ID: user.String(), + Roles: rbac.RoleNames{}, + Scope: rbac.ScopeAll, + }, }, { Name: "Admin", - // Give some extra roles that an admin might have - Roles: []string{rbac.RoleOrgMember(orgs[0]), "auditor", rbac.RoleOwner(), rbac.RoleMember()}, - UserID: user, - Scope: rbac.ScopeAll, - Groups: noiseGroups, + Actor: rbac.Subject{ + // Give some extra roles that an admin might have + Roles: rbac.RoleNames{rbac.RoleOrgMember(orgs[0]), "auditor", rbac.RoleOwner(), rbac.RoleMember()}, + ID: user.String(), + Scope: rbac.ScopeAll, + Groups: noiseGroups, + }, }, { - Name: "OrgAdmin", - Roles: []string{rbac.RoleOrgMember(orgs[0]), rbac.RoleOrgAdmin(orgs[0]), rbac.RoleMember()}, - UserID: user, - Scope: rbac.ScopeAll, - Groups: noiseGroups, + Name: "OrgAdmin", + Actor: rbac.Subject{ + Roles: rbac.RoleNames{rbac.RoleOrgMember(orgs[0]), rbac.RoleOrgAdmin(orgs[0]), rbac.RoleMember()}, + ID: user.String(), + Scope: rbac.ScopeAll, + Groups: noiseGroups, + }, }, { Name: "OrgMember", - // Member of 2 orgs - Roles: []string{rbac.RoleOrgMember(orgs[0]), rbac.RoleOrgMember(orgs[1]), rbac.RoleMember()}, - UserID: user, - Scope: rbac.ScopeAll, - Groups: noiseGroups, + Actor: rbac.Subject{ + // Member of 2 orgs + Roles: rbac.RoleNames{rbac.RoleOrgMember(orgs[0]), rbac.RoleOrgMember(orgs[1]), rbac.RoleMember()}, + ID: user.String(), + Scope: rbac.ScopeAll, + Groups: noiseGroups, + }, }, { Name: "ManyRoles", - // Admin of many orgs - Roles: []string{ - rbac.RoleOrgMember(orgs[0]), rbac.RoleOrgAdmin(orgs[0]), - rbac.RoleOrgMember(orgs[1]), rbac.RoleOrgAdmin(orgs[1]), - rbac.RoleOrgMember(orgs[2]), rbac.RoleOrgAdmin(orgs[2]), - rbac.RoleMember(), + Actor: rbac.Subject{ + // Admin of many orgs + Roles: rbac.RoleNames{ + rbac.RoleOrgMember(orgs[0]), rbac.RoleOrgAdmin(orgs[0]), + rbac.RoleOrgMember(orgs[1]), rbac.RoleOrgAdmin(orgs[1]), + rbac.RoleOrgMember(orgs[2]), rbac.RoleOrgAdmin(orgs[2]), + rbac.RoleMember(), + }, + ID: user.String(), + Scope: rbac.ScopeAll, + Groups: noiseGroups, }, - UserID: user, - Scope: rbac.ScopeAll, - Groups: noiseGroups, }, { Name: "AdminWithScope", - // Give some extra roles that an admin might have - Roles: []string{rbac.RoleOrgMember(orgs[0]), "auditor", rbac.RoleOwner(), rbac.RoleMember()}, - UserID: user, - Scope: rbac.ScopeApplicationConnect, - Groups: noiseGroups, + Actor: rbac.Subject{ + // Give some extra roles that an admin might have + Roles: rbac.RoleNames{rbac.RoleOrgMember(orgs[0]), "auditor", rbac.RoleOwner(), rbac.RoleMember()}, + ID: user.String(), + Scope: rbac.ScopeApplicationConnect, + Groups: noiseGroups, + }, }, } return benchCases, users, orgs @@ -108,7 +117,7 @@ func BenchmarkRBACAuthorize(b *testing.B) { objects := benchmarkSetup(orgs, users, b.N) b.ResetTimer() for i := 0; i < b.N; i++ { - allowed := authorizer.ByRoleName(context.Background(), c.UserID.String(), c.Roles, c.Scope, c.Groups, rbac.ActionRead, objects[b.N%len(objects)]) + allowed := authorizer.Authorize(context.Background(), c.Actor, rbac.ActionRead, objects[b.N%len(objects)]) var _ = allowed } }) @@ -136,8 +145,8 @@ func BenchmarkRBACAuthorizeGroups(b *testing.B) { for _, c := range benchCases { b.Run(c.Name+"GroupACL", func(b *testing.B) { userGroupAllow := uuid.NewString() - c.Groups = append(c.Groups, userGroupAllow) - c.Scope = rbac.ScopeAll + c.Actor.Groups = append(c.Actor.Groups, userGroupAllow) + c.Actor.Scope = rbac.ScopeAll objects := benchmarkSetup(orgs, users, b.N, func(object rbac.Object) rbac.Object { m := map[string][]rbac.Action{ // Add the user's group @@ -149,7 +158,7 @@ func BenchmarkRBACAuthorizeGroups(b *testing.B) { uuid.NewString(): {rbac.ActionRead, rbac.ActionUpdate, rbac.ActionDelete}, uuid.NewString(): {rbac.ActionRead, rbac.ActionUpdate}, } - for _, g := range c.Groups { + for _, g := range c.Actor.Groups { // Every group the user is in will be added, but it will not match the perms. This makes the // authorizer look at many groups before finding the one that matches. m[g] = []rbac.Action{rbac.ActionCreate, rbac.ActionRead, rbac.ActionUpdate, rbac.ActionDelete} @@ -160,7 +169,7 @@ func BenchmarkRBACAuthorizeGroups(b *testing.B) { }) b.ResetTimer() for i := 0; i < b.N; i++ { - allowed := authorizer.ByRoleName(context.Background(), c.UserID.String(), c.Roles, c.Scope, c.Groups, neverMatchAction, objects[b.N%len(objects)]) + allowed := authorizer.Authorize(context.Background(), c.Actor, neverMatchAction, objects[b.N%len(objects)]) var _ = allowed } }) @@ -184,7 +193,7 @@ func BenchmarkRBACFilter(b *testing.B) { b.Run(c.Name, func(b *testing.B) { objects := benchmarkSetup(orgs, users, b.N) b.ResetTimer() - allowed, err := rbac.Filter(context.Background(), authorizer, c.UserID.String(), c.Roles, c.Scope, c.Groups, rbac.ActionRead, objects) + allowed, err := rbac.Filter(context.Background(), authorizer, c.Actor, rbac.ActionRead, objects) require.NoError(b, err) var _ = allowed }) diff --git a/coderd/rbac/builtin.go b/coderd/rbac/builtin.go index 76d7f373a1..4839812459 100644 --- a/coderd/rbac/builtin.go +++ b/coderd/rbac/builtin.go @@ -226,7 +226,10 @@ var ( // CanAssignRole is a helper function that returns true if the user can assign // the specified role. This also can be used for removing a role. // This is a simple implementation for now. -func CanAssignRole(roles []string, assignedRole string) bool { +func CanAssignRole(expandable ExpandableRoles, assignedRole string) bool { + // For CanAssignRole, we only care about the names of the roles. + roles := expandable.Names() + assigned, assignedOrg, err := roleSplit(assignedRole) if err != nil { return false diff --git a/coderd/rbac/builtin_test.go b/coderd/rbac/builtin_test.go index cdbf652367..220d5df412 100644 --- a/coderd/rbac/builtin_test.go +++ b/coderd/rbac/builtin_test.go @@ -15,10 +15,8 @@ import ( type authSubject struct { // Name is helpful for test assertions - Name string - UserID string - Roles rbac.RoleNames - Groups []string + Name string + Actor rbac.Subject } func TestRolePermissions(t *testing.T) { @@ -39,17 +37,17 @@ func TestRolePermissions(t *testing.T) { apiKeyID := uuid.New() // Subjects to user - memberMe := authSubject{Name: "member_me", UserID: currentUser.String(), Roles: []string{rbac.RoleMember()}} - orgMemberMe := authSubject{Name: "org_member_me", UserID: currentUser.String(), Roles: []string{rbac.RoleMember(), rbac.RoleOrgMember(orgID)}} + memberMe := authSubject{Name: "member_me", Actor: rbac.Subject{ID: currentUser.String(), Roles: rbac.RoleNames{rbac.RoleMember()}}} + orgMemberMe := authSubject{Name: "org_member_me", Actor: rbac.Subject{ID: currentUser.String(), Roles: rbac.RoleNames{rbac.RoleMember(), rbac.RoleOrgMember(orgID)}}} - owner := authSubject{Name: "owner", UserID: adminID.String(), Roles: []string{rbac.RoleMember(), rbac.RoleOwner()}} - orgAdmin := authSubject{Name: "org_admin", UserID: adminID.String(), Roles: []string{rbac.RoleMember(), rbac.RoleOrgMember(orgID), rbac.RoleOrgAdmin(orgID)}} + owner := authSubject{Name: "owner", Actor: rbac.Subject{ID: adminID.String(), Roles: rbac.RoleNames{rbac.RoleMember(), rbac.RoleOwner()}}} + orgAdmin := authSubject{Name: "org_admin", Actor: rbac.Subject{ID: adminID.String(), Roles: rbac.RoleNames{rbac.RoleMember(), rbac.RoleOrgMember(orgID), rbac.RoleOrgAdmin(orgID)}}} - otherOrgMember := authSubject{Name: "org_member_other", UserID: uuid.NewString(), Roles: []string{rbac.RoleMember(), rbac.RoleOrgMember(otherOrg)}} - otherOrgAdmin := authSubject{Name: "org_admin_other", UserID: uuid.NewString(), Roles: []string{rbac.RoleMember(), rbac.RoleOrgMember(otherOrg), rbac.RoleOrgAdmin(otherOrg)}} + otherOrgMember := authSubject{Name: "org_member_other", Actor: rbac.Subject{ID: uuid.NewString(), Roles: rbac.RoleNames{rbac.RoleMember(), rbac.RoleOrgMember(otherOrg)}}} + otherOrgAdmin := authSubject{Name: "org_admin_other", Actor: rbac.Subject{ID: uuid.NewString(), Roles: rbac.RoleNames{rbac.RoleMember(), rbac.RoleOrgMember(otherOrg), rbac.RoleOrgAdmin(otherOrg)}}} - templateAdmin := authSubject{Name: "template-admin", UserID: templateAdminID.String(), Roles: []string{rbac.RoleMember(), rbac.RoleTemplateAdmin()}} - userAdmin := authSubject{Name: "user-admin", UserID: templateAdminID.String(), Roles: []string{rbac.RoleMember(), rbac.RoleUserAdmin()}} + templateAdmin := authSubject{Name: "template-admin", Actor: rbac.Subject{ID: templateAdminID.String(), Roles: rbac.RoleNames{rbac.RoleMember(), rbac.RoleTemplateAdmin()}}} + userAdmin := authSubject{Name: "user-admin", Actor: rbac.Subject{ID: templateAdminID.String(), Roles: rbac.RoleNames{rbac.RoleMember(), rbac.RoleUserAdmin()}}} // requiredSubjects are required to be asserted in each test case. This is // to make sure one is not forgotten. @@ -300,7 +298,12 @@ func TestRolePermissions(t *testing.T) { delete(remainingSubjs, subj.Name) msg := fmt.Sprintf("%s as %q doing %q on %q", c.Name, subj.Name, action, c.Resource.Type) // TODO: scopey - err := auth.ByRoleName(context.Background(), subj.UserID, subj.Roles, rbac.ScopeAll, subj.Groups, action, c.Resource) + actor := subj.Actor + // Actor is missing some fields + if actor.Scope == nil { + actor.Scope = rbac.ScopeAll + } + err := auth.Authorize(context.Background(), actor, action, c.Resource) if result { assert.NoError(t, err, fmt.Sprintf("Should pass: %s", msg)) } else { diff --git a/coderd/rbac/partial.go b/coderd/rbac/partial.go index 1f583ab808..19ee0a6c80 100644 --- a/coderd/rbac/partial.go +++ b/coderd/rbac/partial.go @@ -121,13 +121,30 @@ EachQueryLoop: return ForbiddenWithInternal(xerrors.Errorf("policy disallows request"), pa.input, nil) } -func newPartialAuthorizer(ctx context.Context, subjectID string, roles []Role, scope Scope, groups []string, action Action, objectType string) (*PartialAuthorizer, error) { +func newPartialAuthorizer(ctx context.Context, subject Subject, action Action, objectType string) (*PartialAuthorizer, error) { + if subject.Roles == nil { + return nil, xerrors.Errorf("subject must have roles") + } + if subject.Scope == nil { + return nil, xerrors.Errorf("subject must have a scope") + } + + roles, err := subject.Roles.Expand() + if err != nil { + return nil, xerrors.Errorf("expand roles: %w", err) + } + + scope, err := subject.Scope.Expand() + if err != nil { + return nil, xerrors.Errorf("expand scope: %w", err) + } + input := map[string]interface{}{ "subject": authSubject{ - ID: subjectID, + ID: subject.ID, Roles: roles, Scope: scope, - Groups: groups, + Groups: subject.Groups, }, "object": map[string]string{ "type": objectType, diff --git a/coderd/rbac/role.go b/coderd/rbac/role.go index 1aa97f9db0..c181ba6fe8 100644 --- a/coderd/rbac/role.go +++ b/coderd/rbac/role.go @@ -1,5 +1,20 @@ package rbac +// 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 +} + // Permission is the format passed into the rego. type Permission struct { // Negate makes this a negative permission @@ -27,3 +42,17 @@ type Role struct { Org map[string][]Permission `json:"org"` User []Permission `json:"user"` } + +type Roles []Role + +func (roles Roles) Expand() ([]Role, error) { + return roles, nil +} + +func (roles Roles) Names() []string { + names := make([]string, 0, len(roles)) + for _, r := range roles { + return append(names, r.Name) + } + return names +} diff --git a/coderd/rbac/scopes.go b/coderd/rbac/scopes.go index c500aa1333..15cdeb2da8 100644 --- a/coderd/rbac/scopes.go +++ b/coderd/rbac/scopes.go @@ -6,8 +6,23 @@ import ( "golang.org/x/xerrors" ) +type ExpandableScope interface { + Expand() (Scope, error) + // Name is for logging and tracing purposes, we want to know the human + // name of the scope. + Name() string +} + type ScopeName string +func (name ScopeName) Expand() (Scope, error) { + return ExpandScope(name) +} + +func (name ScopeName) Name() string { + return string(name) +} + // Scope acts the exact same as a Role with the addition that is can also // apply an AllowIDList. Any resource being checked against a Scope will // reject any resource that is not in the AllowIDList. @@ -18,6 +33,14 @@ type Scope struct { AllowIDList []string `json:"allow_list"` } +func (s Scope) Expand() (Scope, error) { + return s, nil +} + +func (s Scope) Name() string { + return s.Role.Name +} + const ( ScopeAll ScopeName = "all" ScopeApplicationConnect ScopeName = "application_connect" diff --git a/coderd/rbac/trace.go b/coderd/rbac/trace.go index 642d59b558..9fc796f29f 100644 --- a/coderd/rbac/trace.go +++ b/coderd/rbac/trace.go @@ -7,13 +7,13 @@ import ( // rbacTraceAttributes are the attributes that are added to all spans created by // the rbac package. These attributes should help to debug slow spans. -func rbacTraceAttributes(roles []string, groupCount int, scope ScopeName, action Action, objectType string, extra ...attribute.KeyValue) trace.SpanStartOption { +func rbacTraceAttributes(actor Subject, action Action, objectType string, extra ...attribute.KeyValue) trace.SpanStartOption { return trace.WithAttributes( append(extra, - attribute.StringSlice("subject_roles", roles), - attribute.Int("num_subject_roles", len(roles)), - attribute.Int("num_groups", groupCount), - attribute.String("scope", string(scope)), + attribute.StringSlice("subject_roles", actor.SafeRoleNames()), + attribute.Int("num_subject_roles", len(actor.SafeRoleNames())), + attribute.Int("num_groups", len(actor.Groups)), + attribute.String("scope", actor.SafeScopeName()), attribute.String("action", string(action)), attribute.String("object_type", objectType), )...) diff --git a/coderd/roles.go b/coderd/roles.go index 29be9c4a49..743d2bdba8 100644 --- a/coderd/roles.go +++ b/coderd/roles.go @@ -28,7 +28,7 @@ func (api *API) assignableSiteRoles(rw http.ResponseWriter, r *http.Request) { } roles := rbac.SiteRoles() - httpapi.Write(ctx, rw, http.StatusOK, assignableRoles(actorRoles.Roles, roles)) + httpapi.Write(ctx, rw, http.StatusOK, assignableRoles(actorRoles.Actor.Roles, roles)) } // assignableSiteRoles returns all org wide roles that can be assigned. @@ -52,7 +52,7 @@ func (api *API) assignableOrgRoles(rw http.ResponseWriter, r *http.Request) { } roles := rbac.OrganizationRoles(organization.ID) - httpapi.Write(ctx, rw, http.StatusOK, assignableRoles(actorRoles.Roles, roles)) + httpapi.Write(ctx, rw, http.StatusOK, assignableRoles(actorRoles.Actor.Roles, roles)) } func convertRole(role rbac.Role) codersdk.Role { @@ -62,7 +62,7 @@ func convertRole(role rbac.Role) codersdk.Role { } } -func assignableRoles(actorRoles []string, roles []rbac.Role) []codersdk.AssignableRoles { +func assignableRoles(actorRoles rbac.ExpandableRoles, roles []rbac.Role) []codersdk.AssignableRoles { assignable := make([]codersdk.AssignableRoles, 0) for _, role := range roles { if role.DisplayName == "" { diff --git a/coderd/users.go b/coderd/users.go index 36cd45fdf6..bd2c991a03 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -855,7 +855,7 @@ func (api *API) putUserRoles(rw http.ResponseWriter, r *http.Request) { // Just treat adding & removing as "assigning" for now. for _, roleName := range append(added, removed...) { - if !rbac.CanAssignRole(actorRoles.Roles, roleName) { + if !rbac.CanAssignRole(actorRoles.Actor.Roles, roleName) { httpapi.Forbidden(rw) return } diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go index de47b70616..23ef5170b8 100644 --- a/coderd/workspaceapps.go +++ b/coderd/workspaceapps.go @@ -467,7 +467,7 @@ func (api *API) authorizeWorkspaceApp(r *http.Request, accessMethod workspaceApp // workspaces owned by different users. if isPathApp && sharingLevel == database.AppSharingLevelOwner && - workspace.OwnerID != roles.ID && + workspace.OwnerID.String() != roles.Actor.ID && !api.DeploymentConfig.Dangerous.AllowPathAppSiteOwnerAccess.Value { return false, nil @@ -479,7 +479,7 @@ func (api *API) authorizeWorkspaceApp(r *http.Request, accessMethod workspaceApp // Regardless of share level or whether it's enabled or not, the owner of // the workspace can always access applications (as long as their API key's // scope allows it). - err := api.Authorizer.ByRoleName(ctx, roles.ID.String(), roles.Roles, roles.Scope.ToRBAC(), []string{}, rbac.ActionCreate, workspace.ApplicationConnectRBAC()) + err := api.Authorizer.Authorize(ctx, roles.Actor, rbac.ActionCreate, workspace.ApplicationConnectRBAC()) if err == nil { return true, nil } @@ -494,8 +494,8 @@ func (api *API) authorizeWorkspaceApp(r *http.Request, accessMethod workspaceApp // that they have ApplicationConnect permissions to their own // workspaces. This ensures that the key's scope has permission to // connect to workspace apps. - object := rbac.ResourceWorkspaceApplicationConnect.WithOwner(roles.ID.String()) - err := api.Authorizer.ByRoleName(ctx, roles.ID.String(), roles.Roles, roles.Scope.ToRBAC(), []string{}, rbac.ActionCreate, object) + object := rbac.ResourceWorkspaceApplicationConnect.WithOwner(roles.Actor.ID) + err := api.Authorizer.Authorize(ctx, roles.Actor, rbac.ActionCreate, object) if err == nil { return true, nil }