diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index c07d9f8c0b7..ae416a2655b 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -128,6 +128,7 @@ Experimental features might be changed or removed without prior notice. | `grafanaAPIServer` | Enable Kubernetes API Server for Grafana resources | | `featureToggleAdminPage` | Enable admin page for managing feature toggles from the Grafana front-end | | `awsAsyncQueryCaching` | Enable caching for async queries for Redshift and Athena. Requires that the `useCachingService` feature toggle is enabled and the datasource has caching and async query support enabled | +| `permissionsFilterRemoveSubquery` | Alternative permission filter implementation that does not use subqueries for fetching the dashboard folder | | `prometheusConfigOverhaulAuth` | Update the Prometheus configuration page with the new auth component | ## Development feature toggles diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index f28214c2e4c..bb0df3399fe 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -116,6 +116,7 @@ export interface FeatureToggles { awsAsyncQueryCaching?: boolean; splitScopes?: boolean; azureMonitorDataplane?: boolean; + permissionsFilterRemoveSubquery?: boolean; prometheusConfigOverhaulAuth?: boolean; configurableSchedulerTick?: boolean; } diff --git a/pkg/api/folder_bench_test.go b/pkg/api/folder_bench_test.go index 4d3025804b8..88a0b6eb300 100644 --- a/pkg/api/folder_bench_test.go +++ b/pkg/api/folder_bench_test.go @@ -99,49 +99,49 @@ func BenchmarkFolderListAndSearch(b *testing.B) { desc: "get root folders with nested folders feature enabled", url: "/api/folders", expectedLen: LEVEL0_FOLDER_NUM, - features: featuremgmt.WithFeatures("nestedFolders"), + features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders, featuremgmt.FlagPermissionsFilterRemoveSubquery), }, { desc: "get subfolders with nested folders feature enabled", url: "/api/folders?parentUid=folder0", expectedLen: LEVEL1_FOLDER_NUM, - features: featuremgmt.WithFeatures("nestedFolders"), + features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders, featuremgmt.FlagPermissionsFilterRemoveSubquery), }, { desc: "list all inherited dashboards with nested folders feature enabled", url: "/api/search?type=dash-db&limit=5000", expectedLen: withLimit(all), - features: featuremgmt.WithFeatures("nestedFolders"), + features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders, featuremgmt.FlagPermissionsFilterRemoveSubquery), }, { desc: "search for pattern with nested folders feature enabled", url: "/api/search?type=dash-db&query=dashboard_0_0&limit=5000", expectedLen: withLimit(1 + LEVEL1_DASHBOARD_NUM + LEVEL2_FOLDER_NUM*LEVEL2_DASHBOARD_NUM), - features: featuremgmt.WithFeatures("nestedFolders"), + features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders, featuremgmt.FlagPermissionsFilterRemoveSubquery), }, { desc: "search for specific dashboard nested folders feature enabled", url: "/api/search?type=dash-db&query=dashboard_0_0_0_0", expectedLen: 1, - features: featuremgmt.WithFeatures("nestedFolders"), + features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders, featuremgmt.FlagPermissionsFilterRemoveSubquery), }, { desc: "get root folders with nested folders feature disabled", url: "/api/folders?limit=5000", expectedLen: withLimit(LEVEL0_FOLDER_NUM), - features: featuremgmt.WithFeatures(), + features: featuremgmt.WithFeatures(featuremgmt.FlagPermissionsFilterRemoveSubquery), }, { desc: "list all dashboards with nested folders feature disabled", url: "/api/search?type=dash-db&limit=5000", expectedLen: withLimit(LEVEL0_FOLDER_NUM * LEVEL0_DASHBOARD_NUM), - features: featuremgmt.WithFeatures(), + features: featuremgmt.WithFeatures(featuremgmt.FlagPermissionsFilterRemoveSubquery), }, { desc: "search specific dashboard with nested folders feature disabled", url: "/api/search?type=dash-db&query=dashboard_0_0", expectedLen: 1, - features: featuremgmt.WithFeatures(), + features: featuremgmt.WithFeatures(featuremgmt.FlagPermissionsFilterRemoveSubquery), }, } for _, bm := range benchmarks { diff --git a/pkg/infra/db/sqlbuilder.go b/pkg/infra/db/sqlbuilder.go index 35381ba0b22..ee96a081199 100644 --- a/pkg/infra/db/sqlbuilder.go +++ b/pkg/infra/db/sqlbuilder.go @@ -20,6 +20,7 @@ type SQLBuilder struct { features featuremgmt.FeatureToggles sql bytes.Buffer params []interface{} + leftJoin string recQry string recQryParams []interface{} recursiveQueriesAreSupported bool @@ -43,6 +44,9 @@ func (sb *SQLBuilder) GetSQLString() string { var bf bytes.Buffer bf.WriteString(sb.recQry) bf.WriteString(sb.sql.String()) + if sb.leftJoin != "" { + bf.WriteString(" LEFT OUTER JOIN " + sb.leftJoin) + } return bf.String() } @@ -65,9 +69,11 @@ func (sb *SQLBuilder) WriteDashboardPermissionFilter(user *user.SignedInUser, pe params []interface{} recQry string recQryParams []interface{} + leftJoin string ) filterRBAC := permissions.NewAccessControlDashboardPermissionFilter(user, permission, queryType, sb.features, sb.recursiveQueriesAreSupported) + leftJoin = filterRBAC.LeftJoin() sql, params = filterRBAC.Where() recQry, recQryParams = filterRBAC.With() @@ -75,4 +81,5 @@ func (sb *SQLBuilder) WriteDashboardPermissionFilter(user *user.SignedInUser, pe sb.params = append(sb.params, params...) sb.recQry = recQry sb.recQryParams = recQryParams + sb.leftJoin = leftJoin } diff --git a/pkg/services/alerting/store_test.go b/pkg/services/alerting/store_test.go index 11368a75229..69f7a309e7b 100644 --- a/pkg/services/alerting/store_test.go +++ b/pkg/services/alerting/store_test.go @@ -335,7 +335,7 @@ func TestIntegrationPausingAlerts(t *testing.T) { t.Run("Given an alert", func(t *testing.T) { ss := db.InitTestDB(t) cfg := setting.NewCfg() - sqlStore := sqlStore{db: ss, cfg: cfg, log: log.New(), tagService: tagimpl.ProvideService(ss, ss.Cfg)} + sqlStore := sqlStore{db: ss, cfg: cfg, log: log.New(), tagService: tagimpl.ProvideService(ss, ss.Cfg), features: featuremgmt.WithFeatures()} testDash := insertTestDashboard(t, sqlStore.db, "dashboard with alerts", 1, 0, false, "alert") alert, err := insertTestAlert("Alerting title", "Alerting message", testDash.OrgID, testDash.ID, simplejson.New(), sqlStore) diff --git a/pkg/services/annotations/annotationsimpl/xorm_store.go b/pkg/services/annotations/annotationsimpl/xorm_store.go index 17b49489e70..4eb954698a5 100644 --- a/pkg/services/annotations/annotationsimpl/xorm_store.go +++ b/pkg/services/annotations/annotationsimpl/xorm_store.go @@ -383,7 +383,11 @@ func (r *xormRepositoryImpl) getAccessControlFilter(user *user.SignedInUser) (ac filterRBAC := permissions.NewAccessControlDashboardPermissionFilter(user, dashboards.PERMISSION_VIEW, searchstore.TypeDashboard, r.features, recursiveQueriesAreSupported) dashboardFilter, dashboardParams := filterRBAC.Where() recQueries, recQueriesParams = filterRBAC.With() + leftJoin := filterRBAC.LeftJoin() filter := fmt.Sprintf("a.dashboard_id IN(SELECT id FROM dashboard WHERE %s)", dashboardFilter) + if leftJoin != "" { + filter = fmt.Sprintf("a.dashboard_id IN(SELECT dashboard.id FROM dashboard LEFT OUTER JOIN %s WHERE %s)", leftJoin, dashboardFilter) + } filters = append(filters, filter) params = dashboardParams } diff --git a/pkg/services/annotations/annotationsimpl/xorm_store_test.go b/pkg/services/annotations/annotationsimpl/xorm_store_test.go index ba9a3931fe8..927eeb3d434 100644 --- a/pkg/services/annotations/annotationsimpl/xorm_store_test.go +++ b/pkg/services/annotations/annotationsimpl/xorm_store_test.go @@ -39,7 +39,9 @@ func TestIntegrationAnnotations(t *testing.T) { } sql := db.InitTestDB(t) var maximumTagsLength int64 = 60 - repo := xormRepositoryImpl{db: sql, cfg: setting.NewCfg(), log: log.New("annotation.test"), tagService: tagimpl.ProvideService(sql, sql.Cfg), maximumTagsLength: maximumTagsLength} + repo := xormRepositoryImpl{db: sql, cfg: setting.NewCfg(), log: log.New("annotation.test"), tagService: tagimpl.ProvideService(sql, sql.Cfg), maximumTagsLength: maximumTagsLength, + features: featuremgmt.WithFeatures(), + } testUser := &user.SignedInUser{ OrgID: 1, diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index 1c113caf336..3715fc62de2 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -675,6 +675,12 @@ var ( Owner: grafanaPartnerPluginsSquad, Expression: "true", // on by default }, + { + Name: "permissionsFilterRemoveSubquery", + Description: "Alternative permission filter implementation that does not use subqueries for fetching the dashboard folder", + Stage: FeatureStageExperimental, + Owner: grafanaBackendPlatformSquad, + }, { Name: "prometheusConfigOverhaulAuth", Description: "Update the Prometheus configuration page with the new auth component", diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 6e380fb330b..d131c7fbfa0 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -97,5 +97,6 @@ featureToggleAdminPage,experimental,@grafana/grafana-operator-experience-squad,f awsAsyncQueryCaching,experimental,@grafana/aws-datasources,false,false,false,false splitScopes,preview,@grafana/grafana-authnz-team,false,false,true,false azureMonitorDataplane,GA,@grafana/partner-datasources,false,false,false,false +permissionsFilterRemoveSubquery,experimental,@grafana/backend-platform,false,false,false,false prometheusConfigOverhaulAuth,experimental,@grafana/observability-metrics,false,false,false,false configurableSchedulerTick,experimental,@grafana/alerting-squad,false,false,true,false diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index d7137fdf903..4acc10d0587 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -399,6 +399,10 @@ const ( // Adds dataplane compliant frame metadata in the Azure Monitor datasource FlagAzureMonitorDataplane = "azureMonitorDataplane" + // FlagPermissionsFilterRemoveSubquery + // Alternative permission filter implementation that does not use subqueries for fetching the dashboard folder + FlagPermissionsFilterRemoveSubquery = "permissionsFilterRemoveSubquery" + // FlagPrometheusConfigOverhaulAuth // Update the Prometheus configuration page with the new auth component FlagPrometheusConfigOverhaulAuth = "prometheusConfigOverhaulAuth" diff --git a/pkg/services/sqlstore/permissions/dashboard.go b/pkg/services/sqlstore/permissions/dashboard.go index ea1caf32b84..a5dc197ee0c 100644 --- a/pkg/services/sqlstore/permissions/dashboard.go +++ b/pkg/services/sqlstore/permissions/dashboard.go @@ -34,8 +34,19 @@ type accessControlDashboardPermissionFilter struct { recursiveQueriesAreSupported bool } +type PermissionsFilter interface { + LeftJoin() string + With() (string, []interface{}) + Where() (string, []interface{}) + + buildClauses() + nestedFoldersSelectors(permSelector string, permSelectorArgs []interface{}, leftTableCol string, rightTableCol string) (string, []interface{}) +} + // NewAccessControlDashboardPermissionFilter creates a new AccessControlDashboardPermissionFilter that is configured with specific actions calculated based on the dashboards.PermissionType and query type -func NewAccessControlDashboardPermissionFilter(user *user.SignedInUser, permissionLevel dashboards.PermissionType, queryType string, features featuremgmt.FeatureToggles, recursiveQueriesAreSupported bool) *accessControlDashboardPermissionFilter { +// The filter is configured to use the new permissions filter (without subqueries) if the feature flag is enabled +// The filter is configured to use the old permissions filter (with subqueries) if the feature flag is disabled +func NewAccessControlDashboardPermissionFilter(user *user.SignedInUser, permissionLevel dashboards.PermissionType, queryType string, features featuremgmt.FeatureToggles, recursiveQueriesAreSupported bool) PermissionsFilter { needEdit := permissionLevel > dashboards.PERMISSION_VIEW var folderActions []string @@ -71,13 +82,25 @@ func NewAccessControlDashboardPermissionFilter(user *user.SignedInUser, permissi } } - f := accessControlDashboardPermissionFilter{user: user, folderActions: folderActions, dashboardActions: dashboardActions, features: features, - recursiveQueriesAreSupported: recursiveQueriesAreSupported, + var f PermissionsFilter + if features.IsEnabled(featuremgmt.FlagPermissionsFilterRemoveSubquery) { + f = &accessControlDashboardPermissionFilterNoFolderSubquery{ + accessControlDashboardPermissionFilter: accessControlDashboardPermissionFilter{ + user: user, folderActions: folderActions, dashboardActions: dashboardActions, features: features, + recursiveQueriesAreSupported: recursiveQueriesAreSupported, + }, + } + } else { + f = &accessControlDashboardPermissionFilter{user: user, folderActions: folderActions, dashboardActions: dashboardActions, features: features, + recursiveQueriesAreSupported: recursiveQueriesAreSupported, + } } - f.buildClauses() + return f +} - return &f +func (f *accessControlDashboardPermissionFilter) LeftJoin() string { + return "" } // Where returns: @@ -179,7 +202,7 @@ func (f *accessControlDashboardPermissionFilter) buildClauses() { 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") + nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "dashboard.folder_id", "d.id") builder.WriteRune('(') builder.WriteString(nestedFoldersSelectors) args = append(args, nestedFoldersArgs...) @@ -251,7 +274,7 @@ func (f *accessControlDashboardPermissionFilter) buildClauses() { builder.WriteString("(dashboard.uid IN ") builder.WriteString(fmt.Sprintf("(SELECT uid FROM %s)", recQueryName)) default: - nestedFoldersSelectors, nestedFoldersArgs := nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "uid", "uid") + nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "dashboard.uid", "d.uid") builder.WriteRune('(') builder.WriteString(nestedFoldersSelectors) builder.WriteRune(')') @@ -336,7 +359,7 @@ func actionsToCheck(actions []string, permissions map[string][]string, wildcards return toCheck } -func nestedFoldersSelectors(permSelector string, permSelectorArgs []interface{}, leftTableCol string, rightTableCol string) (string, []interface{}) { +func (f *accessControlDashboardPermissionFilter) nestedFoldersSelectors(permSelector string, permSelectorArgs []interface{}, leftTableCol string, rightTableCol string) (string, []interface{}) { wheres := make([]string, 0, folder.MaxNestedFolderDepth+1) args := make([]interface{}, 0, len(permSelectorArgs)*(folder.MaxNestedFolderDepth+1)) @@ -351,7 +374,7 @@ func nestedFoldersSelectors(permSelector string, permSelectorArgs []interface{}, s := fmt.Sprintf(tmpl, t, prev, onCol, t, prev, t) joins = append(joins, s) - wheres = append(wheres, fmt.Sprintf("(dashboard.%s IN (SELECT d.%s FROM dashboard d %s WHERE %s.uid IN %s)", leftTableCol, rightTableCol, strings.Join(joins, " "), t, permSelector)) + wheres = append(wheres, fmt.Sprintf("(%s IN (SELECT %s FROM dashboard d %s WHERE %s.uid IN %s)", leftTableCol, rightTableCol, strings.Join(joins, " "), t, permSelector)) args = append(args, permSelectorArgs...) prev = t diff --git a/pkg/services/sqlstore/permissions/dashboard_filter_no_subquery.go b/pkg/services/sqlstore/permissions/dashboard_filter_no_subquery.go new file mode 100644 index 00000000000..7bcbba79c88 --- /dev/null +++ b/pkg/services/sqlstore/permissions/dashboard_filter_no_subquery.go @@ -0,0 +1,244 @@ +package permissions + +import ( + "fmt" + "strings" + + "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/dashboards" + "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/folder" + "github.com/grafana/grafana/pkg/services/login" +) + +type accessControlDashboardPermissionFilterNoFolderSubquery struct { + accessControlDashboardPermissionFilter + + folderIsRequired bool +} + +func (f *accessControlDashboardPermissionFilterNoFolderSubquery) LeftJoin() string { + if !f.folderIsRequired { + return "" + } + return " dashboard AS folder ON dashboard.org_id = folder.org_id AND dashboard.folder_id = folder.id" +} + +func (f *accessControlDashboardPermissionFilterNoFolderSubquery) buildClauses() { + if f.user == nil || f.user.Permissions == nil || f.user.Permissions[f.user.OrgID] == nil { + f.where = clause{string: "(1 = 0)"} + return + } + dashWildcards := accesscontrol.WildcardsFromPrefix(dashboards.ScopeDashboardsPrefix) + folderWildcards := accesscontrol.WildcardsFromPrefix(dashboards.ScopeFoldersPrefix) + + filter, params := accesscontrol.UserRolesFilter(f.user.OrgID, f.user.UserID, f.user.Teams, accesscontrol.GetOrgRoles(f.user)) + rolesFilter := " AND role_id IN(SELECT id FROM role " + filter + ") " + var args []interface{} + builder := strings.Builder{} + builder.WriteRune('(') + + permSelector := strings.Builder{} + var permSelectorArgs []interface{} + + // useSelfContainedPermissions is true if the user's permissions are stored and set from the JWT token + // currently it's used for the extended JWT module (when the user is authenticated via a JWT token generated by Grafana) + useSelfContainedPermissions := f.user.AuthenticatedBy == login.ExtendedJWTModule + + if len(f.dashboardActions) > 0 { + toCheck := actionsToCheck(f.dashboardActions, f.user.Permissions[f.user.OrgID], dashWildcards, folderWildcards) + + if len(toCheck) > 0 { + if !useSelfContainedPermissions { + builder.WriteString("(dashboard.uid IN (SELECT substr(scope, 16) FROM permission WHERE scope LIKE 'dashboards:uid:%'") + builder.WriteString(rolesFilter) + args = append(args, params...) + + if len(toCheck) == 1 { + builder.WriteString(" AND action = ?") + args = append(args, toCheck[0]) + } else { + builder.WriteString(" AND action IN (?" + strings.Repeat(", ?", len(toCheck)-1) + ") GROUP BY role_id, scope HAVING COUNT(action) = ?") + args = append(args, toCheck...) + args = append(args, len(toCheck)) + } + builder.WriteString(") AND NOT dashboard.is_folder)") + } else { + actions := parseStringSliceFromInterfaceSlice(toCheck) + + args = getAllowedUIDs(actions, f.user, dashboards.ScopeDashboardsPrefix) + + // Only add the IN clause if we have any dashboards to check + if len(args) > 0 { + builder.WriteString("(dashboard.uid IN (?" + strings.Repeat(", ?", len(args)-1) + "") + builder.WriteString(") AND NOT dashboard.is_folder)") + } else { + builder.WriteString("(1 = 0)") + } + } + + builder.WriteString(" OR ") + + if !useSelfContainedPermissions { + permSelector.WriteString("(SELECT substr(scope, 13) FROM permission WHERE scope LIKE 'folders:uid:%' ") + permSelector.WriteString(rolesFilter) + permSelectorArgs = append(permSelectorArgs, params...) + + if len(toCheck) == 1 { + permSelector.WriteString(" AND action = ?") + permSelectorArgs = append(permSelectorArgs, toCheck[0]) + } else { + permSelector.WriteString(" AND action IN (?" + strings.Repeat(", ?", len(toCheck)-1) + ") GROUP BY role_id, scope HAVING COUNT(action) = ?") + permSelectorArgs = append(permSelectorArgs, toCheck...) + permSelectorArgs = append(permSelectorArgs, len(toCheck)) + } + } else { + actions := parseStringSliceFromInterfaceSlice(toCheck) + + permSelectorArgs = getAllowedUIDs(actions, f.user, dashboards.ScopeFoldersPrefix) + + // Only add the IN clause if we have any folders to check + if len(permSelectorArgs) > 0 { + permSelector.WriteString("(?" + strings.Repeat(", ?", len(permSelectorArgs)-1) + "") + } else { + permSelector.WriteString("(") + } + } + + permSelector.WriteRune(')') + + switch f.features.IsEnabled(featuremgmt.FlagNestedFolders) { + case true: + 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)") + } + default: + builder.WriteString("(") + if len(permSelectorArgs) > 0 { + builder.WriteString("folder.uid IN ") + builder.WriteString(permSelector.String()) + args = append(args, permSelectorArgs...) + f.folderIsRequired = true + } else { + builder.WriteString("1 = 0 ") + } + builder.WriteString(" AND NOT dashboard.is_folder)") + } + } else { + builder.WriteString("NOT dashboard.is_folder") + } + } + + // recycle and reuse + permSelector.Reset() + permSelectorArgs = permSelectorArgs[:0] + + if len(f.folderActions) > 0 { + if len(f.dashboardActions) > 0 { + builder.WriteString(" OR ") + } + + toCheck := actionsToCheck(f.folderActions, f.user.Permissions[f.user.OrgID], folderWildcards) + if len(toCheck) > 0 { + if !useSelfContainedPermissions { + permSelector.WriteString("(SELECT substr(scope, 13) FROM permission WHERE scope LIKE 'folders:uid:%'") + permSelector.WriteString(rolesFilter) + permSelectorArgs = append(permSelectorArgs, params...) + if len(toCheck) == 1 { + permSelector.WriteString(" AND action = ?") + permSelectorArgs = append(permSelectorArgs, toCheck[0]) + } else { + permSelector.WriteString(" AND action IN (?" + strings.Repeat(", ?", len(toCheck)-1) + ") GROUP BY role_id, scope HAVING COUNT(action) = ?") + permSelectorArgs = append(permSelectorArgs, toCheck...) + permSelectorArgs = append(permSelectorArgs, len(toCheck)) + } + } else { + actions := parseStringSliceFromInterfaceSlice(toCheck) + + permSelectorArgs = getAllowedUIDs(actions, f.user, dashboards.ScopeFoldersPrefix) + + if len(permSelectorArgs) > 0 { + permSelector.WriteString("(?" + strings.Repeat(", ?", len(permSelectorArgs)-1) + "") + } else { + permSelector.WriteString("(") + } + } + permSelector.WriteRune(')') + + switch f.features.IsEnabled(featuremgmt.FlagNestedFolders) { + case true: + 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 { + builder.WriteString("(dashboard.uid IN ") + builder.WriteString(permSelector.String()) + args = append(args, permSelectorArgs...) + } else { + builder.WriteString("(1 = 0") + } + } + builder.WriteString(" AND dashboard.is_folder)") + } else { + builder.WriteString("dashboard.is_folder") + } + } + builder.WriteRune(')') + + f.where = clause{string: builder.String(), params: args} +} + +func (f *accessControlDashboardPermissionFilterNoFolderSubquery) nestedFoldersSelectors(permSelector string, permSelectorArgs []interface{}, leftTableCol string, _ string) (string, []interface{}) { + wheres := make([]string, 0, folder.MaxNestedFolderDepth+1) + args := make([]interface{}, 0, len(permSelectorArgs)*(folder.MaxNestedFolderDepth+1)) + + joins := make([]string, 0, folder.MaxNestedFolderDepth+2) + + tmpl := "INNER JOIN folder %s ON %s.parent_uid = %s.uid AND %s.org_id = %s.org_id " + + wheres = append(wheres, fmt.Sprintf("(%s IN (SELECT f1.uid FROM folder f1 WHERE f1.uid IN %s)", leftTableCol, permSelector)) + args = append(args, permSelectorArgs...) + + prev := "f1" + for i := 2; i <= folder.MaxNestedFolderDepth+2; i++ { + t := fmt.Sprintf("f%d", i) + s := fmt.Sprintf(tmpl, t, prev, t, prev, t) + joins = append(joins, s) + + wheres = append(wheres, fmt.Sprintf("(%s IN (SELECT f1.uid FROM folder f1 %s WHERE %s.uid IN %s)", leftTableCol, strings.Join(joins, " "), t, permSelector)) + args = append(args, permSelectorArgs...) + + prev = t + } + + return strings.Join(wheres, ") OR "), args +} diff --git a/pkg/services/sqlstore/permissions/dashboard_test.go b/pkg/services/sqlstore/permissions/dashboard_test.go index f882cc4d274..c1fab23f480 100644 --- a/pkg/services/sqlstore/permissions/dashboard_test.go +++ b/pkg/services/sqlstore/permissions/dashboard_test.go @@ -3,6 +3,7 @@ package permissions_test import ( "context" "strconv" + "strings" "testing" "time" @@ -162,26 +163,39 @@ func TestIntegration_DashboardPermissionFilter(t *testing.T) { } for _, tt := range tests { - t.Run(tt.desc, func(t *testing.T) { - store := setupTest(t, 10, 100, tt.permissions) - recursiveQueriesAreSupported, err := store.RecursiveQueriesAreSupported() - require.NoError(t, err) + store := setupTest(t, 10, 100, tt.permissions) + recursiveQueriesAreSupported, err := store.RecursiveQueriesAreSupported() + require.NoError(t, err) - usr := &user.SignedInUser{OrgID: 1, OrgRole: org.RoleViewer, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}} - filter := permissions.NewAccessControlDashboardPermissionFilter(usr, tt.permission, tt.queryType, featuremgmt.WithFeatures(), recursiveQueriesAreSupported) + usr := &user.SignedInUser{OrgID: 1, OrgRole: org.RoleViewer, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}} - var result int - err = store.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { - q, params := filter.Where() - recQry, recQryParams := filter.With() - params = append(recQryParams, params...) - _, err := sess.SQL(recQry+"\nSELECT COUNT(*) FROM dashboard WHERE "+q, params...).Get(&result) - return err + for _, features := range []*featuremgmt.FeatureManager{featuremgmt.WithFeatures(), featuremgmt.WithFeatures(featuremgmt.FlagPermissionsFilterRemoveSubquery)} { + m := features.GetEnabled(context.Background()) + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + t.Run(tt.desc+" with features "+strings.Join(keys, ","), func(t *testing.T) { + filter := permissions.NewAccessControlDashboardPermissionFilter(usr, tt.permission, tt.queryType, features, recursiveQueriesAreSupported) + + var result int + err = store.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { + q, params := filter.Where() + recQry, recQryParams := filter.With() + params = append(recQryParams, params...) + leftJoin := filter.LeftJoin() + s := recQry + "\nSELECT COUNT(*) FROM dashboard WHERE " + q + if leftJoin != "" { + s = recQry + "\nSELECT COUNT(*) FROM dashboard LEFT OUTER JOIN " + leftJoin + " WHERE " + q + } + _, err := sess.SQL(s, params...).Get(&result) + return err + }) + require.NoError(t, err) + + assert.Equal(t, tt.expectedResult, result) }) - require.NoError(t, err) - - assert.Equal(t, tt.expectedResult, result) - }) + } } } @@ -323,26 +337,39 @@ func TestIntegration_DashboardPermissionFilter_WithSelfContainedPermissions(t *t } for _, tt := range tests { - t.Run(tt.desc, func(t *testing.T) { - store := setupTest(t, 10, 100, []accesscontrol.Permission{}) - recursiveQueriesAreSupported, err := store.RecursiveQueriesAreSupported() - require.NoError(t, err) + store := setupTest(t, 10, 100, []accesscontrol.Permission{}) + recursiveQueriesAreSupported, err := store.RecursiveQueriesAreSupported() + require.NoError(t, err) - usr := &user.SignedInUser{OrgID: 1, OrgRole: org.RoleViewer, AuthenticatedBy: login.ExtendedJWTModule, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.signedInUserPermissions)}} - filter := permissions.NewAccessControlDashboardPermissionFilter(usr, tt.permission, tt.queryType, featuremgmt.WithFeatures(), recursiveQueriesAreSupported) + usr := &user.SignedInUser{OrgID: 1, OrgRole: org.RoleViewer, AuthenticatedBy: login.ExtendedJWTModule, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.signedInUserPermissions)}} - var result int - err = store.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { - q, params := filter.Where() - recQry, recQryParams := filter.With() - params = append(recQryParams, params...) - _, err := sess.SQL(recQry+"\nSELECT COUNT(*) FROM dashboard WHERE "+q, params...).Get(&result) - return err + for _, features := range []*featuremgmt.FeatureManager{featuremgmt.WithFeatures(), featuremgmt.WithFeatures(featuremgmt.FlagPermissionsFilterRemoveSubquery)} { + m := features.GetEnabled(context.Background()) + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + t.Run(tt.desc+" with features "+strings.Join(keys, ","), func(t *testing.T) { + filter := permissions.NewAccessControlDashboardPermissionFilter(usr, tt.permission, tt.queryType, features, recursiveQueriesAreSupported) + + var result int + err = store.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { + q, params := filter.Where() + recQry, recQryParams := filter.With() + params = append(recQryParams, params...) + s := recQry + "\nSELECT COUNT(*) FROM dashboard WHERE " + q + leftJoin := filter.LeftJoin() + if leftJoin != "" { + s = recQry + "\nSELECT COUNT(*) FROM dashboard LEFT OUTER JOIN " + leftJoin + " WHERE " + q + } + _, err := sess.SQL(s, params...).Get(&result) + return err + }) + require.NoError(t, err) + + assert.Equal(t, tt.expectedResult, result) }) - require.NoError(t, err) - - assert.Equal(t, tt.expectedResult, result) - }) + } } } @@ -353,14 +380,14 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) { permission dashboards.PermissionType permissions []accesscontrol.Permission expectedResult []string - features featuremgmt.FeatureToggles + 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: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), + features: []interface{}{featuremgmt.FlagNestedFolders}, expectedResult: nil, }, { @@ -368,14 +395,14 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) { queryType: searchstore.TypeFolder, permission: dashboards.PERMISSION_VIEW, permissions: nil, - features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), + 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: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), + features: []interface{}{featuremgmt.FlagNestedFolders}, expectedResult: nil, }, { @@ -385,7 +412,7 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) { permissions: []accesscontrol.Permission{ {Action: dashboards.ActionDashboardsRead, Scope: dashboards.ScopeFoldersAll}, }, - features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), + features: []interface{}{featuremgmt.FlagNestedFolders}, expectedResult: []string{"dashboard under parent folder", "dashboard under subfolder"}, }, { @@ -395,7 +422,7 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) { permissions: []accesscontrol.Permission{ {Action: dashboards.ActionDashboardsRead, Scope: "folders:uid:parent"}, }, - features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), + features: []interface{}{featuremgmt.FlagNestedFolders}, expectedResult: []string{"dashboard under parent folder", "dashboard under subfolder"}, }, { @@ -405,7 +432,7 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) { permissions: []accesscontrol.Permission{ {Action: dashboards.ActionDashboardsRead, Scope: "folders:uid:parent"}, }, - features: featuremgmt.WithFeatures(), + features: []interface{}{}, expectedResult: []string{"dashboard under parent folder"}, }, { @@ -415,7 +442,7 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) { permissions: []accesscontrol.Permission{ {Action: dashboards.ActionFoldersRead, Scope: "folders:uid:parent"}, }, - features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), + features: []interface{}{featuremgmt.FlagNestedFolders}, expectedResult: []string{"parent", "subfolder"}, }, { @@ -425,7 +452,7 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) { permissions: []accesscontrol.Permission{ {Action: dashboards.ActionFoldersRead, Scope: "folders:uid:parent"}, }, - features: featuremgmt.WithFeatures(), + features: []interface{}{}, expectedResult: []string{"parent"}, }, { @@ -435,7 +462,7 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) { {Action: dashboards.ActionFoldersRead, Scope: "folders:uid:parent"}, {Action: dashboards.ActionDashboardsRead, Scope: "folders:uid:parent"}, }, - features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), + features: []interface{}{featuremgmt.FlagNestedFolders}, expectedResult: []string{"parent", "subfolder", "dashboard under parent folder", "dashboard under subfolder"}, }, { @@ -445,7 +472,7 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) { {Action: dashboards.ActionFoldersRead, Scope: "folders:uid:parent"}, {Action: dashboards.ActionDashboardsRead, Scope: "folders:uid:parent"}, }, - features: featuremgmt.WithFeatures(), + features: []interface{}{}, expectedResult: []string{"parent", "dashboard under parent folder"}, }, } @@ -459,29 +486,43 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) { var orgID int64 = 1 for _, tc := range testCases { - t.Run(tc.desc, func(t *testing.T) { - tc.permissions = append(tc.permissions, accesscontrol.Permission{ - Action: dashboards.ActionFoldersCreate, - }, accesscontrol.Permission{ - Action: dashboards.ActionFoldersWrite, - Scope: dashboards.ScopeFoldersAll, - }) - usr := &user.SignedInUser{OrgID: orgID, OrgRole: org.RoleViewer, Permissions: map[int64]map[string][]string{orgID: accesscontrol.GroupScopesByAction(tc.permissions)}} - db := setupNestedTest(t, usr, tc.permissions, orgID, tc.features) - recursiveQueriesAreSupported, err := db.RecursiveQueriesAreSupported() - require.NoError(t, err) - filter := permissions.NewAccessControlDashboardPermissionFilter(usr, tc.permission, tc.queryType, tc.features, recursiveQueriesAreSupported) - var result []string - err = db.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { - q, params := filter.Where() - recQry, recQryParams := filter.With() - params = append(recQryParams, params...) - err := sess.SQL(recQry+"\nSELECT title FROM dashboard WHERE "+q, params...).Find(&result) - return err - }) - require.NoError(t, err) - assert.Equal(t, tc.expectedResult, result) + tc.permissions = append(tc.permissions, accesscontrol.Permission{ + Action: dashboards.ActionFoldersCreate, + }, accesscontrol.Permission{ + Action: dashboards.ActionFoldersWrite, + Scope: dashboards.ScopeFoldersAll, }) + usr := &user.SignedInUser{OrgID: orgID, OrgRole: org.RoleViewer, Permissions: map[int64]map[string][]string{orgID: accesscontrol.GroupScopesByAction(tc.permissions)}} + + for _, features := range []*featuremgmt.FeatureManager{featuremgmt.WithFeatures(tc.features...), featuremgmt.WithFeatures(append(tc.features, featuremgmt.FlagPermissionsFilterRemoveSubquery)...)} { + m := features.GetEnabled(context.Background()) + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + + t.Run(tc.desc+" with features "+strings.Join(keys, ","), func(t *testing.T) { + db := setupNestedTest(t, usr, tc.permissions, orgID, features) + recursiveQueriesAreSupported, err := db.RecursiveQueriesAreSupported() + require.NoError(t, err) + filter := permissions.NewAccessControlDashboardPermissionFilter(usr, tc.permission, tc.queryType, features, recursiveQueriesAreSupported) + var result []string + err = db.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { + q, params := filter.Where() + recQry, recQryParams := filter.With() + params = append(recQryParams, params...) + s := recQry + "\nSELECT dashboard.title FROM dashboard WHERE " + q + leftJoin := filter.LeftJoin() + if leftJoin != "" { + s = recQry + "\nSELECT dashboard.title FROM dashboard LEFT OUTER JOIN " + leftJoin + " WHERE " + q + "ORDER BY dashboard.id ASC" + } + err := sess.SQL(s, params...).Find(&result) + return err + }) + require.NoError(t, err) + assert.Equal(t, tc.expectedResult, result) + }) + } } } @@ -492,14 +533,14 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission permission dashboards.PermissionType signedInUserPermissions []accesscontrol.Permission expectedResult []string - features featuremgmt.FeatureToggles + 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: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), + features: []interface{}{featuremgmt.FlagNestedFolders}, expectedResult: nil, }, { @@ -507,14 +548,14 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission queryType: searchstore.TypeFolder, permission: dashboards.PERMISSION_VIEW, signedInUserPermissions: nil, - features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), + 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: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), + features: []interface{}{featuremgmt.FlagNestedFolders}, expectedResult: nil, }, { @@ -524,7 +565,7 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission signedInUserPermissions: []accesscontrol.Permission{ {Action: dashboards.ActionDashboardsRead, Scope: dashboards.ScopeFoldersAll}, }, - features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), + features: []interface{}{featuremgmt.FlagNestedFolders}, expectedResult: []string{"dashboard under parent folder", "dashboard under subfolder"}, }, { @@ -534,7 +575,7 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission signedInUserPermissions: []accesscontrol.Permission{ {Action: dashboards.ActionDashboardsRead, Scope: "folders:uid:parent"}, }, - features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), + features: []interface{}{featuremgmt.FlagNestedFolders}, expectedResult: []string{"dashboard under parent folder", "dashboard under subfolder"}, }, { @@ -544,7 +585,7 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission signedInUserPermissions: []accesscontrol.Permission{ {Action: dashboards.ActionDashboardsRead, Scope: "folders:uid:parent"}, }, - features: featuremgmt.WithFeatures(), + features: []interface{}{}, expectedResult: []string{"dashboard under parent folder"}, }, { @@ -554,7 +595,7 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission signedInUserPermissions: []accesscontrol.Permission{ {Action: dashboards.ActionFoldersRead, Scope: "folders:uid:parent"}, }, - features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), + features: []interface{}{featuremgmt.FlagNestedFolders}, expectedResult: []string{"parent", "subfolder"}, }, { @@ -564,7 +605,7 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission signedInUserPermissions: []accesscontrol.Permission{ {Action: dashboards.ActionFoldersRead, Scope: "folders:uid:parent"}, }, - features: featuremgmt.WithFeatures(), + features: []interface{}{}, expectedResult: []string{"parent"}, }, { @@ -574,7 +615,7 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission {Action: dashboards.ActionFoldersRead, Scope: "folders:uid:parent"}, {Action: dashboards.ActionDashboardsRead, Scope: "folders:uid:parent"}, }, - features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), + features: []interface{}{featuremgmt.FlagNestedFolders}, expectedResult: []string{"parent", "subfolder", "dashboard under parent folder", "dashboard under subfolder"}, }, { @@ -584,7 +625,7 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission {Action: dashboards.ActionFoldersRead, Scope: "folders:uid:parent"}, {Action: dashboards.ActionDashboardsRead, Scope: "folders:uid:parent"}, }, - features: featuremgmt.WithFeatures(), + features: []interface{}{}, expectedResult: []string{"parent", "dashboard under parent folder"}, }, { @@ -598,7 +639,7 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission {Action: dashboards.ActionDashboardsRead, Scope: "folders:uid:parent"}, {Action: dashboards.ActionDashboardsWrite, Scope: "folders:uid:parent"}, }, - features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), + features: []interface{}{featuremgmt.FlagNestedFolders}, expectedResult: []string{"subfolder", "dashboard under parent folder", "dashboard under subfolder"}, }, } @@ -612,35 +653,48 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission var orgID int64 = 1 for _, tc := range testCases { - t.Run(tc.desc, func(t *testing.T) { - helperUser := &user.SignedInUser{OrgID: orgID, OrgRole: org.RoleViewer, AuthenticatedBy: login.ExtendedJWTModule, - Permissions: map[int64]map[string][]string{orgID: accesscontrol.GroupScopesByAction([]accesscontrol.Permission{ - { - Action: dashboards.ActionFoldersCreate, - }, - { - Action: dashboards.ActionFoldersWrite, - Scope: dashboards.ScopeFoldersAll, - }, - }), + helperUser := &user.SignedInUser{OrgID: orgID, OrgRole: org.RoleViewer, AuthenticatedBy: login.ExtendedJWTModule, + Permissions: map[int64]map[string][]string{orgID: accesscontrol.GroupScopesByAction([]accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersCreate, }, + { + Action: dashboards.ActionFoldersWrite, + Scope: dashboards.ScopeFoldersAll, + }, + }), + }, + } + for _, features := range []*featuremgmt.FeatureManager{featuremgmt.WithFeatures(tc.features...), featuremgmt.WithFeatures(append(tc.features, featuremgmt.FlagPermissionsFilterRemoveSubquery)...)} { + m := features.GetEnabled(context.Background()) + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) } - usr := &user.SignedInUser{OrgID: orgID, OrgRole: org.RoleViewer, AuthenticatedBy: login.ExtendedJWTModule, Permissions: map[int64]map[string][]string{orgID: accesscontrol.GroupScopesByAction(tc.signedInUserPermissions)}} - db := setupNestedTest(t, helperUser, []accesscontrol.Permission{}, orgID, tc.features) - recursiveQueriesAreSupported, err := db.RecursiveQueriesAreSupported() - require.NoError(t, err) - filter := permissions.NewAccessControlDashboardPermissionFilter(usr, tc.permission, tc.queryType, tc.features, recursiveQueriesAreSupported) - var result []string - err = db.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { - q, params := filter.Where() - recQry, recQryParams := filter.With() - params = append(recQryParams, params...) - err := sess.SQL(recQry+"\nSELECT title FROM dashboard WHERE "+q, params...).Find(&result) - return err + + t.Run(tc.desc+" with features "+strings.Join(keys, ","), func(t *testing.T) { + usr := &user.SignedInUser{OrgID: orgID, OrgRole: org.RoleViewer, AuthenticatedBy: login.ExtendedJWTModule, Permissions: map[int64]map[string][]string{orgID: accesscontrol.GroupScopesByAction(tc.signedInUserPermissions)}} + db := setupNestedTest(t, helperUser, []accesscontrol.Permission{}, orgID, features) + recursiveQueriesAreSupported, err := db.RecursiveQueriesAreSupported() + require.NoError(t, err) + filter := permissions.NewAccessControlDashboardPermissionFilter(usr, tc.permission, tc.queryType, features, recursiveQueriesAreSupported) + var result []string + err = db.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { + q, params := filter.Where() + recQry, recQryParams := filter.With() + params = append(recQryParams, params...) + s := recQry + "\nSELECT dashboard.title FROM dashboard WHERE " + q + leftJoin := filter.LeftJoin() + if leftJoin != "" { + s = recQry + "\nSELECT dashboard.title FROM dashboard LEFT OUTER JOIN " + leftJoin + " WHERE " + q + " ORDER BY dashboard.id ASC" + } + err := sess.SQL(s, params...).Find(&result) + return err + }) + require.NoError(t, err) + assert.Equal(t, tc.expectedResult, result) }) - require.NoError(t, err) - assert.Equal(t, tc.expectedResult, result) - }) + } } } diff --git a/pkg/services/sqlstore/searchstore/builder.go b/pkg/services/sqlstore/searchstore/builder.go index 3fb805262e4..717656e0390 100644 --- a/pkg/services/sqlstore/searchstore/builder.go +++ b/pkg/services/sqlstore/searchstore/builder.go @@ -99,7 +99,10 @@ func (b *Builder) applyFilters() (ordering string) { for _, f := range b.Filters { if f, ok := f.(FilterLeftJoin); ok { - joins = append(joins, fmt.Sprintf(" LEFT OUTER JOIN %s ", f.LeftJoin())) + s := f.LeftJoin() + if s != "" { + joins = append(joins, fmt.Sprintf(" LEFT OUTER JOIN %s ", s)) + } } if f, ok := f.(FilterWhere); ok { diff --git a/pkg/services/sqlstore/searchstore/search_test.go b/pkg/services/sqlstore/searchstore/search_test.go index 86e8bab6cad..8630a9f7e4d 100644 --- a/pkg/services/sqlstore/searchstore/search_test.go +++ b/pkg/services/sqlstore/searchstore/search_test.go @@ -3,6 +3,7 @@ package searchstore_test import ( "context" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -117,12 +118,12 @@ func TestBuilder_RBAC(t *testing.T) { testsCases := []struct { desc string userPermissions []accesscontrol.Permission - features featuremgmt.FeatureToggles + features []interface{} expectedParams []interface{} }{ { desc: "no user permissions", - features: featuremgmt.WithFeatures(), + features: []interface{}{}, expectedParams: []interface{}{ int64(1), }, @@ -132,7 +133,7 @@ func TestBuilder_RBAC(t *testing.T) { userPermissions: []accesscontrol.Permission{ {Action: dashboards.ActionDashboardsRead, Scope: "dashboards:uid:1"}, }, - features: featuremgmt.WithFeatures(), + features: []interface{}{}, expectedParams: []interface{}{ int64(1), int64(1), @@ -169,7 +170,7 @@ func TestBuilder_RBAC(t *testing.T) { userPermissions: []accesscontrol.Permission{ {Action: dashboards.ActionDashboardsRead, Scope: "dashboards:uid:1"}, }, - features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), + features: []interface{}{featuremgmt.FlagNestedFolders}, expectedParams: []interface{}{ int64(1), int64(1), @@ -216,39 +217,47 @@ func TestBuilder_RBAC(t *testing.T) { require.NoError(t, err) for _, tc := range testsCases { - t.Run(tc.desc, func(t *testing.T) { - if len(tc.userPermissions) > 0 { - user.Permissions = map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tc.userPermissions)} + for _, features := range []*featuremgmt.FeatureManager{featuremgmt.WithFeatures(tc.features...), featuremgmt.WithFeatures(append(tc.features, featuremgmt.FlagPermissionsFilterRemoveSubquery)...)} { + m := features.GetEnabled(context.Background()) + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) } - level := dashboards.PERMISSION_EDIT + t.Run(tc.desc+" with features "+strings.Join(keys, ","), func(t *testing.T) { + if len(tc.userPermissions) > 0 { + user.Permissions = map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tc.userPermissions)} + } - builder := &searchstore.Builder{ - Filters: []interface{}{ - searchstore.OrgFilter{OrgId: user.OrgID}, - searchstore.TitleSorter{}, - permissions.NewAccessControlDashboardPermissionFilter( - user, - level, - "", - tc.features, - recursiveQueriesAreSupported, - ), - }, - Dialect: store.GetDialect(), - } + level := dashboards.PERMISSION_EDIT - res := []dashboards.DashboardSearchProjection{} - err := store.WithDbSession(context.Background(), func(sess *db.Session) error { - sql, params := builder.ToSQL(limit, page) - // TODO: replace with a proper test - assert.Equal(t, tc.expectedParams, params) - return sess.SQL(sql, params...).Find(&res) + builder := &searchstore.Builder{ + Filters: []interface{}{ + searchstore.OrgFilter{OrgId: user.OrgID}, + searchstore.TitleSorter{}, + permissions.NewAccessControlDashboardPermissionFilter( + user, + level, + "", + features, + recursiveQueriesAreSupported, + ), + }, + Dialect: store.GetDialect(), + } + + res := []dashboards.DashboardSearchProjection{} + err := store.WithDbSession(context.Background(), func(sess *db.Session) error { + sql, params := builder.ToSQL(limit, page) + // TODO: replace with a proper test + assert.Equal(t, tc.expectedParams, params) + return sess.SQL(sql, params...).Find(&res) + }) + require.NoError(t, err) + + assert.Len(t, res, 0) }) - require.NoError(t, err) - - assert.Len(t, res, 0) - }) + } } }