Permissions: Validate against Team/User permission role update (#29101)

* validate against role field update

* lowercase error string

* make all msgs consistent style

* fix wording

Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>

* sayonara simple json

Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>

pkg/api/dashboard_permission.go

@@ -43,6 +43,10 @@ func GetDashboardPermissionList(c *models.ReqContext) Response {
 }
 
 func UpdateDashboardPermissions(c *models.ReqContext, apiCmd dtos.UpdateDashboardAclCommand) Response {
+	if err := validatePermissionsUpdate(apiCmd); err != nil {
+		return Error(400, err.Error(), err)
+	}
+
 	dashID := c.ParamsInt64(":dashboardId")
 
 	_, rsp := getDashboardHelper(c.OrgId, "", dashID, "")
@@ -93,3 +97,12 @@ func UpdateDashboardPermissions(c *models.ReqContext, apiCmd dtos.UpdateDashboar
 
 	return Success("Dashboard permissions updated")
 }
+
+func validatePermissionsUpdate(apiCmd dtos.UpdateDashboardAclCommand) error {
+	for _, item := range apiCmd.Items {
+		if (item.UserId > 0 || item.TeamId > 0) && item.Role != nil {
+			return models.ErrPermissionsWithRoleNotAllowed
+		}
+	}
+	return nil
+}

pkg/api/dashboard_permission_test.go

@@ -166,6 +166,33 @@ func TestDashboardPermissionAPIEndpoint(t *testing.T) {
 				})
 		})
 
+		t.Run("When trying to update team or user permissions with a role", func(t *testing.T) {
+			role := models.ROLE_EDITOR
+			cmds := []dtos.UpdateDashboardAclCommand{
+				{
+					Items: []dtos.DashboardAclUpdateItem{
+						{UserId: 1000, Permission: models.PERMISSION_ADMIN, Role: &role},
+					},
+				},
+				{
+					Items: []dtos.DashboardAclUpdateItem{
+						{TeamId: 1000, Permission: models.PERMISSION_ADMIN, Role: &role},
+					},
+				},
+			}
+
+			for _, cmd := range cmds {
+				updateDashboardPermissionScenario(t, "When calling POST on", "/api/dashboards/id/1/permissions",
+					"/api/dashboards/id/:id/permissions", cmd, func(sc *scenarioContext) {
+						callUpdateDashboardPermissions(sc)
+						assert.Equal(t, 400, sc.resp.Code)
+						respJSON, err := jsonMap(sc.resp.Body.Bytes())
+						require.NoError(t, err)
+						assert.Equal(t, models.ErrPermissionsWithRoleNotAllowed.Error(), respJSON["error"])
+					})
+			}
+		})
+
 		t.Run("When trying to override inherited permissions with lower precedence", func(t *testing.T) {
 			origNewGuardian := guardian.New
 			t.Cleanup(func() {

pkg/api/folder_permission.go

@@ -49,6 +49,10 @@ func GetFolderPermissionList(c *models.ReqContext) Response {
 }
 
 func UpdateFolderPermissions(c *models.ReqContext, apiCmd dtos.UpdateDashboardAclCommand) Response {
+	if err := validatePermissionsUpdate(apiCmd); err != nil {
+		return Error(400, err.Error(), err)
+	}
+
 	s := dashboards.NewFolderService(c.OrgId, c.SignedInUser)
 	folder, err := s.GetFolderByUID(c.Params(":uid"))
 

pkg/api/folder_permission_test.go

@@ -172,6 +172,33 @@ func TestFolderPermissionAPIEndpoint(t *testing.T) {
 		})
 	})
 
+	t.Run("When trying to update team or user permissions with a role", func(t *testing.T) {
+		role := models.ROLE_ADMIN
+		cmds := []dtos.UpdateDashboardAclCommand{
+			{
+				Items: []dtos.DashboardAclUpdateItem{
+					{UserId: 1000, Permission: models.PERMISSION_ADMIN, Role: &role},
+				},
+			},
+			{
+				Items: []dtos.DashboardAclUpdateItem{
+					{TeamId: 1000, Permission: models.PERMISSION_ADMIN, Role: &role},
+				},
+			},
+		}
+
+		for _, cmd := range cmds {
+			updateFolderPermissionScenario(t, "When calling POST on", "/api/folders/uid/permissions",
+				"/api/folders/:uid/permissions", cmd, func(sc *scenarioContext) {
+					callUpdateFolderPermissions(sc)
+					assert.Equal(t, 400, sc.resp.Code)
+					respJSON, err := jsonMap(sc.resp.Body.Bytes())
+					require.NoError(t, err)
+					assert.Equal(t, models.ErrPermissionsWithRoleNotAllowed.Error(), respJSON["error"])
+				})
+		}
+	})
+
 	t.Run("When trying to override inherited permissions with lower precedence", func(t *testing.T) {
 		origNewGuardian := guardian.New
 		origNewFolderService := dashboards.NewFolderService

pkg/api/utils.go

+package api
+
+import "encoding/json"
+
+func jsonMap(data []byte) (map[string]string, error) {
+	jsonMap := make(map[string]string)
+	err := json.Unmarshal(data, &jsonMap)
+	return jsonMap, err
+}

pkg/models/dashboard_acl.go

@@ -24,10 +24,11 @@ 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")
+	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")
 )
 
 // Dashboard ACL model

public/app/core/components/PermissionList/AddPermission.tsx

@@ -34,6 +34,7 @@ class AddPermissions extends Component<Props, NewDashboardAclItem> {
     return {
       userId: 0,
       teamId: 0,
+      role: undefined,
       type: AclTarget.Team,
       permission: PermissionLevel.View,
     };