Merge pull request #14565 from Budibase/fix/role-permission-update

Fixing role API validator (for saving)

packages/backend-core/src/security/roles.ts

@@ -44,7 +44,7 @@ export class Role implements RoleDoc {
   permissionId: string
   inherits?: string
   version?: string
-  permissions = {}
+  permissions: Record<string, PermissionLevel[]> = {}
 
   constructor(id: string, name: string, permissionId: string) {
     this._id = id
@@ -244,9 +244,9 @@ export async function getUserRoleHierarchy(
 // some templates/older apps will use a simple string instead of array for roles
 // convert the string to an array using the theory that write is higher than read
 export function checkForRoleResourceArray(
-  rolePerms: { [key: string]: string[] },
+  rolePerms: Record<string, PermissionLevel[]>,
   resourceId: string
-) {
+): Record<string, PermissionLevel[]> {
   if (rolePerms && !Array.isArray(rolePerms[resourceId])) {
     const permLevel = rolePerms[resourceId] as any
     rolePerms[resourceId] = [permLevel]

packages/server/src/api/controllers/permission.ts

@@ -75,7 +75,9 @@ async function updatePermissionOnRole(
   // resource from another role and then adding to the new role
   for (let role of dbRoles) {
     let updated = false
-    const rolePermissions = role.permissions ? role.permissions : {}
+    const rolePermissions: Record<string, PermissionLevel[]> = role.permissions
+      ? role.permissions
+      : {}
     // make sure its an array, also handle migrating
     if (
       !rolePermissions[resourceId] ||
@@ -83,7 +85,7 @@ async function updatePermissionOnRole(
     ) {
       rolePermissions[resourceId] =
         typeof rolePermissions[resourceId] === "string"
-          ? [rolePermissions[resourceId] as unknown as string]
+          ? [rolePermissions[resourceId] as unknown as PermissionLevel]
           : []
     }
     // handle the removal/updating the role which has this permission first

packages/server/src/api/controllers/role.ts

@@ -17,7 +17,7 @@ import {
   SaveRoleResponse,
   UserCtx,
   UserMetadata,
-  UserRoles,
+  DocumentType,
 } from "@budibase/types"
 import { sdk as sharedSdk } from "@budibase/shared-core"
 import sdk from "../../sdk"
@@ -80,17 +80,21 @@ export async function save(ctx: UserCtx<SaveRoleRequest, SaveRoleResponse>) {
     _id = dbCore.prefixRoleID(_id)
   }
 
-  let dbRole
+  let dbRole: Role | undefined
-  if (!isCreate) {
+  if (!isCreate && _id?.startsWith(DocumentType.ROLE)) {
-    dbRole = await db.get<UserRoles>(_id)
+    dbRole = await db.get<Role>(_id)
   }
   if (dbRole && dbRole.name !== name && isNewVersion) {
     ctx.throw(400, "Cannot change custom role name")
   }
 
   const role = new roles.Role(_id, name, permissionId).addInheritance(inherits)
-  if (ctx.request.body._rev) {
+  if (dbRole?.permissions && !role.permissions) {
-    role._rev = ctx.request.body._rev
+    role.permissions = dbRole.permissions
+  }
+  const foundRev = ctx.request.body._rev || dbRole?._rev
+  if (foundRev) {
+    role._rev = foundRev
   }
   const result = await db.put(role)
   if (isCreate) {

packages/server/src/api/routes/utils/validators.ts

@@ -200,7 +200,7 @@ export function webhookValidator() {
 
 export function roleValidator() {
   const permLevelArray = Object.values(permissions.PermissionLevel)
-
+  const permissionString = Joi.string().valid(...permLevelArray)
   return auth.joiValidator.body(
     Joi.object({
       _id: OPTIONAL_STRING,
@@ -213,7 +213,13 @@ export function roleValidator() {
         .valid(...Object.values(permissions.BuiltinPermissionID))
         .required(),
       permissions: Joi.object()
-        .pattern(/.*/, [Joi.string().valid(...permLevelArray)])
+        .pattern(
+          /.*/,
+          Joi.alternatives().try(
+            Joi.array().items(permissionString),
+            permissionString
+          )
+        )
         .optional(),
       inherits: OPTIONAL_STRING,
     }).unknown(true)

packages/server/src/sdk/app/permissions/index.ts

@@ -90,7 +90,7 @@ export async function getResourcePerms(
       const rolePerms = allowsExplicitPerm
         ? roles.checkForRoleResourceArray(role.permissions || {}, resourceId)
         : {}
-      if (rolePerms[resourceId]?.indexOf(level) > -1) {
+      if (rolePerms[resourceId]?.indexOf(level as PermissionLevel) > -1) {
         permissions[level] = {
           role: roles.getExternalRoleID(role._id!, role.version),
           type: PermissionSource.EXPLICIT,

packages/types/src/documents/app/role.ts

@@ -1,9 +1,10 @@
 import { Document } from "../document"
+import { PermissionLevel } from "../../sdk"
 
 export interface Role extends Document {
   permissionId: string
   inherits?: string
-  permissions: { [key: string]: string[] }
+  permissions: Record<string, PermissionLevel[]>
   version?: string
   name: string
 }

packages/types/src/documents/global/user.ts

@@ -74,9 +74,8 @@ export enum UserStatus {
   INACTIVE = "inactive",
 }
 
-export interface UserRoles {
+// specifies a map of app ID to role ID
-  [key: string]: string
+export type UserRoles = Record<string, string>
-}
 
 // UTILITY TYPES