From bb1b145ce8407bed0b86f60666f002c0e915826c Mon Sep 17 00:00:00 2001 From: Karl Persson Date: Wed, 10 Aug 2022 11:58:12 +0200 Subject: [PATCH] RBAC: Fix dashboard filter in SQLBuilder (#53379) (#53506) * Reuse DasbhoardPermissionFilter * Use rbac dashboard filter if enabled (cherry picked from commit aa484a60c96e96852aa96b10504c5be06f511732) --- pkg/services/dashboards/database/acl.go | 4 +- pkg/services/libraryelements/database.go | 6 +- .../libraryelements/libraryelements_test.go | 10 +-- pkg/services/sqlstore/alert.go | 2 +- pkg/services/sqlstore/alert_test.go | 2 +- pkg/services/sqlstore/sqlbuilder.go | 80 ++++++------------- pkg/services/sqlstore/sqlbuilder_test.go | 8 +- 7 files changed, 43 insertions(+), 69 deletions(-) diff --git a/pkg/services/dashboards/database/acl.go b/pkg/services/dashboards/database/acl.go index 931d6f9f80b..2ac86a8aa31 100644 --- a/pkg/services/dashboards/database/acl.go +++ b/pkg/services/dashboards/database/acl.go @@ -102,7 +102,7 @@ func (d *DashboardStore) HasEditPermissionInFolders(ctx context.Context, query * return nil } - builder := &sqlstore.SQLBuilder{} + builder := sqlstore.NewSqlBuilder(d.sqlStore.Cfg) builder.Write("SELECT COUNT(dashboard.id) AS count FROM dashboard WHERE dashboard.org_id = ? AND dashboard.is_folder = ?", query.SignedInUser.OrgId, d.dialect.BooleanStr(true)) builder.WriteDashboardPermissionFilter(query.SignedInUser, models.PERMISSION_EDIT) @@ -130,7 +130,7 @@ func (d *DashboardStore) HasAdminPermissionInDashboardsOrFolders(ctx context.Con return nil } - builder := &sqlstore.SQLBuilder{} + builder := sqlstore.NewSqlBuilder(d.sqlStore.Cfg) builder.Write("SELECT COUNT(dashboard.id) AS count FROM dashboard WHERE dashboard.org_id = ?", query.SignedInUser.OrgId) builder.WriteDashboardPermissionFilter(query.SignedInUser, models.PERMISSION_ADMIN) diff --git a/pkg/services/libraryelements/database.go b/pkg/services/libraryelements/database.go index 80ab0141dd3..b2596650442 100644 --- a/pkg/services/libraryelements/database.go +++ b/pkg/services/libraryelements/database.go @@ -223,7 +223,7 @@ func (l *LibraryElementService) deleteLibraryElement(c context.Context, signedIn func getLibraryElements(c context.Context, store *sqlstore.SQLStore, signedInUser *models.SignedInUser, params []Pair) ([]LibraryElementDTO, error) { libraryElements := make([]LibraryElementWithMeta, 0) err := store.WithDbSession(c, func(session *sqlstore.DBSession) error { - builder := sqlstore.SQLBuilder{} + builder := sqlstore.NewSqlBuilder(store.Cfg) builder.Write(selectLibraryElementDTOWithMeta) builder.Write(", 'General' as folder_name ") builder.Write(", '' as folder_uid ") @@ -327,7 +327,7 @@ func (l *LibraryElementService) getAllLibraryElements(c context.Context, signedI return LibraryElementSearchResult{}, folderFilter.parseError } err := l.SQLStore.WithDbSession(c, func(session *sqlstore.DBSession) error { - builder := sqlstore.SQLBuilder{} + builder := sqlstore.NewSqlBuilder(l.Cfg) if folderFilter.includeGeneralFolder { builder.Write(selectLibraryElementDTOWithMeta) builder.Write(", 'General' as folder_name ") @@ -561,7 +561,7 @@ func (l *LibraryElementService) getConnections(c context.Context, signedInUser * return err } var libraryElementConnections []libraryElementConnectionWithMeta - builder := sqlstore.SQLBuilder{} + builder := sqlstore.NewSqlBuilder(l.Cfg) builder.Write("SELECT lec.*, u1.login AS created_by_name, u1.email AS created_by_email, dashboard.uid AS connection_uid") builder.Write(" FROM " + models.LibraryElementConnectionTableName + " AS lec") builder.Write(" LEFT JOIN " + l.SQLStore.Dialect.Quote("user") + " AS u1 ON lec.created_by = u1.id") diff --git a/pkg/services/libraryelements/libraryelements_test.go b/pkg/services/libraryelements/libraryelements_test.go index a2b2ac1464d..dbc1a40bd12 100644 --- a/pkg/services/libraryelements/libraryelements_test.go +++ b/pkg/services/libraryelements/libraryelements_test.go @@ -403,21 +403,21 @@ func testScenario(t *testing.T, desc string, fn func(t *testing.T, sc scenarioCo sqlStore := sqlstore.InitTestDB(t) dashboardStore := database.ProvideDashboardStore(sqlStore, featuremgmt.WithFeatures()) features := featuremgmt.WithFeatures() - cfg := setting.NewCfg() - cfg.IsFeatureToggleEnabled = features.IsEnabled ac := acmock.New() + // TODO: Update tests to work with rbac + sqlStore.Cfg.RBACEnabled = false folderPermissions := acmock.NewMockedPermissionsService() dashboardPermissions := acmock.NewMockedPermissionsService() dashboardService := dashboardservice.ProvideDashboardService( - cfg, dashboardStore, nil, + sqlStore.Cfg, dashboardStore, nil, features, folderPermissions, dashboardPermissions, ac, ) guardian.InitLegacyGuardian(sqlStore, dashboardService) service := LibraryElementService{ - Cfg: cfg, + Cfg: sqlStore.Cfg, SQLStore: sqlStore, folderService: dashboardservice.ProvideFolderService( - cfg, dashboardService, dashboardStore, nil, + sqlStore.Cfg, dashboardService, dashboardStore, nil, features, folderPermissions, ac, busmock.New(), ), } diff --git a/pkg/services/sqlstore/alert.go b/pkg/services/sqlstore/alert.go index d4b07cb64db..e4f3affcf2b 100644 --- a/pkg/services/sqlstore/alert.go +++ b/pkg/services/sqlstore/alert.go @@ -66,7 +66,7 @@ func deleteAlertByIdInternal(alertId int64, reason string, sess *DBSession) erro func (ss *SQLStore) HandleAlertsQuery(ctx context.Context, query *models.GetAlertsQuery) error { return ss.WithDbSession(ctx, func(sess *DBSession) error { - builder := SQLBuilder{} + builder := NewSqlBuilder(ss.Cfg) builder.Write(`SELECT alert.id, diff --git a/pkg/services/sqlstore/alert_test.go b/pkg/services/sqlstore/alert_test.go index cb5beb3ddb3..6ed115e2ca3 100644 --- a/pkg/services/sqlstore/alert_test.go +++ b/pkg/services/sqlstore/alert_test.go @@ -141,7 +141,7 @@ func TestIntegrationAlertingDataAccess(t *testing.T) { t.Run("Viewer can read alerts", func(t *testing.T) { setup(t) - viewerUser := &models.SignedInUser{OrgRole: models.ROLE_VIEWER, OrgId: 1} + viewerUser := &models.SignedInUser{OrgRole: models.ROLE_VIEWER, OrgId: 1, Permissions: map[int64]map[string][]string{1: {"dashboards:read": []string{"dashboards:*"}}}} alertQuery := models.GetAlertsQuery{DashboardIDs: []int64{testDash.Id}, PanelId: 1, OrgId: 1, User: viewerUser} err2 := sqlStore.HandleAlertsQuery(context.Background(), &alertQuery) diff --git a/pkg/services/sqlstore/sqlbuilder.go b/pkg/services/sqlstore/sqlbuilder.go index f135b13c3dc..dbdcc32cd46 100644 --- a/pkg/services/sqlstore/sqlbuilder.go +++ b/pkg/services/sqlstore/sqlbuilder.go @@ -2,12 +2,19 @@ package sqlstore import ( "bytes" - "strings" "github.com/grafana/grafana/pkg/models" + ac "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/sqlstore/permissions" + "github.com/grafana/grafana/pkg/setting" ) +func NewSqlBuilder(cfg *setting.Cfg) SQLBuilder { + return SQLBuilder{cfg: cfg} +} + type SQLBuilder struct { + cfg *setting.Cfg sql bytes.Buffer params []interface{} } @@ -33,61 +40,22 @@ func (sb *SQLBuilder) AddParams(params ...interface{}) { } func (sb *SQLBuilder) WriteDashboardPermissionFilter(user *models.SignedInUser, permission models.PermissionType) { - if user.OrgRole == models.ROLE_ADMIN { - return + var ( + sql string + params []interface{} + ) + if !ac.IsDisabled(sb.cfg) { + sql, params = permissions.NewAccessControlDashboardPermissionFilter(user, permission, "").Where() + } else { + sql, params = permissions.DashboardPermissionFilter{ + OrgRole: user.OrgRole, + Dialect: dialect, + UserId: user.UserId, + OrgId: user.OrgId, + PermissionLevel: permission, + }.Where() } - okRoles := []interface{}{user.OrgRole} - - if user.OrgRole == models.ROLE_EDITOR { - okRoles = append(okRoles, models.ROLE_VIEWER) - } - - falseStr := dialect.BooleanStr(false) - - sb.sql.WriteString(` AND - ( - dashboard.id IN ( - SELECT distinct DashboardId from ( - SELECT d.id AS DashboardId - FROM dashboard AS d - LEFT JOIN dashboard_acl AS da ON - da.dashboard_id = d.id OR - da.dashboard_id = d.folder_id - WHERE - d.org_id = ? AND - da.permission >= ? AND - ( - da.user_id = ? OR - da.team_id IN (SELECT team_id from team_member AS tm WHERE tm.user_id = ?) OR - da.role IN (?` + strings.Repeat(",?", len(okRoles)-1) + `) - ) - UNION - SELECT d.id AS DashboardId - FROM dashboard AS d - LEFT JOIN dashboard AS folder on folder.id = d.folder_id - LEFT JOIN dashboard_acl AS da ON - ( - -- include default permissions --> - da.org_id = -1 AND ( - (folder.id IS NOT NULL AND folder.has_acl = ` + falseStr + `) OR - (folder.id IS NULL AND d.has_acl = ` + falseStr + `) - ) - ) - WHERE - d.org_id = ? AND - da.permission >= ? AND - ( - da.user_id = ? OR - da.role IN (?` + strings.Repeat(",?", len(okRoles)-1) + `) - ) - ) AS a - ) - )`) - - sb.params = append(sb.params, user.OrgId, permission, user.UserId, user.UserId) - sb.params = append(sb.params, okRoles...) - - sb.params = append(sb.params, user.OrgId, permission, user.UserId) - sb.params = append(sb.params, okRoles...) + sb.sql.WriteString(" AND " + sql) + sb.params = append(sb.params, params...) } diff --git a/pkg/services/sqlstore/sqlbuilder_test.go b/pkg/services/sqlstore/sqlbuilder_test.go index 9021f169219..f8554d87bf5 100644 --- a/pkg/services/sqlstore/sqlbuilder_test.go +++ b/pkg/services/sqlstore/sqlbuilder_test.go @@ -299,7 +299,13 @@ func createDummyACL(t *testing.T, sqlStore *SQLStore, dashboardPermission *Dashb func getDashboards(t *testing.T, sqlStore *SQLStore, search Search, aclUserID int64) []*dashboardResponse { t.Helper() - builder := &SQLBuilder{} + old := sqlStore.Cfg.RBACEnabled + sqlStore.Cfg.RBACEnabled = false + defer func() { + sqlStore.Cfg.RBACEnabled = old + }() + + builder := NewSqlBuilder(sqlStore.Cfg) signedInUser := &models.SignedInUser{ UserId: 9999999999, }