Torkel Ödegaard 8 лет назад
Родитель
Сommit
aa634402d9
4 измененных файлов с 115 добавлено и 79 удалено
  1. 1 0
      pkg/api/dashboard.go
  2. 33 33
      pkg/api/dashboard_acl_test.go
  3. 51 39
      pkg/api/dashboard_test.go
  4. 30 7
      pkg/services/guardian/guardian.go

+ 1 - 0
pkg/api/dashboard.go

@@ -51,6 +51,7 @@ func GetDashboard(c *middleware.Context) Response {
 
 
 	guardian := guardian.NewDashboardGuardian(dash.Id, c.OrgId, c.SignedInUser)
 	guardian := guardian.NewDashboardGuardian(dash.Id, c.OrgId, c.SignedInUser)
 	if canView, err := guardian.CanView(); err != nil || !canView {
 	if canView, err := guardian.CanView(); err != nil || !canView {
+		fmt.Printf("%v", err)
 		return dashboardGuardianResponse(err)
 		return dashboardGuardianResponse(err)
 	}
 	}
 
 

+ 33 - 33
pkg/api/dashboard_acl_test.go

@@ -5,40 +5,40 @@ import (
 
 
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/components/simplejson"
-	"github.com/grafana/grafana/pkg/models"
+	m "github.com/grafana/grafana/pkg/models"
 
 
 	. "github.com/smartystreets/goconvey/convey"
 	. "github.com/smartystreets/goconvey/convey"
 )
 )
 
 
 func TestDashboardAclApiEndpoint(t *testing.T) {
 func TestDashboardAclApiEndpoint(t *testing.T) {
 	Convey("Given a dashboard acl", t, func() {
 	Convey("Given a dashboard acl", t, func() {
-		mockResult := []*models.DashboardAcl{
-			{Id: 1, OrgId: 1, DashboardId: 1, UserId: 2, Permission: models.PERMISSION_VIEW},
-			{Id: 2, OrgId: 1, DashboardId: 1, UserId: 3, Permission: models.PERMISSION_EDIT},
-			{Id: 3, OrgId: 1, DashboardId: 1, UserId: 4, Permission: models.PERMISSION_ADMIN},
-			{Id: 4, OrgId: 1, DashboardId: 1, UserGroupId: 1, Permission: models.PERMISSION_VIEW},
-			{Id: 5, OrgId: 1, DashboardId: 1, UserGroupId: 2, Permission: models.PERMISSION_ADMIN},
+		mockResult := []*m.DashboardAclInfoDTO{
+			{Id: 1, OrgId: 1, DashboardId: 1, UserId: 2, Permission: m.PERMISSION_VIEW},
+			{Id: 2, OrgId: 1, DashboardId: 1, UserId: 3, Permission: m.PERMISSION_EDIT},
+			{Id: 3, OrgId: 1, DashboardId: 1, UserId: 4, Permission: m.PERMISSION_ADMIN},
+			{Id: 4, OrgId: 1, DashboardId: 1, UserGroupId: 1, Permission: m.PERMISSION_VIEW},
+			{Id: 5, OrgId: 1, DashboardId: 1, UserGroupId: 2, Permission: m.PERMISSION_ADMIN},
 		}
 		}
 		dtoRes := transformDashboardAclsToDTOs(mockResult)
 		dtoRes := transformDashboardAclsToDTOs(mockResult)
 
 
-		bus.AddHandler("test", func(query *models.GetDashboardAclInfoListQuery) error {
+		bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error {
 			query.Result = dtoRes
 			query.Result = dtoRes
 			return nil
 			return nil
 		})
 		})
 
 
-		bus.AddHandler("test", func(query *models.GetDashboardAclInfoListQuery) error {
+		bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error {
 			query.Result = mockResult
 			query.Result = mockResult
 			return nil
 			return nil
 		})
 		})
 
 
-		userGroupResp := []*models.UserGroup{}
-		bus.AddHandler("test", func(query *models.GetUserGroupsByUserQuery) error {
+		userGroupResp := []*m.UserGroup{}
+		bus.AddHandler("test", func(query *m.GetUserGroupsByUserQuery) error {
 			query.Result = userGroupResp
 			query.Result = userGroupResp
 			return nil
 			return nil
 		})
 		})
 
 
 		Convey("When user is org admin", func() {
 		Convey("When user is org admin", func() {
-			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/1/acl", "/api/dashboards/id/:dashboardsId/acl", models.ROLE_ADMIN, func(sc *scenarioContext) {
+			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/1/acl", "/api/dashboards/id/:dashboardsId/acl", m.ROLE_ADMIN, func(sc *scenarioContext) {
 				Convey("Should be able to access ACL", func() {
 				Convey("Should be able to access ACL", func() {
 					sc.handlerFunc = GetDashboardAclList
 					sc.handlerFunc = GetDashboardAclList
 					sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
 					sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
@@ -49,14 +49,14 @@ func TestDashboardAclApiEndpoint(t *testing.T) {
 					So(err, ShouldBeNil)
 					So(err, ShouldBeNil)
 					So(len(respJSON.MustArray()), ShouldEqual, 5)
 					So(len(respJSON.MustArray()), ShouldEqual, 5)
 					So(respJSON.GetIndex(0).Get("userId").MustInt(), ShouldEqual, 2)
 					So(respJSON.GetIndex(0).Get("userId").MustInt(), ShouldEqual, 2)
-					So(respJSON.GetIndex(0).Get("permission").MustInt(), ShouldEqual, models.PERMISSION_VIEW)
+					So(respJSON.GetIndex(0).Get("permission").MustInt(), ShouldEqual, m.PERMISSION_VIEW)
 				})
 				})
 			})
 			})
 		})
 		})
 
 
 		Convey("When user is editor and has admin permission in the ACL", func() {
 		Convey("When user is 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", models.ROLE_EDITOR, func(sc *scenarioContext) {
-				mockResult = append(mockResult, &models.DashboardAcl{Id: 1, OrgId: 1, DashboardId: 1, UserId: 1, Permission: models.PERMISSION_ADMIN})
+			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: 1, OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_ADMIN})
 
 
 				Convey("Should be able to access ACL", func() {
 				Convey("Should be able to access ACL", func() {
 					sc.handlerFunc = GetDashboardAclList
 					sc.handlerFunc = GetDashboardAclList
@@ -66,10 +66,10 @@ func TestDashboardAclApiEndpoint(t *testing.T) {
 				})
 				})
 			})
 			})
 
 
-			loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/id/1/acl/1", "/api/dashboards/id/:dashboardId/acl/:aclId", models.ROLE_EDITOR, func(sc *scenarioContext) {
-				mockResult = append(mockResult, &models.DashboardAcl{Id: 1, OrgId: 1, DashboardId: 1, UserId: 1, Permission: models.PERMISSION_ADMIN})
+			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: 1, OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_ADMIN})
 
 
-				bus.AddHandler("test3", func(cmd *models.RemoveDashboardAclCommand) error {
+				bus.AddHandler("test3", func(cmd *m.RemoveDashboardAclCommand) error {
 					return nil
 					return nil
 				})
 				})
 
 
@@ -82,10 +82,10 @@ func TestDashboardAclApiEndpoint(t *testing.T) {
 			})
 			})
 
 
 			Convey("When user is a member of a user group in the ACL with admin permission", func() {
 			Convey("When user is a member of a user group 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", models.ROLE_EDITOR, func(sc *scenarioContext) {
-					userGroupResp = append(userGroupResp, &models.UserGroup{Id: 2, OrgId: 1, Name: "UG2"})
+				loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/id/1/acl/1", "/api/dashboards/id/:dashboardsId/acl/:aclId", m.ROLE_EDITOR, func(sc *scenarioContext) {
+					userGroupResp = append(userGroupResp, &m.UserGroup{Id: 2, OrgId: 1, Name: "UG2"})
 
 
-					bus.AddHandler("test3", func(cmd *models.RemoveDashboardAclCommand) error {
+					bus.AddHandler("test3", func(cmd *m.RemoveDashboardAclCommand) error {
 						return nil
 						return nil
 					})
 					})
 
 
@@ -100,8 +100,8 @@ func TestDashboardAclApiEndpoint(t *testing.T) {
 		})
 		})
 
 
 		Convey("When user is editor and has edit permission in the ACL", func() {
 		Convey("When user is editor and has edit permission in the ACL", func() {
-			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/1/acl", "/api/dashboards/id/:dashboardId/acl", models.ROLE_EDITOR, func(sc *scenarioContext) {
-				mockResult = append(mockResult, &models.DashboardAcl{Id: 1, OrgId: 1, DashboardId: 1, UserId: 1, Permission: models.PERMISSION_EDIT})
+			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: 1, OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_EDIT})
 
 
 				Convey("Should not be able to access ACL", func() {
 				Convey("Should not be able to access ACL", func() {
 					sc.handlerFunc = GetDashboardAclList
 					sc.handlerFunc = GetDashboardAclList
@@ -111,10 +111,10 @@ func TestDashboardAclApiEndpoint(t *testing.T) {
 				})
 				})
 			})
 			})
 
 
-			loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/id/1/acl/1", "/api/dashboards/id/:dashboardId/acl/:aclId", models.ROLE_EDITOR, func(sc *scenarioContext) {
-				mockResult = append(mockResult, &models.DashboardAcl{Id: 1, OrgId: 1, DashboardId: 1, UserId: 1, Permission: models.PERMISSION_EDIT})
+			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: 1, OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_EDIT})
 
 
-				bus.AddHandler("test3", func(cmd *models.RemoveDashboardAclCommand) error {
+				bus.AddHandler("test3", func(cmd *m.RemoveDashboardAclCommand) error {
 					return nil
 					return nil
 				})
 				})
 
 
@@ -128,7 +128,7 @@ func TestDashboardAclApiEndpoint(t *testing.T) {
 		})
 		})
 
 
 		Convey("When user is editor and not in the ACL", func() {
 		Convey("When user is editor and not in the ACL", func() {
-			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/1/acl", "/api/dashboards/id/:dashboardsId/acl", models.ROLE_EDITOR, func(sc *scenarioContext) {
+			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/1/acl", "/api/dashboards/id/:dashboardsId/acl", m.ROLE_EDITOR, func(sc *scenarioContext) {
 
 
 				Convey("Should not be able to access ACL", func() {
 				Convey("Should not be able to access ACL", func() {
 					sc.handlerFunc = GetDashboardAclList
 					sc.handlerFunc = GetDashboardAclList
@@ -138,9 +138,9 @@ func TestDashboardAclApiEndpoint(t *testing.T) {
 				})
 				})
 			})
 			})
 
 
-			loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/id/1/acl/user/1", "/api/dashboards/id/:dashboardsId/acl/user/:userId", models.ROLE_EDITOR, func(sc *scenarioContext) {
-				mockResult = append(mockResult, &models.DashboardAcl{Id: 1, OrgId: 1, DashboardId: 1, UserId: 1, Permission: models.PERMISSION_VIEW})
-				bus.AddHandler("test3", func(cmd *models.RemoveDashboardAclCommand) error {
+			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
 					return nil
 				})
 				})
 
 
@@ -155,11 +155,11 @@ func TestDashboardAclApiEndpoint(t *testing.T) {
 	})
 	})
 }
 }
 
 
-func transformDashboardAclsToDTOs(acls []*models.DashboardAcl) []*models.DashboardAclInfoDTO {
-	dtos := make([]*models.DashboardAclInfoDTO, 0)
+func transformDashboardAclsToDTOs(acls []*m.DashboardAclInfoDTO) []*m.DashboardAclInfoDTO {
+	dtos := make([]*m.DashboardAclInfoDTO, 0)
 
 
 	for _, acl := range acls {
 	for _, acl := range acls {
-		dto := &models.DashboardAclInfoDTO{
+		dto := &m.DashboardAclInfoDTO{
 			Id:          acl.Id,
 			Id:          acl.Id,
 			OrgId:       acl.OrgId,
 			OrgId:       acl.OrgId,
 			DashboardId: acl.DashboardId,
 			DashboardId: acl.DashboardId,

+ 51 - 39
pkg/api/dashboard_test.go

@@ -12,7 +12,7 @@ import (
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/middleware"
 	"github.com/grafana/grafana/pkg/middleware"
-	"github.com/grafana/grafana/pkg/models"
+	m "github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/alerting"
 	"github.com/grafana/grafana/pkg/services/alerting"
 
 
 	. "github.com/smartystreets/goconvey/convey"
 	. "github.com/smartystreets/goconvey/convey"
@@ -20,23 +20,35 @@ import (
 
 
 func TestDashboardApiEndpoint(t *testing.T) {
 func TestDashboardApiEndpoint(t *testing.T) {
 	Convey("Given a dashboard with a parent folder which does not have an acl", t, func() {
 	Convey("Given a dashboard with a parent folder which does not have an acl", t, func() {
-		fakeDash := models.NewDashboard("Child dash")
+		fakeDash := m.NewDashboard("Child dash")
 		fakeDash.Id = 1
 		fakeDash.Id = 1
 		fakeDash.ParentId = 1
 		fakeDash.ParentId = 1
 		fakeDash.HasAcl = false
 		fakeDash.HasAcl = false
 
 
-		bus.AddHandler("test", func(query *models.GetDashboardQuery) error {
+		bus.AddHandler("test", func(query *m.GetDashboardQuery) error {
 			query.Result = fakeDash
 			query.Result = fakeDash
 			return nil
 			return nil
 		})
 		})
 
 
-		aclMockResp := []*models.DashboardAcl{}
-		bus.AddHandler("test", func(query *models.GetDashboardAclInfoListQuery) error {
+		viewerRole := m.ROLE_VIEWER
+		editorRole := m.ROLE_EDITOR
+
+		aclMockResp := []*m.DashboardAclInfoDTO{
+			{Role: &viewerRole, Permission: m.PERMISSION_VIEW},
+			{Role: &editorRole, Permission: m.PERMISSION_EDIT},
+		}
+
+		bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error {
 			query.Result = aclMockResp
 			query.Result = aclMockResp
 			return nil
 			return nil
 		})
 		})
 
 
-		cmd := models.SaveDashboardCommand{
+		bus.AddHandler("test", func(query *m.GetUserGroupsByUserQuery) error {
+			query.Result = []*m.UserGroup{}
+			return nil
+		})
+
+		cmd := m.SaveDashboardCommand{
 			Dashboard: simplejson.NewFromAny(map[string]interface{}{
 			Dashboard: simplejson.NewFromAny(map[string]interface{}{
 				"parentId": fakeDash.ParentId,
 				"parentId": fakeDash.ParentId,
 				"title":    fakeDash.Title,
 				"title":    fakeDash.Title,
@@ -45,7 +57,7 @@ func TestDashboardApiEndpoint(t *testing.T) {
 		}
 		}
 
 
 		Convey("When user is an Org Viewer", func() {
 		Convey("When user is an Org Viewer", func() {
-			role := models.ROLE_VIEWER
+			role := m.ROLE_VIEWER
 
 
 			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) {
 			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) {
 				dash := GetDashboardShouldReturn200(sc)
 				dash := GetDashboardShouldReturn200(sc)
@@ -78,12 +90,12 @@ func TestDashboardApiEndpoint(t *testing.T) {
 		})
 		})
 
 
 		Convey("When user is an Org Read Only Editor", func() {
 		Convey("When user is an Org Read Only Editor", func() {
-			role := models.ROLE_READ_ONLY_EDITOR
+			role := m.ROLE_READ_ONLY_EDITOR
 			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) {
 			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) {
 				dash := GetDashboardShouldReturn200(sc)
 				dash := GetDashboardShouldReturn200(sc)
 
 
-				Convey("Should be able to edit but not save the dashboard", func() {
-					So(dash.Meta.CanEdit, ShouldBeTrue)
+				Convey("Should be able to view but not save the dashboard", func() {
+					So(dash.Meta.CanEdit, ShouldBeFalse)
 					So(dash.Meta.CanSave, ShouldBeFalse)
 					So(dash.Meta.CanSave, ShouldBeFalse)
 				})
 				})
 			})
 			})
@@ -110,7 +122,7 @@ func TestDashboardApiEndpoint(t *testing.T) {
 		})
 		})
 
 
 		Convey("When user is an Org Editor", func() {
 		Convey("When user is an Org Editor", func() {
-			role := models.ROLE_EDITOR
+			role := m.ROLE_EDITOR
 
 
 			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) {
 			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) {
 				dash := GetDashboardShouldReturn200(sc)
 				dash := GetDashboardShouldReturn200(sc)
@@ -142,12 +154,12 @@ func TestDashboardApiEndpoint(t *testing.T) {
 			})
 			})
 
 
 			Convey("When saving a dashboard folder in another folder", func() {
 			Convey("When saving a dashboard folder in another folder", func() {
-				bus.AddHandler("test", func(query *models.GetDashboardQuery) error {
+				bus.AddHandler("test", func(query *m.GetDashboardQuery) error {
 					query.Result = fakeDash
 					query.Result = fakeDash
 					query.Result.IsFolder = true
 					query.Result.IsFolder = true
 					return nil
 					return nil
 				})
 				})
-				invalidCmd := models.SaveDashboardCommand{
+				invalidCmd := m.SaveDashboardCommand{
 					ParentId: fakeDash.ParentId,
 					ParentId: fakeDash.ParentId,
 					IsFolder: true,
 					IsFolder: true,
 					Dashboard: simplejson.NewFromAny(map[string]interface{}{
 					Dashboard: simplejson.NewFromAny(map[string]interface{}{
@@ -166,35 +178,35 @@ func TestDashboardApiEndpoint(t *testing.T) {
 	})
 	})
 
 
 	Convey("Given a dashboard with a parent folder which has an acl", t, func() {
 	Convey("Given a dashboard with a parent folder which has an acl", t, func() {
-		fakeDash := models.NewDashboard("Child dash")
+		fakeDash := m.NewDashboard("Child dash")
 		fakeDash.Id = 1
 		fakeDash.Id = 1
 		fakeDash.ParentId = 1
 		fakeDash.ParentId = 1
 		fakeDash.HasAcl = true
 		fakeDash.HasAcl = true
 
 
-		aclMockResp := []*models.DashboardAcl{
+		aclMockResp := []*m.DashboardAclInfoDTO{
 			{
 			{
 				DashboardId: 1,
 				DashboardId: 1,
-				Permission:  models.PERMISSION_EDIT,
+				Permission:  m.PERMISSION_EDIT,
 				UserId:      200,
 				UserId:      200,
 			},
 			},
 		}
 		}
 
 
-		bus.AddHandler("test", func(query *models.GetDashboardAclInfoListQuery) error {
+		bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error {
 			query.Result = aclMockResp
 			query.Result = aclMockResp
 			return nil
 			return nil
 		})
 		})
 
 
-		bus.AddHandler("test", func(query *models.GetDashboardQuery) error {
+		bus.AddHandler("test", func(query *m.GetDashboardQuery) error {
 			query.Result = fakeDash
 			query.Result = fakeDash
 			return nil
 			return nil
 		})
 		})
 
 
-		bus.AddHandler("test", func(query *models.GetUserGroupsByUserQuery) error {
-			query.Result = []*models.UserGroup{}
+		bus.AddHandler("test", func(query *m.GetUserGroupsByUserQuery) error {
+			query.Result = []*m.UserGroup{}
 			return nil
 			return nil
 		})
 		})
 
 
-		cmd := models.SaveDashboardCommand{
+		cmd := m.SaveDashboardCommand{
 			ParentId: fakeDash.ParentId,
 			ParentId: fakeDash.ParentId,
 			Dashboard: simplejson.NewFromAny(map[string]interface{}{
 			Dashboard: simplejson.NewFromAny(map[string]interface{}{
 				"id":       fakeDash.Id,
 				"id":       fakeDash.Id,
@@ -204,7 +216,7 @@ func TestDashboardApiEndpoint(t *testing.T) {
 		}
 		}
 
 
 		Convey("When user is an Org Viewer and has no permissions for this dashboard", func() {
 		Convey("When user is an Org Viewer and has no permissions for this dashboard", func() {
-			role := models.ROLE_VIEWER
+			role := m.ROLE_VIEWER
 
 
 			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) {
 			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) {
 				sc.handlerFunc = GetDashboard
 				sc.handlerFunc = GetDashboard
@@ -237,7 +249,7 @@ func TestDashboardApiEndpoint(t *testing.T) {
 		})
 		})
 
 
 		Convey("When user is an Org Editor and has no permissions for this dashboard", func() {
 		Convey("When user is an Org Editor and has no permissions for this dashboard", func() {
-			role := models.ROLE_EDITOR
+			role := m.ROLE_EDITOR
 
 
 			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) {
 			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) {
 				sc.handlerFunc = GetDashboard
 				sc.handlerFunc = GetDashboard
@@ -270,13 +282,13 @@ func TestDashboardApiEndpoint(t *testing.T) {
 		})
 		})
 
 
 		Convey("When user is an Org Viewer but has an edit permission", func() {
 		Convey("When user is an Org Viewer but has an edit permission", func() {
-			role := models.ROLE_VIEWER
+			role := m.ROLE_VIEWER
 
 
-			mockResult := []*models.DashboardAcl{
-				{Id: 1, OrgId: 1, DashboardId: 2, UserId: 1, Permission: models.PERMISSION_EDIT},
+			mockResult := []*m.DashboardAclInfoDTO{
+				{Id: 1, OrgId: 1, DashboardId: 2, UserId: 1, Permission: m.PERMISSION_EDIT},
 			}
 			}
 
 
-			bus.AddHandler("test", func(query *models.GetDashboardAclInfoListQuery) error {
+			bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error {
 				query.Result = mockResult
 				query.Result = mockResult
 				return nil
 				return nil
 			})
 			})
@@ -312,13 +324,13 @@ func TestDashboardApiEndpoint(t *testing.T) {
 		})
 		})
 
 
 		Convey("When user is an Org Editor but has a view permission", func() {
 		Convey("When user is an Org Editor but has a view permission", func() {
-			role := models.ROLE_EDITOR
+			role := m.ROLE_EDITOR
 
 
-			mockResult := []*models.DashboardAcl{
-				{Id: 1, OrgId: 1, DashboardId: 2, UserId: 1, Permission: models.PERMISSION_VIEW},
+			mockResult := []*m.DashboardAclInfoDTO{
+				{Id: 1, OrgId: 1, DashboardId: 2, UserId: 1, Permission: m.PERMISSION_VIEW},
 			}
 			}
 
 
-			bus.AddHandler("test", func(query *models.GetDashboardAclInfoListQuery) error {
+			bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error {
 				query.Result = mockResult
 				query.Result = mockResult
 				return nil
 				return nil
 			})
 			})
@@ -369,8 +381,8 @@ func GetDashboardShouldReturn200(sc *scenarioContext) dtos.DashboardFullWithMeta
 }
 }
 
 
 func CallGetDashboardVersion(sc *scenarioContext) {
 func CallGetDashboardVersion(sc *scenarioContext) {
-	bus.AddHandler("test", func(query *models.GetDashboardVersionQuery) error {
-		query.Result = &models.DashboardVersion{}
+	bus.AddHandler("test", func(query *m.GetDashboardVersionQuery) error {
+		query.Result = &m.DashboardVersion{}
 		return nil
 		return nil
 	})
 	})
 
 
@@ -379,8 +391,8 @@ func CallGetDashboardVersion(sc *scenarioContext) {
 }
 }
 
 
 func CallGetDashboardVersions(sc *scenarioContext) {
 func CallGetDashboardVersions(sc *scenarioContext) {
-	bus.AddHandler("test", func(query *models.GetDashboardVersionsQuery) error {
-		query.Result = []*models.DashboardVersionDTO{}
+	bus.AddHandler("test", func(query *m.GetDashboardVersionsQuery) error {
+		query.Result = []*m.DashboardVersionDTO{}
 		return nil
 		return nil
 	})
 	})
 
 
@@ -389,7 +401,7 @@ func CallGetDashboardVersions(sc *scenarioContext) {
 }
 }
 
 
 func CallDeleteDashboard(sc *scenarioContext) {
 func CallDeleteDashboard(sc *scenarioContext) {
-	bus.AddHandler("test", func(cmd *models.DeleteDashboardCommand) error {
+	bus.AddHandler("test", func(cmd *m.DeleteDashboardCommand) error {
 		return nil
 		return nil
 	})
 	})
 
 
@@ -402,8 +414,8 @@ func CallPostDashboard(sc *scenarioContext) {
 		return nil
 		return nil
 	})
 	})
 
 
-	bus.AddHandler("test", func(cmd *models.SaveDashboardCommand) error {
-		cmd.Result = &models.Dashboard{Id: 2, Slug: "Dash", Version: 2}
+	bus.AddHandler("test", func(cmd *m.SaveDashboardCommand) error {
+		cmd.Result = &m.Dashboard{Id: 2, Slug: "Dash", Version: 2}
 		return nil
 		return nil
 	})
 	})
 
 
@@ -414,7 +426,7 @@ func CallPostDashboard(sc *scenarioContext) {
 	sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec()
 	sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec()
 }
 }
 
 
-func postDashboardScenario(desc string, url string, routePattern string, role models.RoleType, cmd models.SaveDashboardCommand, fn scenarioFunc) {
+func postDashboardScenario(desc string, url string, routePattern string, role m.RoleType, cmd m.SaveDashboardCommand, fn scenarioFunc) {
 	Convey(desc+" "+url, func() {
 	Convey(desc+" "+url, func() {
 		defer bus.ClearBusHandlers()
 		defer bus.ClearBusHandlers()
 
 

+ 30 - 7
pkg/services/guardian/guardian.go

@@ -50,24 +50,47 @@ func (g *DashboardGuardian) HasPermission(permission m.PermissionType) (bool, er
 		return false, err
 		return false, err
 	}
 	}
 
 
-	userGroups, err := g.getUserGroups()
-	if err != nil {
-		return false, err
+	orgRole := g.user.OrgRole
+	if orgRole == m.ROLE_READ_ONLY_EDITOR {
+		orgRole = m.ROLE_VIEWER
 	}
 	}
 
 
+	userGroupAclItems := []*m.DashboardAclInfoDTO{}
+
 	for _, p := range acl {
 	for _, p := range acl {
+		// user match
 		if p.UserId == g.user.UserId && p.Permission >= permission {
 		if p.UserId == g.user.UserId && p.Permission >= permission {
 			return true, nil
 			return true, nil
 		}
 		}
 
 
-		for _, ug := range userGroups {
-			if ug.Id == p.UserGroupId && p.Permission >= permission {
+		// role match
+		if p.Role != nil {
+			if *p.Role == orgRole && p.Permission >= permission {
 				return true, nil
 				return true, nil
 			}
 			}
 		}
 		}
 
 
-		if p.Role != nil {
-			if *p.Role == g.user.OrgRole && p.Permission >= permission {
+		// remember this rule for later
+		if p.UserGroupId > 0 {
+			userGroupAclItems = append(userGroupAclItems, p)
+		}
+	}
+
+	// do we have group rules?
+	if len(userGroupAclItems) == 0 {
+		return false, nil
+	}
+
+	// load groups
+	userGroups, err := g.getUserGroups()
+	if err != nil {
+		return false, err
+	}
+
+	// evalute group rules
+	for _, p := range acl {
+		for _, ug := range userGroups {
+			if ug.Id == p.UserGroupId && p.Permission >= permission {
 				return true, nil
 				return true, nil
 			}
 			}
 		}
 		}