chore: Turn on rbac check caching.

Should not affect much unless authz_querier experiment is enabled
coder · Emyrk · Feb 15, 2023 · Feb 14, 2023 · Feb 14, 2023 · Feb 14, 2023
commit a70fe5507e6ce8d54d885c55e415383f85e16402
diff --git a/coderd/coderd.go b/coderd/coderd.go
@@ -187,7 +187,7 @@ func New(options *Options) *API {
 		options.PrometheusRegistry = prometheus.NewRegistry()
 	}
 	if options.Authorizer == nil {
-		options.Authorizer = rbac.NewAuthorizer(options.PrometheusRegistry)
+		options.Authorizer = rbac.NewCachingAuthorizer(options.PrometheusRegistry)
 	}
 	if options.TailnetCoordinator == nil {
 		options.TailnetCoordinator = tailnet.NewCoordinator()
@@ -289,6 +289,7 @@ func New(options *Options) *API {
 		tracing.StatusWriterMiddleware,
 		tracing.Middleware(api.TracerProvider),
 		httpmw.AttachRequestID,
+		httpmw.AttachAuthzCache,
 		httpmw.ExtractRealIP(api.RealIPConfig),
 		httpmw.Logger(api.Logger),
 		httpmw.Prometheus(options.PrometheusRegistry),

diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go
@@ -184,7 +184,7 @@ func NewOptions(t *testing.T, options *Options) (func(http.Handler), context.Can
 	if strings.Contains(os.Getenv("CODER_EXPERIMENTS_TEST"), string(codersdk.ExperimentAuthzQuerier)) {
 		if options.Authorizer == nil {
 			options.Authorizer = &RecordingAuthorizer{
-				Wrapped: rbac.NewAuthorizer(prometheus.NewRegistry()),
+				Wrapped: rbac.NewCachingAuthorizer(prometheus.NewRegistry()),
 			}
 		}
 		options.Database = dbauthz.New(options.Database, options.Authorizer, slogtest.Make(t, nil).Leveled(slog.LevelDebug))

diff --git a/coderd/httpmw/authz.go b/coderd/httpmw/authz.go
@@ -3,6 +3,8 @@ package httpmw
 import (
 	"net/http"
 
+	"github.com/coder/coder/coderd/rbac"
+
 	"github.com/coder/coder/coderd/database/dbauthz"
 
 	"github.com/go-chi/chi/v5"
@@ -35,3 +37,16 @@ func AsAuthzSystem(mws ...func(http.Handler) http.Handler) func(http.Handler) ht
 		})
 	}
 }
+
+// AttachAuthzCache enables the authz cache for the authorizer. All rbac checks will
+// run against the cache, meaning duplicate checks will not be performed.
+//
+// Note the cache is safe for multiple actors. So mixing user and system checks
+// is ok.
+func AttachAuthzCache(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
+		ctx := rbac.WithCacheCtx(r.Context())
+
+		next.ServeHTTP(rw, r.WithContext(ctx))
+	})
+}
diff --git a/coderd/rbac/authz.go b/coderd/rbac/authz.go
@@ -158,6 +158,13 @@ var (
 	partialQuery rego.PreparedPartialQuery
 )
 
+// NewCachingAuthorizer returns a new RegoAuthorizer that supports context based
+// caching. To utilize the caching, the context passed to Authorize() must be
+// created with 'WithCacheCtx(ctx)'.
+func NewCachingAuthorizer(registry prometheus.Registerer) Authorizer {
+	return Cacher(NewAuthorizer(registry))
+}
+
 func NewAuthorizer(registry prometheus.Registerer) *RegoAuthorizer {
 	queryOnce.Do(func() {
 		var err error

diff --git a/coderd/rbac/authz_test.go b/coderd/rbac/authz_test.go
@@ -109,7 +109,7 @@ func BenchmarkRBACAuthorize(b *testing.B) {
 		uuid.MustParse("0632b012-49e0-4d70-a5b3-f4398f1dcd52"),
 		uuid.MustParse("70dbaa7a-ea9c-4f68-a781-97b08af8461d"),
 	)
-	authorizer := rbac.NewAuthorizer(prometheus.NewRegistry())
+	authorizer := rbac.NewCachingAuthorizer(prometheus.NewRegistry())
 	// This benchmarks all the simple cases using just user permissions. Groups
 	// are added as noise, but do not do anything.
 	for _, c := range benchCases {
@@ -136,7 +136,7 @@ func BenchmarkRBACAuthorizeGroups(b *testing.B) {
 		uuid.MustParse("0632b012-49e0-4d70-a5b3-f4398f1dcd52"),
 		uuid.MustParse("70dbaa7a-ea9c-4f68-a781-97b08af8461d"),
 	)
-	authorizer := rbac.NewAuthorizer(prometheus.NewRegistry())
+	authorizer := rbac.NewCachingAuthorizer(prometheus.NewRegistry())
 
 	// Same benchmark cases, but this time groups will be used to match.
 	// Some '*' permissions will still match, but using a fake action reduces
@@ -188,7 +188,7 @@ func BenchmarkRBACFilter(b *testing.B) {
 		uuid.MustParse("70dbaa7a-ea9c-4f68-a781-97b08af8461d"),
 	)
 
-	authorizer := rbac.NewAuthorizer(prometheus.NewRegistry())
+	authorizer := rbac.NewCachingAuthorizer(prometheus.NewRegistry())
 
 	for _, c := range benchCases {
 		b.Run("PrepareOnly-"+c.Name, func(b *testing.B) {

diff --git a/coderd/rbac/builtin_test.go b/coderd/rbac/builtin_test.go
@@ -23,7 +23,7 @@ type authSubject struct {
 func TestRolePermissions(t *testing.T) {
 	t.Parallel()
 
-	auth := rbac.NewAuthorizer(prometheus.NewRegistry())
+	auth := rbac.NewCachingAuthorizer(prometheus.NewRegistry())
 
 	// currentUser is anything that references "me", "mine", or "my".
 	currentUser := uuid.New()

diff --git a/coderd/rbac/cache.go b/coderd/rbac/cache.go
@@ -20,6 +20,8 @@ type cachedCalls struct {
 // multiple calls are made to the Authorizer for the same subject, action, and
 // object. The cache is on each `ctx` and is not shared between requests.
 // If no cache is found on the context, the Authorizer is called as normal.
+//
+// Cacher is safe for multiple actors.
 func Cacher(authz Authorizer) Authorizer {
 	return &cachedCalls{authz: authz}
 }

diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go
@@ -47,7 +47,7 @@ func New(ctx context.Context, options *Options) (*API, error) {
 		options.PrometheusRegistry = prometheus.NewRegistry()
 	}
 	if options.Options.Authorizer == nil {
-		options.Options.Authorizer = rbac.NewAuthorizer(options.PrometheusRegistry)
+		options.Options.Authorizer = rbac.NewCachingAuthorizer(options.PrometheusRegistry)
 	}
 	ctx, cancelFunc := context.WithCancel(ctx)
 	api := &API{