From 8a24e891fe49e82a45af61179cbb5ab0ae2771be Mon Sep 17 00:00:00 2001 From: Sofia Papagiannaki <1632407+papagian@users.noreply.github.com> Date: Wed, 2 Aug 2023 14:12:46 +0300 Subject: [PATCH] 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 --- .../sqlstore/permissions/dashboard.go | 53 ++++++++------- .../dashboard_filter_no_subquery.go | 56 +++++++++------- .../sqlstore/permissions/dashboard_test.go | 66 +++++++++++++++++++ 3 files changed, 129 insertions(+), 46 deletions(-) diff --git a/pkg/services/sqlstore/permissions/dashboard.go b/pkg/services/sqlstore/permissions/dashboard.go index 8cce9e96753..a5dc197ee0c 100644 --- a/pkg/services/sqlstore/permissions/dashboard.go +++ b/pkg/services/sqlstore/permissions/dashboard.go @@ -194,17 +194,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 := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "dashboard.folder_id", "d.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 := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "dashboard.folder_id", "d.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 ") @@ -261,18 +266,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 := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "dashboard.uid", "d.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 := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "dashboard.uid", "d.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_filter_no_subquery.go b/pkg/services/sqlstore/permissions/dashboard_filter_no_subquery.go index 2e76407d757..7bcbba79c88 100644 --- a/pkg/services/sqlstore/permissions/dashboard_filter_no_subquery.go +++ b/pkg/services/sqlstore/permissions/dashboard_filter_no_subquery.go @@ -109,19 +109,23 @@ func (f *accessControlDashboardPermissionFilterNoFolderSubquery) 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("(folder.uid IN (SELECT uid FROM " + recQueryName) - default: - nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "folder.uid", "") - builder.WriteRune('(') - builder.WriteString(nestedFoldersSelectors) - 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("(folder.uid IN (SELECT uid FROM " + recQueryName) + default: + nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "folder.uid", "") + builder.WriteRune('(') + builder.WriteString(nestedFoldersSelectors) + args = append(args, nestedFoldersArgs...) + } + f.folderIsRequired = true + builder.WriteString(") AND NOT dashboard.is_folder)") + } else { + builder.WriteString("( 1 = 0 AND NOT dashboard.is_folder)") } - f.folderIsRequired = true - builder.WriteString(") AND NOT dashboard.is_folder)") default: builder.WriteString("(") if len(permSelectorArgs) > 0 { @@ -177,18 +181,22 @@ func (f *accessControlDashboardPermissionFilterNoFolderSubquery) 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 := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "dashboard.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 := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "dashboard.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 18f68add5be..c1fab23f480 100644 --- a/pkg/services/sqlstore/permissions/dashboard_test.go +++ b/pkg/services/sqlstore/permissions/dashboard_test.go @@ -382,6 +382,39 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) { expectedResult []string features []interface{} }{ + { + 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: []interface{}{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: []interface{}{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: []interface{}{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: []interface{}{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, @@ -502,6 +535,39 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission expectedResult []string features []interface{} }{ + { + 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: []interface{}{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: []interface{}{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: []interface{}{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: []interface{}{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,