From be29f3d05f9320938d1014546fd35be63c21614e Mon Sep 17 00:00:00 2001 From: Sofia Papagiannaki <1632407+papagian@users.noreply.github.com> Date: Wed, 2 Aug 2023 15:06:38 +0300 Subject: [PATCH] [v10.1.x] Nested folders: Fix search query for empty self-contained permissions (#72733) Nested folders: Fix search query for empty self-contained permissions (#72727) * Add tests * Fix query for nested folders with zero self-contained permissions * Fix query behind permissionsFilterRemoveSubquery flag * Apply suggestion from code review (cherry picked from commit 8a24e891fe49e82a45af61179cbb5ab0ae2771be) --- .../sqlstore/permissions/dashboard.go | 53 ++++++++------- .../sqlstore/permissions/dashboard_test.go | 66 +++++++++++++++++++ 2 files changed, 97 insertions(+), 22 deletions(-) diff --git a/pkg/services/sqlstore/permissions/dashboard.go b/pkg/services/sqlstore/permissions/dashboard.go index 85a00c5cfed..ea1caf32b84 100644 --- a/pkg/services/sqlstore/permissions/dashboard.go +++ b/pkg/services/sqlstore/permissions/dashboard.go @@ -171,17 +171,22 @@ func (f *accessControlDashboardPermissionFilter) buildClauses() { switch f.features.IsEnabled(featuremgmt.FlagNestedFolders) { case true: - switch f.recursiveQueriesAreSupported { - case true: - recQueryName := fmt.Sprintf("RecQry%d", len(f.recQueries)) - f.addRecQry(recQueryName, permSelector.String(), permSelectorArgs) + if len(permSelectorArgs) > 0 { + switch f.recursiveQueriesAreSupported { + case true: + builder.WriteString("(dashboard.folder_id IN (SELECT d.id FROM dashboard as d ") + recQueryName := fmt.Sprintf("RecQry%d", len(f.recQueries)) + f.addRecQry(recQueryName, permSelector.String(), permSelectorArgs) + builder.WriteString(fmt.Sprintf("WHERE d.uid IN (SELECT uid FROM %s)", recQueryName)) + default: + nestedFoldersSelectors, nestedFoldersArgs := nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "folder_id", "id") + builder.WriteRune('(') + builder.WriteString(nestedFoldersSelectors) + args = append(args, nestedFoldersArgs...) + } + } else { builder.WriteString("(dashboard.folder_id IN (SELECT d.id FROM dashboard as d ") - builder.WriteString(fmt.Sprintf("WHERE d.uid IN (SELECT uid FROM %s)", recQueryName)) - default: - nestedFoldersSelectors, nestedFoldersArgs := nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "folder_id", "id") - builder.WriteRune('(') - builder.WriteString(nestedFoldersSelectors) - args = append(args, nestedFoldersArgs...) + builder.WriteString("WHERE 1 = 0") } default: builder.WriteString("(dashboard.folder_id IN (SELECT d.id FROM dashboard as d ") @@ -238,18 +243,22 @@ func (f *accessControlDashboardPermissionFilter) buildClauses() { switch f.features.IsEnabled(featuremgmt.FlagNestedFolders) { case true: - switch f.recursiveQueriesAreSupported { - case true: - recQueryName := fmt.Sprintf("RecQry%d", len(f.recQueries)) - f.addRecQry(recQueryName, permSelector.String(), permSelectorArgs) - builder.WriteString("(dashboard.uid IN ") - builder.WriteString(fmt.Sprintf("(SELECT uid FROM %s)", recQueryName)) - default: - nestedFoldersSelectors, nestedFoldersArgs := nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "uid", "uid") - builder.WriteRune('(') - builder.WriteString(nestedFoldersSelectors) - builder.WriteRune(')') - args = append(args, nestedFoldersArgs...) + if len(permSelectorArgs) > 0 { + switch f.recursiveQueriesAreSupported { + case true: + recQueryName := fmt.Sprintf("RecQry%d", len(f.recQueries)) + f.addRecQry(recQueryName, permSelector.String(), permSelectorArgs) + builder.WriteString("(dashboard.uid IN ") + builder.WriteString(fmt.Sprintf("(SELECT uid FROM %s)", recQueryName)) + default: + nestedFoldersSelectors, nestedFoldersArgs := nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "uid", "uid") + builder.WriteRune('(') + builder.WriteString(nestedFoldersSelectors) + builder.WriteRune(')') + args = append(args, nestedFoldersArgs...) + } + } else { + builder.WriteString("(1 = 0") } default: if len(permSelectorArgs) > 0 { diff --git a/pkg/services/sqlstore/permissions/dashboard_test.go b/pkg/services/sqlstore/permissions/dashboard_test.go index 9d1a22d13af..f882cc4d274 100644 --- a/pkg/services/sqlstore/permissions/dashboard_test.go +++ b/pkg/services/sqlstore/permissions/dashboard_test.go @@ -355,6 +355,39 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) { expectedResult []string features featuremgmt.FeatureToggles }{ + { + desc: "Should not be able to view dashboards under inherited folders with no permissions if nested folders are enabled", + queryType: searchstore.TypeDashboard, + permission: dashboards.PERMISSION_VIEW, + permissions: nil, + features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), + expectedResult: nil, + }, + { + desc: "Should not be able to view inherited folders with no permissions if nested folders are enabled", + queryType: searchstore.TypeFolder, + permission: dashboards.PERMISSION_VIEW, + permissions: nil, + features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), + expectedResult: nil, + }, + { + desc: "Should not be able to view inherited dashboards and folders with no permissions if nested folders are enabled", + permission: dashboards.PERMISSION_VIEW, + permissions: nil, + features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), + expectedResult: nil, + }, + { + desc: "Should be able to view dashboards under inherited folders with wildcard scope if nested folders are enabled", + queryType: searchstore.TypeDashboard, + permission: dashboards.PERMISSION_VIEW, + permissions: []accesscontrol.Permission{ + {Action: dashboards.ActionDashboardsRead, Scope: dashboards.ScopeFoldersAll}, + }, + features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), + expectedResult: []string{"dashboard under parent folder", "dashboard under subfolder"}, + }, { desc: "Should be able to view dashboards under inherited folders if nested folders are enabled", queryType: searchstore.TypeDashboard, @@ -461,6 +494,39 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission expectedResult []string features featuremgmt.FeatureToggles }{ + { + desc: "Should not be able to view dashboards under inherited folders with no permissions if nested folders are enabled", + queryType: searchstore.TypeDashboard, + permission: dashboards.PERMISSION_VIEW, + signedInUserPermissions: nil, + features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), + expectedResult: nil, + }, + { + desc: "Should not be able to view inherited folders with no permissions if nested folders are enabled", + queryType: searchstore.TypeFolder, + permission: dashboards.PERMISSION_VIEW, + signedInUserPermissions: nil, + features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), + expectedResult: nil, + }, + { + desc: "Should not be able to view inherited dashboards and folders with no permissions if nested folders are enabled", + permission: dashboards.PERMISSION_VIEW, + signedInUserPermissions: nil, + features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), + expectedResult: nil, + }, + { + desc: "Should be able to view dashboards under inherited folders with wildcard scope if nested folders are enabled", + queryType: searchstore.TypeDashboard, + permission: dashboards.PERMISSION_VIEW, + signedInUserPermissions: []accesscontrol.Permission{ + {Action: dashboards.ActionDashboardsRead, Scope: dashboards.ScopeFoldersAll}, + }, + features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), + expectedResult: []string{"dashboard under parent folder", "dashboard under subfolder"}, + }, { desc: "Should be able to view dashboards under inherited folders if nested folders are enabled", queryType: searchstore.TypeDashboard,