Fix unstable RBAC check

pkg/rbac/rule.go

@@ -2,7 +2,6 @@ package rbac
 
 import (
 	"fmt"
-	"sort"
 )
 
 type (
@@ -40,13 +39,6 @@ func indexRules(rules []*Rule) OptRuleSet {
 		i[r.Operation][r.RoleID] = append(i[r.Operation][r.RoleID], r)
 	}
 
-	// sort rules
-	for op := range i {
-		for roleID := range i[op] {
-			sort.Sort(i[op][roleID])
-		}
-	}
-
 	return i
 }
 

pkg/rbac/ruleset_checks.go

@@ -1,5 +1,9 @@
 package rbac
 
+import (
+	"sort"
+)
+
 func check(indexedRules OptRuleSet, rolesByKind partRoles, op, res string) Access {
 	if member(rolesByKind, AnonymousRole) && len(rolesByKind) > 1 {
 		// Integrity check; when user is member of anonymous role
@@ -17,7 +21,7 @@ func check(indexedRules OptRuleSet, rolesByKind partRoles, op, res string) Acces
 		return Inherit
 	}
 
-	var rules []*Rule
+	var rules RuleSet
 
 	// Priority is important here. We want to have
 	// stable RBAC check behaviour and ability
@@ -53,9 +57,11 @@ func check(indexedRules OptRuleSet, rolesByKind partRoles, op, res string) Acces
 }
 
 // Check given resource match and operation on all given rules
-//
-// Function expects rules, sorted by level!
-func checkRulesByResource(set []*Rule, op, res string) Access {
+func checkRulesByResource(set RuleSet, op, res string) Access {
+	// Make sure rules are always sorted (by level)
+	// to avoid any kind of unstable behaviour
+	sort.Sort(set)
+
 	for _, r := range set {
 		if !matchResource(r.Resource, res) {
 			continue

pkg/rbac/ruleset_checks_test.go

@@ -56,6 +56,22 @@ func Test_check(t *testing.T) {
 					{RoleID: 2, Access: Deny},
 				},
 			},
+			{
+				"complex inheritance",
+				Deny,
+				"test::test:test/1/2/3",
+				"",
+				[]*Role{
+					{id: 1, kind: CommonRole},
+					{id: 2, kind: CommonRole},
+				},
+				[]*Rule{
+					{RoleID: 1, Operation: "", Resource: "test::test:test/1/*/*", Access: Allow},
+					{RoleID: 2, Operation: "", Resource: "test::test:test/*/*/3", Access: Allow},
+					{RoleID: 2, Operation: "", Resource: "test::test:test/1/2/3", Access: Deny},
+					{RoleID: 1, Operation: "", Resource: "test::test:test/*/2/3", Access: Allow},
+				},
+			},
 		}
 	)