diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index fad5f7cb74..c14178cacb 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -16,7 +16,8 @@ import { App, } from "@budibase/types" import cloneDeep from "lodash/fp/cloneDeep" -import { RoleColor } from "@budibase/shared-core" +import { RoleColor, helpers } from "@budibase/shared-core" +import { uniqBy } from "lodash" export const BUILTIN_ROLE_IDS = { ADMIN: "ADMIN", @@ -37,12 +38,20 @@ export const RoleIDVersion = { NAME: "name", } +function rolesInList(roleIds: string[], ids: string | string[]) { + if (Array.isArray(ids)) { + return ids.filter(id => roleIds.includes(id)).length === ids.length + } else { + return roleIds.includes(ids) + } +} + export class Role implements RoleDoc { _id: string _rev?: string name: string permissionId: string - inherits?: string + inherits?: string | string[] version?: string permissions: Record = {} uiMetadata?: RoleUIMetadata @@ -61,12 +70,70 @@ export class Role implements RoleDoc { this.version = RoleIDVersion.NAME } - addInheritance(inherits: string) { + 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(prefixRoleIDNoBuiltin) + } this.inherits = inherits return this } } +export class RoleHierarchyTraversal { + allRoles: RoleDoc[] + opts?: { defaultPublic?: boolean } + + constructor(allRoles: RoleDoc[], opts?: { defaultPublic?: boolean }) { + this.allRoles = allRoles + this.opts = opts + } + + walk(role: RoleDoc): RoleDoc[] { + const opts = this.opts, + allRoles = this.allRoles + // this will be a full walked list of roles - which may contain duplicates + let roleList: RoleDoc[] = [] + if (!role || !role._id) { + return roleList + } + roleList.push(role) + if (Array.isArray(role.inherits)) { + for (let roleId of role.inherits) { + const foundRole = findRole(roleId, allRoles, opts) + if (foundRole) { + roleList = roleList.concat(this.walk(foundRole)) + } + } + } else { + const foundRoleIds: string[] = [] + let currentRole: RoleDoc | undefined = role + while ( + currentRole && + currentRole.inherits && + !rolesInList(foundRoleIds, currentRole.inherits) + ) { + if (Array.isArray(currentRole.inherits)) { + return roleList.concat(this.walk(currentRole)) + } else { + foundRoleIds.push(currentRole.inherits) + currentRole = findRole(currentRole.inherits, allRoles, opts) + if (currentRole) { + roleList.push(currentRole) + } + } + // loop now found - stop iterating + if (helpers.roles.checkForRoleInheritanceLoops(roleList)) { + break + } + } + } + return uniqBy(roleList, role => role._id) + } +} + const BUILTIN_ROLES = { ADMIN: new Role( BUILTIN_IDS.ADMIN, @@ -125,7 +192,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 { @@ -153,7 +228,11 @@ export function builtinRoleToNumber(id: string) { if (!role) { break } - role = builtins[role.inherits!] + if (Array.isArray(role.inherits)) { + throw new Error("Built-in roles don't support multi-inheritance") + } else { + role = builtins[role.inherits!] + } count++ } while (role !== null) return count @@ -169,12 +248,31 @@ export async function roleToNumber(id: string) { const hierarchy = (await getUserRoleHierarchy(id, { defaultPublic: true, })) as RoleDoc[] - for (let role of hierarchy) { - if (role?.inherits && isBuiltin(role.inherits)) { + const findNumber = (role: RoleDoc): number => { + if (!role.inherits) { + return 0 + } + if (Array.isArray(role.inherits)) { + // find the built-in roles, get their number, sort it, then get the last one + const highestBuiltin: number | undefined = role.inherits + .map(roleId => { + const foundRole = hierarchy.find(role => role._id === roleId) + if (foundRole) { + return findNumber(foundRole) + 1 + } + }) + .filter(number => number) + .sort() + .pop() + if (highestBuiltin != undefined) { + return highestBuiltin + } + } else if (isBuiltin(role.inherits)) { return builtinRoleToNumber(role.inherits) + 1 } + return 0 } - return 0 + return Math.max(...hierarchy.map(findNumber)) } /** @@ -192,6 +290,53 @@ export function lowerBuiltinRoleID(roleId1?: string, roleId2?: string): string { : roleId1 } +export function compareRoleIds(roleId1: string, roleId2: string) { + // make sure both role IDs are prefixed correctly + return prefixRoleID(roleId1) === prefixRoleID(roleId2) +} + +export function externalRole(role: RoleDoc): RoleDoc { + let _id: string | undefined + if (role._id) { + _id = getExternalRoleID(role._id) + } + return { + ...role, + _id, + inherits: getExternalRoleIDs(role.inherits, role.version), + } +} + +/** + * Given a list of roles, this will pick the role out, accounting for built ins. + */ +export function findRole( + roleId: string, + roles: RoleDoc[], + opts?: { defaultPublic?: boolean } +): RoleDoc | undefined { + // built in roles mostly come from the in-code implementation, + // but can be extended by a doc stored about them (e.g. permissions) + let role: RoleDoc | undefined = getBuiltinRole(roleId) + if (!role) { + // make sure has the prefix (if it has it then it won't be added) + roleId = prefixRoleID(roleId) + } + const dbRole = roles.find( + role => role._id && compareRoleIds(role._id, roleId) + ) + if (!dbRole && !isBuiltin(roleId) && opts?.defaultPublic) { + return cloneDeep(BUILTIN_ROLES.PUBLIC) + } + // combine the roles + role = Object.assign(role || {}, dbRole) + // finalise the ID + if (role?._id) { + role._id = getExternalRoleID(role._id, role.version) + } + return Object.keys(role).length === 0 ? undefined : role +} + /** * Gets the role object, this is mainly useful for two purposes, to check if the level exists and * to check if the role inherits any others. @@ -202,30 +347,28 @@ export function lowerBuiltinRoleID(roleId1?: string, roleId2?: string): string { export async function getRole( roleId: string, opts?: { defaultPublic?: boolean } -): Promise { - // built in roles mostly come from the in-code implementation, - // but can be extended by a doc stored about them (e.g. permissions) - let role: RoleDoc | undefined = getBuiltinRole(roleId) - if (!role) { - // 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.version) - } catch (err) { - if (!isBuiltin(roleId) && opts?.defaultPublic) { - return cloneDeep(BUILTIN_ROLES.PUBLIC) - } - // only throw an error if there is no role at all - if (!role || Object.keys(role).length === 0) { - throw err +): Promise { + const db = getAppDB() + const roleList = [] + if (!isBuiltin(roleId)) { + const role = await db.tryGet(getDBRoleID(roleId)) + if (role) { + roleList.push(role) } } - return role + return findRole(roleId, roleList, opts) +} + +export async function saveRoles(roles: RoleDoc[]) { + const db = getAppDB() + await db.bulkDocs( + roles + .filter(role => role._id) + .map(role => ({ + ...role, + _id: prefixRoleID(role._id!), + })) + ) } /** @@ -235,24 +378,18 @@ async function getAllUserRoles( userRoleId: string, opts?: { defaultPublic?: boolean } ): Promise { + const allRoles = await getAllRoles() // admins have access to all roles if (userRoleId === BUILTIN_IDS.ADMIN) { - return getAllRoles() + return allRoles } - let currentRole = await getRole(userRoleId, opts) - let roles = currentRole ? [currentRole] : [] - let roleIds = [userRoleId] + // get all the inherited roles - while ( - currentRole && - currentRole.inherits && - roleIds.indexOf(currentRole.inherits) === -1 - ) { - roleIds.push(currentRole.inherits) - currentRole = await getRole(currentRole.inherits) - if (currentRole) { - roles.push(currentRole) - } + const foundRole = findRole(userRoleId, allRoles, opts) + let roles: RoleDoc[] = [] + if (foundRole) { + const traversal = new RoleHierarchyTraversal(allRoles, opts) + roles = traversal.walk(foundRole) } return roles } @@ -419,7 +556,10 @@ export class AccessController { this.userHierarchies[userRoleId] = roleIds } - return roleIds?.indexOf(tryingRoleId) !== -1 + return ( + roleIds?.find(roleId => compareRoleIds(roleId, tryingRoleId)) !== + undefined + ) } async checkScreensAccess(screens: Screen[], userRoleId: string) { @@ -461,7 +601,7 @@ export function getDBRoleID(roleName: 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) && + roleId.startsWith(`${DocumentType.ROLE}${SEPARATOR}`) && (isBuiltin(roleId) || version === RoleIDVersion.NAME) ) { const parts = roleId.split(SEPARATOR) @@ -470,3 +610,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 b6b9ac1a29..f26b4bae69 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -19,7 +19,7 @@ import { UserMetadata, DocumentType, } from "@budibase/types" -import { RoleColor, sdk as sharedSdk } from "@budibase/shared-core" +import { RoleColor, sdk as sharedSdk, helpers } from "@budibase/shared-core" import sdk from "../../sdk" const UpdateRolesOptions = { @@ -27,6 +27,30 @@ const UpdateRolesOptions = { REMOVED: "removed", } +async function removeRoleFromOthers(roleId: string) { + const allOtherRoles = await roles.getAllRoles() + const updated: Role[] = [] + for (let role of allOtherRoles) { + let changed = false + if (Array.isArray(role.inherits)) { + const newInherits = role.inherits.filter( + id => !roles.compareRoleIds(id, roleId) + ) + changed = role.inherits.length !== newInherits.length + role.inherits = newInherits + } else if (role.inherits && roles.compareRoleIds(role.inherits, roleId)) { + role.inherits = roles.BUILTIN_ROLE_IDS.PUBLIC + changed = true + } + if (changed) { + updated.push(role) + } + } + if (updated.length) { + await roles.saveRoles(updated) + } +} + async function updateRolesOnUserTable( db: Database, roleId: string, @@ -53,18 +77,25 @@ async function updateRolesOnUserTable( } export async function fetch(ctx: UserCtx) { - ctx.body = await roles.getAllRoles() + ctx.body = (await roles.getAllRoles()).map(role => roles.externalRole(role)) } export async function find(ctx: UserCtx) { - ctx.body = await roles.getRole(ctx.params.roleId) + const role = await roles.getRole(ctx.params.roleId) + if (!role) { + ctx.throw(404, { message: "Role not found" }) + } + ctx.body = roles.externalRole(role) } export async function save(ctx: UserCtx) { const db = context.getAppDB() - let { _id, name, inherits, permissionId, version, uiMetadata } = + let { _id, _rev, name, inherits, permissionId, version, uiMetadata } = ctx.request.body let isCreate = false + if (!_rev && !version) { + version = roles.RoleIDVersion.NAME + } const isNewVersion = version === roles.RoleIDVersion.NAME if (_id && roles.isBuiltin(_id)) { @@ -81,9 +112,13 @@ export async function save(ctx: UserCtx) { _id = dbCore.prefixRoleID(_id) } + const allRoles = (await roles.getAllRoles()).map(role => ({ + ...role, + _id: dbCore.prefixRoleID(role._id!), + })) let dbRole: Role | undefined if (!isCreate && _id?.startsWith(DocumentType.ROLE)) { - dbRole = await db.get(_id) + dbRole = allRoles.find(role => role._id === _id) } if (dbRole && dbRole.name !== name && isNewVersion) { ctx.throw(400, "Cannot change custom role name") @@ -97,7 +132,19 @@ export async function save(ctx: UserCtx) { if (dbRole?.permissions && !role.permissions) { role.permissions = dbRole.permissions } - const foundRev = ctx.request.body._rev || dbRole?._rev + + // add the new role to the list and check for loops + const index = allRoles.findIndex(r => r._id === role._id) + if (index === -1) { + allRoles.push(role) + } else { + allRoles[index] = role + } + if (helpers.roles.checkForRoleInheritanceLoops(allRoles)) { + ctx.throw(400, "Role inheritance contains a loop, this is not supported") + } + + const foundRev = _rev || dbRole?._rev if (foundRev) { role._rev = foundRev } @@ -114,7 +161,7 @@ export async function save(ctx: UserCtx) { role.version ) role._rev = result.rev - ctx.body = role + ctx.body = roles.externalRole(role) const devDb = context.getDevAppDB() const prodDb = context.getProdAppDB() @@ -163,6 +210,10 @@ export async function destroy(ctx: UserCtx) { UpdateRolesOptions.REMOVED, role.version ) + + // clean up inherits + await removeRoleFromOthers(roleId) + ctx.message = `Role ${ctx.params.roleId} deleted successfully` ctx.status = 200 } @@ -172,30 +223,35 @@ 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 const roleHeader = ctx.header?.[Header.PREVIEW_ROLE] as string if (roleHeader && !Object.keys(roles.BUILTIN_ROLE_IDS).includes(roleHeader)) { - const inherits = (await roles.getRole(roleHeader))?.inherits - const orderedRoles = ctx.body.reverse() + const role = await roles.getRole(roleHeader) + const inherits = role?.inherits + const orderedRoles = roleIds.reverse() let filteredRoles = [roleHeader] for (let role of orderedRoles) { filteredRoles = [role, ...filteredRoles] - if (role === inherits) { + if ( + (Array.isArray(inherits) && inherits.includes(role)) || + role === inherits + ) { break } } 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/permissions.spec.ts b/packages/server/src/api/routes/tests/permissions.spec.ts index a479adb4cf..7a9bb2f373 100644 --- a/packages/server/src/api/routes/tests/permissions.spec.ts +++ b/packages/server/src/api/routes/tests/permissions.spec.ts @@ -1,5 +1,5 @@ import { roles } from "@budibase/backend-core" -import { Document, PermissionLevel, Row } from "@budibase/types" +import { Document, PermissionLevel, Role, Row, Table } from "@budibase/types" import * as setup from "./utilities" import { generator, mocks } from "@budibase/backend-core/tests" @@ -288,6 +288,88 @@ describe("/permission", () => { }) }) + describe("multi-inheritance permissions", () => { + let table1: Table, table2: Table, role1: Role, role2: Role + beforeEach(async () => { + // create new app + await config.init() + table1 = await config.createTable() + table2 = await config.createTable() + await config.api.row.save(table1._id!, { + name: "a", + }) + await config.api.row.save(table2._id!, { + name: "b", + }) + role1 = await config.api.roles.save( + { + name: "test_1", + permissionId: PermissionLevel.WRITE, + inherits: BUILTIN_ROLE_IDS.BASIC, + }, + { status: 200 } + ) + role2 = await config.api.roles.save( + { + name: "test_2", + permissionId: PermissionLevel.WRITE, + inherits: BUILTIN_ROLE_IDS.BASIC, + }, + { status: 200 } + ) + await config.api.permission.add({ + roleId: role1._id!, + level: PermissionLevel.READ, + resourceId: table1._id!, + }) + await config.api.permission.add({ + roleId: role2._id!, + level: PermissionLevel.READ, + resourceId: table2._id!, + }) + }) + + it("should be unable to search for table 2 using role 1", async () => { + await config.loginAsRole(role1._id!, async () => { + const response2 = await config.api.row.search( + table2._id!, + { + query: {}, + }, + { status: 403 } + ) + expect(response2.rows).toBeUndefined() + }) + }) + + it("should be able to fetch two tables, with different roles, using multi-inheritance", async () => { + const role3 = await config.api.roles.save({ + name: "role3", + permissionId: PermissionLevel.WRITE, + inherits: [role1._id!, role2._id!], + }) + + await config.loginAsRole(role3._id!, async () => { + const response1 = await config.api.row.search( + table1._id!, + { + query: {}, + }, + { status: 200 } + ) + const response2 = await config.api.row.search( + table2._id!, + { + query: {}, + }, + { status: 200 } + ) + expect(response1.rows[0].name).toEqual("a") + expect(response2.rows[0].name).toEqual("b") + }) + }) + }) + describe("fetch builtins", () => { it("should be able to fetch builtin definitions", async () => { const res = await request diff --git a/packages/server/src/api/routes/tests/role.spec.js b/packages/server/src/api/routes/tests/role.spec.js deleted file mode 100644 index 00025e396a..0000000000 --- a/packages/server/src/api/routes/tests/role.spec.js +++ /dev/null @@ -1,182 +0,0 @@ -const { roles, events, permissions } = require("@budibase/backend-core") -const setup = require("./utilities") -const { PermissionLevel } = require("@budibase/types") -const { basicRole } = setup.structures -const { BUILTIN_ROLE_IDS } = roles -const { BuiltinPermissionID } = permissions - -describe("/roles", () => { - let request = setup.getRequest() - let config = setup.getConfig() - - afterAll(setup.afterAll) - - beforeAll(async () => { - await config.init() - }) - - const createRole = async role => { - if (!role) { - role = basicRole() - } - - return request - .post(`/api/roles`) - .send(role) - .set(config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(200) - } - - describe("create", () => { - it("returns a success message when role is successfully created", async () => { - const role = basicRole() - const res = await createRole(role) - - expect(res.body._id).toBeDefined() - expect(res.body._rev).toBeDefined() - expect(events.role.updated).not.toBeCalled() - expect(events.role.created).toBeCalledTimes(1) - expect(events.role.created).toBeCalledWith(res.body) - }) - }) - - describe("update", () => { - it("updates a role", async () => { - const role = basicRole() - let res = await createRole(role) - jest.clearAllMocks() - res = await createRole(res.body) - - expect(res.body._id).toBeDefined() - expect(res.body._rev).toBeDefined() - expect(events.role.created).not.toBeCalled() - expect(events.role.updated).toBeCalledTimes(1) - expect(events.role.updated).toBeCalledWith(res.body) - }) - }) - - describe("fetch", () => { - beforeAll(async () => { - // Recreate the app - await config.init() - }) - - it("should list custom roles, plus 2 default roles", async () => { - const customRole = await config.createRole() - - const res = await request - .get(`/api/roles`) - .set(config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(200) - - expect(res.body.length).toBe(5) - - const adminRole = res.body.find(r => r._id === BUILTIN_ROLE_IDS.ADMIN) - expect(adminRole).toBeDefined() - expect(adminRole.inherits).toEqual(BUILTIN_ROLE_IDS.POWER) - expect(adminRole.permissionId).toEqual(BuiltinPermissionID.ADMIN) - - const powerUserRole = res.body.find(r => r._id === BUILTIN_ROLE_IDS.POWER) - expect(powerUserRole).toBeDefined() - expect(powerUserRole.inherits).toEqual(BUILTIN_ROLE_IDS.BASIC) - expect(powerUserRole.permissionId).toEqual(BuiltinPermissionID.POWER) - - 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( - BuiltinPermissionID.READ_ONLY - ) - }) - - it("should be able to get the role with a permission added", async () => { - const table = await config.createTable() - await config.api.permission.add({ - roleId: BUILTIN_ROLE_IDS.POWER, - resourceId: table._id, - level: PermissionLevel.READ, - }) - const res = await request - .get(`/api/roles`) - .set(config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(200) - expect(res.body.length).toBeGreaterThan(0) - const power = res.body.find(role => role._id === BUILTIN_ROLE_IDS.POWER) - expect(power.permissions[table._id]).toEqual(["read"]) - }) - }) - - describe("destroy", () => { - it("should delete custom roles", async () => { - const customRole = await config.createRole({ - name: "user", - permissionId: BuiltinPermissionID.READ_ONLY, - inherits: BUILTIN_ROLE_IDS.BASIC, - }) - delete customRole._rev_tree - await request - .delete(`/api/roles/${customRole._id}/${customRole._rev}`) - .set(config.defaultHeaders()) - .expect(200) - await request - .get(`/api/roles/${customRole._id}`) - .set(config.defaultHeaders()) - .expect(404) - expect(events.role.deleted).toBeCalledTimes(1) - expect(events.role.deleted).toBeCalledWith(customRole) - }) - }) - - describe("accessible", () => { - it("should be able to fetch accessible roles (with builder)", async () => { - const res = await request - .get("/api/roles/accessible") - .set(config.defaultHeaders()) - .expect(200) - expect(res.body.length).toBe(5) - expect(typeof res.body[0]).toBe("string") - }) - - it("should be able to fetch accessible roles (basic user)", async () => { - const res = await request - .get("/api/roles/accessible") - .set(await config.basicRoleHeaders()) - .expect(200) - expect(res.body.length).toBe(2) - expect(res.body[0]).toBe("BASIC") - expect(res.body[1]).toBe("PUBLIC") - }) - - it("should be able to fetch accessible roles (no user)", async () => { - const res = await request - .get("/api/roles/accessible") - .set(config.publicHeaders()) - .expect(200) - expect(res.body.length).toBe(1) - expect(res.body[0]).toBe("PUBLIC") - }) - - it("should not fetch higher level accessible roles when a custom role header is provided", async () => { - await createRole({ - name: `custom_role_1`, - inherits: roles.BUILTIN_ROLE_IDS.BASIC, - permissionId: permissions.BuiltinPermissionID.READ_ONLY, - version: "name", - }) - const res = await request - .get("/api/roles/accessible") - .set({ - ...config.defaultHeaders(), - "x-budibase-role": "custom_role_1", - }) - .expect(200) - expect(res.body.length).toBe(3) - expect(res.body[0]).toBe("custom_role_1") - expect(res.body[1]).toBe("BASIC") - expect(res.body[2]).toBe("PUBLIC") - }) - }) -}) diff --git a/packages/server/src/api/routes/tests/role.spec.ts b/packages/server/src/api/routes/tests/role.spec.ts new file mode 100644 index 0000000000..5668295342 --- /dev/null +++ b/packages/server/src/api/routes/tests/role.spec.ts @@ -0,0 +1,337 @@ +import { + roles, + events, + permissions, + db as dbCore, +} from "@budibase/backend-core" +import * as setup from "./utilities" +import { PermissionLevel } from "@budibase/types" + +const { basicRole } = setup.structures +const { BUILTIN_ROLE_IDS } = roles +const { BuiltinPermissionID } = permissions + +const LOOP_ERROR = "Role inheritance contains a loop, this is not supported" + +describe("/roles", () => { + let config = setup.getConfig() + + afterAll(setup.afterAll) + + beforeAll(async () => { + await config.init() + }) + + describe("create", () => { + it("returns a success message when role is successfully created", async () => { + const role = basicRole() + const res = await config.api.roles.save(role, { + status: 200, + }) + + expect(res._id).toBeDefined() + expect(res._rev).toBeDefined() + expect(events.role.updated).not.toHaveBeenCalled() + expect(events.role.created).toHaveBeenCalledTimes(1) + expect(events.role.created).toHaveBeenCalledWith({ + ...res, + _id: dbCore.prefixRoleID(res._id!), + }) + }) + }) + + describe("update", () => { + beforeEach(async () => { + // Recreate the app + await config.init() + }) + + it("updates a role", async () => { + const role = basicRole() + let res = await config.api.roles.save(role, { + status: 200, + }) + jest.clearAllMocks() + res = await config.api.roles.save(res, { + status: 200, + }) + + expect(res._id).toBeDefined() + expect(res._rev).toBeDefined() + expect(events.role.created).not.toHaveBeenCalled() + expect(events.role.updated).toHaveBeenCalledTimes(1) + expect(events.role.updated).toHaveBeenCalledWith({ + ...res, + _id: dbCore.prefixRoleID(res._id!), + }) + }) + + it("disallow loops", async () => { + const role1 = await config.api.roles.save(basicRole(), { + status: 200, + }) + const role2 = await config.api.roles.save( + { + ...basicRole(), + inherits: [role1._id!], + }, + { + status: 200, + } + ) + await config.api.roles.save( + { + ...role1, + inherits: [role2._id!], + }, + { status: 400, body: { message: LOOP_ERROR } } + ) + }) + + it("disallow more complex loops", async () => { + let role1 = await config.api.roles.save({ + ...basicRole(), + name: "role1", + inherits: [BUILTIN_ROLE_IDS.POWER], + }) + let role2 = await config.api.roles.save({ + ...basicRole(), + name: "role2", + inherits: [BUILTIN_ROLE_IDS.POWER, role1._id!], + }) + let role3 = await config.api.roles.save({ + ...basicRole(), + name: "role3", + inherits: [BUILTIN_ROLE_IDS.POWER, role1._id!, role2._id!], + }) + // go back to role1 + await config.api.roles.save( + { + ...role1, + inherits: [BUILTIN_ROLE_IDS.POWER, role2._id!, role3._id!], + }, + { status: 400, body: { message: LOOP_ERROR } } + ) + // go back to role2 + await config.api.roles.save( + { + ...role2, + inherits: [BUILTIN_ROLE_IDS.POWER, role1._id!, role3._id!], + }, + { status: 400, body: { message: LOOP_ERROR } } + ) + }) + + it("frontend example - should deny", async () => { + const id1 = "cb27c4ec9415042f4800411adb346fb7c", + id2 = "cbc72a9d61ab64d49b31d90d1df4c1fdb" + const role1 = await config.api.roles.save({ + _id: id1, + name: id1, + permissions: {}, + permissionId: "write", + version: "name", + inherits: ["POWER"], + }) + await config.api.roles.save({ + _id: id2, + permissions: {}, + name: id2, + permissionId: "write", + version: "name", + inherits: [id1], + }) + await config.api.roles.save( + { + ...role1, + inherits: [BUILTIN_ROLE_IDS.POWER, id2], + }, + { status: 400, body: { message: LOOP_ERROR } } + ) + }) + }) + + describe("fetch", () => { + beforeAll(async () => { + // Recreate the app + await config.init() + }) + + it("should list custom roles, plus 2 default roles", async () => { + const customRole = await config.createRole() + + const res = await config.api.roles.fetch({ + status: 200, + }) + + expect(res.length).toBe(5) + + const adminRole = res.find(r => r._id === BUILTIN_ROLE_IDS.ADMIN) + expect(adminRole).toBeDefined() + expect(adminRole!.inherits).toEqual(BUILTIN_ROLE_IDS.POWER) + expect(adminRole!.permissionId).toEqual(BuiltinPermissionID.ADMIN) + + const powerUserRole = res.find(r => r._id === BUILTIN_ROLE_IDS.POWER) + expect(powerUserRole).toBeDefined() + expect(powerUserRole!.inherits).toEqual(BUILTIN_ROLE_IDS.BASIC) + expect(powerUserRole!.permissionId).toEqual(BuiltinPermissionID.POWER) + + const customRoleFetched = res.find(r => r._id === customRole.name) + expect(customRoleFetched).toBeDefined() + expect(customRoleFetched!.inherits).toEqual(BUILTIN_ROLE_IDS.BASIC) + expect(customRoleFetched!.permissionId).toEqual( + BuiltinPermissionID.READ_ONLY + ) + }) + + it("should be able to get the role with a permission added", async () => { + const table = await config.createTable() + await config.api.permission.add({ + roleId: BUILTIN_ROLE_IDS.POWER, + resourceId: table._id!, + level: PermissionLevel.READ, + }) + const res = await config.api.roles.fetch() + expect(res.length).toBeGreaterThan(0) + const power = res.find(role => role._id === BUILTIN_ROLE_IDS.POWER) + expect(power?.permissions[table._id!]).toEqual(["read"]) + }) + }) + + describe("destroy", () => { + it("should delete custom roles", async () => { + const customRole = await config.createRole({ + name: "user", + permissionId: BuiltinPermissionID.READ_ONLY, + inherits: BUILTIN_ROLE_IDS.BASIC, + }) + await config.api.roles.destroy(customRole, { + status: 200, + }) + await config.api.roles.find(customRole._id!, { + status: 404, + }) + expect(events.role.deleted).toHaveBeenCalledTimes(1) + expect(events.role.deleted).toHaveBeenCalledWith({ + ...customRole, + _id: dbCore.prefixRoleID(customRole._id!), + }) + }) + + it("should disconnection roles when deleted", async () => { + const role1 = await config.api.roles.save({ + name: "role1", + permissionId: BuiltinPermissionID.WRITE, + inherits: [BUILTIN_ROLE_IDS.BASIC], + }) + const role2 = await config.api.roles.save({ + name: "role2", + permissionId: BuiltinPermissionID.WRITE, + inherits: [BUILTIN_ROLE_IDS.BASIC, role1._id!], + }) + const role3 = await config.api.roles.save({ + name: "role3", + permissionId: BuiltinPermissionID.WRITE, + inherits: [BUILTIN_ROLE_IDS.BASIC, role2._id!], + }) + await config.api.roles.destroy(role2, { status: 200 }) + const found = await config.api.roles.find(role3._id!, { status: 200 }) + expect(found.inherits).toEqual([BUILTIN_ROLE_IDS.BASIC]) + }) + }) + + describe("accessible", () => { + beforeAll(async () => { + // new app, reset roles + await config.init() + // create one custom role + await config.createRole() + }) + + it("should be able to fetch accessible roles (with builder)", async () => { + await config.withHeaders(config.defaultHeaders(), async () => { + const res = await config.api.roles.accessible({ + status: 200, + }) + expect(res.length).toBe(5) + expect(typeof res[0]).toBe("string") + }) + }) + + it("should be able to fetch accessible roles (basic user)", async () => { + const headers = await config.basicRoleHeaders() + await config.withHeaders(headers, async () => { + const res = await config.api.roles.accessible({ + status: 200, + }) + expect(res.length).toBe(2) + expect(res[0]).toBe("BASIC") + expect(res[1]).toBe("PUBLIC") + }) + }) + + it("should be able to fetch accessible roles (no user)", async () => { + await config.withHeaders(config.publicHeaders(), async () => { + const res = await config.api.roles.accessible({ + status: 200, + }) + expect(res.length).toBe(1) + expect(res[0]).toBe("PUBLIC") + }) + }) + + it("should not fetch higher level accessible roles when a custom role header is provided", async () => { + const customRoleName = "custom_role_1" + await config.api.roles.save({ + name: customRoleName, + inherits: roles.BUILTIN_ROLE_IDS.BASIC, + permissionId: permissions.BuiltinPermissionID.READ_ONLY, + version: "name", + }) + await config.withHeaders( + { "x-budibase-role": customRoleName }, + async () => { + const res = await config.api.roles.accessible({ + status: 200, + }) + expect(res).toEqual([customRoleName, "BASIC", "PUBLIC"]) + } + ) + }) + }) + + describe("accessible - multi-inheritance", () => { + it("should list access correctly for multi-inheritance role", async () => { + const role1 = "multi_role_1", + role2 = "multi_role_2", + role3 = "multi_role_3" + const { _id: roleId1 } = await config.api.roles.save({ + name: role1, + inherits: roles.BUILTIN_ROLE_IDS.BASIC, + permissionId: permissions.BuiltinPermissionID.WRITE, + version: "name", + }) + const { _id: roleId2 } = await config.api.roles.save({ + name: role2, + inherits: roles.BUILTIN_ROLE_IDS.POWER, + permissionId: permissions.BuiltinPermissionID.POWER, + version: "name", + }) + await config.api.roles.save({ + name: role3, + inherits: [roleId1!, roleId2!], + permissionId: permissions.BuiltinPermissionID.READ_ONLY, + version: "name", + }) + const headers = await config.roleHeaders({ + roleId: role3, + }) + await config.withHeaders(headers, async () => { + const res = await config.api.roles.accessible({ + status: 200, + }) + expect(res).toEqual([role3, role1, "BASIC", "PUBLIC", role2, "POWER"]) + }) + }) + }) +}) diff --git a/packages/server/src/api/routes/tests/screen.spec.js b/packages/server/src/api/routes/tests/screen.spec.js deleted file mode 100644 index 481baac703..0000000000 --- a/packages/server/src/api/routes/tests/screen.spec.js +++ /dev/null @@ -1,103 +0,0 @@ -const { checkBuilderEndpoint } = require("./utilities/TestFunctions") -const setup = require("./utilities") -const { basicScreen } = setup.structures -const { events } = require("@budibase/backend-core") - -describe("/screens", () => { - let request = setup.getRequest() - let config = setup.getConfig() - let screen - - afterAll(setup.afterAll) - - beforeAll(async () => { - await config.init() - screen = await config.createScreen() - }) - - describe("fetch", () => { - it("should be able to create a layout", async () => { - const res = await request - .get(`/api/screens`) - .set(config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(200) - expect(res.body.length).toEqual(1) - expect(res.body.some(s => s._id === screen._id)).toEqual(true) - }) - - it("should apply authorization to endpoint", async () => { - await checkBuilderEndpoint({ - config, - method: "GET", - url: `/api/screens`, - }) - }) - }) - - describe("save", () => { - const saveScreen = async screen => { - const res = await request - .post(`/api/screens`) - .send(screen) - .set(config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(200) - return res - } - - it("should be able to create a screen", async () => { - jest.clearAllMocks() - - const screen = basicScreen() - const res = await saveScreen(screen) - - expect(res.body._rev).toBeDefined() - expect(res.body.name).toEqual(screen.name) - expect(events.screen.created).toBeCalledTimes(1) - }) - - it("should be able to update a screen", async () => { - const screen = basicScreen() - let res = await saveScreen(screen) - screen._id = res.body._id - screen._rev = res.body._rev - screen.name = "edit" - jest.clearAllMocks() - - res = await saveScreen(screen) - - expect(res.body._rev).toBeDefined() - expect(res.body.name).toEqual(screen.name) - expect(events.screen.created).not.toBeCalled() - }) - - it("should apply authorization to endpoint", async () => { - await checkBuilderEndpoint({ - config, - method: "POST", - url: `/api/screens`, - }) - }) - }) - - describe("destroy", () => { - it("should be able to delete the screen", async () => { - const res = await request - .delete(`/api/screens/${screen._id}/${screen._rev}`) - .set(config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(200) - expect(res.body.message).toBeDefined() - expect(events.screen.deleted).toBeCalledTimes(1) - }) - - it("should apply authorization to endpoint", async () => { - await checkBuilderEndpoint({ - config, - method: "DELETE", - url: `/api/screens/${screen._id}/${screen._rev}`, - }) - }) - }) -}) diff --git a/packages/server/src/api/routes/tests/screen.spec.ts b/packages/server/src/api/routes/tests/screen.spec.ts new file mode 100644 index 0000000000..5dfe3d2a44 --- /dev/null +++ b/packages/server/src/api/routes/tests/screen.spec.ts @@ -0,0 +1,171 @@ +import { checkBuilderEndpoint } from "./utilities/TestFunctions" +import * as setup from "./utilities" +import { events, roles } from "@budibase/backend-core" +import { Screen, PermissionLevel, Role } from "@budibase/types" + +const { basicScreen } = setup.structures + +describe("/screens", () => { + let config = setup.getConfig() + let screen: Screen + + afterAll(setup.afterAll) + + beforeAll(async () => { + await config.init() + screen = await config.createScreen() + }) + + describe("fetch", () => { + it("should be able to create a layout", async () => { + const screens = await config.api.screen.list({ status: 200 }) + expect(screens.length).toEqual(1) + expect(screens.some(s => s._id === screen._id)).toEqual(true) + }) + + it("should apply authorization to endpoint", async () => { + await checkBuilderEndpoint({ + config, + method: "GET", + url: `/api/screens`, + }) + }) + }) + + describe("permissions", () => { + let screen1: Screen, screen2: Screen + let role1: Role, role2: Role, multiRole: Role + + beforeAll(async () => { + role1 = await config.api.roles.save({ + name: "role1", + inherits: roles.BUILTIN_ROLE_IDS.BASIC, + permissionId: PermissionLevel.WRITE, + }) + role2 = await config.api.roles.save({ + name: "role2", + inherits: roles.BUILTIN_ROLE_IDS.BASIC, + permissionId: PermissionLevel.WRITE, + }) + multiRole = await config.api.roles.save({ + name: "multiRole", + inherits: [role1._id!, role2._id!], + permissionId: PermissionLevel.WRITE, + }) + screen1 = await config.api.screen.save( + { + ...basicScreen(), + routing: { + roleId: role1._id!, + route: "/foo", + homeScreen: false, + }, + }, + { status: 200 } + ) + screen2 = await config.api.screen.save( + { + ...basicScreen(), + routing: { + roleId: role2._id!, + route: "/bar", + homeScreen: false, + }, + }, + { status: 200 } + ) + // get into prod app + await config.publish() + }) + + async function checkScreens(roleId: string, screenIds: string[]) { + await config.loginAsRole(roleId, async () => { + const res = await config.api.application.getDefinition( + config.prodAppId!, + { + status: 200, + } + ) + // basic and role1 screen + expect(res.screens.length).toEqual(screenIds.length) + expect(res.screens.map(s => s._id).sort()).toEqual(screenIds.sort()) + }) + } + + it("should be able to fetch basic and screen1 with role1", async () => { + await checkScreens(role1._id!, [screen._id!, screen1._id!]) + }) + + it("should be able to fetch basic and screen2 with role2", async () => { + await checkScreens(role2._id!, [screen._id!, screen2._id!]) + }) + + it("should be able to fetch basic, screen1 and screen2 with multi-inheritance role", async () => { + await checkScreens(multiRole._id!, [ + screen._id!, + screen1._id!, + screen2._id!, + ]) + }) + }) + + describe("save", () => { + beforeEach(() => { + jest.clearAllMocks() + }) + + it("should be able to create a screen", async () => { + const screen = basicScreen() + const responseScreen = await config.api.screen.save(screen, { + status: 200, + }) + + expect(responseScreen._rev).toBeDefined() + expect(responseScreen.name).toEqual(screen.name) + expect(events.screen.created).toHaveBeenCalledTimes(1) + }) + + it("should be able to update a screen", async () => { + const screen = basicScreen() + let responseScreen = await config.api.screen.save(screen, { status: 200 }) + screen._id = responseScreen._id + screen._rev = responseScreen._rev + screen.name = "edit" + jest.clearAllMocks() + + responseScreen = await config.api.screen.save(screen, { status: 200 }) + + expect(responseScreen._rev).toBeDefined() + expect(responseScreen.name).toEqual(screen.name) + expect(events.screen.created).not.toHaveBeenCalled() + }) + + it("should apply authorization to endpoint", async () => { + await checkBuilderEndpoint({ + config, + method: "POST", + url: `/api/screens`, + }) + }) + }) + + describe("destroy", () => { + it("should be able to delete the screen", async () => { + const response = await config.api.screen.destroy( + screen._id!, + screen._rev!, + { status: 200 } + ) + expect(response.message).toBeDefined() + expect(events.screen.deleted).toHaveBeenCalledTimes(1) + }) + + it("should apply authorization to endpoint", async () => { + await checkBuilderEndpoint({ + config, + method: "DELETE", + url: `/api/screens/${screen._id}/${screen._rev}`, + }) + }) + }) +}) diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index 22b8d32978..d9dbd96b16 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -225,7 +225,10 @@ export function roleValidator() { ) ) .optional(), - inherits: OPTIONAL_STRING, + inherits: Joi.alternatives().try( + OPTIONAL_STRING, + Joi.array().items(OPTIONAL_STRING) + ), }).unknown(true) ) } diff --git a/packages/server/src/middleware/currentapp.ts b/packages/server/src/middleware/currentapp.ts index ad6f2afa18..d616377298 100644 --- a/packages/server/src/middleware/currentapp.ts +++ b/packages/server/src/middleware/currentapp.ts @@ -59,11 +59,15 @@ export default async (ctx: UserCtx, next: any) => { // Ensure the role is valid by ensuring a definition exists try { if (roleHeader) { - await roles.getRole(roleHeader) - roleId = roleHeader + const role = await roles.getRole(roleHeader) + if (role) { + roleId = roleHeader - // Delete admin and builder flags so that the specified role is honoured - ctx.user = users.removePortalUserPermissions(ctx.user) as ContextUser + // Delete admin and builder flags so that the specified role is honoured + ctx.user = users.removePortalUserPermissions( + ctx.user + ) as ContextUser + } } } catch (error) { // Swallow error and do nothing diff --git a/packages/server/src/tests/utilities/TestConfiguration.ts b/packages/server/src/tests/utilities/TestConfiguration.ts index 214c9498d6..713f8b31de 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.ts +++ b/packages/server/src/tests/utilities/TestConfiguration.ts @@ -110,6 +110,7 @@ export default class TestConfiguration { tenantId?: string api: API csrfToken?: string + temporaryHeaders?: Record constructor(openServer = true) { if (openServer) { @@ -428,6 +429,38 @@ export default class TestConfiguration { // HEADERS + // sets the role for the headers, for the period of a callback + async loginAsRole(roleId: string, cb: () => Promise) { + const roleUser = await this.createUser({ + roles: { + [this.getProdAppId()]: roleId, + }, + builder: { global: false }, + admin: { global: false }, + }) + await this.login({ + roleId, + userId: roleUser._id!, + builder: false, + prodApp: true, + }) + await this.withUser(roleUser, async () => { + await cb() + }) + } + + async withHeaders( + headers: Record, + cb: () => Promise + ) { + this.temporaryHeaders = headers + try { + await cb() + } finally { + this.temporaryHeaders = undefined + } + } + defaultHeaders(extras = {}, prodApp = false) { const tenantId = this.getTenantId() const user = this.getUser() @@ -451,7 +484,10 @@ export default class TestConfiguration { } else if (this.appId) { headers[constants.Header.APP_ID] = this.appId } - return headers + return { + ...headers, + ...this.temporaryHeaders, + } } publicHeaders({ prodApp = true } = {}) { @@ -459,6 +495,7 @@ export default class TestConfiguration { const headers: any = { Accept: "application/json", + Cookie: "", } if (appId) { headers[constants.Header.APP_ID] = appId @@ -466,7 +503,10 @@ export default class TestConfiguration { headers[constants.Header.TENANT_ID] = this.getTenantId() - return headers + return { + ...headers, + ...this.temporaryHeaders, + } } async basicRoleHeaders() { diff --git a/packages/server/src/tests/utilities/api/role.ts b/packages/server/src/tests/utilities/api/role.ts index 4defbc1220..5bd0647384 100644 --- a/packages/server/src/tests/utilities/api/role.ts +++ b/packages/server/src/tests/utilities/api/role.ts @@ -4,6 +4,7 @@ import { FindRoleResponse, SaveRoleRequest, SaveRoleResponse, + Role, } from "@budibase/types" import { Expectations, TestAPI } from "./base" @@ -27,13 +28,13 @@ export class RoleAPI extends TestAPI { }) } - destroy = async (roleId: string, expectations?: Expectations) => { - return await this._delete(`/api/roles/${roleId}`, { + destroy = async (role: Role, expectations?: Expectations) => { + return await this._delete(`/api/roles/${role._id}/${role._rev}`, { expectations, }) } - accesssible = async (expectations?: Expectations) => { + accessible = async (expectations?: Expectations) => { return await this._get(`/api/roles/accessible`, { expectations, }) diff --git a/packages/server/src/tests/utilities/api/screen.ts b/packages/server/src/tests/utilities/api/screen.ts index c8d3e647d8..8b7d7f92c1 100644 --- a/packages/server/src/tests/utilities/api/screen.ts +++ b/packages/server/src/tests/utilities/api/screen.ts @@ -5,4 +5,27 @@ export class ScreenAPI extends TestAPI { list = async (expectations?: Expectations): Promise => { return await this._get(`/api/screens`, { expectations }) } + + save = async ( + screen: Screen, + expectations?: Expectations + ): Promise => { + return await this._post(`/api/screens`, { + expectations, + body: screen, + }) + } + + destroy = async ( + screenId: string, + screenRev: string, + expectations?: Expectations + ): Promise<{ message: string }> => { + return this._delete<{ message: string }>( + `/api/screens/${screenId}/${screenRev}`, + { + expectations, + } + ) + } } diff --git a/packages/server/src/tests/utilities/structures.ts b/packages/server/src/tests/utilities/structures.ts index 16803f19cd..b38cc6484f 100644 --- a/packages/server/src/tests/utilities/structures.ts +++ b/packages/server/src/tests/utilities/structures.ts @@ -31,6 +31,7 @@ import { BBReferenceFieldSubType, JsonFieldSubType, AutoFieldSubType, + Role, CreateViewRequest, } from "@budibase/types" import { LoopInput } from "../../definitions/automations" @@ -511,11 +512,12 @@ export function basicLinkedRow( } } -export function basicRole() { +export function basicRole(): Role { return { name: `NewRole_${utils.newid()}`, inherits: roles.BUILTIN_ROLE_IDS.BASIC, permissionId: permissions.BuiltinPermissionID.READ_ONLY, + permissions: {}, version: "name", } } diff --git a/packages/shared-core/src/helpers/index.ts b/packages/shared-core/src/helpers/index.ts index 503f71e4eb..7603a9b88b 100644 --- a/packages/shared-core/src/helpers/index.ts +++ b/packages/shared-core/src/helpers/index.ts @@ -3,3 +3,4 @@ export * from "./integrations" export * as cron from "./cron" export * as schema from "./schema" export * as views from "./views" +export * as roles from "./roles" diff --git a/packages/shared-core/src/helpers/roles.ts b/packages/shared-core/src/helpers/roles.ts new file mode 100644 index 0000000000..8dee928edd --- /dev/null +++ b/packages/shared-core/src/helpers/roles.ts @@ -0,0 +1,55 @@ +import { Role, DocumentType, SEPARATOR } from "@budibase/types" + +// need to have a way to prefix, so we can check if the ID has its prefix or not +// all new IDs should be the same in the future, but old roles they are never prefixed +// while the role IDs always are - best to check both, also we can't access backend-core here +function prefixForCheck(id: string) { + return `${DocumentType.ROLE}${SEPARATOR}${id}` +} + +// Function to detect loops in roles +export function checkForRoleInheritanceLoops(roles: Role[]): boolean { + const roleMap = new Map() + roles.forEach(role => { + roleMap.set(role._id!, role) + }) + + const checked = new Set() + const checking = new Set() + + function hasLoop(roleId: string): boolean { + const prefixed = prefixForCheck(roleId) + if (checking.has(roleId) || checking.has(prefixed)) { + return true + } + if (checked.has(roleId) || checked.has(prefixed)) { + return false + } + + checking.add(roleId) + + const role = roleMap.get(prefixed) || roleMap.get(roleId) + if (!role) { + // role not found - ignore + checking.delete(roleId) + return false + } + + const inherits = Array.isArray(role.inherits) + ? role.inherits + : [role.inherits] + for (const inheritedId of inherits) { + if (inheritedId && hasLoop(inheritedId)) { + return true + } + } + + // mark this role has been fully checked + checking.delete(roleId) + checked.add(roleId) + + return false + } + + return !!roles.find(role => hasLoop(role._id!)) +} diff --git a/packages/shared-core/src/helpers/tests/roles.spec.ts b/packages/shared-core/src/helpers/tests/roles.spec.ts new file mode 100644 index 0000000000..051a196a5b --- /dev/null +++ b/packages/shared-core/src/helpers/tests/roles.spec.ts @@ -0,0 +1,73 @@ +import { checkForRoleInheritanceLoops } from "../roles" +import { Role } from "@budibase/types" + +/** + * This unit test exists as this utility will be used in the frontend and backend, confirmation + * of its API and expected results is useful since the backend tests won't confirm it works + * exactly as the frontend needs it to - easy to add specific test cases here that the frontend + * might need to check/cover. + */ + +interface TestRole extends Omit { + _id: string +} + +let allRoles: TestRole[] = [] + +function role(id: string, inherits: string | string[]): TestRole { + const role = { + _id: id, + inherits: inherits, + name: "ROLE", + permissionId: "PERMISSION", + permissions: {}, // not needed for this test + } + allRoles.push(role) + return role +} + +describe("role utilities", () => { + let role1: TestRole, role2: TestRole + + beforeEach(() => { + role1 = role("role_1", []) + role2 = role("role_2", [role1._id]) + }) + + afterEach(() => { + allRoles = [] + }) + + function check(hasLoop: boolean) { + const result = checkForRoleInheritanceLoops(allRoles) + expect(result).toBe(hasLoop) + } + + describe("checkForRoleInheritanceLoops", () => { + it("should confirm no loops", () => { + check(false) + }) + + it("should confirm there is a loop", () => { + const role3 = role("role_3", [role2._id]) + const role4 = role("role_4", [role3._id, role2._id, role1._id]) + role3.inherits = [ + ...(Array.isArray(role3.inherits) ? role3.inherits : []), + role4._id, + ] + check(true) + }) + + it("should handle new and old inherits structure", () => { + const role1 = role("role_role_1", "role_1") + role("role_role_2", ["role_1"]) + role1.inherits = "role_2" + check(true) + }) + + it("self reference contains loop", () => { + role("role1", "role1") + check(true) + }) + }) +}) diff --git a/packages/types/src/api/web/role.ts b/packages/types/src/api/web/role.ts index 644222b8f9..df439e84e7 100644 --- a/packages/types/src/api/web/role.ts +++ b/packages/types/src/api/web/role.ts @@ -1,12 +1,14 @@ import { Role, RoleUIMetadata } from "../../documents" +import { PermissionLevel } from "../../sdk" export interface SaveRoleRequest { _id?: string _rev?: string name: string - inherits: string + inherits?: string | string[] permissionId: string - version: string + permissions?: Record + version?: string uiMetadata?: RoleUIMetadata } diff --git a/packages/types/src/documents/app/role.ts b/packages/types/src/documents/app/role.ts index 6557b7e19d..22f4ab9cd3 100644 --- a/packages/types/src/documents/app/role.ts +++ b/packages/types/src/documents/app/role.ts @@ -9,7 +9,7 @@ export interface RoleUIMetadata { export interface Role extends Document { permissionId: string - inherits?: string + inherits?: string | string[] permissions: Record version?: string name: string diff --git a/packages/types/src/sdk/events/role.ts b/packages/types/src/sdk/events/role.ts index b04b9b8ee5..ce17b34dc4 100644 --- a/packages/types/src/sdk/events/role.ts +++ b/packages/types/src/sdk/events/role.ts @@ -3,19 +3,19 @@ import { BaseEvent } from "./event" export interface RoleCreatedEvent extends BaseEvent { roleId: string permissionId: string - inherits?: string + inherits?: string | string[] } export interface RoleUpdatedEvent extends BaseEvent { roleId: string permissionId: string - inherits?: string + inherits?: string | string[] } export interface RoleDeletedEvent extends BaseEvent { roleId: string permissionId: string - inherits?: string + inherits?: string | string[] } export interface RoleAssignedEvent extends BaseEvent {