Shouldn't be able to overwrite a dashboard if you don't have permissions (#10900)

* dashboards: new command for validating dashboard before update

Removes validation logic from saveDashboard and later on use the new command for validating
dashboard before saving a dashboard. This due to the fact that we need to validate permissions
for overwriting other dashboards by uid and title.

* dashboards: use the new command for validating dashboard before saving

Had to refactor dashboard provisioning a bit to be able to sidetrack the permission validation
in a somewhat reasonable way.
Adds some initial tests of the dashboard repository, but needs to be extended later. At least
now you can mock the dashboard guardian

* dashboards: removes validation logic in the save dashboard api layer

Use the dashboard repository solely for create/update dashboards and let it do all
the validation. One exception regarding quota validation which still is in api layer
since that logic is in a macaron middleware.
Need to move out-commented api tests later.

* dashboards: fix database tests for validate and saving dashboards

* dashboards: rename dashboard repository to dashboard service

Split the old dashboard repository interface in two new interfaces, IDashboardService and
IDashboardProvisioningService. Makes it more explicit when using it from the provisioning package
and there's no possibility of calling an incorrect method for saving a dashboard.

* database: make the InitTestDB function available to use from other packages

* dashboards: rename ValidateDashboardForUpdateCommand and some refactoring

* dashboards: integration tests of dashboard service

* dashboard: fix sqlstore test due to folder exist validation

* dashboards: move dashboard service integration tests to sqlstore package

Had to move it to the sqlstore package due to concurrency problems when running
against mysql and postgres. Using InitTestDB from two packages added conflicts
when clearing and running migrations on the test database

* dashboards: refactor how to find id to be used for save permission check

* dashboards: remove duplicated dashboard tests

* dashboards: cleanup dashboard service integration tests

* dashboards: handle save dashboard errors and return correct http status

* fix: remove log statement

* dashboards: import dashboard should use dashboard service

Had to move alerting commands to models package due to problems with import cycles of packages.

* dashboards: cleanup dashboard api tests and add some tests for post dashboard

* dashboards: rename dashboard service interfaces

* dashboards: rename dashboard guardian interface

pkg/api/dashboard_test.go

@@ -2,6 +2,7 @@ package api
 
 import (
 	"encoding/json"
+	"fmt"
 	"testing"
 
 	"github.com/grafana/grafana/pkg/api/dtos"
@@ -9,38 +10,17 @@ 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/alerting"
 	"github.com/grafana/grafana/pkg/services/dashboards"
 	"github.com/grafana/grafana/pkg/setting"
 
 	. "github.com/smartystreets/goconvey/convey"
 )
 
-type fakeDashboardRepo struct {
-	inserted     []*dashboards.SaveDashboardDTO
-	provisioned  []*m.DashboardProvisioning
-	getDashboard []*m.Dashboard
-}
-
-func (repo *fakeDashboardRepo) SaveDashboard(json *dashboards.SaveDashboardDTO) (*m.Dashboard, error) {
-	repo.inserted = append(repo.inserted, json)
-	return json.Dashboard, nil
-}
-
-func (repo *fakeDashboardRepo) SaveProvisionedDashboard(dto *dashboards.SaveDashboardDTO, provisioning *m.DashboardProvisioning) (*m.Dashboard, error) {
-	repo.inserted = append(repo.inserted, dto)
-	return dto.Dashboard, nil
-}
-
-func (repo *fakeDashboardRepo) GetProvisionedDashboardData(name string) ([]*m.DashboardProvisioning, error) {
-	return repo.provisioned, nil
-}
-
-var fakeRepo *fakeDashboardRepo
-
-// This tests two main scenarios. If a user has access to execute an action on a dashboard:
-// 1. and the dashboard is in a folder which does not have an acl
-// 2. and the dashboard is in a folder which does have an acl
+// This tests three main scenarios.
+// If a user has access to execute an action on a dashboard:
+//   1. and the dashboard is in a folder which does not have an acl
+//   2. and the dashboard is in a folder which does have an acl
+// 3. Post dashboard response tests
 
 func TestDashboardApiEndpoint(t *testing.T) {
 	Convey("Given a dashboard with a parent folder which does not have an acl", t, func() {
@@ -81,14 +61,6 @@ func TestDashboardApiEndpoint(t *testing.T) {
 			return nil
 		})
 
-		cmd := m.SaveDashboardCommand{
-			Dashboard: simplejson.NewFromAny(map[string]interface{}{
-				"folderId": fakeDash.FolderId,
-				"title":    fakeDash.Title,
-				"id":       fakeDash.Id,
-			}),
-		}
-
 		// This tests two scenarios:
 		// 1. user is an org viewer
 		// 2. user is an org editor
@@ -151,11 +123,6 @@ func TestDashboardApiEndpoint(t *testing.T) {
 				CallGetDashboardVersions(sc)
 				So(sc.resp.Code, ShouldEqual, 403)
 			})
-
-			postDashboardScenario("When calling POST on", "/api/dashboards", "/api/dashboards", role, cmd, func(sc *scenarioContext) {
-				CallPostDashboard(sc)
-				So(sc.resp.Code, ShouldEqual, 403)
-			})
 		})
 
 		Convey("When user is an Org Editor", func() {
@@ -216,32 +183,6 @@ func TestDashboardApiEndpoint(t *testing.T) {
 				CallGetDashboardVersions(sc)
 				So(sc.resp.Code, ShouldEqual, 200)
 			})
-
-			postDashboardScenario("When calling POST on", "/api/dashboards", "/api/dashboards", role, cmd, func(sc *scenarioContext) {
-				CallPostDashboardShouldReturnSuccess(sc)
-			})
-
-			Convey("When saving a dashboard folder in another folder", func() {
-				bus.AddHandler("test", func(query *m.GetDashboardQuery) error {
-					query.Result = fakeDash
-					query.Result.IsFolder = true
-					return nil
-				})
-				invalidCmd := m.SaveDashboardCommand{
-					FolderId: fakeDash.FolderId,
-					IsFolder: true,
-					Dashboard: simplejson.NewFromAny(map[string]interface{}{
-						"folderId": fakeDash.FolderId,
-						"title":    fakeDash.Title,
-					}),
-				}
-				Convey("Should return an error", func() {
-					postDashboardScenario("When calling POST on", "/api/dashboards", "/api/dashboards", role, invalidCmd, func(sc *scenarioContext) {
-						CallPostDashboard(sc)
-						So(sc.resp.Code, ShouldEqual, 400)
-					})
-				})
-			})
 		})
 	})
 
@@ -284,15 +225,6 @@ func TestDashboardApiEndpoint(t *testing.T) {
 			return nil
 		})
 
-		cmd := m.SaveDashboardCommand{
-			FolderId: fakeDash.FolderId,
-			Dashboard: simplejson.NewFromAny(map[string]interface{}{
-				"id":       fakeDash.Id,
-				"folderId": fakeDash.FolderId,
-				"title":    fakeDash.Title,
-			}),
-		}
-
 		// This tests six scenarios:
 		// 1. user is an org viewer AND has no permissions for this dashboard
 		// 2. user is an org editor AND has no permissions for this dashboard
@@ -357,11 +289,6 @@ func TestDashboardApiEndpoint(t *testing.T) {
 				CallGetDashboardVersions(sc)
 				So(sc.resp.Code, ShouldEqual, 403)
 			})
-
-			postDashboardScenario("When calling POST on", "/api/dashboards", "/api/dashboards", role, cmd, func(sc *scenarioContext) {
-				CallPostDashboard(sc)
-				So(sc.resp.Code, ShouldEqual, 403)
-			})
 		})
 
 		Convey("When user is an Org Editor and has no permissions for this dashboard", func() {
@@ -420,11 +347,6 @@ func TestDashboardApiEndpoint(t *testing.T) {
 				CallGetDashboardVersions(sc)
 				So(sc.resp.Code, ShouldEqual, 403)
 			})
-
-			postDashboardScenario("When calling POST on", "/api/dashboards", "/api/dashboards", role, cmd, func(sc *scenarioContext) {
-				CallPostDashboard(sc)
-				So(sc.resp.Code, ShouldEqual, 403)
-			})
 		})
 
 		Convey("When user is an Org Viewer but has an edit permission", func() {
@@ -494,10 +416,6 @@ func TestDashboardApiEndpoint(t *testing.T) {
 				CallGetDashboardVersions(sc)
 				So(sc.resp.Code, ShouldEqual, 200)
 			})
-
-			postDashboardScenario("When calling POST on", "/api/dashboards", "/api/dashboards", role, cmd, func(sc *scenarioContext) {
-				CallPostDashboardShouldReturnSuccess(sc)
-			})
 		})
 
 		Convey("When user is an Org Viewer and viewers can edit", func() {
@@ -627,10 +545,6 @@ func TestDashboardApiEndpoint(t *testing.T) {
 				CallGetDashboardVersions(sc)
 				So(sc.resp.Code, ShouldEqual, 200)
 			})
-
-			postDashboardScenario("When calling POST on", "/api/dashboards", "/api/dashboards", role, cmd, func(sc *scenarioContext) {
-				CallPostDashboardShouldReturnSuccess(sc)
-			})
 		})
 
 		Convey("When user is an Org Editor but has a view permission", func() {
@@ -698,11 +612,6 @@ func TestDashboardApiEndpoint(t *testing.T) {
 				CallGetDashboardVersions(sc)
 				So(sc.resp.Code, ShouldEqual, 403)
 			})
-
-			postDashboardScenario("When calling POST on", "/api/dashboards", "/api/dashboards", role, cmd, func(sc *scenarioContext) {
-				CallPostDashboard(sc)
-				So(sc.resp.Code, ShouldEqual, 403)
-			})
 		})
 	})
 
@@ -736,6 +645,104 @@ func TestDashboardApiEndpoint(t *testing.T) {
 			})
 		})
 	})
+
+	Convey("Post dashboard response tests", t, func() {
+
+		// This tests that a valid request returns correct response
+
+		Convey("Given a correct request for creating a dashboard", func() {
+			cmd := m.SaveDashboardCommand{
+				OrgId:  1,
+				UserId: 5,
+				Dashboard: simplejson.NewFromAny(map[string]interface{}{
+					"title": "Dash",
+				}),
+				Overwrite: true,
+				FolderId:  3,
+				IsFolder:  false,
+				Message:   "msg",
+			}
+
+			mock := &dashboards.FakeDashboardService{
+				SaveDashboardResult: &m.Dashboard{
+					Id:      2,
+					Uid:     "uid",
+					Title:   "Dash",
+					Slug:    "dash",
+					Version: 2,
+				},
+			}
+
+			postDashboardScenario("When calling POST on", "/api/dashboards", "/api/dashboards", mock, cmd, func(sc *scenarioContext) {
+				CallPostDashboardShouldReturnSuccess(sc)
+
+				Convey("It should call dashboard service with correct data", func() {
+					dto := mock.SavedDashboards[0]
+					So(dto.OrgId, ShouldEqual, cmd.OrgId)
+					So(dto.User.UserId, ShouldEqual, cmd.UserId)
+					So(dto.Dashboard.FolderId, ShouldEqual, 3)
+					So(dto.Dashboard.Title, ShouldEqual, "Dash")
+					So(dto.Overwrite, ShouldBeTrue)
+					So(dto.Message, ShouldEqual, "msg")
+				})
+
+				Convey("It should return correct response data", func() {
+					result := sc.ToJson()
+					So(result.Get("status").MustString(), ShouldEqual, "success")
+					So(result.Get("id").MustInt64(), ShouldEqual, 2)
+					So(result.Get("uid").MustString(), ShouldEqual, "uid")
+					So(result.Get("slug").MustString(), ShouldEqual, "dash")
+					So(result.Get("url").MustString(), ShouldEqual, "/d/uid/dash")
+				})
+			})
+		})
+
+		// This tests that invalid requests returns expected error responses
+
+		Convey("Given incorrect requests for creating a dashboard", func() {
+			testCases := []struct {
+				SaveError          error
+				ExpectedStatusCode int
+			}{
+				{SaveError: m.ErrDashboardNotFound, ExpectedStatusCode: 404},
+				{SaveError: m.ErrFolderNotFound, ExpectedStatusCode: 400},
+				{SaveError: m.ErrDashboardWithSameUIDExists, ExpectedStatusCode: 400},
+				{SaveError: m.ErrDashboardWithSameNameInFolderExists, ExpectedStatusCode: 412},
+				{SaveError: m.ErrDashboardVersionMismatch, ExpectedStatusCode: 412},
+				{SaveError: m.ErrDashboardTitleEmpty, ExpectedStatusCode: 400},
+				{SaveError: m.ErrDashboardFolderCannotHaveParent, ExpectedStatusCode: 400},
+				{SaveError: m.ErrDashboardContainsInvalidAlertData, ExpectedStatusCode: 500},
+				{SaveError: m.ErrDashboardFailedToUpdateAlertData, ExpectedStatusCode: 500},
+				{SaveError: m.ErrDashboardFailedGenerateUniqueUid, ExpectedStatusCode: 500},
+				{SaveError: m.ErrDashboardTypeMismatch, ExpectedStatusCode: 400},
+				{SaveError: m.ErrDashboardFolderWithSameNameAsDashboard, ExpectedStatusCode: 400},
+				{SaveError: m.ErrDashboardWithSameNameAsFolder, ExpectedStatusCode: 400},
+				{SaveError: m.ErrDashboardFolderNameExists, ExpectedStatusCode: 400},
+				{SaveError: m.ErrDashboardUpdateAccessDenied, ExpectedStatusCode: 403},
+				{SaveError: m.ErrDashboardInvalidUid, ExpectedStatusCode: 400},
+				{SaveError: m.ErrDashboardUidToLong, ExpectedStatusCode: 400},
+				{SaveError: m.UpdatePluginDashboardError{PluginId: "plug"}, ExpectedStatusCode: 412},
+			}
+
+			cmd := m.SaveDashboardCommand{
+				OrgId: 1,
+				Dashboard: simplejson.NewFromAny(map[string]interface{}{
+					"title": "",
+				}),
+			}
+
+			for _, tc := range testCases {
+				mock := &dashboards.FakeDashboardService{
+					SaveDashboardError: tc.SaveError,
+				}
+
+				postDashboardScenario(fmt.Sprintf("Expect '%s' error when calling POST on", tc.SaveError.Error()), "/api/dashboards", "/api/dashboards", mock, cmd, func(sc *scenarioContext) {
+					CallPostDashboard(sc)
+					So(sc.resp.Code, ShouldEqual, tc.ExpectedStatusCode)
+				})
+			}
+		})
+	})
 }
 
 func GetDashboardShouldReturn200(sc *scenarioContext) dtos.DashboardFullWithMeta {
@@ -790,19 +797,6 @@ func CallDeleteDashboardByUid(sc *scenarioContext) {
 }
 
 func CallPostDashboard(sc *scenarioContext) {
-	bus.AddHandler("test", func(cmd *alerting.ValidateDashboardAlertsCommand) error {
-		return nil
-	})
-
-	bus.AddHandler("test", func(cmd *m.SaveDashboardCommand) error {
-		cmd.Result = &m.Dashboard{Id: 2, Slug: "Dash", Version: 2}
-		return nil
-	})
-
-	bus.AddHandler("test", func(cmd *alerting.UpdateDashboardAlertsCommand) error {
-		return nil
-	})
-
 	sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec()
 }
 
@@ -810,33 +804,29 @@ func CallPostDashboardShouldReturnSuccess(sc *scenarioContext) {
 	CallPostDashboard(sc)
 
 	So(sc.resp.Code, ShouldEqual, 200)
-	result := sc.ToJson()
-	So(result.Get("status").MustString(), ShouldEqual, "success")
-	So(result.Get("id").MustInt64(), ShouldBeGreaterThan, 0)
-	So(result.Get("uid").MustString(), ShouldNotBeNil)
-	So(result.Get("slug").MustString(), ShouldNotBeNil)
-	So(result.Get("url").MustString(), ShouldNotBeNil)
 }
 
-func postDashboardScenario(desc string, url string, routePattern string, role m.RoleType, cmd m.SaveDashboardCommand, fn scenarioFunc) {
+func postDashboardScenario(desc string, url string, routePattern string, mock *dashboards.FakeDashboardService, cmd m.SaveDashboardCommand, fn scenarioFunc) {
 	Convey(desc+" "+url, func() {
 		defer bus.ClearBusHandlers()
 
 		sc := setupScenarioContext(url)
 		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.SignedInUser = &m.SignedInUser{OrgId: cmd.OrgId, UserId: cmd.UserId}
 
 			return PostDashboard(c, cmd)
 		})
 
-		fakeRepo = &fakeDashboardRepo{}
-		dashboards.SetRepository(fakeRepo)
+		origNewDashboardService := dashboards.NewService
+		dashboards.MockDashboardService(mock)
 
 		sc.m.Post(routePattern, sc.defaultHandler)
 
+		defer func() {
+			dashboards.NewService = origNewDashboardService
+		}()
+
 		fn(sc)
 	})
 }