allow only one admin group and make roles in root namespace apply to …

…all namespaces

pkg/auth/auth.go

@@ -12,16 +12,16 @@ import (
 	"github.com/verifa/horizon/pkg/hz"
 )
 
-func WithAdminGroups(groups ...string) Option {
+func WithAdminGroups(group string) Option {
 	return func(o *authorizerOptions) {
-		o.adminGroups = append(o.adminGroups, groups...)
+		o.adminGroup = group
 	}
 }
 
 type Option func(*authorizerOptions)
 
 type authorizerOptions struct {
-	adminGroups []string
+	adminGroup string
 }
 
 var defaultAuthorizerOptions = authorizerOptions{}
@@ -98,7 +98,7 @@ func (a *Auth) Start(
 		RoleBindings: make(map[string]RoleBinding),
 		Roles:        make(map[string]Role),
 		Permissions:  make(map[string]*Group),
-		AdminGroups:  ao.adminGroups,
+		AdminGroup:   ao.adminGroup,
 	}
 	if err := rbac.Start(ctx); err != nil {
 		return fmt.Errorf("starting rbac: %w", err)

pkg/auth/rbac.go

@@ -19,12 +19,12 @@ type RBAC struct {
 	Conn *nats.Conn
 	// TODO: RoleBindings and Roles maps are not thread safe.
 	// E.g. HandleRoleEvent and refresh both write and read from Roles.
-	RoleBindings map[string]RoleBinding `json:"roleBindings,omitempty"`
-	Roles        map[string]Role        `json:"roles,omitempty"`
+	RoleBindings map[string]RoleBinding
+	Roles        map[string]Role
 
-	Permissions map[string]*Group `json:"permissions,omitempty"`
+	Permissions map[string]*Group
 
-	AdminGroups []string `json:"adminGroups,omitempty"`
+	AdminGroup string
 
 	// init is true if the RBAC has been initialised.
 	// RBAC has been initialised if all watchers have been started and
@@ -129,6 +129,20 @@ type Permissions struct {
 	Deny  []Rule `json:"deny"`
 }
 
+func (p *Permissions) AllowRules() []Rule {
+	if p == nil {
+		return nil
+	}
+	return p.Allow
+}
+
+func (p *Permissions) DenyRules() []Rule {
+	if p == nil {
+		return nil
+	}
+	return p.Deny
+}
+
 type Verb string
 
 const (
@@ -159,6 +173,10 @@ type RequestSubject struct {
 }
 
 func (r *RBAC) Check(ctx context.Context, req Request) bool {
+	// Members of the admin group is allowed to do anything.
+	if slices.Contains(req.Subject.Groups, r.AdminGroup) {
+		return true
+	}
 	r.mx.RLock()
 	defer r.mx.RUnlock()
 
@@ -169,34 +187,34 @@ func (r *RBAC) Check(ctx context.Context, req Request) bool {
 		if !ok {
 			continue
 		}
-		wildcardNS, wildcardOK := group.Namespaces["*"]
+		rootNS, wildcardOK := group.Namespaces[hz.NamespaceRoot]
 		ns, nsOK := group.Namespaces[req.Object.ObjectNamespace()]
-		if !wildcardOK && !nsOK {
+		// Exit early if there are no permissions for either.
+		if !(wildcardOK || nsOK) {
 			continue
 		}
 
+		lenAllow := len(rootNS.AllowRules()) + len(ns.AllowRules())
+		lenDeny := len(rootNS.DenyRules()) + len(ns.DenyRules())
+
 		// Merge wildcard and namespace permissions.
-		if ns == nil {
-			ns = &Permissions{
-				Allow: []Rule{},
-				Deny:  []Rule{},
-			}
-		}
-		if wildcardNS != nil {
-			ns.Allow = append(ns.Allow, wildcardNS.Allow...)
-			ns.Deny = append(ns.Deny, wildcardNS.Deny...)
+		perm := &Permissions{
+			Allow: make([]Rule, 0, lenAllow),
+			Deny:  make([]Rule, 0, lenDeny),
 		}
+		perm.Allow = append(rootNS.AllowRules(), ns.AllowRules()...)
+		perm.Deny = append(rootNS.DenyRules(), ns.DenyRules()...)
 
 		if !isAllow {
-			for _, allow := range ns.Allow {
+			for _, allow := range perm.Allow {
 				isAllow = checkVerb(allow, req.Verb, req.Object)
 				if isAllow {
 					break
 				}
 			}
 		}
 		if !isDeny {
-			for _, deny := range ns.Deny {
+			for _, deny := range perm.Deny {
 				isDeny = checkVerb(deny, req.Verb, req.Object)
 				if isDeny {
 					break
@@ -390,32 +408,6 @@ func (r *RBAC) refresh() {
 		}
 	}
 
-	// Add admin group permissions (if any).
-	for _, adminGroup := range r.AdminGroups {
-		group, ok := cache[adminGroup]
-		if !ok {
-			group = &Group{
-				Name:       adminGroup,
-				Namespaces: make(map[string]*Permissions),
-			}
-			cache[adminGroup] = group
-		}
-		ns, ok := group.Namespaces["*"]
-		if !ok {
-			ns = &Permissions{
-				Allow: []Rule{},
-				Deny:  []Rule{},
-			}
-			group.Namespaces["*"] = ns
-		}
-		ns.Allow = append(ns.Allow, Rule{
-			Group: hz.P("*"),
-			Kind:  hz.P("*"),
-			Name:  hz.P("*"),
-			Verbs: []Verb{VerbAll},
-		})
-	}
-
 	r.mx.Lock()
 	defer r.mx.Unlock()
 	r.Permissions = cache

pkg/auth/rbac_test.go

@@ -17,10 +17,10 @@ func TestRBAC(t *testing.T) {
 	}
 
 	type test struct {
-		name        string
-		adminGroups []string
-		roles       []Role
-		bindings    []RoleBinding
+		name       string
+		adminGroup string
+		roles      []Role
+		bindings   []RoleBinding
 
 		cases []testcase
 	}
@@ -342,8 +342,8 @@ func TestRBAC(t *testing.T) {
 	}
 
 	testAdminGroup := test{
-		name:        "admin-group",
-		adminGroups: []string{"admin"},
+		name:       "admin-group",
+		adminGroup: "admin",
 		cases: []testcase{
 			{
 				req: Request{
@@ -363,19 +363,94 @@ func TestRBAC(t *testing.T) {
 		},
 	}
 
+	testAllowRootNamespace := test{
+		name: "allow-root-namespace",
+		roles: []Role{
+			{
+				ObjectMeta: hz.ObjectMeta{
+					Name:      "role-allow-root-namespace",
+					Namespace: hz.NamespaceRoot,
+				},
+				Spec: RoleSpec{
+					Allow: []Rule{
+						{
+							Group: hz.P("*"),
+							Kind:  hz.P("*"),
+							Verbs: []Verb{VerbAll},
+						},
+					},
+				},
+			},
+		},
+		bindings: []RoleBinding{
+			{
+				ObjectMeta: hz.ObjectMeta{
+					Name:      "rolebinding-allow-root-namespace",
+					Namespace: hz.NamespaceRoot,
+				},
+				Spec: RoleBindingSpec{
+					RoleRef: RoleRef{
+						Group: "core",
+						Kind:  "Role",
+						Name:  "role-allow-root-namespace",
+					},
+					Subjects: []Subject{
+						{
+							Kind: "Group",
+							Name: "group-allow-root-namespace",
+						},
+					},
+				},
+			},
+		},
+		cases: []testcase{
+			{
+				req: Request{
+					Subject: RequestSubject{
+						Groups: []string{"group-allow-root-namespace"},
+					},
+					Verb: "read",
+					Object: hz.ObjectKey{
+						Group:     "group-test",
+						Kind:      "object-test",
+						Namespace: hz.NamespaceRoot,
+						Name:      "superfluous",
+					},
+				},
+				expect: true,
+			},
+			{
+				req: Request{
+					Subject: RequestSubject{
+						Groups: []string{"group-allow-root-namespace"},
+					},
+					Verb: "read",
+					Object: hz.ObjectKey{
+						Group:     "group-test",
+						Kind:      "object-test",
+						Namespace: "any-other-namespace",
+						Name:      "superfluous",
+					},
+				},
+				expect: true,
+			},
+		},
+	}
+
 	tests := []test{
 		testAdminGroup,
 		testCreateAllowsRead,
 		testAllowRun,
 		testDenyDelete,
+		testAllowRootNamespace,
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			rbac := RBAC{
 				RoleBindings: make(map[string]RoleBinding),
 				Roles:        make(map[string]Role),
 				Permissions:  make(map[string]*Group),
-				AdminGroups:  test.adminGroups,
+				AdminGroup:   test.adminGroup,
 			}
 			for _, role := range test.roles {
 				_, err := rbac.HandleRoleEvent(event(t, role))