chore: verify validity of built in rbac roles (#13296)

Verifies our built in roles are valid according to our policy.go. Working on custom roles requires the dynamic roles to adhere to these rules. Feels fair the built in ones do too.

coderd/rbac/object.go

@@ -1,9 +1,8 @@
 package rbac
 
 import (
-	"fmt"
-
 	"github.com/google/uuid"
+	"golang.org/x/xerrors"
 
 	"github.com/coder/coder/v2/coderd/rbac/policy"
 )
@@ -36,10 +35,10 @@ type Object struct {
 func (z Object) ValidAction(action policy.Action) error {
 	perms, ok := policy.RBACPermissions[z.Type]
 	if !ok {
-		return fmt.Errorf("invalid type %q", z.Type)
+		return xerrors.Errorf("invalid type %q", z.Type)
 	}
 	if _, ok := perms.Actions[action]; !ok {
-		return fmt.Errorf("invalid action %q for type %q", action, z.Type)
+		return xerrors.Errorf("invalid action %q for type %q", action, z.Type)
 	}
 
 	return nil

coderd/rbac/roles.go

@@ -1,6 +1,7 @@
 package rbac
 
 import (
+	"errors"
 	"sort"
 	"strings"
 
@@ -369,6 +370,30 @@ type Permission struct {
 	Action       policy.Action `json:"action"`
 }
 
+func (perm Permission) Valid() error {
+	if perm.ResourceType == policy.WildcardSymbol {
+		// Wildcard is tricky to check. Just allow it.
+		return nil
+	}
+
+	resource, ok := policy.RBACPermissions[perm.ResourceType]
+	if !ok {
+		return xerrors.Errorf("invalid resource type %q", perm.ResourceType)
+	}
+
+	// Wildcard action is always valid
+	if perm.Action == policy.WildcardSymbol {
+		return nil
+	}
+
+	_, ok = resource.Actions[perm.Action]
+	if !ok {
+		return xerrors.Errorf("invalid action %q for resource %q", perm.Action, perm.ResourceType)
+	}
+
+	return nil
+}
+
 // Role is a set of permissions at multiple levels:
 // - Site level permissions apply EVERYWHERE
 // - Org level permissions apply to EVERYTHING in a given ORG
@@ -393,6 +418,34 @@ type Role struct {
 	cachedRegoValue ast.Value
 }
 
+// Valid will check all it's permissions and ensure they are all correct
+// according to the policy. This verifies every action specified make sense
+// for the given resource.
+func (role Role) Valid() error {
+	var errs []error
+	for _, perm := range role.Site {
+		if err := perm.Valid(); err != nil {
+			errs = append(errs, xerrors.Errorf("site: %w", err))
+		}
+	}
+
+	for orgID, permissions := range role.Org {
+		for _, perm := range permissions {
+			if err := perm.Valid(); err != nil {
+				errs = append(errs, xerrors.Errorf("org=%q: %w", orgID, err))
+			}
+		}
+	}
+
+	for _, perm := range role.User {
+		if err := perm.Valid(); err != nil {
+			errs = append(errs, xerrors.Errorf("user: %w", err))
+		}
+	}
+
+	return errors.Join(errs...)
+}
+
 type Roles []Role
 
 func (roles Roles) Expand() ([]Role, error) {
@@ -402,7 +455,7 @@ func (roles Roles) Expand() ([]Role, error) {
 func (roles Roles) Names() []string {
 	names := make([]string, 0, len(roles))
 	for _, r := range roles {
-		return append(names, r.Name)
+		names = append(names, r.Name)
 	}
 	return names
 }

coderd/rbac/roles_test.go

@@ -20,6 +20,27 @@ type authSubject struct {
 	Actor rbac.Subject
 }
 
+// TestBuiltInRoles makes sure our built-in roles are valid by our own policy
+// rules. If this is incorrect, that is a mistake.
+func TestBuiltInRoles(t *testing.T) {
+	t.Parallel()
+	for _, r := range rbac.SiteRoles() {
+		r := r
+		t.Run(r.Name, func(t *testing.T) {
+			t.Parallel()
+			require.NoError(t, r.Valid(), "invalid role")
+		})
+	}
+
+	for _, r := range rbac.OrganizationRoles(uuid.New()) {
+		r := r
+		t.Run(r.Name, func(t *testing.T) {
+			t.Parallel()
+			require.NoError(t, r.Valid(), "invalid role")
+		})
+	}
+}
+
 //nolint:tparallel,paralleltest
 func TestOwnerExec(t *testing.T) {
 	owner := rbac.Subject{