[CCXDEV-12885] Fix bug in RBAC logic + simplify (#1379)

* fix bug in RBAC logic + simplify

* prevent nil pointer issues

* fix lint

* fix lint

auth/rbac.go

@@ -65,7 +65,7 @@ func (rc *rbacClientImpl) IsEnforcing() bool {
 // IsAuthorized checks if an account has the correct permissions to access our resources
 func (rc *rbacClientImpl) IsAuthorized(token string) bool {
 	permissions := rc.getPermissions(token)
-	log.Debug().Interface("permissions", permissions).Msg("Account openshift permissions")
+	log.Debug().Interface("permissions", permissions).Msg("Account ocp-advisor permissions")
 	return permissions != nil
 }
 
@@ -75,6 +75,7 @@ func (rc *rbacClientImpl) getPermissions(identityToken string) map[string][]stri
 		log.Debug().Interface("acls", acls).Msg("Account all permissions")
 		permissions := aggregatePermissions(acls)
 		if len(permissions) > 0 {
+			// as we just need a read permission, we accept any > 0 here
 			log.Info().Any("RBAC openshift permissions", permissions).Send()
 			return permissions
 		}
@@ -137,30 +138,31 @@ func (rc *rbacClientImpl) requestAccess(url, identityToken string) []types.RbacD
 
 // aggregatePermissions loop over all the permissions/roles/alcs of the user returned
 // from RBAC and creates and return the map of permissions where key is
-// resourceType (openshift.cluster, openshift.node, openshift.project) and the values are the
+// resourceType (recommendation-results) and the values are the
 // slice of resources (cluster names, node names, project names).
+// We are interested in this ACLs: https://github.com/RedHatInsights/rbac-config/blob/master/configs/prod/permissions/ocp-advisor.json
 func aggregatePermissions(acls []types.RbacData) map[string][]string {
 	permissions := map[string][]string{}
 	for _, acl := range acls {
-		resourceType := strings.Split(acl.Permission, ":")[1]
-		if strings.Contains(resourceType, "openshift") {
+		splits := strings.Split(acl.Permission, ":")
+		if len(splits) < 3 {
+			log.Warn().Str("ACL", acl.Permission).Msg("Unexpected RBAC response")
+			continue
+		}
+		if splits[0] != "ocp-advisor" {
+			// check the ACL is for ocp-advisor, not for other APIs
+			continue
+		}
+		resourceType := splits[1]
+		verb := splits[2]
+		// ignore other kind of permissions, we just want recommendation-results
+		if strings.Contains(resourceType, "recommendation-results") {
 			if _, ok := permissions[resourceType]; !ok {
+				// add the resource type to the permissions map if not already
+				// there so that we can then add the permission verb
 				permissions[resourceType] = []string{}
 			}
-			if len(acl.ResourceDefinitions) == 0 {
-				permissions[resourceType] = append(permissions[resourceType], "*")
-			} else {
-				for _, resourceDefinition := range acl.ResourceDefinitions {
-					switch t := resourceDefinition.AttributeFilter.Value.(type) {
-					case []interface{}:
-						for _, v := range t {
-							permissions[resourceType] = append(permissions[resourceType], fmt.Sprint(v))
-						}
-					case string:
-						permissions[resourceType] = append(permissions[resourceType], t)
-					}
-				}
-			}
+			permissions[resourceType] = append(permissions[resourceType], verb)
 		} else if resourceType == "*" {
 			permissions["*"] = []string{}
 		}

auth/rbac_test.go

@@ -0,0 +1,75 @@
+package auth
+
+import (
+	"testing"
+
+	"github.com/RedHatInsights/insights-results-smart-proxy/types"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestAggregatePermissions(t *testing.T) {
+	type testCase struct {
+		name        string
+		permissions []string
+		want        map[string][]string
+	}
+
+	testCases := []testCase{
+		{
+			name:        "no permissions",
+			permissions: []string{},
+			want:        map[string][]string{},
+		},
+		{
+			name:        "all permissions",
+			permissions: []string{"ocp-advisor:*:*"},
+			want:        map[string][]string{"*": {}},
+		},
+		{
+			name:        "all permissions for recommendations",
+			permissions: []string{"ocp-advisor:recommendation-results:*"},
+			want:        map[string][]string{"recommendation-results": {"*"}},
+		},
+		{
+			name:        "read permissions for recommendations",
+			permissions: []string{"ocp-advisor:recommendation-results:read"},
+			want:        map[string][]string{"recommendation-results": {"read"}},
+		},
+		{
+			name:        "recommendations permissions but not for ocp-advisor",
+			permissions: []string{"other:recommendation-results:read"},
+			want:        map[string][]string{},
+		},
+		{
+			name:        "all permissions but not for ocp-advisor",
+			permissions: []string{"other:recommendation-results:*"},
+			want:        map[string][]string{},
+		},
+		{
+			name:        "permissions on ocp-advisor but not for recommendations",
+			permissions: []string{"ocp-advisor:other:*"},
+			want:        map[string][]string{},
+		},
+		{
+			name:        "all permissions for recommendations and other resources",
+			permissions: []string{"other:other:*", "ocp-advisor:recommendation-results:*"},
+			want:        map[string][]string{"recommendation-results": {"*"}},
+		},
+		{
+			name:        "bad RBAC response (not enough elements)",
+			permissions: []string{"ocp-advisor:recommendation-results"},
+			want:        map[string][]string{},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			acls := []types.RbacData{}
+			for _, permission := range tc.permissions {
+				acls = append(acls, types.RbacData{Permission: permission})
+			}
+			got := aggregatePermissions(acls)
+			assert.Equal(t, tc.want, got)
+		})
+	}
+}