diff --git a/pkg/api/folder_bench_test.go b/pkg/api/folder_bench_test.go index a5bef4a4506..b53c9098d01 100644 --- a/pkg/api/folder_bench_test.go +++ b/pkg/api/folder_bench_test.go @@ -25,6 +25,7 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" 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/contexthandler/ctxkey" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/dashboards" @@ -459,10 +460,10 @@ func setupServer(b testing.TB, sc benchScenario, features featuremgmt.FeatureTog cfg := setting.NewCfg() folderPermissions, err := ossaccesscontrol.ProvideFolderPermissions( - cfg, features, routing.NewRouteRegister(), sc.db, ac, license, &dashboards.FakeDashboardStore{}, folderServiceWithFlagOn, acSvc, sc.teamSvc, sc.userSvc) + cfg, features, routing.NewRouteRegister(), sc.db, ac, license, &dashboards.FakeDashboardStore{}, folderServiceWithFlagOn, acSvc, sc.teamSvc, sc.userSvc, resourcepermissions.NewActionSetService()) require.NoError(b, err) dashboardPermissions, err := ossaccesscontrol.ProvideDashboardPermissions( - cfg, features, routing.NewRouteRegister(), sc.db, ac, license, &dashboards.FakeDashboardStore{}, folderServiceWithFlagOn, acSvc, sc.teamSvc, sc.userSvc) + cfg, features, routing.NewRouteRegister(), sc.db, ac, license, &dashboards.FakeDashboardStore{}, folderServiceWithFlagOn, acSvc, sc.teamSvc, sc.userSvc, resourcepermissions.NewActionSetService()) require.NoError(b, err) dashboardSvc, err := dashboardservice.ProvideDashboardServiceImpl( diff --git a/pkg/server/wireexts_oss.go b/pkg/server/wireexts_oss.go index 6b9cb1407b1..fc625ce30db 100644 --- a/pkg/server/wireexts_oss.go +++ b/pkg/server/wireexts_oss.go @@ -16,6 +16,7 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" "github.com/grafana/grafana/pkg/services/accesscontrol/ossaccesscontrol" + "github.com/grafana/grafana/pkg/services/accesscontrol/resourcepermissions" "github.com/grafana/grafana/pkg/services/anonymous" "github.com/grafana/grafana/pkg/services/anonymous/anonimpl" "github.com/grafana/grafana/pkg/services/apiserver/standalone" @@ -102,6 +103,7 @@ var wireExtsBasicSet = wire.NewSet( wire.Bind(new(auth.IDSigner), new(*idimpl.LocalSigner)), manager.ProvideInstaller, wire.Bind(new(plugins.Installer), new(*manager.PluginInstaller)), + resourcepermissions.NewActionSetService, ) var wireExtsSet = wire.NewSet( diff --git a/pkg/services/accesscontrol/database/database_test.go b/pkg/services/accesscontrol/database/database_test.go index ba0f0f82793..4c3c2e78aca 100644 --- a/pkg/services/accesscontrol/database/database_test.go +++ b/pkg/services/accesscontrol/database/database_test.go @@ -11,7 +11,6 @@ import ( "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/localcache" - "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/accesscontrol" rs "github.com/grafana/grafana/pkg/services/accesscontrol/resourcepermissions" "github.com/grafana/grafana/pkg/services/auth/identity" @@ -394,8 +393,7 @@ func setupTestEnv(t testing.TB) (*AccessControlStore, rs.Store, user.Service, te cfg.AutoAssignOrgRole = "Viewer" cfg.AutoAssignOrgId = 1 acstore := ProvideService(sql) - log := log.New("test") - asService := rs.NewInMemoryActionSets(log) + asService := rs.NewActionSetService() permissionStore := rs.NewStore(sql, featuremgmt.WithFeatures(), &asService) teamService, err := teamimpl.ProvideService(sql, cfg) require.NoError(t, err) diff --git a/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go b/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go index f3910860cb0..693efe7053f 100644 --- a/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go +++ b/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go @@ -47,7 +47,7 @@ var ( func ProvideTeamPermissions( cfg *setting.Cfg, features featuremgmt.FeatureToggles, router routing.RouteRegister, sql db.DB, ac accesscontrol.AccessControl, license licensing.Licensing, service accesscontrol.Service, - teamService team.Service, userService user.Service, + teamService team.Service, userService user.Service, actionSetService resourcepermissions.ActionSetService, ) (*TeamPermissionsService, error) { options := resourcepermissions.Options{ Resource: "teams", @@ -103,7 +103,7 @@ func ProvideTeamPermissions( }, } - srv, err := resourcepermissions.New(cfg, options, features, router, license, ac, service, sql, teamService, userService) + srv, err := resourcepermissions.New(cfg, options, features, router, license, ac, service, sql, teamService, userService, actionSetService) if err != nil { return nil, err } @@ -142,7 +142,7 @@ func getDashboardAdminActions(features featuremgmt.FeatureToggles) []string { func ProvideDashboardPermissions( cfg *setting.Cfg, features featuremgmt.FeatureToggles, router routing.RouteRegister, sql db.DB, ac accesscontrol.AccessControl, license licensing.Licensing, dashboardStore dashboards.Store, folderService folder.Service, service accesscontrol.Service, - teamService team.Service, userService user.Service, + teamService team.Service, userService user.Service, actionSetService resourcepermissions.ActionSetService, ) (*DashboardPermissionsService, error) { getDashboard := func(ctx context.Context, orgID int64, resourceID string) (*dashboards.Dashboard, error) { query := &dashboards.GetDashboardQuery{UID: resourceID, OrgID: orgID} @@ -207,7 +207,7 @@ func ProvideDashboardPermissions( RoleGroup: "Dashboards", } - srv, err := resourcepermissions.New(cfg, options, features, router, license, ac, service, sql, teamService, userService) + srv, err := resourcepermissions.New(cfg, options, features, router, license, ac, service, sql, teamService, userService, actionSetService) if err != nil { return nil, err } @@ -237,7 +237,7 @@ var FolderAdminActions = append(FolderEditActions, []string{dashboards.ActionFol func ProvideFolderPermissions( cfg *setting.Cfg, features featuremgmt.FeatureToggles, router routing.RouteRegister, sql db.DB, accesscontrol accesscontrol.AccessControl, license licensing.Licensing, dashboardStore dashboards.Store, folderService folder.Service, service accesscontrol.Service, - teamService team.Service, userService user.Service, + teamService team.Service, userService user.Service, actionSetService resourcepermissions.ActionSetService, ) (*FolderPermissionsService, error) { options := resourcepermissions.Options{ Resource: "folders", @@ -273,7 +273,7 @@ func ProvideFolderPermissions( WriterRoleName: "Folder permission writer", RoleGroup: "Folders", } - srv, err := resourcepermissions.New(cfg, options, features, router, license, accesscontrol, service, sql, teamService, userService) + srv, err := resourcepermissions.New(cfg, options, features, router, license, accesscontrol, service, sql, teamService, userService, actionSetService) if err != nil { return nil, err } @@ -337,7 +337,7 @@ type ServiceAccountPermissionsService struct { func ProvideServiceAccountPermissions( cfg *setting.Cfg, features featuremgmt.FeatureToggles, router routing.RouteRegister, sql db.DB, ac accesscontrol.AccessControl, license licensing.Licensing, serviceAccountRetrieverService *retriever.Service, service accesscontrol.Service, - teamService team.Service, userService user.Service, + teamService team.Service, userService user.Service, actionSetService resourcepermissions.ActionSetService, ) (*ServiceAccountPermissionsService, error) { options := resourcepermissions.Options{ Resource: "serviceaccounts", @@ -364,7 +364,7 @@ func ProvideServiceAccountPermissions( RoleGroup: "Service accounts", } - srv, err := resourcepermissions.New(cfg, options, features, router, license, ac, service, sql, teamService, userService) + srv, err := resourcepermissions.New(cfg, options, features, router, license, ac, service, sql, teamService, userService, actionSetService) if err != nil { return nil, err } diff --git a/pkg/services/accesscontrol/resourcepermissions/service.go b/pkg/services/accesscontrol/resourcepermissions/service.go index d4e73a74596..9c69645c7e3 100644 --- a/pkg/services/accesscontrol/resourcepermissions/service.go +++ b/pkg/services/accesscontrol/resourcepermissions/service.go @@ -7,7 +7,6 @@ import ( "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/infra/db" - "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/auth/identity" "github.com/grafana/grafana/pkg/services/featuremgmt" @@ -57,13 +56,8 @@ type Store interface { func New(cfg *setting.Cfg, options Options, features featuremgmt.FeatureToggles, router routing.RouteRegister, license licensing.Licensing, ac accesscontrol.AccessControl, service accesscontrol.Service, sqlStore db.DB, - teamService team.Service, userService user.Service, + teamService team.Service, userService user.Service, actionSetService ActionSetService, ) (*Service, error) { - // TODO: add log as a dependency - // TODO: add actionsetstore from wire - log := log.New("accesscontrol.resourcepermissions") - actionSetsStore := NewInMemoryActionSets(log) - permissions := make([]string, 0, len(options.PermissionsToActions)) actionSet := make(map[string]struct{}) for permission, actions := range options.PermissionsToActions { @@ -71,12 +65,7 @@ func New(cfg *setting.Cfg, for _, a := range actions { actionSet[a] = struct{}{} } - // storing the actionset - err := actionSetsStore.StoreActionSet(options.Resource, permission, actions) - if err != nil { - log.Warn("failed to store action set", "error", err) - return nil, err - } + actionSetService.StoreActionSet(options.Resource, permission, actions) } // Sort all permissions based on action length. Will be used when mapping between actions to permissions @@ -91,7 +80,7 @@ func New(cfg *setting.Cfg, s := &Service{ ac: ac, - store: NewStore(sqlStore, features, &actionSetsStore), + store: NewStore(sqlStore, features, &actionSetService), options: options, license: license, permissions: permissions, diff --git a/pkg/services/accesscontrol/resourcepermissions/service_test.go b/pkg/services/accesscontrol/resourcepermissions/service_test.go index 1cae9fa0b69..8aa2108dc2d 100644 --- a/pkg/services/accesscontrol/resourcepermissions/service_test.go +++ b/pkg/services/accesscontrol/resourcepermissions/service_test.go @@ -246,7 +246,7 @@ func setupTestEnvironment(t *testing.T, ops Options) (*Service, db.DB, *setting. acService := &actest.FakeService{} service, err := New( cfg, ops, featuremgmt.WithFeatures(), routing.NewRouteRegister(), license, - ac, acService, sql, teamSvc, userSvc, + ac, acService, sql, teamSvc, userSvc, NewActionSetService(), ) require.NoError(t, err) diff --git a/pkg/services/accesscontrol/resourcepermissions/store.go b/pkg/services/accesscontrol/resourcepermissions/store.go index 1ea1995f1d5..a2807314e06 100644 --- a/pkg/services/accesscontrol/resourcepermissions/store.go +++ b/pkg/services/accesscontrol/resourcepermissions/store.go @@ -3,7 +3,6 @@ package resourcepermissions import ( "context" "fmt" - "slices" "strings" "time" @@ -678,11 +677,7 @@ func (s *store) createPermissions(sess *db.Session, roleID int64, resource, reso Add ACTION SET of managed permissions to in-memory store */ if s.features.IsEnabled(context.TODO(), featuremgmt.FlagAccessActionSets) { - // FIXME: make this only one resource of view, editor, admin - actionSetName, err := s.actionSetService.GetActionSetName(resource, permission) - if err != nil { - return err - } + actionSetName := s.actionSetService.GetActionSetName(resource, permission) p := managedPermission(actionSetName, resource, resourceID, resourceAttribute) p.RoleID = roleID p.Created = time.Now() @@ -740,18 +735,10 @@ actionSet := &ActionSet{ }` */ -type ActionSetGetter interface { - GetActionSet(actionName string) []string - GetActionSetName(resource, permission string) (string, error) -} - -type ActionSetStorer interface { - StoreActionSet(resource, permission string, actions []string) error -} - type ActionSetService interface { - ActionSetGetter - ActionSetStorer + GetActionSet(actionName string) []string + GetActionSetName(resource, permission string) string + StoreActionSet(resource, permission string, actions []string) } type ActionSet struct { @@ -765,11 +752,11 @@ type InMemoryActionSets struct { actionSets map[string][]string } -// NewInMemoryActionSets returns a new instance of InMemoryActionSetService. -func NewInMemoryActionSets(log log.Logger) ActionSetService { +// NewActionSetService returns a new instance of InMemoryActionSetService. +func NewActionSetService() ActionSetService { return &InMemoryActionSets{ actionSets: make(map[string][]string), - log: log, + log: log.New("resourcepermissions.actionsets"), } } @@ -782,33 +769,22 @@ func (s *InMemoryActionSets) GetActionSet(actionName string) []string { return actionSet } -func (s *InMemoryActionSets) StoreActionSet(resource, permission string, actions []string) error { +func (s *InMemoryActionSets) StoreActionSet(resource, permission string, actions []string) { s.log.Debug("storing action set\n") - name, err := s.GetActionSetName(resource, permission) - if err != nil { - return err - } + name := s.GetActionSetName(resource, permission) actionSet := &ActionSet{ // folders:edit Action: name, Actions: actions, } - // TODO: Do we only store the actions, or all of the information about the action set s.actionSets[actionSet.Action] = actions s.log.Debug("stored action set actionname \n", actionSet.Action) - return nil } // GetActionSetName function creates an action set from a list of actions and stores it inmemory. -func (s *InMemoryActionSets) GetActionSetName(resource, permission string) (string, error) { +func (s *InMemoryActionSets) GetActionSetName(resource, permission string) string { // lower cased resource = strings.ToLower(resource) permission = strings.ToLower(permission) - - // TODO: should we also whitelist permissions here? - allowedPermissions := []string{"admin", "edit", "editor", "view", "query", "member"} - if !slices.Contains(allowedPermissions, permission) { - return "", fmt.Errorf("%s not allowed permission", permission) - } - return fmt.Sprintf("%s:%s", resource, permission), nil + return fmt.Sprintf("%s:%s", resource, permission) } diff --git a/pkg/services/accesscontrol/resourcepermissions/store_test.go b/pkg/services/accesscontrol/resourcepermissions/store_test.go index 5bc6b6f210d..d205f550b05 100644 --- a/pkg/services/accesscontrol/resourcepermissions/store_test.go +++ b/pkg/services/accesscontrol/resourcepermissions/store_test.go @@ -10,7 +10,6 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/infra/db" - "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/featuremgmt" @@ -560,8 +559,7 @@ func seedResourcePermissions( func setupTestEnv(t testing.TB) (*store, db.DB, *setting.Cfg) { sql := db.InitTestDB(t) - log := log.New("test") - asService := NewInMemoryActionSets(log) + asService := NewActionSetService() return NewStore(sql, featuremgmt.WithFeatures(), &asService), sql, sql.Cfg } diff --git a/pkg/tests/api/alerting/api_backtesting_test.go b/pkg/tests/api/alerting/api_backtesting_test.go index 2ada906f401..716ba6385f7 100644 --- a/pkg/tests/api/alerting/api_backtesting_test.go +++ b/pkg/tests/api/alerting/api_backtesting_test.go @@ -108,7 +108,7 @@ func TestBacktesting(t *testing.T) { }) // access control permissions store - permissionsStore := resourcepermissions.NewStore(env.SQLStore, featuremgmt.WithFeatures(), resourcepermissions.NewInMemoryActionSets(env.Cfg.Logger)) + permissionsStore := resourcepermissions.NewStore(env.SQLStore, featuremgmt.WithFeatures(), resourcepermissions.NewActionSetService(env.Cfg.Logger)) _, err := permissionsStore.SetUserResourcePermission(context.Background(), accesscontrol.GlobalOrgID, accesscontrol.User{ID: testUserId}, diff --git a/pkg/tests/api/alerting/api_prometheus_test.go b/pkg/tests/api/alerting/api_prometheus_test.go index 87d8434165b..ba66033cfc0 100644 --- a/pkg/tests/api/alerting/api_prometheus_test.go +++ b/pkg/tests/api/alerting/api_prometheus_test.go @@ -668,8 +668,9 @@ func TestIntegrationPrometheusRulesPermissions(t *testing.T) { apiClient := newAlertingApiClient(grafanaListedAddr, "grafana", "password") + asService := resourcepermissions.NewActionSetService(env.Cfg.Logger) // access control permissions store - permissionsStore := resourcepermissions.NewStore(env.SQLStore, featuremgmt.WithFeatures(), resourcepermissions.NewInMemoryActionSets(env.Cfg.Logger)) + permissionsStore := resourcepermissions.NewStore(env.SQLStore, featuremgmt.WithFeatures(), &asService) // Create the namespace we'll save our alerts to. apiClient.CreateFolder(t, "folder1", "folder1") diff --git a/pkg/tests/api/alerting/api_ruler_test.go b/pkg/tests/api/alerting/api_ruler_test.go index 9b1f50108a4..33781a74f91 100644 --- a/pkg/tests/api/alerting/api_ruler_test.go +++ b/pkg/tests/api/alerting/api_ruler_test.go @@ -52,7 +52,8 @@ func TestIntegrationAlertRulePermissions(t *testing.T) { }) grafanaListedAddr, env := testinfra.StartGrafanaEnv(t, dir, p) - permissionsStore := resourcepermissions.NewStore(env.SQLStore, featuremgmt.WithFeatures(), resourcepermissions.NewInMemoryActionSets(env.Cfg.Logger)) + asService := resourcepermissions.NewActionSetService() + permissionsStore := resourcepermissions.NewStore(env.SQLStore, featuremgmt.WithFeatures(), &asService) // Create a user to make authenticated requests userID := createUser(t, env.SQLStore, env.Cfg, user.CreateUserCommand{ @@ -336,7 +337,8 @@ func TestIntegrationAlertRuleNestedPermissions(t *testing.T) { }) grafanaListedAddr, env := testinfra.StartGrafanaEnv(t, dir, p) - permissionsStore := resourcepermissions.NewStore(env.SQLStore, featuremgmt.WithFeatures(), resourcepermissions.NewInMemoryActionSets(env.Cfg.Logger)) + asService := resourcepermissions.NewActionSetService() + permissionsStore := resourcepermissions.NewStore(env.SQLStore, featuremgmt.WithFeatures(), &asService) // Create a user to make authenticated requests userID := createUser(t, env.SQLStore, env.Cfg, user.CreateUserCommand{ @@ -732,7 +734,8 @@ func TestAlertRulePostExport(t *testing.T) { }) grafanaListedAddr, env := testinfra.StartGrafanaEnv(t, dir, p) - permissionsStore := resourcepermissions.NewStore(env.SQLStore, featuremgmt.WithFeatures(), resourcepermissions.NewInMemoryActionSets(env.Cfg.Logger)) + asService := resourcepermissions.NewActionSetService() + permissionsStore := resourcepermissions.NewStore(env.SQLStore, featuremgmt.WithFeatures(), &asService) // Create a user to make authenticated requests userID := createUser(t, env.SQLStore, env.Cfg, user.CreateUserCommand{ @@ -1412,7 +1415,8 @@ func TestIntegrationRuleUpdate(t *testing.T) { AppModeProduction: true, }) grafanaListedAddr, env := testinfra.StartGrafanaEnv(t, dir, path) - permissionsStore := resourcepermissions.NewStore(env.SQLStore, featuremgmt.WithFeatures(), resourcepermissions.NewInMemoryActionSets(env.Cfg.Logger)) + asService := resourcepermissions.NewActionSetService() + permissionsStore := resourcepermissions.NewStore(env.SQLStore, featuremgmt.WithFeatures(), &asService) // Create a user to make authenticated requests userID := createUser(t, env.SQLStore, env.Cfg, user.CreateUserCommand{ diff --git a/pkg/tests/api/alerting/api_testing_test.go b/pkg/tests/api/alerting/api_testing_test.go index f0d4ef96a24..138be67213d 100644 --- a/pkg/tests/api/alerting/api_testing_test.go +++ b/pkg/tests/api/alerting/api_testing_test.go @@ -275,7 +275,8 @@ func TestGrafanaRuleConfig(t *testing.T) { }) // access control permissions store - permissionsStore := resourcepermissions.NewStore(env.SQLStore, featuremgmt.WithFeatures(), resourcepermissions.NewInMemoryActionSets(env.Cfg.Logger)) + asService := resourcepermissions.NewActionSetService() + permissionsStore := resourcepermissions.NewStore(env.SQLStore, featuremgmt.WithFeatures(), &asService) _, err := permissionsStore.SetUserResourcePermission(context.Background(), accesscontrol.GlobalOrgID, accesscontrol.User{ID: testUserId}, diff --git a/pkg/tests/api/folders/api_folders_test.go b/pkg/tests/api/folders/api_folders_test.go index fa2ceaad403..f42d1dcb112 100644 --- a/pkg/tests/api/folders/api_folders_test.go +++ b/pkg/tests/api/folders/api_folders_test.go @@ -65,7 +65,8 @@ func TestGetFolders(t *testing.T) { viewerClient := tests.GetClient(grafanaListedAddr, "viewer", "viewer") // access control permissions store - permissionsStore := resourcepermissions.NewStore(store, featuremgmt.WithFeatures(), resourcepermissions.NewInMemoryActionSets(cfg.Logger)) + actionSetService := resourcepermissions.NewActionSetService() + permissionsStore := resourcepermissions.NewStore(store, featuremgmt.WithFeatures(), &actionSetService) numberOfFolders := 5 indexWithoutPermission := 3