Access Control: Pass db session to hooks (#44428)

* Move hook calls to database and pass session

pkg/server/wireexts_oss.go

@@ -14,6 +14,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	acdb "github.com/grafana/grafana/pkg/services/accesscontrol/database"
 	"github.com/grafana/grafana/pkg/services/accesscontrol/ossaccesscontrol"
+	"github.com/grafana/grafana/pkg/services/accesscontrol/resourcepermissions"
 	"github.com/grafana/grafana/pkg/services/auth"
 	"github.com/grafana/grafana/pkg/services/datasources"
 	"github.com/grafana/grafana/pkg/services/encryption"
@@ -65,7 +66,7 @@ var wireExtsBasicSet = wire.NewSet(
 	provider.ProvideService,
 	wire.Bind(new(plugins.BackendFactoryProvider), new(*provider.Service)),
 	acdb.ProvideService,
-	wire.Bind(new(accesscontrol.ResourcePermissionsStore), new(*acdb.AccessControlStore)),
+	wire.Bind(new(resourcepermissions.Store), new(*acdb.AccessControlStore)),
 	wire.Bind(new(accesscontrol.PermissionsProvider), new(*acdb.AccessControlStore)),
 	osskmsproviders.ProvideService,
 	wire.Bind(new(kmsproviders.Service), new(osskmsproviders.Service)),

pkg/services/accesscontrol/accesscontrol.go

@@ -37,26 +37,11 @@ type ResourcePermissionsService interface {
 	// GetPermissions returns all permissions for given resourceID
 	GetPermissions(ctx context.Context, orgID int64, resourceID string) ([]ResourcePermission, error)
 	// SetUserPermission sets permission on resource for a user
-	SetUserPermission(ctx context.Context, orgID, userID int64, resourceID string, actions []string) (*ResourcePermission, error)
+	SetUserPermission(ctx context.Context, orgID, userID int64, resourceID, permission string) (*ResourcePermission, error)
 	// SetTeamPermission sets permission on resource for a team
-	SetTeamPermission(ctx context.Context, orgID, teamID int64, resourceID string, actions []string) (*ResourcePermission, error)
+	SetTeamPermission(ctx context.Context, orgID, teamID int64, resourceID, permission string) (*ResourcePermission, error)
 	// SetBuiltInRolePermission sets permission on resource for a built-in role (Admin, Editor, Viewer)
-	SetBuiltInRolePermission(ctx context.Context, orgID int64, builtInRole string, resourceID string, actions []string) (*ResourcePermission, error)
+	SetBuiltInRolePermission(ctx context.Context, orgID int64, builtInRole string, resourceID string, permission string) (*ResourcePermission, error)
-	// MapActions will map actions for a ResourcePermissions to it's "friendly" name configured in PermissionsToActions map.
-	MapActions(permission ResourcePermission) string
-	// MapPermission will map a friendly named permission to it's corresponding actions configured in PermissionsToAction map.
-	MapPermission(permission string) []string
-}
-
-type ResourcePermissionsStore interface {
-	// SetUserResourcePermission sets permission for managed user role on a resource
-	SetUserResourcePermission(ctx context.Context, orgID, userID int64, cmd SetResourcePermissionCommand) (*ResourcePermission, error)
-	// SetTeamResourcePermission sets permission for managed team role on a resource
-	SetTeamResourcePermission(ctx context.Context, orgID, teamID int64, cmd SetResourcePermissionCommand) (*ResourcePermission, error)
-	// SetBuiltinResourcePermission sets permissions for managed builtin role on a resource
-	SetBuiltInResourcePermission(ctx context.Context, orgID int64, builtinRole string, cmd SetResourcePermissionCommand) (*ResourcePermission, error)
-	// GetResourcesPermissions will return all permission for all supplied resource ids
-	GetResourcesPermissions(ctx context.Context, orgID int64, query GetResourcesPermissionsQuery) ([]ResourcePermission, error)
 }
 
 // Metadata contains user accesses for a given resource

pkg/services/accesscontrol/database/database_test.go

@@ -64,7 +64,7 @@ func TestAccessControlStore_GetUserPermissions(t *testing.T) {
 					Actions:    []string{"dashboards:read"},
 					Resource:   "dashboards",
 					ResourceID: id,
-				})
+				}, nil)
 				require.NoError(t, err)
 			}
 
@@ -73,7 +73,7 @@ func TestAccessControlStore_GetUserPermissions(t *testing.T) {
 					Actions:    []string{"dashboards:read"},
 					Resource:   "dashboards",
 					ResourceID: id,
-				})
+				}, nil)
 				require.NoError(t, err)
 			}
 
@@ -82,7 +82,7 @@ func TestAccessControlStore_GetUserPermissions(t *testing.T) {
 					Actions:    []string{"dashboards:read"},
 					Resource:   "dashboards",
 					ResourceID: id,
-				})
+				}, nil)
 				require.NoError(t, err)
 			}
 

pkg/services/accesscontrol/database/resource_permissions.go

@@ -32,7 +32,11 @@ func (p *flatResourcePermission) Managed() bool {
 	return strings.HasPrefix(p.RoleName, "managed:")
 }
 
-func (s *AccessControlStore) SetUserResourcePermission(ctx context.Context, orgID, userID int64, cmd accesscontrol.SetResourcePermissionCommand) (*accesscontrol.ResourcePermission, error) {
+func (s *AccessControlStore) SetUserResourcePermission(
+	ctx context.Context, orgID, userID int64,
+	cmd accesscontrol.SetResourcePermissionCommand,
+	hook func(session *sqlstore.DBSession, orgID, userID int64, resourceID, permission string) error,
+) (*accesscontrol.ResourcePermission, error) {
 	if userID == 0 {
 		return nil, models.ErrUserNotFound
 	}
@@ -44,6 +48,9 @@ func (s *AccessControlStore) SetUserResourcePermission(ctx context.Context, orgI
 		if err != nil {
 			return err
 		}
+		if hook != nil {
+			return hook(sess, orgID, userID, cmd.ResourceID, cmd.Permission)
+		}
 		return nil
 	})
 
@@ -54,7 +61,11 @@ func (s *AccessControlStore) SetUserResourcePermission(ctx context.Context, orgI
 	return permission, nil
 }
 
-func (s *AccessControlStore) SetTeamResourcePermission(ctx context.Context, orgID, teamID int64, cmd accesscontrol.SetResourcePermissionCommand) (*accesscontrol.ResourcePermission, error) {
+func (s *AccessControlStore) SetTeamResourcePermission(
+	ctx context.Context, orgID, teamID int64,
+	cmd accesscontrol.SetResourcePermissionCommand,
+	hook func(session *sqlstore.DBSession, orgID, teamID int64, resourceID, permission string) error,
+) (*accesscontrol.ResourcePermission, error) {
 	if teamID == 0 {
 		return nil, models.ErrTeamNotFound
 	}
@@ -67,6 +78,9 @@ func (s *AccessControlStore) SetTeamResourcePermission(ctx context.Context, orgI
 		if err != nil {
 			return err
 		}
+		if hook != nil {
+			return hook(sess, orgID, teamID, cmd.ResourceID, cmd.Permission)
+		}
 		return nil
 	})
 
@@ -77,7 +91,11 @@ func (s *AccessControlStore) SetTeamResourcePermission(ctx context.Context, orgI
 	return permission, nil
 }
 
-func (s *AccessControlStore) SetBuiltInResourcePermission(ctx context.Context, orgID int64, builtInRole string, cmd accesscontrol.SetResourcePermissionCommand) (*accesscontrol.ResourcePermission, error) {
+func (s *AccessControlStore) SetBuiltInResourcePermission(
+	ctx context.Context, orgID int64, builtInRole string,
+	cmd accesscontrol.SetResourcePermissionCommand,
+	hook func(session *sqlstore.DBSession, orgID int64, builtInRole, resourceID, permission string) error,
+) (*accesscontrol.ResourcePermission, error) {
 	if !models.RoleType(builtInRole).IsValid() || builtInRole == accesscontrol.RoleGrafanaAdmin {
 		return nil, fmt.Errorf("invalid role: %s", builtInRole)
 	}
@@ -87,6 +105,12 @@ func (s *AccessControlStore) SetBuiltInResourcePermission(ctx context.Context, o
 
 	err = s.sql.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error {
 		permission, err = s.setResourcePermission(sess, orgID, managedBuiltInRoleName(builtInRole), s.builtInRoleAdder(sess, orgID, builtInRole), cmd)
+		if err != nil {
+			return err
+		}
+		if hook != nil {
+			return hook(sess, orgID, builtInRole, cmd.ResourceID, cmd.Permission)
+		}
 		return err
 	})
 

pkg/services/accesscontrol/database/resource_permissions_bench_test.go

@@ -51,7 +51,7 @@ func benchmarkDSPermissions(b *testing.B, dsNum, usersNum int) {
 	}
 }
 
-func getDSPermissions(b *testing.B, store accesscontrol.ResourcePermissionsStore, dataSources []int64) {
+func getDSPermissions(b *testing.B, store *AccessControlStore, dataSources []int64) {
 	dsId := dataSources[0]
 
 	permissions, err := store.GetResourcesPermissions(context.Background(), accesscontrol.GlobalOrgID, accesscontrol.GetResourcesPermissionsQuery{
@@ -99,6 +99,7 @@ func GenerateDatasourcePermissions(b *testing.B, db *sqlstore.SQLStore, ac *Acce
 					Resource:   dsResource,
 					ResourceID: strconv.Itoa(int(dsID)),
 				},
+				nil,
 			)
 			require.NoError(b, err)
 		}
@@ -115,6 +116,7 @@ func GenerateDatasourcePermissions(b *testing.B, db *sqlstore.SQLStore, ac *Acce
 					Resource:   "datasources",
 					ResourceID: strconv.Itoa(int(dsID)),
 				},
+				nil,
 			)
 			require.NoError(b, err)
 		}

pkg/services/accesscontrol/database/resource_permissions_test.go

@@ -70,7 +70,7 @@ func TestAccessControlStore_SetUserResourcePermission(t *testing.T) {
 			store, _ := setupTestEnv(t)
 
 			for _, s := range test.seeds {
-				_, err := store.SetUserResourcePermission(context.Background(), test.orgID, test.userID, s)
+				_, err := store.SetUserResourcePermission(context.Background(), test.orgID, test.userID, s, nil)
 				require.NoError(t, err)
 			}
 
@@ -78,7 +78,7 @@ func TestAccessControlStore_SetUserResourcePermission(t *testing.T) {
 				Actions:    test.actions,
 				Resource:   test.resource,
 				ResourceID: test.resourceID,
-			})
+			}, nil)
 
 			require.NoError(t, err)
 			if len(test.actions) == 0 {
@@ -148,7 +148,7 @@ func TestAccessControlStore_SetTeamResourcePermission(t *testing.T) {
 			store, _ := setupTestEnv(t)
 
 			for _, s := range test.seeds {
-				_, err := store.SetTeamResourcePermission(context.Background(), test.orgID, test.teamID, s)
+				_, err := store.SetTeamResourcePermission(context.Background(), test.orgID, test.teamID, s, nil)
 				require.NoError(t, err)
 			}
 
@@ -156,7 +156,7 @@ func TestAccessControlStore_SetTeamResourcePermission(t *testing.T) {
 				Actions:    test.actions,
 				Resource:   test.resource,
 				ResourceID: test.resourceID,
-			})
+			}, nil)
 
 			require.NoError(t, err)
 			if len(test.actions) == 0 {
@@ -226,7 +226,7 @@ func TestAccessControlStore_SetBuiltInResourcePermission(t *testing.T) {
 			store, _ := setupTestEnv(t)
 
 			for _, s := range test.seeds {
-				_, err := store.SetBuiltInResourcePermission(context.Background(), test.orgID, test.builtInRole, s)
+				_, err := store.SetBuiltInResourcePermission(context.Background(), test.orgID, test.builtInRole, s, nil)
 				require.NoError(t, err)
 			}
 
@@ -234,7 +234,7 @@ func TestAccessControlStore_SetBuiltInResourcePermission(t *testing.T) {
 				Actions:    test.actions,
 				Resource:   test.resource,
 				ResourceID: test.resourceID,
-			})
+			}, nil)
 
 			require.NoError(t, err)
 			if len(test.actions) == 0 {
@@ -356,7 +356,7 @@ func seedResourcePermissions(t *testing.T, store *AccessControlStore, sql *sqlst
 			Actions:    actions,
 			Resource:   resource,
 			ResourceID: resourceID,
-		})
+		}, nil)
 		require.NoError(t, err)
 	}
 }

pkg/services/accesscontrol/models.go

@@ -237,6 +237,7 @@ type SetResourcePermissionCommand struct {
 	Actions    []string
 	Resource   string
 	ResourceID string
+	Permission string
 }
 
 type GetResourcesPermissionsQuery struct {

pkg/services/accesscontrol/resourcepermissions/api.go

@@ -88,7 +88,7 @@ func (a *api) getPermissions(c *models.ReqContext) response.Response {
 
 	dto := make([]resourcePermissionDTO, 0, len(permissions))
 	for _, p := range permissions {
-		if permission := a.service.MapActions(p); permission != "" {
+		if permission := a.service.mapActions(p); permission != "" {
 			teamAvatarUrl := ""
 			if p.TeamId != 0 {
 				teamAvatarUrl = dtos.GetGravatarUrlWithDefault(p.TeamEmail, p.Team)
@@ -131,7 +131,7 @@ func (a *api) setUserPermission(c *models.ReqContext) response.Response {
 		return response.Error(http.StatusBadRequest, "bad request data", err)
 	}
 
-	_, err = a.service.SetUserPermission(c.Req.Context(), c.OrgId, userID, resourceID, a.service.MapPermission(cmd.Permission))
+	_, err = a.service.SetUserPermission(c.Req.Context(), c.OrgId, userID, resourceID, cmd.Permission)
 	if err != nil {
 		return response.Error(http.StatusBadRequest, "failed to set user permission", err)
 	}
@@ -151,7 +151,7 @@ func (a *api) setTeamPermission(c *models.ReqContext) response.Response {
 		return response.Error(http.StatusBadRequest, "bad request data", err)
 	}
 
-	_, err = a.service.SetTeamPermission(c.Req.Context(), c.OrgId, teamID, resourceID, a.service.MapPermission(cmd.Permission))
+	_, err = a.service.SetTeamPermission(c.Req.Context(), c.OrgId, teamID, resourceID, cmd.Permission)
 	if err != nil {
 		return response.Error(http.StatusBadRequest, "failed to set team permission", err)
 	}
@@ -168,7 +168,7 @@ func (a *api) setBuiltinRolePermission(c *models.ReqContext) response.Response {
 		return response.Error(http.StatusBadRequest, "bad request data", err)
 	}
 
-	_, err := a.service.SetBuiltInRolePermission(c.Req.Context(), c.OrgId, builtInRole, resourceID, a.service.MapPermission(cmd.Permission))
+	_, err := a.service.SetBuiltInRolePermission(c.Req.Context(), c.OrgId, builtInRole, resourceID, cmd.Permission)
 	if err != nil {
 		return response.Error(http.StatusBadRequest, "failed to set role permission", err)
 	}

pkg/services/accesscontrol/resourcepermissions/api_test.go

@@ -155,16 +155,16 @@ func TestApi_getPermissions(t *testing.T) {
 			// seed team 1 with "Edit" permission on dashboard 1
 			team, err := sql.CreateTeam("test", "test@test.com", 1)
 			require.NoError(t, err)
-			_, err = service.SetTeamPermission(context.Background(), team.OrgId, team.Id, tt.resourceID, []string{"dashboards:read", "dashboards:write", "dashboards:delete"})
+			_, err = service.SetTeamPermission(context.Background(), team.OrgId, team.Id, tt.resourceID, "Edit")
 			require.NoError(t, err)
 			// seed user 1 with "View" permission on dashboard 1
 			u, err := sql.CreateUser(context.Background(), models.CreateUserCommand{Login: "test", OrgId: 1})
 			require.NoError(t, err)
-			_, err = service.SetUserPermission(context.Background(), u.OrgId, u.Id, tt.resourceID, []string{"dashboards:read"})
+			_, err = service.SetUserPermission(context.Background(), u.OrgId, u.Id, tt.resourceID, "View")
 			require.NoError(t, err)
 
-			// seed built in role Admin with "View" permission on dashboard 1
+			// seed built in role Admin with "Edit" permission on dashboard 1
-			_, err = service.SetBuiltInRolePermission(context.Background(), 1, "Admin", tt.resourceID, []string{"dashboards:read", "dashboards:write", "dashboards:delete"})
+			_, err = service.SetBuiltInRolePermission(context.Background(), 1, "Admin", tt.resourceID, "Edit")
 			require.NoError(t, err)
 
 			permissions, recorder := getPermission(t, server, testOptions.Resource, tt.resourceID)

pkg/services/accesscontrol/resourcepermissions/error.go

@@ -3,6 +3,6 @@ package resourcepermissions
 import "errors"
 
 var (
-	ErrInvalidActions    = errors.New("invalid actions")
+	ErrInvalidPermission = errors.New("invalid permission")
 	ErrInvalidAssignment = errors.New("invalid assignment")
 )

pkg/services/accesscontrol/resourcepermissions/options.go

@@ -2,6 +2,8 @@ package resourcepermissions
 
 import (
 	"context"
+
+	"github.com/grafana/grafana/pkg/services/sqlstore"
 )
 
 type ResourceValidator func(ctx context.Context, orgID int64, resourceID string) error
@@ -26,9 +28,9 @@ type Options struct {
 	// RoleGroup is the group name for the generated fixed roles
 	RoleGroup string
 	// OnSetUser if configured will be called each time a permission is set for a user
-	OnSetUser func(ctx context.Context, orgID, userID int64, resourceID, permission string) error
+	OnSetUser func(session *sqlstore.DBSession, orgID, userID int64, resourceID, permission string) error
 	// OnSetTeam if configured will be called each time a permission is set for a team
-	OnSetTeam func(ctx context.Context, orgID, teamID int64, resourceID, permission string) error
+	OnSetTeam func(session *sqlstore.DBSession, orgID, teamID int64, resourceID, permission string) error
 	// OnSetBuiltInRole if configured will be called each time a permission is set for a built-in role
-	OnSetBuiltInRole func(ctx context.Context, orgID int64, builtInRole, resourceID, permission string) error
+	OnSetBuiltInRole func(session *sqlstore.DBSession, orgID int64, builtInRole, resourceID, permission string) error
 }

pkg/services/accesscontrol/resourcepermissions/service.go

@@ -12,13 +12,39 @@ import (
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 )
 
-func New(options Options, router routing.RouteRegister, ac accesscontrol.AccessControl, store accesscontrol.ResourcePermissionsStore, sqlStore *sqlstore.SQLStore) (*Service, error) {
+type Store interface {
+	// SetUserResourcePermission sets permission for managed user role on a resource
+	SetUserResourcePermission(
+		ctx context.Context, orgID, userID int64,
+		cmd accesscontrol.SetResourcePermissionCommand,
+		hook func(session *sqlstore.DBSession, orgID, userID int64, resourceID, permission string) error,
+	) (*accesscontrol.ResourcePermission, error)
+
+	// SetTeamResourcePermission sets permission for managed team role on a resource
+	SetTeamResourcePermission(
+		ctx context.Context, orgID, teamID int64,
+		cmd accesscontrol.SetResourcePermissionCommand,
+		hook func(session *sqlstore.DBSession, orgID, teamID int64, resourceID, permission string) error,
+	) (*accesscontrol.ResourcePermission, error)
+
+	// SetBuiltInResourcePermission sets permissions for managed builtin role on a resource
+	SetBuiltInResourcePermission(
+		ctx context.Context, orgID int64, builtinRole string,
+		cmd accesscontrol.SetResourcePermissionCommand,
+		hook func(session *sqlstore.DBSession, orgID int64, builtInRole, resourceID, permission string) error,
+	) (*accesscontrol.ResourcePermission, error)
+
+	// GetResourcesPermissions will return all permission for all supplied resource ids
+	GetResourcesPermissions(ctx context.Context, orgID int64, query accesscontrol.GetResourcesPermissionsQuery) ([]accesscontrol.ResourcePermission, error)
+}
+
+func New(options Options, router routing.RouteRegister, ac accesscontrol.AccessControl, store Store, sqlStore *sqlstore.SQLStore) (*Service, error) {
 	var permissions []string
-	validActions := make(map[string]struct{})
+	actionSet := make(map[string]struct{})
 	for permission, actions := range options.PermissionsToActions {
 		permissions = append(permissions, permission)
 		for _, a := range actions {
-			validActions[a] = struct{}{}
+			actionSet[a] = struct{}{}
 		}
 	}
 
@@ -27,19 +53,18 @@ func New(options Options, router routing.RouteRegister, ac accesscontrol.AccessC
 		return len(options.PermissionsToActions[permissions[i]]) > len(options.PermissionsToActions[permissions[j]])
 	})
 
-	actions := make([]string, 0, len(validActions))
+	actions := make([]string, 0, len(actionSet))
-	for action := range validActions {
+	for action := range actionSet {
 		actions = append(actions, action)
 	}
 
 	s := &Service{
-		ac:           ac,
+		ac:          ac,
-		store:        store,
+		store:       store,
-		options:      options,
+		options:     options,
-		permissions:  permissions,
+		permissions: permissions,
-		actions:      actions,
+		actions:     actions,
-		validActions: validActions,
+		sqlStore:    sqlStore,
-		sqlStore:     sqlStore,
 	}
 
 	s.api = newApi(ac, router, s)
@@ -56,14 +81,13 @@ func New(options Options, router routing.RouteRegister, ac accesscontrol.AccessC
 // Service is used to create access control sub system including api / and service for managed resource permission
 type Service struct {
 	ac    accesscontrol.AccessControl
-	store accesscontrol.ResourcePermissionsStore
+	store Store
 	api   *api
 
-	options      Options
+	options     Options
-	permissions  []string
+	permissions []string
-	actions      []string
+	actions     []string
-	validActions map[string]struct{}
+	sqlStore    *sqlstore.SQLStore
-	sqlStore     *sqlstore.SQLStore
 }
 
 func (s *Service) GetPermissions(ctx context.Context, orgID int64, resourceID string) ([]accesscontrol.ResourcePermission, error) {
@@ -75,13 +99,14 @@ func (s *Service) GetPermissions(ctx context.Context, orgID int64, resourceID st
 	})
 }
 
-func (s *Service) SetUserPermission(ctx context.Context, orgID, userID int64, resourceID string, actions []string) (*accesscontrol.ResourcePermission, error) {
+func (s *Service) SetUserPermission(ctx context.Context, orgID, userID int64, resourceID, permission string) (*accesscontrol.ResourcePermission, error) {
 	if !s.options.Assignments.Users {
 		return nil, ErrInvalidAssignment
 	}
 
-	if !s.validateActions(actions) {
+	actions, err := s.mapPermission(permission)
-		return nil, ErrInvalidActions
+	if err != nil {
+		return nil, err
 	}
 
 	if err := s.validateResource(ctx, orgID, resourceID); err != nil {
@@ -92,29 +117,22 @@ func (s *Service) SetUserPermission(ctx context.Context, orgID, userID int64, re
 		return nil, err
 	}
 
-	permission, err := s.store.SetUserResourcePermission(ctx, orgID, userID, accesscontrol.SetResourcePermissionCommand{
+	return s.store.SetUserResourcePermission(ctx, orgID, userID, accesscontrol.SetResourcePermissionCommand{
 		Actions:    actions,
+		Permission: permission,
 		ResourceID: resourceID,
 		Resource:   s.options.Resource,
-	})
+	}, s.options.OnSetUser)
-	if err != nil {
-		return nil, err
-	}
-
-	if s.options.OnSetUser != nil {
-		if err := s.options.OnSetUser(ctx, orgID, userID, resourceID, s.MapActions(*permission)); err != nil {
-			return nil, err
-		}
-	}
-	return permission, nil
 }
 
-func (s *Service) SetTeamPermission(ctx context.Context, orgID, teamID int64, resourceID string, actions []string) (*accesscontrol.ResourcePermission, error) {
+func (s *Service) SetTeamPermission(ctx context.Context, orgID, teamID int64, resourceID, permission string) (*accesscontrol.ResourcePermission, error) {
 	if !s.options.Assignments.Teams {
 		return nil, ErrInvalidAssignment
 	}
-	if !s.validateActions(actions) {
+
-		return nil, ErrInvalidActions
+	actions, err := s.mapPermission(permission)
+	if err != nil {
+		return nil, err
 	}
 
 	if err := s.validateTeam(ctx, orgID, teamID); err != nil {
@@ -125,30 +143,22 @@ func (s *Service) SetTeamPermission(ctx context.Context, orgID, teamID int64, re
 		return nil, err
 	}
 
-	permission, err := s.store.SetTeamResourcePermission(ctx, orgID, teamID, accesscontrol.SetResourcePermissionCommand{
+	return s.store.SetTeamResourcePermission(ctx, orgID, teamID, accesscontrol.SetResourcePermissionCommand{
 		Actions:    actions,
+		Permission: permission,
 		ResourceID: resourceID,
 		Resource:   s.options.Resource,
-	})
+	}, s.options.OnSetTeam)
-	if err != nil {
-		return nil, err
-	}
-
-	if s.options.OnSetTeam != nil {
-		if err := s.options.OnSetTeam(ctx, orgID, teamID, resourceID, s.MapActions(*permission)); err != nil {
-			return nil, err
-		}
-	}
-	return permission, nil
 }
 
-func (s *Service) SetBuiltInRolePermission(ctx context.Context, orgID int64, builtInRole string, resourceID string, actions []string) (*accesscontrol.ResourcePermission, error) {
+func (s *Service) SetBuiltInRolePermission(ctx context.Context, orgID int64, builtInRole, resourceID, permission string) (*accesscontrol.ResourcePermission, error) {
 	if !s.options.Assignments.BuiltInRoles {
 		return nil, ErrInvalidAssignment
 	}
 
-	if !s.validateActions(actions) {
+	actions, err := s.mapPermission(permission)
-		return nil, ErrInvalidActions
+	if err != nil {
+		return nil, err
 	}
 
 	if err := s.validateBuiltinRole(ctx, builtInRole); err != nil {
@@ -159,24 +169,15 @@ func (s *Service) SetBuiltInRolePermission(ctx context.Context, orgID int64, bui
 		return nil, err
 	}
 
-	permission, err := s.store.SetBuiltInResourcePermission(ctx, orgID, builtInRole, accesscontrol.SetResourcePermissionCommand{
+	return s.store.SetBuiltInResourcePermission(ctx, orgID, builtInRole, accesscontrol.SetResourcePermissionCommand{
 		Actions:    actions,
+		Permission: permission,
 		ResourceID: resourceID,
 		Resource:   s.options.Resource,
-	})
+	}, s.options.OnSetBuiltInRole)
-	if err != nil {
-		return nil, err
-	}
-
-	if s.options.OnSetBuiltInRole != nil {
-		if err := s.options.OnSetBuiltInRole(ctx, orgID, builtInRole, resourceID, s.MapActions(*permission)); err != nil {
-			return nil, err
-		}
-	}
-	return permission, nil
 }
 
-func (s *Service) MapActions(permission accesscontrol.ResourcePermission) string {
+func (s *Service) mapActions(permission accesscontrol.ResourcePermission) string {
 	for _, p := range s.permissions {
 		if permission.Contains(s.options.PermissionsToActions[p]) {
 			return p
@@ -185,13 +186,17 @@ func (s *Service) MapActions(permission accesscontrol.ResourcePermission) string
 	return ""
 }
 
-func (s *Service) MapPermission(permission string) []string {
+func (s *Service) mapPermission(permission string) ([]string, error) {
+	if permission == "" {
+		return []string{}, nil
+	}
+
 	for k, v := range s.options.PermissionsToActions {
 		if permission == k {
-			return v
+			return v, nil
 		}
 	}
-	return []string{}
+	return nil, ErrInvalidPermission
 }
 
 func (s *Service) validateResource(ctx context.Context, orgID int64, resourceID string) error {
@@ -201,15 +206,6 @@ func (s *Service) validateResource(ctx context.Context, orgID int64, resourceID
 	return nil
 }
 
-func (s *Service) validateActions(actions []string) bool {
-	for _, a := range actions {
-		if _, ok := s.validActions[a]; !ok {
-			return false
-		}
-	}
-	return true
-}
-
 func (s *Service) validateUser(ctx context.Context, orgID, userID int64) error {
 	if err := sqlstore.GetSignedInUser(ctx, &models.GetSignedInUserQuery{OrgId: orgID, UserId: userID}); err != nil {
 		return err

pkg/services/accesscontrol/resourcepermissions/service_test.go

@@ -46,13 +46,13 @@ func TestService_SetUserPermission(t *testing.T) {
 
 			var hookCalled bool
 			if tt.callHook {
-				service.options.OnSetUser = func(ctx context.Context, orgID, userID int64, resourceID, permission string) error {
+				service.options.OnSetUser = func(session *sqlstore.DBSession, orgID, userID int64, resourceID, permission string) error {
 					hookCalled = true
 					return nil
 				}
 			}
 
-			_, err = service.SetUserPermission(context.Background(), user.OrgId, user.Id, "1", []string{})
+			_, err = service.SetUserPermission(context.Background(), user.OrgId, user.Id, "1", "")
 			require.NoError(t, err)
 			assert.Equal(t, tt.callHook, hookCalled)
 		})
@@ -90,13 +90,13 @@ func TestService_SetTeamPermission(t *testing.T) {
 
 			var hookCalled bool
 			if tt.callHook {
-				service.options.OnSetTeam = func(ctx context.Context, orgID, teamID int64, resourceID, permission string) error {
+				service.options.OnSetTeam = func(session *sqlstore.DBSession, orgID, teamID int64, resourceID, permission string) error {
 					hookCalled = true
 					return nil
 				}
 			}
 
-			_, err = service.SetTeamPermission(context.Background(), team.OrgId, team.Id, "1", []string{})
+			_, err = service.SetTeamPermission(context.Background(), team.OrgId, team.Id, "1", "")
 			require.NoError(t, err)
 			assert.Equal(t, tt.callHook, hookCalled)
 		})
@@ -130,13 +130,13 @@ func TestService_SetBuiltInRolePermission(t *testing.T) {
 
 			var hookCalled bool
 			if tt.callHook {
-				service.options.OnSetBuiltInRole = func(ctx context.Context, orgID int64, builtInRole, resourceID, permission string) error {
+				service.options.OnSetBuiltInRole = func(session *sqlstore.DBSession, orgID int64, builtInRole, resourceID, permission string) error {
 					hookCalled = true
 					return nil
 				}
 			}
 
-			_, err := service.SetBuiltInRolePermission(context.Background(), 1, "Viewer", "1", []string{})
+			_, err := service.SetBuiltInRolePermission(context.Background(), 1, "Viewer", "1", "")
 			require.NoError(t, err)
 			assert.Equal(t, tt.callHook, hookCalled)
 		})