Add unit tests for InTx and Ping

Emyrk · Emyrk · commit 5928c37a72f7 · 2023-02-03T11:06:42.000-06:00
diff --git a/coderd/authzquery/authz_test.go b/coderd/authzquery/authz_test.go
@@ -6,18 +6,17 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/google/uuid"
 	"github.com/stretchr/testify/require"
-
-	"cdr.dev/slog/sloggers/slogtest"
-
 	"golang.org/x/xerrors"
 
-	"github.com/google/uuid"
-
 	"cdr.dev/slog"
+	"cdr.dev/slog/sloggers/slogtest"
 	"github.com/coder/coder/coderd/authzquery"
 	"github.com/coder/coder/coderd/coderdtest"
+	"github.com/coder/coder/coderd/database"
 	"github.com/coder/coder/coderd/database/dbfake"
+	"github.com/coder/coder/coderd/database/dbgen"
 	"github.com/coder/coder/coderd/rbac"
 )
 
@@ -36,6 +35,15 @@ func TestNotAuthorizedError(t *testing.T) {
 		require.ErrorAs(t, err, &authErr, "must be a NotAuthorizedError")
 		require.ErrorIs(t, authErr.Err, testErr, "internal error must match")
 	})
+
+	t.Run("MissingActor", func(t *testing.T) {
+		q := authzquery.NewAuthzQuerier(dbfake.New(), &coderdtest.RecordingAuthorizer{
+			Wrapped: &coderdtest.FakeAuthorizer{AlwaysReturn: nil},
+		}, slog.Make())
+		// This should fail because the actor is missing.
+		_, err := q.GetWorkspaceByID(context.Background(), uuid.New())
+		require.ErrorIs(t, err, authzquery.NoActorError, "must be a NoActorError")
+	})
 }
 
 // TestAuthzQueryRecursive is a simple test to search for infinite recursion
@@ -72,6 +80,40 @@ func TestAuthzQueryRecursive(t *testing.T) {
 	}
 }
 
+func TestPing(t *testing.T) {
+	t.Parallel()
+
+	q := authzquery.NewAuthzQuerier(dbfake.New(), &coderdtest.RecordingAuthorizer{}, slog.Make())
+	_, err := q.Ping(context.Background())
+	require.NoError(t, err, "must not error")
+}
+
+// TestInTX is not perfect, just checks that it properly checks auth.
+func TestInTX(t *testing.T) {
+	t.Parallel()
+
+	db := dbfake.New()
+	q := authzquery.NewAuthzQuerier(db, &coderdtest.RecordingAuthorizer{
+		Wrapped: &coderdtest.FakeAuthorizer{AlwaysReturn: xerrors.New("custom error")},
+	}, slog.Make())
+	actor := rbac.Subject{
+		ID:     uuid.NewString(),
+		Roles:  rbac.RoleNames{rbac.RoleOwner()},
+		Groups: []string{},
+		Scope:  rbac.ScopeAll,
+	}
+
+	w := dbgen.Workspace(t, db, database.Workspace{})
+	ctx := authzquery.WithAuthorizeContext(context.Background(), actor)
+	err := q.InTx(func(tx database.Store) error {
+		// The inner tx should use the parent's authz
+		_, err := tx.GetWorkspaceByID(ctx, w.ID)
+		return err
+	}, nil)
+	require.Error(t, err, "must error")
+	require.ErrorAs(t, err, &authzquery.NotAuthorizedError{}, "must be an authorized error")
+}
+
 func must[T any](value T, err error) T {
 	if err != nil {
 		panic(err)
diff --git a/coderd/authzquery/authzquerier.go b/coderd/authzquery/authzquerier.go
@@ -47,7 +47,7 @@ func (q *AuthzQuerier) InTx(function func(querier database.Store) error, txOpts
 	// TODO: @emyrk verify this works.
 	return q.db.InTx(func(tx database.Store) error {
 		// Wrap the transaction store in an AuthzQuerier.
-		wrapped := NewAuthzQuerier(tx, q.auth, slog.Make())
+		wrapped := NewAuthzQuerier(tx, q.auth, q.log)
 		return function(wrapped)
 	}, txOpts)
 }

Original file line number	Diff line number	Diff line change
`@@ -47,7 +47,7 @@ func (q *AuthzQuerier) InTx(function func(querier database.Store) error, txOpts`
`47`	`47`	`// TODO: @emyrk verify this works.`
`48`	`48`	`return q.db.InTx(func(tx database.Store) error {`
`49`	`49`	`// Wrap the transaction store in an AuthzQuerier.`
`50`		`- wrapped := NewAuthzQuerier(tx, q.auth, slog.Make())`
	`50`	`+ wrapped := NewAuthzQuerier(tx, q.auth, q.log)`
`51`	`51`	`return function(wrapped)`
`52`	`52`	`}, txOpts)`
`53`	`53`	`}`