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/15] 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 829182a8195..a1a308d6497 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 8b712c73ece..8d4af9544d9 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 5322dfd4748..c57d15a48d5 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/15] 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 c03bf7963b8..1320663f630 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 45f121dd0d0..32b75e80cc0 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 e43e57ed5c0..d6b7e305daf 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 e80b3cad4dc..4a45c561d57 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 933487650e3..202b519207d 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 b448561494d..05795b7f2df 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 f3fd81ebbe2..42c83da8810 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 a1a308d6497..ae91d1d41f3 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 8d4af9544d9..8fbb9c0d813 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 b32a4dfed1d..40d6cf5bcb2 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 de7cdf19927..7de4c5f5701 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 bebe59f4238..fb76c3fa9d6 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 a65b7226eb6..2830733c96a 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 74769891256..92dca0220ca 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/15] 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 32b75e80cc0..d15a575a05e 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 202b519207d..0e14a3bfd71 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 ae91d1d41f3..6e7175335f3 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 db45714136e..e3f3ee56d75 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 0a0572ed86e..dbdc1682f6b 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 b215dad2391..a77235ecc30 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 3140b8fcc0c..2ab5b948440 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 a7c90d13da0..7838744c541 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 92dca0220ca..c4873cb9c01 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/15] 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 e3f3ee56d75..adc2bec3d81 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 2ab5b948440..1bec7003f1f 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 00f9b93dcfd..bfa9fbbbc5a 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/15] 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 07ccfdbbef5..cfdcebf3063 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 adc2bec3d81..5e473c25869 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 1bec7003f1f..f1d96aaf358 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 a96ff36a376..8899931255f 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/15] 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 2f85d6f55e8..342eaf556c6 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/15] 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 0d0904c99ea..d19ec848ab2 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/15] 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 cfdcebf3063..4dcd07ffb48 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 f1d96aaf358..f0da7b31881 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 8899931255f..7761ecc13c7 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 00000000000..00c60902fbc --- /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 00000000000..aec3e6b7e5b --- /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 00000000000..a6581072a17 --- /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 00000000000..85d9b7109f4 --- /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 c701cc1249e..bf66d4dc68d 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 77b8ccd7f542bec40d1a80bb00ca772ee04ced15 Mon Sep 17 00:00:00 2001 From: Patrick O'Carroll Date: Mon, 9 Apr 2018 17:01:58 +0200 Subject: [PATCH 09/15] 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 f0da7b31881..229c774c639 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 c431875f28962355db566309798c0729f6cb4cda Mon Sep 17 00:00:00 2001 From: Patrick O'Carroll Date: Fri, 13 Apr 2018 13:50:15 +0200 Subject: [PATCH 10/15] 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 5e473c25869..5e2497d983e 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 229c774c639..ee1108a6998 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 7761ecc13c7..833d1bdaac7 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 6b4ef7f5981811651e010bce19346b2500f78a05 Mon Sep 17 00:00:00 2001 From: Patrick O'Carroll Date: Mon, 16 Apr 2018 10:42:39 +0200 Subject: [PATCH 11/15] 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 c3bc6016e50..2bb2f1b6a0c 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 ce3dcadfeffbd12b02b014ebf1314e033720fe36 Mon Sep 17 00:00:00 2001 From: Patrick O'Carroll Date: Mon, 16 Apr 2018 14:30:50 +0200 Subject: [PATCH 12/15] 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 2bb2f1b6a0c..0bfcadb4874 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 13/15] 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 0bfcadb4874..24f1705367a 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 58e6b11809e6e1b3115e88e7dd4a80d423cfa956 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 16 Apr 2018 21:51:53 +0200 Subject: [PATCH 14/15] add some more sort order asserts for permissions store tests --- .../stores/PermissionsStore/PermissionsStore.jest.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/public/app/stores/PermissionsStore/PermissionsStore.jest.ts b/public/app/stores/PermissionsStore/PermissionsStore.jest.ts index 24f1705367a..f1713542be5 100644 --- a/public/app/stores/PermissionsStore/PermissionsStore.jest.ts +++ b/public/app/stores/PermissionsStore/PermissionsStore.jest.ts @@ -74,6 +74,9 @@ describe('PermissionsStore', () => { }); it('should be sorted by sort rank and alphabetically', async () => { + expect(store.items[0].name).toBe('MyTestTeam'); + expect(store.items[1].name).toBe('Editor'); + expect(store.items[2].name).toBe('Viewer'); expect(store.items[3].name).toBe('MyTestTeam2'); expect(store.items[4].name).toBe('MyTestUser'); }); @@ -96,5 +99,14 @@ describe('PermissionsStore', () => { it('should add new overriding permission', () => { expect(store.items.length).toBe(6); }); + + it('should be sorted by sort rank and alphabetically', async () => { + expect(store.items[0].name).toBe('MyTestTeam'); + expect(store.items[1].name).toBe('Editor'); + expect(store.items[2].name).toBe('Viewer'); + expect(store.items[3].name).toBe('MyTestTeam'); + expect(store.items[4].name).toBe('MyTestTeam2'); + expect(store.items[5].name).toBe('MyTestUser'); + }); }); }); From a01e6fc9c7d221385467fe4d4e2b7ccd2715b18b Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 17 Apr 2018 09:19:17 +0200 Subject: [PATCH 15/15] add some more sort order asserts for permissions store tests --- public/app/stores/PermissionsStore/PermissionsStore.jest.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/public/app/stores/PermissionsStore/PermissionsStore.jest.ts b/public/app/stores/PermissionsStore/PermissionsStore.jest.ts index f1713542be5..d6a20e25846 100644 --- a/public/app/stores/PermissionsStore/PermissionsStore.jest.ts +++ b/public/app/stores/PermissionsStore/PermissionsStore.jest.ts @@ -75,6 +75,7 @@ describe('PermissionsStore', () => { it('should be sorted by sort rank and alphabetically', async () => { expect(store.items[0].name).toBe('MyTestTeam'); + expect(store.items[0].dashboardId).toBe(10); expect(store.items[1].name).toBe('Editor'); expect(store.items[2].name).toBe('Viewer'); expect(store.items[3].name).toBe('MyTestTeam2'); @@ -102,9 +103,11 @@ describe('PermissionsStore', () => { it('should be sorted by sort rank and alphabetically', async () => { expect(store.items[0].name).toBe('MyTestTeam'); + expect(store.items[0].dashboardId).toBe(10); expect(store.items[1].name).toBe('Editor'); expect(store.items[2].name).toBe('Viewer'); expect(store.items[3].name).toBe('MyTestTeam'); + expect(store.items[3].dashboardId).toBe(1); expect(store.items[4].name).toBe('MyTestTeam2'); expect(store.items[5].name).toBe('MyTestUser'); });