AuthZ service: Expand the logic to also evaluate action sets (#112124)

* expand AuthZ service logic to also evaluate action sets

* handle folder creation

* fix test

* simplify mapper code

Co-authored-by: gamab <gabi.mabs@gmail.com>

* more accurate variable name Co-authored-by: gamab <gabi.mabs@gmail.com>

* break alerting import cycle

* Apply suggestion from @gamab

---------

Co-authored-by: gamab <gabi.mabs@gmail.com>
Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>

pkg/services/authz/rbac/mapper.go

@@ -2,8 +2,10 @@ package rbac
 
 import (
 	"fmt"
+	"slices"
 
 	"github.com/grafana/grafana/pkg/apimachinery/utils"
+	"github.com/grafana/grafana/pkg/services/accesscontrol/ossaccesscontrol"
 )
 
 // Mapping maps a verb to a RBAC action and a resource name to a RBAC scope.
@@ -11,6 +13,9 @@ type Mapping interface {
 	// action returns the action for the given verb.
 	// If no action is found, it returns false.
 	Action(verb string) (string, bool)
+	// ActionSets returns the action sets for the given verb.
+	// If no action sets are found, it returns an empty slice. This is expected for resources that do not have action sets (anything apart from dashboards and folders).
+	ActionSets(verb string) []string
 	// scope returns the scope for the given resource name.
 	Scope(name string) string
 	// prefix returns the scope prefix for the translation.
@@ -27,6 +32,7 @@ type translation struct {
 	resource          string
 	attribute         string
 	verbMapping       map[string]string
+	actionSetMapping  map[string][]string
 	folderSupport     bool
 	skipScopeOnCreate bool
 }
@@ -36,6 +42,11 @@ func (t translation) Action(verb string) (string, bool) {
 	return action, ok
 }
 
+func (t translation) ActionSets(verb string) []string {
+	actionSets := t.actionSetMapping[verb]
+	return actionSets
+}
+
 func (t translation) Scope(name string) string {
 	return t.resource + ":" + t.attribute + ":" + name
 }
@@ -101,13 +112,67 @@ func newResourceTranslation(resource string, attribute string, folderSupport, sk
 	}
 }
 
+// newDashboardTranslation creates a translation for dashboards and also maps the actions to action sets
+func newDashboardTranslation() translation {
+	dashTranslation := newResourceTranslation("dashboards", "uid", true, false)
+
+	actionSetMapping := make(map[string][]string)
+	for verb, rbacAction := range dashTranslation.verbMapping {
+		var dashActionSets []string
+		if slices.Contains(ossaccesscontrol.DashboardViewActions, rbacAction) {
+			dashActionSets = append(dashActionSets, "dashboards:view")
+			dashActionSets = append(dashActionSets, "folders:view")
+		}
+		if slices.Contains(ossaccesscontrol.DashboardEditActions, rbacAction) {
+			dashActionSets = append(dashActionSets, "dashboards:edit")
+			dashActionSets = append(dashActionSets, "folders:edit")
+		}
+		if slices.Contains(ossaccesscontrol.DashboardAdminActions, rbacAction) {
+			dashActionSets = append(dashActionSets, "dashboards:admin")
+			dashActionSets = append(dashActionSets, "folders:admin")
+		}
+		actionSetMapping[verb] = dashActionSets
+	}
+
+	dashTranslation.actionSetMapping = actionSetMapping
+	return dashTranslation
+}
+
+// newFolderTranslation creates a translation for folders and also maps the actions to action sets
+func newFolderTranslation() translation {
+	folderTranslation := newResourceTranslation("folders", "uid", true, false)
+
+	actionSetMapping := make(map[string][]string)
+	for verb, rbacAction := range folderTranslation.verbMapping {
+		var actionSets []string
+		// Folder creation has not been added to the FolderEditActions and FolderAdminActions slices (https://github.com/grafana/identity-access-team/issues/794)
+		// so we handle it as a special case for now
+		if rbacAction == "folders:create" {
+			actionSets = append(actionSets, "folders:edit")
+			actionSets = append(actionSets, "folders:admin")
+		}
+		if slices.Contains(ossaccesscontrol.FolderViewActions, rbacAction) {
+			actionSets = append(actionSets, "folders:view")
+		}
+		if slices.Contains(ossaccesscontrol.FolderEditActions, rbacAction) {
+			actionSets = append(actionSets, "folders:edit")
+		}
+		if slices.Contains(ossaccesscontrol.FolderAdminActions, rbacAction) {
+			actionSets = append(actionSets, "folders:admin")
+		}
+		actionSetMapping[verb] = actionSets
+	}
+	folderTranslation.actionSetMapping = actionSetMapping
+	return folderTranslation
+}
+
 func NewMapperRegistry() MapperRegistry {
 	mapper := mapper(map[string]map[string]translation{
 		"dashboard.grafana.app": {
-			"dashboards": newResourceTranslation("dashboards", "uid", true, false),
+			"dashboards": newDashboardTranslation(),
 		},
 		"folder.grafana.app": {
-			"folders": newResourceTranslation("folders", "uid", true, false),
+			"folders": newFolderTranslation(),
 		},
 		"iam.grafana.app": {
 			// Users is a special case. We translate user permissions from id to uid based.

pkg/services/authz/rbac/models.go

@@ -7,6 +7,7 @@ type checkRequest struct {
 	IdentityType claims.IdentityType
 	UserUID      string
 	Action       string // Verb has been mapped into an action
+	ActionSets   []string
 	Group        string
 	Resource     string
 	Verb         string
@@ -22,6 +23,7 @@ type listRequest struct {
 	Resource     string
 	Verb         string
 	Action       string
+	ActionSets   []string
 	Options      *ListRequestOptions
 }
 

pkg/services/authz/rbac/service.go

@@ -160,7 +160,7 @@ func (s *Service) Check(ctx context.Context, req *authzv1.CheckRequest) (*authzv
 	}
 	s.metrics.permissionCacheUsage.WithLabelValues("false", checkReq.Action).Inc()
 
-	permissions, err := s.getIdentityPermissions(ctx, checkReq.Namespace, checkReq.IdentityType, checkReq.UserUID, checkReq.Action)
+	permissions, err := s.getIdentityPermissions(ctx, checkReq.Namespace, checkReq.IdentityType, checkReq.UserUID, checkReq.Action, checkReq.ActionSets)
 	if err != nil {
 		ctxLogger.Error("could not get user permissions", "subject", req.GetSubject(), "error", err)
 		s.metrics.requestCount.WithLabelValues("true", "true", req.GetVerb(), req.GetGroup(), req.GetResource()).Inc()
@@ -223,7 +223,7 @@ func (s *Service) List(ctx context.Context, req *authzv1.ListRequest) (*authzv1.
 	if err != nil || listReq.Options.SkipCache {
 		s.metrics.permissionCacheUsage.WithLabelValues("false", listReq.Action).Inc()
 
-		permissions, err = s.getIdentityPermissions(ctx, listReq.Namespace, listReq.IdentityType, listReq.UserUID, listReq.Action)
+		permissions, err = s.getIdentityPermissions(ctx, listReq.Namespace, listReq.IdentityType, listReq.UserUID, listReq.Action, listReq.ActionSets)
 		if err != nil {
 			ctxLogger.Error("could not get user permissions", "subject", req.GetSubject(), "error", err)
 			s.metrics.requestCount.WithLabelValues("true", "true", req.GetVerb(), req.GetGroup(), req.GetResource()).Inc()
@@ -260,7 +260,7 @@ func (s *Service) validateCheckRequest(ctx context.Context, req *authzv1.CheckRe
 		return nil, err
 	}
 
-	action, err := s.validateAction(ctx, req.GetGroup(), req.GetResource(), req.GetVerb())
+	action, actionSets, err := s.validateAction(ctx, req.GetGroup(), req.GetResource(), req.GetVerb())
 	if err != nil {
 		return nil, err
 	}
@@ -270,6 +270,7 @@ func (s *Service) validateCheckRequest(ctx context.Context, req *authzv1.CheckRe
 		UserUID:      userUID,
 		IdentityType: idType,
 		Action:       action,
+		ActionSets:   actionSets,
 		Group:        req.GetGroup(),
 		Resource:     req.GetResource(),
 		Verb:         req.GetVerb(),
@@ -293,7 +294,7 @@ func (s *Service) validateListRequest(ctx context.Context, req *authzv1.ListRequ
 		return nil, err
 	}
 
-	action, err := s.validateAction(ctx, req.GetGroup(), req.GetResource(), req.GetVerb())
+	action, actionSets, err := s.validateAction(ctx, req.GetGroup(), req.GetResource(), req.GetVerb())
 	if err != nil {
 		return nil, err
 	}
@@ -311,6 +312,7 @@ func (s *Service) validateListRequest(ctx context.Context, req *authzv1.ListRequ
 		UserUID:      userUID,
 		IdentityType: idType,
 		Action:       action,
+		ActionSets:   actionSets,
 		Group:        req.GetGroup(),
 		Resource:     req.GetResource(),
 		Verb:         req.GetVerb(),
@@ -359,34 +361,30 @@ func (s *Service) validateSubject(ctx context.Context, subject string) (string,
 }
 
 // Find the action for a selected verb
-func (s *Service) validateAction(ctx context.Context, group, resource, verb string) (string, error) {
+func (s *Service) validateAction(ctx context.Context, group, resource, verb string) (string, []string, error) {
 	ctxLogger := s.logger.FromContext(ctx)
 
 	t, ok := s.mapper.Get(group, resource)
 	if !ok {
 		ctxLogger.Error("unsupported resource", "group", group, "resource", resource)
-		return "", status.Error(codes.NotFound, "unsupported resource")
+		return "", nil, status.Error(codes.NotFound, "unsupported resource")
 	}
 
 	action, ok := t.Action(verb)
 	if !ok {
 		ctxLogger.Error("unsupported verb", "group", group, "resource", resource, "verb", verb)
-		return "", status.Error(codes.NotFound, "unsupported verb")
+		return "", nil, status.Error(codes.NotFound, "unsupported verb")
 	}
 
-	return action, nil
+	actionSets := t.ActionSets(verb)
+
+	return action, actionSets, nil
 }
 
-func (s *Service) getIdentityPermissions(ctx context.Context, ns types.NamespaceInfo, idType types.IdentityType, userID, action string) (map[string]bool, error) {
+func (s *Service) getIdentityPermissions(ctx context.Context, ns types.NamespaceInfo, idType types.IdentityType, userID, action string, actionSets []string) (map[string]bool, error) {
 	ctx, span := s.tracer.Start(ctx, "authz_direct_db.service.getIdentityPermissions")
 	defer span.End()
 
-	// When checking folder creation permissions, also check edit and admin action sets for folder, as the scoped folder create actions aren't stored in the DB separately
-	var actionSets []string
-	if action == "folders:create" {
-		actionSets = append(actionSets, "folders:edit", "folders:admin")
-	}
-
 	switch idType {
 	case types.TypeAnonymous:
 		return s.getAnonymousPermissions(ctx, ns, action, actionSets)

pkg/services/authz/rbac/service_test.go

@@ -3,6 +3,7 @@ package rbac
 import (
 	"context"
 	"fmt"
+	"slices"
 	"testing"
 	"time"
 
@@ -363,6 +364,7 @@ func TestService_mapping(t *testing.T) {
 			},
 			output: &checkRequest{
 				Action:       "folders:create",
+				ActionSets:   []string{"folders:edit", "folders:admin"},
 				Group:        "folder.grafana.app",
 				Resource:     "folders",
 				Name:         "aaa",
@@ -618,6 +620,7 @@ func TestService_getUserPermissions(t *testing.T) {
 	type testCase struct {
 		name          string
 		permissions   []accesscontrol.Permission
+		action        string
 		cacheHit      bool
 		expectedPerms map[string]bool
 	}
@@ -628,12 +631,14 @@ func TestService_getUserPermissions(t *testing.T) {
 			permissions: []accesscontrol.Permission{
 				{Action: "dashboards:read", Scope: "dashboards:uid:some_dashboard"},
 			},
+			action:        "dashboards:read",
 			cacheHit:      false,
 			expectedPerms: map[string]bool{"dashboards:uid:some_dashboard": true},
 		},
 		{
 			name:          "should return error if store fails",
 			permissions:   nil,
+			action:        "dashboards:read",
 			cacheHit:      false,
 			expectedPerms: map[string]bool{},
 		},
@@ -642,6 +647,7 @@ func TestService_getUserPermissions(t *testing.T) {
 			permissions: []accesscontrol.Permission{
 				{Action: "teams:read", Scope: "teams:id:1"},
 			},
+			action:        "teams:read",
 			cacheHit:      false,
 			expectedPerms: map[string]bool{"teams:uid:t1": true},
 		},
@@ -654,10 +660,9 @@ func TestService_getUserPermissions(t *testing.T) {
 			ns := types.NamespaceInfo{Value: "stacks-12", OrgID: 1, StackID: 12}
 
 			userID := &store.UserIdentifiers{UID: "test-uid", ID: 112}
-			action := "dashboards:read"
 
 			if tc.cacheHit {
-				s.permCache.Set(ctx, userPermCacheKey(ns.Value, userID.UID, action), tc.expectedPerms)
+				s.permCache.Set(ctx, userPermCacheKey(ns.Value, userID.UID, tc.action), tc.expectedPerms)
 			}
 
 			store := &fakeStore{
@@ -677,7 +682,7 @@ func TestService_getUserPermissions(t *testing.T) {
 				disableNsCheck: true,
 			}
 
-			perms, err := s.getIdentityPermissions(ctx, ns, types.TypeUser, userID.UID, action)
+			perms, err := s.getIdentityPermissions(ctx, ns, types.TypeUser, userID.UID, tc.action, nil)
 			require.NoError(t, err)
 			require.Len(t, perms, len(tc.expectedPerms))
 			for scope := range perms {
@@ -1086,6 +1091,53 @@ func TestService_Check(t *testing.T) {
 			},
 			expected: true,
 		},
+		{
+			name: "should take into account action sets",
+			req: &authzv1.CheckRequest{
+				Namespace: "org-12",
+				Subject:   "user:test-uid",
+				Group:     "dashboard.grafana.app",
+				Resource:  "dashboards",
+				Verb:      "get",
+				Name:      "dash1",
+			},
+			permissions: []accesscontrol.Permission{
+				{Action: "dashboards:admin", Scope: "dashboards:uid:dash1"},
+			},
+			expected: true,
+		},
+		{
+			name: "should take into account folder action sets for dashboard access",
+			req: &authzv1.CheckRequest{
+				Namespace: "org-12",
+				Subject:   "user:test-uid",
+				Group:     "dashboard.grafana.app",
+				Resource:  "dashboards",
+				Verb:      "get",
+				Name:      "dash1",
+				Folder:    "some_folder",
+			},
+			permissions: []accesscontrol.Permission{
+				{Action: "folders:edit", Scope: "folders:uid:some_folder"},
+			},
+			expected: true,
+		},
+		{
+			name: "lower level action set or action set on a different resource should not grant higher level access",
+			req: &authzv1.CheckRequest{
+				Namespace: "org-12",
+				Subject:   "user:test-uid",
+				Group:     "folder.grafana.app",
+				Resource:  "folders",
+				Verb:      "delete",
+				Name:      "folder1",
+			},
+			permissions: []accesscontrol.Permission{
+				{Action: "folders:view", Scope: "folders:uid:folder1"},
+				{Action: "folders:edit", Scope: "folders:uid:other_folder"},
+			},
+			expected: false,
+		},
 		{
 			// We've had cases where permissions were saved to the database
 			// without splitting the scope into 'kind', 'attribute', and 'identifier'.
@@ -1135,6 +1187,10 @@ func TestService_Check(t *testing.T) {
 				if tc.req.Resource == "teams" {
 					expAction = "teams:read"
 				}
+				if tc.req.Resource == "folders" {
+					expAction = "folders:delete"
+				}
+
 				perms, ok := s.permCache.Get(ctx, userPermCacheKey("org-12", "test-uid", expAction))
 				require.True(t, ok)
 				require.Len(t, perms, 1)
@@ -1492,6 +1548,27 @@ func TestService_List(t *testing.T) {
 				Folders: []string{"fold1"},
 			},
 		},
+		{
+			name: "should list permissions for user with permission or action set permissions",
+			req: &authzv1.ListRequest{
+				Namespace: "org-12",
+				Subject:   "user:test-uid",
+				Group:     "dashboard.grafana.app",
+				Resource:  "dashboards",
+				Verb:      "get",
+			},
+			permissions: []accesscontrol.Permission{
+				{Action: "dashboards:read", Scope: "dashboards:uid:dash1"},
+				{Action: "dashboards:read", Scope: "dashboards:uid:dash2"},
+				{Action: "dashboards:read", Scope: "folders:uid:fold1"},
+				{Action: "dashboards:edit", Scope: "dashboards:uid:dash3"},
+				{Action: "folders:view", Scope: "folders:uid:fold2"},
+			},
+			expected: &authzv1.ListResponse{
+				Items:   []string{"dash1", "dash2", "dash3"},
+				Folders: []string{"fold1", "fold2"},
+			},
+		},
 		{
 			name: "should return empty list for user without permission",
 			req: &authzv1.ListRequest{
@@ -1824,7 +1901,13 @@ func (f *fakeStore) GetUserPermissions(ctx context.Context, namespace types.Name
 	if f.err {
 		return nil, fmt.Errorf("store error")
 	}
-	return f.userPermissions, nil
+	var permissions []accesscontrol.Permission
+	for _, p := range f.userPermissions {
+		if p.Action == query.Action || slices.Contains(query.ActionSets, p.Action) {
+			permissions = append(permissions, p)
+		}
+	}
+	return permissions, nil
 }
 
 func (f *fakeStore) ListFolders(ctx context.Context, namespace types.NamespaceInfo) ([]store.Folder, error) {