Skip to content

feat: Turn on rbac check caching #6202

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Feb 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion coderd/coderd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion coderd/coderdtest/coderdtest.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
18 changes: 16 additions & 2 deletions coderd/httpmw/authz.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ package httpmw
import (
"net/http"

"github.com/coder/coder/coderd/database/dbauthz"

"github.com/go-chi/chi/v5"

"github.com/coder/coder/coderd/database/dbauthz"
"github.com/coder/coder/coderd/rbac"
)

// AsAuthzSystem is a chained handler that temporarily sets the dbauthz context
Expand Down Expand Up @@ -35,3 +36,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))
})
}
7 changes: 7 additions & 0 deletions coderd/rbac/authz.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions coderd/rbac/authz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion coderd/rbac/builtin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 2 additions & 0 deletions coderd/rbac/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
}
Expand Down
28 changes: 28 additions & 0 deletions coderd/rbac/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package rbac_test

import (
"context"
"fmt"
"testing"

"github.com/stretchr/testify/require"
Expand All @@ -10,6 +11,33 @@ import (
"github.com/coder/coder/coderd/rbac"
)

// BenchmarkCacher benchmarks the performance of the cacher with a given
// cache size. The expected cache size in prod will usually be 1-2. In Filter
// cases it can get as high as 10.
func BenchmarkCacher(b *testing.B) {
b.ResetTimer()
// Size of the cache.
sizes := []int{1, 10, 100, 1000}
for _, size := range sizes {
b.Run(fmt.Sprintf("Size%d", size), func(b *testing.B) {
ctx := rbac.WithCacheCtx(context.Background())
authz := rbac.Cacher(&coderdtest.FakeAuthorizer{AlwaysReturn: nil})
for i := 0; i < size; i++ {
// Preload the cache of a given size
subj, obj, action := coderdtest.RandomRBACSubject(), coderdtest.RandomRBACObject(), coderdtest.RandomRBACAction()
_ = authz.Authorize(ctx, subj, action, obj)
}

// Cache is loaded as a slice, so this cache hit is always the last element.
subj, obj, action := coderdtest.RandomRBACSubject(), coderdtest.RandomRBACObject(), coderdtest.RandomRBACAction()
b.ResetTimer()
for i := 0; i < b.N; i++ {
_ = authz.Authorize(ctx, subj, action, obj)
}
})
}
}

func TestCacher(t *testing.T) {
t.Parallel()

Expand Down
2 changes: 1 addition & 1 deletion enterprise/coderd/coderd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down