Dashboard acl query fixes (#10909)

* initial fixes for dashboard permission acl list query, fixes #10864

* permissions: refactoring of acl api and query

pkg/api/api.go

@@ -269,7 +269,6 @@ func (hs *HttpServer) registerRoutes() {
 				dashIdRoute.Group("/acl", func(aclRoute RouteRegister) {
 					aclRoute.Get("/", wrap(GetDashboardAclList))
 					aclRoute.Post("/", bind(dtos.UpdateDashboardAclCommand{}), wrap(UpdateDashboardAcl))
-					aclRoute.Delete("/:aclId", wrap(DeleteDashboardAcl))
 				})
 			})
 		})

pkg/api/dashboard_acl.go

@@ -84,33 +84,3 @@ func UpdateDashboardAcl(c *middleware.Context, apiCmd dtos.UpdateDashboardAclCom
 
 	return ApiSuccess("Dashboard acl updated")
 }
-
-func DeleteDashboardAcl(c *middleware.Context) Response {
-	dashId := c.ParamsInt64(":dashboardId")
-	aclId := c.ParamsInt64(":aclId")
-
-	_, rsp := getDashboardHelper(c.OrgId, "", dashId, "")
-	if rsp != nil {
-		return rsp
-	}
-
-	guardian := guardian.NewDashboardGuardian(dashId, c.OrgId, c.SignedInUser)
-	if canAdmin, err := guardian.CanAdmin(); err != nil || !canAdmin {
-		return dashboardGuardianResponse(err)
-	}
-
-	if okToDelete, err := guardian.CheckPermissionBeforeRemove(m.PERMISSION_ADMIN, aclId); err != nil || !okToDelete {
-		if err != nil {
-			return ApiError(500, "Error while checking dashboard permissions", err)
-		}
-
-		return ApiError(403, "Cannot remove own admin permission for a folder", nil)
-	}
-
-	cmd := m.RemoveDashboardAclCommand{OrgId: c.OrgId, AclId: aclId}
-	if err := bus.Dispatch(&cmd); err != nil {
-		return ApiError(500, "Failed to delete permission for user", err)
-	}
-
-	return Json(200, "")
-}

pkg/api/dashboard_acl_test.go

@@ -15,11 +15,11 @@ import (
 func TestDashboardAclApiEndpoint(t *testing.T) {
 	Convey("Given a dashboard acl", t, func() {
 		mockResult := []*m.DashboardAclInfoDTO{
-			{Id: 1, OrgId: 1, DashboardId: 1, UserId: 2, Permission: m.PERMISSION_VIEW},
+			{OrgId: 1, DashboardId: 1, UserId: 2, Permission: m.PERMISSION_VIEW},
-			{Id: 2, OrgId: 1, DashboardId: 1, UserId: 3, Permission: m.PERMISSION_EDIT},
+			{OrgId: 1, DashboardId: 1, UserId: 3, Permission: m.PERMISSION_EDIT},
-			{Id: 3, OrgId: 1, DashboardId: 1, UserId: 4, Permission: m.PERMISSION_ADMIN},
+			{OrgId: 1, DashboardId: 1, UserId: 4, Permission: m.PERMISSION_ADMIN},
-			{Id: 4, OrgId: 1, DashboardId: 1, TeamId: 1, Permission: m.PERMISSION_VIEW},
+			{OrgId: 1, DashboardId: 1, TeamId: 1, Permission: m.PERMISSION_VIEW},
-			{Id: 5, OrgId: 1, DashboardId: 1, TeamId: 2, Permission: m.PERMISSION_ADMIN},
+			{OrgId: 1, DashboardId: 1, TeamId: 2, Permission: m.PERMISSION_ADMIN},
 		}
 		dtoRes := transformDashboardAclsToDTOs(mockResult)
 
@@ -92,21 +92,11 @@ func TestDashboardAclApiEndpoint(t *testing.T) {
 					So(sc.resp.Code, ShouldEqual, 404)
 				})
 			})
-
-			loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/id/2/acl/6", "/api/dashboards/id/:dashboardId/acl/:aclId", m.ROLE_ADMIN, func(sc *scenarioContext) {
-				getDashboardNotFoundError = m.ErrDashboardNotFound
-				sc.handlerFunc = DeleteDashboardAcl
-				sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec()
-
-				Convey("Should not be able to delete non-existing dashboard", func() {
-					So(sc.resp.Code, ShouldEqual, 404)
-				})
-			})
 		})
 
 		Convey("When user is org editor and has admin permission in the ACL", func() {
 			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/1/acl", "/api/dashboards/id/:dashboardId/acl", m.ROLE_EDITOR, func(sc *scenarioContext) {
-				mockResult = append(mockResult, &m.DashboardAclInfoDTO{Id: 6, OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_ADMIN})
+				mockResult = append(mockResult, &m.DashboardAclInfoDTO{OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_ADMIN})
 
 				Convey("Should be able to access ACL", func() {
 					sc.handlerFunc = GetDashboardAclList
@@ -116,36 +106,6 @@ func TestDashboardAclApiEndpoint(t *testing.T) {
 				})
 			})
 
-			loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/id/1/acl/1", "/api/dashboards/id/:dashboardId/acl/:aclId", m.ROLE_EDITOR, func(sc *scenarioContext) {
-				mockResult = append(mockResult, &m.DashboardAclInfoDTO{Id: 6, OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_ADMIN})
-
-				bus.AddHandler("test3", func(cmd *m.RemoveDashboardAclCommand) error {
-					return nil
-				})
-
-				Convey("Should be able to delete permission", func() {
-					sc.handlerFunc = DeleteDashboardAcl
-					sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec()
-
-					So(sc.resp.Code, ShouldEqual, 200)
-				})
-			})
-
-			loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/id/1/acl/6", "/api/dashboards/id/:dashboardId/acl/:aclId", m.ROLE_EDITOR, func(sc *scenarioContext) {
-				mockResult = append(mockResult, &m.DashboardAclInfoDTO{Id: 6, OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_ADMIN})
-
-				bus.AddHandler("test3", func(cmd *m.RemoveDashboardAclCommand) error {
-					return nil
-				})
-
-				Convey("Should not be able to delete their own Admin permission", func() {
-					sc.handlerFunc = DeleteDashboardAcl
-					sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec()
-
-					So(sc.resp.Code, ShouldEqual, 403)
-				})
-			})
-
 			Convey("Should not be able to downgrade their own Admin permission", func() {
 				cmd := dtos.UpdateDashboardAclCommand{
 					Items: []dtos.DashboardAclUpdateItem{
@@ -154,7 +114,7 @@ func TestDashboardAclApiEndpoint(t *testing.T) {
 				}
 
 				postAclScenario("When calling POST on", "/api/dashboards/id/1/acl", "/api/dashboards/id/:dashboardId/acl", m.ROLE_EDITOR, cmd, func(sc *scenarioContext) {
-					mockResult = append(mockResult, &m.DashboardAclInfoDTO{Id: 6, OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_ADMIN})
+					mockResult = append(mockResult, &m.DashboardAclInfoDTO{OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_ADMIN})
 
 					CallPostAcl(sc)
 					So(sc.resp.Code, ShouldEqual, 403)
@@ -170,34 +130,18 @@ func TestDashboardAclApiEndpoint(t *testing.T) {
 				}
 
 				postAclScenario("When calling POST on", "/api/dashboards/id/1/acl", "/api/dashboards/id/:dashboardId/acl", m.ROLE_EDITOR, cmd, func(sc *scenarioContext) {
-					mockResult = append(mockResult, &m.DashboardAclInfoDTO{Id: 6, OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_ADMIN})
+					mockResult = append(mockResult, &m.DashboardAclInfoDTO{OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_ADMIN})
 
 					CallPostAcl(sc)
 					So(sc.resp.Code, ShouldEqual, 200)
 				})
 			})
 
-			Convey("When user is a member of a team in the ACL with admin permission", func() {
-				loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/id/1/acl/1", "/api/dashboards/id/:dashboardsId/acl/:aclId", m.ROLE_EDITOR, func(sc *scenarioContext) {
-					teamResp = append(teamResp, &m.Team{Id: 2, OrgId: 1, Name: "UG2"})
-
-					bus.AddHandler("test3", func(cmd *m.RemoveDashboardAclCommand) error {
-						return nil
-					})
-
-					Convey("Should be able to delete permission", func() {
-						sc.handlerFunc = DeleteDashboardAcl
-						sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec()
-
-						So(sc.resp.Code, ShouldEqual, 200)
-					})
-				})
-			})
 		})
 
 		Convey("When user is org viewer and has edit permission in the ACL", func() {
 			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/1/acl", "/api/dashboards/id/:dashboardId/acl", m.ROLE_VIEWER, func(sc *scenarioContext) {
-				mockResult = append(mockResult, &m.DashboardAclInfoDTO{Id: 1, OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_EDIT})
+				mockResult = append(mockResult, &m.DashboardAclInfoDTO{OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_EDIT})
 
 				// Getting the permissions is an Admin permission
 				Convey("Should not be able to get list of permissions from ACL", func() {
@@ -207,21 +151,6 @@ func TestDashboardAclApiEndpoint(t *testing.T) {
 					So(sc.resp.Code, ShouldEqual, 403)
 				})
 			})
-
-			loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/id/1/acl/1", "/api/dashboards/id/:dashboardId/acl/:aclId", m.ROLE_VIEWER, func(sc *scenarioContext) {
-				mockResult = append(mockResult, &m.DashboardAclInfoDTO{Id: 1, OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_EDIT})
-
-				bus.AddHandler("test3", func(cmd *m.RemoveDashboardAclCommand) error {
-					return nil
-				})
-
-				Convey("Should be not be able to delete permission", func() {
-					sc.handlerFunc = DeleteDashboardAcl
-					sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec()
-
-					So(sc.resp.Code, ShouldEqual, 403)
-				})
-			})
 		})
 
 		Convey("When user is org editor and not in the ACL", func() {
@@ -234,20 +163,6 @@ func TestDashboardAclApiEndpoint(t *testing.T) {
 					So(sc.resp.Code, ShouldEqual, 403)
 				})
 			})
-
-			loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/id/1/acl/user/1", "/api/dashboards/id/:dashboardsId/acl/user/:userId", m.ROLE_EDITOR, func(sc *scenarioContext) {
-				mockResult = append(mockResult, &m.DashboardAclInfoDTO{Id: 1, OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_VIEW})
-				bus.AddHandler("test3", func(cmd *m.RemoveDashboardAclCommand) error {
-					return nil
-				})
-
-				Convey("Should be not be able to delete permission", func() {
-					sc.handlerFunc = DeleteDashboardAcl
-					sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec()
-
-					So(sc.resp.Code, ShouldEqual, 403)
-				})
-			})
 		})
 	})
 }
@@ -257,7 +172,6 @@ func transformDashboardAclsToDTOs(acls []*m.DashboardAclInfoDTO) []*m.DashboardA
 
 	for _, acl := range acls {
 		dto := &m.DashboardAclInfoDTO{
-			Id:          acl.Id,
 			OrgId:       acl.OrgId,
 			DashboardId: acl.DashboardId,
 			Permission:  acl.Permission,

pkg/api/dashboard_test.go

@@ -431,7 +431,7 @@ func TestDashboardApiEndpoint(t *testing.T) {
 			role := m.ROLE_VIEWER
 
 			mockResult := []*m.DashboardAclInfoDTO{
-				{Id: 1, OrgId: 1, DashboardId: 2, UserId: 1, Permission: m.PERMISSION_EDIT},
+				{OrgId: 1, DashboardId: 2, UserId: 1, Permission: m.PERMISSION_EDIT},
 			}
 
 			bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error {
@@ -505,7 +505,7 @@ func TestDashboardApiEndpoint(t *testing.T) {
 			setting.ViewersCanEdit = true
 
 			mockResult := []*m.DashboardAclInfoDTO{
-				{Id: 1, OrgId: 1, DashboardId: 2, UserId: 1, Permission: m.PERMISSION_VIEW},
+				{OrgId: 1, DashboardId: 2, UserId: 1, Permission: m.PERMISSION_VIEW},
 			}
 
 			bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error {
@@ -564,7 +564,7 @@ func TestDashboardApiEndpoint(t *testing.T) {
 			role := m.ROLE_VIEWER
 
 			mockResult := []*m.DashboardAclInfoDTO{
-				{Id: 1, OrgId: 1, DashboardId: 2, UserId: 1, Permission: m.PERMISSION_ADMIN},
+				{OrgId: 1, DashboardId: 2, UserId: 1, Permission: m.PERMISSION_ADMIN},
 			}
 
 			bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error {
@@ -637,7 +637,7 @@ func TestDashboardApiEndpoint(t *testing.T) {
 			role := m.ROLE_EDITOR
 
 			mockResult := []*m.DashboardAclInfoDTO{
-				{Id: 1, OrgId: 1, DashboardId: 2, UserId: 1, Permission: m.PERMISSION_VIEW},
+				{OrgId: 1, DashboardId: 2, UserId: 1, Permission: m.PERMISSION_VIEW},
 			}
 
 			bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error {

pkg/models/dashboard_acl.go

@@ -44,7 +44,6 @@ type DashboardAcl struct {
 }
 
 type DashboardAclInfoDTO struct {
-	Id          int64 `json:"id"`
 	OrgId       int64 `json:"-"`
 	DashboardId int64 `json:"dashboardId"`
 
@@ -75,21 +74,6 @@ type UpdateDashboardAclCommand struct {
 	Items       []*DashboardAcl
 }
 
-type SetDashboardAclCommand struct {
-	DashboardId int64
-	OrgId       int64
-	UserId      int64
-	TeamId      int64
-	Permission  PermissionType
-
-	Result DashboardAcl
-}
-
-type RemoveDashboardAclCommand struct {
-	AclId int64
-	OrgId int64
-}
-
 //
 // QUERIES
 //

pkg/services/guardian/guardian.go

@@ -106,26 +106,6 @@ func (g *DashboardGuardian) checkAcl(permission m.PermissionType, acl []*m.Dashb
 	return false, nil
 }
 
-func (g *DashboardGuardian) CheckPermissionBeforeRemove(permission m.PermissionType, aclIdToRemove int64) (bool, error) {
-	if g.user.OrgRole == m.ROLE_ADMIN {
-		return true, nil
-	}
-
-	acl, err := g.GetAcl()
-	if err != nil {
-		return false, err
-	}
-
-	for i, p := range acl {
-		if p.Id == aclIdToRemove {
-			acl = append(acl[:i], acl[i+1:]...)
-			break
-		}
-	}
-
-	return g.checkAcl(permission, acl)
-}
-
 func (g *DashboardGuardian) CheckPermissionBeforeUpdate(permission m.PermissionType, updatePermissions []*m.DashboardAcl) (bool, error) {
 	if g.user.OrgRole == m.ROLE_ADMIN {
 		return true, nil

pkg/services/sqlstore/dashboard.go

@@ -79,11 +79,6 @@ func saveDashboard(sess *DBSession, cmd *m.SaveDashboardCommand) error {
 		dash.Data.Set("uid", uid)
 	}
 
-	err = setHasAcl(sess, dash)
-	if err != nil {
-		return err
-	}
-
 	parentVersion := dash.Version
 	affectedRows := int64(0)
 
@@ -100,7 +95,7 @@ func saveDashboard(sess *DBSession, cmd *m.SaveDashboardCommand) error {
 			dash.Updated = cmd.UpdatedAt
 		}
 
-		affectedRows, err = sess.MustCols("folder_id", "has_acl").ID(dash.Id).Update(dash)
+		affectedRows, err = sess.MustCols("folder_id").ID(dash.Id).Update(dash)
 	}
 
 	if err != nil {
@@ -233,31 +228,6 @@ func generateNewDashboardUid(sess *DBSession, orgId int64) (string, error) {
 	return "", m.ErrDashboardFailedGenerateUniqueUid
 }
 
-func setHasAcl(sess *DBSession, dash *m.Dashboard) error {
-	// check if parent has acl
-	if dash.FolderId > 0 {
-		var parent m.Dashboard
-		if hasParent, err := sess.Where("folder_id=?", dash.FolderId).Get(&parent); err != nil {
-			return err
-		} else if hasParent && parent.HasAcl {
-			dash.HasAcl = true
-		}
-	}
-
-	// check if dash has its own acl
-	if dash.Id > 0 {
-		if res, err := sess.Query("SELECT 1 from dashboard_acl WHERE dashboard_id =?", dash.Id); err != nil {
-			return err
-		} else {
-			if len(res) > 0 {
-				dash.HasAcl = true
-			}
-		}
-	}
-
-	return nil
-}
-
 func GetDashboard(query *m.GetDashboardQuery) error {
 	dashboard := m.Dashboard{Slug: query.Slug, OrgId: query.OrgId, Id: query.Id, Uid: query.Uid}
 	has, err := x.Get(&dashboard)

pkg/services/sqlstore/dashboard_acl.go

@@ -1,17 +1,12 @@
 package sqlstore
 
 import (
-	"fmt"
-	"time"
-
 	"github.com/grafana/grafana/pkg/bus"
 	m "github.com/grafana/grafana/pkg/models"
 )
 
 func init() {
-	bus.AddHandler("sql", SetDashboardAcl)
 	bus.AddHandler("sql", UpdateDashboardAcl)
-	bus.AddHandler("sql", RemoveDashboardAcl)
 	bus.AddHandler("sql", GetDashboardAclInfoList)
 }
 
@@ -24,7 +19,7 @@ func UpdateDashboardAcl(cmd *m.UpdateDashboardAclCommand) error {
 		}
 
 		for _, item := range cmd.Items {
-			if item.UserId == 0 && item.TeamId == 0 && !item.Role.IsValid() {
+			if item.UserId == 0 && item.TeamId == 0 && (item.Role == nil || !item.Role.IsValid()) {
 				return m.ErrDashboardAclInfoMissing
 			}
 
@@ -40,92 +35,13 @@ func UpdateDashboardAcl(cmd *m.UpdateDashboardAclCommand) error {
 
 		// Update dashboard HasAcl flag
 		dashboard := m.Dashboard{HasAcl: true}
-		if _, err := sess.Cols("has_acl").Where("id=? OR folder_id=?", cmd.DashboardId, cmd.DashboardId).Update(&dashboard); err != nil {
+		if _, err := sess.Cols("has_acl").Where("id=?", cmd.DashboardId).Update(&dashboard); err != nil {
 			return err
 		}
 		return nil
 	})
 }
 
-func SetDashboardAcl(cmd *m.SetDashboardAclCommand) error {
-	return inTransaction(func(sess *DBSession) error {
-		if cmd.UserId == 0 && cmd.TeamId == 0 {
-			return m.ErrDashboardAclInfoMissing
-		}
-
-		if cmd.DashboardId == 0 {
-			return m.ErrDashboardPermissionDashboardEmpty
-		}
-
-		if res, err := sess.Query("SELECT 1 from "+dialect.Quote("dashboard_acl")+" WHERE dashboard_id =? and (team_id=? or user_id=?)", cmd.DashboardId, cmd.TeamId, cmd.UserId); err != nil {
-			return err
-		} else if len(res) == 1 {
-
-			entity := m.DashboardAcl{
-				Permission: cmd.Permission,
-				Updated:    time.Now(),
-			}
-
-			if _, err := sess.Cols("updated", "permission").Where("dashboard_id =? and (team_id=? or user_id=?)", cmd.DashboardId, cmd.TeamId, cmd.UserId).Update(&entity); err != nil {
-				return err
-			}
-
-			return nil
-		}
-
-		entity := m.DashboardAcl{
-			OrgId:       cmd.OrgId,
-			TeamId:      cmd.TeamId,
-			UserId:      cmd.UserId,
-			Created:     time.Now(),
-			Updated:     time.Now(),
-			DashboardId: cmd.DashboardId,
-			Permission:  cmd.Permission,
-		}
-
-		cols := []string{"org_id", "created", "updated", "dashboard_id", "permission"}
-
-		if cmd.UserId != 0 {
-			cols = append(cols, "user_id")
-		}
-
-		if cmd.TeamId != 0 {
-			cols = append(cols, "team_id")
-		}
-
-		_, err := sess.Cols(cols...).Insert(&entity)
-		if err != nil {
-			return err
-		}
-
-		cmd.Result = entity
-
-		// Update dashboard HasAcl flag
-		dashboard := m.Dashboard{
-			HasAcl: true,
-		}
-
-		if _, err := sess.Cols("has_acl").Where("id=? OR folder_id=?", cmd.DashboardId, cmd.DashboardId).Update(&dashboard); err != nil {
-			return err
-		}
-
-		return nil
-	})
-}
-
-// RemoveDashboardAcl removes a specified permission from the dashboard acl
-func RemoveDashboardAcl(cmd *m.RemoveDashboardAclCommand) error {
-	return inTransaction(func(sess *DBSession) error {
-		var rawSQL = "DELETE FROM " + dialect.Quote("dashboard_acl") + " WHERE org_id =? and id=?"
-		_, err := sess.Exec(rawSQL, cmd.OrgId, cmd.AclId)
-		if err != nil {
-			return err
-		}
-
-		return err
-	})
-}
-
 // GetDashboardAclInfoList returns a list of permissions for a dashboard. They can be fetched from three
 // different places.
 // 1) Permissions for the dashboard
@@ -134,6 +50,8 @@ func RemoveDashboardAcl(cmd *m.RemoveDashboardAclCommand) error {
 func GetDashboardAclInfoList(query *m.GetDashboardAclInfoListQuery) error {
 	var err error
 
+	falseStr := dialect.BooleanStr(false)
+
 	if query.DashboardId == 0 {
 		sql := `SELECT
 		da.id,
@@ -151,18 +69,13 @@ func GetDashboardAclInfoList(query *m.GetDashboardAclInfoListQuery) error {
 		'' as title,
 		'' as slug,
 		'' as uid,` +
-			dialect.BooleanStr(false) + ` AS is_folder
+			falseStr + ` AS is_folder
 		FROM dashboard_acl as da
 		WHERE da.dashboard_id = -1`
 		query.Result = make([]*m.DashboardAclInfoDTO, 0)
 		err = x.SQL(sql).Find(&query.Result)
 
 	} else {
-		dashboardFilter := fmt.Sprintf(`IN (
-			SELECT %d
-			UNION
-			SELECT folder_id from dashboard where id = %d
-		  )`, query.DashboardId, query.DashboardId)
 
 		rawSQL := `
 			-- get permissions for the dashboard and its parent folder
@@ -183,41 +96,21 @@ func GetDashboardAclInfoList(query *m.GetDashboardAclInfoListQuery) error {
 				d.slug,
 				d.uid,
 				d.is_folder
-		  FROM` + dialect.Quote("dashboard_acl") + ` as da
+			FROM dashboard as d
-				LEFT OUTER JOIN ` + dialect.Quote("user") + ` AS u ON u.id = da.user_id
+				LEFT JOIN dashboard folder on folder.id = d.folder_id
-				LEFT OUTER JOIN team ug on ug.id = da.team_id
+				LEFT JOIN dashboard_acl AS da ON
-				LEFT OUTER JOIN dashboard d on da.dashboard_id = d.id
+				da.dashboard_id = d.id OR
-			WHERE dashboard_id ` + dashboardFilter + ` AND da.org_id = ?
+				da.dashboard_id = d.folder_id OR
-
+				(
-			-- Also include default permissions if folder or dashboard field "has_acl" is false
+					-- include default permissions -->
-
+					da.org_id = -1 AND (
-			UNION
+					  (folder.id IS NOT NULL AND folder.has_acl = ` + falseStr + `) OR
-				SELECT
+					  (folder.id IS NULL AND d.has_acl = ` + falseStr + `)
-					da.id,
+					)
-					da.org_id,
+				)
-					da.dashboard_id,
+				LEFT JOIN ` + dialect.Quote("user") + ` AS u ON u.id = da.user_id
-					da.user_id,
+				LEFT JOIN team ug on ug.id = da.team_id
-					da.team_id,
+			WHERE d.org_id = ? AND d.id = ? AND da.id IS NOT NULL
-					da.permission,
-					da.role,
-					da.created,
-					da.updated,
-					'' as user_login,
-					'' as user_email,
-					'' as team,
-					folder.title,
-					folder.slug,
-					folder.uid,
-					folder.is_folder
-				FROM dashboard_acl as da,
-				dashboard as dash
-				LEFT OUTER JOIN dashboard folder on dash.folder_id = folder.id
-					WHERE
-						dash.id = ? AND (
-							dash.has_acl = ` + dialect.BooleanStr(false) + ` or
-							folder.has_acl = ` + dialect.BooleanStr(false) + `
-						) AND
-						da.dashboard_id = -1
 			ORDER BY 1 ASC
 			`
 

pkg/services/sqlstore/dashboard_acl_test.go

@@ -17,7 +17,7 @@ func TestDashboardAclDataAccess(t *testing.T) {
 			childDash := insertTestDashboard("2 test dash", 1, savedFolder.Id, false, "prod", "webapp")
 
 			Convey("When adding dashboard permission with userId and teamId set to 0", func() {
-				err := SetDashboardAcl(&m.SetDashboardAclCommand{
+				err := testHelperUpdateDashboardAcl(savedFolder.Id, m.DashboardAcl{
 					OrgId:       1,
 					DashboardId: savedFolder.Id,
 					Permission:  m.PERMISSION_EDIT,
@@ -41,8 +41,25 @@ func TestDashboardAclDataAccess(t *testing.T) {
 				})
 			})
 
+			Convey("Given dashboard folder with removed default permissions", func() {
+				err := UpdateDashboardAcl(&m.UpdateDashboardAclCommand{
+					DashboardId: savedFolder.Id,
+					Items:       []*m.DashboardAcl{},
+				})
+				So(err, ShouldBeNil)
+
+				Convey("When reading dashboard acl should return no acl items", func() {
+					query := m.GetDashboardAclInfoListQuery{DashboardId: childDash.Id, OrgId: 1}
+
+					err := GetDashboardAclInfoList(&query)
+					So(err, ShouldBeNil)
+
+					So(len(query.Result), ShouldEqual, 0)
+				})
+			})
+
 			Convey("Given dashboard folder permission", func() {
-				err := SetDashboardAcl(&m.SetDashboardAclCommand{
+				err := testHelperUpdateDashboardAcl(savedFolder.Id, m.DashboardAcl{
 					OrgId:       1,
 					UserId:      currentUser.Id,
 					DashboardId: savedFolder.Id,
@@ -61,7 +78,7 @@ func TestDashboardAclDataAccess(t *testing.T) {
 				})
 
 				Convey("Given child dashboard permission", func() {
-					err := SetDashboardAcl(&m.SetDashboardAclCommand{
+					err := testHelperUpdateDashboardAcl(childDash.Id, m.DashboardAcl{
 						OrgId:       1,
 						UserId:      currentUser.Id,
 						DashboardId: childDash.Id,
@@ -83,7 +100,7 @@ func TestDashboardAclDataAccess(t *testing.T) {
 			})
 
 			Convey("Given child dashboard permission in folder with no permissions", func() {
-				err := SetDashboardAcl(&m.SetDashboardAclCommand{
+				err := testHelperUpdateDashboardAcl(childDash.Id, m.DashboardAcl{
 					OrgId:       1,
 					UserId:      currentUser.Id,
 					DashboardId: childDash.Id,
@@ -108,17 +125,12 @@ func TestDashboardAclDataAccess(t *testing.T) {
 			})
 
 			Convey("Should be able to add dashboard permission", func() {
-				setDashAclCmd := m.SetDashboardAclCommand{
+				err := testHelperUpdateDashboardAcl(savedFolder.Id, m.DashboardAcl{
 					OrgId:       1,
 					UserId:      currentUser.Id,
 					DashboardId: savedFolder.Id,
 					Permission:  m.PERMISSION_EDIT,
-				}
+				})
-
-				err := SetDashboardAcl(&setDashAclCmd)
-				So(err, ShouldBeNil)
-
-				So(setDashAclCmd.Result.Id, ShouldEqual, 3)
 
 				q1 := &m.GetDashboardAclInfoListQuery{DashboardId: savedFolder.Id, OrgId: 1}
 				err = GetDashboardAclInfoList(q1)
@@ -130,42 +142,9 @@ func TestDashboardAclDataAccess(t *testing.T) {
 				So(q1.Result[0].UserId, ShouldEqual, currentUser.Id)
 				So(q1.Result[0].UserLogin, ShouldEqual, currentUser.Login)
 				So(q1.Result[0].UserEmail, ShouldEqual, currentUser.Email)
-				So(q1.Result[0].Id, ShouldEqual, setDashAclCmd.Result.Id)
-
-				Convey("Should update hasAcl field to true for dashboard folder and its children", func() {
-					q2 := &m.GetDashboardsQuery{DashboardIds: []int64{savedFolder.Id, childDash.Id}}
-					err := GetDashboards(q2)
-					So(err, ShouldBeNil)
-					So(q2.Result[0].HasAcl, ShouldBeTrue)
-					So(q2.Result[1].HasAcl, ShouldBeTrue)
-				})
-
-				Convey("Should be able to update an existing permission", func() {
-					err := SetDashboardAcl(&m.SetDashboardAclCommand{
-						OrgId:       1,
-						UserId:      1,
-						DashboardId: savedFolder.Id,
-						Permission:  m.PERMISSION_ADMIN,
-					})
-
-					So(err, ShouldBeNil)
-
-					q3 := &m.GetDashboardAclInfoListQuery{DashboardId: savedFolder.Id, OrgId: 1}
-					err = GetDashboardAclInfoList(q3)
-					So(err, ShouldBeNil)
-					So(len(q3.Result), ShouldEqual, 1)
-					So(q3.Result[0].DashboardId, ShouldEqual, savedFolder.Id)
-					So(q3.Result[0].Permission, ShouldEqual, m.PERMISSION_ADMIN)
-					So(q3.Result[0].UserId, ShouldEqual, 1)
-
-				})
 
 				Convey("Should be able to delete an existing permission", func() {
-					err := RemoveDashboardAcl(&m.RemoveDashboardAclCommand{
+					err := testHelperUpdateDashboardAcl(savedFolder.Id)
-						OrgId: 1,
-						AclId: setDashAclCmd.Result.Id,
-					})
-
 					So(err, ShouldBeNil)
 
 					q3 := &m.GetDashboardAclInfoListQuery{DashboardId: savedFolder.Id, OrgId: 1}
@@ -181,14 +160,12 @@ func TestDashboardAclDataAccess(t *testing.T) {
 				So(err, ShouldBeNil)
 
 				Convey("Should be able to add a user permission for a team", func() {
-					setDashAclCmd := m.SetDashboardAclCommand{
+					err := testHelperUpdateDashboardAcl(savedFolder.Id, m.DashboardAcl{
 						OrgId:       1,
 						TeamId:      group1.Result.Id,
 						DashboardId: savedFolder.Id,
 						Permission:  m.PERMISSION_EDIT,
-					}
+					})
-
-					err := SetDashboardAcl(&setDashAclCmd)
 					So(err, ShouldBeNil)
 
 					q1 := &m.GetDashboardAclInfoListQuery{DashboardId: savedFolder.Id, OrgId: 1}
@@ -197,23 +174,10 @@ func TestDashboardAclDataAccess(t *testing.T) {
 					So(q1.Result[0].DashboardId, ShouldEqual, savedFolder.Id)
 					So(q1.Result[0].Permission, ShouldEqual, m.PERMISSION_EDIT)
 					So(q1.Result[0].TeamId, ShouldEqual, group1.Result.Id)
-
-					Convey("Should be able to delete an existing permission for a team", func() {
-						err := RemoveDashboardAcl(&m.RemoveDashboardAclCommand{
-							OrgId: 1,
-							AclId: setDashAclCmd.Result.Id,
-						})
-
-						So(err, ShouldBeNil)
-						q3 := &m.GetDashboardAclInfoListQuery{DashboardId: savedFolder.Id, OrgId: 1}
-						err = GetDashboardAclInfoList(q3)
-						So(err, ShouldBeNil)
-						So(len(q3.Result), ShouldEqual, 0)
-					})
 				})
 
 				Convey("Should be able to update an existing permission for a team", func() {
-					err := SetDashboardAcl(&m.SetDashboardAclCommand{
+					err := testHelperUpdateDashboardAcl(savedFolder.Id, m.DashboardAcl{
 						OrgId:       1,
 						TeamId:      group1.Result.Id,
 						DashboardId: savedFolder.Id,
@@ -229,7 +193,6 @@ func TestDashboardAclDataAccess(t *testing.T) {
 					So(q3.Result[0].Permission, ShouldEqual, m.PERMISSION_ADMIN)
 					So(q3.Result[0].TeamId, ShouldEqual, group1.Result.Id)
 				})
-
 			})
 		})
 

pkg/services/sqlstore/dashboard_folder_test.go

@@ -41,7 +41,7 @@ func TestDashboardFolderDataAccess(t *testing.T) {
 
 			Convey("and acl is set for dashboard folder", func() {
 				var otherUser int64 = 999
-				updateTestDashboardWithAcl(folder.Id, otherUser, m.PERMISSION_EDIT)
+				testHelperUpdateDashboardAcl(folder.Id, m.DashboardAcl{DashboardId: folder.Id, OrgId: 1, UserId: otherUser, Permission: m.PERMISSION_EDIT})
 
 				Convey("should not return folder", func() {
 					query := &search.FindPersistedDashboardsQuery{
@@ -55,7 +55,7 @@ func TestDashboardFolderDataAccess(t *testing.T) {
 				})
 
 				Convey("when the user is given permission", func() {
-					updateTestDashboardWithAcl(folder.Id, currentUser.Id, m.PERMISSION_EDIT)
+					testHelperUpdateDashboardAcl(folder.Id, m.DashboardAcl{DashboardId: folder.Id, OrgId: 1, UserId: currentUser.Id, Permission: m.PERMISSION_EDIT})
 
 					Convey("should be able to access folder", func() {
 						query := &search.FindPersistedDashboardsQuery{
@@ -93,9 +93,8 @@ func TestDashboardFolderDataAccess(t *testing.T) {
 
 			Convey("and acl is set for dashboard child and folder has all permissions removed", func() {
 				var otherUser int64 = 999
-				aclId := updateTestDashboardWithAcl(folder.Id, otherUser, m.PERMISSION_EDIT)
+				testHelperUpdateDashboardAcl(folder.Id)
-				removeAcl(aclId)
+				testHelperUpdateDashboardAcl(childDash.Id, m.DashboardAcl{DashboardId: folder.Id, OrgId: 1, UserId: otherUser, Permission: m.PERMISSION_EDIT})
-				updateTestDashboardWithAcl(childDash.Id, otherUser, m.PERMISSION_EDIT)
 
 				Convey("should not return folder or child", func() {
 					query := &search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1, OrgRole: m.ROLE_VIEWER}, OrgId: 1, DashboardIds: []int64{folder.Id, childDash.Id, dashInRoot.Id}}
@@ -106,7 +105,7 @@ func TestDashboardFolderDataAccess(t *testing.T) {
 				})
 
 				Convey("when the user is given permission to child", func() {
-					updateTestDashboardWithAcl(childDash.Id, currentUser.Id, m.PERMISSION_EDIT)
+					testHelperUpdateDashboardAcl(childDash.Id, m.DashboardAcl{DashboardId: childDash.Id, OrgId: 1, UserId: currentUser.Id, Permission: m.PERMISSION_EDIT})
 
 					Convey("should be able to search for child dashboard but not folder", func() {
 						query := &search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1, OrgRole: m.ROLE_VIEWER}, OrgId: 1, DashboardIds: []int64{folder.Id, childDash.Id, dashInRoot.Id}}
@@ -165,11 +164,10 @@ func TestDashboardFolderDataAccess(t *testing.T) {
 
 			Convey("and acl is set for one dashboard folder", func() {
 				var otherUser int64 = 999
-				updateTestDashboardWithAcl(folder1.Id, otherUser, m.PERMISSION_EDIT)
+				testHelperUpdateDashboardAcl(folder1.Id, m.DashboardAcl{DashboardId: folder1.Id, OrgId: 1, UserId: otherUser, Permission: m.PERMISSION_EDIT})
 
 				Convey("and a dashboard is moved from folder without acl to the folder with an acl", func() {
-					movedDash := moveDashboard(1, childDash2.Data, folder1.Id)
+					moveDashboard(1, childDash2.Data, folder1.Id)
-					So(movedDash.HasAcl, ShouldBeTrue)
 
 					Convey("should not return folder with acl or its children", func() {
 						query := &search.FindPersistedDashboardsQuery{
@@ -184,9 +182,7 @@ func TestDashboardFolderDataAccess(t *testing.T) {
 					})
 				})
 				Convey("and a dashboard is moved from folder with acl to the folder without an acl", func() {
-
+					moveDashboard(1, childDash1.Data, folder2.Id)
-					movedDash := moveDashboard(1, childDash1.Data, folder2.Id)
-					So(movedDash.HasAcl, ShouldBeFalse)
 
 					Convey("should return folder without acl and its children", func() {
 						query := &search.FindPersistedDashboardsQuery{
@@ -205,9 +201,8 @@ func TestDashboardFolderDataAccess(t *testing.T) {
 				})
 
 				Convey("and a dashboard with an acl is moved to the folder without an acl", func() {
-					updateTestDashboardWithAcl(childDash1.Id, otherUser, m.PERMISSION_EDIT)
+					testHelperUpdateDashboardAcl(childDash1.Id, m.DashboardAcl{DashboardId: childDash1.Id, OrgId: 1, UserId: otherUser, Permission: m.PERMISSION_EDIT})
-					movedDash := moveDashboard(1, childDash1.Data, folder2.Id)
+					moveDashboard(1, childDash1.Data, folder2.Id)
-					So(movedDash.HasAcl, ShouldBeTrue)
 
 					Convey("should return folder without acl but not the dashboard with acl", func() {
 						query := &search.FindPersistedDashboardsQuery{
@@ -308,7 +303,7 @@ func TestDashboardFolderDataAccess(t *testing.T) {
 				})
 
 				Convey("Should have write access to one dashboard folder if default role changed to view for one folder", func() {
-					updateTestDashboardWithAcl(folder1.Id, editorUser.Id, m.PERMISSION_VIEW)
+					testHelperUpdateDashboardAcl(folder1.Id, m.DashboardAcl{DashboardId: folder1.Id, OrgId: 1, UserId: editorUser.Id, Permission: m.PERMISSION_VIEW})
 
 					err := SearchDashboards(&query)
 					So(err, ShouldBeNil)
@@ -352,7 +347,7 @@ func TestDashboardFolderDataAccess(t *testing.T) {
 				})
 
 				Convey("Should be able to get one dashboard folder if default role changed to edit for one folder", func() {
-					updateTestDashboardWithAcl(folder1.Id, viewerUser.Id, m.PERMISSION_EDIT)
+					testHelperUpdateDashboardAcl(folder1.Id, m.DashboardAcl{DashboardId: folder1.Id, OrgId: 1, UserId: viewerUser.Id, Permission: m.PERMISSION_EDIT})
 
 					err := SearchDashboards(&query)
 					So(err, ShouldBeNil)

pkg/services/sqlstore/dashboard_test.go

@@ -663,25 +663,6 @@ func createUser(name string, role string, isAdmin bool) m.User {
 	return currentUserCmd.Result
 }
 
-func updateTestDashboardWithAcl(dashId int64, userId int64, permissions m.PermissionType) int64 {
-	cmd := &m.SetDashboardAclCommand{
-		OrgId:       1,
-		UserId:      userId,
-		DashboardId: dashId,
-		Permission:  permissions,
-	}
-
-	err := SetDashboardAcl(cmd)
-	So(err, ShouldBeNil)
-
-	return cmd.Result.Id
-}
-
-func removeAcl(aclId int64) {
-	err := RemoveDashboardAcl(&m.RemoveDashboardAclCommand{AclId: aclId, OrgId: 1})
-	So(err, ShouldBeNil)
-}
-
 func moveDashboard(orgId int64, dashboard *simplejson.Json, newFolderId int64) *m.Dashboard {
 	cmd := m.SaveDashboardCommand{
 		OrgId:     orgId,

pkg/services/sqlstore/org_test.go

@@ -199,10 +199,13 @@ func TestAccountDataAccess(t *testing.T) {
 					So(err, ShouldBeNil)
 					So(len(query.Result), ShouldEqual, 3)
 
-					err = SetDashboardAcl(&m.SetDashboardAclCommand{DashboardId: 1, OrgId: ac1.OrgId, UserId: ac3.Id, Permission: m.PERMISSION_EDIT})
+					dash1 := insertTestDashboard("1 test dash", ac1.OrgId, 0, false, "prod", "webapp")
+					dash2 := insertTestDashboard("2 test dash", ac3.OrgId, 0, false, "prod", "webapp")
+
+					err = testHelperUpdateDashboardAcl(dash1.Id, m.DashboardAcl{DashboardId: dash1.Id, OrgId: ac1.OrgId, UserId: ac3.Id, Permission: m.PERMISSION_EDIT})
 					So(err, ShouldBeNil)
 
-					err = SetDashboardAcl(&m.SetDashboardAclCommand{DashboardId: 2, OrgId: ac3.OrgId, UserId: ac3.Id, Permission: m.PERMISSION_EDIT})
+					err = testHelperUpdateDashboardAcl(dash2.Id, m.DashboardAcl{DashboardId: dash2.Id, OrgId: ac3.OrgId, UserId: ac3.Id, Permission: m.PERMISSION_EDIT})
 					So(err, ShouldBeNil)
 
 					Convey("When org user is deleted", func() {
@@ -234,3 +237,11 @@ func TestAccountDataAccess(t *testing.T) {
 		})
 	})
 }
+
+func testHelperUpdateDashboardAcl(dashboardId int64, items ...m.DashboardAcl) error {
+	cmd := m.UpdateDashboardAclCommand{DashboardId: dashboardId}
+	for _, item := range items {
+		cmd.Items = append(cmd.Items, &item)
+	}
+	return UpdateDashboardAcl(&cmd)
+}

pkg/services/sqlstore/team_test.go

@@ -99,7 +99,7 @@ func TestTeamCommandsAndQueries(t *testing.T) {
 				So(err, ShouldBeNil)
 				err = AddTeamMember(&m.AddTeamMemberCommand{OrgId: testOrgId, TeamId: groupId, UserId: userIds[2]})
 				So(err, ShouldBeNil)
-				err = SetDashboardAcl(&m.SetDashboardAclCommand{DashboardId: 1, OrgId: testOrgId, Permission: m.PERMISSION_EDIT, TeamId: groupId})
+				err = testHelperUpdateDashboardAcl(1, m.DashboardAcl{DashboardId: 1, OrgId: testOrgId, Permission: m.PERMISSION_EDIT, TeamId: groupId})
 
 				err = DeleteTeam(&m.DeleteTeamCommand{OrgId: testOrgId, Id: groupId})
 				So(err, ShouldBeNil)

pkg/services/sqlstore/user_test.go

@@ -99,7 +99,7 @@ func TestUserDataAccess(t *testing.T) {
 				err = AddOrgUser(&m.AddOrgUserCommand{LoginOrEmail: users[0].Login, Role: m.ROLE_VIEWER, OrgId: users[0].OrgId})
 				So(err, ShouldBeNil)
 
-				err = SetDashboardAcl(&m.SetDashboardAclCommand{DashboardId: 1, OrgId: users[0].OrgId, UserId: users[0].Id, Permission: m.PERMISSION_EDIT})
+				testHelperUpdateDashboardAcl(1, m.DashboardAcl{DashboardId: 1, OrgId: users[0].OrgId, UserId: users[0].Id, Permission: m.PERMISSION_EDIT})
 				So(err, ShouldBeNil)
 
 				err = SavePreferences(&m.SavePreferencesCommand{UserId: users[0].Id, OrgId: users[0].OrgId, HomeDashboardId: 1, Theme: "dark"})

public/app/stores/PermissionsStore/PermissionsStoreItem.ts

@@ -1,9 +1,8 @@
-import { types } from 'mobx-state-tree';
+import { types } from 'mobx-state-tree';
 
 export const PermissionsStoreItem = types
   .model('PermissionsStoreItem', {
     dashboardId: types.optional(types.number, -1),
-    id: types.maybe(types.number),
     permission: types.number,
     permissionName: types.maybe(types.string),
     role: types.maybe(types.string),