Sfoglia il codice sorgente

refactoring dashoard folder guardian

Torkel Ödegaard 8 anni fa
parent
commit
cbbbccf12a

+ 60 - 107
pkg/api/dashboard.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"os"
 	"path"
-	"strings"
 
 	"github.com/grafana/grafana/pkg/api/dtos"
 	"github.com/grafana/grafana/pkg/bus"
@@ -37,15 +36,11 @@ func isDashboardStarredByUser(c *middleware.Context, dashId int64) (bool, error)
 }
 
 func GetDashboard(c *middleware.Context) Response {
-	slug := strings.ToLower(c.Params(":slug"))
-
-	query := m.GetDashboardQuery{Slug: slug, OrgId: c.OrgId}
-
-	if err := bus.Dispatch(&query); err != nil {
-		return ApiError(404, "Dashboard not found", err)
+	dash, rsp := getDashboardHelper(c.OrgId, c.Params(":slug"), 0)
+	if rsp != nil {
+		return rsp
 	}
 
-	dash := query.Result
 	guardian := guardian.NewDashboardGuardian(dash, c.SignedInUser)
 
 	if canView, err := guardian.CanView(); err != nil {
@@ -54,6 +49,9 @@ func GetDashboard(c *middleware.Context) Response {
 		return ApiError(403, "Access denied to this dashboard", nil)
 	}
 
+	canEdit, _ := guardian.CanEdit()
+	canSave, _ := guardian.CanSave()
+
 	isStarred, err := isDashboardStarredByUser(c, dash.Id)
 	if err != nil {
 		return ApiError(500, "Error while checking if dashboard was starred by user", err)
@@ -70,7 +68,7 @@ func GetDashboard(c *middleware.Context) Response {
 
 	meta := dtos.DashboardMeta{
 		IsStarred:   isStarred,
-		Slug:        slug,
+		Slug:        dash.Slug,
 		Type:        m.DashTypeDB,
 		CanStar:     c.IsSignedIn,
 		CanSave:     canSave,
@@ -107,40 +105,6 @@ func GetDashboard(c *middleware.Context) Response {
 	return Json(200, dto)
 }
 
-func getPermissions(dash *m.Dashboard, orgRole m.RoleType, isGrafanaAdmin bool, userId int64) (bool, bool, bool, error) {
-	if !dash.HasAcl {
-		return true, canEditDashboard(orgRole), orgRole == m.ROLE_ADMIN || orgRole == m.ROLE_EDITOR, nil
-	}
-
-	dashId := dash.Id
-
-	if !dash.IsFolder {
-		dashId = dash.ParentId
-	}
-
-	canView, canEdit, canSave, err := guardian.CheckDashboardPermissions(dashId, orgRole, isGrafanaAdmin, userId)
-	if err != nil {
-		return false, false, false, err
-	}
-
-	return canView, canEdit, canSave, nil
-}
-
-func checkIfCanSaveDashboard(dashboardId int64, orgId int64, orgRole m.RoleType, isGrafanaAdmin bool, userId int64) (bool, error) {
-	dashQuery := m.GetDashboardQuery{Id: dashboardId, OrgId: orgId}
-	err := bus.Dispatch(&dashQuery)
-	if err != nil {
-		return false, err
-	}
-
-	_, _, canSave, err := getPermissions(dashQuery.Result, orgRole, isGrafanaAdmin, userId)
-	if err != nil {
-		return false, err
-	}
-
-	return canSave, nil
-}
-
 func getUserLogin(userId int64) string {
 	query := m.GetUserByIdQuery{Id: userId}
 	err := bus.Dispatch(&query)
@@ -152,15 +116,21 @@ func getUserLogin(userId int64) string {
 	}
 }
 
-func DeleteDashboard(c *middleware.Context) Response {
-	slug := c.Params(":slug")
-
-	query := m.GetDashboardQuery{Slug: slug, OrgId: c.OrgId}
+func getDashboardHelper(orgId int64, slug string, id int64) (*m.Dashboard, Response) {
+	query := m.GetDashboardQuery{Slug: slug, Id: id, OrgId: orgId}
 	if err := bus.Dispatch(&query); err != nil {
-		return ApiError(404, "Dashboard not found", err)
+		return nil, ApiError(404, "Dashboard not found", err)
+	}
+	return query.Result, nil
+}
+
+func DeleteDashboard(c *middleware.Context) Response {
+	dash, rsp := getDashboardHelper(c.OrgId, c.Params(":slug"), 0)
+	if rsp != nil {
+		return rsp
 	}
 
-	guardian := guardian.NewDashboardGuardian(query.Result, c.SignedInUser)
+	guardian := guardian.NewDashboardGuardian(dash, c.SignedInUser)
 
 	if canSave, err := guardian.CanSave(); err != nil {
 		return ApiError(500, "Error while checking dashboard permissions", err)
@@ -168,13 +138,12 @@ func DeleteDashboard(c *middleware.Context) Response {
 		return ApiError(403, "Does not have permission to delete this dashboard", nil)
 	}
 
-	cmd := m.DeleteDashboardCommand{Slug: slug, OrgId: c.OrgId}
+	cmd := m.DeleteDashboardCommand{OrgId: c.OrgId, Id: dash.Id}
 	if err := bus.Dispatch(&cmd); err != nil {
 		return ApiError(500, "Failed to delete dashboard", err)
 	}
 
-	var resp = map[string]interface{}{"title": query.Result.Title}
-
+	var resp = map[string]interface{}{"title": dash.Title}
 	return Json(200, resp)
 }
 
@@ -184,22 +153,18 @@ func PostDashboard(c *middleware.Context, cmd m.SaveDashboardCommand) Response {
 
 	dash := cmd.GetDashboardModel()
 
-	query := m.GetDashboardQuery{Slug: dash.Slug, OrgId: c.OrgId}
-	err := bus.Dispatch(&query)
-	if err == nil {
-		dash.IsFolder = query.Result.IsFolder
-		if cmd.ParentId == 0 {
-			dash.ParentId = query.Result.ParentId
+	// look up existing dashboard
+	if dash.Id > 0 {
+		if existing, _ := getDashboardHelper(c.OrgId, "", dash.Id); existing != nil {
+			dash.HasAcl = existing.HasAcl
 		}
-		dash.HasAcl = query.Result.HasAcl
 	}
 
-	_, _, canSave, err := getPermissions(dash, c.OrgRole, c.IsGrafanaAdmin, c.UserId)
-	if err != nil {
-		return ApiError(500, "Error while checking dashboard permissions", err)
-	}
+	guardian := guardian.NewDashboardGuardian(dash, c.SignedInUser)
 
-	if !canSave {
+	if canSave, err := guardian.CanSave(); err != nil {
+		return ApiError(500, "Error while checking dashboard permissions", err)
+	} else if !canSave {
 		return ApiError(403, "Does not have permission to save this dashboard", nil)
 	}
 
@@ -232,7 +197,7 @@ func PostDashboard(c *middleware.Context, cmd m.SaveDashboardCommand) Response {
 		return ApiError(500, "Invalid alert data. Cannot save dashboard", err)
 	}
 
-	err = bus.Dispatch(&cmd)
+	err := bus.Dispatch(&cmd)
 	if err != nil {
 		if err == m.ErrDashboardWithSameNameExists {
 			return Json(412, util.DynMap{"status": "name-exists", "message": err.Error()})
@@ -268,10 +233,6 @@ func PostDashboard(c *middleware.Context, cmd m.SaveDashboardCommand) Response {
 	return Json(200, util.DynMap{"status": "success", "slug": cmd.Result.Slug, "version": cmd.Result.Version, "id": cmd.Result.Id})
 }
 
-func canEditDashboard(role m.RoleType) bool {
-	return role == m.ROLE_ADMIN || role == m.ROLE_EDITOR || role == m.ROLE_READ_ONLY_EDITOR
-}
-
 func GetHomeDashboard(c *middleware.Context) Response {
 	prefsQuery := m.GetPreferencesWithDefaultsQuery{OrgId: c.OrgId, UserId: c.UserId}
 	if err := bus.Dispatch(&prefsQuery); err != nil {
@@ -297,7 +258,7 @@ func GetHomeDashboard(c *middleware.Context) Response {
 
 	dash := dtos.DashboardFullWithMeta{}
 	dash.Meta.IsHome = true
-	dash.Meta.CanEdit = canEditDashboard(c.OrgRole)
+	dash.Meta.CanEdit = c.SignedInUser.HasRole(m.ROLE_READ_ONLY_EDITOR)
 	dash.Meta.FolderTitle = "Root"
 
 	jsonParser := json.NewDecoder(file)
@@ -338,39 +299,34 @@ func GetDashboardFromJsonFile(c *middleware.Context) {
 
 	dash := dtos.DashboardFullWithMeta{Dashboard: dashboard.Data}
 	dash.Meta.Type = m.DashTypeJson
-	dash.Meta.CanEdit = canEditDashboard(c.OrgRole)
+	dash.Meta.CanEdit = c.SignedInUser.HasRole(m.ROLE_READ_ONLY_EDITOR)
 
 	c.JSON(200, &dash)
 }
 
 // GetDashboardVersions returns all dashboard versions as JSON
 func GetDashboardVersions(c *middleware.Context) Response {
-	dashboardId := c.ParamsInt64(":dashboardId")
-	limit := c.QueryInt("limit")
-	start := c.QueryInt("start")
-
-	canSave, err := checkIfCanSaveDashboard(dashboardId, c.OrgId, c.OrgRole, c.IsGrafanaAdmin, c.UserId)
-	if err != nil {
-		return ApiError(500, "Error while checking dashboard permissions", err)
+	dash, rsp := getDashboardHelper(c.OrgId, "", c.ParamsInt64(":dashboardId"))
+	if rsp != nil {
+		return rsp
 	}
 
-	if !canSave {
-		return ApiError(403, "Does not have permission to save this dashboard", nil)
-	}
-
-	if limit == 0 {
-		limit = 1000
+	guardian := guardian.NewDashboardGuardian(dash, c.SignedInUser)
+	if canSave, err := guardian.CanSave(); err != nil {
+		return ApiError(500, "Error while checking dashboard permissions", err)
+	} else if !canSave {
+		return ApiError(403, "Dashboard access denied", nil)
 	}
 
 	query := m.GetDashboardVersionsQuery{
 		OrgId:       c.OrgId,
-		DashboardId: dashboardId,
-		Limit:       limit,
-		Start:       start,
+		DashboardId: dash.Id,
+		Limit:       c.QueryInt("limit"),
+		Start:       c.QueryInt("start"),
 	}
 
 	if err := bus.Dispatch(&query); err != nil {
-		return ApiError(404, fmt.Sprintf("No versions found for dashboardId %d", dashboardId), err)
+		return ApiError(404, fmt.Sprintf("No versions found for dashboardId %d", dash.Id), err)
 	}
 
 	for _, version := range query.Result {
@@ -394,26 +350,26 @@ func GetDashboardVersions(c *middleware.Context) Response {
 
 // GetDashboardVersion returns the dashboard version with the given ID.
 func GetDashboardVersion(c *middleware.Context) Response {
-	dashboardId := c.ParamsInt64(":dashboardId")
-	version := c.ParamsInt(":id")
-
-	canSave, err := checkIfCanSaveDashboard(dashboardId, c.OrgId, c.OrgRole, c.IsGrafanaAdmin, c.UserId)
-	if err != nil {
-		return ApiError(500, "Error while checking dashboard permissions", err)
+	dash, rsp := getDashboardHelper(c.OrgId, "", c.ParamsInt64(":dashboardId"))
+	if rsp != nil {
+		return rsp
 	}
 
-	if !canSave {
-		return ApiError(403, "Does not have permission to save this dashboard", nil)
+	guardian := guardian.NewDashboardGuardian(dash, c.SignedInUser)
+	if canSave, err := guardian.CanSave(); err != nil {
+		return ApiError(500, "Error while checking dashboard permissions", err)
+	} else if !canSave {
+		return ApiError(403, "Dashboard access denied", nil)
 	}
 
 	query := m.GetDashboardVersionQuery{
 		OrgId:       c.OrgId,
-		DashboardId: dashboardId,
-		Version:     version,
+		DashboardId: dash.Id,
+		Version:     c.ParamsInt(":id"),
 	}
 
 	if err := bus.Dispatch(&query); err != nil {
-		return ApiError(500, fmt.Sprintf("Dashboard version %d not found for dashboardId %d", version, dashboardId), err)
+		return ApiError(500, fmt.Sprintf("Dashboard version %d not found for dashboardId %d", query.Version, dash.Id), err)
 	}
 
 	creator := "Anonymous"
@@ -464,19 +420,16 @@ func CalculateDashboardDiff(c *middleware.Context, apiOptions dtos.CalculateDiff
 
 // RestoreDashboardVersion restores a dashboard to the given version.
 func RestoreDashboardVersion(c *middleware.Context, apiCmd dtos.RestoreDashboardVersionCommand) Response {
-	dashboardId := c.ParamsInt64(":dashboardId")
-
-	dashQuery := m.GetDashboardQuery{Id: dashboardId, OrgId: c.OrgId}
-	if err := bus.Dispatch(&dashQuery); err != nil {
-		return ApiError(404, "Dashboard not found", nil)
+	dash, rsp := getDashboardHelper(c.OrgId, "", c.ParamsInt64(":dashboardId"))
+	if rsp != nil {
+		return rsp
 	}
 
-	versionQuery := m.GetDashboardVersionQuery{DashboardId: dashboardId, Version: apiCmd.Version, OrgId: c.OrgId}
+	versionQuery := m.GetDashboardVersionQuery{DashboardId: dash.Id, Version: apiCmd.Version, OrgId: c.OrgId}
 	if err := bus.Dispatch(&versionQuery); err != nil {
 		return ApiError(404, "Dashboard version not found", nil)
 	}
 
-	dashboard := dashQuery.Result
 	version := versionQuery.Result
 
 	saveCmd := m.SaveDashboardCommand{}
@@ -484,7 +437,7 @@ func RestoreDashboardVersion(c *middleware.Context, apiCmd dtos.RestoreDashboard
 	saveCmd.OrgId = c.OrgId
 	saveCmd.UserId = c.UserId
 	saveCmd.Dashboard = version.Data
-	saveCmd.Dashboard.Set("version", dashboard.Version)
+	saveCmd.Dashboard.Set("version", dash.Version)
 	saveCmd.Message = fmt.Sprintf("Restored from version %d", version.Version)
 
 	return PostDashboard(c, saveCmd)

+ 8 - 7
pkg/api/dashboard_acl.go

@@ -10,20 +10,21 @@ import (
 )
 
 func GetDashboardAcl(c *middleware.Context) Response {
-	dashboardId := c.ParamsInt64(":id")
+	dash, rsp := getDashboardHelper(c.OrgId, "", c.ParamsInt64(":id"))
+	if rsp != nil {
+		return rsp
+	}
 
-	hasPermission, err := guardian.CanViewAcl(dashboardId, c.OrgRole, c.IsGrafanaAdmin, c.OrgId, c.UserId)
+	guardian := guardian.NewDashboardGuardian(dash, c.SignedInUser)
 
+	canView, err := guardian.CanView(dashboardId, c.OrgRole, c.IsGrafanaAdmin, c.OrgId, c.UserId)
 	if err != nil {
 		return ApiError(500, "Failed to get Dashboard ACL", err)
-	}
-
-	if !hasPermission {
-		return Json(403, util.DynMap{"status": "Forbidden", "message": "Does not have access to this Dashboard ACL"})
+	} else if !hasPermission {
+		return ApiError(403, "Does not have access to this Dashboard ACL")
 	}
 
 	query := m.GetDashboardPermissionsQuery{DashboardId: dashboardId}
-
 	if err := bus.Dispatch(&query); err != nil {
 		return ApiError(500, "Failed to get Dashboard ACL", err)
 	}

+ 8 - 8
pkg/api/dashboard_acl_test.go

@@ -13,10 +13,10 @@ import (
 func TestDashboardAclApiEndpoint(t *testing.T) {
 	Convey("Given a dashboard acl", t, func() {
 		mockResult := []*models.DashboardAclInfoDTO{
-			{Id: 1, OrgId: 1, DashboardId: 1, UserId: 2, PermissionType: models.PERMISSION_EDIT},
-			{Id: 2, OrgId: 1, DashboardId: 1, UserId: 3, PermissionType: models.PERMISSION_VIEW},
-			{Id: 3, OrgId: 1, DashboardId: 1, UserGroupId: 1, PermissionType: models.PERMISSION_EDIT},
-			{Id: 4, OrgId: 1, DashboardId: 1, UserGroupId: 2, PermissionType: models.PERMISSION_READ_ONLY_EDIT},
+			{Id: 1, OrgId: 1, DashboardId: 1, UserId: 2, Permissions: models.PERMISSION_EDIT},
+			{Id: 2, OrgId: 1, DashboardId: 1, UserId: 3, Permissions: models.PERMISSION_VIEW},
+			{Id: 3, OrgId: 1, DashboardId: 1, UserGroupId: 1, Permissions: models.PERMISSION_EDIT},
+			{Id: 4, OrgId: 1, DashboardId: 1, UserGroupId: 2, Permissions: models.PERMISSION_READ_ONLY_EDIT},
 		}
 		bus.AddHandler("test", func(query *models.GetDashboardPermissionsQuery) error {
 			query.Result = mockResult
@@ -39,14 +39,14 @@ func TestDashboardAclApiEndpoint(t *testing.T) {
 					respJSON, err := simplejson.NewJson(sc.resp.Body.Bytes())
 					So(err, ShouldBeNil)
 					So(respJSON.GetIndex(0).Get("userId").MustInt(), ShouldEqual, 2)
-					So(respJSON.GetIndex(0).Get("permissionType").MustInt(), ShouldEqual, models.PERMISSION_EDIT)
+					So(respJSON.GetIndex(0).Get("permissions").MustInt(), ShouldEqual, models.PERMISSION_EDIT)
 				})
 			})
 		})
 
 		Convey("When user is editor and in the ACL", func() {
 			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/1/acl", "/api/dashboards/:id/acl", models.ROLE_EDITOR, func(sc *scenarioContext) {
-				mockResult = append(mockResult, &models.DashboardAclInfoDTO{Id: 1, OrgId: 1, DashboardId: 1, UserId: 1, PermissionType: models.PERMISSION_EDIT})
+				mockResult = append(mockResult, &models.DashboardAclInfoDTO{Id: 1, OrgId: 1, DashboardId: 1, UserId: 1, Permissions: models.PERMISSION_EDIT})
 
 				bus.AddHandler("test2", func(query *models.GetAllowedDashboardsQuery) error {
 					query.Result = []int64{1}
@@ -62,7 +62,7 @@ func TestDashboardAclApiEndpoint(t *testing.T) {
 			})
 
 			loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/1/acl/user/1", "/api/dashboards/:id/acl/user/:userId", models.ROLE_EDITOR, func(sc *scenarioContext) {
-				mockResult = append(mockResult, &models.DashboardAclInfoDTO{Id: 1, OrgId: 1, DashboardId: 1, UserId: 1, PermissionType: models.PERMISSION_EDIT})
+				mockResult = append(mockResult, &models.DashboardAclInfoDTO{Id: 1, OrgId: 1, DashboardId: 1, UserId: 1, Permissions: models.PERMISSION_EDIT})
 
 				bus.AddHandler("test3", func(cmd *models.RemoveDashboardPermissionCommand) error {
 					return nil
@@ -115,7 +115,7 @@ func TestDashboardAclApiEndpoint(t *testing.T) {
 			})
 
 			loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/1/acl/user/1", "/api/dashboards/:id/acl/user/:userId", models.ROLE_EDITOR, func(sc *scenarioContext) {
-				mockResult = append(mockResult, &models.DashboardAclInfoDTO{Id: 1, OrgId: 1, DashboardId: 1, UserId: 1, PermissionType: models.PERMISSION_VIEW})
+				mockResult = append(mockResult, &models.DashboardAclInfoDTO{Id: 1, OrgId: 1, DashboardId: 1, UserId: 1, Permissions: models.PERMISSION_VIEW})
 				bus.AddHandler("test3", func(cmd *models.RemoveDashboardPermissionCommand) error {
 					return nil
 				})

+ 8 - 2
pkg/api/dashboard_test.go

@@ -21,6 +21,7 @@ import (
 func TestDashboardApiEndpoint(t *testing.T) {
 	Convey("Given a dashboard with a parent folder which does not have an acl", t, func() {
 		fakeDash := models.NewDashboard("Child dash")
+		fakeDash.Id = 1
 		fakeDash.ParentId = 1
 		fakeDash.HasAcl = false
 
@@ -33,6 +34,7 @@ func TestDashboardApiEndpoint(t *testing.T) {
 			Dashboard: simplejson.NewFromAny(map[string]interface{}{
 				"parentId": fakeDash.ParentId,
 				"title":    fakeDash.Title,
+				"id":       fakeDash.Id,
 			}),
 		}
 
@@ -140,6 +142,8 @@ func TestDashboardApiEndpoint(t *testing.T) {
 					return nil
 				})
 				invalidCmd := models.SaveDashboardCommand{
+					ParentId: fakeDash.ParentId,
+					IsFolder: true,
 					Dashboard: simplejson.NewFromAny(map[string]interface{}{
 						"parentId": fakeDash.ParentId,
 						"title":    fakeDash.Title,
@@ -157,6 +161,7 @@ func TestDashboardApiEndpoint(t *testing.T) {
 
 	Convey("Given a dashboard with a parent folder which has an acl", t, func() {
 		fakeDash := models.NewDashboard("Child dash")
+		fakeDash.Id = 1
 		fakeDash.ParentId = 1
 		fakeDash.HasAcl = true
 
@@ -173,6 +178,7 @@ func TestDashboardApiEndpoint(t *testing.T) {
 		cmd := models.SaveDashboardCommand{
 			ParentId: fakeDash.ParentId,
 			Dashboard: simplejson.NewFromAny(map[string]interface{}{
+				"id":       fakeDash.Id,
 				"parentId": fakeDash.ParentId,
 				"title":    fakeDash.Title,
 			}),
@@ -257,7 +263,7 @@ func TestDashboardApiEndpoint(t *testing.T) {
 			role := models.ROLE_VIEWER
 
 			mockResult := []*models.DashboardAclInfoDTO{
-				{Id: 1, OrgId: 1, DashboardId: 2, UserId: 1, PermissionType: models.PERMISSION_EDIT},
+				{Id: 1, OrgId: 1, DashboardId: 2, UserId: 1, Permissions: models.PERMISSION_EDIT},
 			}
 
 			bus.AddHandler("test", func(query *models.GetDashboardPermissionsQuery) error {
@@ -299,7 +305,7 @@ func TestDashboardApiEndpoint(t *testing.T) {
 			role := models.ROLE_EDITOR
 
 			mockResult := []*models.DashboardAclInfoDTO{
-				{Id: 1, OrgId: 1, DashboardId: 2, UserId: 1, PermissionType: models.PERMISSION_VIEW},
+				{Id: 1, OrgId: 1, DashboardId: 2, UserId: 1, Permissions: models.PERMISSION_VIEW},
 			}
 
 			bus.AddHandler("test", func(query *models.GetDashboardPermissionsQuery) error {

+ 5 - 5
pkg/models/dashboard_acl.go

@@ -63,11 +63,11 @@ type DashboardAclInfoDTO struct {
 //
 
 type AddOrUpdateDashboardPermissionCommand struct {
-	DashboardId    int64          `json:"-"`
-	OrgId          int64          `json:"-"`
-	UserId         int64          `json:"userId"`
-	UserGroupId    int64          `json:"userGroupId"`
-	PermissionType PermissionType `json:"permissionType" binding:"Required"`
+	DashboardId int64          `json:"-"`
+	OrgId       int64          `json:"-"`
+	UserId      int64          `json:"userId"`
+	UserGroupId int64          `json:"userGroupId"`
+	Permissions PermissionType `json:"permissionType" binding:"Required"`
 
 	Result DashboardAcl `json:"-"`
 }

+ 1 - 1
pkg/models/dashboards.go

@@ -151,7 +151,7 @@ type SaveDashboardCommand struct {
 }
 
 type DeleteDashboardCommand struct {
-	Slug  string
+	Id    int64
 	OrgId int64
 }
 

+ 12 - 3
pkg/models/org_user.go

@@ -32,11 +32,20 @@ func (r RoleType) Includes(other RoleType) bool {
 	if r == ROLE_ADMIN {
 		return true
 	}
-	if r == ROLE_EDITOR || r == ROLE_READ_ONLY_EDITOR {
-		return other != ROLE_ADMIN
+
+	if other == ROLE_READ_ONLY_EDITOR {
+		return r == ROLE_EDITOR || r == ROLE_READ_ONLY_EDITOR
+	}
+
+	if other == ROLE_EDITOR {
+		return r == ROLE_EDITOR
+	}
+
+	if other == ROLE_VIEWER {
+		return r == ROLE_READ_ONLY_EDITOR || r == ROLE_EDITOR || r == ROLE_VIEWER
 	}
 
-	return r == other
+	return false
 }
 
 func (r *RoleType) UnmarshalJSON(data []byte) error {

+ 5 - 2
pkg/plugins/dashboards.go

@@ -15,6 +15,7 @@ type PluginDashboardInfoDTO struct {
 	Imported         bool   `json:"imported"`
 	ImportedUri      string `json:"importedUri"`
 	Slug             string `json:"slug"`
+	DashboardId      int64  `json:"dashboardId"`
 	ImportedRevision int64  `json:"importedRevision"`
 	Revision         int64  `json:"revision"`
 	Description      string `json:"description"`
@@ -60,6 +61,7 @@ func GetPluginDashboards(orgId int64, pluginId string) ([]*PluginDashboardInfoDT
 		// find existing dashboard
 		for _, existingDash := range query.Result {
 			if existingDash.Slug == dashboard.Slug {
+				res.DashboardId = existingDash.Id
 				res.Imported = true
 				res.ImportedUri = "db/" + existingDash.Slug
 				res.ImportedRevision = existingDash.Data.Get("revision").MustInt64(1)
@@ -74,8 +76,9 @@ func GetPluginDashboards(orgId int64, pluginId string) ([]*PluginDashboardInfoDT
 	for _, dash := range query.Result {
 		if _, exists := existingMatches[dash.Id]; !exists {
 			result = append(result, &PluginDashboardInfoDTO{
-				Slug:    dash.Slug,
-				Removed: true,
+				Slug:        dash.Slug,
+				DashboardId: dash.Id,
+				Removed:     true,
 			})
 		}
 	}

+ 2 - 2
pkg/plugins/dashboards_updater.go

@@ -75,7 +75,7 @@ func syncPluginDashboards(pluginDef *PluginBase, orgId int64) {
 		if dash.Removed {
 			plog.Info("Deleting plugin dashboard", "pluginId", pluginDef.Id, "dashboard", dash.Slug)
 
-			deleteCmd := m.DeleteDashboardCommand{OrgId: orgId, Slug: dash.Slug}
+			deleteCmd := m.DeleteDashboardCommand{OrgId: orgId, Id: dash.DashboardId}
 			if err := bus.Dispatch(&deleteCmd); err != nil {
 				plog.Error("Failed to auto update app dashboard", "pluginId", pluginDef.Id, "error", err)
 				return
@@ -124,7 +124,7 @@ func handlePluginStateChanged(event *m.PluginStateChangedEvent) error {
 			return err
 		} else {
 			for _, dash := range query.Result {
-				deleteCmd := m.DeleteDashboardCommand{OrgId: dash.OrgId, Slug: dash.Slug}
+				deleteCmd := m.DeleteDashboardCommand{OrgId: dash.OrgId, Id: dash.Id}
 
 				plog.Info("Deleting plugin dashboard", "pluginId", event.PluginId, "dashboard", dash.Slug)
 

+ 0 - 143
pkg/services/guardian/guardian.go

@@ -1,143 +0,0 @@
-package guardian
-
-import (
-	"github.com/grafana/grafana/pkg/bus"
-	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 {
-		return true, nil
-	}
-
-	filteredList, err := getAllowedDashboards([]int64{dashboardId}, orgId, userId)
-	if err != nil {
-		return false, err
-	}
-
-	if len(filteredList) > 0 && filteredList[0] == dashboardId {
-		return true, nil
-	}
-
-	return false, nil
-}
-
-// CanDeleteFromAcl determines if a user has permission to delete from a dashboard's ACL
-func CanDeleteFromAcl(dashboardId int64, role m.RoleType, isGrafanaAdmin bool, orgId int64, userId int64) (bool, error) {
-	if role == m.ROLE_ADMIN || isGrafanaAdmin {
-		return true, nil
-	}
-
-	permissions, err := getDashboardPermissions(dashboardId)
-	if err != nil {
-		return false, err
-	}
-
-	if len(permissions) == 0 {
-		return true, nil
-	}
-
-	minimumPermission := m.PERMISSION_EDIT
-	return checkPermission(minimumPermission, permissions, userId)
-}
-
-// CheckDashboardPermissions determines if a user has permission to view, edit or save a dashboard
-func CheckDashboardPermissions(dashboardId int64, role m.RoleType, isGrafanaAdmin bool, userId int64) (bool, bool, bool, error) {
-	if role == m.ROLE_ADMIN || isGrafanaAdmin {
-		return true, true, true, nil
-	}
-
-	permissions, err := getDashboardPermissions(dashboardId)
-	if err != nil {
-		return false, false, false, err
-	}
-
-	if len(permissions) == 0 {
-		return false, false, false, nil
-	}
-
-	minimumPermission := m.PERMISSION_VIEW
-	canView, err := checkPermission(minimumPermission, permissions, userId)
-	if err != nil {
-		return false, false, false, err
-	}
-
-	minimumPermission = m.PERMISSION_READ_ONLY_EDIT
-	canEdit, err := checkPermission(minimumPermission, permissions, userId)
-	if err != nil {
-		return false, false, false, err
-	}
-
-	minimumPermission = m.PERMISSION_EDIT
-	canSave, err := checkPermission(minimumPermission, permissions, userId)
-	if err != nil {
-		return false, false, false, err
-	}
-
-	return canView, canEdit, canSave, nil
-}
-
-func checkPermission(minimumPermission m.PermissionType, permissions []*m.DashboardAclInfoDTO, userId int64) (bool, error) {
-	userGroups, err := getUserGroupsByUser(userId)
-	if err != nil {
-		return false, err
-	}
-
-	for _, p := range permissions {
-		if p.UserId == userId && p.Permissions >= minimumPermission {
-			return true, nil
-		}
-
-		for _, ug := range userGroups {
-			if ug.Id == p.UserGroupId && p.Permissions >= minimumPermission {
-				return true, nil
-			}
-		}
-	}
-
-	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)
-
-	return query.Result, err
-}
-
-func getDashboardPermissions(dashboardId int64) ([]*m.DashboardAclInfoDTO, error) {
-	query := m.GetDashboardPermissionsQuery{DashboardId: dashboardId}
-	err := bus.Dispatch(&query)
-
-	return query.Result, err
-}
-
-func getUserGroupsByUser(userId int64) ([]*m.UserGroup, error) {
-	query := m.GetUserGroupsByUserQuery{UserId: userId}
-	err := bus.Dispatch(&query)
-
-	return query.Result, err
-}

+ 0 - 70
pkg/services/guardian/guardian_test.go

@@ -1,70 +0,0 @@
-package guardian
-
-import (
-	"testing"
-
-	"github.com/grafana/grafana/pkg/bus"
-	m "github.com/grafana/grafana/pkg/models"
-	. "github.com/smartystreets/goconvey/convey"
-)
-
-func TestGuardian(t *testing.T) {
-
-	Convey("Given a user with list of dashboards that they have access to", t, func() {
-		hitList := []int64{1, 2}
-
-		var orgId int64 = 1
-		var userId int64 = 1
-
-		Convey("And the user is a Grafana admin", func() {
-			bus.AddHandler("test", func(query *m.GetSignedInUserQuery) error {
-				query.Result = &m.SignedInUser{IsGrafanaAdmin: true}
-				return nil
-			})
-
-			filteredHitlist, err := FilterRestrictedDashboards(hitList, orgId, userId)
-			So(err, ShouldBeNil)
-
-			Convey("should return all dashboards", func() {
-				So(len(filteredHitlist), ShouldEqual, 2)
-				So(filteredHitlist[0], ShouldEqual, 1)
-				So(filteredHitlist[1], ShouldEqual, 2)
-			})
-		})
-
-		Convey("And the user is an org admin", func() {
-			bus.AddHandler("test", func(query *m.GetSignedInUserQuery) error {
-				query.Result = &m.SignedInUser{IsGrafanaAdmin: false, OrgRole: m.ROLE_ADMIN}
-				return nil
-			})
-
-			filteredHitlist, err := FilterRestrictedDashboards(hitList, orgId, userId)
-			So(err, ShouldBeNil)
-
-			Convey("should return all dashboards", func() {
-				So(len(filteredHitlist), ShouldEqual, 2)
-				So(filteredHitlist[0], ShouldEqual, 1)
-				So(filteredHitlist[1], ShouldEqual, 2)
-			})
-		})
-
-		Convey("And the user is an editor", func() {
-			bus.AddHandler("test", func(query *m.GetSignedInUserQuery) error {
-				query.Result = &m.SignedInUser{IsGrafanaAdmin: false, OrgRole: m.ROLE_EDITOR}
-				return nil
-			})
-			bus.AddHandler("test2", func(query *m.GetAllowedDashboardsQuery) error {
-				query.Result = []int64{1}
-				return nil
-			})
-
-			filteredHitlist, err := FilterRestrictedDashboards(hitList, orgId, userId)
-			So(err, ShouldBeNil)
-
-			Convey("should return dashboard that editor has access to", func() {
-				So(len(filteredHitlist), ShouldEqual, 1)
-				So(filteredHitlist[0], ShouldEqual, 1)
-			})
-		})
-	})
-}

+ 10 - 1
pkg/services/guardian/models.go

@@ -1,6 +1,8 @@
 package guardian
 
 import (
+	"fmt"
+
 	"github.com/grafana/grafana/pkg/bus"
 	m "github.com/grafana/grafana/pkg/models"
 )
@@ -20,6 +22,7 @@ func NewDashboardGuardian(dash *m.Dashboard, user *m.SignedInUser) *DashboardGua
 }
 
 func (g *DashboardGuardian) CanSave() (bool, error) {
+	fmt.Printf("user %v, %v", g.user.OrgRole, g.user.HasRole(m.ROLE_EDITOR))
 	if !g.dashboard.HasAcl {
 		return g.user.HasRole(m.ROLE_EDITOR), nil
 	}
@@ -69,12 +72,18 @@ func (g *DashboardGuardian) HasPermission(permission m.PermissionType) (bool, er
 	return false, nil
 }
 
+// Returns dashboard acl
 func (g *DashboardGuardian) getAcl() ([]*m.DashboardAclInfoDTO, error) {
 	if g.acl != nil {
 		return g.acl, nil
 	}
 
-	query := m.GetDashboardPermissionsQuery{DashboardId: g.dashboard.Id}
+	dashId := g.dashboard.Id
+	if g.dashboard.ParentId != 0 {
+		dashId = g.dashboard.ParentId
+	}
+
+	query := m.GetDashboardPermissionsQuery{DashboardId: dashId}
 	if err := bus.Dispatch(&query); err != nil {
 		return nil, err
 	}

+ 0 - 29
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"
 )
 
@@ -76,11 +75,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)
 
@@ -102,29 +96,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 {

+ 1 - 1
pkg/services/sqlstore/alert_test.go

@@ -192,7 +192,7 @@ func TestAlertingDataAccess(t *testing.T) {
 
 			err = DeleteDashboard(&m.DeleteDashboardCommand{
 				OrgId: 1,
-				Slug:  testDash.Slug,
+				Id:    testDash.Id,
 			})
 
 			So(err, ShouldBeNil)

+ 1 - 1
pkg/services/sqlstore/dashboard.go

@@ -357,7 +357,7 @@ func GetDashboardTags(query *m.GetDashboardTagsQuery) error {
 
 func DeleteDashboard(cmd *m.DeleteDashboardCommand) error {
 	return inTransaction(func(sess *DBSession) error {
-		dashboard := m.Dashboard{Slug: cmd.Slug, OrgId: cmd.OrgId}
+		dashboard := m.Dashboard{Id: cmd.Id, OrgId: cmd.OrgId}
 		has, err := sess.Get(&dashboard)
 		if err != nil {
 			return err

+ 3 - 2
pkg/services/sqlstore/dashboard_acl.go

@@ -23,7 +23,7 @@ func AddOrUpdateDashboardPermission(cmd *m.AddOrUpdateDashboardPermissionCommand
 			return err
 		} else if len(res) == 1 {
 			entity := m.DashboardAcl{
-				Permissions: cmd.PermissionType,
+				Permissions: cmd.Permissions,
 				Updated:     time.Now(),
 			}
 			if _, err := sess.Cols("updated", "permissions").Where("dashboard_id =? and (user_group_id=? or user_id=?)", cmd.DashboardId, cmd.UserGroupId, cmd.UserId).Update(&entity); err != nil {
@@ -40,7 +40,7 @@ func AddOrUpdateDashboardPermission(cmd *m.AddOrUpdateDashboardPermissionCommand
 			Created:     time.Now(),
 			Updated:     time.Now(),
 			DashboardId: cmd.DashboardId,
-			Permissions: cmd.PermissionType,
+			Permissions: cmd.Permissions,
 		}
 
 		cols := []string{"org_id", "created", "updated", "dashboard_id", "permissions"}
@@ -64,6 +64,7 @@ func AddOrUpdateDashboardPermission(cmd *m.AddOrUpdateDashboardPermissionCommand
 		dashboard := m.Dashboard{
 			HasAcl: true,
 		}
+
 		if _, err := sess.Cols("has_acl").Where("id=? OR parent_id=?", cmd.DashboardId, cmd.DashboardId).Update(&dashboard); err != nil {
 			return err
 		}

+ 24 - 24
pkg/services/sqlstore/dashboard_acl_test.go

@@ -18,19 +18,19 @@ func TestDashboardAclDataAccess(t *testing.T) {
 
 			Convey("When adding dashboard permission with userId and userGroupId set to 0", func() {
 				err := AddOrUpdateDashboardPermission(&m.AddOrUpdateDashboardPermissionCommand{
-					OrgId:          1,
-					DashboardId:    savedFolder.Id,
-					PermissionType: m.PERMISSION_EDIT,
+					OrgId:       1,
+					DashboardId: savedFolder.Id,
+					Permissions: m.PERMISSION_EDIT,
 				})
 				So(err, ShouldEqual, m.ErrDashboardPermissionUserOrUserGroupEmpty)
 			})
 
 			Convey("Should be able to add dashboard permission", func() {
 				err := AddOrUpdateDashboardPermission(&m.AddOrUpdateDashboardPermissionCommand{
-					OrgId:          1,
-					UserId:         currentUser.Id,
-					DashboardId:    savedFolder.Id,
-					PermissionType: m.PERMISSION_EDIT,
+					OrgId:       1,
+					UserId:      currentUser.Id,
+					DashboardId: savedFolder.Id,
+					Permissions: m.PERMISSION_EDIT,
 				})
 				So(err, ShouldBeNil)
 
@@ -38,8 +38,8 @@ func TestDashboardAclDataAccess(t *testing.T) {
 				err = GetDashboardPermissions(q1)
 				So(err, ShouldBeNil)
 				So(q1.Result[0].DashboardId, ShouldEqual, savedFolder.Id)
-				So(q1.Result[0].PermissionType, ShouldEqual, m.PERMISSION_EDIT)
-				So(q1.Result[0].Permissions, ShouldEqual, "Edit")
+				So(q1.Result[0].Permissions, ShouldEqual, m.PERMISSION_EDIT)
+				So(q1.Result[0].PermissionName, ShouldEqual, "Edit")
 				So(q1.Result[0].UserId, ShouldEqual, currentUser.Id)
 				So(q1.Result[0].UserLogin, ShouldEqual, currentUser.Login)
 				So(q1.Result[0].UserEmail, ShouldEqual, currentUser.Email)
@@ -54,10 +54,10 @@ func TestDashboardAclDataAccess(t *testing.T) {
 
 				Convey("Should be able to update an existing permission", func() {
 					err := AddOrUpdateDashboardPermission(&m.AddOrUpdateDashboardPermissionCommand{
-						OrgId:          1,
-						UserId:         1,
-						DashboardId:    savedFolder.Id,
-						PermissionType: m.PERMISSION_READ_ONLY_EDIT,
+						OrgId:       1,
+						UserId:      1,
+						DashboardId: savedFolder.Id,
+						Permissions: m.PERMISSION_READ_ONLY_EDIT,
 					})
 					So(err, ShouldBeNil)
 
@@ -66,7 +66,7 @@ func TestDashboardAclDataAccess(t *testing.T) {
 					So(err, ShouldBeNil)
 					So(len(q3.Result), ShouldEqual, 1)
 					So(q3.Result[0].DashboardId, ShouldEqual, savedFolder.Id)
-					So(q3.Result[0].PermissionType, ShouldEqual, m.PERMISSION_READ_ONLY_EDIT)
+					So(q3.Result[0].Permissions, ShouldEqual, m.PERMISSION_READ_ONLY_EDIT)
 					So(q3.Result[0].UserId, ShouldEqual, 1)
 
 				})
@@ -93,10 +93,10 @@ func TestDashboardAclDataAccess(t *testing.T) {
 
 				Convey("Should be able to add a user permission for a user group", func() {
 					err := AddOrUpdateDashboardPermission(&m.AddOrUpdateDashboardPermissionCommand{
-						OrgId:          1,
-						UserGroupId:    group1.Result.Id,
-						DashboardId:    savedFolder.Id,
-						PermissionType: m.PERMISSION_EDIT,
+						OrgId:       1,
+						UserGroupId: group1.Result.Id,
+						DashboardId: savedFolder.Id,
+						Permissions: m.PERMISSION_EDIT,
 					})
 					So(err, ShouldBeNil)
 
@@ -104,16 +104,16 @@ func TestDashboardAclDataAccess(t *testing.T) {
 					err = GetDashboardPermissions(q1)
 					So(err, ShouldBeNil)
 					So(q1.Result[0].DashboardId, ShouldEqual, savedFolder.Id)
-					So(q1.Result[0].PermissionType, ShouldEqual, m.PERMISSION_EDIT)
+					So(q1.Result[0].Permissions, ShouldEqual, m.PERMISSION_EDIT)
 					So(q1.Result[0].UserGroupId, ShouldEqual, group1.Result.Id)
 				})
 
 				Convey("Should be able to update an existing permission for a user group", func() {
 					err := AddOrUpdateDashboardPermission(&m.AddOrUpdateDashboardPermissionCommand{
-						OrgId:          1,
-						UserGroupId:    group1.Result.Id,
-						DashboardId:    savedFolder.Id,
-						PermissionType: m.PERMISSION_READ_ONLY_EDIT,
+						OrgId:       1,
+						UserGroupId: group1.Result.Id,
+						DashboardId: savedFolder.Id,
+						Permissions: m.PERMISSION_READ_ONLY_EDIT,
 					})
 					So(err, ShouldBeNil)
 
@@ -122,7 +122,7 @@ func TestDashboardAclDataAccess(t *testing.T) {
 					So(err, ShouldBeNil)
 					So(len(q3.Result), ShouldEqual, 1)
 					So(q3.Result[0].DashboardId, ShouldEqual, savedFolder.Id)
-					So(q3.Result[0].PermissionType, ShouldEqual, m.PERMISSION_READ_ONLY_EDIT)
+					So(q3.Result[0].Permissions, ShouldEqual, m.PERMISSION_READ_ONLY_EDIT)
 					So(q3.Result[0].UserGroupId, ShouldEqual, group1.Result.Id)
 
 				})

+ 2 - 5
pkg/services/sqlstore/dashboard_test.go

@@ -5,7 +5,6 @@ import (
 
 	. "github.com/smartystreets/goconvey/convey"
 
-	"github.com/gosimple/slug"
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	m "github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/search"
@@ -69,12 +68,10 @@ func TestDashboardDataAccess(t *testing.T) {
 			})
 
 			Convey("Should be able to delete dashboard", func() {
-				insertTestDashboard("delete me", 1, 0, false, "delete this")
-
-				dashboardSlug := slug.Make("delete me")
+				dash := insertTestDashboard("delete me", 1, 0, false, "delete this")
 
 				err := DeleteDashboard(&m.DeleteDashboardCommand{
-					Slug:  dashboardSlug,
+					Id:    dash.Id,
 					OrgId: 1,
 				})
 

+ 4 - 0
pkg/services/sqlstore/dashboard_version.go

@@ -32,6 +32,10 @@ func GetDashboardVersion(query *m.GetDashboardVersionQuery) error {
 
 // GetDashboardVersions gets all dashboard versions for the given dashboard ID.
 func GetDashboardVersions(query *m.GetDashboardVersionsQuery) error {
+	if query.Limit == 0 {
+		query.Limit = 1000
+	}
+
 	err := x.Table("dashboard_version").
 		Select(`dashboard_version.id,
 				dashboard_version.dashboard_id,

+ 2 - 2
pkg/services/sqlstore/org_test.go

@@ -174,10 +174,10 @@ func TestAccountDataAccess(t *testing.T) {
 					So(err, ShouldBeNil)
 					So(len(query.Result), ShouldEqual, 3)
 
-					err = AddOrUpdateDashboardPermission(&m.AddOrUpdateDashboardPermissionCommand{DashboardId: 1, OrgId: ac1.OrgId, UserId: ac3.Id, PermissionType: m.PERMISSION_EDIT})
+					err = AddOrUpdateDashboardPermission(&m.AddOrUpdateDashboardPermissionCommand{DashboardId: 1, OrgId: ac1.OrgId, UserId: ac3.Id, Permissions: m.PERMISSION_EDIT})
 					So(err, ShouldBeNil)
 
-					err = AddOrUpdateDashboardPermission(&m.AddOrUpdateDashboardPermissionCommand{DashboardId: 2, OrgId: ac3.OrgId, UserId: ac3.Id, PermissionType: m.PERMISSION_EDIT})
+					err = AddOrUpdateDashboardPermission(&m.AddOrUpdateDashboardPermissionCommand{DashboardId: 2, OrgId: ac3.OrgId, UserId: ac3.Id, Permissions: m.PERMISSION_EDIT})
 					So(err, ShouldBeNil)
 
 					Convey("When org user is deleted", func() {