diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index c14178cacb..a301abea8b 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -213,6 +213,22 @@ export function getBuiltinRole(roleId: string): Role | undefined { return cloneDeep(role) } +export function validInherits( + allRoles: RoleDoc[], + inherits?: string | string[] +): boolean { + if (!inherits) { + return false + } + const find = (id: string) => allRoles.find(r => compareRoleIds(r._id!, id)) + if (Array.isArray(inherits)) { + const filtered = inherits.filter(roleId => find(roleId)) + return inherits.length !== 0 && filtered.length === inherits.length + } else { + return !!find(inherits) + } +} + /** * Works through the inheritance ranks to see how far up the builtin stack this ID is. */ diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index 2d8556b581..fca0d287eb 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -18,10 +18,12 @@ import { UserCtx, UserMetadata, DocumentType, + PermissionLevel, } from "@budibase/types" import { RoleColor, sdk as sharedSdk, helpers } from "@budibase/shared-core" import sdk from "../../sdk" import { builderSocket } from "../../websockets" +import { validInherits } from "@budibase/backend-core/src/security/roles" const UpdateRolesOptions = { CREATED: "created", @@ -125,6 +127,17 @@ export async function save(ctx: UserCtx) { ctx.throw(400, "Cannot change custom role name") } + // custom roles should always inherit basic - if they don't inherit anything else + if (!inherits && roles.validInherits(allRoles, dbRole?.inherits)) { + inherits = dbRole?.inherits + } else if (!roles.validInherits(allRoles, inherits)) { + inherits = [roles.BUILTIN_ROLE_IDS.BASIC] + } + // assume write permission level for newly created roles + if (isCreate && !permissionId) { + permissionId = PermissionLevel.WRITE + } + const role = new roles.Role(_id, name, permissionId, { displayName: uiMetadata?.displayName || name, description: uiMetadata?.description || "Custom role", diff --git a/packages/server/src/api/routes/tests/role.spec.ts b/packages/server/src/api/routes/tests/role.spec.ts index 5668295342..2c9ce32937 100644 --- a/packages/server/src/api/routes/tests/role.spec.ts +++ b/packages/server/src/api/routes/tests/role.spec.ts @@ -38,6 +38,16 @@ describe("/roles", () => { _id: dbCore.prefixRoleID(res._id!), }) }) + + it("handle a role with no inherits", async () => { + const role = basicRole() + role.inherits = [] + + const res = await config.api.roles.save(role, { + status: 200, + }) + expect(res.inherits).toEqual([BUILTIN_ROLE_IDS.BASIC]) + }) }) describe("update", () => { @@ -149,6 +159,19 @@ describe("/roles", () => { { status: 400, body: { message: LOOP_ERROR } } ) }) + + it("handle updating a role, without its inherits", async () => { + const res = await config.api.roles.save({ + ...basicRole(), + inherits: [BUILTIN_ROLE_IDS.ADMIN], + }) + // save again + const updatedRes = await config.api.roles.save({ + ...res, + inherits: undefined, + }) + expect(updatedRes.inherits).toEqual([BUILTIN_ROLE_IDS.ADMIN]) + }) }) describe("fetch", () => {