WIP: move guardian logic for search into the sql query

pkg/api/playlist.go

@@ -130,7 +130,7 @@ func GetPlaylistItems(c *middleware.Context) Response {
 func GetPlaylistDashboards(c *middleware.Context) Response {
 	playlistId := c.ParamsInt64(":id")
 
-	playlists, err := LoadPlaylistDashboards(c.OrgId, c.UserId, playlistId)
+	playlists, err := LoadPlaylistDashboards(c.OrgId, c.SignedInUser, playlistId)
 	if err != nil {
 		return ApiError(500, "Could not load dashboards", err)
 	}

pkg/api/playlist_play.go

@@ -34,18 +34,18 @@ func populateDashboardsById(dashboardByIds []int64, dashboardIdOrder map[int64]i
 	return result, nil
 }
 
-func populateDashboardsByTag(orgId, userId int64, dashboardByTag []string, dashboardTagOrder map[string]int) dtos.PlaylistDashboardsSlice {
+func populateDashboardsByTag(orgId int64, signedInUser *m.SignedInUser, dashboardByTag []string, dashboardTagOrder map[string]int) dtos.PlaylistDashboardsSlice {
 	result := make(dtos.PlaylistDashboardsSlice, 0)
 
 	if len(dashboardByTag) > 0 {
 		for _, tag := range dashboardByTag {
 			searchQuery := search.Query{
-				Title:     "",
-				Tags:      []string{tag},
-				UserId:    userId,
-				Limit:     100,
-				IsStarred: false,
-				OrgId:     orgId,
+				Title:        "",
+				Tags:         []string{tag},
+				SignedInUser: signedInUser,
+				Limit:        100,
+				IsStarred:    false,
+				OrgId:        orgId,
 			}
 
 			if err := bus.Dispatch(&searchQuery); err == nil {
@@ -64,7 +64,7 @@ func populateDashboardsByTag(orgId, userId int64, dashboardByTag []string, dashb
 	return result
 }
 
-func LoadPlaylistDashboards(orgId, userId, playlistId int64) (dtos.PlaylistDashboardsSlice, error) {
+func LoadPlaylistDashboards(orgId int64, signedInUser *m.SignedInUser, playlistId int64) (dtos.PlaylistDashboardsSlice, error) {
 	playlistItems, _ := LoadPlaylistItems(playlistId)
 
 	dashboardByIds := make([]int64, 0)
@@ -89,7 +89,7 @@ func LoadPlaylistDashboards(orgId, userId, playlistId int64) (dtos.PlaylistDashb
 
 	var k, _ = populateDashboardsById(dashboardByIds, dashboardIdOrder)
 	result = append(result, k...)
-	result = append(result, populateDashboardsByTag(orgId, userId, dashboardByTag, dashboardTagOrder)...)
+	result = append(result, populateDashboardsByTag(orgId, signedInUser, dashboardByTag, dashboardTagOrder)...)
 
 	sort.Sort(result)
 	return result, nil

pkg/api/search.go

@@ -26,9 +26,9 @@ func Search(c *middleware.Context) {
 		mode = "list"
 	}
 
-	dbids := make([]int, 0)
+	dbids := make([]int64, 0)
 	for _, id := range c.QueryStrings("dashboardIds") {
-		dashboardId, err := strconv.Atoi(id)
+		dashboardId, err := strconv.ParseInt(id, 10, 64)
 		if err == nil {
 			dbids = append(dbids, dashboardId)
 		}
@@ -37,7 +37,7 @@ func Search(c *middleware.Context) {
 	searchQuery := search.Query{
 		Title:        query,
 		Tags:         tags,
-		UserId:       c.UserId,
+		SignedInUser: c.SignedInUser,
 		Limit:        limit,
 		IsStarred:    starred == "true",
 		OrgId:        c.OrgId,

pkg/services/guardian/guardian.go

@@ -5,22 +5,6 @@ import (
 	m "github.com/grafana/grafana/pkg/models"
 )
 
-// FilterRestrictedDashboards filters out dashboards from the list that the user does have access to
-func FilterRestrictedDashboards(dashList []int64, orgId int64, userId int64) ([]int64, error) {
-	user, err := getUser(userId)
-	if err != nil {
-		return nil, err
-	}
-
-	if user.IsGrafanaAdmin || user.OrgRole == m.ROLE_ADMIN {
-		return dashList, nil
-	}
-
-	filteredList, err := getAllowedDashboards(dashList, orgId, userId)
-
-	return filteredList, err
-}
-
 // CanViewAcl determines if a user has permission to view a dashboard's ACL
 func CanViewAcl(dashboardId int64, role m.RoleType, isGrafanaAdmin bool, orgId int64, userId int64) (bool, error) {
 	if role == m.ROLE_ADMIN || isGrafanaAdmin {
@@ -115,13 +99,6 @@ func checkPermission(minimumPermission m.PermissionType, permissions []*m.Dashbo
 	return false, nil
 }
 
-func getUser(userId int64) (*m.SignedInUser, error) {
-	query := m.GetSignedInUserQuery{UserId: userId}
-	err := bus.Dispatch(&query)
-
-	return query.Result, err
-}
-
 func getAllowedDashboards(dashList []int64, orgId int64, userId int64) ([]int64, error) {
 	query := m.GetAllowedDashboardsQuery{UserId: userId, OrgId: orgId, DashList: dashList}
 	err := bus.Dispatch(&query)

pkg/services/search/handlers.go

@@ -7,7 +7,6 @@ import (
 
 	"github.com/grafana/grafana/pkg/bus"
 	m "github.com/grafana/grafana/pkg/models"
-	"github.com/grafana/grafana/pkg/services/guardian"
 	"github.com/grafana/grafana/pkg/setting"
 )
 
@@ -41,9 +40,8 @@ func searchHandler(query *Query) error {
 
 	dashQuery := FindPersistedDashboardsQuery{
 		Title:        query.Title,
-		UserId:       query.UserId,
+		SignedInUser: query.SignedInUser,
 		IsStarred:    query.IsStarred,
-		OrgId:        query.OrgId,
 		DashboardIds: query.DashboardIds,
 		Type:         query.Type,
 		ParentId:     query.FolderId,
@@ -76,11 +74,6 @@ func searchHandler(query *Query) error {
 		hits = filtered
 	}
 
-	hits, err := removeRestrictedDashboardsFromList(hits, query)
-	if err != nil {
-		return err
-	}
-
 	// sort main result array
 	sort.Sort(hits)
 
@@ -94,7 +87,7 @@ func searchHandler(query *Query) error {
 	}
 
 	// add isStarred info
-	if err := setIsStarredFlagOnSearchResults(query.UserId, hits); err != nil {
+	if err := setIsStarredFlagOnSearchResults(query.SignedInUser.UserId, hits); err != nil {
 		return err
 	}
 
@@ -102,29 +95,6 @@ func searchHandler(query *Query) error {
 	return nil
 }
 
-func removeRestrictedDashboardsFromList(hits HitList, query *Query) (HitList, error) {
-	var dashboardIds = []int64{}
-	for _, hit := range hits {
-		dashboardIds = append(dashboardIds, hit.Id)
-	}
-
-	filteredHits, err := guardian.FilterRestrictedDashboards(dashboardIds, query.OrgId, query.UserId)
-	if err != nil {
-		return nil, err
-	}
-
-	filtered := HitList{}
-	for _, hit := range hits {
-		for _, dashId := range filteredHits {
-			if hit.Id == dashId {
-				filtered = append(filtered, hit)
-			}
-		}
-	}
-
-	return filtered, nil
-}
-
 func stringInSlice(a string, list []string) bool {
 	for _, b := range list {
 		if b == a {

pkg/services/search/handlers_test.go

@@ -12,7 +12,7 @@ func TestSearch(t *testing.T) {
 
 	Convey("Given search query", t, func() {
 		jsonDashIndex = NewJsonDashIndex("../../../public/dashboards/")
-		query := Query{Limit: 2000}
+		query := Query{Limit: 2000, SignedInUser: &m.SignedInUser{IsGrafanaAdmin: true}}
 
 		bus.AddHandler("test", func(query *FindPersistedDashboardsQuery) error {
 			query.Result = HitList{

pkg/services/search/models.go

 package search
 
 import "strings"
+import "github.com/grafana/grafana/pkg/models"
 
 type HitType string
 
@@ -43,11 +44,11 @@ type Query struct {
 	Title        string
 	Tags         []string
 	OrgId        int64
-	UserId       int64
+	SignedInUser *models.SignedInUser
 	Limit        int
 	IsStarred    bool
 	Type         string
-	DashboardIds []int
+	DashboardIds []int64
 	FolderId     int64
 	Mode         string
 
@@ -57,9 +58,9 @@ type Query struct {
 type FindPersistedDashboardsQuery struct {
 	Title        string
 	OrgId        int64
-	UserId       int64
+	SignedInUser *models.SignedInUser
 	IsStarred    bool
-	DashboardIds []int
+	DashboardIds []int64
 	Type         string
 	ParentId     int64
 	Mode         string

pkg/services/sqlstore/dashboard.go

@@ -166,8 +166,10 @@ func findDashboards(query *search.FindPersistedDashboardsQuery) ([]DashboardSear
 					  dashboard.slug,
 					  dashboard_tag.term,
             dashboard.is_folder,
-            dashboard.parent_id
+            dashboard.parent_id,
+			      pd.title as folder_title
 					FROM dashboard
+					LEFT OUTER JOIN dashboard pd on pd.id = dashboard.parent_id
 					LEFT OUTER JOIN dashboard_tag on dashboard_tag.dashboard_id = dashboard.id`)
 
 	if query.IsStarred {
@@ -175,12 +177,11 @@ func findDashboards(query *search.FindPersistedDashboardsQuery) ([]DashboardSear
 	}
 
 	sql.WriteString(` WHERE dashboard.org_id=?`)
-
-	params = append(params, query.OrgId)
+	params = append(params, query.SignedInUser.OrgId)
 
 	if query.IsStarred {
 		sql.WriteString(` AND star.user_id=?`)
-		params = append(params, query.UserId)
+		params = append(params, query.SignedInUser.UserId)
 	}
 
 	if len(query.DashboardIds) > 0 {
@@ -196,6 +197,23 @@ func findDashboards(query *search.FindPersistedDashboardsQuery) ([]DashboardSear
 		sql.WriteString(")")
 	}
 
+	if query.SignedInUser.OrgRole != m.ROLE_ADMIN {
+		allowedDashboardsSubQuery := ` AND (dashboard.has_acl = 0 OR dashboard.id in (
+		SELECT distinct d.id AS DashboardId
+			FROM dashboard AS d
+				LEFT JOIN dashboard AS df ON d.parent_id = df.id
+				LEFT JOIN dashboard_acl as dfa on d.parent_id = dfa.dashboard_id or d.id = dfa.dashboard_id
+				LEFT JOIN user_group_member as ugm on ugm.user_group_id =  dfa.user_group_id
+			WHERE
+			  d.has_acl = 1 and
+				(dfa.user_id = ? or ugm.user_id = ?)
+			  and d.org_id = ?
+			  ))`
+
+		sql.WriteString(allowedDashboardsSubQuery)
+		params = append(params, query.SignedInUser.UserId, query.SignedInUser.UserId, query.SignedInUser.OrgId)
+	}
+
 	if len(query.Title) > 0 {
 		sql.WriteString(" AND dashboard.title " + dialect.LikeStr() + " ?")
 		params = append(params, "%"+query.Title+"%")

pkg/services/sqlstore/dashboard_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	m "github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/search"
+	"github.com/grafana/grafana/pkg/setting"
 )
 
 func insertTestDashboard(title string, orgId int64, parentId int64, isFolder bool, tags ...interface{}) *m.Dashboard {
@@ -116,9 +117,10 @@ func TestDashboardDataAccess(t *testing.T) {
 
 			Convey("Should be able to search for dashboard and return in folder hierarchy", func() {
 				query := search.FindPersistedDashboardsQuery{
-					Title: "test dash 23",
-					OrgId: 1,
-					Mode:  "tree",
+					Title:        "test dash 23",
+					OrgId:        1,
+					Mode:         "tree",
+					SignedInUser: &m.SignedInUser{OrgId: 1},
 				}
 
 				err := SearchDashboards(&query)
@@ -135,8 +137,9 @@ func TestDashboardDataAccess(t *testing.T) {
 
 			Convey("Should be able to search for dashboard folder", func() {
 				query := search.FindPersistedDashboardsQuery{
-					Title: "1 test dash folder",
-					OrgId: 1,
+					Title:        "1 test dash folder",
+					OrgId:        1,
+					SignedInUser: &m.SignedInUser{OrgId: 1},
 				}
 
 				err := SearchDashboards(&query)
@@ -149,8 +152,9 @@ func TestDashboardDataAccess(t *testing.T) {
 
 			Convey("Should be able to search for a dashboard folder's children", func() {
 				query := search.FindPersistedDashboardsQuery{
-					OrgId:    1,
-					ParentId: savedFolder.Id,
+					OrgId:        1,
+					ParentId:     savedFolder.Id,
+					SignedInUser: &m.SignedInUser{OrgId: 1},
 				}
 
 				err := SearchDashboards(&query)
@@ -164,9 +168,9 @@ func TestDashboardDataAccess(t *testing.T) {
 			Convey("Should be able to search for dashboard by dashboard ids", func() {
 				Convey("should be able to find two dashboards by id", func() {
 					query := search.FindPersistedDashboardsQuery{
-						DashboardIds: []int{2, 3},
-						OrgId:        1,
+						DashboardIds: []int64{2, 3},
 						Mode:         "tree",
+						SignedInUser: &m.SignedInUser{OrgId: 1},
 					}
 
 					err := SearchDashboards(&query)
@@ -183,8 +187,8 @@ func TestDashboardDataAccess(t *testing.T) {
 
 				Convey("DashboardIds that does not exists should not cause errors", func() {
 					query := search.FindPersistedDashboardsQuery{
-						DashboardIds: []int{1000},
-						OrgId:        1,
+						DashboardIds: []int64{1000},
+						SignedInUser: &m.SignedInUser{OrgId: 1},
 					}
 
 					err := SearchDashboards(&query)
@@ -253,8 +257,9 @@ func TestDashboardDataAccess(t *testing.T) {
 				So(err, ShouldBeNil)
 
 				query := search.FindPersistedDashboardsQuery{
-					OrgId:    1,
-					ParentId: savedFolder.Id,
+					OrgId:        1,
+					ParentId:     savedFolder.Id,
+					SignedInUser: &m.SignedInUser{},
 				}
 
 				err = SearchDashboards(&query)
@@ -285,7 +290,7 @@ func TestDashboardDataAccess(t *testing.T) {
 				})
 
 				Convey("Should be able to search for starred dashboards", func() {
-					query := search.FindPersistedDashboardsQuery{OrgId: 1, UserId: 10, IsStarred: true}
+					query := search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: 10, OrgId: 1}, IsStarred: true}
 					err := SearchDashboards(&query)
 
 					So(err, ShouldBeNil)
@@ -294,5 +299,95 @@ func TestDashboardDataAccess(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")
+			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", "Viewer", false)
+
+			Convey("and no acls are set", func() {
+				Convey("should return all dashboards", func() {
+					query := &search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, OrgId: 1, DashboardIds: []int64{folder.Id, dashInRoot.Id}}
+					err := SearchDashboards(query)
+					So(err, ShouldBeNil)
+					So(len(query.Result), ShouldEqual, 2)
+					So(query.Result[0].Id, ShouldEqual, folder.Id)
+					So(query.Result[1].Id, ShouldEqual, dashInRoot.Id)
+				})
+			})
+
+			Convey("and acl is set for dashboard folder", func() {
+				var otherUser int64 = 999
+				updateTestDashboardWithAcl(folder.Id, otherUser, m.PERMISSION_EDIT)
+
+				Convey("should not return folder", func() {
+					query := &search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, OrgId: 1, DashboardIds: []int64{folder.Id, dashInRoot.Id}}
+					err := SearchDashboards(query)
+					So(err, ShouldBeNil)
+					So(len(query.Result), ShouldEqual, 1)
+					So(query.Result[0].Id, ShouldEqual, dashInRoot.Id)
+				})
+
+				Convey("when the user is given permission", func() {
+					updateTestDashboardWithAcl(folder.Id, currentUser.Id, m.PERMISSION_EDIT)
+
+					Convey("should be able to access folder", func() {
+						query := &search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, OrgId: 1, DashboardIds: []int64{folder.Id, dashInRoot.Id}}
+						err := SearchDashboards(query)
+						So(err, ShouldBeNil)
+						So(len(query.Result), ShouldEqual, 2)
+						So(query.Result[0].Id, ShouldEqual, folder.Id)
+						So(query.Result[1].Id, ShouldEqual, dashInRoot.Id)
+					})
+				})
+
+				Convey("when the user is an admin", func() {
+					Convey("should be able to access folder", func() {
+						query := &search.FindPersistedDashboardsQuery{
+							SignedInUser: &m.SignedInUser{
+								UserId:  currentUser.Id,
+								OrgId:   1,
+								OrgRole: m.ROLE_ADMIN,
+							},
+							OrgId:        1,
+							DashboardIds: []int64{folder.Id, dashInRoot.Id},
+						}
+						err := SearchDashboards(query)
+						So(err, ShouldBeNil)
+						So(len(query.Result), ShouldEqual, 2)
+						So(query.Result[0].Id, ShouldEqual, folder.Id)
+						So(query.Result[1].Id, ShouldEqual, dashInRoot.Id)
+					})
+				})
+			})
+		})
+	})
+}
+
+func createUser(name string, role string, isAdmin bool) m.User {
+	setting.AutoAssignOrg = true
+	setting.AutoAssignOrgRole = role
+
+	currentUserCmd := m.CreateUserCommand{Login: name, Email: name + "@test.com", Name: "a " + name, IsAdmin: isAdmin}
+	err := CreateUser(&currentUserCmd)
+	So(err, ShouldBeNil)
+
+	q1 := m.GetUserOrgListQuery{UserId: currentUserCmd.Result.Id}
+	GetUserOrgList(&q1)
+	So(q1.Result[0].Role, ShouldEqual, role)
+
+	return currentUserCmd.Result
+}
+
+func updateTestDashboardWithAcl(dashId int64, userId int64, permissionType m.PermissionType) {
+	err := AddOrUpdateDashboardPermission(&m.AddOrUpdateDashboardPermissionCommand{
+		OrgId:          1,
+		UserId:         userId,
+		DashboardId:    dashId,
+		PermissionType: permissionType,
 	})
+	So(err, ShouldBeNil)
 }

pkg/services/sqlstore/guardian.go

@@ -21,7 +21,7 @@ from dashboard as d
 	left join dashboard_acl as dfa on d.parent_id = dfa.dashboard_id or d.id = dfa.dashboard_id
 	left join user_group_member as ugm on ugm.user_group_id =  dfa.user_group_id
 where (
-  (d.has_acl = 1 and (dfa.user_id = ? or ugm.user_id = ? or df.created_by = ? or (d.is_folder = 1 and d.created_by = ?)))
+  (d.has_acl = 1 and (dfa.user_id = ? or ugm.user_id = ?))
   or d.has_acl = 0)
   and d.org_id = ?`
 

pkg/services/sqlstore/guardian_test.go

@@ -4,7 +4,6 @@ import (
 	"testing"
 
 	m "github.com/grafana/grafana/pkg/models"
-	"github.com/grafana/grafana/pkg/setting"
 	. "github.com/smartystreets/goconvey/convey"
 )
 
@@ -60,28 +59,3 @@ func TestGuardianDataAccess(t *testing.T) {
 		})
 	})
 }
-
-func createUser(name string, role string, isAdmin bool) m.User {
-	setting.AutoAssignOrg = true
-	setting.AutoAssignOrgRole = role
-
-	currentUserCmd := m.CreateUserCommand{Login: name, Email: name + "@test.com", Name: "a " + name, IsAdmin: isAdmin}
-	err := CreateUser(&currentUserCmd)
-	So(err, ShouldBeNil)
-
-	q1 := m.GetUserOrgListQuery{UserId: currentUserCmd.Result.Id}
-	GetUserOrgList(&q1)
-	So(q1.Result[0].Role, ShouldEqual, role)
-
-	return currentUserCmd.Result
-}
-
-func updateTestDashboardWithAcl(dashId int64, userId int64, permissionType m.PermissionType) {
-	err := AddOrUpdateDashboardPermission(&m.AddOrUpdateDashboardPermissionCommand{
-		OrgId:          1,
-		UserId:         userId,
-		DashboardId:    dashId,
-		PermissionType: permissionType,
-	})
-	So(err, ShouldBeNil)
-}