Skip to content

Commit f63d7b3

Browse files
EmyrkEmyrk
committed
chore: Implement standard rbac.Subject to be reused everywhere (#5881)
* chore: Implement standard rbac.Subject to be reused everywhere An rbac subject is created in multiple spots because of the way we expand roles, scopes, etc. This difference in use creates a list of arguments which is unwieldy. Use of the expander interface lets us conform to a single subject in every case
1 parent 57b1830 commit f63d7b3

18 files changed

+449
-355
lines changed

‎coderd/authorize.go

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,15 @@ import (
1919
// This is faster than calling Authorize() on each object.
2020
func AuthorizeFilter[O rbac.Objecter](h *HTTPAuthorizer, r *http.Request, action rbac.Action, objects []O) ([]O, error) {
2121
roles := httpmw.UserAuthorization(r)
22-
objects, err := rbac.Filter(r.Context(), h.Authorizer, roles.ID.String(), roles.Roles, roles.Scope.ToRBAC(), roles.Groups, action, objects)
22+
objects, err := rbac.Filter(r.Context(), h.Authorizer, roles.Actor, action, objects)
2323
if err != nil {
2424
// Log the error as Filter should not be erroring.
2525
h.Logger.Error(r.Context(), "filter failed",
2626
slog.Error(err),
27-
slog.F("user_id", roles.ID),
27+
slog.F("user_id", roles.Actor.ID),
2828
slog.F("username", roles.Username),
29-
slog.F("scope", roles.Scope),
29+
slog.F("roles", roles.Actor.SafeRoleNames()),
30+
slog.F("scope", roles.Actor.SafeScopeName()),
3031
slog.F("route", r.URL.Path),
3132
slog.F("action", action),
3233
)
@@ -64,7 +65,7 @@ func (api *API) Authorize(r *http.Request, action rbac.Action, object rbac.Objec
6465
// }
6566
func (h *HTTPAuthorizer) Authorize(r *http.Request, action rbac.Action, object rbac.Objecter) bool {
6667
roles := httpmw.UserAuthorization(r)
67-
err := h.Authorizer.ByRoleName(r.Context(), roles.ID.String(), roles.Roles, roles.Scope.ToRBAC(), roles.Groups, action, object.RBACObject())
68+
err := h.Authorizer.Authorize(r.Context(), roles.Actor, action, object.RBACObject())
6869
if err != nil {
6970
// Log the errors for debugging
7071
internalError := new(rbac.UnauthorizedError)
@@ -75,10 +76,10 @@ func (h *HTTPAuthorizer) Authorize(r *http.Request, action rbac.Action, object r
7576
// Log information for debugging. This will be very helpful
7677
// in the early days
7778
logger.Warn(r.Context(), "unauthorized",
78-
slog.F("roles", roles.Roles),
79-
slog.F("user_id", roles.ID),
79+
slog.F("roles", roles.Actor.SafeRoleNames()),
80+
slog.F("user_id", roles.Actor.ID),
8081
slog.F("username", roles.Username),
81-
slog.F("scope", roles.Scope),
82+
slog.F("scope", roles.Actor.SafeScopeName()),
8283
slog.F("route", r.URL.Path),
8384
slog.F("action", action),
8485
slog.F("object", object),
@@ -96,7 +97,7 @@ func (h *HTTPAuthorizer) Authorize(r *http.Request, action rbac.Action, object r
9697
// Note the authorization is only for the given action and object type.
9798
func (h *HTTPAuthorizer) AuthorizeSQLFilter(r *http.Request, action rbac.Action, objectType string) (rbac.PreparedAuthorized, error) {
9899
roles := httpmw.UserAuthorization(r)
99-
prepared, err := h.Authorizer.PrepareByRoleName(r.Context(), roles.ID.String(), roles.Roles, roles.Scope.ToRBAC(), roles.Groups, action, objectType)
100+
prepared, err := h.Authorizer.Prepare(r.Context(), roles.Actor, action, objectType)
100101
if err != nil {
101102
return nil, xerrors.Errorf("prepare filter: %w", err)
102103
}
@@ -127,9 +128,10 @@ func (api *API) checkAuthorization(rw http.ResponseWriter, r *http.Request) {
127128

128129
api.Logger.Debug(ctx, "check-auth",
129130
slog.F("my_id", httpmw.APIKey(r).UserID),
130-
slog.F("got_id", auth.ID),
131+
slog.F("got_id", auth.Actor.ID),
131132
slog.F("name", auth.Username),
132-
slog.F("roles", auth.Roles), slog.F("scope", auth.Scope),
133+
slog.F("roles", auth.Actor.SafeRoleNames()),
134+
slog.F("scope", auth.Actor.SafeScopeName()),
133135
)
134136

135137
response := make(codersdk.AuthorizationResponse)
@@ -169,7 +171,7 @@ func (api *API) checkAuthorization(rw http.ResponseWriter, r *http.Request) {
169171
Type: v.Object.ResourceType,
170172
}
171173
if obj.Owner == "me" {
172-
obj.Owner = auth.ID.String()
174+
obj.Owner = auth.Actor.ID
173175
}
174176

175177
// If a resource ID is specified, fetch that specific resource.
@@ -217,7 +219,7 @@ func (api *API) checkAuthorization(rw http.ResponseWriter, r *http.Request) {
217219
obj = dbObj.RBACObject()
218220
}
219221

220-
err := api.Authorizer.ByRoleName(ctx, auth.ID.String(), auth.Roles, auth.Scope.ToRBAC(), auth.Groups, rbac.Action(v.Action), obj)
222+
err := api.Authorizer.Authorize(ctx, auth.Actor, rbac.Action(v.Action), obj)
221223
response[k] = err == nil
222224
}
223225

‎coderd/coderdtest/authorize.go

Lines changed: 14 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -533,12 +533,9 @@ func (a *AuthTester) Test(ctx context.Context, assertRoute map[string]RouteCheck
533533
}
534534

535535
type authCall struct {
536-
SubjectID string
537-
Roles rbac.ExpandableRoles
538-
Groups []string
539-
Scope rbac.ScopeName
540-
Action rbac.Action
541-
Object rbac.Object
536+
Subject rbac.Subject
537+
Action rbac.Action
538+
Object rbac.Object
542539
}
543540

544541
type RecordingAuthorizer struct {
@@ -548,33 +545,27 @@ type RecordingAuthorizer struct {
548545

549546
var _ rbac.Authorizer = (*RecordingAuthorizer)(nil)
550547

551-
// ByRoleNameSQL does not record the call. This matches the postgres behavior
548+
// AuthorizeSQL does not record the call. This matches the postgres behavior
552549
// of not calling Authorize()
553-
func (r *RecordingAuthorizer) ByRoleNameSQL(_ context.Context, _ string, _ rbac.ExpandableRoles, _ rbac.ScopeName, _ []string, _ rbac.Action, _ rbac.Object) error {
550+
func (r *RecordingAuthorizer) AuthorizeSQL(_ context.Context, _ rbac.Subject, _ rbac.Action, _ rbac.Object) error {
554551
return r.AlwaysReturn
555552
}
556553

557-
func (r *RecordingAuthorizer) ByRoleName(_ context.Context, subjectID string, roleNames rbac.ExpandableRoles, scope rbac.ScopeName, groups []string, action rbac.Action, object rbac.Object) error {
554+
func (r *RecordingAuthorizer) Authorize(_ context.Context, subject rbac.Subject, action rbac.Action, object rbac.Object) error {
558555
r.Called = &authCall{
559-
SubjectID: subjectID,
560-
Roles: roleNames,
561-
Groups: groups,
562-
Scope: scope,
563-
Action: action,
564-
Object: object,
556+
Subject: subject,
557+
Action: action,
558+
Object: object,
565559
}
566560
return r.AlwaysReturn
567561
}
568562

569-
func (r *RecordingAuthorizer) PrepareByRoleName(_ context.Context, subjectID string, roles rbac.ExpandableRoles, scope rbac.ScopeName, groups []string, action rbac.Action, _ string) (rbac.PreparedAuthorized, error) {
563+
func (r *RecordingAuthorizer) Prepare(_ context.Context, subject rbac.Subject, action rbac.Action, _ string) (rbac.PreparedAuthorized, error) {
570564
return &fakePreparedAuthorizer{
571565
Original: r,
572-
SubjectID: subjectID,
573-
Roles: roles,
574-
Scope: scope,
566+
Subject: subject,
575567
Action: action,
576568
HardCodedSQLString: "true",
577-
Groups: groups,
578569
}, nil
579570
}
580571

@@ -584,17 +575,14 @@ func (r *RecordingAuthorizer) reset() {
584575

585576
type fakePreparedAuthorizer struct {
586577
Original *RecordingAuthorizer
587-
SubjectID string
588-
Roles rbac.ExpandableRoles
589-
Scope rbac.ScopeName
578+
Subject rbac.Subject
590579
Action rbac.Action
591-
Groups []string
592580
HardCodedSQLString string
593581
HardCodedRegoString string
594582
}
595583

596584
func (f *fakePreparedAuthorizer) Authorize(ctx context.Context, object rbac.Object) error {
597-
return f.Original.ByRoleName(ctx, f.SubjectID, f.Roles, f.Scope, f.Groups, f.Action, object)
585+
return f.Original.Authorize(ctx, f.Subject, f.Action, object)
598586
}
599587

600588
// CompileToSQL returns a compiled version of the authorizer that will work for
@@ -604,7 +592,7 @@ func (fakePreparedAuthorizer) CompileToSQL(_ context.Context, _ regosql.ConvertC
604592
}
605593

606594
func (f *fakePreparedAuthorizer) Eval(object rbac.Object) bool {
607-
return f.Original.ByRoleNameSQL(context.Background(), f.SubjectID, f.Roles, f.Scope, f.Groups, f.Action, object) == nil
595+
return f.Original.AuthorizeSQL(context.Background(), f.Subject, f.Action, object) == nil
608596
}
609597

610598
func (f fakePreparedAuthorizer) RegoString() string {

‎coderd/httpmw/apikey.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,10 @@ func APIKey(r *http.Request) database.APIKey {
5353
type userAuthKey struct{}
5454

5555
type Authorization struct {
56-
ID uuid.UUID
56+
Actor rbac.Subject
57+
// Username is required for logging and human friendly related
58+
// identification.
5759
Username string
58-
Roles rbac.RoleNames
59-
Groups []string
60-
Scope database.APIKeyScope
6160
}
6261

6362
// UserAuthorizationOptional may return the roles and scope used for
@@ -345,11 +344,13 @@ func ExtractAPIKey(cfg ExtractAPIKeyConfig) func(http.Handler) http.Handler {
345344

346345
ctx = context.WithValue(ctx, apiKeyContextKey{}, key)
347346
ctx = context.WithValue(ctx, userAuthKey{}, Authorization{
348-
ID: key.UserID,
349347
Username: roles.Username,
350-
Roles: roles.Roles,
351-
Scope: key.Scope,
352-
Groups: roles.Groups,
348+
Actor: rbac.Subject{
349+
ID: key.UserID.String(),
350+
Roles: rbac.RoleNames(roles.Roles),
351+
Groups: roles.Groups,
352+
Scope: rbac.ScopeName(key.Scope),
353+
},
353354
})
354355
// Set the auth context for the authzquerier as well.
355356
ctx = authzquery.WithAuthorizeContext(ctx,

‎coderd/httpmw/authorize_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,8 @@ func TestExtractUserRoles(t *testing.T) {
126126
)
127127
rtr.Get("/", func(_ http.ResponseWriter, r *http.Request) {
128128
roles := httpmw.UserAuthorization(r)
129-
require.ElementsMatch(t, user.ID, roles.ID)
130-
require.ElementsMatch(t, expRoles, roles.Roles)
129+
require.Equal(t, user.ID.String(), roles.Actor.ID)
130+
require.ElementsMatch(t, expRoles, roles.Actor.Roles.Names())
131131
})
132132

133133
req := httptest.NewRequest("GET", "/", nil)

‎coderd/httpmw/ratelimit.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func RateLimit(count int, window time.Duration) func(http.Handler) http.Handler
4747

4848
// We avoid using rbac.Authorizer since rego is CPU-intensive
4949
// and undermines the DoS-prevention goal of the rate limiter.
50-
for _, role := range auth.Roles {
50+
for _, role := range auth.Actor.SafeRoleNames() {
5151
if role == rbac.RoleOwner() {
5252
// HACK: use a random key each time to
5353
// de facto disable rate limiting. The

‎coderd/members.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ func (api *API) putMemberRoles(rw http.ResponseWriter, r *http.Request) {
6767

6868
// Just treat adding & removing as "assigning" for now.
6969
for _, roleName := range append(added, removed...) {
70-
if !rbac.CanAssignRole(actorRoles.Roles, roleName) {
70+
if !rbac.CanAssignRole(actorRoles.Actor.Roles, roleName) {
7171
httpapi.Forbidden(rw)
7272
return
7373
}

0 commit comments

Comments
 (0)