coder · Emyrk · Aug 11, 2022 · Aug 10, 2022 · Aug 10, 2022 · Aug 10, 2022
diff --git a/coderd/authorize.go b/coderd/authorize.go
@@ -10,9 +10,27 @@ import (
 	"github.com/coder/coder/coderd/rbac"
 )
 
-func AuthorizeFilter[O rbac.Objecter](api *API, r *http.Request, action rbac.Action, objects []O) []O {
+func AuthorizeFilter[O rbac.Objecter](api *API, r *http.Request, action rbac.Action, objects []O) ([]O, error) {
 	roles := httpmw.AuthorizationUserRoles(r)
-	return rbac.Filter(r.Context(), api.Authorizer, roles.ID.String(), roles.Roles, action, objects)
+
+	if len(objects) == 0 {
+		return objects, nil
+	}
+	objecType := objects[0].RBACObject().Type
+	objects, err := rbac.Filter(r.Context(), api.Authorizer, roles.ID.String(), roles.Roles, action, objecType, objects)
+	if err != nil {
+		api.Logger.Error(r.Context(), "filter failed",
+			slog.Error(err),
+			slog.F("object_type", objecType),
+			slog.F("user_id", roles.ID),
+			slog.F("username", roles.Username),
+			slog.F("route", r.URL.Path),
+			slog.F("action", action),
+		)
+		// Hide the underlying error in case it has sensitive information
+		return nil, xerrors.Errorf("failed to filter requested objects")
+	}
+	return objects, nil
 }
 
 // Authorize will return false if the user is not authorized to do the action.

diff --git a/coderd/coderd_test.go b/coderd/coderd_test.go
@@ -63,7 +63,7 @@ func TestBuildInfo(t *testing.T) {
 // TestAuthorizeAllEndpoints will check `authorize` is called on every endpoint registered.
 func TestAuthorizeAllEndpoints(t *testing.T) {
 	t.Parallel()
-	authorizer := &fakeAuthorizer{}
+	authorizer := newRecordingAuthorizer()
 
 	ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
 	defer cancel()
@@ -563,21 +563,29 @@ type authCall struct {
 	Object    rbac.Object
 }
 
-type fakeAuthorizer struct {
+type recordingAuthorizer struct {
+	*rbac.FakeAuthorizer
 	Called       *authCall
 	AlwaysReturn error
 }
 
-func (f *fakeAuthorizer) ByRoleName(_ context.Context, subjectID string, roleNames []string, action rbac.Action, object rbac.Object) error {
-	f.Called = &authCall{
-		SubjectID: subjectID,
-		Roles:     roleNames,
-		Action:    action,
-		Object:    object,
+func newRecordingAuthorizer() *recordingAuthorizer {
+	r := &recordingAuthorizer{}
+	// Use the fake authorizer by rbac to handle prepared authorizers.
+	r.FakeAuthorizer = &rbac.FakeAuthorizer{
+		AuthFunc: func(ctx context.Context, subjectID string, roleNames []string, action rbac.Action, object rbac.Object) error {
+			r.Called = &authCall{
+				SubjectID: subjectID,
+				Roles:     roleNames,
+				Action:    action,
+				Object:    object,
+			}
+			return r.AlwaysReturn
+		},
 	}
-	return f.AlwaysReturn
+	return r
 }
 
-func (f *fakeAuthorizer) reset() {
+func (f *recordingAuthorizer) reset() {
 	f.Called = nil
 }
diff --git a/coderd/provisionerdaemons.go b/coderd/provisionerdaemons.go
@@ -50,7 +50,14 @@ func (api *API) provisionerDaemons(rw http.ResponseWriter, r *http.Request) {
 	if daemons == nil {
 		daemons = []database.ProvisionerDaemon{}
 	}
-	daemons = AuthorizeFilter(api, r, rbac.ActionRead, daemons)
+	daemons, err = AuthorizeFilter(api, r, rbac.ActionRead, daemons)
+	if err != nil {
+		httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
+			Message: "Internal error fetching provisioner daemons.",
+			Detail:  err.Error(),
+		})
+		return
+	}
 
 	httpapi.Write(rw, http.StatusOK, daemons)
 }

diff --git a/coderd/rbac/auth_fake.go b/coderd/rbac/auth_fake.go
@@ -0,0 +1,31 @@
+package rbac
+
+import "context"
+
+type FakeAuthorizer struct {
+	AuthFunc func(ctx context.Context, subjectID string, roleNames []string, action Action, object Object) error
+}
+
+func (f FakeAuthorizer) ByRoleName(ctx context.Context, subjectID string, roleNames []string, action Action, object Object) error {
+	return f.AuthFunc(ctx, subjectID, roleNames, action, object)
+}
+
+func (f FakeAuthorizer) PrepareByRoleName(_ context.Context, subjectID string, roles []string, action Action, _ string) (PreparedAuthorized, error) {
+	return &fakePreparedAuthorizer{
+		Original:  f,
+		SubjectID: subjectID,
+		Roles:     roles,
+		Action:    action,
+	}, nil
+}
+
+type fakePreparedAuthorizer struct {
+	Original  Authorizer
+	SubjectID string
+	Roles     []string
+	Action    Action
+}
+
+func (f fakePreparedAuthorizer) Authorize(ctx context.Context, object Object) error {
+	return f.Original.ByRoleName(ctx, f.SubjectID, f.Roles, f.Action, object)
+}
diff --git a/coderd/rbac/authz.go b/coderd/rbac/authz.go
@@ -11,24 +11,35 @@ import (
 
 type Authorizer interface {
 	ByRoleName(ctx context.Context, subjectID string, roleNames []string, action Action, object Object) error
+	PrepareByRoleName(ctx context.Context, subjectID string, roles []string, action Action, objectType string) (PreparedAuthorized, error)
+}
+
+type PreparedAuthorized interface {
+	Authorize(ctx context.Context, object Object) error
 }
 
 // Filter takes in a list of objects, and will filter the list removing all
-// the elements the subject does not have permission for.
-// Filter does not allocate a new slice, and will use the existing one
-// passed in. This can cause memory leaks if the slice is held for a prolonged
-// period of time.
-func Filter[O Objecter](ctx context.Context, auth Authorizer, subjID string, subjRoles []string, action Action, objects []O) []O {
+// the elements the subject does not have permission for. All objects must be
+// of the same type.
+func Filter[O Objecter](ctx context.Context, auth Authorizer, subjID string, subjRoles []string, action Action, objectType string, objects []O) ([]O, error) {
 	filtered := make([]O, 0)
+	prepared, err := auth.PrepareByRoleName(ctx, subjID, subjRoles, action, objectType)
+	if err != nil {
+		return nil, xerrors.Errorf("prepare: %w", err)
+	}
 
 	for i := range objects {
 		object := objects[i]
-		err := auth.ByRoleName(ctx, subjID, subjRoles, action, object.RBACObject())
+		rbacObj := object.RBACObject()
+		if rbacObj.Type != objectType {
+			return nil, xerrors.Errorf("object types must be uniform across the set (%s), found %s", objectType, object.RBACObject().Type)
+		}
+		err := prepared.Authorize(ctx, rbacObj)
 		if err == nil {
 			filtered = append(filtered, object)
 		}
 	}
-	return filtered
+	return filtered, nil
 }
 
 // RegoAuthorizer will use a prepared rego query for performing authorize()
@@ -45,7 +56,7 @@ func NewAuthorizer() (*RegoAuthorizer, error) {
 	query, err := rego.New(
 		// allowed is the `allow` field from the prepared query. This is the field to check if authorization is
 		// granted.
-		rego.Query("allowed = data.authz.allow"),
+		rego.Query("data.authz.allow"),
 		rego.Module("policy.rego", policy),
 	).PrepareForEval(ctx)
 
@@ -64,14 +75,11 @@ type authSubject struct {
 // 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, action Action, object Object) error {
-	roles := make([]Role, 0, len(roleNames))
-	for _, n := range roleNames {
-		r, err := RoleByName(n)
-		if err != nil {
-			return xerrors.Errorf("get role permissions: %w", err)
-		}
-		roles = append(roles, r)
+	roles, err := RolesByNames(roleNames)
+	if err != nil {
+		return err
 	}
+
 	return a.Authorize(ctx, subjectID, roles, action, object)
 }
 
@@ -92,18 +100,29 @@ func (a RegoAuthorizer) Authorize(ctx context.Context, subjectID string, roles [
 		return ForbiddenWithInternal(xerrors.Errorf("eval rego: %w", err), input, results)
 	}
 
-	if len(results) != 1 {
-		return ForbiddenWithInternal(xerrors.Errorf("expect only 1 result, got %d", len(results)), input, results)
+	if !results.Allowed() {
+		return ForbiddenWithInternal(xerrors.Errorf("policy disallows request"), input, results)
 	}
 
-	allowedResult, ok := (results[0].Bindings["allowed"]).(bool)
-	if !ok {
-		return ForbiddenWithInternal(xerrors.Errorf("expected allowed to be a bool but got %T", allowedResult), input, results)
+	return 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, action Action, objectType string) (*PartialAuthorizer, error) {
+	auth, err := newPartialAuthorizer(ctx, subjectID, roles, action, objectType)
+	if err != nil {
+		return nil, xerrors.Errorf("new partial authorizer: %w", err)
 	}
 
-	if !allowedResult {
-		return ForbiddenWithInternal(xerrors.Errorf("policy disallows request"), input, results)
+	return auth, nil
+}
+
+func (a RegoAuthorizer) PrepareByRoleName(ctx context.Context, subjectID string, roleNames []string, action Action, objectType string) (PreparedAuthorized, error) {
+	roles, err := RolesByNames(roleNames)
+	if err != nil {
+		return nil, err
 	}
 
-	return nil
+	return a.Prepare(ctx, subjectID, roles, action, objectType)
 }