refactor roles to use a verb list, this is much more ergonomic

pkg/auth/rbac.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"log/slog"
+	"slices"
 	"strings"
 	"sync"
 	"time"
@@ -124,8 +125,8 @@ type Group struct {
 }
 
 type Permissions struct {
-	Allow []Verbs `json:"allow"`
-	Deny  []Verbs `json:"deny"`
+	Allow []Rule `json:"allow"`
+	Deny  []Rule `json:"deny"`
 }
 
 type Verb string
@@ -146,6 +147,7 @@ const (
 	VerbDelete Verb = "delete"
 	// VerbRun allows a user to run actions for an actor.
 	VerbRun Verb = "run"
+	VerbAll Verb = "*"
 )
 
 type RBACRequest struct {
@@ -174,8 +176,8 @@ func (r *RBAC) Check(ctx context.Context, req RBACRequest) bool {
 		// Merge wildcard and namespace permissions.
 		if ns == nil {
 			ns = &Permissions{
-				Allow: []Verbs{},
-				Deny:  []Verbs{},
+				Allow: []Rule{},
+				Deny:  []Rule{},
 			}
 		}
 		if wildcardNS != nil {
@@ -185,50 +187,15 @@ func (r *RBAC) Check(ctx context.Context, req RBACRequest) bool {
 
 		if !isAllow {
 			for _, allow := range ns.Allow {
-				switch req.Verb {
-				case VerbRead:
-					isAllow = checkVerbFilter(allow.Read, req.Object) ||
-						checkVerbFilter(allow.Update, req.Object) ||
-						checkVerbFilter(allow.Create, req.Object) ||
-						checkVerbFilter(allow.Delete, req.Object)
-
-				case VerbUpdate:
-					isAllow = checkVerbFilter(allow.Update, req.Object)
-				case VerbCreate:
-					isAllow = checkVerbFilter(allow.Create, req.Object)
-				case VerbDelete:
-					isAllow = checkVerbFilter(allow.Delete, req.Object)
-				case VerbRun:
-					isAllow = checkVerbFilter(allow.Run, req.Object)
-				default:
-					// Unknown verb.
-					return false
-				}
+				isAllow = checkVerb(allow, req.Verb, req.Object)
 				if isAllow {
 					break
 				}
 			}
 		}
 		if !isDeny {
 			for _, deny := range ns.Deny {
-				switch req.Verb {
-				case VerbRead:
-					isDeny = checkVerbFilter(deny.Read, req.Object)
-				case VerbUpdate:
-					isDeny = checkVerbFilter(deny.Read, req.Object) ||
-						checkVerbFilter(deny.Update, req.Object)
-				case VerbCreate:
-					isDeny = checkVerbFilter(deny.Read, req.Object) ||
-						checkVerbFilter(deny.Create, req.Object)
-				case VerbDelete:
-					isDeny = checkVerbFilter(deny.Read, req.Object) ||
-						checkVerbFilter(deny.Delete, req.Object)
-				case VerbRun:
-					isDeny = checkVerbFilter(deny.Run, req.Object)
-				default:
-					// Unknown verb.
-					return false
-				}
+				isDeny = checkVerb(deny, req.Verb, req.Object)
 				if isDeny {
 					break
 				}
@@ -238,20 +205,22 @@ func (r *RBAC) Check(ctx context.Context, req RBACRequest) bool {
 	return isAllow && !isDeny
 }
 
-func checkVerbFilter(vf *VerbFilter, obj hz.ObjectKeyer) bool {
-	if vf == nil {
-		return false
+func checkVerb(rule Rule, verb Verb, obj hz.ObjectKeyer) bool {
+	// If the rule does not specify the wildcard ("*") verb, check if the verb is allowed.
+	if !slices.Contains(rule.Verbs, VerbAll) {
+		if !slices.Contains(rule.Verbs, verb) {
+			return false
+		}
 	}
-	if !checkStringPattern(vf.Group, obj.ObjectGroup()) {
+	if !checkStringPattern(rule.Group, obj.ObjectGroup()) {
 		return false
 	}
-	if !checkStringPattern(vf.Kind, obj.ObjectKind()) {
+	if !checkStringPattern(rule.Kind, obj.ObjectKind()) {
 		return false
 	}
-	if !checkStringPattern(vf.Name, obj.ObjectName()) {
+	if !checkStringPattern(rule.Name, obj.ObjectName()) {
 		return false
 	}
-
 	return true
 }
 
@@ -355,8 +324,8 @@ func (r *RBAC) refresh() {
 			permissions, ok := group.Namespaces[roleBinding.Namespace]
 			if !ok {
 				permissions = &Permissions{
-					Allow: []Verbs{},
-					Deny:  []Verbs{},
+					Allow: []Rule{},
+					Deny:  []Rule{},
 				}
 				group.Namespaces[roleBinding.Namespace] = permissions
 			}
@@ -403,18 +372,16 @@ func (r *RBAC) refresh() {
 				rootNS, ok := group.Namespaces[hz.RootNamespace]
 				if !ok {
 					rootNS = &Permissions{
-						Allow: []Verbs{},
-						Deny:  []Verbs{},
+						Allow: []Rule{},
+						Deny:  []Rule{},
 					}
 					group.Namespaces[hz.RootNamespace] = rootNS
 				}
 
-				rootNS.Allow = append(rootNS.Allow, Verbs{
-					Read: &VerbFilter{
-						Name: &localNS,
-						Kind: hz.P(core.ObjectKindNamespace),
-						// Group: hz.P("TODO"),
-					},
+				rootNS.Allow = append(rootNS.Allow, Rule{
+					Name:  &localNS,
+					Kind:  hz.P(core.ObjectKindNamespace),
+					Verbs: []Verb{VerbRead},
 				})
 			}
 		}
@@ -433,17 +400,16 @@ func (r *RBAC) refresh() {
 		ns, ok := group.Namespaces["*"]
 		if !ok {
 			ns = &Permissions{
-				Allow: []Verbs{},
-				Deny:  []Verbs{},
+				Allow: []Rule{},
+				Deny:  []Rule{},
 			}
 			group.Namespaces["*"] = ns
 		}
-		ns.Allow = append(ns.Allow, Verbs{
-			Read:   &VerbFilter{},
-			Update: &VerbFilter{},
-			Create: &VerbFilter{},
-			Delete: &VerbFilter{},
-			Run:    &VerbFilter{},
+		ns.Allow = append(ns.Allow, Rule{
+			Group: hz.P("*"),
+			Kind:  hz.P("*"),
+			Name:  hz.P("*"),
+			Verbs: []Verb{VerbAll},
 		})
 	}
 

pkg/auth/rbac_server_test.go

@@ -12,7 +12,7 @@ import (
 	"github.com/verifa/horizon/pkg/testutil"
 )
 
-func TestRBACServer(t *testing.T) {
+func TestServerRBAC(t *testing.T) {
 	ctx := context.Background()
 	ts := server.Test(t, ctx)
 
@@ -45,13 +45,12 @@ func TestRBACServer(t *testing.T) {
 			Namespace: "team-a",
 		},
 		Spec: auth.RoleSpec{
-			Allow: []auth.Verbs{
+			Allow: []auth.Rule{
 				{
-					Create: &auth.VerbFilter{
-						Group: hz.P("*"),
-						Kind:  hz.P("*"),
-						Name:  hz.P("*"),
-					},
+					Group: hz.P("*"),
+					Kind:  hz.P("*"),
+					Name:  hz.P("*"),
+					Verbs: []auth.Verb{auth.VerbCreate},
 				},
 			},
 		},
@@ -89,13 +88,12 @@ func TestRBACServer(t *testing.T) {
 			Namespace: "team-b",
 		},
 		Spec: auth.RoleSpec{
-			Allow: []auth.Verbs{
+			Allow: []auth.Rule{
 				{
-					Create: &auth.VerbFilter{
-						Group: hz.P("*"),
-						Kind:  hz.P("*"),
-						Name:  hz.P("*"),
-					},
+					Group: hz.P("*"),
+					Kind:  hz.P("*"),
+					Name:  hz.P("*"),
+					Verbs: []auth.Verb{auth.VerbCreate},
 				},
 			},
 		},

pkg/auth/rbac_test.go

@@ -34,12 +34,11 @@ func TestRBAC(t *testing.T) {
 					Namespace: "namespace-test",
 				},
 				Spec: RoleSpec{
-					Allow: []Verbs{
+					Allow: []Rule{
 						{
-							Create: &VerbFilter{
-								Kind:  hz.P("object-test"),
-								Group: hz.P("group-test"),
-							},
+							Kind:  hz.P("object-test"),
+							Group: hz.P("group-test"),
+							Verbs: []Verb{VerbRead, VerbCreate},
 						},
 					},
 				},
@@ -157,12 +156,11 @@ func TestRBAC(t *testing.T) {
 					Namespace: "namespace-test",
 				},
 				Spec: RoleSpec{
-					Allow: []Verbs{
+					Allow: []Rule{
 						{
-							Run: &VerbFilter{
-								Group: hz.P("group-test"),
-								Kind:  hz.P("object-test"),
-							},
+							Group: hz.P("group-test"),
+							Kind:  hz.P("object-test"),
+							Verbs: []Verb{VerbRun},
 						},
 					},
 				},
@@ -214,13 +212,12 @@ func TestRBAC(t *testing.T) {
 					Namespace: "namespace-test",
 				},
 				Spec: RoleSpec{
-					Allow: []Verbs{
+					Allow: []Rule{
 						{
-							Read:   &VerbFilter{},
-							Update: &VerbFilter{},
-							Create: &VerbFilter{},
-							Delete: &VerbFilter{},
-							Run:    &VerbFilter{},
+							Group: hz.P("*"),
+							Name:  hz.P("*"),
+							Kind:  hz.P("*"),
+							Verbs: []Verb{VerbAll},
 						},
 					},
 				},
@@ -231,9 +228,12 @@ func TestRBAC(t *testing.T) {
 					Namespace: "namespace-test",
 				},
 				Spec: RoleSpec{
-					Deny: []Verbs{
+					Deny: []Rule{
 						{
-							Delete: &VerbFilter{},
+							Group: hz.P("*"),
+							Name:  hz.P("*"),
+							Kind:  hz.P("*"),
+							Verbs: []Verb{VerbDelete},
 						},
 					},
 				},
@@ -368,7 +368,7 @@ func TestRBAC(t *testing.T) {
 			for index, tc := range test.cases {
 				ok := rbac.Check(ctx, tc.req)
 				if ok != tc.expect {
-					t.Fatal("test case failed: ", index)
+					t.Fatal("test case failed: ", index, tc.req)
 				}
 			}
 		})

pkg/auth/role.go

@@ -23,20 +23,17 @@ func (Role) ObjectKind() string {
 }
 
 type RoleSpec struct {
-	Allow []Verbs `json:"allow,omitempty"`
-	Deny  []Verbs `json:"deny,omitempty"`
+	Allow []Rule `json:"allow,omitempty"`
+	Deny  []Rule `json:"deny,omitempty"`
 }
 
-type Verbs struct {
-	Read   *VerbFilter `json:"read,omitempty"`
-	Update *VerbFilter `json:"update,omitempty"`
-	Create *VerbFilter `json:"create,omitempty"`
-	Delete *VerbFilter `json:"delete,omitempty"`
-	Run    *VerbFilter `json:"run,omitempty"`
-}
-
-type VerbFilter struct {
-	Name  *string `json:"name,omitempty" cue:""`
-	Kind  *string `json:"kind,omitempty" cue:""`
+type Rule struct {
+	// Name of a resource that this rule targets.
+	Name *string `json:"name,omitempty" cue:""`
+	// Kind of a resource that this rule targets.
+	Kind *string `json:"kind,omitempty" cue:""`
+	// Group of a resource that this rule targets.
 	Group *string `json:"group,omitempty" cue:""`
+	// Verbs that this rule enforces.
+	Verbs []Verb `json:"verbs,omitempty" cue:""`
 }