From 0c8228edad3b1a288c5232e891ed0745f4774a53 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 5 Aug 2024 15:45:49 +0100 Subject: [PATCH 01/89] Initial work - some re-typing and updating the role tests to typescript - using role test API to make this a bit easier to adjust going forward. --- packages/backend-core/src/security/roles.ts | 42 +++- packages/server/src/api/controllers/role.ts | 5 +- .../server/src/api/routes/tests/role.spec.js | 182 ------------------ .../server/src/api/routes/tests/role.spec.ts | 164 ++++++++++++++++ .../src/tests/utilities/TestConfiguration.ts | 1 + .../server/src/tests/utilities/api/role.ts | 11 +- .../server/src/tests/utilities/structures.ts | 4 +- packages/types/src/api/web/role.ts | 4 +- packages/types/src/documents/app/role.ts | 2 +- packages/types/src/sdk/events/role.ts | 6 +- 10 files changed, 218 insertions(+), 203 deletions(-) delete mode 100644 packages/server/src/api/routes/tests/role.spec.js create mode 100644 packages/server/src/api/routes/tests/role.spec.ts diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index a64be6b319..61d9786d32 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -42,7 +42,7 @@ export class Role implements RoleDoc { _rev?: string name: string permissionId: string - inherits?: string + inherits?: string | string[] version?: string permissions = {} @@ -54,8 +54,10 @@ export class Role implements RoleDoc { this.version = RoleIDVersion.NAME } - addInheritance(inherits: string) { - this.inherits = inherits + addInheritance(inherits?: string | string[]) { + if (inherits) { + this.inherits = inherits + } return this } } @@ -113,7 +115,11 @@ export function builtinRoleToNumber(id: string) { if (!role) { break } - role = builtins[role.inherits!] + if (Array.isArray(role.inherits)) { + // TODO: role inheritance + } else { + role = builtins[role.inherits!] + } count++ } while (role !== null) return count @@ -130,7 +136,12 @@ export async function roleToNumber(id: string) { defaultPublic: true, })) as RoleDoc[] for (let role of hierarchy) { - if (role?.inherits && isBuiltin(role.inherits)) { + if (!role.inherits) { + continue + } + if (Array.isArray(role.inherits)) { + // TODO: role inheritance + } else if (isBuiltin(role.inherits)) { return builtinRoleToNumber(role.inherits) + 1 } } @@ -202,16 +213,27 @@ async function getAllUserRoles( let currentRole = await getRole(userRoleId, opts) let roles = currentRole ? [currentRole] : [] let roleIds = [userRoleId] + const rolesFound = (ids: string | string[]) => { + if (Array.isArray(ids)) { + return ids.filter(id => roleIds.includes(id)).length === ids.length + } else { + return roleIds.includes(ids) + } + } // get all the inherited roles while ( currentRole && currentRole.inherits && - roleIds.indexOf(currentRole.inherits) === -1 + !rolesFound(currentRole.inherits) ) { - roleIds.push(currentRole.inherits) - currentRole = await getRole(currentRole.inherits) - if (currentRole) { - roles.push(currentRole) + if (Array.isArray(currentRole.inherits)) { + // TODO: role inheritance + } else { + roleIds.push(currentRole.inherits) + currentRole = await getRole(currentRole.inherits) + if (currentRole) { + roles.push(currentRole) + } } } return roles diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index 3398c8102c..3431724d7d 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -182,7 +182,10 @@ export async function accessible(ctx: UserCtx) { let filteredRoles = [roleHeader] for (let role of orderedRoles) { filteredRoles = [role, ...filteredRoles] - if (role === inherits) { + if ( + (Array.isArray(inherits) && inherits.includes(role)) || + role === inherits + ) { break } } 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 4575f9b213..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`, - 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", - }) - .expect(200) - expect(res.body.length).toBe(3) - expect(res.body[0]).toBe("CUSTOM_ROLE") - 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..127be789b9 --- /dev/null +++ b/packages/server/src/api/routes/tests/role.spec.ts @@ -0,0 +1,164 @@ +import { roles, events, permissions } 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 + +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) + }) + }) + + describe("update", () => { + 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) + }) + }) + + 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) + }) + }) + + describe("accessible", () => { + it("should be able to fetch accessible roles (with builder)", async () => { + const res = await config.api.roles.accessible(config.defaultHeaders(), { + 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() + const res = await config.api.roles.accessible(headers, { + 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 () => { + const res = await config.api.roles.accessible(config.publicHeaders(), { + 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" + await config.api.roles.save({ + name: customRoleName, + inherits: roles.BUILTIN_ROLE_IDS.BASIC, + permissionId: permissions.BuiltinPermissionID.READ_ONLY, + version: "name", + }) + const res = await config.api.roles.accessible( + { "x-budibase-role": customRoleName }, + { + status: 200, + } + ) + expect(res.length).toBe(3) + expect(res[0]).toBe(customRoleName) + expect(res[1]).toBe("BASIC") + expect(res[2]).toBe("PUBLIC") + }) + }) +}) diff --git a/packages/server/src/tests/utilities/TestConfiguration.ts b/packages/server/src/tests/utilities/TestConfiguration.ts index 3d53149385..0255268097 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.ts +++ b/packages/server/src/tests/utilities/TestConfiguration.ts @@ -517,6 +517,7 @@ export default class TestConfiguration { const headers: any = { Accept: "application/json", + Cookie: "", } if (appId) { headers[constants.Header.APP_ID] = appId diff --git a/packages/server/src/tests/utilities/api/role.ts b/packages/server/src/tests/utilities/api/role.ts index 4defbc1220..31bffc6f85 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,14 +28,18 @@ 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 ( + headers: Record, + expectations?: Expectations + ) => { return await this._get(`/api/roles/accessible`, { + headers, expectations, }) } diff --git a/packages/server/src/tests/utilities/structures.ts b/packages/server/src/tests/utilities/structures.ts index 698f6d8236..e572447ab4 100644 --- a/packages/server/src/tests/utilities/structures.ts +++ b/packages/server/src/tests/utilities/structures.ts @@ -30,6 +30,7 @@ import { BBReferenceFieldSubType, JsonFieldSubType, AutoFieldSubType, + Role, } from "@budibase/types" import { LoopInput } from "../../definitions/automations" import { merge } from "lodash" @@ -492,11 +493,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/types/src/api/web/role.ts b/packages/types/src/api/web/role.ts index c37dee60e0..642f815cc4 100644 --- a/packages/types/src/api/web/role.ts +++ b/packages/types/src/api/web/role.ts @@ -4,9 +4,9 @@ export interface SaveRoleRequest { _id?: string _rev?: string name: string - inherits: string + inherits?: string | string[] permissionId: string - version: string + version?: string } export interface SaveRoleResponse extends Role {} diff --git a/packages/types/src/documents/app/role.ts b/packages/types/src/documents/app/role.ts index f32ba810b0..669e8f523c 100644 --- a/packages/types/src/documents/app/role.ts +++ b/packages/types/src/documents/app/role.ts @@ -2,7 +2,7 @@ import { Document } from "../document" export interface Role extends Document { permissionId: string - inherits?: string + inherits?: string | string[] permissions: { [key: string]: string[] } 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 { From 10e187014a30a6b5b3703a90735f1984f0d5f632 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 26 Sep 2024 18:02:32 +0100 Subject: [PATCH 02/89] Fixing type issues. --- packages/types/src/api/web/role.ts | 2 +- packages/types/src/documents/app/role.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/types/src/api/web/role.ts b/packages/types/src/api/web/role.ts index 9a768e3ac3..4e56f6cd14 100644 --- a/packages/types/src/api/web/role.ts +++ b/packages/types/src/api/web/role.ts @@ -6,7 +6,7 @@ export interface SaveRoleRequest { name: string inherits?: string | string[] permissionId: string - version: string + 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 From fa9bb030c923dd62561388e2e3fb4000d8c952d3 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 26 Sep 2024 18:26:35 +0100 Subject: [PATCH 03/89] Utility for detecting loops in a list of roles. --- packages/shared-core/src/helpers/roles.ts | 47 ++++++++++++++ .../src/helpers/tests/roles.spec.ts | 61 +++++++++++++++++++ 2 files changed, 108 insertions(+) create mode 100644 packages/shared-core/src/helpers/roles.ts create mode 100644 packages/shared-core/src/helpers/tests/roles.spec.ts diff --git a/packages/shared-core/src/helpers/roles.ts b/packages/shared-core/src/helpers/roles.ts new file mode 100644 index 0000000000..27f28cff82 --- /dev/null +++ b/packages/shared-core/src/helpers/roles.ts @@ -0,0 +1,47 @@ +import { Role } from "@budibase/types" + +// 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 { + if (checking.has(roleId)) { + return true + } + if (checked.has(roleId)) { + return false + } + + checking.add(roleId) + + const role = 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..7e52ded3f6 --- /dev/null +++ b/packages/shared-core/src/helpers/tests/roles.spec.ts @@ -0,0 +1,61 @@ +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) + }) + }) +}) From d6d4da221da18029f635ab5baf6d9728e9fa9fba Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 27 Sep 2024 17:05:03 +0100 Subject: [PATCH 04/89] Updating role validator. --- packages/server/src/api/routes/utils/validators.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index b589d44b31..925593a8cd 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -226,7 +226,10 @@ export function roleValidator() { ) ) .optional(), - inherits: OPTIONAL_STRING, + inherits: Joi.alternatives().try( + OPTIONAL_STRING, + Joi.array().items(OPTIONAL_STRING) + ), }).unknown(true) ) } From 5f91c7d8da4960b7fd45d8999000e4f8bc836749 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 10 Oct 2024 16:11:03 +0100 Subject: [PATCH 05/89] new test case. --- .../server/src/api/routes/tests/role.spec.ts | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/packages/server/src/api/routes/tests/role.spec.ts b/packages/server/src/api/routes/tests/role.spec.ts index 127be789b9..6531461e43 100644 --- a/packages/server/src/api/routes/tests/role.spec.ts +++ b/packages/server/src/api/routes/tests/role.spec.ts @@ -161,4 +161,37 @@ describe("/roles", () => { expect(res[2]).toBe("PUBLIC") }) }) + + describe("accessible - multi-inheritance", () => { + it("should list access correctly for multi-inheritance role", async () => { + const role1 = "custom_role_1", + role2 = "custom_role_2", + role3 = "custom_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: role1, + permissionId: permissions.BuiltinPermissionID.READ_ONLY, + version: "name", + }) + const headers = await config.roleHeaders({ + roleId: role3, + }) + const res = await config.api.roles.accessible(headers, { + status: 200, + }) + expect(res.length).toBe(4) + }) + }) }) From 324616be59e4832b6026edbeae73fff0d9b9d853 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 10 Oct 2024 18:15:23 +0100 Subject: [PATCH 06/89] Finishing multi-inheritance test case and getting accessibility to be detected correctly. --- packages/backend-core/src/security/roles.ts | 121 ++++++++++++------ .../server/src/api/routes/tests/role.spec.ts | 9 +- packages/shared-core/src/helpers/index.ts | 1 + 3 files changed, 86 insertions(+), 45 deletions(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index 108bc0414c..9d23463fa3 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -9,7 +9,7 @@ import { import { getAppDB } from "../context" import { Screen, Role as RoleDoc, RoleUIMetadata } from "@budibase/types" import cloneDeep from "lodash/fp/cloneDeep" -import { RoleColor } from "@budibase/shared-core" +import { RoleColor, helpers } from "@budibase/shared-core" export const BUILTIN_ROLE_IDS = { ADMIN: "ADMIN", @@ -204,6 +204,36 @@ export function lowerBuiltinRoleID(roleId1?: string, roleId2?: string): string { : roleId1 } +/** + * 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 { + // 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 && role._id === getExternalRoleID(roleId, role.version) + ) + if (!dbRole && !isBuiltin(roleId) && opts?.defaultPublic) { + return cloneDeep(BUILTIN_ROLES.PUBLIC) + } + if (!dbRole && (!role || Object.keys(role).length === 0)) { + throw new Error("Role could not be found") + } + role = Object.assign(role || {}, dbRole) + // finalise the ID + role._id = getExternalRoleID(role._id!, role.version) + return 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. @@ -215,29 +245,15 @@ 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 + 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) } /** @@ -247,13 +263,14 @@ async function getAllUserRoles( userRoleId: string, opts?: { defaultPublic?: boolean } ): Promise { + const allRoles = await getAllRoles() + if (helpers.roles.checkForRoleInheritanceLoops(allRoles)) { + throw new Error("Loop detected in roles - cannot list roles") + } // 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] const rolesFound = (ids: string | string[]) => { if (Array.isArray(ids)) { return ids.filter(id => roleIds.includes(id)).length === ids.length @@ -261,23 +278,49 @@ async function getAllUserRoles( return roleIds.includes(ids) } } - // get all the inherited roles - while ( - currentRole && - currentRole.inherits && - !rolesFound(currentRole.inherits) - ) { - if (Array.isArray(currentRole.inherits)) { - // TODO: role inheritance + + const roleIds = [userRoleId] + const roles: RoleDoc[] = [] + const iterateInherited = (role: RoleDoc) => { + if (!role || !role._id) { + return + } + roleIds.push(role._id) + roles.push(role) + if (Array.isArray(role.inherits)) { + role.inherits.forEach(roleId => { + const foundRole = findRole(roleId, allRoles, opts) + if (foundRole) { + iterateInherited(foundRole) + } + }) } else { - roleIds.push(currentRole.inherits) - currentRole = await getRole(currentRole.inherits) - if (currentRole) { - roles.push(currentRole) + while (role && role.inherits && !rolesFound(role.inherits)) { + if (Array.isArray(role.inherits)) { + iterateInherited(role) + break + } else { + roleIds.push(role.inherits) + role = findRole(role.inherits, allRoles, opts) + if (role) { + roles.push(role) + } + } } } } - return roles + + // get all the inherited roles + iterateInherited(findRole(userRoleId, allRoles, opts)) + const foundRoleIds: string[] = [] + return roles.filter(role => { + if (role._id && !foundRoleIds.includes(role._id)) { + foundRoleIds.push(role._id) + return true + } else { + return false + } + }) } export async function getUserRoleIdHierarchy( diff --git a/packages/server/src/api/routes/tests/role.spec.ts b/packages/server/src/api/routes/tests/role.spec.ts index f9eb1c3651..682ebf2f7a 100644 --- a/packages/server/src/api/routes/tests/role.spec.ts +++ b/packages/server/src/api/routes/tests/role.spec.ts @@ -155,10 +155,7 @@ describe("/roles", () => { status: 200, } ) - expect(res.length).toBe(3) - expect(res[0]).toBe(customRoleName) - expect(res[1]).toBe("BASIC") - expect(res[2]).toBe("PUBLIC") + expect(res).toEqual([customRoleName, "BASIC", "PUBLIC"]) }) }) @@ -181,7 +178,7 @@ describe("/roles", () => { }) await config.api.roles.save({ name: role3, - inherits: role1, + inherits: [roleId1!, roleId2!], permissionId: permissions.BuiltinPermissionID.READ_ONLY, version: "name", }) @@ -191,7 +188,7 @@ describe("/roles", () => { const res = await config.api.roles.accessible(headers, { status: 200, }) - expect(res.length).toBe(4) + expect(res).toEqual([role3, role1, "BASIC", "PUBLIC", role2, "POWER"]) }) }) }) 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" From 676cb3f92e9dcc49b7f2a5de1177d34217bf06f2 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 14 Oct 2024 18:00:41 +0100 Subject: [PATCH 07/89] Handling role numbering. --- packages/backend-core/src/security/roles.ts | 29 +++++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index 9d23463fa3..3c36a91c48 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -157,7 +157,7 @@ export function builtinRoleToNumber(id: string) { break } if (Array.isArray(role.inherits)) { - // TODO: role inheritance + throw new Error("Built-in roles don't support multi-inheritance") } else { role = builtins[role.inherits!] } @@ -176,17 +176,36 @@ export async function roleToNumber(id: string) { const hierarchy = (await getUserRoleHierarchy(id, { defaultPublic: true, })) as RoleDoc[] - for (let role of hierarchy) { + const findNumber = (role: RoleDoc): number => { if (!role.inherits) { - continue + return 0 } if (Array.isArray(role.inherits)) { - // TODO: role inheritance + // 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 + let highest = 0 + for (let role of hierarchy) { + const roleNumber = findNumber(role) + highest = Math.max(roleNumber, highest) + } + return highest } /** From 26ee50b10b54534022c63262b948fbcbafca57f1 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 14 Oct 2024 18:57:46 +0100 Subject: [PATCH 08/89] Adding test case for multi-inheritance --- .../src/api/routes/tests/permissions.spec.ts | 82 ++++++++++++++++++- .../src/tests/utilities/TestConfiguration.ts | 28 +++++++ 2 files changed, 109 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/routes/tests/permissions.spec.ts b/packages/server/src/api/routes/tests/permissions.spec.ts index a479adb4cf..46ce656459 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,86 @@ describe("/permission", () => { }) }) + describe("multi-inheritance permissions", () => { + let table1: Table, table2: Table, role1: Role, role2: Role + beforeEach(async () => { + 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: "role1", + permissionId: PermissionLevel.WRITE, + inherits: BUILTIN_ROLE_IDS.BASIC, + }, + { status: 200 } + ) + role2 = await config.api.roles.save( + { + name: "role2", + 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.setRole(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.setRole(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/tests/utilities/TestConfiguration.ts b/packages/server/src/tests/utilities/TestConfiguration.ts index f320df2ff8..a3f2fb7adc 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.ts +++ b/packages/server/src/tests/utilities/TestConfiguration.ts @@ -428,6 +428,34 @@ export default class TestConfiguration { // HEADERS + // sets the role for the headers, for the period of a callback + async setRole(roleId: string, cb: () => Promise) { + const roleUser = await this.createUser({ + roles: { + [this.prodAppId!]: roleId, + }, + builder: { global: false }, + admin: { global: false }, + }) + await this.login({ + roleId, + userId: roleUser._id!, + builder: false, + prodApp: true, + }) + const temp = this.user + this.user = roleUser + await cb() + if (temp) { + this.user = temp + await this.login({ + userId: temp._id!, + builder: true, + prodApp: false, + }) + } + } + defaultHeaders(extras = {}, prodApp = false) { const tenantId = this.getTenantId() const user = this.getUser() From a56a22804240e973d8c46f93b7a2e327b83b7136 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 14 Oct 2024 18:57:54 +0100 Subject: [PATCH 09/89] Fixes based on test case. --- packages/backend-core/src/security/roles.ts | 2 +- packages/server/src/tests/utilities/api/role.ts | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index 3c36a91c48..cd55e2f728 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -516,7 +516,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) diff --git a/packages/server/src/tests/utilities/api/role.ts b/packages/server/src/tests/utilities/api/role.ts index 31bffc6f85..05165cd38e 100644 --- a/packages/server/src/tests/utilities/api/role.ts +++ b/packages/server/src/tests/utilities/api/role.ts @@ -22,6 +22,10 @@ export class RoleAPI extends TestAPI { } save = async (body: SaveRoleRequest, expectations?: Expectations) => { + // the tests should always be creating the "new" version of roles + if (body.version === undefined) { + body.version = "name" + } return await this._post(`/api/roles`, { body, expectations, From 76d0107d4da9ae180b3b9f030330cbe82a0d3a57 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 15 Oct 2024 10:10:15 +0200 Subject: [PATCH 10/89] Handle empty relationships --- packages/backend-core/src/sql/sql.ts | 81 +++++++++++++++++++++------- 1 file changed, 61 insertions(+), 20 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index b415a6f1b7..b35b2b5ec8 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -23,12 +23,14 @@ import { InternalSearchFilterOperator, JsonFieldMetadata, JsonTypes, + LogicalOperator, Operation, prefixed, QueryJson, QueryOptions, RangeOperator, RelationshipsJson, + SearchFilterKey, SearchFilters, SortOrder, SqlClient, @@ -96,6 +98,22 @@ function isSqs(table: Table): boolean { ) } +const allowEmptyRelationships: Record = { + [BasicOperator.EQUAL]: false, + [BasicOperator.NOT_EQUAL]: true, + [BasicOperator.EMPTY]: false, + [BasicOperator.NOT_EMPTY]: true, + [BasicOperator.FUZZY]: false, + [BasicOperator.STRING]: false, + [RangeOperator.RANGE]: false, + [ArrayOperator.CONTAINS]: false, + [ArrayOperator.NOT_CONTAINS]: true, + [ArrayOperator.CONTAINS_ANY]: false, + [ArrayOperator.ONE_OF]: false, + [LogicalOperator.AND]: false, + [LogicalOperator.OR]: false, +} + class InternalBuilder { private readonly client: SqlClient private readonly query: QueryJson @@ -405,6 +423,7 @@ class InternalBuilder { addRelationshipForFilter( query: Knex.QueryBuilder, + allowEmptyRelationships: boolean, filterKey: string, whereCb: (query: Knex.QueryBuilder) => Knex.QueryBuilder ): Knex.QueryBuilder { @@ -426,9 +445,10 @@ class InternalBuilder { relationship.to && relationship.tableName ) { - let subQuery = mainKnex + const joinTable = mainKnex .select(mainKnex.raw(1)) .from({ [toAlias]: relatedTableName }) + let subQuery = joinTable.clone() const manyToMany = validateManyToMany(relationship) if (manyToMany) { const throughAlias = @@ -440,7 +460,6 @@ class InternalBuilder { subQuery = subQuery // add a join through the junction table .innerJoin(throughTable, function () { - // @ts-ignore this.on( `${toAlias}.${manyToMany.toPrimary}`, "=", @@ -460,18 +479,33 @@ class InternalBuilder { if (this.client === SqlClient.SQL_LITE) { subQuery = this.addJoinFieldCheck(subQuery, manyToMany) } + + query = query.whereExists(whereCb(subQuery)) + if (allowEmptyRelationships) { + query = query.orWhereNotExists( + joinTable.clone().innerJoin(throughTable, function () { + this.on( + `${fromAlias}.${manyToMany.fromPrimary}`, + "=", + `${throughAlias}.${manyToMany.from}` + ) + }) + ) + } } else { + const foreignKey = `${fromAlias}.${relationship.from}` // "join" to the main table, making sure the ID matches that of the main subQuery = subQuery.where( `${toAlias}.${relationship.to}`, "=", - mainKnex.raw( - this.quotedIdentifier(`${fromAlias}.${relationship.from}`) - ) + mainKnex.raw(this.quotedIdentifier(foreignKey)) ) + + query = query.whereExists(whereCb(subQuery)) + if (allowEmptyRelationships) { + query = query.orWhereNull(foreignKey) + } } - query = query.whereExists(whereCb(subQuery)) - break } } return query @@ -502,6 +536,7 @@ class InternalBuilder { } function iterate( structure: AnySearchFilter, + operation: SearchFilterKey, fn: ( query: Knex.QueryBuilder, key: string, @@ -558,9 +593,14 @@ class InternalBuilder { if (allOr) { query = query.or } - query = builder.addRelationshipForFilter(query, updatedKey, q => { - return handleRelationship(q, updatedKey, value) - }) + query = builder.addRelationshipForFilter( + query, + allowEmptyRelationships[operation], + updatedKey, + q => { + return handleRelationship(q, updatedKey, value) + } + ) } } } @@ -592,7 +632,7 @@ class InternalBuilder { return `[${value.join(",")}]` } if (this.client === SqlClient.POSTGRES) { - iterate(mode, (q, key, value) => { + iterate(mode, ArrayOperator.CONTAINS, (q, key, value) => { const wrap = any ? "" : "'" const op = any ? "\\?| array" : "@>" const fieldNames = key.split(/\./g) @@ -610,7 +650,7 @@ class InternalBuilder { this.client === SqlClient.MARIADB ) { const jsonFnc = any ? "JSON_OVERLAPS" : "JSON_CONTAINS" - iterate(mode, (q, key, value) => { + iterate(mode, ArrayOperator.CONTAINS, (q, key, value) => { return q[rawFnc]( `${not}COALESCE(${jsonFnc}(${key}, '${stringifyArray( value @@ -619,7 +659,7 @@ class InternalBuilder { }) } else { const andOr = mode === filters?.containsAny ? " OR " : " AND " - iterate(mode, (q, key, value) => { + iterate(mode, ArrayOperator.CONTAINS, (q, key, value) => { let statement = "" const identifier = this.quotedIdentifier(key) for (let i in value) { @@ -673,6 +713,7 @@ class InternalBuilder { const fnc = allOr ? "orWhereIn" : "whereIn" iterate( filters.oneOf, + ArrayOperator.ONE_OF, (q, key: string, array) => { if (this.client === SqlClient.ORACLE) { key = this.convertClobs(key) @@ -697,7 +738,7 @@ class InternalBuilder { ) } if (filters.string) { - iterate(filters.string, (q, key, value) => { + iterate(filters.string, BasicOperator.STRING, (q, key, value) => { const fnc = allOr ? "orWhere" : "where" // postgres supports ilike, nothing else does if (this.client === SqlClient.POSTGRES) { @@ -712,10 +753,10 @@ class InternalBuilder { }) } if (filters.fuzzy) { - iterate(filters.fuzzy, like) + iterate(filters.fuzzy, BasicOperator.FUZZY, like) } if (filters.range) { - iterate(filters.range, (q, key, value) => { + iterate(filters.range, RangeOperator.RANGE, (q, key, value) => { const isEmptyObject = (val: any) => { return ( val && @@ -781,7 +822,7 @@ class InternalBuilder { }) } if (filters.equal) { - iterate(filters.equal, (q, key, value) => { + iterate(filters.equal, BasicOperator.EQUAL, (q, key, value) => { const fnc = allOr ? "orWhereRaw" : "whereRaw" if (this.client === SqlClient.MS_SQL) { return q[fnc]( @@ -801,7 +842,7 @@ class InternalBuilder { }) } if (filters.notEqual) { - iterate(filters.notEqual, (q, key, value) => { + iterate(filters.notEqual, BasicOperator.NOT_EQUAL, (q, key, value) => { const fnc = allOr ? "orWhereRaw" : "whereRaw" if (this.client === SqlClient.MS_SQL) { return q[fnc]( @@ -822,13 +863,13 @@ class InternalBuilder { }) } if (filters.empty) { - iterate(filters.empty, (q, key) => { + iterate(filters.empty, BasicOperator.EMPTY, (q, key) => { const fnc = allOr ? "orWhereNull" : "whereNull" return q[fnc](key) }) } if (filters.notEmpty) { - iterate(filters.notEmpty, (q, key) => { + iterate(filters.notEmpty, BasicOperator.NOT_EMPTY, (q, key) => { const fnc = allOr ? "orWhereNotNull" : "whereNotNull" return q[fnc](key) }) From 57da952f6944b5874fe20e4539ce33b5ac2c3055 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 15 Oct 2024 11:34:35 +0200 Subject: [PATCH 11/89] Fix "parenthesis" --- packages/backend-core/src/sql/sql.ts | 30 +++++++++++++--------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index b35b2b5ec8..3e6032fc99 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -480,18 +480,20 @@ class InternalBuilder { subQuery = this.addJoinFieldCheck(subQuery, manyToMany) } - query = query.whereExists(whereCb(subQuery)) - if (allowEmptyRelationships) { - query = query.orWhereNotExists( - joinTable.clone().innerJoin(throughTable, function () { - this.on( - `${fromAlias}.${manyToMany.fromPrimary}`, - "=", - `${throughAlias}.${manyToMany.from}` - ) - }) - ) - } + query = query.where(q => { + q.whereExists(whereCb(subQuery)) + if (allowEmptyRelationships) { + q.orWhereNotExists( + joinTable.clone().innerJoin(throughTable, function () { + this.on( + `${fromAlias}.${manyToMany.fromPrimary}`, + "=", + `${throughAlias}.${manyToMany.from}` + ) + }) + ) + } + }) } else { const foreignKey = `${fromAlias}.${relationship.from}` // "join" to the main table, making sure the ID matches that of the main @@ -1265,12 +1267,10 @@ class InternalBuilder { }) : undefined if (!throughTable) { - // @ts-ignore query = query.leftJoin(toTableWithSchema, function () { for (let relationship of columns) { const from = relationship.from, to = relationship.to - // @ts-ignore this.orOn(`${fromAlias}.${from}`, "=", `${toAlias}.${to}`) } }) @@ -1281,7 +1281,6 @@ class InternalBuilder { for (let relationship of columns) { const fromPrimary = relationship.fromPrimary const from = relationship.from - // @ts-ignore this.orOn( `${fromAlias}.${fromPrimary}`, "=", @@ -1293,7 +1292,6 @@ class InternalBuilder { for (let relationship of columns) { const toPrimary = relationship.toPrimary const to = relationship.to - // @ts-ignore this.orOn(`${toAlias}.${toPrimary}`, `${throughAlias}.${to}`) } }) From 5782b20509e3f3ea209e692d15ce2446940d7bef Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 15 Oct 2024 11:36:29 +0200 Subject: [PATCH 12/89] Update test --- packages/server/src/api/routes/tests/search.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 3ab35c9294..23fb5a056f 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2458,7 +2458,7 @@ describe.each([ }).toContainExactly([ { name: "foo", productCat: [{ _id: productCatRows[0]._id }] }, { name: "bar", productCat: [{ _id: productCatRows[1]._id }] }, - // { name: "baz", productCat: undefined }, // TODO + { name: "baz", productCat: undefined }, ]) }) @@ -2506,7 +2506,7 @@ describe.each([ }).toContainExactly([ { name: "foo", productCat: [{ _id: productCatRows[0]._id }] }, { name: "bar", productCat: [{ _id: productCatRows[1]._id }] }, - // { name: "baz", productCat: undefined }, // TODO + { name: "baz", productCat: undefined }, ]) } ) @@ -2530,7 +2530,7 @@ describe.each([ }).toContainExactly([ { name: "foo", productCat: [{ _id: productCatRows[0]._id }] }, { name: "bar", productCat: [{ _id: productCatRows[1]._id }] }, - // { name: "baz", productCat: undefined }, // TODO + { name: "baz", productCat: undefined }, ]) }) }) From 975945a23a0b9d30a2b448fc9af73ec75f1f56ae Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 15 Oct 2024 12:03:23 +0200 Subject: [PATCH 13/89] Improve tests --- .../src/api/routes/tests/search.spec.ts | 28 +++++++++++++++---- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 23fb5a056f..798157b122 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2278,12 +2278,16 @@ describe.each([ // It also can't work for in-memory searching because the related table name // isn't available. !isInMemory && - describe("relations", () => { + describe.each([ + RelationshipType.ONE_TO_MANY, + RelationshipType.MANY_TO_ONE, + RelationshipType.MANY_TO_MANY, + ])("relations", relationshipType => { let productCategoryTable: Table, productCatRows: Row[] beforeAll(async () => { const { relatedTable, tableId } = await basicRelationshipTables( - RelationshipType.ONE_TO_MANY + relationshipType ) tableOrViewId = tableId productCategoryTable = relatedTable @@ -2380,7 +2384,10 @@ describe.each([ ], }, }).toContainExactly([ - { name: "foo", productCat: [{ _id: productCatRows[0]._id }] }, + { + name: "foo", + productCat: [{ _id: productCatRows[0]._id }], + }, ]) } ) @@ -2480,7 +2487,10 @@ describe.each([ ], }, }).toContainExactly([ - { name: "foo", productCat: [{ _id: productCatRows[0]._id }] }, + { + name: "foo", + productCat: [{ _id: productCatRows[0]._id }], + }, ]) } ) @@ -2504,8 +2514,14 @@ describe.each([ ], }, }).toContainExactly([ - { name: "foo", productCat: [{ _id: productCatRows[0]._id }] }, - { name: "bar", productCat: [{ _id: productCatRows[1]._id }] }, + { + name: "foo", + productCat: [{ _id: productCatRows[0]._id }], + }, + { + name: "bar", + productCat: [{ _id: productCatRows[1]._id }], + }, { name: "baz", productCat: undefined }, ]) } From bb43049d55e5a43d9019ca0d6dae54d6552ed1a6 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 15 Oct 2024 12:07:31 +0100 Subject: [PATCH 14/89] Adding loop protection. --- packages/server/src/api/controllers/role.ts | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index f3fd5d7b46..76638eea23 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 = { @@ -81,9 +81,10 @@ export async function save(ctx: UserCtx) { _id = dbCore.prefixRoleID(_id) } + const allRoles = await roles.getAllRoles() 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,6 +98,18 @@ export async function save(ctx: UserCtx) { if (dbRole?.permissions && !role.permissions) { role.permissions = dbRole.permissions } + + // 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 = ctx.request.body._rev || dbRole?._rev if (foundRev) { role._rev = foundRev From 41405ea39db26b03fbde5664add1cb116cc74467 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 15 Oct 2024 14:15:45 +0200 Subject: [PATCH 15/89] Add extra rows --- .../src/api/routes/tests/search.spec.ts | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 3ab35c9294..ece84435bf 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2609,6 +2609,21 @@ describe.each([ related1: [relatedRows[2]._id!], related2: [relatedRows[3]._id!], }), + config.api.row.save(tableOrViewId, { + name: "test3", + related1: [], + related2: [], + }), + config.api.row.save(tableOrViewId, { + name: "test4", + related1: [relatedRows[1]._id!], + related2: [], + }), + config.api.row.save(tableOrViewId, { + name: "test5", + related1: [], + related2: [relatedRows[1]._id!], + }), ]) }) @@ -2626,6 +2641,17 @@ describe.each([ related1: [{ _id: relatedRows[2]._id }], related2: [{ _id: relatedRows[3]._id }], }, + { + name: "test3", + }, + { + name: "test4", + related1: [{ _id: relatedRows[1]._id }], + }, + { + name: "test5", + related2: [{ _id: relatedRows[1]._id! }], + }, ]) }) From 808096f31b8e0cc96ea186b8baaa76fb8fe24483 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 15 Oct 2024 14:34:32 +0100 Subject: [PATCH 16/89] Updating screen test case. --- .../src/api/routes/tests/screen.spec.js | 103 ------------------ .../src/api/routes/tests/screen.spec.ts | 94 ++++++++++++++++ .../server/src/tests/utilities/api/screen.ts | 23 ++++ 3 files changed, 117 insertions(+), 103 deletions(-) delete mode 100644 packages/server/src/api/routes/tests/screen.spec.js create mode 100644 packages/server/src/api/routes/tests/screen.spec.ts 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..73c3062058 --- /dev/null +++ b/packages/server/src/api/routes/tests/screen.spec.ts @@ -0,0 +1,94 @@ +import { checkBuilderEndpoint } from "./utilities/TestFunctions" +import * as setup from "./utilities" +import { events } from "@budibase/backend-core" +import { Screen } 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("save", () => { + const saveScreen = async (screen: Screen) => { + return await config.api.screen.save(screen, { status: 200 }) + } + + it("should be able to create a screen", async () => { + jest.clearAllMocks() + + const screen = basicScreen() + const responseScreen = await saveScreen(screen) + + 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 saveScreen(screen) + screen._id = responseScreen._id + screen._rev = responseScreen._rev + screen.name = "edit" + jest.clearAllMocks() + + responseScreen = await saveScreen(screen) + + 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/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, + } + ) + } } From 6688ccf1ab29ef958075d17ccc7df1b46d467a20 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 15 Oct 2024 15:50:52 +0200 Subject: [PATCH 17/89] Ensure we replace only on when starting with --- packages/server/src/sdk/app/rows/search/filters.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/server/src/sdk/app/rows/search/filters.ts b/packages/server/src/sdk/app/rows/search/filters.ts index 4049fc5352..64656b1a37 100644 --- a/packages/server/src/sdk/app/rows/search/filters.ts +++ b/packages/server/src/sdk/app/rows/search/filters.ts @@ -53,8 +53,12 @@ export function updateFilterKeys( ) if (possibleKey && possibleKey.original !== possibleKey.updated) { // only replace the first, not replaceAll - filter[key.replace(possibleKey.original, possibleKey.updated)] = - filter[key] + filter[ + key.replace( + new RegExp(`^${possibleKey.original}\\.`), + `${possibleKey.updated}.` + ) + ] = filter[key] delete filter[key] } } From 225d062fccce09ba68e9c431dbfaa04fd6ff6b8b Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 15 Oct 2024 16:30:10 +0200 Subject: [PATCH 18/89] Fix --- packages/server/src/sdk/app/rows/search/filters.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/server/src/sdk/app/rows/search/filters.ts b/packages/server/src/sdk/app/rows/search/filters.ts index 64656b1a37..9fab045554 100644 --- a/packages/server/src/sdk/app/rows/search/filters.ts +++ b/packages/server/src/sdk/app/rows/search/filters.ts @@ -44,10 +44,11 @@ export function updateFilterKeys( if (!isPlainObject(filter)) { continue } - for (let [key, keyFilter] of Object.entries(filter)) { + for (const [key, keyFilter] of Object.entries(filter)) { if (keyFilter === "") { delete filter[key] } + const possibleKey = updates.find(({ original }) => key.match(makeFilterKeyRegex(original)) ) @@ -55,8 +56,8 @@ export function updateFilterKeys( // only replace the first, not replaceAll filter[ key.replace( - new RegExp(`^${possibleKey.original}\\.`), - `${possibleKey.updated}.` + new RegExp(`^(\\d+:)?${possibleKey.original}\\.`), + `$1${possibleKey.updated}.` ) ] = filter[key] delete filter[key] From 86e5718705df0c3fb8c3eaa3aa5577435280eb6f Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 15 Oct 2024 17:40:03 +0200 Subject: [PATCH 19/89] Fix tests --- packages/server/src/integrations/tests/sqlAlias.spec.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/server/src/integrations/tests/sqlAlias.spec.ts b/packages/server/src/integrations/tests/sqlAlias.spec.ts index 890c8c4663..ddc84ae1ce 100644 --- a/packages/server/src/integrations/tests/sqlAlias.spec.ts +++ b/packages/server/src/integrations/tests/sqlAlias.spec.ts @@ -78,8 +78,7 @@ describe("Captures of real examples", () => { bindings: ["assembling", primaryLimit, relationshipLimit], sql: expect.stringContaining( multiline( - `where exists (select 1 from "tasks" as "b" inner join "products_tasks" as "c" on "b"."taskid" = "c"."taskid" where "c"."productid" = "a"."productid" - and (COALESCE("b"."taskname" = $1, FALSE))` + `where (exists (select 1 from "tasks" as "b" inner join "products_tasks" as "c" on "b"."taskid" = "c"."taskid" where "c"."productid" = "a"."productid" and (COALESCE("b"."taskname" = $1, FALSE)))` ) ), }) @@ -133,6 +132,8 @@ describe("Captures of real examples", () => { expect(query).toEqual({ bindings: [ + rangeValue.low, + rangeValue.high, rangeValue.low, rangeValue.high, equalValue, @@ -144,7 +145,7 @@ describe("Captures of real examples", () => { ], sql: expect.stringContaining( multiline( - `where exists (select 1 from "persons" as "c" where "c"."personid" = "a"."executorid" and ("c"."year" between $1 and $2))` + `where exists (select 1 from "persons" as "c" where "c"."personid" = "a"."executorid" and ("c"."year" between $1 and $2)) and exists (select 1 from "persons" as "c" where "c"."personid" = "a"."qaid" and ("c"."year" between $3 and $4)) and (exists (select 1 from "products" as "b" inner join "products_tasks" as "d" on "b"."productid" = "d"."productid" where "d"."taskid" = "a"."taskid" and (COALESCE("b"."productname" = $5, FALSE)))` ) ), }) From 8a6dbef24938d99f448e1fb3aa9e0be6224f036c Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 15 Oct 2024 18:50:58 +0200 Subject: [PATCH 20/89] Fix sqs --- packages/backend-core/src/sql/sql.ts | 34 ++++++++++++++----- .../src/sdk/app/rows/search/internal/sqs.ts | 33 ++---------------- 2 files changed, 28 insertions(+), 39 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index b415a6f1b7..83a4fd0d59 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -406,23 +406,26 @@ class InternalBuilder { addRelationshipForFilter( query: Knex.QueryBuilder, filterKey: string, - whereCb: (query: Knex.QueryBuilder) => Knex.QueryBuilder + whereCb: (filterKey: string, query: Knex.QueryBuilder) => Knex.QueryBuilder ): Knex.QueryBuilder { const mainKnex = this.knex const { relationships, endpoint, tableAliases: aliases } = this.query const tableName = endpoint.entityId const fromAlias = aliases?.[tableName] || tableName - const matches = (possibleTable: string) => - filterKey.startsWith(`${possibleTable}`) + const matches = (value: string) => filterKey.startsWith(`${value}`) if (!relationships) { return query } for (const relationship of relationships) { const relatedTableName = relationship.tableName const toAlias = aliases?.[relatedTableName] || relatedTableName + + const matchesTableName = matches(relatedTableName) || matches(toAlias) + const matchesRelationName = matches(relationship.column) + // this is the relationship which is being filtered if ( - (matches(relatedTableName) || matches(toAlias)) && + (matchesTableName || matchesRelationName) && relationship.to && relationship.tableName ) { @@ -430,6 +433,17 @@ class InternalBuilder { .select(mainKnex.raw(1)) .from({ [toAlias]: relatedTableName }) const manyToMany = validateManyToMany(relationship) + let updatedKey + + if (matchesRelationName) { + updatedKey = filterKey.replace( + new RegExp(`^${relationship.column}.`), + `${aliases![relationship.tableName]}.` + ) + } else { + updatedKey = filterKey + } + if (manyToMany) { const throughAlias = aliases?.[manyToMany.through] || relationship.through @@ -470,7 +484,7 @@ class InternalBuilder { ) ) } - query = query.whereExists(whereCb(subQuery)) + query = query.whereExists(whereCb(updatedKey, subQuery)) break } } @@ -558,9 +572,13 @@ class InternalBuilder { if (allOr) { query = query.or } - query = builder.addRelationshipForFilter(query, updatedKey, q => { - return handleRelationship(q, updatedKey, value) - }) + query = builder.addRelationshipForFilter( + query, + updatedKey, + (updatedKey, q) => { + return handleRelationship(q, updatedKey, value) + } + ) } } } diff --git a/packages/server/src/sdk/app/rows/search/internal/sqs.ts b/packages/server/src/sdk/app/rows/search/internal/sqs.ts index 916e20957b..e3bbca271e 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -39,11 +39,6 @@ import AliasTables from "../../sqlAlias" import { outputProcessing } from "../../../../../utilities/rowProcessor" import pick from "lodash/pick" import { processRowCountResponse } from "../../utils" -import { - getRelationshipColumns, - getTableIDList, - updateFilterKeys, -} from "../filters" import { dataFilters, helpers, @@ -133,31 +128,7 @@ async function buildInternalFieldList( return [...new Set(fieldList)] } -function cleanupFilters( - filters: SearchFilters, - table: Table, - allTables: Table[] -) { - // get a list of all relationship columns in the table for updating - const relationshipColumns = getRelationshipColumns(table) - // get table names to ID map for relationships - const tableNameToID = getTableIDList(allTables) - // all should be applied at once - filters = updateFilterKeys( - filters, - relationshipColumns - .map(({ name, definition }) => ({ - original: name, - updated: definition.tableId, - })) - .concat( - tableNameToID.map(({ name, id }) => ({ - original: name, - updated: id, - })) - ) - ) - +function cleanupFilters(filters: SearchFilters, allTables: Table[]) { // generate a map of all possible column names (these can be duplicated across tables // the map of them will always be the same const userColumnMap: Record = {} @@ -356,7 +327,7 @@ export async function search( const relationships = buildInternalRelationships(table, allTables) const searchFilters: SearchFilters = { - ...cleanupFilters(query, table, allTables), + ...cleanupFilters(query, allTables), documentType: DocumentType.ROW, } From 9fceef0fc26c43090e98a52f9d0ccc08d32e3578 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 15 Oct 2024 17:53:48 +0100 Subject: [PATCH 21/89] Some more fixes and test case for screen access. --- packages/backend-core/src/security/roles.ts | 12 ++- .../src/api/routes/tests/screen.spec.ts | 94 ++++++++++++++++--- 2 files changed, 93 insertions(+), 13 deletions(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index cd55e2f728..41a55c55bd 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -223,6 +223,11 @@ export function lowerBuiltinRoleID(roleId1?: string, roleId2?: string): string { : roleId1 } +function compareRoleIds(roleId1: string, roleId2: string) { + // make sure both role IDs are prefixed correctly + return prefixRoleID(roleId1) === prefixRoleID(roleId2) +} + /** * Given a list of roles, this will pick the role out, accounting for built ins. */ @@ -239,7 +244,7 @@ export function findRole( roleId = prefixRoleID(roleId) } const dbRole = roles.find( - role => role._id && role._id === getExternalRoleID(roleId, role.version) + role => role._id && compareRoleIds(role._id, roleId) ) if (!dbRole && !isBuiltin(roleId) && opts?.defaultPublic) { return cloneDeep(BUILTIN_ROLES.PUBLIC) @@ -474,7 +479,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) { diff --git a/packages/server/src/api/routes/tests/screen.spec.ts b/packages/server/src/api/routes/tests/screen.spec.ts index 73c3062058..ff13d669ad 100644 --- a/packages/server/src/api/routes/tests/screen.spec.ts +++ b/packages/server/src/api/routes/tests/screen.spec.ts @@ -1,7 +1,7 @@ import { checkBuilderEndpoint } from "./utilities/TestFunctions" import * as setup from "./utilities" -import { events } from "@budibase/backend-core" -import { Screen } from "@budibase/types" +import { events, roles } from "@budibase/backend-core" +import { Screen, PermissionLevel, Role } from "@budibase/types" const { basicScreen } = setup.structures @@ -32,16 +32,89 @@ describe("/screens", () => { }) }) - describe("save", () => { - const saveScreen = async (screen: Screen) => { - return await config.api.screen.save(screen, { status: 200 }) + 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.setRole(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 create a screen", async () => { - jest.clearAllMocks() + it("should be able to fetch only screen1 with role1", async () => { + await checkScreens(role1._id!, [screen._id!, screen1._id!]) + }) + it("should be able to fetch only screen2 with role2", async () => { + await checkScreens(role2._id!, [screen._id!, screen2._id!]) + }) + + it("should be able to fetch all three screens with multi-inheritance role", async () => { + await checkScreens(multiRole._id!, [ + screen._id!, + screen1._id!, + screen2._id!, + ]) + }) + }) + + describe("save", () => { + it("should be able to create a screen", async () => { const screen = basicScreen() - const responseScreen = await saveScreen(screen) + const responseScreen = await config.api.screen.save(screen, { + status: 200, + }) expect(responseScreen._rev).toBeDefined() expect(responseScreen.name).toEqual(screen.name) @@ -50,13 +123,12 @@ describe("/screens", () => { it("should be able to update a screen", async () => { const screen = basicScreen() - let responseScreen = await saveScreen(screen) + 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 saveScreen(screen) + responseScreen = await config.api.screen.save(screen, { status: 200 }) expect(responseScreen._rev).toBeDefined() expect(responseScreen.name).toEqual(screen.name) From 171ffd8aa33893b62913a221d745df3e217c0687 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 15 Oct 2024 17:54:34 +0100 Subject: [PATCH 22/89] Update test name. --- packages/server/src/api/routes/tests/screen.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/server/src/api/routes/tests/screen.spec.ts b/packages/server/src/api/routes/tests/screen.spec.ts index ff13d669ad..f660589951 100644 --- a/packages/server/src/api/routes/tests/screen.spec.ts +++ b/packages/server/src/api/routes/tests/screen.spec.ts @@ -92,15 +92,15 @@ describe("/screens", () => { }) } - it("should be able to fetch only screen1 with role1", async () => { + 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 only screen2 with role2", async () => { + 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 all three screens with multi-inheritance role", async () => { + it("should be able to fetch basic, screen1 and screen2 with multi-inheritance role", async () => { await checkScreens(multiRole._id!, [ screen._id!, screen1._id!, From b01564c934922c0acabfd5bea53861f511163fb5 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 16 Oct 2024 10:21:17 +0200 Subject: [PATCH 23/89] Fix multiple relations to same table for external --- packages/backend-core/src/sql/sql.ts | 6 +- .../api/controllers/row/ExternalRequest.ts | 12 ---- packages/server/src/sdk/app/rows/index.ts | 2 - .../server/src/sdk/app/rows/search/filters.ts | 70 ------------------- 4 files changed, 3 insertions(+), 87 deletions(-) delete mode 100644 packages/server/src/sdk/app/rows/search/filters.ts diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index 83a4fd0d59..c47eabca57 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -412,7 +412,8 @@ class InternalBuilder { const { relationships, endpoint, tableAliases: aliases } = this.query const tableName = endpoint.entityId const fromAlias = aliases?.[tableName] || tableName - const matches = (value: string) => filterKey.startsWith(`${value}`) + const matches = (value: string) => + filterKey.match(new RegExp(`^${value}\\.`)) if (!relationships) { return query } @@ -435,7 +436,7 @@ class InternalBuilder { const manyToMany = validateManyToMany(relationship) let updatedKey - if (matchesRelationName) { + if (!matchesTableName) { updatedKey = filterKey.replace( new RegExp(`^${relationship.column}.`), `${aliases![relationship.tableName]}.` @@ -485,7 +486,6 @@ class InternalBuilder { ) } query = query.whereExists(whereCb(updatedKey, subQuery)) - break } } return query diff --git a/packages/server/src/api/controllers/row/ExternalRequest.ts b/packages/server/src/api/controllers/row/ExternalRequest.ts index 56522acb33..7ac13d3200 100644 --- a/packages/server/src/api/controllers/row/ExternalRequest.ts +++ b/packages/server/src/api/controllers/row/ExternalRequest.ts @@ -204,18 +204,6 @@ export class ExternalRequest { filters: SearchFilters, table: Table ): SearchFilters { - // replace any relationship columns initially, table names and relationship column names are acceptable - const relationshipColumns = sdk.rows.filters.getRelationshipColumns(table) - filters = sdk.rows.filters.updateFilterKeys( - filters, - relationshipColumns.map(({ name, definition }) => { - const { tableName } = breakExternalTableId(definition.tableId) - return { - original: name, - updated: tableName, - } - }) - ) const primary = table.primary // if passed in array need to copy for shifting etc let idCopy: undefined | string | any[] = cloneDeep(id) diff --git a/packages/server/src/sdk/app/rows/index.ts b/packages/server/src/sdk/app/rows/index.ts index fb077509a9..c117941419 100644 --- a/packages/server/src/sdk/app/rows/index.ts +++ b/packages/server/src/sdk/app/rows/index.ts @@ -3,14 +3,12 @@ import * as rows from "./rows" import * as search from "./search" import * as utils from "./utils" import * as external from "./external" -import * as filters from "./search/filters" import AliasTables from "./sqlAlias" export default { ...attachments, ...rows, ...search, - filters, utils, external, AliasTables, diff --git a/packages/server/src/sdk/app/rows/search/filters.ts b/packages/server/src/sdk/app/rows/search/filters.ts deleted file mode 100644 index 9fab045554..0000000000 --- a/packages/server/src/sdk/app/rows/search/filters.ts +++ /dev/null @@ -1,70 +0,0 @@ -import { - FieldType, - RelationshipFieldMetadata, - SearchFilters, - Table, -} from "@budibase/types" -import { isPlainObject } from "lodash" -import { dataFilters } from "@budibase/shared-core" - -export function getRelationshipColumns(table: Table): { - name: string - definition: RelationshipFieldMetadata -}[] { - // performing this with a for loop rather than an array filter improves - // type guarding, as no casts are required - const linkEntries: [string, RelationshipFieldMetadata][] = [] - for (let entry of Object.entries(table.schema)) { - if (entry[1].type === FieldType.LINK) { - const linkColumn: RelationshipFieldMetadata = entry[1] - linkEntries.push([entry[0], linkColumn]) - } - } - return linkEntries.map(entry => ({ - name: entry[0], - definition: entry[1], - })) -} - -export function getTableIDList( - tables: Table[] -): { name: string; id: string }[] { - return tables - .filter(table => table.originalName && table._id) - .map(table => ({ id: table._id!, name: table.originalName! })) -} - -export function updateFilterKeys( - filters: SearchFilters, - updates: { original: string; updated: string }[] -): SearchFilters { - const makeFilterKeyRegex = (str: string) => - new RegExp(`^${str}\\.|:${str}\\.`) - for (let filter of Object.values(filters)) { - if (!isPlainObject(filter)) { - continue - } - for (const [key, keyFilter] of Object.entries(filter)) { - if (keyFilter === "") { - delete filter[key] - } - - const possibleKey = updates.find(({ original }) => - key.match(makeFilterKeyRegex(original)) - ) - if (possibleKey && possibleKey.original !== possibleKey.updated) { - // only replace the first, not replaceAll - filter[ - key.replace( - new RegExp(`^(\\d+:)?${possibleKey.original}\\.`), - `$1${possibleKey.updated}.` - ) - ] = filter[key] - delete filter[key] - } - } - } - return dataFilters.recurseLogicalOperators(filters, (f: SearchFilters) => { - return updateFilterKeys(f, updates) - }) -} From 2d460f19557c866b1087634d850229e91835747b Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 16 Oct 2024 12:26:13 +0200 Subject: [PATCH 24/89] Fix supporting filter using table names --- .../server/src/api/controllers/row/index.ts | 54 ++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts index 680a7671d5..9122dc5ff3 100644 --- a/packages/server/src/api/controllers/row/index.ts +++ b/packages/server/src/api/controllers/row/index.ts @@ -15,13 +15,16 @@ import { ExportRowsResponse, FieldType, GetRowResponse, + isRelationshipField, PatchRowRequest, PatchRowResponse, Row, RowAttachment, RowSearchParams, + SearchFilters, SearchRowRequest, SearchRowResponse, + Table, UserCtx, ValidateResponse, } from "@budibase/types" @@ -33,6 +36,8 @@ import sdk from "../../../sdk" import * as exporters from "../view/exporters" import { Format } from "../view/exporters" import { apiFileReturn } from "../../../utilities/fileSystem" +import { dataFilters } from "@budibase/shared-core" +import { isPlainObject } from "lodash" export * as views from "./views" @@ -211,13 +216,16 @@ export async function search(ctx: Ctx) { await context.ensureSnippetContext(true) - const enrichedQuery = await utils.enrichSearchContext( + let enrichedQuery: SearchFilters = await utils.enrichSearchContext( { ...ctx.request.body.query }, { user: sdk.users.getUserContextBindings(ctx.user), } ) + const allTables = await sdk.tables.getAllTables() + enrichedQuery = replaceTableNamesInFilters(tableId, enrichedQuery, allTables) + const searchParams: RowSearchParams = { ...ctx.request.body, query: enrichedQuery, @@ -229,6 +237,50 @@ export async function search(ctx: Ctx) { ctx.body = await sdk.rows.search(searchParams) } +function replaceTableNamesInFilters( + tableId: string, + filters: SearchFilters, + allTables: Table[] +): SearchFilters { + for (const filter of Object.values(filters)) { + if (!isPlainObject(filter)) { + continue + } + for (const key of Object.keys(filter)) { + const matches = key.match(`^(?.+)\\.(?.+)`) + + const relation = matches?.groups?.["relation"] + const field = matches?.groups?.["field"] + + if (!relation || !field) { + continue + } + + const table = allTables.find(r => r._id === tableId)! + if (Object.values(table.schema).some(f => f.name === relation)) { + continue + } + + const matchedTable = allTables.find(t => t.name === relation) + const relationship = Object.values(table.schema).find( + f => isRelationshipField(f) && f.tableId === matchedTable?._id + ) + if (!relationship) { + continue + } + + const updatedField = `${relationship.name}.${field}` + if (updatedField && updatedField !== key) { + filter[updatedField] = filter[key] + delete filter[key] + } + } + } + return dataFilters.recurseLogicalOperators(filters, (f: SearchFilters) => { + return replaceTableNamesInFilters(tableId, f, allTables) + }) +} + export async function validate(ctx: Ctx) { const source = await utils.getSource(ctx) const table = await utils.getTableFromSource(source) From 83ffee31947284cf2b0bf7e7df2500ef26ce506c Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 16 Oct 2024 12:33:19 +0200 Subject: [PATCH 25/89] Fix tests --- .../src/api/routes/tests/search.spec.ts | 25 +++---------------- 1 file changed, 3 insertions(+), 22 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index ece84435bf..5d778ed6d2 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2614,16 +2614,6 @@ describe.each([ related1: [], related2: [], }), - config.api.row.save(tableOrViewId, { - name: "test4", - related1: [relatedRows[1]._id!], - related2: [], - }), - config.api.row.save(tableOrViewId, { - name: "test5", - related1: [], - related2: [relatedRows[1]._id!], - }), ]) }) @@ -2644,19 +2634,11 @@ describe.each([ { name: "test3", }, - { - name: "test4", - related1: [{ _id: relatedRows[1]._id }], - }, - { - name: "test5", - related2: [{ _id: relatedRows[1]._id! }], - }, ]) }) isSqs && - it("should be able to filter down to second row with equal", async () => { + it("should be able to filter via the first relation field with equal", async () => { await expectSearch({ query: { equal: { @@ -2672,11 +2654,11 @@ describe.each([ }) isSqs && - it("should be able to filter down to first row with not equal", async () => { + it("should be able to filter via the second relation field with not equal", async () => { await expectSearch({ query: { notEqual: { - ["1:related2.name"]: "bar", + ["1:related2.name"]: "foo", ["2:related2.name"]: "baz", ["3:related2.name"]: "boo", }, @@ -2684,7 +2666,6 @@ describe.each([ }).toContainExactly([ { name: "test", - related1: [{ _id: relatedRows[0]._id }], }, ]) }) From 7266fb3c1bd0cf7b9d3f344002e46602bd324677 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 16 Oct 2024 12:38:38 +0200 Subject: [PATCH 26/89] Run tests for all --- .../server/src/api/controllers/row/index.ts | 16 ++--- .../src/api/routes/tests/search.spec.ts | 59 +++++++++---------- 2 files changed, 37 insertions(+), 38 deletions(-) diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts index 9122dc5ff3..51bdb12ca2 100644 --- a/packages/server/src/api/controllers/row/index.ts +++ b/packages/server/src/api/controllers/row/index.ts @@ -216,15 +216,15 @@ export async function search(ctx: Ctx) { await context.ensureSnippetContext(true) - let enrichedQuery: SearchFilters = await utils.enrichSearchContext( - { ...ctx.request.body.query }, - { - user: sdk.users.getUserContextBindings(ctx.user), - } - ) + let { query } = ctx.request.body + if (!isPlainObject(query)) { + const allTables = await sdk.tables.getAllTables() + query = replaceTableNamesInFilters(tableId, query, allTables) + } - const allTables = await sdk.tables.getAllTables() - enrichedQuery = replaceTableNamesInFilters(tableId, enrichedQuery, allTables) + let enrichedQuery: SearchFilters = await utils.enrichSearchContext(query, { + user: sdk.users.getUserContextBindings(ctx.user), + }) const searchParams: RowSearchParams = { ...ctx.request.body, diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 5d778ed6d2..7132115d94 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2567,7 +2567,8 @@ describe.each([ expect(response.rows[0].productCat).toBeArrayOfSize(11) }) }) - ;(isSqs || isLucene) && + + isSql && describe("relations to same table", () => { let relatedTable: string, relatedRows: Row[] @@ -2637,38 +2638,36 @@ describe.each([ ]) }) - isSqs && - it("should be able to filter via the first relation field with equal", async () => { - await expectSearch({ - query: { - equal: { - ["related1.name"]: "baz", - }, + it("should be able to filter via the first relation field with equal", async () => { + await expectSearch({ + query: { + equal: { + ["related1.name"]: "baz", }, - }).toContainExactly([ - { - name: "test2", - related1: [{ _id: relatedRows[2]._id }], - }, - ]) - }) + }, + }).toContainExactly([ + { + name: "test2", + related1: [{ _id: relatedRows[2]._id }], + }, + ]) + }) - isSqs && - it("should be able to filter via the second relation field with not equal", async () => { - await expectSearch({ - query: { - notEqual: { - ["1:related2.name"]: "foo", - ["2:related2.name"]: "baz", - ["3:related2.name"]: "boo", - }, + it("should be able to filter via the second relation field with not equal", async () => { + await expectSearch({ + query: { + notEqual: { + ["1:related2.name"]: "foo", + ["2:related2.name"]: "baz", + ["3:related2.name"]: "boo", }, - }).toContainExactly([ - { - name: "test", - }, - ]) - }) + }, + }).toContainExactly([ + { + name: "test", + }, + ]) + }) }) isInternal && From 88d4ebc725f3a87cb3ad4aa2b34655cab82b1879 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 16 Oct 2024 12:44:25 +0200 Subject: [PATCH 27/89] Add more tests --- .../src/api/routes/tests/search.spec.ts | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 7132115d94..da0fd4fbba 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2612,8 +2612,8 @@ describe.each([ }), config.api.row.save(tableOrViewId, { name: "test3", - related1: [], - related2: [], + related1: [relatedRows[1]._id], + related2: [relatedRows[2]._id!], }), ]) }) @@ -2634,6 +2634,8 @@ describe.each([ }, { name: "test3", + related1: [{ _id: relatedRows[1]._id }], + related2: [{ _id: relatedRows[2]._id }], }, ]) }) @@ -2668,6 +2670,21 @@ describe.each([ }, ]) }) + + it("should be able to filter on both fields", async () => { + await expectSearch({ + query: { + notEqual: { + ["related1.name"]: "foo", + ["related2.name"]: "baz", + }, + }, + }).toContainExactly([ + { + name: "test2", + }, + ]) + }) }) isInternal && From 830ed0cda25c914cca5364a1c5de88b5c7258be6 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 16 Oct 2024 12:48:07 +0200 Subject: [PATCH 28/89] Fix undefineds --- packages/server/src/api/controllers/row/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts index 51bdb12ca2..40b41f410f 100644 --- a/packages/server/src/api/controllers/row/index.ts +++ b/packages/server/src/api/controllers/row/index.ts @@ -217,7 +217,7 @@ export async function search(ctx: Ctx) { await context.ensureSnippetContext(true) let { query } = ctx.request.body - if (!isPlainObject(query)) { + if (query && !isPlainObject(query)) { const allTables = await sdk.tables.getAllTables() query = replaceTableNamesInFilters(tableId, query, allTables) } From ba80880ab65d76d938e9746f896513528c4119a4 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 16 Oct 2024 13:02:53 +0200 Subject: [PATCH 29/89] Fix --- packages/server/src/api/controllers/row/index.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts index 40b41f410f..60775ce628 100644 --- a/packages/server/src/api/controllers/row/index.ts +++ b/packages/server/src/api/controllers/row/index.ts @@ -37,7 +37,6 @@ import * as exporters from "../view/exporters" import { Format } from "../view/exporters" import { apiFileReturn } from "../../../utilities/fileSystem" import { dataFilters } from "@budibase/shared-core" -import { isPlainObject } from "lodash" export * as views from "./views" @@ -217,7 +216,7 @@ export async function search(ctx: Ctx) { await context.ensureSnippetContext(true) let { query } = ctx.request.body - if (query && !isPlainObject(query)) { + if (query) { const allTables = await sdk.tables.getAllTables() query = replaceTableNamesInFilters(tableId, query, allTables) } @@ -243,9 +242,6 @@ function replaceTableNamesInFilters( allTables: Table[] ): SearchFilters { for (const filter of Object.values(filters)) { - if (!isPlainObject(filter)) { - continue - } for (const key of Object.keys(filter)) { const matches = key.match(`^(?.+)\\.(?.+)`) From 57ce8b7e855126c3f4bd905e7121137dd22d25ab Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 16 Oct 2024 13:17:07 +0200 Subject: [PATCH 30/89] Fix test --- packages/server/src/integrations/tests/sqlAlias.spec.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/server/src/integrations/tests/sqlAlias.spec.ts b/packages/server/src/integrations/tests/sqlAlias.spec.ts index 890c8c4663..81013b328c 100644 --- a/packages/server/src/integrations/tests/sqlAlias.spec.ts +++ b/packages/server/src/integrations/tests/sqlAlias.spec.ts @@ -78,8 +78,7 @@ describe("Captures of real examples", () => { bindings: ["assembling", primaryLimit, relationshipLimit], sql: expect.stringContaining( multiline( - `where exists (select 1 from "tasks" as "b" inner join "products_tasks" as "c" on "b"."taskid" = "c"."taskid" where "c"."productid" = "a"."productid" - and (COALESCE("b"."taskname" = $1, FALSE))` + `where exists (select 1 from "tasks" as "b" inner join "products_tasks" as "c" on "b"."taskid" = "c"."taskid" where "c"."productid" = "a"."productid" and (COALESCE("b"."taskname" = $1, FALSE))` ) ), }) @@ -133,6 +132,8 @@ describe("Captures of real examples", () => { expect(query).toEqual({ bindings: [ + rangeValue.low, + rangeValue.high, rangeValue.low, rangeValue.high, equalValue, @@ -144,7 +145,7 @@ describe("Captures of real examples", () => { ], sql: expect.stringContaining( multiline( - `where exists (select 1 from "persons" as "c" where "c"."personid" = "a"."executorid" and ("c"."year" between $1 and $2))` + `where exists (select 1 from "persons" as "c" where "c"."personid" = "a"."executorid" and ("c"."year" between $1 and $2)) and exists (select 1 from "persons" as "c" where "c"."personid" = "a"."qaid" and ("c"."year" between $3 and $4)) and exists (select 1 from "products" as "b" inner join "products_tasks" as "d" on "b"."productid" = "d"."productid" where "d"."taskid" = "a"."taskid" and (COALESCE("b"."productname" = $5, FALSE))` ) ), }) From 189b17606083e58157091300044fb9c1a0122d6e Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 16 Oct 2024 12:27:15 +0100 Subject: [PATCH 31/89] Test for loops in role save API. --- packages/server/src/api/controllers/role.ts | 5 ++++- .../server/src/api/routes/tests/role.spec.ts | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index 76638eea23..1e7f58e566 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -81,7 +81,10 @@ export async function save(ctx: UserCtx) { _id = dbCore.prefixRoleID(_id) } - const allRoles = await roles.getAllRoles() + const allRoles = (await roles.getAllRoles()).map(role => ({ + ...role, + _id: dbCore.prefixRoleID(role._id!), + })) let dbRole: Role | undefined if (!isCreate && _id?.startsWith(DocumentType.ROLE)) { dbRole = allRoles.find(role => role._id === _id) diff --git a/packages/server/src/api/routes/tests/role.spec.ts b/packages/server/src/api/routes/tests/role.spec.ts index 682ebf2f7a..396afeb236 100644 --- a/packages/server/src/api/routes/tests/role.spec.ts +++ b/packages/server/src/api/routes/tests/role.spec.ts @@ -47,6 +47,25 @@ describe("/roles", () => { expect(events.role.updated).toHaveBeenCalledTimes(1) expect(events.role.updated).toHaveBeenCalledWith(res) }) + + it("disallow loops", async () => { + let role1 = basicRole() + role1 = await config.api.roles.save(role1, { + status: 200, + }) + let role2 = basicRole() + role2.inherits = [role1._id!] + role2 = await config.api.roles.save(role2, { + status: 200, + }) + role1.inherits = [role2._id!] + await config.api.roles.save(role1, { + status: 400, + body: { + message: "Role inheritance contains a loop, this is not supported", + }, + }) + }) }) describe("fetch", () => { From 5f1cd4eb9f562f3bf4e179fbf7a5caaa8d877977 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 16 Oct 2024 13:47:09 +0200 Subject: [PATCH 32/89] Test multiple relationship types --- .../server/src/api/routes/tests/search.spec.ts | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index da0fd4fbba..0f5797f8de 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2278,12 +2278,16 @@ describe.each([ // It also can't work for in-memory searching because the related table name // isn't available. !isInMemory && - describe("relations", () => { + describe.each([ + RelationshipType.ONE_TO_MANY, + RelationshipType.MANY_TO_ONE, + RelationshipType.MANY_TO_MANY, + ])("relations (%s)", relationshipType => { let productCategoryTable: Table, productCatRows: Row[] beforeAll(async () => { const { relatedTable, tableId } = await basicRelationshipTables( - RelationshipType.ONE_TO_MANY + relationshipType ) tableOrViewId = tableId productCategoryTable = relatedTable @@ -2538,10 +2542,13 @@ describe.each([ }) isSql && - describe("big relations", () => { + describe.each([ + RelationshipType.MANY_TO_ONE, + RelationshipType.MANY_TO_MANY, + ])("big relations (%s)", relationshipType => { beforeAll(async () => { const { relatedTable, tableId } = await basicRelationshipTables( - RelationshipType.MANY_TO_ONE + relationshipType ) tableOrViewId = tableId const mainRow = await config.api.row.save(tableOrViewId, { From 3ea8e240e4716361850ddbb8c027d1fae968f531 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 16 Oct 2024 14:05:48 +0200 Subject: [PATCH 33/89] Fix one-to-many --- packages/backend-core/src/sql/sql.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index b7b499b92c..662558415e 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -518,10 +518,12 @@ class InternalBuilder { mainKnex.raw(this.quotedIdentifier(foreignKey)) ) - query = query.whereExists(whereCb(updatedKey, subQuery)) - if (allowEmptyRelationships) { - query = query.orWhereNull(foreignKey) - } + query = query.where(q => { + q.whereExists(whereCb(updatedKey, subQuery)) + if (allowEmptyRelationships) { + q.orWhereNull(foreignKey) + } + }) } } } From 3fbe937bf43443647ce6709942bd32e3717045b2 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 16 Oct 2024 14:23:58 +0200 Subject: [PATCH 34/89] Fix sqlalias test --- packages/server/src/integrations/tests/sqlAlias.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/integrations/tests/sqlAlias.spec.ts b/packages/server/src/integrations/tests/sqlAlias.spec.ts index ddc84ae1ce..739d3a4aee 100644 --- a/packages/server/src/integrations/tests/sqlAlias.spec.ts +++ b/packages/server/src/integrations/tests/sqlAlias.spec.ts @@ -145,7 +145,7 @@ describe("Captures of real examples", () => { ], sql: expect.stringContaining( multiline( - `where exists (select 1 from "persons" as "c" where "c"."personid" = "a"."executorid" and ("c"."year" between $1 and $2)) and exists (select 1 from "persons" as "c" where "c"."personid" = "a"."qaid" and ("c"."year" between $3 and $4)) and (exists (select 1 from "products" as "b" inner join "products_tasks" as "d" on "b"."productid" = "d"."productid" where "d"."taskid" = "a"."taskid" and (COALESCE("b"."productname" = $5, FALSE)))` + `where (exists (select 1 from "persons" as "c" where "c"."personid" = "a"."executorid" and ("c"."year" between $1 and $2))) and (exists (select 1 from "persons" as "c" where "c"."personid" = "a"."qaid" and ("c"."year" between $3 and $4))) and (exists (select 1 from "products" as "b" inner join "products_tasks" as "d" on "b"."productid" = "d"."productid" where "d"."taskid" = "a"."taskid" and (COALESCE("b"."productname" = $5, FALSE))))` ) ), }) From b6874f52f6d8e5228b57842a9844eba28f72e9e6 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 16 Oct 2024 16:16:39 +0200 Subject: [PATCH 35/89] Fix many-to-one --- packages/backend-core/src/sql/sql.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index 662558415e..949d7edf1b 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -510,18 +510,19 @@ class InternalBuilder { } }) } else { + const toKey = `${toAlias}.${relationship.to}` const foreignKey = `${fromAlias}.${relationship.from}` // "join" to the main table, making sure the ID matches that of the main subQuery = subQuery.where( - `${toAlias}.${relationship.to}`, + toKey, "=", mainKnex.raw(this.quotedIdentifier(foreignKey)) ) query = query.where(q => { - q.whereExists(whereCb(updatedKey, subQuery)) + q.whereExists(whereCb(updatedKey, subQuery.clone())) if (allowEmptyRelationships) { - q.orWhereNull(foreignKey) + q.orWhereNotExists(subQuery) } }) } From 8b9bb784c49d56aeb1d04b67f6a8bc048fbf430e Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 16 Oct 2024 17:06:15 +0200 Subject: [PATCH 36/89] Initial appMetadata sdk usage --- .../server/src/api/controllers/application.ts | 19 ++++++++----------- .../server/src/sdk/app/appMetadata/index.ts | 5 +++++ .../src/sdk/app/appMetadata/metadata.ts | 8 ++++++++ packages/server/src/sdk/index.ts | 2 ++ 4 files changed, 23 insertions(+), 11 deletions(-) create mode 100644 packages/server/src/sdk/app/appMetadata/index.ts create mode 100644 packages/server/src/sdk/app/appMetadata/metadata.ts diff --git a/packages/server/src/api/controllers/application.ts b/packages/server/src/api/controllers/application.ts index 830acc55bf..660d35c29d 100644 --- a/packages/server/src/api/controllers/application.ts +++ b/packages/server/src/api/controllers/application.ts @@ -208,9 +208,8 @@ export async function fetchAppDefinition( export async function fetchAppPackage( ctx: UserCtx ) { - const db = context.getAppDB() const appId = context.getAppId() - let application = await db.get(DocumentType.APP_METADATA) + const application = await sdk.appMetadata.get() const layouts = await getLayouts() let screens = await getScreens() const license = await licensing.cache.getCachedLicense() @@ -315,7 +314,7 @@ async function performAppCreate(ctx: UserCtx) { // If we used a template or imported an app there will be an existing doc. // Fetch and migrate some metadata from the existing app. try { - const existing: App = await db.get(DocumentType.APP_METADATA) + const existing = await sdk.appMetadata.get() const keys: (keyof App)[] = [ "_rev", "navigation", @@ -489,8 +488,7 @@ export async function update( export async function updateClient(ctx: UserCtx) { // Get current app version - const db = context.getAppDB() - const application = await db.get(DocumentType.APP_METADATA) + const application = await sdk.appMetadata.get() const currentVersion = application.version let manifest @@ -518,8 +516,7 @@ export async function updateClient(ctx: UserCtx) { export async function revertClient(ctx: UserCtx) { // Check app can be reverted - const db = context.getAppDB() - const application = await db.get(DocumentType.APP_METADATA) + const application = await sdk.appMetadata.get() if (!application.revertableVersion) { ctx.throw(400, "There is no version to revert to") } @@ -577,7 +574,7 @@ async function destroyApp(ctx: UserCtx) { const db = dbCore.getDB(devAppId) // standard app deletion flow - const app = await db.get(DocumentType.APP_METADATA) + const app = await sdk.appMetadata.get() const result = await db.destroy() await quotas.removeApp() await events.app.deleted(app) @@ -728,7 +725,7 @@ export async function updateAppPackage( ) { return context.doInAppContext(appId, async () => { const db = context.getAppDB() - const application = await db.get(DocumentType.APP_METADATA) + const application = await sdk.appMetadata.get() const newAppPackage: App = { ...application, ...appPackage } if (appPackage._rev !== application._rev) { @@ -754,7 +751,7 @@ export async function setRevertableVersion( return } const db = context.getAppDB() - const app = await db.get(DocumentType.APP_METADATA) + const app = await sdk.appMetadata.get() app.revertableVersion = ctx.request.body.revertableVersion await db.put(app) @@ -763,7 +760,7 @@ export async function setRevertableVersion( async function migrateAppNavigation() { const db = context.getAppDB() - const existing: App = await db.get(DocumentType.APP_METADATA) + const existing = await sdk.appMetadata.get() const layouts: Layout[] = await getLayouts() const screens: Screen[] = await getScreens() diff --git a/packages/server/src/sdk/app/appMetadata/index.ts b/packages/server/src/sdk/app/appMetadata/index.ts new file mode 100644 index 0000000000..267899a214 --- /dev/null +++ b/packages/server/src/sdk/app/appMetadata/index.ts @@ -0,0 +1,5 @@ +import * as metadata from "./metadata" + +export default { + ...metadata, +} diff --git a/packages/server/src/sdk/app/appMetadata/metadata.ts b/packages/server/src/sdk/app/appMetadata/metadata.ts new file mode 100644 index 0000000000..78d2a040b9 --- /dev/null +++ b/packages/server/src/sdk/app/appMetadata/metadata.ts @@ -0,0 +1,8 @@ +import { context, DocumentType } from "@budibase/backend-core" +import { App } from "@budibase/types" + +export async function get() { + const db = context.getAppDB() + const application = await db.get(DocumentType.APP_METADATA) + return application +} diff --git a/packages/server/src/sdk/index.ts b/packages/server/src/sdk/index.ts index a871546b60..44df8b50fb 100644 --- a/packages/server/src/sdk/index.ts +++ b/packages/server/src/sdk/index.ts @@ -11,6 +11,7 @@ import { default as plugins } from "./plugins" import * as views from "./app/views" import * as permissions from "./app/permissions" import * as rowActions from "./app/rowActions" +import { default as appMetadata } from "./app/appMetadata" const sdk = { backups, @@ -26,6 +27,7 @@ const sdk = { permissions, links, rowActions, + appMetadata, } // default export for TS From 9c92288f7f94932698ff09e31de2012ee6f5d459 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 16 Oct 2024 15:18:37 +0100 Subject: [PATCH 37/89] Fixing some issues with finding roles. --- packages/backend-core/src/security/roles.ts | 21 +++--- packages/server/src/api/controllers/role.ts | 10 ++- .../server/src/api/routes/tests/role.spec.ts | 75 ++++++++++++++++--- packages/server/src/middleware/currentapp.ts | 12 ++- 4 files changed, 91 insertions(+), 27 deletions(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index 41a55c55bd..03660b313d 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -235,7 +235,7 @@ export function findRole( roleId: string, roles: RoleDoc[], opts?: { defaultPublic?: boolean } -): RoleDoc { +): 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) @@ -249,13 +249,13 @@ export function findRole( if (!dbRole && !isBuiltin(roleId) && opts?.defaultPublic) { return cloneDeep(BUILTIN_ROLES.PUBLIC) } - if (!dbRole && (!role || Object.keys(role).length === 0)) { - throw new Error("Role could not be found") - } + // combine the roles role = Object.assign(role || {}, dbRole) // finalise the ID - role._id = getExternalRoleID(role._id!, role.version) - return role + if (role?._id) { + role._id = getExternalRoleID(role._id, role.version) + } + return Object.keys(role).length === 0 ? undefined : role } /** @@ -268,7 +268,7 @@ export function findRole( export async function getRole( roleId: string, opts?: { defaultPublic?: boolean } -): Promise { +): Promise { const db = getAppDB() const roleList = [] if (!isBuiltin(roleId)) { @@ -305,7 +305,7 @@ async function getAllUserRoles( const roleIds = [userRoleId] const roles: RoleDoc[] = [] - const iterateInherited = (role: RoleDoc) => { + const iterateInherited = (role: RoleDoc | undefined) => { if (!role || !role._id) { return } @@ -335,7 +335,10 @@ async function getAllUserRoles( } // get all the inherited roles - iterateInherited(findRole(userRoleId, allRoles, opts)) + const foundRole = findRole(userRoleId, allRoles, opts) + if (foundRole) { + iterateInherited(foundRole) + } const foundRoleIds: string[] = [] return roles.filter(role => { if (role._id && !foundRoleIds.includes(role._id)) { diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index 1e7f58e566..d2f15b84c2 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -57,7 +57,12 @@ export async function fetch(ctx: UserCtx) { } 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" }) + } else { + ctx.body = role + } } export async function save(ctx: UserCtx) { @@ -202,7 +207,8 @@ export async function accessible(ctx: UserCtx) { // 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 role = await roles.getRole(roleHeader) + const inherits = role?.inherits const orderedRoles = ctx.body.reverse() let filteredRoles = [roleHeader] for (let role of orderedRoles) { diff --git a/packages/server/src/api/routes/tests/role.spec.ts b/packages/server/src/api/routes/tests/role.spec.ts index 396afeb236..3a2ba3ce9d 100644 --- a/packages/server/src/api/routes/tests/role.spec.ts +++ b/packages/server/src/api/routes/tests/role.spec.ts @@ -6,6 +6,8 @@ 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() @@ -31,6 +33,11 @@ describe("/roles", () => { }) 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, { @@ -49,22 +56,59 @@ describe("/roles", () => { }) it("disallow loops", async () => { - let role1 = basicRole() - role1 = await config.api.roles.save(role1, { + const role1 = await config.api.roles.save(basicRole(), { status: 200, }) - let role2 = basicRole() - role2.inherits = [role1._id!] - role2 = await config.api.roles.save(role2, { - status: 200, - }) - role1.inherits = [role2._id!] - await config.api.roles.save(role1, { - status: 400, - body: { - message: "Role inheritance contains a loop, this is not supported", + 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 + 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 + role2 = await config.api.roles.save( + { + ...role2, + inherits: [BUILTIN_ROLE_IDS.POWER, role1._id!, role3._id!], + }, + { status: 400, body: { message: LOOP_ERROR } } + ) }) }) @@ -134,6 +178,13 @@ describe("/roles", () => { }) 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 () => { const res = await config.api.roles.accessible(config.defaultHeaders(), { status: 200, 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 From 33990931631e96cc0bb633f38a8c701c857f8087 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 16 Oct 2024 15:46:03 +0100 Subject: [PATCH 38/89] Role save API returns role ID in correct format. --- packages/server/src/api/controllers/role.ts | 5 ++++- .../server/src/api/routes/tests/role.spec.ts | 22 +++++++++++++++---- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index d2f15b84c2..8a838543b0 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -135,7 +135,10 @@ export async function save(ctx: UserCtx) { role.version ) role._rev = result.rev - ctx.body = role + ctx.body = { + ...role, + _id: roles.getExternalRoleID(role._id!, role.version), + } const devDb = context.getDevAppDB() const prodDb = context.getProdAppDB() diff --git a/packages/server/src/api/routes/tests/role.spec.ts b/packages/server/src/api/routes/tests/role.spec.ts index 3a2ba3ce9d..ef3a919173 100644 --- a/packages/server/src/api/routes/tests/role.spec.ts +++ b/packages/server/src/api/routes/tests/role.spec.ts @@ -1,4 +1,9 @@ -import { roles, events, permissions } from "@budibase/backend-core" +import { + roles, + events, + permissions, + db as dbCore, +} from "@budibase/backend-core" import * as setup from "./utilities" import { PermissionLevel } from "@budibase/types" @@ -28,7 +33,10 @@ describe("/roles", () => { expect(res._rev).toBeDefined() expect(events.role.updated).not.toHaveBeenCalled() expect(events.role.created).toHaveBeenCalledTimes(1) - expect(events.role.created).toHaveBeenCalledWith(res) + expect(events.role.created).toHaveBeenCalledWith({ + ...res, + _id: dbCore.prefixRoleID(res._id!), + }) }) }) @@ -52,7 +60,10 @@ describe("/roles", () => { expect(res._rev).toBeDefined() expect(events.role.created).not.toHaveBeenCalled() expect(events.role.updated).toHaveBeenCalledTimes(1) - expect(events.role.updated).toHaveBeenCalledWith(res) + expect(events.role.updated).toHaveBeenCalledWith({ + ...res, + _id: dbCore.prefixRoleID(res._id!), + }) }) it("disallow loops", async () => { @@ -173,7 +184,10 @@ describe("/roles", () => { status: 404, }) expect(events.role.deleted).toHaveBeenCalledTimes(1) - expect(events.role.deleted).toHaveBeenCalledWith(customRole) + expect(events.role.deleted).toHaveBeenCalledWith({ + ...customRole, + _id: dbCore.prefixRoleID(customRole._id!), + }) }) }) From c40e4a72885bd551b5923c6afea11955cfcd4079 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 16 Oct 2024 17:26:48 +0100 Subject: [PATCH 39/89] fixing rbac --- packages/backend-core/src/security/roles.ts | 29 ++++++++++++++- packages/server/src/api/controllers/role.ts | 36 ++++++++++++------- .../server/src/api/routes/tests/role.spec.ts | 4 +-- packages/shared-core/src/helpers/roles.ts | 2 +- 4 files changed, 54 insertions(+), 17 deletions(-) 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 8a838543b0..7b9f2d7658 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -27,6 +27,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, @@ -53,7 +65,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) { @@ -61,7 +73,7 @@ export async function find(ctx: UserCtx) { if (!role) { ctx.throw(404, { message: "Role not found" }) } else { - ctx.body = role + ctx.body = externalRole(role) } } @@ -135,10 +147,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() @@ -196,15 +205,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 @@ -212,7 +220,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] @@ -224,6 +232,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 { From 1f9c33c53a6bb12a8b50ab7db5d862f6ea33226d Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 16 Oct 2024 21:29:33 +0100 Subject: [PATCH 40/89] Test cases based on frontend. --- .../server/src/api/routes/tests/role.spec.ts | 28 +++++++++++++++++++ packages/types/src/api/web/role.ts | 2 ++ 2 files changed, 30 insertions(+) diff --git a/packages/server/src/api/routes/tests/role.spec.ts b/packages/server/src/api/routes/tests/role.spec.ts index 6ed5dfd30f..134078e6bf 100644 --- a/packages/server/src/api/routes/tests/role.spec.ts +++ b/packages/server/src/api/routes/tests/role.spec.ts @@ -121,6 +121,34 @@ describe("/roles", () => { { 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", () => { diff --git a/packages/types/src/api/web/role.ts b/packages/types/src/api/web/role.ts index 4e56f6cd14..df439e84e7 100644 --- a/packages/types/src/api/web/role.ts +++ b/packages/types/src/api/web/role.ts @@ -1,4 +1,5 @@ import { Role, RoleUIMetadata } from "../../documents" +import { PermissionLevel } from "../../sdk" export interface SaveRoleRequest { _id?: string @@ -6,6 +7,7 @@ export interface SaveRoleRequest { name: string inherits?: string | string[] permissionId: string + permissions?: Record version?: string uiMetadata?: RoleUIMetadata } From 97b1c52f70c318323f74ad481326196770f30f04 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 16 Oct 2024 22:08:07 +0100 Subject: [PATCH 41/89] Protect against old roles with differences when it comes to loops. --- packages/shared-core/src/helpers/roles.ts | 16 ++++++++++++---- .../shared-core/src/helpers/tests/roles.spec.ts | 7 +++++++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/packages/shared-core/src/helpers/roles.ts b/packages/shared-core/src/helpers/roles.ts index 24b65501ce..8dee928edd 100644 --- a/packages/shared-core/src/helpers/roles.ts +++ b/packages/shared-core/src/helpers/roles.ts @@ -1,4 +1,11 @@ -import { Role, SEPARATOR, DocumentType } from "@budibase/types" +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 { @@ -11,16 +18,17 @@ export function checkForRoleInheritanceLoops(roles: Role[]): boolean { const checking = new Set() function hasLoop(roleId: string): boolean { - if (checking.has(roleId)) { + const prefixed = prefixForCheck(roleId) + if (checking.has(roleId) || checking.has(prefixed)) { return true } - if (checked.has(roleId)) { + if (checked.has(roleId) || checked.has(prefixed)) { return false } checking.add(roleId) - const role = roleMap.get(roleId) + const role = roleMap.get(prefixed) || roleMap.get(roleId) if (!role) { // role not found - ignore checking.delete(roleId) diff --git a/packages/shared-core/src/helpers/tests/roles.spec.ts b/packages/shared-core/src/helpers/tests/roles.spec.ts index 7e52ded3f6..7b67e4e9e9 100644 --- a/packages/shared-core/src/helpers/tests/roles.spec.ts +++ b/packages/shared-core/src/helpers/tests/roles.spec.ts @@ -57,5 +57,12 @@ describe("role utilities", () => { ] 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) + }) }) }) From 42086f36a2194c0d2ce038d5e81ee36c27d4ca16 Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Thu, 17 Oct 2024 07:30:11 +0000 Subject: [PATCH 42/89] Bump version to 2.33.2 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index caaa07683a..df50828b30 100644 --- a/lerna.json +++ b/lerna.json @@ -1,6 +1,6 @@ { "$schema": "node_modules/lerna/schemas/lerna-schema.json", - "version": "2.33.1", + "version": "2.33.2", "npmClient": "yarn", "packages": [ "packages/*", From ab517bf86da556af6f0c5ca9cd8dabb0e5d87dc8 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 17 Oct 2024 10:32:17 +0200 Subject: [PATCH 43/89] Persist created version --- .../server/src/api/controllers/application.ts | 24 +++++++++++-------- .../src/sdk/app/appMetadata/metadata.ts | 10 ++++++++ packages/types/src/documents/app/app.ts | 1 + 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/packages/server/src/api/controllers/application.ts b/packages/server/src/api/controllers/application.ts index 660d35c29d..b8f523c331 100644 --- a/packages/server/src/api/controllers/application.ts +++ b/packages/server/src/api/controllers/application.ts @@ -271,6 +271,7 @@ async function performAppCreate(ctx: UserCtx) { path: ctx.request.body.file?.path, } } + const tenantId = tenancy.isMultiTenant() ? tenancy.getTenantId() : null const appId = generateDevAppID(generateAppID(tenantId)) @@ -278,7 +279,7 @@ async function performAppCreate(ctx: UserCtx) { const instance = await createInstance(appId, instanceConfig) const db = context.getAppDB() - let newApplication: App = { + const newApplication: App = { _id: DocumentType.APP_METADATA, _rev: undefined, appId, @@ -309,12 +310,18 @@ async function performAppCreate(ctx: UserCtx) { disableUserMetadata: true, skeletonLoader: true, }, + creationVersion: undefined, } + const isImport = !!instanceConfig.file + if (!isImport) { + newApplication.creationVersion = envCore.VERSION + } + + const existing = await sdk.appMetadata.tryGet() // If we used a template or imported an app there will be an existing doc. // Fetch and migrate some metadata from the existing app. - try { - const existing = await sdk.appMetadata.get() + if (existing) { const keys: (keyof App)[] = [ "_rev", "navigation", @@ -322,6 +329,7 @@ async function performAppCreate(ctx: UserCtx) { "customTheme", "icon", "snippets", + "creationVersion", ] keys.forEach(key => { if (existing[key]) { @@ -339,14 +347,10 @@ async function performAppCreate(ctx: UserCtx) { } // Migrate navigation settings and screens if required - if (existing) { - const navigation = await migrateAppNavigation() - if (navigation) { - newApplication.navigation = navigation - } + const navigation = await migrateAppNavigation() + if (navigation) { + newApplication.navigation = navigation } - } catch (err) { - // Nothing to do } const response = await db.put(newApplication, { force: true }) diff --git a/packages/server/src/sdk/app/appMetadata/metadata.ts b/packages/server/src/sdk/app/appMetadata/metadata.ts index 78d2a040b9..bbda55c085 100644 --- a/packages/server/src/sdk/app/appMetadata/metadata.ts +++ b/packages/server/src/sdk/app/appMetadata/metadata.ts @@ -1,8 +1,18 @@ import { context, DocumentType } from "@budibase/backend-core" import { App } from "@budibase/types" +/** + * @deprecated the plan is to get everything using `tryGet` instead, then rename + * `tryGet` to `get`. + */ export async function get() { const db = context.getAppDB() const application = await db.get(DocumentType.APP_METADATA) return application } + +export async function tryGet() { + const db = context.getAppDB() + const application = await db.tryGet(DocumentType.APP_METADATA) + return application +} diff --git a/packages/types/src/documents/app/app.ts b/packages/types/src/documents/app/app.ts index 69bf3489e1..06fca8307c 100644 --- a/packages/types/src/documents/app/app.ts +++ b/packages/types/src/documents/app/app.ts @@ -27,6 +27,7 @@ export interface App extends Document { usedPlugins?: Plugin[] upgradableVersion?: string snippets?: Snippet[] + creationVersion?: string } export interface AppInstance { From 8cdc5be38e27cb8555d8c0aae14eacdcfa867d9a Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 17 Oct 2024 11:36:02 +0200 Subject: [PATCH 44/89] Store proper version even on local --- packages/backend-core/src/environment.ts | 50 ++++++++++++++++-------- 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/packages/backend-core/src/environment.ts b/packages/backend-core/src/environment.ts index 2ab8c550cc..87dd2c5e87 100644 --- a/packages/backend-core/src/environment.ts +++ b/packages/backend-core/src/environment.ts @@ -54,30 +54,46 @@ function getPackageJsonFields(): { VERSION: string SERVICE_NAME: string } { - function findFileInAncestors( - fileName: string, - currentDir: string - ): string | null { - const filePath = `${currentDir}/${fileName}` - if (existsSync(filePath)) { - return filePath + function getParentFile(file: string) { + function findFileInAncestors( + fileName: string, + currentDir: string + ): string | null { + const filePath = `${currentDir}/${fileName}` + if (existsSync(filePath)) { + return filePath + } + + const parentDir = `${currentDir}/..` + if (parentDir === currentDir) { + // reached root directory + return null + } + + return findFileInAncestors(fileName, parentDir) } - const parentDir = `${currentDir}/..` - if (parentDir === currentDir) { - // reached root directory - return null - } + const packageJsonFile = findFileInAncestors(file, process.cwd()) + const content = readFileSync(packageJsonFile!, "utf-8") + const parsedContent = JSON.parse(content) + return parsedContent + } - return findFileInAncestors(fileName, parentDir) + let localVersion: string | undefined + if (isDev()) { + try { + const lerna = getParentFile("lerna.json") + localVersion = lerna.version + } catch { + // + } } try { - const packageJsonFile = findFileInAncestors("package.json", process.cwd()) - const content = readFileSync(packageJsonFile!, "utf-8") - const parsedContent = JSON.parse(content) + const parsedContent = getParentFile("package.json") return { - VERSION: process.env.BUDIBASE_VERSION || parsedContent.version, + VERSION: + localVersion || process.env.BUDIBASE_VERSION || parsedContent.version, SERVICE_NAME: parsedContent.name, } } catch { From 15bb730c591f4b8879485ee20e36cc74c8414bc1 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 17 Oct 2024 11:37:41 +0200 Subject: [PATCH 45/89] Remove power role for apps created at >= 3.0.0 --- packages/backend-core/src/security/roles.ts | 49 ++++++++++++++++----- 1 file changed, 38 insertions(+), 11 deletions(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index fa2d114d7d..ea4cb1b38a 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -1,3 +1,4 @@ +import semver from "semver" import { BuiltinPermissionID, PermissionLevel } from "./permissions" import { prefixRoleID, @@ -7,7 +8,13 @@ import { doWithDB, } from "../db" import { getAppDB } from "../context" -import { Screen, Role as RoleDoc, RoleUIMetadata } from "@budibase/types" +import { + Screen, + Role as RoleDoc, + RoleUIMetadata, + Database, + App, +} from "@budibase/types" import cloneDeep from "lodash/fp/cloneDeep" import { RoleColor } from "@budibase/shared-core" @@ -23,14 +30,6 @@ const BUILTIN_IDS = { BUILDER: "BUILDER", } -// exclude internal roles like builder -const EXTERNAL_BUILTIN_ROLE_IDS = [ - BUILTIN_IDS.ADMIN, - BUILTIN_IDS.POWER, - BUILTIN_IDS.BASIC, - BUILTIN_IDS.PUBLIC, -] - export const RoleIDVersion = { // original version, with a UUID based ID UUID: undefined, @@ -319,7 +318,7 @@ export async function getAllRoles(appId?: string): Promise { } return internal(appDB) } - async function internal(db: any) { + async function internal(db: Database | undefined) { let roles: RoleDoc[] = [] if (db) { const body = await db.allDocs( @@ -334,8 +333,26 @@ export async function getAllRoles(appId?: string): Promise { } const builtinRoles = getBuiltinRoles() + // exclude internal roles like builder + let externalBuiltinRoles = [] + + if (db && !(await shouldIncludePowerRole(db))) { + externalBuiltinRoles = [ + BUILTIN_IDS.ADMIN, + BUILTIN_IDS.POWER, + BUILTIN_IDS.BASIC, + BUILTIN_IDS.PUBLIC, + ] + } else { + externalBuiltinRoles = [ + BUILTIN_IDS.ADMIN, + BUILTIN_IDS.BASIC, + BUILTIN_IDS.PUBLIC, + ] + } + // need to combine builtin with any DB record of them (for sake of permissions) - for (let builtinRoleId of EXTERNAL_BUILTIN_ROLE_IDS) { + for (let builtinRoleId of externalBuiltinRoles) { const builtinRole = builtinRoles[builtinRoleId] const dbBuiltin = roles.filter( dbRole => @@ -366,6 +383,16 @@ export async function getAllRoles(appId?: string): Promise { } } +async function shouldIncludePowerRole(db: Database) { + const app = await db.get(DocumentType.APP_METADATA) + const { creationVersion } = app + if (semver.gte(creationVersion || "", "3.0.0")) { + return true + } + + return false +} + export class AccessController { userHierarchies: { [key: string]: string[] } constructor() { From 1155be45304ca24eb24850a3341b6d33729c5fe9 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 17 Oct 2024 11:52:03 +0200 Subject: [PATCH 46/89] Fix --- packages/backend-core/src/security/roles.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index ea4cb1b38a..cd9217d600 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -336,7 +336,7 @@ export async function getAllRoles(appId?: string): Promise { // exclude internal roles like builder let externalBuiltinRoles = [] - if (db && !(await shouldIncludePowerRole(db))) { + if (!db || (await shouldIncludePowerRole(db))) { externalBuiltinRoles = [ BUILTIN_IDS.ADMIN, BUILTIN_IDS.POWER, @@ -386,11 +386,13 @@ export async function getAllRoles(appId?: string): Promise { async function shouldIncludePowerRole(db: Database) { const app = await db.get(DocumentType.APP_METADATA) const { creationVersion } = app - if (semver.gte(creationVersion || "", "3.0.0")) { + if (!creationVersion) { + // Old apps don't have creationVersion, so we should include it for backward compatibility return true } - return false + const isGreaterThan3x = semver.gte(creationVersion, "3.0.0") + return !isGreaterThan3x } export class AccessController { From 8008d2ced1fa11b0e77b99bd98ddf7c27f801e37 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 17 Oct 2024 11:53:20 +0200 Subject: [PATCH 47/89] Fix all references --- packages/backend-core/src/security/roles.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index cd9217d600..2fcde768bc 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -384,8 +384,8 @@ export async function getAllRoles(appId?: string): Promise { } async function shouldIncludePowerRole(db: Database) { - const app = await db.get(DocumentType.APP_METADATA) - const { creationVersion } = app + const app = await db.tryGet(DocumentType.APP_METADATA) + const creationVersion = app?.creationVersion if (!creationVersion) { // Old apps don't have creationVersion, so we should include it for backward compatibility return true From 7bb69d7ffdb61353c12006d7356c87e82e7b09d6 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 17 Oct 2024 12:17:01 +0200 Subject: [PATCH 48/89] Add tests --- packages/backend-core/src/security/roles.ts | 2 +- .../src/api/routes/global/tests/roles.spec.ts | 57 ++++++++++++++++++- 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index 2fcde768bc..fad5f7cb74 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -386,7 +386,7 @@ export async function getAllRoles(appId?: string): Promise { async function shouldIncludePowerRole(db: Database) { const app = await db.tryGet(DocumentType.APP_METADATA) const creationVersion = app?.creationVersion - if (!creationVersion) { + if (!creationVersion || !semver.valid(creationVersion)) { // Old apps don't have creationVersion, so we should include it for backward compatibility return true } diff --git a/packages/worker/src/api/routes/global/tests/roles.spec.ts b/packages/worker/src/api/routes/global/tests/roles.spec.ts index 11de06328e..1ca9ff7e29 100644 --- a/packages/worker/src/api/routes/global/tests/roles.spec.ts +++ b/packages/worker/src/api/routes/global/tests/roles.spec.ts @@ -1,6 +1,6 @@ import { structures, TestConfiguration } from "../../../../tests" import { context, db, permissions, roles } from "@budibase/backend-core" -import { Database } from "@budibase/types" +import { App, Database } from "@budibase/types" jest.mock("@budibase/backend-core", () => { const core = jest.requireActual("@budibase/backend-core") @@ -30,6 +30,14 @@ async function addAppMetadata() { }) } +async function updateAppMetadata(update: Partial>) { + const app = await appDb.get("app_metadata") + await appDb.put({ + ...app, + ...update, + }) +} + describe("/api/global/roles", () => { const config = new TestConfiguration() @@ -69,6 +77,53 @@ describe("/api/global/roles", () => { expect(res.body[appId].roles.length).toEqual(5) expect(res.body[appId].roles.map((r: any) => r._id)).toContain(ROLE_NAME) }) + + it.each(["3.0.0", "3.0.1", "3.1.0"])( + "exclude POWER roles after v3 (%s)", + async creationVersion => { + await updateAppMetadata({ creationVersion }) + const res = await config.api.roles.get() + expect(res.body).toBeDefined() + expect(res.body[appId].roles.map((r: any) => r._id)).toEqual([ + ROLE_NAME, + roles.BUILTIN_ROLE_IDS.ADMIN, + roles.BUILTIN_ROLE_IDS.BASIC, + roles.BUILTIN_ROLE_IDS.PUBLIC, + ]) + } + ) + + it.each(["2.9.0", "1.0.0"])( + "include POWER roles before v3 (%s)", + async creationVersion => { + await updateAppMetadata({ creationVersion }) + const res = await config.api.roles.get() + expect(res.body).toBeDefined() + expect(res.body[appId].roles.map((r: any) => r._id)).toEqual([ + ROLE_NAME, + roles.BUILTIN_ROLE_IDS.ADMIN, + roles.BUILTIN_ROLE_IDS.POWER, + roles.BUILTIN_ROLE_IDS.BASIC, + roles.BUILTIN_ROLE_IDS.PUBLIC, + ]) + } + ) + + it.each(["invalid", ""])( + "include POWER roles when the version is corrupted (%s)", + async creationVersion => { + await updateAppMetadata({ creationVersion }) + const res = await config.api.roles.get() + + expect(res.body[appId].roles.map((r: any) => r._id)).toEqual([ + ROLE_NAME, + roles.BUILTIN_ROLE_IDS.ADMIN, + roles.BUILTIN_ROLE_IDS.POWER, + roles.BUILTIN_ROLE_IDS.BASIC, + roles.BUILTIN_ROLE_IDS.PUBLIC, + ]) + } + ) }) describe("GET api/global/roles/:appId", () => { From c1128ffe2a953f3f4f1e1056083541eb8787d2cc Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 17 Oct 2024 12:20:17 +0200 Subject: [PATCH 49/89] Fix test --- packages/backend-core/src/environment.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend-core/src/environment.ts b/packages/backend-core/src/environment.ts index 87dd2c5e87..4e93e8d9ee 100644 --- a/packages/backend-core/src/environment.ts +++ b/packages/backend-core/src/environment.ts @@ -80,7 +80,7 @@ function getPackageJsonFields(): { } let localVersion: string | undefined - if (isDev()) { + if (isDev() && !isTest()) { try { const lerna = getParentFile("lerna.json") localVersion = lerna.version From cf3f5b97c6f7c7312d5e6c46ae120fd8892ee823 Mon Sep 17 00:00:00 2001 From: saewoohan <112225610+saewoohan@users.noreply.github.com> Date: Thu, 17 Oct 2024 23:14:47 +0900 Subject: [PATCH 50/89] fix: updated url to point to settings --- packages/builder/src/pages/builder/portal/apps/index.svelte | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/builder/src/pages/builder/portal/apps/index.svelte b/packages/builder/src/pages/builder/portal/apps/index.svelte index ddd37dc4a3..c3fcfd65ff 100644 --- a/packages/builder/src/pages/builder/portal/apps/index.svelte +++ b/packages/builder/src/pages/builder/portal/apps/index.svelte @@ -76,9 +76,7 @@ const params = new URLSearchParams({ open: "error", }) - $goto( - `/builder/app/${appId}/settings/automation-history?${params.toString()}` - ) + $goto(`/builder/app/${appId}/settings/automations?${params.toString()}`) } const errorCount = errors => { From b2e718504c90dda132f1a5ac714bfbaeb7a551b4 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 17 Oct 2024 15:17:36 +0100 Subject: [PATCH 51/89] Adri PR comments. --- packages/server/src/api/controllers/role.ts | 6 ++++-- packages/server/src/api/routes/tests/permissions.spec.ts | 4 ++-- packages/server/src/api/routes/tests/screen.spec.ts | 2 +- packages/server/src/tests/utilities/TestConfiguration.ts | 2 +- packages/server/src/tests/utilities/api/role.ts | 4 ---- 5 files changed, 8 insertions(+), 10 deletions(-) diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index 7b9f2d7658..fef68a650c 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -72,9 +72,8 @@ export async function find(ctx: UserCtx) { const role = await roles.getRole(ctx.params.roleId) if (!role) { ctx.throw(404, { message: "Role not found" }) - } else { - ctx.body = externalRole(role) } + ctx.body = externalRole(role) } export async function save(ctx: UserCtx) { @@ -82,6 +81,9 @@ export async function save(ctx: UserCtx) { let { _id, name, inherits, permissionId, version, uiMetadata } = ctx.request.body let isCreate = false + if (!ctx.request.body._rev && !version) { + version = roles.RoleIDVersion.NAME + } const isNewVersion = version === roles.RoleIDVersion.NAME if (_id && roles.isBuiltin(_id)) { diff --git a/packages/server/src/api/routes/tests/permissions.spec.ts b/packages/server/src/api/routes/tests/permissions.spec.ts index 46ce656459..90060eef81 100644 --- a/packages/server/src/api/routes/tests/permissions.spec.ts +++ b/packages/server/src/api/routes/tests/permissions.spec.ts @@ -328,7 +328,7 @@ describe("/permission", () => { }) it("should be unable to search for table 2 using role 1", async () => { - await config.setRole(role1._id!, async () => { + await config.loginAsRole(role1._id!, async () => { const response2 = await config.api.row.search( table2._id!, { @@ -347,7 +347,7 @@ describe("/permission", () => { inherits: [role1._id!, role2._id!], }) - await config.setRole(role3._id!, async () => { + await config.loginAsRole(role3._id!, async () => { const response1 = await config.api.row.search( table1._id!, { diff --git a/packages/server/src/api/routes/tests/screen.spec.ts b/packages/server/src/api/routes/tests/screen.spec.ts index f660589951..5853f44bac 100644 --- a/packages/server/src/api/routes/tests/screen.spec.ts +++ b/packages/server/src/api/routes/tests/screen.spec.ts @@ -79,7 +79,7 @@ describe("/screens", () => { }) async function checkScreens(roleId: string, screenIds: string[]) { - await config.setRole(roleId, async () => { + await config.loginAsRole(roleId, async () => { const res = await config.api.application.getDefinition( config.prodAppId!, { diff --git a/packages/server/src/tests/utilities/TestConfiguration.ts b/packages/server/src/tests/utilities/TestConfiguration.ts index a3f2fb7adc..fc5a4a7f9c 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.ts +++ b/packages/server/src/tests/utilities/TestConfiguration.ts @@ -429,7 +429,7 @@ export default class TestConfiguration { // HEADERS // sets the role for the headers, for the period of a callback - async setRole(roleId: string, cb: () => Promise) { + async loginAsRole(roleId: string, cb: () => Promise) { const roleUser = await this.createUser({ roles: { [this.prodAppId!]: roleId, diff --git a/packages/server/src/tests/utilities/api/role.ts b/packages/server/src/tests/utilities/api/role.ts index 05165cd38e..31bffc6f85 100644 --- a/packages/server/src/tests/utilities/api/role.ts +++ b/packages/server/src/tests/utilities/api/role.ts @@ -22,10 +22,6 @@ export class RoleAPI extends TestAPI { } save = async (body: SaveRoleRequest, expectations?: Expectations) => { - // the tests should always be creating the "new" version of roles - if (body.version === undefined) { - body.version = "name" - } return await this._post(`/api/roles`, { body, expectations, From 3b597f6405e23ed81e997ab89d457048dd542053 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 17 Oct 2024 15:25:34 +0100 Subject: [PATCH 52/89] Fixing test case. --- packages/server/src/api/routes/tests/permissions.spec.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/routes/tests/permissions.spec.ts b/packages/server/src/api/routes/tests/permissions.spec.ts index 90060eef81..7a9bb2f373 100644 --- a/packages/server/src/api/routes/tests/permissions.spec.ts +++ b/packages/server/src/api/routes/tests/permissions.spec.ts @@ -291,6 +291,8 @@ 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!, { @@ -301,7 +303,7 @@ describe("/permission", () => { }) role1 = await config.api.roles.save( { - name: "role1", + name: "test_1", permissionId: PermissionLevel.WRITE, inherits: BUILTIN_ROLE_IDS.BASIC, }, @@ -309,7 +311,7 @@ describe("/permission", () => { ) role2 = await config.api.roles.save( { - name: "role2", + name: "test_2", permissionId: PermissionLevel.WRITE, inherits: BUILTIN_ROLE_IDS.BASIC, }, From ca974cf2f509f4497f571ef8723de994da29261a Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 17 Oct 2024 16:30:22 +0200 Subject: [PATCH 53/89] Renames --- .../server/src/api/controllers/application.ts | 16 ++++++++-------- packages/server/src/sdk/app/appMetadata/index.ts | 5 ----- .../server/src/sdk/app/applications/index.ts | 2 ++ .../{appMetadata => applications}/metadata.ts | 0 packages/server/src/sdk/index.ts | 2 -- 5 files changed, 10 insertions(+), 15 deletions(-) delete mode 100644 packages/server/src/sdk/app/appMetadata/index.ts rename packages/server/src/sdk/app/{appMetadata => applications}/metadata.ts (100%) diff --git a/packages/server/src/api/controllers/application.ts b/packages/server/src/api/controllers/application.ts index b8f523c331..59f67540fe 100644 --- a/packages/server/src/api/controllers/application.ts +++ b/packages/server/src/api/controllers/application.ts @@ -209,7 +209,7 @@ export async function fetchAppPackage( ctx: UserCtx ) { const appId = context.getAppId() - const application = await sdk.appMetadata.get() + const application = await sdk.applications.metadata.get() const layouts = await getLayouts() let screens = await getScreens() const license = await licensing.cache.getCachedLicense() @@ -318,7 +318,7 @@ async function performAppCreate(ctx: UserCtx) { newApplication.creationVersion = envCore.VERSION } - const existing = await sdk.appMetadata.tryGet() + const existing = await sdk.applications.metadata.tryGet() // If we used a template or imported an app there will be an existing doc. // Fetch and migrate some metadata from the existing app. if (existing) { @@ -492,7 +492,7 @@ export async function update( export async function updateClient(ctx: UserCtx) { // Get current app version - const application = await sdk.appMetadata.get() + const application = await sdk.applications.metadata.get() const currentVersion = application.version let manifest @@ -520,7 +520,7 @@ export async function updateClient(ctx: UserCtx) { export async function revertClient(ctx: UserCtx) { // Check app can be reverted - const application = await sdk.appMetadata.get() + const application = await sdk.applications.metadata.get() if (!application.revertableVersion) { ctx.throw(400, "There is no version to revert to") } @@ -578,7 +578,7 @@ async function destroyApp(ctx: UserCtx) { const db = dbCore.getDB(devAppId) // standard app deletion flow - const app = await sdk.appMetadata.get() + const app = await sdk.applications.metadata.get() const result = await db.destroy() await quotas.removeApp() await events.app.deleted(app) @@ -729,7 +729,7 @@ export async function updateAppPackage( ) { return context.doInAppContext(appId, async () => { const db = context.getAppDB() - const application = await sdk.appMetadata.get() + const application = await sdk.applications.metadata.get() const newAppPackage: App = { ...application, ...appPackage } if (appPackage._rev !== application._rev) { @@ -755,7 +755,7 @@ export async function setRevertableVersion( return } const db = context.getAppDB() - const app = await sdk.appMetadata.get() + const app = await sdk.applications.metadata.get() app.revertableVersion = ctx.request.body.revertableVersion await db.put(app) @@ -764,7 +764,7 @@ export async function setRevertableVersion( async function migrateAppNavigation() { const db = context.getAppDB() - const existing = await sdk.appMetadata.get() + const existing = await sdk.applications.metadata.get() const layouts: Layout[] = await getLayouts() const screens: Screen[] = await getScreens() diff --git a/packages/server/src/sdk/app/appMetadata/index.ts b/packages/server/src/sdk/app/appMetadata/index.ts deleted file mode 100644 index 267899a214..0000000000 --- a/packages/server/src/sdk/app/appMetadata/index.ts +++ /dev/null @@ -1,5 +0,0 @@ -import * as metadata from "./metadata" - -export default { - ...metadata, -} diff --git a/packages/server/src/sdk/app/applications/index.ts b/packages/server/src/sdk/app/applications/index.ts index 04ed3b2919..f315e84896 100644 --- a/packages/server/src/sdk/app/applications/index.ts +++ b/packages/server/src/sdk/app/applications/index.ts @@ -2,10 +2,12 @@ import * as sync from "./sync" import * as utils from "./utils" import * as applications from "./applications" import * as imports from "./import" +import * as metadata from "./metadata" export default { ...sync, ...utils, ...applications, ...imports, + metadata, } diff --git a/packages/server/src/sdk/app/appMetadata/metadata.ts b/packages/server/src/sdk/app/applications/metadata.ts similarity index 100% rename from packages/server/src/sdk/app/appMetadata/metadata.ts rename to packages/server/src/sdk/app/applications/metadata.ts diff --git a/packages/server/src/sdk/index.ts b/packages/server/src/sdk/index.ts index 44df8b50fb..a871546b60 100644 --- a/packages/server/src/sdk/index.ts +++ b/packages/server/src/sdk/index.ts @@ -11,7 +11,6 @@ import { default as plugins } from "./plugins" import * as views from "./app/views" import * as permissions from "./app/permissions" import * as rowActions from "./app/rowActions" -import { default as appMetadata } from "./app/appMetadata" const sdk = { backups, @@ -27,7 +26,6 @@ const sdk = { permissions, links, rowActions, - appMetadata, } // default export for TS From 4c688b9734a95b19110b79419b5208928fa19a8d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 17 Oct 2024 16:40:26 +0200 Subject: [PATCH 54/89] Add more tests --- packages/worker/src/api/routes/global/tests/roles.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/worker/src/api/routes/global/tests/roles.spec.ts b/packages/worker/src/api/routes/global/tests/roles.spec.ts index 1ca9ff7e29..28977d8521 100644 --- a/packages/worker/src/api/routes/global/tests/roles.spec.ts +++ b/packages/worker/src/api/routes/global/tests/roles.spec.ts @@ -78,7 +78,7 @@ describe("/api/global/roles", () => { expect(res.body[appId].roles.map((r: any) => r._id)).toContain(ROLE_NAME) }) - it.each(["3.0.0", "3.0.1", "3.1.0"])( + it.each(["3.0.0", "3.0.1", "3.1.0", "3.0.0+2146.b125a7c"])( "exclude POWER roles after v3 (%s)", async creationVersion => { await updateAppMetadata({ creationVersion }) @@ -93,7 +93,7 @@ describe("/api/global/roles", () => { } ) - it.each(["2.9.0", "1.0.0"])( + it.each(["2.9.0", "1.0.0", "0.0.0", "2.32.17+2146.b125a7c"])( "include POWER roles before v3 (%s)", async creationVersion => { await updateAppMetadata({ creationVersion }) From cfc5848d140d02f3f01de4d52a3a0a17e3c34190 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 17 Oct 2024 16:10:32 +0100 Subject: [PATCH 55/89] Improving how traversal is performed for role inheritance. --- packages/backend-core/src/security/roles.ts | 109 +++++++++++--------- 1 file changed, 61 insertions(+), 48 deletions(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index fdea09944a..44abc64d00 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -10,6 +10,7 @@ import { getAppDB } from "../context" import { Screen, Role as RoleDoc, RoleUIMetadata } from "@budibase/types" import cloneDeep from "lodash/fp/cloneDeep" import { RoleColor, helpers } from "@budibase/shared-core" +import { uniqBy } from "lodash" export const BUILTIN_ROLE_IDS = { ADMIN: "ADMIN", @@ -38,6 +39,14 @@ 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 @@ -76,6 +85,54 @@ export class Role implements RoleDoc { } } +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 + const 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) { + return 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 this.walk(currentRole) + } else { + foundRoleIds.push(currentRole.inherits) + currentRole = findRole(currentRole.inherits, allRoles, opts) + if (role) { + roleList.push(role) + } + } + } + } + return uniqBy(roleList, role => role._id) + } +} + const BUILTIN_ROLES = { ADMIN: new Role( BUILTIN_IDS.ADMIN, @@ -309,59 +366,15 @@ async function getAllUserRoles( if (userRoleId === BUILTIN_IDS.ADMIN) { return allRoles } - const rolesFound = (ids: string | string[]) => { - if (Array.isArray(ids)) { - return ids.filter(id => roleIds.includes(id)).length === ids.length - } else { - return roleIds.includes(ids) - } - } - - const roleIds = [userRoleId] - const roles: RoleDoc[] = [] - const iterateInherited = (role: RoleDoc | undefined) => { - if (!role || !role._id) { - return - } - roleIds.push(role._id) - roles.push(role) - if (Array.isArray(role.inherits)) { - role.inherits.forEach(roleId => { - const foundRole = findRole(roleId, allRoles, opts) - if (foundRole) { - iterateInherited(foundRole) - } - }) - } else { - while (role && role.inherits && !rolesFound(role.inherits)) { - if (Array.isArray(role.inherits)) { - iterateInherited(role) - break - } else { - roleIds.push(role.inherits) - role = findRole(role.inherits, allRoles, opts) - if (role) { - roles.push(role) - } - } - } - } - } // get all the inherited roles const foundRole = findRole(userRoleId, allRoles, opts) + let roles: RoleDoc[] = [] if (foundRole) { - iterateInherited(foundRole) + const traversal = new RoleHierarchyTraversal(allRoles, opts) + roles = traversal.walk(foundRole) } - const foundRoleIds: string[] = [] - return roles.filter(role => { - if (role._id && !foundRoleIds.includes(role._id)) { - foundRoleIds.push(role._id) - return true - } else { - return false - } - }) + return roles } export async function getUserRoleIdHierarchy( From 3da3bccc01b29266241f507ee81de5c90763305a Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 17 Oct 2024 16:27:47 +0100 Subject: [PATCH 56/89] Some fixes for traverser. --- packages/backend-core/src/security/roles.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index 44abc64d00..b3c18202d2 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -98,7 +98,7 @@ export class RoleHierarchyTraversal { const opts = this.opts, allRoles = this.allRoles // this will be a full walked list of roles - which may contain duplicates - const roleList: RoleDoc[] = [] + let roleList: RoleDoc[] = [] if (!role || !role._id) { return roleList } @@ -107,7 +107,7 @@ export class RoleHierarchyTraversal { for (let roleId of role.inherits) { const foundRole = findRole(roleId, allRoles, opts) if (foundRole) { - return this.walk(foundRole) + roleList = roleList.concat(this.walk(foundRole)) } } } else { @@ -119,14 +119,18 @@ export class RoleHierarchyTraversal { !rolesInList(foundRoleIds, currentRole.inherits) ) { if (Array.isArray(currentRole.inherits)) { - return this.walk(currentRole) + return roleList.concat(this.walk(currentRole)) } else { foundRoleIds.push(currentRole.inherits) currentRole = findRole(currentRole.inherits, allRoles, opts) - if (role) { - roleList.push(role) + if (currentRole) { + roleList.push(currentRole) } } + // loop now found - stop iterating + if (helpers.roles.checkForRoleInheritanceLoops(roleList)) { + break + } } } return uniqBy(roleList, role => role._id) @@ -359,9 +363,6 @@ async function getAllUserRoles( opts?: { defaultPublic?: boolean } ): Promise { const allRoles = await getAllRoles() - if (helpers.roles.checkForRoleInheritanceLoops(allRoles)) { - throw new Error("Loop detected in roles - cannot list roles") - } // admins have access to all roles if (userRoleId === BUILTIN_IDS.ADMIN) { return allRoles From 2ae1836b9ad75464a4f85d3eabb05a66a741ff7c Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 17 Oct 2024 16:58:51 +0100 Subject: [PATCH 57/89] PR comments. --- packages/backend-core/src/security/roles.ts | 8 +++----- packages/server/src/api/controllers/role.ts | 6 +++--- .../src/tests/utilities/TestConfiguration.ts | 16 ++++------------ .../shared-core/src/helpers/tests/roles.spec.ts | 5 +++++ 4 files changed, 15 insertions(+), 20 deletions(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index b3c18202d2..366e74bec5 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -76,11 +76,9 @@ export class Role implements RoleDoc { 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 + inherits = inherits.map(prefixRoleIDNoBuiltin) } + this.inherits = inherits return this } } @@ -264,7 +262,7 @@ export async function roleToNumber(id: string) { return findNumber(foundRole) + 1 } }) - .filter(number => !!number) + .filter(number => number) .sort() .pop() if (highestBuiltin != undefined) { diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index fef68a650c..89125f566e 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -78,10 +78,10 @@ export async function find(ctx: UserCtx) { 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 (!ctx.request.body._rev && !version) { + if (!_rev && !version) { version = roles.RoleIDVersion.NAME } const isNewVersion = version === roles.RoleIDVersion.NAME @@ -132,7 +132,7 @@ export async function save(ctx: UserCtx) { ctx.throw(400, "Role inheritance contains a loop, this is not supported") } - const foundRev = ctx.request.body._rev || dbRole?._rev + const foundRev = _rev || dbRole?._rev if (foundRev) { role._rev = foundRev } diff --git a/packages/server/src/tests/utilities/TestConfiguration.ts b/packages/server/src/tests/utilities/TestConfiguration.ts index fc5a4a7f9c..ca0a6e420a 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.ts +++ b/packages/server/src/tests/utilities/TestConfiguration.ts @@ -432,7 +432,7 @@ export default class TestConfiguration { async loginAsRole(roleId: string, cb: () => Promise) { const roleUser = await this.createUser({ roles: { - [this.prodAppId!]: roleId, + [this.getProdAppId()]: roleId, }, builder: { global: false }, admin: { global: false }, @@ -443,17 +443,9 @@ export default class TestConfiguration { builder: false, prodApp: true, }) - const temp = this.user - this.user = roleUser - await cb() - if (temp) { - this.user = temp - await this.login({ - userId: temp._id!, - builder: true, - prodApp: false, - }) - } + await this.withUser(roleUser, async () => { + await cb() + }) } defaultHeaders(extras = {}, prodApp = false) { diff --git a/packages/shared-core/src/helpers/tests/roles.spec.ts b/packages/shared-core/src/helpers/tests/roles.spec.ts index 7b67e4e9e9..051a196a5b 100644 --- a/packages/shared-core/src/helpers/tests/roles.spec.ts +++ b/packages/shared-core/src/helpers/tests/roles.spec.ts @@ -64,5 +64,10 @@ describe("role utilities", () => { role1.inherits = "role_2" check(true) }) + + it("self reference contains loop", () => { + role("role1", "role1") + check(true) + }) }) }) From bd10a3d831b0e97fe5df2090d9ceed4e127ff76e Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 17 Oct 2024 17:00:40 +0100 Subject: [PATCH 58/89] Missed comment. --- packages/backend-core/src/security/roles.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index 366e74bec5..56037f7403 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -273,12 +273,7 @@ export async function roleToNumber(id: string) { } return 0 } - let highest = 0 - for (let role of hierarchy) { - const roleNumber = findNumber(role) - highest = Math.max(roleNumber, highest) - } - return highest + return Math.max(...hierarchy.map(findNumber)) } /** From c74577c512fd3b6b063e54f7a52dd202dc92c412 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 17 Oct 2024 17:15:41 +0100 Subject: [PATCH 59/89] Updating accessible utility API. --- .../server/src/api/routes/tests/role.spec.ts | 50 +++++++++++-------- .../src/tests/utilities/TestConfiguration.ts | 23 ++++++++- .../server/src/tests/utilities/api/role.ts | 6 +-- 3 files changed, 52 insertions(+), 27 deletions(-) diff --git a/packages/server/src/api/routes/tests/role.spec.ts b/packages/server/src/api/routes/tests/role.spec.ts index 134078e6bf..a79f219311 100644 --- a/packages/server/src/api/routes/tests/role.spec.ts +++ b/packages/server/src/api/routes/tests/role.spec.ts @@ -228,29 +228,35 @@ describe("/roles", () => { }) it("should be able to fetch accessible roles (with builder)", async () => { - const res = await config.api.roles.accessible(config.defaultHeaders(), { - status: 200, + 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") }) - 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() - const res = await config.api.roles.accessible(headers, { - status: 200, + 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") }) - 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 () => { - const res = await config.api.roles.accessible(config.publicHeaders(), { - status: 200, + 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") }) - 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 () => { @@ -261,13 +267,15 @@ describe("/roles", () => { permissionId: permissions.BuiltinPermissionID.READ_ONLY, version: "name", }) - const res = await config.api.roles.accessible( + await config.withHeaders( { "x-budibase-role": customRoleName }, - { - status: 200, + async () => { + const res = await config.api.roles.accessible({ + status: 200, + }) + expect(res).toEqual([customRoleName, "BASIC", "PUBLIC"]) } ) - expect(res).toEqual([customRoleName, "BASIC", "PUBLIC"]) }) }) @@ -297,10 +305,12 @@ describe("/roles", () => { const headers = await config.roleHeaders({ roleId: role3, }) - const res = await config.api.roles.accessible(headers, { - status: 200, + await config.withHeaders(headers, async () => { + const res = await config.api.roles.accessible({ + status: 200, + }) + expect(res).toEqual([role3, role1, "BASIC", "PUBLIC", role2, "POWER"]) }) - expect(res).toEqual([role3, role1, "BASIC", "PUBLIC", role2, "POWER"]) }) }) }) diff --git a/packages/server/src/tests/utilities/TestConfiguration.ts b/packages/server/src/tests/utilities/TestConfiguration.ts index ca0a6e420a..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) { @@ -448,6 +449,18 @@ export default class TestConfiguration { }) } + 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() @@ -471,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 } = {}) { @@ -487,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 31bffc6f85..5bd0647384 100644 --- a/packages/server/src/tests/utilities/api/role.ts +++ b/packages/server/src/tests/utilities/api/role.ts @@ -34,12 +34,8 @@ export class RoleAPI extends TestAPI { }) } - accessible = async ( - headers: Record, - expectations?: Expectations - ) => { + accessible = async (expectations?: Expectations) => { return await this._get(`/api/roles/accessible`, { - headers, expectations, }) } From 47de88f42fd08108c6d9eb08045c1710c8347151 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 17 Oct 2024 17:47:55 +0100 Subject: [PATCH 60/89] Destroy test case. --- packages/backend-core/src/security/roles.ts | 26 ++++++++++++- packages/server/src/api/controllers/role.ts | 38 +++++++++++++------ .../server/src/api/routes/tests/role.spec.ts | 21 ++++++++++ 3 files changed, 73 insertions(+), 12 deletions(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index e549d2f3af..c14178cacb 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -290,11 +290,23 @@ export function lowerBuiltinRoleID(roleId1?: string, roleId2?: string): string { : roleId1 } -function compareRoleIds(roleId1: string, roleId2: string) { +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. */ @@ -347,6 +359,18 @@ export async function getRole( 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!), + })) + ) +} + /** * Simple function to get all the roles based on the top level user role ID. */ diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index 89125f566e..f26b4bae69 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -27,15 +27,27 @@ const UpdateRolesOptions = { REMOVED: "removed", } -function externalRole(role: Role): Role { - let _id: string | undefined - if (role._id) { - _id = roles.getExternalRoleID(role._id) +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) + } } - return { - ...role, - _id, - inherits: roles.getExternalRoleIDs(role.inherits, role.version), + if (updated.length) { + await roles.saveRoles(updated) } } @@ -65,7 +77,7 @@ async function updateRolesOnUserTable( } export async function fetch(ctx: UserCtx) { - ctx.body = (await roles.getAllRoles()).map(role => externalRole(role)) + ctx.body = (await roles.getAllRoles()).map(role => roles.externalRole(role)) } export async function find(ctx: UserCtx) { @@ -73,7 +85,7 @@ export async function find(ctx: UserCtx) { if (!role) { ctx.throw(404, { message: "Role not found" }) } - ctx.body = externalRole(role) + ctx.body = roles.externalRole(role) } export async function save(ctx: UserCtx) { @@ -149,7 +161,7 @@ export async function save(ctx: UserCtx) { role.version ) role._rev = result.rev - ctx.body = externalRole(role) + ctx.body = roles.externalRole(role) const devDb = context.getDevAppDB() const prodDb = context.getProdAppDB() @@ -198,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 } diff --git a/packages/server/src/api/routes/tests/role.spec.ts b/packages/server/src/api/routes/tests/role.spec.ts index a79f219311..5668295342 100644 --- a/packages/server/src/api/routes/tests/role.spec.ts +++ b/packages/server/src/api/routes/tests/role.spec.ts @@ -217,6 +217,27 @@ describe("/roles", () => { _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", () => { From d0df75c9e421b6f9caa981f1234b1ded1affb768 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 17 Oct 2024 17:50:51 +0100 Subject: [PATCH 61/89] Adding back mock clearing. --- packages/server/src/api/routes/tests/screen.spec.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/server/src/api/routes/tests/screen.spec.ts b/packages/server/src/api/routes/tests/screen.spec.ts index 5853f44bac..5dfe3d2a44 100644 --- a/packages/server/src/api/routes/tests/screen.spec.ts +++ b/packages/server/src/api/routes/tests/screen.spec.ts @@ -110,6 +110,10 @@ describe("/screens", () => { }) 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, { @@ -127,6 +131,7 @@ describe("/screens", () => { screen._id = responseScreen._id screen._rev = responseScreen._rev screen.name = "edit" + jest.clearAllMocks() responseScreen = await config.api.screen.save(screen, { status: 200 }) From 7941cb660292396fb52e292406064875d74478f0 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 18 Oct 2024 11:36:11 +0200 Subject: [PATCH 62/89] Allow passing test to row action test --- .../automation/AutomationBuilder/FlowChart/TestDataModal.svelte | 1 + .../components/automation/SetupPanel/AutomationBlockSetup.svelte | 1 + packages/builder/src/constants/backend/automations.js | 1 + 3 files changed, 3 insertions(+) diff --git a/packages/builder/src/components/automation/AutomationBuilder/FlowChart/TestDataModal.svelte b/packages/builder/src/components/automation/AutomationBuilder/FlowChart/TestDataModal.svelte index f656a1ecb9..21cdc4b893 100644 --- a/packages/builder/src/components/automation/AutomationBuilder/FlowChart/TestDataModal.svelte +++ b/packages/builder/src/components/automation/AutomationBuilder/FlowChart/TestDataModal.svelte @@ -19,6 +19,7 @@ AutomationEventType.ROW_DELETE, AutomationEventType.ROW_UPDATE, AutomationEventType.ROW_SAVE, + AutomationEventType.ROW_ACTION, ] /** diff --git a/packages/builder/src/components/automation/SetupPanel/AutomationBlockSetup.svelte b/packages/builder/src/components/automation/SetupPanel/AutomationBlockSetup.svelte index bc65c234e9..bf5fd702cd 100644 --- a/packages/builder/src/components/automation/SetupPanel/AutomationBlockSetup.svelte +++ b/packages/builder/src/components/automation/SetupPanel/AutomationBlockSetup.svelte @@ -76,6 +76,7 @@ TriggerStepID.ROW_UPDATED, TriggerStepID.ROW_SAVED, TriggerStepID.ROW_DELETED, + TriggerStepID.ROW_ACTION, ] const rowEvents = [ diff --git a/packages/builder/src/constants/backend/automations.js b/packages/builder/src/constants/backend/automations.js index 7c3e17e225..37ae35aea0 100644 --- a/packages/builder/src/constants/backend/automations.js +++ b/packages/builder/src/constants/backend/automations.js @@ -2,6 +2,7 @@ export const TriggerStepID = { ROW_SAVED: "ROW_SAVED", ROW_UPDATED: "ROW_UPDATED", ROW_DELETED: "ROW_DELETED", + ROW_ACTION: "ROW_ACTION", WEBHOOK: "WEBHOOK", APP: "APP", CRON: "CRON", From a26b64bb03b97879d7e2cfc3eefa6415230e2f92 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Fri, 18 Oct 2024 15:05:17 +0100 Subject: [PATCH 63/89] pass user through to automation context --- .../server/src/api/controllers/automation.ts | 4 +++ .../server/src/api/controllers/row/index.ts | 25 +++++++++++--- .../src/api/controllers/rowAction/run.ts | 2 +- .../tests/scenarios/scenarios.spec.ts | 34 +++++++++++++++++++ packages/server/src/automations/triggers.ts | 10 +++++- .../server/src/definitions/automations.ts | 3 +- packages/server/src/events/BudibaseEmitter.ts | 7 ++-- packages/server/src/events/utils.ts | 6 +++- packages/server/src/sdk/app/rowActions.ts | 9 ++++- packages/server/src/threads/automation.ts | 5 ++- .../documents/app/automation/automation.ts | 1 + packages/types/src/sdk/automations/index.ts | 3 +- 12 files changed, 95 insertions(+), 14 deletions(-) diff --git a/packages/server/src/api/controllers/automation.ts b/packages/server/src/api/controllers/automation.ts index 6177868303..df23f9f3b7 100644 --- a/packages/server/src/api/controllers/automation.ts +++ b/packages/server/src/api/controllers/automation.ts @@ -13,6 +13,7 @@ import { UserCtx, DeleteAutomationResponse, FetchAutomationResponse, + User, } from "@budibase/types" import { getActionDefinitions as actionDefs } from "../../automations/actions" import sdk from "../../sdk" @@ -159,6 +160,7 @@ export async function trigger(ctx: UserCtx) { automation, { fields: ctx.request.body.fields, + user: ctx.user as User, timeout: ctx.request.body.timeout * 1000 || env.AUTOMATION_THREAD_TIMEOUT, }, @@ -183,6 +185,7 @@ export async function trigger(ctx: UserCtx) { await triggers.externalTrigger(automation, { ...ctx.request.body, appId: ctx.appId, + user: ctx.user as User, }) ctx.body = { message: `Automation ${automation._id} has been triggered.`, @@ -212,6 +215,7 @@ export async function test(ctx: UserCtx) { { ...testInput, appId: ctx.appId, + user: ctx.user, }, { getResponses: true } ) diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts index 60775ce628..67502f8b36 100644 --- a/packages/server/src/api/controllers/row/index.ts +++ b/packages/server/src/api/controllers/row/index.ts @@ -65,7 +65,14 @@ export async function patch( } ctx.status = 200 ctx.eventEmitter && - ctx.eventEmitter.emitRow(`row:update`, appId, row, table, oldRow) + ctx.eventEmitter.emitRow( + `row:update`, + appId, + row, + table, + oldRow, + ctx.user + ) ctx.message = `${table.name} updated successfully.` ctx.body = row gridSocket?.emitRowUpdate(ctx, row) @@ -96,7 +103,8 @@ export const save = async (ctx: UserCtx) => { sdk.rows.save(sourceId, ctx.request.body, ctx.user?._id) ) ctx.status = 200 - ctx.eventEmitter && ctx.eventEmitter.emitRow(`row:save`, appId, row, table) + ctx.eventEmitter && + ctx.eventEmitter.emitRow(`row:save`, appId, row, table, null, ctx.user) ctx.message = `${table.name} saved successfully` // prefer squashed for response ctx.body = row || squashed @@ -168,7 +176,8 @@ async function deleteRows(ctx: UserCtx) { } for (let row of rows) { - ctx.eventEmitter && ctx.eventEmitter.emitRow(`row:delete`, appId, row) + ctx.eventEmitter && + ctx.eventEmitter.emitRow(`row:delete`, appId, row, null, null, ctx.user) gridSocket?.emitRowDeletion(ctx, row) } @@ -184,7 +193,15 @@ async function deleteRow(ctx: UserCtx) { await quotas.removeRow() } - ctx.eventEmitter && ctx.eventEmitter.emitRow(`row:delete`, appId, resp.row) + ctx.eventEmitter && + ctx.eventEmitter.emitRow( + `row:delete`, + appId, + resp.row, + null, + null, + ctx.user + ) gridSocket?.emitRowDeletion(ctx, resp.row) return resp diff --git a/packages/server/src/api/controllers/rowAction/run.ts b/packages/server/src/api/controllers/rowAction/run.ts index c4b6a276bf..05bdb7bace 100644 --- a/packages/server/src/api/controllers/rowAction/run.ts +++ b/packages/server/src/api/controllers/rowAction/run.ts @@ -5,6 +5,6 @@ export async function run(ctx: Ctx) { const { tableId, actionId } = ctx.params const { rowId } = ctx.request.body - await sdk.rowActions.run(tableId, actionId, rowId) + await sdk.rowActions.run(tableId, actionId, rowId, ctx.user) ctx.status = 200 } diff --git a/packages/server/src/automations/tests/scenarios/scenarios.spec.ts b/packages/server/src/automations/tests/scenarios/scenarios.spec.ts index 850a04a95a..358aba35c8 100644 --- a/packages/server/src/automations/tests/scenarios/scenarios.spec.ts +++ b/packages/server/src/automations/tests/scenarios/scenarios.spec.ts @@ -482,4 +482,38 @@ describe("Automation Scenarios", () => { } ) }) + + it("Check user is passed through from row trigger", async () => { + const table = await config.createTable() + + const builder = createAutomationBuilder({ + name: "Test a user is successfully passed from the trigger", + }) + + const results = await builder + .rowUpdated( + { tableId: table._id! }, + { + row: { name: "Test", description: "TEST" }, + id: "1234", + } + ) + .serverLog({ text: "{{ [user].[email] }}" }) + .run() + + expect(results.steps[0].outputs.message).toContain("example.com") + }) + + it("Check user is passed through from app trigger", async () => { + const builder = createAutomationBuilder({ + name: "Test a user is successfully passed from the trigger", + }) + + const results = await builder + .appAction({ fields: {} }) + .serverLog({ text: "{{ [user].[email] }}" }) + .run() + + expect(results.steps[0].outputs.message).toContain("example.com") + }) }) diff --git a/packages/server/src/automations/triggers.ts b/packages/server/src/automations/triggers.ts index 110ccfa37a..efcbf27644 100644 --- a/packages/server/src/automations/triggers.ts +++ b/packages/server/src/automations/triggers.ts @@ -19,6 +19,7 @@ import { AutomationStoppedReason, AutomationStatus, AutomationRowEvent, + User, } from "@budibase/types" import { executeInThread } from "../threads/automation" import { dataFilters, sdk } from "@budibase/shared-core" @@ -140,9 +141,15 @@ function rowPassesFilters(row: Row, filters: SearchFilters) { export async function externalTrigger( automation: Automation, - params: { fields: Record; timeout?: number; appId?: string }, + params: { + fields: Record + timeout?: number + appId?: string + user?: User + }, { getResponses }: { getResponses?: boolean } = {} ): Promise { + console.log("user: " + params.user) if (automation.disabled) { throw new Error("Automation is disabled") } @@ -189,6 +196,7 @@ export async function externalTrigger( appId: context.getAppId(), automation, } + console.log(data) return executeInThread({ data } as AutomationJob) } else { return automationQueue.add(data, JOB_OPTS) diff --git a/packages/server/src/definitions/automations.ts b/packages/server/src/definitions/automations.ts index 9433075da7..70faed9327 100644 --- a/packages/server/src/definitions/automations.ts +++ b/packages/server/src/definitions/automations.ts @@ -1,4 +1,4 @@ -import { AutomationResults, LoopStepType } from "@budibase/types" +import { AutomationResults, LoopStepType, User } from "@budibase/types" export interface LoopInput { option: LoopStepType @@ -18,5 +18,6 @@ export interface AutomationContext extends AutomationResults { stepsById: Record stepsByName: Record env?: Record + user?: User trigger: any } diff --git a/packages/server/src/events/BudibaseEmitter.ts b/packages/server/src/events/BudibaseEmitter.ts index 8feb36bbf5..d09ea20a5d 100644 --- a/packages/server/src/events/BudibaseEmitter.ts +++ b/packages/server/src/events/BudibaseEmitter.ts @@ -1,6 +1,6 @@ import { EventEmitter } from "events" import { rowEmission, tableEmission } from "./utils" -import { Table, Row } from "@budibase/types" +import { Table, Row, User } from "@budibase/types" /** * keeping event emitter in one central location as it might be used for things other than @@ -18,9 +18,10 @@ class BudibaseEmitter extends EventEmitter { appId: string, row: Row, table?: Table, - oldRow?: Row + oldRow?: Row, + user?: User ) { - rowEmission({ emitter: this, eventName, appId, row, table, oldRow }) + rowEmission({ emitter: this, eventName, appId, row, table, oldRow, user }) } emitTable(eventName: string, appId: string, table?: Table) { diff --git a/packages/server/src/events/utils.ts b/packages/server/src/events/utils.ts index b972c8e473..5e4a1bebbf 100644 --- a/packages/server/src/events/utils.ts +++ b/packages/server/src/events/utils.ts @@ -1,4 +1,4 @@ -import { Table, Row } from "@budibase/types" +import { Table, Row, User } from "@budibase/types" import BudibaseEmitter from "./BudibaseEmitter" type BBEventOpts = { @@ -9,6 +9,7 @@ type BBEventOpts = { row?: Row oldRow?: Row metadata?: any + user?: User } interface BBEventTable extends Table { @@ -24,6 +25,7 @@ type BBEvent = { id?: string revision?: string metadata?: any + user?: User } export function rowEmission({ @@ -34,12 +36,14 @@ export function rowEmission({ table, metadata, oldRow, + user, }: BBEventOpts) { let event: BBEvent = { row, oldRow, appId, tableId: row?.tableId, + user, } if (table) { event.table = table diff --git a/packages/server/src/sdk/app/rowActions.ts b/packages/server/src/sdk/app/rowActions.ts index 3d8a6fd9be..ed87133b5d 100644 --- a/packages/server/src/sdk/app/rowActions.ts +++ b/packages/server/src/sdk/app/rowActions.ts @@ -4,6 +4,7 @@ import { AutomationTriggerStepId, SEPARATOR, TableRowActions, + User, VirtualDocumentType, } from "@budibase/types" import { generateRowActionsID } from "../../db/utils" @@ -236,7 +237,12 @@ export async function remove(tableId: string, rowActionId: string) { }) } -export async function run(tableId: any, rowActionId: any, rowId: string) { +export async function run( + tableId: any, + rowActionId: any, + rowId: string, + user: User +) { const table = await sdk.tables.getTable(tableId) if (!table) { throw new HTTPError("Table not found", 404) @@ -258,6 +264,7 @@ export async function run(tableId: any, rowActionId: any, rowId: string) { row, table, }, + user, appId: context.getAppId(), }, { getResponses: true } diff --git a/packages/server/src/threads/automation.ts b/packages/server/src/threads/automation.ts index 3b47634663..a75c0d2870 100644 --- a/packages/server/src/threads/automation.ts +++ b/packages/server/src/threads/automation.ts @@ -26,6 +26,7 @@ import { BranchStep, LoopStep, SearchFilters, + User, } from "@budibase/types" import { AutomationContext, TriggerOutput } from "../definitions/automations" import { WorkerCallback } from "./definitions" @@ -75,6 +76,7 @@ class Orchestrator { private loopStepOutputs: LoopStep[] private stopped: boolean private executionOutput: Omit + private currentUser: User | undefined constructor(job: AutomationJob) { let automation = job.data.automation @@ -106,6 +108,7 @@ class Orchestrator { this.updateExecutionOutput(triggerId, triggerStepId, null, triggerOutput) this.loopStepOutputs = [] this.stopped = false + this.currentUser = triggerOutput.user } cleanupTriggerOutputs(stepId: string, triggerOutput: TriggerOutput) { @@ -258,6 +261,7 @@ class Orchestrator { automationId: this.automation._id, }) this.context.env = await sdkUtils.getEnvironmentVariables() + this.context.user = this.currentUser let metadata @@ -572,7 +576,6 @@ class Orchestrator { originalStepInput, this.processContext(this.context) ) - inputs = automationUtils.cleanInputValues(inputs, step.schema.inputs) const outputs = await stepFn({ diff --git a/packages/types/src/documents/app/automation/automation.ts b/packages/types/src/documents/app/automation/automation.ts index effe99a328..1af892d8d1 100644 --- a/packages/types/src/documents/app/automation/automation.ts +++ b/packages/types/src/documents/app/automation/automation.ts @@ -261,6 +261,7 @@ export type UpdatedRowEventEmitter = { oldRow: Row table: Table appId: string + user: User } export enum LoopStepType { diff --git a/packages/types/src/sdk/automations/index.ts b/packages/types/src/sdk/automations/index.ts index d04f126c32..f5c57b54d8 100644 --- a/packages/types/src/sdk/automations/index.ts +++ b/packages/types/src/sdk/automations/index.ts @@ -1,4 +1,4 @@ -import { Automation, AutomationMetadata, Row } from "../../documents" +import { Automation, AutomationMetadata, Row, User } from "../../documents" import { Job } from "bull" export interface AutomationDataEvent { @@ -8,6 +8,7 @@ export interface AutomationDataEvent { timeout?: number row?: Row oldRow?: Row + user?: User } export interface AutomationData { From 2b7742d96fba71cb16ee3741e9732ab35eddb476 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Fri, 18 Oct 2024 15:12:26 +0100 Subject: [PATCH 64/89] pro --- packages/pro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pro b/packages/pro index fc4c7f4925..1a749caba9 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit fc4c7f4925139af078480217965c3d6338dc0a7f +Subproject commit 1a749caba9c85aab2645e5d00db479eb53d3f80f From 4cabc09f8abb80fca08bab6375ea35f4353f43d1 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Fri, 18 Oct 2024 15:35:31 +0100 Subject: [PATCH 65/89] fix row actions test --- packages/server/src/api/routes/tests/rowAction.spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/server/src/api/routes/tests/rowAction.spec.ts b/packages/server/src/api/routes/tests/rowAction.spec.ts index 7140f3486e..d3b90f6f6b 100644 --- a/packages/server/src/api/routes/tests/rowAction.spec.ts +++ b/packages/server/src/api/routes/tests/rowAction.spec.ts @@ -783,6 +783,7 @@ describe("/rowsActions", () => { ...(await config.api.table.get(tableId)), views: expect.anything(), }, + user: expect.anything(), automation: expect.objectContaining({ _id: rowAction.automationId, }), From 0f705763b0656558b2262b5b7aad30be0a037c1e Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 18 Oct 2024 16:46:21 +0200 Subject: [PATCH 66/89] Change QA branch trigger --- .github/workflows/budibase_ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/budibase_ci.yml b/.github/workflows/budibase_ci.yml index 4b9ebf1e5d..49e77ab13a 100644 --- a/.github/workflows/budibase_ci.yml +++ b/.github/workflows/budibase_ci.yml @@ -9,7 +9,7 @@ on: # but only for the master branch push: branches: - - master + - v3-ui pull_request: workflow_dispatch: workflow_call: From f119c517306fc4f9c8408dd855a5077fd2e76ca7 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 18 Oct 2024 16:52:22 +0200 Subject: [PATCH 67/89] Change QA branch trigger --- .github/workflows/budibase_ci.yml | 2 +- .github/workflows/deploy-qa.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/budibase_ci.yml b/.github/workflows/budibase_ci.yml index 49e77ab13a..4b9ebf1e5d 100644 --- a/.github/workflows/budibase_ci.yml +++ b/.github/workflows/budibase_ci.yml @@ -9,7 +9,7 @@ on: # but only for the master branch push: branches: - - v3-ui + - master pull_request: workflow_dispatch: workflow_call: diff --git a/.github/workflows/deploy-qa.yml b/.github/workflows/deploy-qa.yml index 1339ad2eb9..2a30e44def 100644 --- a/.github/workflows/deploy-qa.yml +++ b/.github/workflows/deploy-qa.yml @@ -3,7 +3,7 @@ name: Deploy QA on: push: branches: - - master + - v3-ui workflow_dispatch: jobs: From 94789ff03d953a39463189f453ac1a09cab690f4 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 18 Oct 2024 16:51:23 +0100 Subject: [PATCH 68/89] Adding all feature flags to enum, just so there is a simple reference to see all feature flags that are available. --- packages/backend-core/src/features/features.ts | 6 +++--- packages/types/src/sdk/featureFlag.ts | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/backend-core/src/features/features.ts b/packages/backend-core/src/features/features.ts index e95472a784..39b8558d2a 100644 --- a/packages/backend-core/src/features/features.ts +++ b/packages/backend-core/src/features/features.ts @@ -267,9 +267,9 @@ export class FlagSet, T extends { [key: string]: V }> { // All of the machinery in this file is to make sure that flags have their // default values set correctly and their types flow through the system. export const flags = new FlagSet({ - DEFAULT_VALUES: Flag.boolean(env.isDev()), - AUTOMATION_BRANCHING: Flag.boolean(env.isDev()), - SQS: Flag.boolean(true), + [FeatureFlag.DEFAULT_VALUES]: Flag.boolean(env.isDev()), + [FeatureFlag.AUTOMATION_BRANCHING]: Flag.boolean(env.isDev()), + [FeatureFlag.SQS]: Flag.boolean(true), [FeatureFlag.AI_CUSTOM_CONFIGS]: Flag.boolean(env.isDev()), [FeatureFlag.ENRICHED_RELATIONSHIPS]: Flag.boolean(env.isDev()), [FeatureFlag.TABLES_DEFAULT_ADMIN]: Flag.boolean(env.isDev()), diff --git a/packages/types/src/sdk/featureFlag.ts b/packages/types/src/sdk/featureFlag.ts index e35929efe9..a1033236d7 100644 --- a/packages/types/src/sdk/featureFlag.ts +++ b/packages/types/src/sdk/featureFlag.ts @@ -1,7 +1,10 @@ export enum FeatureFlag { PER_CREATOR_PER_USER_PRICE = "PER_CREATOR_PER_USER_PRICE", PER_CREATOR_PER_USER_PRICE_ALERT = "PER_CREATOR_PER_USER_PRICE_ALERT", + AUTOMATION_BRANCHING = "AUTOMATION_BRANCHING", + SQS = "SQS", AI_CUSTOM_CONFIGS = "AI_CUSTOM_CONFIGS", + DEFAULT_VALUES = "DEFAULT_VALUES", ENRICHED_RELATIONSHIPS = "ENRICHED_RELATIONSHIPS", TABLES_DEFAULT_ADMIN = "TABLES_DEFAULT_ADMIN", } From 87bdd68afaf57aade6bf560f670a3522d68ba3d1 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 18 Oct 2024 17:07:35 +0100 Subject: [PATCH 69/89] Updating to use enum for feature flags across the board. --- .../backend-core/src/db/couch/DatabaseImpl.ts | 3 +- packages/pro | 2 +- .../server/src/api/controllers/table/utils.ts | 31 ++++++++++--------- packages/server/src/automations/actions.ts | 3 +- packages/server/src/sdk/app/rows/search.ts | 5 +-- packages/server/src/sdk/app/tables/getters.ts | 3 +- .../src/utilities/rowProcessor/index.ts | 3 +- .../src/api/controllers/system/environment.ts | 7 +++-- 8 files changed, 33 insertions(+), 24 deletions(-) diff --git a/packages/backend-core/src/db/couch/DatabaseImpl.ts b/packages/backend-core/src/db/couch/DatabaseImpl.ts index 8ca20bf8e1..b807db0ee3 100644 --- a/packages/backend-core/src/db/couch/DatabaseImpl.ts +++ b/packages/backend-core/src/db/couch/DatabaseImpl.ts @@ -10,6 +10,7 @@ import { DatabaseQueryOpts, DBError, Document, + FeatureFlag, isDocument, RowResponse, RowValue, @@ -456,7 +457,7 @@ export class DatabaseImpl implements Database { async destroy() { if ( - (await flags.isEnabled("SQS")) && + (await flags.isEnabled(FeatureFlag.SQS)) && (await this.exists(SQLITE_DESIGN_DOC_ID)) ) { // delete the design document, then run the cleanup operation diff --git a/packages/pro b/packages/pro index fc4c7f4925..cad8edfba8 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit fc4c7f4925139af078480217965c3d6338dc0a7f +Subproject commit cad8edfba856058cd12dd9e7363b998ddea76034 diff --git a/packages/server/src/api/controllers/table/utils.ts b/packages/server/src/api/controllers/table/utils.ts index 106d0e23a6..96c01a15b8 100644 --- a/packages/server/src/api/controllers/table/utils.ts +++ b/packages/server/src/api/controllers/table/utils.ts @@ -1,34 +1,35 @@ -import { parse, isSchema, isRows } from "../../../utilities/schema" -import { getRowParams, generateRowID, InternalTables } from "../../../db/utils" +import { isRows, isSchema, parse } from "../../../utilities/schema" +import { generateRowID, getRowParams, InternalTables } from "../../../db/utils" import isEqual from "lodash/isEqual" import { - GOOGLE_SHEETS_PRIMARY_KEY, - USERS_TABLE_SCHEMA, - SwitchableTypes, CanSwitchTypes, + GOOGLE_SHEETS_PRIMARY_KEY, + SwitchableTypes, + USERS_TABLE_SCHEMA, } from "../../../constants" import { - inputProcessing, AttachmentCleanup, + inputProcessing, } from "../../../utilities/rowProcessor" import { getViews, saveView } from "../view/utils" import viewTemplate from "../view/viewBuilder" import { cloneDeep } from "lodash/fp" import { quotas } from "@budibase/pro" -import { events, context, features } from "@budibase/backend-core" +import { context, events, features } from "@budibase/backend-core" import { AutoFieldSubType, + Database, Datasource, + FeatureFlag, + FieldSchema, + FieldType, + NumberFieldMetadata, + RelationshipFieldMetadata, + RenameColumn, Row, SourceName, Table, - Database, - RenameColumn, - NumberFieldMetadata, - FieldSchema, View, - RelationshipFieldMetadata, - FieldType, } from "@budibase/types" import sdk from "../../../sdk" import env from "../../../environment" @@ -329,7 +330,7 @@ class TableSaveFunctions { importRows: this.importRows, userId: this.userId, }) - if (await features.flags.isEnabled("SQS")) { + if (await features.flags.isEnabled(FeatureFlag.SQS)) { await sdk.tables.sqs.addTable(table) } return table @@ -523,7 +524,7 @@ export async function internalTableCleanup(table: Table, rows?: Row[]) { if (rows) { await AttachmentCleanup.tableDelete(table, rows) } - if (await features.flags.isEnabled("SQS")) { + if (await features.flags.isEnabled(FeatureFlag.SQS)) { await sdk.tables.sqs.removeTable(table) } } diff --git a/packages/server/src/automations/actions.ts b/packages/server/src/automations/actions.ts index f78174aa7a..e921e569c9 100644 --- a/packages/server/src/automations/actions.ts +++ b/packages/server/src/automations/actions.ts @@ -26,6 +26,7 @@ import { Hosting, ActionImplementation, AutomationStepDefinition, + FeatureFlag, } from "@budibase/types" import sdk from "../sdk" import { getAutomationPlugin } from "../utilities/fileSystem" @@ -100,7 +101,7 @@ if (env.SELF_HOSTED) { } export async function getActionDefinitions() { - if (await features.flags.isEnabled("AUTOMATION_BRANCHING")) { + if (await features.flags.isEnabled(FeatureFlag.AUTOMATION_BRANCHING)) { BUILTIN_ACTION_DEFINITIONS["BRANCH"] = branch.definition } const actionDefinitions = BUILTIN_ACTION_DEFINITIONS diff --git a/packages/server/src/sdk/app/rows/search.ts b/packages/server/src/sdk/app/rows/search.ts index 7ac3bb8ead..f80c1c1f8a 100644 --- a/packages/server/src/sdk/app/rows/search.ts +++ b/packages/server/src/sdk/app/rows/search.ts @@ -1,5 +1,6 @@ import { EmptyFilterOption, + FeatureFlag, LegacyFilter, LogicalOperator, Row, @@ -101,7 +102,7 @@ export async function search( viewQuery = checkFilters(table, viewQuery) delete viewQuery?.onEmptyFilter - const sqsEnabled = await features.flags.isEnabled("SQS") + const sqsEnabled = await features.flags.isEnabled(FeatureFlag.SQS) const supportsLogicalOperators = isExternalTableID(view.tableId) || sqsEnabled @@ -168,7 +169,7 @@ export async function search( if (isExternalTable) { span?.addTags({ searchType: "external" }) result = await external.search(options, source) - } else if (await features.flags.isEnabled("SQS")) { + } else if (await features.flags.isEnabled(FeatureFlag.SQS)) { span?.addTags({ searchType: "sqs" }) result = await internal.sqs.search(options, source) } else { diff --git a/packages/server/src/sdk/app/tables/getters.ts b/packages/server/src/sdk/app/tables/getters.ts index 49944bce85..7b3d6913cf 100644 --- a/packages/server/src/sdk/app/tables/getters.ts +++ b/packages/server/src/sdk/app/tables/getters.ts @@ -12,6 +12,7 @@ import { TableResponse, TableSourceType, TableViewsResponse, + FeatureFlag, } from "@budibase/types" import datasources from "../datasources" import sdk from "../../../sdk" @@ -39,7 +40,7 @@ export async function processTable(table: Table): Promise { sourceId: table.sourceId || INTERNAL_TABLE_SOURCE_ID, sourceType: TableSourceType.INTERNAL, } - const sqsEnabled = await features.flags.isEnabled("SQS") + const sqsEnabled = await features.flags.isEnabled(FeatureFlag.SQS) if (sqsEnabled) { processed.sql = true } diff --git a/packages/server/src/utilities/rowProcessor/index.ts b/packages/server/src/utilities/rowProcessor/index.ts index dd17bc9ba6..6872924f58 100644 --- a/packages/server/src/utilities/rowProcessor/index.ts +++ b/packages/server/src/utilities/rowProcessor/index.ts @@ -19,6 +19,7 @@ import { Table, User, ViewV2, + FeatureFlag, } from "@budibase/types" import { cloneDeep } from "lodash/fp" import { @@ -417,7 +418,7 @@ export async function coreOutputProcessing( // remove null properties to match internal API const isExternal = isExternalTableID(table._id!) - if (isExternal || (await features.flags.isEnabled("SQS"))) { + if (isExternal || (await features.flags.isEnabled(FeatureFlag.SQS))) { for (const row of rows) { for (const key of Object.keys(row)) { if (row[key] === null) { diff --git a/packages/worker/src/api/controllers/system/environment.ts b/packages/worker/src/api/controllers/system/environment.ts index 90f303d3a8..b6352ea5e7 100644 --- a/packages/worker/src/api/controllers/system/environment.ts +++ b/packages/worker/src/api/controllers/system/environment.ts @@ -1,4 +1,4 @@ -import { Ctx, MaintenanceType } from "@budibase/types" +import { Ctx, MaintenanceType, FeatureFlag } from "@budibase/types" import env from "../../../environment" import { env as coreEnv, db as dbCore, features } from "@budibase/backend-core" import nodeFetch from "node-fetch" @@ -29,7 +29,10 @@ async function isSqsAvailable() { } async function isSqsMissing() { - return (await features.flags.isEnabled("SQS")) && !(await isSqsAvailable()) + return ( + (await features.flags.isEnabled(FeatureFlag.SQS)) && + !(await isSqsAvailable()) + ) } export const fetch = async (ctx: Ctx) => { From b94498e5832ab7f96a6785e352e8fc3e355a6107 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 18 Oct 2024 17:29:22 +0100 Subject: [PATCH 70/89] Backporting types from v3 to support updating pro submodule. --- packages/backend-core/src/sql/sqlTable.ts | 5 +- packages/shared-core/src/table.ts | 3 + packages/types/src/documents/app/row.ts | 7 ++ .../src/documents/app/table/constants.ts | 1 + .../types/src/documents/app/table/schema.ts | 13 +++ packages/types/src/sdk/ai.ts | 91 +++++++++++++++++++ packages/types/src/sdk/index.ts | 1 + 7 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 packages/types/src/sdk/ai.ts diff --git a/packages/backend-core/src/sql/sqlTable.ts b/packages/backend-core/src/sql/sqlTable.ts index f5b02cc4e4..84f4e290aa 100644 --- a/packages/backend-core/src/sql/sqlTable.ts +++ b/packages/backend-core/src/sql/sqlTable.ts @@ -17,7 +17,7 @@ import SchemaBuilder = Knex.SchemaBuilder import CreateTableBuilder = Knex.CreateTableBuilder function isIgnoredType(type: FieldType) { - const ignored = [FieldType.LINK, FieldType.FORMULA] + const ignored = [FieldType.LINK, FieldType.FORMULA, FieldType.AI] return ignored.indexOf(type) !== -1 } @@ -144,6 +144,9 @@ function generateSchema( case FieldType.FORMULA: // This is allowed, but nothing to do on the external datasource break + case FieldType.AI: + // This is allowed, but nothing to do on the external datasource + break case FieldType.ATTACHMENTS: case FieldType.ATTACHMENT_SINGLE: case FieldType.SIGNATURE_SINGLE: diff --git a/packages/shared-core/src/table.ts b/packages/shared-core/src/table.ts index e81e8266cc..73e3ad6cb7 100644 --- a/packages/shared-core/src/table.ts +++ b/packages/shared-core/src/table.ts @@ -8,6 +8,7 @@ const allowDisplayColumnByType: Record = { [FieldType.NUMBER]: true, [FieldType.DATETIME]: true, [FieldType.FORMULA]: true, + [FieldType.AI]: true, [FieldType.AUTO]: true, [FieldType.INTERNAL]: true, [FieldType.BARCODEQR]: true, @@ -38,6 +39,7 @@ const allowSortColumnByType: Record = { [FieldType.JSON]: true, [FieldType.FORMULA]: false, + [FieldType.AI]: false, [FieldType.ATTACHMENTS]: false, [FieldType.ATTACHMENT_SINGLE]: false, [FieldType.SIGNATURE_SINGLE]: false, @@ -62,6 +64,7 @@ const allowDefaultColumnByType: Record = { [FieldType.BIGINT]: false, [FieldType.BOOLEAN]: false, [FieldType.FORMULA]: false, + [FieldType.AI]: false, [FieldType.ATTACHMENTS]: false, [FieldType.ATTACHMENT_SINGLE]: false, [FieldType.SIGNATURE_SINGLE]: false, diff --git a/packages/types/src/documents/app/row.ts b/packages/types/src/documents/app/row.ts index 673d481e3c..b0c5267b37 100644 --- a/packages/types/src/documents/app/row.ts +++ b/packages/types/src/documents/app/row.ts @@ -76,6 +76,13 @@ export enum FieldType { * that is part of the initial formula definition, the formula will be live evaluated in the browser. */ AUTO = "auto", + /** + * A complex type, called an AI column within Budibase. This type is only supported against internal tables + * and calculates the output based on a chosen operation (summarise text, translation etc) which passes to + * the configured Budibase Large Language Model to retrieve the output and write it back into the row. + * AI fields function in a similar fashion to static formulas, and possess many of the same characteristics. + */ + AI = "ai", /** * a JSON type, called JSON within Budibase. This type allows any arbitrary JSON to be input to this column * type, which will be represented as a JSON object in the row. This type depends on a schema being diff --git a/packages/types/src/documents/app/table/constants.ts b/packages/types/src/documents/app/table/constants.ts index fffaddc5df..117cfa0eba 100644 --- a/packages/types/src/documents/app/table/constants.ts +++ b/packages/types/src/documents/app/table/constants.ts @@ -30,6 +30,7 @@ export enum JsonFieldSubType { export enum FormulaType { STATIC = "static", DYNAMIC = "dynamic", + AI = "ai", } export enum BBReferenceFieldSubType { diff --git a/packages/types/src/documents/app/table/schema.ts b/packages/types/src/documents/app/table/schema.ts index b98a0a3d4a..45d0aa78d2 100644 --- a/packages/types/src/documents/app/table/schema.ts +++ b/packages/types/src/documents/app/table/schema.ts @@ -9,6 +9,7 @@ import { JsonFieldSubType, RelationshipType, } from "./constants" +import { AIOperationEnum } from "../../../sdk/ai" export interface UIFieldMetadata { order?: number @@ -116,6 +117,16 @@ export interface FormulaFieldMetadata extends BaseFieldSchema { formulaType?: FormulaType } +export interface AIFieldMetadata extends BaseFieldSchema { + type: FieldType.AI + operation: AIOperationEnum + columns?: string[] + column?: string + categories?: string[] + prompt?: string + language?: string +} + export interface BBReferenceFieldMetadata extends Omit { type: FieldType.BB_REFERENCE @@ -194,6 +205,7 @@ interface OtherFieldMetadata extends BaseFieldSchema { | FieldType.LINK | FieldType.AUTO | FieldType.FORMULA + | FieldType.AI | FieldType.NUMBER | FieldType.LONGFORM | FieldType.BB_REFERENCE @@ -211,6 +223,7 @@ export type FieldSchema = | RelationshipFieldMetadata | AutoColumnFieldMetadata | FormulaFieldMetadata + | AIFieldMetadata | NumberFieldMetadata | LongFormFieldMetadata | StringFieldMetadata diff --git a/packages/types/src/sdk/ai.ts b/packages/types/src/sdk/ai.ts new file mode 100644 index 0000000000..345effa939 --- /dev/null +++ b/packages/types/src/sdk/ai.ts @@ -0,0 +1,91 @@ +export enum AIOperationEnum { + SUMMARISE_TEXT = "SUMMARISE_TEXT", + CLEAN_DATA = "CLEAN_DATA", + TRANSLATE = "TRANSLATE", + CATEGORISE_TEXT = "CATEGORISE_TEXT", + SENTIMENT_ANALYSIS = "SENTIMENT_ANALYSIS", + PROMPT = "PROMPT", + SEARCH_WEB = "SEARCH_WEB", +} + +export enum OperationFieldTypeEnum { + MULTI_COLUMN = "columns", + COLUMN = "column", + BINDABLE_TEXT = "prompt", +} + +export type OperationFieldsType = { + [AIOperationEnum.SUMMARISE_TEXT]: { + columns: OperationFieldTypeEnum.MULTI_COLUMN + } + [AIOperationEnum.CLEAN_DATA]: { + column: OperationFieldTypeEnum.COLUMN + } + [AIOperationEnum.TRANSLATE]: { + column: OperationFieldTypeEnum.COLUMN + language: OperationFieldTypeEnum.BINDABLE_TEXT + } + [AIOperationEnum.CATEGORISE_TEXT]: { + columns: OperationFieldTypeEnum.MULTI_COLUMN + categories: OperationFieldTypeEnum.BINDABLE_TEXT + } + [AIOperationEnum.SENTIMENT_ANALYSIS]: { + column: OperationFieldTypeEnum.COLUMN + } + [AIOperationEnum.PROMPT]: { + prompt: OperationFieldTypeEnum.BINDABLE_TEXT + } + [AIOperationEnum.SEARCH_WEB]: { + columns: OperationFieldTypeEnum.MULTI_COLUMN + } +} + +type BaseSchema = { + operation: AIOperationEnum +} + +type SummariseTextSchema = BaseSchema & { + operation: AIOperationEnum.SUMMARISE_TEXT + columns: string[] +} + +type CleanDataSchema = BaseSchema & { + operation: AIOperationEnum.CLEAN_DATA + column: string +} + +type TranslateSchema = BaseSchema & { + operation: AIOperationEnum.TRANSLATE + column: string + language: string +} + +type CategoriseTextSchema = BaseSchema & { + operation: AIOperationEnum.CATEGORISE_TEXT + columns: string[] + categories: string[] +} + +type SentimentAnalysisSchema = BaseSchema & { + operation: AIOperationEnum.SENTIMENT_ANALYSIS + column: string +} + +type PromptSchema = BaseSchema & { + operation: AIOperationEnum.PROMPT + prompt: string +} + +type SearchWebSchema = BaseSchema & { + operation: AIOperationEnum.SEARCH_WEB + columns: string[] +} + +export type AIColumnSchema = + | SummariseTextSchema + | CleanDataSchema + | TranslateSchema + | CategoriseTextSchema + | SentimentAnalysisSchema + | PromptSchema + | SearchWebSchema diff --git a/packages/types/src/sdk/index.ts b/packages/types/src/sdk/index.ts index d87ec58b0c..86eb5b1a24 100644 --- a/packages/types/src/sdk/index.ts +++ b/packages/types/src/sdk/index.ts @@ -1,3 +1,4 @@ +export * from "./ai" export * from "./automations" export * from "./hosting" export * from "./context" From e7a6e587515d9ca2893cb6796403d707c8e9d08b Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 18 Oct 2024 17:59:26 +0100 Subject: [PATCH 71/89] Updating some further types - added 2 ts-ignores which will be removed on v3 branch. --- packages/server/src/api/routes/tests/row.spec.ts | 1 + packages/server/src/api/routes/tests/viewV2.spec.ts | 8 ++++---- packages/server/src/integrations/googlesheets.ts | 4 +++- packages/server/src/integrations/utils/utils.ts | 1 + packages/server/src/sdk/app/tables/internal/sqs.ts | 1 + packages/server/src/tests/utilities/structures.ts | 1 + 6 files changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 2717a1b7d5..31067ab40f 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -2074,6 +2074,7 @@ describe.each([ ) tableId = table._id! + // @ts-ignore - until AI implemented const rowValues: Record = { [FieldType.STRING]: generator.guid(), [FieldType.LONGFORM]: generator.paragraph(), diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 7979fac555..dfd4f50bd1 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -206,7 +206,7 @@ describe.each([ visible: false, icon: "ic", }, - } as Record, + } as ViewV2Schema, } const createdView = await config.api.viewV2.create(newView) @@ -250,7 +250,7 @@ describe.each([ name: "Category", type: FieldType.STRING, }, - } as Record, + } as ViewV2Schema, } await config.api.viewV2.create(newView, { @@ -1044,7 +1044,7 @@ describe.each([ visible: false, icon: "ic", }, - } as Record, + } as ViewV2Schema, }) expect(updatedView).toEqual({ @@ -1078,7 +1078,7 @@ describe.each([ name: "Category", type: FieldType.STRING, }, - } as Record, + } as ViewV2Schema, }, { status: 200, diff --git a/packages/server/src/integrations/googlesheets.ts b/packages/server/src/integrations/googlesheets.ts index 831528f84d..5f61791683 100644 --- a/packages/server/src/integrations/googlesheets.ts +++ b/packages/server/src/integrations/googlesheets.ts @@ -56,6 +56,7 @@ interface AuthTokenResponse { const isTypeAllowed: Record = { [FieldType.STRING]: true, [FieldType.FORMULA]: true, + [FieldType.AI]: true, [FieldType.NUMBER]: true, [FieldType.LONGFORM]: true, [FieldType.DATETIME]: true, @@ -490,7 +491,8 @@ export class GoogleSheetsIntegration implements DatasourcePlus { } if ( !sheet.headerValues.includes(key) && - column.type !== FieldType.FORMULA + column.type !== FieldType.FORMULA && + column.type !== FieldType.AI ) { updatedHeaderValues.push(key) } diff --git a/packages/server/src/integrations/utils/utils.ts b/packages/server/src/integrations/utils/utils.ts index 43302de36f..73b2f637f8 100644 --- a/packages/server/src/integrations/utils/utils.ts +++ b/packages/server/src/integrations/utils/utils.ts @@ -242,6 +242,7 @@ function copyExistingPropsOver( let shouldKeepSchema = false switch (existingColumnType) { case FieldType.FORMULA: + case FieldType.AI: case FieldType.AUTO: case FieldType.INTERNAL: shouldKeepSchema = true diff --git a/packages/server/src/sdk/app/tables/internal/sqs.ts b/packages/server/src/sdk/app/tables/internal/sqs.ts index bd71644537..7533e2b22a 100644 --- a/packages/server/src/sdk/app/tables/internal/sqs.ts +++ b/packages/server/src/sdk/app/tables/internal/sqs.ts @@ -19,6 +19,7 @@ const FieldTypeMap: Record = { [FieldType.BOOLEAN]: SQLiteType.NUMERIC, [FieldType.DATETIME]: SQLiteType.TEXT, [FieldType.FORMULA]: SQLiteType.TEXT, + [FieldType.AI]: SQLiteType.TEXT, [FieldType.LONGFORM]: SQLiteType.TEXT, [FieldType.NUMBER]: SQLiteType.REAL, [FieldType.STRING]: SQLiteType.TEXT, diff --git a/packages/server/src/tests/utilities/structures.ts b/packages/server/src/tests/utilities/structures.ts index cf6d47053a..ad2ec5bec3 100644 --- a/packages/server/src/tests/utilities/structures.ts +++ b/packages/server/src/tests/utilities/structures.ts @@ -605,6 +605,7 @@ export function fullSchemaWithoutLinks({ }): { [type in Exclude]: FieldSchema & { type: type } } { + // @ts-ignore - until AI implemented return { [FieldType.STRING]: { name: "string", From aaa4e7b8dec03ceb3e1e47e9d25ef21abb1d8acf Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 18 Oct 2024 18:31:06 +0100 Subject: [PATCH 72/89] Pro to master. --- packages/pro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pro b/packages/pro index cad8edfba8..297fdc937e 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit cad8edfba856058cd12dd9e7363b998ddea76034 +Subproject commit 297fdc937e9c650b4964fc1a942b60022b195865 From 8f46119817b0fd6758e9c7b188571655ba9d86cb Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Mon, 21 Oct 2024 10:04:51 +0100 Subject: [PATCH 73/89] refs --- packages/account-portal | 2 +- packages/pro | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/account-portal b/packages/account-portal index 8cd052ce82..fedf9957c1 160000 --- a/packages/account-portal +++ b/packages/account-portal @@ -1 +1 @@ -Subproject commit 8cd052ce8288f343812a514d06c5a9459b3ba1a8 +Subproject commit fedf9957c1acc240b1a3bccb7882d7d763d8f499 diff --git a/packages/pro b/packages/pro index 1a749caba9..297fdc937e 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit 1a749caba9c85aab2645e5d00db479eb53d3f80f +Subproject commit 297fdc937e9c650b4964fc1a942b60022b195865 From f857e2a3e97c917aad958ff4498f2214825c6f4a Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 21 Oct 2024 12:56:59 +0100 Subject: [PATCH 74/89] Adding BUDIBASE_AI feature flag. --- packages/backend-core/src/features/features.ts | 1 + packages/types/src/sdk/featureFlag.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/backend-core/src/features/features.ts b/packages/backend-core/src/features/features.ts index 39b8558d2a..e2f8d9b6a1 100644 --- a/packages/backend-core/src/features/features.ts +++ b/packages/backend-core/src/features/features.ts @@ -273,6 +273,7 @@ export const flags = new FlagSet({ [FeatureFlag.AI_CUSTOM_CONFIGS]: Flag.boolean(env.isDev()), [FeatureFlag.ENRICHED_RELATIONSHIPS]: Flag.boolean(env.isDev()), [FeatureFlag.TABLES_DEFAULT_ADMIN]: Flag.boolean(env.isDev()), + [FeatureFlag.BUDIBASE_AI]: Flag.boolean(env.isDev()), }) type UnwrapPromise = T extends Promise ? U : T diff --git a/packages/types/src/sdk/featureFlag.ts b/packages/types/src/sdk/featureFlag.ts index a1033236d7..99ee0f9996 100644 --- a/packages/types/src/sdk/featureFlag.ts +++ b/packages/types/src/sdk/featureFlag.ts @@ -7,6 +7,7 @@ export enum FeatureFlag { DEFAULT_VALUES = "DEFAULT_VALUES", ENRICHED_RELATIONSHIPS = "ENRICHED_RELATIONSHIPS", TABLES_DEFAULT_ADMIN = "TABLES_DEFAULT_ADMIN", + BUDIBASE_AI = "BUDIBASE_AI", } export interface TenantFeatureFlags { From 01edb34b6e01149e73e5618eec55848524f21f71 Mon Sep 17 00:00:00 2001 From: mikesealey Date: Mon, 21 Oct 2024 15:16:45 +0100 Subject: [PATCH 75/89] deprecates Section component --- .../[componentId]/new/_components/componentStructure.json | 2 +- .../client/src/components/app/{ => deprecated}/Section.svelte | 2 +- packages/client/src/components/app/index.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename packages/client/src/components/app/{ => deprecated}/Section.svelte (97%) diff --git a/packages/builder/src/pages/builder/app/[application]/design/[screenId]/[componentId]/new/_components/componentStructure.json b/packages/builder/src/pages/builder/app/[application]/design/[screenId]/[componentId]/new/_components/componentStructure.json index ff58a66221..84e1bd75b8 100644 --- a/packages/builder/src/pages/builder/app/[application]/design/[screenId]/[componentId]/new/_components/componentStructure.json +++ b/packages/builder/src/pages/builder/app/[application]/design/[screenId]/[componentId]/new/_components/componentStructure.json @@ -14,7 +14,7 @@ { "name": "Layout", "icon": "ClassicGridView", - "children": ["container", "section", "sidepanel", "modal"] + "children": ["container", "sidepanel", "modal"] }, { "name": "Data", diff --git a/packages/client/src/components/app/Section.svelte b/packages/client/src/components/app/deprecated/Section.svelte similarity index 97% rename from packages/client/src/components/app/Section.svelte rename to packages/client/src/components/app/deprecated/Section.svelte index b86c5cc352..245d280dc0 100644 --- a/packages/client/src/components/app/Section.svelte +++ b/packages/client/src/components/app/deprecated/Section.svelte @@ -1,6 +1,6 @@