From 7689b2f5ec1bb6354742738dbf5f8b5b0f84e31d Mon Sep 17 00:00:00 2001 From: "Grot (@grafanabot)" <43478413+grafanabot@users.noreply.github.com> Date: Wed, 13 Oct 2021 23:10:40 +0100 Subject: [PATCH] Forbid creation of dashboard permissions with both a user and a team (#40104) (#40412) These permissions could be created through the API but would not show correctly in the UI. (cherry picked from commit 5af96544f34cdc4f65a60cfe5c9d445eee54678a) Co-authored-by: Jesse Weaver --- pkg/api/dashboard_permission.go | 4 +++ pkg/api/dashboard_permission_test.go | 41 ++++++++++++++++++++++++++++ pkg/models/dashboard_acl.go | 11 ++++---- 3 files changed, 51 insertions(+), 5 deletions(-) diff --git a/pkg/api/dashboard_permission.go b/pkg/api/dashboard_permission.go index 4867d68e41c..9952ed0840e 100644 --- a/pkg/api/dashboard_permission.go +++ b/pkg/api/dashboard_permission.go @@ -112,6 +112,10 @@ func (hs *HTTPServer) UpdateDashboardPermissions(c *models.ReqContext, apiCmd dt func validatePermissionsUpdate(apiCmd dtos.UpdateDashboardAclCommand) error { for _, item := range apiCmd.Items { + if item.UserID > 0 && item.TeamID > 0 { + return models.ErrPermissionsWithUserAndTeamNotAllowed + } + if (item.UserID > 0 || item.TeamID > 0) && item.Role != nil { return models.ErrPermissionsWithRoleNotAllowed } diff --git a/pkg/api/dashboard_permission_test.go b/pkg/api/dashboard_permission_test.go index 93e148a6148..91a081e4250 100644 --- a/pkg/api/dashboard_permission_test.go +++ b/pkg/api/dashboard_permission_test.go @@ -158,6 +158,47 @@ func TestDashboardPermissionAPIEndpoint(t *testing.T) { }, hs) }) + t.Run("When trying to add permissions with both a team and user", func(t *testing.T) { + origNewGuardian := guardian.New + t.Cleanup(func() { + guardian.New = origNewGuardian + }) + + guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{ + CanAdminValue: true, + CheckPermissionBeforeUpdateValue: true, + }) + + setUp := func() { + getDashboardQueryResult := models.NewDashboard("Dash") + bus.AddHandlerCtx("test", func(ctx context.Context, query *models.GetDashboardQuery) error { + query.Result = getDashboardQueryResult + return nil + }) + } + + cmd := dtos.UpdateDashboardAclCommand{ + Items: []dtos.DashboardAclUpdateItem{ + {UserID: 1000, TeamID: 1, Permission: models.PERMISSION_ADMIN}, + }, + } + + updateDashboardPermissionScenario(t, updatePermissionContext{ + desc: "When calling POST on", + url: "/api/dashboards/id/1/permissions", + routePattern: "/api/dashboards/id/:id/permissions", + cmd: cmd, + fn: func(sc *scenarioContext) { + setUp() + callUpdateDashboardPermissions(t, sc) + assert.Equal(t, 400, sc.resp.Code) + respJSON, err := jsonMap(sc.resp.Body.Bytes()) + require.NoError(t, err) + assert.Equal(t, models.ErrPermissionsWithUserAndTeamNotAllowed.Error(), respJSON["error"]) + }, + }, hs) + }) + t.Run("When trying to update permissions with duplicate permissions", func(t *testing.T) { origNewGuardian := guardian.New t.Cleanup(func() { diff --git a/pkg/models/dashboard_acl.go b/pkg/models/dashboard_acl.go index f1fb2ef17d1..1a7ee44f47a 100644 --- a/pkg/models/dashboard_acl.go +++ b/pkg/models/dashboard_acl.go @@ -24,11 +24,12 @@ func (p PermissionType) String() string { // Typed errors var ( - ErrDashboardAclInfoMissing = errors.New("user id and team id cannot both be empty for a dashboard permission") - ErrDashboardPermissionDashboardEmpty = errors.New("dashboard id must be greater than zero for a dashboard permission") - ErrFolderAclInfoMissing = errors.New("user id and team id cannot both be empty for a folder permission") - ErrFolderPermissionFolderEmpty = errors.New("folder id must be greater than zero for a folder permission") - ErrPermissionsWithRoleNotAllowed = errors.New("team and user permissions cannot have an associated role") + ErrDashboardAclInfoMissing = errors.New("user id and team id cannot both be empty for a dashboard permission") + ErrDashboardPermissionDashboardEmpty = errors.New("dashboard id must be greater than zero for a dashboard permission") + ErrFolderAclInfoMissing = errors.New("user id and team id cannot both be empty for a folder permission") + ErrFolderPermissionFolderEmpty = errors.New("folder id must be greater than zero for a folder permission") + ErrPermissionsWithRoleNotAllowed = errors.New("permissions cannot have both a user and team") + ErrPermissionsWithUserAndTeamNotAllowed = errors.New("team and user permissions cannot have an associated role") ) // Dashboard ACL model