From db89ac4134088d1558c80284735043c211d75009 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Wed, 14 Feb 2018 11:50:58 +0100 Subject: [PATCH 01/52] initial fixes for dashboard permission acl list query, fixes #10864 --- pkg/services/sqlstore/dashboard_acl.go | 62 +++++++-------------- pkg/services/sqlstore/dashboard_acl_test.go | 17 ++++++ pkg/services/sqlstore/org_test.go | 15 ++++- 3 files changed, 49 insertions(+), 45 deletions(-) diff --git a/pkg/services/sqlstore/dashboard_acl.go b/pkg/services/sqlstore/dashboard_acl.go index 829182a81959..a1a308d6497e 100644 --- a/pkg/services/sqlstore/dashboard_acl.go +++ b/pkg/services/sqlstore/dashboard_acl.go @@ -1,7 +1,6 @@ package sqlstore import ( - "fmt" "time" "github.com/grafana/grafana/pkg/bus" @@ -40,7 +39,7 @@ 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 @@ -134,6 +133,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 +152,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 +179,21 @@ func GetDashboardAclInfoList(query *m.GetDashboardAclInfoListQuery) error { d.slug, d.uid, d.is_folder - FROM` + dialect.Quote("dashboard_acl") + ` as da - LEFT OUTER JOIN ` + dialect.Quote("user") + ` AS u ON u.id = da.user_id - LEFT OUTER JOIN team ug on ug.id = da.team_id - LEFT OUTER JOIN dashboard d on da.dashboard_id = d.id - WHERE dashboard_id ` + dashboardFilter + ` AND da.org_id = ? - - -- Also include default permissions if folder or dashboard field "has_acl" is false - - UNION - SELECT - da.id, - da.org_id, - da.dashboard_id, - da.user_id, - da.team_id, - 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 + FROM dashboard as d + LEFT JOIN dashboard folder on folder.id = d.folder_id + LEFT JOIN dashboard_acl AS da ON + da.dashboard_id = d.id OR + da.dashboard_id = d.folder_id OR + ( + -- include default permissions --> + da.org_id = -1 AND ( + (folder.id IS NOT NULL AND folder.has_acl = ` + falseStr + `) OR + (folder.id IS NULL AND d.has_acl = ` + falseStr + `) + ) + ) + LEFT JOIN ` + dialect.Quote("user") + ` AS u ON u.id = da.user_id + LEFT JOIN team ug on ug.id = da.team_id + WHERE d.org_id = ? AND d.id = ? AND da.id IS NOT NULL ORDER BY 1 ASC ` diff --git a/pkg/services/sqlstore/dashboard_acl_test.go b/pkg/services/sqlstore/dashboard_acl_test.go index 8b712c73ece7..8d4af9544d98 100644 --- a/pkg/services/sqlstore/dashboard_acl_test.go +++ b/pkg/services/sqlstore/dashboard_acl_test.go @@ -41,6 +41,23 @@ 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{ OrgId: 1, diff --git a/pkg/services/sqlstore/org_test.go b/pkg/services/sqlstore/org_test.go index 5322dfd47483..c57d15a48d5f 100644 --- a/pkg/services/sqlstore/org_test.go +++ b/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) +} From ec6f0f94b80c10e30226d2bdccb8d9db5c885b86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Wed, 14 Feb 2018 14:31:20 +0100 Subject: [PATCH 02/52] permissions: refactoring of acl api and query --- pkg/api/api.go | 1 - pkg/api/dashboard_acl.go | 30 ----- pkg/api/dashboard_acl_test.go | 104 ++---------------- pkg/api/dashboard_test.go | 8 +- pkg/models/dashboard_acl.go | 16 --- pkg/services/guardian/guardian.go | 20 ---- pkg/services/sqlstore/dashboard.go | 32 +----- pkg/services/sqlstore/dashboard_acl.go | 85 +------------- pkg/services/sqlstore/dashboard_acl_test.go | 74 ++----------- .../sqlstore/dashboard_folder_test.go | 29 ++--- pkg/services/sqlstore/dashboard_test.go | 19 ---- pkg/services/sqlstore/team_test.go | 2 +- pkg/services/sqlstore/user_test.go | 2 +- .../PermissionsStore/PermissionsStoreItem.ts | 3 +- 14 files changed, 40 insertions(+), 385 deletions(-) diff --git a/pkg/api/api.go b/pkg/api/api.go index c03bf7963b86..1320663f6300 100644 --- a/pkg/api/api.go +++ b/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)) }) }) }) diff --git a/pkg/api/dashboard_acl.go b/pkg/api/dashboard_acl.go index 45f121dd0d0d..32b75e80cc0d 100644 --- a/pkg/api/dashboard_acl.go +++ b/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, "") -} diff --git a/pkg/api/dashboard_acl_test.go b/pkg/api/dashboard_acl_test.go index e43e57ed5c00..d6b7e305daf3 100644 --- a/pkg/api/dashboard_acl_test.go +++ b/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}, - {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, TeamId: 1, Permission: m.PERMISSION_VIEW}, - {Id: 5, OrgId: 1, DashboardId: 1, TeamId: 2, Permission: m.PERMISSION_ADMIN}, + {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) @@ -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, diff --git a/pkg/api/dashboard_test.go b/pkg/api/dashboard_test.go index e80b3cad4dcf..4a45c561d579 100644 --- a/pkg/api/dashboard_test.go +++ b/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 { diff --git a/pkg/models/dashboard_acl.go b/pkg/models/dashboard_acl.go index 933487650e37..202b519207d7 100644 --- a/pkg/models/dashboard_acl.go +++ b/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 // diff --git a/pkg/services/guardian/guardian.go b/pkg/services/guardian/guardian.go index b448561494d5..05795b7f2dfc 100644 --- a/pkg/services/guardian/guardian.go +++ b/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 diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index f3fd81ebbe2f..42c83da88102 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/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) diff --git a/pkg/services/sqlstore/dashboard_acl.go b/pkg/services/sqlstore/dashboard_acl.go index a1a308d6497e..ae91d1d41f35 100644 --- a/pkg/services/sqlstore/dashboard_acl.go +++ b/pkg/services/sqlstore/dashboard_acl.go @@ -1,16 +1,12 @@ package sqlstore import ( - "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) } @@ -23,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 } @@ -46,85 +42,6 @@ func UpdateDashboardAcl(cmd *m.UpdateDashboardAclCommand) error { }) } -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 diff --git a/pkg/services/sqlstore/dashboard_acl_test.go b/pkg/services/sqlstore/dashboard_acl_test.go index 8d4af9544d98..8fbb9c0d8136 100644 --- a/pkg/services/sqlstore/dashboard_acl_test.go +++ b/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, @@ -59,7 +59,7 @@ func TestDashboardAclDataAccess(t *testing.T) { }) Convey("Given dashboard folder permission", func() { - err := SetDashboardAcl(&m.SetDashboardAclCommand{ + err := testHelperUpdateDashboardAcl(savedFolder.Id, m.DashboardAcl{ OrgId: 1, UserId: currentUser.Id, DashboardId: savedFolder.Id, @@ -78,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, @@ -100,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, @@ -125,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) @@ -147,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{ - OrgId: 1, - AclId: setDashAclCmd.Result.Id, - }) - + err := testHelperUpdateDashboardAcl(savedFolder.Id) So(err, ShouldBeNil) q3 := &m.GetDashboardAclInfoListQuery{DashboardId: savedFolder.Id, OrgId: 1} @@ -198,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} @@ -214,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, @@ -246,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) }) - }) }) diff --git a/pkg/services/sqlstore/dashboard_folder_test.go b/pkg/services/sqlstore/dashboard_folder_test.go index b32a4dfed1d6..40d6cf5bcb23 100644 --- a/pkg/services/sqlstore/dashboard_folder_test.go +++ b/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) - removeAcl(aclId) - updateTestDashboardWithAcl(childDash.Id, otherUser, m.PERMISSION_EDIT) + testHelperUpdateDashboardAcl(folder.Id) + testHelperUpdateDashboardAcl(childDash.Id, m.DashboardAcl{DashboardId: folder.Id, OrgId: 1, UserId: otherUser, Permission: 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) - So(movedDash.HasAcl, ShouldBeTrue) + moveDashboard(1, childDash2.Data, folder1.Id) 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() { - - movedDash := moveDashboard(1, childDash1.Data, folder2.Id) - So(movedDash.HasAcl, ShouldBeFalse) + moveDashboard(1, childDash1.Data, folder2.Id) 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) - movedDash := moveDashboard(1, childDash1.Data, folder2.Id) - So(movedDash.HasAcl, ShouldBeTrue) + testHelperUpdateDashboardAcl(childDash1.Id, m.DashboardAcl{DashboardId: childDash1.Id, OrgId: 1, UserId: otherUser, Permission: m.PERMISSION_EDIT}) + moveDashboard(1, childDash1.Data, folder2.Id) 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) diff --git a/pkg/services/sqlstore/dashboard_test.go b/pkg/services/sqlstore/dashboard_test.go index de7cdf199270..7de4c5f57012 100644 --- a/pkg/services/sqlstore/dashboard_test.go +++ b/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, diff --git a/pkg/services/sqlstore/team_test.go b/pkg/services/sqlstore/team_test.go index bebe59f42385..fb76c3fa9d66 100644 --- a/pkg/services/sqlstore/team_test.go +++ b/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) diff --git a/pkg/services/sqlstore/user_test.go b/pkg/services/sqlstore/user_test.go index a65b7226eb66..2830733c96af 100644 --- a/pkg/services/sqlstore/user_test.go +++ b/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"}) diff --git a/public/app/stores/PermissionsStore/PermissionsStoreItem.ts b/public/app/stores/PermissionsStore/PermissionsStoreItem.ts index 74769891256f..92dca0220ca3 100644 --- a/public/app/stores/PermissionsStore/PermissionsStoreItem.ts +++ b/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), From 73eaba076e4de50289c2403e8ab87a1a4485b213 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Wed, 14 Feb 2018 15:02:42 +0100 Subject: [PATCH 03/52] wip: dashboard acl ux2, #10747 --- pkg/api/dashboard_acl.go | 2 ++ pkg/models/dashboard_acl.go | 3 +++ pkg/services/sqlstore/dashboard_acl.go | 1 + .../DisabledPermissionsListItem.tsx | 6 +++--- .../components/Permissions/Permissions.tsx | 3 +-- .../Permissions/PermissionsList.tsx | 4 ++-- .../Permissions/PermissionsListItem.tsx | 19 +++++++++++++++---- .../PermissionsStore/PermissionsStore.ts | 11 +++-------- .../PermissionsStore/PermissionsStoreItem.ts | 5 +++-- 9 files changed, 33 insertions(+), 21 deletions(-) diff --git a/pkg/api/dashboard_acl.go b/pkg/api/dashboard_acl.go index 32b75e80cc0d..d15a575a05eb 100644 --- a/pkg/api/dashboard_acl.go +++ b/pkg/api/dashboard_acl.go @@ -30,6 +30,8 @@ func GetDashboardAclList(c *middleware.Context) Response { } for _, perm := range acl { + perm.UserAvatarUrl = dtos.GetGravatarUrl(perm.UserEmail) + perm.TeamAvatarUrl = dtos.GetGravatarUrl(perm.TeamEmail) if perm.Slug != "" { perm.Url = m.GetDashboardFolderUrl(perm.IsFolder, perm.Uid, perm.Slug) } diff --git a/pkg/models/dashboard_acl.go b/pkg/models/dashboard_acl.go index 202b519207d7..0e14a3bfd71b 100644 --- a/pkg/models/dashboard_acl.go +++ b/pkg/models/dashboard_acl.go @@ -53,7 +53,10 @@ type DashboardAclInfoDTO struct { UserId int64 `json:"userId"` UserLogin string `json:"userLogin"` UserEmail string `json:"userEmail"` + UserAvatarUrl string `json:"userAvatarUrl"` TeamId int64 `json:"teamId"` + TeamEmail string `json:"teamEmail"` + TeamAvatarUrl string `json:"teamAvatarUrl"` Team string `json:"team"` Role *RoleType `json:"role,omitempty"` Permission PermissionType `json:"permission"` diff --git a/pkg/services/sqlstore/dashboard_acl.go b/pkg/services/sqlstore/dashboard_acl.go index ae91d1d41f35..6e7175335f39 100644 --- a/pkg/services/sqlstore/dashboard_acl.go +++ b/pkg/services/sqlstore/dashboard_acl.go @@ -92,6 +92,7 @@ func GetDashboardAclInfoList(query *m.GetDashboardAclInfoListQuery) error { u.login AS user_login, u.email AS user_email, ug.name AS team, + ug.email AS team_email, d.title, d.slug, d.uid, diff --git a/public/app/core/components/Permissions/DisabledPermissionsListItem.tsx b/public/app/core/components/Permissions/DisabledPermissionsListItem.tsx index db45714136e9..e3f3ee56d757 100644 --- a/public/app/core/components/Permissions/DisabledPermissionsListItem.tsx +++ b/public/app/core/components/Permissions/DisabledPermissionsListItem.tsx @@ -1,4 +1,4 @@ -import React, { Component } from 'react'; +import React, { Component } from 'react'; import DescriptionPicker from 'app/core/components/Picker/DescriptionPicker'; import { permissionOptions } from 'app/stores/PermissionsStore/PermissionsStore'; @@ -12,10 +12,10 @@ export default class DisabledPermissionListItem extends Component { return ( - + - + {item.name} Can diff --git a/public/app/core/components/Permissions/Permissions.tsx b/public/app/core/components/Permissions/Permissions.tsx index 0a0572ed86e7..dbdc1682f6b1 100644 --- a/public/app/core/components/Permissions/Permissions.tsx +++ b/public/app/core/components/Permissions/Permissions.tsx @@ -15,9 +15,8 @@ export interface DashboardAcl { permissionName?: string; role?: string; icon?: string; - nameHtml?: string; + name?: string; inherited?: boolean; - sortName?: string; sortRank?: number; } diff --git a/public/app/core/components/Permissions/PermissionsList.tsx b/public/app/core/components/Permissions/PermissionsList.tsx index b215dad2391a..a77235ecc309 100644 --- a/public/app/core/components/Permissions/PermissionsList.tsx +++ b/public/app/core/components/Permissions/PermissionsList.tsx @@ -1,4 +1,4 @@ -import React, { Component } from 'react'; +import React, { Component } from 'react'; import PermissionsListItem from './PermissionsListItem'; import DisabledPermissionsListItem from './DisabledPermissionsListItem'; import { observer } from 'mobx-react'; @@ -23,7 +23,7 @@ class PermissionsList extends Component { Admin Role', + name: 'Admin', permission: 4, icon: 'fa fa-fw fa-street-view', }} diff --git a/public/app/core/components/Permissions/PermissionsListItem.tsx b/public/app/core/components/Permissions/PermissionsListItem.tsx index 3140b8fcc0cb..2ab5b948440c 100644 --- a/public/app/core/components/Permissions/PermissionsListItem.tsx +++ b/public/app/core/components/Permissions/PermissionsListItem.tsx @@ -1,4 +1,4 @@ -import React from 'react'; +import React from 'react'; import { observer } from 'mobx-react'; import DescriptionPicker from 'app/core/components/Picker/DescriptionPicker'; import { permissionOptions } from 'app/stores/PermissionsStore/PermissionsStore'; @@ -7,6 +7,16 @@ const setClassNameHelper = inherited => { return inherited ? 'gf-form-disabled' : ''; }; +function ItemAvatar({ item }) { + if (item.userAvatarUrl) { + return ; + } + if (item.teamAvatarUrl) { + return ; + } + return ; +} + export default observer(({ item, removeItem, permissionChanged, itemIndex, folderInfo }) => { const handleRemoveItem = evt => { evt.preventDefault(); @@ -18,13 +28,14 @@ export default observer(({ item, removeItem, permissionChanged, itemIndex, folde }; const inheritedFromRoot = item.dashboardId === -1 && folderInfo && folderInfo.id === 0; + console.log(item.name); return ( - - - + + + {item.name} {item.inherited && folderInfo && ( diff --git a/public/app/stores/PermissionsStore/PermissionsStore.ts b/public/app/stores/PermissionsStore/PermissionsStore.ts index a7c90d13da09..7838744c5419 100644 --- a/public/app/stores/PermissionsStore/PermissionsStore.ts +++ b/public/app/stores/PermissionsStore/PermissionsStore.ts @@ -231,19 +231,14 @@ const prepareItem = (item, dashboardId: number, isFolder: boolean, isInRoot: boo item.sortRank = 0; if (item.userId > 0) { - item.icon = 'fa fa-fw fa-user'; - item.nameHtml = item.userLogin; - item.sortName = item.userLogin; + item.name = item.userLogin; item.sortRank = 10; } else if (item.teamId > 0) { - item.icon = 'fa fa-fw fa-users'; - item.nameHtml = item.team; - item.sortName = item.team; + item.name = item.team; item.sortRank = 20; } else if (item.role) { item.icon = 'fa fa-fw fa-street-view'; - item.nameHtml = `Everyone with ${item.role} Role`; - item.sortName = item.role; + item.name = item.role; item.sortRank = 30; if (item.role === 'Viewer') { item.sortRank += 1; diff --git a/public/app/stores/PermissionsStore/PermissionsStoreItem.ts b/public/app/stores/PermissionsStore/PermissionsStoreItem.ts index 92dca0220ca3..c4873cb9c01f 100644 --- a/public/app/stores/PermissionsStore/PermissionsStoreItem.ts +++ b/public/app/stores/PermissionsStore/PermissionsStoreItem.ts @@ -14,8 +14,9 @@ export const PermissionsStoreItem = types inherited: types.maybe(types.boolean), sortRank: types.maybe(types.number), icon: types.maybe(types.string), - nameHtml: types.maybe(types.string), - sortName: types.maybe(types.string), + name: types.maybe(types.string), + teamAvatarUrl: types.maybe(types.string), + userAvatarUrl: types.maybe(types.string), }) .actions(self => ({ updateRole: role => { From e037ef21f790f6ea4aea3c3e0ee29e66375e636c Mon Sep 17 00:00:00 2001 From: Patrick O'Carroll Date: Mon, 26 Feb 2018 10:21:24 +0100 Subject: [PATCH 04/52] added admin icon and permission member definitions(role,team,user) --- .../Permissions/DisabledPermissionsListItem.tsx | 7 +++++-- .../Permissions/PermissionsListItem.tsx | 16 ++++++++++++++-- public/sass/components/_filter-table.scss | 4 ++++ 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/public/app/core/components/Permissions/DisabledPermissionsListItem.tsx b/public/app/core/components/Permissions/DisabledPermissionsListItem.tsx index e3f3ee56d757..adc2bec3d813 100644 --- a/public/app/core/components/Permissions/DisabledPermissionsListItem.tsx +++ b/public/app/core/components/Permissions/DisabledPermissionsListItem.tsx @@ -13,9 +13,12 @@ export default class DisabledPermissionListItem extends Component { return ( - + + + + {item.name} + (Role) - {item.name} Can diff --git a/public/app/core/components/Permissions/PermissionsListItem.tsx b/public/app/core/components/Permissions/PermissionsListItem.tsx index 2ab5b948440c..1bec7003f1fb 100644 --- a/public/app/core/components/Permissions/PermissionsListItem.tsx +++ b/public/app/core/components/Permissions/PermissionsListItem.tsx @@ -14,7 +14,17 @@ function ItemAvatar({ item }) { if (item.teamAvatarUrl) { return ; } - return ; + return ; +} + +function ItemDescription({ item }) { + if (item.userId) { + return (User); + } + if (item.teamId) { + return (Team); + } + return (Role); } export default observer(({ item, removeItem, permissionChanged, itemIndex, folderInfo }) => { @@ -35,7 +45,9 @@ export default observer(({ item, removeItem, permissionChanged, itemIndex, folde - {item.name} + + {item.name} + {item.inherited && folderInfo && ( diff --git a/public/sass/components/_filter-table.scss b/public/sass/components/_filter-table.scss index 00f9b93dcfd7..bfa9fbbbc5a4 100644 --- a/public/sass/components/_filter-table.scss +++ b/public/sass/components/_filter-table.scss @@ -85,3 +85,7 @@ } } } +.filter-table__weak-italic { + font-style: italic; + color: $text-color-weak; +} From 8b076d921f9b18f9f8bca173c12a2203c044415e Mon Sep 17 00:00:00 2001 From: Patrick O'Carroll Date: Tue, 3 Apr 2018 15:20:39 +0200 Subject: [PATCH 05/52] fixes for avatar on adding permission and size for gicon --- public/app/core/components/Permissions/AddPermissions.tsx | 2 +- .../components/Permissions/DisabledPermissionsListItem.tsx | 2 +- .../app/core/components/Permissions/PermissionsListItem.tsx | 2 +- public/app/stores/PermissionsStore/PermissionsStore.ts | 6 +++++- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/public/app/core/components/Permissions/AddPermissions.tsx b/public/app/core/components/Permissions/AddPermissions.tsx index 07ccfdbbef52..cfdcebf30631 100644 --- a/public/app/core/components/Permissions/AddPermissions.tsx +++ b/public/app/core/components/Permissions/AddPermissions.tsx @@ -39,7 +39,7 @@ class AddPermissions extends Component { permissions.newItem.setUser(null, null); return; } - return permissions.newItem.setUser(user.id, user.login); + return permissions.newItem.setUser(user.id, user.login, user.avatarUrl); } teamPicked(team: Team) { diff --git a/public/app/core/components/Permissions/DisabledPermissionsListItem.tsx b/public/app/core/components/Permissions/DisabledPermissionsListItem.tsx index adc2bec3d813..5e473c258696 100644 --- a/public/app/core/components/Permissions/DisabledPermissionsListItem.tsx +++ b/public/app/core/components/Permissions/DisabledPermissionsListItem.tsx @@ -13,7 +13,7 @@ export default class DisabledPermissionListItem extends Component { return ( - + {item.name} diff --git a/public/app/core/components/Permissions/PermissionsListItem.tsx b/public/app/core/components/Permissions/PermissionsListItem.tsx index 1bec7003f1fb..f1d96aaf3583 100644 --- a/public/app/core/components/Permissions/PermissionsListItem.tsx +++ b/public/app/core/components/Permissions/PermissionsListItem.tsx @@ -28,6 +28,7 @@ function ItemDescription({ item }) { } export default observer(({ item, removeItem, permissionChanged, itemIndex, folderInfo }) => { + console.log(item); const handleRemoveItem = evt => { evt.preventDefault(); removeItem(itemIndex); @@ -38,7 +39,6 @@ export default observer(({ item, removeItem, permissionChanged, itemIndex, folde }; const inheritedFromRoot = item.dashboardId === -1 && folderInfo && folderInfo.id === 0; - console.log(item.name); return ( diff --git a/public/app/stores/PermissionsStore/PermissionsStore.ts b/public/app/stores/PermissionsStore/PermissionsStore.ts index a96ff36a3762..8899931255ff 100644 --- a/public/app/stores/PermissionsStore/PermissionsStore.ts +++ b/public/app/stores/PermissionsStore/PermissionsStore.ts @@ -30,6 +30,7 @@ export const NewPermissionsItem = types ), userId: types.maybe(types.number), userLogin: types.maybe(types.string), + userAvatarUrl: types.maybe(types.string), teamId: types.maybe(types.number), team: types.maybe(types.string), permission: types.optional(types.number, 1), @@ -50,9 +51,10 @@ export const NewPermissionsItem = types }, })) .actions(self => ({ - setUser(userId: number, userLogin: string) { + setUser(userId: number, userLogin: string, userAvatarUrl: string) { self.userId = userId; self.userLogin = userLogin; + self.userAvatarUrl = userAvatarUrl; self.teamId = null; self.team = null; }, @@ -121,6 +123,7 @@ export const PermissionsStore = types teamId: undefined, userLogin: undefined, userId: undefined, + userAvatarUrl: undefined, role: undefined, }; switch (self.newItem.type) { @@ -131,6 +134,7 @@ export const PermissionsStore = types case aclTypeValues.USER.value: item.userLogin = self.newItem.userLogin; item.userId = self.newItem.userId; + item.userAvatarUrl = self.newItem.userAvatarUrl; break; case aclTypeValues.VIEWER.value: case aclTypeValues.EDITOR.value: From ebad863f95a419a8330e0ec4fc03904faed353cf Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Wed, 4 Apr 2018 15:50:45 +0200 Subject: [PATCH 06/52] permission: generate team avatar url with default --- pkg/api/dashboard_permission.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/api/dashboard_permission.go b/pkg/api/dashboard_permission.go index 2f85d6f55e8e..342eaf556c66 100644 --- a/pkg/api/dashboard_permission.go +++ b/pkg/api/dashboard_permission.go @@ -30,7 +30,10 @@ func GetDashboardPermissionList(c *m.ReqContext) Response { for _, perm := range acl { perm.UserAvatarUrl = dtos.GetGravatarUrl(perm.UserEmail) - perm.TeamAvatarUrl = dtos.GetGravatarUrl(perm.TeamEmail) + + if perm.TeamId > 0 { + perm.TeamAvatarUrl = dtos.GetGravatarUrlWithDefault(perm.TeamEmail, perm.Team) + } if perm.Slug != "" { perm.Url = m.GetDashboardFolderUrl(perm.IsFolder, perm.Uid, perm.Slug) } From 8f94cecf0f4fbc9347dd2baec55196b972545307 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Wed, 4 Apr 2018 15:51:19 +0200 Subject: [PATCH 07/52] permissions: return user and team avatar in folder permissions api --- pkg/api/folder_permission.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/api/folder_permission.go b/pkg/api/folder_permission.go index 0d0904c99ea5..d19ec848ab2d 100644 --- a/pkg/api/folder_permission.go +++ b/pkg/api/folder_permission.go @@ -33,6 +33,12 @@ func GetFolderPermissionList(c *m.ReqContext) Response { perm.FolderId = folder.Id perm.DashboardId = 0 + perm.UserAvatarUrl = dtos.GetGravatarUrl(perm.UserEmail) + + if perm.TeamId > 0 { + perm.TeamAvatarUrl = dtos.GetGravatarUrlWithDefault(perm.TeamEmail, perm.Team) + } + if perm.Slug != "" { perm.Url = m.GetDashboardFolderUrl(perm.IsFolder, perm.Uid, perm.Slug) } From b363e160d99f3e1684cb79ba6c6bbdebf1123c02 Mon Sep 17 00:00:00 2001 From: Patrick O'Carroll Date: Fri, 6 Apr 2018 09:43:59 +0200 Subject: [PATCH 08/52] added icons for viewer and editor, fixed add permission team avatar --- .../components/Permissions/AddPermissions.tsx | 2 +- .../Permissions/PermissionsListItem.tsx | 8 ++++++-- .../PermissionsStore/PermissionsStore.ts | 6 +++++- public/img/icons_dark_theme/icon_editor.svg | 19 +++++++++++++++++++ public/img/icons_dark_theme/icon_viewer.svg | 17 +++++++++++++++++ public/img/icons_light_theme/icon_editor.svg | 19 +++++++++++++++++++ public/img/icons_light_theme/icon_viewer.svg | 17 +++++++++++++++++ public/sass/base/_icons.scss | 8 ++++++++ 8 files changed, 92 insertions(+), 4 deletions(-) create mode 100644 public/img/icons_dark_theme/icon_editor.svg create mode 100644 public/img/icons_dark_theme/icon_viewer.svg create mode 100644 public/img/icons_light_theme/icon_editor.svg create mode 100644 public/img/icons_light_theme/icon_viewer.svg diff --git a/public/app/core/components/Permissions/AddPermissions.tsx b/public/app/core/components/Permissions/AddPermissions.tsx index cfdcebf30631..4dcd07ffb48a 100644 --- a/public/app/core/components/Permissions/AddPermissions.tsx +++ b/public/app/core/components/Permissions/AddPermissions.tsx @@ -48,7 +48,7 @@ class AddPermissions extends Component { permissions.newItem.setTeam(null, null); return; } - return permissions.newItem.setTeam(team.id, team.name); + return permissions.newItem.setTeam(team.id, team.name, team.avatarUrl); } permissionPicked(permission: OptionWithDescription) { diff --git a/public/app/core/components/Permissions/PermissionsListItem.tsx b/public/app/core/components/Permissions/PermissionsListItem.tsx index f1d96aaf3583..f0da7b318815 100644 --- a/public/app/core/components/Permissions/PermissionsListItem.tsx +++ b/public/app/core/components/Permissions/PermissionsListItem.tsx @@ -8,13 +8,18 @@ const setClassNameHelper = inherited => { }; function ItemAvatar({ item }) { + console.log(item); if (item.userAvatarUrl) { return ; } if (item.teamAvatarUrl) { return ; } - return ; + if (item.role === 'Editor') { + return ; + } + + return ; } function ItemDescription({ item }) { @@ -28,7 +33,6 @@ function ItemDescription({ item }) { } export default observer(({ item, removeItem, permissionChanged, itemIndex, folderInfo }) => { - console.log(item); const handleRemoveItem = evt => { evt.preventDefault(); removeItem(itemIndex); diff --git a/public/app/stores/PermissionsStore/PermissionsStore.ts b/public/app/stores/PermissionsStore/PermissionsStore.ts index 8899931255ff..7761ecc13c78 100644 --- a/public/app/stores/PermissionsStore/PermissionsStore.ts +++ b/public/app/stores/PermissionsStore/PermissionsStore.ts @@ -31,6 +31,7 @@ export const NewPermissionsItem = types userId: types.maybe(types.number), userLogin: types.maybe(types.string), userAvatarUrl: types.maybe(types.string), + teamAvatarUrl: types.maybe(types.string), teamId: types.maybe(types.number), team: types.maybe(types.string), permission: types.optional(types.number, 1), @@ -58,11 +59,12 @@ export const NewPermissionsItem = types self.teamId = null; self.team = null; }, - setTeam(teamId: number, team: string) { + setTeam(teamId: number, team: string, teamAvatarUrl: string) { self.userId = null; self.userLogin = null; self.teamId = teamId; self.team = team; + self.teamAvatarUrl = teamAvatarUrl; }, setPermission(permission: number) { self.permission = permission; @@ -124,12 +126,14 @@ export const PermissionsStore = types userLogin: undefined, userId: undefined, userAvatarUrl: undefined, + teamAvatarUrl: undefined, role: undefined, }; switch (self.newItem.type) { case aclTypeValues.GROUP.value: item.team = self.newItem.team; item.teamId = self.newItem.teamId; + item.teamAvatarUrl = self.newItem.teamAvatarUrl; break; case aclTypeValues.USER.value: item.userLogin = self.newItem.userLogin; diff --git a/public/img/icons_dark_theme/icon_editor.svg b/public/img/icons_dark_theme/icon_editor.svg new file mode 100644 index 000000000000..00c60902fbcc --- /dev/null +++ b/public/img/icons_dark_theme/icon_editor.svg @@ -0,0 +1,19 @@ + + + + + + + + + + + + + diff --git a/public/img/icons_dark_theme/icon_viewer.svg b/public/img/icons_dark_theme/icon_viewer.svg new file mode 100644 index 000000000000..aec3e6b7e5b7 --- /dev/null +++ b/public/img/icons_dark_theme/icon_viewer.svg @@ -0,0 +1,17 @@ + + + + + + + + + + diff --git a/public/img/icons_light_theme/icon_editor.svg b/public/img/icons_light_theme/icon_editor.svg new file mode 100644 index 000000000000..a6581072a17d --- /dev/null +++ b/public/img/icons_light_theme/icon_editor.svg @@ -0,0 +1,19 @@ + + + + + + + + + + + + + diff --git a/public/img/icons_light_theme/icon_viewer.svg b/public/img/icons_light_theme/icon_viewer.svg new file mode 100644 index 000000000000..85d9b7109f45 --- /dev/null +++ b/public/img/icons_light_theme/icon_viewer.svg @@ -0,0 +1,17 @@ + + + + + + + + + + diff --git a/public/sass/base/_icons.scss b/public/sass/base/_icons.scss index c701cc1249e8..bf66d4dc68db 100644 --- a/public/sass/base/_icons.scss +++ b/public/sass/base/_icons.scss @@ -120,6 +120,10 @@ background-image: url('../img/icons_#{$theme-name}_theme/icon_data_sources.svg'); } +.gicon-editor { + background-image: url('../img/icons_#{$theme-name}_theme/icon_editor.svg'); +} + .gicon-folder-new { background-image: url('../img/icons_#{$theme-name}_theme/icon_add_folder.svg'); } @@ -180,6 +184,10 @@ background-image: url('../img/icons_#{$theme-name}_theme/icon_variable.svg'); } +.gicon-viewer { + background-image: url('../img/icons_#{$theme-name}_theme/icon_viewer.svg'); +} + .gicon-zoom-out { background-image: url('../img/icons_#{$theme-name}_theme/icon_zoom_out.svg'); } From 84d034c68861240f2fa85ecfef5e46fa6128c13c Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Fri, 6 Apr 2018 14:08:23 +0200 Subject: [PATCH 09/52] api: allow authenticated users to search current org users and teams Any authenticated user of an organization should be able to - search its organization's users - search its organization's users - retrieve a single user of its organization (this how it was implemented earlier) --- pkg/api/api.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/pkg/api/api.go b/pkg/api/api.go index 3c7b81e472dc..96b764b95b9e 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -149,8 +149,6 @@ func (hs *HTTPServer) registerRoutes() { // team (admin permission required) apiRoute.Group("/teams", func(teamsRoute RouteRegister) { - teamsRoute.Get("/:teamId", wrap(GetTeamByID)) - teamsRoute.Get("/search", wrap(SearchTeams)) teamsRoute.Post("/", bind(m.CreateTeamCommand{}), wrap(CreateTeam)) teamsRoute.Put("/:teamId", bind(m.UpdateTeamCommand{}), wrap(UpdateTeam)) teamsRoute.Delete("/:teamId", wrap(DeleteTeamByID)) @@ -159,6 +157,12 @@ func (hs *HTTPServer) registerRoutes() { teamsRoute.Delete("/:teamId/members/:userId", wrap(RemoveTeamMember)) }, reqOrgAdmin) + // team without requirement of user to be org admin + apiRoute.Group("/teams", func(teamsRoute RouteRegister) { + teamsRoute.Get("/:teamId", wrap(GetTeamByID)) + teamsRoute.Get("/search", wrap(SearchTeams)) + }) + // org information available to all users. apiRoute.Group("/org", func(orgRoute RouteRegister) { orgRoute.Get("/", wrap(GetOrgCurrent)) @@ -170,7 +174,6 @@ func (hs *HTTPServer) registerRoutes() { orgRoute.Put("/", bind(dtos.UpdateOrgForm{}), wrap(UpdateOrgCurrent)) orgRoute.Put("/address", bind(dtos.UpdateOrgAddressForm{}), wrap(UpdateOrgAddressCurrent)) orgRoute.Post("/users", quota("user"), bind(m.AddOrgUserCommand{}), wrap(AddOrgUserToCurrentOrg)) - orgRoute.Get("/users", wrap(GetOrgUsersForCurrentOrg)) orgRoute.Patch("/users/:userId", bind(m.UpdateOrgUserCommand{}), wrap(UpdateOrgUserForCurrentOrg)) orgRoute.Delete("/users/:userId", wrap(RemoveOrgUserForCurrentOrg)) @@ -184,6 +187,11 @@ func (hs *HTTPServer) registerRoutes() { orgRoute.Put("/preferences", bind(dtos.UpdatePrefsCmd{}), wrap(UpdateOrgPreferences)) }, reqOrgAdmin) + // current org without requirement of user to be org admin + apiRoute.Group("/org", func(orgRoute RouteRegister) { + orgRoute.Get("/users", wrap(GetOrgUsersForCurrentOrg)) + }) + // create new org apiRoute.Post("/orgs", quota("org"), bind(m.CreateOrgCommand{}), wrap(CreateOrg)) From f3e1557761c9fe533c559f98ebbb44f12b572dcd Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Sun, 8 Apr 2018 15:06:22 +0200 Subject: [PATCH 10/52] guardian: when updating permissions should verify existing permissions Before in CheckPermissionBeforeUpdate, access was verified for updated permissions. Now access is verified for existing permissions. Refactored guardian tests to cover more test cases for org admin, editor and viewer roles --- pkg/services/guardian/guardian.go | 2 +- pkg/services/guardian/guardian_test.go | 1281 +++++++++---------- pkg/services/guardian/guardian_util_test.go | 256 ++++ 3 files changed, 874 insertions(+), 665 deletions(-) create mode 100644 pkg/services/guardian/guardian_util_test.go diff --git a/pkg/services/guardian/guardian.go b/pkg/services/guardian/guardian.go index 811b38cac862..649a6cbd661a 100644 --- a/pkg/services/guardian/guardian.go +++ b/pkg/services/guardian/guardian.go @@ -173,7 +173,7 @@ func (g *dashboardGuardianImpl) CheckPermissionBeforeUpdate(permission m.Permiss return true, nil } - return g.checkAcl(permission, acl) + return g.checkAcl(permission, existingPermissions) } // GetAcl returns dashboard acl diff --git a/pkg/services/guardian/guardian_test.go b/pkg/services/guardian/guardian_test.go index bb7e6bd1a72e..9de12c60fea7 100644 --- a/pkg/services/guardian/guardian_test.go +++ b/pkg/services/guardian/guardian_test.go @@ -2,710 +2,663 @@ package guardian import ( "fmt" + "runtime" "testing" - "github.com/grafana/grafana/pkg/bus" - m "github.com/grafana/grafana/pkg/models" . "github.com/smartystreets/goconvey/convey" ) -func TestGuardian(t *testing.T) { - Convey("Guardian permission tests", t, func() { - orgRoleScenario("Given user has admin org role", m.ROLE_ADMIN, func(sc *scenarioContext) { - canAdmin, _ := sc.g.CanAdmin() - canEdit, _ := sc.g.CanEdit() - canSave, _ := sc.g.CanSave() - canView, _ := sc.g.CanView() - So(canAdmin, ShouldBeTrue) - So(canEdit, ShouldBeTrue) - So(canSave, ShouldBeTrue) - So(canView, ShouldBeTrue) - - Convey("When trying to update permissions", func() { - Convey("With duplicate user permissions should return error", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_VIEW}, - {OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_ADMIN}, - } - _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(err, ShouldEqual, ErrGuardianPermissionExists) - }) - - Convey("With duplicate team permissions should return error", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 1, TeamId: 1, Permission: m.PERMISSION_VIEW}, - {OrgId: 1, DashboardId: 1, TeamId: 1, Permission: m.PERMISSION_ADMIN}, - } - _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(err, ShouldEqual, ErrGuardianPermissionExists) - }) - - Convey("With duplicate everyone with editor role permission should return error", func() { - r := m.ROLE_EDITOR - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 1, Role: &r, Permission: m.PERMISSION_VIEW}, - {OrgId: 1, DashboardId: 1, Role: &r, Permission: m.PERMISSION_ADMIN}, - } - _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(err, ShouldEqual, ErrGuardianPermissionExists) - }) - - Convey("With duplicate everyone with viewer role permission should return error", func() { - r := m.ROLE_VIEWER - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 1, Role: &r, Permission: m.PERMISSION_VIEW}, - {OrgId: 1, DashboardId: 1, Role: &r, Permission: m.PERMISSION_ADMIN}, - } - _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(err, ShouldEqual, ErrGuardianPermissionExists) - }) - - Convey("With everyone with admin role permission should return error", func() { - r := m.ROLE_ADMIN - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 1, Role: &r, Permission: m.PERMISSION_ADMIN}, - } - _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(err, ShouldEqual, ErrGuardianPermissionExists) - }) - }) - - Convey("Given default permissions", func() { - editor := m.ROLE_EDITOR - viewer := m.ROLE_VIEWER - existingPermissions := []*m.DashboardAclInfoDTO{ - {OrgId: 1, DashboardId: -1, Role: &editor, Permission: m.PERMISSION_EDIT}, - {OrgId: 1, DashboardId: -1, Role: &viewer, Permission: m.PERMISSION_VIEW}, - } - - bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error { - query.Result = existingPermissions - return nil - }) - - Convey("When trying to update dashboard permissions without everyone with role editor can edit should be allowed", func() { - r := m.ROLE_VIEWER - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 1, Role: &r, Permission: m.PERMISSION_VIEW}, - } - ok, _ := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(ok, ShouldBeTrue) - }) - - Convey("When trying to update dashboard permissions without everyone with role viewer can view should be allowed", func() { - r := m.ROLE_EDITOR - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 1, Role: &r, Permission: m.PERMISSION_EDIT}, - } - ok, _ := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(ok, ShouldBeTrue) - }) - }) - - Convey("Given parent folder has user admin permission", func() { - existingPermissions := []*m.DashboardAclInfoDTO{ - {OrgId: 1, DashboardId: 2, UserId: 1, Permission: m.PERMISSION_ADMIN}, - } - - bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error { - query.Result = existingPermissions - return nil - }) - - Convey("When trying to update dashboard permissions with admin user permission should return error", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 3, UserId: 1, Permission: m.PERMISSION_ADMIN}, - } - _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(err, ShouldEqual, ErrGuardianOverride) - }) - - Convey("When trying to update dashboard permissions with edit user permission should return error", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 3, UserId: 1, Permission: m.PERMISSION_EDIT}, - } - _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(err, ShouldEqual, ErrGuardianOverride) - }) - - Convey("When trying to update dashboard permissions with view user permission should return error", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 3, UserId: 1, Permission: m.PERMISSION_VIEW}, - } - _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(err, ShouldEqual, ErrGuardianOverride) - }) - }) - - Convey("Given parent folder has user edit permission", func() { - existingPermissions := []*m.DashboardAclInfoDTO{ - {OrgId: 1, DashboardId: 2, UserId: 1, Permission: m.PERMISSION_EDIT}, - } +var ( + orgID = int64(1) + defaultDashboardID = int64(-1) + dashboardID = int64(1) + parentFolderID = int64(2) + childDashboardID = int64(3) + userID = int64(1) + otherUserID = int64(2) + teamID = int64(1) + otherTeamID = int64(2) + adminRole = m.ROLE_ADMIN + editorRole = m.ROLE_EDITOR + viewerRole = m.ROLE_VIEWER +) - bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error { - query.Result = existingPermissions - return nil - }) - - Convey("When trying to update dashboard permissions with admin user permission should be allowed", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 3, UserId: 1, Permission: m.PERMISSION_ADMIN}, - } - ok, _ := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(ok, ShouldBeTrue) - }) - - Convey("When trying to update dashboard permissions with edit user permission should return error", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 3, UserId: 1, Permission: m.PERMISSION_EDIT}, - } - _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(err, ShouldEqual, ErrGuardianOverride) - }) - - Convey("When trying to update dashboard permissions with view user permission should return error", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 3, UserId: 1, Permission: m.PERMISSION_VIEW}, - } - _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(err, ShouldEqual, ErrGuardianOverride) - }) - }) - - Convey("Given parent folder has user view permission", func() { - existingPermissions := []*m.DashboardAclInfoDTO{ - {OrgId: 1, DashboardId: 2, UserId: 1, Permission: m.PERMISSION_VIEW}, - } +func TestGuardianAdmin(t *testing.T) { + Convey("Guardian admin org role tests", t, func() { + orgRoleScenario("Given user has admin org role", t, m.ROLE_ADMIN, func(sc *scenarioContext) { + // dashboard has default permissions + sc.defaultPermissionScenario(USER, m.PERMISSION_ADMIN, FULL_ACCESS) + + // dashboard has user with permission + sc.dashboardPermissionScenario(USER, m.PERMISSION_ADMIN, FULL_ACCESS) + sc.dashboardPermissionScenario(USER, m.PERMISSION_EDIT, FULL_ACCESS) + sc.dashboardPermissionScenario(USER, m.PERMISSION_VIEW, FULL_ACCESS) + + // dashboard has team with permission + sc.dashboardPermissionScenario(TEAM, m.PERMISSION_ADMIN, FULL_ACCESS) + sc.dashboardPermissionScenario(TEAM, m.PERMISSION_EDIT, FULL_ACCESS) + sc.dashboardPermissionScenario(TEAM, m.PERMISSION_VIEW, FULL_ACCESS) + + // dashboard has editor role with permission + sc.dashboardPermissionScenario(EDITOR, m.PERMISSION_ADMIN, FULL_ACCESS) + sc.dashboardPermissionScenario(EDITOR, m.PERMISSION_EDIT, FULL_ACCESS) + sc.dashboardPermissionScenario(EDITOR, m.PERMISSION_VIEW, FULL_ACCESS) + + // dashboard has viewer role with permission + sc.dashboardPermissionScenario(VIEWER, m.PERMISSION_ADMIN, FULL_ACCESS) + sc.dashboardPermissionScenario(VIEWER, m.PERMISSION_EDIT, FULL_ACCESS) + sc.dashboardPermissionScenario(VIEWER, m.PERMISSION_VIEW, FULL_ACCESS) + + // parent folder has user with permission + sc.parentFolderPermissionScenario(USER, m.PERMISSION_ADMIN, FULL_ACCESS) + sc.parentFolderPermissionScenario(USER, m.PERMISSION_EDIT, FULL_ACCESS) + sc.parentFolderPermissionScenario(USER, m.PERMISSION_VIEW, FULL_ACCESS) + + // parent folder has team with permission + sc.parentFolderPermissionScenario(TEAM, m.PERMISSION_ADMIN, FULL_ACCESS) + sc.parentFolderPermissionScenario(TEAM, m.PERMISSION_EDIT, FULL_ACCESS) + sc.parentFolderPermissionScenario(TEAM, m.PERMISSION_VIEW, FULL_ACCESS) + + // parent folder has editor role with permission + sc.parentFolderPermissionScenario(EDITOR, m.PERMISSION_ADMIN, FULL_ACCESS) + sc.parentFolderPermissionScenario(EDITOR, m.PERMISSION_EDIT, FULL_ACCESS) + sc.parentFolderPermissionScenario(EDITOR, m.PERMISSION_VIEW, FULL_ACCESS) + + // parent folder has viweer role with permission + sc.parentFolderPermissionScenario(VIEWER, m.PERMISSION_ADMIN, FULL_ACCESS) + sc.parentFolderPermissionScenario(VIEWER, m.PERMISSION_EDIT, FULL_ACCESS) + sc.parentFolderPermissionScenario(VIEWER, m.PERMISSION_VIEW, FULL_ACCESS) + }) + }) +} - bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error { - query.Result = existingPermissions - return nil - }) - - Convey("When trying to update dashboard permissions with admin user permission should be allowed", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 3, UserId: 1, Permission: m.PERMISSION_ADMIN}, - } - ok, _ := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(ok, ShouldBeTrue) - }) - - Convey("When trying to update dashboard permissions with edit user permission should be allowed", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 3, UserId: 1, Permission: m.PERMISSION_EDIT}, - } - ok, _ := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(ok, ShouldBeTrue) - }) - - Convey("When trying to update dashboard permissions with view user permission should return error", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 3, UserId: 1, Permission: m.PERMISSION_VIEW}, - } - _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(err, ShouldEqual, ErrGuardianOverride) - }) - }) - - Convey("Given parent folder has team admin permission", func() { - existingPermissions := []*m.DashboardAclInfoDTO{ - {OrgId: 1, DashboardId: 2, TeamId: 1, Permission: m.PERMISSION_ADMIN}, - } +func TestGuardianEditor(t *testing.T) { + Convey("Guardian editor org role tests", t, func() { + orgRoleScenario("Given user has editor org role", t, m.ROLE_EDITOR, func(sc *scenarioContext) { + // dashboard has user with permission + sc.dashboardPermissionScenario(USER, m.PERMISSION_ADMIN, FULL_ACCESS) + sc.dashboardPermissionScenario(USER, m.PERMISSION_EDIT, EDITOR_ACCESS) + sc.dashboardPermissionScenario(USER, m.PERMISSION_VIEW, CAN_VIEW) + + // dashboard has team with permission + sc.dashboardPermissionScenario(TEAM, m.PERMISSION_ADMIN, FULL_ACCESS) + sc.dashboardPermissionScenario(TEAM, m.PERMISSION_EDIT, EDITOR_ACCESS) + sc.dashboardPermissionScenario(TEAM, m.PERMISSION_VIEW, CAN_VIEW) + + // dashboard has editor role with permission + sc.dashboardPermissionScenario(EDITOR, m.PERMISSION_ADMIN, FULL_ACCESS) + sc.dashboardPermissionScenario(EDITOR, m.PERMISSION_EDIT, EDITOR_ACCESS) + sc.dashboardPermissionScenario(EDITOR, m.PERMISSION_VIEW, VIEWER_ACCESS) + + // dashboard has viewer role with permission + sc.dashboardPermissionScenario(VIEWER, m.PERMISSION_ADMIN, NO_ACCESS) + sc.dashboardPermissionScenario(VIEWER, m.PERMISSION_EDIT, NO_ACCESS) + sc.dashboardPermissionScenario(VIEWER, m.PERMISSION_VIEW, NO_ACCESS) + + // parent folder has user with permission + sc.parentFolderPermissionScenario(USER, m.PERMISSION_ADMIN, FULL_ACCESS) + sc.parentFolderPermissionScenario(USER, m.PERMISSION_EDIT, EDITOR_ACCESS) + sc.parentFolderPermissionScenario(USER, m.PERMISSION_VIEW, VIEWER_ACCESS) + + // parent folder has team with permission + sc.parentFolderPermissionScenario(TEAM, m.PERMISSION_ADMIN, FULL_ACCESS) + sc.parentFolderPermissionScenario(TEAM, m.PERMISSION_EDIT, EDITOR_ACCESS) + sc.parentFolderPermissionScenario(TEAM, m.PERMISSION_VIEW, VIEWER_ACCESS) + + // parent folder has editor role with permission + sc.parentFolderPermissionScenario(EDITOR, m.PERMISSION_ADMIN, FULL_ACCESS) + sc.parentFolderPermissionScenario(EDITOR, m.PERMISSION_EDIT, EDITOR_ACCESS) + sc.parentFolderPermissionScenario(EDITOR, m.PERMISSION_VIEW, VIEWER_ACCESS) + + // parent folder has viweer role with permission + sc.parentFolderPermissionScenario(VIEWER, m.PERMISSION_ADMIN, NO_ACCESS) + sc.parentFolderPermissionScenario(VIEWER, m.PERMISSION_EDIT, NO_ACCESS) + sc.parentFolderPermissionScenario(VIEWER, m.PERMISSION_VIEW, NO_ACCESS) + }) + }) +} - bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error { - query.Result = existingPermissions - return nil - }) - - Convey("When trying to update dashboard permissions with admin team permission should return error", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 3, TeamId: 1, Permission: m.PERMISSION_ADMIN}, - } - _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(err, ShouldEqual, ErrGuardianOverride) - }) - - Convey("When trying to update dashboard permissions with edit team permission should return error", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 3, TeamId: 1, Permission: m.PERMISSION_EDIT}, - } - _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(err, ShouldEqual, ErrGuardianOverride) - }) - - Convey("When trying to update dashboard permissions with view team permission should return error", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 3, TeamId: 1, Permission: m.PERMISSION_VIEW}, - } - _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(err, ShouldEqual, ErrGuardianOverride) - }) - }) - - Convey("Given parent folder has team edit permission", func() { - existingPermissions := []*m.DashboardAclInfoDTO{ - {OrgId: 1, DashboardId: 2, TeamId: 1, Permission: m.PERMISSION_EDIT}, - } +func TestGuardianViewer(t *testing.T) { + Convey("Guardian viewer org role tests", t, func() { + orgRoleScenario("Given user has viewer org role", t, m.ROLE_VIEWER, func(sc *scenarioContext) { + // dashboard has user with permission + sc.dashboardPermissionScenario(USER, m.PERMISSION_ADMIN, FULL_ACCESS) + sc.dashboardPermissionScenario(USER, m.PERMISSION_EDIT, EDITOR_ACCESS) + sc.dashboardPermissionScenario(USER, m.PERMISSION_VIEW, VIEWER_ACCESS) + + // dashboard has team with permission + sc.dashboardPermissionScenario(TEAM, m.PERMISSION_ADMIN, FULL_ACCESS) + sc.dashboardPermissionScenario(TEAM, m.PERMISSION_EDIT, EDITOR_ACCESS) + sc.dashboardPermissionScenario(TEAM, m.PERMISSION_VIEW, VIEWER_ACCESS) + + // dashboard has editor role with permission + sc.dashboardPermissionScenario(EDITOR, m.PERMISSION_ADMIN, NO_ACCESS) + sc.dashboardPermissionScenario(EDITOR, m.PERMISSION_EDIT, NO_ACCESS) + sc.dashboardPermissionScenario(EDITOR, m.PERMISSION_VIEW, NO_ACCESS) + + // dashboard has viewer role with permission + sc.dashboardPermissionScenario(VIEWER, m.PERMISSION_ADMIN, FULL_ACCESS) + sc.dashboardPermissionScenario(VIEWER, m.PERMISSION_EDIT, EDITOR_ACCESS) + sc.dashboardPermissionScenario(VIEWER, m.PERMISSION_VIEW, VIEWER_ACCESS) + + // parent folder has user with permission + sc.parentFolderPermissionScenario(USER, m.PERMISSION_ADMIN, FULL_ACCESS) + sc.parentFolderPermissionScenario(USER, m.PERMISSION_EDIT, EDITOR_ACCESS) + sc.parentFolderPermissionScenario(USER, m.PERMISSION_VIEW, VIEWER_ACCESS) + + // parent folder has team with permission + sc.parentFolderPermissionScenario(TEAM, m.PERMISSION_ADMIN, FULL_ACCESS) + sc.parentFolderPermissionScenario(TEAM, m.PERMISSION_EDIT, EDITOR_ACCESS) + sc.parentFolderPermissionScenario(TEAM, m.PERMISSION_VIEW, VIEWER_ACCESS) + + // parent folder has editor role with permission + sc.parentFolderPermissionScenario(EDITOR, m.PERMISSION_ADMIN, NO_ACCESS) + sc.parentFolderPermissionScenario(EDITOR, m.PERMISSION_EDIT, NO_ACCESS) + sc.parentFolderPermissionScenario(EDITOR, m.PERMISSION_VIEW, NO_ACCESS) + + // parent folder has viweer role with permission + sc.parentFolderPermissionScenario(VIEWER, m.PERMISSION_ADMIN, FULL_ACCESS) + sc.parentFolderPermissionScenario(VIEWER, m.PERMISSION_EDIT, EDITOR_ACCESS) + sc.parentFolderPermissionScenario(VIEWER, m.PERMISSION_VIEW, VIEWER_ACCESS) + }) + }) +} - bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error { - query.Result = existingPermissions - return nil - }) - - Convey("When trying to update dashboard permissions with admin team permission should be allowed", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 3, TeamId: 1, Permission: m.PERMISSION_ADMIN}, - } - ok, _ := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(ok, ShouldBeTrue) - }) - - Convey("When trying to update dashboard permissions with edit team permission should return error", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 3, TeamId: 1, Permission: m.PERMISSION_EDIT}, - } - _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(err, ShouldEqual, ErrGuardianOverride) - }) - - Convey("When trying to update dashboard permissions with view team permission should return error", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 3, TeamId: 1, Permission: m.PERMISSION_VIEW}, - } - _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(err, ShouldEqual, ErrGuardianOverride) - }) - }) - - Convey("Given parent folder has team view permission", func() { - existingPermissions := []*m.DashboardAclInfoDTO{ - {OrgId: 1, DashboardId: 2, TeamId: 1, Permission: m.PERMISSION_VIEW}, - } +func (sc *scenarioContext) defaultPermissionScenario(pt permissionType, permission m.PermissionType, flag permissionFlags) { + _, callerFile, callerLine, _ := runtime.Caller(1) + sc.callerFile = callerFile + sc.callerLine = callerLine + existingPermissions := []*m.DashboardAclInfoDTO{ + toDto(newEditorRolePermission(defaultDashboardID, m.PERMISSION_EDIT)), + toDto(newViewerRolePermission(defaultDashboardID, m.PERMISSION_VIEW)), + } - bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error { - query.Result = existingPermissions - return nil - }) - - Convey("When trying to update dashboard permissions with admin team permission should be allowed", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 3, TeamId: 1, Permission: m.PERMISSION_ADMIN}, - } - ok, _ := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(ok, ShouldBeTrue) - }) - - Convey("When trying to update dashboard permissions with edit team permission should be allowed", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 3, TeamId: 1, Permission: m.PERMISSION_EDIT}, - } - ok, _ := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(ok, ShouldBeTrue) - }) - - Convey("When trying to update dashboard permissions with view team permission should return error", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 3, TeamId: 1, Permission: m.PERMISSION_VIEW}, - } - _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(err, ShouldEqual, ErrGuardianOverride) - }) - }) - - Convey("Given parent folder has editor role with edit permission", func() { - r := m.ROLE_EDITOR - existingPermissions := []*m.DashboardAclInfoDTO{ - {OrgId: 1, DashboardId: 2, Role: &r, Permission: m.PERMISSION_EDIT}, - } + permissionScenario("and existing permissions is the default permissions (everyone with editor role can edit, everyone with viewer role can view)", dashboardID, sc, existingPermissions, func(sc *scenarioContext) { + sc.expectedFlags = flag + sc.verifyExpectedPermissionsFlags() + sc.verifyDuplicatePermissionsShouldNotBeAllowed() + sc.verifyUpdateDashboardPermissionsShouldBeAllowed(pt) + sc.verifyUpdateDashboardPermissionsShouldNotBeAllowed(pt) + }) +} - bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error { - query.Result = existingPermissions - return nil - }) - - Convey("When trying to update dashboard permissions with everyone with editor role can admin permission should be allowed", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 3, Role: &r, Permission: m.PERMISSION_ADMIN}, - } - ok, _ := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(ok, ShouldBeTrue) - }) - - Convey("When trying to update dashboard permissions with everyone with editor role can edit permission should return error", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 3, Role: &r, Permission: m.PERMISSION_EDIT}, - } - _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(err, ShouldEqual, ErrGuardianOverride) - }) - - Convey("When trying to update dashboard permissions with everyone with editor role can view permission should return error", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 3, Role: &r, Permission: m.PERMISSION_VIEW}, - } - _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(err, ShouldEqual, ErrGuardianOverride) - }) - }) - - Convey("Given parent folder has editor role with view permission", func() { - r := m.ROLE_EDITOR - existingPermissions := []*m.DashboardAclInfoDTO{ - {OrgId: 1, DashboardId: 2, Role: &r, Permission: m.PERMISSION_VIEW}, - } +func (sc *scenarioContext) dashboardPermissionScenario(pt permissionType, permission m.PermissionType, flag permissionFlags) { + _, callerFile, callerLine, _ := runtime.Caller(1) + sc.callerFile = callerFile + sc.callerLine = callerLine + var existingPermissions []*m.DashboardAclInfoDTO + + switch pt { + case USER: + existingPermissions = []*m.DashboardAclInfoDTO{{OrgId: orgID, DashboardId: dashboardID, UserId: userID, Permission: permission}} + case TEAM: + existingPermissions = []*m.DashboardAclInfoDTO{{OrgId: orgID, DashboardId: dashboardID, TeamId: teamID, Permission: permission}} + case EDITOR: + existingPermissions = []*m.DashboardAclInfoDTO{{OrgId: orgID, DashboardId: dashboardID, Role: &editorRole, Permission: permission}} + case VIEWER: + existingPermissions = []*m.DashboardAclInfoDTO{{OrgId: orgID, DashboardId: dashboardID, Role: &viewerRole, Permission: permission}} + } - bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error { - query.Result = existingPermissions - return nil - }) - - Convey("When trying to update dashboard permissions with everyone with viewer role can admin permission should be allowed", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 3, Role: &r, Permission: m.PERMISSION_ADMIN}, - } - ok, _ := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(ok, ShouldBeTrue) - }) - - Convey("When trying to update dashboard permissions with everyone with viewer role can edit permission should be allowed", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 3, Role: &r, Permission: m.PERMISSION_EDIT}, - } - ok, _ := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(ok, ShouldBeTrue) - }) - - Convey("When trying to update dashboard permissions with everyone with viewer role can view permission should return error", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 3, Role: &r, Permission: m.PERMISSION_VIEW}, - } - _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(err, ShouldEqual, ErrGuardianOverride) - }) - }) - }) + permissionScenario(fmt.Sprintf("and %s has permission to %s dashboard", pt.String(), permission.String()), dashboardID, sc, existingPermissions, func(sc *scenarioContext) { + sc.expectedFlags = flag + sc.verifyExpectedPermissionsFlags() + sc.verifyDuplicatePermissionsShouldNotBeAllowed() + sc.verifyUpdateDashboardPermissionsShouldBeAllowed(pt) + sc.verifyUpdateDashboardPermissionsShouldNotBeAllowed(pt) + }) +} - orgRoleScenario("Given user has editor org role", m.ROLE_EDITOR, func(sc *scenarioContext) { - everyoneWithRoleScenario(m.ROLE_EDITOR, m.PERMISSION_ADMIN, sc, func(sc *scenarioContext) { - canAdmin, _ := sc.g.CanAdmin() - canEdit, _ := sc.g.CanEdit() - canSave, _ := sc.g.CanSave() - canView, _ := sc.g.CanView() - So(canAdmin, ShouldBeTrue) - So(canEdit, ShouldBeTrue) - So(canSave, ShouldBeTrue) - So(canView, ShouldBeTrue) - }) - - everyoneWithRoleScenario(m.ROLE_EDITOR, m.PERMISSION_EDIT, sc, func(sc *scenarioContext) { - canAdmin, _ := sc.g.CanAdmin() - canEdit, _ := sc.g.CanEdit() - canSave, _ := sc.g.CanSave() - canView, _ := sc.g.CanView() - So(canAdmin, ShouldBeFalse) - So(canEdit, ShouldBeTrue) - So(canSave, ShouldBeTrue) - So(canView, ShouldBeTrue) - }) - - everyoneWithRoleScenario(m.ROLE_EDITOR, m.PERMISSION_VIEW, sc, func(sc *scenarioContext) { - canAdmin, _ := sc.g.CanAdmin() - canEdit, _ := sc.g.CanEdit() - canSave, _ := sc.g.CanSave() - canView, _ := sc.g.CanView() - So(canAdmin, ShouldBeFalse) - So(canEdit, ShouldBeFalse) - So(canSave, ShouldBeFalse) - So(canView, ShouldBeTrue) - }) - - everyoneWithRoleScenario(m.ROLE_VIEWER, m.PERMISSION_ADMIN, sc, func(sc *scenarioContext) { - canAdmin, _ := sc.g.CanAdmin() - canEdit, _ := sc.g.CanEdit() - canSave, _ := sc.g.CanSave() - canView, _ := sc.g.CanView() - So(canAdmin, ShouldBeFalse) - So(canEdit, ShouldBeFalse) - So(canSave, ShouldBeFalse) - So(canView, ShouldBeFalse) - }) - - everyoneWithRoleScenario(m.ROLE_VIEWER, m.PERMISSION_EDIT, sc, func(sc *scenarioContext) { - canAdmin, _ := sc.g.CanAdmin() - canEdit, _ := sc.g.CanEdit() - canSave, _ := sc.g.CanSave() - canView, _ := sc.g.CanView() - So(canAdmin, ShouldBeFalse) - So(canEdit, ShouldBeFalse) - So(canSave, ShouldBeFalse) - So(canView, ShouldBeFalse) - }) - - everyoneWithRoleScenario(m.ROLE_VIEWER, m.PERMISSION_VIEW, sc, func(sc *scenarioContext) { - canAdmin, _ := sc.g.CanAdmin() - canEdit, _ := sc.g.CanEdit() - canSave, _ := sc.g.CanSave() - canView, _ := sc.g.CanView() - So(canAdmin, ShouldBeFalse) - So(canEdit, ShouldBeFalse) - So(canSave, ShouldBeFalse) - So(canView, ShouldBeFalse) - }) - - userWithPermissionScenario(m.PERMISSION_ADMIN, sc, func(sc *scenarioContext) { - canAdmin, _ := sc.g.CanAdmin() - canEdit, _ := sc.g.CanEdit() - canSave, _ := sc.g.CanSave() - canView, _ := sc.g.CanView() - So(canAdmin, ShouldBeTrue) - So(canEdit, ShouldBeTrue) - So(canSave, ShouldBeTrue) - So(canView, ShouldBeTrue) - }) - - userWithPermissionScenario(m.PERMISSION_EDIT, sc, func(sc *scenarioContext) { - canAdmin, _ := sc.g.CanAdmin() - canEdit, _ := sc.g.CanEdit() - canSave, _ := sc.g.CanSave() - canView, _ := sc.g.CanView() - So(canAdmin, ShouldBeFalse) - So(canEdit, ShouldBeTrue) - So(canSave, ShouldBeTrue) - So(canView, ShouldBeTrue) - }) - - userWithPermissionScenario(m.PERMISSION_VIEW, sc, func(sc *scenarioContext) { - canAdmin, _ := sc.g.CanAdmin() - canEdit, _ := sc.g.CanEdit() - canSave, _ := sc.g.CanSave() - canView, _ := sc.g.CanView() - So(canAdmin, ShouldBeFalse) - So(canEdit, ShouldBeFalse) - So(canSave, ShouldBeFalse) - So(canView, ShouldBeTrue) - }) - - teamWithPermissionScenario(m.PERMISSION_ADMIN, sc, func(sc *scenarioContext) { - canAdmin, _ := sc.g.CanAdmin() - canEdit, _ := sc.g.CanEdit() - canSave, _ := sc.g.CanSave() - canView, _ := sc.g.CanView() - So(canAdmin, ShouldBeTrue) - So(canEdit, ShouldBeTrue) - So(canSave, ShouldBeTrue) - So(canView, ShouldBeTrue) - }) - - teamWithPermissionScenario(m.PERMISSION_EDIT, sc, func(sc *scenarioContext) { - canAdmin, _ := sc.g.CanAdmin() - canEdit, _ := sc.g.CanEdit() - canSave, _ := sc.g.CanSave() - canView, _ := sc.g.CanView() - So(canAdmin, ShouldBeFalse) - So(canEdit, ShouldBeTrue) - So(canSave, ShouldBeTrue) - So(canView, ShouldBeTrue) - }) - - teamWithPermissionScenario(m.PERMISSION_VIEW, sc, func(sc *scenarioContext) { - canAdmin, _ := sc.g.CanAdmin() - canEdit, _ := sc.g.CanEdit() - canSave, _ := sc.g.CanSave() - canView, _ := sc.g.CanView() - So(canAdmin, ShouldBeFalse) - So(canEdit, ShouldBeFalse) - So(canSave, ShouldBeFalse) - So(canView, ShouldBeTrue) - }) - - Convey("When trying to update permissions should return false", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_VIEW}, - {OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_ADMIN}, - } - ok, _ := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(ok, ShouldBeFalse) - }) - }) +func (sc *scenarioContext) parentFolderPermissionScenario(pt permissionType, permission m.PermissionType, flag permissionFlags) { + _, callerFile, callerLine, _ := runtime.Caller(1) + sc.callerFile = callerFile + sc.callerLine = callerLine + var folderPermissionList []*m.DashboardAclInfoDTO + + switch pt { + case USER: + folderPermissionList = []*m.DashboardAclInfoDTO{{OrgId: orgID, DashboardId: parentFolderID, UserId: userID, Permission: permission}} + case TEAM: + folderPermissionList = []*m.DashboardAclInfoDTO{{OrgId: orgID, DashboardId: parentFolderID, TeamId: teamID, Permission: permission}} + case EDITOR: + folderPermissionList = []*m.DashboardAclInfoDTO{{OrgId: orgID, DashboardId: parentFolderID, Role: &editorRole, Permission: permission}} + case VIEWER: + folderPermissionList = []*m.DashboardAclInfoDTO{{OrgId: orgID, DashboardId: parentFolderID, Role: &viewerRole, Permission: permission}} + } - orgRoleScenario("Given user has viewer org role", m.ROLE_VIEWER, func(sc *scenarioContext) { - everyoneWithRoleScenario(m.ROLE_EDITOR, m.PERMISSION_ADMIN, sc, func(sc *scenarioContext) { - canAdmin, _ := sc.g.CanAdmin() - canEdit, _ := sc.g.CanEdit() - canSave, _ := sc.g.CanSave() - canView, _ := sc.g.CanView() - So(canAdmin, ShouldBeFalse) - So(canEdit, ShouldBeFalse) - So(canSave, ShouldBeFalse) - So(canView, ShouldBeFalse) - }) - - everyoneWithRoleScenario(m.ROLE_EDITOR, m.PERMISSION_EDIT, sc, func(sc *scenarioContext) { - canAdmin, _ := sc.g.CanAdmin() - canEdit, _ := sc.g.CanEdit() - canSave, _ := sc.g.CanSave() - canView, _ := sc.g.CanView() - So(canAdmin, ShouldBeFalse) - So(canEdit, ShouldBeFalse) - So(canSave, ShouldBeFalse) - So(canView, ShouldBeFalse) - }) - - everyoneWithRoleScenario(m.ROLE_EDITOR, m.PERMISSION_VIEW, sc, func(sc *scenarioContext) { - canAdmin, _ := sc.g.CanAdmin() - canEdit, _ := sc.g.CanEdit() - canSave, _ := sc.g.CanSave() - canView, _ := sc.g.CanView() - So(canAdmin, ShouldBeFalse) - So(canEdit, ShouldBeFalse) - So(canSave, ShouldBeFalse) - So(canView, ShouldBeFalse) - }) - - everyoneWithRoleScenario(m.ROLE_VIEWER, m.PERMISSION_ADMIN, sc, func(sc *scenarioContext) { - canAdmin, _ := sc.g.CanAdmin() - canEdit, _ := sc.g.CanEdit() - canSave, _ := sc.g.CanSave() - canView, _ := sc.g.CanView() - So(canAdmin, ShouldBeTrue) - So(canEdit, ShouldBeTrue) - So(canSave, ShouldBeTrue) - So(canView, ShouldBeTrue) - }) - - everyoneWithRoleScenario(m.ROLE_VIEWER, m.PERMISSION_EDIT, sc, func(sc *scenarioContext) { - canAdmin, _ := sc.g.CanAdmin() - canEdit, _ := sc.g.CanEdit() - canSave, _ := sc.g.CanSave() - canView, _ := sc.g.CanView() - So(canAdmin, ShouldBeFalse) - So(canEdit, ShouldBeTrue) - So(canSave, ShouldBeTrue) - So(canView, ShouldBeTrue) - }) - - everyoneWithRoleScenario(m.ROLE_VIEWER, m.PERMISSION_VIEW, sc, func(sc *scenarioContext) { - canAdmin, _ := sc.g.CanAdmin() - canEdit, _ := sc.g.CanEdit() - canSave, _ := sc.g.CanSave() - canView, _ := sc.g.CanView() - So(canAdmin, ShouldBeFalse) - So(canEdit, ShouldBeFalse) - So(canSave, ShouldBeFalse) - So(canView, ShouldBeTrue) - }) - - userWithPermissionScenario(m.PERMISSION_ADMIN, sc, func(sc *scenarioContext) { - canAdmin, _ := sc.g.CanAdmin() - canEdit, _ := sc.g.CanEdit() - canSave, _ := sc.g.CanSave() - canView, _ := sc.g.CanView() - So(canAdmin, ShouldBeTrue) - So(canEdit, ShouldBeTrue) - So(canSave, ShouldBeTrue) - So(canView, ShouldBeTrue) - }) - - userWithPermissionScenario(m.PERMISSION_EDIT, sc, func(sc *scenarioContext) { - canAdmin, _ := sc.g.CanAdmin() - canEdit, _ := sc.g.CanEdit() - canSave, _ := sc.g.CanSave() - canView, _ := sc.g.CanView() - So(canAdmin, ShouldBeFalse) - So(canEdit, ShouldBeTrue) - So(canSave, ShouldBeTrue) - So(canView, ShouldBeTrue) - }) - - userWithPermissionScenario(m.PERMISSION_VIEW, sc, func(sc *scenarioContext) { - canAdmin, _ := sc.g.CanAdmin() - canEdit, _ := sc.g.CanEdit() - canSave, _ := sc.g.CanSave() - canView, _ := sc.g.CanView() - So(canAdmin, ShouldBeFalse) - So(canEdit, ShouldBeFalse) - So(canSave, ShouldBeFalse) - So(canView, ShouldBeTrue) - }) - - Convey("When trying to update permissions should return false", func() { - p := []*m.DashboardAcl{ - {OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_VIEW}, - {OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_ADMIN}, - } - ok, _ := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - So(ok, ShouldBeFalse) - }) - }) + permissionScenario(fmt.Sprintf("and parent folder has %s with permission to %s", pt.String(), permission.String()), childDashboardID, sc, folderPermissionList, func(sc *scenarioContext) { + sc.expectedFlags = flag + sc.verifyExpectedPermissionsFlags() + sc.verifyDuplicatePermissionsShouldNotBeAllowed() + sc.verifyUpdateChildDashboardPermissionsShouldBeAllowed(pt, permission) + sc.verifyUpdateChildDashboardPermissionsShouldNotBeAllowed(pt, permission) + sc.verifyUpdateChildDashboardPermissionsWithOverrideShouldBeAllowed(pt, permission) + sc.verifyUpdateChildDashboardPermissionsWithOverrideShouldNotBeAllowed(pt, permission) }) } -type scenarioContext struct { - g DashboardGuardian -} +func (sc *scenarioContext) verifyExpectedPermissionsFlags() { + canAdmin, _ := sc.g.CanAdmin() + canEdit, _ := sc.g.CanEdit() + canSave, _ := sc.g.CanSave() + canView, _ := sc.g.CanView() -type scenarioFunc func(c *scenarioContext) + tc := fmt.Sprintf("should have permissions to %s", sc.expectedFlags.String()) + Convey(tc, func() { + var actualFlag permissionFlags -func orgRoleScenario(desc string, role m.RoleType, fn scenarioFunc) { - user := &m.SignedInUser{ - UserId: 1, - OrgId: 1, - OrgRole: role, - } - guard := New(1, 1, user) - sc := &scenarioContext{ - g: guard, - } + if canAdmin { + actualFlag |= CAN_ADMIN + } + + if canEdit { + actualFlag |= CAN_EDIT + } + + if canSave { + actualFlag |= CAN_SAVE + } + + if canView { + actualFlag |= CAN_VIEW + } + + if actualFlag.noAccess() { + actualFlag = NO_ACCESS + } + + if sc.expectedFlags&actualFlag != sc.expectedFlags { + sc.reportFailure(tc, sc.expectedFlags.String(), actualFlag.String()) + } - Convey(desc, func() { - fn(sc) + sc.reportSuccess() }) } -func permissionScenario(desc string, sc *scenarioContext, permissions []*m.DashboardAclInfoDTO, fn scenarioFunc) { - bus.ClearBusHandlers() +func (sc *scenarioContext) verifyDuplicatePermissionsShouldNotBeAllowed() { + if !sc.expectedFlags.canAdmin() { + return + } + + tc := "When updating dashboard permissions with duplicate permission for user should not be allowed" + Convey(tc, func() { + p := []*m.DashboardAcl{ + newDefaultUserPermission(dashboardID, m.PERMISSION_VIEW), + newDefaultUserPermission(dashboardID, m.PERMISSION_ADMIN), + } + sc.updatePermissions = p + _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error { - query.Result = permissions - return nil + if err != ErrGuardianPermissionExists { + sc.reportFailure(tc, ErrGuardianPermissionExists, err) + } + sc.reportSuccess() }) - teams := []*m.Team{} + tc = "When updating dashboard permissions with duplicate permission for team should not be allowed" + Convey(tc, func() { + p := []*m.DashboardAcl{ + newDefaultTeamPermission(dashboardID, m.PERMISSION_VIEW), + newDefaultTeamPermission(dashboardID, m.PERMISSION_ADMIN), + } + sc.updatePermissions = p + _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) - for _, p := range permissions { - if p.TeamId > 0 { - teams = append(teams, &m.Team{Id: p.TeamId}) + if err != ErrGuardianPermissionExists { + sc.reportFailure(tc, ErrGuardianPermissionExists, err) } - } + sc.reportSuccess() + }) + + tc = "When updating dashboard permissions with duplicate permission for editor role should not be allowed" + Convey(tc, func() { + p := []*m.DashboardAcl{ + newEditorRolePermission(dashboardID, m.PERMISSION_VIEW), + newEditorRolePermission(dashboardID, m.PERMISSION_ADMIN), + } + sc.updatePermissions = p + _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) + + if err != ErrGuardianPermissionExists { + sc.reportFailure(tc, ErrGuardianPermissionExists, err) + } + sc.reportSuccess() + }) - bus.AddHandler("test", func(query *m.GetTeamsByUserQuery) error { - query.Result = teams - return nil + tc = "When updating dashboard permissions with duplicate permission for viewer role should not be allowed" + Convey(tc, func() { + p := []*m.DashboardAcl{ + newViewerRolePermission(dashboardID, m.PERMISSION_VIEW), + newViewerRolePermission(dashboardID, m.PERMISSION_ADMIN), + } + sc.updatePermissions = p + _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) + + if err != ErrGuardianPermissionExists { + sc.reportFailure(tc, ErrGuardianPermissionExists, err) + } + sc.reportSuccess() }) - Convey(desc, func() { - fn(sc) + tc = "When updating dashboard permissions with duplicate permission for admin role should not be allowed" + Convey(tc, func() { + p := []*m.DashboardAcl{ + newAdminRolePermission(dashboardID, m.PERMISSION_ADMIN), + } + sc.updatePermissions = p + _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, p) + + if err != ErrGuardianPermissionExists { + sc.reportFailure(tc, ErrGuardianPermissionExists, err) + } + sc.reportSuccess() }) } -func userWithPermissionScenario(permission m.PermissionType, sc *scenarioContext, fn scenarioFunc) { - p := []*m.DashboardAclInfoDTO{ - {OrgId: 1, DashboardId: 1, UserId: 1, Permission: permission}, +func (sc *scenarioContext) verifyUpdateDashboardPermissionsShouldBeAllowed(pt permissionType) { + if !sc.expectedFlags.canAdmin() { + return + } + + for _, p := range []m.PermissionType{m.PERMISSION_ADMIN, m.PERMISSION_EDIT, m.PERMISSION_VIEW} { + tc := fmt.Sprintf("When updating dashboard permissions with %s permissions should be allowed", p.String()) + + Convey(tc, func() { + permissionList := []*m.DashboardAcl{} + switch pt { + case USER: + permissionList = []*m.DashboardAcl{ + newEditorRolePermission(dashboardID, p), + newViewerRolePermission(dashboardID, p), + newCustomUserPermission(dashboardID, otherUserID, p), + newDefaultTeamPermission(dashboardID, p), + } + case TEAM: + permissionList = []*m.DashboardAcl{ + newEditorRolePermission(dashboardID, p), + newViewerRolePermission(dashboardID, p), + newDefaultUserPermission(dashboardID, p), + newCustomTeamPermission(dashboardID, otherTeamID, p), + } + case EDITOR, VIEWER: + permissionList = []*m.DashboardAcl{ + newEditorRolePermission(dashboardID, p), + newViewerRolePermission(dashboardID, p), + newDefaultUserPermission(dashboardID, p), + newDefaultTeamPermission(dashboardID, p), + } + } + + sc.updatePermissions = permissionList + ok, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, permissionList) + + if err != nil { + sc.reportFailure(tc, nil, err) + } + if !ok { + sc.reportFailure(tc, false, true) + } + sc.reportSuccess() + }) + } +} + +func (sc *scenarioContext) verifyUpdateDashboardPermissionsShouldNotBeAllowed(pt permissionType) { + if sc.expectedFlags.canAdmin() { + return + } + + for _, p := range []m.PermissionType{m.PERMISSION_ADMIN, m.PERMISSION_EDIT, m.PERMISSION_VIEW} { + tc := fmt.Sprintf("When updating dashboard permissions with %s permissions should NOT be allowed", p.String()) + + Convey(tc, func() { + permissionList := []*m.DashboardAcl{ + newEditorRolePermission(dashboardID, p), + newViewerRolePermission(dashboardID, p), + } + switch pt { + case USER: + permissionList = append(permissionList, []*m.DashboardAcl{ + newCustomUserPermission(dashboardID, otherUserID, p), + newDefaultTeamPermission(dashboardID, p), + }...) + case TEAM: + permissionList = append(permissionList, []*m.DashboardAcl{ + newDefaultUserPermission(dashboardID, p), + newCustomTeamPermission(dashboardID, otherTeamID, p), + }...) + } + + sc.updatePermissions = permissionList + ok, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, permissionList) + + if err != nil { + sc.reportFailure(tc, nil, err) + } + if ok { + sc.reportFailure(tc, true, false) + } + sc.reportSuccess() + }) + } +} + +func (sc *scenarioContext) verifyUpdateChildDashboardPermissionsShouldBeAllowed(pt permissionType, parentFolderPermission m.PermissionType) { + if !sc.expectedFlags.canAdmin() { + return + } + + for _, p := range []m.PermissionType{m.PERMISSION_ADMIN, m.PERMISSION_EDIT, m.PERMISSION_VIEW} { + tc := fmt.Sprintf("When updating child dashboard permissions with %s permissions should be allowed", p.String()) + + Convey(tc, func() { + permissionList := []*m.DashboardAcl{} + switch pt { + case USER: + permissionList = []*m.DashboardAcl{ + newEditorRolePermission(childDashboardID, p), + newViewerRolePermission(childDashboardID, p), + newCustomUserPermission(childDashboardID, otherUserID, p), + newDefaultTeamPermission(childDashboardID, p), + } + case TEAM: + permissionList = []*m.DashboardAcl{ + newEditorRolePermission(childDashboardID, p), + newViewerRolePermission(childDashboardID, p), + newDefaultUserPermission(childDashboardID, p), + newCustomTeamPermission(childDashboardID, otherTeamID, p), + } + case EDITOR: + permissionList = []*m.DashboardAcl{ + newViewerRolePermission(childDashboardID, p), + newDefaultUserPermission(childDashboardID, p), + newDefaultTeamPermission(childDashboardID, p), + } + + // permission to update is higher than parent folder permission + if p > parentFolderPermission { + permissionList = append(permissionList, newEditorRolePermission(childDashboardID, p)) + } + case VIEWER: + permissionList = []*m.DashboardAcl{ + newEditorRolePermission(childDashboardID, p), + newDefaultUserPermission(childDashboardID, p), + newDefaultTeamPermission(childDashboardID, p), + } + + // permission to update is higher than parent folder permission + if p > parentFolderPermission { + permissionList = append(permissionList, newViewerRolePermission(childDashboardID, p)) + } + } + + sc.updatePermissions = permissionList + ok, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, permissionList) + + if err != nil { + sc.reportFailure(tc, nil, err) + } + if !ok { + sc.reportFailure(tc, false, true) + } + sc.reportSuccess() + }) + } +} + +func (sc *scenarioContext) verifyUpdateChildDashboardPermissionsShouldNotBeAllowed(pt permissionType, parentFolderPermission m.PermissionType) { + if sc.expectedFlags.canAdmin() { + return + } + + for _, p := range []m.PermissionType{m.PERMISSION_ADMIN, m.PERMISSION_EDIT, m.PERMISSION_VIEW} { + tc := fmt.Sprintf("When updating child dashboard permissions with %s permissions should NOT be allowed", p.String()) + + Convey(tc, func() { + permissionList := []*m.DashboardAcl{} + switch pt { + case USER: + permissionList = []*m.DashboardAcl{ + newEditorRolePermission(childDashboardID, p), + newViewerRolePermission(childDashboardID, p), + newCustomUserPermission(childDashboardID, otherUserID, p), + newDefaultTeamPermission(childDashboardID, p), + } + case TEAM: + permissionList = []*m.DashboardAcl{ + newEditorRolePermission(childDashboardID, p), + newViewerRolePermission(childDashboardID, p), + newDefaultUserPermission(childDashboardID, p), + newCustomTeamPermission(childDashboardID, otherTeamID, p), + } + case EDITOR: + permissionList = []*m.DashboardAcl{ + newViewerRolePermission(childDashboardID, p), + newDefaultUserPermission(childDashboardID, p), + newDefaultTeamPermission(childDashboardID, p), + } + + // perminssion to update is higher than parent folder permission + if p > parentFolderPermission { + permissionList = append(permissionList, newEditorRolePermission(childDashboardID, p)) + } + case VIEWER: + permissionList = []*m.DashboardAcl{ + newEditorRolePermission(childDashboardID, p), + newDefaultUserPermission(childDashboardID, p), + newDefaultTeamPermission(childDashboardID, p), + } + + // perminssion to update is higher than parent folder permission + if p > parentFolderPermission { + permissionList = append(permissionList, newViewerRolePermission(childDashboardID, p)) + } + } + + sc.updatePermissions = permissionList + ok, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, permissionList) + + if err != nil { + sc.reportFailure(tc, nil, err) + } + if ok { + sc.reportFailure(tc, true, false) + } + sc.reportSuccess() + }) } - permissionScenario(fmt.Sprintf("and user has permission to %s item", permission), sc, p, fn) } -func teamWithPermissionScenario(permission m.PermissionType, sc *scenarioContext, fn scenarioFunc) { - p := []*m.DashboardAclInfoDTO{ - {OrgId: 1, DashboardId: 1, TeamId: 1, Permission: permission}, +func (sc *scenarioContext) verifyUpdateChildDashboardPermissionsWithOverrideShouldBeAllowed(pt permissionType, parentFolderPermission m.PermissionType) { + if !sc.expectedFlags.canAdmin() { + return + } + + for _, p := range []m.PermissionType{m.PERMISSION_ADMIN, m.PERMISSION_EDIT, m.PERMISSION_VIEW} { + // perminssion to update is higher tban parent folder permission + if p > parentFolderPermission { + continue + } + + tc := fmt.Sprintf("When updating child dashboard permissions overriding parent %s permission with %s permission should NOT be allowed", pt.String(), p.String()) + + Convey(tc, func() { + permissionList := []*m.DashboardAcl{} + switch pt { + case USER: + permissionList = []*m.DashboardAcl{ + newDefaultUserPermission(childDashboardID, p), + } + case TEAM: + permissionList = []*m.DashboardAcl{ + newDefaultTeamPermission(childDashboardID, p), + } + case EDITOR: + permissionList = []*m.DashboardAcl{ + newEditorRolePermission(childDashboardID, p), + } + case VIEWER: + permissionList = []*m.DashboardAcl{ + newViewerRolePermission(childDashboardID, p), + } + } + + sc.updatePermissions = permissionList + _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, permissionList) + + if err != ErrGuardianOverride { + sc.reportFailure(tc, ErrGuardianOverride, err) + } + sc.reportSuccess() + }) } - permissionScenario(fmt.Sprintf("and team has permission to %s item", permission), sc, p, fn) } -func everyoneWithRoleScenario(role m.RoleType, permission m.PermissionType, sc *scenarioContext, fn scenarioFunc) { - p := []*m.DashboardAclInfoDTO{ - {OrgId: 1, DashboardId: 1, UserId: -1, Role: &role, Permission: permission}, +func (sc *scenarioContext) verifyUpdateChildDashboardPermissionsWithOverrideShouldNotBeAllowed(pt permissionType, parentFolderPermission m.PermissionType) { + if !sc.expectedFlags.canAdmin() { + return + } + + for _, p := range []m.PermissionType{m.PERMISSION_ADMIN, m.PERMISSION_EDIT, m.PERMISSION_VIEW} { + // perminssion to update is lower than/equal parent folder permission + if p <= parentFolderPermission { + continue + } + + tc := fmt.Sprintf("When updating child dashboard permissions overriding parent %s permission with %s permission should be allowed", pt.String(), p.String()) + + Convey(tc, func() { + permissionList := []*m.DashboardAcl{} + switch pt { + case USER: + permissionList = []*m.DashboardAcl{ + newDefaultUserPermission(childDashboardID, p), + } + case TEAM: + permissionList = []*m.DashboardAcl{ + newDefaultTeamPermission(childDashboardID, p), + } + case EDITOR: + permissionList = []*m.DashboardAcl{ + newEditorRolePermission(childDashboardID, p), + } + case VIEWER: + permissionList = []*m.DashboardAcl{ + newViewerRolePermission(childDashboardID, p), + } + } + + _, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, permissionList) + sc.updatePermissions = permissionList + ok, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, permissionList) + + if err != nil { + sc.reportFailure(tc, nil, err) + } + if !ok { + sc.reportFailure(tc, false, true) + } + sc.reportSuccess() + }) } - permissionScenario(fmt.Sprintf("and everyone with %s role can %s item", role, permission), sc, p, fn) } diff --git a/pkg/services/guardian/guardian_util_test.go b/pkg/services/guardian/guardian_util_test.go new file mode 100644 index 000000000000..b065c4194ad8 --- /dev/null +++ b/pkg/services/guardian/guardian_util_test.go @@ -0,0 +1,256 @@ +package guardian + +import ( + "bytes" + "fmt" + "strings" + "testing" + + "github.com/grafana/grafana/pkg/bus" + m "github.com/grafana/grafana/pkg/models" + . "github.com/smartystreets/goconvey/convey" +) + +type scenarioContext struct { + t *testing.T + orgRoleScenario string + permissionScenario string + g DashboardGuardian + givenUser *m.SignedInUser + givenDashboardID int64 + givenPermissions []*m.DashboardAclInfoDTO + givenTeams []*m.Team + updatePermissions []*m.DashboardAcl + expectedFlags permissionFlags + callerFile string + callerLine int +} + +type scenarioFunc func(c *scenarioContext) + +func orgRoleScenario(desc string, t *testing.T, role m.RoleType, fn scenarioFunc) { + user := &m.SignedInUser{ + UserId: userID, + OrgId: orgID, + OrgRole: role, + } + guard := New(dashboardID, orgID, user) + sc := &scenarioContext{ + t: t, + orgRoleScenario: desc, + givenUser: user, + givenDashboardID: dashboardID, + g: guard, + } + + Convey(desc, func() { + fn(sc) + }) +} + +func permissionScenario(desc string, dashboardID int64, sc *scenarioContext, permissions []*m.DashboardAclInfoDTO, fn scenarioFunc) { + bus.ClearBusHandlers() + + bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error { + if query.OrgId != sc.givenUser.OrgId { + sc.reportFailure("Invalid organization id for GetDashboardAclInfoListQuery", sc.givenUser.OrgId, query.OrgId) + } + if query.DashboardId != sc.givenDashboardID { + sc.reportFailure("Invalid dashboard id for GetDashboardAclInfoListQuery", sc.givenDashboardID, query.DashboardId) + } + + query.Result = permissions + return nil + }) + + teams := []*m.Team{} + + for _, p := range permissions { + if p.TeamId > 0 { + teams = append(teams, &m.Team{Id: p.TeamId}) + } + } + + bus.AddHandler("test", func(query *m.GetTeamsByUserQuery) error { + if query.OrgId != sc.givenUser.OrgId { + sc.reportFailure("Invalid organization id for GetTeamsByUserQuery", sc.givenUser.OrgId, query.OrgId) + } + if query.UserId != sc.givenUser.UserId { + sc.reportFailure("Invalid user id for GetTeamsByUserQuery", sc.givenUser.UserId, query.UserId) + } + + query.Result = teams + return nil + }) + + sc.permissionScenario = desc + sc.g = New(dashboardID, sc.givenUser.OrgId, sc.givenUser) + sc.givenDashboardID = dashboardID + sc.givenPermissions = permissions + sc.givenTeams = teams + + Convey(desc, func() { + fn(sc) + }) +} + +type permissionType uint8 + +const ( + USER permissionType = 1 << iota + TEAM + EDITOR + VIEWER +) + +func (p permissionType) String() string { + names := map[uint8]string{ + uint8(USER): "user", + uint8(TEAM): "team", + uint8(EDITOR): "editor role", + uint8(VIEWER): "viewer role", + } + return names[uint8(p)] +} + +type permissionFlags uint8 + +const ( + NO_ACCESS permissionFlags = 1 << iota + CAN_ADMIN + CAN_EDIT + CAN_SAVE + CAN_VIEW + FULL_ACCESS = CAN_ADMIN | CAN_EDIT | CAN_SAVE | CAN_VIEW + EDITOR_ACCESS = CAN_EDIT | CAN_SAVE | CAN_VIEW + VIEWER_ACCESS = CAN_VIEW +) + +func (flag permissionFlags) canAdmin() bool { + return flag&CAN_ADMIN != 0 +} + +func (flag permissionFlags) canEdit() bool { + return flag&CAN_EDIT != 0 +} + +func (flag permissionFlags) canSave() bool { + return flag&CAN_SAVE != 0 +} + +func (flag permissionFlags) canView() bool { + return flag&CAN_VIEW != 0 +} + +func (flag permissionFlags) noAccess() bool { + return flag&(CAN_ADMIN|CAN_EDIT|CAN_SAVE|CAN_VIEW) == 0 +} + +func (f permissionFlags) String() string { + r := []string{} + + if f.canAdmin() { + r = append(r, "admin") + } + + if f.canEdit() { + r = append(r, "edit") + } + + if f.canSave() { + r = append(r, "save") + } + + if f.canView() { + r = append(r, "view") + } + + if f.noAccess() { + r = append(r, "") + } + + return strings.Join(r[:], ", ") +} + +func (sc *scenarioContext) reportSuccess() { + So(true, ShouldBeTrue) +} + +func (sc *scenarioContext) reportFailure(desc string, expected interface{}, actual interface{}) { + var buf bytes.Buffer + buf.WriteString("\n") + buf.WriteString(sc.orgRoleScenario) + buf.WriteString(" ") + buf.WriteString(sc.permissionScenario) + buf.WriteString("\n ") + buf.WriteString(desc) + buf.WriteString("\n") + buf.WriteString(fmt.Sprintf("Source test: %s:%d\n", sc.callerFile, sc.callerLine)) + buf.WriteString(fmt.Sprintf("Expected: %v\n", expected)) + buf.WriteString(fmt.Sprintf("Actual: %v\n", actual)) + buf.WriteString("Context:") + buf.WriteString(fmt.Sprintf("\n Given user: orgRole=%s, id=%d, orgId=%d", sc.givenUser.OrgRole, sc.givenUser.UserId, sc.givenUser.OrgId)) + buf.WriteString(fmt.Sprintf("\n Given dashboard id: %d", sc.givenDashboardID)) + + for i, p := range sc.givenPermissions { + r := "" + if p.Role != nil { + r = string(*p.Role) + } + buf.WriteString(fmt.Sprintf("\n Given permission (%d): dashboardId=%d, userId=%d, teamId=%d, role=%v, permission=%s", i, p.DashboardId, p.UserId, p.TeamId, r, p.Permission.String())) + } + + for i, t := range sc.givenTeams { + buf.WriteString(fmt.Sprintf("\n Given team (%d): id=%d", i, t.Id)) + } + + for i, p := range sc.updatePermissions { + r := "" + if p.Role != nil { + r = string(*p.Role) + } + buf.WriteString(fmt.Sprintf("\n Update permission (%d): dashboardId=%d, userId=%d, teamId=%d, role=%v, permission=%s", i, p.DashboardId, p.UserId, p.TeamId, r, p.Permission.String())) + } + + sc.t.Fatalf(buf.String()) +} + +func newCustomUserPermission(dashboardID int64, userID int64, permission m.PermissionType) *m.DashboardAcl { + return &m.DashboardAcl{OrgId: orgID, DashboardId: dashboardID, UserId: userID, Permission: permission} +} + +func newDefaultUserPermission(dashboardID int64, permission m.PermissionType) *m.DashboardAcl { + return newCustomUserPermission(dashboardID, userID, permission) +} + +func newCustomTeamPermission(dashboardID int64, teamID int64, permission m.PermissionType) *m.DashboardAcl { + return &m.DashboardAcl{OrgId: orgID, DashboardId: dashboardID, TeamId: teamID, Permission: permission} +} + +func newDefaultTeamPermission(dashboardID int64, permission m.PermissionType) *m.DashboardAcl { + return newCustomTeamPermission(dashboardID, teamID, permission) +} + +func newAdminRolePermission(dashboardID int64, permission m.PermissionType) *m.DashboardAcl { + return &m.DashboardAcl{OrgId: orgID, DashboardId: dashboardID, Role: &adminRole, Permission: permission} +} + +func newEditorRolePermission(dashboardID int64, permission m.PermissionType) *m.DashboardAcl { + return &m.DashboardAcl{OrgId: orgID, DashboardId: dashboardID, Role: &editorRole, Permission: permission} +} + +func newViewerRolePermission(dashboardID int64, permission m.PermissionType) *m.DashboardAcl { + return &m.DashboardAcl{OrgId: orgID, DashboardId: dashboardID, Role: &viewerRole, Permission: permission} +} + +func toDto(acl *m.DashboardAcl) *m.DashboardAclInfoDTO { + return &m.DashboardAclInfoDTO{ + OrgId: acl.OrgId, + DashboardId: acl.DashboardId, + UserId: acl.UserId, + TeamId: acl.TeamId, + Role: acl.Role, + Permission: acl.Permission, + PermissionName: acl.Permission.String(), + } +} From 77b8ccd7f542bec40d1a80bb00ca772ee04ced15 Mon Sep 17 00:00:00 2001 From: Patrick O'Carroll Date: Mon, 9 Apr 2018 17:01:58 +0200 Subject: [PATCH 11/52] removed console.log --- public/app/core/components/Permissions/PermissionsListItem.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/public/app/core/components/Permissions/PermissionsListItem.tsx b/public/app/core/components/Permissions/PermissionsListItem.tsx index f0da7b318815..229c774c639a 100644 --- a/public/app/core/components/Permissions/PermissionsListItem.tsx +++ b/public/app/core/components/Permissions/PermissionsListItem.tsx @@ -8,7 +8,6 @@ const setClassNameHelper = inherited => { }; function ItemAvatar({ item }) { - console.log(item); if (item.userAvatarUrl) { return ; } From b4863002c98ce57d060df1a01bc1edeec92e99d0 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Wed, 11 Apr 2018 00:23:50 +0200 Subject: [PATCH 12/52] permission: fix user with org viewer save/move permissions User with org viewer role should not be able to save/move dashboards in/to general folder. --- pkg/models/dashboards.go | 5 + pkg/services/dashboards/dashboard_service.go | 10 + .../dashboards/dashboard_service_test.go | 1 + .../dashboards/folder_service_test.go | 2 + pkg/services/sqlstore/dashboard.go | 13 ++ .../dashboard_service_integration_test.go | 194 +++++++++++++++--- 6 files changed, 191 insertions(+), 34 deletions(-) diff --git a/pkg/models/dashboards.go b/pkg/models/dashboards.go index 8cd2b01811c8..33d1dd64db6b 100644 --- a/pkg/models/dashboards.go +++ b/pkg/models/dashboards.go @@ -224,6 +224,10 @@ func GetFolderUrl(folderUid string, slug string) string { return fmt.Sprintf("%s/dashboards/f/%s/%s", setting.AppSubUrl, folderUid, slug) } +type ValidateDashboardBeforeSaveResult struct { + IsParentFolderChanged bool +} + // // COMMANDS // @@ -268,6 +272,7 @@ type ValidateDashboardBeforeSaveCommand struct { OrgId int64 Dashboard *Dashboard Overwrite bool + Result *ValidateDashboardBeforeSaveResult } // diff --git a/pkg/services/dashboards/dashboard_service.go b/pkg/services/dashboards/dashboard_service.go index 02a6ffc83309..1656bb74c9cb 100644 --- a/pkg/services/dashboards/dashboard_service.go +++ b/pkg/services/dashboards/dashboard_service.go @@ -103,6 +103,16 @@ func (dr *dashboardServiceImpl) buildSaveDashboardCommand(dto *SaveDashboardDTO, return nil, err } + if validateBeforeSaveCmd.Result.IsParentFolderChanged { + folderGuardian := guardian.New(dash.FolderId, dto.OrgId, dto.User) + if canSave, err := folderGuardian.CanSave(); err != nil || !canSave { + if err != nil { + return nil, err + } + return nil, models.ErrDashboardUpdateAccessDenied + } + } + guard := guardian.New(dash.GetDashboardIdForSavePermissionCheck(), dto.OrgId, dto.User) if canSave, err := guard.CanSave(); err != nil || !canSave { if err != nil { diff --git a/pkg/services/dashboards/dashboard_service_test.go b/pkg/services/dashboards/dashboard_service_test.go index 965b10655b33..d2c5863d9945 100644 --- a/pkg/services/dashboards/dashboard_service_test.go +++ b/pkg/services/dashboards/dashboard_service_test.go @@ -51,6 +51,7 @@ func TestDashboardService(t *testing.T) { }) bus.AddHandler("test", func(cmd *models.ValidateDashboardBeforeSaveCommand) error { + cmd.Result = &models.ValidateDashboardBeforeSaveResult{} return nil }) diff --git a/pkg/services/dashboards/folder_service_test.go b/pkg/services/dashboards/folder_service_test.go index 6c0413d1878b..1e678e3b1a11 100644 --- a/pkg/services/dashboards/folder_service_test.go +++ b/pkg/services/dashboards/folder_service_test.go @@ -32,6 +32,7 @@ func TestFolderService(t *testing.T) { }) bus.AddHandler("test", func(cmd *models.ValidateDashboardBeforeSaveCommand) error { + cmd.Result = &models.ValidateDashboardBeforeSaveResult{} return models.ErrDashboardUpdateAccessDenied }) @@ -92,6 +93,7 @@ func TestFolderService(t *testing.T) { }) bus.AddHandler("test", func(cmd *models.ValidateDashboardBeforeSaveCommand) error { + cmd.Result = &models.ValidateDashboardBeforeSaveResult{} return nil }) diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index 8a89c3d942c2..beda4b150ca8 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -544,6 +544,10 @@ func getExistingDashboardByIdOrUidForUpdate(sess *DBSession, cmd *m.ValidateDash dash.SetId(existingByUid.Id) dash.SetUid(existingByUid.Uid) existing = existingByUid + + if !dash.IsFolder { + cmd.Result.IsParentFolderChanged = true + } } if (existing.IsFolder && !dash.IsFolder) || @@ -551,6 +555,10 @@ func getExistingDashboardByIdOrUidForUpdate(sess *DBSession, cmd *m.ValidateDash return m.ErrDashboardTypeMismatch } + if !dash.IsFolder && dash.FolderId != existing.FolderId { + cmd.Result.IsParentFolderChanged = true + } + // check for is someone else has written in between if dash.Version != existing.Version { if cmd.Overwrite { @@ -586,6 +594,10 @@ func getExistingDashboardByTitleAndFolder(sess *DBSession, cmd *m.ValidateDashbo return m.ErrDashboardFolderWithSameNameAsDashboard } + if !dash.IsFolder && (dash.FolderId != existing.FolderId || dash.Id == 0) { + cmd.Result.IsParentFolderChanged = true + } + if cmd.Overwrite { dash.SetId(existing.Id) dash.SetUid(existing.Uid) @@ -599,6 +611,7 @@ func getExistingDashboardByTitleAndFolder(sess *DBSession, cmd *m.ValidateDashbo } func ValidateDashboardBeforeSave(cmd *m.ValidateDashboardBeforeSaveCommand) (err error) { + cmd.Result = &m.ValidateDashboardBeforeSaveResult{} return inTransaction(func(sess *DBSession) error { if err = getExistingDashboardByIdOrUidForUpdate(sess, cmd); err != nil { return err diff --git a/pkg/services/sqlstore/dashboard_service_integration_test.go b/pkg/services/sqlstore/dashboard_service_integration_test.go index d005270c33cc..21ebc6505bb5 100644 --- a/pkg/services/sqlstore/dashboard_service_integration_test.go +++ b/pkg/services/sqlstore/dashboard_service_integration_test.go @@ -74,7 +74,7 @@ func TestIntegratedDashboardService(t *testing.T) { Convey("Given organization B", func() { var otherOrgId int64 = 2 - Convey("When saving a dashboard with id that are saved in organization A", func() { + Convey("When creating a dashboard with same id as dashboard in organization A", func() { cmd := models.SaveDashboardCommand{ OrgId: otherOrgId, Dashboard: simplejson.NewFromAny(map[string]interface{}{ @@ -93,7 +93,7 @@ func TestIntegratedDashboardService(t *testing.T) { }) permissionScenario("Given user has permission to save", true, func(sc *dashboardPermissionScenarioContext) { - Convey("When saving a dashboard with uid that are saved in organization A", func() { + Convey("When creating a dashboard with same uid as dashboard in organization A", func() { var otherOrgId int64 = 2 cmd := models.SaveDashboardCommand{ OrgId: otherOrgId, @@ -106,7 +106,7 @@ func TestIntegratedDashboardService(t *testing.T) { res := callSaveWithResult(cmd) - Convey("It should create dashboard in other organization", func() { + Convey("It should create a new dashboard in organization B", func() { So(res, ShouldNotBeNil) query := models.GetDashboardQuery{OrgId: otherOrgId, Uid: savedDashInFolder.Uid} @@ -126,7 +126,7 @@ func TestIntegratedDashboardService(t *testing.T) { permissionScenario("Given user has no permission to save", false, func(sc *dashboardPermissionScenarioContext) { - Convey("When trying to create a new dashboard in the General folder", func() { + Convey("When creating a new dashboard in the General folder", func() { cmd := models.SaveDashboardCommand{ OrgId: testOrgId, Dashboard: simplejson.NewFromAny(map[string]interface{}{ @@ -138,7 +138,7 @@ func TestIntegratedDashboardService(t *testing.T) { err := callSaveWithError(cmd) - Convey("It should call dashboard guardian with correct arguments and result in access denied error", func() { + Convey("It should create dashboard guardian for General Folder with correct arguments and result in access denied error", func() { So(err, ShouldNotBeNil) So(err, ShouldEqual, models.ErrDashboardUpdateAccessDenied) @@ -148,7 +148,7 @@ func TestIntegratedDashboardService(t *testing.T) { }) }) - Convey("When trying to create a new dashboard in other folder", func() { + Convey("When creating a new dashboard in other folder", func() { cmd := models.SaveDashboardCommand{ OrgId: testOrgId, Dashboard: simplejson.NewFromAny(map[string]interface{}{ @@ -161,7 +161,7 @@ func TestIntegratedDashboardService(t *testing.T) { err := callSaveWithError(cmd) - Convey("It should call dashboard guardian with correct arguments and rsult in access denied error", func() { + Convey("It should create dashboard guardian for other folder with correct arguments and rsult in access denied error", func() { So(err, ShouldNotBeNil) So(err, ShouldEqual, models.ErrDashboardUpdateAccessDenied) @@ -171,7 +171,54 @@ func TestIntegratedDashboardService(t *testing.T) { }) }) - Convey("When trying to update a dashboard by existing id in the General folder", func() { + Convey("When creating a new dashboard by existing title in folder", func() { + cmd := models.SaveDashboardCommand{ + OrgId: testOrgId, + Dashboard: simplejson.NewFromAny(map[string]interface{}{ + "title": savedDashInFolder.Title, + }), + FolderId: savedFolder.Id, + UserId: 10000, + Overwrite: true, + } + + err := callSaveWithError(cmd) + + Convey("It should create dashboard guardian for folder with correct arguments and result in access denied error", func() { + So(err, ShouldNotBeNil) + So(err, ShouldEqual, models.ErrDashboardUpdateAccessDenied) + + So(sc.dashboardGuardianMock.DashId, ShouldEqual, savedFolder.Id) + So(sc.dashboardGuardianMock.OrgId, ShouldEqual, cmd.OrgId) + So(sc.dashboardGuardianMock.User.UserId, ShouldEqual, cmd.UserId) + }) + }) + + Convey("When creating a new dashboard by existing uid in folder", func() { + cmd := models.SaveDashboardCommand{ + OrgId: testOrgId, + Dashboard: simplejson.NewFromAny(map[string]interface{}{ + "uid": savedDashInFolder.Uid, + "title": "New dash", + }), + FolderId: savedFolder.Id, + UserId: 10000, + Overwrite: true, + } + + err := callSaveWithError(cmd) + + Convey("It should create dashboard guardian for folder with correct arguments and result in access denied error", func() { + So(err, ShouldNotBeNil) + So(err, ShouldEqual, models.ErrDashboardUpdateAccessDenied) + + So(sc.dashboardGuardianMock.DashId, ShouldEqual, savedFolder.Id) + So(sc.dashboardGuardianMock.OrgId, ShouldEqual, cmd.OrgId) + So(sc.dashboardGuardianMock.User.UserId, ShouldEqual, cmd.UserId) + }) + }) + + Convey("When updating a dashboard by existing id in the General folder", func() { cmd := models.SaveDashboardCommand{ OrgId: testOrgId, Dashboard: simplejson.NewFromAny(map[string]interface{}{ @@ -185,7 +232,7 @@ func TestIntegratedDashboardService(t *testing.T) { err := callSaveWithError(cmd) - Convey("It should call dashboard guardian with correct arguments and result in access denied error", func() { + Convey("It should create dashboard guardian for dashboard with correct arguments and result in access denied error", func() { So(err, ShouldNotBeNil) So(err, ShouldEqual, models.ErrDashboardUpdateAccessDenied) @@ -195,7 +242,7 @@ func TestIntegratedDashboardService(t *testing.T) { }) }) - Convey("When trying to update a dashboard by existing id in other folder", func() { + Convey("When updating a dashboard by existing id in other folder", func() { cmd := models.SaveDashboardCommand{ OrgId: testOrgId, Dashboard: simplejson.NewFromAny(map[string]interface{}{ @@ -209,7 +256,7 @@ func TestIntegratedDashboardService(t *testing.T) { err := callSaveWithError(cmd) - Convey("It should call dashboard guardian with correct arguments and result in access denied error", func() { + Convey("It should create dashboard guardian for dashboard with correct arguments and result in access denied error", func() { So(err, ShouldNotBeNil) So(err, ShouldEqual, models.ErrDashboardUpdateAccessDenied) @@ -218,6 +265,102 @@ func TestIntegratedDashboardService(t *testing.T) { So(sc.dashboardGuardianMock.User.UserId, ShouldEqual, cmd.UserId) }) }) + + Convey("When moving a dashboard by existing id to other folder from General folder", func() { + cmd := models.SaveDashboardCommand{ + OrgId: testOrgId, + Dashboard: simplejson.NewFromAny(map[string]interface{}{ + "id": savedDashInGeneralFolder.Id, + "title": "Dash", + }), + FolderId: otherSavedFolder.Id, + UserId: 10000, + Overwrite: true, + } + + err := callSaveWithError(cmd) + + Convey("It should create dashboard guardian for other folder with correct arguments and result in access denied error", func() { + So(err, ShouldNotBeNil) + So(err, ShouldEqual, models.ErrDashboardUpdateAccessDenied) + + So(sc.dashboardGuardianMock.DashId, ShouldEqual, otherSavedFolder.Id) + So(sc.dashboardGuardianMock.OrgId, ShouldEqual, cmd.OrgId) + So(sc.dashboardGuardianMock.User.UserId, ShouldEqual, cmd.UserId) + }) + }) + + Convey("When moving a dashboard by existing id to the General folder from other folder", func() { + cmd := models.SaveDashboardCommand{ + OrgId: testOrgId, + Dashboard: simplejson.NewFromAny(map[string]interface{}{ + "id": savedDashInFolder.Id, + "title": "Dash", + }), + FolderId: 0, + UserId: 10000, + Overwrite: true, + } + + err := callSaveWithError(cmd) + + Convey("It should create dashboard guardian for General folder with correct arguments and result in access denied error", func() { + So(err, ShouldNotBeNil) + So(err, ShouldEqual, models.ErrDashboardUpdateAccessDenied) + + So(sc.dashboardGuardianMock.DashId, ShouldEqual, 0) + So(sc.dashboardGuardianMock.OrgId, ShouldEqual, cmd.OrgId) + So(sc.dashboardGuardianMock.User.UserId, ShouldEqual, cmd.UserId) + }) + }) + + Convey("When moving a dashboard by existing uid to other folder from General folder", func() { + cmd := models.SaveDashboardCommand{ + OrgId: testOrgId, + Dashboard: simplejson.NewFromAny(map[string]interface{}{ + "uid": savedDashInGeneralFolder.Uid, + "title": "Dash", + }), + FolderId: otherSavedFolder.Id, + UserId: 10000, + Overwrite: true, + } + + err := callSaveWithError(cmd) + + Convey("It should create dashboard guardian for other folder with correct arguments and result in access denied error", func() { + So(err, ShouldNotBeNil) + So(err, ShouldEqual, models.ErrDashboardUpdateAccessDenied) + + So(sc.dashboardGuardianMock.DashId, ShouldEqual, otherSavedFolder.Id) + So(sc.dashboardGuardianMock.OrgId, ShouldEqual, cmd.OrgId) + So(sc.dashboardGuardianMock.User.UserId, ShouldEqual, cmd.UserId) + }) + }) + + Convey("When moving a dashboard by existing uid to the General folder from other folder", func() { + cmd := models.SaveDashboardCommand{ + OrgId: testOrgId, + Dashboard: simplejson.NewFromAny(map[string]interface{}{ + "uid": savedDashInFolder.Uid, + "title": "Dash", + }), + FolderId: 0, + UserId: 10000, + Overwrite: true, + } + + err := callSaveWithError(cmd) + + Convey("It should create dashboard guardian for General folder with correct arguments and result in access denied error", func() { + So(err, ShouldNotBeNil) + So(err, ShouldEqual, models.ErrDashboardUpdateAccessDenied) + + So(sc.dashboardGuardianMock.DashId, ShouldEqual, 0) + So(sc.dashboardGuardianMock.OrgId, ShouldEqual, cmd.OrgId) + So(sc.dashboardGuardianMock.User.UserId, ShouldEqual, cmd.UserId) + }) + }) }) // Given user has permission to save @@ -668,7 +811,7 @@ func TestIntegratedDashboardService(t *testing.T) { }) }) - Convey("When trying to update existing folder to a dashboard using id", func() { + Convey("When updating existing folder to a dashboard using id", func() { cmd := models.SaveDashboardCommand{ OrgId: 1, Dashboard: simplejson.NewFromAny(map[string]interface{}{ @@ -687,7 +830,7 @@ func TestIntegratedDashboardService(t *testing.T) { }) }) - Convey("When trying to update existing dashboard to a folder using id", func() { + Convey("When updating existing dashboard to a folder using id", func() { cmd := models.SaveDashboardCommand{ OrgId: 1, Dashboard: simplejson.NewFromAny(map[string]interface{}{ @@ -706,7 +849,7 @@ func TestIntegratedDashboardService(t *testing.T) { }) }) - Convey("When trying to update existing folder to a dashboard using uid", func() { + Convey("When updating existing folder to a dashboard using uid", func() { cmd := models.SaveDashboardCommand{ OrgId: 1, Dashboard: simplejson.NewFromAny(map[string]interface{}{ @@ -725,7 +868,7 @@ func TestIntegratedDashboardService(t *testing.T) { }) }) - Convey("When trying to update existing dashboard to a folder using uid", func() { + Convey("When updating existing dashboard to a folder using uid", func() { cmd := models.SaveDashboardCommand{ OrgId: 1, Dashboard: simplejson.NewFromAny(map[string]interface{}{ @@ -744,7 +887,7 @@ func TestIntegratedDashboardService(t *testing.T) { }) }) - Convey("When trying to update existing folder to a dashboard using title", func() { + Convey("When updating existing folder to a dashboard using title", func() { cmd := models.SaveDashboardCommand{ OrgId: 1, Dashboard: simplejson.NewFromAny(map[string]interface{}{ @@ -762,7 +905,7 @@ func TestIntegratedDashboardService(t *testing.T) { }) }) - Convey("When trying to update existing dashboard to a folder using title", func() { + Convey("When updating existing dashboard to a folder using title", func() { cmd := models.SaveDashboardCommand{ OrgId: 1, Dashboard: simplejson.NewFromAny(map[string]interface{}{ @@ -850,23 +993,6 @@ func callSaveWithError(cmd models.SaveDashboardCommand) error { return err } -func dashboardServiceScenario(desc string, mock *guardian.FakeDashboardGuardian, fn scenarioFunc) { - Convey(desc, func() { - origNewDashboardGuardian := guardian.New - guardian.MockDashboardGuardian(mock) - - sc := &scenarioContext{ - dashboardGuardianMock: mock, - } - - defer func() { - guardian.New = origNewDashboardGuardian - }() - - fn(sc) - }) -} - func saveTestDashboard(title string, orgId int64, folderId int64) *models.Dashboard { cmd := models.SaveDashboardCommand{ OrgId: orgId, From e73479ef3389e9530446e522d0dbdfebbfcb42ce Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Wed, 11 Apr 2018 00:26:16 +0200 Subject: [PATCH 13/52] folders: fix permissions in folder picker component Only enable creating new folders from folder picker if user has org roles admin or editor. Only render General option in folder picker if user has org roles admin or editor. --- .../dashboard/folder_picker/folder_picker.ts | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/public/app/features/dashboard/folder_picker/folder_picker.ts b/public/app/features/dashboard/folder_picker/folder_picker.ts index cbf23e3ea4b8..b8ae18b14d38 100644 --- a/public/app/features/dashboard/folder_picker/folder_picker.ts +++ b/public/app/features/dashboard/folder_picker/folder_picker.ts @@ -19,9 +19,12 @@ export class FolderPickerCtrl { newFolderNameTouched: boolean; hasValidationError: boolean; validationError: any; + isEditor: boolean; /** @ngInject */ - constructor(private backendSrv, private validationSrv) { + constructor(private backendSrv, private validationSrv, private contextSrv) { + this.isEditor = this.contextSrv.isEditor; + if (!this.labelClass) { this.labelClass = 'width-7'; } @@ -38,19 +41,20 @@ export class FolderPickerCtrl { return this.backendSrv.get('api/search', params).then(result => { if ( - query === '' || - query.toLowerCase() === 'g' || - query.toLowerCase() === 'ge' || - query.toLowerCase() === 'gen' || - query.toLowerCase() === 'gene' || - query.toLowerCase() === 'gener' || - query.toLowerCase() === 'genera' || - query.toLowerCase() === 'general' + this.isEditor && + (query === '' || + query.toLowerCase() === 'g' || + query.toLowerCase() === 'ge' || + query.toLowerCase() === 'gen' || + query.toLowerCase() === 'gene' || + query.toLowerCase() === 'gener' || + query.toLowerCase() === 'genera' || + query.toLowerCase() === 'general') ) { result.unshift({ title: this.rootName, id: 0 }); } - if (this.enableCreateNew && query === '') { + if (this.isEditor && this.enableCreateNew && query === '') { result.unshift({ title: '-- New Folder --', id: -1 }); } From 229486015d91133c23083b493c5fe202884c178a Mon Sep 17 00:00:00 2001 From: Patrick O'Carroll Date: Thu, 12 Apr 2018 10:44:00 +0200 Subject: [PATCH 14/52] added styling to fontawesome icons so they have same size as the other icons --- public/sass/components/_dashboard_settings.scss | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/public/sass/components/_dashboard_settings.scss b/public/sass/components/_dashboard_settings.scss index 11d943eb13c8..6be7fab53e5a 100644 --- a/public/sass/components/_dashboard_settings.scss +++ b/public/sass/components/_dashboard_settings.scss @@ -3,6 +3,20 @@ height: 100%; display: flex; flex-direction: row; + + //fix for fontawesome icons + .fa-lock { + &::before { + font-size: 20px; + text-align: center; + padding-left: 1px; + } + } + .fa-history { + &::before { + font-size: 17px; + } + } } .dashboard-page--settings-opening { From c431875f28962355db566309798c0729f6cb4cda Mon Sep 17 00:00:00 2001 From: Patrick O'Carroll Date: Fri, 13 Apr 2018 13:50:15 +0200 Subject: [PATCH 15/52] permissions sorting fixed + icon same size as avatrs --- .../Permissions/DisabledPermissionsListItem.tsx | 2 +- .../components/Permissions/PermissionsListItem.tsx | 4 ++-- .../app/stores/PermissionsStore/PermissionsStore.ts | 12 ++++++++---- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/public/app/core/components/Permissions/DisabledPermissionsListItem.tsx b/public/app/core/components/Permissions/DisabledPermissionsListItem.tsx index 5e473c258696..5e2497d983e5 100644 --- a/public/app/core/components/Permissions/DisabledPermissionsListItem.tsx +++ b/public/app/core/components/Permissions/DisabledPermissionsListItem.tsx @@ -13,7 +13,7 @@ export default class DisabledPermissionListItem extends Component { return ( - + {item.name} diff --git a/public/app/core/components/Permissions/PermissionsListItem.tsx b/public/app/core/components/Permissions/PermissionsListItem.tsx index 229c774c639a..ee1108a69980 100644 --- a/public/app/core/components/Permissions/PermissionsListItem.tsx +++ b/public/app/core/components/Permissions/PermissionsListItem.tsx @@ -15,10 +15,10 @@ function ItemAvatar({ item }) { return ; } if (item.role === 'Editor') { - return ; + return ; } - return ; + return ; } function ItemDescription({ item }) { diff --git a/public/app/stores/PermissionsStore/PermissionsStore.ts b/public/app/stores/PermissionsStore/PermissionsStore.ts index 7761ecc13c78..833d1bdaac70 100644 --- a/public/app/stores/PermissionsStore/PermissionsStore.ts +++ b/public/app/stores/PermissionsStore/PermissionsStore.ts @@ -155,6 +155,8 @@ export const PermissionsStore = types try { yield updateItems(self, updatedItems); self.items.push(newItem); + let sortedItems = self.items.sort((a, b) => b.sortRank - a.sortRank || a.name.localeCompare(b.name)); + self.items = sortedItems; resetNewTypeInternal(); } catch {} yield Promise.resolve(); @@ -214,9 +216,11 @@ const updateItems = (self, items) => { }; const prepareServerResponse = (response, dashboardId: number, isFolder: boolean, isInRoot: boolean) => { - return response.map(item => { - return prepareItem(item, dashboardId, isFolder, isInRoot); - }); + return response + .map(item => { + return prepareItem(item, dashboardId, isFolder, isInRoot); + }) + .sort((a, b) => b.sortRank - a.sortRank || a.name.localeCompare(b.name)); }; const prepareItem = (item, dashboardId: number, isFolder: boolean, isInRoot: boolean) => { @@ -233,7 +237,7 @@ const prepareItem = (item, dashboardId: number, isFolder: boolean, isInRoot: boo item.icon = 'fa fa-fw fa-street-view'; item.name = item.role; item.sortRank = 30; - if (item.role === 'Viewer') { + if (item.role === 'Editor') { item.sortRank += 1; } } From 9ad8a77a21badc331c5467221eb63da91978c9a2 Mon Sep 17 00:00:00 2001 From: Patrick O'Carroll Date: Fri, 13 Apr 2018 14:53:36 +0200 Subject: [PATCH 16/52] ordered user orgs alphabeticaly fixes #11556 --- pkg/services/sqlstore/user.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index f42ff5fb2ed6..db7e851435cd 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -333,6 +333,7 @@ func GetUserOrgList(query *m.GetUserOrgListQuery) error { sess.Join("INNER", "org", "org_user.org_id=org.id") sess.Where("org_user.user_id=?", query.UserId) sess.Cols("org.name", "org_user.role", "org_user.org_id") + sess.OrderBy("org.name") err := sess.Find(&query.Result) return err } From 1161c7bc3e4a5baabbde9b7eaec8f444fbc80fac Mon Sep 17 00:00:00 2001 From: Sven Klemm Date: Sun, 15 Apr 2018 17:56:56 +0200 Subject: [PATCH 17/52] add postgresVersion to postgres settings --- public/app/plugins/datasource/postgres/module.ts | 8 +++++++- .../datasource/postgres/partials/config.html | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/public/app/plugins/datasource/postgres/module.ts b/public/app/plugins/datasource/postgres/module.ts index acd23318b6d7..766e7b2ec375 100644 --- a/public/app/plugins/datasource/postgres/module.ts +++ b/public/app/plugins/datasource/postgres/module.ts @@ -8,8 +8,14 @@ class PostgresConfigCtrl { /** @ngInject **/ constructor($scope) { - this.current.jsonData.sslmode = this.current.jsonData.sslmode || 'require'; + this.current.jsonData.sslmode = this.current.jsonData.sslmode || 'verify-full'; } + + /* the values are chosen to be equivalent to `select current_setting('server_version_num');` */ + postgresVersions = [ + { name: '8.0+', value: 80000 }, + { name: '8.1+', value: 80100 }, + ]; } const defaultQuery = `SELECT diff --git a/public/app/plugins/datasource/postgres/partials/config.html b/public/app/plugins/datasource/postgres/partials/config.html index 77f0dcfa4a58..51fd66d7ed64 100644 --- a/public/app/plugins/datasource/postgres/partials/config.html +++ b/public/app/plugins/datasource/postgres/partials/config.html @@ -38,6 +38,22 @@

PostgreSQL Connection

+

PostgreSQL details

+ +
+
+ + Version + + This option controls what functions are used when expanding grafana macros. + + + + + +
+
+
User Permission
From 9b61ffb48ad9544b52b35f5870bfca1db098ae47 Mon Sep 17 00:00:00 2001 From: Sven Klemm Date: Sun, 15 Apr 2018 18:38:20 +0200 Subject: [PATCH 18/52] make timefilter macro aware of pg version --- pkg/tsdb/postgres/macros.go | 12 +++++- pkg/tsdb/postgres/macros_test.go | 65 +++++++++++++++++++++++++++++--- 2 files changed, 70 insertions(+), 7 deletions(-) diff --git a/pkg/tsdb/postgres/macros.go b/pkg/tsdb/postgres/macros.go index bd0ac0cc6205..b9a7580b3ce3 100644 --- a/pkg/tsdb/postgres/macros.go +++ b/pkg/tsdb/postgres/macros.go @@ -79,11 +79,19 @@ func (m *PostgresMacroEngine) evaluateMacro(name string, args []string) (string, } return fmt.Sprintf("extract(epoch from %s) as \"time\"", args[0]), nil case "__timeFilter": - // dont use to_timestamp in this macro for redshift compatibility #9566 if len(args) == 0 { return "", fmt.Errorf("missing time column argument for macro %v", name) } - return fmt.Sprintf("extract(epoch from %s) BETWEEN %d AND %d", args[0], m.TimeRange.GetFromAsSecondsEpoch(), m.TimeRange.GetToAsSecondsEpoch()), nil + + pg_version := m.Query.DataSource.JsonData.Get("postgresVersion").MustInt(0) + if pg_version >= 80100 { + // postgres has to_timestamp(double) starting with 8.1 + return fmt.Sprintf("%s BETWEEN to_timestamp(%d) AND to_timestamp(%d)", args[0], m.TimeRange.GetFromAsSecondsEpoch(), m.TimeRange.GetToAsSecondsEpoch()), nil + } + + // dont use to_timestamp in this macro for redshift compatibility #9566 + return fmt.Sprintf("%s BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", args[0], m.TimeRange.GetFromAsSecondsEpoch(), m.TimeRange.GetToAsSecondsEpoch()), nil + case "__timeFrom": return fmt.Sprintf("to_timestamp(%d)", m.TimeRange.GetFromAsSecondsEpoch()), nil case "__timeTo": diff --git a/pkg/tsdb/postgres/macros_test.go b/pkg/tsdb/postgres/macros_test.go index f441690a4290..b4ee043a87dc 100644 --- a/pkg/tsdb/postgres/macros_test.go +++ b/pkg/tsdb/postgres/macros_test.go @@ -6,14 +6,27 @@ import ( "testing" "time" + "github.com/grafana/grafana/pkg/components/simplejson" + "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/tsdb" . "github.com/smartystreets/goconvey/convey" ) func TestMacroEngine(t *testing.T) { Convey("MacroEngine", t, func() { - engine := &PostgresMacroEngine{} - query := &tsdb.Query{} + engine := NewPostgresMacroEngine() + // datasource with no pg version specified + ds := &models.DataSource{Id: 1, Type: "postgres", JsonData: simplejson.New()} + // datasource with postgres 8.0 configured + ds_80 := &models.DataSource{Id: 2, Type: "postgres", JsonData: simplejson.New()} + ds_80.JsonData.Set("postgresVersion", 80000) + // datasource with postgres 8.1 configured + ds_81 := &models.DataSource{Id: 3, Type: "postgres", JsonData: simplejson.New()} + ds_81.JsonData.Set("postgresVersion", 80100) + + query := &tsdb.Query{RefId: "A", DataSource: ds} + query_80 := &tsdb.Query{RefId: "A", DataSource: ds_80} + query_81 := &tsdb.Query{RefId: "A", DataSource: ds_81} Convey("Given a time range between 2018-04-12 00:00 and 2018-04-12 00:05", func() { from := time.Date(2018, 4, 12, 18, 0, 0, 0, time.UTC) @@ -38,7 +51,21 @@ func TestMacroEngine(t *testing.T) { sql, err := engine.Interpolate(query, timeRange, "WHERE $__timeFilter(time_column)") So(err, ShouldBeNil) - So(sql, ShouldEqual, fmt.Sprintf("WHERE extract(epoch from time_column) BETWEEN %d AND %d", from.Unix(), to.Unix())) + So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", from.Unix(), to.Unix())) + }) + + Convey("interpolate __timeFilter function for postgres 8.0", func() { + sql, err := engine.Interpolate(query_80, timeRange, "WHERE $__timeFilter(time_column)") + So(err, ShouldBeNil) + + So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", from.Unix(), to.Unix())) + }) + + Convey("interpolate __timeFilter function for postgres 8.1", func() { + sql, err := engine.Interpolate(query_81, timeRange, "WHERE $__timeFilter(time_column)") + So(err, ShouldBeNil) + + So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN to_timestamp(%d) AND to_timestamp(%d)", from.Unix(), to.Unix())) }) Convey("interpolate __timeFrom function", func() { @@ -102,7 +129,21 @@ func TestMacroEngine(t *testing.T) { sql, err := engine.Interpolate(query, timeRange, "WHERE $__timeFilter(time_column)") So(err, ShouldBeNil) - So(sql, ShouldEqual, fmt.Sprintf("WHERE extract(epoch from time_column) BETWEEN %d AND %d", from.Unix(), to.Unix())) + So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", from.Unix(), to.Unix())) + }) + + Convey("interpolate __timeFilter function for 8.0", func() { + sql, err := engine.Interpolate(query_80, timeRange, "WHERE $__timeFilter(time_column)") + So(err, ShouldBeNil) + + So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", from.Unix(), to.Unix())) + }) + + Convey("interpolate __timeFilter function for 8.1", func() { + sql, err := engine.Interpolate(query_81, timeRange, "WHERE $__timeFilter(time_column)") + So(err, ShouldBeNil) + + So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN to_timestamp(%d) AND to_timestamp(%d)", from.Unix(), to.Unix())) }) Convey("interpolate __timeFrom function", func() { @@ -150,7 +191,21 @@ func TestMacroEngine(t *testing.T) { sql, err := engine.Interpolate(query, timeRange, "WHERE $__timeFilter(time_column)") So(err, ShouldBeNil) - So(sql, ShouldEqual, fmt.Sprintf("WHERE extract(epoch from time_column) BETWEEN %d AND %d", from.Unix(), to.Unix())) + So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", from.Unix(), to.Unix())) + }) + + Convey("interpolate __timeFilter function for 8.0", func() { + sql, err := engine.Interpolate(query_80, timeRange, "WHERE $__timeFilter(time_column)") + So(err, ShouldBeNil) + + So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", from.Unix(), to.Unix())) + }) + + Convey("interpolate __timeFilter function for 8.1", func() { + sql, err := engine.Interpolate(query_81, timeRange, "WHERE $__timeFilter(time_column)") + So(err, ShouldBeNil) + + So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN to_timestamp(%d) AND to_timestamp(%d)", from.Unix(), to.Unix())) }) Convey("interpolate __timeFrom function", func() { From 7534f0bff6e702970daa48695c9143d1124f6e5d Mon Sep 17 00:00:00 2001 From: Kim Christensen Date: Sun, 15 Apr 2018 21:37:34 +0200 Subject: [PATCH 19/52] Support deleting empty playlist --- pkg/api/playlist.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/api/playlist.go b/pkg/api/playlist.go index d2413dfbb4cc..a90b6425cb62 100644 --- a/pkg/api/playlist.go +++ b/pkg/api/playlist.go @@ -33,7 +33,7 @@ func ValidateOrgPlaylist(c *m.ReqContext) { return } - if len(items) == 0 { + if len(items) == 0 && c.Context.Req.Method != "DELETE" { c.JsonApiErr(404, "Playlist is empty", itemsErr) return } From 6d3da9a73df9f3cc74648d2913555437d9bbea1a Mon Sep 17 00:00:00 2001 From: Sven Klemm Date: Sun, 15 Apr 2018 22:14:13 +0200 Subject: [PATCH 20/52] remove postgresversion and convert unix timestamp in go --- pkg/tsdb/postgres/macros.go | 10 +-- pkg/tsdb/postgres/macros_test.go | 63 ++----------------- .../app/plugins/datasource/postgres/module.ts | 5 -- .../datasource/postgres/partials/config.html | 16 ----- 4 files changed, 5 insertions(+), 89 deletions(-) diff --git a/pkg/tsdb/postgres/macros.go b/pkg/tsdb/postgres/macros.go index b9a7580b3ce3..a13912f0e1df 100644 --- a/pkg/tsdb/postgres/macros.go +++ b/pkg/tsdb/postgres/macros.go @@ -83,15 +83,7 @@ func (m *PostgresMacroEngine) evaluateMacro(name string, args []string) (string, return "", fmt.Errorf("missing time column argument for macro %v", name) } - pg_version := m.Query.DataSource.JsonData.Get("postgresVersion").MustInt(0) - if pg_version >= 80100 { - // postgres has to_timestamp(double) starting with 8.1 - return fmt.Sprintf("%s BETWEEN to_timestamp(%d) AND to_timestamp(%d)", args[0], m.TimeRange.GetFromAsSecondsEpoch(), m.TimeRange.GetToAsSecondsEpoch()), nil - } - - // dont use to_timestamp in this macro for redshift compatibility #9566 - return fmt.Sprintf("%s BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", args[0], m.TimeRange.GetFromAsSecondsEpoch(), m.TimeRange.GetToAsSecondsEpoch()), nil - + return fmt.Sprintf("%s BETWEEN '%s' AND '%s'", args[0], m.TimeRange.MustGetFrom().UTC().Format(time.RFC3339), m.TimeRange.MustGetTo().UTC().Format(time.RFC3339)), nil case "__timeFrom": return fmt.Sprintf("to_timestamp(%d)", m.TimeRange.GetFromAsSecondsEpoch()), nil case "__timeTo": diff --git a/pkg/tsdb/postgres/macros_test.go b/pkg/tsdb/postgres/macros_test.go index b4ee043a87dc..d1bcaff796d0 100644 --- a/pkg/tsdb/postgres/macros_test.go +++ b/pkg/tsdb/postgres/macros_test.go @@ -6,8 +6,6 @@ import ( "testing" "time" - "github.com/grafana/grafana/pkg/components/simplejson" - "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/tsdb" . "github.com/smartystreets/goconvey/convey" ) @@ -15,18 +13,7 @@ import ( func TestMacroEngine(t *testing.T) { Convey("MacroEngine", t, func() { engine := NewPostgresMacroEngine() - // datasource with no pg version specified - ds := &models.DataSource{Id: 1, Type: "postgres", JsonData: simplejson.New()} - // datasource with postgres 8.0 configured - ds_80 := &models.DataSource{Id: 2, Type: "postgres", JsonData: simplejson.New()} - ds_80.JsonData.Set("postgresVersion", 80000) - // datasource with postgres 8.1 configured - ds_81 := &models.DataSource{Id: 3, Type: "postgres", JsonData: simplejson.New()} - ds_81.JsonData.Set("postgresVersion", 80100) - - query := &tsdb.Query{RefId: "A", DataSource: ds} - query_80 := &tsdb.Query{RefId: "A", DataSource: ds_80} - query_81 := &tsdb.Query{RefId: "A", DataSource: ds_81} + query := &tsdb.Query{} Convey("Given a time range between 2018-04-12 00:00 and 2018-04-12 00:05", func() { from := time.Date(2018, 4, 12, 18, 0, 0, 0, time.UTC) @@ -51,21 +38,7 @@ func TestMacroEngine(t *testing.T) { sql, err := engine.Interpolate(query, timeRange, "WHERE $__timeFilter(time_column)") So(err, ShouldBeNil) - So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", from.Unix(), to.Unix())) - }) - - Convey("interpolate __timeFilter function for postgres 8.0", func() { - sql, err := engine.Interpolate(query_80, timeRange, "WHERE $__timeFilter(time_column)") - So(err, ShouldBeNil) - - So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", from.Unix(), to.Unix())) - }) - - Convey("interpolate __timeFilter function for postgres 8.1", func() { - sql, err := engine.Interpolate(query_81, timeRange, "WHERE $__timeFilter(time_column)") - So(err, ShouldBeNil) - - So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN to_timestamp(%d) AND to_timestamp(%d)", from.Unix(), to.Unix())) + So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN '%s' AND '%s'", from.Format(time.RFC3339), to.Format(time.RFC3339))) }) Convey("interpolate __timeFrom function", func() { @@ -129,21 +102,7 @@ func TestMacroEngine(t *testing.T) { sql, err := engine.Interpolate(query, timeRange, "WHERE $__timeFilter(time_column)") So(err, ShouldBeNil) - So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", from.Unix(), to.Unix())) - }) - - Convey("interpolate __timeFilter function for 8.0", func() { - sql, err := engine.Interpolate(query_80, timeRange, "WHERE $__timeFilter(time_column)") - So(err, ShouldBeNil) - - So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", from.Unix(), to.Unix())) - }) - - Convey("interpolate __timeFilter function for 8.1", func() { - sql, err := engine.Interpolate(query_81, timeRange, "WHERE $__timeFilter(time_column)") - So(err, ShouldBeNil) - - So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN to_timestamp(%d) AND to_timestamp(%d)", from.Unix(), to.Unix())) + So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN '%s' AND '%s'", from.Format(time.RFC3339), to.Format(time.RFC3339))) }) Convey("interpolate __timeFrom function", func() { @@ -191,21 +150,7 @@ func TestMacroEngine(t *testing.T) { sql, err := engine.Interpolate(query, timeRange, "WHERE $__timeFilter(time_column)") So(err, ShouldBeNil) - So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", from.Unix(), to.Unix())) - }) - - Convey("interpolate __timeFilter function for 8.0", func() { - sql, err := engine.Interpolate(query_80, timeRange, "WHERE $__timeFilter(time_column)") - So(err, ShouldBeNil) - - So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", from.Unix(), to.Unix())) - }) - - Convey("interpolate __timeFilter function for 8.1", func() { - sql, err := engine.Interpolate(query_81, timeRange, "WHERE $__timeFilter(time_column)") - So(err, ShouldBeNil) - - So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN to_timestamp(%d) AND to_timestamp(%d)", from.Unix(), to.Unix())) + So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN '%s' AND '%s'", from.Format(time.RFC3339), to.Format(time.RFC3339))) }) Convey("interpolate __timeFrom function", func() { diff --git a/public/app/plugins/datasource/postgres/module.ts b/public/app/plugins/datasource/postgres/module.ts index 766e7b2ec375..9deb79091676 100644 --- a/public/app/plugins/datasource/postgres/module.ts +++ b/public/app/plugins/datasource/postgres/module.ts @@ -11,11 +11,6 @@ class PostgresConfigCtrl { this.current.jsonData.sslmode = this.current.jsonData.sslmode || 'verify-full'; } - /* the values are chosen to be equivalent to `select current_setting('server_version_num');` */ - postgresVersions = [ - { name: '8.0+', value: 80000 }, - { name: '8.1+', value: 80100 }, - ]; } const defaultQuery = `SELECT diff --git a/public/app/plugins/datasource/postgres/partials/config.html b/public/app/plugins/datasource/postgres/partials/config.html index 51fd66d7ed64..77f0dcfa4a58 100644 --- a/public/app/plugins/datasource/postgres/partials/config.html +++ b/public/app/plugins/datasource/postgres/partials/config.html @@ -38,22 +38,6 @@

PostgreSQL Connection

-

PostgreSQL details

- -
-
- - Version - - This option controls what functions are used when expanding grafana macros. - - - - - -
-
-
User Permission
From 6b4ef7f5981811651e010bce19346b2500f78a05 Mon Sep 17 00:00:00 2001 From: Patrick O'Carroll Date: Mon, 16 Apr 2018 10:42:39 +0200 Subject: [PATCH 21/52] wip: writing tests for permission sorting --- .../PermissionsStore/PermissionsStore.jest.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/public/app/stores/PermissionsStore/PermissionsStore.jest.ts b/public/app/stores/PermissionsStore/PermissionsStore.jest.ts index c3bc6016e502..2bb2f1b6a0c0 100644 --- a/public/app/stores/PermissionsStore/PermissionsStore.jest.ts +++ b/public/app/stores/PermissionsStore/PermissionsStore.jest.ts @@ -17,6 +17,22 @@ describe('PermissionsStore', () => { teamId: 1, teamName: 'MyTestTeam', }, + { + id: 5, + dashboardId: 10, + permission: 1, + permissionName: 'View', + userId: 1, + userName: 'MyTestUser', + }, + { + id: 6, + dashboardId: 10, + permission: 1, + permissionName: 'Edit', + teamId: 2, + teamName: 'MyTestTeam2', + }, ]) ); @@ -32,7 +48,10 @@ describe('PermissionsStore', () => { } ); + console.log(store); + await store.load(1, false, false); + console.log(store); }); it('should save update on permission change', async () => { From 712212d6aa36bab1881ba8697ffce5bd0ad6fb7c Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Mon, 16 Apr 2018 13:37:05 +0200 Subject: [PATCH 22/52] Show Grafana version and build in Help menu * establishes Help as the single place to look for the Grafana version * version is passed as menu sub-title to side menu * added rendering of sub-title, plus styles * sub-title was used by profile menu (its value is the login string), but was not shown; now showing this value on condition that login name is different from user name --- pkg/api/index.go | 8 +++++++- public/app/core/components/sidemenu/sidemenu.html | 5 ++++- public/sass/components/_sidemenu.scss | 8 ++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/pkg/api/index.go b/pkg/api/index.go index a1d21d1c6867..0f8b5a6fc780 100644 --- a/pkg/api/index.go +++ b/pkg/api/index.go @@ -118,9 +118,14 @@ func setIndexViewData(c *m.ReqContext) (*dtos.IndexViewData, error) { }) if c.IsSignedIn { + // Only set login if it's different from the name + var login string + if c.SignedInUser.Login != c.SignedInUser.NameOrFallback() { + login = c.SignedInUser.Login + } profileNode := &dtos.NavLink{ Text: c.SignedInUser.NameOrFallback(), - SubTitle: c.SignedInUser.Login, + SubTitle: login, Id: "profile", Img: data.User.GravatarUrl, Url: setting.AppSubUrl + "/profile", @@ -284,6 +289,7 @@ func setIndexViewData(c *m.ReqContext) (*dtos.IndexViewData, error) { data.NavTree = append(data.NavTree, &dtos.NavLink{ Text: "Help", + SubTitle: fmt.Sprintf(`Grafana version: %s+%s`, setting.BuildVersion, setting.BuildCommit), Id: "help", Url: "#", Icon: "gicon gicon-question", diff --git a/public/app/core/components/sidemenu/sidemenu.html b/public/app/core/components/sidemenu/sidemenu.html index 1b301363e62a..a9ebbe2681d4 100644 --- a/public/app/core/components/sidemenu/sidemenu.html +++ b/public/app/core/components/sidemenu/sidemenu.html @@ -70,9 +70,12 @@ {{::child.text}} +
  • + {{::item.subTitle}} +
  • {{::item.text}}
    • -
    + \ No newline at end of file diff --git a/public/sass/components/_sidemenu.scss b/public/sass/components/_sidemenu.scss index d13724840749..dde01c2ba9cc 100644 --- a/public/sass/components/_sidemenu.scss +++ b/public/sass/components/_sidemenu.scss @@ -149,6 +149,14 @@ color: #ebedf2; } +.side-menu-subtitle { + padding: 0.5rem 0.5rem 0.5rem 1rem; + font-size: $font-size-sm; + color: $text-color-weak; + border-top: 1px solid $dropdownDividerBottom; + margin-top: 0.25rem; +} + li.sidemenu-org-switcher { border-bottom: 1px solid $dropdownDividerBottom; } From ce3dcadfeffbd12b02b014ebf1314e033720fe36 Mon Sep 17 00:00:00 2001 From: Patrick O'Carroll Date: Mon, 16 Apr 2018 14:30:50 +0200 Subject: [PATCH 23/52] addeds test for sort order --- .../PermissionsStore/PermissionsStore.jest.ts | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/public/app/stores/PermissionsStore/PermissionsStore.jest.ts b/public/app/stores/PermissionsStore/PermissionsStore.jest.ts index 2bb2f1b6a0c0..0bfcadb48740 100644 --- a/public/app/stores/PermissionsStore/PermissionsStore.jest.ts +++ b/public/app/stores/PermissionsStore/PermissionsStore.jest.ts @@ -11,27 +11,27 @@ describe('PermissionsStore', () => { { id: 3, dashboardId: 1, role: 'Editor', permission: 1, permissionName: 'Edit' }, { id: 4, - dashboardId: 10, + dashboardId: 1, permission: 1, permissionName: 'View', teamId: 1, - teamName: 'MyTestTeam', + team: 'MyTestTeam', }, { id: 5, - dashboardId: 10, + dashboardId: 1, permission: 1, permissionName: 'View', userId: 1, - userName: 'MyTestUser', + userLogin: 'MyTestUser', }, { id: 6, - dashboardId: 10, + dashboardId: 1, permission: 1, permissionName: 'Edit', teamId: 2, - teamName: 'MyTestTeam2', + team: 'MyTestTeam2', }, ]) ); @@ -48,15 +48,12 @@ describe('PermissionsStore', () => { } ); - console.log(store); - await store.load(1, false, false); - console.log(store); }); it('should save update on permission change', async () => { expect(store.items[0].permission).toBe(1); - expect(store.items[0].permissionName).toBe('View'); + expect(store.items[0].permissionName).toBe('Edit'); await store.updatePermissionOnIndex(0, 2, 'Edit'); @@ -67,15 +64,20 @@ describe('PermissionsStore', () => { }); it('should save removed permissions automatically', async () => { - expect(store.items.length).toBe(3); + expect(store.items.length).toBe(5); await store.removeStoreItem(2); - expect(store.items.length).toBe(2); + expect(store.items.length).toBe(4); expect(backendSrv.post.mock.calls.length).toBe(1); expect(backendSrv.post.mock.calls[0][0]).toBe('/api/dashboards/id/1/permissions'); }); + it('should be sorted by sort rank and alphabetically', async () => { + expect(store.items[3].name).toBe('MyTestTeam2'); + expect(store.items[4].name).toBe('MyTestUser'); + }); + describe('when one inherited and one not inherited team permission are added', () => { beforeEach(async () => { const overridingItemForChildDashboard = { @@ -92,7 +94,7 @@ describe('PermissionsStore', () => { }); it('should add new overriding permission', () => { - expect(store.items.length).toBe(4); + expect(store.items.length).toBe(6); }); }); }); From 5200196092091972983c0e3dbed2c5c263e98d51 Mon Sep 17 00:00:00 2001 From: Patrick O'Carroll Date: Mon, 16 Apr 2018 14:49:47 +0200 Subject: [PATCH 24/52] added fix for test --- public/app/stores/PermissionsStore/PermissionsStore.jest.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/public/app/stores/PermissionsStore/PermissionsStore.jest.ts b/public/app/stores/PermissionsStore/PermissionsStore.jest.ts index 0bfcadb48740..24f1705367ae 100644 --- a/public/app/stores/PermissionsStore/PermissionsStore.jest.ts +++ b/public/app/stores/PermissionsStore/PermissionsStore.jest.ts @@ -11,7 +11,7 @@ describe('PermissionsStore', () => { { id: 3, dashboardId: 1, role: 'Editor', permission: 1, permissionName: 'Edit' }, { id: 4, - dashboardId: 1, + dashboardId: 10, permission: 1, permissionName: 'View', teamId: 1, @@ -53,7 +53,7 @@ describe('PermissionsStore', () => { it('should save update on permission change', async () => { expect(store.items[0].permission).toBe(1); - expect(store.items[0].permissionName).toBe('Edit'); + expect(store.items[0].permissionName).toBe('View'); await store.updatePermissionOnIndex(0, 2, 'Edit'); From 8d963e27332a6a80fba7f42a3c0726a40f1608f3 Mon Sep 17 00:00:00 2001 From: Leonard Gram Date: Mon, 16 Apr 2018 15:45:55 +0200 Subject: [PATCH 25/52] changelog: improved docker image --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 90b92efc9792..093360845707 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ * **Table**: Table plugin value mappings [#7119](https://github.com/grafana/grafana/issues/7119), thx [infernix](https://github.com/infernix) * **IE11**: IE 11 compatibility [#11165](https://github.com/grafana/grafana/issues/11165) * **Scrolling**: Better scrolling experience [#11053](https://github.com/grafana/grafana/issues/11053), [#11252](https://github.com/grafana/grafana/issues/11252), [#10836](https://github.com/grafana/grafana/issues/10836), [#11185](https://github.com/grafana/grafana/issues/11185), [#11168](https://github.com/grafana/grafana/issues/11168) +* **Docker**: Improved docker image (breaking changes regarding file ownership) [grafana-docker #141](https://github.com/grafana/grafana-docker/issues/141), thx [@Spindel](https://github.com/Spindel), [@ChristianKniep](https://github.com/ChristianKniep), [@brancz](https://github.com/brancz) and [@jangaraj](https://github.com/jangaraj) ### Minor * **OpsGenie**: Add triggered alerts as description [#11046](https://github.com/grafana/grafana/pull/11046), thx [@llamashoes](https://github.com/llamashoes) From 90ed046ce35a75b020632b6d5704c0fa475e3dec Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 16 Apr 2018 16:03:50 +0200 Subject: [PATCH 26/52] docs: elasticsearch and influxdb docs for group by time interval option (#11609) --- .../features/datasources/elasticsearch.md | 16 ++++++++++++++++ docs/sources/features/datasources/influxdb.md | 16 ++++++++++++++++ .../elasticsearch/partials/config.html | 2 +- .../plugins/datasource/influxdb/query_help.md | 5 ++--- 4 files changed, 35 insertions(+), 4 deletions(-) diff --git a/docs/sources/features/datasources/elasticsearch.md b/docs/sources/features/datasources/elasticsearch.md index db17aafd2713..7e6e281df7ed 100644 --- a/docs/sources/features/datasources/elasticsearch.md +++ b/docs/sources/features/datasources/elasticsearch.md @@ -55,6 +55,22 @@ a time pattern for the index name or a wildcard. Be sure to specify your Elasticsearch version in the version selection dropdown. This is very important as there are differences how queries are composed. Currently only 2.x and 5.x are supported. +### Min time interval +A lower limit for the auto group by time interval. Recommended to be set to write frequency, for example `1m` if your data is written every minute. +This option can also be overridden/configured in a dashboard panel under data source options. It's important to note that this value **needs** to be formated as a +number followed by a valid time identifier, e.g. `1m` (1 minute) or `30s` (30 seconds). The following time identifiers are supported: + +Identifier | Description +------------ | ------------- +`y` | year +`M` | month +`w` | week +`d` | day +`h` | hour +`m` | minute +`s` | second +`ms` | millisecond + ## Metric Query editor ![](/img/docs/elasticsearch/query_editor.png) diff --git a/docs/sources/features/datasources/influxdb.md b/docs/sources/features/datasources/influxdb.md index b49e0f9dfc6b..fccdd3cc35eb 100644 --- a/docs/sources/features/datasources/influxdb.md +++ b/docs/sources/features/datasources/influxdb.md @@ -39,6 +39,22 @@ Proxy access means that the Grafana backend will proxy all requests from the bro `grafana-server`. This means that the URL you specify needs to be accessible from the server you are running Grafana on. Proxy access mode is also more secure as the username & password will never reach the browser. +### Min time interval +A lower limit for the auto group by time interval. Recommended to be set to write frequency, for example `1m` if your data is written every minute. +This option can also be overridden/configured in a dashboard panel under data source options. It's important to note that this value **needs** to be formated as a +number followed by a valid time identifier, e.g. `1m` (1 minute) or `30s` (30 seconds). The following time identifiers are supported: + +Identifier | Description +------------ | ------------- +`y` | year +`M` | month +`w` | week +`d` | day +`h` | hour +`m` | minute +`s` | second +`ms` | millisecond + ## Query Editor {{< docs-imagebox img="/img/docs/v45/influxdb_query_still.png" class="docs-image--no-shadow" animated-gif="/img/docs/v45/influxdb_query.gif" >}} diff --git a/public/app/plugins/datasource/elasticsearch/partials/config.html b/public/app/plugins/datasource/elasticsearch/partials/config.html index da23e9ddab1c..def595186249 100644 --- a/public/app/plugins/datasource/elasticsearch/partials/config.html +++ b/public/app/plugins/datasource/elasticsearch/partials/config.html @@ -35,7 +35,7 @@

    Elasticsearch details

    - Min interval + Min time interval A lower limit for the auto group by time interval. Recommended to be set to write frequency, diff --git a/public/app/plugins/datasource/influxdb/query_help.md b/public/app/plugins/datasource/influxdb/query_help.md index 0d4fd941ca5e..4930ccbc83f5 100644 --- a/public/app/plugins/datasource/influxdb/query_help.md +++ b/public/app/plugins/datasource/influxdb/query_help.md @@ -10,7 +10,7 @@ - When stacking is enabled it is important that points align - If there are missing points for one series it can cause gaps or missing bars - You must use fill(0), and select a group by time low limit -- Use the group by time option below your queries and specify for example >10s if your metrics are written every 10 seconds +- Use the group by time option below your queries and specify for example 10s if your metrics are written every 10 seconds - This will insert zeros for series that are missing measurements and will make stacking work properly #### Group by time @@ -18,8 +18,7 @@ - Leave the group by time field empty for each query and it will be calculated based on time range and pixel width of the graph - If you use fill(0) or fill(null) set a low limit for the auto group by time interval - The low limit can only be set in the group by time option below your queries -- You set a low limit by adding a greater sign before the interval -- Example: >60s if you write metrics to InfluxDB every 60 seconds +- Example: 60s if you write metrics to InfluxDB every 60 seconds #### Documentation links: From 5a29c1728225643b3aa9018fd319214889dc8edf Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Mon, 16 Apr 2018 16:25:28 +0200 Subject: [PATCH 27/52] moved version in help menu to top --- pkg/api/index.go | 2 +- public/app/core/components/sidemenu/sidemenu.html | 6 +++--- public/sass/components/_sidemenu.scss | 9 +++++---- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/pkg/api/index.go b/pkg/api/index.go index 0f8b5a6fc780..94094706f68c 100644 --- a/pkg/api/index.go +++ b/pkg/api/index.go @@ -289,7 +289,7 @@ func setIndexViewData(c *m.ReqContext) (*dtos.IndexViewData, error) { data.NavTree = append(data.NavTree, &dtos.NavLink{ Text: "Help", - SubTitle: fmt.Sprintf(`Grafana version: %s+%s`, setting.BuildVersion, setting.BuildCommit), + SubTitle: fmt.Sprintf(`Grafana v%s (%s)`, setting.BuildVersion, setting.BuildCommit), Id: "help", Url: "#", Icon: "gicon gicon-question", diff --git a/public/app/core/components/sidemenu/sidemenu.html b/public/app/core/components/sidemenu/sidemenu.html index a9ebbe2681d4..9de61345cd04 100644 --- a/public/app/core/components/sidemenu/sidemenu.html +++ b/public/app/core/components/sidemenu/sidemenu.html @@ -54,6 +54,9 @@