Skip to content

Commit

Permalink
RHINENG-11257: return error when operation is not 'in' for inventory …
Browse files Browse the repository at this point in the history 
…groups
  • Loading branch information
@psegedy
psegedy committed Jul 11, 2024
1 parent 6471a44 commit 893ec7d
Show file tree
Hide file tree
Showing 5 changed files with 146 additions and 85 deletions.
5 changes: 3 additions & 2 deletions base/rbac/rbac.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ type ResourceDefinition struct {
type AttributeFilterValue []*string

type AttributeFilter struct {
Key string `json:"key"`
Value AttributeFilterValue `json:"value"`
Key string `json:"key"`
Value AttributeFilterValue `json:"value"`
Operation string `json:"operation"`
}

type inventoryGroup struct {
Expand Down
91 changes: 50 additions & 41 deletions base/rbac/rbac_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,53 +7,62 @@ import (
"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) {
data := []byte(`
{
"resourceDefinitions": [
{"attributeFilter": {
"key": "single_string",
"value": "string"
}},
{"attributeFilter": {
"key": "comma_separated",
"value": "comma,separated"
}},
{"attributeFilter": {
"key": "null",
"value": null
}},
{"attributeFilter": {
"key": "string_array",
"value": ["string", "array"]
}},
{"attributeFilter": {
"key": "string_array_with_null",
"value": ["string", "array", null]
}},
{"attributeFilter": {
"key": "null_array",
"value": [null]
}},
{"attributeFilter": {
"key": "empty_array",
"value": []
}}
]
}
`)
stringS := "string"
commaS := "comma,separated"
arrayS := "array"

expected := []ResourceDefinition{
{AttributeFilter: AttributeFilter{Key: "single_string", Value: []*string{&stringS}}},
{AttributeFilter: AttributeFilter{Key: "comma_separated", Value: []*string{&commaS}}},
{AttributeFilter: AttributeFilter{Key: "null", Value: []*string{nil}}},
{AttributeFilter: AttributeFilter{Key: "string_array", Value: []*string{&stringS, &arrayS}}},
{AttributeFilter: AttributeFilter{Key: "string_array_with_null", Value: []*string{&stringS, &arrayS, nil}}},
{AttributeFilter: AttributeFilter{Key: "null_array", Value: []*string{nil}}},
{AttributeFilter: AttributeFilter{Key: "empty_array", Value: []*string{}}},
{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
Expand Down
25 changes: 20 additions & 5 deletions manager/middlewares/rbac.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand All @@ -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] = "[]"
Expand All @@ -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 {
Expand Down
105 changes: 70 additions & 35 deletions manager/middlewares/rbac_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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",
},
}},
},
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down
5 changes: 3 additions & 2 deletions platform/rbac.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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},
},
}},
},
Expand Down

0 comments on commit 893ec7d

Please sign in to comment.