diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index 03660b313d..fdea09944a 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -63,6 +63,12 @@ export class Role implements RoleDoc { } addInheritance(inherits?: string | string[]) { + // make sure IDs are correct format + if (inherits && typeof inherits === "string") { + inherits = prefixRoleIDNoBuiltin(inherits) + } else if (inherits && Array.isArray(inherits)) { + inherits = inherits.map(inherit => prefixRoleIDNoBuiltin(inherit)) + } if (inherits) { this.inherits = inherits } @@ -128,7 +134,15 @@ export function getBuiltinRoles(): { [key: string]: RoleDoc } { } export function isBuiltin(role: string) { - return getBuiltinRole(role) !== undefined + return Object.values(BUILTIN_ROLE_IDS).includes(role) +} + +export function prefixRoleIDNoBuiltin(roleId: string) { + if (isBuiltin(roleId)) { + return roleId + } else { + return prefixRoleID(roleId) + } } export function getBuiltinRole(roleId: string): Role | undefined { @@ -536,3 +550,16 @@ export function getExternalRoleID(roleId: string, version?: string) { } return roleId } + +export function getExternalRoleIDs( + roleIds: string | string[] | undefined, + version?: string +) { + if (!roleIds) { + return roleIds + } else if (typeof roleIds === "string") { + return getExternalRoleID(roleIds, version) + } else { + return roleIds.map(roleId => getExternalRoleID(roleId, version)) + } +} diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index 790325390b..34455d2c52 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -28,6 +28,18 @@ const UpdateRolesOptions = { REMOVED: "removed", } +function externalRole(role: Role): Role { + let _id: string | undefined + if (role._id) { + _id = roles.getExternalRoleID(role._id) + } + return { + ...role, + _id, + inherits: roles.getExternalRoleIDs(role.inherits, role.version), + } +} + async function updateRolesOnUserTable( db: Database, roleId: string, @@ -54,7 +66,7 @@ async function updateRolesOnUserTable( } export async function fetch(ctx: UserCtx) { - ctx.body = await roles.getAllRoles() + ctx.body = (await roles.getAllRoles()).map(role => externalRole(role)) } export async function find(ctx: UserCtx) { @@ -62,7 +74,7 @@ export async function find(ctx: UserCtx) { if (!role) { ctx.throw(404, { message: "Role not found" }) } else { - ctx.body = role + ctx.body = externalRole(role) } } @@ -136,10 +148,7 @@ export async function save(ctx: UserCtx) { role.version ) role._rev = result.rev - ctx.body = { - ...role, - _id: roles.getExternalRoleID(role._id!, role.version), - } + ctx.body = externalRole(role) const devDb = context.getDevAppDB() const prodDb = context.getProdAppDB() @@ -199,15 +208,14 @@ export async function accessible(ctx: UserCtx) { if (!roleId) { roleId = roles.BUILTIN_ROLE_IDS.PUBLIC } + let roleIds: string[] = [] if (ctx.user && sharedSdk.users.isAdminOrBuilder(ctx.user)) { const appId = context.getAppId() - if (!appId) { - ctx.body = [] - } else { - ctx.body = await roles.getAllRoleIds(appId) + if (appId) { + roleIds = await roles.getAllRoleIds(appId) } } else { - ctx.body = await roles.getUserRoleIdHierarchy(roleId!) + roleIds = await roles.getUserRoleIdHierarchy(roleId!) } // If a custom role is provided in the header, filter out higher level roles @@ -215,7 +223,7 @@ export async function accessible(ctx: UserCtx) { if (roleHeader && !Object.keys(roles.BUILTIN_ROLE_IDS).includes(roleHeader)) { const role = await roles.getRole(roleHeader) const inherits = role?.inherits - const orderedRoles = ctx.body.reverse() + const orderedRoles = roleIds.reverse() let filteredRoles = [roleHeader] for (let role of orderedRoles) { filteredRoles = [role, ...filteredRoles] @@ -227,6 +235,8 @@ export async function accessible(ctx: UserCtx) { } } filteredRoles.pop() - ctx.body = [roleHeader, ...filteredRoles] + roleIds = [roleHeader, ...filteredRoles] } + + ctx.body = roleIds.map(roleId => roles.getExternalRoleID(roleId)) } diff --git a/packages/server/src/api/routes/tests/role.spec.ts b/packages/server/src/api/routes/tests/role.spec.ts index ef3a919173..6ed5dfd30f 100644 --- a/packages/server/src/api/routes/tests/role.spec.ts +++ b/packages/server/src/api/routes/tests/role.spec.ts @@ -105,7 +105,7 @@ describe("/roles", () => { inherits: [BUILTIN_ROLE_IDS.POWER, role1._id!, role2._id!], }) // go back to role1 - role1 = await config.api.roles.save( + await config.api.roles.save( { ...role1, inherits: [BUILTIN_ROLE_IDS.POWER, role2._id!, role3._id!], @@ -113,7 +113,7 @@ describe("/roles", () => { { status: 400, body: { message: LOOP_ERROR } } ) // go back to role2 - role2 = await config.api.roles.save( + await config.api.roles.save( { ...role2, inherits: [BUILTIN_ROLE_IDS.POWER, role1._id!, role3._id!], diff --git a/packages/shared-core/src/helpers/roles.ts b/packages/shared-core/src/helpers/roles.ts index 27f28cff82..24b65501ce 100644 --- a/packages/shared-core/src/helpers/roles.ts +++ b/packages/shared-core/src/helpers/roles.ts @@ -1,4 +1,4 @@ -import { Role } from "@budibase/types" +import { Role, SEPARATOR, DocumentType } from "@budibase/types" // Function to detect loops in roles export function checkForRoleInheritanceLoops(roles: Role[]): boolean {