dashboards: handle new guardian error responses and update tests

Using a mocked guardian instead and relies on the actual guardian tests for verifying
permission handling correctness

pkg/api/dashboard_permission.go

@@ -18,13 +18,13 @@ func GetDashboardPermissionList(c *middleware.Context) Response {
 		return rsp
 	}
 
-	guardian := guardian.New(dashId, c.OrgId, c.SignedInUser)
+	g := guardian.New(dashId, c.OrgId, c.SignedInUser)
 
-	if canAdmin, err := guardian.CanAdmin(); err != nil || !canAdmin {
+	if canAdmin, err := g.CanAdmin(); err != nil || !canAdmin {
 		return dashboardGuardianResponse(err)
 	}
 
-	acl, err := guardian.GetAcl()
+	acl, err := g.GetAcl()
 	if err != nil {
 		return ApiError(500, "Failed to get dashboard permissions", err)
 	}
@@ -46,8 +46,8 @@ func UpdateDashboardPermissions(c *middleware.Context, apiCmd dtos.UpdateDashboa
 		return rsp
 	}
 
-	guardian := guardian.New(dashId, c.OrgId, c.SignedInUser)
-	if canAdmin, err := guardian.CanAdmin(); err != nil || !canAdmin {
+	g := guardian.New(dashId, c.OrgId, c.SignedInUser)
+	if canAdmin, err := g.CanAdmin(); err != nil || !canAdmin {
 		return dashboardGuardianResponse(err)
 	}
 
@@ -67,8 +67,13 @@ func UpdateDashboardPermissions(c *middleware.Context, apiCmd dtos.UpdateDashboa
 		})
 	}
 
-	if okToUpdate, err := guardian.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, cmd.Items); err != nil || !okToUpdate {
+	if okToUpdate, err := g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, cmd.Items); err != nil || !okToUpdate {
 		if err != nil {
+			if err == guardian.ErrGuardianDuplicatePermission ||
+				err == guardian.ErrGuardianOverrideLowerPresedence {
+				return ApiError(400, err.Error(), err)
+			}
+
 			return ApiError(500, "Error while checking dashboard permissions", err)
 		}
 

pkg/api/dashboard_permission_test.go

@@ -8,183 +8,180 @@ import (
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/middleware"
 	m "github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/guardian"
 
 	. "github.com/smartystreets/goconvey/convey"
 )
 
 func TestDashboardPermissionApiEndpoint(t *testing.T) {
-	Convey("Given a dashboard with permissions", t, func() {
-		mockResult := []*m.DashboardAclInfoDTO{
-			{OrgId: 1, DashboardId: 1, UserId: 2, Permission: m.PERMISSION_VIEW},
-			{OrgId: 1, DashboardId: 1, UserId: 3, Permission: m.PERMISSION_EDIT},
-			{OrgId: 1, DashboardId: 1, UserId: 4, Permission: m.PERMISSION_ADMIN},
-			{OrgId: 1, DashboardId: 1, TeamId: 1, Permission: m.PERMISSION_VIEW},
-			{OrgId: 1, DashboardId: 1, TeamId: 2, Permission: m.PERMISSION_ADMIN},
-		}
-		dtoRes := transformDashboardAclsToDTOs(mockResult)
-
-		getDashboardQueryResult := m.NewDashboard("Dash")
-		var getDashboardNotFoundError error
-
-		bus.AddHandler("test", func(query *m.GetDashboardQuery) error {
-			query.Result = getDashboardQueryResult
-			return getDashboardNotFoundError
-		})
+	Convey("Dashboard permissions test", t, func() {
+		Convey("Given dashboard not exists", func() {
+			bus.AddHandler("test", func(query *m.GetDashboardQuery) error {
+				return m.ErrDashboardNotFound
+			})
 
-		bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error {
-			query.Result = dtoRes
-			return nil
-		})
+			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/1/permissions", "/api/dashboards/id/:id/permissions", m.ROLE_EDITOR, func(sc *scenarioContext) {
+				callGetDashboardPermissions(sc)
+				So(sc.resp.Code, ShouldEqual, 404)
+			})
 
-		bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error {
-			query.Result = mockResult
-			return nil
-		})
+			cmd := dtos.UpdateDashboardAclCommand{
+				Items: []dtos.DashboardAclUpdateItem{
+					{UserId: 1000, Permission: m.PERMISSION_ADMIN},
+				},
+			}
 
-		teamResp := []*m.Team{}
-		bus.AddHandler("test", func(query *m.GetTeamsByUserQuery) error {
-			query.Result = teamResp
-			return nil
+			updateDashboardPermissionScenario("When calling POST on", "/api/dashboards/id/1/permissions", "/api/dashboards/id/:id/permissions", cmd, func(sc *scenarioContext) {
+				callUpdateDashboardPermissions(sc)
+				So(sc.resp.Code, ShouldEqual, 404)
+			})
 		})
 
-		// This tests four scenarios:
-		// 1. user is an org admin
-		// 2. user is an org editor AND has been granted admin permission for the dashboard
-		// 3. user is an org viewer AND has been granted edit permission for the dashboard
-		// 4. user is an org editor AND has no permissions for the dashboard
+		Convey("Given user has no admin permissions", func() {
+			origNewGuardian := guardian.New
+			guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanAdminValue: false})
 
-		Convey("When user is org admin", func() {
-			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/1/permissions", "/api/dashboards/id/:dashboardsId/permissions", m.ROLE_ADMIN, func(sc *scenarioContext) {
-				Convey("Should be able to access ACL", func() {
-					sc.handlerFunc = GetDashboardPermissionList
-					sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
-
-					So(sc.resp.Code, ShouldEqual, 200)
+			getDashboardQueryResult := m.NewDashboard("Dash")
+			bus.AddHandler("test", func(query *m.GetDashboardQuery) error {
+				query.Result = getDashboardQueryResult
+				return nil
+			})
 
-					respJSON, err := simplejson.NewJson(sc.resp.Body.Bytes())
-					So(err, ShouldBeNil)
-					So(len(respJSON.MustArray()), ShouldEqual, 5)
-					So(respJSON.GetIndex(0).Get("userId").MustInt(), ShouldEqual, 2)
-					So(respJSON.GetIndex(0).Get("permission").MustInt(), ShouldEqual, m.PERMISSION_VIEW)
-				})
+			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/1/permissions", "/api/dashboards/id/:id/permissions", m.ROLE_EDITOR, func(sc *scenarioContext) {
+				callGetDashboardPermissions(sc)
+				So(sc.resp.Code, ShouldEqual, 403)
 			})
 
-			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/2/permissions", "/api/dashboards/id/:dashboardId/permissions", m.ROLE_ADMIN, func(sc *scenarioContext) {
-				getDashboardNotFoundError = m.ErrDashboardNotFound
-				sc.handlerFunc = GetDashboardPermissionList
-				sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
+			cmd := dtos.UpdateDashboardAclCommand{
+				Items: []dtos.DashboardAclUpdateItem{
+					{UserId: 1000, Permission: m.PERMISSION_ADMIN},
+				},
+			}
 
-				Convey("Should not be able to access ACL", func() {
-					So(sc.resp.Code, ShouldEqual, 404)
-				})
+			updateDashboardPermissionScenario("When calling POST on", "/api/dashboards/id/1/permissions", "/api/dashboards/id/:id/permissions", cmd, func(sc *scenarioContext) {
+				callUpdateDashboardPermissions(sc)
+				So(sc.resp.Code, ShouldEqual, 403)
 			})
 
-			Convey("Should not be able to update permissions for non-existing dashboard", func() {
-				cmd := dtos.UpdateDashboardAclCommand{
-					Items: []dtos.DashboardAclUpdateItem{
-						{UserId: 1000, Permission: m.PERMISSION_ADMIN},
-					},
-				}
-
-				postAclScenario("When calling POST on", "/api/dashboards/id/1/permissions", "/api/dashboards/id/:dashboardId/permissions", m.ROLE_ADMIN, cmd, func(sc *scenarioContext) {
-					getDashboardNotFoundError = m.ErrDashboardNotFound
-					CallPostAcl(sc)
-					So(sc.resp.Code, ShouldEqual, 404)
-				})
+			Reset(func() {
+				guardian.New = origNewGuardian
 			})
 		})
 
-		Convey("When user is org editor and has admin permission in the ACL", func() {
-			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/1/permissions", "/api/dashboards/id/:dashboardId/permissions", m.ROLE_EDITOR, func(sc *scenarioContext) {
-				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 = GetDashboardPermissionList
-					sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
+		Convey("Given user has admin permissions and permissions to update", func() {
+			origNewGuardian := guardian.New
+			guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{
+				CanAdminValue:                    true,
+				CheckPermissionBeforeUpdateValue: true,
+				GetAclValue: []*m.DashboardAclInfoDTO{
+					{OrgId: 1, DashboardId: 1, UserId: 2, Permission: m.PERMISSION_VIEW},
+					{OrgId: 1, DashboardId: 1, UserId: 3, Permission: m.PERMISSION_EDIT},
+					{OrgId: 1, DashboardId: 1, UserId: 4, Permission: m.PERMISSION_ADMIN},
+					{OrgId: 1, DashboardId: 1, TeamId: 1, Permission: m.PERMISSION_VIEW},
+					{OrgId: 1, DashboardId: 1, TeamId: 2, Permission: m.PERMISSION_ADMIN},
+				},
+			})
 
-					So(sc.resp.Code, ShouldEqual, 200)
-				})
+			getDashboardQueryResult := m.NewDashboard("Dash")
+			bus.AddHandler("test", func(query *m.GetDashboardQuery) error {
+				query.Result = getDashboardQueryResult
+				return nil
 			})
 
-			Convey("Should not be able to downgrade their own Admin permission", func() {
-				cmd := dtos.UpdateDashboardAclCommand{
-					Items: []dtos.DashboardAclUpdateItem{
-						{UserId: TestUserID, Permission: m.PERMISSION_EDIT},
-					},
-				}
+			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/1/permissions", "/api/dashboards/id/:id/permissions", m.ROLE_ADMIN, func(sc *scenarioContext) {
+				callGetDashboardPermissions(sc)
+				So(sc.resp.Code, ShouldEqual, 200)
+				respJSON, err := simplejson.NewJson(sc.resp.Body.Bytes())
+				So(err, ShouldBeNil)
+				So(len(respJSON.MustArray()), ShouldEqual, 5)
+				So(respJSON.GetIndex(0).Get("userId").MustInt(), ShouldEqual, 2)
+				So(respJSON.GetIndex(0).Get("permission").MustInt(), ShouldEqual, m.PERMISSION_VIEW)
+			})
 
-				postAclScenario("When calling POST on", "/api/dashboards/id/1/permissions", "/api/dashboards/id/:dashboardId/permissions", m.ROLE_EDITOR, cmd, func(sc *scenarioContext) {
-					mockResult = append(mockResult, &m.DashboardAclInfoDTO{OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_ADMIN})
+			cmd := dtos.UpdateDashboardAclCommand{
+				Items: []dtos.DashboardAclUpdateItem{
+					{UserId: 1000, Permission: m.PERMISSION_ADMIN},
+				},
+			}
 
-					CallPostAcl(sc)
-					So(sc.resp.Code, ShouldEqual, 403)
-				})
+			updateDashboardPermissionScenario("When calling POST on", "/api/dashboards/id/1/permissions", "/api/dashboards/id/:id/permissions", cmd, func(sc *scenarioContext) {
+				callUpdateDashboardPermissions(sc)
+				So(sc.resp.Code, ShouldEqual, 200)
 			})
 
-			Convey("Should be able to update permissions", func() {
-				cmd := dtos.UpdateDashboardAclCommand{
-					Items: []dtos.DashboardAclUpdateItem{
-						{UserId: TestUserID, Permission: m.PERMISSION_ADMIN},
-						{UserId: 2, Permission: m.PERMISSION_EDIT},
-					},
-				}
-
-				postAclScenario("When calling POST on", "/api/dashboards/id/1/permissions", "/api/dashboards/id/:dashboardId/permissions", m.ROLE_EDITOR, cmd, func(sc *scenarioContext) {
-					mockResult = append(mockResult, &m.DashboardAclInfoDTO{OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_ADMIN})
+			Reset(func() {
+				guardian.New = origNewGuardian
+			})
+		})
 
-					CallPostAcl(sc)
-					So(sc.resp.Code, ShouldEqual, 200)
-				})
+		Convey("When trying to update permissions with duplicate permissions", func() {
+			origNewGuardian := guardian.New
+			guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{
+				CanAdminValue:                    true,
+				CheckPermissionBeforeUpdateValue: false,
+				CheckPermissionBeforeUpdateError: guardian.ErrGuardianDuplicatePermission,
 			})
 
-		})
+			getDashboardQueryResult := m.NewDashboard("Dash")
+			bus.AddHandler("test", func(query *m.GetDashboardQuery) error {
+				query.Result = getDashboardQueryResult
+				return nil
+			})
 
-		Convey("When user is org viewer and has edit permission in the ACL", func() {
-			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/1/permissions", "/api/dashboards/id/:dashboardId/permissions", m.ROLE_VIEWER, func(sc *scenarioContext) {
-				mockResult = append(mockResult, &m.DashboardAclInfoDTO{OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_EDIT})
+			cmd := dtos.UpdateDashboardAclCommand{
+				Items: []dtos.DashboardAclUpdateItem{
+					{UserId: 1000, Permission: m.PERMISSION_ADMIN},
+				},
+			}
 
-				// Getting the permissions is an Admin permission
-				Convey("Should not be able to get list of permissions from ACL", func() {
-					sc.handlerFunc = GetDashboardPermissionList
-					sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
+			updateDashboardPermissionScenario("When calling POST on", "/api/dashboards/id/1/permissions", "/api/dashboards/id/:id/permissions", cmd, func(sc *scenarioContext) {
+				callUpdateDashboardPermissions(sc)
+				So(sc.resp.Code, ShouldEqual, 400)
+			})
 
-					So(sc.resp.Code, ShouldEqual, 403)
-				})
+			Reset(func() {
+				guardian.New = origNewGuardian
 			})
 		})
 
-		Convey("When user is org editor and not in the ACL", func() {
-			loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/1/permissions", "/api/dashboards/id/:dashboardsId/permissions", m.ROLE_EDITOR, func(sc *scenarioContext) {
+		Convey("When trying to override inherited permissions with lower presedence", func() {
+			origNewGuardian := guardian.New
+			guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{
+				CanAdminValue:                    true,
+				CheckPermissionBeforeUpdateValue: false,
+				CheckPermissionBeforeUpdateError: guardian.ErrGuardianOverrideLowerPresedence},
+			)
+
+			getDashboardQueryResult := m.NewDashboard("Dash")
+			bus.AddHandler("test", func(query *m.GetDashboardQuery) error {
+				query.Result = getDashboardQueryResult
+				return nil
+			})
+
+			cmd := dtos.UpdateDashboardAclCommand{
+				Items: []dtos.DashboardAclUpdateItem{
+					{UserId: 1000, Permission: m.PERMISSION_ADMIN},
+				},
+			}
 
-				Convey("Should not be able to access ACL", func() {
-					sc.handlerFunc = GetDashboardPermissionList
-					sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
+			updateDashboardPermissionScenario("When calling POST on", "/api/dashboards/id/1/permissions", "/api/dashboards/id/:id/permissions", cmd, func(sc *scenarioContext) {
+				callUpdateDashboardPermissions(sc)
+				So(sc.resp.Code, ShouldEqual, 400)
+			})
 
-					So(sc.resp.Code, ShouldEqual, 403)
-				})
+			Reset(func() {
+				guardian.New = origNewGuardian
 			})
 		})
 	})
 }
 
-func transformDashboardAclsToDTOs(acls []*m.DashboardAclInfoDTO) []*m.DashboardAclInfoDTO {
-	dtos := make([]*m.DashboardAclInfoDTO, 0)
-
-	for _, acl := range acls {
-		dto := &m.DashboardAclInfoDTO{
-			OrgId:       acl.OrgId,
-			DashboardId: acl.DashboardId,
-			Permission:  acl.Permission,
-			UserId:      acl.UserId,
-			TeamId:      acl.TeamId,
-		}
-		dtos = append(dtos, dto)
-	}
-
-	return dtos
+func callGetDashboardPermissions(sc *scenarioContext) {
+	sc.handlerFunc = GetDashboardPermissionList
+	sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
 }
 
-func CallPostAcl(sc *scenarioContext) {
+func callUpdateDashboardPermissions(sc *scenarioContext) {
 	bus.AddHandler("test", func(cmd *m.UpdateDashboardAclCommand) error {
 		return nil
 	})
@@ -192,7 +189,7 @@ func CallPostAcl(sc *scenarioContext) {
 	sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec()
 }
 
-func postAclScenario(desc string, url string, routePattern string, role m.RoleType, cmd dtos.UpdateDashboardAclCommand, fn scenarioFunc) {
+func updateDashboardPermissionScenario(desc string, url string, routePattern string, cmd dtos.UpdateDashboardAclCommand, fn scenarioFunc) {
 	Convey(desc+" "+url, func() {
 		defer bus.ClearBusHandlers()
 
@@ -200,9 +197,8 @@ func postAclScenario(desc string, url string, routePattern string, role m.RoleTy
 
 		sc.defaultHandler = wrap(func(c *middleware.Context) Response {
 			sc.context = c
-			sc.context.UserId = TestUserID
 			sc.context.OrgId = TestOrgID
-			sc.context.OrgRole = role
+			sc.context.UserId = TestUserID
 
 			return UpdateDashboardPermissions(c, cmd)
 		})