diff --git a/pkg/rbac/rule.go b/pkg/rbac/rule.go index 3c5585ca1..134b70cfa 100644 --- a/pkg/rbac/rule.go +++ b/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 } diff --git a/pkg/rbac/ruleset_checks.go b/pkg/rbac/ruleset_checks.go index 6d0034134..a2e840f1b 100644 --- a/pkg/rbac/ruleset_checks.go +++ b/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 diff --git a/pkg/rbac/ruleset_checks_test.go b/pkg/rbac/ruleset_checks_test.go index 876ecaff3..5f24378ff 100644 --- a/pkg/rbac/ruleset_checks_test.go +++ b/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}, + }, + }, } )