From 890ebcbb7b606e7ea83fbef7027ea19551741b74 Mon Sep 17 00:00:00 2001 From: Daniel Lee Date: Wed, 3 May 2017 10:47:20 +0200 Subject: [PATCH] WIP: limit GetAllowedDashboards sql query with a where in --- pkg/services/sqlstore/guardian.go | 23 ++++++++++++----------- pkg/services/sqlstore/guardian_test.go | 23 ++++++++++++++++++----- 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/pkg/services/sqlstore/guardian.go b/pkg/services/sqlstore/guardian.go index 6710642586b..1ba346def2c 100644 --- a/pkg/services/sqlstore/guardian.go +++ b/pkg/services/sqlstore/guardian.go @@ -1,7 +1,8 @@ package sqlstore import ( - "strconv" + "fmt" + "strings" "github.com/grafana/grafana/pkg/bus" m "github.com/grafana/grafana/pkg/models" @@ -12,6 +13,7 @@ func init() { } func GetAllowedDashboards(query *m.GetAllowedDashboardsQuery) error { + dashboardIds := arrayToString(query.DashList, ",") rawSQL := `select distinct d.id as DashboardId from dashboard as d @@ -23,19 +25,18 @@ where ( or d.has_acl = 0) and d.org_id = ?` - res, err := x.Query(rawSQL, query.UserId, query.UserId, query.UserId, query.UserId, query.OrgId) + rawSQL = fmt.Sprintf("%v and d.id in(%v)", rawSQL, dashboardIds) + + query.Result = make([]int64, 0) + err := x.In("DashboardId", query.DashList).SQL(rawSQL, query.UserId, query.UserId, query.UserId, query.UserId, query.OrgId).Find(&query.Result) + if err != nil { return err } - query.Result = make([]int64, 0) - for _, dash := range res { - id, err := strconv.ParseInt(string(dash["DashboardId"]), 10, 64) - if err != nil { - return err - } - query.Result = append(query.Result, id) - } - return nil } + +func arrayToString(a []int64, delim string) string { + return strings.Trim(strings.Replace(fmt.Sprint(a), " ", delim, -1), "[]") +} diff --git a/pkg/services/sqlstore/guardian_test.go b/pkg/services/sqlstore/guardian_test.go index dc60d3a76cb..ed77131c787 100644 --- a/pkg/services/sqlstore/guardian_test.go +++ b/pkg/services/sqlstore/guardian_test.go @@ -15,9 +15,9 @@ func TestGuardianDataAccess(t *testing.T) { Convey("Given one dashboard folder with two dashboard and one dashboard in the root folder", func() { folder := insertTestDashboard("1 test dash folder", 1, 0, true, "prod", "webapp") - // insertTestDashboard("test dash 23", 1, folder.Id, false, "prod", "webapp") - // insertTestDashboard("test dash 45", 1, folder.Id, false, "prod") dashInRoot := insertTestDashboard("test dash 67", 1, 0, false, "prod", "webapp") + insertTestDashboard("test dash 23", 1, folder.Id, false, "prod", "webapp") + insertTestDashboard("test dash 45", 1, folder.Id, false, "prod") currentUser := createUser("viewer") @@ -33,16 +33,29 @@ func TestGuardianDataAccess(t *testing.T) { }) Convey("and acl is set for dashboard folder", func() { - Convey("should not return folder", func() { - var otherUser int64 = 999 - updateTestDashboardWithAcl(folder.Id, otherUser, m.PERMISSION_EDIT) + var otherUser int64 = 999 + updateTestDashboardWithAcl(folder.Id, otherUser, m.PERMISSION_EDIT) + Convey("should not return folder", func() { query := &m.GetAllowedDashboardsQuery{UserId: currentUser.Id, OrgId: 1, DashList: []int64{folder.Id, dashInRoot.Id}} err := GetAllowedDashboards(query) So(err, ShouldBeNil) So(len(query.Result), ShouldEqual, 1) So(query.Result[0], ShouldEqual, dashInRoot.Id) }) + + Convey("when the user is given permission", func() { + updateTestDashboardWithAcl(folder.Id, currentUser.Id, m.PERMISSION_EDIT) + + Convey("should folder", func() { + query := &m.GetAllowedDashboardsQuery{UserId: currentUser.Id, OrgId: 1, DashList: []int64{folder.Id, dashInRoot.Id}} + err := GetAllowedDashboards(query) + So(err, ShouldBeNil) + So(len(query.Result), ShouldEqual, 2) + So(query.Result[0], ShouldEqual, folder.Id) + So(query.Result[1], ShouldEqual, dashInRoot.Id) + }) + }) }) }) })