From e89042b2e3f924181c749f80d22e52d09d7482e8 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 11 Sep 2024 18:06:05 +0100 Subject: [PATCH] Fixing some role typing issues, as well as fixing an issue with the validator not allowing the structure that roles are expected to have. --- packages/backend-core/src/security/roles.ts | 6 +++--- .../server/src/api/controllers/permission.ts | 6 ++++-- packages/server/src/api/controllers/role.ts | 16 ++++++++++------ .../server/src/api/routes/utils/validators.ts | 10 ++++++++-- packages/server/src/sdk/app/permissions/index.ts | 2 +- packages/types/src/documents/app/role.ts | 3 ++- packages/types/src/documents/global/user.ts | 5 ++--- 7 files changed, 30 insertions(+), 18 deletions(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index a64be6b319..50850219a3 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/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 = {} 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, resourceId: string -) { +): Record { if (rolePerms && !Array.isArray(rolePerms[resourceId])) { const permLevel = rolePerms[resourceId] as any rolePerms[resourceId] = [permLevel] diff --git a/packages/server/src/api/controllers/permission.ts b/packages/server/src/api/controllers/permission.ts index cdfa6d8b1c..9444dfa251 100644 --- a/packages/server/src/api/controllers/permission.ts +++ b/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 = 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 diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index 3398c8102c..605b462842 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/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) { _id = dbCore.prefixRoleID(_id) } - let dbRole - if (!isCreate) { - dbRole = await db.get(_id) + let dbRole: Role | undefined + if (!isCreate && _id?.startsWith(DocumentType.ROLE)) { + dbRole = await db.get(_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) { - role._rev = ctx.request.body._rev + if (dbRole?.permissions && !role.permissions) { + 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) { diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index 5e2a585b4a..f0192b380b 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/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) diff --git a/packages/server/src/sdk/app/permissions/index.ts b/packages/server/src/sdk/app/permissions/index.ts index 18a376aaf0..dd4085d69e 100644 --- a/packages/server/src/sdk/app/permissions/index.ts +++ b/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, diff --git a/packages/types/src/documents/app/role.ts b/packages/types/src/documents/app/role.ts index f32ba810b0..7ccfb2e7e9 100644 --- a/packages/types/src/documents/app/role.ts +++ b/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 version?: string name: string } diff --git a/packages/types/src/documents/global/user.ts b/packages/types/src/documents/global/user.ts index af5c11374d..99e01fedf0 100644 --- a/packages/types/src/documents/global/user.ts +++ b/packages/types/src/documents/global/user.ts @@ -74,9 +74,8 @@ export enum UserStatus { INACTIVE = "inactive", } -export interface UserRoles { - [key: string]: string -} +// specifies a map of app ID to role ID +export type UserRoles = Record // UTILITY TYPES