From f17076076599c50e84c41e6af09b38ded15defb8 Mon Sep 17 00:00:00 2001
From: Denis Arh <denis.arh@gmail.com>
Date: Thu, 28 Jul 2022 17:35:27 +0200
Subject: [PATCH] Fix RBAC rule filtering

---
 pkg/rbac/rule.go                 | 26 +++++++++++++-------------
 pkg/rbac/rule_test.go            | 14 ++++++++++++++
 tests/system/permissions_test.go | 26 +++++++++++++-------------
 3 files changed, 40 insertions(+), 26 deletions(-)

diff --git a/pkg/rbac/rule.go b/pkg/rbac/rule.go
index c8e28dc43..94f4648fa 100644
--- a/pkg/rbac/rule.go
+++ b/pkg/rbac/rule.go
@@ -60,23 +60,23 @@ func (set RuleSet) FilterAccess(a Access) (out RuleSet) {
 	return out
 }
 
+// FilterResource returns rules that match given list of resources
+// Wildcards are not used!
+//
+// Note that empty resource list will return ALL rules!
 func (set RuleSet) FilterResource(rr ...Resource) (out RuleSet) {
-	var (
-		ruleMap    = make(map[string]bool)
-		uniqRuleID = func(r *Rule) string {
-			return fmt.Sprintf("%s|%s|%d", r.Resource, r.Operation, r.RoleID)
-		}
-	)
+	if len(rr) == 0 {
+		return set
+	}
 
-	for _, res := range rr {
-		for _, rule := range set {
-			if !matchResource(res.RbacResource(), rule.Resource) {
+	out = RuleSet{}
+	for _, rule := range set {
+		for _, res := range rr {
+			if res.RbacResource() != rule.Resource {
 				continue
 			}
-			if _, ok := ruleMap[uniqRuleID(rule)]; !ok {
-				out = append(out, rule)
-				ruleMap[uniqRuleID(rule)] = true
-			}
+
+			out = append(out, rule)
 		}
 	}
 
diff --git a/pkg/rbac/rule_test.go b/pkg/rbac/rule_test.go
index 34a022aab..bce2beee2 100644
--- a/pkg/rbac/rule_test.go
+++ b/pkg/rbac/rule_test.go
@@ -38,3 +38,17 @@ func TestRuleSetSort(t *testing.T) {
 	req.Equal(":::/1/*/*", rr[i()].Resource)
 	req.Equal(":::/*/*/*", rr[i()].Resource)
 }
+
+func TestRuleSet_FilterResource(t *testing.T) {
+	var (
+		req = require.New(t)
+		rr  = RuleSet{
+			{Resource: ":::/*/*/*"},
+			{Resource: ":::/1/2/3"},
+		}
+	)
+
+	req.Len(rr.FilterResource(NewResource(":::/1/2/3")), 1)
+	req.Len(rr.FilterResource(NewResource(":::/*/*/*")), 1)
+	req.Len(rr.FilterResource(NewResource(":::/*")), 0)
+}
diff --git a/tests/system/permissions_test.go b/tests/system/permissions_test.go
index 0c2509159..c2073be6f 100644
--- a/tests/system/permissions_test.go
+++ b/tests/system/permissions_test.go
@@ -69,28 +69,28 @@ func TestPermissionsReadWithFilter(t *testing.T) {
 	helpers.AllowMe(h, types.UserRbacResource(testID), "read")
 	helpers.AllowMe(h, types.UserRbacResource(id.Next()), "update")
 
-	// all rules
+	t.Log("all component-level and wildcard rules")
 	h.apiInit().
 		Getf("/permissions/%d/rules", h.roleID).
 		Header("Accept", "application/json").
 		Expect(t).
 		Status(http.StatusOK).
 		Assert(helpers.AssertNoErrors).
-		Assert(jsonpath.Len(`$.response`, 4)).
-		End()
-
-	// Resource related rules
-	h.apiInit().
-		Getf("/permissions/%d/rules", h.roleID).
-		Query("resource", "corteza::system:user/*").
-		Header("Accept", "application/json").
-		Expect(t).
-		Status(http.StatusOK).
-		Assert(helpers.AssertNoErrors).
 		Assert(jsonpath.Len(`$.response`, 2)).
 		End()
 
-	// Only specific resource rules with `specific: 2` filter
+	t.Log("no rules for all-users")
+	h.apiInit().
+		Getf("/permissions/%d/rules", h.roleID).
+		Query("resource", "corteza::system:user/*").
+		Header("Accept", "application/json").
+		Expect(t).
+		Status(http.StatusOK).
+		Assert(helpers.AssertNoErrors).
+		Assert(jsonpath.Len(`$.response`, 0)).
+		End()
+
+	t.Log("1 rule for specific user")
 	h.apiInit().
 		Getf("/permissions/%d/rules", h.roleID).
 		Query("resource", fmt.Sprintf("corteza::system:user/%d", testID)).