From b4ff398865f1fb3b3fcd7b09946a83ce12429e45 Mon Sep 17 00:00:00 2001 From: Alexander Akhmetov Date: Fri, 22 Aug 2025 13:15:53 +0200 Subject: [PATCH] Alerting: Fix folder permissions for Editor role in Prometheus import (#109977) Alerting: Fix folder permisisons for Editor role in Prometheus import --- .../ngalert/api/api_convert_prometheus.go | 22 +++++++- pkg/services/ngalert/api/persist.go | 2 +- pkg/services/ngalert/store/namespace.go | 16 +++--- pkg/services/ngalert/store/namespace_test.go | 3 +- pkg/services/ngalert/tests/fakes/rules.go | 6 +-- .../alerting/api_convert_prometheus_test.go | 51 +++++++++++++++++++ 6 files changed, 87 insertions(+), 13 deletions(-) diff --git a/pkg/services/ngalert/api/api_convert_prometheus.go b/pkg/services/ngalert/api/api_convert_prometheus.go index d6fb86d31e2..72ea84396ae 100644 --- a/pkg/services/ngalert/api/api_convert_prometheus.go +++ b/pkg/services/ngalert/api/api_convert_prometheus.go @@ -461,7 +461,7 @@ func (srv *ConvertPrometheusSrv) RouteConvertPrometheusPostRuleGroups(c *context func (srv *ConvertPrometheusSrv) getOrCreateNamespace(c *contextmodel.ReqContext, title string, logger log.Logger, workingFolderUID string) (*folder.FolderReference, response.Response) { logger.Debug("Getting or creating a new folder") - ns, err := srv.ruleStore.GetOrCreateNamespaceByTitle( + ns, created, err := srv.ruleStore.GetOrCreateNamespaceByTitle( c.Req.Context(), title, c.GetOrgID(), @@ -473,6 +473,26 @@ func (srv *ConvertPrometheusSrv) getOrCreateNamespace(c *contextmodel.ReqContext return nil, namespaceErrorResponse(err) } + // Not all users have global-scoped permissions, even if they can create folders. + // For example, Editor users can create folders, but they have UID-scoped folder permissions. + // Permissions are populated in a middleware before this handler, and the folder we just created + // is not included in the permissions yet. We add it manually. + if created { + orgID := c.GetOrgID() + if c.Permissions == nil { + c.Permissions = make(map[int64]map[string][]string) + } + if c.Permissions[orgID] == nil { + c.Permissions[orgID] = make(map[string][]string) + } + + folderScope := dashboards.ScopeFoldersProvider.GetResourceScopeUID(ns.UID) + if c.Permissions[orgID][dashboards.ActionFoldersRead] == nil { + c.Permissions[orgID][dashboards.ActionFoldersRead] = []string{} + } + c.Permissions[orgID][dashboards.ActionFoldersRead] = append(c.Permissions[orgID][dashboards.ActionFoldersRead], folderScope) + } + logger.Debug("Using folder for the converted rules", "folder_uid", ns.UID) return ns, nil diff --git a/pkg/services/ngalert/api/persist.go b/pkg/services/ngalert/api/persist.go index 5a60c2e0637..a3eb9af2b7c 100644 --- a/pkg/services/ngalert/api/persist.go +++ b/pkg/services/ngalert/api/persist.go @@ -16,7 +16,7 @@ type RuleStore interface { GetUserVisibleNamespaces(context.Context, int64, identity.Requester) (map[string]*folder.Folder, error) GetNamespaceByUID(ctx context.Context, uid string, orgID int64, user identity.Requester) (*folder.Folder, error) GetNamespaceByTitle(ctx context.Context, fullpath string, orgID int64, user identity.Requester, parentUID string) (*folder.FolderReference, error) - GetOrCreateNamespaceByTitle(ctx context.Context, title string, orgID int64, user identity.Requester, parentUID string) (*folder.FolderReference, error) + GetOrCreateNamespaceByTitle(ctx context.Context, title string, orgID int64, user identity.Requester, parentUID string) (*folder.FolderReference, bool, error) // GetNamespaceChildren returns all children (first level) of the namespace with the given id. GetNamespaceChildren(ctx context.Context, uid string, orgID int64, user identity.Requester) ([]*folder.FolderReference, error) diff --git a/pkg/services/ngalert/store/namespace.go b/pkg/services/ngalert/store/namespace.go index 2e28ee94ba6..645f72cbd70 100644 --- a/pkg/services/ngalert/store/namespace.go +++ b/pkg/services/ngalert/store/namespace.go @@ -95,24 +95,25 @@ func (st DBstore) GetNamespaceByTitle(ctx context.Context, title string, orgID i // // To avoid race conditions when two concurrent requests try to create the same folder, // we create folders with a deterministic UID based on the parent UID, title, and organization ID. -func (st DBstore) GetOrCreateNamespaceByTitle(ctx context.Context, title string, orgID int64, user identity.Requester, parentUID string) (*folder.FolderReference, error) { +func (st DBstore) GetOrCreateNamespaceByTitle(ctx context.Context, title string, orgID int64, user identity.Requester, parentUID string) (*folder.FolderReference, bool, error) { if len(title) == 0 { - return nil, fmt.Errorf("title is empty") + return nil, false, fmt.Errorf("title is empty") } var f *folder.FolderReference var err error + var created bool f, err = st.GetNamespaceByTitle(ctx, title, orgID, user, parentUID) if err != nil && !errors.Is(err, dashboards.ErrFolderNotFound) { - return nil, err + return nil, false, err } if f == nil { // Generate a deterministic UID with an alerting prefix uid, err := generateAlertingFolderUID(title, parentUID, orgID) if err != nil { - return nil, fmt.Errorf("error creating a new folder: %w", err) + return nil, false, fmt.Errorf("error creating a new folder: %w", err) } cmd := &folder.CreateFolderCommand{ @@ -129,20 +130,21 @@ func (st DBstore) GetOrCreateNamespaceByTitle(ctx context.Context, title string, // the folder between our check and creation attempt existingFolder, lookupErr := st.GetNamespaceByTitle(ctx, title, orgID, user, parentUID) if lookupErr == nil { - return existingFolder, nil + return existingFolder, false, nil } // If we couldn't find it, return errors - return nil, fmt.Errorf("failed to get or create folder: %w", errors.Join( + return nil, false, fmt.Errorf("failed to get or create folder: %w", errors.Join( fmt.Errorf("create folder: %w", err), fmt.Errorf("lookup folder: %w", lookupErr), )) } f = newFolder.ToFolderReference() + created = true } - return f, nil + return f, created, nil } // generateAlertingFolderUID creates a deterministic UID for folders diff --git a/pkg/services/ngalert/store/namespace_test.go b/pkg/services/ngalert/store/namespace_test.go index 03fea1b514e..fd17edec7b3 100644 --- a/pkg/services/ngalert/store/namespace_test.go +++ b/pkg/services/ngalert/store/namespace_test.go @@ -78,7 +78,8 @@ func TestGetNamespaceByTitle(t *testing.T) { func TestGetOrCreateNamespaceByTitle(t *testing.T) { store := DBstore{} - _, err := store.GetOrCreateNamespaceByTitle(context.Background(), "", 1, nil, folder.RootFolderUID) + _, created, err := store.GetOrCreateNamespaceByTitle(context.Background(), "", 1, nil, folder.RootFolderUID) + require.False(t, created) require.Error(t, err) require.Contains(t, err.Error(), "title is empty") diff --git a/pkg/services/ngalert/tests/fakes/rules.go b/pkg/services/ngalert/tests/fakes/rules.go index c17c70bf6c9..8e7d92666a5 100644 --- a/pkg/services/ngalert/tests/fakes/rules.go +++ b/pkg/services/ngalert/tests/fakes/rules.go @@ -363,13 +363,13 @@ func (f *RuleStore) GetNamespaceByUID(_ context.Context, uid string, orgID int64 return nil, dashboards.ErrFolderNotFound } -func (f *RuleStore) GetOrCreateNamespaceByTitle(ctx context.Context, title string, orgID int64, user identity.Requester, parentUID string) (*folder.FolderReference, error) { +func (f *RuleStore) GetOrCreateNamespaceByTitle(ctx context.Context, title string, orgID int64, user identity.Requester, parentUID string) (*folder.FolderReference, bool, error) { f.mtx.Lock() defer f.mtx.Unlock() for _, folder := range f.Folders[orgID] { if folder.Title == title && folder.ParentUID == parentUID { - return folder.ToFolderReference(), nil + return folder.ToFolderReference(), false, nil } } @@ -382,7 +382,7 @@ func (f *RuleStore) GetOrCreateNamespaceByTitle(ctx context.Context, title strin } f.Folders[orgID] = append(f.Folders[orgID], newFolder) - return newFolder.ToFolderReference(), nil + return newFolder.ToFolderReference(), true, nil } func (f *RuleStore) GetNamespaceByTitle(ctx context.Context, title string, orgID int64, user identity.Requester, parentUID string) (*folder.FolderReference, error) { diff --git a/pkg/tests/api/alerting/api_convert_prometheus_test.go b/pkg/tests/api/alerting/api_convert_prometheus_test.go index 1b5ab0145dc..4d6a319c90d 100644 --- a/pkg/tests/api/alerting/api_convert_prometheus_test.go +++ b/pkg/tests/api/alerting/api_convert_prometheus_test.go @@ -1167,6 +1167,57 @@ func TestIntegrationConvertPrometheusEndpoints_Delete(t *testing.T) { }) } +func TestIntegrationConvertPrometheusEndpoints_Editor(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + testinfra.SQLiteIntegrationTest(t) + + dir, gpath := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{ + DisableLegacyAlerting: true, + EnableUnifiedAlerting: true, + DisableAnonymous: true, + AppModeProduction: true, + EnableRecordingRules: true, + }) + + grafanaListedAddr, env := testinfra.StartGrafanaEnv(t, dir, gpath) + + adminClient := newAlertingApiClient(grafanaListedAddr, "admin", "admin") + + createUser(t, env.SQLStore, env.Cfg, user.CreateUserCommand{ + DefaultOrgRole: string(org.RoleEditor), + Password: "password", + Login: "editor", + }) + editorClient := newAlertingApiClient(grafanaListedAddr, "editor", "password") + + ds := adminClient.CreateDatasource(t, datasources.DS_PROMETHEUS) + + testGroup := apimodels.PrometheusRuleGroup{ + Name: "test-group-permission-cache", + Interval: prommodel.Duration(60 * time.Second), + Rules: []apimodels.PrometheusRule{ + { + Alert: "test-editor-permissions", + Expr: "vector(0)", + For: util.Pointer(prommodel.Duration(1 * time.Minute)), + }, + }, + } + + ns := util.GenerateShortUID() + + t.Run("editor can import rules that create new folder", func(t *testing.T) { + editorClient.ConvertPrometheusPostRuleGroup(t, ns, ds.Body.Datasource.UID, testGroup, nil) + + group := editorClient.ConvertPrometheusGetRuleGroupRules(t, ns, testGroup.Name, nil) + require.Equal(t, testGroup.Name, group.Name) + require.Len(t, group.Rules, 1) + require.Equal(t, testGroup.Rules[0].Alert, group.Rules[0].Alert) + }) +} + func TestIntegrationConvertPrometheusEndpoints_GroupLabels(t *testing.T) { if testing.Short() { t.Skip("skipping integration test in short mode")