Jan 26, 2024 · Jan 26, 2024
diff --git a/coderd/rbac/authz.go b/coderd/rbac/authz.go
 "crypto/sha256"
 _ "embed"
 "encoding/json"
 "errors"
 "strings"
 "sync"
 "time"
 authz Authorizer
 }

 // Cacher returns an Authorizer that can use a cachestored on a context
 //to short circuit duplicatecalls to the Authorizer. This is useful when
 //multiple calls are made to theAuthorizer for the same subject, action, and
 //object. The cacheison each `ctx` and is notshared between requests.
 // Cacher returns an Authorizer that can use a cacheto short circuit duplicate
 // calls to the Authorizer. This is useful when multiple calls are made to the
 // Authorizer for the same subject, action, and object.
 //Thisisa GLOBAL cacheshared between all requests.
 // If no cache is found on the context, the Authorizer is called as normal.
 //
 // Cacher is safe for multiple actors.
 err, _, ok := c.cache.Get(authorizeCacheKey)
 if !ok {
 err = c.authz.Authorize(ctx, subject, action, object)
 // In case there is a caching bug, bound the TTL to 1 minute.
 c.cache.Set(authorizeCacheKey, err, time.Minute)
 // If there is a transient error such as a context cancellation, do not
 // cache it.
 if !errors.Is(err, context.Canceled) {
 // In case there is a caching bug, bound the TTL to 1 minute.
 c.cache.Set(authorizeCacheKey, err, time.Minute)
 }
 }

 return err
diff --git a/coderd/rbac/authz_test.go b/coderd/rbac/authz_test.go

 "github.com/google/uuid"
 "github.com/prometheus/client_golang/prometheus"
 "github.com/stretchr/testify/assert"
 "github.com/stretchr/testify/require"

 "github.com/coder/coder/v2/coderd/coderdtest"
 "github.com/coder/coder/v2/coderd/rbac"
 "github.com/coder/coder/v2/testutil"
 )

 type benchmarkCase struct {
 require.NoError(t, rec.AllAsserted(), "all assertions should have been made")
 })

 t.Run("DontCacheTransientErrors", func(t *testing.T) {
 t.Parallel()

 var (
 ctx           = testutil.Context(t, testutil.WaitShort)
 authOut       = make(chan error, 1) // buffered to not block
 authorizeFunc = func(ctx context.Context, subject rbac.Subject, action rbac.Action, object rbac.Object) error {
 // Just return what you're told.
 return testutil.RequireRecvCtx(ctx, t, authOut)
 }
 ma                = &rbac.MockAuthorizer{AuthorizeFunc: authorizeFunc}
 rec               = &coderdtest.RecordingAuthorizer{Wrapped: ma}
 authz             = rbac.Cacher(rec)
 subj, obj, action = coderdtest.RandomRBACSubject(), coderdtest.RandomRBACObject(), coderdtest.RandomRBACAction()
 )

 // First call will result in a transient error. This should not be cached.
 testutil.RequireSendCtx(ctx, t, authOut, context.Canceled)
 err := authz.Authorize(ctx, subj, action, obj)
 assert.ErrorIs(t, err, context.Canceled)

 // A subsequent call should still hit the authorizer.
 testutil.RequireSendCtx(ctx, t, authOut, nil)
 err = authz.Authorize(ctx, subj, action, obj)
 assert.NoError(t, err)
 // This should be cached and not hit the wrapped authorizer again.
 err = authz.Authorize(ctx, subj, action, obj)
 assert.NoError(t, err)

 // Let's change the subject.
 subj, obj, action = coderdtest.RandomRBACSubject(), coderdtest.RandomRBACObject(), coderdtest.RandomRBACAction()

 // A third will be a legit error
 testutil.RequireSendCtx(ctx, t, authOut, assert.AnError)
 err = authz.Authorize(ctx, subj, action, obj)
 assert.EqualError(t, err, assert.AnError.Error())
 // This should be cached and not hit the wrapped authorizer again.
 err = authz.Authorize(ctx, subj, action, obj)
 assert.EqualError(t, err, assert.AnError.Error())
 })

 t.Run("MultipleSubjects", func(t *testing.T) {
 t.Parallel()
Original file line number	Diff line number	Diff line change
Expand Up		@@ -5,6 +5,7 @@ import (
		"crypto/sha256"
		_ "embed"
		"encoding/json"
		"errors"
		"strings"
		"sync"
		"time"
Expand DownExpand Up		@@ -653,10 +654,10 @@ type authCache struct {
		authz Authorizer
		}

		// Cacher returns an Authorizer that can use a cachestored on a context
		//to short circuit duplicatecalls to the Authorizer. This is useful when
		//multiple calls are made to theAuthorizer for the same subject, action, and
		//object. The cacheison each `ctx` and is notshared between requests.
		// Cacher returns an Authorizer that can use a cacheto short circuit duplicate
		// calls to the Authorizer. This is useful when multiple calls are made to the
		// Authorizer for the same subject, action, and object.
		//Thisisa GLOBAL cacheshared between all requests.
		// If no cache is found on the context, the Authorizer is called as normal.
		//
		// Cacher is safe for multiple actors.
Expand All		@@ -676,8 +677,12 @@ func (c *authCache) Authorize(ctx context.Context, subject Subject, action Actio
		err, _, ok := c.cache.Get(authorizeCacheKey)
		if !ok {
		err = c.authz.Authorize(ctx, subject, action, object)
		// In case there is a caching bug, bound the TTL to 1 minute.
		c.cache.Set(authorizeCacheKey, err, time.Minute)
		// If there is a transient error such as a context cancellation, do not
		// cache it.
		if !errors.Is(err, context.Canceled) {
		// In case there is a caching bug, bound the TTL to 1 minute.
		c.cache.Set(authorizeCacheKey, err, time.Minute)
		}
		}

		return err
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -7,10 +7,12 @@ import (

		"github.com/google/uuid"
		"github.com/prometheus/client_golang/prometheus"
		"github.com/stretchr/testify/assert"
		"github.com/stretchr/testify/require"

		"github.com/coder/coder/v2/coderd/coderdtest"
		"github.com/coder/coder/v2/coderd/rbac"
		"github.com/coder/coder/v2/testutil"
		)

		type benchmarkCase struct {
Expand DownExpand Up		@@ -351,6 +353,47 @@ func TestCacher(t *testing.T) {
		require.NoError(t, rec.AllAsserted(), "all assertions should have been made")
		})

		t.Run("DontCacheTransientErrors", func(t *testing.T) {
		t.Parallel()

		var (
		ctx = testutil.Context(t, testutil.WaitShort)
		authOut = make(chan error, 1) // buffered to not block
		authorizeFunc = func(ctx context.Context, subject rbac.Subject, action rbac.Action, object rbac.Object) error {
		// Just return what you're told.
		return testutil.RequireRecvCtx(ctx, t, authOut)
		}
		ma = &rbac.MockAuthorizer{AuthorizeFunc: authorizeFunc}
		rec = &coderdtest.RecordingAuthorizer{Wrapped: ma}
		authz = rbac.Cacher(rec)
		subj, obj, action = coderdtest.RandomRBACSubject(), coderdtest.RandomRBACObject(), coderdtest.RandomRBACAction()
		)

		// First call will result in a transient error. This should not be cached.
		testutil.RequireSendCtx(ctx, t, authOut, context.Canceled)
		err := authz.Authorize(ctx, subj, action, obj)
		assert.ErrorIs(t, err, context.Canceled)

		// A subsequent call should still hit the authorizer.
		testutil.RequireSendCtx(ctx, t, authOut, nil)
		err = authz.Authorize(ctx, subj, action, obj)
		assert.NoError(t, err)
		// This should be cached and not hit the wrapped authorizer again.
		err = authz.Authorize(ctx, subj, action, obj)
		assert.NoError(t, err)

		// Let's change the subject.
		subj, obj, action = coderdtest.RandomRBACSubject(), coderdtest.RandomRBACObject(), coderdtest.RandomRBACAction()

		// A third will be a legit error
		testutil.RequireSendCtx(ctx, t, authOut, assert.AnError)
		err = authz.Authorize(ctx, subj, action, obj)
		assert.EqualError(t, err, assert.AnError.Error())
		// This should be cached and not hit the wrapped authorizer again.
		err = authz.Authorize(ctx, subj, action, obj)
		assert.EqualError(t, err, assert.AnError.Error())
		})

		t.Run("MultipleSubjects", func(t *testing.T) {
		t.Parallel()

Expand Down