Skip to content

Commit

Permalink
Nested folders: Fix search query for empty self-contained permissions (…
Browse files Browse the repository at this point in the history 
…#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 8a24e89)
  • Loading branch information
@papagian
papagian committed Aug 2, 2023
1 parent 5a05fad commit d769768
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 22 deletions.
53 changes: 31 additions & 22 deletions pkg/services/sqlstore/permissions/dashboard.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 ")
Expand Down Expand Up @@ -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 {
Expand Down
66 changes: 66 additions & 0 deletions pkg/services/sqlstore/permissions/dashboard_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit d769768

Please sign in to comment.