From f64637c2c53f12f0d382b565546a3dcd911a6c81 Mon Sep 17 00:00:00 2001 From: Daniel Lee Date: Thu, 18 Jan 2018 14:30:04 +0100 Subject: [PATCH] dashfolders: stop user locking themselves out of a folder --- pkg/api/dashboard_acl.go | 16 ++++++ pkg/api/dashboard_acl_test.go | 96 ++++++++++++++++++++++++++++++- pkg/services/guardian/guardian.go | 38 ++++++++++++ 3 files changed, 148 insertions(+), 2 deletions(-) diff --git a/pkg/api/dashboard_acl.go b/pkg/api/dashboard_acl.go index 88cc74b9d1c..6eb11047723 100644 --- a/pkg/api/dashboard_acl.go +++ b/pkg/api/dashboard_acl.go @@ -51,6 +51,14 @@ func UpdateDashboardAcl(c *middleware.Context, apiCmd dtos.UpdateDashboardAclCom }) } + if okToUpdate, err := guardian.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, cmd.Items); err != nil || !okToUpdate { + if err != nil { + return ApiError(500, "Error while checking dashboard permissions", err) + } + + return ApiError(403, "Cannot remove own admin permission for a folder", nil) + } + if err := bus.Dispatch(&cmd); err != nil { if err == m.ErrDashboardAclInfoMissing || err == m.ErrDashboardPermissionDashboardEmpty { return ApiError(409, err.Error(), err) @@ -70,6 +78,14 @@ func DeleteDashboardAcl(c *middleware.Context) Response { 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) diff --git a/pkg/api/dashboard_acl_test.go b/pkg/api/dashboard_acl_test.go index e22e625dcf9..c617a70cc0d 100644 --- a/pkg/api/dashboard_acl_test.go +++ b/pkg/api/dashboard_acl_test.go @@ -1,11 +1,16 @@ package api import ( + "path/filepath" "testing" + "github.com/go-macaron/session" + "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/components/simplejson" + "github.com/grafana/grafana/pkg/middleware" m "github.com/grafana/grafana/pkg/models" + macaron "gopkg.in/macaron.v1" . "github.com/smartystreets/goconvey/convey" ) @@ -56,7 +61,7 @@ func TestDashboardAclApiEndpoint(t *testing.T) { Convey("When user is editor and has admin permission in the ACL", func() { loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/1/acl", "/api/dashboards/id/:dashboardId/acl", m.ROLE_EDITOR, func(sc *scenarioContext) { - mockResult = append(mockResult, &m.DashboardAclInfoDTO{Id: 1, OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_ADMIN}) + mockResult = append(mockResult, &m.DashboardAclInfoDTO{Id: 6, OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_ADMIN}) Convey("Should be able to access ACL", func() { sc.handlerFunc = GetDashboardAclList @@ -67,7 +72,7 @@ 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: 1, OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_ADMIN}) + 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 @@ -81,6 +86,52 @@ func TestDashboardAclApiEndpoint(t *testing.T) { }) }) + 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{ + {UserId: TestUserID, Permission: m.PERMISSION_EDIT}, + }, + } + + 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}) + + CallPostAcl(sc) + So(sc.resp.Code, ShouldEqual, 403) + }) + }) + + Convey("Should be able to update permissions", func() { + cmd := dtos.UpdateDashboardAclCommand{ + Items: []dtos.DashboardAclUpdateItem{ + {UserId: TestUserID, Permission: m.PERMISSION_ADMIN}, + {UserId: 2, Permission: m.PERMISSION_EDIT}, + }, + } + + postAclScenario("When calling POST on", "/api/dashboards/id/1/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}) + + 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"}) @@ -172,3 +223,44 @@ func transformDashboardAclsToDTOs(acls []*m.DashboardAclInfoDTO) []*m.DashboardA return dtos } + +func CallPostAcl(sc *scenarioContext) { + bus.AddHandler("test", func(cmd *m.UpdateDashboardAclCommand) error { + return nil + }) + + sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec() +} + +func postAclScenario(desc string, url string, routePattern string, role m.RoleType, cmd dtos.UpdateDashboardAclCommand, fn scenarioFunc) { + Convey(desc+" "+url, func() { + defer bus.ClearBusHandlers() + + sc := &scenarioContext{ + url: url, + } + viewsPath, _ := filepath.Abs("../../public/views") + + sc.m = macaron.New() + sc.m.Use(macaron.Renderer(macaron.RenderOptions{ + Directory: viewsPath, + Delims: macaron.Delims{Left: "[[", Right: "]]"}, + })) + + sc.m.Use(middleware.GetContextHandler()) + sc.m.Use(middleware.Sessioner(&session.Options{})) + + sc.defaultHandler = wrap(func(c *middleware.Context) Response { + sc.context = c + sc.context.UserId = TestUserID + sc.context.OrgId = TestOrgID + sc.context.OrgRole = role + + return UpdateDashboardAcl(c, cmd) + }) + + sc.m.Post(routePattern, sc.defaultHandler) + + fn(sc) + }) +} diff --git a/pkg/services/guardian/guardian.go b/pkg/services/guardian/guardian.go index 1b664c11385..dc6f32a2e26 100644 --- a/pkg/services/guardian/guardian.go +++ b/pkg/services/guardian/guardian.go @@ -55,6 +55,10 @@ func (g *DashboardGuardian) HasPermission(permission m.PermissionType) (bool, er return false, err } + return g.checkAcl(permission, acl) +} + +func (g *DashboardGuardian) checkAcl(permission m.PermissionType, acl []*m.DashboardAclInfoDTO) (bool, error) { orgRole := g.user.OrgRole teamAclItems := []*m.DashboardAclInfoDTO{} @@ -102,6 +106,40 @@ func (g *DashboardGuardian) HasPermission(permission m.PermissionType) (bool, er 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 + } + + acl := []*m.DashboardAclInfoDTO{} + + for _, p := range updatePermissions { + acl = append(acl, &m.DashboardAclInfoDTO{UserId: p.UserId, TeamId: p.TeamId, Role: p.Role, Permission: p.Permission}) + } + + return g.checkAcl(permission, acl) +} + // Returns dashboard acl func (g *DashboardGuardian) GetAcl() ([]*m.DashboardAclInfoDTO, error) { if g.acl != nil {