Update unit tests to cover the no actor case

Emyrk · Emyrk · commit a93c2d552a89 · 2023-02-10T11:54:14.000-06:00
diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go
@@ -137,12 +137,21 @@ func AsSystem(ctx context.Context) context.Context {
 	)
 }
 
+var AsRemoveActor = rbac.Subject{
+	ID: "remove-actor",
+}
+
 // As returns a context with the given actor stored in the context.
 // This is used for cases where the actor touching the database is not the
 // actor stored in the context.
 // When you use this function, be sure to add a //nolint comment
 // explaining why it is necessary.
 func As(ctx context.Context, actor rbac.Subject) context.Context {
+	if actor.Equal(AsRemoveActor) {
+		// AsRemoveActor is a special case that is used to indicate that the actor
+		// should be removed from the context.
+		return context.WithValue(ctx, authContextKey{}, nil)
+	}
 	return context.WithValue(ctx, authContextKey{}, actor)
 }
 
diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go
@@ -18,6 +18,36 @@ import (
 	"github.com/coder/coder/coderd/rbac"
 )
 
+func TestAsNoActor(t *testing.T) {
+	t.Parallel()
+
+	t.Run("AsRemoveActor", func(t *testing.T) {
+		t.Parallel()
+		_, ok := dbauthz.ActorFromContext(context.Background())
+		require.False(t, ok, "no actor should be present")
+	})
+
+	t.Run("AsActor", func(t *testing.T) {
+		t.Parallel()
+		ctx := dbauthz.As(context.Background(), coderdtest.RandomRBACSubject())
+		_, ok := dbauthz.ActorFromContext(ctx)
+		require.True(t, ok, "actor present")
+	})
+
+	t.Run("DeleteActor", func(t *testing.T) {
+		t.Parallel()
+		// First set an actor
+		ctx := dbauthz.As(context.Background(), coderdtest.RandomRBACSubject())
+		_, ok := dbauthz.ActorFromContext(ctx)
+		require.True(t, ok, "actor present")
+
+		// Delete the actor
+		ctx = dbauthz.As(ctx, dbauthz.AsRemoveActor)
+		_, ok = dbauthz.ActorFromContext(ctx)
+		require.False(t, ok, "actor should be deleted")
+	})
+}
+
 func TestPing(t *testing.T) {
 	t.Parallel()
 
diff --git a/coderd/database/dbauthz/setup_test.go b/coderd/database/dbauthz/setup_test.go
@@ -190,7 +190,7 @@ func (s *MethodTestSuite) Subtest(testCaseF func(db database.Store, check *expec
 }
 
 func (s *MethodTestSuite) NoActorErrorTest(callMethod func(ctx context.Context) ([]reflect.Value, error)) {
-	s.Run("NoActor", func() {
+	s.Run("AsRemoveActor", func() {
 		// Call without any actor
 		_, err := callMethod(context.Background())
 		s.ErrorIs(err, dbauthz.NoActorError, "method should return NoActorError error when no actor is provided")
diff --git a/coderd/httpmw/authz.go b/coderd/httpmw/authz.go
@@ -18,7 +18,11 @@ func AsAuthzSystem(mws ...func(http.Handler) http.Handler) func(http.Handler) ht
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
 			ctx := r.Context()
-			before, _ := dbauthz.ActorFromContext(r.Context())
+			before, beforeExists := dbauthz.ActorFromContext(r.Context())
+			if !beforeExists {
+				// AsRemoveActor will actually remove the actor from the context.
+				before = dbauthz.AsRemoveActor
+			}
 
 			r = r.WithContext(dbauthz.AsSystem(ctx))
 			chain.Handler(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
diff --git a/coderd/httpmw/authz_test.go b/coderd/httpmw/authz_test.go
@@ -6,17 +6,15 @@ import (
 	"testing"
 
 	"github.com/go-chi/chi/v5"
+	"github.com/stretchr/testify/assert"
 
-	"github.com/coder/coder/coderd/httpmw"
-
+	"github.com/coder/coder/coderd/coderdtest"
 	"github.com/coder/coder/coderd/database/dbauthz"
-	"github.com/coder/coder/coderd/rbac"
-	"github.com/google/uuid"
-	"github.com/stretchr/testify/assert"
+	"github.com/coder/coder/coderd/httpmw"
 )
 
 func TestAsAuthzSystem(t *testing.T) {
-	userActor := rbac.Subject{ID: uuid.NewString()}
+	userActor := coderdtest.RandomRBACSubject()
 
 	base := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
 		actor, ok := dbauthz.ActorFromContext(r.Context())
@@ -59,6 +57,13 @@ func TestAsAuthzSystem(t *testing.T) {
 		r.Use(
 			// First assert there is no actor context
 			mwAssertNoUser,
+			httpmw.AsAuthzSystem(
+				// Assert the system actor
+				mwAssertSystem,
+				mwAssertSystem,
+			),
+			mwAssertNoUser,
+			// ----
 			// Set to the user actor
 			mwSetUser,
 			// Assert the user actor

Original file line number	Diff line number	Diff line change
`@@ -190,7 +190,7 @@ func (s MethodTestSuite) Subtest(testCaseF func(db database.Store, check expec`
`190`	`190`	`}`
`191`	`191`
`192`	`192`	`func (s *MethodTestSuite) NoActorErrorTest(callMethod func(ctx context.Context) ([]reflect.Value, error)) {`
`193`		`- s.Run("NoActor", func() {`
	`193`	`+ s.Run("AsRemoveActor", func() {`
`194`	`194`	`// Call without any actor`
`195`	`195`	`_, err := callMethod(context.Background())`
`196`	`196`	`s.ErrorIs(err, dbauthz.NoActorError, "method should return NoActorError error when no actor is provided")`