diff --git a/packages/backend-core/src/docIds/ids.ts b/packages/backend-core/src/docIds/ids.ts index 152977b3af..e0ac85b3df 100644 --- a/packages/backend-core/src/docIds/ids.ts +++ b/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) } /** diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index e8a3c76c0a..cf5c6bc406 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/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 diff --git a/packages/builder/src/components/backend/DataTable/modals/EditRoles.svelte b/packages/builder/src/components/backend/DataTable/modals/EditRoles.svelte index 600e331d3e..b638d23168 100644 --- a/packages/builder/src/components/backend/DataTable/modals/EditRoles.svelte +++ b/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) @@ -108,7 +119,7 @@ title="Edit Roles" confirmText={isCreating ? "Create" : "Save"} onConfirm={saveRole} - disabled={!valid || !hasUniqueRoleName} + disabled={!valid || roleNameError} > {#if errors.length} @@ -129,7 +140,7 @@ label="Name" bind:value={selectedRole.name} disabled={shouldDisableRoleInput} - error={!hasUniqueRoleName ? "Select a unique role name." : null} + error={roleNameError} />