Authz: Simplify mapper and only check folders if its supported (#99357)

* Simplify mapper and only check folders if its supported

pkg/services/authz/mappers/rbac_mapper.go

@@ -1,72 +0,0 @@
-package mappers
-
-import (
-	"fmt"
-
-	"github.com/grafana/grafana/pkg/apimachinery/utils"
-)
-
-const defaultAttribute = "uid"
-
-type VerbMapping map[string]string                           // e.g. "get" -> "read"
-type ResourceVerbMapping map[string]VerbMapping              // e.g. "dashboards" -> VerbToAction
-type GroupResourceVerbMapping map[string]ResourceVerbMapping // e.g. "dashboard.grafana.app" -> ResourceVerbToAction
-
-type ResourceAttributeMapping map[string]string                        // e.g. "dashboards" -> "uid"
-type GroupResourceAttributeMapping map[string]ResourceAttributeMapping // e.g. "dashboard.grafana.app" -> ResourceToAttribute
-
-type K8sRbacMapper struct {
-	GroupResourceVerbMapping      GroupResourceVerbMapping
-	GroupResourceAttributeMapping GroupResourceAttributeMapping
-}
-
-func NewK8sRbacMapper() *K8sRbacMapper {
-	defaultMapping := func(r string) VerbMapping {
-		return map[string]string{
-			utils.VerbGet:              fmt.Sprintf("%s:read", r),
-			utils.VerbList:             fmt.Sprintf("%s:read", r),
-			utils.VerbWatch:            fmt.Sprintf("%s:read", r),
-			utils.VerbCreate:           fmt.Sprintf("%s:create", r),
-			utils.VerbUpdate:           fmt.Sprintf("%s:write", r),
-			utils.VerbPatch:            fmt.Sprintf("%s:write", r),
-			utils.VerbDelete:           fmt.Sprintf("%s:delete", r),
-			utils.VerbDeleteCollection: fmt.Sprintf("%s:delete", r),
-			utils.VerbGetPermissions:   fmt.Sprintf("%s.permissions:read", r),
-			utils.VerbSetPermissions:   fmt.Sprintf("%s.permissions:write", r),
-		}
-	}
-
-	return &K8sRbacMapper{
-		GroupResourceAttributeMapping: GroupResourceAttributeMapping{},
-		GroupResourceVerbMapping: GroupResourceVerbMapping{
-			"dashboard.grafana.app": ResourceVerbMapping{"dashboards": defaultMapping("dashboards")},
-			"folder.grafana.app":    ResourceVerbMapping{"folders": defaultMapping("folders")},
-		},
-	}
-}
-
-func (m *K8sRbacMapper) Action(group, resource, verb string) (string, bool) {
-	if resourceActions, ok := m.GroupResourceVerbMapping[group]; ok {
-		if actions, ok := resourceActions[resource]; ok {
-			if action, ok := actions[verb]; ok {
-				// If the action is explicitly set empty
-				// it means that the action is not allowed
-				if action == "" {
-					return "", false
-				}
-				return action, true
-			}
-		}
-	}
-	return "", false
-}
-
-func (m *K8sRbacMapper) Scope(group, resource, name string) (string, bool) {
-	if resourceAttributes, ok := m.GroupResourceAttributeMapping[group]; ok {
-		if attribute, ok := resourceAttributes[resource]; ok {
-			return resource + ":" + attribute + ":" + name, true
-		}
-	}
-
-	return resource + ":" + defaultAttribute + ":" + name, true
-}

pkg/services/authz/rbac/mapper.go

@@ -0,0 +1,81 @@
+package rbac
+
+import (
+	"fmt"
+
+	"github.com/grafana/grafana/pkg/apimachinery/utils"
+)
+
+type translation struct {
+	resource      string
+	attribute     string
+	verbMapping   map[string]string
+	folderSupport bool
+}
+
+func (t translation) action(verb string) (string, bool) {
+	action, ok := t.verbMapping[verb]
+	return action, ok
+}
+
+func (t translation) scope(name string) string {
+	return t.resource + ":" + t.attribute + ":" + name
+}
+
+func (t translation) prefix() string {
+	return t.resource + ":" + t.attribute + ":"
+}
+
+func newResourceTranslation(resource string, attribute string, folderSupport bool) translation {
+	defaultMapping := func(r string) map[string]string {
+		return map[string]string{
+			utils.VerbGet:              fmt.Sprintf("%s:read", r),
+			utils.VerbList:             fmt.Sprintf("%s:read", r),
+			utils.VerbWatch:            fmt.Sprintf("%s:read", r),
+			utils.VerbCreate:           fmt.Sprintf("%s:create", r),
+			utils.VerbUpdate:           fmt.Sprintf("%s:write", r),
+			utils.VerbPatch:            fmt.Sprintf("%s:write", r),
+			utils.VerbDelete:           fmt.Sprintf("%s:delete", r),
+			utils.VerbDeleteCollection: fmt.Sprintf("%s:delete", r),
+			utils.VerbGetPermissions:   fmt.Sprintf("%s.permissions:read", r),
+			utils.VerbSetPermissions:   fmt.Sprintf("%s.permissions:write", r),
+		}
+	}
+
+	return translation{
+		resource:      resource,
+		attribute:     attribute,
+		verbMapping:   defaultMapping(resource),
+		folderSupport: folderSupport,
+	}
+}
+
+type mapper map[string]map[string]translation
+
+func newMapper() mapper {
+	return map[string]map[string]translation{
+		"dashboard.grafana.app": {
+			"dashboards": newResourceTranslation("dashboards", "uid", true),
+		},
+		"folder.grafana.app": {
+			"folders": newResourceTranslation("folders", "uid", true),
+		},
+		"iam.grafana.app": {
+			"teams": newResourceTranslation("teams", "id", false),
+		},
+	}
+}
+
+func (m mapper) translation(group, resource string) (translation, bool) {
+	resources, ok := m[group]
+	if !ok {
+		return translation{}, false
+	}
+
+	t, ok := resources[resource]
+	if !ok {
+		return translation{}, false
+	}
+
+	return t, true
+}

pkg/services/authz/rbac/service.go

@@ -23,7 +23,6 @@ import (
 	"github.com/grafana/grafana/pkg/registry/apis/iam/common"
 	"github.com/grafana/grafana/pkg/registry/apis/iam/legacy"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
-	"github.com/grafana/grafana/pkg/services/authz/mappers"
 	authzextv1 "github.com/grafana/grafana/pkg/services/authz/proto/v1"
 	"github.com/grafana/grafana/pkg/services/authz/rbac/store"
 	"github.com/grafana/grafana/pkg/services/dashboards"
@@ -44,7 +43,8 @@ type Service struct {
 	store           store.Store
 	permissionStore store.PermissionStore
 	identityStore   legacy.LegacyIdentityStore
-	actionMapper    *mappers.K8sRbacMapper
+
+	mapper mapper
 
 	logger log.Logger
 	tracer tracing.Tracer
@@ -72,9 +72,9 @@ func NewService(
 		store:           store.NewStore(sql, tracer),
 		permissionStore: permissionStore,
 		identityStore:   identityStore,
-		actionMapper:    mappers.NewK8sRbacMapper(),
 		logger:          logger,
 		tracer:          tracer,
+		mapper:          newMapper(),
 		idCache:         localcache.New(longCacheTTL, longCleanupInterval),
 		permCache:       localcache.New(shortCacheTTL, shortCleanupInterval),
 		teamCache:       localcache.New(shortCacheTTL, shortCleanupInterval),
@@ -236,14 +236,19 @@ func (s *Service) validateSubject(ctx context.Context, subject string) (string,
 
 func (s *Service) validateAction(ctx context.Context, group, resource, verb string) (string, error) {
 	ctxLogger := s.logger.FromContext(ctx)
-	if group == "" || resource == "" || verb == "" {
+
-		return "", status.Error(codes.InvalidArgument, "group, resource and verb are required")
+	t, ok := s.mapper.translation(group, resource)
-	}
-	action, ok := s.actionMapper.Action(group, resource, verb)
 	if !ok {
-		ctxLogger.Error("could not find associated rbac action", "group", group, "resource", resource, "verb", verb)
+		ctxLogger.Error("unsupport resource", "group", group, "resource", resource)
-		return "", status.Error(codes.NotFound, "could not find associated rbac action")
+		return "", status.Error(codes.NotFound, "unsupported resource")
 	}
+
+	action, ok := t.action(verb)
+	if !ok {
+		ctxLogger.Error("unsupport verb", "group", group, "resource", resource, "verb", verb)
+		return "", status.Error(codes.NotFound, "unsupported verb")
+	}
+
 	return action, nil
 }
 
@@ -437,15 +442,20 @@ func (s *Service) checkPermission(ctx context.Context, scopeMap map[string]bool,
 		return true, nil
 	}
 
-	scope, has := s.actionMapper.Scope(req.Group, req.Resource, req.Name)
+	t, ok := s.mapper.translation(req.Group, req.Resource)
-	if !has {
+	if !ok {
-		ctxLogger.Error("could not get attribute for resource", "resource", req.Resource)
+		ctxLogger.Error("unsupport resource", "group", req.Group, "resource", req.Resource)
-		return false, fmt.Errorf("could not get attribute for resource")
+		return false, status.Error(codes.NotFound, "unsupported resource")
 	}
-	if scopeMap[scope] {
+
+	if scopeMap[t.scope(req.Name)] {
 		return true, nil
 	}
 
+	if !t.folderSupport {
+		return false, nil
+	}
+
 	return s.checkInheritedPermissions(ctx, scopeMap, req)
 }
 
@@ -556,25 +566,37 @@ func (s *Service) listPermission(ctx context.Context, scopeMap map[string]bool,
 	defer span.End()
 	ctxLogger := s.logger.FromContext(ctx)
 
-	folderMap, err := s.buildFolderTree(ctx, req.Namespace)
+	t, ok := s.mapper.translation(req.Group, req.Resource)
-	if err != nil {
+	if !ok {
-		ctxLogger.Error("could not build folder and dashboard tree", "error", err)
+		ctxLogger.Error("unsupport resource", "group", req.Group, "resource", req.Resource)
-		return nil, err
+		return nil, status.Error(codes.NotFound, "unsupported resource")
+	}
+
+	var folderMap map[string]FolderNode
+	if t.folderSupport {
+		var err error
+		folderMap, err = s.buildFolderTree(ctx, req.Namespace)
+		if err != nil {
+			ctxLogger.Error("could not build folder and dashboard tree", "error", err)
+			return nil, err
+		}
 	}
 
 	folderSet := make(map[string]struct{}, len(scopeMap))
-	dashSet := make(map[string]struct{}, len(scopeMap))
+
+	prefix := t.prefix()
+	itemSet := make(map[string]struct{}, len(scopeMap))
 	for scope := range scopeMap {
 		if strings.HasPrefix(scope, "folders:uid:") {
-			identifier := scope[len("folders:uid:"):]
+			identifier := strings.TrimPrefix(scope, "folders:uid:")
 			if _, ok := folderSet[identifier]; ok {
 				continue
 			}
 			folderSet[identifier] = struct{}{}
 			getChildren(folderMap, identifier, folderSet)
-		} else if strings.HasPrefix(scope, "dashboards:uid:") {
+		} else {
-			identifier := scope[len("dashboards:uid:"):]
+			identifier := strings.TrimPrefix(scope, prefix)
-			dashSet[identifier] = struct{}{}
+			itemSet[identifier] = struct{}{}
 		}
 	}
 
@@ -583,13 +605,13 @@ func (s *Service) listPermission(ctx context.Context, scopeMap map[string]bool,
 		folderList = append(folderList, folder)
 	}
 
-	dashList := make([]string, 0, len(dashSet))
+	itemList := make([]string, 0, len(itemSet))
-	for dash := range dashSet {
+	for item := range itemSet {
-		dashList = append(dashList, dash)
+		itemList = append(itemList, item)
 	}
 
-	span.SetAttributes(attribute.Int("num_folders", len(folderList)), attribute.Int("num_dashboards", len(dashList)))
+	span.SetAttributes(attribute.Int("num_folders", len(folderList)), attribute.Int("num_items", len(itemList)))
-	return &authzv1.ListResponse{Folders: folderList, Items: dashList}, nil
+	return &authzv1.ListResponse{Folders: folderList, Items: itemList}, nil
 }
 
 func getChildren(folderMap map[string]FolderNode, folderUID string, folderSet map[string]struct{}) {

pkg/services/authz/rbac/service_test.go

@@ -16,7 +16,6 @@ import (
 	"github.com/grafana/grafana/pkg/infra/tracing"
 	"github.com/grafana/grafana/pkg/registry/apis/iam/legacy"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
-	"github.com/grafana/grafana/pkg/services/authz/mappers"
 	"github.com/grafana/grafana/pkg/services/authz/rbac/store"
 )
 
@@ -150,7 +149,11 @@ func TestService_checkPermission(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			s := &Service{logger: log.New("test"), actionMapper: mappers.NewK8sRbacMapper(), tracer: tracing.NewNoopTracerService()}
+			s := &Service{
+				logger: log.NewNopLogger(),
+				tracer: tracing.NewNoopTracerService(),
+				mapper: newMapper(),
+			}
 			got, err := s.checkPermission(context.Background(), getScopeMap(tc.permissions), &tc.check)
 			require.NoError(t, err)
 			assert.Equal(t, tc.expected, got)
@@ -366,9 +369,9 @@ func TestService_getUserPermissions(t *testing.T) {
 				store:           store,
 				permissionStore: store,
 				identityStore:   &fakeIdentityStore{teams: []int64{1, 2}},
-				actionMapper:    mappers.NewK8sRbacMapper(),
+				logger:          log.NewNopLogger(),
-				logger:          log.New("test"),
 				tracer:          tracing.NewNoopTracerService(),
+				mapper:          newMapper(),
 				idCache:         localcache.New(longCacheTTL, longCleanupInterval),
 				permCache:       cacheService,
 				sf:              new(singleflight.Group),
@@ -649,10 +652,10 @@ func TestService_listPermission(t *testing.T) {
 				folderCache.Set(folderCacheKey("default"), tc.folderTree, 0)
 			}
 			s := &Service{
-				logger:       log.New("test"),
+				logger:      log.New("test"),
-				actionMapper: mappers.NewK8sRbacMapper(),
+				folderCache: folderCache,
-				folderCache:  folderCache,
+				mapper:      newMapper(),
-				tracer:       tracing.NewNoopTracerService(),
+				tracer:      tracing.NewNoopTracerService(),
 			}
 			tc.list.Namespace = claims.NamespaceInfo{Value: "default", OrgID: 1}
 			got, err := s.listPermission(context.Background(), getScopeMap(tc.permissions), &tc.list)