RHINENG-11257: fix attributeFilter parsing for string value

base/rbac/rbac.go

@@ -1,5 +1,9 @@
 package rbac
 
+import (
+	"encoding/json"
+)
+
 type AccessPagination struct {
 	Data []Access `json:"data"`
 }
@@ -13,9 +17,12 @@ type ResourceDefinition struct {
 	AttributeFilter AttributeFilter `json:"attributeFilter,omitempty"`
 }
 
+type AttributeFilterValue []*string
+
 type AttributeFilter struct {
-	Key   string    `json:"key"`
-	Value []*string `json:"value"`
+	Key       string               `json:"key"`
+	Value     AttributeFilterValue `json:"value"`
+	Operation string               `json:"operation"`
 }
 
 type inventoryGroup struct {
@@ -24,3 +31,33 @@ type inventoryGroup struct {
 }
 
 type InventoryGroup []inventoryGroup
+
+func (a *AttributeFilterValue) UnmarshalJSON(data []byte) error {
+	var (
+		array []*string
+		value *string
+		err   error
+	)
+
+	if err = json.Unmarshal(data, &array); err != nil {
+		// parsing of AttributeFilter Value into []*string failed
+		// try to parse it as *string
+		if err = json.Unmarshal(data, &value); err != nil {
+			// fail, the value is neither []*string nor *string
+			return err
+		}
+		if value != nil {
+			// according to RBAC team, value is a single string value
+			// not comma delimited strings, multiple values are always in array
+			array = append(array, value)
+		}
+	}
+	if array == nil && value == nil {
+		// in this case we got `"value": null`
+		// we should apply the permission to systems with no inventory groups
+		array = append(array, value)
+	}
+
+	*a = array
+	return nil
+}

base/rbac/rbac_test.go

@@ -0,0 +1,73 @@
+package rbac
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+var data = []byte(`
+{
+	"resourceDefinitions": [
+		{"attributeFilter": {
+			"key": "single_string",
+			"operation": "equal",
+			"value": "string"
+		}},
+		{"attributeFilter": {
+			"key": "comma_separated",
+			"operation": "equal",
+			"value": "comma,separated"
+		}},
+		{"attributeFilter": {
+			"key": "null",
+			"operation": "equal",
+			"value": null
+		}},
+		{"attributeFilter": {
+			"key": "string_array",
+			"operation": "in",
+			"value": ["string", "array"]
+		}},
+		{"attributeFilter": {
+			"key": "string_array_with_null",
+			"operation": "in",
+			"value": ["string", "array", null]
+		}},
+		{"attributeFilter": {
+			"key": "null_array",
+			"operation": "in",
+			"value": [null]
+		}},
+		{"attributeFilter": {
+			"key": "empty_array",
+			"operation": "in",
+			"value": []
+		}}
+	]
+}
+`)
+
+func TestParsing(t *testing.T) {
+	stringS := "string"
+	commaS := "comma,separated"
+	arrayS := "array"
+
+	expected := []ResourceDefinition{
+		{AttributeFilter: AttributeFilter{Operation: "equal", Key: "single_string", Value: []*string{&stringS}}},
+		{AttributeFilter: AttributeFilter{Operation: "equal", Key: "comma_separated", Value: []*string{&commaS}}},
+		{AttributeFilter: AttributeFilter{Operation: "equal", Key: "null", Value: []*string{nil}}},
+		{AttributeFilter: AttributeFilter{Operation: "in", Key: "string_array", Value: []*string{&stringS, &arrayS}}},
+		{AttributeFilter: AttributeFilter{Operation: "in", Key: "string_array_with_null",
+			Value: []*string{&stringS, &arrayS, nil}}},
+		{AttributeFilter: AttributeFilter{Operation: "in", Key: "null_array", Value: []*string{nil}}},
+		{AttributeFilter: AttributeFilter{Operation: "in", Key: "empty_array", Value: []*string{}}},
+	}
+
+	var v Access
+	err := json.Unmarshal(data, &v)
+	if assert.NoError(t, err) {
+		assert.Equal(t, expected, v.ResourceDefinitions)
+	}
+}

manager/middlewares/rbac.go

@@ -123,16 +123,21 @@ func isAccessGranted(c *gin.Context) bool {
 	granted := checkPermissions(&access, handlerName, c.Request.Method)
 	if granted {
 		// collect inventory groups
-		c.Set(utils.KeyInventoryGroups, findInventoryGroups(&access))
+		groups, err := findInventoryGroups(&access)
+		if err != nil {
+			utils.LogError("err", err.Error(), "RBAC")
+			granted = false
+		}
+		c.Set(utils.KeyInventoryGroups, groups)
 	}
 	return granted
 }
 
-func findInventoryGroups(access *rbac.AccessPagination) map[string]string {
+func findInventoryGroups(access *rbac.AccessPagination) (map[string]string, error) {
 	res := make(map[string]string)
 
 	if len(access.Data) == 0 {
-		return res
+		return res, nil
 	}
 	inventoryHostsReadPerms := expandedPermission(inventoryHostsReadPerm)
 	groups := []string{}
@@ -144,12 +149,22 @@ func findInventoryGroups(access *rbac.AccessPagination) map[string]string {
 
 		if len(a.ResourceDefinitions) == 0 {
 			// access to all groups
-			return nil
+			return nil, nil
 		}
 		for _, rd := range a.ResourceDefinitions {
 			if rd.AttributeFilter.Key != "group.id" {
 				continue
 			}
+
+			// https://github.com/RedHatInsights/insights-host-inventory/
+			//     blob/a7c8a7c980012c89e18ec0f7074609e216b37a8d/lib/middleware.py#L124
+			if rd.AttributeFilter.Operation != "in" {
+				err := fmt.Errorf(
+					"invalid value '%s' for attributeFilter.Operation",
+					rd.AttributeFilter.Operation,
+				)
+				return nil, err
+			}
 			for _, v := range rd.AttributeFilter.Value {
 				if v == nil {
 					res[utils.KeyUngrouped] = "[]"
@@ -168,7 +183,7 @@ func findInventoryGroups(access *rbac.AccessPagination) map[string]string {
 	if len(groups) > 0 {
 		res[utils.KeyGrouped] = fmt.Sprintf("{%s}", strings.Join(groups, ","))
 	}
-	return res
+	return res, nil
 }
 
 func RBAC() gin.HandlerFunc {

manager/middlewares/rbac_test.go

@@ -247,20 +247,23 @@ func TestFindInventoryGroupsGrouped(t *testing.T) {
 			Permission: "inventory:hosts:read",
 			ResourceDefinitions: []rbac.ResourceDefinition{{
 				AttributeFilter: rbac.AttributeFilter{
-					Key:   "group.id",
-					Value: []*string{&group1},
+					Key:       "group.id",
+					Value:     []*string{&group1},
+					Operation: "in",
 				},
 			}},
 		}},
 	}
-	groups := findInventoryGroups(access)
-	assert.Equal(t,
-		`{"[{\"id\":\"df57820e-965c-49a6-b0bc-797b7dd60581\"}]"}`,
-		groups[utils.KeyGrouped],
-	)
-	val, ok := groups[utils.KeyUngrouped]
-	assert.Equal(t, "", val)
-	assert.Equal(t, false, ok)
+	groups, err := findInventoryGroups(access)
+	if assert.NoError(t, err) {
+		assert.Equal(t,
+			`{"[{\"id\":\"df57820e-965c-49a6-b0bc-797b7dd60581\"}]"}`,
+			groups[utils.KeyGrouped],
+		)
+		val, ok := groups[utils.KeyUngrouped]
+		assert.Equal(t, "", val)
+		assert.Equal(t, false, ok)
+	}
 }
 
 func TestFindInventoryGroupsUnrouped(t *testing.T) {
@@ -269,17 +272,20 @@ func TestFindInventoryGroupsUnrouped(t *testing.T) {
 			Permission: "inventory:hosts:read",
 			ResourceDefinitions: []rbac.ResourceDefinition{{
 				AttributeFilter: rbac.AttributeFilter{
-					Key:   "group.id",
-					Value: []*string{nil},
+					Key:       "group.id",
+					Value:     []*string{nil},
+					Operation: "in",
 				},
 			}},
 		}},
 	}
-	groups := findInventoryGroups(access)
-	val, ok := groups[utils.KeyGrouped]
-	assert.Equal(t, "", val)
-	assert.Equal(t, false, ok)
-	assert.Equal(t, "[]", groups[utils.KeyUngrouped])
+	groups, err := findInventoryGroups(access)
+	if assert.NoError(t, err) {
+		val, ok := groups[utils.KeyGrouped]
+		assert.Equal(t, "", val)
+		assert.Equal(t, false, ok)
+		assert.Equal(t, "[]", groups[utils.KeyUngrouped])
+	}
 }
 
 func TestFindInventoryGroups(t *testing.T) {
@@ -288,18 +294,21 @@ func TestFindInventoryGroups(t *testing.T) {
 			Permission: "inventory:hosts:read",
 			ResourceDefinitions: []rbac.ResourceDefinition{{
 				AttributeFilter: rbac.AttributeFilter{
-					Key:   "group.id",
-					Value: []*string{&group1, &group2, nil},
+					Key:       "group.id",
+					Value:     []*string{&group1, &group2, nil},
+					Operation: "in",
 				},
 			}},
 		}},
 	}
-	groups := findInventoryGroups(access)
-	assert.Equal(t,
-		`{"[{\"id\":\"df57820e-965c-49a6-b0bc-797b7dd60581\"}]","[{\"id\":\"df3f0efd-c853-41b5-80a1-86881d5343d1\"}]"}`,
-		groups[utils.KeyGrouped],
-	)
-	assert.Equal(t, "[]", groups[utils.KeyUngrouped])
+	groups, err := findInventoryGroups(access)
+	if assert.NoError(t, err) {
+		assert.Equal(t,
+			`{"[{\"id\":\"df57820e-965c-49a6-b0bc-797b7dd60581\"}]","[{\"id\":\"df3f0efd-c853-41b5-80a1-86881d5343d1\"}]"}`,
+			groups[utils.KeyGrouped],
+		)
+		assert.Equal(t, "[]", groups[utils.KeyUngrouped])
+	}
 }
 
 func TestFindInventoryGroupsOverwrite(t *testing.T) {
@@ -309,8 +318,9 @@ func TestFindInventoryGroupsOverwrite(t *testing.T) {
 				Permission: "inventory:hosts:read",
 				ResourceDefinitions: []rbac.ResourceDefinition{{
 					AttributeFilter: rbac.AttributeFilter{
-						Key:   "group.id",
-						Value: []*string{&group1, nil},
+						Key:       "group.id",
+						Value:     []*string{&group1, nil},
+						Operation: "in",
 					},
 				}},
 			},
@@ -320,9 +330,11 @@ func TestFindInventoryGroupsOverwrite(t *testing.T) {
 			},
 		},
 	}
-	groups := findInventoryGroups(access)
-	// we expect access to all groups (empty map)
-	assert.Equal(t, 0, len(groups))
+	groups, err := findInventoryGroups(access)
+	if assert.NoError(t, err) {
+		// we expect access to all groups (empty map)
+		assert.Equal(t, 0, len(groups))
+	}
 }
 
 func TestFindInventoryGroupsOverwrite2(t *testing.T) {
@@ -336,16 +348,39 @@ func TestFindInventoryGroupsOverwrite2(t *testing.T) {
 				Permission: "inventory:hosts:read",
 				ResourceDefinitions: []rbac.ResourceDefinition{{
 					AttributeFilter: rbac.AttributeFilter{
-						Key:   "group.id",
-						Value: []*string{&group1, nil},
+						Key:       "group.id",
+						Value:     []*string{&group1, nil},
+						Operation: "in",
+					},
+				}},
+			},
+		},
+	}
+	groups, err := findInventoryGroups(access)
+	if assert.NoError(t, err) {
+		// we expect access to all groups (empty map)
+		assert.Equal(t, 0, len(groups))
+	}
+}
+
+func TestFindInventoryGroupsInvalidOp(t *testing.T) {
+	access := &rbac.AccessPagination{
+		Data: []rbac.Access{
+			{
+				Permission: "inventory:hosts:read",
+				ResourceDefinitions: []rbac.ResourceDefinition{{
+					AttributeFilter: rbac.AttributeFilter{
+						Key:       "group.id",
+						Value:     []*string{},
+						Operation: "equal",
 					},
 				}},
 			},
 		},
 	}
-	groups := findInventoryGroups(access)
-	// we expect access to all groups (empty map)
-	assert.Equal(t, 0, len(groups))
+	groups, err := findInventoryGroups(access)
+	assert.Error(t, err)
+	assert.Nil(t, groups)
 }
 
 func TestMultiplePermissions(t *testing.T) {

platform/rbac.go

@@ -20,8 +20,9 @@ func rbacHandler(c *gin.Context) {
 				Permission: "inventory:hosts:read",
 				ResourceDefinitions: []rbac.ResourceDefinition{{
 					AttributeFilter: rbac.AttributeFilter{
-						Key:   "group.id",
-						Value: []*string{&inventoryGroup, nil},
+						Key:       "group.id",
+						Operation: "in",
+						Value:     []*string{&inventoryGroup, nil},
 					},
 				}},
 			},