Skip to content

Commit 3ae42f4

Browse files
EmyrkEmyrk
authored
chore: Update rego to be partial execution friendly (#3449)
- Improves performance of batch authorization calls - Enables possibility to convert rego auth calls into SQL WHERE clauses
1 parent 4a17e0d commit 3ae42f4

16 files changed

+1124
-818
lines changed

‎coderd/authorize.go

+14-2
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,21 @@ import (
1010
"github.com/coder/coder/coderd/rbac"
1111
)
1212

13-
func AuthorizeFilter[O rbac.Objecter](api *API, r *http.Request, action rbac.Action, objects []O) []O {
13+
func AuthorizeFilter[O rbac.Objecter](api *API, r *http.Request, action rbac.Action, objects []O) ([]O, error) {
1414
roles := httpmw.AuthorizationUserRoles(r)
15-
return rbac.Filter(r.Context(), api.Authorizer, roles.ID.String(), roles.Roles, action, objects)
15+
objects, err := rbac.Filter(r.Context(), api.Authorizer, roles.ID.String(), roles.Roles, action, objects)
16+
if err != nil {
17+
// Log the error as Filter should not be erroring.
18+
api.Logger.Error(r.Context(), "filter failed",
19+
slog.Error(err),
20+
slog.F("user_id", roles.ID),
21+
slog.F("username", roles.Username),
22+
slog.F("route", r.URL.Path),
23+
slog.F("action", action),
24+
)
25+
return nil, err
26+
}
27+
return objects, nil
1628
}
1729

1830
// Authorize will return false if the user is not authorized to do the action.

‎coderd/coderd_test.go

+27-7
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func TestBuildInfo(t *testing.T) {
6363
// TestAuthorizeAllEndpoints will check `authorize` is called on every endpoint registered.
6464
func TestAuthorizeAllEndpoints(t *testing.T) {
6565
t.Parallel()
66-
authorizer := &fakeAuthorizer{}
66+
authorizer := &recordingAuthorizer{}
6767

6868
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
6969
defer cancel()
@@ -563,21 +563,41 @@ type authCall struct {
563563
Object rbac.Object
564564
}
565565

566-
type fakeAuthorizer struct {
566+
type recordingAuthorizer struct {
567567
Called *authCall
568568
AlwaysReturn error
569569
}
570570

571-
func (f *fakeAuthorizer) ByRoleName(_ context.Context, subjectID string, roleNames []string, action rbac.Action, object rbac.Object) error {
572-
f.Called = &authCall{
571+
func (r *recordingAuthorizer) ByRoleName(_ context.Context, subjectID string, roleNames []string, action rbac.Action, object rbac.Object) error {
572+
r.Called = &authCall{
573573
SubjectID: subjectID,
574574
Roles: roleNames,
575575
Action: action,
576576
Object: object,
577577
}
578-
return f.AlwaysReturn
578+
return r.AlwaysReturn
579579
}
580580

581-
func (f *fakeAuthorizer) reset() {
582-
f.Called = nil
581+
func (r *recordingAuthorizer) PrepareByRoleName(_ context.Context, subjectID string, roles []string, action rbac.Action, _ string) (rbac.PreparedAuthorized, error) {
582+
return &fakePreparedAuthorizer{
583+
Original: r,
584+
SubjectID: subjectID,
585+
Roles: roles,
586+
Action: action,
587+
}, nil
588+
}
589+
590+
func (r *recordingAuthorizer) reset() {
591+
r.Called = nil
592+
}
593+
594+
type fakePreparedAuthorizer struct {
595+
Original *recordingAuthorizer
596+
SubjectID string
597+
Roles []string
598+
Action rbac.Action
599+
}
600+
601+
func (f *fakePreparedAuthorizer) Authorize(ctx context.Context, object rbac.Object) error {
602+
return f.Original.ByRoleName(ctx, f.SubjectID, f.Roles, f.Action, object)
583603
}

‎coderd/provisionerdaemons.go

+8-1
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,14 @@ func (api *API) provisionerDaemons(rw http.ResponseWriter, r *http.Request) {
5050
if daemons == nil {
5151
daemons = []database.ProvisionerDaemon{}
5252
}
53-
daemons = AuthorizeFilter(api, r, rbac.ActionRead, daemons)
53+
daemons, err = AuthorizeFilter(api, r, rbac.ActionRead, daemons)
54+
if err != nil {
55+
httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
56+
Message: "Internal error fetching provisioner daemons.",
57+
Detail: err.Error(),
58+
})
59+
return
60+
}
5461

5562
httpapi.Write(rw, http.StatusOK, daemons)
5663
}

‎coderd/rbac/authz.go

+48-23
Original file line numberDiff line numberDiff line change
@@ -11,24 +11,41 @@ import (
1111

1212
type Authorizer interface {
1313
ByRoleName(ctx context.Context, subjectID string, roleNames []string, action Action, object Object) error
14+
PrepareByRoleName(ctx context.Context, subjectID string, roleNames []string, action Action, objectType string) (PreparedAuthorized, error)
15+
}
16+
17+
type PreparedAuthorized interface {
18+
Authorize(ctx context.Context, object Object) error
1419
}
1520

1621
// Filter takes in a list of objects, and will filter the list removing all
17-
// the elements the subject does not have permission for.
18-
// Filter does not allocate a new slice, and will use the existing one
19-
// passed in. This can cause memory leaks if the slice is held for a prolonged
20-
// period of time.
21-
func Filter[O Objecter](ctx context.Context, auth Authorizer, subjID string, subjRoles []string, action Action, objects []O) []O {
22+
// the elements the subject does not have permission for. All objects must be
23+
// of the same type.
24+
func Filter[O Objecter](ctx context.Context, auth Authorizer, subjID string, subjRoles []string, action Action, objects []O) ([]O, error) {
25+
if len(objects) == 0 {
26+
// Nothing to filter
27+
return objects, nil
28+
}
29+
objectType := objects[0].RBACObject().Type
30+
2231
filtered := make([]O, 0)
32+
prepared, err := auth.PrepareByRoleName(ctx, subjID, subjRoles, action, objectType)
33+
if err != nil {
34+
return nil, xerrors.Errorf("prepare: %w", err)
35+
}
2336

2437
for i := range objects {
2538
object := objects[i]
26-
err := auth.ByRoleName(ctx, subjID, subjRoles, action, object.RBACObject())
39+
rbacObj := object.RBACObject()
40+
if rbacObj.Type != objectType {
41+
return nil, xerrors.Errorf("object types must be uniform across the set (%s), found %s", objectType, object.RBACObject().Type)
42+
}
43+
err := prepared.Authorize(ctx, rbacObj)
2744
if err == nil {
2845
filtered = append(filtered, object)
2946
}
3047
}
31-
return filtered
48+
return filtered, nil
3249
}
3350

3451
// RegoAuthorizer will use a prepared rego query for performing authorize()
@@ -45,7 +62,7 @@ func NewAuthorizer() (*RegoAuthorizer, error) {
4562
query, err := rego.New(
4663
// allowed is the `allow` field from the prepared query. This is the field to check if authorization is
4764
// granted.
48-
rego.Query("allowed = data.authz.allow"),
65+
rego.Query("data.authz.allow"),
4966
rego.Module("policy.rego", policy),
5067
).PrepareForEval(ctx)
5168

@@ -64,14 +81,11 @@ type authSubject struct {
6481
// This is the function intended to be used outside this package.
6582
// The role is fetched from the builtin map located in memory.
6683
func (a RegoAuthorizer) ByRoleName(ctx context.Context, subjectID string, roleNames []string, action Action, object Object) error {
67-
roles := make([]Role, 0, len(roleNames))
68-
for _, n := range roleNames {
69-
r, err := RoleByName(n)
70-
if err != nil {
71-
return xerrors.Errorf("get role permissions: %w", err)
72-
}
73-
roles = append(roles, r)
84+
roles, err := RolesByNames(roleNames)
85+
if err != nil {
86+
return err
7487
}
88+
7589
return a.Authorize(ctx, subjectID, roles, action, object)
7690
}
7791

@@ -92,18 +106,29 @@ func (a RegoAuthorizer) Authorize(ctx context.Context, subjectID string, roles [
92106
return ForbiddenWithInternal(xerrors.Errorf("eval rego: %w", err), input, results)
93107
}
94108

95-
if len(results) != 1 {
96-
return ForbiddenWithInternal(xerrors.Errorf("expect only 1 result, got %d", len(results)), input, results)
109+
if !results.Allowed() {
110+
return ForbiddenWithInternal(xerrors.Errorf("policy disallows request"), input, results)
97111
}
98112

99-
allowedResult, ok := (results[0].Bindings["allowed"]).(bool)
100-
if !ok {
101-
return ForbiddenWithInternal(xerrors.Errorf("expected allowed to be a bool but got %T", allowedResult), input, results)
113+
return nil
114+
}
115+
116+
// Prepare will partially execute the rego policy leaving the object fields unknown (except for the type).
117+
// This will vastly speed up performance if batch authorization on the same type of objects is needed.
118+
func (RegoAuthorizer) Prepare(ctx context.Context, subjectID string, roles []Role, action Action, objectType string) (*PartialAuthorizer, error) {
119+
auth, err := newPartialAuthorizer(ctx, subjectID, roles, action, objectType)
120+
if err != nil {
121+
return nil, xerrors.Errorf("new partial authorizer: %w", err)
102122
}
103123

104-
if !allowedResult {
105-
return ForbiddenWithInternal(xerrors.Errorf("policy disallows request"), input, results)
124+
return auth, nil
125+
}
126+
127+
func (a RegoAuthorizer) PrepareByRoleName(ctx context.Context, subjectID string, roleNames []string, action Action, objectType string) (PreparedAuthorized, error) {
128+
roles, err := RolesByNames(roleNames)
129+
if err != nil {
130+
return nil, err
106131
}
107132

108-
return nil
133+
return a.Prepare(ctx, subjectID, roles, action, objectType)
109134
}

0 commit comments

Comments
 (0)