Merge pull request #11001 from Budibase/feature/custom-role-readable-ids

Custom roles - readable IDs

packages/backend-core/src/docIds/ids.ts

@@ -81,8 +81,19 @@ export function generateAppUserID(prodAppId: string, userId: string) {
  * Generates a new role ID.
  * @returns {string} The new role ID which the role doc can be stored under.
  */
-export function generateRoleID(id?: any) {
-  return `${DocumentType.ROLE}${SEPARATOR}${id || newid()}`
+export function generateRoleID(name: string) {
+  const prefix = `${DocumentType.ROLE}${SEPARATOR}`
+  if (name.startsWith(prefix)) {
+    return name
+  }
+  return `${prefix}${name}`
+}
+
+/**
+ * Utility function to be more verbose.
+ */
+export function prefixRoleID(name: string) {
+  return generateRoleID(name)
 }
 
 /**

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

@@ -1,5 +1,5 @@
 import { BuiltinPermissionID, PermissionLevel } from "./permissions"
-import { generateRoleID, getRoleParams, DocumentType, SEPARATOR } from "../db"
+import { prefixRoleID, getRoleParams, DocumentType, SEPARATOR } from "../db"
 import { getAppDB } from "../context"
 import { doWithDB } from "../db"
 import { Screen, Role as RoleDoc } from "@budibase/types"
@@ -25,18 +25,28 @@ const EXTERNAL_BUILTIN_ROLE_IDS = [
   BUILTIN_IDS.PUBLIC,
 ]
 
+export const RoleIDVersion = {
+  // original version, with a UUID based ID
+  UUID: undefined,
+  // new version - with name based ID
+  NAME: "name",
+}
+
 export class Role implements RoleDoc {
   _id: string
   _rev?: string
   name: string
   permissionId: string
   inherits?: string
+  version?: string
   permissions = {}
 
   constructor(id: string, name: string, permissionId: string) {
     this._id = id
     this.name = name
     this.permissionId = permissionId
+    // version for managing the ID - removing the role_ when responding
+    this.version = RoleIDVersion.NAME
   }
 
   addInheritance(inherits: string) {
@@ -157,13 +167,16 @@ export async function getRole(
     role = cloneDeep(
       Object.values(BUILTIN_ROLES).find(role => role._id === roleId)
     )
+  } else {
+    // make sure has the prefix (if it has it then it won't be added)
+    roleId = prefixRoleID(roleId)
   }
   try {
     const db = getAppDB()
     const dbRole = await db.get(getDBRoleID(roleId))
     role = Object.assign(role, dbRole)
     // finalise the ID
-    role._id = getExternalRoleID(role._id)
+    role._id = getExternalRoleID(role._id, role.version)
   } catch (err) {
     if (!isBuiltin(roleId) && opts?.defaultPublic) {
       return cloneDeep(BUILTIN_ROLES.PUBLIC)
@@ -261,6 +274,9 @@ export async function getAllRoles(appId?: string) {
         })
       )
       roles = body.rows.map((row: any) => row.doc)
+      roles.forEach(
+        role => (role._id = getExternalRoleID(role._id!, role.version))
+      )
     }
     const builtinRoles = getBuiltinRoles()
 
@@ -268,14 +284,15 @@ export async function getAllRoles(appId?: string) {
     for (let builtinRoleId of EXTERNAL_BUILTIN_ROLE_IDS) {
       const builtinRole = builtinRoles[builtinRoleId]
       const dbBuiltin = roles.filter(
-        dbRole => getExternalRoleID(dbRole._id) === builtinRoleId
+        dbRole =>
+          getExternalRoleID(dbRole._id!, dbRole.version) === builtinRoleId
       )[0]
       if (dbBuiltin == null) {
         roles.push(builtinRole || builtinRoles.BASIC)
       } else {
         // remove role and all back after combining with the builtin
         roles = roles.filter(role => role._id !== dbBuiltin._id)
-        dbBuiltin._id = getExternalRoleID(dbBuiltin._id)
+        dbBuiltin._id = getExternalRoleID(dbBuiltin._id!, dbBuiltin.version)
         roles.push(Object.assign(builtinRole, dbBuiltin))
       }
     }
@@ -381,19 +398,22 @@ export class AccessController {
 /**
  * Adds the "role_" for builtin role IDs which are to be written to the DB (for permissions).
  */
-export function getDBRoleID(roleId?: string) {
-  if (roleId?.startsWith(DocumentType.ROLE)) {
-    return roleId
+export function getDBRoleID(roleName: string) {
+  if (roleName?.startsWith(DocumentType.ROLE)) {
+    return roleName
   }
-  return generateRoleID(roleId)
+  return prefixRoleID(roleName)
 }
 
 /**
  * Remove the "role_" from builtin role IDs that have been written to the DB (for permissions).
  */
-export function getExternalRoleID(roleId?: string) {
+export function getExternalRoleID(roleId: string, version?: string) {
   // for built-in roles we want to remove the DB role ID element (role_)
-  if (roleId?.startsWith(DocumentType.ROLE) && isBuiltin(roleId)) {
+  if (
+    (roleId.startsWith(DocumentType.ROLE) && isBuiltin(roleId)) ||
+    version === RoleIDVersion.NAME
+  ) {
     return roleId.split(`${DocumentType.ROLE}${SEPARATOR}`)[1]
   }
   return roleId

packages/builder/src/components/backend/DataTable/modals/EditRoles.svelte

@@ -12,15 +12,14 @@
   let selectedRole = BASE_ROLE
   let errors = []
   let builtInRoles = ["Admin", "Power", "Basic", "Public"]
+  let validRegex = /^[a-zA-Z0-9_]*$/
   // Don't allow editing of public role
   $: editableRoles = $roles.filter(role => role._id !== "PUBLIC")
   $: selectedRoleId = selectedRole._id
   $: otherRoles = editableRoles.filter(role => role._id !== selectedRoleId)
   $: isCreating = selectedRoleId == null || selectedRoleId === ""
 
-  $: hasUniqueRoleName = !otherRoles
-    ?.map(role => role.name)
-    ?.includes(selectedRole.name)
+  $: roleNameError = getRoleNameError(selectedRole.name)
 
   $: valid =
     selectedRole.name &&
@@ -101,6 +100,18 @@
     }
   }
 
+  const getRoleNameError = name => {
+    const hasUniqueRoleName = !otherRoles
+      ?.map(role => role.name)
+      ?.includes(name)
+    const invalidRoleName = !validRegex.test(name)
+    if (!hasUniqueRoleName) {
+      return "Select a unique role name."
+    } else if (invalidRoleName) {
+      return "Please enter a role name consisting of only alphanumeric symbols and underscores"
+    }
+  }
+
   onMount(fetchBasePermissions)
 </script>
 
@@ -108,7 +119,7 @@
   title="Edit Roles"
   confirmText={isCreating ? "Create" : "Save"}
   onConfirm={saveRole}
-  disabled={!valid || !hasUniqueRoleName}
+  disabled={!valid || roleNameError}
 >
   {#if errors.length}
     <ErrorsBox {errors} />
@@ -129,7 +140,7 @@
       label="Name"
       bind:value={selectedRole.name}
       disabled={shouldDisableRoleInput}
-      error={!hasUniqueRoleName ? "Select a unique role name." : null}
+      error={roleNameError}
     />
     <Select
       label="Inherits Role"

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

@@ -5,7 +5,7 @@ import {
   getBasePermissions,
 } from "../../utilities/security"
 import { removeFromArray } from "../../utilities"
-import { BBContext, Database, Role } from "@budibase/types"
+import { UserCtx, Database, Role } from "@budibase/types"
 
 const PermissionUpdateType = {
   REMOVE: "remove",
@@ -38,12 +38,12 @@ async function updatePermissionOnRole(
   const isABuiltin = roles.isBuiltin(roleId)
   const dbRoleId = roles.getDBRoleID(roleId)
   const dbRoles = await getAllDBRoles(db)
-  const docUpdates = []
+  const docUpdates: Role[] = []
 
   // the permission is for a built in, make sure it exists
   if (isABuiltin && !dbRoles.some(role => role._id === dbRoleId)) {
     const builtin = roles.getBuiltinRoles()[roleId]
-    builtin._id = roles.getDBRoleID(builtin._id)
+    builtin._id = roles.getDBRoleID(builtin._id!)
     dbRoles.push(builtin)
   }
 
@@ -88,22 +88,23 @@ async function updatePermissionOnRole(
 
   const response = await db.bulkDocs(docUpdates)
   return response.map((resp: any) => {
-    resp._id = roles.getExternalRoleID(resp.id)
+    const version = docUpdates.find(role => role._id === resp.id)?.version
+    resp._id = roles.getExternalRoleID(resp.id, version)
     delete resp.id
     return resp
   })
 }
 
-export function fetchBuiltin(ctx: BBContext) {
+export function fetchBuiltin(ctx: UserCtx) {
   ctx.body = Object.values(permissions.getBuiltinPermissions())
 }
 
-export function fetchLevels(ctx: BBContext) {
+export function fetchLevels(ctx: UserCtx) {
   // for now only provide the read/write perms externally
   ctx.body = SUPPORTED_LEVELS
 }
 
-export async function fetch(ctx: BBContext) {
+export async function fetch(ctx: UserCtx) {
   const db = context.getAppDB()
   const dbRoles: Role[] = await getAllDBRoles(db)
   let permissions: any = {}
@@ -112,7 +113,7 @@ export async function fetch(ctx: BBContext) {
     if (!role.permissions) {
       continue
     }
-    const roleId = roles.getExternalRoleID(role._id)
+    const roleId = roles.getExternalRoleID(role._id!, role.version)
     if (!roleId) {
       ctx.throw(400, "Unable to retrieve role")
     }
@@ -132,7 +133,7 @@ export async function fetch(ctx: BBContext) {
   ctx.body = finalPermissions
 }
 
-export async function getResourcePerms(ctx: BBContext) {
+export async function getResourcePerms(ctx: UserCtx) {
   const resourceId = ctx.params.resourceId
   const db = context.getAppDB()
   const body = await db.allDocs(
@@ -154,14 +155,14 @@ export async function getResourcePerms(ctx: BBContext) {
         rolePerms[resourceId] &&
         rolePerms[resourceId].indexOf(level) !== -1
       ) {
-        permissions[level] = roles.getExternalRoleID(role._id)!
+        permissions[level] = roles.getExternalRoleID(role._id, role.version)!
       }
     }
   }
   ctx.body = Object.assign(getBasePermissions(resourceId), permissions)
 }
 
-export async function addPermission(ctx: BBContext) {
+export async function addPermission(ctx: UserCtx) {
   ctx.body = await updatePermissionOnRole(
     ctx.appId,
     ctx.params,
@@ -169,7 +170,7 @@ export async function addPermission(ctx: BBContext) {
   )
 }
 
-export async function removePermission(ctx: BBContext) {
+export async function removePermission(ctx: UserCtx) {
   ctx.body = await updatePermissionOnRole(
     ctx.appId,
     ctx.params,

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

@@ -14,7 +14,8 @@ const UpdateRolesOptions = {
 async function updateRolesOnUserTable(
   db: Database,
   roleId: string,
-  updateOption: string
+  updateOption: string,
+  roleVersion: string | undefined
 ) {
   const table = await db.get(InternalTables.USER_METADATA)
   const schema = table.schema
@@ -24,11 +25,15 @@ async function updateRolesOnUserTable(
     if (prop === "roleId") {
       updated = true
       const constraints = schema[prop].constraints
-      const indexOf = constraints.inclusion.indexOf(roleId)
-      if (remove && indexOf !== -1) {
-        constraints.inclusion.splice(indexOf, 1)
-      } else if (!remove && indexOf === -1) {
-        constraints.inclusion.push(roleId)
+      const updatedRoleId =
+        roleVersion === roles.RoleIDVersion.NAME
+          ? roles.getExternalRoleID(roleId, roleVersion)
+          : roleId
+      const indexOfRoleId = constraints.inclusion.indexOf(updatedRoleId)
+      if (remove && indexOfRoleId !== -1) {
+        constraints.inclusion.splice(indexOfRoleId, 1)
+      } else if (!remove && indexOfRoleId === -1) {
+        constraints.inclusion.push(updatedRoleId)
       }
       break
     }
@@ -48,14 +53,23 @@ export async function find(ctx: UserCtx) {
 
 export async function save(ctx: UserCtx) {
   const db = context.getAppDB()
-  let { _id, name, inherits, permissionId } = ctx.request.body
+  let { _id, name, inherits, permissionId, version } = ctx.request.body
   let isCreate = false
-  if (!_id) {
-    _id = generateRoleID()
-    isCreate = true
-  } else if (roles.isBuiltin(_id)) {
+
+  if (_id && roles.isBuiltin(_id)) {
     ctx.throw(400, "Cannot update builtin roles.")
   }
+
+  // if not id found, then its creation
+  if (!_id) {
+    _id = generateRoleID(name)
+    isCreate = true
+  }
+  // version 2 roles need updated to add back role_
+  else if (version === roles.RoleIDVersion.NAME) {
+    _id = generateRoleID(name)
+  }
+
   const role = new roles.Role(_id, name, permissionId).addInheritance(inherits)
   if (ctx.request.body._rev) {
     role._rev = ctx.request.body._rev
@@ -66,7 +80,12 @@ export async function save(ctx: UserCtx) {
   } else {
     await events.role.updated(role)
   }
-  await updateRolesOnUserTable(db, _id, UpdateRolesOptions.CREATED)
+  await updateRolesOnUserTable(
+    db,
+    _id,
+    UpdateRolesOptions.CREATED,
+    role.version
+  )
   role._rev = result.rev
   ctx.body = role
   ctx.message = `Role '${role.name}' created successfully.`
@@ -74,11 +93,14 @@ export async function save(ctx: UserCtx) {
 
 export async function destroy(ctx: UserCtx) {
   const db = context.getAppDB()
-  const roleId = ctx.params.roleId
-  const role = await db.get(roleId)
+  let roleId = ctx.params.roleId
   if (roles.isBuiltin(roleId)) {
     ctx.throw(400, "Cannot delete builtin role.")
+  } else {
+    // make sure has the prefix (if it has it then it won't be added)
+    roleId = generateRoleID(roleId)
   }
+  const role = await db.get(roleId)
   // first check no users actively attached to role
   const users = (
     await db.allDocs(
@@ -97,7 +119,8 @@ export async function destroy(ctx: UserCtx) {
   await updateRolesOnUserTable(
     db,
     ctx.params.roleId,
-    UpdateRolesOptions.REMOVED
+    UpdateRolesOptions.REMOVED,
+    role.version
   )
   ctx.message = `Role ${ctx.params.roleId} deleted successfully`
   ctx.status = 200

packages/server/src/api/routes/tests/role.spec.js

@@ -29,10 +29,11 @@ describe("/roles", () => {
 
   describe("create", () => {
     it("returns a success message when role is successfully created", async () => {
-      const res = await createRole()
+      const role = basicRole()
+      const res = await createRole(role)
 
       expect(res.res.statusMessage).toEqual(
-        "Role 'NewRole' created successfully."
+        `Role '${role.name}' created successfully.`
       )
       expect(res.body._id).toBeDefined()
       expect(res.body._rev).toBeDefined()
@@ -44,12 +45,13 @@ describe("/roles", () => {
 
   describe("update", () => {
     it("updates a role", async () => {
-      let res = await createRole()
+      const role = basicRole()
+      let res = await createRole(role)
       jest.clearAllMocks()
       res = await createRole(res.body)
 
       expect(res.res.statusMessage).toEqual(
-        "Role 'NewRole' created successfully."
+        `Role '${role.name}' created successfully.`
       )
       expect(res.body._id).toBeDefined()
       expect(res.body._rev).toBeDefined()
@@ -86,7 +88,7 @@ describe("/roles", () => {
       expect(powerUserRole.inherits).toEqual(BUILTIN_ROLE_IDS.BASIC)
       expect(powerUserRole.permissionId).toEqual(BuiltinPermissionID.POWER)
 
-      const customRoleFetched = res.body.find(r => r._id === customRole._id)
+      const customRoleFetched = res.body.find(r => r._id === customRole.name)
       expect(customRoleFetched).toBeDefined()
       expect(customRoleFetched.inherits).toEqual(BUILTIN_ROLE_IDS.BASIC)
       expect(customRoleFetched.permissionId).toEqual(

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

@@ -134,7 +134,7 @@ export function roleValidator() {
   return auth.joiValidator.body(Joi.object({
     _id: OPTIONAL_STRING,
     _rev: OPTIONAL_STRING,
-    name: Joi.string().required(),
+    name: Joi.string().regex(/^[a-zA-Z0-9_]*$/).required(),
     // this is the base permission ID (for now a built in)
     permissionId: Joi.string().valid(...Object.values(permissions.BuiltinPermissionID)).required(),
     permissions: Joi.object()

packages/server/src/sdk/app/backups/exports.ts

@@ -22,12 +22,12 @@ import tar from "tar"
 
 const MemoryStream = require("memorystream")
 
-interface DBDumpOpts {
+export interface DBDumpOpts {
   filter?: any
   exportPath?: string
 }
 
-interface ExportOpts extends DBDumpOpts {
+export interface ExportOpts extends DBDumpOpts {
   tar?: boolean
   excludeRows?: boolean
   excludeLogs?: boolean
@@ -57,7 +57,10 @@ function tarFilesToTmp(tmpDir: string, files: string[]) {
  * a filter function or the name of the export.
  * @return {*} either a readable stream or a string
  */
-export async function exportDB(dbName: string, opts: DBDumpOpts = {}) {
+export async function exportDB(
+  dbName: string,
+  opts: DBDumpOpts = {}
+): Promise<DBDumpOpts> {
   const exportOpts = {
     filter: opts?.filter,
     batch_size: 1000,
@@ -178,6 +181,7 @@ export async function exportApp(appId: string, config?: ExportOpts) {
  * Streams a backup of the database state for an app
  * @param {string} appId The ID of the app which is to be backed up.
  * @param {boolean} excludeRows Flag to state whether the export should include data.
+ * @param {string} encryptPassword password for encrypting the export.
  * @returns {*} a readable stream of the backup which is written in real time
  */
 export async function streamExportApp({

packages/server/src/tests/utilities/structures.ts

@@ -282,9 +282,10 @@ export function basicLinkedRow(
 
 export function basicRole() {
   return {
-    name: "NewRole",
+    name: `NewRole_${utils.newid()}`,
     inherits: roles.BUILTIN_ROLE_IDS.BASIC,
     permissionId: permissions.BuiltinPermissionID.READ_ONLY,
+    version: "name",
   }
 }
 

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

@@ -4,4 +4,5 @@ export interface Role extends Document {
   permissionId: string
   inherits?: string
   permissions: { [key: string]: string[] }
+  version?: string
 }

packages/worker/src/api/routes/global/tests/roles.spec.ts

@@ -19,6 +19,7 @@ jest.mock("@budibase/backend-core", () => {
 
 let appId: string
 let appDb: Database
+const ROLE_NAME = "newRole"
 
 async function addAppMetadata() {
   await appDb.put({
@@ -34,7 +35,7 @@ describe("/api/global/roles", () => {
   const config = new TestConfiguration()
 
   const role = new roles.Role(
-    db.generateRoleID("newRole"),
+    db.generateRoleID(ROLE_NAME),
     roles.BUILTIN_ROLE_IDS.BASIC,
     permissions.BuiltinPermissionID.READ_ONLY
   )
@@ -66,7 +67,7 @@ describe("/api/global/roles", () => {
       const res = await config.api.roles.get()
       expect(res.body).toBeDefined()
       expect(res.body[appId].roles.length).toEqual(5)
-      expect(res.body[appId].roles.map((r: any) => r._id)).toContain(role._id)
+      expect(res.body[appId].roles.map((r: any) => r._id)).toContain(ROLE_NAME)
     })
   })